komoot / photon

an open source geocoder for openstreetmap data
Apache License 2.0
1.83k stars 278 forks source link

Simplify handling of tag filter parameters #653

Closed lonvia closed 2 years ago

lonvia commented 2 years ago

Instead of grouping tag filter parameters (from the osm_tag query 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 in osm_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 the key:!value syntax 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.