AssociationProxy is not properly initialized when created by a declared_attr of a mixin class

[sqlalchemy-bot]

Migrated issue, originally created by Eric Atkin (Omni)

After upgrading from 1.1.14 to 1.2.2, my models broke (later testing shows the error in 1.1.15 as well). A simplified representation is included below that demonstrates the issue. If the Parent.children association proxy is not accessed before any subclasses are defined, it's owning_class attribute will be None.

from sqlalchemy import Column, ForeignKey, Integer, Text
from sqlalchemy.ext.associationproxy import association_proxy
from sqlalchemy.ext.declarative import declared_attr, declarative_base
from sqlalchemy.orm import backref, relationship
from sqlalchemy.orm.collections import attribute_mapped_collection

Declarative_Base = declarative_base()


class Mixin:
    @declared_attr
    def children(cls):
        class Child(Declarative_Base):
            parent_id = Column(Integer, ForeignKey(cls.id), primary_key=True)
            key = Column(Text, primary_key=True)
            value = Column(Text, nullable=False)
            
            parent = relationship(cls, backref=backref(
                '_children',
                cascade='all, delete-orphan',
                collection_class=attribute_mapped_collection('key')
            ))
            
            __tablename__ = f'{cls.__tablename__}_child'
        
        return association_proxy(
            '_children',
            'value',
            creator=lambda k, v: Child(key=k, value=v),
        )


class Parent(Mixin, Declarative_Base):
    id = Column(Integer, primary_key=True)
    
    __tablename__ = 'parent'


#Parent.children  # uncomment this for workaround


class SubParent(Parent):
    id = Column(Integer, ForeignKey(Parent.id), primary_key=True)
    
    __tablename__ = 'subparent'


p = Parent()
p.children

This seems to be caused by the fix for issue #4116 in commit f14a58d

I don't know how to fix this, but am able to work around it for now by accessing the proxy attribute prior to the subclass definitions. I hope that by demonstrating it, you would be able to design a change that addresses the original issue and mitigates this side-effect.

Thank you,
Eric

[sqlalchemy-bot]

Changes by Eric Atkin (Omni):

• edited description

[sqlalchemy-bot]

Changes by Eric Atkin (Omni):

• edited description

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

that commit was cherry-picked to 1.1. but no issue in 1.1?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

also:

1. test case does not produce any error for me.

2. why do you need to access Parent.children.owning_class explicitly?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

oh, the return value is what's not right. OK, test cases shoudl really give me a stack trace, that's much more helpful

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

OK here is the answer to #1 and #2:

p1 = Parent()
print(p1.children)

now i understand the error

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

and it occurs in 1.1 as well. because the code is the same. mysteries continue

[sqlalchemy-bot]

Eric Atkin (Omni) wrote:

Updated test case to actually trigger an exception

[sqlalchemy-bot]

Changes by Eric Atkin (Omni):

• edited description

[sqlalchemy-bot]

Eric Atkin (Omni) wrote:

Something is wrong with bitbucket. I've been trying to respond to each of your questions, but when I submit a comment, I just get an infinite spinner. I can edit the issue description apparently. So re: 1.1, I upgraded from 1.1.14 where I didn't have the issue. I assume you cherry-picked it into 1.1.15 where we both see the problem in addition to 1.2.

[sqlalchemy-bot]

Changes by Eric Atkin (Omni):

• edited description

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

great that makes sense thanks

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

your situation was a little more unlikely than you might assume but nonetheless this was pretty broken, in https://gerrit.sqlalchemy.org/#/q/I611b590df2babe077ce6c19bea89e84251d1a7f4 i hopefully have the last time I have to keep fixing this

[sqlalchemy-bot]

Eric Atkin (Omni) wrote:

I'm sure my model design was very unlikely, but a perfect storm for this
issue.
Thanks for the quick fix.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Search through mapper superclass hierarchy for owner

Fixed regression caused by fix for issue 🎫4116 affecting versions
1.2.2 as well as 1.1.15, which had the effect of mis-calculation of the
"owning class" of an :class:.AssociationProxy as the NoneType class
in some declarative mixin/inheritance situations as well as if the
association proxy were accessed off of an un-mapped class. The "figure out
the owner" logic has been replaced by an in-depth routine that searches
through the complete mapper hierarchy assigned to the class or subclass to
determine the correct (we hope) match; will not assign the owner if no
match is found. An exception is now raised if the proxy is used
against an un-mapped instance.

