Changeset 19200 in josm for trunk/test/unit/org/openstreetmap

Timestamp:

2024-08-19T20:16:02+02:00 (6 months ago)

Author:

taylor.smock

Message:

Fix #23821: Ensure that remote control commands are processed in order

This reverts or partially reverts r19153 and r19196 in favour of forcing ordering
in the RequestProcessor#run method. This does not block the server thread, but
it can mean that we have a bunch of processor threads that are waiting on the
previous processor thread.

File:

• trunk/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java (modified) (6 diffs)

trunk/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java

@@ -11 +11 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.io.IOException;
+import java.net.URI;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
@@ -31 +33 @@
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.io.remotecontrol.RemoteControl;
 import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerBadRequestException;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
 
@@ -39 +43 @@
 import org.openstreetmap.josm.testutils.annotations.Projection;
 import org.openstreetmap.josm.testutils.annotations.ThreadSync;
+import org.openstreetmap.josm.tools.HttpClient;
 
 /**
@@ -47 +52 @@
 @ExtendWith(BasicWiremock.OsmApiExtension.class)
 class LoadAndZoomHandlerTest {
-    private static final String DEFAULT_BBOX_URL = "https://localhost/load_and_zoom?left=0&bottom=0&right=0.001&top=0.001";
-    private static final String DEFAULT_BBOX_URL_2 = "https://localhost/load_and_zoom?left=0.00025&bottom=0.00025&right=0.00075&top=0.00125";
+    private static final String DEFAULT_BBOX_URL = "http://localhost/load_and_zoom?left=0&bottom=0&right=0.001&top=0.001";
+    private static final String DEFAULT_BBOX_URL_2 = "http://localhost/load_and_zoom?left=0.00025&bottom=0.00025&right=0.00075&top=0.00125";
     private static LoadAndZoomHandler newHandler(String url) throws RequestHandlerBadRequestException {
         LoadAndZoomHandler req = new LoadAndZoomHandler();
@@ -237 +242 @@
     /**
      * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/23821">#23821</a>
-     * @throws RequestHandlerBadRequestException If there is an issue with the handler
-     */
-    @Test
-    void testNonRegression23821() throws RequestHandlerBadRequestException {
+     * @param wireMockRuntimeInfo The runtime info
+     */
+    @Test
+    void testNonRegression23821(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOException {
+        // Start remote control on a random port to avoid failing when JOSM is open.
+        final int port = wireMockRuntimeInfo.getHttpPort() + 1;
+        Config.getPref().putInt("remote.control.port", port);
         final AtomicBoolean block = new AtomicBoolean(false);
         final Runnable runnable = () -> {
@@ -257 +265 @@
         MainApplication.getLayerManager().addLayer(new OsmDataLayer(wrongDataset,
                 "LoadAndZoomHandlerTest#testNonRegression23821", null));
-        ForkJoinTask<?> task1;
-        ForkJoinTask<?> task2;
         try {
-            GuiHelper.runInEDT(runnable);
-            MainApplication.worker.submit(runnable);
-            // The processor makes a new handler for each request
-            // It is single-threaded, so blocking on one handler would fix the problem with the other handler.
-            // But we might as well work on multi-threading, since it is easier to test. :)
-            final LoadAndZoomHandler handler1 = newHandler(DEFAULT_BBOX_URL + "&new_layer=true&layer_name=OSMData");
-            final LoadAndZoomHandler handler2 = newHandler(DEFAULT_BBOX_URL_2 + "&new_layer=false&layer_name=OSMData");
-            // Use a separate threads to avoid blocking on this thread
-            final ForkJoinPool pool = ForkJoinPool.commonPool();
-            task1 = pool.submit(() -> assertDoesNotThrow(handler1::handle));
-
-            // Make certain there is enough time for the first task to block
-            Awaitility.await().until(() -> true);
-            task2 = pool.submit(() -> assertDoesNotThrow(handler2::handle));
+            ForkJoinTask<?> task1;
+            ForkJoinTask<?> task2;
+            try {
+                GuiHelper.runInEDT(runnable);
+                MainApplication.worker.submit(runnable);
+                // The processor makes a new handler for each request
+                // It is single-threaded, so blocking on one handler would fix the problem with the other handler.
+                // But we might as well work on multi-threading, since it is easier to test. :)
+                final HttpClient first = HttpClient.create(URI.create(DEFAULT_BBOX_URL.replace("localhost", "localhost:" + port)
+                        + "&new_layer=true&layer_name=OSMData").toURL());
+                final HttpClient second = HttpClient.create(URI.create(DEFAULT_BBOX_URL_2.replace("localhost", "localhost:" + port)
+                        + "&new_layer=false&layer_name=OSMData").toURL());
+                // Use a separate threads to avoid blocking on this thread.
+                final ForkJoinPool pool = ForkJoinPool.commonPool();
+                RemoteControl.start();
+                task1 = pool.submit(() -> assertDoesNotThrow(() -> {
+                    try {
+                        assertEquals("OK\r\n", first.connect().fetchContent());
+                    } finally {
+                        first.disconnect();
+                    }
+                }));
+                // Make certain there is enough time for the first task to block
+                Awaitility.await().until(() -> true);
+                task2 = pool.submit(() -> assertDoesNotThrow(() -> {
+                    try {
+                        assertEquals("OK\r\n", second.connect().fetchContent());
+                    } finally {
+                        second.disconnect();
+                    }
+                }));
+            } finally {
+                // Unblock UI/worker threads
+                synchronized (block) {
+                    block.set(true);
+                    block.notifyAll();
+                }
+            }
+
+            task1.join();
+            task2.join();
+
+            syncThreads();
         } finally {
-            // Unblock UI/worker threads
-            synchronized (block) {
-                block.set(true);
-                block.notifyAll();
-            }
+            // We have to stop remote control _after_ we join the tasks and sync threads.
+            RemoteControl.stop();
         }
-
-        task1.join();
-        task2.join();
-
-        syncThreads();
         assertEquals(2, MainApplication.getLayerManager().getLayers().size());
         final DataSet ds = MainApplication.getLayerManager().getEditDataSet();