Missing relation type check (osmlab#455)

* MissingRelationType new Atlas check.

* added javadoc, fixed arrangement

* LeftCurly fix

config/configuration.json

@@ -612,6 +612,15 @@
       "difficulty": "MEDIUM"
     }
   },
+  "MissingRelationTypeCheck": {
+    "ignore.tags.filter":"disused:type->!&disabled:type->!",
+    "challenge": {
+      "description": "Tasks contain Relations with missing type tag.",
+      "blurb": "Relations with missing type tag.",
+      "instruction": "Open your favorite editor and add Relation type tag.",
+      "difficulty": "NORMAL"
+    }
+  },
   "NodeValenceCheck": {
     "connections.maximum": 10,
     "challenge": {

docs/available_checks.md

@@ -64,6 +64,7 @@ This document is a list of tables with a description and link to documentation f
 | :--------- | :---------------- |
 | InvalidMultiPolygonRelationCheck |  This check is designed to scan through MultiPolygon relations and flag them for invalid geometry. |
 | [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. |
+| [MissingRelationType](checks/missingRelationType.md) | The purpose of this check is to identify Relations without relation type. |
 | 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. |

docs/checks/missingRelationType.md

@@ -0,0 +1,26 @@
+# Missing Relation Type Check
+
+#### Description
+This check identifies relations without relation type. In OSM, a [Relation](https://wiki.openstreetmap.org/wiki/Elements#Relation)
+suppose to have one of proposed relation [Type](https://wiki.openstreetmap.org/wiki/Types_of_relation). 
+
+#### Live Example
+The following examples illustrate two cases where a Relation is missing relation type.
+1) This relation [id:7508794](https://www.openstreetmap.org/relation/7508794) is missing "multipolygon" relation type.
+2) This relation [id:9769731](https://www.openstreetmap.org/relation/9769731) is missing "boundary" relation type.
+
+#### Code Review
+In [Atlas](https://github.com/osmlab/atlas), OSM elements are represented as Edges, Points, Lines, 
+Nodes & Relations; in our case, we’re working with [Relations](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Relation.java).
+
+Our first goal is to validate the incoming Atlas Object. We use some preliminary filtering to target
+Relation objects. Therefore, we use:
+* Must be a Relation
+* Must not have a Relation Type tag
+* Must not be a "One Member Relation"
+* Must not have disused:type or disabled:type tags [reference](https://wiki.openstreetmap.org/wiki/Key:disused:)
+
+After the preliminary filtering, Relation will be flagged.
+
+To learn more about the code, please look at the comments in the source code for the check.
+[MissingRelationTypeCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/relations/MissingRelationTypeCheck.java)

src/main/java/org/openstreetmap/atlas/checks/validation/relations/MissingRelationTypeCheck.java

@@ -0,0 +1,62 @@
+package org.openstreetmap.atlas.checks.validation.relations;
+
+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.checks.utility.CommonMethods;
+import org.openstreetmap.atlas.geography.atlas.items.AtlasObject;
+import org.openstreetmap.atlas.geography.atlas.items.Relation;
+import org.openstreetmap.atlas.tags.RelationTypeTag;
+import org.openstreetmap.atlas.tags.filters.TaggableFilter;
+import org.openstreetmap.atlas.utilities.configuration.Configuration;
+
+/**
+ * This check flags {@link Relation}s with missing Type tag. Refer to:
+ * https://wiki.openstreetmap.org/wiki/Types_of_relation
+ *
+ * @author Vladimir Lemberg
+ */
+public class MissingRelationTypeCheck extends BaseCheck<Object>
+{
+    public static final String MISSING_TYPE_INSTRUCTIONS = "This relation, {0,number,#}, is missing type tag";
+    private static final List<String> FALLBACK_INSTRUCTIONS = Collections
+            .singletonList(MISSING_TYPE_INSTRUCTIONS);
+    private static final String TAG_FILTER_IGNORE_DEFAULT = "disused:type->!&disabled:type->!";
+    private static final long serialVersionUID = 5171171744111206430L;
+    private final TaggableFilter tagFilterIgnore;
+
+    public MissingRelationTypeCheck(final Configuration configuration)
+    {
+        super(configuration);
+        this.tagFilterIgnore = this.configurationValue(configuration, "ignore.tags.filter",
+                TAG_FILTER_IGNORE_DEFAULT, TaggableFilter::forDefinition);
+    }
+
+    @Override
+    public boolean validCheckForObject(final AtlasObject object)
+    {
+        return object instanceof Relation
+                // Missing relation Type tag
+                && object.getTag(RelationTypeTag.KEY).isEmpty()
+                // Is not "One Member Relation"
+                && CommonMethods.getOSMRelationMemberSize((Relation) object) > 1
+                // Relation is not disused or disabled
+                && this.tagFilterIgnore.test(object);
+    }
+
+    @Override
+    protected Optional<CheckFlag> flag(final AtlasObject object)
+    {
+        return Optional.of(this.createFlag(object,
+                this.getLocalizedInstruction(0, object.getOsmIdentifier())));
+    }
+
+    @Override
+    protected List<String> getFallbackInstructions()
+    {
+        return FALLBACK_INSTRUCTIONS;
+    }
+}

src/test/java/org/openstreetmap/atlas/checks/utility/CommonMethodsTest.java

@@ -18,17 +18,31 @@ public class CommonMethodsTest
     public CommonMethodsTestRule setup = new CommonMethodsTestRule();
 
     @Test
-    public void testOneMemberRelationReversed()
+    public void testOneMemberRelationEdge()
+    {
+        assertEquals(1, CommonMethods
+                .getOSMRelationMemberSize(this.setup.getOneMemberRelationEdge().relation(123)));
+    }
+
+    @Test
+    public void testOneMemberRelationNode()
     {
         assertEquals(1, CommonMethods
-                .getOSMRelationMemberSize(this.setup.getOneMemberRelationReversed().relation(123)));
+                .getOSMRelationMemberSize(this.setup.getOneMemberRelationNode().relation(123)));
+    }
+
+    @Test
+    public void testOneMemberRelationReversed()
+    {
+        assertEquals(1, CommonMethods.getOSMRelationMemberSize(
+                this.setup.getOneMemberRelationReversedEdge().relation(123)));
     }
 
     @Test
     public void testOneMemberRelationSectioned()
     {
         assertEquals(1, CommonMethods.getOSMRelationMemberSize(
-                this.setup.getOneMemberRelationSectioned().relation(123)));
+                this.setup.getOneMemberRelationSectionedEdge().relation(123)));
     }
 
     @Test

src/test/java/org/openstreetmap/atlas/checks/utility/CommonMethodsTestRule.java

@@ -41,6 +41,26 @@ public class CommonMethodsTestRule extends CoreTestRule
                     @Member(id = "23000001", type = "edge", role = "") }) })
     private Atlas validRelation;
 