Change-Id: I611b590df2babe077ce6c19bea89e84251d1a7f4
Fixes: #4185

→ 650b9ed

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Search through mapper superclass hierarchy for owner

Fixed regression caused by fix for issue 🎫4116 affecting versions
1.2.2 as well as 1.1.15, which had the effect of mis-calculation of the
"owning class" of an :class:.AssociationProxy as the NoneType class
in some declarative mixin/inheritance situations as well as if the
association proxy were accessed off of an un-mapped class. The "figure out
the owner" logic has been replaced by an in-depth routine that searches
through the complete mapper hierarchy assigned to the class or subclass to
determine the correct (we hope) match; will not assign the owner if no
match is found. An exception is now raised if the proxy is used
against an un-mapped instance.

Change-Id: I611b590df2babe077ce6c19bea89e84251d1a7f4
Fixes: #4185
(cherry picked from commit 650b9ed)

→ cc6cef5

[sqlalchemy-bot]

Robbe Block (@robbeblock) wrote:

Hey guys,

Unfortunately, after upgrading to 1.2.3 from 1.2.2, our models now break and I think it might be related to this fix. I've added the relevant part of the stack trace below, but basically it boils down to SQLAlchemy being unable to detect the owner (which lead me to think it has to do something with these changes)

  File "/Users/robbe/Workspace/Platform/Backend/venv/lib/python3.6/site-packages/sqlalchemy/ext/associationproxy.py", line 278, in __get__
    self._calc_owner(obj, class_)
  File "/Users/robbe/Workspace/Platform/Backend/venv/lib/python3.6/site-packages/sqlalchemy/ext/associationproxy.py", line 274, in _calc_owner
    self.owning_class = owner.class_
AttributeError: 'NoneType' object has no attribute 'class_'

I don't have a fully distilled example of how to reproduce it for now, but in broad strokes it comes down to having 2 models, File and Resource and 2 relationships between these two models. We created the double relationship by creating intermediate models: ExternalFiles and InternalFiles. The models are declared in order and the relationships are defined on ExternalFiles and InternalFiles (so that they can refer to the already existing models instead of using a string reference to their name). However, the association proxies are defined on Resource as

class File(Base):
    # Declaration of properties of File model
    ...

class Resource(Base):
    # Declaration of Resource's properties
    ...

    internal_files = association_proxy('resourceinsternalfiles', 'file')
    external_files = association_proxy('resourceexternalfiles', 'file')


class FileBasedResourceInternalFile(Base):
    # Declaration of relationships through some extra abstraction on top of SQLAlchemy
    ...

Could it have to do with the fact that I'm declaring the proxy before the actual relationship is declared?

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

not sure but I really rewrote that stuff, so not surprised it broke yet again

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to reopened

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

@robbeblock I really need the complete stack trace to know how you are accessing this association proxy (class level, instance level, mappers configured yet or not, etc)

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

@robbeblock I can imagine that your mappings use mixins or something like that? I can reproduce your error but not quite with the segment of mappings you're showing.

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

OK, the condition you describe is revised in https://gerrit.sqlalchemy.org/#/q/I87d0ac09f695dc285bd4bbe0a547f1d5ce23e068 which will go in and be released very soon, it likely fixes your issue as well though I'd still like to see specifically how you are getting that error.

[sqlalchemy-bot]

Robbe Block (@robbeblock) wrote:

I'm afraid the stack trace on its own won't do you much good, but I added it below anyway.

#!

