From ab2dd1e70b491aaa86473c9323a38842516af7c1 Mon Sep 17 00:00:00 2001 From: Gabe Reichenberger Date: Wed, 16 Dec 2020 16:48:53 -0800 Subject: [PATCH] Open boundary check (#441) * check seems to be successful, still need to write tests * adding documentation for check and moved check file to relations directory. * Passed unit tests. pased unit tests. * spotles apply * latlon function to clean up flag function * added final parameter in latlon function. * added this. * spotless apply again. * clearing changes * updating docker ubuntu. * added this * fixing config --- config/configuration.json | 8 + docs/available_checks.md | 1 + docs/checks/openBoundaryCheck.md | 31 ++++ .../relations/OpenBoundaryCheck.java | 166 ++++++++++++++++++ .../relations/OpenBoundaryCheckTest.java | 39 ++++ .../relations/OpenBoundaryCheckTestRule.java | 87 +++++++++ 6 files changed, 332 insertions(+) create mode 100644 docs/checks/openBoundaryCheck.md create mode 100644 src/main/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheck.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTest.java create mode 100644 src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTestRule.java diff --git a/config/configuration.json b/config/configuration.json index ace244d1d..2727f51e7 100644 --- a/config/configuration.json +++ b/config/configuration.json @@ -630,6 +630,14 @@ "difficulty": "HARD" } }, + "OpenBoundaryCheck": { + "challenge": { + "description": "Admin boundaries should be closed polygons. This check catches admin boundaries that are not closed..", + "blurb": "Open Boundary Check", + "instruction": "Please close the boundary", + "difficulty": "MEDIUM" + } + }, "OverlappingAOIPolygonCheck":{ "aoi.tags.filters": ["amenity->FESTIVAL_GROUNDS", "amenity->GRAVE_YARD|landuse->CEMETERY", "boundary->NATIONAL_PARK,PROTECTED_AREA|leisure->NATURE_RESERVE,PARK", diff --git a/docs/available_checks.md b/docs/available_checks.md index c5ddc0998..c0d12a4a6 100644 --- a/docs/available_checks.md +++ b/docs/available_checks.md @@ -66,6 +66,7 @@ This document is a list of tables with a description and link to documentation f | [InvalidTurnRestrictionCheck](checks/invalidTurnRestrictionCheck.md) | The purpose of this check is to identify invalid turn restrictions in OSM. Invalid turn restrictions occur in a variety of ways from invalid members, Edge geometry issues, not being routable, or wrong topology. | | InvalidSignBoardRelationCheck | Identifies signboard relations that do not meet all the requirements for a signboard relation. | | [OneMemberRelationCheck](checks/oneMemberRelationCheck.md) | The purpose of this check is to identify Relations in OSM which only have one Member. | +| [OpenBoundaryCheck](checks/openBoundaryCheck.md) | This check attempts to check for Admin Boundary Relations that should be closed polygons but are not closed. | ## Tags | Check Name | Check Description | diff --git a/docs/checks/openBoundaryCheck.md b/docs/checks/openBoundaryCheck.md new file mode 100644 index 000000000..f599f6ed3 --- /dev/null +++ b/docs/checks/openBoundaryCheck.md @@ -0,0 +1,31 @@ +# OpenBoundaryCheck + +#### Description +This check attempts to check for Admin Boundary Relations that should be closed polygons but are not closed. + +#### Configurables + +#### Live Examples +Yakintas neighborhood is not a vclosed boundary but should be. +1. The relation [id:8284136](https://www.openstreetmap.org/relation/8284136) is not a closed boundary. + +Seattle is a false positive since it is a closed boundary. +1. The relation [id:237385](https://www.openstreetmap.org/relation/237385) is a closed boundary and would not be flagged. + +#### Code Review +This check evaluates [Relations](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Relation.java) +, finding admin boundaries that are not closed. + +##### Validating the Object +We first validate that the incoming object is: +* Is a Relation +* Is of type Boundary +* Has admin_level tags + +##### Flagging the Relation +Catch the [OpenPolygonException](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/converters/MultiplePolyLineToPolygonsConverter.java) while using the [RelationOrAreaToMultiPolygonConverter](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/complex/RelationOrAreaToMultiPolygonConverter.java) +to inform that the polygon is not closed. + +##### Miscellaneous +To learn more about the code, please look at the comments in the source code for the check. +[OpenBoundaryCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheck.java) \ No newline at end of file diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheck.java new file mode 100644 index 000000000..e871f67a1 --- /dev/null +++ b/src/main/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheck.java @@ -0,0 +1,166 @@ +package org.openstreetmap.atlas.checks.validation.relations; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +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.Location; +import org.openstreetmap.atlas.geography.atlas.items.AtlasObject; +import org.openstreetmap.atlas.geography.atlas.items.Relation; +import org.openstreetmap.atlas.geography.atlas.items.complex.RelationOrAreaToMultiPolygonConverter; +import org.openstreetmap.atlas.geography.converters.MultiplePolyLineToPolygonsConverter; +import org.openstreetmap.atlas.tags.AdministrativeLevelTag; +import org.openstreetmap.atlas.tags.RelationTypeTag; +import org.openstreetmap.atlas.tags.SyntheticGeometrySlicedTag; +import org.openstreetmap.atlas.tags.SyntheticRelationMemberAdded; +import org.openstreetmap.atlas.tags.annotations.validation.Validators; +import org.openstreetmap.atlas.utilities.configuration.Configuration; + +/** + * OpenBoundaryCheck This check flags boundaries that should be closed polygons but are not. + * + * @author v-garei + */ +public class OpenBoundaryCheck extends BaseCheck +{ + private static final long serialVersionUID = 6655863145391887741L; + private static final String OPEN_BOUNDARY_INSTRUCTIONS = "The Multipolygon relation {0,number,#} with members : {1} is not closed at some locations : {2}"; + private static final List FALLBACK_INSTRUCTIONS = Collections + .singletonList(OPEN_BOUNDARY_INSTRUCTIONS); + private static final RelationOrAreaToMultiPolygonConverter RELATION_OR_AREA_TO_MULTI_POLYGON_CONVERTER = new RelationOrAreaToMultiPolygonConverter(); + + /** + * @param configuration + * the JSON configuration for this check + */ + public OpenBoundaryCheck(final Configuration configuration) + { + super(configuration); + } + + /** + * 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) + { + final Map tags = object.getTags(); + return object instanceof Relation && !isFlagged(object.getOsmIdentifier()) + && Validators.isOfType(object, RelationTypeTag.class, RelationTypeTag.BOUNDARY) + && !SyntheticRelationMemberAdded.hasAddedRelationMember(object) + && !SyntheticGeometrySlicedTag.isGeometrySliced(object) + && this.hasAdminLevelTag(tags) && !this.hasIsInCountry(tags); + } + + /** + * 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) + { + markAsFlagged(object.getOsmIdentifier()); + final Relation relation = (Relation) object; + + if (this.hasAllMembersLoaded(relation)) + { + try + { + RELATION_OR_AREA_TO_MULTI_POLYGON_CONVERTER.convert(relation); + return Optional.empty(); + } + catch (final MultiplePolyLineToPolygonsConverter.OpenPolygonException exception) + { + final List openLocations = exception.getOpenLocations(); + final Set latlonSet = new HashSet<>(); + + final Set memberIds = relation.members().stream() + .map(member -> member.getEntity().getOsmIdentifier()) + .collect(Collectors.toSet()); + + openLocations.stream().map(this::getLatLon).forEach(latlonSet::add); + + if (!openLocations.isEmpty()) + { + return Optional.of(this.createFlag(object, this.getLocalizedInstruction(0, + relation.getOsmIdentifier(), memberIds, latlonSet))); + + } + } + catch (final Exception exception) + { + return Optional.empty(); + } + } + + return Optional.empty(); + } + + @Override + protected List getFallbackInstructions() + { + return FALLBACK_INSTRUCTIONS; + } + + /** + * get lat lon coordinates from open location geometry. + * + * @param openLocation + * whole in boundary location + * @return 'lat,lon' string + */ + private String getLatLon(final Location openLocation) + { + final String lat = openLocation.getLatitude().toString(); + final String lon = openLocation.getLongitude().toString(); + return lat + ", " + lon; + } + + /** + * Check if the tags contain admin_level OSM tag + * + * @param tags + * osm tags + * @return boolean if tags contain admin_level tag + */ + private boolean hasAdminLevelTag(final Map tags) + { + return tags.containsKey(AdministrativeLevelTag.KEY); + } + + /** + * Checks to make sure relation is fully loaded by checking if the all known members count is + * the same as the amount of members the relation in question has + * + * @param relation + * relational boundary in question + * @return true if relation is fully loaded from shards. + */ + private boolean hasAllMembersLoaded(final Relation relation) + { + return relation.members().size() == relation.allKnownOsmMembers().size(); + } + + /** + * @param tags + * relation tags + * @return boolean for if the relation tags contains is_in:country_code + */ + private boolean hasIsInCountry(final Map tags) + { + return tags.containsKey("is_in:country_code"); + } +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTest.java b/src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTest.java new file mode 100644 index 000000000..1eeb8719f --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTest.java @@ -0,0 +1,39 @@ +package org.openstreetmap.atlas.checks.validation.relations; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver; +import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier; + +/** + * Tests for {@link OpenBoundaryCheck} + * + * @author v-garei + */ +public class OpenBoundaryCheckTest +{ + + @Rule + public OpenBoundaryCheckTestRule setup = new OpenBoundaryCheckTestRule(); + + @Rule + public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier(); + + private final OpenBoundaryCheck check = new OpenBoundaryCheck(ConfigurationResolver + .inlineConfiguration("{\"OpenBoundaryCheck\": {\"minHighwayType\": \"tertiary\"}}")); + + @Test + public void polygonClosedFalsePositive() + { + this.verifier.actual(this.setup.polygonClosedFalsePositive(), this.check); + this.verifier.verifyEmpty(); + } + + @Test + public void polygonNotClosedTruePositive() + { + this.verifier.actual(this.setup.polygonNotClosedTruePositive(), this.check); + this.verifier.verifyExpectedSize(1); + } + +} diff --git a/src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTestRule.java b/src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTestRule.java new file mode 100644 index 000000000..d1e000ced --- /dev/null +++ b/src/test/java/org/openstreetmap/atlas/checks/validation/relations/OpenBoundaryCheckTestRule.java @@ -0,0 +1,87 @@ +package org.openstreetmap.atlas.checks.validation.relations; + +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; + +/** + * Tests for {@link OpenBoundaryCheck} + * + * @author v-garei + */ +public class OpenBoundaryCheckTestRule extends CoreTestRule +{ + + private static final String POLYGON_NODE_1 = "47.6367419, -122.2342792"; + private static final String POLYGON_NODE_2 = "47.6370622, -122.2360741"; + private static final String POLYGON_NODE_3 = "47.6372561, -122.2370973"; + + private static final String POLYGON_2_NODE_1 = "47.6364969, -122.2379916"; + private static final String POLYGON_2_NODE_2 = "47.6365008, -122.2371735"; + private static final String POLYGON_2_NODE_3 = "47.6357896, -122.2371660"; + private static final String POLYGON_2_NODE_4 = "47.6357857, -122.2379841"; + + private static final String RELATION_ID_OPEN_MULTIPOLYGON = "123"; + private static final String RELATION_ID_OPEN_MULTIPOLYGON2 = "124"; + + @TestAtlas( + // nodes + nodes = { @Node(coordinates = @Loc(value = POLYGON_2_NODE_1)), + @Node(coordinates = @Loc(value = POLYGON_2_NODE_2)), + @Node(coordinates = @Loc(value = POLYGON_2_NODE_3)), + @Node(coordinates = @Loc(value = POLYGON_2_NODE_4)) }, + // lines + edges = { + @Edge(id = "2000001", coordinates = { @TestAtlas.Loc(value = POLYGON_2_NODE_1), + @Loc(value = POLYGON_2_NODE_2) }, tags = { "natural=water" }), + @Edge(id = "2000002", coordinates = { @TestAtlas.Loc(value = POLYGON_2_NODE_2), + @Loc(value = POLYGON_2_NODE_3) }, tags = { "natural=water" }), + @Edge(id = "2000003", coordinates = { @TestAtlas.Loc(value = POLYGON_2_NODE_3), + @Loc(value = POLYGON_2_NODE_4) }, tags = { "natural=water" }), + @Edge(id = "2000004", coordinates = { @TestAtlas.Loc(value = POLYGON_2_NODE_4), + @Loc(value = POLYGON_2_NODE_1) }, tags = { "natural=water" }) }, + // relations + relations = { @Relation(id = RELATION_ID_OPEN_MULTIPOLYGON2, members = { + @Member(id = "2000001", type = "edge", role = "outer"), + @Member(id = "2000002", type = "edge", role = "outer"), + @Member(id = "2000003", type = "edge", role = "outer"), + @Member(id = "2000004", type = "edge", role = "outer") }, tags = { + "type=boundary", "admin_level=3" }) }) + + private Atlas polygonClosedFalsePositive; + + @TestAtlas( + // nodes + nodes = { @Node(coordinates = @Loc(value = POLYGON_NODE_1)), + @Node(coordinates = @Loc(value = POLYGON_NODE_2)), + @Node(coordinates = @Loc(value = POLYGON_NODE_3)) }, + // lines + edges = { + @Edge(id = "1000001", coordinates = { @TestAtlas.Loc(value = POLYGON_NODE_1), + @Loc(value = POLYGON_NODE_2) }), + @Edge(id = "1000002", coordinates = { @TestAtlas.Loc(value = POLYGON_NODE_2), + @Loc(value = POLYGON_NODE_3) }), }, + // relations + relations = { @Relation(id = RELATION_ID_OPEN_MULTIPOLYGON, members = { + @Member(id = "1000001", type = "edge", role = "outer"), + @Member(id = "1000002", type = "edge", role = "outer"), }, tags = { + "type=boundary", "admin_level=3" }) }) + + private Atlas polygonNotClosedTruePositive; + + public Atlas polygonClosedFalsePositive() + { + return this.polygonClosedFalsePositive; + } + + public Atlas polygonNotClosedTruePositive() + { + return this.polygonNotClosedTruePositive; + } + +}