diff --git a/build.gradle.kts b/build.gradle.kts index 9d3234ce1..5d75f8cd0 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -61,6 +61,7 @@ dependencies { testImplementation("org.awaitility:awaitility:3.1.6") testImplementation("org.jmockit:jmockit:1.45") testImplementation("com.github.spotbugs:spotbugs-annotations:3.1.12") + testImplementation("org.skyscreamer:jsonassert:1.5.0") } sourceSets { diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoder.java b/src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoder.java index 685363c66..caece9573 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoder.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoder.java @@ -7,10 +7,17 @@ import javax.json.JsonArrayBuilder; import javax.json.JsonObjectBuilder; +import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.plugins.mapillary.MapillaryImage; import org.openstreetmap.josm.plugins.mapillary.MapillaryLocationChangeset; +/** + * Encodes in JSON a location changeset. + * Former location and compass angle (CA) are systematically provided, + * even if not changed. + */ public final class JsonLocationChangesetEncoder { + private JsonLocationChangesetEncoder() { // Private constructor to avoid instantiation } @@ -27,29 +34,28 @@ public static JsonObjectBuilder encodeLocationChangeset(MapillaryLocationChanges .add("request_comment", "JOSM-created"); } - private static JsonObjectBuilder encodeImageChanges(MapillaryImage img) { + protected static JsonObjectBuilder encodeImageChanges(MapillaryImage img) { Objects.requireNonNull(img); - final JsonObjectBuilder to = Json.createObjectBuilder(); - if (!img.getTempLatLon().equalsEpsilon(img.getLatLon())) { - to.add("geometry", Json.createObjectBuilder() - .add("coordinates", Json.createArrayBuilder() - .add(img.getTempLatLon().getX()) - .add(img.getTempLatLon().getY()) - ).add("type", "Point") - ); - } - if (Math.abs(img.getCa() - img.getTempCa()) > 1e-9) { - to.add("properties", Json.createObjectBuilder().add("ca", img.getTempCa())); - } else { - to.add("properties", Json.createObjectBuilder()); - } - if (!img.getTempLatLon().equalsEpsilon(img.getLatLon())) { - to.add("type", "Feature"); - } + final JsonObjectBuilder from = getChangeJsonBuilder(img.getLatLon(), img.getCa()); + final JsonObjectBuilder to = getChangeJsonBuilder(img.getTempLatLon(), img.getTempCa()); return Json.createObjectBuilder() .add("image_key", img.getKey()) + .add("from", from) .add("to", to); } + + private static JsonObjectBuilder getChangeJsonBuilder(LatLon tempLatLon, double tempCa) { + final JsonObjectBuilder to = Json.createObjectBuilder(); + to.add("type", "Feature"); + to.add("geometry", Json.createObjectBuilder() + .add("coordinates", Json.createArrayBuilder() + .add(tempLatLon.getX()) + .add(tempLatLon.getY()) + ).add("type", "Point") + ); + to.add("properties", Json.createObjectBuilder().add("ca", tempCa)); + return to; + } } diff --git a/test/data/api/v3/requests/changeset.json b/test/data/api/v3/requests/changeset.json index 1ef957bd3..f5a68ae28 100644 --- a/test/data/api/v3/requests/changeset.json +++ b/test/data/api/v3/requests/changeset.json @@ -3,36 +3,79 @@ "changes": [ { "image_key": "wMAqAFr3xE9072G8Al6WLQ", + "from": { + "type": "Feature", + "properties": { + "ca": 10.0 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 13.3323, + 50.44612 + ] + } + }, "to": { "geometry": { - "coordinates": [13.3323, 50.44612], + "coordinates": [ + 13.4434, + 50.66834 + ], "type": "Point" }, - "properties": {"ca": 273.3}, + "properties": { + "ca": 283.3 + }, "type": "Feature" } }, { "image_key": "7erPn382xDMtmfdh0xtvUw", + "from": { + "type": "Feature", + "properties": { + "ca": 0.0 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 0.0, + 0.0 + ] + } + }, "to": { "geometry": { - "coordinates": [13.3328, 50.44619], + "coordinates": [ + 13.3328, + 50.44619 + ], "type": "Point" }, "properties": {}, "type": "Feature" } }, - { - "image_key": "31KDbCOzla0fJBtIeoBr1A", - "to": { - "properties": {"ca": 13.4} - } - }, { "image_key": "invalid image key will be ignored", + "from": { + "type": "Feature", + "properties": { + "ca": 0.0 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 0.0, + 0.0 + ] + } + }, "to": { - "properties": {"ca": 13.4} + "properties": { + "ca": 13.4 + } } } ], diff --git a/test/data/api/v3/requests/rotation_only_changeset.json b/test/data/api/v3/requests/rotation_only_changeset.json new file mode 100644 index 000000000..71d6e63b3 --- /dev/null +++ b/test/data/api/v3/requests/rotation_only_changeset.json @@ -0,0 +1,29 @@ +{ + "image_key": "rotationOnlyChangesetImageKey", + "from": { + "type": "Feature", + "properties": { + "ca": 10.0 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 13.3323, + 50.44612 + ] + } + }, + "to": { + "geometry": { + "coordinates": [ + 13.3323, + 50.44612 + ], + "type": "Point" + }, + "properties": { + "ca": -70.3 + }, + "type": "Feature" + } +} diff --git a/test/data/api/v3/requests/single_image_changeset.json b/test/data/api/v3/requests/single_image_changeset.json new file mode 100644 index 000000000..ad28e3195 --- /dev/null +++ b/test/data/api/v3/requests/single_image_changeset.json @@ -0,0 +1,29 @@ +{ + "image_key": "wMAqAFr3xE9072G8Al6WLQ", + "from": { + "type": "Feature", + "properties": { + "ca": 10.0 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 13.3323, + 50.44612 + ] + } + }, + "to": { + "geometry": { + "coordinates": [ + 13.4434, + 50.66834 + ], + "type": "Point" + }, + "properties": { + "ca": 283.3 + }, + "type": "Feature" + } +} diff --git a/test/data/api/v3/requests/translation_only_changeset.json b/test/data/api/v3/requests/translation_only_changeset.json new file mode 100644 index 000000000..d0cfd4974 --- /dev/null +++ b/test/data/api/v3/requests/translation_only_changeset.json @@ -0,0 +1,29 @@ +{ + "image_key": "translationOnlyChangesetImageKey", + "from": { + "type": "Feature", + "properties": { + "ca": 10.0 + }, + "geometry": { + "type": "Point", + "coordinates": [ + 13.3323, + 50.44612 + ] + } + }, + "to": { + "geometry": { + "coordinates": [ + 13.4434, + 50.66834 + ], + "type": "Point" + }, + "properties": { + "ca": 10.0 + }, + "type": "Feature" + } +} diff --git a/test/unit/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoderTest.java b/test/unit/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoderTest.java index de9dbb138..21d939f5c 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoderTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoderTest.java @@ -1,19 +1,16 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.mapillary.utils.api; -import static org.junit.Assert.assertEquals; - -import java.util.Comparator; -import java.util.Map.Entry; - -import javax.json.Json; -import javax.json.JsonArray; -import javax.json.JsonArrayBuilder; -import javax.json.JsonObject; -import javax.json.JsonObjectBuilder; -import javax.json.JsonValue; +import java.io.IOException; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.stream.Stream; +import org.json.JSONException; import org.junit.Test; +import org.skyscreamer.jsonassert.JSONAssert; import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.plugins.mapillary.MapillaryImage; @@ -21,47 +18,75 @@ import org.openstreetmap.josm.plugins.mapillary.utils.TestUtil; public class JsonLocationChangesetEncoderTest { + + @Test + public void testSingleImageChangeset() throws JSONException, URISyntaxException { + MapillaryImage image = new MapillaryImage("wMAqAFr3xE9072G8Al6WLQ", new LatLon(50.44612, 13.3323), 10.0, false); + image.move(0.1111, 0.22222); + image.turn(273.3); + image.stopMoving(); + + String expected = readLines("/api/v3/requests/single_image_changeset.json"); + String actual = JsonLocationChangesetEncoder.encodeImageChanges(image).build().toString(); + JSONAssert.assertEquals(expected, actual, true); + } + @Test - public void test() throws IllegalArgumentException { - MapillaryImage img1 = new MapillaryImage("wMAqAFr3xE9072G8Al6WLQ", new LatLon(0, 0), 0, false); - img1.move(13.3323, 50.44612); - img1.turn(273.3); - img1.stopMoving(); - MapillaryImage img2 = new MapillaryImage("7erPn382xDMtmfdh0xtvUw", new LatLon(0, 0), 0, false); - img2.move(13.3328, 50.44619); - img2.stopMoving(); - MapillaryImage img3 = new MapillaryImage("31KDbCOzla0fJBtIeoBr1A", new LatLon(0, 0), 0, false); - img3.turn(13.4); - img3.stopMoving(); - MapillaryImage img4 = new MapillaryImage("invalid image key will be ignored", new LatLon(0, 0), 0, false); - img4.turn(13.4); - img4.stopMoving(); + public void testTranslationOnlyChangeset() throws JSONException, URISyntaxException { + MapillaryImage image = new MapillaryImage("translationOnlyChangesetImageKey", new LatLon(50.44612, 13.3323), 10.0, false); + image.move(0.1111, 0.22222); + image.stopMoving(); + + String expected = readLines("/api/v3/requests/translation_only_changeset.json"); + String actual = JsonLocationChangesetEncoder.encodeImageChanges(image).build().toString(); + JSONAssert.assertEquals(expected, actual, true); + } - MapillaryLocationChangeset cs = new MapillaryLocationChangeset(); - cs.add(img1); - cs.add(img2); - cs.add(img3); - cs.add(img4); + @Test + public void testRotationOnlyChangeset() throws JSONException, URISyntaxException { + MapillaryImage image = new MapillaryImage("rotationOnlyChangesetImageKey", new LatLon(50.44612, 13.3323), 10.0, false); + image.turn(-80.3); + image.stopMoving(); - assertEquals( - sortChanges(Json.createReader(JsonLocationChangesetEncoderTest.class.getResourceAsStream("/api/v3/requests/changeset.json")).readObject()), - sortChanges(JsonLocationChangesetEncoder.encodeLocationChangeset(cs).build()) - ); + String expected = readLines("/api/v3/requests/rotation_only_changeset.json"); + String actual = JsonLocationChangesetEncoder.encodeImageChanges(image).build().toString(); + JSONAssert.assertEquals(expected, actual, true); } - private static JsonObject sortChanges(JsonObject changeset) { - final JsonObjectBuilder result = Json.createObjectBuilder(); - for (Entry e : changeset.entrySet()) { - if ("changes".equals(e.getKey())) { - final JsonArray changes = (JsonArray) e.getValue(); - final JsonArrayBuilder sortedChanges = Json.createArrayBuilder(); - changes.stream().sorted(Comparator.comparing(o -> ((JsonObject) o).getString("image_key"))).forEachOrdered(sortedChanges::add); - result.add(e.getKey(), sortedChanges); - } else { - result.add(e.getKey(), e.getValue()); - } + private static String readLines(String filePath) throws URISyntaxException { + StringBuilder contentBuilder = new StringBuilder(); + try (Stream stream = Files.lines( Paths.get(JsonLocationChangesetEncoderTest.class.getResource(filePath).toURI()), StandardCharsets.UTF_8)) + { + stream.forEach(s -> contentBuilder.append(s).append("\n")); } - return result.build(); + catch (IOException e) + { + e.printStackTrace(); + } + return contentBuilder.toString(); + } + + @Test + public void testMultipleImagesChangeset() throws IllegalArgumentException, URISyntaxException, JSONException { + MapillaryImage image1 = new MapillaryImage("wMAqAFr3xE9072G8Al6WLQ", new LatLon(50.44612, 13.3323), 10.0, false); + image1.move(0.1111, 0.22222); + image1.turn(273.3); + image1.stopMoving(); + MapillaryImage image2 = new MapillaryImage("7erPn382xDMtmfdh0xtvUw", new LatLon(0, 0), 0, false); + image2.move(13.3328, 50.44619); + image2.stopMoving(); + MapillaryImage image3 = new MapillaryImage("invalid image key will be ignored", new LatLon(0, 0), 0, false); + image3.turn(13.4); + image3.stopMoving(); + + MapillaryLocationChangeset changeset = new MapillaryLocationChangeset(); + changeset.add(image1); + changeset.add(image2); + changeset.add(image3); + + String expected = readLines("/api/v3/requests/changeset.json"); + String actual = JsonLocationChangesetEncoder.encodeLocationChangeset(changeset).build().toString(); + JSONAssert.assertEquals(expected, actual, false); } @Test