Changeset 18433 in josm for trunk

Timestamp:

2022-04-19T22:29:22+02:00 (3 years ago)

Author:

taylor.smock

Message:

Fix #22024: DataIntegrityProblemException during data upload

This is caused by a race condition between the upload methods and updates to
the relation editor. If the upload takes long enough, the user can cause an
update to the relation editor, which can trigger the problem.

This works around the problem by returning the original relation when the
dataset is locked. This keeps compatibility with the interface method javadocs.

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/gui/dialogs/relation/actions/RelationEditorActionsTest.java (modified) (3 diffs)

trunk/src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java

@@ -75 +75 @@
     default IRelation<?> getChangedRelation() {
         final Relation newRelation;
-        if (getEditor().getRelation() != null) {
-            newRelation = new Relation(getEditor().getRelation());
+        final Relation oldRelation = getEditor().getRelation();
+        if (oldRelation != null && oldRelation.getDataSet() != null && oldRelation.getDataSet().isLocked()) {
+            // If the dataset is locked, then we cannot change the relation. See JOSM #22024.
+            // This is due to the `setMembers` -> `addReferrer` call chain requires that the dataset is not read only.
+            return oldRelation;
+        } else if (oldRelation != null) {
+            newRelation = new Relation(oldRelation);
         } else {
             newRelation = new Relation();

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/relation/actions/RelationEditorActionsTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.dialogs.relation.actions;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -14 +15 @@
 import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.TestUtils;
+import org.openstreetmap.josm.data.coor.LatLon;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker;
 
@@ -142 +150 @@
         assertTrue(jopMockerCalled[0]);
     }
+
+    /**
+     * Non-regression test for JOSM #22024.
+     * This is due to a race condition between uploading and refreshing the relation in the editor.
+     */
+    @Test
+    void testNonRegression22024() {
+        final DataSet ds = new DataSet();
+        final Node node = new Node(LatLon.ZERO);
+        Relation relation = TestUtils.newRelation("type=restriction", new RelationMember("", node));
+        ds.addPrimitive(node);
+        ds.addPrimitive(relation);
+        MainApplication.getLayerManager().prepareLayerForUpload(new OsmDataLayer(ds, "testNonRegression22024", null));
+        // Sanity check that behavior hasn't changed
+        assertTrue(ds.isLocked(), "The dataset should be locked when it is being uploaded.");
+        relationEditorAccess.getEditor().setRelation(relation);
+        relationEditorAccess.getMemberTableModel().populate(relation);
+        relationEditorAccess.getTagModel().initFromPrimitive(relation);
+        relationEditorAccess.getEditor().reloadDataFromRelation();
+        assertDoesNotThrow(relationEditorAccess::getChangedRelation);
+    }
 }