Changeset 36233 in osm for applications/editors/josm/plugins/reverter/test

Timestamp:

2024-03-28T15:04:04+01:00 (13 months ago)

Author:

taylor.smock

Message:

Fix #23582: Reverter plugin reverts nodes that were not modified by the reverted changeset

Location:

applications/editors/josm/plugins/reverter/test

Files:

• data/regress/23582 (added)
• data/regress/23582/mappings (added)
• data/regress/23582/mappings/0.6_changeset_149181932-c5e3af31-9ec6-411a-a730-f4d5182dc7ab.json (added)
• data/regress/23582/mappings/0.6_changeset_149181932_download-9c68cf67-51e0-44e5-850b-4470024a49d3.json (added)
• data/regress/23582/mappings/0.6_nodes-de271621-dcda-4355-af16-1121284cab60.json (added)
• data/regress/23582/mappings/0.6_nodes-f46384a6-d001-42c0-a70d-b64bc4cc4004.json (added)
• data/regress/23582/mappings/0.6_ways-1d794d03-79cb-435c-a041-f16e3a3b0c3e.json (added)
• data/regress/23582/mappings/0.6_ways-bc252594-155c-417a-9e37-093c523bd23f.json (added)
• data/regress/23582/mappings/capabilities-09c1dd2c-032b-4ba8-923f-86ebc1d68234.json (added)
• data/regress/23582/nodes.json (added)
• data/regress/23582/ways.json (added)
• data/regress/README.md (added)
• unit/reverter/ChangesetReverterTest.java (modified) (9 diffs)

applications/editors/josm/plugins/reverter/test/unit/reverter/ChangesetReverterTest.java

@@ -4 +4 @@
 import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -21 +23 @@
 import java.util.stream.Collectors;
 
-import jakarta.json.Json;
-import jakarta.json.JsonArray;
-import jakarta.json.JsonNumber;
-import jakarta.json.JsonObject;
-import jakarta.json.JsonReader;
-import jakarta.json.JsonString;
-import jakarta.json.JsonValue;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.openstreetmap.josm.TestUtils;
+import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask;
+import org.openstreetmap.josm.actions.downloadtasks.DownloadParams;
+import org.openstreetmap.josm.data.UndoRedoHandler;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
+import org.openstreetmap.josm.data.osm.PrimitiveId;
+import org.openstreetmap.josm.data.osm.SimplePrimitiveId;
+import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
+import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.spi.preferences.Config;
+import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
+import org.openstreetmap.josm.testutils.annotations.HTTP;
+import org.openstreetmap.josm.testutils.annotations.Main;
+import org.openstreetmap.josm.testutils.annotations.Projection;
+import org.openstreetmap.josm.tools.JosmRuntimeException;
+import org.openstreetmap.josm.tools.Logging;
 
 import com.github.tomakehurst.wiremock.client.WireMock;
@@ -38 +55 @@
 import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
 import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
-import org.openstreetmap.josm.TestUtils;
-import org.openstreetmap.josm.actions.downloadtasks.DownloadOsmTask;
-import org.openstreetmap.josm.actions.downloadtasks.DownloadParams;
-import org.openstreetmap.josm.data.UndoRedoHandler;
-import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
-import org.openstreetmap.josm.data.osm.PrimitiveId;
-import org.openstreetmap.josm.data.osm.SimplePrimitiveId;
-import org.openstreetmap.josm.gui.MainApplication;
-import org.openstreetmap.josm.gui.layer.OsmDataLayer;
-import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
-import org.openstreetmap.josm.gui.util.GuiHelper;
-import org.openstreetmap.josm.spi.preferences.Config;
-import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
-import org.openstreetmap.josm.testutils.annotations.HTTP;
-import org.openstreetmap.josm.tools.JosmRuntimeException;
-import org.openstreetmap.josm.tools.Logging;
+import jakarta.json.Json;
+import jakarta.json.JsonArray;
+import jakarta.json.JsonNumber;
+import jakarta.json.JsonObject;
+import jakarta.json.JsonReader;
+import jakarta.json.JsonString;
+import jakarta.json.JsonValue;
 
 /**
@@ -62 +68 @@
 @BasicPreferences
 @HTTP
+@Main
+@Projection
 class ChangesetReverterTest {
     @RegisterExtension
@@ -74 +82 @@
     void testTicket22520(WireMockRuntimeInfo wireMockRuntimeInfo) throws ExecutionException, InterruptedException {
         wireMockRuntimeInfo.getWireMock().loadMappingsFrom(TestUtils.getRegressionDataDir(22520));
-        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/nodes")).willReturn(WireMock.aResponse().withTransformers("MultiplePrimitiveTransformer")));
-        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/relations")).willReturn(WireMock.aResponse().withTransformers("MultiplePrimitiveTransformer")));
-        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/ways")).willReturn(WireMock.aResponse().withTransformers("MultiplePrimitiveTransformer")));
+        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/nodes")).willReturn(WireMock.aResponse().withTransformer("MultiplePrimitiveTransformer", "ticket", 22520)));
+        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/relations")).willReturn(WireMock.aResponse().withTransformer("MultiplePrimitiveTransformer", "ticket", 22520)));
+        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/ways")).willReturn(WireMock.aResponse().withTransformer("MultiplePrimitiveTransformer", "ticket", 22520)));
         Config.getPref().put("osm-server.url", wireMockRuntimeInfo.getHttpBaseUrl());
         PrimitiveId building = new SimplePrimitiveId(233056719, OsmPrimitiveType.WAY);
@@ -90 +98 @@
 
     /**
+     * Non-regression test for #23582: Nodes that were not modified by a changeset should not be reverted.
+     * Note: This might not be the intended behavior, but it was the behavior prior to r36230.
+     */
+    @Test
+    void testTicket23582(WireMockRuntimeInfo wireMockRuntimeInfo) {
+        wireMockRuntimeInfo.getWireMock().loadMappingsFrom(TestUtils.getRegressionDataDir(23582));
+        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/nodes")).willReturn(WireMock.aResponse().withTransformer("MultiplePrimitiveTransformer", "ticket", 23582)));
+        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/relations")).willReturn(WireMock.aResponse().withTransformer("MultiplePrimitiveTransformer", "ticket", 23582)));
+        wireMockRuntimeInfo.getWireMock().register(WireMock.get(WireMock.urlPathMatching("/0.6/ways")).willReturn(WireMock.aResponse().withTransformer("MultiplePrimitiveTransformer", "ticket", 23582)));
+        Config.getPref().put("osm-server.url", wireMockRuntimeInfo.getHttpBaseUrl());
+        final RevertChangesetTask task = new RevertChangesetTask(149181932, ChangesetReverter.RevertType.FULL, true, true);
+        task.run();
+        GuiHelper.runInEDTAndWait(() -> { /* Sync UI thread (some actions are taken on this thread which modify primitives) */ });
+        final DataSet reverted = MainApplication.getLayerManager().getEditDataSet();
+        assertEquals(1, reverted.allModifiedPrimitives().size());
+        final Way oldWay = assertInstanceOf(Way.class, reverted.allModifiedPrimitives().iterator().next());
+        assertEquals(8, oldWay.getNodesCount());
+    }
+
+    /**
      * A transformer for the /nodes?node, /ways?ways, and /relations?relations endpoints. This is needed since we don't always do the requests in the same order.
      */
@@ -100 +128 @@
         @Override
         public Response transform(Request request, Response response, FileSource files, Parameters parameters) {
+            final int ticket = parameters.getInt("ticket");
             final QueryParameter wayParam = request.queryParameter("ways");
             final QueryParameter nodeParam = request.queryParameter("nodes");
             final QueryParameter relParam = request.queryParameter("relations");
             if (wayParam.isPresent()) {
-                return Response.Builder.like(response).but().body(getReturnXml(OsmPrimitiveType.WAY, wayParam)).build();
+                return Response.Builder.like(response).but().body(getReturnXml(ticket, OsmPrimitiveType.WAY, wayParam)).build();
             } else if (nodeParam.isPresent()) {
-                return Response.Builder.like(response).but().body(getReturnXml(OsmPrimitiveType.NODE, nodeParam)).build();
+                return Response.Builder.like(response).but().body(getReturnXml(ticket, OsmPrimitiveType.NODE, nodeParam)).build();
             } else if (relParam.isPresent()) {
-                return Response.Builder.like(response).but().body(getReturnXml(OsmPrimitiveType.RELATION, relParam)).build();
+                return Response.Builder.like(response).but().body(getReturnXml(ticket, OsmPrimitiveType.RELATION, relParam)).build();
             } else {
                 IllegalArgumentException e = new IllegalArgumentException("No query parameter present");
@@ -121 +150 @@
         }
 
-        private static String getReturnXml(OsmPrimitiveType type, QueryParameter parameter) {
+        private static String getReturnXml(int ticket, OsmPrimitiveType type, QueryParameter parameter) {
             final String file;
             switch (type) {
@@ -140 +169 @@
                     : parameter.values())
                     .stream().map(s -> new OsmParameterInformation(type, s)).collect(Collectors.toSet());
-            try (InputStream fis = Files.newInputStream(Paths.get(TestUtils.getRegressionDataDir(22520), file));
+            try (InputStream fis = Files.newInputStream(Paths.get(TestUtils.getRegressionDataDir(ticket), file));
                  JsonReader reader = Json.createReader(fis)) {
                 String version = "0.6";