art1415926535 / graphene-sqlalchemy-filter

Filters for Graphene SQLAlchemy integration
https://pypi.org/project/graphene-sqlalchemy-filter/
MIT License
118 stars 34 forks source link

Support for `hybrid_property` fields? #30

Closed AlexEshoo closed 4 years ago

AlexEshoo commented 4 years ago

I tried using the FilterSet Class to create a filter on a field defined by a hybrid_property in my sqlalchemy model, but it seems to have no effect. Graphene-Sqlalchemy does resolve the field on the type, but the filters argument does not accept the hybrid_property field I defined. I tried both the shortcut [...] as well as explicit filter types but neither worked. Is this supported?

Here's an example (overly simplified for clarity):

models.py

class Post(db.Model):
    __tablename__ = "projects"

    id = sa.Column(sa.Integer, primary_key=True, autoincrement=True)

    @hybrid_property
    def status(self):
        return "Submitted"

filters.py

class PostFilter(FilterSet):
    class Meta:
        model = Post
        fields = {
            'status': ["eq", "in"]
        }

class CustomFilter(FilterableConnectionField):
    filters = {
        Post: PostFilter(),
    }

schema.py

class Post(SQLAlchemyObjectType):
    class Meta:
        model = PostModel
        interfaces = (Node,)
        connection_field_factory = CustomFilter.factory
        connection_class = CountableConnection

Running this query:

query {
  allPosts (filters: {status: "Submitted"}) {
    edges {
      node {
        status
      }
    }
  }
}

Returns:

{
  "errors": [
    {
      "message": "Argument \"filters\" has invalid value {status: \"Submitted\"}.\nIn field \"status\": Unknown field.",
      "locations": [
        {
          "line": 2,
          "column": 25
        }
      ]
    }
  ]
}
art1415926535 commented 4 years ago

Hybrid property is not supported at this time. I'll try to add support.

art1415926535 commented 4 years ago

Done! New release.

Example: models.py, filters.py

[...] is not supported because we cannot predict the type of the hybrid property.

AlexEshoo commented 4 years ago

Awesome! I will test this out!

AlexEshoo commented 4 years ago

I tried this out on the latest version but I'm getting this error when including the hybrid property field status in the filters arugment block:

Neither 'hybrid_property' object nor 'ExprComparator' object associated with Post.status has an attribute 'type'

art1415926535 commented 4 years ago

Are you doing the correct sqlalchemy hybrid_property? Can you provide an example for getting this error?

AlexEshoo commented 4 years ago

Below is full running example that generates the error:

from sqlalchemy import Column, ForeignKey, Integer, String, create_engine
from sqlalchemy.orm import relationship, scoped_session, sessionmaker
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.hybrid import hybrid_property
import graphene
from graphene import Connection, Node
from graphene_sqlalchemy import SQLAlchemyObjectType
from graphene_sqlalchemy_filter import FilterableConnectionField, FilterSet

Base = declarative_base()

class PostModel(Base):
    __tablename__ = "posts"

    id = Column(Integer, primary_key=True, autoincrement=True)
    user_id = Column(Integer, ForeignKey("users.id"))
    name = Column(String(255))
    user = relationship("UserModel", back_populates="posts")

    @hybrid_property
    def status(self):
        return "SUBMITTED"

class UserModel(Base):
    __tablename__ = "users"

    id = Column(Integer, primary_key=True, autoincrement=True)
    username = Column(String(255))
    posts = relationship("PostModel", back_populates="user")

class PostFilter(FilterSet):
    class Meta:
        model = PostModel
        fields = {
            "name": [...],
            "status": ["eq"]
        }

class MyFilterableConnectionField(FilterableConnectionField):
    filters = {PostModel: PostFilter()}

class UserNode(SQLAlchemyObjectType):
    class Meta:
        model = UserModel
        interfaces = (Node,)
        connection_field_factory = MyFilterableConnectionField.factory

class UserConnection(Connection):
    class Meta:
        node = UserNode

class PostNode(SQLAlchemyObjectType):
    class Meta:
        model = PostModel
        interfaces = (Node,)
        connection_field_factory = MyFilterableConnectionField.factory

class PostConnection(Connection):
    class Meta:
        node = PostNode

class Query(graphene.ObjectType):
    user = graphene.relay.Node.Field(UserNode)
    all_users = MyFilterableConnectionField(UserConnection)

    post = graphene.relay.Node.Field(PostNode)
    all_posts = MyFilterableConnectionField(PostConnection)

schema = graphene.Schema(query=Query)

engine = create_engine('sqlite://', echo=True)  # in-memory
db_session = scoped_session(sessionmaker(bind=engine))
Base.query = db_session.query_property()

Base.metadata.create_all(bind=engine)
db_session.commit()

result = schema.execute('query {allPosts (filters: {status: "SUBMITTED"}) { edges { node { id } } } }')

print(result)

Full Traceback:

An error occurred while resolving field Query.allPosts
Traceback (most recent call last):
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 374, in __getattr__
    return getattr(descriptor, attribute)
