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

Django style filtering for GetEntitySetRequest #113

Closed

Conversation

bartonip
Copy link
Contributor

@bartonip bartonip commented Jun 28, 2020

The existing filtering mechanism is not very intuitive, requiring interpolation of OData queries or using the convoluted boolean logic syntax of GetEntitySetFilter to achieve complex queries.

I have implemented a Django-like filtering system that allows filtering of EntitySets in a much more intuitive way.

Syntax

For example, a simple equivalency query was previously written like this:

request = service.entity_sets.Employees.get_entities().filter("FirstName eq 'Tim' and LastName eq 'Smith'")

or even more confusingly:

request = service.entity_sets.Employees.get_entities().filter(sef.and_(
                                                             request.FirstName == 'Tim',
                                                             request.LastName == 'Smith'))

With this proposed update, future calls will look like this.

request = service.entity_sets.Employees.get_entities().filter(FirstName="Tim", LastName="Smith")

Tell me that's not way more beautiful. ;)

Filter Expressions

Next, I have implemented some lookups using Django-like syntax that opens up some more advanced query scenarios.
The operators lt, lte, gt, gte, in, contains, startswith, endswith, length and range have been implemented.

Examples below:

request = service.entity_sets.Employees.get_entities().filter(ID__lte=20)

request = service.entity_sets.Employees.get_entities().filter(ID__gt=20)

request = service.entity_sets.Employees.get_entities().filter(ID__in=[20, 21, 22])

request = service.entity_sets.Employees.get_entities().filter(FirstName__contains="Tim")

request = service.entity_sets.Employees.get_entities().filter(FirstName__length=3)

There is also the introduction of the FilterExpression class which is equivalent to Django's Q class.

This allows common queries to be stored as objects:

from pyodata.v2.service import FilterExpression as Q
company = Q(CompanyID=1234)

request = service.entity_sets.Employees.get_entities().filter(company, FirstName="Tim", LastName__startswith="S")

This also allows the or operator to be used to or statements:

request = service.entity_sets.Employees.get_entities().filter(Q(FirstName="Tim") | Q(FirstName="John"), LastName="Smith))

All changes are backwards compatible and do not disrupt existing implementations of this library, and upgrading to this syntax should be simple if not trivial.

@filak-sap
Copy link
Contributor

I am not sure what a linter you use but neither pylint nor flake8 are configured to enforce use of double quotes.

In general, please avoid code style changes - my ideology: what was once accepted to master should stay formatted as it is unless there are other reason for touching it (code style changes makes reviews almost impossible).

@bartonip bartonip force-pushed the feature/django-style-filtration branch from d7a08a3 to f5000cc Compare June 28, 2020 23:07
@bartonip
Copy link
Contributor Author

Apologies, VSCode was pointed to the wrong env and had our internal linting rules applied.

This has been rectified.

Copy link
Contributor

@filak-sap filak-sap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to test your branch on my machine without the style fixes commit which you reverted few moments ago. I got a merge conflict in cherry-pick to the current upstream master. I wanted to fix the merge conflict but I ended up fixing all the changes. This is my review:
filak-sap@548c12e

(it contains your commit ff2288a )

@filak-sap
Copy link
Contributor

Would you mind if I push the version after applying the patch filak-sap@548c12e ?

@filak-sap
Copy link
Contributor

You can see the version after my changes at: master...filak-sap:django_style_filter_review

@bartonip
Copy link
Contributor Author

bartonip commented Jun 28, 2020

Looks good to me!

@filak-sap I'm currently writing a feature that lets us access the ETag header in an entity to be used in time based If-Match queries and fixing a bug where huge responses are causing a parsing error in ODataHttpResponse. After I'm done with these could we please look at making a release?

@filak-sap filak-sap closed this in df1fc72 Jun 29, 2020
filak-sap pushed a commit that referenced this pull request Jun 29, 2020
Closes #113

Signed-off-by: Jakub Filak <jakub.filak@sap.com>
IvanShirokikh pushed a commit to RYDLAB/python-pyodata that referenced this pull request Oct 3, 2020
Closes SAP#113

Signed-off-by: Jakub Filak <jakub.filak@sap.com>
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

Successfully merging this pull request may close these issues.

2 participants