From 514b3e7c4f1429f5602239bb408fb123b404f36b Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Tue, 30 Apr 2024 06:51:21 -0600 Subject: [PATCH 1/2] Fix image navigation pane Signed-off-by: Taylor Smock --- .../data/mapillary/ImageNavigation.java | 22 ++++++++------ .../data/mapillary/MapillarySequence.java | 7 ++--- .../gui/dialog/ImageNavigationDialog.java | 29 ++++++++++++------- .../layer/geoimage/MapillaryImageEntry.java | 3 +- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/ImageNavigation.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/ImageNavigation.java index 5784e62d0..994660aad 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/ImageNavigation.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/ImageNavigation.java @@ -75,17 +75,21 @@ public ImageNavigation(final VectorDataSet dataset, final MapillaryNode node) { final EastNorth nodeEn = node.getEastNorth(); final double travelAngle; MapillarySequence seq = this.node.getSequence(); - if (Objects.equals(this.node, seq.firstNode()) && seq.getNodes().size() > 1) { - travelAngle = Math.toDegrees(this.node.bearing(seq.getNode(1))); - } else if (Objects.equals(this.node, seq.lastNode()) && seq.getNodes().size() > 1) { - travelAngle = Math.toDegrees(seq.getNode(seq.getNodesCount() - 1).bearing(this.node)); - } else { - int idx = seq.getNodes().indexOf(this.node); - if (seq.getNodes().size() <= 3) { - travelAngle = MapillaryImageUtils.getAngle(this.node); + if (seq != null) { + if (Objects.equals(this.node, seq.firstNode()) && seq.getNodes().size() > 1) { + travelAngle = Math.toDegrees(this.node.bearing(seq.getNode(1))); + } else if (Objects.equals(this.node, seq.lastNode()) && seq.getNodes().size() > 1) { + travelAngle = Math.toDegrees(seq.getNode(seq.getNodesCount() - 1).bearing(this.node)); } else { - travelAngle = Math.toDegrees(seq.getNode(idx - 1).bearing(seq.getNode(idx + 1))); + int idx = seq.getNodes().indexOf(this.node); + if (seq.getNodes().size() <= 3) { + travelAngle = MapillaryImageUtils.getAngle(this.node); + } else { + travelAngle = Math.toDegrees(seq.getNode(idx - 1).bearing(seq.getNode(idx + 1))); + } } + } else { + travelAngle = Double.NaN; } this.originalSort = surroundingNodes.stream().map(n -> sort(isPano, false, angle, travelAngle, n, nodeEn)) .filter(Objects::nonNull) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillarySequence.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillarySequence.java index e9e0aa601..4a8659243 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillarySequence.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillarySequence.java @@ -5,7 +5,6 @@ import java.util.Collections; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; import org.openstreetmap.josm.data.osm.BBox; import org.openstreetmap.josm.data.osm.INode; @@ -74,7 +73,7 @@ public List getNodes() { @Override public List getNodeIds() { - return Arrays.stream(this.nodes).map(MapillaryNode::getId).collect(Collectors.toList()); + return Arrays.stream(this.nodes).map(MapillaryNode::getId).toList(); } @Override @@ -124,8 +123,8 @@ public OsmPrimitiveType getType() { @Override public boolean equals(Object other) { - if (other instanceof IWay) { - return Objects.equals(MapillarySequenceUtils.getKey(this), MapillarySequenceUtils.getKey((IWay) other)); + if (other instanceof IWay iway) { + return Objects.equals(MapillarySequenceUtils.getKey(this), MapillarySequenceUtils.getKey(iway)); } return false; } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/ImageNavigationDialog.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/ImageNavigationDialog.java index d28c9ec57..924133e84 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/ImageNavigationDialog.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/ImageNavigationDialog.java @@ -30,6 +30,7 @@ import org.openstreetmap.josm.plugins.mapillary.data.mapillary.MapillaryNode; import org.openstreetmap.josm.plugins.mapillary.gui.boilerplate.MapillaryButton; import org.openstreetmap.josm.plugins.mapillary.gui.layer.MapillaryLayer; +import org.openstreetmap.josm.plugins.mapillary.gui.layer.geoimage.MapillaryImageEntry; import org.openstreetmap.josm.tools.Shortcut; /** @@ -69,16 +70,15 @@ public class ImageNavigationDialog extends ToggleDialog */ public ImageNavigationDialog() { super(tr("Mapillary: Image Navigation"), "mapillary-main", tr("Navigate between different Mapillary images"), - Shortcut.registerShortcut("mapillary:image_navigation", tr("Mapillary: Image Navigation"), KeyEvent.CHAR_UNDEFINED, - Shortcut.NONE), + Shortcut.registerShortcut("mapillary:image_navigation", tr("Mapillary: Image Navigation"), + KeyEvent.CHAR_UNDEFINED, Shortcut.NONE), 80); MainApplication.getLayerManager().addAndFireLayerChangeListener(this); } @Override public void layerAdded(LayerManager.LayerAddEvent e) { - if (e.getAddedLayer() instanceof MapillaryLayer) { - MapillaryLayer layer = (MapillaryLayer) e.getAddedLayer(); + if (e.getAddedLayer() instanceof MapillaryLayer layer) { layer.addImageChangeListener(this); } } @@ -96,11 +96,18 @@ public void layerOrderChanged(LayerManager.LayerOrderChangeEvent e) { @Override public void imageChanged(IGeoImageLayer source, List> oldImages, List> newImages) { - if (source instanceof MapillaryLayer - && newImages.stream().filter(MapillaryNode.class::isInstance).count() == 1) { - MapillaryLayer layer = (MapillaryLayer) source; - MapillaryNode node = (MapillaryNode) newImages.get(0); - if (node != null && layer.getData() != null && node.getSequence() != null) { + if (source instanceof MapillaryLayer layer + && newImages.stream() + .filter(n -> n instanceof MapillaryNode + || (n instanceof MapillaryImageEntry mie && mie.getImage() instanceof MapillaryNode)) + .count() == 1) { + final MapillaryNode node; + if (newImages.get(0) instanceof MapillaryNode tNode) { + node = tNode; + } else { + node = (MapillaryNode) ((MapillaryImageEntry) newImages.get(0)).getImage(); + } + if (node != null && layer.getData() != null) { this.imageNavigation = new ImageNavigation(layer.getData(), node); } else { this.imageNavigation = null; @@ -201,8 +208,8 @@ private static final class ImageAction extends JosmAction { @Override public void actionPerformed(ActionEvent e) { MapillaryLayer layer = MapillaryLayer.getInstance(); - if (this.toSelect instanceof MapillaryNode) { - layer.setCurrentImage((MapillaryNode) this.toSelect); + if (this.toSelect instanceof MapillaryNode node) { + layer.setCurrentImage(node); } else { layer.getData().setSelected(this.toSelect); } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/geoimage/MapillaryImageEntry.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/geoimage/MapillaryImageEntry.java index e5d160709..e5685aaf8 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/geoimage/MapillaryImageEntry.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/geoimage/MapillaryImageEntry.java @@ -196,10 +196,11 @@ public MapillaryImageEntry getLastImage() { @Override public void selectImage(ImageViewerDialog imageViewerDialog, IImageEntry entry) { - IImageEntry.super.selectImage(imageViewerDialog, entry); + // If the image is a Mapillary image, we want to use our custom path *first* if (entry instanceof MapillaryImageEntry mapillaryImageEntry) { selectImage(mapillaryImageEntry); } + IImageEntry.super.selectImage(imageViewerDialog, entry); } private static void selectImage(@Nullable final MapillaryImageEntry entry) { From 6ff61bfbff2321bd795de222da26e5db7fefcf40 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Tue, 30 Apr 2024 08:39:45 -0600 Subject: [PATCH 2/2] Fix unit tests for WireMock 3 Signed-off-by: Taylor Smock --- .../annotations/MapillaryURLWireMock.java | 50 +++++++++++-------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/test/unit/org/openstreetmap/josm/plugins/mapillary/testutils/annotations/MapillaryURLWireMock.java b/test/unit/org/openstreetmap/josm/plugins/mapillary/testutils/annotations/MapillaryURLWireMock.java index 4d43c6362..8bca5bcb9 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapillary/testutils/annotations/MapillaryURLWireMock.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapillary/testutils/annotations/MapillaryURLWireMock.java @@ -30,18 +30,29 @@ import javax.imageio.ImageIO; +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.ExtendWith; +import org.junit.jupiter.api.extension.ExtensionContext; +import org.junit.platform.commons.support.AnnotationSupport; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.plugins.mapillary.spi.preferences.IMapillaryUrls; +import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryConfig; +import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryUrls; +import org.openstreetmap.josm.tools.Utils; + import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.WireMock; -import com.github.tomakehurst.wiremock.common.FileSource; import com.github.tomakehurst.wiremock.common.TextFile; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; -import com.github.tomakehurst.wiremock.extension.Parameters; -import com.github.tomakehurst.wiremock.extension.ResponseTransformer; +import com.github.tomakehurst.wiremock.extension.ResponseTransformerV2; import com.github.tomakehurst.wiremock.extension.responsetemplating.ResponseTemplateTransformer; -import com.github.tomakehurst.wiremock.http.Request; +import com.github.tomakehurst.wiremock.extension.responsetemplating.TemplateEngine; import com.github.tomakehurst.wiremock.http.Response; import com.github.tomakehurst.wiremock.matching.AnythingPattern; import com.github.tomakehurst.wiremock.matching.StringValuePattern; +import com.github.tomakehurst.wiremock.stubbing.ServeEvent; import com.github.tomakehurst.wiremock.stubbing.StubMapping; import com.github.tomakehurst.wiremock.verification.LoggedRequest; import jakarta.json.Json; @@ -52,16 +63,6 @@ import jakarta.json.JsonReader; import jakarta.json.JsonString; import jakarta.json.JsonValue; -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.ExtendWith; -import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.platform.commons.support.AnnotationSupport; -import org.openstreetmap.josm.plugins.mapillary.spi.preferences.IMapillaryUrls; -import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryConfig; -import org.openstreetmap.josm.plugins.mapillary.spi.preferences.MapillaryUrls; -import org.openstreetmap.josm.tools.Utils; /** * Mock Mapillary API calls @@ -122,7 +123,8 @@ public void beforeAll(final ExtensionContext context) throws Exception { final WireMockConfiguration wireMockConfiguration = new WireMockConfiguration().dynamicPort(); final FillerUrlReplacer fillerUrlReplacer = new FillerUrlReplacer(); - wireMockConfiguration.extensions(new CollectionEndpoint(), new ResponseTemplateTransformer(false), + wireMockConfiguration.extensions(new CollectionEndpoint(), + new ResponseTemplateTransformer(TemplateEngine.defaultTemplateEngine(), false, null, null), fillerUrlReplacer); // See JOSM #21121 for why this is necessary Path directory = Paths.get("test", "resources"); @@ -215,18 +217,21 @@ public void beforeAll(final ExtensionContext context) throws Exception { /** * Account for collection endpoints */ - private static final class CollectionEndpoint extends ResponseTransformer { + private static final class CollectionEndpoint implements ResponseTransformerV2 { @Override - public Response transform(Request request, Response response, FileSource files, Parameters parameters) { + public Response transform(Response response, ServeEvent serveEvent) { + final var request = serveEvent.getRequest(); if (request.queryParameter("image_ids").isPresent()) { // Not implemented currently since I don't know if I need to split on `,` or `%2C` final List imageIds = request.queryParameter("image_ids").values().stream() .map(str -> str.split(",", -1)).flatMap(Stream::of).filter(Objects::nonNull) - .collect(Collectors.toList()); + .toList(); final List imageText = imageIds.stream() - .map(image -> files.getTextFileNamed("api/v4/responses/graph/" + image + ".json")) - .collect(Collectors.toList()); + .map(image -> Path.of(TestUtils.getTestDataRoot(), "__files", "api", "v4", "responses", "graph", image + ".json")) + .map(Path::toUri) + .map(TextFile::new) + .toList(); // We need to get the actual bytes prior to returning, so we need to read the files. final String body = imageText.stream().map(TextFile::readContentsAsString) .collect(Collectors.joining(",", "{\"data\":[", "]}")); @@ -244,7 +249,7 @@ public String getName() { /** * Replace filler urls with wiremock URLs */ - private static final class FillerUrlReplacer extends ResponseTransformer { + private static final class FillerUrlReplacer implements ResponseTransformerV2 { private static final Pattern FILLER_URL_PATTERN = Pattern.compile("([a-zA-Z0-9_]*?)_filler_url"); WireMockServer server; @@ -255,10 +260,11 @@ public String getName() { } @Override - public Response transform(Request request, Response response, FileSource files, Parameters parameters) { + public Response transform(Response response, ServeEvent serveEvent) { if (server == null) { fail("No wiremock server set"); } + final var request = serveEvent.getRequest(); // If the user is logged in, we technically don't need the access_token in parameters if (!request.queryParameter("access_token").isPresent() && !request.containsHeader("Authorization")) { fail("Always pass the access token: " + request.getUrl());