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

Improve handling of MongoDB ObjectID #557

Merged
merged 6 commits into from
Oct 27, 2020
Merged

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Oct 19, 2020

This PR moves the BSON ObjectID handling code out of the general mapper and into the mongo-specific entry collection. It also adds a post-processing method to the MongoTransformer to handle queries on the underlying Mongo _id (e.g. if you aliased immutable_id to _id) by converting them to the corresponding {"$oid": {"$eq": "123456"} query, disallowing other string filters. This means it can now be aliased in the same way as any other field.

  • The test server now uses _id as the underlying field for the immutable_id OPTIMADE field.
  • MongoDB projection is now done in a dict format to allow for explicit exclusion of _id if it has not been aliased (by default, our existing list-based projection always includes _id, which is why we had to delete it every time in the mapper).

@ml-evs ml-evs added priority/low Issue or PR with a consensus of low priority server Issues pertaining to the example server implementation transformer/MongoDB Related to filter transformer for MongoDB labels Oct 19, 2020
@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #557 into master will increase coverage by 0.03%.
The diff coverage is 96.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #557      +/-   ##
==========================================
+ Coverage   91.48%   91.51%   +0.03%     
==========================================
  Files          62       62              
  Lines        3112     3135      +23     
==========================================
+ Hits         2847     2869      +22     
- Misses        265      266       +1     
Flag Coverage Δ
#project 91.51% <96.55%> (+0.03%) ⬆️
#validator 63.41% <58.62%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
optimade/server/mappers/entries.py 98.30% <ø> (-0.06%) ⬇️
optimade/server/entry_collections/mongo.py 95.31% <50.00%> (-1.47%) ⬇️
optimade/filtertransformers/mongo.py 97.36% <100.00%> (+0.21%) ⬆️
...made/server/entry_collections/entry_collections.py 98.85% <100.00%> (+0.02%) ⬆️
optimade/server/exceptions.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e73dc81...ada46fb. Read the comment docs.

@ml-evs ml-evs force-pushed the ml-evs/mongo_id_postprocessing branch from 53cff39 to 4c3205c Compare October 19, 2020 10:54
@ml-evs ml-evs force-pushed the ml-evs/mongo_id_postprocessing branch from d9e182f to c80a5ac Compare October 19, 2020 11:03
@ml-evs ml-evs requested review from CasperWA and shyamd October 19, 2020 22:03
shyamd
shyamd previously approved these changes Oct 26, 2020
@ml-evs
Copy link
Member Author

ml-evs commented Oct 26, 2020

Made a couple of tweaks here too, in 068c3b3, which adds a mixin for HTTPExceptions that defines __str__() to be the same as __repr__ for easier use with pytest.raises. I don't think there will be any other side effects from this but I'll leave it until tomorrow to merge this one...

@shyamd shyamd self-requested a review October 26, 2020 22:11
@ml-evs ml-evs merged commit fbbad72 into master Oct 27, 2020
@ml-evs ml-evs deleted the ml-evs/mongo_id_postprocessing branch October 27, 2020 11:53
@ml-evs
Copy link
Member Author

ml-evs commented Oct 29, 2020

Hmm, either this PR or one of the more recent ones should have triggered a test failure during validation, as our test server responds with BadRequest (rather than some nasty pymongo error) when requesting e.g. immutable_id STARTS WITH 'blah'. I'm going to have to make another PR that changes the BadRequest to NotImplementedError so that it can pass our validator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/low Issue or PR with a consensus of low priority server Issues pertaining to the example server implementation transformer/MongoDB Related to filter transformer for MongoDB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants