Open boundary check (osmlab#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

@@ -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",

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 |

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)

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<Long>
+{
+    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<String> 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<String, String> 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<CheckFlag> 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<Location> openLocations = exception.getOpenLocations();
+                final Set<String> latlonSet = new HashSet<>();
+
+                final Set<Long> 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<String> 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<String, String> 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<String, String> tags)
+    {
+        return tags.containsKey("is_in:country_code");
+    }
+}

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

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