+    @TestAtlas(
+            // nodes
+            nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
+                    @Node(id = "2", coordinates = @Loc(value = TWO)) },
+            // edges
+            edges = { @Edge(id = "12000001", coordinates = { @Loc(value = ONE),
+                    @Loc(value = TWO) }) },
+            // relations
+            relations = { @Relation(id = "123", members = {
+                    @Member(id = "12000001", type = "edge", role = ""), }) })
+    private Atlas oneMemberRelationEdge;
+
+    @TestAtlas(
+            // nodes
+            nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)) },
+            // relations
+            relations = { @Relation(id = "123", members = {
+                    @Member(id = "1", type = "node", role = ""), }) })
+    private Atlas oneMemberRelationNode;
+
     @TestAtlas(
             // nodes
             nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@@ -54,7 +74,7 @@ public class CommonMethodsTestRule extends CoreTestRule
             relations = { @Relation(id = "123", members = {
                     @Member(id = "12000001", type = "edge", role = ""),
                     @Member(id = "-12000001", type = "edge", role = "") }) })
-    private Atlas oneMemberRelationReversed;
+    private Atlas oneMemberRelationReversedEdge;
 
     @TestAtlas(
             // nodes
@@ -73,16 +93,26 @@ public class CommonMethodsTestRule extends CoreTestRule
                     @Member(id = "12000001", type = "edge", role = ""),
                     @Member(id = "12000002", type = "edge", role = ""),
                     @Member(id = "12000003", type = "edge", role = "") }) })
