Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix image navigation pane #231

Merged
merged 2 commits into from
Apr 30, 2024
Merged

Fix image navigation pane #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
514b3e7
Fix image navigation pane
tsmock Apr 30, 2024
6ff61bf
Fix unit tests for WireMock 3
tsmock Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/ImageNavigation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/org/openstreetmap/josm/plugins/mapillary/data/mapillary/MapillarySequence.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,7 +73,7 @@ public List<MapillaryNode> getNodes() {

@Override
public List<Long> getNodeIds() {
return Arrays.stream(this.nodes).map(MapillaryNode::getId).collect(Collectors.toList());
return Arrays.stream(this.nodes).map(MapillaryNode::getId).toList();
}

@Override
Expand Down Expand Up @@ -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;
}
Expand Down
29 changes: 18 additions & 11 deletions src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/dialog/ImageNavigationDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -96,11 +96,18 @@ public void layerOrderChanged(LayerManager.LayerOrderChangeEvent e) {
@Override
public void imageChanged(IGeoImageLayer source, List<? extends IImageEntry<?>> oldImages,
List<? extends IImageEntry<?>> 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;
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion ...java/org/openstreetmap/josm/plugins/mapillary/gui/layer/geoimage/MapillaryImageEntry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
50 changes: 28 additions & 22 deletions .../org/openstreetmap/josm/plugins/mapillary/testutils/annotations/MapillaryURLWireMock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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<String> imageIds = request.queryParameter("image_ids").values().stream()
.map(str -> str.split(",", -1)).flatMap(Stream::of).filter(Objects::nonNull)
.collect(Collectors.toList());
.toList();
final List<TextFile> 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\":[", "]}"));
Expand All @@ -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;
Expand All @@ -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 <i>technically</i> 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());
Expand Down