1. Michael Bayer
  2. sqlalchemy

Issues

Issue #2943 new

@validates on inherited subclasses

Michael Bayer
repo owner created an issue
from sqlalchemy import *
from sqlalchemy.orm import *
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()

class A(Base):
    __tablename__ = 'a'

    id = Column(Integer, primary_key=True)
    data = Column(String)

    @validates('data')
    def validate(self, key, value):
        return "yup A " + value


class B(A):
    @validates('data')
    def validate(self, key, value):
        return "yup B " + value

for obj in (A(), B()):
    obj.data = "value"
    print(obj.data)

so here's a patch that makes the second one print "yup A yup B value", e.g. runs both validators:

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 2c18e81..fb8e0aa 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -38,26 +38,21 @@ def _register_attribute(strategy, mapper, useobject,

     attribute_ext = list(util.to_list(prop.extension, default=[]))
-    listen_hooks = []

+    pre_validate_hooks =[]
+    post_validate_hooks = []

     if useobject and prop.single_parent:
-        listen_hooks.append(single_parent_validator)
+        pre_validate_hooks.append(single_parent_validator)

-    if prop.key in prop.parent.validators:
-        fn, opts = prop.parent.validators[prop.key](prop.key)
-        listen_hooks.append(
-            lambda desc, prop: orm_util._validator_events(desc,
-                                prop.key, fn, **opts)
-            )

     if useobject:
-        listen_hooks.append(unitofwork.track_cascade_events)
+        post_validate_hooks.append(unitofwork.track_cascade_events)

     # need to assemble backref listeners
     # after the singleparentvalidator, mapper validator
     backref = kw.pop('backref', None)
     if backref:
-        listen_hooks.append(
+        post_validate_hooks.append(
             lambda desc, prop: attributes.backref_listeners(desc,
                                 backref,
                                 uselist)
@@ -85,7 +80,15 @@ def _register_attribute(strategy, mapper, useobject,
                 **kw
                 )

-            for hook in listen_hooks:
+            for hook in pre_validate_hooks:
+                hook(desc, prop)
+
+            for super_m in m.iterate_to_root():
+                if prop.key in super_m.validators:
+                    fn, opts = super_m.validators[prop.key]
+                    orm_util._validator_events(desc, prop.key, fn, **opts)
+
+            for hook in post_validate_hooks:
                 hook(desc, prop)

 @properties.ColumnProperty.strategy_for(instrument=False, deferred=False)

but is that what we want? or should B's validator replace A's?

if the latter then we just do this:

            for super_m in m.iterate_to_root():
                if prop.key in super_m.validators:
                    fn, opts = super_m.validators[prop.key]
                    orm_util._validator_events(desc, prop.key, fn, **opts)
                    break   # just the one

Comments (18)

  1. Michael Bayer reporter

    as I'm concerned about relying on B@validates doing nothing silently in existing applications, possibly the safest approach now is:

    1. if @validates is placed on B, emit a warning that it does nothing (or even just throw an error, hmmm)

    2. add new argument "override=True" to @validates, so that B->@validates actually runs.

    3. if they want A.@validates to still run, they haev to call super(B, self).validates()

  2. Michael Bayer reporter

    here's that....

    im a little concerned about setup performance here.

    diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
    index 26f105b..7928310 100644
    --- a/lib/sqlalchemy/orm/mapper.py
    +++ b/lib/sqlalchemy/orm/mapper.py
    @@ -2603,6 +2603,7 @@ def validates(*names, **kw):
          argument "is_remove" which will be a boolean.
    
          .. versionadded:: 0.7.7
    +
         :param include_backrefs: defaults to ``True``; if ``False``, the
          validation function will not emit if the originator is an attribute
          event related via a backref.  This can be used for bi-directional
    @@ -2611,6 +2612,15 @@ def validates(*names, **kw):
    
          .. versionadded:: 0.9.0
    
    +    :param override: if ``True``, this validation function can be
    +     specified on a subclass which inherits the mapped attribute,
    +     where it will override any existing validators on the superclass.
    +     If not present, a ``@validates`` decorator on a subclass raises an
    +     error, as it currently takes no effect.
    +
    +     .. versionadded:: 0.9.3
    +
    +
         .. seealso::
    
           :ref:`simple_validators` - usage examples for :func:`.validates`
    @@ -2618,12 +2628,14 @@ def validates(*names, **kw):
         """
         include_removes = kw.pop('include_removes', False)
         include_backrefs = kw.pop('include_backrefs', True)
    +    override = kw.pop('override', False)
    
         def wrap(fn):
             fn.__sa_validators__ = names
             fn.__sa_validation_opts__ = {
                       "include_removes": include_removes,
    -                  "include_backrefs": include_backrefs
    +                  "include_backrefs": include_backrefs,
    +                  "override": override
                     }
             return fn
         return wrap
    diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
    index 2c18e81..95b449d 100644
    --- a/lib/sqlalchemy/orm/strategies.py
    +++ b/lib/sqlalchemy/orm/strategies.py
    @@ -38,26 +38,20 @@ def _register_attribute(strategy, mapper, useobject,
    
         attribute_ext = list(util.to_list(prop.extension, default=[]))
    -    listen_hooks = []
    
    +    pre_validate_hooks = []
    +    post_validate_hooks = []
    
         if useobject and prop.single_parent:
    -        listen_hooks.append(single_parent_validator)
    -
    -    if prop.key in prop.parent.validators:
    -        fn, opts = prop.parent.validators[prop.key]
    -        listen_hooks.append(
    -            lambda desc, prop: orm_util._validator_events(desc,
    -                                prop.key, fn, **opts)
    -            )
    +        pre_validate_hooks.append(single_parent_validator)
    
         if useobject:
    -        listen_hooks.append(unitofwork.track_cascade_events)
    +        post_validate_hooks.append(unitofwork.track_cascade_events)
    
         # need to assemble backref listeners
         # after the singleparentvalidator, mapper validator
         backref = kw.pop('backref', None)
         if backref:
    -        listen_hooks.append(
    +        post_validate_hooks.append(
                 lambda desc, prop: attributes.backref_listeners(desc,
                                     backref,
                                     uselist)
    @@ -85,7 +79,21 @@ def _register_attribute(strategy, mapper, useobject,
                     **kw
                     )
    
    -            for hook in listen_hooks:
    +            for hook in pre_validate_hooks:
    +                hook(desc, prop)
    +
    +            for super_m in m.iterate_to_root():
    +                if prop.key in super_m.validators:
    +                    fn, opts = super_m.validators[prop.key]
    +                    if not opts["override"] and super_m is not mapper:
    +                        raise sa_exc.InvalidRequestError(
    +                                    "validator on inheriting class %s.%s won't "
    +                                    "run by default; please specify override=True" %
    +                                    (super_m.class_.__name__, prop.key))
    +                    orm_util._validator_events(desc, prop.key, fn, **opts)
    +                    break
    +
    +            for hook in post_validate_hooks:
                     hook(desc, prop)
    
     @properties.ColumnProperty.strategy_for(instrument=False, deferred=False)
    diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
    index dd85f2e..3a05b84 100644
    --- a/lib/sqlalchemy/orm/util.py
    +++ b/lib/sqlalchemy/orm/util.py
    @@ -70,7 +70,7 @@ class CascadeOptions(frozenset):
             )
    
    
    -def _validator_events(desc, key, validator, include_removes, include_backrefs):
    +def _validator_events(desc, key, validator, include_removes, include_backrefs, override):
         """Runs a validation method on an attribute value to be set or appended."""
    
         if not include_backrefs:
    
  3. Michael Bayer reporter

    here's a diff for that:

    diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
    index 26f105b..0b2be3a 100644
    --- a/lib/sqlalchemy/orm/mapper.py
    +++ b/lib/sqlalchemy/orm/mapper.py
    @@ -2162,10 +2162,16 @@ class Mapper(_InspectionAttr):
             return bool(m)
    
         def iterate_to_root(self):
    +        return iter(self._parents)
    +
    +    @_memoized_configured_property
    +    def _parents(self):
    +        parents = []
             m = self
             while m:
    -            yield m
    +            parents.append(m)
                 m = m.inherits
    +        return parents
    
         @_memoized_configured_property
         def self_and_descendants(self):
    

    then call mapper._parents whereever iterate_to_root is now.

  4. Michael Bayer reporter

    latest patch, includes override='add' or override='replace', also includes two new exception cases to prevent listeners or @validates from being attached to synonyms as these also don't take effect right now.

  5. Michael Bayer reporter

    this is only for convenience. you can work around it, given the top example:

    class B(A):
        @validates('data')
        def validate(self, key, value):
            value = super(B, self).validate(key, value)
            return "yup B " + value
    

    and you can also just use attribute events, which aren't impacted by this

    @event.listens_for(A.data, "set", propagate=True)
    def validate(obj, key, value):
       # ...
    
  6. Seungha Kim

    Thank you for your answer, sincerely! I tried that before and not worked, but it maybe broke because I added validate method to joined table inheritance class... Is my guess right? If so, what's your plan to support validation in joined table inherited class?

  7. Michael Bayer reporter

    this is an enhancement and also w/ a pretty backwards-incompatible tilt as folks who make sure both validators run directly right now will have them called double w/ this change.

  8. Sam Bourne

    +1 for this (tested on sqlalchemy version 0.9.9)

    I have a scenario where I am inheriting from a model which I do not want to modify (pip installed). I would like to override a @validates method for an attribute. Given your workaround example above, it does not appear to call B.validates method at all. @validates seem to only work on classes where the attribute/column is also defined.

    from sqlalchemy import *
    from sqlalchemy.orm import *
    from sqlalchemy.ext.declarative import declarative_base
    
    Base = declarative_base()
    
    class A(Base):
        __tablename__ = 'a'
    
        id = Column(Integer, primary_key=True)
        data = Column(String)
    
        @validates('data')
        def validate(self, key, value):
            return "yup A " + value
    
    
    class B(A):
    
        foo = Column(String)
    
        @validates('data')
        def validate(self, key, value):
            return "yup B " + value
    
        @validates('foo')
        def validate_too(self, key, value):
            return "foo" + value
    
    
    class C(B):
        @validates('foo')
        def validate_too(self, key, value):
            raise Exception('You cannot get here!')
    
    
    for obj in (A(), B(), C()):
        obj.data = "value"
        obj.foo = "bar"
        print(obj.__class__.__name__, obj.data, obj.foo)
    
    # ('A', 'yup A value', 'bar')
    # ('B', 'yup A value', 'foobar')
    # ('C', 'yup A value', 'foobar')
    

    I feel like this behavior is very confusing. I've hacked a workaround to my problem by modifying the mapper of the class before inheriting from it...

    from sqlalchemy.util import immutabledict
    from sqlalchemy import *
    from sqlalchemy.orm import *
    from sqlalchemy.ext.declarative import declarative_base
    
    Base = declarative_base()
    
    class A(Base):
        __tablename__ = 'a'
    
        id = Column(Integer, primary_key=True)
        data = Column(String)
    
        @validates('data')
        def validate(self, key, value):
            return "yup A " + value
    
    A.__mapper__.validators = immutabledict(
        {k: v for k, v in A.__mapper__.validators.iteritems() if k!= 'data'})
    
    
    class B(A):
        pass
    
    obj = B()
    obj.data = 'value'
    print(obj.data)
    # 'value'
    

    This may be slightly esoteric, but for what it's worth I would prefer this behavior:

    • subclasses can override validate methods provided the same args and method name
    • multiple methods can be used to validate the same attribute
    from sqlalchemy import *
    from sqlalchemy.orm import *
    from sqlalchemy.ext.declarative import declarative_base
    
    Base = declarative_base()
    
    class A(Base):
        __tablename__ = 'a'
    
        id = Column(Integer, primary_key=True)
        data = Column(String)
        foo = Column(String)
    
        @validates('data')
        def validate(self, key, value):
            return "yup A " + value
    
        @validates('foo')
        def validate_foo(self, key, value):
            return "foo" + value
    
    
    class B(A):
    
        @validates('data')
        def validate(self, key, value):
            return "yup B " + value
    
        @validates('foo')
        def validate_foo2(self, key, value):
            return "spangle" + value
    
    
    class C(B):
    
        @validates('foo')
        def validate_foo(self, key, value):
            return "u" + value
    
        @validates('foo')
        def validate_foo2(self, key, value):
            return "f" + value
    
    
    for obj in (A(), B(), C()):
        obj.data = "value"
        obj.foo = "bar"
        print(obj.__class__.__name__, obj.data, obj.foo)
    
    # produces
    # ('A', 'yup A value', 'foobar')
    # ('B', 'yup A value', 'foobar')
    # ('C', 'yup A value', 'foobar')
    
    # *should* produce
    # ('A', 'yup A value', 'foobar')
    # ('B', 'yup B value', 'foospanglebar')
    # ('C', 'yup B value', 'fubar')
    
  9. Log in to comment