-    private Atlas oneMemberRelationSectioned;
+    private Atlas oneMemberRelationSectionedEdge;
+
+    public Atlas getOneMemberRelationEdge()
+    {
+        return this.oneMemberRelationEdge;
+    }
+
+    public Atlas getOneMemberRelationNode()
+    {
+        return this.oneMemberRelationNode;
+    }
 
-    public Atlas getOneMemberRelationReversed()
+    public Atlas getOneMemberRelationReversedEdge()
     {
-        return this.oneMemberRelationReversed;
+        return this.oneMemberRelationReversedEdge;
     }
 
-    public Atlas getOneMemberRelationSectioned()
+    public Atlas getOneMemberRelationSectionedEdge()
     {
-        return this.oneMemberRelationSectioned;
+        return this.oneMemberRelationSectionedEdge;
     }
 
     public Atlas getValidRelation()

src/test/java/org/openstreetmap/atlas/checks/validation/relations/MissingRelationTypeCheckTest.java

@@ -0,0 +1,56 @@
+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;
+import org.openstreetmap.atlas.utilities.configuration.Configuration;
+
+/**
+ * Tests for {@link MissingRelationTypeCheck}
+ *
+ * @author vlemberg
+ */
+public class MissingRelationTypeCheckTest
+{
+    @Rule
+    public final MissingRelationTypeCheckTestRule setup = new MissingRelationTypeCheckTestRule();
+
+    @Rule
+    public final ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier();
+
+    private final Configuration inlineConfiguration = ConfigurationResolver.inlineConfiguration(
+            "{\"MissingRelationTypeCheck\":{\"ignore.tags.filter\":\"disused:type->!&disabled:type->!\"}}");
+
+    @Test
+    public void missingRelationType()
+    {
+        this.verifier.actual(this.setup.getMissingRelationType(),
+                new MissingRelationTypeCheck(this.inlineConfiguration));
+        this.verifier.verifyExpectedSize(1);
+    }
+
+    @Test
+    public void missingRelationTypeDisabled()
+    {
+        this.verifier.actual(this.setup.getMissingRelationTypeDisabled(),
+                new MissingRelationTypeCheck(this.inlineConfiguration));
+        this.verifier.verifyEmpty();
+    }
+
+    @Test
+    public void missingRelationTypeDisused()
+    {
+        this.verifier.actual(this.setup.getMissingRelationTypeDisused(),
+                new MissingRelationTypeCheck(this.inlineConfiguration));
+        this.verifier.verifyEmpty();
+    }
+
+    @Test
+    public void validRelation()
+    {
+        this.verifier.actual(this.setup.getValidRelation(),
+                new MissingRelationTypeCheck(this.inlineConfiguration));
+        this.verifier.verifyEmpty();
+    }
+}

src/test/java/org/openstreetmap/atlas/checks/validation/relations/MissingRelationTypeCheckTestRule.java

