search

pallets-eco / flask-admin

Simple and extensible administrative interface framework for Flask
https://flask-admin.readthedocs.io
BSD 3-Clause "New" or "Revised" License
5.8k stars 1.58k forks source link

Many-to-Many relationships broken on SQLAlchemy>1.3.5 #2099

Closed ix5 closed 3 years ago

ix5 commented 3 years ago

Take the following model:

entry_tags_table = db.Table(
    "entry_tags",
    db.Model.metadata,
    db.Column("entry_id", db.Integer, db.ForeignKey("entry.id")),
    db.Column("tag_id", db.Integer, db.ForeignKey("tag.id")),
)
class Entry(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    title = db.Column(db.String)
    tags = db.relationship("Tag", secondary=entry_tags_table)

class Tag(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.Unicode, nullable=False)
    entries = db.relationship("Entry", secondary=entry_tags_table)

[...]
class DefaultView(sqla.ModelView):
    column_searchable_list = [
        "title",
        "tags.name",
    ]

admin = admin.Admin(app, name='Test', template_mode='bootstrap4', index_view=DefaultView(Entry, db.session))

This will throw an error on SQLALchemy 1.4.x

/home/me/.virtualenvs/vp/lib/python3.9/site-packages/sqlalchemy/orm/relationships.py:3435: SAWarning:
relationship 'Tag.entries' will copy column tag.id to column entry_tags.tag_id,
which conflicts with relationship(s): 'Entry.tags' (copies tag.id to entry_tags.tag_id).
If this is not the intention, consider if these relationships should be linked with back_populates,
or if viewonly=True should be applied to one or more if they are read-only.
For the less common case that foreign key constraints are partially overlapping,
the orm.foreign() annotation can be used to isolate the columns that should be written towards.
The 'overlaps' parameter may be used to remove this warning.

Adding back_populates or backrefs as suggested by the error message and as documented in Basic Relationship Patterns like this:

[...]
    entries = db.relationship(Entry, secondary=entry_tags_table, back_populates="tags")
[...]
    tags = db.relationship(Tag, secondary=entry_tags_table, back_populates="entries")

results in a new error:

Traceback (most recent call last):
  File "/home/me/data/sixfive/main.py", line 24, in <module>
    from admin import admin
  File "/home/me/data/sixfive/admin/__init__.py", line 1, in <module>
    from .view import admin
  File "/home/me/data/sixfive/admin/view.py", line 727, in <module>
    index_view=DefaultView(Entry, db.session, url="/admin"),
  File "/home/me/data/sixfive/admin/view.py", line 45, in __init__
    super(DefaultView, self).__init__(model, session, *args, **kwargs)
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/view.py", line 327, in __init__
    super(ModelView, self).__init__(model, name, category, endpoint, url, static_folder,
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/model/base.py", line 818, in __init__
    self._refresh_cache()
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/model/base.py", line 913, in _refresh_cache
    self._search_supported = self.init_search()
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/view.py", line 581, in init_search
    if tools.is_hybrid_property(self.model, name):
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/tools.py", line 215, in is_hybrid_property
    return last_name in get_hybrid_properties(last_model)
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/flask_admin/contrib/sqla/tools.py", line 195, in get_hybrid_properties
    for key, prop in inspect(model).all_orm_descriptors.items()
  File "/home/me/.virtualenvs/vp/lib/python3.9/site-packages/sqlalchemy/inspection.py", line 71, in inspect
    raise exc.NoInspectionAvailable(
sqlalchemy.exc.NoInspectionAvailable: No inspection system is available for object of type <class 'str'>
make: *** [Makefile:22: runserver] Error 1

The call to inspect() fails at is_hybrid_property() for the attr_name tags.name

Link to gist for reproducing: app.py

Library versions:

Flask-Admin      1.5.7
Flask-SQLAlchemy 2.5.1
SQLAlchemy       1.4.2

Possibly related:

mrjoes commented 3 years ago

There are few things that are not working with SQLAlchemy 1.4:

  1. sqlalchemy-utils is not compatible:

    File "/home/travis/build/flask-admin/flask-admin/.tox/py37-WTForms2/lib/python3.7/site-packages/sqlalchemy_utils/functions/orm.py", line 14, in from sqlalchemy.orm.query import _ColumnEntity

    More here: https://github.com/kvesteri/sqlalchemy-utils/issues/505

  2. I'm checking what's going with the is_hybrid_property. There was a suggested change from the SQLAlchemy author - will see if it helps.

mrjoes commented 3 years ago

I tried to reproduce the M2M issue with the latest SQLAlchemy 1.4.2 and it works as expected with and without back_populates.

Here's a GIST: https://gist.github.com/mrjoes/30cf8cd7865484ade357e966ad82439d

sqlalchemy-utils is still a problem, our tests are failing.

ix5 commented 3 years ago

Sorry, reproduction was missing a crucial bit: column_searchable_list is where it fails to resolve. See gist: app.py

mrjoes commented 3 years ago

Does not reproduce even if I add column_searchable_list. Your gist does not start due to ValueError: urls must start with a leading slash

image

Just in case - this is latest master branch.

ix5 commented 3 years ago

I did not run master branch, but rather this constellation:

Flask-Admin      1.5.7
Flask-SQLAlchemy 2.5.1
SQLAlchemy       1.4.2

(flask-admin 1.5.7 is the latest in pypi)

If the master branch indeed fixes the issue, a new release should be made.

alexa-infra commented 3 years ago

is_hybrid_property has been fixed recently in #2098

ix5 commented 3 years ago

Guess this fixes it then.

ersanjay5991 commented 2 years ago

If this is not the intention, consider if these relationships should be linked with back_populates, flask sqlalchemy

I change to model relation

users = db.relationship('User', secondary=association_table,back_populates="events")