OceanBleedingCheck update - coastlines and ferries (osmlab#285)

* OceanBleeding - flag intersections w natural=coastline * spotless * saving some work * fix intersecting/within * spotless * equalsIgnoreCase tag; flag nonexplicit intersections * unit tests * reduce cognitive complexity * nit and docs Co-authored-by: Sean Coulter <[email protected]> Co-authored-by: Daniel B <[email protected]>
sayas01 · May 13, 2020 · b7e88b2 · b7e88b2
1 parent 50748af
commit b7e88b2
Show file tree

Hide file tree

Showing 6 changed files with 307 additions and 47 deletions.
diff --git a/config/configuration.json b/config/configuration.json
@@ -975,7 +975,8 @@
   "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->*"
+      "invalid": "man_made->breakwater,pier|natural->beach,marsh,swamp|water->marsh|wetland->bog,fen,mangrove,marsh,saltern,saltmarsh,string_bog,swamp,wet_meadow|landuse->*",
+      "boundary": "natural->coastline"
     },
     "highway": {
       "minimum": "path",

diff --git a/docs/checks/oceanBleedingCheck.md b/docs/checks/oceanBleedingCheck.md
@@ -1,6 +1,6 @@
 # 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.
+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 only exception to this rule is streets that end at ocean boundaries and are tagged with amenity->ferry_terminal; such streets are not flagged. 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 describe ocean features are configurable. A valid ocean feature (that is considered for the check) must conform to "ocean.valid" and must not conform to "ocean.invalid", OR must conform to "ocean.boundary". The latter is by default natural->coastline.
 
 #### Live Examples
 
@@ -11,4 +11,4 @@ This check aims to flag streets, railways, and buildings that bleed into (inters
 
 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)
+Please see the 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/utility/IntersectionUtilities.java b/src/main/java/org/openstreetmap/atlas/checks/utility/IntersectionUtilities.java
@@ -73,27 +73,27 @@ public static double findIntersectionPercentage(final Polygon polygon,
     }
 
     /**
-     * Verifies intersections of given {@link Polygon} and {@link LineItem} are explicit
+     * Verifies intersections of given {@link PolyLine} and {@link LineItem} are explicit
      * {@link Location}s for both items
      *
-     * @param areaCrossed
-     *            {@link Polygon} being crossed
+     * @param lineCrossed
+     *            {@link PolyLine} being crossed
      * @param crossingItem
      *            {@link LineItem} crossing
-     * @return whether given {@link Polygon} and {@link LineItem}'s intersections are actual
+     * @return whether given {@link PolyLine} and {@link LineItem}'s intersections are actual
      *         {@link Location}s for both items
      */
-    public static boolean haveExplicitLocationsForIntersections(final Polygon areaCrossed,
+    public static boolean haveExplicitLocationsForIntersections(final PolyLine lineCrossed,
             final LineItem crossingItem)
     {
         // Find out intersections
         final PolyLine crossingItemAsPolyLine = crossingItem.asPolyLine();
-        final Set<Location> intersections = areaCrossed.intersections(crossingItemAsPolyLine);
+        final Set<Location> intersections = lineCrossed.intersections(crossingItemAsPolyLine);
 
         // Verify intersections are explicit locations for both geometries
         for (final Location intersection : intersections)
         {
-            if (!areaCrossed.contains(intersection)
+            if (!lineCrossed.contains(intersection)
                     || !crossingItemAsPolyLine.contains(intersection))
             {
                 return false;

diff --git a/...main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java b/...main/java/org/openstreetmap/atlas/checks/validation/intersections/OceanBleedingCheck.java
@@ -1,20 +1,26 @@
 package org.openstreetmap.atlas.checks.validation.intersections;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 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.checks.utility.IntersectionUtilities;
+import org.openstreetmap.atlas.geography.Location;
+import org.openstreetmap.atlas.geography.PolyLine;
 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.AmenityTag;
 import org.openstreetmap.atlas.tags.BridgeTag;
 import org.openstreetmap.atlas.tags.BuildingTag;
 import org.openstreetmap.atlas.tags.HighwayTag;
@@ -23,7 +29,9 @@
 
 /**
  * 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}.
+ * ocean is defined by a set of ocean tags, and can be an {@link Area} or {@link LineItem}. Differs
+ * from {@link LineCrossingWaterBodyCheck} in that that check has a different set of tags/geometries
+ * to define waterbodies.
  *
  * @author seancoulter
  */
@@ -37,15 +45,17 @@ public class OceanBleedingCheck extends BaseCheck<Long>
             + "|wetland->bog,fen,mangrove,marsh,saltern,saltmarsh,string_bog,swamp,wet_meadow"
             + "|landuse->*";
     private final TaggableFilter invalidOceanTags;
+    private static final String DEFAULT_OCEAN_BOUNDARY_TAGS = "natural->coastline";
+    private final TaggableFilter oceanBoundaryTags;
     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<String> DEFAULT_HIGHWAYS_EXCLUDE = Collections.emptyList();
     private final List<HighwayTag> 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 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<String> FALLBACK_INSTRUCTIONS = Arrays.asList(
             BLEEDING_BUILDING_INSTRUCTION, BLEEDING_LINEITEM_INSTRUCTION, OCEAN_INSTRUCTION);
     private static final long serialVersionUID = -2229281211747728380L;
@@ -74,6 +84,8 @@ public OceanBleedingCheck(final Configuration configuration)
                 .configurationValue(configuration, "highway.exclude", DEFAULT_HIGHWAYS_EXCLUDE)
                 .stream().map(element -> Enum.valueOf(HighwayTag.class, element.toUpperCase()))
                 .collect(Collectors.toList());
+        this.oceanBoundaryTags = TaggableFilter.forDefinition(this.configurationValue(configuration,
+                "ocean.boundary", DEFAULT_OCEAN_BOUNDARY_TAGS));
     }
 
     /**
@@ -86,13 +98,14 @@ public OceanBleedingCheck(final Configuration configuration)
     @Override
     public boolean validCheckForObject(final AtlasObject object)
     {
-        return this.validOceanTags.test(object) && !this.invalidOceanTags.test(object)
-                && (object instanceof Area || object instanceof LineItem);
+        return (object instanceof Area || object instanceof LineItem)
+                && (this.validOceanTags.test(object) && !this.invalidOceanTags.test(object)
+                        || this.oceanBoundaryTags.test(object));
     }
 
     /**
-     * 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.
+     * We flag railways, streets, and buildings that intersect or are within certain ocean features,
+     * so each flag is a collection of all invalid interactions for a given ocean feature.
      *
      * @param object
      *            the atlas object supplied by the Atlas-Checks framework for evaluation
@@ -104,55 +117,152 @@ protected Optional<CheckFlag> 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
+        // offending land 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<LineItem> 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<Area> intersectingBuildings = object.getAtlas().areasIntersecting(
-                oceanBoundary,
-                area -> (oceanIsArea
-                        || ((LineItem) object).asPolyLine().intersects(area.asPolygon()))
-                        && BuildingTag.isBuilding(area));
+        // Differentiate between a coastline area (sometimes seen as islands) and a waterbody area
+        final boolean oceanFeatureIsAWaterBody = this.validOceanTags.test(object);
 
+        final ArrayList<LineItem> offendingLineItems = new ArrayList<>();
+        final ArrayList<Area> offendingBuildings = new ArrayList<>();
+
+        // Check if a land feature (building or line item) interacts with the ocean feature
+        // invalidly. The land feature is assumed to be an Area or LineItem. Interactions that
+        // should be flagged are as follows: -- the ocean feature is a waterbody Area and the land
+        // feature is within or intersects the ocean feature -- the ocean feature is a LineItem or a
+        // coastline Area (with natural=coastline) and the land feature intersects the ocean feature
+        // Interactions that should not be flagged: -- the ocean feature is a coastline area or
+        // lineitem, or a waterbody LineItem, and the land feature is within the ocean feature
+
+        if (oceanIsArea && oceanFeatureIsAWaterBody)
+        {
+            // Collect invalid line items contained within and intersecting with the waterbody ocean
+            // feature
+            final Iterable<LineItem> intersectingLinearFeatures = object.getAtlas()
+                    .lineItemsIntersecting(oceanBoundary,
+                            isInvalidlyInteractingWithOcean(oceanBoundary));
+            final Iterable<Area> intersectingBuildingFeatures = object.getAtlas()
+                    .areasIntersecting(oceanBoundary, BuildingTag::isBuilding);
+            intersectingLinearFeatures.forEach(offendingLineItems::add);
+            intersectingBuildingFeatures.forEach(offendingBuildings::add);
+        }
+        else
+        {
+            // Collect invalid buildings items intersecting the ocean feature, which is either a
+            // coastline landmass or linear waterbody
+            final Iterable<LineItem> intersectingLinearFeatures = object.getAtlas()
+                    .lineItemsIntersecting(oceanBoundary, lineItem -> (oceanIsArea
+                            && !oceanBoundary.fullyGeometricallyEncloses(lineItem.asPolyLine())
+                            || object instanceof LineItem && ((LineItem) object).asPolyLine()
+                                    .intersects(lineItem.asPolyLine()))
+                            && isInvalidlyInteractingWithOcean(
+                                    oceanIsArea ? oceanBoundary : ((LineItem) object).asPolyLine())
+                                            .test(lineItem));
+            final Iterable<Area> intersectingBuildingFeatures = object.getAtlas().areasIntersecting(
+                    oceanBoundary,
+                    area -> (oceanIsArea
+                            && !oceanBoundary.fullyGeometricallyEncloses(area.asPolygon())
+                            || object instanceof LineItem && ((LineItem) object).asPolyLine()
+                                    .intersects(area.asPolygon()))
+                            && BuildingTag.isBuilding(area));
+            intersectingLinearFeatures.forEach(offendingLineItems::add);
+            intersectingBuildingFeatures.forEach(offendingBuildings::add);
+        }
+        return this.generateFlag(object, offendingLineItems, offendingBuildings);
+    }
+
+    @Override
+    protected List<String> getFallbackInstructions()
+    {
+        return FALLBACK_INSTRUCTIONS;
+    }
+
+    /**
+     * Generate and return flag for this ocean feature if there were offending items
+     * 
+     * @param object
+     *            the ocean feature
+     * @param offendingLineItems
+     *            offending streets/railways
+     * @param offendingBuildings
+     *            offending buildings
+     * @return the flag for this ocean feature if flaggable items were found
+     */
+    private Optional<CheckFlag> generateFlag(final AtlasObject object,
+            final ArrayList<LineItem> offendingLineItems, final ArrayList<Area> offendingBuildings)
+    {
         // Unify all offenders in storage so the flag id is generated from a single set of flagged
         // objects
         final HashSet<AtlasObject> flaggedObjects = new HashSet<>();
         final StringBuilder instructions = new StringBuilder();
         instructions.append(this.getLocalizedInstruction(2, object.getOsmIdentifier()));
-        intersectingBuildings.forEach(building ->
+        offendingBuildings.forEach(building ->
         {
             flaggedObjects.add(building);
-            instructions.append(this.getLocalizedInstruction(0, building.getOsmIdentifier(),
-                    object.getOsmIdentifier()));
+            instructions.append(this.getLocalizedInstruction(0, building.getOsmIdentifier()));
         });
-        intersectingRoads.forEach(road ->
+        final Set<Long> seenLineItems = new HashSet<>();
+        offendingLineItems.forEach(lineItem ->
         {
-            flaggedObjects.add(road);
-            instructions.append(this.getLocalizedInstruction(1, road.getOsmIdentifier(),
-                    object.getOsmIdentifier()));
+            flaggedObjects.add(lineItem);
+            if (!seenLineItems.contains(lineItem.getOsmIdentifier()))
+            {
+                instructions.append(this.getLocalizedInstruction(1, lineItem.getOsmIdentifier()));
+                seenLineItems.add(lineItem.getOsmIdentifier());
+            }
         });
         return flaggedObjects.isEmpty() ? Optional.empty()
                 : Optional.of(this.createFlag(flaggedObjects, instructions.toString()));
     }
 
-    @Override
-    protected List<String> getFallbackInstructions()
+    /**
+     * Checks if the provided {@link LineItem} should be flagged. It should be flagged if it's not a
+     * bridge and is either an edge with the correct highway type that is not explicitly connected
+     * to a ferry terminal, or a Line that has at least one of the configurable offending tags
+     *
+     * @return true if lineItem should be flagged, false otherwise
+     */
+    private Predicate<LineItem> isInvalidlyInteractingWithOcean(final PolyLine oceanFeature)
     {
-        return FALLBACK_INSTRUCTIONS;
+        return lineItem ->
+        {
+            if (BridgeTag.isBridge(lineItem))
+            {
+                return false;
+            }
+            if (!(lineItem instanceof Edge))
+            {
+                return this.defaultOffendingLineitems.test(lineItem);
+            }
+            if (!this.validHighwayType().test((Edge) lineItem))
+            {
+                return false;
+            }
+            if (IntersectionUtilities.haveExplicitLocationsForIntersections(oceanFeature, lineItem))
+            {
+                // All intersections are explicit (or there are none -> full containment), so make
+                // sure they're marked as ferry terminals
+                final Set<Location> intersections = oceanFeature
+                        .intersections(lineItem.asPolyLine());
+
+                if (!intersections.contains(((Edge) lineItem).start().getLocation())
+                        && !intersections.contains(((Edge) lineItem).end().getLocation()))
+                {
+                    // The point of intersection was at an intermediate position along the edge, or
+                    // there were no intersections (full containment in the waterbody)
+                    return true;
+                }
+                return ((Edge) lineItem).connectedNodes().stream()
+                        .filter(node -> intersections.contains(node.getLocation()))
+                        .anyMatch(node -> !node.getTag(AmenityTag.KEY).orElse("")
+                                .equalsIgnoreCase(AmenityTag.FERRY_TERMINAL.name()));
+            }
+            return true;
+        };
     }
 
     /**
@@ -165,4 +275,5 @@ private Predicate<Edge> validHighwayType()
         return edge -> edge.highwayTag().isMoreImportantThanOrEqualTo(this.highwayMinimum)
                 && !this.highwaysExclude.contains(edge.highwayTag());
     }
+
 }