@@ -0,0 +1,99 @@
+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;
+
+/**
+ * {@link MissingRelationTypeCheckTest} data generator
+ *
+ * @author vlemberg
+ */
+public class MissingRelationTypeCheckTestRule extends CoreTestRule
+{
+    private static final String ONE = "18.4360044, -71.7194204";
+    private static final String TWO = "18.4360737, -71.6970306";
+    private static final String THREE = "18.4273807, -71.7052283";
+    @TestAtlas(nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
+            @Node(id = "2", coordinates = @Loc(value = TWO)),
+            @Node(id = "3", coordinates = @Loc(value = THREE)) }, edges = {
+                    @Edge(id = "12", coordinates = { @Loc(value = ONE), @Loc(value = TWO) }),
+                    @Edge(id = "23", coordinates = { @Loc(value = TWO), @Loc(value = THREE) }),
+                    @Edge(id = "31", coordinates = { @Loc(value = THREE),
+                            @Loc(value = ONE) }) }, relations = {
+                                    @Relation(id = "123", members = {
+                                            @Member(id = "12", type = "edge", role = "any"),
+                                            @Member(id = "2", type = "node", role = "any"),
+                                            @Member(id = "23", type = "edge", role = "any") }, tags = {
+                                                    "type=any" }) })
+    private Atlas validRelation;
+
+    @TestAtlas(nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
+            @Node(id = "2", coordinates = @Loc(value = TWO)),
+            @Node(id = "3", coordinates = @Loc(value = THREE)) }, edges = {
+                    @Edge(id = "12", coordinates = { @Loc(value = ONE), @Loc(value = TWO) }),
+                    @Edge(id = "23", coordinates = { @Loc(value = TWO), @Loc(value = THREE) }),
+                    @Edge(id = "31", coordinates = { @Loc(value = THREE),
+                            @Loc(value = ONE) }) }, relations = {
+                                    @Relation(id = "123", members = {
+                                            @Member(id = "12", type = "edge", role = "any"),
+                                            @Member(id = "2", type = "node", role = "any"),
+                                            @Member(id = "23", type = "edge", role = "any") }) })
+    private Atlas missingRelationType;
+
+    @TestAtlas(nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
+            @Node(id = "2", coordinates = @Loc(value = TWO)),
+            @Node(id = "3", coordinates = @Loc(value = THREE)) }, edges = {
+                    @Edge(id = "12", coordinates = { @Loc(value = ONE), @Loc(value = TWO) }),
+                    @TestAtlas.Edge(id = "23", coordinates = { @Loc(value = TWO),
+                            @Loc(value = THREE) }),
+                    @TestAtlas.Edge(id = "31", coordinates = { @Loc(value = THREE),
+                            @Loc(value = ONE) }) }, relations = {
+                                    @Relation(id = "123", members = {
+                                            @Member(id = "12", type = "edge", role = "any"),
+                                            @Member(id = "2", type = "node", role = "any"),
+                                            @Member(id = "23", type = "edge", role = "any") }, tags = {
+                                                    "disused:type=any" }) })
+    private Atlas missingRelationTypeDisused;
+
+    @TestAtlas(nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
+            @Node(id = "2", coordinates = @Loc(value = TWO)),
+            @Node(id = "3", coordinates = @Loc(value = THREE)) }, edges = {
+                    @TestAtlas.Edge(id = "12", coordinates = { @Loc(value = ONE),
+                            @Loc(value = TWO) }),
+                    @TestAtlas.Edge(id = "23", coordinates = { @Loc(value = TWO),
+                            @Loc(value = THREE) }),
+                    @TestAtlas.Edge(id = "31", coordinates = { @Loc(value = THREE),
+                            @Loc(value = ONE) }) }, relations = {
+                                    @Relation(id = "123", members = {
+                                            @Member(id = "12", type = "edge", role = "any"),
+                                            @Member(id = "2", type = "node", role = "any"),
+                                            @Member(id = "23", type = "edge", role = "any") }, tags = {
+                                                    "disabled:type=any" }) })
+    private Atlas missingRelationTypeDisabled;
+
+    public Atlas getMissingRelationType()
+    {
+        return this.missingRelationType;
+    }
+
+    public Atlas getMissingRelationTypeDisabled()
+    {
+        return this.missingRelationTypeDisabled;
+    }
+
+    public Atlas getMissingRelationTypeDisused()
+    {
+        return this.missingRelationTypeDisused;
+    }
+
+    public Atlas getValidRelation()
+    {
+        return this.validRelation;
+    }
+}