Force/propose/automatically download of incomplete multipolygon relations for validation

[Geimas5]

In the latest version of JOSM, there are some validation rules for multipolygon relations, however, these rules are only run when the relation has been completely downloaded. This can result in uploads in which the user unintentionally breaks these multipolygons due to preventable problems such as them not being closed. Even for users who are aware of the need to manually download incomplete multipolygons, which is far from everyone, it can be easy to not notice having touched an incomplete multipolygon in areas with a high multipolygon density.

What I am suggesting is for the implementation of some way to either force the user to download all incomplete multipolygons, or at the least, make the user aware of having touched incomplete multipolygons and providing a method for the user to download all of the incomplete relations they have touched.

[GerdP]

The same problem occurs with other incomplete relations. On the other hand, you should see a popup message when you modify a relation in a way that can corrupt it. So, it is likely that mapper simply ignores the warning.
If you don't see such a popup please let us know how to reproduce the problem.

[Geimas5]

I could not remember having seen any such popup, I have therefore tried a few different scenarios.

Deleting a way which is part of a relation
I do get a message about the way being part of a relation, but not anything about the validations not working because not all members of the relation has been downloaded.

Switching outer/inner
No popup/validation error until after having downloaded all members of the relation.

Adding a random not-connected member to a relation
No popup/validation error until after having downloaded all members of the relation.

Creating a self-intersecting relation by moving a node
I do get a validation error if the self-intersection is only involving one way. But not when I use two ways. The validations are of course working when the the entire relation has been downloaded.

[GerdP]

Replying to Geimas5:

I could not remember having seen any such popup, I have therefore tried a few different scenarios.

Deleting a way which is part of a relation
I do get a message about the way being part of a relation, but not anything about the validations not working because not all members of the relation has been downloaded.

This is more or less always the case. Incomplete data is typically ignored in validation tests.

Switching outer/inner
No popup/validation error until after having downloaded all members of the relation.

I think this is not OK. It should produce a popup.

Adding a random not-connected member to a relation
No popup/validation error until after having downloaded all members of the relation.

Again, this should produce the popup.

Creating a self-intersecting relation by moving a node
I do get a validation error if the self-intersection is only involving one way. But not when I use two ways. The validations are of course working when the the entire relation has been downloaded.

Hmm, that is a rather special case. The current implementation of MultipolygonTest doesn't check complex self-intersections when the relation is incomplete. Simple self-intersections or crossing ways are found in another check. Please attach a sample file.

[Geimas5]

Replying to GerdP:
I have added a Sample for the last scenario.

[GerdP]

OK, I see. It seems I was wrong, the ways are not tested in the CrossingWays test.

[GerdP]

A 1st patch to address these issues.

• Warn if incomplete multipolygon relation (MP) was modified.
• Perform check for crossing ways for all complete ways of the MP

Open issues:

• most of the mentioned issues seem to depend on the way how you work with relations
  1. it seems that changes made in the "Edit Relation" popup are always considered to be wanted, therefore no warnigs are produced when you do something like adding members or changing roles. I think that makes sense. Maybe it would be a good idea to perform a complete relation check when you press OK to close the edit dialog.
  2. If you try to delete way 668064148 which is a member of tow relations you'll see the popup that asks for confirmation.
  3. If you select way 345099033 and right click on relation 9295910 you see an entry "add selection to 1 relation". If you click this you see a warning popup in the lower left corner

• the status modified is not set for the relation in your sample file. Although the geometry of the MP was changed only the moved node has the status modified. This corresponds to the OSM changeset logic: Neither the parent way nor the relation would be uploaded. Same problem occurs when you move the node of a normal way like 345099033 so that the way is self intersecting. This change doesn't set the modifiedstatus for the way and thus it is not verified on upload.

Maybe we should introduce a new status moved. Usage: If a node is moved it would have the moved
flag set. The flag would be used when you run validator on a selection of objects or when validator is executed because you started an upload.
The flag would be used to check all ways that refer to the moved node as well as the relations which refer to those ways.

[GerdP]

@team I am busy with project mkgmap, so I'll work on this again next month. The patch is rather simple, so maybe I should commit it before 19.10 is released?

[GerdP]

In 15510/josm:

see #18240 : apply 18240.patch

• Warn if incomplete multipolygon relation (MP) was modified.
• Perform check for crossing ways for all complete ways of the MP

[Don-vip]

Can you please write more useful commit messages? Reading the changelog I have no idea what the commit is about. You can edit SVN messages after commit on the JOSM SVN repository.

[GerdP]

Done

[Don-vip]

Thanks! :)

[GerdP]

Replying to GerdP:

Maybe we should introduce a new status moved. Usage: If a node is moved it would have the moved
flag set. The flag would be used when you run validator on a selection of objects or when validator is executed because you started an upload.
The flag would be used to check all ways that refer to the moved node as well as the relations which refer to those ways.

Thought again about this. The flag would not help much for the various tests about geometry (crossing ways, overlapping ways or areas, amenity=parking inside amenity=parking etc). Most of those tests require more input to calculate reasonable results and - to me - it seems impossible to predict which objects are needed.
The only solution that I see is to perform a full (and maybe time consuming) validation on upload and filter the results so that only those errors (or warnings) are shown which refer to modified objects or parents of the modified objects.
I am not happy with this idea.
I think the basic idea of the validation tests is to warn when something is wrong. The user should NOT deduce that his change is correct just because JOSM wasn't able to find something wrong. No idea how to communicate that :(