Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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 Don-vip)

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)

EndsNearError.osm (2.8 KB ) - added by mdk 5 years ago.
18223.patch (5.8 KB ) - added by GerdP 5 years ago.

Download all attachments as: .zip

Change History (13)

by mdk, 5 years ago

Attachment: EndsNearError.osm added

comment:1 by Don-vip, 5 years ago

Cc: GerdP added
Component: CoreCore validator
Description: modified (diff)
Keywords: ignore added

comment:2 by GerdP, 5 years ago

Milestone: 19.10

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...

comment:3 by mdk, 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 GerdP, 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 GerdP, 5 years ago

Attachment: 18223.patch added

comment:5 by GerdP, 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 GerdP, 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&apos;NEQ&apos;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:7 by GerdP, 5 years ago

Resolution: fixed
Status: newclosed

In 15459/josm:

fix #17837 and #18223, add unit tests

TODO:

  • Will havily blow up size of preferences.xml when user ignores lots of "single elements" instead of the group.
  • Maybe ignore entries in preferences with many (how many?) elements, as those probably were caused by error #18223 and are meaningless

comment:8 by GerdP, 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?

comment:9 by stoecker, 5 years ago

What about on own data structure for this purpose?

in reply to:  9 ; comment:10 by GerdP, 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?

in reply to:  10 comment:11 by stoecker, 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.