Modify

Opened 3 days ago

Last modified 44 hours ago

#19956 assigned defect

[RFC] [Patch] Double check if error still exists before executing autofix

Reported by: GerdP Owned by: GerdP
Priority: normal Milestone: 20.10
Component: Core validator Version:
Keywords: template_report Cc:

Description

What steps will reproduce the problem?

  1. load attached file with duplicate relations
  2. run validator, should produce error "Duplicated relations (1)"
  3. double click on the error entry so that both relations are selected
  4. select one of the two relations
  5. change name from "same" to "other"
  6. click on Fix button without running validator again

What is the expected result?

No further change, error was fixed manually

What happens instead?

One relation is deleted

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

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2020-10-03 13:42:38 +0200 (Sat, 03 Oct 2020)
Build-Date:2020-10-04 01:30:47
Revision:17084
Relative:URL: ^/trunk

Identification: JOSM/1.5 (17084 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 2004 (19041)
Memory Usage: 1144 MB / 3641 MB (661 MB allocated, but free)
Java version: 1.8.0_221-b11, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080 (scaling 1.0x1.0)
Maximum Screen Size: 1920x1080
Best cursor sizes: 16x16 -> 32x32, 32x32 -> 32x32
VM arguments: [-XX:StartFlightRecording=name=MyRecording2,settings=d:\dbg\gerd.jfc, -XX:FlightRecorderOptions=defaultrecording=true,dumponexit=true,dumponexitpath=e:\ld\perf_20201017_154712.jfr]
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (35579)
+ PolygonCutOut (v0.7)
+ apache-commons (35524)
+ buildings_tools (35579)
+ continuosDownload (91)
+ ejml (35313)
+ geotools (35169)
+ jaxb (35092)
+ jts (35122)
+ merge-overlap (35583)
+ o5m (35248)
+ opendata (35513)
+ pbf (35446)
+ poly (35248)
+ reverter (35579)
+ undelete (35521)
+ utilsplugin2 (35580)

Attachments (3)

dup-rels.osm (551 bytes) - added by GerdP 3 days ago.
19956.patch (7.3 KB) - added by GerdP 44 hours ago.
19956.2.patch (3.7 KB) - added by GerdP 44 hours ago.
implement double check in class TestError, should work for all java tests, but cannot double check fixable mapcss errors

Download all attachments as: .zip

Change History (7)

Changed 3 days ago by GerdP

Attachment: dup-rels.osm added

comment:1 Changed 3 days ago by GerdP

Milestone: 20.10
Owner: changed from team to GerdP
Status: newassigned
Summary: Duplicate Relation fix removes already fixed relatiosDuplicate Relation fix removes already fixed relations

comment:2 Changed 3 days ago by GerdP

Similar problem with other tests like DuplicateWay. Both don't re-evaluate the problem with the primitives in their current state.

Changed 44 hours ago by GerdP

Attachment: 19956.patch added

comment:3 Changed 44 hours ago by GerdP

Patch contains a few minor changes, this is the important part:

        // see #19956 check again
        Test test2 = new DuplicateRelation();
        test2.startTest(NullProgressMonitor.INSTANCE);
        toFix.forEach(test2::visit);
        test2.endTest();
        boolean errorStillExists = false;
        for (TestError e : test2.getErrors()) {
            if (e.getCode() == testError.getCode() && e.getPrimitives().size() >= 2) {
                // refresh primitives, might be fewer than before
                toFix = e.primitives(Relation.class).collect(Collectors.toSet());
                errorStillExists = true;
                break;
            }
        }
        if (!errorStillExists)
            return null;

I don't like the idea to implement such a double check in each test that offers an autofix (11 java tests in core) and I think the problem also exists in the mapcss tests.

I'll try again to code this in double check in ValidatorDialog.

Changed 44 hours ago by GerdP

Attachment: 19956.2.patch added

implement double check in class TestError, should work for all java tests, but cannot double check fixable mapcss errors

comment:4 Changed 44 hours ago by GerdP

Summary: Duplicate Relation fix removes already fixed relations[RFC] [Patch] Double check if error still exists before executing autofix

Modify Ticket

Change Properties
Set your email in Preferences
Action
as assigned The owner will remain GerdP.
as The resolution will be set.
to The owner will be changed from GerdP to the specified user.
The owner will change to GerdP
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.