1. Mike Bayer Avatar Mike Bayer
  2. Untitled project
  3. sqlalchemy_old
  4. Issues

@validates on inherited subclasses

Issue #2943 new
Mike 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. Mike 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. Mike 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. Mike 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.

  9. Mike 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.

  10. Mike Bayer reporter

    This patch can possibly go into 0.9 but it needs a lot of well-written tests. So this will stay on 1.0 unless we can get dev resources for 0.9.

  14. Mike 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):
   # ...
  15. 김승하

    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?

  17. Mike 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.

  18. 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')
  19. Log in to comment
Assignee
Type
enhancement
Priority
major
Status
new
Component
orm
Milestone
1.3
Votes
1
Watchers
3
Jira: the preferred issue tracker for Bitbucket. Join the team!