From eb41b22f84bf1aa186b82d4c422389fd681c95df Mon Sep 17 00:00:00 2001 From: Taylor Smock <45215054+tsmock@users.noreply.github.com> Date: Wed, 8 Dec 2021 14:10:53 -0700 Subject: [PATCH] WaterWayCheck: Add autofix suggestion (geometry) (#477) * MapRoulette: Allow upload of OSC to MapRoulette Signed-off-by: Taylor Smock * WaterWayCheck: Add autofix suggestion (geometry) Signed-off-by: Taylor Smock --- build.gradle | 1 + dependencies.gradle | 1 + .../serializer/CheckFlagDeserializer.java | 14 ++- .../atlas/checks/maproulette/data/Task.java | 41 +++++- .../CooperativeChallengeOperation.java | 65 ++++++++-- .../GeometryChangeOperation.java | 69 +++++++++++ .../OpenStreetMapCheckFlagConverter.java | 2 + .../linear/lines/WaterWayCheck.java | 22 +++- .../serializer/CheckFlagDeserializerTest.java | 32 +++++ .../checks/maproulette/data/TaskTest.java | 107 ++++++++++++++++ .../checks/maproulette/data/TaskTestRule.java | 27 ++++ .../GeometryChangeOperationTest.java | 117 ++++++++++++++++++ .../GeometryChangeOperationTestRule.java | 27 ++++ .../linear/lines/WaterWayCheckTest.java | 80 +++++++++++- 14 files changed, 584 insertions(+), 21 deletions(-) create mode 100644 src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperation.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTestRule.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTestRule.java diff --git a/build.gradle b/build.gradle index 0641085b9..1749d784c 100644 --- a/build.gradle +++ b/build.gradle @@ -101,6 +101,7 @@ dependencies // Support Junit 5 tests testImplementation packages.junit.api + testImplementation packages.junit.params testRuntimeOnly packages.junit.engine // Support JUnit 3/4 tests testCompileOnly packages.junit.junit4 diff --git a/dependencies.gradle b/dependencies.gradle index e2a2a06d5..005815570 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -30,6 +30,7 @@ project.ext.packages = [ vintage: "org.junit.vintage:junit-vintage-engine:${versions.junit}", api: "org.junit.jupiter:junit-jupiter-api:${versions.junit}", engine: "org.junit.jupiter:junit-jupiter-engine:${versions.junit}", + params: "org.junit.jupiter:junit-jupiter-params:${versions.junit}", ], sqlite: "org.xerial:sqlite-jdbc:${versions.sqlite}", log4j: "log4j:log4j:${versions.log4j}" diff --git a/src/main/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializer.java b/src/main/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializer.java index 076cf50d3..f6b3d8498 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializer.java +++ b/src/main/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializer.java @@ -354,11 +354,16 @@ private Optional getFixSuggestion(final AtlasEntity beforeView, { return Optional.empty(); } + // Get the OSC if present + final Optional osc = Optional.ofNullable(fixSuggestion.get("osc")) + .map(JsonElement::getAsString); // Create a remove FeatureChange from a shallow copy of the before view if (ChangeDescriptorType.REMOVE.equals(suggestionType)) { - return Optional.of(new FeatureChange(ChangeType.REMOVE, - CompleteEntity.shallowFrom(beforeView), beforeView)); + final FeatureChange featureChange = new FeatureChange(ChangeType.REMOVE, + CompleteEntity.shallowFrom(beforeView), beforeView); + osc.ifPresent(featureChange::withOsc); + return Optional.of(featureChange); } // Create a full copy for the after view @@ -402,6 +407,9 @@ private Optional getFixSuggestion(final AtlasEntity beforeView, this.applyGeometryChanges(afterView, geometryChangeDescriptors); } - return Optional.of(new FeatureChange(ChangeType.ADD, (AtlasEntity) afterView, beforeView)); + final var featureChange = new FeatureChange(ChangeType.ADD, (AtlasEntity) afterView, + beforeView); + osc.ifPresent(featureChange::withOsc); + return Optional.of(featureChange); } } diff --git a/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/Task.java b/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/Task.java index e90c89277..9dbd46fed 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/Task.java +++ b/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/Task.java @@ -8,7 +8,7 @@ import java.util.stream.Collectors; import org.apache.commons.codec.binary.StringUtils; -import org.openstreetmap.atlas.checks.maproulette.data.cooperative_challenge.TagChangeOperation; +import org.openstreetmap.atlas.checks.maproulette.data.cooperative_challenge.CooperativeChallengeOperation; import org.openstreetmap.atlas.exception.CoreException; import org.openstreetmap.atlas.geography.Location; import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; @@ -73,7 +73,18 @@ private List convert() for (final FeatureChange featureChange : this.featureChanges) { final ChangeDescription whatChanged = featureChange.explain(); - operationsList.add(new TagChangeOperation(whatChanged).create().getJson()); + final Class classType = CooperativeChallengeOperation + .getAppropriateChangeOperation(whatChanged); + try + { + operationsList.add(classType.getConstructor(ChangeDescription.class) + .newInstance(whatChanged).create().getJson()); + } + catch (final ReflectiveOperationException reflectiveOperationException) + { + throw new CoreException("Class {0} does not have an appropriate constructor", + reflectiveOperationException, classType.getName()); + } } // convert each featureChange to a cooperative work object return operationsList; @@ -233,11 +244,29 @@ private JsonObject generateTaskCooperativeWork(final List cooperativ final JsonObject cooperativeWorkObject = new JsonObject(); final JsonObject meta = new JsonObject(); meta.add("version", new JsonPrimitive(2)); - meta.add("type", new JsonPrimitive(1)); + final byte type; + if (cooperativeWork.size() == 1 && cooperativeWork.get(0).has("format")) + { + // This is an OSC cooperative work challenge type + type = 2; + } + else + { + // Default to the tag fix cooperative work challenge type + type = 1; + } + meta.add("type", new JsonPrimitive(type)); cooperativeWorkObject.add("meta", meta); - final JsonArray cooperativeWorkArray = new JsonArray(); - cooperativeWork.forEach(cooperativeWorkArray::add); - cooperativeWorkObject.add("operations", cooperativeWorkArray); + if (type == 2) + { + cooperativeWorkObject.add("file", cooperativeWork.get(0)); + } + else + { + final var cooperativeWorkArray = new JsonArray(); + cooperativeWork.forEach(cooperativeWorkArray::add); + cooperativeWorkObject.add("operations", cooperativeWorkArray); + } return cooperativeWorkObject; } diff --git a/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/CooperativeChallengeOperation.java b/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/CooperativeChallengeOperation.java index d6a12c383..0d11ed0e6 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/CooperativeChallengeOperation.java +++ b/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/CooperativeChallengeOperation.java @@ -4,6 +4,7 @@ import java.util.function.Predicate; import java.util.stream.Collectors; +import org.openstreetmap.atlas.exception.CoreException; import org.openstreetmap.atlas.geography.atlas.change.description.ChangeDescription; import org.openstreetmap.atlas.geography.atlas.change.description.ChangeDescriptorType; import org.openstreetmap.atlas.geography.atlas.change.description.descriptors.ChangeDescriptor; @@ -13,16 +14,17 @@ /*** * This represents an operation that is embedded in a cooperative challenge. Example: Given a - * FeatureCollection geojson uploaded to MapRoulette: - * + * FeatureCollection geojson uploaded to MapRoulette:
+ * For Tag changes: + * * 
  * {
  *   "type": "FeatureCollection",
  *   "features": [ ... ],            // omitted for readability
- *   "cooperativeWork": {            // special `cooperativeWork` property
+ *   "cooperativeWork": {            // special `cooperativeWork` property
  *     "meta": {
- *       "version": 2,               // must be format version `2`
- *       "type": 1                   // `1` for tag fix type
+ *       "version": 2,               // must be format version `2`
+ *       "type": 1                   // `1` for tag fix type
  *     },
  *     "operations": [               // Operations section (see below)
  *       ...
@@ -30,7 +32,28 @@
  *   }
  * }
  *
- * + * + * For geometry changes: + * + * 
+ * {
+ *   "type": "FeatureCollection",
+ *   "features": [ ... ],            // omitted for readability
+ *   "cooperativeWork": {            // special `cooperativeWork` property
+ *     "meta": {
+ *       "version": 2,               // must be format version `2`
+ *       "type": 2                   // `2` for change file type
+ *     },
+ *     "file": {                     // Operations section (see below)
+ *       "type": "xml",              // only xml is supported at this time
+ *       "format": "osc",            // only osc is supported at this time
+ *       "encoding": "base64",       // only base64 is supported at this time
+ *       "content": "..."            // The base64 encoded OSC file
+ *     }
+ *   }
+ * }
+ *
+ * * This class is an abstract representation of the json material to be found under "operations". Its * subclasses should handle the various ChangeDescriptor types, including GeometryChangeDescriptor, * LongElementChangeDescriptor, RelationMemberChangeDescriptor, TagChangeDescriptor @@ -51,6 +74,35 @@ public abstract class CooperativeChallengeOperation private static final String DELIMITER = "/"; private static final int ATLAS_SECTIONING_IDENTIFIER_LENGTH = 6; + /** + * Get the appropriate change operation class + * + * @param changeDescription + * The change descriptor + * @return A class appropriate for the change descriptor + */ + public static Class getAppropriateChangeOperation( + final ChangeDescription changeDescription) + { + for (final ChangeDescriptor changeDescriptor : changeDescription.getChangeDescriptors()) + { + switch (changeDescriptor.getName()) + { + case GEOMETRY: + // GEOMETRY requires OSC, which covers just about everything else. + return GeometryChangeOperation.class; + case TAG: + break; + default: + throw new CoreException( + "No cooperative challenge converter is available for {0}", + changeDescriptor.getName()); + } + } + return changeDescription.getOsc().isPresent() ? GeometryChangeOperation.class + : TagChangeOperation.class; + } + /** * Return the string representation of the OSM data type best representing the itemType * @@ -114,5 +166,4 @@ protected void setJson(final JsonObject json) { this.json = json; } - } diff --git a/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperation.java b/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperation.java new file mode 100644 index 000000000..4d81458fe --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperation.java @@ -0,0 +1,69 @@ +package org.openstreetmap.atlas.checks.maproulette.data.cooperative_challenge; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.Predicate; + +import org.openstreetmap.atlas.geography.atlas.change.description.ChangeDescription; +import org.openstreetmap.atlas.geography.atlas.change.description.descriptors.ChangeDescriptor; +import org.openstreetmap.atlas.geography.atlas.change.description.descriptors.ChangeDescriptorName; + +import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; + +/** + * Support geometry change operations + * + * @author Taylor Smock + */ +public class GeometryChangeOperation extends CooperativeChallengeOperation +{ + private static final String CONTENT = "content"; + private static final Map FILE_MAP = new HashMap<>(3); + static + { + FILE_MAP.put("type", "xml"); + FILE_MAP.put("format", "osc"); + FILE_MAP.put("encoding", "base64"); + } + + /** The OSC for the geometry change */ + private final String osc; + + public GeometryChangeOperation(final ChangeDescription changeDescription) + { + super(changeDescription); + final var optionalOsc = changeDescription.getOsc(); + if (optionalOsc.isPresent()) + { + this.osc = optionalOsc.get(); + } + else if (changeDescription.toJsonElement().getAsJsonObject().has("osc")) + { + this.osc = changeDescription.toJsonElement().getAsJsonObject().get("osc").getAsString(); + } + else + { + this.osc = null; + } + } + + @Override + public GeometryChangeOperation create() + { + final var json = new JsonObject(); + if (this.osc != null && !this.osc.isBlank()) + { + FILE_MAP.forEach((key, value) -> json.add(key, new JsonPrimitive(value))); + json.add(CONTENT, new JsonPrimitive(this.osc)); + } + this.setJson(json); + return this; + } + + @Override + protected Predicate operationFilter() + { + return changeDescriptor -> changeDescriptor.getName() == ChangeDescriptorName.GEOMETRY; + } +} diff --git a/src/main/java/org/openstreetmap/atlas/checks/utility/OpenStreetMapCheckFlagConverter.java b/src/main/java/org/openstreetmap/atlas/checks/utility/OpenStreetMapCheckFlagConverter.java index 0a682f805..a383c7915 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/utility/OpenStreetMapCheckFlagConverter.java +++ b/src/main/java/org/openstreetmap/atlas/checks/utility/OpenStreetMapCheckFlagConverter.java @@ -128,6 +128,8 @@ private static void convertFixSuggestions(final Map> final Optional concatenatedAfterPolyline = entry.getValue().stream() .map(FeatureChange::getAfterView) .sorted(Comparator.comparing(AtlasEntity::getIdentifier)) + // Removed edges may not have a polyline + .filter(entity -> ((Edge) entity).asPolyLine() != null) .map(entity -> ((Edge) entity).asPolyLine()).reduce(PolyLine::append); if (concatenatedBeforePolyline.isPresent() && concatenatedAfterPolyline.isPresent()) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheck.java index e8c8fc057..6e1cd7b69 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheck.java @@ -23,6 +23,9 @@ import org.openstreetmap.atlas.geography.PolyLine; import org.openstreetmap.atlas.geography.Segment; import org.openstreetmap.atlas.geography.atlas.Atlas; +import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteEntity; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.geography.atlas.items.LineItem; import org.openstreetmap.atlas.geography.atlas.items.LocationItem; @@ -494,15 +497,26 @@ private CheckFlag flagIncline(final CheckFlag flag, final LineItem line, final L if (uphill && this.minResolutionDistance .isGreaterThanOrEqualTo(this.elevationUtils.getResolution(first))) { - final CheckFlag returnFlag = flag; final String instruction = this.getLocalizedInstruction( FALLBACK_INSTRUCTIONS.indexOf(GOES_UPHILL), line.getOsmIdentifier(), this.elevationUtils.getResolution(first).asMeters()); - if (returnFlag == null) + final CheckFlag returnFlag; + if (flag == null) + { + returnFlag = this.createFlag(line, instruction); + } + else { - return this.createFlag(line, instruction); + returnFlag = flag; + returnFlag.addInstruction(instruction); } - returnFlag.addInstruction(instruction); + final CompleteEntity entity = (CompleteEntity) CompleteEntity.shallowFrom(line); + final List points = Iterables.stream(line).collectToList(); + Collections.reverse(points); + entity.withGeometry(points); + final var featureChange = FeatureChange.add((AtlasEntity) entity, line.getAtlas(), + FeatureChange.Options.OSC_IF_POSSIBLE); + returnFlag.addFixSuggestion(featureChange); return returnFlag; } return flag; diff --git a/src/test/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializerTest.java b/src/test/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializerTest.java index 6765190ec..5d89b7a7f 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializerTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/flag/serializer/CheckFlagDeserializerTest.java @@ -3,6 +3,7 @@ import java.io.BufferedReader; import java.io.FileReader; import java.io.IOException; +import java.lang.reflect.Field; import java.util.Arrays; import java.util.Collections; import java.util.Date; @@ -24,6 +25,7 @@ import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; import org.openstreetmap.atlas.geography.atlas.complete.CompleteArea; import org.openstreetmap.atlas.geography.atlas.complete.CompleteEdge; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteLine; import org.openstreetmap.atlas.geography.atlas.complete.CompleteNode; import org.openstreetmap.atlas.geography.atlas.complete.CompletePoint; import org.openstreetmap.atlas.geography.atlas.complete.CompleteRelation; @@ -167,6 +169,36 @@ public void instructionsDeserializationTest() throws IOException Assert.assertNotEquals(0, instructions.length()); } + @Test + public void oscDeserializationTest() throws ReflectiveOperationException + { + final Line line = this.rule.atlas().line(1000000L); + final List locations = Iterables.asList(line.asPolyLine()); + Collections.reverse(locations); + final CheckFlag lineFlag = new CheckFlag("1000000", Collections.singleton(line), + Collections.singletonList("Instruction"), locations, + Collections.singleton(FeatureChange.add( + (AtlasEntity) CompleteLine.from(line).withGeometry(locations), + this.rule.atlas(), FeatureChange.Options.OSC_IF_POSSIBLE))); + + final Map contextualProperties = new HashMap<>(); + contextualProperties.put("generator", "NullCheck"); + contextualProperties.put("timestamp", new Date().toString()); + + final String serializedJson = CheckFlagEvent.flagToJson(lineFlag, contextualProperties) + .toString(); + final CheckFlag deserializedFlag = gson.fromJson( + CheckFlagEvent.flagToJson(lineFlag, contextualProperties), CheckFlag.class); + final Field oscField = FeatureChange.class.getDeclaredField("osc"); + Assert.assertTrue(oscField.trySetAccessible()); + Assert.assertEquals(1, deserializedFlag.getFixSuggestions().size()); + Assert.assertEquals(1, lineFlag.getFixSuggestions().size()); + Assert.assertEquals( + lineFlag.getFixSuggestions().iterator().next().explain().toJsonElement() + .getAsJsonObject().get("osc").getAsString(), + oscField.get(deserializedFlag.getFixSuggestions().iterator().next())); + } + @Test public void pointDeserializationTest() { diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTest.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTest.java new file mode 100644 index 000000000..d12341cb5 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTest.java @@ -0,0 +1,107 @@ +package org.openstreetmap.atlas.checks.maproulette.data; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.Base64; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteLine; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; +import org.openstreetmap.atlas.utilities.collections.Iterables; + +/** + * Test class for {@link Task}. + * + * @author Taylor Smock + */ +public class TaskTest +{ + @Rule + public TaskTestRule rule = new TaskTestRule(); + + /** + * Create a basic task + * + * @param points + * The points + * @return The created task + */ + private static Task createTaskSkeleton(final Location... points) + { + return createTaskSkeleton(Stream.of(points).collect(Collectors.toSet())); + } + + /** + * Create a basic task + * + * @param points + * The points + * @return The created task + */ + private static Task createTaskSkeleton(final Set points) + { + final var task = new Task(); + // Need to see challenge/identifier to avoid NPE + task.setChallengeName("Test challenge"); + task.setTaskIdentifier("Test identifier"); + + task.setPoints(points); + task.setInstruction("Test instruction"); + return task; + } + + /** + * Test for the private inner class FixSuggestionToCooperativeWorkConverter (Geometry) + */ + @Test + public void testChangeDescriptionCreationGeometry() + { + final var line = this.rule.getAtlas().line(1000000L); + final var linePoints = Iterables.asList(line.asPolyLine()); + Collections.reverse(linePoints); + final var task = createTaskSkeleton(linePoints.get(0)); + task.setCooperativeWork(Collections.singleton(FeatureChange.add( + (AtlasEntity) CompleteLine.shallowFrom(line).withGeometry(linePoints), + line.getAtlas(), FeatureChange.Options.OSC_IF_POSSIBLE))); + final var taskJson = task.generateTask(1); + final var cooperativeWork = taskJson.getAsJsonObject("geometries") + .getAsJsonObject("cooperativeWork"); + assertEquals( + "", + new String(Base64.getDecoder().decode(cooperativeWork.getAsJsonObject("file") + .getAsJsonPrimitive("content").getAsString()))); + assertEquals(2, cooperativeWork.getAsJsonObject("meta").get("type").getAsByte()); + } + + @Test + public void testChangeDescriptionCreationTag() + { + final var line = this.rule.getAtlas().line(1000000L); + final var linePoints = Iterables.asList(line.asPolyLine()); + Collections.reverse(linePoints); + final var task = createTaskSkeleton(linePoints.get(0)); + task.setCooperativeWork(Collections.singleton( + FeatureChange.add(CompleteLine.shallowFrom(line).withAddedTag("test", "tag"), + line.getAtlas(), FeatureChange.Options.OSC_IF_POSSIBLE))); + final var taskJson = task.generateTask(1); + final var cooperativeWork = taskJson.getAsJsonObject("geometries") + .getAsJsonObject("cooperativeWork"); + assertEquals(1, cooperativeWork.getAsJsonObject("meta").get("type").getAsByte()); + final var operations = cooperativeWork.getAsJsonArray("operations"); + assertEquals(1, operations.size()); + final var operation = operations.get(0).getAsJsonObject().getAsJsonObject("data") + .getAsJsonArray("operations").get(0).getAsJsonObject(); + assertEquals("setTags", operation.get("operation").getAsString()); + final var entries = operation.getAsJsonObject("data").entrySet(); + assertEquals(1, entries.size()); + assertEquals("test", entries.iterator().next().getKey()); + assertEquals("tag", entries.iterator().next().getValue().getAsString()); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTestRule.java new file mode 100644 index 000000000..50d884668 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/TaskTestRule.java @@ -0,0 +1,27 @@ +package org.openstreetmap.atlas.checks.maproulette.data; + +import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.atlas.Atlas; +import org.openstreetmap.atlas.utilities.testing.CoreTestRule; +import org.openstreetmap.atlas.utilities.testing.TestAtlas; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Line; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Loc; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Point; + +/** + * Atlases for {@link TaskTest} + * + * @author Taylor Smock + */ +public class TaskTestRule extends CoreTestRule +{ + @TestAtlas(points = { @Point(id = "1000000", coordinates = @Loc(Location.TEST_1_COORDINATES)), + @Point(id = "2000000", coordinates = @Loc(Location.TEST_2_COORDINATES)) }, lines = @Line(id = "1000000", coordinates = { + @Loc(Location.TEST_1_COORDINATES), @Loc(Location.TEST_2_COORDINATES) })) + private Atlas atlas; + + public Atlas getAtlas() + { + return this.atlas; + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTest.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTest.java new file mode 100644 index 000000000..32b0daff3 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTest.java @@ -0,0 +1,117 @@ +package org.openstreetmap.atlas.checks.maproulette.data.cooperative_challenge; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Base64; +import java.util.Collections; +import java.util.List; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteEdge; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; + +import com.google.gson.JsonObject; + +/** + * Test class for {@link GeometryChangeOperationTestRule} + * + * @author Taylor Smock + */ +public class GeometryChangeOperationTest +{ + private static final String EDGE_ATLAS_OSC = ""; + @Rule + public final GeometryChangeOperationTestRule setup = new GeometryChangeOperationTestRule(); + + /** + * Check that an OSC is generated and/or returned properly + */ + @Test + public void testCreationOfOscTask() + { + final CompleteEdge edge = CompleteEdge.from(this.setup.getEdgeAtlas().edge(1L)); + final List locationCollection = new ArrayList<>(edge.asPolyLine()); + Collections.reverse(locationCollection); + final FeatureChange featureChange = FeatureChange.add( + (AtlasEntity) CompleteEdge.shallowFrom(edge).withGeometry(locationCollection) + .withAddedTag("random", "probably"), + this.setup.getEdgeAtlas(), FeatureChange.Options.OSC_IF_POSSIBLE); + final GeometryChangeOperation geometryChangeOperation = new GeometryChangeOperation( + featureChange.explain()).create(); + final JsonObject json = geometryChangeOperation.getJson(); + + assertEquals("xml", json.get("type").getAsString()); + assertEquals("osc", json.get("format").getAsString()); + assertEquals("base64", json.get("encoding").getAsString()); + // Decode the content so that if it breaks, a human can see what changed between the + // expected osm change and the actual osm change + assertEquals(EDGE_ATLAS_OSC, + new String(Base64.getDecoder().decode(json.get("content").getAsString()))); + } + + /** + * This check ensures that an empty OSC is reflected in the generated json + */ + @Test + public void testEmptyOscTask() + { + final CompleteEdge edge = CompleteEdge.from(this.setup.getEdgeAtlas().edge(1L)); + final FeatureChange featureChange = FeatureChange.add( + CompleteEdge.shallowFrom(edge).withAddedTag("random", "probably"), + this.setup.getEdgeAtlas(), FeatureChange.Options.OSC_IF_POSSIBLE); + featureChange.withOsc(""); + final GeometryChangeOperation geometryChangeOperation = new GeometryChangeOperation( + featureChange.explain()).create(); + final JsonObject json = geometryChangeOperation.getJson(); + + assertTrue(json.entrySet().isEmpty()); + } + + /** + * This check ensures that there isn't a crash due to a missing "osc" entry + */ + @Test + public void testNoCreationOfOscTask() + { + final CompleteEdge edge = CompleteEdge.from(this.setup.getEdgeAtlas().edge(1L)); + final FeatureChange featureChange = FeatureChange.add( + CompleteEdge.shallowFrom(edge).withAddedTag("random", "probably"), + this.setup.getEdgeAtlas(), FeatureChange.Options.OSC_IF_POSSIBLE); + final GeometryChangeOperation geometryChangeOperation = new GeometryChangeOperation( + featureChange.explain()).create(); + final JsonObject json = geometryChangeOperation.getJson(); + + assertTrue(json.entrySet().isEmpty(), json.toString()); + } + + /** + * This check ensures that there isn't a crash due to a missing "osc" entry + */ + @Test + public void testSetOfOscTask() + { + final CompleteEdge edge = CompleteEdge.from(this.setup.getEdgeAtlas().edge(1L)); + final FeatureChange featureChange = FeatureChange.add( + CompleteEdge.shallowFrom(edge).withAddedTag("random", "probably"), + this.setup.getEdgeAtlas(), FeatureChange.Options.OSC_IF_POSSIBLE); + featureChange.withOsc(Base64.getEncoder() + .encodeToString(EDGE_ATLAS_OSC.getBytes(StandardCharsets.UTF_8))); + final GeometryChangeOperation geometryChangeOperation = new GeometryChangeOperation( + featureChange.explain()).create(); + final JsonObject json = geometryChangeOperation.getJson(); + + assertEquals("xml", json.get("type").getAsString()); + assertEquals("osc", json.get("format").getAsString()); + assertEquals("base64", json.get("encoding").getAsString()); + // Decode the content so that if it breaks, a human can see what changed between the + // expected osm change and the actual osm change + assertEquals(EDGE_ATLAS_OSC, + new String(Base64.getDecoder().decode(json.get("content").getAsString()))); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTestRule.java new file mode 100644 index 000000000..e1c93aae5 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/data/cooperative_challenge/GeometryChangeOperationTestRule.java @@ -0,0 +1,27 @@ +package org.openstreetmap.atlas.checks.maproulette.data.cooperative_challenge; + +import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.atlas.Atlas; +import org.openstreetmap.atlas.utilities.testing.CoreTestRule; +import org.openstreetmap.atlas.utilities.testing.TestAtlas; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Edge; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Loc; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Node; + +/** + * Test rule for {@link GeometryChangeOperationTest} + * + * @author Taylor Smock + */ +public class GeometryChangeOperationTestRule extends CoreTestRule +{ + @TestAtlas(nodes = { @Node(id = "1000000", coordinates = @Loc(Location.TEST_1_COORDINATES)), + @Node(id = "2000000", coordinates = @Loc(Location.TEST_2_COORDINATES)) }, edges = @Edge(id = "1", tags = "highway=residential", coordinates = { + @Loc(Location.TEST_1_COORDINATES), @Loc(Location.TEST_2_COORDINATES) })) + private Atlas edgeAtlas; + + public Atlas getEdgeAtlas() + { + return this.edgeAtlas; + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheckTest.java index 9c801b0f2..3cdd2b773 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheckTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/linear/lines/WaterWayCheckTest.java @@ -1,7 +1,15 @@ package org.openstreetmap.atlas.checks.validation.linear.lines; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; + import java.lang.reflect.Field; +import java.util.Collections; +import java.util.List; +import org.apache.commons.collections.ListUtils; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -9,6 +17,9 @@ import org.openstreetmap.atlas.checks.utility.ElevationUtilities; import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier; import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.atlas.change.FeatureChange; +import org.openstreetmap.atlas.geography.atlas.items.Line; +import org.openstreetmap.atlas.utilities.collections.Iterables; /** * WaterWayCheck test @@ -41,6 +52,13 @@ public void setUp() public void testCircularWaterway() { this.verifier.actual(this.atlases.getCircularWaterway(), this.defaultWaterWayCheck); + this.verifier.verify(flag -> assertEquals(2, flag.getRawInstructions().size())); + // We don't care about the order of the instructions + this.verifier.verify(flag -> assertTrue(flag.getRawInstructions().stream().anyMatch( + "The waterway 0 loops back on itself. This is typically impossible."::equals))); + this.verifier.verify(flag -> assertTrue(flag.getRawInstructions().stream().anyMatch( + "The waterway 0 does not end in a sink (ocean/sinkhole/waterway/drain)."::equals))); + this.verifier.verify(flag -> assertTrue(flag.getFixSuggestions().isEmpty())); this.verifier.verifyExpectedSize(1); } @@ -99,6 +117,23 @@ public void testCoastWaterwayConnectedElevationReversed() throws ReflectiveOpera .get(check); elevationUtils.putMap(Location.forString("16.9906416,-88.3188021"), map); this.verifier.actual(this.atlases.getCoastlineWaterwayConnected(), check); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals( + "The waterway 130 probably does not go up hill.\nPlease check (source elevation data resolution was about 26,585.386 meters).", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertEquals(1, flag.getFixSuggestions().size())); + this.verifier.verify(flag -> + { + final FeatureChange suggestion = flag.getFixSuggestions().iterator().next(); + final Line after = (Line) suggestion.getAfterView(); + final Line before = (Line) suggestion.getBeforeView(); + assertNull(suggestion.getTags()); + final List afterLocations = Iterables.stream(after).collectToList(); + final List beforeLocations = Iterables.stream(before).collectToList(); + assertFalse(ListUtils.isEqualList(afterLocations, beforeLocations)); + Collections.reverse(beforeLocations); + assertTrue(ListUtils.isEqualList(afterLocations, beforeLocations)); + }); this.verifier.verifyExpectedSize(1); } @@ -106,7 +141,7 @@ public void testCoastWaterwayConnectedElevationReversed() throws ReflectiveOpera public void testCoastWaterwayConnectedElevationReversedLongDistance() throws ReflectiveOperationException { - final short[][] map = new short[][] { { 0, 1, 2, 3 }, { 4, 5, 6, 7 }, { 8, 9, 10, 11 }, + final short[][] map = { { 0, 1, 2, 3 }, { 4, 5, 6, 7 }, { 8, 9, 10, 11 }, { 12, 13, 14, 15 } }; final WaterWayCheck check = new WaterWayCheck(ConfigurationResolver.inlineConfiguration( "{\"WaterWayCheck\": {\"waterway.elevation.resolution.min.uphill\": 30000.0, \"waterway.elevation.distance.min.start.end\": 1.0}}"), @@ -118,6 +153,23 @@ public void testCoastWaterwayConnectedElevationReversedLongDistance() .get(check); elevationUtils.putMap(Location.forString("16.9906416,-88.3188021"), map); this.verifier.actual(this.atlases.getCoastlineWaterwayConnected(), check); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals("The waterway 130 probably does not go up hill.\n" + + "Please check (source elevation data resolution was about 26,585.386 meters).", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertEquals(1, flag.getFixSuggestions().size())); + this.verifier.verify(flag -> + { + final FeatureChange suggestion = flag.getFixSuggestions().iterator().next(); + final Line after = (Line) suggestion.getAfterView(); + final Line before = (Line) suggestion.getBeforeView(); + assertNull(suggestion.getTags()); + final List afterLocations = Iterables.stream(after).collectToList(); + final List beforeLocations = Iterables.stream(before).collectToList(); + assertFalse(ListUtils.isEqualList(afterLocations, beforeLocations)); + Collections.reverse(beforeLocations); + assertTrue(ListUtils.isEqualList(afterLocations, beforeLocations)); + }); this.verifier.verifyExpectedSize(1); } @@ -146,6 +198,13 @@ public void testCoastWaterwayReversed() { this.verifier.actual(this.atlases.getCoastlineWaterwayReversed(), this.defaultWaterWayCheck); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals( + "The waterway 542 does not end in a sink (ocean/sinkhole/waterway/drain).\n" + + "The waterway crosses a coastline, which means it is possible for the coastline to have an incorrect direction.\n" + + "Land should be to the LEFT of the coastline and the ocean should be to the RIGHT of the coastline (for more information, see https://wiki.osm.org/Tag:natural=coastline).", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertTrue(flag.getFixSuggestions().isEmpty())); this.verifier.verifyExpectedSize(1); } @@ -156,6 +215,10 @@ public void testCoastWaterwayReversed() public void testCrossingWaterway() { this.verifier.actual(this.atlases.getCrossingWaterways(), this.defaultWaterWayCheck); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals("The waterway 0 crosses the waterway 0.", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertTrue(flag.getFixSuggestions().isEmpty())); this.verifier.verifyExpectedSize(1); } @@ -188,6 +251,11 @@ public void testCrossingWaterwayDifferentLayers() public void testDeadendWaterway() { this.verifier.actual(this.atlases.getDeadendWaterway(), this.defaultWaterWayCheck); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals( + "The waterway 538 does not end in a sink (ocean/sinkhole/waterway/drain).", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertTrue(flag.getFixSuggestions().isEmpty())); this.verifier.verifyExpectedSize(1); } @@ -198,6 +266,11 @@ public void testDeadendWaterway() public void testDeadendWaterways() { this.verifier.actual(this.atlases.getDeadendWaterways(), this.defaultWaterWayCheck); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals( + "The waterway 538 does not end in a sink (ocean/sinkhole/waterway/drain).", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertTrue(flag.getFixSuggestions().isEmpty())); this.verifier.verifyExpectedSize(1); } @@ -220,6 +293,11 @@ public void testDifferingLayersConnectedWaterway() public void testSinkholeAreaWaterway() { this.verifier.actual(this.atlases.getSinkholeArea(), this.defaultWaterWayCheck); + this.verifier.verify(flag -> assertEquals(1, flag.getRawInstructions().size())); + this.verifier.verify(flag -> assertEquals( + "The waterway 538 does not end in a sink (ocean/sinkhole/waterway/drain).", + flag.getRawInstructions().get(0))); + this.verifier.verify(flag -> assertTrue(flag.getFixSuggestions().isEmpty())); this.verifier.verifyExpectedSize(1); }