AttributeError: 'hybrid_property' object has no attribute 'type'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 394, in __getattr__
    return getattr(comparator, attribute)
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\ext\hybrid.py", line 1139, in __getattr__
    return getattr(self.expression, key)
AttributeError: 'str' object has no attribute 'type'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "C:\repo\Workspace\venv\lib\site-packages\graphql\execution\executor.py", line 452, in resolve_or_error
    return executor.execute(resolve_fn, source, info, **args)
  File "C:\repo\Workspace\venv\lib\site-packages\graphql\execution\executors\sync.py", line 16, in execute
    return fn(*args, **kwargs)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy\fields.py", line 78, in connection_resolver
    return on_resolve(resolved)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy\fields.py", line 51, in resolve_connection
    resolved = cls.get_query(model, info, **args)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\connection_field.py", line 88, in get_query
    query = filter_set.filter(info, query, request_filters)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\filters.py", line 752, in filter
    query, sqla_filters = cls._translate_many_filter(info, query, filters)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\filters.py", line 878, in _translate_many_filter
    query, r = cls._translate_filter(info, query, k, v)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\filters.py", line 837, in _translate_filter
    model_field_type = model_field.type
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 407, in __getattr__
    replace_context=err3,
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\util\compat.py", line 182, in raise_
    raise exception
AttributeError: Neither 'hybrid_property' object nor 'ExprComparator' object associated with PostModel.status has an attribute 'type'
Traceback (most recent call last):
  File "C:\repo\Workspace\venv\lib\site-packages\graphql\execution\executor.py", line 452, in resolve_or_error
    return executor.execute(resolve_fn, source, info, **args)
  File "C:\repo\Workspace\venv\lib\site-packages\graphql\execution\executors\sync.py", line 16, in execute
    return fn(*args, **kwargs)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy\fields.py", line 78, in connection_resolver
    return on_resolve(resolved)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy\fields.py", line 51, in resolve_connection
    resolved = cls.get_query(model, info, **args)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\connection_field.py", line 88, in get_query
    query = filter_set.filter(info, query, request_filters)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\filters.py", line 752, in filter
    query, sqla_filters = cls._translate_many_filter(info, query, filters)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\filters.py", line 878, in _translate_many_filter
    query, r = cls._translate_filter(info, query, k, v)
  File "C:\repo\Workspace\venv\lib\site-packages\graphene_sqlalchemy_filter\filters.py", line 837, in _translate_filter
    model_field_type = model_field.type
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\orm\attributes.py", line 407, in __getattr__
    replace_context=err3,
  File "C:\repo\Workspace\venv\lib\site-packages\sqlalchemy\util\compat.py", line 182, in raise_
    raise exception
graphql.error.located_error.GraphQLLocatedError: Neither 'hybrid_property' object nor 'ExprComparator' object associated with PostModel.status has an attribute 'type'
art1415926535 commented 4 years ago

Can you test new release?

AlexEshoo commented 4 years ago

I tested the new release with the code I posted above and it works. I added some logic to the hybrid_property attribute, however, and encountered some strange results. It seems as if when I add conditionals to hybrid_property based on the values of other columns it is now treating the boolean value of the referenced column as true always. To demonstrate I modified the above code with:

@hybrid_property
def status(self):
    if self.name:
        return "NAMED"

    return "UNNAMED"

And added a user and post to the database...

user = UserModel(username="aUser")
db_session.add(user)
db_session.add(PostModel(user_id=user.id))
db_session.commit()

But when querying

result = schema.execute('query {allPosts (filters: {status: "NAMED"}) { edges { node { id name status } } } }')

I get this result:

{"data": {"allPosts": {"edges": [{"node": {"id": "UG9zdE5vZGU6MQ==", "name": null, "status": "UNNAMED"}}]}}}

Clearly the column's comparison operation is evaluated properly by the hybrid_property on query (hence why it shows "UNNAMED" in the json), but it is not evaluated correctly when filtered.

art1415926535 commented 4 years ago

Your hybrid_property has a logic error.

try update your code like this

@hybrid_property
def status(self):
    print(self.name, type(self.name), bool(self.name))

    if self.name:
        return "NAMED"

    return "UNNAMED"

upon execution you get Model.name <class 'sqlalchemy.orm.attributes.InstrumentedAttribute'> True, so hybrid property returns "NAMED" every time.


To solve this problem, I recommend making a simple filter.

AlexEshoo commented 4 years ago

Shouldn't an InstrumentedAttribute's boolean value should be the boolean value of the column's value? That's how I interpreted the functionality given that the property actually returns the correct value in the query resolver.

I can look into simple filter as a different implementation strategy.

art1415926535 commented 4 years ago

Filtering is performed for insertion into a database query. The hybrid property value is calculated after the database query is executed.

Also maybe this will help you solve the problem.

AlexEshoo commented 4 years ago

That makes more sense. Thanks for pointing out that case statement, I just tried it and it works perfectly, so I think it's safe to close now. Thanks for all your help!