Merge pull request #98 from mapillary/fix/incomplete-mapillary-changeset

fix(change image location): incomplete Mapillary changeset

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 {

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;
+  }
 }

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
+        }
       }
     }
   ],

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"
+  }
+}

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"
+  }
+}

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"
+  }
+}

test/unit/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonLocationChangesetEncoderTest.java

@@ -1,67 +1,92 @@
 // 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;
 import org.openstreetmap.josm.plugins.mapillary.MapillaryLocationChangeset;
 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<String, JsonValue> 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<String> 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