Invalid Access Tag Check (osmlab#46)

* Created HighwayAccessTag check. getConnectedLineItems v1.0. * HighwayAccessTagCheck v1 complete, including unit tests and config. * Delete HighwayAccessTagCheckTest.java * Fixed private gate unit test, added hasGate(), fixed config. * fiddled with config * Fixed non-highway connections and minimum highway priority. * Fixed config input handler, and updated config * Only creates one tag per osm way. Fixed Atlas 2way edge based error. Fixed )'s in validCheckForObject. * Removed access=private handling. Added do not flag key/value config. * Added java docs, changed unintuitive variabel names, refactored config names, changed congig defaults to use tag classes, refactored Math.abs() to Edge.isMasterEdgeId(). * Added area/realtion landuse=military do not flag, changed default config minimum.highway.type to residential * updated unit test to reflect configurable minimum.highway.type * Moved landuse=military check call to increase run speed. * Fixed landuse=military for relations, and added military=* check. * Updated isInMilitaryArea() java doc. * Added contians check to relation portion of isInMilitaryArea() * Fixed area contains portion of isInMilitaryArea() * Removed connected highways. * Replaced do-not-flag with tags.filter * Refactored to be InvalidAccessTagCheck, moved from checks.validation.linear to checks.validation.tag, updated javadocs. * Changed isInMilitaryArea() to use Validator, removed outdated config, moved boolean checks around * Added InvalidAccessTag.md, updated comments, added MR challenge info. * Added serial version uid, updated MD headings, updated MR challange tags. * Updated InvalidAccessTagCheck.md * Updated InvalidAccessTagCheck instruction * fixed typo in InvalidAccessTagCheck.md * spotless * fixed InvalidAccessTagHCeck.md heading, updated MR instruction config. * Changed unit test coordinates to be more realistic, removed unnecessary unit test nodes. * Added reference to tags.filter in InvalidTagCheck java doc. * Changed isInMilitary() to use areasIntersecting() instead of areas(). * changed isInMilitaryArea to use relationsWithEntitiesIntersecting * Update configuration.json
sayas01 · Jun 26, 2018 · ec4e6c1 · ec4e6c1
1 parent 57194ed
commit ec4e6c1
Show file tree

Hide file tree

Showing 5 changed files with 730 additions and 0 deletions.
diff --git a/config/configuration.json b/config/configuration.json
@@ -118,6 +118,17 @@
       "tags":"building"
     }
   },
