-
Notifications
You must be signed in to change notification settings - Fork 42
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
Enable filtering on relationships with mongo #234
Conversation
32a5eab
to
773e91c
Compare
773e91c
to
0e49c16
Compare
Codecov Report
@@ Coverage Diff @@
## master #234 +/- ##
==========================================
+ Coverage 87.62% 87.77% +0.14%
==========================================
Files 45 45
Lines 1932 1955 +23
==========================================
+ Hits 1693 1716 +23
Misses 239 239
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @ml-evs !
Seems all right to me.
Looking through the spec, I see no issue with this approach, as long as the database is self-consistent. In the sense that relationship links between resources are existent twice (once for each linked resource), i.e., a structure resource linked to a reference is aware of this link, as well is the reference aware of the link to the structure. If this is always upheld, then this approach is fine, even though it differs from the mentioned approach in the spec of a multi-step query.
The only thing I have is whether or not you will put in a debug logger for the transform
method. Either is fine :)
So this doesn't circumvent the need for a multi-step query as it only works on the fields stored under |
03dee04
to
1e1e2fa
Compare
Thinking about of ways that this could be extended to cover that multi-step process... Without much of a logic chance, in post-processing we could return a succession of queries to run, e.g. Probably a separate PR for another day though X) EDIT: this would not be possible unless collections can see/talk to each other, as the first step needs to query e.g. |
I've added an error for queries on e.g. |
49be114
to
8471265
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all right by me, thanks @ml-evs ! :)
Great with the additional error handler as well.
Yeah - then you need to move into SQL territory to benefit the most from it :) Or create mixed-collections upon starting the server? |
For some reason the test client handles NotImplementedError differently to other errors. This commit adds a dedicated NotImplementedError that also imbues it with the correct 501 status code.
8471265
to
16c56ac
Compare
Now #222 is done, this PR adds the ability to filter on relationships, e.g.
/structures?filter=references.id HAS "dijkstra1968"
. The relevant part of the spec is section 6.2.6 (please check I am interpreting it correctly).This is done with another post-processing method that looks for the pattern
<entry_type>.<property>
in a query, and replaces it with a search over therelationships
dictionary.