From e2057dfe58736fcc5f03cd26f0db26f195058842 Mon Sep 17 00:00:00 2001 From: kkonishi2 <37551535+kkonishi2@users.noreply.github.com> Date: Mon, 10 Feb 2020 11:18:50 -0800 Subject: [PATCH 01/15] bumping to 6.0.1 (#257) --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index a70107230..34f92ef3b 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ group=org.openstreetmap.atlas -version=6.0.0-SNAPSHOT +version=6.0.1-SNAPSHOT maven2_url=https://oss.sonatype.org/service/local/staging/deploy/maven2/ snapshot_url=https://oss.sonatype.org/content/repositories/snapshots/ From a2c1dae0caacc6c2db614c87d5b2743133da185a Mon Sep 17 00:00:00 2001 From: Sugandhi Maheshwaram Date: Wed, 12 Feb 2020 21:33:28 -0600 Subject: [PATCH 02/15] Inconsistent-road-classification bug fix (#252) * Inconsistent-road-classification, add validation of master edges on individual out edges and connected edges plus OsmWayWalker. * Inconsistent-road-classification, fixing formatting. Co-authored-by: Daniel B --- .../edges/InconsistentRoadClassificationCheck.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/InconsistentRoadClassificationCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/InconsistentRoadClassificationCheck.java index 74f516cff..3ae81a4b3 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/InconsistentRoadClassificationCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/InconsistentRoadClassificationCheck.java @@ -16,6 +16,7 @@ import org.openstreetmap.atlas.geography.Segment; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.geography.atlas.items.Edge; +import org.openstreetmap.atlas.geography.atlas.walker.OsmWayWalker; import org.openstreetmap.atlas.tags.HighwayTag; import org.openstreetmap.atlas.tags.JunctionTag; import org.openstreetmap.atlas.utilities.collections.Iterables; @@ -85,7 +86,8 @@ public InconsistentRoadClassificationCheck(final Configuration configuration) @Override public boolean validCheckForObject(final AtlasObject object) { - if (object instanceof Edge && !isFlagged(object.getOsmIdentifier())) + if (object instanceof Edge && !isFlagged(object.getOsmIdentifier()) + && ((Edge) object).isMasterEdge()) { final Edge edge = (Edge) object; @@ -112,7 +114,7 @@ protected Optional flag(final AtlasObject item) .findInconsistentEdges(edge); if (!inconsistentEdgeTuples.isEmpty()) { - final CheckFlag flag = createFlag(item, + final CheckFlag flag = createFlag(new OsmWayWalker((Edge) item).collectEdges(), this.getLocalizedInstruction(0, edge.getOsmIdentifier(), edge.highwayTag())); markAsFlagged(edge.getOsmIdentifier()); inconsistentEdgeTuples.forEach(inconsistentEdgeTuple -> @@ -230,7 +232,7 @@ private boolean areInTheSimilarDirection(final Edge edge, final Edge anotherEdge private Stream connectionsSimilarToReferenceEdge(final HighwayTag referenceHighwayType, final Edge edge) { - return edge.outEdges().stream() + return edge.outEdges().stream().filter(Edge::isMasterEdge) .filter(connectedEdge -> referenceHighwayType .isOfEqualClassification(connectedEdge.highwayTag()) && this.areInTheSimilarDirection(edge, connectedEdge)); @@ -249,6 +251,7 @@ private List>> findInconsistentEdges(final Edge referenceE // Split the outEdges into those that are inconsistent links and those that are not final Map> edgesAreProblematicLinks = referenceEdge.outEdges().stream() + .filter(Edge::isMasterEdge) .filter(this.allConnectedEdgesFilter(referenceEdge, referenceHighwayType)) .collect(Collectors.partitioningBy( edge -> this.isProblematicLink(edge, referenceHighwayType))); @@ -325,7 +328,7 @@ private Function>>> getSimilarEdgesTuple( */ private boolean isBypassed(final Edge inconsistency, final HighwayTag referenceHighwayTag) { - return inconsistency.start().outEdges().stream() + return inconsistency.start().outEdges().stream().filter(Edge::isMasterEdge) .anyMatch(edge -> !edge.equals(inconsistency) && edge.end().equals(inconsistency.end()) && edge.highwayTag().isIdenticalClassification(referenceHighwayTag)); @@ -340,7 +343,7 @@ private boolean isBypassed(final Edge inconsistency, final HighwayTag referenceH */ private boolean isContinuousOutgoingEdge(final Edge edge) { - return edge.outEdges().stream() + return edge.outEdges().stream().filter(Edge::isMasterEdge) .anyMatch(connectedEdge -> edge.highwayTag() .isOfEqualClassification(connectedEdge.highwayTag()) && this.areInTheSimilarDirection(edge, connectedEdge)); From 1127968f9e9086e188dfb46751ab3fd76d0a13c0 Mon Sep 17 00:00:00 2001 From: Sugandhi Maheshwaram Date: Mon, 17 Feb 2020 11:44:12 -0600 Subject: [PATCH 03/15] SnakeRoadCheck, trying to clean up commit count. (#243) Co-authored-by: Daniel B --- .../checks/validation/linear/edges/SnakeRoadCheck.java | 10 ++++++++-- .../validation/linear/edges/SnakeRoadNetworkWalk.java | 3 +-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadCheck.java index bad7cb009..36c98885c 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadCheck.java @@ -3,6 +3,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.Set; import org.openstreetmap.atlas.checks.atlas.predicates.TagPredicates; import org.openstreetmap.atlas.checks.atlas.predicates.TypePredicates; @@ -168,9 +169,14 @@ private void walkNetwork(final Edge current, final SnakeRoadNetworkWalk walk) // Process it walk.visitEdge(current, connection); + final Set oneLayerRemovedConnections = walk + .getConnectedMasterEdgeOfTheSameWay(connection); + oneLayerRemovedConnections.forEach( + onceRemovedConnection -> walk.checkIfEdgeHeadingDifferenceExceedsThreshold( + connection, onceRemovedConnection)); + // Add its neighbors to the next layer - walk.populateOneLayerRemovedConnections( - walk.getConnectedMasterEdgeOfTheSameWay(connection)); + walk.populateOneLayerRemovedConnections(oneLayerRemovedConnections); // If we've processed all directly connected edges, check the next layer of connections if (walk.getDirectConnections().isEmpty()) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadNetworkWalk.java b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadNetworkWalk.java index 4b8f915b2..82fb4bb60 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadNetworkWalk.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SnakeRoadNetworkWalk.java @@ -48,7 +48,7 @@ protected SnakeRoadNetworkWalk(final Edge edge, final Angle threshold) // We use a TreeSet here to order the edges by their Atlas identifier. This helps us easily // grab the first and last Edge making up the road and check connections for false // positives. - this.visitedEdges = new TreeSet((one, two) -> + this.visitedEdges = new TreeSet<>((one, two) -> { return Long.compare(one.getIdentifier(), two.getIdentifier()); }); @@ -186,7 +186,6 @@ protected void visitEdge(final Edge comingFrom, final Edge comingTo) this.visitedEdges.add(comingTo); this.setGreatestValence(comingTo.start()); this.setGreatestValence(comingTo.end()); - checkIfEdgeHeadingDifferenceExceedsThreshold(comingFrom, comingTo); } /** From 840b9b01d5ee7a9e625ee07baa9cc9ad43c96100 Mon Sep 17 00:00:00 2001 From: Daniel B Date: Wed, 19 Feb 2020 13:08:13 -0800 Subject: [PATCH 04/15] [MapRouletteUploadCommand] Fix custom challenge name bug (#260) * fix custom challenge name bug * More tests! * extra lines * fix tests * sonar * remove extra lines --- .../maproulette/MapRouletteUploadCommand.java | 37 ++++++---- .../MapRouletteUploadCommandTest.java | 71 ++++++++++++++++--- .../MapRouletteUploadCommandTestRule.java | 12 +++- .../checks/maproulette/checksConfig.json | 17 +++++ 4 files changed, 111 insertions(+), 26 deletions(-) create mode 100644 src/test/resources/org/openstreetmap/atlas/checks/maproulette/checksConfig.json diff --git a/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommand.java b/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommand.java index 411f64937..fb9a581d8 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommand.java +++ b/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommand.java @@ -73,6 +73,20 @@ public MapRouletteUploadCommand() this.checkNameChallengeMap = new HashMap<>(); } + /** + * Returns a comma separated string of country display names + * + * @param countryCode + * - iso3 country code string. Can contain more than one country i.e (USA,MEX) + * @return comma separated String of iso3 country codes + */ + public String getCountryDisplayName(final Optional countryCode) + { + return countryCode.isPresent() ? Arrays.stream(countryCode.get().split(",")) + .map(country -> IsoCountry.displayCountry(country).orElse(country)) + .collect(Collectors.joining(", ")) : ""; + } + @Override public SwitchList switches() { @@ -123,18 +137,8 @@ protected void execute(final CommandMap commandMap, { try { - final Challenge challenge = this - .getChallenge(task.getChallengeName(), instructions); - // Prepend the challenge name with the full country name if one - // exists, else country code if it exists. Then try to add the task - // for upload - countryCode.ifPresent(iso -> challenge.setName(String.join(" - ", - Arrays.stream(countryCode.get().split(",")) - .map(country -> IsoCountry.displayCountry(country) - .orElse(country)) - .collect(Collectors.toList()).toString() - .replace("[", "").replace("]", ""), - task.getChallengeName()))); + final Challenge challenge = this.getChallenge( + task.getChallengeName(), instructions, countryCode); this.addTask(challenge, task); } catch (URISyntaxException | UnsupportedEncodingException error) @@ -162,10 +166,12 @@ protected void execute(final CommandMap commandMap, * the name of the check * @param fallbackConfiguration * the full configuration, which contains challenge parameters for checkName. + * @param countryCode + * the CheckFlag iso3 country code * @return the check's challenge parameters, stored as a Challenge object. */ private Challenge getChallenge(final String checkName, - final Configuration fallbackConfiguration) + final Configuration fallbackConfiguration, final Optional countryCode) { return this.checkNameChallengeMap.computeIfAbsent(checkName, name -> { @@ -174,7 +180,10 @@ private Challenge getChallenge(final String checkName, final Gson gson = new GsonBuilder().disableHtmlEscaping() .registerTypeAdapter(Challenge.class, new ChallengeDeserializer()).create(); final Challenge result = gson.fromJson(gson.toJson(challengeMap), Challenge.class); - result.setName(result.getName().isEmpty() ? checkName : result.getName()); + // Prepend the challenge name with the full country name if one exists + final String challengeName = String.join(" - ", this.getCountryDisplayName(countryCode), + result.getName().isEmpty() ? checkName : result.getName()); + result.setName(challengeName); return result; }); } diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java index 52ef18aba..b79bb9ea2 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java @@ -1,7 +1,10 @@ package org.openstreetmap.atlas.checks.maproulette; import java.util.Collections; +import java.util.List; +import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.lang.ArrayUtils; import org.junit.AfterClass; @@ -57,7 +60,37 @@ public void testCountryFilter() public void testExecute() { final String[] additionalArguments = {}; - this.runAndTest(additionalArguments, 1, 2, 2); + this.runAndTest(additionalArguments, 1, 3, 3); + } + + @Test + public void testGetCountryDisplayName() + { + final Optional countries1 = Optional.of("CAN,MEX"); + final Optional countries2 = Optional.of("USA"); + final MapRouletteUploadCommand command = new MapRouletteUploadCommand(); + final String displayCountryNames1 = command.getCountryDisplayName(countries1); + final String displayCountryNames2 = command.getCountryDisplayName(countries2); + + Assert.assertEquals("Canada, Mexico", displayCountryNames1); + Assert.assertEquals("United States", displayCountryNames2); + } + + @Test + public void testGetCustomChallengeName() + { + final String[] additionalArguments = { String.format("-config=%s", + MapRouletteUploadCommandTest.class.getResource("checksConfig.json").getPath()) }; + final TestMapRouletteConnection connection = this.run(additionalArguments); + final Set projects = connection.uploadedProjects(); + final List challengeNames = projects.stream().flatMap(project -> connection + .challengesForProject(project).stream().map(Challenge::getName)) + .collect(Collectors.toList()); + + Collections.sort(challengeNames); + Assert.assertEquals("Canada - Spiky Buildings", challengeNames.get(0)); + Assert.assertEquals("Mexico, Belize - Intersecting Lines", challengeNames.get(1)); + Assert.assertEquals("United States - Address Point Match", challengeNames.get(2)); } @Before @@ -70,6 +103,8 @@ public void writeFiles() new SparkFileHelper(Collections.emptyMap()), FOLDER.child("unzipped.log").toString()).withCompression(false); unzippedProcessor.process(this.setup.getOneBasicFlag()); + unzippedProcessor.process(this.setup.getTwoCountryFlag()); + unzippedProcessor.process(this.setup.getAnotherBasicFlag()); unzippedProcessor.process(new ShutdownEvent()); // Create a zipped file @@ -83,6 +118,30 @@ public void writeFiles() } } + /** + * Similar to runAndTest, however this function will return a {@link TestMapRouletteConnection} + * + * @param additionalArguments + * String[] of extra arguments for {@link MapRouletteUploadCommand}, to add to the + * data i/o locations and server config + * @return TestMapRouletteConnection + */ + private TestMapRouletteConnection run(final String[] additionalArguments) + { + // Set up some arguments + final MapRouletteCommand command = new MapRouletteUploadCommand(); + final String[] arguments = { String.format("-logfiles=%s", FOLDER.getPath()), + MAPROULETTE_CONFIG }; + final CommandMap map = command + .getCommandMap((String[]) ArrayUtils.addAll(arguments, additionalArguments)); + final TestMapRouletteConnection connection = new TestMapRouletteConnection(); + + // Run the command + command.onRun(map, configuration -> new MapRouletteClient(configuration, connection)); + + return connection; + } + /** * Runs {@link MapRouletteUploadCommand} using a {@link TestMapRouletteConnection}. Tests that * the correct number of projects, challenges, and tasks are created. @@ -101,15 +160,7 @@ private void runAndTest(final String[] additionalArguments, final int expectedPr final int expectedChallenges, final int expectedTasks) { // Set up some arguments - final MapRouletteCommand command = new MapRouletteUploadCommand(); - final String[] arguments = { String.format("-logfiles=%s", FOLDER.getPath()), - MAPROULETTE_CONFIG }; - final CommandMap map = command - .getCommandMap((String[]) ArrayUtils.addAll(arguments, additionalArguments)); - final TestMapRouletteConnection connection = new TestMapRouletteConnection(); - - // Run the command - command.onRun(map, configuration -> new MapRouletteClient(configuration, connection)); + final TestMapRouletteConnection connection = this.run(additionalArguments); // Test the results final Set projects = connection.uploadedProjects(); diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTestRule.java index eb34bfd2c..9c51aa9b1 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTestRule.java +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTestRule.java @@ -20,14 +20,17 @@ public class MapRouletteUploadCommandTestRule extends CoreTestRule private static final String CENTER = "0,0"; private static final String IDENTIFIER_1 = "1"; private static final String IDENTIFIER_2 = "2"; + private static final String IDENTIFIER_3 = "2"; private static final String CHALLENGE_1 = "SomeCheck"; private static final String CHALLENGE_2 = "SomeOtherCheck"; + private static final String CHALLENGE_3 = "AnotherCheck"; private static final String INSTRUCTIONS = "Instructions."; @TestAtlas(points = { @Point(coordinates = @Loc(value = CENTER), id = "1", tags = { "iso_country_code=USA" }), - @Point(coordinates = @Loc(value = CENTER), id = "2", tags = { - "iso_country_code=CAN" }) }) + @Point(coordinates = @Loc(value = CENTER), id = "2", tags = { "iso_country_code=CAN" }), + @Point(coordinates = @Loc(value = CENTER), id = "3", tags = { + "iso_country_code=MEX,BLZ" }) }) private Atlas basicAtlas; public CheckFlagEvent getAnotherBasicFlag() @@ -40,6 +43,11 @@ public CheckFlagEvent getOneBasicFlag() return this.getBasicFlag(IDENTIFIER_1, this.basicAtlas.point(1L), CHALLENGE_1); } + public CheckFlagEvent getTwoCountryFlag() + { + return this.getBasicFlag(IDENTIFIER_3, this.basicAtlas.point(3L), CHALLENGE_3); + } + private CheckFlagEvent getBasicFlag(final String identifier, final AtlasObject object, final String challenge) { diff --git a/src/test/resources/org/openstreetmap/atlas/checks/maproulette/checksConfig.json b/src/test/resources/org/openstreetmap/atlas/checks/maproulette/checksConfig.json new file mode 100644 index 000000000..4a66a77d5 --- /dev/null +++ b/src/test/resources/org/openstreetmap/atlas/checks/maproulette/checksConfig.json @@ -0,0 +1,17 @@ +{ + "SomeCheck": { + "challenge": { + "name": "Address Point Match" + } + }, + "SomeOtherCheck": { + "challenge": { + "name": "Spiky Buildings" + } + }, + "AnotherCheck": { + "challenge": { + "name": "Intersecting Lines" + } + } +} From cb1c7d138cb6012a70792eabe2855c726ffebbbe Mon Sep 17 00:00:00 2001 From: seancoulter <31253489+seancoulter@users.noreply.github.com> Date: Wed, 19 Feb 2020 14:19:00 -0800 Subject: [PATCH 05/15] [SinkIslandCheck] sink islands surrounded by pedestrian ways (#227) * sinkIsland updates * add unit tests; cleanup * add motor_vehicle=yes criteria * spotlessApply * add stream filter to outEdges * SinkIsland nondeterminism allay attempt * Update configuration.json Co-authored-by: Daniel B --- config/configuration.json | 1 + .../linear/edges/SinkIslandCheck.java | 79 ++++++++++++------- .../linear/edges/SinkIslandCheckTest.java | 27 +++++++ .../linear/edges/SinkIslandCheckTestRule.java | 27 +++++++ 4 files changed, 105 insertions(+), 29 deletions(-) diff --git a/config/configuration.json b/config/configuration.json index 647dc874b..2186190da 100644 --- a/config/configuration.json +++ b/config/configuration.json @@ -799,6 +799,7 @@ "SinkIslandCheck": { "tree.size": 50, "minimum.highway.type": "service", + "filter.pedestrian.network": false, "challenge": { "description": "Tasks that identify islands of roads where it is impossible to get out. The simplest is a one-way that dead-ends; that would be a one-edge island.", "blurb": "Identify islands of roads.", diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheck.java index 58d974f8e..415e75262 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheck.java @@ -39,6 +39,7 @@ * @author savannahostrowski * @author nachtm * @author sayas01 + * @author seancoulter */ public class SinkIslandCheck extends BaseCheck { @@ -54,11 +55,16 @@ public class SinkIslandCheck extends BaseCheck || Validators.isOfType(object, VehicleTag.class, VehicleTag.YES); private static final Predicate SERVICE_ROAD = object -> Validators.isOfType(object, HighwayTag.class, HighwayTag.SERVICE); + private static final Predicate IS_AT_LEAST_SERVICE_ROAD = object -> ((Edge) object) + .highwayTag().isMoreImportantThanOrEqualTo(HighwayTag.SERVICE); private static final long TREE_SIZE_DEFAULT = 50; + private static final boolean DEFAULT_SERVICE_IN_PEDESTRIAN_FILTER = false; private static final long serialVersionUID = -1432150496331502258L; private final HighwayTag minimumHighwayType; private final int storeSize; private final int treeSize; + // This can be turned on if we want to flag service roads surrounded by pedestrian networks. + private final boolean serviceInPedestrianNetworkFilter; /** * Default constructor @@ -79,6 +85,8 @@ public SinkIslandCheck(final Configuration configuration) // this.treeSize // Therefore underlying map/queue we will never re-double the capacity this.storeSize = (int) (this.treeSize / LOAD_FACTOR); + this.serviceInPedestrianNetworkFilter = configurationValue(configuration, + "filter.pedestrian.network", DEFAULT_SERVICE_IN_PEDESTRIAN_FILTER); } @Override @@ -96,14 +104,14 @@ public boolean validCheckForObject(final AtlasObject object) protected Optional flag(final AtlasObject object) { // Flag to keep track of whether we found an issue or not - boolean emptyFlag = false; + boolean haltedSearch = false; // The current edge to be explored Edge candidate = (Edge) object; // A set of all edges that we have already explored final Set explored = new HashSet<>(this.storeSize, LOAD_FACTOR); - // A set of all edges that we explore that have no outgoing edges + // A set of all sink edges final Set terminal = new HashSet<>(); // Current queue of candidates that we can draw from final Queue candidates = new ArrayDeque<>(this.storeSize); @@ -118,7 +126,8 @@ protected Optional flag(final AtlasObject object) // flag it. if (this.edgeCharacteristicsToIgnore(candidate)) { - emptyFlag = true; + haltedSearch = true; + explored.add(candidate); break; } @@ -126,28 +135,39 @@ protected Optional flag(final AtlasObject object) final Set outEdges = candidate.outEdges().stream().filter(this::validEdge) .collect(Collectors.toSet()); + // Validate highway=pedestrian edges connected to candidate if candidate is + // motor_vehicle=yes (add to outEdges) + if (candidate.getTag(MotorVehicleTag.KEY).orElse(MotorVehicleTag.NO.name()) + .equals(MotorVehicleTag.YES.name())) + { + outEdges.addAll(candidate.outEdges().stream() + .filter(HighwayTag::isPedestrianNavigableHighway) + .collect(Collectors.toSet())); + } + if (outEdges.isEmpty()) { // Sink edge. Don't mark the edge explored until we know how big the tree is terminal.add(candidate); } + else { // Add the current candidate to the set of already explored edges explored.add(candidate); - // From the list of outgoing edges from the current candidate filter out any edges - // that have already been explored and add all the rest to the queue of possible - // candidates - outEdges.stream().filter(outEdge -> !explored.contains(outEdge)) - .forEach(candidates::add); + // From the list of outgoing edges from the current candidate filter out any + // highway=pedestrian edges that were picked up and filter out any edges that have + // already been explored and add all the rest to the queue of possible candidates + outEdges.stream().filter(this::validEdge) + .filter(outEdge -> !explored.contains(outEdge)).forEach(candidates::add); // If the number of available candidates and the size of the currently explored // items is larger then the configurable tree size, then we can break out of the // loop and assume that this is not a SinkIsland if (candidates.size() + explored.size() > this.treeSize) { - emptyFlag = true; + haltedSearch = true; break; } } @@ -156,25 +176,22 @@ protected Optional flag(final AtlasObject object) candidate = candidates.poll(); } - // If we exit due to tree size (emptyFlag == true) and there are terminal edges we could - // cache them and check on entry to this method. However it seems to happen too rare in - // practice. So these edges (if any) will be processed as all others. Even though they would - // not generate any candidates. Otherwise if we covered the whole tree, there is no need to - // delay processing of terminal edges. We should add them to the geometry we are going to - // flag. - if (!emptyFlag) + // Unify all explored edges and mark them so we don't process them more than once + explored.addAll(terminal); + explored.forEach(marked -> this.markAsFlagged(marked.getIdentifier())); + + if (!haltedSearch) { // Include all touched edges - explored.addAll(terminal); + return Optional.of(createFlag(explored, this.getLocalizedInstruction(0))); } - - // Set every explored edge as flagged for any other processes to know that we have already - // process all those edges - explored.forEach(marked -> this.markAsFlagged(marked.getIdentifier())); - - // Create the flag if and only if the empty flag value is not set to false - return emptyFlag ? Optional.empty() - : Optional.of(createFlag(explored, this.getLocalizedInstruction(0))); + else if (!terminal.isEmpty()) + { + // Include only edges explicitly marked as sink islands during processing + return Optional.of(createFlag(terminal, this.getLocalizedInstruction(0))); + } + // No encountered sink edges, and a stop criteria was met. + return Optional.empty(); } @Override @@ -203,10 +220,14 @@ private boolean edgeCharacteristicsToIgnore(final Edge edge) // of creating a false positive due to the sectioning of the way || SyntheticBoundaryNodeTag.isBoundaryNode(edge.end()) || SyntheticBoundaryNodeTag.isBoundaryNode(edge.start()) - // Ignore edges that are of type service and is connected to pedestrian navigable - // ways or that ends in a building or is within an airport polygon - || SERVICE_ROAD.test(edge) && (this.isConnectedToPedestrianNavigableHighway(edge) - || this.intersectsAirportOrBuilding(edge)); + // If the serviceInPedestrianNetworkFilter switch is off, ignore edges that are of + // type at least service and are surrounded by pedestrian navigable ways. To flag + // such edges, the filter must be on and it's implied that the edge must not have + // the motor_vehicle tag. + || !this.serviceInPedestrianNetworkFilter && IS_AT_LEAST_SERVICE_ROAD.test(edge) + && this.isConnectedToPedestrianNavigableHighway(edge) + // Ignore service edges that end in a building or are within an airport polygon + || SERVICE_ROAD.test(edge) && this.intersectsAirportOrBuilding(edge); } /** diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTest.java index d32b00c01..3722a235c 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTest.java @@ -13,6 +13,7 @@ * @author gpogulsky * @author nachtm * @author sayas01 + * @author seancoulter */ public class SinkIslandCheckTest { @@ -80,6 +81,32 @@ public void testParkingGarageEntranceOrExit() this.verifier.verifyEmpty(); } + @Test + public void testPedestrianRoadAndMotorVehicleYesRoad() + { + this.verifier.actual(this.setup.getPedestrianRoadAndMotorVehicleYesRoad(), + new SinkIslandCheck(ConfigurationResolver + .inlineConfiguration("{\"SinkIslandCheck.tree.size\": 3}"))); + this.verifier.verifyEmpty(); + } + + @Test + public void testPsvAndMotorVehicleNoRoad() + { + this.verifier.actual(this.setup.getPsvAndMotorVehicleNoRoad(), new SinkIslandCheck( + ConfigurationResolver.inlineConfiguration("{\"SinkIslandCheck.tree.size\": 3}"))); + this.verifier.verifyEmpty(); + } + + @Test + public void testServiceInPedestrianNetworkFilterOn() + { + this.verifier.actual(this.setup.getEdgeConnectedToPedestrianNetwork(), + new SinkIslandCheck(ConfigurationResolver.inlineConfiguration( + "{\"SinkIslandCheck.filter.pedestrian.network\":true}"))); + this.verifier.verifyExpectedSize(1); + } + @Test public void testServiceSinkIsland() { diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTestRule.java index 80ea19db8..760a457f1 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTestRule.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/SinkIslandCheckTestRule.java @@ -14,6 +14,7 @@ * @author gpogulsky * @author nachtm * @author sayas01 + * @author seancoulter */ public class SinkIslandCheckTestRule extends CoreTestRule { @@ -190,6 +191,22 @@ public class SinkIslandCheckTestRule extends CoreTestRule @Loc(value = TEST_14) }, tags = { "highway=service" }) }) private Atlas edgesWithinAirportAtlas; + @TestAtlas(nodes = { @Node(coordinates = @Loc(value = TEST_1)), + @Node(coordinates = @Loc(value = TEST_2)), + @Node(coordinates = @Loc(value = TEST_3)) }, edges = { + @Edge(coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_2) }, tags = { + "highway=secondary", "psv=bus", "motor_vehicle=no" }) }) + private Atlas psvAndMotorVehicleNoRoad; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(value = TEST_1)), + @Node(coordinates = @Loc(value = TEST_2)), + @Node(coordinates = @Loc(value = TEST_3)) }, edges = { + @Edge(coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_2) }, tags = { + "highway=pedestrian" }), + @Edge(coordinates = { @Loc(value = TEST_1), @Loc(value = TEST_2) }, tags = { + "highway=service", "motor_vehicle=yes" }) }) + private Atlas pedestrianRoadAndMotorVehicleYesRoad; + public Atlas getEdgeConnectedToPedestrianNetwork() { return this.pedestrianNetwork; @@ -225,6 +242,16 @@ public Atlas getParkingGarageEntranceOrExit() return this.parkingGarageEntranceOrExitAtlas; } + public Atlas getPedestrianRoadAndMotorVehicleYesRoad() + { + return this.pedestrianRoadAndMotorVehicleYesRoad; + } + + public Atlas getPsvAndMotorVehicleNoRoad() + { + return this.psvAndMotorVehicleNoRoad; + } + public Atlas getServiceSinkIsland() { return this.serviceSinkIsland; From 2fc5086545bebbe55500c9f79ab363c74915734a Mon Sep 17 00:00:00 2001 From: Sayana Saithu <42149840+sayas01@users.noreply.github.com> Date: Wed, 19 Feb 2020 14:54:19 -0800 Subject: [PATCH 06/15] Add new AtGradeSignPostCheck (#258) * Add check * Handle roundabouts * spotless and checkstyle * Update roundabout req * Use edge direction * Spotless * Update java doc and spotless * Add doc * Fix typo * Fix checkstyle errors * Clean up * Address Pr comments * Rename method for clarity * Spotlessapply * Fix checkstyle and update doc * Add challenge config * Typo * Rename variable * SpotlessApply * nit * Rename variable * Fix checkstyle issues * remove enabled config key Co-authored-by: Daniel B --- config/configuration.json | 14 + docs/available_checks.md | 1 + docs/checks/atGradeSignPostCheck.md | 31 + .../intersections/AtGradeSignPostCheck.java | 637 ++++++++++++++++++ .../AtGradeSignPostCheckTest.java | 109 +++ .../AtGradeSignPostCheckTestRule.java | 215 ++++++ 6 files changed, 1007 insertions(+) create mode 100644 docs/checks/atGradeSignPostCheck.md create mode 100644 src/main/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheck.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTestRule.java diff --git a/config/configuration.json b/config/configuration.json index 2186190da..1d05adabd 100644 --- a/config/configuration.json +++ b/config/configuration.json @@ -941,5 +941,19 @@ "difficulty": "MEDIUM", "defaultPriority": "MEDIUM" } + }, + "AtGradeSignPostCheck": { + "connected.highway.types": { + "primary": ["trunk","primary","secondary"], + "trunk": ["primary"], + "secondary": ["primary"] + }, + "challenge":{ + "description":"This tasks contains at-grade intersections that are not part of destination_sign relations.", + "blurb":"Add/Fix destination_sign relations to at-grade intersections", + "instruction":"Open your favorite editor and check the instruction fr the task and add a destination_sign relation or destination_sign tag to the at-grade intersection.", + "difficulty":"NORMAL", + "defaultPriority":"MEDIUM" + } } } diff --git a/docs/available_checks.md b/docs/available_checks.md index b964ba6b9..d0a784ec6 100644 --- a/docs/available_checks.md +++ b/docs/available_checks.md @@ -27,6 +27,7 @@ This document is a list of tables with a description and link to documentation f | [SinkIslandCheck](tutorials/tutorial3-SinkIslandCheck.md) | The purpose of this check is to identify whether a network of car-navigable Edges can be exited. | | [SnakeRoadCheck](checks/snakeRoadCheck.md) | The purpose of the SnakeRoad check is to identify roads that should be split into two or more roads. | | [InvalidPiersCheck](checks/invalidPiersCheck.md) | The purpose of this check is to identify piers(OSM Ways with man_made=pier tag) that are ingested in Atlas as edges with linear or polygonal geometry without an area=yes tag | +| [AtGradeSignPostCheck](checks/atGradeSignPostCheck.md) | The purpose of this check is to identify at-grade intersections that are not part of destination sign relations. | ## Nodes | Check Name | Check Description | diff --git a/docs/checks/atGradeSignPostCheck.md b/docs/checks/atGradeSignPostCheck.md new file mode 100644 index 000000000..0f424b632 --- /dev/null +++ b/docs/checks/atGradeSignPostCheck.md @@ -0,0 +1,31 @@ +# AtGradeSignPostCheck + +#### Description + +This check identifies at-grade intersections that are not modeled into destination_sign relations. +An at-grade intersection is an intersection with two or more edges at the same level with valid +highway classifications. The highway classifications of the in and out edges of an at-grade intersection are specified in the configuration file and are as follows: +1. If in edge is of type primary, the intersection is valid if there are at least two out edges, at the same z level as the in edge, that are either trunk, primary or secondary. +2. If in edge is of type trunk or secondary, the intersection is valid if there are at least two out edges, at the same z level as the in edge, that are primary. + +#### Live Examples + +1. Node [id:393673917](https://www.openstreetmap.org/node/393673917) forms an at-grade intersection +with ways [393673917](https://www.openstreetmap.org/way/202447272), +[34370252](https://www.openstreetmap.org/way/34370252) and +[41234996](https://www.openstreetmap.org/way/41234996) but is not a member of a "destination_sign" relation. +2. Node [id:5351792253](https://www.openstreetmap.org/node/5351792253) forms an at-grade intersection with its exit road +[554569602](https://www.openstreetmap.org/way/554569602) but is not a member of a "destination_sign" relation. + +#### Code Review + +The check ensures that the Atlas object being evaluated is a [Node](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Node.java) +with a minimum node valence of 3. The node is a valid candidate for the check if it forms an intersection with edges at the same z-level with highway classification specified in the configurable. +Once the node is evaluated to be a valid at-grade intersection, check if it is a member of any destination_sign relations. +If the node is not part of a destination_sign relation, then flag the node along with the intersecting edges. Since a node can be part of multiple destination_sign +relations, check if all the valid intersecting edges are members of destination_sign relations. If there are edges missing destination_sign +relations, flag the node and the edges. If all the intersecting edges are members of relations, check if all the relations have destination tags. +If the relations are missing destination_sign tag, flag it. If the node is part of a roundabout, flag all the roundabout edges when flagging the node and its intersecting edges. + +To learn more about the code, please look at the comments in the source code for the check. +[AtGradeSignPostCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheck.java) \ No newline at end of file diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheck.java new file mode 100644 index 000000000..aa7ec2886 --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheck.java @@ -0,0 +1,637 @@ +package org.openstreetmap.atlas.checks.validation.intersections; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.openstreetmap.atlas.checks.base.BaseCheck; +import org.openstreetmap.atlas.checks.flag.CheckFlag; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; +import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; +import org.openstreetmap.atlas.geography.atlas.items.Edge; +import org.openstreetmap.atlas.geography.atlas.items.Node; +import org.openstreetmap.atlas.geography.atlas.items.Relation; +import org.openstreetmap.atlas.geography.atlas.items.RelationMember; +import org.openstreetmap.atlas.geography.atlas.items.RelationMemberList; +import org.openstreetmap.atlas.geography.atlas.walker.SimpleEdgeWalker; +import org.openstreetmap.atlas.tags.DestinationForwardTag; +import org.openstreetmap.atlas.tags.DestinationTag; +import org.openstreetmap.atlas.tags.HighwayTag; +import org.openstreetmap.atlas.tags.JunctionTag; +import org.openstreetmap.atlas.tags.LayerTag; +import org.openstreetmap.atlas.tags.LevelTag; +import org.openstreetmap.atlas.tags.OneWayTag; +import org.openstreetmap.atlas.tags.RelationTypeTag; +import org.openstreetmap.atlas.utilities.collections.Iterables; +import org.openstreetmap.atlas.utilities.collections.StringList; +import org.openstreetmap.atlas.utilities.configuration.Configuration; +import org.openstreetmap.atlas.utilities.direction.EdgeDirectionComparator; +import org.openstreetmap.atlas.utilities.scalars.Angle; + +import com.google.common.collect.ImmutableMap; + +/** + * Check flags at-grade intersections with missing destination_sign tags or destinations_sign + * relations. An at-grade intersection is an intersection between two edges at the same z-level. In + * case of roundabout edges, intersection between exit edge and roundabout edge are checked for + * either destination_sign relation or destination:forward tag, in case of bi-directional exit edge, + * or destination tag, in case of unidirectional exit edge. + * + * @author sayas01 + */ +public class AtGradeSignPostCheck extends BaseCheck +{ + /** + * A class for holding flagged intersection items and corresponding flag instruction index + */ + private class FlaggedIntersection + { + private int instructionIndex; + private Set setOfFlaggedItems; + + FlaggedIntersection(final int instructionIndex, final Set setOfFlaggedItems) + { + this.instructionIndex = instructionIndex; + this.setOfFlaggedItems = setOfFlaggedItems; + } + + private Set getFlaggedItems() + { + return this.setOfFlaggedItems; + } + + private Integer getInstructionIndex() + { + return this.instructionIndex; + } + } + + private static final long serialVersionUID = -7428641176420422187L; + // Primary road (inEdge) connected to trunk, primary, secondary roads (outEdges) are treated as + // valid intersection + private static final List CONNECTIONS_TO_PRIMARY = Arrays.asList("trunk", "primary", + "secondary"); + // Trunk road (inEdge) connected to primary road (outEdge) is treated as valid intersection + private static final List CONNECTIONS_TO_TRUNK = Collections.singletonList("primary"); + // Secondary road (inEdge) connected to primary road (outEdge) is treated as valid intersection + private static final List CONNECTIONS_TO_SECONDARY = Collections + .singletonList("primary"); + private static final ImmutableMap> CONNECTED_HIGHWAY_TYPES_MAP = ImmutableMap + .of("primary", CONNECTIONS_TO_PRIMARY, "trunk", CONNECTIONS_TO_TRUNK, "secondary", + CONNECTIONS_TO_SECONDARY); + private static final String NO_DESTINATION_SIGN_RELATION_INSTRUCTION = "Node {0,number,#} forms an at-grade junction but is not part of " + + "a destination sign relation. Verify and create a destination sign relation with the node as \"intersection\" member and following " + + "connected edges {1}, " + "as \"to\" and \"from\" members."; + private static final String DESTINATION_SIGN_RELATION_MISSING_DESTINATION_TAG_INSTRUCTION = "Node {0,number,#} form an at-grade junction. It is part of destination sign relation(s): " + + "{1}" + "but the relation(s) are missing \"destination\" tags."; + private static final String INCOMPLETE_DESTINATION_RELATION_INSTRUCTION = "Node {0,number,#} forms an " + + "at-grade junction and is part of destination sign relation(s). But the following connected edges {1} " + + "could also form destination sign relations with this node. Create new destination sign relation " + + "with these edges and the node."; + private static final String ROUNDABOUT_EDGE_MISSING_DESTINATION_SIGN_RELATION = "Node {0,number,#} is part of a roundabout and forms an " + + "at-grade junction with connected edges. Add destination sign relations with the node as \"intersection\" member and following " + + "connected edges {1}, as \"to\" and \"from\" members and add destination sign tag to the connected edges."; + private static final String ROUNDABOUT_EDGE_INCOMPLETE_DESTINATION_SIGN_RELATION = "Node {0,number,#} is part of a roundabout and forms an " + + "at-grade junction. It is part of destination sign relation(s). Either the existing relations are missing destination sign tag or following connected edges {1} " + + "could also form destination sign relations with this node. Either add destination tags to existing relations or create new destination sign relation " + + "with these edges and the node."; + private static final String NON_ROUNDABOUT_INTERSECTION_MAP = "nonRoundaboutMatchingEdgesMap"; + private static final String ROUNDABOUT_INTERSECTION_MAP = "roundaboutMatchingEdgesMap"; + + private static final int INSTRUCTION_INDEX_ZERO = 0; + private static final int INSTRUCTION_INDEX_ONE = 1; + private static final int INSTRUCTION_INDEX_TWO = 2; + private static final int INSTRUCTION_INDEX_THREE = 3; + private static final int INSTRUCTION_INDEX_FOUR = 4; + + private static final int MINIMUM_NODE_VALENCE = 3; + private static final Angle DEFAULT_OPPOSITE_DIRECTION_LOWER_LIMIT = Angle.degrees(171); + private static final Angle DEFAULT_OPPOSITE_DIRECTION_UPPER_LIMIT = Angle.degrees(-171); + private static final Angle DEFAULT_SAME_DIRECTION_LOWER_LIMIT = Angle.degrees(-100); + private static final Angle DEFAULT_SAME_DIRECTION_UPPER_LIMIT = Angle.degrees(9); + private static final EdgeDirectionComparator EDGE_DIRECTION_COMPARATOR = new EdgeDirectionComparator( + DEFAULT_OPPOSITE_DIRECTION_LOWER_LIMIT, DEFAULT_OPPOSITE_DIRECTION_UPPER_LIMIT, + DEFAULT_SAME_DIRECTION_LOWER_LIMIT, DEFAULT_SAME_DIRECTION_UPPER_LIMIT); + + private static final List FALLBACK_INSTRUCTIONS = Arrays.asList( + NO_DESTINATION_SIGN_RELATION_INSTRUCTION, + DESTINATION_SIGN_RELATION_MISSING_DESTINATION_TAG_INSTRUCTION, + INCOMPLETE_DESTINATION_RELATION_INSTRUCTION, + ROUNDABOUT_EDGE_MISSING_DESTINATION_SIGN_RELATION, + ROUNDABOUT_EDGE_INCOMPLETE_DESTINATION_SIGN_RELATION); + + private final Set highwayFilter; + private Map> connectedHighwayTypes; + + /** + * The default constructor that must be supplied. The Atlas Checks framework will generate the + * checks with this constructor, supplying a configuration that can be used to adjust any + * parameters that the check uses during operation. + * + * @param configuration + * the JSON configuration for this check + */ + public AtGradeSignPostCheck(final Configuration configuration) + { + super(configuration); + this.connectedHighwayTypes = this.configurationValue(configuration, + "connected.highway.types", CONNECTED_HIGHWAY_TYPES_MAP); + this.highwayFilter = new HashSet<>(this.connectedHighwayTypes.keySet()); + } + + /** + * This function will validate if the supplied atlas object is valid for the check. + * + * @param object + * the atlas object supplied by the Atlas-Checks framework for evaluation + * @return {@code true} if this object should be checked + */ + @Override + public boolean validCheckForObject(final AtlasObject object) + { + return object instanceof Node && ((Node) object).valence() >= MINIMUM_NODE_VALENCE + && !this.isFlagged(String.valueOf(object.getIdentifier())); + } + + /** + * This is the actual function that will check to see whether the object needs to be flagged. + * + * @param object + * the atlas object supplied by the Atlas-Checks framework for evaluation + * @return an optional {@link CheckFlag} object that + */ + @Override + protected Optional flag(final AtlasObject object) + { + final Node intersectingNode = (Node) object; + // Filter and sort all in edges that have valid highway types + final List inEdges = intersectingNode.inEdges().stream() + .filter(this::isValidIntersectingEdge) + .sorted(Comparator.comparingLong(AtlasObject::getIdentifier)) + .collect(Collectors.toList()); + // Filter all out edges that have valid highway types + final Set outEdges = intersectingNode.outEdges().stream() + .filter(this::isValidIntersectingEdge).collect(Collectors.toSet()); + // Terminate if there isn't at least one inEdge or two out edges with valid highway types + if (inEdges.isEmpty() || outEdges.size() < 2) + { + return Optional.empty(); + } + // For each inEdge, get the list of potentially matching out edges + // Matching out edges are based on z level and highway type. + // For each inEdge, store the inEdge and corresponding outEdges in + // nonRoundaboutInEdgeToOutEdgeMap + // If any of the out edge is a roundabout edge, store the roundabout edges and the inEdge in + // roundAboutInEdgeToOutEdgeMap + final Map>> mapOfMatchingInAndOutEdges = this + .populateInEdgeToOutEdgeMaps(inEdges, outEdges); + final Map> nonRoundaboutInEdgeToOutEdgeMap = mapOfMatchingInAndOutEdges + .get(NON_ROUNDABOUT_INTERSECTION_MAP); + final Map> roundAboutInEdgeToOutEdgeMap = mapOfMatchingInAndOutEdges + .get(ROUNDABOUT_INTERSECTION_MAP); + // If there are no valid intersection, return Optional.empty() + if ((nonRoundaboutInEdgeToOutEdgeMap == null || nonRoundaboutInEdgeToOutEdgeMap.isEmpty()) + && (roundAboutInEdgeToOutEdgeMap == null || roundAboutInEdgeToOutEdgeMap.isEmpty())) + { + return Optional.empty(); + } + // Collect all destination sign relations, the node is a member of + final Optional> destinationSignRelations = this + .getParentDestinationSignRelations(intersectingNode); + final FlaggedIntersection flaggedIntersection = destinationSignRelations.isEmpty() + ? this.getFlaggedIntersection(roundAboutInEdgeToOutEdgeMap, + nonRoundaboutInEdgeToOutEdgeMap) + : this.getIntersectionsWithIncompleteDestinationSignRelation( + roundAboutInEdgeToOutEdgeMap, nonRoundaboutInEdgeToOutEdgeMap, + intersectingNode, destinationSignRelations.get()); + final int instructionIndex = flaggedIntersection.getInstructionIndex(); + if (instructionIndex == -1) + { + return Optional.empty(); + } + final Set entitiesToBeFlagged = flaggedIntersection.getFlaggedItems(); + final List identifiers = this.getIdentifiers(entitiesToBeFlagged); + entitiesToBeFlagged.add(intersectingNode); + this.markAsFlagged(String.valueOf(intersectingNode.getIdentifier())); + return Optional.of( + this.createFlag(entitiesToBeFlagged, this.getLocalizedInstruction(instructionIndex, + intersectingNode.getIdentifier(), new StringList(identifiers).join(", ")))); + } + + @Override + protected List getFallbackInstructions() + { + return FALLBACK_INSTRUCTIONS; + } + + /** + * Collects all edges that are not part of a relation + * + * @param fromEdge + * inEdge + * @param toEdges + * set of out edges + * @param destinationSignRelations + * set of existing destination sign relations + * @return Optional> if there are edges that are not part of destination sign + * relations + */ + private Optional> connectedEdgesNotPartOfRelation(final AtlasEntity fromEdge, + final Set toEdges, final Set destinationSignRelations) + { + final Set outEdges = new HashSet<>(toEdges); + final Set filteredOutEdges = outEdges.stream() + .filter(outEdge -> this.isMissingDestinationTag((Edge) outEdge)) + .collect(Collectors.toSet()); + final Set allRelationMembers = destinationSignRelations.stream() + .flatMap(destinationSignRelation -> + { + final RelationMemberList relationMembers = destinationSignRelation + .allKnownOsmMembers(); + return relationMembers.stream().map(RelationMember::getEntity); + }).collect(Collectors.toSet()); + if (!allRelationMembers.contains(fromEdge)) + { + filteredOutEdges.add(fromEdge); + return Optional.of(filteredOutEdges); + } + allRelationMembers.forEach(filteredOutEdges::remove); + if (!filteredOutEdges.isEmpty()) + { + filteredOutEdges.add(fromEdge); + return Optional.of(filteredOutEdges); + } + return Optional.empty(); + } + + /** + * Collect all roundabout edges and exit edges that are missing destination sign relations or + * missing destination sign tags + * + * @param roundAboutInEdgeToOutEdgeMap + * Map> where key is inEdge and value is set of + * outEdges of the rounabout + * @param destinationSignRelations + * set of existing destoination sign relations + * @return set of edges that meet the above criteria + */ + private Set getAllRoundaboutEdgesMissingTagsOrRelations( + final Map> roundAboutInEdgeToOutEdgeMap, + final Set destinationSignRelations) + { + final Set entitiesToBeFlagged = new HashSet<>(); + roundAboutInEdgeToOutEdgeMap.forEach((inEdge, setOfOutEdge) -> + { + final Optional roundaboutEdge = setOfOutEdge.stream() + .filter(JunctionTag::isRoundabout).findFirst(); + final Optional roundaboutExitEdge = setOfOutEdge.stream() + .filter(outEdge -> !JunctionTag.isRoundabout(outEdge)).findFirst(); + if (roundaboutEdge.isPresent() && roundaboutExitEdge.isPresent()) + { + final Set roundaboutEdges = this + .getRoundaboutEdges((Edge) roundaboutEdge.get()); + final AtlasEntity exitEdge = roundaboutExitEdge.get(); + // If the destination sign relation is missing destination tag, flag it + if (this.isMissingDestinationTag((Edge) exitEdge)) + { + entitiesToBeFlagged.addAll(roundaboutEdges); + entitiesToBeFlagged.add(roundaboutExitEdge.get()); + } + final Optional> roundAboutEdgesNotPartOfRelations = this + .connectedEdgesNotPartOfRelation(exitEdge, roundaboutEdges, + destinationSignRelations); + // If there are missing destination sign relations, flag it + roundAboutEdgesNotPartOfRelations.ifPresent(entitiesToBeFlagged::addAll); + } + }); + return entitiesToBeFlagged; + } + + /** + * @param nonRoundaboutInEdgeToOutEdgeMap + * inEdge to outEdge map + * @param destinationSignRelations + * destinationSignRelations + * @return + */ + private Set getConnectedEdgesNotFormDestinationRelation( + final Map> nonRoundaboutInEdgeToOutEdgeMap, + final Set destinationSignRelations) + { + return nonRoundaboutInEdgeToOutEdgeMap.entrySet().stream().flatMap(atlasEntitySetEntry -> + { + final AtlasEntity key = atlasEntitySetEntry.getKey(); + return this.connectedEdgesNotPartOfRelation(key, + nonRoundaboutInEdgeToOutEdgeMap.get(key), destinationSignRelations).stream(); + }).flatMap(Collection::stream).collect(Collectors.toSet()); + } + + /** + * Return a FlaggedIntersection with the items in the input params and appropriate instruction + * index based on the input params. + * + * @param roundAboutInEdgeToOutEdgeMap + * map with roundabout inEdge and outEdges + * @param nonRoundaboutInEdgeToOutEdgeMap + * map with non-roundabout inEdge and outEdges + * @return FlaggedIntersection with instruction index and set of flagged items based on the + * input params + */ + private FlaggedIntersection getFlaggedIntersection( + final Map> roundAboutInEdgeToOutEdgeMap, + final Map> nonRoundaboutInEdgeToOutEdgeMap) + { + final Set entitiesToBeFlagged = new HashSet<>(); + int instructionIndex = -1; + // Flag all in and out edges + if (roundAboutInEdgeToOutEdgeMap.isEmpty() && nonRoundaboutInEdgeToOutEdgeMap != null) + { + nonRoundaboutInEdgeToOutEdgeMap.forEach((inEdge, setOfOutEdge) -> + { + entitiesToBeFlagged.add(inEdge); + entitiesToBeFlagged.addAll(setOfOutEdge); + }); + if (!entitiesToBeFlagged.isEmpty()) + { + // + instructionIndex = INSTRUCTION_INDEX_ZERO; + } + } + // Flag all roundabout edges + else + { + roundAboutInEdgeToOutEdgeMap.forEach((inEdge, setOfOutEdge) -> + { + // Ideally there would only be one roundabout edge and one exit edge per node + final Optional roundaboutEdge = JunctionTag.isRoundabout(inEdge) + ? Optional.of(inEdge) + : setOfOutEdge.stream().filter(JunctionTag::isRoundabout).findFirst(); + final Optional roundaboutExitEdge = setOfOutEdge.stream() + .filter(outEdge -> !JunctionTag.isRoundabout(outEdge)).findFirst(); + if (roundaboutEdge.isPresent() && roundaboutExitEdge.isPresent()) + { + entitiesToBeFlagged + .addAll(this.getRoundaboutEdges((Edge) roundaboutEdge.get())); + entitiesToBeFlagged.add(roundaboutExitEdge.get()); + } + }); + if (!entitiesToBeFlagged.isEmpty()) + { + instructionIndex = INSTRUCTION_INDEX_THREE; + } + } + return new FlaggedIntersection(instructionIndex, entitiesToBeFlagged); + } + + /** + * Collects all atlas identifiers of given set of {@link AtlasObject}s + * + * @param objects + * set of {@link AtlasObject}s + * @return {@link Iterable} containing the atlas identifiers of input objects + */ + private List getIdentifiers(final Set objects) + { + return Iterables.stream(objects).map(AtlasEntity::getIdentifier).map(String::valueOf) + .collectToList(); + } + + /** + * Node could be part of multiple destination_sign relations. This method collect the + * intersecting items that are not part of existing destination_sign relation of the node or if + * the relations are missing a destination_sign tag. + * + * @param roundAboutInEdgeToOutEdgeMap + * map with roundabout inEdge and outEdges + * @param nonRoundaboutInEdgeToOutEdgeMap + * map with non-roundabout inEdge and outEdges + * @param intersectingNode + * {@link Node} + * @param destinationSignRelations + * set of relations the node is a member of + * @return FlaggedIntersection with instruction index and set of flagged items with incomplete + * destination_sign relation + */ + private FlaggedIntersection getIntersectionsWithIncompleteDestinationSignRelation( + final Map> roundAboutInEdgeToOutEdgeMap, + final Map> nonRoundaboutInEdgeToOutEdgeMap, + final Node intersectingNode, final Set destinationSignRelations) + { + // If the node is part of destination sign relation, check if destination tag of the + // relation is missing or if there are any missing relations that the node could be part of + // based on from and to edges + final Set entitiesToBeFlagged = new HashSet<>(); + int instructionIndex = -1; + // Flag all roundabout edges that are missing destination sign relations or missing + // destination sign tags for existing relations + if (!roundAboutInEdgeToOutEdgeMap.isEmpty()) + { + final Set allRoundaboutEdgesMissingTagsOrRelations = this + .getAllRoundaboutEdgesMissingTagsOrRelations(roundAboutInEdgeToOutEdgeMap, + destinationSignRelations); + if (!allRoundaboutEdgesMissingTagsOrRelations.isEmpty()) + { + entitiesToBeFlagged.addAll(allRoundaboutEdgesMissingTagsOrRelations); + instructionIndex = INSTRUCTION_INDEX_FOUR; + } + } + else + { + final Set destinationSignRelationsMissingTag = this + .getRelationsWithMissingDestinationTag(destinationSignRelations); + // Flag if destination sign tag is missing + if (!destinationSignRelationsMissingTag.isEmpty()) + { + this.markAsFlagged(String.valueOf(intersectingNode.getIdentifier())); + instructionIndex = INSTRUCTION_INDEX_ONE; + entitiesToBeFlagged.addAll(destinationSignRelationsMissingTag); + } + else if (nonRoundaboutInEdgeToOutEdgeMap != null) + { + // If there are any missing destination sign relation that the node should be + // part of, flag it + final Set connectedEdgesNotFormDestinationRelation = this + .getConnectedEdgesNotFormDestinationRelation( + nonRoundaboutInEdgeToOutEdgeMap, destinationSignRelations); + if (!connectedEdgesNotFormDestinationRelation.isEmpty()) + { + instructionIndex = INSTRUCTION_INDEX_TWO; + entitiesToBeFlagged.addAll(connectedEdgesNotFormDestinationRelation); + } + } + } + return new FlaggedIntersection(instructionIndex, entitiesToBeFlagged); + } + + /** + * Collect all destination sign relations that the input atlas entity is member of. + * + * @param atlasEntity + * any {@link AtlasEntity} + * @return Optional> that the atlasEntity is member of + */ + private Optional> getParentDestinationSignRelations(final AtlasEntity atlasEntity) + { + final Set setOfDestinationSignRelations = atlasEntity.relations().stream() + .filter(relation -> RelationTypeTag.DESTINATION_SIGN.toString() + .equalsIgnoreCase(relation.tag(RelationTypeTag.KEY))) + .collect(Collectors.toSet()); + return setOfDestinationSignRelations.isEmpty() ? Optional.empty() + : Optional.of(setOfDestinationSignRelations); + } + + /** + * Collects all destination sign relations with missing destination sign tag + * + * @param destinationSignRelations + * set of destination sign relations + * @return set of relations with missing destination sign tag + */ + private Set getRelationsWithMissingDestinationTag( + final Set destinationSignRelations) + { + return destinationSignRelations.stream() + .filter(relation -> relation.tag(DestinationTag.KEY) == null) + .collect(Collectors.toSet()); + } + + /** + * Collects all roundabout edges starting with the given edge + * + * @param startEdge + * {@link Edge} + * @return Set of roundabout edges + */ + private Set getRoundaboutEdges(final Edge startEdge) + { + return new SimpleEdgeWalker(startEdge, this.isRoundaboutEdge()).collectEdges().stream() + .map(AtlasEntity.class::cast).collect(Collectors.toSet()); + } + + /** + * Checks if given outEdge is at the same z level and in the same direction as that of the + * inEdge + * + * @param inEdge + * inEdge + * @param outEdge + * outEdge + * @return true if the outEdge matches the above criteria for the given inEdge + */ + private boolean isMatchingOutEdge(final Edge inEdge, final Edge outEdge) + { + return LevelTag.areOnSameLevel(inEdge, outEdge) && LayerTag.areOnSameLayer(inEdge, outEdge) + && EDGE_DIRECTION_COMPARATOR.isSameDirection(inEdge, outEdge, true); + } + + /** + * Checks if the edge is missing DestinationForwardTag if two way or is missing a destination + * tag if one way + * + * @param edge + * any edge + * @return true if the edge is missing the destination tags + */ + private boolean isMissingDestinationTag(final Edge edge) + { + return (OneWayTag.isExplicitlyTwoWay(edge) && edge.tag(DestinationForwardTag.KEY) == null) + || (!OneWayTag.isExplicitlyTwoWay(edge) && edge.tag(DestinationTag.KEY) == null); + } + + /** + * Function for {@link SimpleEdgeWalker} that gathers connected edges that are part of a + * roundabout. + * + * @return {@link Function} for {@link SimpleEdgeWalker} + */ + private Function> isRoundaboutEdge() + { + return edge -> edge.connectedEdges().stream() + .filter(connected -> JunctionTag.isRoundabout(connected) + && HighwayTag.isCarNavigableHighway(connected)); + } + + /** + * Checks if given edge is a valid intersecting edge for an at-grade intersection + * + * @param edge + * edge + * @return true if the edge is valid intersecting edge + */ + private boolean isValidIntersectingEdge(final Edge edge) + { + return edge.isMasterEdge() && HighwayTag.highwayTag(edge).isPresent() + && this.highwayFilter.contains(edge.highwayTag().getTagValue()); + } + + /** + * Collect matching out edges and corresponding in edge in a map. Store the roundabout edges and + * non roundabout edges in separate maps. + * + * @param inEdges + * List inEdges + * @param outEdges + * Set outEdges + * @return Map>> with Map of inEdge to outEdges for + * roundabout and non roundabout edges + */ + private Map>> populateInEdgeToOutEdgeMaps( + final List inEdges, final Set outEdges) + { + final Map> nonRoundaboutInEdgeToOutEdgeMap = new HashMap<>(); + final Map> roundAboutInEdgeToOutEdgeMap = new HashMap<>(); + inEdges.forEach(inEdge -> + { + final Optional highwayTag = HighwayTag.highwayTag(inEdge); + if (highwayTag.isPresent() + && this.connectedHighwayTypes.containsKey(highwayTag.get().getTagValue())) + { + // Filter out edges based on level and layer tags and valid highway types + final Set filteredOutEdges = outEdges.stream() + .filter(outEdge -> this.isMatchingOutEdge(inEdge, outEdge)) + .collect(Collectors.toSet()); + // There should be at least 2 valid outEdges + if (filteredOutEdges.size() >= 2) + { + final String inEdgeHighwayType = highwayTag.get().getTagValue(); + final List validHighwayTypesOfOutEdge = this.connectedHighwayTypes + .get(inEdgeHighwayType); + final Set filteredByHighways = filteredOutEdges.stream() + .filter(atlasEntity -> + { + final Optional atlasEntityHighway = HighwayTag + .highwayTag(atlasEntity); + return atlasEntityHighway.isPresent() && validHighwayTypesOfOutEdge + .contains(atlasEntityHighway.get().getTagValue()); + }).collect(Collectors.toSet()); + // If any of the edges is a roundabout, add it to roundabout map + if (filteredByHighways.stream().anyMatch(JunctionTag::isRoundabout) + || JunctionTag.isRoundabout(inEdge)) + { + roundAboutInEdgeToOutEdgeMap.put(inEdge, filteredByHighways); + } + else if (!filteredByHighways.isEmpty()) + { + nonRoundaboutInEdgeToOutEdgeMap.put(inEdge, filteredByHighways); + } + } + + } + }); + final Map>> mapOfMatchingInAndOutEdges = new HashMap<>(); + mapOfMatchingInAndOutEdges.put(NON_ROUNDABOUT_INTERSECTION_MAP, + nonRoundaboutInEdgeToOutEdgeMap); + mapOfMatchingInAndOutEdges.put(ROUNDABOUT_INTERSECTION_MAP, roundAboutInEdgeToOutEdgeMap); + return mapOfMatchingInAndOutEdges; + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTest.java new file mode 100644 index 000000000..b653b25bd --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTest.java @@ -0,0 +1,109 @@ +package org.openstreetmap.atlas.checks.validation.intersections; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver; +import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier; +import org.openstreetmap.atlas.utilities.configuration.Configuration; + +/** + * Unit tests for {@link AtGradeSignPostCheck} + * + * @author sayas01 + */ +public class AtGradeSignPostCheckTest +{ + @Rule + public AtGradeSignPostCheckTestRule setup = new AtGradeSignPostCheckTestRule(); + @Rule + public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier(); + private final Configuration inlineConfiguration = ConfigurationResolver + .inlineConfiguration("{ \"AtGradeSignPostCheck\": {\n" + " \"enabled\": true,\n" + + " \"highway.filter\": \"highway->trunk,primary,secondary\",\n" + + " \"connected.highway.types\": {\n" + + " \"primary\": [\"trunk\",\"primary\",\"secondary\"],\n" + + " \"trunk\": [\"primary\"],\n" + " \"secondary\": [\"primary\"]\n" + + " }\n" + " }}"); + + @Test + public void testIncompleteDestinationSignRelation() + { + this.verifier.actual(this.setup.getIncompleteDestinationSignRelationAtlas(), + new AtGradeSignPostCheck(this.inlineConfiguration)); + this.verifier.verifyNotEmpty(); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(3, flag.getFlaggedObjects().size())); + this.verifier.verify(flag -> + { + Assert.assertTrue(flag.getInstructions().contains("could also form destination sign " + + "relations with this node. Create new destination sign relation with these " + + "edges and the node")); + }); + } + + @Test + public void testMissingDestinationSignRelation() + { + this.verifier.actual(this.setup.getMissingDestinationSignRelationAtlas(), + new AtGradeSignPostCheck(this.inlineConfiguration)); + // this.verifier.verifyNotEmpty(); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(4, flag.getFlaggedObjects().size())); + this.verifier.verify(flag -> + { + Assert.assertTrue( + flag.getInstructions().contains("is not part of a destination sign relation")); + }); + } + + @Test + public void testMissingDestinationSignTag() + { + this.verifier.actual(this.setup.getMissingDestinationSignTagAtlas(), + new AtGradeSignPostCheck(this.inlineConfiguration)); + this.verifier.verifyNotEmpty(); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size())); + this.verifier.verify(flag -> + { + Assert.assertTrue(flag.getInstructions().contains("missing \"destination\" tags")); + }); + } + + @Test + public void testRoundaboutConnectorDestinationSignTag() + { + this.verifier.actual(this.setup.getRoundaboutConnectorMissingDestinationSignTagAtlas(), + new AtGradeSignPostCheck(this.inlineConfiguration)); + this.verifier.verifyNotEmpty(); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(8, flag.getFlaggedObjects().size())); + this.verifier.verify(flag -> + { + Assert.assertTrue(flag.getInstructions() + .contains("is part of a roundabout and forms an " + + "at-grade junction. It is part of destination sign relation(s). " + + "Either the existing relations are missing destination sign tag or " + + "following connected edges")); + }); + } + + @Test + public void testRoundaboutIntersectionDestinationSignRelation() + { + this.verifier.actual(this.setup.getRoundaboutIntersectionMissingDestinationSignRelation(), + new AtGradeSignPostCheck(this.inlineConfiguration)); + this.verifier.verifyNotEmpty(); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(8, flag.getFlaggedObjects().size())); + this.verifier.verify(flag -> + { + Assert.assertTrue(flag.getInstructions().contains("is part of a roundabout and " + + "forms an at-grade junction with connected edges. Add destination sign " + + "relations with the node as \"intersection\" member and following connected " + + "edges")); + }); + } + +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTestRule.java new file mode 100644 index 000000000..708c72c68 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/AtGradeSignPostCheckTestRule.java @@ -0,0 +1,215 @@ +package org.openstreetmap.atlas.checks.validation.intersections; + +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; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Relation; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Relation.Member; + +/** + * Test atlases for {@link AtGradeSignPostCheckTest} + * + * @author sayas01 + */ +public class AtGradeSignPostCheckTestRule extends CoreTestRule +{ + + private static final String INTERSECTING_LOC_ONE = "42.6843556, 23.3008163"; + private static final String EDGE_ONE_START_LOC = "42.6844473, 23.3007951"; + private static final String EDGE_ONE_END_LOC = "42.6833464, 23.3010915"; + private static final String EDGE_TWO_START_LOC = "42.6843436, 23.3007210"; + private static final String EDGE_TWO_END_LOC = "42.6843649, 23.3008922"; + private static final String ROUNDABOUT_LOC_ONE = "43.2097940, 23.5532672"; + private static final String ROUNDABOUT_LOC_TWO = "43.2097318, 23.5529942"; + private static final String ROUNDABOUT_LOC_THREE = "43.2095883, 23.5530627"; + private static final String ROUNDABOUT_LOC_FOUR = "43.2095262, 23.5532058"; + private static final String ROUNDABOUT_LOC_FIVE = "43.2095410, 23.5532935"; + private static final String ROUNDABOUT_LOC_SIX = "43.2097333, 23.5533566"; + private static final String ROUNDABOUT_CONNECTOR_LOC_ONE = "43.2100746, 23.5528153"; + + @TestAtlas( + // Nodes + nodes = { @Node(coordinates = @Loc(value = INTERSECTING_LOC_ONE)), + @Node(coordinates = @Loc(value = EDGE_ONE_START_LOC)), + @Node(coordinates = @Loc(value = EDGE_ONE_END_LOC)), + @Node(coordinates = @Loc(value = EDGE_TWO_START_LOC)), + @Node(coordinates = @Loc(value = EDGE_TWO_END_LOC)) }, + // Edges + edges = { + @Edge(id = "100010001", coordinates = { @Loc(value = EDGE_ONE_START_LOC), + @Loc(value = INTERSECTING_LOC_ONE) }, tags = { "highway=primary" }), + @Edge(id = "100010002", coordinates = { @Loc(value = INTERSECTING_LOC_ONE), + @Loc(value = EDGE_ONE_END_LOC) }, tags = { "highway=primary" }), + @Edge(id = "100010003", coordinates = { @Loc(value = EDGE_TWO_START_LOC), + @Loc(value = INTERSECTING_LOC_ONE) }, tags = { "highway=residential" }), + @Edge(id = "100010004", coordinates = { @Loc(value = INTERSECTING_LOC_ONE), + @Loc(value = EDGE_TWO_END_LOC) }, tags = { "highway=secondary" }) }) + private Atlas missingDestinationSignRelationAtlas; + + @TestAtlas( + // Nodes + nodes = { @Node(id = "300010000", coordinates = @Loc(value = INTERSECTING_LOC_ONE)), + @Node(coordinates = @Loc(value = EDGE_ONE_START_LOC)), + @Node(coordinates = @Loc(value = EDGE_ONE_END_LOC)), + @Node(coordinates = @Loc(value = EDGE_TWO_START_LOC)), + @Node(coordinates = @Loc(value = EDGE_TWO_END_LOC)) }, + // Edges + edges = { + @Edge(id = "100010001", coordinates = { @Loc(value = EDGE_ONE_START_LOC), + @Loc(value = INTERSECTING_LOC_ONE) }, tags = { "highway=primary" }), + @Edge(id = "100010002", coordinates = { @Loc(value = INTERSECTING_LOC_ONE), + @Loc(value = EDGE_ONE_END_LOC) }, tags = { "highway=primary" }), + @Edge(id = "100010003", coordinates = { @Loc(value = EDGE_TWO_START_LOC), + @Loc(value = INTERSECTING_LOC_ONE) }, tags = { "highway=residential" }), + @Edge(id = "100010004", coordinates = { @Loc(value = INTERSECTING_LOC_ONE), + @Loc(value = EDGE_TWO_END_LOC) }, tags = { + "highway=secondary" }) }, relations = { + @Relation(id = "200010000", members = { + @Member(id = "100010001", type = "edge", role = "from"), + @Member(id = "300010000", type = "node", role = "intersection"), + @Member(id = "100010004", type = "edge", role = "to") }, tags = { + "destination=Център", + "type=destination_sign" }) }) + private Atlas incompleteDestinationSignRelationAtlas; + + @TestAtlas( + // Nodes + nodes = { @Node(id = "300010000", coordinates = @Loc(value = INTERSECTING_LOC_ONE)), + @Node(coordinates = @Loc(value = EDGE_ONE_START_LOC)), + @Node(coordinates = @Loc(value = EDGE_ONE_END_LOC)), + @Node(coordinates = @Loc(value = EDGE_TWO_START_LOC)), + @Node(coordinates = @Loc(value = EDGE_TWO_END_LOC)) }, + // Edges + edges = { + @Edge(id = "100010001", coordinates = { @Loc(value = EDGE_ONE_START_LOC), + @Loc(value = INTERSECTING_LOC_ONE) }, tags = { "highway=primary" }), + @Edge(id = "100010002", coordinates = { @Loc(value = INTERSECTING_LOC_ONE), + @Loc(value = EDGE_ONE_END_LOC) }, tags = { "highway=primary" }), + @Edge(id = "100010003", coordinates = { @Loc(value = EDGE_TWO_START_LOC), + @Loc(value = INTERSECTING_LOC_ONE) }, tags = { "highway=residential" }), + @Edge(id = "100010004", coordinates = { @Loc(value = INTERSECTING_LOC_ONE), + @Loc(value = EDGE_TWO_END_LOC) }, tags = { + "highway=secondary" }) }, relations = { + @Relation(id = "200010000", members = { + @Member(id = "100010001", type = "edge", role = "from"), + @Member(id = "300010000", type = "node", role = "intersection"), + @Member(id = "100010004", type = "edge", role = "to") }, tags = { + "destination=Център", + "type=destination_sign" }), + @Relation(id = "400010000", members = { + @Member(id = "100010001", type = "edge", role = "from"), + @Member(id = "300010000", type = "node", role = "intersection"), + @Member(id = "100010004", type = "edge", role = "to") }, tags = { + "type=destination_sign" }), }) + private Atlas missingDestinationSignTagAtlas; + + @TestAtlas( + // Nodes + nodes = { + @Node(id = "300010000", coordinates = @Loc(value = ROUNDABOUT_CONNECTOR_LOC_ONE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_FIVE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_FOUR)), + @Node(id = "700010000", coordinates = @Loc(value = ROUNDABOUT_LOC_ONE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_THREE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_TWO)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_SIX)), }, + // Edges + edges = { + @Edge(id = "100010001", coordinates = { @Loc(value = ROUNDABOUT_LOC_ONE), + @Loc(value = ROUNDABOUT_LOC_TWO) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010002", coordinates = { @Loc(value = ROUNDABOUT_LOC_TWO), + @Loc(value = ROUNDABOUT_LOC_THREE) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010003", coordinates = { @Loc(value = ROUNDABOUT_LOC_THREE), + @Loc(value = ROUNDABOUT_LOC_FOUR) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010004", coordinates = { @Loc(value = ROUNDABOUT_LOC_FOUR), + @Loc(value = ROUNDABOUT_LOC_FIVE) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010005", coordinates = { @Loc(value = ROUNDABOUT_LOC_FIVE), + @Loc(value = ROUNDABOUT_LOC_SIX) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010006", coordinates = { @Loc(value = ROUNDABOUT_LOC_SIX), + @Loc(value = ROUNDABOUT_LOC_ONE) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "200010004", coordinates = { @Loc(value = ROUNDABOUT_LOC_ONE), + @Loc(value = ROUNDABOUT_CONNECTOR_LOC_ONE) }, tags = { + "highway=primary", "oneway=yes" }) }, + // Relations + relations = { + @Relation(id = "200010000", members = { + @Member(id = "100010006", type = "edge", role = "from"), + @Member(id = "700010000", type = "node", role = "intersection"), + @Member(id = "200010004", type = "edge", role = "to") }, tags = { + "destination=Център", "type=destination_sign" }), + @Relation(id = "400010000", members = { + @Member(id = "100010001", type = "edge", role = "from"), + @Member(id = "300010000", type = "node", role = "intersection"), + @Member(id = "100010004", type = "edge", role = "to") }, tags = { + "type=destination_sign" }), }) + private Atlas roundaboutConnectorMissingDestinationSignTagAtlas; + + @TestAtlas( + // Nodes + nodes = { + @Node(id = "300010000", coordinates = @Loc(value = ROUNDABOUT_CONNECTOR_LOC_ONE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_FIVE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_FOUR)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_ONE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_THREE)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_TWO)), + @Node(coordinates = @Loc(value = ROUNDABOUT_LOC_SIX)), }, + // Edges + edges = { + @Edge(id = "100010001", coordinates = { @Loc(value = ROUNDABOUT_LOC_ONE), + @Loc(value = ROUNDABOUT_LOC_TWO) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010002", coordinates = { @Loc(value = ROUNDABOUT_LOC_TWO), + @Loc(value = ROUNDABOUT_LOC_THREE) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010003", coordinates = { @Loc(value = ROUNDABOUT_LOC_THREE), + @Loc(value = ROUNDABOUT_LOC_FOUR) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010004", coordinates = { @Loc(value = ROUNDABOUT_LOC_FOUR), + @Loc(value = ROUNDABOUT_LOC_FIVE) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010005", coordinates = { @Loc(value = ROUNDABOUT_LOC_FIVE), + @Loc(value = ROUNDABOUT_LOC_SIX) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "100010006", coordinates = { @Loc(value = ROUNDABOUT_LOC_SIX), + @Loc(value = ROUNDABOUT_LOC_ONE) }, tags = { "highway=primary", + "junction=roundabout" }), + @Edge(id = "200010004", coordinates = { @Loc(value = ROUNDABOUT_LOC_ONE), + @Loc(value = ROUNDABOUT_CONNECTOR_LOC_ONE) }, tags = { + "highway=primary", "oneway=yes" }) }) + private Atlas roundaboutIntersectionMissingDestinationSignRelationAtlas; + + public Atlas getIncompleteDestinationSignRelationAtlas() + { + return this.incompleteDestinationSignRelationAtlas; + } + + public Atlas getMissingDestinationSignRelationAtlas() + { + return this.missingDestinationSignRelationAtlas; + } + + public Atlas getMissingDestinationSignTagAtlas() + { + return this.missingDestinationSignTagAtlas; + } + + public Atlas getRoundaboutConnectorMissingDestinationSignTagAtlas() + { + return this.roundaboutConnectorMissingDestinationSignTagAtlas; + } + + public Atlas getRoundaboutIntersectionMissingDestinationSignRelation() + { + return this.roundaboutIntersectionMissingDestinationSignRelationAtlas; + } +} From 976b15b6e5969b9f2d03201984e3741460aec2f4 Mon Sep 17 00:00:00 2001 From: Daniel B Date: Wed, 19 Feb 2020 16:38:50 -0800 Subject: [PATCH 07/15] Bump atlas dependencies & fix broken BigNodeBadDataCheck unit test (#261) * update dependencies and fix broken unit test * fix formatting --- dependencies.gradle | 4 ++-- .../intersections/BigNodeBadDataCheckTestRule.java | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/dependencies.gradle b/dependencies.gradle index ad7baf7be..42e2e1226 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -1,9 +1,9 @@ project.ext.versions = [ checkstyle: '8.18', jacoco: '0.8.3', - atlas: '6.0.1', + atlas: '6.0.2', commons:'2.6', - atlas_generator: '5.0.0', + atlas_generator: '5.0.1', atlas_checkstyle: '5.6.9', postgis: '2.1.7.2', postgres: '42.2.6', diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/BigNodeBadDataCheckTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/BigNodeBadDataCheckTestRule.java index fd457542d..d59b800e9 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/BigNodeBadDataCheckTestRule.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/BigNodeBadDataCheckTestRule.java @@ -337,13 +337,17 @@ public class BigNodeBadDataCheckTestRule extends CoreTestRule // Fourth example @Edge(id = "3230", coordinates = { @Loc(value = THIRTYTWO), - @Loc(value = THIRTY) }, tags = { "highway=residential" }), + @Loc(value = THIRTY) }, tags = { "highway=residential", + "name=Bentley" }), @Edge(id = "3133", coordinates = { @Loc(value = THIRTYONE), - @Loc(value = THIRTYTHREE) }, tags = { "highway=residential" }), + @Loc(value = THIRTYTHREE) }, tags = { "highway=residential", + "name=Bentley" }), @Edge(id = "3432", coordinates = { @Loc(value = THIRTYFOUR), - @Loc(value = THIRTYTWO) }, tags = { "highway=residential" }), + @Loc(value = THIRTYTWO) }, tags = { "highway=residential", + "name=Bentley" }), @Edge(id = "3335", coordinates = { @Loc(value = THIRTYTHREE), - @Loc(value = THIRTYFIVE) }, tags = { "highway=residential" }), + @Loc(value = THIRTYFIVE) }, tags = { "highway=residential", + "name=Bentley" }), @Edge(id = "3632", coordinates = { @Loc(value = THIRTYSIX), @Loc(value = THIRTYTWO) }, tags = { "highway=residential" }), @Edge(id = "3233", coordinates = { @Loc(value = THIRTYTWO), From c79ef742e68ed9ae2dbed43c4f43b2d36e18ba63 Mon Sep 17 00:00:00 2001 From: Sugandhi Maheshwaram Date: Tue, 25 Feb 2020 12:24:11 -0600 Subject: [PATCH 08/15] Edge-crossing-edge-check bug fix. (#253) * Edge-crossing-edge-check, working functionality. * Edge-crossing-edge-check, making sure edges with same osm identifier won't be flagged duplicate. * Edge-crossing-edge-check, formatting fix. * Edge-crossover-edge, added functionality to collect all crossing edges locations. * Edge-crossing-edge, modified code to add edge to a flag and mark it as flagged if it has max size of crossing edges. * Edge-crossing-edge, fixing checkstyle issues. * Edge-crossing-edge, addressing review comments. Co-authored-by: Daniel B --- .../intersections/EdgeCrossingEdgeCheck.java | 96 +++++++++++++++---- .../EdgeCrossingEdgeCheckTest.java | 9 +- 2 files changed, 85 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheck.java index 9c054f68d..28831df51 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheck.java @@ -5,6 +5,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.openstreetmap.atlas.checks.atlas.predicates.TagPredicates; @@ -24,6 +25,8 @@ import org.openstreetmap.atlas.utilities.collections.Iterables; import org.openstreetmap.atlas.utilities.configuration.Configuration; +import scala.Tuple2; + /** * Flags edges that are crossing other edges invalidly. If two edges are crossing each other, then * they should have an intersection location shared in both edges. Otherwise, their layer tag should @@ -45,11 +48,11 @@ private class EdgeCrossingEdgeWalker extends EdgeWalker } } - private static final String INSTRUCTION_FORMAT = "The road with id {0,number,#} has invalid crossings." + private static final String INSTRUCTION_FORMAT = "The road with id {0} has invalid crossings with {1}." + " If two roads are crossing each other, then they should have nodes at intersection" + " locations unless they are explicitly marked as crossing. Otherwise, crossing roads" + " should have different layer tags."; - private static final String INVALID_EDGE_FORMAT = "Edge {0,number,#} is crossing invalidly."; + private static final String INVALID_EDGE_FORMAT = "Edge {0} is crossing invalidly with {1}."; private static final List FALLBACK_INSTRUCTIONS = Arrays.asList(INSTRUCTION_FORMAT, INVALID_EDGE_FORMAT); private static final String MINIMUM_HIGHWAY_DEFAULT = HighwayTag.NO.toString(); @@ -105,22 +108,40 @@ protected Optional flag(final AtlasObject object) // edges from being flagged more than once. final Set collectedEdges = new EdgeCrossingEdgeWalker((Edge) object, this.getInvalidCrossingEdges()).collectEdges(); - if (collectedEdges.size() > 1) { - final CheckFlag newFlag = new CheckFlag(getTaskIdentifier(object)); - this.markAsFlagged(object.getIdentifier()); - newFlag.addObject(object); - newFlag.addInstruction(this.getLocalizedInstruction(0, object.getOsmIdentifier())); - - collectedEdges.forEach(invalidEdge -> + final List>> edgeCrossPairs = collectedEdges + .stream().filter( + edge -> edge.getOsmIdentifier() != object.getOsmIdentifier()) + .map(edge -> new Tuple2<>(edge, + new EdgeCrossingEdgeWalker(edge, this.getInvalidCrossingEdges()) + .collectEdges())) + .collect(Collectors.toList()); + edgeCrossPairs.add(new Tuple2<>((Edge) object, collectedEdges)); + final Optional>> maxPair = edgeCrossPairs.stream().max( + (entry1, entry2) -> Integer.compare(entry1._2().size(), entry2._2().size())); + if (maxPair.isPresent()) { - this.markAsFlagged(invalidEdge.getIdentifier()); - newFlag.addObject(invalidEdge); - newFlag.addInstruction( - this.getLocalizedInstruction(1, invalidEdge.getOsmIdentifier())); - }); - return Optional.of(newFlag); + final int maxSize = (maxPair.get()._2()).size(); + + // max edges object is not the one passed to this flag. + final List>> maxEdgePairs = edgeCrossPairs.stream() + .filter(crossPair -> (crossPair._2()).size() == maxSize) + .collect(Collectors.toList()); + final Optional>> minIdentifierPair = maxEdgePairs.stream() + .reduce((edge1, edge2) -> + // reduce to get the minimum osm identifier edge pair. + edge1._1().getOsmIdentifier() <= edge2._1().getOsmIdentifier() ? edge1 + : edge2); + if (minIdentifierPair.isPresent()) + { + final Tuple2> minPair = minIdentifierPair.get(); + if (!this.isFlagged(minPair._1().getIdentifier())) + { + return createEdgeCrossCheckFlag(minPair._1(), minPair._2()); + } + } + } } return Optional.empty(); } @@ -131,6 +152,47 @@ protected List getFallbackInstructions() return FALLBACK_INSTRUCTIONS; } + /** + * Function creates edge cross check flag. + * + * @param edge + * Atlas object. + * @param collectedEdges + * collected edges for a given atlas object. + * @return newly created edge cross check glag including crossing edges locations. + */ + private Optional createEdgeCrossCheckFlag(final Edge edge, + final Set collectedEdges) + { + final CheckFlag newFlag = new CheckFlag(getTaskIdentifier(edge)); + this.markAsFlagged(edge.getIdentifier()); + final Set points = collectedEdges.stream() + .filter(crossEdge -> crossEdge.getIdentifier() != edge.getIdentifier()) + .flatMap(crossEdge -> getIntersection(edge, crossEdge).stream()) + .collect(Collectors.toSet()); + newFlag.addInstruction(this.getLocalizedInstruction(0, edge.getIdentifier(), collectedEdges + .stream().map(AtlasObject::getIdentifier).collect(Collectors.toList()))); + newFlag.addPoints(points); + newFlag.addObject(edge); + return Optional.of(newFlag); + } + + /** + * This function returns set of intersections locations for given params. + * + * @param edge1 + * Atlas object + * @param edge2 + * crossing edge + * @return set of intersection locations. + */ + private Set getIntersection(final Edge edge1, final Edge edge2) + { + final PolyLine edge1AsPolyLine = edge1.asPolyLine(); + final PolyLine edge2AsPolyLine = edge2.asPolyLine(); + return edge1AsPolyLine.intersections(edge2AsPolyLine); + } + /** * A {@link Function} for an {@link EdgeWalker} that collects all connected invalid crossings. * @@ -155,6 +217,8 @@ private Function> getInvalidCrossingEdges() crossingEdge -> edge.getIdentifier() != crossingEdge.getIdentifier() && this.isValidCrossingEdge(crossingEdge))) .stream() + .filter(crossingEdge -> crossingEdge.getOsmIdentifier() != edge + .getOsmIdentifier()) // Go through crossing items and collect invalid crossings // NOTE: Due to way sectioning same OSM way could be marked multiple times here. // However, @@ -188,7 +252,7 @@ private Function> getInvalidCrossingEdges() private boolean isValidCrossingEdge(final AtlasObject object) { if (Edge.isMasterEdgeIdentifier(object.getIdentifier()) - && !TagPredicates.IS_AREA.test(object) && !this.isFlagged(object.getIdentifier())) + && !TagPredicates.IS_AREA.test(object)) { final Optional highway = HighwayTag.highwayTag(object); if (highway.isPresent()) diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheckTest.java index 2d92a400b..80ed2861f 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheckTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/EdgeCrossingEdgeCheckTest.java @@ -41,8 +41,8 @@ public void testInvalidCrossingItemsWithInvalidLayerTagAtlas() this.verifier.actual(this.setup.invalidCrossingItemsWithInvalidLayerTagAtlas(), new EdgeCrossingEdgeCheck(this.configuration)); this.verifier.verifyNotEmpty(); - this.verifier.globallyVerify(flags -> Assert.assertEquals(flags.size(), 1)); - this.verifier.verify(flag -> Assert.assertEquals(flag.getFlaggedObjects().size(), 4)); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(3, flag.getFlaggedObjects().size())); } @Test @@ -51,8 +51,8 @@ public void testInvalidCrossingItemsWithSameLayerTagAtlas() this.verifier.actual(this.setup.invalidCrossingItemsWithSameLayerTagAtlas(), new EdgeCrossingEdgeCheck(ConfigurationResolver.inlineConfiguration( "{\"EdgeCrossingEdgeCheck\":{\"minimum.highway.type\":\"track\"}}"))); - this.verifier.globallyVerify(flags -> Assert.assertEquals(flags.size(), 1)); - this.verifier.verify(flag -> Assert.assertEquals(flag.getFlaggedObjects().size(), 2)); + this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size())); } @Test @@ -62,6 +62,7 @@ public void testInvalidCrossingNonMasterItemsAtlas() new EdgeCrossingEdgeCheck(this.configuration)); this.verifier.verifyNotEmpty(); this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size())); + this.verifier.verify(flag -> Assert.assertEquals(4, flag.getFlaggedObjects().size())); } @Test From d9e72eed2da04f8e0b16a9f6142c5791c75e2347 Mon Sep 17 00:00:00 2001 From: Sugandhi Maheshwaram Date: Tue, 25 Feb 2020 12:24:45 -0600 Subject: [PATCH 09/15] Road link check (#264) * Road-link-check, working functionality. * Road-link-check, formatting fix. --- .../validation/linear/edges/RoadLinkCheck.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/RoadLinkCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/RoadLinkCheck.java index aaec04d02..c563369f7 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/RoadLinkCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/RoadLinkCheck.java @@ -5,11 +5,13 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import org.openstreetmap.atlas.checks.base.BaseCheck; import org.openstreetmap.atlas.checks.flag.CheckFlag; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.geography.atlas.items.Edge; +import org.openstreetmap.atlas.geography.atlas.walker.OsmWayWalker; import org.openstreetmap.atlas.utilities.configuration.Configuration; import org.openstreetmap.atlas.utilities.scalars.Distance; @@ -20,7 +22,6 @@ */ public class RoadLinkCheck extends BaseCheck { - public static final double DISTANCE_MILES_DEFAULT = 1; private static final String INVALID_LINK_DISTANCE_INSTRUCTION = "Invalid link, distance, {0}, greater than maximum, {1}."; private static final String NO_SAME_CLASSIFICATION_INSTRUCTION = "None of the connected edges contain any edges with the same classification [{0}]"; @@ -40,7 +41,7 @@ public RoadLinkCheck(final Configuration configuration) public boolean validCheckForObject(final AtlasObject object) { return object instanceof Edge && ((Edge) object).highwayTag().isLink() - && !this.isFlagged(object.getOsmIdentifier()); + && ((Edge) object).isMasterEdge() && !this.isFlagged(object.getOsmIdentifier()); } @Override @@ -50,16 +51,20 @@ protected Optional flag(final AtlasObject object) this.markAsFlagged(edge.getOsmIdentifier()); if (edge.length().isGreaterThan(this.maximumLength)) { - return Optional.of(this.createFlag(object, + return Optional.of(this.createFlag(new OsmWayWalker(edge).collectEdges(), this.getLocalizedInstruction(0, edge.length(), this.maximumLength))); } - else if (edge.connectedEdges().stream().noneMatch( + else if (edge.connectedEdges().stream().filter(Edge::isMasterEdge).noneMatch( connected -> connected.highwayTag().isOfEqualClassification(edge.highwayTag()))) { final Set geometry = new HashSet<>(); geometry.add(edge); - geometry.addAll(edge.connectedEdges()); - final CheckFlag flag = this.createFlag(geometry, + geometry.addAll(edge.connectedEdges().stream().filter(Edge::isMasterEdge) + .collect(Collectors.toSet())); + final Set flagEdges = geometry.stream() + .flatMap(obj -> new OsmWayWalker((Edge) obj).collectEdges().stream()) + .collect(Collectors.toSet()); + final CheckFlag flag = this.createFlag(flagEdges, this.getLocalizedInstruction(1, edge.highwayTag().toString())); flag.addPoint(edge.start().getLocation().midPoint(edge.end().getLocation())); return Optional.of(flag); From 4596d6915ac08c7f9a1f079709862411696a8480 Mon Sep 17 00:00:00 2001 From: Bentley Breithaupt Date: Fri, 28 Feb 2020 14:42:53 -0800 Subject: [PATCH 10/15] Sharded Integrity Checks Job (#259) * save work * framework up * formatting * processor * Produce flags set up * testing begining * Testing improvements * Clean up * testing additions and adjustments * Rearrangment * javadoc fix * update IncompleteSharding * loadChecksForCountry checks white/blacklists * multipolygon fix * dynamic atlas * spotless * contrify parralelize * count * spotless * para stream * pool * unique ids * rm 2nd metrics * unique container tests * test rule * revert MR * sharded task tests * flagged relation tests * spark job tests * sharded job clean up * docs * no grouping * rm deffered loading * pre load * deffer and laod * bring back multi * return multi option * clean up switches * spotless * input note * add md docs * clean up * clean up 2 * clean up 3 * clean up 4 * clean up 5 * code smells * hard fails Co-authored-by: Jack Klamer Co-authored-by: Daniel B --- docs/cluster.md | 5 +- docs/shardedchecks.md | 58 +++ .../checks/base/CheckResourceLoader.java | 25 +- .../AtlasChecksGeoJSONDiffSubCommand.java | 23 +- .../AtlasChecksLogDiffSubCommand.java | 41 +- .../commands/JSONFlagDiffSubCommand.java | 1 + .../distributed/IntegrityCheckSparkJob.java | 119 +---- .../IntegrityChecksCommandArguments.java | 199 ++++++++ .../checks/distributed/RunnableCheck.java | 20 + .../checks/distributed/RunnableCheckBase.java | 26 +- .../distributed/ShardedCheckFlagsTask.java | 52 +++ .../ShardedIntegrityChecksSparkJob.java | 433 ++++++++++++++++++ .../atlas/checks/event/CheckFlagEvent.java | 11 +- .../checks/event/CheckFlagFileProcessor.java | 3 +- .../event/CheckFlagGeoJsonProcessor.java | 2 + .../event/CheckFlagTippecanoeProcessor.java | 2 +- .../atlas/checks/event/Event.java | 28 -- .../atlas/checks/event/EventService.java | 122 ----- .../atlas/checks/event/FileProcessor.java | 2 + .../atlas/checks/event/MetricEvent.java | 2 +- .../checks/event/MetricFileGenerator.java | 10 +- .../atlas/checks/event/Processor.java | 33 -- .../atlas/checks/event/ShutdownEvent.java | 12 - .../atlas/checks/flag/CheckFlag.java | 46 +- .../atlas/checks/flag/FlaggedObject.java | 14 +- .../atlas/checks/flag/FlaggedPoint.java | 29 +- .../atlas/checks/flag/FlaggedPolyline.java | 29 ++ .../atlas/checks/flag/FlaggedRelation.java | 74 ++- .../checks/utility/IncompleteSharding.java | 80 ++++ .../utility/UniqueCheckFlagContainer.java | 109 +++++ .../checks/base/CheckResourceLoaderTest.java | 24 + .../AtlasChecksGeoJSONDiffSubCommandTest.java | 2 +- .../AtlasChecksLogDiffSubCommandTest.java | 2 +- .../FlagStatisticsSubCommandTest.java | 2 +- .../ShardedCheckFlagsTaskTest.java | 52 +++ .../ShardedIntegrityChecksSparkJobTest.java | 139 ++++++ ...hardedIntegrityChecksSparkJobTestRule.java | 48 ++ .../event/CheckFlagFileProcessorTest.java | 1 + .../event/CheckFlagGeoJsonProcessorTest.java | 1 + .../CheckFlagTippecanoeProcessorTest.java | 1 + .../atlas/checks/event/EventBusTest.java | 181 -------- .../atlas/checks/event/EventServiceTest.java | 236 ---------- .../checks/event/MetricFileGeneratorTest.java | 1 + .../atlas/checks/event/TestEvent.java | 21 - .../atlas/checks/event/TestProcessor.java | 47 -- .../atlas/checks/flag/CheckFlagTest.java | 58 +++ .../checks/flag/FlaggedRelationTest.java | 62 +++ .../checks/flag/FlaggedRelationTestRule.java | 81 ++++ .../MapRouletteUploadCommandTest.java | 2 +- .../utility/UniqueCheckFlagContainerTest.java | 99 ++++ .../UniqueCheckFlagContainerTestRule.java | 33 ++ .../checks/validation/EdgesTestCheck.java | 46 ++ .../distributed/test_configuration.json | 10 + 53 files changed, 1908 insertions(+), 851 deletions(-) create mode 100644 docs/shardedchecks.md create mode 100644 src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityChecksCommandArguments.java create mode 100644 src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTask.java create mode 100644 src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJob.java delete mode 100644 src/main/java/org/openstreetmap/atlas/checks/event/Event.java delete mode 100644 src/main/java/org/openstreetmap/atlas/checks/event/EventService.java delete mode 100644 src/main/java/org/openstreetmap/atlas/checks/event/Processor.java delete mode 100644 src/main/java/org/openstreetmap/atlas/checks/event/ShutdownEvent.java create mode 100644 src/main/java/org/openstreetmap/atlas/checks/utility/IncompleteSharding.java create mode 100644 src/main/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainer.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTaskTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTestRule.java delete mode 100644 src/test/java/org/openstreetmap/atlas/checks/event/EventBusTest.java delete mode 100644 src/test/java/org/openstreetmap/atlas/checks/event/EventServiceTest.java delete mode 100644 src/test/java/org/openstreetmap/atlas/checks/event/TestEvent.java delete mode 100644 src/test/java/org/openstreetmap/atlas/checks/event/TestProcessor.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTestRule.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTestRule.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/EdgesTestCheck.java create mode 100644 src/test/resources/org/openstreetmap/atlas/checks/distributed/test_configuration.json diff --git a/docs/cluster.md b/docs/cluster.md index f298118f4..c77678318 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -88,7 +88,8 @@ To get the job running against a different version of Spark could be very challe If you see an exception while running in your environment, like `ClassNotFoundException`, `ClassCastException`, thrown from a standard library, such as Guava, Slf4j, then you are hitting the dependency conflict problem. To solve it, try to figure out the name of the conflicted library, then update dependency configuration in `build.gradle` file to force a working version. -#### large country support +#### Large country support By default atlas-check run jobs by country. This means one worker node will have to be able to load all the data for one country in memory. -This is not a good way to distribute workload and could make it very challenging to run large countries like USA, RUS, or CHN. Before a better solution gets implemented, you'll have to allocate large memories if you need to check against an enormous country. +This is not a good way to distribute workload and could make it very challenging to run large countries like USA, RUS, or CHN. +For running large countries see [Sharded Checks](shardedchecks.md). diff --git a/docs/shardedchecks.md b/docs/shardedchecks.md new file mode 100644 index 000000000..3b969c869 --- /dev/null +++ b/docs/shardedchecks.md @@ -0,0 +1,58 @@ +# Sharded Atlas Checks + +Sharded Atlas Checks is an implementation of the Atlas Checks framework that seeks to provide a memory efficient way to run large data sets. This is achieved using a "sharded" input schema and parallel processing. + +Normally when running Atlas Checks the size of input data is capped on a per country level by the amount of memory available. This means that even in a distributed environment enormous amounts of RAM are required to run countries like USA, RUS, and CHN. This implementation circumvents that cap by processing a country as multiple small sections. Each section is loaded as an individual process. Each process runs checks on the given section and returns a set of flags. Flags from all sections of a country are combined in the output. + +While the normal Atlas Checks job can run on either PBF or Atlas files, this job is restricted to specialized atlas files. These files should be in a countrified and sharded format. These files can be produced by using [Atlas Generator](https://github.com/osmlab/atlas-generator). + +## How To Run + +Sharded Checks is designed to be run in a Spark [cluster](cluster.md) for best performance, but can also be run as a standalone Java execution. The following details the parameters required for running the job in either environment, and parameters that are unique to Sharded Checks. + +#### Main Class +The main class of this job is org.openstreetmap.atlas.checks.distributed.ShardedIntegrityChecksSparkJob. + +#### Input Data Path +Input Atlas files should be organized by country and sub-region (shard). The root of the input folder should contain sub-folders that are named by ISO3 county codes. Each sub-folder should contain atlas files named with the schema `___.atlas`. An example of the full structure would be: +```text +- root + | - USA + | - SGP + | - SGP_10-806-508.atlas + | - SGP_11-1614-1016.atlas + | - ... +``` + +#### Expansion Distance +To maintain geographic completeness and avoid edge effects while running subsections of countries, each process is allowed to expand the area of a country that is loaded up to a set amount. The distance given in this parameter defines that expansion as shards within the given distance (in kilometers). + +#### Sharding Schema +In order to load geographically connected shards together the job requires a definition of the sharding schema used for the input Atlas files. This can be supplied in 2 ways. A dynamic sharding definition can be supplied by placing a sharding.txt file in the input path. Alternatively, a schema can be provided using the `sharding` parameter. For more on this see the [sharding package](https://github.com/osmlab/atlas/tree/dev/src/main/java/org/openstreetmap/atlas/geography/sharding) in Atlas. + +#### In Memory Atlas Type +By default Sharded Checks uses a [Dynamic Atlas](https://github.com/osmlab/atlas/tree/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/dynamic). It is also possible to use a [Multi Atlas](https://github.com/osmlab/atlas/tree/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/multi) to load Atlas files. This can be done by setting the `multiAtlas` parameter to `true`. It has been found that a Multi Atlas is the more performant in non-distributed environments. + +#### Spark Storage +By default Spark uses memory before disk when storing an RDD. In environments with large mounts of available memory this works well. In memory limited environments this can bog down when trying to process large data sets. In these scenarios it is more efficient to save everything directly to disk. This can be done by setting the `sparkStorageDiskOnly` to `true`. + +#### Shared Arguments +The following are brief descriptions of the parameters that Sharded Atlas Checks shares with the normal job + +| Parameter | Description | +|---|---| +| Output Path | Local or remote folder to save outputs to | +| Output Formats | Comma separated list of output types (flags,geojson,metrics,tippecanoe); MapRoulette output is not available in Sharded Checks | +| Countries | Comma separated list of ISO3 country codes of countries to run | +| Configuration File | Comma separated list of resource URIs for checks configuration json files | +| Master | Spark cluster master URL (just `local` for local environments) | + +## Limitations + +#### Limited Input +Currently Sharded Checks only supports sharded Atlas files as an input data source. Eventually this may be expanded to include PBF files. + +#### Large Relations +There is a known issue with large relations being flagged incorrectly. This can occur when a relation spans many shards and even at maximum expansion it cannot all be loaded. + + diff --git a/src/main/java/org/openstreetmap/atlas/checks/base/CheckResourceLoader.java b/src/main/java/org/openstreetmap/atlas/checks/base/CheckResourceLoader.java index c51095048..6f7cebecc 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/base/CheckResourceLoader.java +++ b/src/main/java/org/openstreetmap/atlas/checks/base/CheckResourceLoader.java @@ -60,6 +60,10 @@ public class CheckResourceLoader private final MultiMap countryGroups = new MultiMap<>(); private final Boolean enabledByDefault; private final String enabledKeyTemplate; + private static final String COUNTRY_WHITELIST_TEMPLATE = "%s." + + BaseCheck.PARAMETER_WHITELIST_COUNTRIES; + private static final String COUNTRY_BLACKLIST_TEMPLATE = "%s." + + BaseCheck.PARAMETER_BLACKLIST_COUNTRIES; private final Set packages; private final Optional> checkWhiteList; private final Optional> checkBlackList; @@ -181,9 +185,8 @@ public Set loadChecks() public Set loadChecksForCountry(final String country) { final Configuration countryConfiguration = this.getConfigurationForCountry(country); - return loadChecks( - checkClass -> this.isEnabledByConfiguration(countryConfiguration, checkClass), - countryConfiguration); + return loadChecks(checkClass -> this.isEnabledByConfiguration(countryConfiguration, + checkClass, country), countryConfiguration); } public Set loadChecksUsingConstructors( @@ -310,4 +313,20 @@ private boolean isEnabledByConfiguration(final Configuration configuration, final String key = String.format(this.enabledKeyTemplate, checkClass.getSimpleName()); return configuration.get(key, this.enabledByDefault).value(); } + + private boolean isEnabledByConfiguration(final Configuration configuration, + final Class checkClass, final String country) + { + final List countryWhitelist = configuration + .get(String.format(COUNTRY_WHITELIST_TEMPLATE, checkClass.getSimpleName()), + Collections.emptyList()) + .value(); + final List countryBlacklist = configuration + .get(String.format(COUNTRY_BLACKLIST_TEMPLATE, checkClass.getSimpleName()), + Collections.emptyList()) + .value(); + return this.isEnabledByConfiguration(configuration, checkClass) + && countryWhitelist.isEmpty() ? !countryBlacklist.contains(country) + : countryWhitelist.contains(country); + } } diff --git a/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommand.java b/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommand.java index 2f3789969..bf6180c55 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommand.java +++ b/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommand.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; import com.google.gson.stream.JsonReader; @@ -74,8 +75,8 @@ protected Map, JsonObject>> mapFeatures(final File file) checkFeatureMap.putIfAbsent(checkName, new HashMap<>()); // Add the geoJSON as a value checkFeatureMap.get(checkName) - .put(this.getAtlasIdentifiers(feature.getAsJsonObject().get(PROPERTIES) - .getAsJsonObject().get(FEATURE_PROPERTIES).getAsJsonArray()), + .put(this.getIdentifiers( + feature.getAsJsonObject().get(PROPERTIES).getAsJsonObject()), jsonFeature); }); } @@ -101,17 +102,21 @@ protected void writeSetToGeoJSON(final Map> flags, final } /** - * Get the atlas ids from an array of feature properties. + * Get the unique ids for a flag. Fall back to getting the atlas ids from the feature properties + * for reverse compatibility. * * @param properties - * a {@link JsonArray} of feature properties + * a {@link JsonObject} of a flag * @return a {@link Set} of {@link String} ids */ - private Set getAtlasIdentifiers(final JsonArray properties) + private Set getIdentifiers(final JsonObject properties) { - return Iterables.stream(properties) - .filter(object -> object.getAsJsonObject().has(IDENTIFIER)) - .map(object -> object.getAsJsonObject().get(IDENTIFIER).getAsString()) - .collectToSet(); + return properties.has(IDENTIFIERS) + ? Iterables.stream(properties.get(IDENTIFIERS).getAsJsonArray()) + .map(JsonElement::getAsString).collectToSet() + : Iterables.stream(properties.get(FEATURE_PROPERTIES).getAsJsonArray()) + .filter(object -> object.getAsJsonObject().has(IDENTIFIER)) + .map(object -> object.getAsJsonObject().get(IDENTIFIER).getAsString()) + .collectToSet(); } } diff --git a/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommand.java b/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommand.java index c34a63e30..385946ef9 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommand.java +++ b/src/main/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommand.java @@ -11,6 +11,7 @@ import java.io.InputStreamReader; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.zip.GZIPInputStream; @@ -19,7 +20,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; /** @@ -64,9 +65,12 @@ protected Map, JsonObject>> mapFeatures(final File file) // Add the check name as a key checkFeatureMap.putIfAbsent(checkName, new HashMap<>()); // Add the geoJSON as a value - checkFeatureMap.get(checkName).put( - this.getAtlasIdentifiers(source.get(FEATURES).getAsJsonArray()), - source); + if (checkFeatureMap.get(checkName).containsKey(this.getIdentifiers(source))) + { + logger.info("Duplicate flag found in {}: {}", file.getAbsolutePath(), + source); + } + checkFeatureMap.get(checkName).put(this.getIdentifiers(source), source); } } } @@ -78,19 +82,28 @@ protected Map, JsonObject>> mapFeatures(final File file) } /** - * Get the atlas ids from an array of features. + * Get the unique ids for a flag. Fall back to getting the atlas ids from the features for + * reverse compatibility. * - * @param features - * a {@link JsonArray} of features + * @param flagJson + * a {@link JsonObject} of a flag * @return a {@link Set} of {@link String} ids */ - private Set getAtlasIdentifiers(final JsonArray features) + private Set getIdentifiers(final JsonObject flagJson) { - return Iterables.stream(features) - .filter(object -> object.getAsJsonObject().get(PROPERTIES).getAsJsonObject() - .has(IDENTIFIER)) - .map(object -> object.getAsJsonObject().get(PROPERTIES).getAsJsonObject() - .get(IDENTIFIER).getAsString()) - .collectToSet(); + final JsonObject flagProperties = flagJson.get(PROPERTIES).getAsJsonObject(); + return flagProperties.has(IDENTIFIERS) + ? Iterables.stream(flagProperties.get(IDENTIFIERS).getAsJsonArray()) + .map(JsonElement::getAsString).collectToSet() + : Iterables.stream(flagJson.get(FEATURES).getAsJsonArray()).filter(object -> object + .getAsJsonObject().get(PROPERTIES).getAsJsonObject().has(IDENTIFIER) + || object.getAsJsonObject().get(PROPERTIES).getAsJsonObject().has("ItemId")) + .map(object -> Optional + .ofNullable(object.getAsJsonObject().get(PROPERTIES) + .getAsJsonObject().get(IDENTIFIER)) + .orElse(object.getAsJsonObject().get(PROPERTIES).getAsJsonObject() + .get("ItemId")) + .getAsString()) + .collectToSet(); } } diff --git a/src/main/java/org/openstreetmap/atlas/checks/commands/JSONFlagDiffSubCommand.java b/src/main/java/org/openstreetmap/atlas/checks/commands/JSONFlagDiffSubCommand.java index 7863d9c91..856665817 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/commands/JSONFlagDiffSubCommand.java +++ b/src/main/java/org/openstreetmap/atlas/checks/commands/JSONFlagDiffSubCommand.java @@ -33,6 +33,7 @@ public abstract class JSONFlagDiffSubCommand implements FlexibleSubCommand static final String FEATURE_PROPERTIES = "feature_properties"; static final String GENERATOR = "generator"; static final String NAME = "name"; + static final String IDENTIFIERS = "identifiers"; private static final Command.Switch REFERENCE_FILE_PARAMETER = new Command.Switch<>( "reference", "A file or directory of files containing atlas-checks flags to use as a baseline for comparison.", diff --git a/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityCheckSparkJob.java b/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityCheckSparkJob.java index 9de9eec49..827cfe0a0 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityCheckSparkJob.java +++ b/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityCheckSparkJob.java @@ -1,7 +1,6 @@ package org.openstreetmap.atlas.checks.distributed; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -22,10 +21,10 @@ import org.openstreetmap.atlas.checks.event.CheckFlagFileProcessor; import org.openstreetmap.atlas.checks.event.CheckFlagGeoJsonProcessor; import org.openstreetmap.atlas.checks.event.CheckFlagTippecanoeProcessor; -import org.openstreetmap.atlas.checks.event.EventService; import org.openstreetmap.atlas.checks.event.MetricFileGenerator; import org.openstreetmap.atlas.checks.maproulette.MapRouletteClient; import org.openstreetmap.atlas.checks.maproulette.MapRouletteConfiguration; +import org.openstreetmap.atlas.event.EventService; import org.openstreetmap.atlas.exception.CoreException; import org.openstreetmap.atlas.generator.tools.spark.SparkJob; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; @@ -38,13 +37,10 @@ import org.openstreetmap.atlas.geography.atlas.items.complex.ComplexEntity; import org.openstreetmap.atlas.geography.atlas.items.complex.Finder; import org.openstreetmap.atlas.streaming.resource.FileSuffix; -import org.openstreetmap.atlas.utilities.collections.Iterables; -import org.openstreetmap.atlas.utilities.collections.MultiIterable; import org.openstreetmap.atlas.utilities.collections.StringList; import org.openstreetmap.atlas.utilities.configuration.Configuration; import org.openstreetmap.atlas.utilities.configuration.MergedConfiguration; import org.openstreetmap.atlas.utilities.configuration.StandardConfiguration; -import org.openstreetmap.atlas.utilities.conversion.StringConverter; import org.openstreetmap.atlas.utilities.runtime.CommandMap; import org.openstreetmap.atlas.utilities.scalars.Duration; import org.openstreetmap.atlas.utilities.threads.Pool; @@ -60,68 +56,18 @@ * * @author mgostintsev */ -public class IntegrityCheckSparkJob extends SparkJob +public class IntegrityCheckSparkJob extends IntegrityChecksCommandArguments { - /** - * @author brian_l_davis - */ - private enum OutputFormats - { - FLAGS, - GEOJSON, - METRICS, - TIPPECANOE - } - - private static final Switch ATLAS_FOLDER = new Switch<>("inputFolder", - "Path of folder which contains Atlas file(s)", StringConverter.IDENTITY, - Optionality.OPTIONAL); - // Configuration - private static final Switch CONFIGURATION_FILES = new Switch<>("configFiles", - "Comma-separated list of configuration datasources.", - value -> StringList.split(value, CommonConstants.COMMA), Optionality.OPTIONAL); - private static final Switch CONFIGURATION_JSON = new Switch<>("configJson", - "Json formatted configuration.", StringConverter.IDENTITY, Optionality.OPTIONAL); - private static final Switch COUNTRIES = new Switch<>("countries", - "Comma-separated list of country ISO3 codes to be processed", StringConverter.IDENTITY, - Optionality.REQUIRED); - private static final Switch MAP_ROULETTE = new Switch<>("maproulette", - "Map roulette server information, format :::, projectName is optional.", - MapRouletteConfiguration::parse, Optionality.OPTIONAL); - private static final Switch PBF_BOUNDING_BOX = new Switch<>("pbfBoundingBox", - "OSM protobuf data will be loaded only in this bounding box", Rectangle::forString, - Optionality.OPTIONAL); - private static final Switch PBF_SAVE_INTERMEDIATE_ATLAS = new Switch<>("savePbfAtlas", - "Saves intermediate atlas files created when processing OSM protobuf data.", - Boolean::valueOf, Optionality.OPTIONAL, "false"); - private static final Switch> OUTPUT_FORMATS = new Switch<>("outputFormats", - "Comma-separated list of output formats (flags, metrics, geojson, tippecanoe).", - csv_formats -> Stream.of(csv_formats.split(",")) - .map(format -> Enum.valueOf(OutputFormats.class, format.toUpperCase())) - .collect(Collectors.toSet()), - Optionality.OPTIONAL, "flags,metrics"); - private static final Switch> CHECK_FILTER = new Switch<>("checkFilter", - "Comma-separated list of checks to run", - checks -> Arrays.asList(checks.split(CommonConstants.COMMA)), Optionality.OPTIONAL); + public static final String METRICS_FILENAME = "check-run-time.csv"; // Indicator key for ignored countries private static final String IGNORED_KEY = "Ignored"; - // Outputs - private static final String OUTPUT_FLAG_FOLDER = "flag"; - private static final String OUTPUT_GEOJSON_FOLDER = "geojson"; - private static final String OUTPUT_TIPPECANOE_FOLDER = "tippecanoe"; - private static final String OUTPUT_ATLAS_FOLDER = "atlas"; private static final String INTERMEDIATE_ATLAS_EXTENSION = FileSuffix.ATLAS.toString() + FileSuffix.GZIP.toString(); - private static final String OUTPUT_METRIC_FOLDER = "metric"; - - private static final String METRICS_FILENAME = "check-run-time.csv"; - private static final Logger logger = LoggerFactory.getLogger(IntegrityCheckSparkJob.class); - - private static final long serialVersionUID = 2990087219645942330L; - // Thread pool settings private static final Duration POOL_DURATION_BEFORE_KILL = Duration.minutes(300); + private static final Logger logger = LoggerFactory.getLogger(IntegrityCheckSparkJob.class); + private static final long serialVersionUID = 2990087219645942330L; /** * Main entry point for the Spark job @@ -153,35 +99,11 @@ private static void executeChecks(final String country, final Atlas atlas, { final Pool checkExecutionPool = new Pool(checksToRun.size(), "Check execution pool", POOL_DURATION_BEFORE_KILL); - checksToRun.stream().filter(check -> check.validCheckForCountry(country)) - .forEach(check -> checkExecutionPool.queue(new RunnableCheck(country, check, - new MultiIterable<>(atlas.items(), atlas.relations(), - findComplexEntities(check, atlas)), - MapRouletteClient.instance(configuration)))); + checksToRun.forEach(check -> checkExecutionPool.queue(new RunnableCheck(country, check, + objectsToCheck(atlas, check), MapRouletteClient.instance(configuration)))); checkExecutionPool.close(); } - /** - * Gets complex entities - * - * @param check - * A {@link BaseCheck} object - * @param atlas - * An {@link Atlas} object - * @return An {@link Iterable} of {@link ComplexEntity}s - */ - private static Iterable findComplexEntities(final BaseCheck check, - final Atlas atlas) - { - final Optional finderOptional = check.finder(); - if (finderOptional.isPresent()) - { - return Iterables.stream(finderOptional.get().find(atlas)); - } - - return Collections.emptyList(); - } - private static SparkFilePath initializeOutput(final String output, final TaskContext context, final String country, final String temporaryOutputFolder, final String targetOutputFolder) @@ -211,7 +133,7 @@ public String getName() return "Integrity Check Spark Job"; } - @SuppressWarnings({ "rawtypes" }) + @SuppressWarnings({ "rawtypes", "unchecked" }) @Override public void start(final CommandMap commandMap) { @@ -445,23 +367,6 @@ public void start(final CommandMap commandMap) } } - /** - * Gets the {@link AtlasDataSource} object to load the Atlas from - * - * @param sparkContext - * The Spark context - * @param checksConfiguration - * configuration for all the checks - * @param pbfBoundary - * The pbf boundary of type {@link Rectangle} - * @return A {@link AtlasDataSource} - */ - protected AtlasDataSource getAtlasDataSource(final Map sparkContext, - final Configuration checksConfiguration, final Rectangle pbfBoundary) - { - return new AtlasDataSource(sparkContext, checksConfiguration, pbfBoundary); - } - /** * Defines all the folders to clean before a run * @@ -480,14 +385,6 @@ protected List outputToClean(final CommandMap command) return staticPaths; } - @Override - protected SwitchList switches() - { - return super.switches().with(ATLAS_FOLDER, MAP_ROULETTE, COUNTRIES, CONFIGURATION_FILES, - CONFIGURATION_JSON, PBF_BOUNDING_BOX, PBF_SAVE_INTERMEDIATE_ATLAS, OUTPUT_FORMATS, - CHECK_FILTER); - } - /** * Basic sanity check to ensure we aren't processing an empty list of countries or integrity * checks diff --git a/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityChecksCommandArguments.java b/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityChecksCommandArguments.java new file mode 100644 index 000000000..7a76bd3ea --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/distributed/IntegrityChecksCommandArguments.java @@ -0,0 +1,199 @@ +package org.openstreetmap.atlas.checks.distributed; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.openstreetmap.atlas.checks.atlas.CountrySpecificAtlasFilePathFilter; +import org.openstreetmap.atlas.checks.base.Check; +import org.openstreetmap.atlas.checks.constants.CommonConstants; +import org.openstreetmap.atlas.checks.maproulette.MapRouletteConfiguration; +import org.openstreetmap.atlas.generator.tools.filesystem.FileSystemHelper; +import org.openstreetmap.atlas.generator.tools.spark.SparkJob; +import org.openstreetmap.atlas.geography.Rectangle; +import org.openstreetmap.atlas.geography.atlas.Atlas; +import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; +import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; +import org.openstreetmap.atlas.geography.sharding.Shard; +import org.openstreetmap.atlas.geography.sharding.SlippyTile; +import org.openstreetmap.atlas.utilities.collections.Iterables; +import org.openstreetmap.atlas.utilities.collections.MultiIterable; +import org.openstreetmap.atlas.utilities.collections.StringList; +import org.openstreetmap.atlas.utilities.configuration.Configuration; +import org.openstreetmap.atlas.utilities.conversion.StringConverter; +import org.openstreetmap.atlas.utilities.maps.MultiMap; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Handles arguments and base functionality for integrity check sparkjobs generating commands + * + * @author jklamer + */ +public abstract class IntegrityChecksCommandArguments extends SparkJob +{ + /** + * @author brian_l_davis + */ + protected enum OutputFormats + { + FLAGS, + GEOJSON, + METRICS, + TIPPECANOE + } + + /** + * @deprecated in favor of INPUT from SparkJob + */ + @Deprecated(since = "6.0.2") + protected static final Switch ATLAS_FOLDER = new Switch<>("inputFolder", + "Path of folder which contains Atlas file(s)", StringConverter.IDENTITY, + Optionality.OPTIONAL); + protected static final String OUTPUT_ATLAS_FOLDER = "atlas"; + // Outputs + protected static final String OUTPUT_FLAG_FOLDER = "flag"; + protected static final String OUTPUT_GEOJSON_FOLDER = "geojson"; + protected static final String OUTPUT_METRIC_FOLDER = "metric"; + protected static final String OUTPUT_TIPPECANOE_FOLDER = "tippecanoe"; + static final Switch> CHECK_FILTER = new Switch<>("checkFilter", + "Comma-separated list of checks to run", + checks -> Arrays.asList(checks.split(CommonConstants.COMMA)), Optionality.OPTIONAL); + // Configuration + static final Switch CONFIGURATION_FILES = new Switch<>("configFiles", + "Comma-separated list of configuration datasources.", + value -> StringList.split(value, CommonConstants.COMMA), Optionality.OPTIONAL); + static final Switch CONFIGURATION_JSON = new Switch<>("configJson", + "Json formatted configuration.", StringConverter.IDENTITY, Optionality.OPTIONAL); + static final Switch COUNTRIES = new Switch<>("countries", + "Comma-separated list of country ISO3 codes to be processed", StringConverter.IDENTITY, + Optionality.REQUIRED); + static final Switch MAP_ROULETTE = new Switch<>("maproulette", + "Map roulette server information, format :::, projectName is optional.", + MapRouletteConfiguration::parse, Optionality.OPTIONAL); + static final Switch> OUTPUT_FORMATS = new Switch<>("outputFormats", + "Comma-separated list of output formats (flags, metrics, geojson, tippecanoe).", + csvFormats -> Stream.of(csvFormats.split(",")) + .map(format -> Enum.valueOf(OutputFormats.class, format.toUpperCase())) + .collect(Collectors.toSet()), + Optionality.OPTIONAL, "flags,metrics"); + static final Switch PBF_BOUNDING_BOX = new Switch<>("pbfBoundingBox", + "OSM protobuf data will be loaded only in this bounding box", Rectangle::forString, + Optionality.OPTIONAL); + static final Switch PBF_SAVE_INTERMEDIATE_ATLAS = new Switch<>("savePbfAtlas", + "Saves intermediate atlas files created when processing OSM protobuf data.", + Boolean::valueOf, Optionality.OPTIONAL, "false"); + private static final String ATLAS_FILENAME_PATTERN_FORMAT = "^%s_([0-9]+)-([0-9]+)-([0-9]+)"; + private static final Logger logger = LoggerFactory + .getLogger(IntegrityChecksCommandArguments.class); + private static final long serialVersionUID = 3411367641498888770L; + + /** + * Creates a map from country name to {@link List} of {@link Shard} definitions from + * {@link Atlas} files. + * + * @param countries + * Set of countries to find out shards for + * @param pathResolver + * {@link AtlasFilePathResolver} to search for {@link Atlas} files + * @param atlasFolder + * Path to {@link Atlas} folder + * @param sparkContext + * Spark context (or configuration) as a key-value map + * @return A map from country name to {@link List} of {@link Shard} definitions + */ + public static MultiMap countryShardMapFromShardFiles(final Set countries, + final AtlasFilePathResolver pathResolver, final String atlasFolder, + final Map sparkContext) + { + final MultiMap countryShardMap = new MultiMap<>(); + logger.info("Building country shard map from country shard files."); + + countries.forEach(country -> + { + final String countryDirectory = pathResolver.resolvePath(atlasFolder, country); + final CountrySpecificAtlasFilePathFilter atlasFilter = new CountrySpecificAtlasFilePathFilter( + country); + final Pattern atlasFilePattern = Pattern + .compile(String.format(ATLAS_FILENAME_PATTERN_FORMAT, country)); + + // Go over shard files for the country and use file name pattern to find out shards + FileSystemHelper.listResourcesRecursively(countryDirectory, sparkContext, atlasFilter) + .forEach(shardFile -> + { + final String shardFileName = shardFile.getName(); + final Matcher matcher = atlasFilePattern.matcher(shardFileName); + if (matcher.find()) + { + try + { + final String zoomString = matcher.group(1); + final String xString = matcher.group(2); + final String yString = matcher.group(3); + countryShardMap.add(country, + new SlippyTile(Integer.parseInt(xString), + Integer.parseInt(yString), + Integer.parseInt(zoomString))); + } + catch (final Exception e) + { + logger.warn(String.format("Couldn't parse shard file name %s.", + shardFileName), e); + } + } + else + { + logger.warn(String.format( + "Skipping atlas file %s, its name does not conform to the sharded standard.", + shardFileName)); + } + }); + }); + + return countryShardMap; + } + + protected static Iterable objectsToCheck(final Atlas atlas, final Check check) + { + return objectsToCheck(atlas, check, atlasEntity -> true); + } + + protected static Iterable objectsToCheck(final Atlas atlas, final Check check, + final Predicate geoFilter) + { + return new MultiIterable<>(Iterables.filter(atlas.entities(), geoFilter), + check.finder().map(finder -> finder.find(atlas)).orElse(Collections.emptyList())); + } + + /** + * Gets the {@link AtlasDataSource} object to load the Atlas from + * + * @param sparkContext + * The Spark context + * @param checksConfiguration + * configuration for all the checks + * @param pbfBoundary + * The pbf boundary of type {@link Rectangle} + * @return A {@link AtlasDataSource} + */ + protected AtlasDataSource getAtlasDataSource(final Map sparkContext, + final Configuration checksConfiguration, final Rectangle pbfBoundary) + { + return new AtlasDataSource(sparkContext, checksConfiguration, pbfBoundary); + } + + @Override + protected SwitchList switches() + { + return super.switches().with(ATLAS_FOLDER, MAP_ROULETTE, COUNTRIES, CONFIGURATION_FILES, + CONFIGURATION_JSON, PBF_BOUNDING_BOX, PBF_SAVE_INTERMEDIATE_ATLAS, OUTPUT_FORMATS, + CHECK_FILTER); + } +} diff --git a/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheck.java b/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheck.java index 296667498..f4df75b5d 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheck.java @@ -7,6 +7,7 @@ import org.openstreetmap.atlas.checks.event.MetricEvent; import org.openstreetmap.atlas.checks.flag.CheckFlag; import org.openstreetmap.atlas.checks.maproulette.MapRouletteClient; +import org.openstreetmap.atlas.event.EventService; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.utilities.scalars.Duration; import org.openstreetmap.atlas.utilities.time.Time; @@ -19,6 +20,7 @@ * {@link RunnableCheckBase#eventService} along with {@link MapRouletteClient} for handling. * * @author mkalender + * @author bbreithaupt */ public final class RunnableCheck extends RunnableCheckBase implements Runnable { @@ -42,6 +44,24 @@ public RunnableCheck(final String country, final Check check, super(country, check, objects, client); } + /** + * Default constructor + * + * @param country + * country that is being processed + * @param check + * check that is being executed + * @param objects + * {@link AtlasObject}s that are going to be executed + * @param eventService + * {@link EventService} to post to + */ + public RunnableCheck(final String country, final Check check, + final Iterable objects, final EventService eventService) + { + super(country, check, objects, null, eventService); + } + /** * Runs the {@link Check} over {@link AtlasObject}s, posting resulting {@link CheckFlag}s to * {@link RunnableCheckBase#eventService} and {@link MapRouletteClient} diff --git a/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheckBase.java b/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheckBase.java index 6a568fe15..4c3ba550c 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheckBase.java +++ b/src/main/java/org/openstreetmap/atlas/checks/distributed/RunnableCheckBase.java @@ -2,10 +2,10 @@ import org.apache.commons.lang.StringUtils; import org.openstreetmap.atlas.checks.base.Check; -import org.openstreetmap.atlas.checks.event.EventService; import org.openstreetmap.atlas.checks.flag.CheckFlag; import org.openstreetmap.atlas.checks.maproulette.MapRouletteClient; import org.openstreetmap.atlas.checks.maproulette.data.Challenge; +import org.openstreetmap.atlas.event.EventService; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.utilities.scalars.Duration; import org.openstreetmap.atlas.utilities.threads.Pool; @@ -16,6 +16,7 @@ * Base class to with helper methods for {@link RunnableCheck} * * @author mkalender + * @author bbreithaupt * @param * this would either be a {@link Check} type */ @@ -60,6 +61,27 @@ private static Duration maxDurationForBatch(final int batchSize) */ public RunnableCheckBase(final String country, final T check, final Iterable objects, final MapRouletteClient client) + { + this(country, check, objects, client, EventService.get(country)); + } + + /** + * Default constructor + * + * @param country + * country that is being processed + * @param check + * check that is being executed + * @param objects + * {@link AtlasObject}s that are going to be executed + * @param client + * {@link MapRouletteClient} that will upload the tasks to MapRoulette + * @param eventService + * {@link EventService} to post to + */ + public RunnableCheckBase(final String country, final T check, + final Iterable objects, final MapRouletteClient client, + final EventService eventService) { this.country = country; this.check = check; @@ -71,7 +93,7 @@ public RunnableCheckBase(final String country, final T check, } this.objects = objects; this.client = client; - this.eventService = EventService.get(country); + this.eventService = eventService; } /** diff --git a/src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTask.java b/src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTask.java new file mode 100644 index 000000000..9900d5afb --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTask.java @@ -0,0 +1,52 @@ +package org.openstreetmap.atlas.checks.distributed; + +import java.io.Serializable; +import java.util.List; + +import org.openstreetmap.atlas.checks.base.Check; +import org.openstreetmap.atlas.event.EventService; +import org.openstreetmap.atlas.geography.sharding.Shard; + +/** + * Meta data holder for sharded flag generation task + * + * @author jklamer + */ +public class ShardedCheckFlagsTask implements Serializable +{ + private final List checks; + private final String country; + private final Shard shard; + + public ShardedCheckFlagsTask(final String country, final Shard shard, final List checks) + { + this.country = country; + this.shard = shard; + this.checks = checks; + } + + public List getChecks() + { + return this.checks; + } + + public String getCountry() + { + return this.country; + } + + public EventService getEventService() + { + return EventService.get(this.getUniqueTaskIdentifier()); + } + + public Shard getShard() + { + return this.shard; + } + + public String getUniqueTaskIdentifier() + { + return this.country + "_" + this.shard.getName(); + } +} diff --git a/src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJob.java b/src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJob.java new file mode 100644 index 000000000..1e33e139b --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJob.java @@ -0,0 +1,433 @@ +package org.openstreetmap.atlas.checks.distributed; + +import static org.openstreetmap.atlas.checks.distributed.IntegrityCheckSparkJob.METRICS_FILENAME; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +import org.apache.spark.api.java.JavaPairRDD; +import org.apache.spark.api.java.function.PairFunction; +import org.apache.spark.api.java.function.VoidFunction; +import org.apache.spark.broadcast.Broadcast; +import org.apache.spark.storage.StorageLevel; +import org.openstreetmap.atlas.checks.base.Check; +import org.openstreetmap.atlas.checks.base.CheckResourceLoader; +import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver; +import org.openstreetmap.atlas.checks.constants.CommonConstants; +import org.openstreetmap.atlas.checks.event.CheckFlagEvent; +import org.openstreetmap.atlas.checks.event.CheckFlagFileProcessor; +import org.openstreetmap.atlas.checks.event.CheckFlagGeoJsonProcessor; +import org.openstreetmap.atlas.checks.event.CheckFlagTippecanoeProcessor; +import org.openstreetmap.atlas.checks.event.MetricFileGenerator; +import org.openstreetmap.atlas.checks.utility.UniqueCheckFlagContainer; +import org.openstreetmap.atlas.event.EventService; +import org.openstreetmap.atlas.event.Processor; +import org.openstreetmap.atlas.event.ShutdownEvent; +import org.openstreetmap.atlas.exception.CoreException; +import org.openstreetmap.atlas.generator.sharding.AtlasSharding; +import org.openstreetmap.atlas.generator.tools.caching.HadoopAtlasFileCache; +import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; +import org.openstreetmap.atlas.geography.atlas.Atlas; +import org.openstreetmap.atlas.geography.atlas.AtlasResourceLoader; +import org.openstreetmap.atlas.geography.atlas.dynamic.DynamicAtlas; +import org.openstreetmap.atlas.geography.atlas.dynamic.policy.DynamicAtlasPolicy; +import org.openstreetmap.atlas.geography.atlas.multi.MultiAtlas; +import org.openstreetmap.atlas.geography.sharding.Shard; +import org.openstreetmap.atlas.geography.sharding.Sharding; +import org.openstreetmap.atlas.utilities.collections.StringList; +import org.openstreetmap.atlas.utilities.configuration.Configuration; +import org.openstreetmap.atlas.utilities.configuration.MergedConfiguration; +import org.openstreetmap.atlas.utilities.configuration.StandardConfiguration; +import org.openstreetmap.atlas.utilities.conversion.StringConverter; +import org.openstreetmap.atlas.utilities.filters.AtlasEntityPolygonsFilter; +import org.openstreetmap.atlas.utilities.maps.MultiMap; +import org.openstreetmap.atlas.utilities.runtime.CommandMap; +import org.openstreetmap.atlas.utilities.scalars.Distance; +import org.openstreetmap.atlas.utilities.threads.Pool; +import org.openstreetmap.atlas.utilities.time.Time; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.eventbus.AllowConcurrentEvents; +import com.google.common.eventbus.Subscribe; + +import scala.Serializable; +import scala.Tuple2; + +/** + * A spark job for generating integrity checks in a sharded fashion. This allows for a lower local + * memory profile as well as better parallelization.
+ * This implementation currently only supports Atlas files as an input data source. They must be in + * the structure: folder/iso_code/iso_z-x-y.atlas
+ * Also required is a reference for how the Atlases are sharded. This can be provided as a + * sharding.txt file in the input path, or through the {@code sharding} argument. + * + * @author jklamer + * @author bbreithaupt + */ +public class ShardedIntegrityChecksSparkJob extends IntegrityChecksCommandArguments +{ + private static final Switch EXPANSION_DISTANCE = new Switch<>("shardBufferDistance", + "Distance to expand the bounds of the shard group to create a network in kilometers", + distanceString -> Distance.kilometers(Double.valueOf(distanceString)), + Optionality.OPTIONAL, "10.0"); + private static final String ATLAS_SHARDING_FILE = "sharding.txt"; + private static final Switch SHARDING = new Switch<>("sharding", + "Sharding to load in place of sharding file in Atlas path", StringConverter.IDENTITY, + Optionality.OPTIONAL); + private static final Switch MULTI_ATLAS = new Switch<>("multiAtlas", + "If true then use a multi atlas, else use a dynamic atlas. This works better for running on a single machine", + Boolean::getBoolean, Optionality.OPTIONAL, "false"); + private static final Switch SPARK_STORAGE_DISK_ONLY = new Switch<>( + "sparkStorageDiskOnly", "Store cached RDDs exclusively on disk", + StringConverter.IDENTITY, Optionality.OPTIONAL); + + private static final Logger logger = LoggerFactory + .getLogger(ShardedIntegrityChecksSparkJob.class); + private static final long serialVersionUID = -8038802870994470017L; + + private final MultiMap countryChecks = new MultiMap<>(); + + public static void main(final String[] args) + { + new ShardedIntegrityChecksSparkJob().run(args); + } + + @Override + public String getName() + { + return "Sharded Integrity Checks Spark Job"; + } + + @Override + public void start(final CommandMap commandMap) + { + final Time start = Time.now(); + final String atlasDirectory = (String) commandMap.get(ATLAS_FOLDER); + final String input = Optional.ofNullable(input(commandMap)).orElse(atlasDirectory); + + // Gather arguments + final String output = output(commandMap); + @SuppressWarnings("unchecked") + final Set outputFormats = (Set) commandMap + .get(OUTPUT_FORMATS); + final StringList countries = StringList.split((String) commandMap.get(COUNTRIES), + CommonConstants.COMMA); + @SuppressWarnings("unchecked") + final Optional> checkFilter = (Optional>) commandMap + .getOption(CHECK_FILTER); + + final Configuration checksConfiguration = new MergedConfiguration(Stream + .concat(Stream.of(ConfigurationResolver.loadConfiguration(commandMap, + CONFIGURATION_FILES, CONFIGURATION_JSON)), + Stream.of(checkFilter + . map(whitelist -> new StandardConfiguration( + "WhiteListConfiguration", + Collections.singletonMap( + "CheckResourceLoader.checks.whitelist", whitelist))) + .orElse(ConfigurationResolver.emptyConfiguration()))) + .collect(Collectors.toList())); + + final Map sparkContext = configurationMap(); + + // File loading helpers + final AtlasFilePathResolver resolver = new AtlasFilePathResolver(checksConfiguration); + final SparkFileHelper fileHelper = new SparkFileHelper(sparkContext); + final CheckResourceLoader checkLoader = new CheckResourceLoader(checksConfiguration); + + // Spark storage argument + final StorageLevel storageLevel = Optional + .ofNullable(commandMap.get(SPARK_STORAGE_DISK_ONLY)).orElse("false").equals("true") + ? StorageLevel.DISK_ONLY() + : StorageLevel.MEMORY_AND_DISK(); + + // Get sharding + @SuppressWarnings("unchecked") + final Optional alternateShardingFile = (Optional) commandMap + .getOption(SHARDING); + final String shardingPathInAtlas = "dynamic@" + + SparkFileHelper.combine(input, ATLAS_SHARDING_FILE); + final String shardingFilePath = alternateShardingFile.orElse(shardingPathInAtlas); + final Sharding sharding = AtlasSharding.forString(shardingFilePath, + this.configurationMap()); + final Broadcast shardingBroadcast = this.getContext().broadcast(sharding); + final Distance distanceToLoadShards = (Distance) commandMap.get(EXPANSION_DISTANCE); + + // Check inputs + if (countries.isEmpty()) + { + throw new CoreException("No countries found to run."); + } + for (final String country : countries) + { + final Set checksLoadedForCountry = checkLoader.loadChecksForCountry(country); + if (checksLoadedForCountry.isEmpty()) + { + logger.warn("No checks loaded for country {}. Skipping execution", country); + } + else + { + checksLoadedForCountry.forEach(check -> this.countryChecks.add(country, check)); + } + } + if (this.countryChecks.isEmpty()) + { + throw new CoreException("No checks loaded for any of the countries provided."); + } + + // Find the shards for each country atlas files + final MultiMap countryShards = countryShardMapFromShardFiles( + countries.stream().collect(Collectors.toSet()), resolver, input, sparkContext); + if (countryShards.isEmpty()) + { + throw new CoreException("No atlas files found in input."); + } + + if (!countries.stream().allMatch(countryShards::containsKey)) + { + final Set missingCountries = countries.stream() + .filter(aCountry -> !countryShards.containsKey(aCountry)) + .collect(Collectors.toSet()); + throw new CoreException( + "Unable to find standardized named shard files in the path {}/ for the countries {}. \n Files must be in format ___.atlas", + input, missingCountries); + } + + // List of spark RDDS/tasks + final List> countryRdds = new ArrayList<>(); + + // Countrify spark parallelization for better debugging + try (Pool checkPool = new Pool(countryShards.size(), "Countries Execution Pool")) + { + for (final Map.Entry> countryShard : countryShards.entrySet()) + { + checkPool.queue(() -> + { + // Generate a task for each shard + final List tasksForCountry = countryShard.getValue() + .stream() + .map(shard -> new ShardedCheckFlagsTask(countryShard.getKey(), shard, + this.countryChecks.get(countryShard.getKey()))) + .collect(Collectors.toList()); + + // Set spark UI job title + this.getContext().setJobGroup("0", + String.format("Running checks on %s", + tasksForCountry.get(0).getCountry())); + // Run each task in spark, producing UniqueCheckFlagContainers + final JavaPairRDD rdd = this.getContext() + .parallelize(tasksForCountry, tasksForCountry.size()) + .mapToPair(produceFlags(input, output, this.configurationMap(), + fileHelper, shardingBroadcast, distanceToLoadShards, + (Boolean) commandMap.get(MULTI_ATLAS))); + // Save the RDD + rdd.persist(storageLevel); + // Use a fast spark action to overcome spark lazy elocution. This is necessary + // to get the UI title to appear in the right spot. + rdd.count(); + // Add the RDDs to the list + countryRdds.add(rdd); + }); + } + } + + // Set up for unioning + final JavaPairRDD firstCountryRdd = countryRdds.get(0); + countryRdds.remove(0); + + // Add UI title + this.getContext().setJobGroup("0", "Conflate flags and generate outputs"); + // union the RDDs, combine the UniqueCheckFlagContainers by country, and proccess the flags + // through the output event service + this.getContext().union(firstCountryRdd, countryRdds) + .reduceByKey(UniqueCheckFlagContainer::combine) + .foreach(processFlags(output, fileHelper, outputFormats)); + + logger.info("Sharded checks completed in {}", start.elapsedSince()); + } + + @Override + protected SwitchList switches() + { + return super.switches().with(EXPANSION_DISTANCE, MULTI_ATLAS, SPARK_STORAGE_DISK_ONLY, + SHARDING); + } + + /** + * Get the fetcher to use for Atlas files. The fetcher uses a hadoop cache to reduce remote + * reads. + * + * @param input + * {@link String} input folder path + * @param country + * {@link String} country code + * @param configuration + * {@link org.openstreetmap.atlas.generator.tools.spark.SparkJob} configuration map + * @return {@link Function} that fetches atlases/ + */ + private Function> atlasFetcher(final String input, final String country, + final Map configuration) + { + final HadoopAtlasFileCache cache = new HadoopAtlasFileCache(input, configuration); + final AtlasResourceLoader loader = new AtlasResourceLoader(); + return (Function> & Serializable) shard -> cache.get(country, shard) + .map(loader::load); + } + + /** + * Process {@link org.openstreetmap.atlas.checks.flag.CheckFlag}s through an event service to + * produce output files. + * + * @param output + * {@link String} output folder path + * @param fileHelper + * {@link SparkFileHelper} + * @param outputFormats + * {@link Set} of + * {@link org.openstreetmap.atlas.checks.distributed.IntegrityChecksCommandArguments.OutputFormats} + * @return {@link VoidFunction} that takes a {@link Tuple2} of a {@link String} country code and + * a {@link UniqueCheckFlagContainer} + */ + @SuppressWarnings("unchecked") + private VoidFunction> processFlags(final String output, + final SparkFileHelper fileHelper, final Set outputFormats) + { + return tuple -> + { + final String country = tuple._1(); + final UniqueCheckFlagContainer flagContainer = tuple._2(); + final EventService eventService = EventService.get(country); + + if (outputFormats.contains(OutputFormats.FLAGS)) + { + eventService.register(new CheckFlagFileProcessor(fileHelper, + SparkFileHelper.combine(output, OUTPUT_FLAG_FOLDER, country))); + } + + if (outputFormats.contains(OutputFormats.GEOJSON)) + { + + eventService.register(new CheckFlagGeoJsonProcessor(fileHelper, + SparkFileHelper.combine(output, OUTPUT_GEOJSON_FOLDER, country))); + } + + if (outputFormats.contains(OutputFormats.TIPPECANOE)) + { + eventService.register(new CheckFlagTippecanoeProcessor(fileHelper, + SparkFileHelper.combine(output, OUTPUT_TIPPECANOE_FOLDER, country))); + } + + flagContainer.reconstructEvents().parallel().forEach(eventService::post); + eventService.complete(); + }; + } + + /** + * {@link PairFunction} to run each {@link ShardedCheckFlagsTask} through to produce + * {@link org.openstreetmap.atlas.checks.flag.CheckFlag}s. + * + * @param input + * {@link String} input folder path + * @param output + * {@link String} output folder path + * @param configurationMap + * {@link org.openstreetmap.atlas.generator.tools.spark.SparkJob} configuration map + * @param fileHelper + * {@link SparkFileHelper} + * @param sharding + * spark {@link Broadcast} of the current {@link Sharding} + * @param shardDistanceExpansion + * {@link Distance} to expand the shard group + * @param multiAtlas + * boolean whether to use a multi or dynamic Atlas + * @return {@link PairFunction} that takes {@link ShardedCheckFlagsTask} and returns a + * {@link Tuple2} of a {@link String} country code and {@link UniqueCheckFlagContainer} + */ + @SuppressWarnings("unchecked") + private PairFunction produceFlags( + final String input, final String output, final Map configurationMap, + final SparkFileHelper fileHelper, final Broadcast sharding, + final Distance shardDistanceExpansion, final boolean multiAtlas) + { + return task -> + { + // Get the atlas + final Function> fetcher = this.atlasFetcher(input, + task.getCountry(), configurationMap); + final Atlas atlas; + + // Use dynamic or multi atlas (multi runs faster locally) + if (multiAtlas) + { + atlas = new MultiAtlas( + StreamSupport + .stream(sharding.getValue() + .shards(task.getShard().bounds() + .expand(shardDistanceExpansion)) + .spliterator(), true) + .map(fetcher).filter(Optional::isPresent).map(Optional::get) + .collect(Collectors.toList())); + } + else + { + final DynamicAtlasPolicy policy = new DynamicAtlasPolicy(fetcher, + sharding.getValue(), Collections.singleton(task.getShard()), + task.getShard().bounds().expand(shardDistanceExpansion)) + .withDeferredLoading(true).withAggressivelyExploreRelations(true) + .withExtendIndefinitely(false); + atlas = new DynamicAtlas(policy); + ((DynamicAtlas) atlas).preemptiveLoad(); + } + + final AtlasEntityPolygonsFilter boundaryFilter = AtlasEntityPolygonsFilter.Type.INCLUDE + .polygons(Collections.singleton(task.getShard().bounds())); + + // Prepare the event service + final EventService eventService = task.getEventService(); + final UniqueCheckFlagContainer container = new UniqueCheckFlagContainer(); + eventService.register(new Processor() + { + @Override + public void process(final ShutdownEvent event) + { + // no-op + } + + @Override + @Subscribe + @AllowConcurrentEvents + public void process(final CheckFlagEvent event) + { + container.add(event.getCheckName(), event.getCheckFlag().makeComplete()); + } + }); + // Metrics are output on a per shard level + final MetricFileGenerator metricFileGenerator = new MetricFileGenerator( + task.getShard().getName() + "_" + METRICS_FILENAME, fileHelper, + SparkFileHelper.combine(output, OUTPUT_METRIC_FOLDER, task.getCountry())); + eventService.register(metricFileGenerator); + + // Run all checks in parallel + try (Pool checkPool = new Pool(task.getChecks().size(), + "Sharded Checks Execution Pool")) + { + for (final Check check : task.getChecks()) + { + checkPool.queue(new RunnableCheck(task.getCountry(), check, + objectsToCheck(atlas, check, boundaryFilter), eventService)); + } + } + + eventService.complete(); + return new Tuple2<>(task.getCountry(), container); + }; + } +} diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagEvent.java b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagEvent.java index 7faa0246d..c59d0d1f8 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagEvent.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagEvent.java @@ -18,19 +18,21 @@ import org.openstreetmap.atlas.checks.flag.CheckFlag; import org.openstreetmap.atlas.checks.flag.FlaggedObject; import org.openstreetmap.atlas.checks.flag.FlaggedRelation; +import org.openstreetmap.atlas.event.Event; import org.openstreetmap.atlas.geography.geojson.GeoJsonBuilder; import org.openstreetmap.atlas.geography.geojson.GeoJsonBuilder.GeometryWithProperties; import org.openstreetmap.atlas.geography.geojson.GeoJsonObject; import org.openstreetmap.atlas.tags.HighwayTag; +import com.google.gson.Gson; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonPrimitive; /** - * Wraps a {@link CheckFlag} for submission to the {@link EventService} for handling {@link Check} - * results + * Wraps a {@link CheckFlag} for submission to the + * {@link org.openstreetmap.atlas.event.EventService} for handling {@link Check} results * * @author mkalender, bbreithaupt */ @@ -42,6 +44,9 @@ public final class CheckFlagEvent extends Event private static final String FEATURES = "features"; private static final String FEATURE_COLLECTION = "FeatureCollection"; private static final String INSTRUCTIONS = "instructions"; + private static final String IDENTIFIERS = "identifiers"; + + private static final Gson GSON = new Gson(); private final String checkName; private final CheckFlag flag; @@ -143,6 +148,7 @@ else if (flaggedRelations.size() != 1 && !feature.has(GEOMETRY)) flagProperties.add("feature_properties", featureProperties); flagProperties.add("feature_osmids", uniqueFeatureOsmIds); flagProperties.addProperty("feature_count", featureProperties.size()); + flagProperties.add(IDENTIFIERS, GSON.toJsonTree(flag.getUniqueIdentifiers())); feature.addProperty("id", flag.getIdentifier()); feature.add("properties", flagProperties); @@ -189,6 +195,7 @@ public static JsonObject flagToJson(final CheckFlag flag, final JsonObject flagPropertiesJson = new JsonObject(); flagPropertiesJson.addProperty("id", flag.getIdentifier()); flagPropertiesJson.addProperty(INSTRUCTIONS, flag.getInstructions()); + flagPropertiesJson.add(IDENTIFIERS, GSON.toJsonTree(flag.getUniqueIdentifiers())); // Add additional properties additionalProperties.forEach(flagPropertiesJson::addProperty); diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessor.java b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessor.java index e92cb3e2f..bb6d09028 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessor.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessor.java @@ -1,5 +1,6 @@ package org.openstreetmap.atlas.checks.event; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -8,7 +9,7 @@ import com.google.common.eventbus.Subscribe; /** - * A {@link Processor} for {@link CheckFlagEvent}s to write them into line delimited GeoJson files + * A {@link } for {@link CheckFlagEvent}s to write them into line delimited GeoJson files * * @author mkalender */ diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessor.java b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessor.java index 615861760..a4b25b5c3 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessor.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessor.java @@ -8,6 +8,8 @@ import org.openstreetmap.atlas.checks.constants.CommonConstants; import org.openstreetmap.atlas.checks.distributed.GeoJsonPathFilter; +import org.openstreetmap.atlas.event.Processor; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessor.java b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessor.java index 01b40532a..dc2cc6d94 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessor.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessor.java @@ -46,7 +46,7 @@ public void process(final CheckFlagEvent event) @Override @Subscribe - public void process(final ShutdownEvent event) + public void process(final org.openstreetmap.atlas.event.ShutdownEvent event) { try { diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/Event.java b/src/main/java/org/openstreetmap/atlas/checks/event/Event.java deleted file mode 100644 index 72a77cf4e..000000000 --- a/src/main/java/org/openstreetmap/atlas/checks/event/Event.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -import java.util.Date; - -/** - * Useful base class to hold common information for {@link Event} implementations - * - * @deprecated - use {@link org.openstreetmap.atlas.event.Event}. - * @author mkalender - */ -@Deprecated -public abstract class Event -{ - private final Date timestamp; - - /** - * Default constructor - */ - protected Event() - { - this.timestamp = new Date(); - } - - protected Date getTimestamp() - { - return this.timestamp; - } -} diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/EventService.java b/src/main/java/org/openstreetmap/atlas/checks/event/EventService.java deleted file mode 100644 index 12565c229..000000000 --- a/src/main/java/org/openstreetmap/atlas/checks/event/EventService.java +++ /dev/null @@ -1,122 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -import java.util.Collection; -import java.util.HashSet; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicBoolean; - -import org.openstreetmap.atlas.utilities.threads.Pool; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.eventbus.EventBus; - -/** - * A simple in-memory publish-subscribe service built on top of {@link EventBus} - * - * @deprecated - use {@link org.openstreetmap.atlas.event.EventService} - * @author mkalender - */ -@Deprecated -public final class EventService -{ - private static final Logger logger = LoggerFactory.getLogger(EventService.class); - - // A key-value mapping for multiple event services - private static Map serviceMap = new ConcurrentHashMap<>(); - - // Event bus to dispatch events - private final EventBus eventBus; - - // Container of processors on the EventService - private final Collection> processors = new HashSet<>(); - - // Thread-safe complete indicator - private final AtomicBoolean completed = new AtomicBoolean(); - - /** - * @param key - * key to retrieve {@link EventService} - * @return {@link EventService} instance for given key - */ - public static EventService get(final String key) - { - serviceMap.putIfAbsent(key, new EventService()); - return serviceMap.get(key); - } - - private EventService() - { - this.eventBus = new EventBus((exception, context) -> logger - .warn("An exception is thrown in EventBus.", exception)); - } - - /** - * Stops event processing {@link Pool} and posts a {@link ShutdownEvent} event - */ - public void complete() - { - if (!this.completed.compareAndSet(false, true)) - { - logger.warn("EventService is already completed. Skipping completion."); - return; - } - - this.eventBus.post(new ShutdownEvent()); - new HashSet<>(this.processors).forEach(this::unregister); - } - - /** - * Publishes/posts a new event {@link Object} - * - * @param - * event type that is going to be posted - * @param event - * {@link Object} to post - */ - public void post(final T event) - { - if (event == null) - { - logger.warn("EventService received a null event. Skipping posting."); - return; - } - - if (this.completed.get()) - { - logger.warn("EventService is already completed. Skipping posting."); - return; - } - - this.eventBus.post(event); - } - - /** - * Registers given {@link Processor} to subscribe for events - * - * @param - * processor event type that is going to be registered - * @param processor - * {@link Processor} to register - */ - public void register(final Processor processor) - { - this.eventBus.register(processor); - this.processors.add(processor); - } - - /** - * Unregisters given {@link Processor} - * - * @param - * processor event type that is going to be registered - * @param processor - * {@link Processor} to unregister - */ - public void unregister(final Processor processor) - { - this.eventBus.unregister(processor); - this.processors.remove(processor); - } -} diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/FileProcessor.java b/src/main/java/org/openstreetmap/atlas/checks/event/FileProcessor.java index 7241d7e70..6cf28956b 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/FileProcessor.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/FileProcessor.java @@ -6,6 +6,8 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import org.openstreetmap.atlas.checks.distributed.LogFilePathFilter; +import org.openstreetmap.atlas.event.Event; +import org.openstreetmap.atlas.event.Processor; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/MetricEvent.java b/src/main/java/org/openstreetmap/atlas/checks/event/MetricEvent.java index 3f83c4706..0aadd8539 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/MetricEvent.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/MetricEvent.java @@ -9,7 +9,7 @@ * * @author mkalender */ -public final class MetricEvent extends Event +public final class MetricEvent extends org.openstreetmap.atlas.event.Event { private final String name; private final Duration duration; diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/MetricFileGenerator.java b/src/main/java/org/openstreetmap/atlas/checks/event/MetricFileGenerator.java index c87f775d1..e0870cb12 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/event/MetricFileGenerator.java +++ b/src/main/java/org/openstreetmap/atlas/checks/event/MetricFileGenerator.java @@ -13,10 +13,10 @@ import com.google.common.eventbus.Subscribe; /** - * A {@link Processor} for {@link MetricEvent}s to write them into files. By default this supports - * maximum {@code FileProcessor.BATCH_SIZE} metrics. If number of metrics go beyond that limit, the - * newest metrics will override the previous ones in the file, because the file name is going to be - * the same for both write operations. + * A {@link org.openstreetmap.atlas.event.Processor} for {@link MetricEvent}s to write them into + * files. By default this supports maximum {@code FileProcessor.BATCH_SIZE} metrics. If number of + * metrics go beyond that limit, the newest metrics will override the previous ones in the file, + * because the file name is going to be the same for both write operations. * * @author mkalender */ @@ -69,7 +69,7 @@ public void process(final MetricEvent event) @Override @Subscribe - public void process(final ShutdownEvent event) + public void process(final org.openstreetmap.atlas.event.ShutdownEvent event) { try { diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/Processor.java b/src/main/java/org/openstreetmap/atlas/checks/event/Processor.java deleted file mode 100644 index 40f35a8e7..000000000 --- a/src/main/java/org/openstreetmap/atlas/checks/event/Processor.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -/** - * The {@link Processor} interface provides simple hooks for implementations to handle events. - * - * @deprecated - use {@link org.openstreetmap.atlas.event.Processor} - * @author mkalender - * @param - * Type that is going to be processed - */ -@Deprecated -public interface Processor -{ - /** - * Method to process {@link ShutdownEvent}. This method will be called only once.
- * Please make sure to add {@code @Subscribe} annotation to the method that is - * implementing this method. - * - * @param event - * {@link ShutdownEvent} to process - */ - void process(ShutdownEvent event); - - /** - * Method to process {@link Event}. If your method can process multiple events simultaneously, - * then mark your method with {@code @AllowConcurrentEvents} annotation.Please make sure - * to add {@code @Subscribe} annotation to the method that is implementing this method. - * - * @param event - * {@link Event} to process - */ - void process(T event); -} diff --git a/src/main/java/org/openstreetmap/atlas/checks/event/ShutdownEvent.java b/src/main/java/org/openstreetmap/atlas/checks/event/ShutdownEvent.java deleted file mode 100644 index 9ec549c04..000000000 --- a/src/main/java/org/openstreetmap/atlas/checks/event/ShutdownEvent.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -/** - * An {@link Event} that is posted when {@link EventService} is shutting down - * - * @deprecated - use {@link org.openstreetmap.atlas.event.ShutdownEvent} - * @author mkalender - */ -@Deprecated -public class ShutdownEvent extends Event -{ -} diff --git a/src/main/java/org/openstreetmap/atlas/checks/flag/CheckFlag.java b/src/main/java/org/openstreetmap/atlas/checks/flag/CheckFlag.java index b1894b07d..9ddae044c 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/flag/CheckFlag.java +++ b/src/main/java/org/openstreetmap/atlas/checks/flag/CheckFlag.java @@ -5,6 +5,7 @@ import java.io.Serializable; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; @@ -48,18 +49,18 @@ * @author cuthbertm * @author mgostintsev * @author brian_l_davis + * @author bbreithaupt */ public class CheckFlag implements Iterable, Located, Serializable { - private static final long serialVersionUID = -1287808902452203852L; - private static final Logger logger = LoggerFactory.getLogger(CheckFlag.class); - private static final Distance TEN_METERS = Distance.meters(10); - - private final String identifier; + private static final String NULL_IDENTIFIERS = "nullnull"; + private static final Logger logger = LoggerFactory.getLogger(CheckFlag.class); + private static final long serialVersionUID = -1287808902452203852L; private String challengeName = null; + private Set flaggedObjects = new LinkedHashSet<>(); + private final String identifier; private final List instructions = new ArrayList<>(); - private final Set flaggedObjects = new LinkedHashSet<>(); /** * A basic constructor that simply flags some identifying value @@ -438,6 +439,23 @@ public Iterable> getShapes() .map(polyLine -> (Iterable) polyLine).collect(Collectors.toList())); } + /** + * Generates an id {@link Set} for unique flag identification. The set is comprised of the item + * type + atlas id of the flagged objects. If there are no objects with atlas ids then the set + * only contains the check flag id. + * + * @return a {@link Set} of the unique ids + */ + public Set getUniqueIdentifiers() + { + final Set flaggedObjectIdentifiers = this.flaggedObjects.stream() + .map(object -> object.getProperties().get(FlaggedObject.ITEM_TYPE_TAG) + + object.getProperties().get(FlaggedObject.ITEM_IDENTIFIER_TAG)) + .filter(string -> !string.equals(NULL_IDENTIFIERS)).collect(Collectors.toSet()); + return flaggedObjectIdentifiers.isEmpty() ? Collections.singleton(this.identifier) + : flaggedObjectIdentifiers; + } + @Override public int hashCode() { @@ -451,6 +469,22 @@ public Iterator iterator() return new MultiIterable<>(getShapes()).iterator(); } + /** + * Decouple the {@link CheckFlag} from any + * {@link org.openstreetmap.atlas.geography.atlas.Atlas}s by making all the + * {@link FlaggedObject}s complete. + * + * @return this + */ + public CheckFlag makeComplete() + { + final LinkedHashSet completeFlaggedObjects = new LinkedHashSet<>(); + this.flaggedObjects.forEach(flaggedObject -> completeFlaggedObjects + .add(flaggedObject.getAsCompleteFlaggedObject())); + this.flaggedObjects = completeFlaggedObjects; + return this; + } + /** * Writes the string value of this {@link CheckFlag} to the {@link WritableResource} * diff --git a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedObject.java b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedObject.java index af17e1019..e759194a8 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedObject.java +++ b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedObject.java @@ -3,6 +3,7 @@ import java.io.Serializable; import java.util.Map; import java.util.Objects; +import java.util.Optional; import org.openstreetmap.atlas.geography.Located; import org.openstreetmap.atlas.geography.Location; @@ -17,14 +18,14 @@ */ public abstract class FlaggedObject implements Serializable, Located { - protected static final String COUNTRY_MISSING = "NA"; protected static final String AREA_TAG = "Area"; + protected static final String COUNTRY_MISSING = "NA"; protected static final String EDGE_TAG = "Edge"; - protected static final String OSM_IDENTIFIER_TAG = "osmIdentifier"; protected static final String ITEM_IDENTIFIER_TAG = "identifier"; protected static final String ITEM_TYPE_TAG = "itemType"; protected static final String LINE_TAG = "Line"; protected static final String NODE_TAG = "Node"; + protected static final String OSM_IDENTIFIER_TAG = "osmIdentifier"; protected static final String POINT_TAG = "Point"; private static final long serialVersionUID = -2898518269816777421L; @@ -56,6 +57,13 @@ public boolean equals(final Object other) && Objects.equals(this.getProperties(), otherObject.getProperties()); } + /** + * Return either itself or a copy of itself where there is no memory reference to an Atlas. + * + * @return Flagged Object with no memory reference to an Atlas + */ + public abstract FlaggedObject getAsCompleteFlaggedObject(); + /** * @return the flagged object's country code */ @@ -87,4 +95,6 @@ public int hashCode() { return Objects.hash(this.getCountry(), this.getGeometry(), this.getProperties()); } + + protected abstract Optional getObject(); } diff --git a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPoint.java b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPoint.java index 3f4fbad73..984b7c38e 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPoint.java +++ b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPoint.java @@ -2,9 +2,13 @@ import java.util.Collections; import java.util.Map; +import java.util.Optional; import org.openstreetmap.atlas.geography.Location; import org.openstreetmap.atlas.geography.Rectangle; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteNode; +import org.openstreetmap.atlas.geography.atlas.complete.CompletePoint; +import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.geography.atlas.items.LocationItem; import org.openstreetmap.atlas.geography.atlas.items.Node; import org.openstreetmap.atlas.geography.atlas.items.Point; @@ -21,13 +25,12 @@ */ public class FlaggedPoint extends FlaggedObject { - private static final long serialVersionUID = -5912453173756416690L; private static final Logger logger = LoggerFactory.getLogger(FlaggedPoint.class); + private static final long serialVersionUID = -5912453173756416690L; + private LocationItem locationItem; private final Location point; private final Map properties; - private final LocationItem locationItem; - @SuppressWarnings("unchecked") public FlaggedPoint(final Location point) { @@ -83,6 +86,20 @@ public Rectangle bounds() } } + @Override + public FlaggedObject getAsCompleteFlaggedObject() + { + if (this.locationItem instanceof Point) + { + this.locationItem = CompletePoint.from((Point) this.locationItem); + } + else if (this.locationItem instanceof Node) + { + this.locationItem = CompleteNode.from((Node) this.locationItem); + } + return this; + } + @Override public Iterable getGeometry() { @@ -96,6 +113,12 @@ public Map getProperties() return this.properties; } + @Override + protected Optional getObject() + { + return Optional.ofNullable(this.locationItem); + } + @SuppressWarnings("unchecked") private Map initProperties(final LocationItem locationItem) { diff --git a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPolyline.java b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPolyline.java index 0ebfaa4aa..d19b7cd92 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPolyline.java +++ b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedPolyline.java @@ -1,10 +1,15 @@ package org.openstreetmap.atlas.checks.flag; import java.util.Map; +import java.util.Optional; +import org.openstreetmap.atlas.exception.CoreException; import org.openstreetmap.atlas.geography.Location; import org.openstreetmap.atlas.geography.PolyLine; import org.openstreetmap.atlas.geography.Rectangle; +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.items.Area; import org.openstreetmap.atlas.geography.atlas.items.AtlasItem; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; @@ -70,6 +75,24 @@ public Rectangle bounds() return this.atlasItem.bounds(); } + @Override + public FlaggedObject getAsCompleteFlaggedObject() + { + if (this.atlasItem instanceof Area) + { + return new FlaggedPolyline(CompleteArea.from((Area) this.atlasItem)); + } + else if (this.atlasItem instanceof Edge) + { + return new FlaggedPolyline(CompleteEdge.from((Edge) this.atlasItem)); + } + else if (this.atlasItem instanceof Line) + { + return new FlaggedPolyline(CompleteLine.from((Line) this.atlasItem)); + } + throw new CoreException("FlaggedPolyline has improper Atlas Item {}", this.atlasItem); + } + @Override public String getCountry() { @@ -88,6 +111,12 @@ public Map getProperties() return this.properties; } + @Override + protected Optional getObject() + { + return Optional.ofNullable(this.atlasItem); + } + private String initCountry(final AtlasObject object) { final Map tags = object.getTags(); diff --git a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedRelation.java b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedRelation.java index fb3faafa1..9d25ad5e3 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedRelation.java +++ b/src/main/java/org/openstreetmap/atlas/checks/flag/FlaggedRelation.java @@ -1,16 +1,23 @@ package org.openstreetmap.atlas.checks.flag; import java.util.Map; +import java.util.Optional; +import org.openstreetmap.atlas.exception.CoreException; import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.MultiPolygon; import org.openstreetmap.atlas.geography.Rectangle; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteRelation; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; import org.openstreetmap.atlas.geography.atlas.items.Relation; import org.openstreetmap.atlas.geography.atlas.items.RelationMemberList; +import org.openstreetmap.atlas.geography.atlas.items.complex.RelationOrAreaToMultiPolygonConverter; import org.openstreetmap.atlas.geography.geojson.GeoJsonUtils; import org.openstreetmap.atlas.tags.ISOCountryTag; import org.openstreetmap.atlas.tags.RelationTypeTag; import org.openstreetmap.atlas.tags.annotations.validation.Validators; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.gson.JsonObject; @@ -18,20 +25,32 @@ * A flag for a {@link Relation} * * @author sayas01 + * @author bbreithaupt */ public class FlaggedRelation extends FlaggedObject { private static final long serialVersionUID = 81887932468503688L; - - private final Relation relation; - private final Map properties; + private static final RelationOrAreaToMultiPolygonConverter MULTI_POLYGON_CONVERTER = new RelationOrAreaToMultiPolygonConverter(); + private static final Logger logger = LoggerFactory.getLogger(FlaggedRelation.class); private final String country; + private final Map properties; + private final Relation relation; + private final MultiPolygon multipolygonGeometry; public FlaggedRelation(final Relation relation) { this.relation = relation; this.properties = initProperties(relation); this.country = initCountry(relation); + this.multipolygonGeometry = this.relationGeometry(relation); + } + + public FlaggedRelation(final Relation relation, final MultiPolygon geoJsonGeometry) + { + this.relation = relation; + this.properties = initProperties(relation); + this.country = initCountry(relation); + this.multipolygonGeometry = geoJsonGeometry; } /** @@ -44,11 +63,11 @@ public FlaggedRelation(final Relation relation) @Override public JsonObject asGeoJsonFeature(final String flagIdentifier) { - final JsonObject geoJsonGeometry = this.relation.asGeoJsonGeometry(); final JsonObject featureProperties = this.relation.getGeoJsonProperties(); featureProperties.addProperty("flag:id", flagIdentifier); featureProperties.addProperty("flag:type", FlaggedRelation.class.getSimpleName()); - return GeoJsonUtils.feature(geoJsonGeometry, featureProperties); + return GeoJsonUtils.feature(this.multipolygonGeometry.asGeoJsonGeometry(), + featureProperties); } /** @@ -66,6 +85,12 @@ public boolean equals(final Object other) return super.equals(other); } + @Override + public FlaggedObject getAsCompleteFlaggedObject() + { + return new FlaggedRelation(CompleteRelation.from(this.relation), this.multipolygonGeometry); + } + /** * @return ISO country code of the country */ @@ -120,6 +145,12 @@ public RelationMemberList members() return this.relation.members(); } + @Override + protected Optional getObject() + { + return Optional.of(this.relation); + } + private String initCountry(final AtlasObject object) { final Map tags = object.getTags(); @@ -144,4 +175,37 @@ private Map initProperties(final Relation relation) tags.put(ITEM_TYPE_TAG, "Relation"); return tags; } + + /** + * Get the geometry for a relation as either a {@link MultiPolygon} or bounding + * {@link Rectangle}. + * + * @param relation + * {@link Relation} + * @return {@link MultiPolygon} geometry + */ + private MultiPolygon relationGeometry(final Relation relation) + { + if (relation.isMultiPolygon()) + { + try + { + return MULTI_POLYGON_CONVERTER.convert(relation); + } + catch (final CoreException exception) + { + final String message = String.format("%s - %s", + exception.getClass().getSimpleName(), exception.getMessage()); + logger.error("Unable to recreate multipolygon for relation {}. {}", + relation.getIdentifier(), message); + return MultiPolygon.forOuters(relation.bounds()); + } + } + // Otherwise, we'll fall back to just providing the properties of the relation with the + // bounding box as a polygon geometry. + else + { + return MultiPolygon.forOuters(relation.bounds()); + } + } } diff --git a/src/main/java/org/openstreetmap/atlas/checks/utility/IncompleteSharding.java b/src/main/java/org/openstreetmap/atlas/checks/utility/IncompleteSharding.java new file mode 100644 index 000000000..8214d5db0 --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/utility/IncompleteSharding.java @@ -0,0 +1,80 @@ +package org.openstreetmap.atlas.checks.utility; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +import org.openstreetmap.atlas.exception.CoreException; +import org.openstreetmap.atlas.geography.GeometricSurface; +import org.openstreetmap.atlas.geography.Location; +import org.openstreetmap.atlas.geography.PolyLine; +import org.openstreetmap.atlas.geography.index.RTree; +import org.openstreetmap.atlas.geography.sharding.Shard; +import org.openstreetmap.atlas.geography.sharding.Sharding; +import org.openstreetmap.atlas.geography.sharding.SlippyTile; +import org.openstreetmap.atlas.utilities.scalars.Distance; + +/** + * A sharding interface with just the shards that are available for a certain run. + * + * @author jklamer + */ +public class IncompleteSharding implements Sharding +{ + private static final long serialVersionUID = 7177712260432658102L; + private final RTree shards; + private final Set shardSet = new HashSet<>(); + + public IncompleteSharding(final Iterable shards) + { + this.shards = RTree.forLocated(shards); + shards.forEach(this.shardSet::add); + } + + @Override + public Iterable neighbors(final Shard shard) + { + if (this.shardSet.contains(shard)) + { + return this.shards.get(shard.bounds().expand(Distance.ONE_METER)).stream() + .filter(aShard -> !aShard.equals(shard)).collect(Collectors.toList()); + } + return Collections.emptyList(); + } + + @Override + public Shard shardForName(final String name) + { + final SlippyTile result = SlippyTile.forName(name); + + if (!this.shardSet.contains(result)) + { + throw new CoreException("This sharding does not include tile {}", name); + } + + return result; + } + + @Override + public Iterable shards(final GeometricSurface surface) + { + return this.shards.get(surface.bounds()).stream() + .filter(shard -> surface.overlaps(shard.bounds())).collect(Collectors.toList()); + } + + @Override + public Iterable shardsCovering(final Location location) + { + return this.shards.get(location.bounds()).stream() + .filter(shard -> shard.bounds().fullyGeometricallyEncloses(location)) + .collect(Collectors.toList()); + } + + @Override + public Iterable shardsIntersecting(final PolyLine polyLine) + { + return this.shards.get(polyLine.bounds()).stream() + .filter(shard -> shard.bounds().overlaps(polyLine)).collect(Collectors.toList()); + } +} diff --git a/src/main/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainer.java b/src/main/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainer.java new file mode 100644 index 000000000..e2c069d4c --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainer.java @@ -0,0 +1,109 @@ +package org.openstreetmap.atlas.checks.utility; + +import java.io.Serializable; +import java.util.Collection; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Stream; + +import org.openstreetmap.atlas.checks.event.CheckFlagEvent; +import org.openstreetmap.atlas.checks.flag.CheckFlag; + +/** + * A container that will deduplicate check flags based on source and unique IDs + * + * @author jklamer + * @author bbreithaupt + */ +public class UniqueCheckFlagContainer implements Serializable +{ + + private final ConcurrentHashMap, CheckFlag>> uniqueFlags; + + /** + * Combines to containers. This deduplicates {@link CheckFlag}s by overwiting ones with matching + * sources and IDs. + * + * @param container1 + * {@link UniqueCheckFlagContainer} + * @param container2 + * {@link UniqueCheckFlagContainer} + * @return merged {@link UniqueCheckFlagContainer} + */ + public static UniqueCheckFlagContainer combine(final UniqueCheckFlagContainer container1, + final UniqueCheckFlagContainer container2) + { + container2.uniqueFlags.entrySet() + .forEach(entry -> container1.addAll(entry.getKey(), entry.getValue().values())); + return container1; + } + + public UniqueCheckFlagContainer() + { + this.uniqueFlags = new ConcurrentHashMap<>(); + } + + @SuppressWarnings("s1144") + // Ignore unused constructor warning, this is used for deserialization + private UniqueCheckFlagContainer( + final ConcurrentHashMap, CheckFlag>> flags) + { + this.uniqueFlags = flags; + } + + /** + * Add a {@link CheckFlag} to the container based on its source. + * + * @param flagSource + * {@link String} source (check that generated the flag) + * @param flag + * {@link CheckFlag} + */ + public void add(final String flagSource, final CheckFlag flag) + { + this.uniqueFlags.putIfAbsent(flagSource, new ConcurrentHashMap<>()); + final Set uniqueObjectIdentifiers = flag.getUniqueIdentifiers(); + this.uniqueFlags.get(flagSource) + .putIfAbsent(uniqueObjectIdentifiers.isEmpty() + ? Collections.singleton(flag.getIdentifier()) + : uniqueObjectIdentifiers, flag); + } + + /** + * Batch add {@link CheckFlag} from a single source. + * + * @param flagSource + * {@link String} source (check that generated the flags) + * @param flags + * {@link Iterable} of {@link CheckFlag}s + */ + public void addAll(final String flagSource, final Iterable flags) + { + flags.forEach(flag -> this.add(flagSource, flag)); + + } + + /** + * Convert the {@link CheckFlag}s into a {@link Stream} of {@link CheckFlagEvent}s. + * + * @return a {@link Stream} of {@link CheckFlagEvent}s + */ + public Stream reconstructEvents() + { + return this.uniqueFlags.keySet().stream() + .flatMap(checkName -> this.uniqueFlags.get(checkName).values().stream() + .map(checkFlag -> new CheckFlagEvent(checkName, checkFlag))); + } + + /** + * Get the contents of the container as a stream. + * + * @return a {@link Stream} of {@link CheckFlag}s + */ + public Stream stream() + { + return this.uniqueFlags.values().stream().map(ConcurrentHashMap::values) + .flatMap(Collection::stream); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/base/CheckResourceLoaderTest.java b/src/test/java/org/openstreetmap/atlas/checks/base/CheckResourceLoaderTest.java index 6e740a140..80419d754 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/base/CheckResourceLoaderTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/base/CheckResourceLoaderTest.java @@ -65,6 +65,30 @@ public void testBlacklistNoOp() .filter(name -> name.startsWith("CheckResource")).distinct().count()); } + @Test + public void testCheckCountryBlacklist() + { + final String configSource = "{\"CheckResourceLoader.scanUrls\": [\"org.openstreetmap.atlas.checks.base.checks\"],\"CheckResourceLoaderTestCheck\":{\"enabled\": true, \"countries.blacklist\":[\"ABC\"]}}"; + + final Configuration configuration = ConfigurationResolver.inlineConfiguration(configSource); + final CheckResourceLoader checkResourceLoader = new CheckResourceLoader(configuration); + + Assert.assertEquals(0, checkResourceLoader.loadChecksForCountry("ABC").size()); + Assert.assertEquals(1, checkResourceLoader.loadChecksForCountry("DEF").size()); + } + + @Test + public void testCheckCountryWhitelist() + { + final String configSource = "{\"CheckResourceLoader.scanUrls\": [\"org.openstreetmap.atlas.checks.base.checks\"],\"CheckResourceLoaderTestCheck\":{\"enabled\": true, \"countries\":[\"ABC\"]}}"; + + final Configuration configuration = ConfigurationResolver.inlineConfiguration(configSource); + final CheckResourceLoader checkResourceLoader = new CheckResourceLoader(configuration); + + Assert.assertEquals(1, checkResourceLoader.loadChecksForCountry("ABC").size()); + Assert.assertEquals(0, checkResourceLoader.loadChecksForCountry("DEF").size()); + } + /** * Test the check loading using country keyword specific configurations. Assert that each * country gets its own version of the check. diff --git a/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommandTest.java b/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommandTest.java index 36cacaee0..8dee5bb2a 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommandTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksGeoJSONDiffSubCommandTest.java @@ -11,7 +11,7 @@ import org.junit.Test; import org.openstreetmap.atlas.checks.event.CheckFlagEvent; import org.openstreetmap.atlas.checks.event.CheckFlagGeoJsonProcessor; -import org.openstreetmap.atlas.checks.event.ShutdownEvent; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.File; diff --git a/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommandTest.java b/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommandTest.java index e24133a19..f0b8f560f 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommandTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/commands/AtlasChecksLogDiffSubCommandTest.java @@ -12,7 +12,7 @@ import org.openstreetmap.atlas.checks.event.CheckFlagEvent; import org.openstreetmap.atlas.checks.event.CheckFlagFileProcessor; import org.openstreetmap.atlas.checks.event.FileProcessor; -import org.openstreetmap.atlas.checks.event.ShutdownEvent; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.File; diff --git a/src/test/java/org/openstreetmap/atlas/checks/commands/FlagStatisticsSubCommandTest.java b/src/test/java/org/openstreetmap/atlas/checks/commands/FlagStatisticsSubCommandTest.java index 88188a10a..22e9202ca 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/commands/FlagStatisticsSubCommandTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/commands/FlagStatisticsSubCommandTest.java @@ -15,7 +15,7 @@ import org.openstreetmap.atlas.checks.event.CheckFlagEvent; import org.openstreetmap.atlas.checks.event.CheckFlagFileProcessor; import org.openstreetmap.atlas.checks.event.FileProcessor; -import org.openstreetmap.atlas.checks.event.ShutdownEvent; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.File; diff --git a/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTaskTest.java b/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTaskTest.java new file mode 100644 index 000000000..c200374ed --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedCheckFlagsTaskTest.java @@ -0,0 +1,52 @@ +package org.openstreetmap.atlas.checks.distributed; + +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.openstreetmap.atlas.checks.base.Check; +import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver; +import org.openstreetmap.atlas.checks.validation.tag.InvalidLanesTagCheck; +import org.openstreetmap.atlas.geography.sharding.Shard; +import org.openstreetmap.atlas.geography.sharding.SlippyTile; + +/** + * Unit tests for {@link ShardedCheckFlagsTask}. + * + * @author bbreithaupt + */ +public class ShardedCheckFlagsTaskTest +{ + private static final String COUNTRY = "CAN"; + private static final Shard SHARD = new SlippyTile(0, 0, 0); + private static final List CHECKS = Collections + .singletonList(new InvalidLanesTagCheck(ConfigurationResolver.emptyConfiguration())); + private static final ShardedCheckFlagsTask TASK = new ShardedCheckFlagsTask(COUNTRY, SHARD, + CHECKS); + + @Test + public void getChecks() + { + Assert.assertEquals(CHECKS, TASK.getChecks()); + } + + @Test + public void getCountry() + { + Assert.assertEquals(COUNTRY, TASK.getCountry()); + } + + @Test + public void getShardGroup() + { + Assert.assertEquals(SHARD, TASK.getShard()); + } + + @Test + public void getUniqueTaskIdentifier() + { + Assert.assertEquals(String.format("%s_%s", COUNTRY, SHARD.getName()), + TASK.getUniqueTaskIdentifier()); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTest.java b/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTest.java new file mode 100644 index 000000000..8ab951374 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTest.java @@ -0,0 +1,139 @@ +package org.openstreetmap.atlas.checks.distributed; + +import java.io.BufferedReader; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.InputStreamReader; +import java.util.Set; +import java.util.stream.Collectors; + +import org.apache.commons.io.FilenameUtils; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.geography.sharding.SlippyTileSharding; +import org.openstreetmap.atlas.streaming.resource.File; + +/** + * Unit tests for {@link ShardedIntegrityChecksSparkJob}. Runs the spark job with test inputs and + * validates the output files. + * + * @author bbreithaupt + */ +public class ShardedIntegrityChecksSparkJobTest +{ + private static final String COUNTRY_CODE = "UNK"; + private static final int ZOOM_LEVEL = 4; + private static final File INPUT = File.temporaryFolder(); + private static final File OUTPUT = File.temporaryFolder(); + + @Rule + public ShardedIntegrityChecksSparkJobTestRule setup = new ShardedIntegrityChecksSparkJobTestRule(); + + @AfterClass + public static void cleanUp() + { + INPUT.deleteRecursively(); + OUTPUT.deleteRecursively(); + } + + @Test + public void countFlagsTest() throws FileNotFoundException + { + generateData(); + + Assert.assertTrue(OUTPUT.child("flag").child(COUNTRY_CODE).exists()); + final Set flagFiles = OUTPUT.child("flag").child(COUNTRY_CODE).listFilesRecursively() + .stream().filter(file -> file.getName().startsWith(COUNTRY_CODE)) + .collect(Collectors.toSet()); + for (final File file : flagFiles) + { + Assert.assertEquals(2, + new BufferedReader( + new InputStreamReader(new FileInputStream(file.getAbsolutePath()))) + .lines().count()); + } + } + + @Test + public void countGeojsonTest() + { + generateData(); + + Assert.assertTrue(OUTPUT.child("geojson").child(COUNTRY_CODE).exists()); + Assert.assertTrue(OUTPUT.child("geojson").child(COUNTRY_CODE).listFilesRecursively() + .stream().anyMatch(file -> file.getName().startsWith("EdgesTestCheck"))); + } + + @Test + public void countMetricsTest() + { + generateData(); + + Assert.assertTrue(OUTPUT.child("metric").child(COUNTRY_CODE).exists()); + Assert.assertEquals(2, OUTPUT.child("metric").child(COUNTRY_CODE).listFilesRecursively() + .stream().filter(file -> file.getName().endsWith(".csv")).count()); + } + + @Test + public void tippecanoeTest() + { + generateData(); + + Assert.assertTrue(OUTPUT.child("tippecanoe").child(COUNTRY_CODE).exists()); + } + + /** + * Generate test atlas files with the correct sharding zoom level. + */ + private void generateAtlases() + { + final File countryFolder = new File( + FilenameUtils.concat(INPUT.getAbsolutePath(), COUNTRY_CODE)); + countryFolder.mkdirs(); + + final SlippyTileSharding sharding = new SlippyTileSharding(ZOOM_LEVEL); + + this.setup.bcAtlas() + .save(new File(FilenameUtils.concat(countryFolder.getAbsolutePath(), + String.format("UNK_%s.atlas", sharding.shards(this.setup.bcAtlas().bounds()) + .iterator().next().getName())))); + this.setup.nzAtlas() + .save(new File(FilenameUtils.concat(countryFolder.getAbsolutePath(), + String.format("UNK_%s.atlas", sharding.shards(this.setup.nzAtlas().bounds()) + .iterator().next().getName())))); + } + + /** + * Generate test data and populate the input folder. + */ + private void generateData() + { + if (OUTPUT.listFilesRecursively().isEmpty()) + { + this.generateAtlases(); + this.runShardedIntegrityChecksSparkJob(); + } + } + + /** + * Run {@link ShardedIntegrityChecksSparkJob} with test inputs. + */ + private void runShardedIntegrityChecksSparkJob() + { + final String[] arguments = { String.format("-inputFolder=%s", INPUT.getAbsolutePath()), + String.format("-startedFolder=%s", INPUT.getAbsolutePath()), + String.format("-output=%s", OUTPUT.getAbsolutePath()), + String.format("-sharding=slippy@%s", ZOOM_LEVEL), "-maxShardLoad=1", + "-outputFormats=flags,geojson,metrics,tippecanoe", "-compressOutput=false", + String.format("-countries=%s", COUNTRY_CODE), "-saveCheckOutput=true", + "-master=local", + String.format("-configFiles=file:%s", + ShardedIntegrityChecksSparkJobTest.class + .getResource("test_configuration.json").getPath()), + "-sparkOptions=spark.executor.memory->4g,spark.driver.memory->16g,spark.rdd.compress->true" }; + + new ShardedIntegrityChecksSparkJob().runWithoutQuitting(arguments); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTestRule.java new file mode 100644 index 000000000..51145635d --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/distributed/ShardedIntegrityChecksSparkJobTestRule.java @@ -0,0 +1,48 @@ +package org.openstreetmap.atlas.checks.distributed; + +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 ShardedIntegrityChecksSparkJobTest}. + * + * @author bbreithaupt + */ +public class ShardedIntegrityChecksSparkJobTestRule extends CoreTestRule +{ + private static final String TEST1_1 = "48.4199329, -123.3708146"; + private static final String TEST1_2 = "48.4197566, -123.3695244"; + private static final String TEST2_1 = "-41.2774703, 174.7770302"; + private static final String TEST2_2 = "-41.2783868, 174.7770197"; + + @TestAtlas( + // Nodes + nodes = { @Node(coordinates = @Loc(value = TEST1_1)), + @Node(coordinates = @Loc(value = TEST1_2)) }, + // Edges + edges = { @Edge(coordinates = { @Loc(value = TEST1_1), @Loc(value = TEST1_2) }), + @Edge(coordinates = { @Loc(value = TEST1_2), @Loc(value = TEST1_1) }) }) + private Atlas bcAtlas; + + @TestAtlas( + // Nodes + nodes = { @Node(coordinates = @Loc(value = TEST2_1)), + @Node(coordinates = @Loc(value = TEST2_2)) }, + // Edges + edges = { @Edge(coordinates = { @Loc(value = TEST2_1), @Loc(value = TEST2_2) }) }) + private Atlas nzAtlas; + + public Atlas bcAtlas() + { + return this.bcAtlas; + } + + public Atlas nzAtlas() + { + return this.nzAtlas; + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessorTest.java b/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessorTest.java index af499d220..ca6924fff 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessorTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagFileProcessorTest.java @@ -9,6 +9,7 @@ import org.junit.Assert; import org.junit.Test; import org.openstreetmap.atlas.checks.flag.CheckFlag; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.filesystem.FileSystemHelper; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.Resource; diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessorTest.java b/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessorTest.java index 5355f419b..bb2020813 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessorTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagGeoJsonProcessorTest.java @@ -11,6 +11,7 @@ import org.junit.Assert; import org.junit.Rule; import org.junit.Test; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.filesystem.FileSystemHelper; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.Resource; diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessorTest.java b/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessorTest.java index 2d091411b..d83e438f8 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessorTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/event/CheckFlagTippecanoeProcessorTest.java @@ -9,6 +9,7 @@ import org.junit.Assert; import org.junit.Rule; import org.junit.Test; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.filesystem.FileSystemHelper; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.Resource; diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/EventBusTest.java b/src/test/java/org/openstreetmap/atlas/checks/event/EventBusTest.java deleted file mode 100644 index d019ab55d..000000000 --- a/src/test/java/org/openstreetmap/atlas/checks/event/EventBusTest.java +++ /dev/null @@ -1,181 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -import org.junit.Assert; -import org.junit.Test; -import org.openstreetmap.atlas.utilities.scalars.Duration; -import org.openstreetmap.atlas.utilities.threads.Pool; - -import com.google.common.eventbus.EventBus; - -/** - * Tests for {@link EventBus}. - * - * @author mkalender - */ -public class EventBusTest -{ - @Test - public void testEventResult() - { - testEventResult(new TestEvent(null)); - testEventResult(new TestEvent("")); - testEventResult(new TestEvent("This is a test!!!")); - } - - @Test - public void testOneComplete() - { - testCompleteCount(1); - } - - @Test - public void testOneThousandComplete() - { - testCompleteCount(1000); - } - - @Test - public void testSingleThreadOneEvent() - { - testProcessCount(1, 1); - } - - @Test - public void testSingleThreadOneMillionEvent() - { - testProcessCount(1, 1000000); - } - - @Test - public void testSingleThreadOneThousandEvent() - { - testProcessCount(1, 1000); - } - - @Test - public void testSingleThreadTenThousandEvent() - { - testProcessCount(1, 10000); - } - - @Test - public void testSingleThreadZeroEvent() - { - testProcessCount(1, 0); - } - - @Test - public void testTenThreadsOneEvent() - { - testProcessCount(10, 1); - } - - @Test - public void testTenThreadsOneThousandEvent() - { - testProcessCount(10, 1000); - } - - @Test - public void testTenThreadsZeroEvent() - { - testProcessCount(10, 0); - } - - @Test - public void testTwoComplete() - { - testCompleteCount(2); - } - - @Test - public void testTwoThreadsOneEvent() - { - testProcessCount(2, 1); - } - - @Test - public void testTwoThreadsOneThousandEvent() - { - testProcessCount(2, 1000); - } - - @Test - public void testTwoThreadsTenThousandEvent() - { - testProcessCount(2, 10000); - } - - @Test - public void testTwoThreadsZeroEvent() - { - testProcessCount(2, 0); - } - - @Test - public void testZeroComplete() - { - testCompleteCount(0); - } - - private void testCompleteCount(final int completeCount) - { - final TestProcessor testProcessor = new TestProcessor(); - final EventBus eventBus = new EventBus(); - eventBus.register(testProcessor); - - // Straight complete - for (int index = 0; index < completeCount; index++) - { - eventBus.post(new ShutdownEvent()); - } - - // Validate - Assert.assertEquals(0, testProcessor.getProcessCount()); - Assert.assertEquals(completeCount, testProcessor.getCompleteCount()); - } - - private void testEventResult(final TestEvent event) - { - final TestProcessor testProcessor = new TestProcessor(); - final EventBus eventBus = new EventBus(); - eventBus.register(testProcessor); - - // Send event and complete - eventBus.post(event); - eventBus.post(new ShutdownEvent()); - - // Validate - Assert.assertEquals(event.getMessage(), testProcessor.getLastEventMessage()); - Assert.assertEquals(event != null ? 1 : 0, testProcessor.getProcessCount()); - Assert.assertEquals(1, testProcessor.getCompleteCount()); - } - - private void testProcessCount(final int threadCount, final int eventCount) - { - final TestProcessor testProcessor = new TestProcessor(); - final EventBus eventBus = new EventBus(); - eventBus.register(testProcessor); - - // Send events - final Pool threadPool = new Pool(threadCount, "Test event pool", Duration.ONE_MINUTE); - for (int threadIndex = 0; threadIndex < threadCount; threadIndex++) - { - threadPool.queue(() -> - { - for (int index = 0; index < eventCount; index++) - { - eventBus.post(new TestEvent("test " + index)); - } - }); - } - threadPool.close(); - - // Complete - eventBus.post(new ShutdownEvent()); - - // Validate - Assert.assertEquals(eventCount * threadCount, testProcessor.getProcessCount()); - Assert.assertEquals(1, testProcessor.getCompleteCount()); - } -} diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/EventServiceTest.java b/src/test/java/org/openstreetmap/atlas/checks/event/EventServiceTest.java deleted file mode 100644 index 94c666652..000000000 --- a/src/test/java/org/openstreetmap/atlas/checks/event/EventServiceTest.java +++ /dev/null @@ -1,236 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -import org.junit.Assert; -import org.junit.Test; -import org.openstreetmap.atlas.utilities.scalars.Duration; -import org.openstreetmap.atlas.utilities.threads.Pool; - -/** - * Tests for {@link EventService}.isua - * - * @author mkalender - */ -public class EventServiceTest -{ - @Test - public void testEventResult() - { - testEventResult(new TestEvent(null)); - testEventResult(new TestEvent("")); - testEventResult(new TestEvent("This is a test!!!")); - } - - @Test - public void testNullEvent() - { - final TestProcessor testProcessor = new TestProcessor(); - final EventService eventService = EventService.get("Test service for null event"); - eventService.register(testProcessor); - - // Send event and complete - eventService.post(null); - eventService.complete(); - - // Validate - Assert.assertEquals(0, testProcessor.getProcessCount()); - Assert.assertEquals(1, testProcessor.getCompleteCount()); - } - - @Test - public void testOneComplete() - { - testCompleteCount(1); - } - - @Test - public void testOneThousandComplete() - { - testCompleteCount(1000); - } - - @Test - public void testPostAfterComplete() - { - final TestProcessor testProcessor = new TestProcessor(); - final EventService eventService = EventService - .get("Test service for posting after completion"); - eventService.register(testProcessor); - - // Send event and complete - eventService.complete(); - eventService.post(new TestEvent("a message")); - - // Validate - Assert.assertEquals(null, testProcessor.getLastEventMessage()); - Assert.assertEquals(0, testProcessor.getProcessCount()); - Assert.assertEquals(1, testProcessor.getCompleteCount()); - } - - @Test - public void testSingleThreadOneEvent() - { - testProcessCount(1, 1); - } - - @Test - public void testSingleThreadOneMillionEvent() - { - testProcessCount(1, 1000000); - } - - @Test - public void testSingleThreadOneThousandEvent() - { - testProcessCount(1, 1000); - } - - @Test - public void testSingleThreadTenThousandEvent() - { - testProcessCount(1, 10000); - } - - @Test - public void testSingleThreadZeroEvent() - { - testProcessCount(1, 0); - } - - @Test - public void testTenThreadsOneEvent() - { - testProcessCount(10, 1); - } - - @Test - public void testTenThreadsOneThousandEvent() - { - testProcessCount(10, 1000); - } - - @Test - public void testTenThreadsZeroEvent() - { - testProcessCount(10, 0); - } - - @Test - public void testTwoComplete() - { - testCompleteCount(2); - } - - @Test - public void testTwoThreadsOneEvent() - { - testProcessCount(2, 1); - } - - @Test - public void testTwoThreadsOneThousandEvent() - { - testProcessCount(2, 1000); - } - - @Test - public void testTwoThreadsTenThousandEvent() - { - testProcessCount(2, 10000); - } - - @Test - public void testTwoThreadsZeroEvent() - { - testProcessCount(2, 0); - } - - @Test - public void testUnregisterProcessor() - { - final TestProcessor testProcessor = new TestProcessor(); - final EventService eventService = EventService - .get("Event service for unregistering processor"); - - eventService.register(testProcessor); - eventService.unregister(testProcessor); - eventService.post(new TestEvent("TESTING")); - - eventService.complete(); - - Assert.assertEquals(0, testProcessor.getCompleteCount()); - Assert.assertEquals(0, testProcessor.getProcessCount()); - } - - @Test - public void testZeroComplete() - { - testCompleteCount(0); - } - - private void testCompleteCount(final int completeCount) - { - final TestProcessor testProcessor = new TestProcessor(); - final EventService eventService = EventService - .get("Test service complete " + completeCount); - eventService.register(testProcessor); - - // Straight complete - for (int index = 0; index < completeCount; index++) - { - eventService.complete(); - } - - // Validate - Assert.assertEquals(0, testProcessor.getProcessCount()); - Assert.assertEquals(Math.min(completeCount, 1), testProcessor.getCompleteCount()); - } - - private void testEventResult(final TestEvent event) - { - final TestProcessor testProcessor = new TestProcessor(); - final EventService eventService = EventService - .get("Test service for " + event.getMessage()); - eventService.register(testProcessor); - - // Send event and complete - eventService.post(event); - eventService.complete(); - - // Validate - Assert.assertEquals(event.getMessage(), testProcessor.getLastEventMessage()); - Assert.assertEquals(1, testProcessor.getProcessCount()); - Assert.assertEquals(1, testProcessor.getCompleteCount()); - } - - private void testProcessCount(final int threadCount, final int eventCount) - { - final TestProcessor testProcessor = new TestProcessor(); - final TestProcessor otherProcessor = new TestProcessor(); - final EventService eventService = EventService - .get("Test service for " + threadCount + "-" + eventCount); - eventService.register(testProcessor); - eventService.register(otherProcessor); - - // Send events - final Pool threadPool = new Pool(threadCount, - "Test pool for " + threadCount + "-" + eventCount, Duration.ONE_MINUTE); - for (int threadIndex = 0; threadIndex < threadCount; threadIndex++) - { - threadPool.queue(() -> - { - for (int index = 0; index < eventCount; index++) - { - eventService.post(new TestEvent("test " + index)); - } - }); - } - threadPool.close(); - - // Complete - eventService.complete(); - - // Validate - Assert.assertEquals(eventCount * threadCount, testProcessor.getProcessCount()); - Assert.assertEquals(1, testProcessor.getCompleteCount()); - } -} diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/MetricFileGeneratorTest.java b/src/test/java/org/openstreetmap/atlas/checks/event/MetricFileGeneratorTest.java index 4af8b28aa..054e628f0 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/event/MetricFileGeneratorTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/event/MetricFileGeneratorTest.java @@ -10,6 +10,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.filesystem.FileSystemHelper; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.Resource; diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/TestEvent.java b/src/test/java/org/openstreetmap/atlas/checks/event/TestEvent.java deleted file mode 100644 index fb5873623..000000000 --- a/src/test/java/org/openstreetmap/atlas/checks/event/TestEvent.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -/** - * Sample {@link Event} to be used for testing. - * - * @author mkalender - */ -public class TestEvent extends Event -{ - private final String message; - - public TestEvent(final String message) - { - this.message = message; - } - - public String getMessage() - { - return this.message; - } -} diff --git a/src/test/java/org/openstreetmap/atlas/checks/event/TestProcessor.java b/src/test/java/org/openstreetmap/atlas/checks/event/TestProcessor.java deleted file mode 100644 index 3d213187a..000000000 --- a/src/test/java/org/openstreetmap/atlas/checks/event/TestProcessor.java +++ /dev/null @@ -1,47 +0,0 @@ -package org.openstreetmap.atlas.checks.event; - -import java.util.concurrent.atomic.AtomicInteger; - -import com.google.common.eventbus.Subscribe; - -/** - * Sample {@link Processor} for testing. - * - * @author mkalender - */ -public class TestProcessor implements Processor -{ - private String lastMessage; - private final AtomicInteger completeCount = new AtomicInteger(0); - private final AtomicInteger processCount = new AtomicInteger(0); - - @Override - @Subscribe - public void process(final ShutdownEvent event) - { - this.completeCount.incrementAndGet(); - } - - @Override - @Subscribe - public void process(final TestEvent event) - { - this.lastMessage = event.getMessage(); - this.processCount.incrementAndGet(); - } - - int getCompleteCount() - { - return this.completeCount.get(); - } - - String getLastEventMessage() - { - return this.lastMessage; - } - - int getProcessCount() - { - return this.processCount.get(); - } -} diff --git a/src/test/java/org/openstreetmap/atlas/checks/flag/CheckFlagTest.java b/src/test/java/org/openstreetmap/atlas/checks/flag/CheckFlagTest.java index 76a74437a..0aa98f010 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/flag/CheckFlagTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/flag/CheckFlagTest.java @@ -5,12 +5,18 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.openstreetmap.atlas.checks.event.CheckFlagEvent; +import org.openstreetmap.atlas.geography.atlas.complete.CompleteEntity; +import org.openstreetmap.atlas.geography.atlas.items.ItemType; +import org.openstreetmap.atlas.geography.atlas.items.Node; import com.google.gson.JsonObject; @@ -94,6 +100,58 @@ public void testFlaggedRelations() Assert.assertEquals(2, flag.getFlaggedRelations().size()); } + @Test + public void testGetUniqueObjectIdentifiers() + { + final CheckFlag flag = new CheckFlag("a-identifier"); + flag.addObject(this.setup.getAtlasWithRelations().entity(1, ItemType.NODE)); + flag.addObject(this.setup.getAtlasWithRelations().entity(2, ItemType.NODE)); + flag.addObject(this.setup.getAtlasWithRelations().entity(12, ItemType.EDGE)); + flag.addPoint( + ((Node) this.setup.getAtlasWithRelations().entity(1, ItemType.NODE)).getLocation()); + Assert.assertEquals(new HashSet<>(Arrays.asList("Node1", "Node2", "Edge12")), + flag.getUniqueIdentifiers()); + + final CheckFlag pointFlag = new CheckFlag("a-identifier"); + flag.addPoint( + ((Node) this.setup.getAtlasWithRelations().entity(1, ItemType.NODE)).getLocation()); + Assert.assertEquals(new HashSet<>(Collections.singletonList("a-identifier")), + pointFlag.getUniqueIdentifiers()); + } + + @Test + public void testMakeComplete() + { + final CheckFlag flag = new CheckFlag("a-identifier"); + flag.addInstruction("first instruction"); + flag.addInstruction("second instruction"); + this.setup.getAtlas().entities().forEach(flag::addObject); + + final CheckFlag otherFlag = new CheckFlag("a-identifier"); + this.setup.getAtlasWithRelations().entities().forEach(otherFlag::addObject); + + flag.makeComplete(); + otherFlag.makeComplete(); + + flag.getFlaggedObjects().stream().forEach(flaggedObject -> + { + Assert.assertTrue(flaggedObject.getObject().isPresent()); + flaggedObject.getObject().ifPresent(object -> + { + Assert.assertTrue(object instanceof CompleteEntity); + }); + }); + + otherFlag.getFlaggedObjects().stream().forEach(flaggedObject -> + { + Assert.assertTrue(flaggedObject.getObject().isPresent()); + flaggedObject.getObject().ifPresent(object -> + { + Assert.assertTrue(object instanceof CompleteEntity); + }); + }); + } + @Test public void testMembersOfFlaggedRelations() { diff --git a/src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTest.java b/src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTest.java new file mode 100644 index 000000000..5cbea9407 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTest.java @@ -0,0 +1,62 @@ +package org.openstreetmap.atlas.checks.flag; + +import static org.openstreetmap.atlas.geography.geojson.GeoJsonConstants.COORDINATES; +import static org.openstreetmap.atlas.geography.geojson.GeoJsonConstants.GEOMETRY; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; + +import com.google.gson.JsonArray; + +/** + * Unit tests for {@link FlaggedRelation}. + * + * @author bbreithaupt + */ +public class FlaggedRelationTest +{ + @Rule + public final FlaggedRelationTestRule setup = new FlaggedRelationTestRule(); + + @Test + public void completeGeometryBadMultipolygonTest() + { + final FlaggedRelation flaggedRelation = new FlaggedRelation( + this.setup.badMultipolygonAtlas().relation(1000000L)); + Assert.assertTrue(flaggedRelation.isMultipolygonRelation()); + final JsonArray atlasBackedGeometry = flaggedRelation.asGeoJsonFeature("1").get(GEOMETRY) + .getAsJsonObject().get(COORDINATES).getAsJsonArray(); + final JsonArray completeGeometry = flaggedRelation.getAsCompleteFlaggedObject() + .asGeoJsonFeature("1").get(GEOMETRY).getAsJsonObject().get(COORDINATES) + .getAsJsonArray(); + Assert.assertEquals(atlasBackedGeometry, completeGeometry); + } + + @Test + public void completeGeometryBoundsTest() + { + final FlaggedRelation flaggedRelation = new FlaggedRelation( + this.setup.circuitAtlas().relation(1000000L)); + final JsonArray atlasBackedGeometry = flaggedRelation.asGeoJsonFeature("1").get(GEOMETRY) + .getAsJsonObject().get(COORDINATES).getAsJsonArray(); + final JsonArray completeGeometry = flaggedRelation.getAsCompleteFlaggedObject() + .asGeoJsonFeature("1").get(GEOMETRY).getAsJsonObject().get(COORDINATES) + .getAsJsonArray(); + Assert.assertEquals(atlasBackedGeometry, completeGeometry); + } + + @Test + public void completeGeometryMultipolygonTest() + { + final FlaggedRelation flaggedRelation = new FlaggedRelation( + this.setup.multipolygonAtlas().relation(1000000L)); + Assert.assertTrue(flaggedRelation.isMultipolygonRelation()); + final JsonArray atlasBackedGeometry = flaggedRelation.asGeoJsonFeature("1").get(GEOMETRY) + .getAsJsonObject().get(COORDINATES).getAsJsonArray(); + final JsonArray completeGeometry = flaggedRelation.getAsCompleteFlaggedObject() + .asGeoJsonFeature("1").get(GEOMETRY).getAsJsonObject().get(COORDINATES) + .getAsJsonArray(); + Assert.assertEquals(atlasBackedGeometry, completeGeometry); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTestRule.java new file mode 100644 index 000000000..f2659735a --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/flag/FlaggedRelationTestRule.java @@ -0,0 +1,81 @@ +package org.openstreetmap.atlas.checks.flag; + +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.Relation; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Relation.Member; + +/** + * Unit test rule for {@link FlaggedRelationTest}. + * + * @author bbreithaupt + */ +public class FlaggedRelationTestRule extends CoreTestRule +{ + public static final String TEST_1 = "1.2887014, 103.8628407"; + public static final String TEST_2 = "1.2895026, 103.8645045"; + public static final String TEST_3 = "1.2943360, 103.8638678"; + public static final String TEST_4 = "1.2944915, 103.8631437"; + public static final String TEST_5 = "1.2941844, 103.8637712"; + public static final String TEST_6 = "1.2895198, 103.8643810"; + + @TestAtlas( + // lines + lines = { + @Line(id = "1000000", coordinates = { @Loc(value = TEST_1), + @Loc(value = TEST_2), @Loc(value = TEST_3), @Loc(value = TEST_4) }), + @Line(id = "2000000", coordinates = { @Loc(value = TEST_4), + @Loc(value = TEST_5), @Loc(value = TEST_6), @Loc(value = TEST_1) }) }, + // relations + relations = { @Relation(id = "1000000", members = { + @Member(id = "1000000", type = "line", role = "outer"), + @Member(id = "2000000", type = "line", role = "outer"), }, tags = { + "type=multipolygon" }) }) + private Atlas multipolygonAtlas; + + @TestAtlas( + // lines + lines = { + @Line(id = "1000000", coordinates = { @Loc(value = TEST_2), + @Loc(value = TEST_3) }), + @Line(id = "2000000", coordinates = { @Loc(value = TEST_4), + @Loc(value = TEST_5), @Loc(value = TEST_6), @Loc(value = TEST_1) }) }, + // relations + relations = { @Relation(id = "1000000", members = { + @Member(id = "1000000", type = "line", role = "outer"), + @Member(id = "2000000", type = "line", role = "outer"), }, tags = { + "type=multipolygon" }) }) + private Atlas badMultipolygonAtlas; + + @TestAtlas( + // lines + lines = { + @Line(id = "1000000", coordinates = { @Loc(value = TEST_1), + @Loc(value = TEST_2), @Loc(value = TEST_3), @Loc(value = TEST_4) }), + @Line(id = "2000000", coordinates = { @Loc(value = TEST_1), + @Loc(value = TEST_6), @Loc(value = TEST_5), @Loc(value = TEST_4) }) }, + // relations + relations = { @Relation(id = "1000000", members = { + @Member(id = "1000000", type = "line", role = "forward"), + @Member(id = "2000000", type = "line", role = "backward"), }, tags = { + "type=circuit" }) }) + private Atlas circuitAtlas; + + public Atlas badMultipolygonAtlas() + { + return this.badMultipolygonAtlas; + } + + public Atlas circuitAtlas() + { + return this.circuitAtlas; + } + + public Atlas multipolygonAtlas() + { + return this.multipolygonAtlas; + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java b/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java index b79bb9ea2..a621dc2d7 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteUploadCommandTest.java @@ -15,9 +15,9 @@ import org.openstreetmap.atlas.checks.event.CheckFlagEvent; import org.openstreetmap.atlas.checks.event.CheckFlagFileProcessor; import org.openstreetmap.atlas.checks.event.FileProcessor; -import org.openstreetmap.atlas.checks.event.ShutdownEvent; import org.openstreetmap.atlas.checks.maproulette.data.Challenge; import org.openstreetmap.atlas.checks.maproulette.data.Project; +import org.openstreetmap.atlas.event.ShutdownEvent; import org.openstreetmap.atlas.generator.tools.spark.utilities.SparkFileHelper; import org.openstreetmap.atlas.streaming.resource.File; import org.openstreetmap.atlas.utilities.runtime.CommandMap; diff --git a/src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTest.java b/src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTest.java new file mode 100644 index 000000000..7a2ad5a96 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTest.java @@ -0,0 +1,99 @@ +package org.openstreetmap.atlas.checks.utility; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.checks.flag.CheckFlag; + +/** + * Test for {@link UniqueCheckFlagContainer} + * + * @author jklamer + * @author bbreithaupt + */ +public class UniqueCheckFlagContainerTest +{ + private static final String source1 = "source1"; + private static final String source2 = "source2"; + @Rule + public UniqueCheckFlagContainerTestRule setup = new UniqueCheckFlagContainerTestRule(); + private final CheckFlag flag1 = new CheckFlag("example-flag-1"); + private final CheckFlag flag2 = new CheckFlag("example-flag-2"); + private final CheckFlag flag3 = new CheckFlag("example-flag-3"); + private final CheckFlag sameIDFlag1 = new CheckFlag("example-flag-1"); + private final CheckFlag sameObjectFlag2 = new CheckFlag("example-flag-4"); + + @Test + public void testStreaming() + { + final UniqueCheckFlagContainer container = new UniqueCheckFlagContainer(); + container.add(source1, this.flag1); + container.add(source1, this.flag2); + container.add(source1, this.flag3); + container.add(source2, this.flag1); + container.add(source2, this.flag2); + container.add(source2, this.flag3); + + final Set source1Flags = new HashSet<>( + Arrays.asList(this.flag1, this.flag2, this.flag3)); + final Set source2Flags = new HashSet<>( + Arrays.asList(this.flag1, this.flag2, this.flag3)); + + container.reconstructEvents().forEach(flagEvent -> + { + if (flagEvent.getCheckName().equals(source1)) + { + source1Flags.remove(flagEvent.getCheckFlag()); + } + else if (flagEvent.getCheckName().equals(source2)) + { + source2Flags.remove(flagEvent.getCheckFlag()); + } + }); + + Assert.assertTrue(source1Flags.isEmpty()); + Assert.assertTrue(source2Flags.isEmpty()); + } + + @Test + public void testUniqueness() + { + // Add Object to flags + this.flag2.addObject(this.setup.atlas().node(1000000L)); + this.flag3.addObject(this.setup.atlas().edge(1000000L)); + this.sameObjectFlag2.addObject(this.setup.atlas().node(1000000L)); + + final UniqueCheckFlagContainer container = new UniqueCheckFlagContainer(); + // shouldn't deduplicate + container.add(source1, this.flag1); + container.add(source1, this.flag2); + Assert.assertEquals(2L, container.stream().count()); + + // should deduplicate + container.add(source1, this.sameIDFlag1); + Assert.assertEquals(2L, container.stream().count()); + + // shouldn't deduplicate + container.add(source2, this.flag1); + Assert.assertEquals(3L, container.stream().count()); + + // should deduplicate + container.add(source2, this.flag1); + container.add(source2, this.flag1); + container.add(source2, this.flag1); + container.add(source2, this.flag1); + Assert.assertEquals(3L, container.stream().count()); + + // Shouldn't deduplicate + container.add(source1, this.flag3); + Assert.assertEquals(4L, container.stream().count()); + + // Should deduplicate + container.add(source1, this.sameObjectFlag2); + Assert.assertEquals(4L, container.stream().count()); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTestRule.java new file mode 100644 index 000000000..25476dbe6 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/utility/UniqueCheckFlagContainerTestRule.java @@ -0,0 +1,33 @@ +package org.openstreetmap.atlas.checks.utility; + +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 UniqueCheckFlagContainerTest} + * + * @author bbreithaupt + */ +public class UniqueCheckFlagContainerTestRule extends CoreTestRule +{ + private static final String TEST_1 = "47.8586236, -122.6417178"; + private static final String TEST_2 = "47.8598930, -122.6398508"; + + @TestAtlas( + // nodes + nodes = { @Node(id = "1000000", coordinates = @Loc(value = TEST_1)), + @Node(id = "2000000", coordinates = @Loc(value = TEST_2)) }, + // edges + edges = { @Edge(id = "1000000", coordinates = { @Loc(value = TEST_1), + @Loc(value = TEST_2) }) }) + private Atlas atlas; + + public Atlas atlas() + { + return this.atlas; + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/EdgesTestCheck.java b/src/test/java/org/openstreetmap/atlas/checks/validation/EdgesTestCheck.java new file mode 100644 index 000000000..de1e7b6a8 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/EdgesTestCheck.java @@ -0,0 +1,46 @@ +package org.openstreetmap.atlas.checks.validation; + +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +import org.openstreetmap.atlas.checks.base.BaseCheck; +import org.openstreetmap.atlas.checks.flag.CheckFlag; +import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; +import org.openstreetmap.atlas.geography.atlas.items.Edge; +import org.openstreetmap.atlas.utilities.configuration.Configuration; + +/** + * A check for testing purposes. Flags all Edges. + * + * @author bbreithaupt + */ +public class EdgesTestCheck extends BaseCheck +{ + private static final long serialVersionUID = -4874724193461354673L; + private static final List FALLBACK_INSTRUCTIONS = Collections + .singletonList("Test check flag, please ignore."); + + public EdgesTestCheck(final Configuration configuration) + { + super(configuration); + } + + @Override + public Optional flag(final AtlasObject object) + { + return Optional.of(this.createFlag(object, this.getFallbackInstructions().get(0))); + } + + @Override + public boolean validCheckForObject(final AtlasObject object) + { + return object instanceof Edge; + } + + @Override + protected List getFallbackInstructions() + { + return FALLBACK_INSTRUCTIONS; + } +} diff --git a/src/test/resources/org/openstreetmap/atlas/checks/distributed/test_configuration.json b/src/test/resources/org/openstreetmap/atlas/checks/distributed/test_configuration.json new file mode 100644 index 000000000..747503301 --- /dev/null +++ b/src/test/resources/org/openstreetmap/atlas/checks/distributed/test_configuration.json @@ -0,0 +1,10 @@ +{ + "CheckResourceLoader": { + "scanUrls": [ + "org.openstreetmap.atlas.checks.validation" + ], + "type": "org.openstreetmap.atlas.checks.base.BaseCheck", + "enabled.value.default": false + }, + "EdgesTestCheck": {"enabled": true} +} \ No newline at end of file From 0e6f13a1d2c1b9d33b1645229e82eae2e2dd1d8f Mon Sep 17 00:00:00 2001 From: seancoulter <31253489+seancoulter@users.noreply.github.com> Date: Mon, 2 Mar 2020 11:51:51 -0800 Subject: [PATCH 11/15] [LineCrossingWaterBody] Expand waterbody definition and add flaggable items configurables (#263) * lcwb update to flag intersecting buildings * small fixes; update instruction * LCWB add switch for flagging specific lineItems * nonoffending building tags; introduce level tag * LCWB add more unit tests; make tags configurable * LCWB add more configurables * LCWB javadoc nit * spotlessApply * LCWB use isBuilding() * LCWB warning suppression Co-authored-by: Daniel B --- config/configuration.json | 11 +- .../LineCrossingWaterBodyCheck.java | 193 ++++++++++++++---- .../LineCrossingWaterBodyCheckTest.java | 33 +++ .../LineCrossingWaterBodyCheckTestRule.java | 62 ++++++ 4 files changed, 260 insertions(+), 39 deletions(-) diff --git a/config/configuration.json b/config/configuration.json index 1d05adabd..144099998 100644 --- a/config/configuration.json +++ b/config/configuration.json @@ -436,7 +436,16 @@ "tags": "building" } }, - "LineCrossingWaterBodyCheck": {}, + "LineCrossingWaterBodyCheck": { + "enabled": true, + "highway.minimum": "path", + "highways.exclude": [ + "bus_guideway" + ], + "lineItems.offending": "railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature", + "lineItems.non_offending": "waterway->*|boundary->*|landuse->*|bridge->yes,viaduct,aqueduct,boardwalk,covered,low_water_crossing,movable,suspension|tunnel->yes,culvert,building_passage|embankment->yes|location->underwater,underground|power->line,minor_line|man_made->pier,breakwater,embankment,groyne,dyke,pipeline|route->ferry|highway->proposed,construction|ice_road->yes|ford->yes|winter_road->yes|snowmobile->yes|ski->yes", + "buildings.flag": true + }, "LongSegmentCheck": { "length.minimum.kilometers": 10.0, "challenge": { diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheck.java index c34b4eef5..80dbb53bf 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheck.java +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheck.java @@ -6,10 +6,10 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.openstreetmap.atlas.checks.atlas.predicates.TagPredicates; import org.openstreetmap.atlas.checks.atlas.predicates.TypePredicates; import org.openstreetmap.atlas.checks.base.BaseCheck; import org.openstreetmap.atlas.checks.flag.CheckFlag; @@ -20,10 +20,15 @@ import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity; import org.openstreetmap.atlas.geography.atlas.items.AtlasItem; import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; +import org.openstreetmap.atlas.geography.atlas.items.Edge; import org.openstreetmap.atlas.geography.atlas.items.Line; import org.openstreetmap.atlas.geography.atlas.items.LineItem; import org.openstreetmap.atlas.geography.atlas.items.Relation; import org.openstreetmap.atlas.tags.AdministrativeLevelTag; +import org.openstreetmap.atlas.tags.BridgeTag; +import org.openstreetmap.atlas.tags.BuildingTag; +import org.openstreetmap.atlas.tags.HighwayTag; +import org.openstreetmap.atlas.tags.LevelTag; import org.openstreetmap.atlas.tags.NaturalTag; import org.openstreetmap.atlas.tags.NotesTag; import org.openstreetmap.atlas.tags.PlaceTag; @@ -35,56 +40,100 @@ import org.openstreetmap.atlas.utilities.configuration.Configuration; /** - * Flags line items (edges or lines) that are crossing water bodies invalidly. + * Flags line items (edges or lines) and optionally buildings that are crossing water bodies + * invalidly. Configurable values and * {@code LineCrossingWaterBodyCheck#canCrossWaterBody(AtlasItem)} and - * {@code Utilities#haveExplicitLocationsForIntersections(Polygon, AtlasItem)} is used to decide + * {@code Utilities#haveExplicitLocationsForIntersections(Polygon, AtlasItem)} are used to decide * whether a crossing is valid or not. * * @author mertk * @author savannahostrowski * @author sayana_saithu + * @author seancoulter */ public class LineCrossingWaterBodyCheck extends BaseCheck { private static final String LINEAR_INSTRUCTION = "Linear item {0,number,#} is crossing water body invalidly."; + private static final String BUILDING_INSTRUCTION = "Building item {0,number,#} is intersecting water body invalidly."; private static final String WATERBODY_INSTRUCTION = "The water body with id {0,number,#} has invalid crossings."; private static final List FALLBACK_INSTRUCTIONS = Arrays.asList(WATERBODY_INSTRUCTION, - LINEAR_INSTRUCTION); + LINEAR_INSTRUCTION, BUILDING_INSTRUCTION); private static final String ADDRESS_PREFIX_KEY = "addr"; // Whitelist for line tags - private static final Set VALID_LINE_TAGS = Stream.of(NotesTag.KEY, SourceTag.KEY, NaturalTag.KEY, PlaceTag.KEY, AdministrativeLevelTag.KEY).collect(Collectors.toSet()); // Whitelisted tags filter for multipolygon relations. Multipolygon relations with these tags - // are - // expected to cross water bodies. + // are expected to cross water bodies. private static final TaggableFilter VALID_RELATIONS_TAG_FILTER = TaggableFilter .forDefinition("natural->*|place->*|landuse->*|waterway->*|admin_level->*|boundary->*"); - private static final String CAN_CROSS_WATER_BODY_TAGS = "waterway->*|boundary->*|landuse->*|" + private static final String DEFAULT_CAN_CROSS_WATER_BODY_TAGS = "waterway->*|boundary->*|landuse->*|" + "bridge->yes,viaduct,aqueduct,boardwalk,covered,low_water_crossing,movable,suspension|tunnel->yes,culvert,building_passage|" + "embankment->yes|location->underwater,underground|power->line,minor_line|" + "man_made->pier,breakwater,embankment,groyne,dyke,pipeline|route->ferry|highway->proposed,construction|ice_road->yes|ford->yes|winter_road->yes|snowmobile->yes|ski->yes"; - private static final TaggableFilter CAN_CROSS_WATER_BODY_FILTER = TaggableFilter - .forDefinition(CAN_CROSS_WATER_BODY_TAGS); - private static final long serialVersionUID = 6048659185833217159L; + private TaggableFilter canCrossWaterBodyFilter; + private TaggableFilter lineItemsOffending; + private boolean flagBuildings; + private static final String DEFAULT_HIGHWAY_MINIMUM = "TOLL_GANTRY"; + private HighwayTag highwayMinimum; + private static final List DEFAULT_HIGHWAYS_EXCLUDE = Collections.emptyList(); + private List highwaysExclude; + private static final String BUILDING_TAGS_DO_NOT_FLAG = "public_transport->station,aerialway=station"; + private static final TaggableFilter NONOFFENDING_BUILDINGS = TaggableFilter + .forDefinition(BUILDING_TAGS_DO_NOT_FLAG); - /** - * Checks if given {@link AtlasItem} can cross a water body - * - * @param crossingItem - * {@link AtlasItem} crossing - * @return whether given {@link AtlasItem} can cross a water body - */ - private static boolean canCrossWaterBody(final AtlasItem crossingItem) - { - // In the following cases, given item can cross a water body + private static final String WATER_BODY_TAGS = + // Lakes + "natural->spring,hot_spring&name->*" + "|natural->lake,pond" + "|water:type->lake" + + "|landuse->pond" + "|water->lake,pond,oxbow,salt_lake" + - return CAN_CROSS_WATER_BODY_FILTER.test(crossingItem) - // It has a tag starting with addr - || crossingItem.containsKeyStartsWith(Collections.singleton(ADDRESS_PREFIX_KEY)) - // If crossing item is a line and meets the conditions for a boundary - || crossingItem instanceof Line && isBoundary(crossingItem); - } + // Rivers + "|natural->stream" + + "|water->canal,river,lock,moat,riverbank,creek,stream,stream_pool" + + "|waterway->river,riverbank,brook,ditch,stream,creek,canal,derelict_canal" + + "|stream->*" + "|waterway->drain&name->*" + "|water->drain&name->*" + + + // Reservoirs + "|water->reservoir" + "|water->dam&natural->water" + "|landuse->reservoir" + + "|natural->reservoir" + "|seamark:type->dam&natural->water" + + + // Miscellaneous + "|natural->water" + "|waterway->water" + "|water->water,perennial" + + "|landuse->water" + + + // Wetlands + "|wetland->tidalflat,reedbed" + "|water->tidalflat,reedbed" + + "|natural->tidalflat,reedbed" + + + // Lagoons + "|natural->lagoon" + "|water->lagoon" + "|waterway->lagoon" + + + // Intermittent/Dry lakes + "|intermittent->dry" + "|seasonal->dry" + "|natural->dry_lake" + + + // Unique + "|waterway->billabong,navigablechannel,river;stream,reservoir"; + private static final TaggableFilter VALID_WATER_BODY_TAGS = TaggableFilter + .forDefinition(WATER_BODY_TAGS); + + private static final String WATER_BODY_EXCLUDE_TAGS = "natural->dock,water_point,floodway,spillway,wastewater,waterhole" + + "|waterway->lock_gate,dock,water_point,floodway,spillway,wastewater,waterhole,culvert,dam,waterfall,fish_pass,dry_dock,construction,boat_lift,weir,breakwater,boatyard" + + "|water->lock_gate,dock,water_point,floodway,spillway,wastewater,waterhole,pool,reflecting_pool,swimming_pool,salt_pool,fountain,tank,fish_pass" + + "|tunnel->culvert" + "|waterway->drain&name->!" + "|water->drain&name->!" + + "|wetland->tidalflat,reedbed&seasonal->yes" + + "|water->tidalflat,reedbed&seasonal->yes" + + "|natural->tidalflat,reedbed&seasonal->yes" + "|covered->yes" + "|highway->*" + + "|natural->strait,channel,fjord,sound,bay" + "|harbour->*&harbour->!no" + + "|estuary->*&estuary->!no" + "|bay->*&bay->!no" + + "|seamark:type->harbour,harbour_basin,sea_area" + "|place->sea" + + "|water->bay,cove,harbour" + "|waterway->artificial,dock" + + "|man_made->breakwater,pier" + "|natural->beach,marsh,swamp" + "|water->marsh" + + "|wetland->bog,fen,mangrove,marsh,saltern,saltmarsh,string_bog,swamp,wet_meadow" + + "|waterway->drainage_channel,glacier,Minnow Falls, pumping_station" + + "|water->tank,Earth_Tank_,_Off_Stream_Flow_Dam,treatment_pond,re#,swamp_-_occasional,trough,Trough,waste_water"; + private static final TaggableFilter INVALID_WATER_BODY_TAGS = TaggableFilter + .forDefinition(WATER_BODY_EXCLUDE_TAGS); + + private static final long serialVersionUID = 6048659185833217159L; /** * Checks if the relation has whitelisted tags that makes its members cross water bodies @@ -146,23 +195,46 @@ private static boolean isBoundary(final AtlasEntity crossingLine) public LineCrossingWaterBodyCheck(final Configuration configuration) { super(configuration); + this.lineItemsOffending = TaggableFilter + .forDefinition(this.configurationValue(configuration, "lineItems.offending", "")); + this.flagBuildings = this.configurationValue(configuration, "buildings.flag", false); + this.canCrossWaterBodyFilter = TaggableFilter.forDefinition(this.configurationValue( + configuration, "lineItems.non_offending", DEFAULT_CAN_CROSS_WATER_BODY_TAGS)); + this.highwayMinimum = Enum.valueOf(HighwayTag.class, + this.configurationValue(configuration, "highway.minimum", DEFAULT_HIGHWAY_MINIMUM) + .toUpperCase()); + this.highwaysExclude = this + .configurationValue(configuration, "highways.exclude", DEFAULT_HIGHWAYS_EXCLUDE) + .stream().map(element -> Enum.valueOf(HighwayTag.class, element.toUpperCase())) + .collect(Collectors.toList()); } @Override public boolean validCheckForObject(final AtlasObject object) { - return TypePredicates.IS_AREA.test(object) && TagPredicates.IS_WATER_BODY.test(object); + // We only consider water body areas, not linear water bodies + return TypePredicates.IS_AREA.test(object) && !INVALID_WATER_BODY_TAGS.test(object) + && VALID_WATER_BODY_TAGS.test(object); } @Override + @SuppressWarnings("S2293") protected Optional flag(final AtlasObject object) { - // First retrieve the crossing edges and lines + // First retrieve the crossing edges, lines, buildings final Area objectAsArea = (Area) object; final Polygon areaAsPolygon = objectAsArea.asPolygon(); final Atlas atlas = object.getAtlas(); - final Iterable crossingLinearItems = new MultiIterable<>( - atlas.edgesIntersecting(areaAsPolygon), atlas.linesIntersecting(areaAsPolygon)); + final Iterable allCrossingItems = this.flagBuildings + ? new MultiIterable<>(atlas.lineItemsIntersecting( + areaAsPolygon, + lineItem -> isOffendingLineItem(object, areaAsPolygon).test(lineItem)), + atlas.areasIntersecting(areaAsPolygon, + area -> BuildingTag.isBuilding(area) + && !NONOFFENDING_BUILDINGS.test(area) + && LevelTag.areOnSameLevel(object, area))) + : new MultiIterable(atlas.lineItemsIntersecting(areaAsPolygon, + lineItem -> isOffendingLineItem(object, areaAsPolygon).test(lineItem))); // Assume there is no invalid crossing boolean hasInvalidCrossings = false; @@ -177,17 +249,17 @@ protected Optional flag(final AtlasObject object) // MapRoulette will display way-sectioned edges in case there is an invalid crossing. // Therefore, if an OSM way crosses a water body multiple times in separate edges, then // each edge will be marked explicitly. - for (final AtlasItem crossingLineItem : crossingLinearItems) + for (final AtlasItem crossingItem : allCrossingItems) { - // Check whether crossing linear item can actually cross - if (!canCrossWaterBody(crossingLineItem) + // Flag all buildings or if line item, check if it can actually cross + if (crossingItem instanceof Area || !this.canCrossWaterBody(crossingItem) && !IntersectionUtilities.haveExplicitLocationsForIntersections(areaAsPolygon, - (LineItem) crossingLineItem)) + (LineItem) crossingItem)) { // Update the flag - newFlag.addObject(crossingLineItem); - newFlag.addInstruction( - this.getLocalizedInstruction(1, crossingLineItem.getOsmIdentifier())); + newFlag.addObject(crossingItem); + newFlag.addInstruction(this.getLocalizedInstruction( + crossingItem instanceof Area ? 2 : 1, crossingItem.getOsmIdentifier())); // Set indicator to make sure we return invalid crossings hasInvalidCrossings = true; } @@ -207,4 +279,49 @@ protected List getFallbackInstructions() { return FALLBACK_INSTRUCTIONS; } + + /** + * Checks if given {@link AtlasItem} can cross a water body + * + * @param crossingItem + * {@link AtlasItem} crossing + * @return whether given {@link AtlasItem} can cross a water body + */ + private boolean canCrossWaterBody(final AtlasItem crossingItem) + { + // In the following cases, given item can cross a water body + + return this.canCrossWaterBodyFilter.test(crossingItem) + // It has a tag starting with addr + || crossingItem.containsKeyStartsWith(Collections.singleton(ADDRESS_PREFIX_KEY)) + // If crossing item is a line and meets the conditions for a boundary + || crossingItem instanceof Line && isBoundary(crossingItem); + } + + /** + * True if the incoming AtlasObject is an {@link Edge} with the correct highway type or + * {@link LineItem} with the correct tag combination AND the lineItem intersects the parameter + * waterbody polygon AND the lineItem is not a bridge and is on the same level as the parameter + * waterbody. + * + * @param object + * The waterbody + * @param areaAsPolygon + * The waterbody as a polygon + * @return True if the incoming AtlasObject is an Edge with the correct highway type or Line + * with the correct tag combination AND the lineItem intersects the parameter area AND + * the lineItem is not a bridge and is on the same level as the parameter area. + */ + private Predicate isOffendingLineItem(final AtlasObject object, + final Polygon areaAsPolygon) + { + return lineItem -> (lineItem instanceof Edge + && ((Edge) lineItem).highwayTag().isMoreImportantThanOrEqualTo(this.highwayMinimum) + && !this.highwaysExclude.contains(((Edge) lineItem).highwayTag()) + || this.lineItemsOffending.test(lineItem)) + && lineItem.intersects(areaAsPolygon) + && (!Validators.hasValuesFor(lineItem, BridgeTag.class) + || Validators.isOfType(lineItem, BridgeTag.class, BridgeTag.NO)) + && LevelTag.areOnSameLevel(object, lineItem); + } } diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTest.java index 600f8d87e..d56919153 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTest.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTest.java @@ -9,6 +9,7 @@ /** * @author mkalender * @author sayana_saithu + * @author seancoulter */ public class LineCrossingWaterBodyCheckTest { @@ -21,6 +22,18 @@ public class LineCrossingWaterBodyCheckTest @Rule public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier(); + @Test + public void testBuildingCrossing() + { + this.verifier.actual(this.setup.invalidCrossingBuildingAtlas(), + new LineCrossingWaterBodyCheck(ConfigurationResolver.inlineConfiguration( + "{ \"LineCrossingWaterBodyCheck\": {" + " \"enabled\": true," + + " \"lineItems.offending\": \"railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature\"," + + " \"buildings.flag\": true" + " }}"))); + this.verifier.verifyExpectedSize(1); + this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size())); + } + @Test public void testCrossingLineWithNoOsmTagAtlas() { @@ -61,6 +74,16 @@ public void testInvalidLineCrossingAtlas() this.verifier.verify(flag -> Assert.assertEquals(5, flag.getFlaggedObjects().size())); } + @Test + public void testInvalidLineItemCrossing() + { + this.verifier.actual(this.setup.invalidCrossingLineItemAtlas(), + new LineCrossingWaterBodyCheck(ConfigurationResolver.inlineConfiguration( + "{ \"LineCrossingWaterBodyCheck\": {" + " \"enabled\": true," + + " \"lineItems.offending\": \"railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature\" }}"))); + this.verifier.verify(flag -> Assert.assertEquals(1, flag.getFlaggedObjects().size())); + } + @Test public void testMultipolygonMemberCrossingAtlas() { @@ -89,4 +112,14 @@ public void testValidIntersectionItemsAtlas() this.verifier.actual(this.setup.validIntersectionItemsAtlas(), check); this.verifier.verifyEmpty(); } + + @Test + public void testValidLineItemCrossing() + { + this.verifier.actual(this.setup.validCrossingLineItemAtlas(), + new LineCrossingWaterBodyCheck(ConfigurationResolver.inlineConfiguration( + "{ \"LineCrossingWaterBodyCheck\": {" + " \"enabled\": true," + + " \"lineItems.offending\": \"railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature\" }}"))); + this.verifier.verifyEmpty(); + } } diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTestRule.java index 28b0b3543..89308150d 100644 --- a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTestRule.java +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/LineCrossingWaterBodyCheckTestRule.java @@ -409,6 +409,53 @@ public class LineCrossingWaterBodyCheckTestRule extends CoreTestRule @Loc(value = LOCATION_OUTSIDE_AREA_4) }) }) private Atlas crossingLineWithValidLineTagAtlas; + @TestAtlas( + // Nodes + nodes = { @Node(coordinates = @Loc(value = AREA_LOCATION_1)), + @Node(coordinates = @Loc(value = AREA_LOCATION_2)), + @Node(coordinates = @Loc(value = AREA_LOCATION_3)), + @Node(coordinates = @Loc(value = AREA_LOCATION_4)), + @Node(coordinates = @Loc(value = AREA_LOCATION_5)), + @Node(coordinates = @Loc(value = LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(value = AREA_LOCATION_BETWEEN_2_AND_3)) }, + // Area + areas = { + @Area(coordinates = { @Loc(value = AREA_LOCATION_1), + @Loc(value = AREA_LOCATION_2), @Loc(value = AREA_LOCATION_3), + @Loc(value = AREA_LOCATION_4), + @Loc(value = AREA_LOCATION_5) }, tags = { "landuse=reservoir" }), + @Area(coordinates = { @Loc(LOCATION_OUTSIDE_AREA_1), @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_BETWEEN_2_AND_3) }, tags = { "building=hut" }) }) + private Atlas invalidCrossingBuildingAtlas; + + @TestAtlas( + // Nodes + nodes = { @Node(coordinates = @Loc(value = AREA_LOCATION_1)), + @Node(coordinates = @Loc(value = AREA_LOCATION_2)), + @Node(coordinates = @Loc(value = AREA_LOCATION_3)), + @Node(coordinates = @Loc(value = AREA_LOCATION_4)), + @Node(coordinates = @Loc(value = AREA_LOCATION_5)), + @Node(coordinates = @Loc(value = LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(value = AREA_LOCATION_BETWEEN_2_AND_3)) }, + // Lines + lines = { @Line(coordinates = { @Loc(AREA_LOCATION_1), + @Loc(LOCATION_OUTSIDE_AREA_1) }, tags = "railway=abandoned") }) + private Atlas validCrossingLineItemAtlas; + + @TestAtlas( + // Nodes + nodes = { @Node(coordinates = @Loc(value = AREA_LOCATION_1)), + @Node(coordinates = @Loc(value = AREA_LOCATION_2)), + @Node(coordinates = @Loc(value = AREA_LOCATION_3)), + @Node(coordinates = @Loc(value = AREA_LOCATION_4)), + @Node(coordinates = @Loc(value = AREA_LOCATION_5)), + @Node(coordinates = @Loc(value = LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(value = AREA_LOCATION_BETWEEN_2_AND_3)) }, + // Lines + lines = { @Line(coordinates = { @Loc(AREA_LOCATION_1), + @Loc(LOCATION_OUTSIDE_AREA_1) }, tags = "railway=tram") }) + private Atlas invalidCrossingLineItemAtlas; + public Atlas crossingLineWithNoOsmTagAtlas() { return this.crossingLineWithNoOsmTagAtlas; @@ -419,11 +466,21 @@ public Atlas crossingLineWithValidLineTagAtlas() return this.crossingLineWithValidLineTagAtlas; } + public Atlas invalidCrossingBuildingAtlas() + { + return this.invalidCrossingBuildingAtlas; + } + public Atlas invalidCrossingItemsAtlas() { return this.invalidCrossingItemsAtlas; } + public Atlas invalidCrossingLineItemAtlas() + { + return this.invalidCrossingLineItemAtlas; + } + public Atlas invalidIntersectionItemsAtlas() { return this.invalidIntersectionItemsAtlas; @@ -449,6 +506,11 @@ public Atlas validCrossingItemsAtlas() return this.validCrossingItemsAtlas; } + public Atlas validCrossingLineItemAtlas() + { + return this.validCrossingLineItemAtlas; + } + public Atlas validIntersectionItemsAtlas() { return this.validIntersectionItemsAtlas; From 307c085e84f502746f0296a769285c006d13a184 Mon Sep 17 00:00:00 2001 From: Daniel B Date: Tue, 3 Mar 2020 08:07:06 -0800 Subject: [PATCH 12/15] bump after release (#262) --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 34f92ef3b..76e83e453 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ group=org.openstreetmap.atlas -version=6.0.1-SNAPSHOT +version=6.0.2-SNAPSHOT maven2_url=https://oss.sonatype.org/service/local/staging/deploy/maven2/ snapshot_url=https://oss.sonatype.org/content/repositories/snapshots/ From 227593146c88ffa68cef41b90e5b33e5416d3cb2 Mon Sep 17 00:00:00 2001 From: Daniel B Date: Wed, 4 Mar 2020 13:16:35 -0800 Subject: [PATCH 13/15] use URIUtil (#265) --- .../checks/maproulette/MapRouletteConnection.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteConnection.java b/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteConnection.java index 282234bfb..e95be9acd 100644 --- a/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteConnection.java +++ b/src/main/java/org/openstreetmap/atlas/checks/maproulette/MapRouletteConnection.java @@ -3,12 +3,13 @@ import java.io.Serializable; import java.io.UnsupportedEncodingException; import java.net.URISyntaxException; -import java.net.URLEncoder; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Set; +import org.apache.commons.httpclient.URIException; +import org.apache.commons.httpclient.util.URIUtil; import org.apache.http.HttpStatus; import org.apache.http.client.utils.URIBuilder; import org.apache.http.entity.ContentType; @@ -143,9 +144,18 @@ public long createChallenge(final Project project, final Challenge challenge) { final JsonObject challengeJson = challenge.toJson(challenge.getName()); final String type = challengeJson.has(Survey.KEY_ANSWERS) ? KEY_SURVEY : KEY_CHALLENGE; + String encodedChallengeQuery = challenge.getName(); + try + { + encodedChallengeQuery = URIUtil.encodeQuery(challenge.getName()); + } + catch (final URIException error) + { + logger.info("Unable to encode Challenge name {}.", challenge.getName()); + } return create( String.format("/api/v2/project/%d/challenge/%s", project.getId(), - URLEncoder.encode(challenge.getName(), "UTF-8")), + encodedChallengeQuery), String.format("/api/v2/%s", type), String.format("/api/v2/%s/", type) + "%s", challengeJson, String.format("Created/Updated Challenge with ID {} and name %s", challenge.getName())); From bc161543f4d61d203c2f3763e30441d13e3fc282 Mon Sep 17 00:00:00 2001 From: Mara Alexandria Rae Date: Wed, 4 Mar 2020 15:10:03 -0800 Subject: [PATCH 14/15] OceanBleedingCheck new check (#266) * intital commit ocean bleeding check * add invalid ocean tags * add LCWB config data; update OceanBleedingCheck * OceanBleeding check make configurable items * add readme, update unit tests * spotlessApply * revert change * update config value name --- config/configuration.json | 28 ++- docs/available_checks.md | 1 + docs/checks/oceanBleedingCheck.md | 14 ++ .../intersections/OceanBleedingCheck.java | 168 ++++++++++++++++++ .../intersections/OceanBleedingCheckTest.java | 70 ++++++++ .../OceanBleedingCheckTestRule.java | 122 +++++++++++++ 6 files changed, 401 insertions(+), 2 deletions(-) create mode 100644 docs/checks/oceanBleedingCheck.md create mode 100644 src/main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTestRule.java diff --git a/config/configuration.json b/config/configuration.json index 144099998..90af61653 100644 --- a/config/configuration.json +++ b/config/configuration.json @@ -437,14 +437,19 @@ } }, "LineCrossingWaterBodyCheck": { - "enabled": true, "highway.minimum": "path", "highways.exclude": [ "bus_guideway" ], "lineItems.offending": "railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature", "lineItems.non_offending": "waterway->*|boundary->*|landuse->*|bridge->yes,viaduct,aqueduct,boardwalk,covered,low_water_crossing,movable,suspension|tunnel->yes,culvert,building_passage|embankment->yes|location->underwater,underground|power->line,minor_line|man_made->pier,breakwater,embankment,groyne,dyke,pipeline|route->ferry|highway->proposed,construction|ice_road->yes|ford->yes|winter_road->yes|snowmobile->yes|ski->yes", - "buildings.flag": true + "buildings.flag": true, + "challenge": { + "description": "Certain OSM features should not cross waterbodies.", + "blurb": "Edit features overlapping the waterbody so they either validly overlap or do not overlap.", + "instruction": "Open your favorite editor and edit the features overlapping the waterbody so they either validly overlap or do not overlap.", + "difficulty": "EASY" + } }, "LongSegmentCheck": { "length.minimum.kilometers": 10.0, @@ -964,5 +969,24 @@ "difficulty":"NORMAL", "defaultPriority":"MEDIUM" } + }, + "OceanBleedingCheck": { + "ocean": { + "valid": "natural->strait,channel,fjord,sound,bay|harbour->*&harbour->!no|estuary->*&estuary->!no|bay->*&bay->!no|place->sea|seamark:type->harbour,harbour_basin,sea_area|water->bay,cove,harbour|waterway->artificial,dock", + "invalid": "man_made->breakwater,pier|natural->beach,marsh,swamp|water->marsh|wetland->bog,fen,mangrove,marsh,saltern,saltmarsh,string_bog,swamp,wet_meadow|landuse->*" + }, + "highway": { + "minimum": "path", + "exclude": [ + "bus_guideway" + ] + }, + "lineItems.offending": "railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature", + "challenge": { + "description": "Certain OSM features should not bleed into (intersect) oceans.", + "blurb": "Edit features overlapping the ocean feature so they either validly intersect or do not overlap.", + "instruction": "Open your favorite editor and edit the features overlapping the ocean so they either validly overlap or do not overlap.", + "difficulty": "EASY" + } } } diff --git a/docs/available_checks.md b/docs/available_checks.md index d0a784ec6..ea6ff0869 100644 --- a/docs/available_checks.md +++ b/docs/available_checks.md @@ -11,6 +11,7 @@ This document is a list of tables with a description and link to documentation f | [ShadowDetectionCheck](checks/shadowDetectionCheck.md) | The purpose of this check is to identify floating buildings. | | [SpikyBuildingCheck](checks/spikyBuildingCheck.md) | The purpose of this check is to identify buildings with extremely sharp angles in their geometry. | | [WaterbodyAndIslandSizeCheck](checks/waterbodyAndIslandSizeCheck.md) | The purpose of this check is to identify waterbodies and islands which are either too small or too large in size. | +| [OceanBleedingCheck](checks/oceanBleedingCheck.md) | The purpose of this check is to identify streets, buildings, and railways that bleed into (intersect) an ocean feature. | ## Highways | Check Name | Check Description | diff --git a/docs/checks/oceanBleedingCheck.md b/docs/checks/oceanBleedingCheck.md new file mode 100644 index 000000000..df177e77d --- /dev/null +++ b/docs/checks/oceanBleedingCheck.md @@ -0,0 +1,14 @@ +# Ocean Bleeding Check + +This check aims to flag streets, railways, and buildings that bleed into (intersect) ocean features. Intersection includes any geometrical interaction between the ocean feature and the land feature. The definition of streets and railways can be changed in the configuration for the check ("lineItems.offending" for railways, "highway.minimum" and "highway.exclude" for streets) Additionally, tags that should be considered when validating/invalidating an ocean feature are configurable. + +#### Live Examples + +1. Building [id:355294262](https://www.openstreetmap.org/way/355294262#map=19/22.36138/114.09546&layers=C) extends into ocean feature [id:243872591](https://www.openstreetmap.org/way/243872591#map=16/22.3630/114.0932&layers=C) invalidly. +2. Street [id:327223335](https://www.openstreetmap.org/way/327223335#map=17/25.21095/55.24491&layers=C) extends into ocean feature [id:87287185](https://www.openstreetmap.org/way/87287185#map=17/25.21143/55.24443&layers=C) invalidly. + +#### Code Review + +The check starts off by validating certain waterbodies (Atlas Areas or LineItems) as being ocean features. Then it collects all valid buildings, streets, and railways that intersect the given ocean feature. A single flag is created which includes all intersecting land features for the ocean feature. The check repeats this process for every Area and LineItem in the supplied atlas. + +Please see source code for OceanBleedingCheck here: [OceanBleedingCheck](../../src/main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java) diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java new file mode 100644 index 000000000..3221dfdda --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java @@ -0,0 +1,168 @@ +package org.openstreetmap.atlas.checks.validation.intersections; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +import org.openstreetmap.atlas.checks.base.BaseCheck; +import org.openstreetmap.atlas.checks.flag.CheckFlag; +import org.openstreetmap.atlas.geography.Polygon; +import org.openstreetmap.atlas.geography.atlas.items.Area; +import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; +import org.openstreetmap.atlas.geography.atlas.items.Edge; +import org.openstreetmap.atlas.geography.atlas.items.LineItem; +import org.openstreetmap.atlas.tags.BridgeTag; +import org.openstreetmap.atlas.tags.BuildingTag; +import org.openstreetmap.atlas.tags.HighwayTag; +import org.openstreetmap.atlas.tags.filters.TaggableFilter; +import org.openstreetmap.atlas.utilities.configuration.Configuration; + +/** + * Flags railways (configurable), streets (configurable), buildings that bleed into an ocean. An + * ocean is defined by a set of ocean tags, and can be an {@link Area} or {@link LineItem}. + * + * @author seancoulter + */ +public class OceanBleedingCheck extends BaseCheck +{ + private static final String DEFAULT_VALID_OCEAN_TAGS = "natural->strait,channel,fjord,sound,bay|" + + "harbour->*&harbour->!no|estuary->*&estuary->!no|bay->*&bay->!no|place->sea|seamark:type->harbour,harbour_basin,sea_area|water->bay,cove,harbour|waterway->artificial,dock"; + private final TaggableFilter validOceanTags; + private static final String DEFAULT_INVALID_OCEAN_TAGS = "man_made->breakwater,pier" + + "|natural->beach,marsh,swamp" + "|water->marsh" + + "|wetland->bog,fen,mangrove,marsh,saltern,saltmarsh,string_bog,swamp,wet_meadow" + + "|landuse->*"; + private final TaggableFilter invalidOceanTags; + private static final String DEFAULT_OFFENDING_MISCELLANEOUS_LINEITEMS = "railway->rail,narrow_gauge,preserved,subway,disused,monorail,tram,light_rail,funicular,construction,miniature"; + private final TaggableFilter defaultOffendingLineitems; + private static final String DEFAULT_HIGHWAY_MINIMUM = "TOLL_GANTRY"; + private final HighwayTag highwayMinimum; + private static final List DEFAULT_HIGHWAYS_EXCLUDE = Collections.emptyList(); + private final List highwaysExclude; + private static final String OCEAN_INSTRUCTION = "Ocean feature {0,number,#} has invalid intersections."; + private static final String BLEEDING_BUILDING_INSTRUCTION = "Building {0,number,#} intersects the ocean feature."; + private static final String BLEEDING_LINEITEM_INSTRUCTION = "Way {0,number,#} intersects the ocean feature."; + private static final List FALLBACK_INSTRUCTIONS = Arrays.asList( + BLEEDING_BUILDING_INSTRUCTION, BLEEDING_LINEITEM_INSTRUCTION, OCEAN_INSTRUCTION); + private static final long serialVersionUID = -2229281211747728380L; + + /** + * The default constructor that must be supplied. The Atlas Checks framework will generate the + * checks with this constructor, supplying a configuration that can be used to adjust any + * parameters that the check uses during operation. + * + * @param configuration + * the JSON configuration for this check + */ + public OceanBleedingCheck(final Configuration configuration) + { + super(configuration); + this.validOceanTags = TaggableFilter.forDefinition( + this.configurationValue(configuration, "ocean.valid", DEFAULT_VALID_OCEAN_TAGS)); + this.invalidOceanTags = TaggableFilter.forDefinition(this.configurationValue(configuration, + "ocean.invalid", DEFAULT_INVALID_OCEAN_TAGS)); + this.defaultOffendingLineitems = TaggableFilter.forDefinition(this.configurationValue( + configuration, "lineItems.offending", DEFAULT_OFFENDING_MISCELLANEOUS_LINEITEMS)); + this.highwayMinimum = Enum.valueOf(HighwayTag.class, + this.configurationValue(configuration, "highway.minimum", DEFAULT_HIGHWAY_MINIMUM) + .toUpperCase()); + this.highwaysExclude = this + .configurationValue(configuration, "highway.exclude", DEFAULT_HIGHWAYS_EXCLUDE) + .stream().map(element -> Enum.valueOf(HighwayTag.class, element.toUpperCase())) + .collect(Collectors.toList()); + } + + /** + * This function will validate ocean features that are tagged appropriately. + * + * @param object + * the atlas object supplied by the Atlas-Checks framework for evaluation + * @return {@code true} if this object should be checked + */ + @Override + public boolean validCheckForObject(final AtlasObject object) + { + return this.validOceanTags.test(object) && !this.invalidOceanTags.test(object) + && (object instanceof Area || object instanceof LineItem); + } + + /** + * We flag railways, streets, and buildings that intersect the ocean feature, so each flag is a + * collection of all intersections for a given ocean feature. + * + * @param object + * the atlas object supplied by the Atlas-Checks framework for evaluation + * @return an optional {@link CheckFlag} object that flags the objects that invalidly intersect + * the ocean feature + */ + @Override + protected Optional flag(final AtlasObject object) + { + // Use this flag to see if we need to check for actual intersection (not just intersection + // on the closed surface representation of the PolyLine) when we query the Atlas looking for + // intersecting features + final boolean oceanIsArea = object instanceof Area; + + // Ocean boundary, make it a closed polygon + final Polygon oceanBoundary = oceanIsArea ? ((Area) object).asPolygon() + : new Polygon(((LineItem) object).asPolyLine()); + + // Collect offending line items (non-bridges) and buildings + // We do a second check in the predicate for actual intersection on the ocean boundary if + // the ocean boundary is a LineItem. Or else we just use the area polygon. + final Iterable intersectingRoads = object.getAtlas().lineItemsIntersecting( + oceanBoundary, + lineItem -> (oceanIsArea + || (((LineItem) object).asPolyLine()).intersects(lineItem.asPolyLine())) + && !BridgeTag.isBridge(lineItem) + && (lineItem instanceof Edge + && this.validHighwayType().test((Edge) lineItem) + || this.defaultOffendingLineitems.test(lineItem))); + final Iterable intersectingBuildings = object.getAtlas().areasIntersecting( + oceanBoundary, + area -> (oceanIsArea + || ((LineItem) object).asPolyLine().intersects(area.asPolygon())) + && BuildingTag.isBuilding(area)); + + // Unify all offenders in storage so the flag id is generated from a single set of flagged + // objects + final HashSet flaggedObjects = new HashSet<>(); + final StringBuilder instructions = new StringBuilder(); + instructions.append(this.getLocalizedInstruction(2, object.getOsmIdentifier())); + intersectingBuildings.forEach(building -> + { + flaggedObjects.add(building); + instructions.append(this.getLocalizedInstruction(0, building.getOsmIdentifier(), + object.getOsmIdentifier())); + }); + intersectingRoads.forEach(road -> + { + flaggedObjects.add(road); + instructions.append(this.getLocalizedInstruction(1, road.getOsmIdentifier(), + object.getOsmIdentifier())); + }); + return flaggedObjects.isEmpty() ? Optional.empty() + : Optional.of(this.createFlag(flaggedObjects, instructions.toString())); + } + + @Override + protected List getFallbackInstructions() + { + return FALLBACK_INSTRUCTIONS; + } + + /** + * Validates the supplied street {@link Edge} + * + * @return true if the Edge is a valid street, false otherwise + */ + private Predicate validHighwayType() + { + return edge -> edge.highwayTag().isMoreImportantThanOrEqualTo(this.highwayMinimum) + && !this.highwaysExclude.contains(edge.highwayTag()); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTest.java new file mode 100644 index 000000000..3922afb33 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTest.java @@ -0,0 +1,70 @@ +package org.openstreetmap.atlas.checks.validation.intersections; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver; +import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier; + +/** + * Unit tests for {@link OceanBleedingCheck} + * + * @author seancoulter + */ +public class OceanBleedingCheckTest +{ + @Rule + public OceanBleedingCheckTestRule setup = new OceanBleedingCheckTestRule(); + + @Rule + public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier(); + + @Test + public void invalidBuildingBleedingIntoOcean() + { + this.verifier.actual(this.setup.getInvalidBuildingBleedingIntoOcean(), + new OceanBleedingCheck(ConfigurationResolver.emptyConfiguration())); + this.verifier.verifyExpectedSize(1); + } + + @Test + public void invalidRailwayBleedingIntoOcean() + { + this.verifier.actual(this.setup.getInvalidRailwayBleedingIntoOcean(), + new OceanBleedingCheck(ConfigurationResolver.emptyConfiguration())); + this.verifier.verifyExpectedSize(1); + } + + @Test + public void invalidStreetBleedingIntoOcean() + { + this.verifier.actual(this.setup.getInvalidStreetBleedingIntoOcean(), + new OceanBleedingCheck(ConfigurationResolver.emptyConfiguration())); + this.verifier.verifyExpectedSize(1); + } + + @Test + public void validBridgeOverOceanBoundary() + { + this.verifier.actual(this.setup.getValidBridgeOverOceanBoundary(), + new OceanBleedingCheck(ConfigurationResolver.emptyConfiguration())); + this.verifier.verifyEmpty(); + } + + @Test + public void validPathIntoOcean() + { + this.verifier.actual(this.setup.getPathBleedingIntoOcean(), + new OceanBleedingCheck(ConfigurationResolver.inlineConfiguration( + "{\"OceanBleedingCheck\": {\"highway.minimum\": \"service\"}}"))); + this.verifier.verifyEmpty(); + } + + @Test + public void validRailwayBleedingIntoWaterbodyNonOcean() + { + this.verifier.actual(this.setup.getValidRailwayBleedingIntoWaterbodyNonOcean(), + new OceanBleedingCheck(ConfigurationResolver.emptyConfiguration())); + this.verifier.verifyEmpty(); + } + +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTestRule.java new file mode 100644 index 000000000..05a008e47 --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheckTestRule.java @@ -0,0 +1,122 @@ +package org.openstreetmap.atlas.checks.validation.intersections; + +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.Area; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Edge; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Line; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Loc; +import org.openstreetmap.atlas.utilities.testing.TestAtlas.Node; + +/** + * Unit test rule for {@link OceanBleedingCheck} + * + * @author seancoulter + */ +public class OceanBleedingCheckTestRule extends CoreTestRule +{ + private static final String AREA_LOCATION_1 = "47.6312, -122.3406"; + private static final String AREA_LOCATION_2 = "47.6263, -122.3370"; + private static final String AREA_LOCATION_3 = "47.6328, -122.3271"; + + private static final String LOCATION_INSIDE_OCEAN = "47.6348, -122.3352"; + private static final String LOCATION_OUTSIDE_AREA_1 = "47.6182, -122.3366"; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(AREA_LOCATION_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)), @Node(coordinates = @Loc(AREA_LOCATION_3)), + @Node(coordinates = @Loc(LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(LOCATION_INSIDE_OCEAN)) }, areas = { + @Area(tags = { "natural=bay" }, coordinates = { @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_2), @Loc(AREA_LOCATION_3) }) }, lines = { + @Line(tags = { "railway=rail" }, coordinates = { + @Loc(LOCATION_OUTSIDE_AREA_1), + @Loc(LOCATION_INSIDE_OCEAN) }) }) + private Atlas invalidRailwayBleedingIntoOcean; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(AREA_LOCATION_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)), @Node(coordinates = @Loc(AREA_LOCATION_3)), + @Node(coordinates = @Loc(LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(LOCATION_INSIDE_OCEAN)) }, areas = { + @Area(tags = { "place=sea" }, coordinates = { @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_2), @Loc(AREA_LOCATION_3) }), + @Area(tags = { "building=greenhouse" }, coordinates = { + @Loc(LOCATION_OUTSIDE_AREA_1), @Loc(LOCATION_INSIDE_OCEAN), + @Loc(AREA_LOCATION_1) }) }) + private Atlas invalidBuildingBleedingIntoOcean; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(AREA_LOCATION_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)), @Node(coordinates = @Loc(AREA_LOCATION_3)), + @Node(coordinates = @Loc(LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(LOCATION_INSIDE_OCEAN)) }, areas = { + @Area(tags = { "seamark:type=sea_area" }, coordinates = { @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_2), @Loc(AREA_LOCATION_3) }) }, edges = { + @Edge(tags = { "highway=tertiary" }, coordinates = { + @Loc(LOCATION_OUTSIDE_AREA_1), + @Loc(LOCATION_INSIDE_OCEAN) }) }) + private Atlas invalidStreetBleedingIntoOcean; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(AREA_LOCATION_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)), @Node(coordinates = @Loc(AREA_LOCATION_3)), + @Node(coordinates = @Loc(LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(LOCATION_INSIDE_OCEAN)) }, areas = { + @Area(tags = { "seamark:type=sea_area" }, coordinates = { @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_2), @Loc(AREA_LOCATION_3) }) }, edges = { + @Edge(tags = { "highway=path" }, coordinates = { + @Loc(LOCATION_OUTSIDE_AREA_1), + @Loc(LOCATION_INSIDE_OCEAN) }) }) + private Atlas pathBleedingIntoOcean; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(AREA_LOCATION_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)), @Node(coordinates = @Loc(AREA_LOCATION_3)), + @Node(coordinates = @Loc(LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(LOCATION_INSIDE_OCEAN)) }, areas = { + @Area(tags = { "place=ocean" }, coordinates = { @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_2), @Loc(AREA_LOCATION_3) }) }, lines = { + @Line(tags = { "railway=rail" }, coordinates = { + @Loc(LOCATION_OUTSIDE_AREA_1), + @Loc(LOCATION_INSIDE_OCEAN) }) }) + private Atlas validRailwayBleedingIntoWaterbodyNonOcean; + + @TestAtlas(nodes = { @Node(coordinates = @Loc(AREA_LOCATION_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)), @Node(coordinates = @Loc(AREA_LOCATION_3)), + @Node(coordinates = @Loc(LOCATION_OUTSIDE_AREA_1)), + @Node(coordinates = @Loc(AREA_LOCATION_2)) }, areas = { + @Area(tags = { "natural=fjord" }, coordinates = { @Loc(AREA_LOCATION_1), + @Loc(AREA_LOCATION_2), @Loc(AREA_LOCATION_3) }) }, lines = { + @Line(tags = { "bridge=yes" }, coordinates = { + @Loc(LOCATION_OUTSIDE_AREA_1), + @Loc(LOCATION_INSIDE_OCEAN) }) }) + private Atlas validBridgeOverOceanBoundary; + + public Atlas getInvalidBuildingBleedingIntoOcean() + { + return this.invalidBuildingBleedingIntoOcean; + } + + public Atlas getInvalidRailwayBleedingIntoOcean() + { + return this.invalidRailwayBleedingIntoOcean; + } + + public Atlas getInvalidStreetBleedingIntoOcean() + { + return this.invalidStreetBleedingIntoOcean; + } + + public Atlas getPathBleedingIntoOcean() + { + return this.pathBleedingIntoOcean; + } + + public Atlas getValidBridgeOverOceanBoundary() + { + return this.validBridgeOverOceanBoundary; + } + + public Atlas getValidRailwayBleedingIntoWaterbodyNonOcean() + { + return this.validRailwayBleedingIntoWaterbodyNonOcean; + } + +} From 241f6a38575b45d7a4e4d36f18d82a571854bd68 Mon Sep 17 00:00:00 2001 From: Daniel B Date: Wed, 4 Mar 2020 15:52:20 -0800 Subject: [PATCH 15/15] Bump to Atlas version 6.1.0 (#267) * Bump to Atlas version 6.0.3 * Bump to 6.1.0 --- dependencies.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencies.gradle b/dependencies.gradle index 42e2e1226..9fcbcf783 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -1,7 +1,7 @@ project.ext.versions = [ checkstyle: '8.18', jacoco: '0.8.3', - atlas: '6.0.2', + atlas: '6.1.0', commons:'2.6', atlas_generator: '5.0.1', atlas_checkstyle: '5.6.9',