#18223 closed defect (fixed)
[Patch] Problem with ignoring validator rules when more than one object is involved
Reported by: | mdk | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | 19.10 |
Component: | Core validator | Version: | latest |
Keywords: | ignore | Cc: | GerdP |
Description (last modified by )
To reproduce load attached file, but be carefull not to upload the data, because they are real nodes and ways (because Ignoring dosn't work with new objects).
After validation you get two "Way end node near other way" warnings. When you ignore the first, you get in preferences an ignorelist like:
<tag key='1351:n_6871910559:w_733713588' value='Way end node near other way'/>
First strange observation: instead of one line in the "Validator ignore List Management", I get two, which I could separatly treat with "don't ignore". The result looks like:
<tag key='1351:w_733713588' value='Way end node near other way'/>
Here I see a problem: Removing a single objects from the key for a single warning dosn't make sense for me. In my oppinion only removing the whole (instance) warning make sense.
When I ignore both warnings, It's confusing, that they are all show together in the Validator ignore List Management". In the preferences I get the same picture:
<tag key='1351:n_2449148994:w_236955234:n_6871910559:w_733713588' value='Way end node near other way'/>
But now this rule dosn't match the problems, so the validator shows again two warnings.
Instead I would expect two rules (one for each warning) like;
<tag key='1351:n_2449148994:w_236955234' value='Way end node near other way'/> <tag key='1351:n_6871910559:w_733713588' value='Way end node near other way'/>
After manually editing the prefenrences that way, the ignoring of validator messages works as expected. But the "Validator ignore List Management" still shows all 4 object in one list below "1351" instead two entries.
When I quit JOSM, the preferences still contain the two tags.
But when I add a third ignore rule, I get again only one key (which dosn't work):
<tag key='1351:n_2449148994:w_236955234:n_6871910559:w_724754869:n_6871910559:w_733713588' value='Way end node near other way'/>
where node n_6871910559 is included twice in one key. Instead I would expect
<tag key='1351:n_2449148994:w_236955234' value='Way end node near other way'/> <tag key='1351:n_6871910559:w_724754869' value='Way end node near other way'/> <tag key='1351:n_6871910559:w_733713588' value='Way end node near other way'/>
The same happens with crossing waterway/highway as Arthtoach notes in #17837.
Attachments (2)
Change History (13)
by , 5 years ago
Attachment: | EndsNearError.osm added |
---|
comment:1 by , 5 years ago
Cc: | added |
---|---|
Component: | Core → Core validator |
Description: | modified (diff) |
Keywords: | ignore added |
comment:2 by , 5 years ago
Milestone: | → 19.10 |
---|
comment:3 by , 5 years ago
As far as I understand, the ":" separates entries in a generic way.
Perhaps we could generate a different key like
<tag key='1351+n_2449148994+w_236955234' value='Way end node near other way'/>
(separate with a different character to generate a "simple" key) instead of
<tag key='1351:n_2449148994:w_236955234' value='Way end node near other way'/>
where the entries (n_2449148994 and w_236955234) are sorted in a predictible way.
comment:4 by , 5 years ago
The problem is in the routines which build the tree in the dialog and rebuild the error list from that tree. Seems neither Taylor nor I tried with tests that check combinations of OSM elements :(
by , 5 years ago
Attachment: | 18223.patch added |
---|
comment:5 by , 5 years ago
Summary: | Problem with ignoring validator rules when more than one object is involved → [Patch] Problem with ignoring validator rules when more than one object is involved |
---|
I think the patch fixes the problem. We have to handle a lot of special cases, esp. with the migration of old file ignorederrors, so I plan to add some unit tests before committing this.
The patch doesn't depend on the one for #17837.
comment:6 by , 5 years ago
Hmm, IIGTR the code to combine the element ids into a single list was intended to reduce the size of the preferences.xml and thus the time needed to load or maintain it. Probably a good idea, but method hasIgnoredError()
needs to be changed to make that work.
Think of a user pressing "ignore" for an entry like "Unnamed unclassified highway (1192)" and chosing button "single entries" which is preselected.
A single entry for this error looks like this in preferences.xml:
key='3000_way[highway=unclassified][!name][noname'NEQ'yes]:w_102141835' value='Unnamed unclassified highway'/>
With 1192 repeated lines for such an error we increase the size of the file a lot more than with the combined version.
I see 14 KB against 154 KB with the patch :(
So, I try again to find a better solution.
comment:8 by , 5 years ago
I see two possible solutions for the size problem, but they are rather complex:
- We might change the stored structure in the preferences.xml so that the element ids are stored in a subtree. This would be incompatible with older release of JOSM
- We could change the code to create a single line like
<tag key='1351[:n_2449148994:w_236955234][:n_6871910559:w_724754869][:n_6871910559:w_733713588]' value='Way end node near other way'/>
to store three groups of elements for the same error key instead of
<tag key='1351:n_2449148994:w_236955234' value='Way end node near other way'/> <tag key='1351:n_6871910559:w_724754869' value='Way end node near other way'/> <tag key='1351:n_6871910559:w_733713588' value='Way end node near other way'/>
Or maybe we should implement a limit so that one cannot ignore more than 20 individual element groups for a given error key?
follow-up: 11 comment:10 by , 5 years ago
Replying to stoecker:
What about on own data structure for this purpose?
I thought about something like
<map> <tag key='key' value='1351'/> <tag key='description' value='Way end node near other way'/> <list> <entry value=':n_2449148994:w_236955234'/> <entry value=':n_6871910559:w_724754869'/> <entry value=':n_6871910559:w_733713588'/> </list> </map>
The list part should be optional for the case that the group is ignored.
Would that be supported by the routines which read/write preferences.xml?
How would we handle the situation that a user swiches between different versions of JOSM, one using the old format, one using the newer?
comment:11 by , 5 years ago
Replying to GerdP:
Replying to stoecker:
What about on own data structure for this purpose?
I thought about something like
<map> <tag key='key' value='1351'/> <tag key='description' value='Way end node near other way'/> <list> <entry value=':n_2449148994:w_236955234'/> <entry value=':n_6871910559:w_724754869'/> <entry value=':n_6871910559:w_733713588'/> </list> </map>The list part should be optional for the case that the group is ignored.
Would that be supported by the routines which read/write preferences.xml?
Vincent should know. I never used the structure serialization myself yet. When I last touched this we had key/value only :-)
How would we handle the situation that a user swiches between different versions of JOSM, one using the old format, one using the newer?
Convert old data to new format: There is a conversion stage in preferences loading for such cases. New and old format should use different names/keys, so no conflict happens when we after some time (e.g. half a year) drop the update mechanism. In this case old data simply lingers around.
Seems the code simply doesn't handle the case that an error only has a meaning for a given list of OSM objects.
There is a lot of code that tries to combine or extract the element ids for a given error. I am not yet sure if this code should be enhanced or simply removed...