#20680 closed defect (fixed)
[Patch] Upload validator doesn't detect overlapping ways on updates
Reported by: | Solarspot | Owned by: | GerdP |
---|---|---|---|
Priority: | normal | Milestone: | 24.01 |
Component: | Core validator | Version: | tested |
Keywords: | template_report upload | Cc: | Gpar |
Description
What steps will reproduce the problem?
- Download data
- Move some existing ways and nodes (e.g. buildings and residential areas) so that they overlap each other
- 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?
Attachments (6)
Change History (17)
by , 3 years ago
Attachment: | Overlapping ways.png added |
---|
by , 3 years ago
Attachment: | Normal validator.png added |
---|
by , 3 years ago
Attachment: | 20680.patch added |
---|
comment:1 by , 3 years ago
by , 3 years ago
Attachment: | 20680-2.patch added |
---|
when validating a selection check also the parent ways of modified nodes
by , 3 years ago
Attachment: | 20680-3.patch added |
---|
when validating a selection check also the parent ways of modified nodes and relations refering to those ways
comment:2 by , 3 years ago
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
comment:3 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Summary: | Upload validator doesn't detect overlapping ways on updates → [Patch] Upload validator doesn't detect overlapping ways on updates |
comment:4 by , 3 years ago
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.
by , 3 years ago
Attachment: | 20680-4.patch added |
---|
comment:5 by , 3 years ago
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.
comment:8 by , 2 years ago
Cc: | added |
---|
comment:10 by , 8 months ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:11 by , 8 months ago
Milestone: | → 24.01 |
---|
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.