+  "InvalidAccessTagCheck":{
+    "tags.filter":"public_transport->!yes&psv->!yes&bus->!yes&emergency->!yes&motor_vehicle->!no&vehicle->!no&motorcar->!no",
+    "minimum.highway.type":"residential",
+    "challenge": {
+      "description": "Tasks containing invalid access tags",
+      "blurb": "Invalid Access Tags",
+      "instruction": "Correct the displayed invalid access=no tag.",
+      "difficulty": "NORMAL",
+      "tags":"access,highway"
+    }
+  },
   "InvalidTurnRestrictionCheck": {
     "challenge": {
       "description": "Tasks containing invalid turn restrictions",

diff --git a/docs/checks/invalidAccessTagCheck.md b/docs/checks/invalidAccessTagCheck.md
@@ -0,0 +1,96 @@
+# Invalid Access Tag Check
+
+#### Description
+
+This check flags roads with misused `access=no` tags. 
+
+Misuse is measured by a lack of supporting tags. Tags, such as `public_transport=yes`, can be paired with `access=no` to indicate what does or does not have access. If one or more of these supporting tags are present, `access=no` is valid.
+
+#### Live Example
+
+The way [id:440449063](https://www.openstreetmap.org/way/440449063) has an invalid `access=no` tag, as there are no supporting tags.  
+
+#### Code Review
+
+The first step is to filter out objects that contain tags that could be used to support an `access=no` tag.
+
+The following is a filter that is defined in the configuration to do so. 
+
+```json
+"tags.filter":"public_transport->!yes&psv->!yes&bus->!yes&emergency->!yes&motor_vehicle->!no&vehicle->!no&motorcar->!no"
+```
+
+In [Atlas](https://github.com/osmlab/atlas), OSM elements are represented as Edges, Points, Lines, 
+Nodes & Relations; in our case, we’re working with [Edges]((https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Edge.java)) and [Lines]((https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Line.java)).
+
+The next step is to filter in roads, and make sure they have the tag `access=no`.
+
+The following does so by looking for Edges and Lines with a `highway` tag containing a value greater than a minimum, and the `access=no` tag.
+In [Atlas](https://github.com/osmlab/atlas) roads can only be Edges and Lines, and the minimum value ensures that things like trails and bicycle paths are not included.
+
+```java
+@Override
+    public boolean validCheckForObject(final AtlasObject object)
+    {
+        return AccessTag.isNo(object) && ((object instanceof Edge) || (object instanceof Line))
+                && Edge.isMasterEdgeIdentifier(object.getIdentifier())
+                && !this.isFlagged(object.getOsmIdentifier()) && isMinimumHighway(object);
+    }
+```
+
+The test for a minimum `highway` value is handled by the following. It uses built in Atlas methods and `highway` value priorities.
+
+```java
+private boolean isMinimumHighway(final AtlasObject object)
+    {
+        final Optional<HighwayTag> result = HighwayTag.highwayTag(object);
+        return result.isPresent()
+                && result.get().isMoreImportantThanOrEqualTo(this.minimumHighwayType);
+    }
+```
+
+Finally, there is a test made to see if the road is inside a military zone. Roads in military zones do not require any supporting tags to be validly tagged with `access=no`.
+
+The following performs this test by checking for the road being checked inside all [Areas]((https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Area.java)) and [Relations]((https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Relation.java)) that have a `landuse=military` or `military` tag.
+
+```java
+private boolean isInMilitaryArea(final LineItem object)
+    {
+        for (final Area area : object.getAtlas()
+                .areas(area -> Validators.isOfType(area, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(area, MilitaryTag.class)))
+        {
+            final Polygon areaPolygon = area.asPolygon();
+            if (object.intersects(areaPolygon)
+                    || areaPolygon.fullyGeometricallyEncloses(object.asPolyLine()))
+            {
+                return true;
+            }
+        }
+        for (final Relation relation : object.getAtlas().relationsWithEntitiesIntersecting(
+                        object.bounds(),
+                        relation -> (Validators.isOfType(relation, LandUseTag.class, LandUseTag.MILITARY)
+                                || Validators.hasValuesFor(relation, MilitaryTag.class))
+                                && relation.isMultiPolygon()))
+        {
+            try
+            {
+                final MultiPolygon relationPolygon = new RelationOrAreaToMultiPolygonConverter()
+                        .convert(relation);
+                if (object.intersects(relationPolygon)
+                        || relationPolygon.fullyGeometricallyEncloses(object.asPolyLine()))
+                {
+                    return true;
+                }
+            }
+            catch (final MultiplePolyLineToPolygonsConverter.OpenPolygonException e)
+            {
+                continue;
+            }
+        }
+        return false;
+    }
+```
+
+To learn more about the code, please look at the comments in the source code for the check.
+[InvalidAccessTagCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java)
diff --git a/src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java b/src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java
@@ -0,0 +1,165 @@
+package org.openstreetmap.atlas.checks.validation.tag;
+
+import java.util.Arrays;
+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.MultiPolygon;
+import org.openstreetmap.atlas.geography.PolyLine;
+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.Line;
+import org.openstreetmap.atlas.geography.atlas.items.LineItem;
+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.AccessTag;
+import org.openstreetmap.atlas.tags.HighwayTag;
+import org.openstreetmap.atlas.tags.LandUseTag;
+import org.openstreetmap.atlas.tags.MilitaryTag;
+import org.openstreetmap.atlas.tags.annotations.validation.Validators;
+import org.openstreetmap.atlas.utilities.configuration.Configuration;
+
+/**
+ * This check flags {@link Edge}s and {@link Line}s that include an access tag with a value of no,
+ * and does not have any supporting tags. Supporting tags declare what is or is not included in
+ * {@code access=no}. For example a supporting tag of {@code public_transport=yes} would mean only
+ * public transport vehicles are allowed. Items with supporting tags are filtered out through the
+ * use of the {@code tags.filter} configurable.
+ *
+ * @author bbreithaupt
+ */
+
+public class InvalidAccessTagCheck extends BaseCheck
+{
+
+    private static final String MINIMUM_HIGHWAY_TYPE_DEFAULT = HighwayTag.RESIDENTIAL.toString();
+    private static final List<String> FALLBACK_INSTRUCTIONS = Arrays.asList(
+            "This way {0,number,#} has an invalid access tag value, resulting from improper tag combinations. Investigate ground truth and properly correct them.");
+
+    private final HighwayTag minimumHighwayType;
+
+    private static final long serialVersionUID = 5197703822744690835L;
+
+    /**
+     * 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 InvalidAccessTagCheck(final Configuration configuration)
+    {
+        super(configuration);
+
+        final String highwayType = (String) this.configurationValue(configuration,
+                "minimum.highway.type", MINIMUM_HIGHWAY_TYPE_DEFAULT);
+        this.minimumHighwayType = Enum.valueOf(HighwayTag.class, highwayType.toUpperCase());
+    }
+
+    /**
+     * This function will validate if the supplied atlas object is valid for the check. Objects
+     * passed to this function have already been filtered by the tags.filter parameter in the
+     * configuration file.
+     *
+     * @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 AccessTag.isNo(object) && ((object instanceof Edge) || (object instanceof Line))
+                && Edge.isMasterEdgeIdentifier(object.getIdentifier())
+                && !this.isFlagged(object.getOsmIdentifier()) && isMinimumHighway(object);
+    }
+
+    /**
+     * 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)
+    {
+        if (!isInMilitaryArea((LineItem) object))
+        {
+            this.markAsFlagged(object.getOsmIdentifier());
+            return Optional.of(this.createFlag(object,
+                    this.getLocalizedInstruction(0, object.getOsmIdentifier())));
+        }
+        return Optional.empty();
+    }
+
+    /**
+     * Checks if {@link LineItem} is inside an {@link Area} or {@link Relation} with tag
+     * {@code landuse=MILITARY} or tag key {@code military}.
+     *
+     * @param object
+     *            {@link LineItem} to check
+     * @return {@code true} if input {@link LineItem} is in an {@link Area} or {@link Relation} with
+     *         tag {@code landuse=MILITARY} or tag key {@code military}
+     */
+    private boolean isInMilitaryArea(final LineItem object)
+    {
+        final PolyLine objectAsPolyLine = object.asPolyLine();
+        if (object.getAtlas().areasIntersecting(object.bounds(),
+                area -> (Validators.isOfType(area, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(area, MilitaryTag.class))
+                        && (object.intersects(area.asPolygon())
+                                || area.asPolygon().fullyGeometricallyEncloses(objectAsPolyLine)))
+                .iterator().hasNext())
+        {
+            return true;
+        }
+        for (final Relation relation : object.getAtlas().relationsWithEntitiesIntersecting(
+                object.bounds(),
+                relation -> (Validators.isOfType(relation, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(relation, MilitaryTag.class))
+                        && relation.isMultiPolygon()))
+        {
+            try
+            {
+                final MultiPolygon relationPolygon = new RelationOrAreaToMultiPolygonConverter()
+                        .convert(relation);
+                if (object.intersects(relationPolygon)
+                        || relationPolygon.fullyGeometricallyEncloses(object.asPolyLine()))
+                {
+                    return true;
+                }
+            }
+            catch (final MultiplePolyLineToPolygonsConverter.OpenPolygonException e)
+            {
+                continue;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Checks if an {@link AtlasObject} is of an equal or greater priority than the minimum. The
+     * minimum is supplied as a configuration parameter, the default is {@code "tertiary"}.
+     *
+     * @param object
+     *            an {@link AtlasObject}
+     * @return {@code true} if this object is >= the minimum
+     */
+    private boolean isMinimumHighway(final AtlasObject object)
+    {
+        final Optional<HighwayTag> result = HighwayTag.highwayTag(object);
+        return result.isPresent()
+                && result.get().isMoreImportantThanOrEqualTo(this.minimumHighwayType);
+    }
+
+    @Override
+    protected List<String> getFallbackInstructions()
+    {
+        return FALLBACK_INSTRUCTIONS;
+    }
+}