Fix errors parsing IDs that contain slashes

[ml-evs]

Hopefully this will eventually close #181...

This PR so far just adds tests that should pass and currently don't. When filtering on included data (which is done by default) any "/" in the ID (totally allowed under spec) will throw a lark parser error, e.g.

parser.parse("id=dummy/2019")

E           optimade.filterparser.lark_parser.ParserError: No terminal defined for '/' at line 1 col 9
E
E           id=dummy/2019
E                   ^
E
E           Expecting: {'ESCAPED_STRING', 'SIGNED_INT', 'DOT', 'SIGNED_FLOAT', '_OR', '_AND', 'IDENTIFIER', 'NOT', 'LPAR'}

[CasperWA]

So looking into this issue a bit, I've found that the issue is that we are not enclosing the value in quotes ("value").
But that then amounts to the question of whether id's should be queryable without adding quotes to the value?

Edit: Tried to make this comment long time ago - GitHub finally worked.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@23159c7). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #183   +/-   ##
=========================================
  Coverage          ?   86.62%           
=========================================
  Files             ?       39           
  Lines             ?     1854           
  Branches          ?        0           
=========================================
  Hits              ?     1606           
  Misses            ?      248           
  Partials          ?        0           

Flag	Coverage Δ
#unittests	86.62% <ø> (?)

Impacted Files	Coverage Δ
optimade/models/entries.py	100.00% <0.00%> (ø)
optimade/validator/validator.py	62.33% <0.00%> (ø)
optimade/models/references.py	97.61% <0.00%> (ø)
optimade/filtertransformers/mongo.py	95.29% <0.00%> (ø)
optimade/models/baseinfo.py	89.13% <0.00%> (ø)
optimade/server/mappers/__init__.py	100.00% <0.00%> (ø)
optimade/server/routers/landing.py	66.66% <0.00%> (ø)
optimade/models/jsonapi.py	90.00% <0.00%> (ø)
optimade/server/routers/structures.py	100.00% <0.00%> (ø)
optimade/server/routers/references.py	100.00% <0.00%> (ø)
... and 29 more

---

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 23159c7...5ff98d5. Read the comment docs.

[CasperWA]

So I've "hacked" the grammar to allow for un-quoted id values ... it would be better if just said ids should be in quotes as every other string value, but for ids this maybe doesn't make sense?

[CasperWA]

From Skype conversation with @ml-evs (all from me):

it seems the issue is (simply) that to be recognized as an escaped string, it needs to be in quotations
i.e., id="example/test" will work fine.
This then raises the question if the value for id is special, and it has to be treated as an escaped string even if there are no quotes around it?

I've been testing it further, implementing a separate grammar that adds an ID_ESCAPED_STRING, implementing this to the extent it's needed. However, this also means that it doesn't enclose .............. never mind - just fixed it

[ml-evs]

Thanks for this fix @CasperWA, it seems sensible to me! I'd like to test that this works on my own server later on, as the initial problem I had was an internal server issue where it could not return documents with this ID in included, which lead me to writing the initial tests...

[ml-evs]

This does indeed fix the error I was getting @CasperWA, but I see what you mean about treating ID's on a separate footing. A small change to router.utils.get_included_relationships will have the same effect, so I guess we should decide which fix to make... I'll push the fix here too, and we can decide whether to revert the grammar after.

[CasperWA]

This does indeed fix the error I was getting @CasperWA, but I see what you mean about treating ID's on a separate footing. A small change to router.utils.get_included_relationships will have the same effect, so I guess we should decide which fix to make... I'll push the fix here too, and we can decide whether to revert the grammar after.

The fix in the router is quite straightforward, but for me it doesn't solve the underlying question: Should id values be enclosed in quotation marks or not? If yes, then we fix it in the router, if no, then we need to fix it in the grammar (unfortunately). To me, the grammar fix is quite ugly.

[ml-evs]

I'd rather fix it in the router, so I'll remove the grammar changes.

[CasperWA]

I'd rather fix it in the router, so I'll remove the grammar changes.

Sure. Following the discussion in Materials-Consortia/OPTIMADE#259 we should however be able to not have to enclose the value in quotation marks ...

But this of course shouldn't have to actually result in a grammar change for the filter query parameter input.

Edit: I think I mixed up some issues with this comment - never mind, you do you 😅

[CasperWA]

I have just added tests (they should currently fail) for the include parameter to align with the results of the discussion in Materials-Consortia/OPTIMADE#259

[CasperWA]

I have just added tests (they should currently fail) for the include parameter to align with the results of the discussion in Materials-Consortia/OPTiMaDe#259

Maybe this is for a different PR - I will revert and create a new PR

[ml-evs]

I'm happy with this when you are @CasperWA, we can make the required changes later based on the spec-level decision.