OSM area processing obeys tag mapping

[vesameskanen]

Summary

OSM parsing and graph building code always applied hard coded bicycle and pedestrian permissions to areas. Now the code uses the configured wayPropertySet and national tag mapping to define the permissions.

Secondly, detailed propertly rule for highway=footway;bicycle=yes;area=yes tag combination overruled simpler rules like plain highway=footway because the former rule often has more matching tags. This is a bit surprising because absence of bicycle=yes tag does not reduce scores. This problem is fixed by using ExactMatchSpecifier.

Thirdly, OSM tag mapping is a typical example of a situation where OOP works very well. Class hierarchy is now refactored to use very simple inheritance with a base class and national sub classes.

Issue

Closes #5354

Unit tests

Area permission tests for Finland and Germany tag mappers added.

Documentation

Updated.

[codecov]

Codecov Report

Attention: Patch coverage is 99.48320% with 2 lines in your changes missing coverage. Please review.

Project coverage is 69.75%. Comparing base (ce9118c) to head (e6d383b).
Report is 133 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
.../graph_builder/module/osm/WalkableAreaBuilder.java	90.90%	1 Missing ⚠️
...ripplanner/osm/tagmapping/ConstantSpeedMapper.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6164      +/-   ##
=============================================
- Coverage      69.95%   69.75%   -0.20%     
+ Complexity     17756    17658      -98     
=============================================
  Files           1999     2006       +7     
  Lines          75500    75571      +71     
  Branches        7728     7734       +6     
=============================================
- Hits           52813    52718      -95     
- Misses         20013    20140     +127     
- Partials        2674     2713      +39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leonardehrenfried]

About your second point, this kind of fuzzy matching has caused me problems before, so I added

OpenTripPlanner/application/src/main/java/org/opentripplanner/osm/wayproperty/specifier/ExactMatchSpecifier.java

Lines 8 to 20 in f4bfedf

	/**
	* This specifier allows you to specify a very precise match. It will only result in a positive when
	* _all_ key/value pairs match exactly.
	* <p>
	* It's useful when you want to have a long, very specific specifier that should only match a very
	* limited number of ways.
	* <p>
	* If you'd use a {@link BestMatchSpecifier} then the likelihood of the long spec matching unwanted
	* ways would be high.
	*
	* @see org.opentripplanner.osm.tagmapping.HoustonMapper
	*/
	public class ExactMatchSpecifier implements OsmSpecifier {

It think it could be helpful here.

There was talk about making it the default but we were scared of the unintended consequences.

[vesameskanen]

I wonder if absence of a tag in BestMatchSpecifier should actually reduce match scores? Less than conflicting value but something at least.

[leonardehrenfried]

I find BestMatchSpecifier quite confusing (for a while it was the only one we had) and would love to see it go away entirely. That said, there are probably improvements we can make to it.

[leonardehrenfried]

And you need to resolve the conflicts.

[optionsome]

This way vs area thing here is slightly confusing, but I think it's a common problem that we use these way properties for areas as well so if we were to rename things, we would need to rename a lot of things.

[optionsome]

It's a bit unfortunate that we now lose all history on this file but since we are combining two files, I think it's impossible to avoid some history being lost.