Simplify handling of tag filter parameters #653
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tag filters are now handled as a list of simple filters instead of being grouped by filter kind. That greatly simplifies the code when parsing the incoming request and when building the query. The ES query for excluded tags has changed to from (not t1 and not t2 and not t3...) to not (t1 or t2 or t3). This is equivalent and more simple to handle.
Also adds some additional test cases for filter combinations.
Also fixes some minor parsing issues found with the new tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of grouping tag filter parameters (from the
osm_tagquery parameter) by the type of filter, the new code simply collects all parameters into a list of simple TagFilter objects. They are still sorted by include/exclude filter when building the query but otherwise just strung together in a big or filter. This results in a massive simplification of the code.The outcome is the same except for the rather curious syntax of
osm_tag=key:!value. Stringing two of these together with the same key (as inosm_tag=highway:!motorway&osm_tag=highway:!trunk) would give you all objects with the given keys except the ones with any of the given values (i.e all highways except motorways and trunks). Now the two queries are or'ed together independently, so you end up getting all objects with the given key (i.e. highways that are not motorways or highways that are not trunks). Given that thekey:!valuesyntax was never documented or tested, I don't really see an issue with the change in semantics. The alternative would be to drop that interpretation completely and stick with the documented ones.The PR also includes some simplifications around the tests for the PhotonRequestFactory, now making heavy use of parametrized tests (thank you, JUnit5) and fixes a few smaller issue around parameter parsing found while completing the tests.