Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding relationships attributes filtering #83

Open
shipperizer opened this issue May 27, 2016 · 9 comments
Open

Adding relationships attributes filtering #83

shipperizer opened this issue May 27, 2016 · 9 comments

Comments

@shipperizer
Copy link
Contributor

Would it be useful to anyone having a filtering on relationships' attributes?

I have a chunk of code (not really well tested but seems to be working) that allow me to query on a relationship attribute django-style ('A.relationship__attribute')

just overriding the SQLAlchemyManager can work:

def _init_filter(self, filter_class, name, field, attribute):
        if field.attribute and '__' in field.attribute:
            rel_model, rel_attr = field.attribute.split('__')
            model = db.inspect(self.model).relationships[rel_model].mapper.class_
            column = getattr(model, rel_attr)
        else:
            column = getattr(self.model, field.attribute or attribute)

it would be good to know if there are better ways to do that and if it could be useful to someone else as well

@lyschoening
Copy link
Contributor

lyschoening commented May 27, 2016

It would be an interesting addition as an optional filter.

The filtering right now is inspired by the Mongo model (property={"$lte": value} not the Django model (property__lte=value). They shouldn't be mixed and matched. You could make a full filter implentation for the Django model and that would be a very nice addition, but also a lot of work.

Mongo's subdocument querying doesn't translate well to Potion. Instead you should make a custom filter, call it AttrsFilter. The filter would create a schema for properties of the resource it is connected to through the ToOne/ToMany field. You would do a query like user={"$attrs": {"first_name": "Jane"}}, equivalent to user__first_name="Jane" in your approach.

@shipperizer
Copy link
Contributor Author

Not really sure how to implement the second option, the AttrsFilter given that I would recursively apply the standard filters on the related model fields.
Are you saying I should lookup the Resource by Resource.model == relationship.mapper.class_ ?

@lyschoening
Copy link
Contributor

lyschoening commented May 27, 2016

Something along the lines of:

class AttrFilter(BaseFilter):
    def _get_relationship_fields(self):
        relationship = self.field.target
        relationship_fields = relationship.schema.fields
        # NOTE we do not want to use fields that are themselves relationship fields.
        return {key: field for key, field in relationship_fields.items():
                    if not isinstance(field, (fields.ToOne, fields.ToMany))}

    def _field_filters_schema(self, filters):
        if len(filters) == 1:
            return next(iter(filters.values())).request
        else:
            return {"anyOf": [filter.request for filter in filters.values()]}

    def _schema(self):
        filters = filters_for_fields(self._get_relationship_fields(), self.field.target.meta.filter)
        return {
            "type": "object",
            "properties": {
                name: _field_filters_schema(field_filters)
                for name, field_filters in filters.items()
            }
        }

    def op(self, a, b):
        pass # TODO

I will be too busy at least the next month to implement any more of this myself. However, if you can submit a PR with some tests I will be happy to review and merge it.

@shipperizer
Copy link
Contributor Author

Thanks for the hint, I'll see what I can get

@kolanos
Copy link
Contributor

kolanos commented May 27, 2016

The now defunct flask-peewee project had something like this as well:
http://docs.peewee-orm.com/projects/flask-peewee/en/latest/rest-api.html#filtering-records-and-querying

Peewee and MongoEngine support for such a filter would be nice as well. Or at the very lease, a clear path to adding support (an extensible implementation).

@shipperizer
Copy link
Contributor Author

shipperizer commented Jun 4, 2016

@lyschoening so I'm trying to follow your hint but here https://github.com/shipperizer/potion/blob/relationship_attr_querying/flask_potion/filters.py#L243
I get this as result:

{'properties': {'$attr': {'properties': {'name': {'anyOf': [<werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>]}, 'alpha_id': {'anyOf': [<werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>, <werkzeug.utils.cached_property object at 0x7f4c295a82e8>]}}, 'type': 'object'}}, 'additionalProperties': False, 'required': ['$attr'], 'type': 'object'}

that fails the json schema validation.
Unfortunately I am not really confident with all the code base so it's taking me a lot to figure out what's wrong with this(the filter is a class, not initialized, that's the main problem it seems, that's why I can't call the schema method), do you have any clue?

@lyschoening
Copy link
Contributor

@shipperizer I forgot the step where the filter class is initialized in my quick example earlier. The code should be written so that it generates filters for all fields on the target resource except those that would result in loops. This is a similar implementation as on the manager:

https://github.com/shipperizer/potion/blob/relationship_attr_querying/flask_potion/manager.py#L50

Coming to think of it, instead of initializing new filters from scratch, you could just access the target field manager's .filters property and simply copy the filters from there. {key: filter for key, filter in relationship.manager.filters.items(): if not isinstance(filter, AttrFilter)} might do the trick.

@shipperizer
Copy link
Contributor Author

Looks like I got it working for the base case (1 relationship, one field), https://github.com/shipperizer/potion/commits/relationship_attr_querying
took me quite a while figuring out a lot of the machinery.
I'll start with the tests asap, I was wondering though how I can get the no name EqualFilter to work though, there is something I can't catch and that makes it not filtering

@shipperizer
Copy link
Contributor Author

Having some issues after starting to test, looks like the field.target is a cached property as well, so for some reason when the field.ToMany or toOne have the resource that is not an object but the string name get an error as the API is not bound to it. I tried to enforce it with the target_reference.resolve method but i still have issues as the resource gets initialised later in the code. Any idea on how to fix it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants