Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Highway Intersection Check #664

Merged
merged 11 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions config/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,12 @@
"description": "Checks if the object access is overly permissive"
}
},
"HighwayIntersectionCheck": {
"challenge": {
"description": "This challenge contains navigable ways that share nodes with non-navigable ways.",
"instruction": "Please fix the ways tags or modify the nodes that cause the issue."
}
},
"HighwayMissingNameAndRefTagCheck": {
"min.nonContiguous.angle": 30.0,
"min.highway.type": "tertiary",
Expand Down
3 changes: 2 additions & 1 deletion docs/available_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,5 @@ This document is a list of tables with a description and link to documentation f
| [LineCrossingWaterBodyCheck](checks/lineCrossingWaterBodyCheck.md) | The purpose of this check is to identify line items (edges or lines) and optionally buildings, that cross water bodies invalidly. |
| [MalformedPolyLineCheck](checks/malformedPolyLineCheck.md) | This check identifies lines that have only one point, or none, and the ones that are too long. |
| [SelfIntersectingPolylineCheck](checks/selfIntersectingPolylineCheck.md) | The purpose of this check is to identify all edges/lines/areas in Atlas that have self-intersecting polylines, or geometries that intersects itself in some form. |
| [WaterWayCheck](checks/waterWayChecks.md) | This check finds closed waterways (circular water motion), waterways without a place for water to go (a "sink"), crossing waterways, and waterways that go uphill (requires elevation data). |
| [WaterWayCheck](checks/waterWayCheck.md) | This check finds closed waterways (circular water motion), waterways without a place for water to go (a "sink"), crossing waterways, and waterways that go uphill (requires elevation data). |
| [HighwayIntersectionCheck](checks/highwayIntersectionCheck.md) | The purpose of this check is to identify highways intersecting power or waterway objects invalidly. |
19 changes: 19 additions & 0 deletions docs/checks/highwayIntersectionCheck.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Highway Intersection Check

#### Description

The purpose of this check is to flag highways intersecting power or waterway objects.

*dam* and *weir* waterway objects are not flagged, these type of waterways can intersect with highways.

For waterway objects the intersection with a highway having *leisure=slipway* or *ford=yes* tag is allowed, and they are not flagged.

#### Configuration

There are no configurables for this check.

#### Code Review

#### More Information

Please see the source code for HighwayIntersectionCheck here: [HighwayIntersectionCheck](../../src/main/java/org/openstreetmap/atlas/checks/validation/intersections/HighwayIntersectionCheck.java)
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
package org.openstreetmap.atlas.checks.validation.intersections;

import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.openstreetmap.atlas.checks.atlas.predicates.TagPredicates;
import org.openstreetmap.atlas.checks.atlas.predicates.TypePredicates;
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.Edge;
import org.openstreetmap.atlas.geography.atlas.items.Node;
import org.openstreetmap.atlas.geography.atlas.walker.OsmWayWalker;
import org.openstreetmap.atlas.tags.FordTag;
import org.openstreetmap.atlas.tags.HighwayTag;
import org.openstreetmap.atlas.tags.LeisureTag;
import org.openstreetmap.atlas.tags.Taggable;
import org.openstreetmap.atlas.tags.WaterwayTag;
import org.openstreetmap.atlas.tags.annotations.validation.Validators;
import org.openstreetmap.atlas.utilities.configuration.Configuration;

/**
* Flags waterway and power line edge items that are crossed by navigable edges (having way specific
* highway tag). If the way is a waterway and the crossing way has {@code ford=yes} or
* {@code leisure=slipway} tags, then the crossing is accepted. {@code dam} and {@code weir}
* waterways are not checked, those type of ways can cross other highways.
*
* @author pako.todea
* @author brianjor
*/
public class HighwayIntersectionCheck extends BaseCheck<Long>
{
private static final long serialVersionUID = -2100623356724302728L;
private static final String INSTRUCTION_FORMAT = "The way with id {0,number,#} has invalid intersections with {1}. A navigable way should not share nodes with non-navigable features. Either the way is inproperly tagged or is a combination of what should be two separate ways (highway and the other non-navigable feature).";
brianjor marked this conversation as resolved.
Show resolved Hide resolved
private static final List<String> FALLBACK_INSTRUCTIONS = Collections
.singletonList(INSTRUCTION_FORMAT);

public HighwayIntersectionCheck(final Configuration configuration)
{
super(configuration);
}

@Override
public boolean validCheckForObject(final AtlasObject object)
{
return !this.isFlagged(object) && TypePredicates.IS_EDGE.test(object)
&& ((Edge) object).isMainEdge() && HighwayTag.isCarNavigableHighway(object);
}

@Override
protected Optional<CheckFlag> flag(final AtlasObject object)
{
final Edge edge = (Edge) object;
final OsmWayWalker wayWalker = new OsmWayWalker(edge);
final Set<Edge> wayEdges = wayWalker.collectEdges();
final Stream<Edge> edgesConnectedToWay = wayEdges.stream().map(Edge::connectedEdges)
.flatMap(Set::stream)
.filter(connectedEdge -> !this.hasSameOSMId(connectedEdge, edge));

final Set<Edge> invalidconnectingEdges = edgesConnectedToWay
.filter(connectedEdge -> TagPredicates.IS_POWER_LINE.test(connectedEdge)
|| !FordTag.isYes(edge))
.filter(connectedEdge -> TagPredicates.IS_POWER_LINE.test(connectedEdge)
|| this.isWaterwayToCheck(connectedEdge))
.filter(connectedEdge -> TagPredicates.IS_POWER_LINE.test(connectedEdge)
|| !this.isSlipway(edge))
brianjor marked this conversation as resolved.
Show resolved Hide resolved
.collect(Collectors.toSet());

if (!invalidconnectingEdges.isEmpty())
{
return this.createFlag(edge, wayEdges, invalidconnectingEdges);
}

return Optional.empty();
}

@Override
protected List<String> getFallbackInstructions()
{
return FALLBACK_INSTRUCTIONS;
}

/**
* Function that creates highway intersection check flag.
*
* @param edge
* Atlas object.
* @param crossingEdges
* collected edges for a given atlas object.
* @return newly created highway intersection check flag including crossing edges locations.
*/
private Optional<CheckFlag> createFlag(final Edge edge, final Set<Edge> wayEdges,
final Set<Edge> crossingEdges)
{
final List<Location> points = wayEdges.stream()
.map(wayEdge -> crossingEdges.stream()
.map(crossEdge -> this.getIntersections(wayEdge, crossEdge))
.flatMap(Set::stream).map(Node::getLocation).collect(Collectors.toList()))
.flatMap(List::stream).distinct().collect(Collectors.toList());

this.markAsFlagged(edge.getOsmIdentifier());
final Set<Long> crossingIds = crossingEdges.stream().map(AtlasObject::getOsmIdentifier)
.collect(Collectors.toSet());
final String instruction = this.getLocalizedInstruction(0, edge.getOsmIdentifier(),
crossingIds);
return Optional.of(this.createFlag(wayEdges, instruction, points));
}

/**
* This function returns the set of intersection locations for the given edges.
*
* @param firstEdge
* the first Edge
* @param secondEdge
* the second edge
* @return set of intersection locations.
*/
private Set<Node> getIntersections(final Edge firstEdge, final Edge secondEdge)
{
final Set<Node> locations = firstEdge.connectedNodes();
locations.retainAll(secondEdge.connectedNodes());
return locations;
}

/**
* Checks if two AtlasObjects have the same OSM Identifier.
*
* @param first
* the first AtlasObject to check
* @param second
* the second AtlasObject to check
* @return true if both have the same OSM Identifier, false otherwise.
*/
private boolean hasSameOSMId(final AtlasObject first, final AtlasObject second)
{
return first.getOsmIdentifier() == second.getOsmIdentifier();
}

/**
* Check if the AtlasObject is flagged, uses OSM Identifier.
*
* @param object
* the AtlasObject to check if flagged
* @return true if flagged, false otherwise.
*/
private boolean isFlagged(final AtlasObject object)
{
return this.isFlagged(object.getOsmIdentifier());
}

/**
* Check if AtlasObject contains "leisure=slipway" tag.
*
* @param object
* the object to check the tags for
* @return true if object has tag "leisure=slipway", false otherwise.
*/
private boolean isSlipway(final AtlasObject object)
{
return Validators.isOfType(object, LeisureTag.class, LeisureTag.SLIPWAY);
}

private boolean isWaterWay(final Taggable taggable)
{
return Validators.hasValuesFor(taggable, WaterwayTag.class);
}

/**
* Checks whether the given {@link AtlasObject} is a waterway to check.
*
* @param object
* the {@link AtlasObject} to check
* @return true if the given {@link AtlasObject} should be ckecked
*/
private boolean isWaterwayToCheck(final AtlasObject object)
{
boolean validForCheck = false;
if (this.isWaterWay(object))
{
final Optional<WaterwayTag> waterwayTagValue = WaterwayTag.get(object);
if (waterwayTagValue.isPresent())
{
final String waterwayTag = waterwayTagValue.get().name();
validForCheck = !(HighwayTag.highwayTag(object).isPresent()
&& (WaterwayTag.DAM.toString().equalsIgnoreCase(waterwayTag)
|| WaterwayTag.WEIR.toString().equalsIgnoreCase(waterwayTag)));
}
}
return validForCheck;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package org.openstreetmap.atlas.checks.validation.intersections;

import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.openstreetmap.atlas.checks.configuration.ConfigurationResolver;
import org.openstreetmap.atlas.checks.validation.verifier.ConsumerBasedExpectedCheckVerifier;

/**
* @author pako.todea
* @author brianjor
*/
public class HighwayIntersectionCheckTest
{

@Rule
public HighwayIntersectionTestCaseRule setup = new HighwayIntersectionTestCaseRule();

@Rule
public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier();

private final HighwayIntersectionCheck check = new HighwayIntersectionCheck(
ConfigurationResolver.emptyConfiguration());

@Test
public void testInvalidCrossingHighwayLeisureEdges()
{
this.verifier.actual(this.setup.invalidCrossingWaterwayLeisureEdges(), this.check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size()));
}

@Test
public void testInvalidCrossingHighwayPowerLineEdges()
{
this.verifier.actual(this.setup.invalidCrossingHighwayPowerLineEdges(), this.check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size()));
}

@Test
public void testInvalidCrossingHighwayWaterEdges()
{
this.verifier.actual(this.setup.invalidCrossingHighwayWaterEdges(), this.check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size()));
}

@Test
public void testInvalidCrossingPowerLineFordYesEdges()
{
this.verifier.actual(this.setup.invalidCrossingPowerLineFordYesEdges(), this.check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size()));
}

@Test
public void testInvalidCrossingPowerLineLeisureSlipwayEdges()
{
this.verifier.actual(this.setup.invalidCrossingPowerLineLeisureSlipwayEdges(), this.check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size()));
}

@Test
public void testInvalidMultipleCrossingHighwayWaterEdges()
{
this.verifier.actual(this.setup.invalidMultipleCrossingHighwayWaterEdges(), this.check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(2, flags.size()));
this.verifier.verify(flag -> Assert.assertEquals(2, flag.getFlaggedObjects().size()));
}

@Test
public void testNoCrossingHighwayPowerLineEdges()
{
this.verifier.actual(this.setup.noCrossingHighwayPowerLineEdges(), this.check);
this.verifier.verifyEmpty();
}

@Test
public void testNoCrossingHighwayWaterEdges()
{
this.verifier.actual(this.setup.noCrossingHighwayWaterEdges(), this.check);
this.verifier.verifyEmpty();
}

@Test
public void testValidCrossingHighwayFordYesEdges()
{
this.verifier.actual(this.setup.validCrossingWaterwayFordYesEdges(), this.check);
this.verifier.verifyEmpty();
}

@Test
public void testValidCrossingHighwayLeisureSlipwayEdges()
{
this.verifier.actual(this.setup.validCrossingWaterwayLeisureSlipwayEdges(), this.check);
this.verifier.verifyEmpty();
}

@Test
public void testValidCrossingHighwayWaterwayDamEdges()
{
this.verifier.actual(this.setup.validCrossingHighwayWaterwayDamEdges(), this.check);
this.verifier.verifyEmpty();
}

@Test
public void testValidCrossingHighwayWaterwayWeirEdges()
{
this.verifier.actual(this.setup.validCrossingHighwayWaterwayWeirEdges(), this.check);
this.verifier.verifyEmpty();
}
}
Loading