[Patch] Upload validator doesn't detect overlapping ways on updates

[Solarspot]

What steps will reproduce the problem?

1. Download data
2. Move some existing ways and nodes (e.g. buildings and residential areas) so that they overlap each other
3. Initiate upload

What is the expected result?

Upload validator shuld detect overlapping ways and report warnings

What happens instead?

Overlapping ways are not detected.

Please provide any additional information below. Attach a screenshot if possible.

See screenshots.

Issues are detected during upload if the ways are newly created but not, if existing ways are moved.

The normal validator is working as expected and finds all overlapping ways.

#20378 is similar

I understand that this is a known limitation of the upload validator, that it only validates changed elements. In this case it validates only moved nodes but not the associated ways (buildings and residential areas).

So maybe we can detect nodes that were moved and additionally validate all the ways that these nodes are part of?

[GerdP]

The patch is a partial solution. It will find new or modified ways which cross others. It doesn't yet fix the case that only nodes are moved.

[GerdP]

when validating a selection check also the parent ways of modified nodes

[GerdP]

when validating a selection check also the parent ways of modified nodes and relations refering to those ways

[GerdP]

TODO:

• add relations to the collection when way members were modified
• add code to show a progress bar when validation for upload takes too long (or just always), as these changes can trigger a lot more complex tests
• maybe add an option to disable this new (slower) behaviour

[GerdP]

Hm, I fear the approach with the AggregatePrimitivesVisitor doesn't work well. It is likely to show many problems which already existed in the original data.

[GerdP]

Similar changes are required in other tests which care about the geometry of ways. Do we already have a ticket for this?
These ones are obvious candidates:

• LongSegment (already seems to work when node is moved)
• MapCssTagChecker (but only for tests like those in geometry.mapcss)
• MultipolygonTest (doesn't work if existing way node is moved so that the multipolygon geometry is invalid)
• OverlappingWays (modified nodes are no problem, but a new way sharing segments with an old way)
• RightAngleBuildingTest (doesn't work if existing building node is moved)
• SelfIntersectingWay (probably no problem, moved nodes cannot trigger this error)
• SharpAngles (doesn't work if existing way node is moved to build a sharp angle)
• UnconnectedNodes (doesn't work if existing node of a highway is moved closer to an unconnected node)

So, it would be good to have common code for them to avoid code duplication.

[skyper]

Found #11348 for crossing ways.

[taylor.smock]

Ticket #22219 has been marked as a duplicate of this ticket.

[GerdP]

see #23397, more or less a duplicate of this ticket.

[GerdP]

I hope this was fixed with the commits for #23397, esp. r18960 and r18961

Please open new ticket for remaining issues.