Index: trunk/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java (revision 19142) +++ trunk/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java (revision 19152) @@ -2,19 +2,46 @@ package org.openstreetmap.josm.io.remotecontrol.handler; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; - +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.Collection; + +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.extension.ExtendWith; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerBadRequestException; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.junit.jupiter.api.Test; +import org.openstreetmap.josm.testutils.annotations.BasicWiremock; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; +import org.openstreetmap.josm.testutils.annotations.ThreadSync; /** * Unit tests of {@link LoadAndZoomHandler} class. */ +@BasicPreferences +@BasicWiremock +@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 LoadAndZoomHandler newHandler(String url) throws RequestHandlerBadRequestException { LoadAndZoomHandler req = new LoadAndZoomHandler(); + req.myCommand = LoadAndZoomHandler.command; if (url != null) req.setUrl(url); @@ -22,11 +49,29 @@ } + private static void syncThreads() { + // There are calls to the worker thread and EDT + new ThreadSync.ThreadSyncExtension().threadSync(); + } + + @BeforeEach + void setup(WireMockRuntimeInfo wireMockRuntimeInfo) { + String common = "visible=\"true\" version=\"1\" changeset=\"1\" timestamp=\"2000-01-01T00:00:00Z\" user=\"tsmock\" uid=\"1\""; + wireMockRuntimeInfo.getWireMock().register(WireMock.get("/api/0.6/map?bbox=0.0,0.0,0.001,0.001") + .willReturn(WireMock.aResponse().withBody("\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""))); + } + /** * Unit test for bad request - no param. - * @throws Exception if any error occurs - */ - @Test - void testBadRequestNoParam() throws Exception { - Exception e = assertThrows(RequestHandlerBadRequestException.class, () -> newHandler(null).handle()); + */ + @Test + void testBadRequestNoParam() { + final LoadAndZoomHandler handler = assertDoesNotThrow(() -> newHandler(null)); + Exception e = assertThrows(RequestHandlerBadRequestException.class, handler::handle); assertEquals("NumberFormatException (empty String)", e.getMessage()); } @@ -34,9 +79,9 @@ /** * Unit test for bad request - invalid URL. - * @throws Exception if any error occurs - */ - @Test - void testBadRequestInvalidUrl() throws Exception { - Exception e = assertThrows(RequestHandlerBadRequestException.class, () -> newHandler("invalid_url").handle()); + */ + @Test + void testBadRequestInvalidUrl() { + final LoadAndZoomHandler handler = assertDoesNotThrow(() -> newHandler("invalid_url")); + Exception e = assertThrows(RequestHandlerBadRequestException.class, handler::handle); assertEquals("The following keys are mandatory, but have not been provided: bottom, top, left, right", e.getMessage()); } @@ -44,20 +89,141 @@ /** * Unit test for bad request - incomplete URL. - * @throws Exception if any error occurs - */ - @Test - void testBadRequestIncompleteUrl() throws Exception { - Exception e = assertThrows(RequestHandlerBadRequestException.class, () -> newHandler("https://localhost").handle()); + */ + @Test + void testBadRequestIncompleteUrl() { + final LoadAndZoomHandler handler = assertDoesNotThrow(() -> newHandler("https://localhost")); + Exception e = assertThrows(RequestHandlerBadRequestException.class, handler::handle); assertEquals("The following keys are mandatory, but have not been provided: bottom, top, left, right", e.getMessage()); } /** - * Unit test for nominal request - local data file. - * @throws Exception if any error occurs - */ - @Test - @BasicPreferences - void testNominalRequest() throws Exception { - assertDoesNotThrow(() -> newHandler("https://localhost?bottom=0&top=0&left=1&right=1").handle()); + * Ensure that a download is called and completed + * @param wireMockRuntimeInfo The wiremock information + * @throws RequestHandlerBadRequestException If there is an issue with the handler + */ + @Test + void testDownload(WireMockRuntimeInfo wireMockRuntimeInfo) throws RequestHandlerBadRequestException { + LoadAndZoomHandler handler = newHandler(DEFAULT_BBOX_URL); + assertDoesNotThrow(handler::handle); + syncThreads(); + final DataSet ds = MainApplication.getLayerManager().getEditDataSet(); + assertNotNull(ds); + assertAll(() -> assertNotNull(ds.getPrimitiveById(1, OsmPrimitiveType.NODE)), + () -> assertNotNull(ds.getPrimitiveById(2, OsmPrimitiveType.NODE)), + () -> assertNotNull(ds.getPrimitiveById(3, OsmPrimitiveType.NODE)), + () -> assertNull(ds.getPrimitiveById(4, OsmPrimitiveType.NODE)), + () -> assertTrue(ds.selectionEmpty())); + wireMockRuntimeInfo.getWireMock().verifyThat(1, + RequestPatternBuilder.newRequestPattern().withUrl("/api/0.6/map?bbox=0.0,0.0,0.001,0.001")); + } + + /** + * Ensure that an area isn't downloaded twice + * @param wireMockRuntimeInfo The wiremock information + * @throws RequestHandlerBadRequestException If there is an issue with the handler + */ + @Test + void testDoubleDownload(WireMockRuntimeInfo wireMockRuntimeInfo) throws RequestHandlerBadRequestException { + testDownload(wireMockRuntimeInfo); + testDownload(wireMockRuntimeInfo); + // testDownload checks that the URL has been called once. Since it doesn't reset anything, we don't need + // a specific test here. + } + + /** + * Ensure that an overlapping area is trimmed before download + * @param wireMockRuntimeInfo The wiremock information + * @throws RequestHandlerBadRequestException If there is an issue with the handler + */ + @Test + void testOverlappingArea(WireMockRuntimeInfo wireMockRuntimeInfo) throws RequestHandlerBadRequestException { + LoadAndZoomHandler handler = newHandler(DEFAULT_BBOX_URL); + assertDoesNotThrow(handler::handle); + syncThreads(); + // The scientific notation is ok server-side. + final String mapCall = "/api/0.6/map?bbox=2.5E-4,0.001,7.5E-4,0.00125"; + final String commonNode = "visible=\"true\" version=\"1\" changeset=\"1\" timestamp=\"2000-01-01T00:00:00Z\" user=\"tsmock\" uid=\"1\""; + wireMockRuntimeInfo.getWireMock().register(WireMock.get(mapCall) + .willReturn(WireMock.aResponse().withBody("\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""))); + String request = "https://localhost/load_and_zoom?left=0.00025&bottom=0.00025&right=0.00075&top=0.00125"; + handler = newHandler(request); + assertDoesNotThrow(handler::handle); + syncThreads(); + wireMockRuntimeInfo.getWireMock().verifyThat(1, RequestPatternBuilder.newRequestPattern().withUrl(mapCall)); + final DataSet ds = MainApplication.getLayerManager().getEditDataSet(); + assertNotNull(ds); + assertAll(() -> assertNotNull(ds.getPrimitiveById(1, OsmPrimitiveType.NODE)), + () -> assertNotNull(ds.getPrimitiveById(2, OsmPrimitiveType.NODE)), + () -> assertNotNull(ds.getPrimitiveById(3, OsmPrimitiveType.NODE)), + () -> assertNotNull(ds.getPrimitiveById(4, OsmPrimitiveType.NODE))); + } + + /** + * Check search and zoom functionality + * @throws RequestHandlerBadRequestException If there is an issue with the handler + */ + @Main + @Projection + @Test + void testSearchAndZoom() throws RequestHandlerBadRequestException { + final LoadAndZoomHandler handler = newHandler(DEFAULT_BBOX_URL + "&search=id:1"); + assertDoesNotThrow(handler::handle); + syncThreads(); + final DataSet ds = MainApplication.getLayerManager().getEditDataSet(); + Collection selected = ds.getSelected(); + assertEquals(1, selected.size()); + assertTrue(selected.contains(ds.getPrimitiveById(1, OsmPrimitiveType.NODE))); + assertTrue(ds.searchNodes(MainApplication.getMap().mapView.getRealBounds().toBBox()) + .contains((Node) ds.getPrimitiveById(1, OsmPrimitiveType.NODE))); + } + + /** + * Check select and zoom functionality + * @throws RequestHandlerBadRequestException If there is an issue with the handler + */ + @Main + @Projection + @Test + void testSelectAndZoom() throws RequestHandlerBadRequestException { + final LoadAndZoomHandler handler = newHandler(DEFAULT_BBOX_URL + "&select=n1"); + assertDoesNotThrow(handler::handle); + syncThreads(); + final DataSet ds = MainApplication.getLayerManager().getEditDataSet(); + Collection selected = ds.getSelected(); + assertEquals(1, selected.size()); + assertTrue(selected.contains(ds.getPrimitiveById(1, OsmPrimitiveType.NODE))); + assertTrue(ds.searchNodes(MainApplication.getMap().mapView.getRealBounds().toBBox()) + .contains((Node) ds.getPrimitiveById(1, OsmPrimitiveType.NODE))); + } + + /** + * Check changeset tag functionality + * @throws RequestHandlerBadRequestException If there is an issue with the handler + */ + @Test + void testChangesetTags() throws RequestHandlerBadRequestException { + final String comment = "Add buildings, roads, and other random stuff"; + final String source = "This isn't Bing"; + final String hashtag = "#test-hashcodes"; + final String customTags = "custom=tag|is=here"; + final LoadAndZoomHandler handler = newHandler(DEFAULT_BBOX_URL + + "&changeset_comment=" + URLEncoder.encode(comment, StandardCharsets.UTF_8) + + "&changeset_source=" + URLEncoder.encode(source, StandardCharsets.UTF_8) + + "&changeset_hashtags=" + URLEncoder.encode(hashtag, StandardCharsets.UTF_8) + + "&changeset_tags=" + URLEncoder.encode(customTags, StandardCharsets.UTF_8)); + assertDoesNotThrow(handler::handle); + syncThreads(); + final DataSet ds = MainApplication.getLayerManager().getEditDataSet(); + assertEquals(comment, ds.getChangeSetTags().get("comment")); + assertEquals(source, ds.getChangeSetTags().get("source")); + assertEquals(hashtag, ds.getChangeSetTags().get("hashtags")); + assertEquals("tag", ds.getChangeSetTags().get("custom")); + assertEquals("here", ds.getChangeSetTags().get("is")); } } Index: trunk/test/unit/org/openstreetmap/josm/testutils/annotations/BasicWiremock.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/testutils/annotations/BasicWiremock.java (revision 19142) +++ trunk/test/unit/org/openstreetmap/josm/testutils/annotations/BasicWiremock.java (revision 19152) @@ -3,5 +3,5 @@ import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.fail; @@ -19,8 +19,8 @@ import java.util.Arrays; import java.util.List; -import java.util.stream.Collectors; - + +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; import org.junit.jupiter.api.extension.AfterAllCallback; -import org.junit.jupiter.api.extension.AfterEachCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; @@ -29,9 +29,7 @@ import org.junit.jupiter.api.extension.ParameterContext; import org.junit.jupiter.api.extension.ParameterResolutionException; -import org.junit.jupiter.api.extension.ParameterResolver; import org.junit.platform.commons.support.AnnotationSupport; import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; -import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.io.OsmApi; import org.openstreetmap.josm.spi.preferences.Config; @@ -43,5 +41,4 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.extension.ResponseTransformerV2; -import com.github.tomakehurst.wiremock.verification.LoggedRequest; /** @@ -81,6 +78,19 @@ * */ - class WireMockExtension - implements AfterAllCallback, AfterEachCallback, BeforeAllCallback, BeforeEachCallback, ParameterResolver { + class WireMockExtension extends com.github.tomakehurst.wiremock.junit5.WireMockExtension { + protected WireMockExtension() { + this(defaultOptions()); + } + + /** + * Create a new extension with options + * + * @param builder a {@link Builder} + * instance holding the initialisation parameters for the extension. + */ + protected WireMockExtension(Builder builder) { + super(builder); + } + /** * Get the default wiremock server @@ -98,12 +108,9 @@ new Pair<>(new Class[] {ExtensionContext.class }, new Object[] {context }), new Pair<>(new Class[0], new Object[0]))) { - try { - Constructor constructor = responseTransformer - .getConstructor(parameterMapping.a); - transformers.add(constructor.newInstance(parameterMapping.b)); - break; - } catch (ReflectiveOperationException e) { - fail(e); - } + Constructor constructor = assertDoesNotThrow(() -> + responseTransformer.getConstructor(parameterMapping.a)); + ResponseTransformerV2 transformerV2 = assertDoesNotThrow(() -> constructor.newInstance(parameterMapping.b)); + transformers.add(transformerV2); + break; } } @@ -114,4 +121,11 @@ } + static Builder defaultOptions() { + WireMockConfiguration options = WireMockConfiguration.options() + .usingFilesUnderDirectory(TestUtils.getTestDataRoot()) + .dynamicPort(); + return extensionOptions().options(options); + } + /** * Replace URL servers with wiremock @@ -120,5 +134,7 @@ * @param url The URL to fix * @return A url that points at the wiremock server + * @deprecated since 19152 (not used in core; no known users) */ + @Deprecated(forRemoval = true, since = "19152") public static String replaceUrl(WireMockServer wireMockServer, String url) { try { @@ -132,36 +148,18 @@ @Override - public void afterAll(ExtensionContext context) throws Exception { - // Run in EDT to avoid stopping wiremock server before wiremock requests finish. - GuiHelper.runInEDTAndWait(getWiremock(context)::stop); - } - - @Override - public void afterEach(ExtensionContext context) throws Exception { - List missed = getWiremock(context).findUnmatchedRequests().getRequests(); - missed.forEach(r -> Logging.error(r.getAbsoluteUrl())); - try { - assertTrue(missed.isEmpty(), missed.stream().map(LoggedRequest::getUrl).collect(Collectors.joining("\n\n"))); - } finally { - getWiremock(context).resetRequests(); - getWiremock(context).resetToDefaultMappings(); - getWiremock(context).resetScenarios(); - if (AnnotationUtils.elementIsAnnotated(context.getElement(), BasicWiremock.class) - || getWiremock(context) == null) { - this.afterAll(context); - } - } - } - - @Override - public void beforeAll(ExtensionContext context) throws Exception { - getWiremock(context).start(); - } - - @Override - public void beforeEach(ExtensionContext context) throws Exception { - if (AnnotationUtils.elementIsAnnotated(context.getElement(), BasicWiremock.class) || getWiremock(context) == null) { - this.beforeAll(context); - } + protected void onBeforeAll(ExtensionContext extensionContext, WireMockRuntimeInfo wireMockRuntimeInfo) { + extensionContext.getStore(ExtensionContext.Namespace.create(BasicWiremock.WireMockExtension.class)) + .put(BasicWiremock.WireMockExtension.class, this); + } + + @Override + protected void onAfterAll(ExtensionContext extensionContext, WireMockRuntimeInfo wireMockRuntimeInfo) { + // Sync threads to ensure that no further wiremock requests will be made + final ThreadSync.ThreadSyncExtension threadSyncExtension = new ThreadSync.ThreadSyncExtension(); + assertDoesNotThrow(() -> threadSyncExtension.afterEach(extensionContext)); + } + + @Override + protected void onBeforeEach(ExtensionContext context, WireMockRuntimeInfo wireMockRuntimeInfo) { if (context.getTestClass().isPresent()) { List wireMockFields = AnnotationSupport.findAnnotatedFields(context.getRequiredTestClass(), BasicWiremock.class); @@ -172,4 +170,6 @@ try { field.set(context.getTestInstance().orElse(null), getWiremock(context)); + } catch (IllegalAccessException e) { + fail(e); } finally { field.setAccessible(isAccessible); @@ -185,4 +185,10 @@ public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException { + if (super.supportsParameter(parameterContext, extensionContext)) { + return true; + } + if (WireMockRuntimeInfo.class.isAssignableFrom(parameterContext.getParameter().getType())) { + return true; + } return parameterContext.getParameter().getAnnotation(BasicWiremock.class) != null && parameterContext.getParameter().getType() == WireMockServer.class; @@ -192,4 +198,10 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) throws ParameterResolutionException { + if (super.supportsParameter(parameterContext, extensionContext)) { + return super.resolveParameter(parameterContext, extensionContext); + } + if (WireMockRuntimeInfo.class.isAssignableFrom(parameterContext.getParameter().getType())) { + return getRuntimeInfo(); + } return getWiremock(extensionContext); } @@ -199,12 +211,8 @@ * A class specifically to mock OSM API calls */ - class OsmApiExtension extends WireMockExtension { - @Override - public void afterAll(ExtensionContext context) throws Exception { - try { - super.afterAll(context); - } finally { - Config.getPref().put("osm-server.url", "https://invalid.url"); - } + class OsmApiExtension implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback { + @Override + public void afterAll(ExtensionContext context) { + Config.getPref().put("osm-server.url", "https://invalid.url"); } @@ -214,11 +222,19 @@ fail("OsmApiExtension requires @BasicPreferences"); } - super.beforeAll(context); - Config.getPref().put("osm-server.url", getWiremock(context).baseUrl() + "/api"); - getWiremock(context).stubFor(WireMock.get("/api/0.6/capabilities") + this.beforeEach(context); + } + + @Override + public void beforeEach(ExtensionContext extensionContext) throws Exception { + BasicWiremock.WireMockExtension extension = + extensionContext.getStore(ExtensionContext.Namespace.create(BasicWiremock.WireMockExtension.class)) + .get(BasicWiremock.WireMockExtension.class, BasicWiremock.WireMockExtension.class); + WireMockRuntimeInfo wireMockRuntimeInfo = extension.getRuntimeInfo(); + Config.getPref().put("osm-server.url", wireMockRuntimeInfo.getHttpBaseUrl() + "/api"); + wireMockRuntimeInfo.getWireMock().register(WireMock.get("/api/0.6/capabilities") .willReturn(WireMock.aResponse().withBodyFile("api/0.6/capabilities"))); - getWiremock(context).stubFor(WireMock.get("/api/capabilities") + wireMockRuntimeInfo.getWireMock().register(WireMock.get("/api/capabilities") .willReturn(WireMock.aResponse().withBodyFile("api/capabilities"))); - OsmApi.getOsmApi().initialize(NullProgressMonitor.INSTANCE); + assertDoesNotThrow(() -> OsmApi.getOsmApi().initialize(NullProgressMonitor.INSTANCE)); } }