/Users/robbe/Workspace/Platform/Backend/venv/bin/python /Applications/PyCharm.app/Contents/helpers/pydev/pydev_run_in_console.py 63719 63720 /Users/robbe/Library/Preferences/PyCharm2017.3/scratches/scratch.py
Running /Users/robbe/Library/Preferences/PyCharm2017.3/scratches/scratch.py
import sys; print('Python %s on %s' % (sys.version, sys.platform))
sys.path.extend(['/Users/robbe/Workspace/Platform/Backend', '/Users/robbe/Workspace/Platform/Python-Client', '/Users/robbe/Workspace/Platform/Utils', '/Users/robbe/Workspace/Platform/CloudStacks', '/Users/robbe/Library/Preferences/PyCharm2017.3/scratches'])
Traceback (most recent call last):
  File "/Applications/PyCharm.app/Contents/helpers/pydev/pydev_run_in_console.py", line 53, in run_file
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/Applications/PyCharm.app/Contents/helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/Users/robbe/Library/Preferences/PyCharm2017.3/scratches/scratch.py", line 6, in <module>
    print(x.external_files)
  File "/Users/robbe/Workspace/Platform/Backend/venv/lib/python3.6/site-packages/sqlalchemy/ext/associationproxy.py", line 278, in __get__
    self._calc_owner(obj, class_)
  File "/Users/robbe/Workspace/Platform/Backend/venv/lib/python3.6/site-packages/sqlalchemy/ext/associationproxy.py", line 274, in _calc_owner
    self.owning_class = owner.class_
AttributeError: 'NoneType' object has no attribute 'class_'
PyDev console: starting.
Python 3.6.3 (default, Nov 18 2017, 01:12:02) 
[GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.38)] on darwin

The error pops up as soon as I try to print the proxied value (as in print(resource.external_files).

I indeed use mixins in my mappings abstracted away by some home-grown decorators (which seemed like a good idea at the time, but now it seems that it complicates things too much especially when trying to extract the pure SQLAlchemy models). If it is not resolved by your proposed fix, I'll try my best to squeeze out some time to provide you with a decent example. In the mean time, what I do is declare all my models with their 'normal' properties (as in: all non-relationships properties) in regular Python classes, then add in relationships between the models using the custom made decorators and later then mix in the resulting model with flask-sqlalchemy's db.Model - looking at SQLAlchemy's code, this boils down to a class being fed to declarative_base() - to make the final version known to SQLAlchemy. (It seems that I might have oversimplified the mental picture I had of my models to describe the problem in my previous post :-))

Anyway, already many thanks to the speedy response!

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

so already that tells me a lot, e.g. you have the instance, not the class, that you are accessing the attribute from.

next, this "external_files" association proxy -where it is present in your source code, is the immediate parent class a mixin and not a mapped class? that is:

class SomeMixin(object):
    external_files = association_proxy(...)

class MyMappedClass(SomeMixin, Base):
   # ...

that would be the other side of it.

can you try the patch at https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/673/ (you can download https://gerrit.sqlalchemy.org/changes/673/revisions/93881f7873048403b62cc3e179354712ba8e9282/patch?zip as one option) . then we'll know this fixes it for you. i can maybe release today or tomorrow.

[sqlalchemy-bot]

Robbe Block (@robbeblock) wrote:

Yes, the immediate parent class of the association proxy is a mixin and not mapped at the moment of declaration. I'll get back to you as soon as I tried the patch.

[sqlalchemy-bot]

Robbe Block (@robbeblock) wrote:

I just checked the patch and is seems to work. Nice job!

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Default to using current mapped class as owner if none found

Repaired regression caused in 1.2.3 and 1.1.16 regarding association proxy
objects, revising the approach to 🎫4185 when calculating the
"owning class" of an association proxy to default to choosing the current
class if the proxy object is not directly associated with a mapped class,
such as a mixin.

Change-Id: I87d0ac09f695dc285bd4bbe0a547f1d5ce23e068
Fixes: #4185

→ 66a246b

[sqlalchemy-bot]

Michael Bayer (@zzzeek) wrote:

Default to using current mapped class as owner if none found

Repaired regression caused in 1.2.3 and 1.1.16 regarding association proxy
objects, revising the approach to 🎫4185 when calculating the
"owning class" of an association proxy to default to choosing the current
class if the proxy object is not directly associated with a mapped class,
such as a mixin.

Change-Id: I87d0ac09f695dc285bd4bbe0a547f1d5ce23e068
Fixes: #4185
(cherry picked from commit 93881f7873048403b62cc3e179354712ba8e9282)

→ b37f007

[sqlalchemy-bot]

Changes by Michael Bayer (@zzzeek):

• changed status to closed