Ticket #21423: 21423.2.patch

src/org/openstreetmap/josm/data/validation/OsmValidator.java

@@ -246 +246 @@
 
     private static void removeLegacyEntries(boolean force) {
         // see #19053:
+        boolean wasChanged = removeLegacyEntry(force, true, "3000");
+        // see #18230 (pt_assistant, RightAngleBuildingTest)
+        wasChanged |= removeLegacyEntry(force, false, "3701");
+
+        if (wasChanged) {
+            saveIgnoredErrors();
+        }
+    }
+
+    private static boolean removeLegacyEntry(boolean force, boolean keep, String prefix) {
         boolean wasChanged = false;
         if (force) {
             Iterator<Entry<String, String>> iter = ignoredErrors.entrySet().iterator();
             while (iter.hasNext()) {
                 Entry<String, String> entry = iter.next();
-                if (entry.getKey().startsWith("3000_")) {
+                if (entry.getKey().startsWith(prefix + "_")) {
                     Logging.warn(tr("Cannot handle ignore list entry {0}", entry));
                     iter.remove();
                     wasChanged = true;
                 }
             }
         }
-        String legacyEntry = ignoredErrors.remove("3000");
-        if (legacyEntry != null) {
+        String legacyEntry = ignoredErrors.remove(prefix);
+        if (keep && legacyEntry != null) {
             if (!legacyEntry.isEmpty()) {
-                addIgnoredError("3000_" + legacyEntry, legacyEntry);
+                addIgnoredError(prefix + "_" + legacyEntry, legacyEntry);
             }
             wasChanged = true;
         }
-        if (wasChanged) {
-            saveIgnoredErrors();
-        }
+        return wasChanged;
     }
 
     /**
@@ -502 +510 @@
         List<Map<String, String>> list = new ArrayList<>();
         cleanupIgnoredErrors();
         ignoredErrors.remove("3000"); // see #19053
+        ignoredErrors.remove("3701"); // see #18230
         list.add(ignoredErrors);
         int i = 0;
         while (i < list.size()) {

src/org/openstreetmap/josm/data/validation/TestError.java

@@ -4 +4 @@
 import java.awt.geom.Area;
 import java.awt.geom.PathIterator;
 import java.text.MessageFormat;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.TreeSet;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -33 +35 @@
  * @since 3669
  */
 public class TestError implements Comparable<TestError> {
+    /**
+     * Used to switch users over to new ignore system, UNIQUE_CODE_MESSAGE_STATE
+     * 1_704_067_200L -> 2024-01-01
+     * We can probably remove this and the supporting code in 2025.
+     */
+    private static final boolean SWITCH_OVER = Instant.now().isAfter(Instant.ofEpochMilli(1_704_067_200L));
     /** is this error on the ignore list */
     private boolean ignored;
     /** Severity */
@@ -50 +58 @@
     private final Test tester;
     /** Internal code used by testers to classify errors */
     private final int code;
+    /** Internal code used by testers to classify errors. Used for moving between JOSM versions. */
+    private final int uniqueCode;
     /** If this error is selected */
     private boolean selected;
     /** Supplying a command to fix the error */
@@ -63 +73 @@
         private final Test tester;
         private final Severity severity;
         private final int code;
+        private final int uniqueCode;
         private String message;
         private String description;
         private String descriptionEn;
@@ -74 +85 @@
             this.tester = tester;
             this.severity = severity;
             this.code = code;
+            this.uniqueCode = this.tester.getClass().getName().hashCode();
         }
 
         /**
@@ -254 +266 @@
         this.primitives = builder.primitives;
         this.highlighted = builder.highlighted;
         this.code = builder.code;
+        this.uniqueCode = builder.uniqueCode;
         this.fixingCommand = builder.fixingCommand;
     }
 
@@ -306 +319 @@
      * @return the ignore state for this error or null if any primitive is new
      */
     public String getIgnoreState() {
+        return getIgnoreState(false);
+    }
+    private String getIgnoreState(boolean useOriginal) {
         Collection<String> strings = new TreeSet<>();
         for (OsmPrimitive o : primitives) {
             // ignore data not yet uploaded
@@ -321 +337 @@
             }
             strings.add(type + '_' + o.getId());
         }
-        return strings.stream().map(o -> ':' + o).collect(Collectors.joining("", getIgnoreSubGroup(), ""));
+        return strings.stream().map(o -> ':' + o).collect(Collectors.joining("", getIgnoreSubGroup(useOriginal), ""));
     }
 
     /**
@@ -335 +351 @@
     }
 
     private boolean calcIgnored() {
+        // Begin code removal section (backwards compatibility)
+        if (OsmValidator.hasIgnoredError(getIgnoreGroup(true))) {
+            updateIgnoreList(getIgnoreGroup(true), getIgnoreGroup(false));
+            return true;
+        }
+        if (OsmValidator.hasIgnoredError(getIgnoreSubGroup(true))) {
+            updateIgnoreList(getIgnoreSubGroup(true), getIgnoreSubGroup(false));
+            return true;
+        }
+        String oldState = getIgnoreState(true);
+        String state = getIgnoreState(false);
+        if (oldState != null && OsmValidator.hasIgnoredError(oldState)) {
+            updateIgnoreList(oldState, state);
+            return true;
+        }
+        // End code removal section
         if (OsmValidator.hasIgnoredError(getIgnoreGroup()))
             return true;
         if (OsmValidator.hasIgnoredError(getIgnoreSubGroup()))
             return true;
-        String state = getIgnoreState();
         return state != null && OsmValidator.hasIgnoredError(state);
     }
 
+    /**
+     * Convert old keys to new keys. Only takes effect when {@link #SWITCH_OVER} is true
+     * @param oldKey The key to replace
+     * @param newKey The new key
+     */
+    private static void updateIgnoreList(String oldKey, String newKey) {
+        if (SWITCH_OVER) {
+            Map<String, String> errors = OsmValidator.getIgnoredErrors();
+            if (errors.containsKey(oldKey)) {
+                String value = errors.remove(oldKey);
+                errors.put(newKey, value);
+            }
+        }
+    }
+
     /**
      * Gets the ignores subgroup that is more specialized than {@link #getIgnoreGroup()}
      * @return The ignore sub group
      */
     public String getIgnoreSubGroup() {
+        return getIgnoreSubGroup(false);
+    }
+
+    private String getIgnoreSubGroup(boolean useOriginal) {
         if (code == 3000) {
             // see #19053
             return "3000_" + (description == null ? message : description);
         }
-        String ignorestring = getIgnoreGroup();
+        String ignorestring = getIgnoreGroup(useOriginal);
         if (descriptionEn != null) {
             ignorestring += '_' + descriptionEn;
         }
@@ -365 +415 @@
      * @see TestError#getIgnoreSubGroup()
      */
     public String getIgnoreGroup() {
+        return getIgnoreGroup(false);
+    }
+
+    private String getIgnoreGroup(boolean useOriginal) {
         if (code == 3000) {
             // see #19053
             return "3000_" + getMessage();
         }
-        return Integer.toString(code);
+        if (useOriginal) {
+            return Integer.toString(this.code);
+        }
+        return this.uniqueCode + "_" + this.code;
     }
 
     /**
@@ -404 +461 @@
         return code;
     }
 
+    /**
+     * Get the unique code for this test. Used for ignore lists.
+     * @return The unique code (generated with {@code tester.getClass().getName().hashCode() + code}).
+     * @since xxx
+     */
+    public int getUniqueCode() {
+        return this.uniqueCode;
+    }
+
     /**
      * Returns true if the error can be fixed automatically
      *
@@ -546 +612 @@
      * @return true if two errors are similar
      */
     public boolean isSimilar(TestError other) {
-        return getCode() == other.getCode()
+        return getUniqueCode() == other.getUniqueCode()
+                && getCode() == other.getCode()
                 && getMessage().equals(other.getMessage())
                 && getPrimitives().size() == other.getPrimitives().size()
                 && getPrimitives().containsAll(other.getPrimitives())
@@ -570 +637 @@
 
     @Override
     public String toString() {
-        return "TestError [tester=" + tester + ", code=" + code + ", message=" + message + ']';
+        return "TestError [tester=" + tester + ", unique code=" + this.uniqueCode +
+                ", code=" + code + ", message=" + message + ']';
     }
 
 }

src/org/openstreetmap/josm/io/GeoJSONMapRouletteWriter.java

@@ -45 +45 @@
         final JsonObjectBuilder propertiesBuilder = Json.createObjectBuilder();
         propertiesBuilder.add("message", testError.getMessage());
         Optional.ofNullable(testError.getDescription()).ifPresent(description -> propertiesBuilder.add("description", description));
-        propertiesBuilder.add("code", testError.getCode());
+        propertiesBuilder.add("code", testError.getUniqueCode());
         propertiesBuilder.add("fixable", testError.isFixable());
         propertiesBuilder.add("severity", testError.getSeverity().toString());
         propertiesBuilder.add("severityInteger", testError.getSeverity().getLevel());