Skip to content

Commit

Permalink
Add InvalidPiersCheck and Update Atlas version (osmlab#209)
Browse files Browse the repository at this point in the history
* Initial commit

* Add config

* Refactor to include new requirements(overlapping + same level/layer)

* Fix checkstyle errors

* SpotlessApply

* Add polygonal building/ferry as valid case

* Add sorting of edges

* Add unit tests and md doc

* Add java docs

* Update doc and spotlessApply

* Fix typo

* Fix sonar bug

* Update instructions
  • Loading branch information
sayas01 authored and danielduhh committed Oct 30, 2019
1 parent 01a8766 commit 7245262
Show file tree
Hide file tree
Showing 11 changed files with 678 additions and 1 deletion.
13 changes: 13 additions & 0 deletions config/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -484,5 +484,18 @@
"difficulty": "MEDIUM",
"defaultPriority": "MEDIUM"
}
},
"InvalidPiersCheck": {
"highway.type.minimum": {
"overlapping": "toll_gantry",
"pier": "toll_gantry"
},
"challenge": {
"description": "Piers should have polygon geometry and tags man_made=pier and area=yes",
"blurb": "Verify invalid piers.",
"instruction": "Open your favorite editor and check the instruction for the task to either update the geometry of the way to be a polygon or add area=yes tag or both.",
"difficulty": "MEDIUM",
"defaultPriority": "MEDIUM"
}
}
}
2 changes: 1 addition & 1 deletion dependencies.gradle
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
project.ext.versions = [
checkstyle: '8.18',
jacoco: '0.8.3',
atlas: '5.8.0',
atlas: '5.8.3',
commons:'2.6',
atlas_generator: '4.3.5',
atlas_checkstyle: '5.6.9',
Expand Down
1 change: 1 addition & 0 deletions docs/available_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ This document is a list of tables with a description and link to documentation f
| [SingleSegmentMotorwayCheck](checks/singleSegmentMotorwayCheck.md) | The purpose of this check is to identify ways tagged with highway=motorway that are not connected to any ways tagged the same. |
| [SinkIslandCheck](tutorials/tutorial3-SinkIslandCheck.md) | The purpose of this check is to identify whether a network of car-navigable Edges can be exited. |
| [SnakeRoadCheck](checks/snakeRoadCheck.md) | The purpose of the SnakeRoad check is to identify roads that should be split into two or more roads. |
| [InvalidPiersCheck](checks/invalidPiersCheck.md) | The purpose of this check is to identify piers(OSM Ways with man_made=pier tag) that are ingested in Atlas as edges with linear or polygonal geometry without an area=yes tag |

## Nodes
| Check Name | Check Description |
Expand Down
34 changes: 34 additions & 0 deletions docs/checks/invalidPiersCheck.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# InvalidPiersCheck

#### Description

The purpose of this check is to identify piers (OSM ways with man_made=Pier tag) that: (1) have a
linear geometry but _DO NOT_ have an area=Yes Tag, or (2) have a polygonal geometry but _DO NOT_
have an area=Yes Tag. A pier must: (A) have a highway Tag or (B) have an overlapping highway at the
same level and same layer as the pier or (C) be connected to a ferry route at the same layer and
same level as the pier or (D) be connected to or overlapping a building/amenity=Ferry_Terminal at
the same layer and same level as the pier. In addition, if a pier is polygonal, then it will also
be considered a valid check candidate if it has building and/or amenity=Ferry_Terminal Tags.
If a pier does not meet one or more of these criteria, this check will
not flag the pier.

#### Live Examples

1. Edge [id:691592715](https://www.openstreetmap.org/way/691592715) is a linear pier with a highway
tag and does not have an area=yes tag
2. Edge [id:106157819](https://www.openstreetmap.org/way/106157819) is a polygonal pier with an
overlapping highway and no area=yes tag.

#### Code Review

The check ensures that the Atlas object being evaluated is an [Edge](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Edge.java)
with man_made = pier tag and no area = yes tag. All the way sectioned edges of the OSM way are first collected.
Using all the collected edges, the geometry formed is verified to be either linear or polygonal. We
can flag the edge if it has a highway tag with priority greater than or equal to minimum type set in
the configurable or if the pier is polygonal with a building tag or is polygonal pier with amenity=ferry_terminal.
If the above conditions are not met, then check if the way formed from the edges overlaps a highway
or building or is connected to a ferry route or amenity=ferry_terminal. If any of these conditions
are met, we can flag the edges.

To learn more about the code, please look at the comments in the source code for the check.
[InvalidPiersCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/linear/edges/InvalidPiersCheck.java)

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.openstreetmap.atlas.checks.validation.linear.edges;

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;

/**
* Unit test for {@link InvalidPiersCheck}
*
* @author sayas01
*/
public class InvalidPiersCheckTest
{
@Rule
public InvalidPiersCheckTestRule setup = new InvalidPiersCheckTestRule();

@Rule
public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier();

@Test
public void testLinearPierConnectedToBuilding()
{
this.verifier.actual(this.setup.getLinearPierConnectedToBuildingAtlas(),
new InvalidPiersCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.verifyNotNull();
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testLinearPierConnectedToFerry()
{
this.verifier.actual(this.setup.getLinearPierConnectedToFerryAtlas(),
new InvalidPiersCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.verifyNotNull();
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testLinearPierWithHighwayTag()
{
this.verifier.actual(this.setup.getLinearPierWithHighwayTag(),
new InvalidPiersCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.verifyNotNull();
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testMinimumHighwayConfiguration()
{
this.verifier.actual(this.setup.getLinearPierWithHighwayTag(),
new InvalidPiersCheck(ConfigurationResolver.inlineConfiguration(
"{\"InvalidPiersCheck\":{\"highway.type.minimum.pier\":\"footway\"}}")));
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testPolygonalPierConnectedToBuilding()
{
this.verifier.actual(this.setup.getPolygonalPierConnectedToBuildingAtlas(),
new InvalidPiersCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.verifyNotNull();
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testPolygonalPierOverlappingHighway()
{
this.verifier.actual(this.setup.getPolygonalPierOverlappingHighwayAtlas(),
new InvalidPiersCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.verifyNotNull();
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testValidPolygonalPier()
{
this.verifier.actual(this.setup.getValidPierAtlas(),
new InvalidPiersCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.verifyEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package org.openstreetmap.atlas.checks.validation.linear.edges;

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;

/**
* {@link InvalidPiersCheckTest} test data
*
* @author sayas01
*/
public class InvalidPiersCheckTestRule extends CoreTestRule
{
private static final String TEST_1 = "1.2042824, 103.7984000";
private static final String TEST_2 = "1.2052837, 103.7992780";
private static final String TEST_3 = "1.4216577, 103.9106552";
private static final String TEST_4 = "1.4217830, 103.9106544";
private static final String TEST_5 = "1.4217858, 103.9107547";
private static final String TEST_6 = "1.4221270, 103.9107607";

@TestAtlas(
// Nodes
nodes = { @Node(coordinates = @TestAtlas.Loc(value = TEST_1)),
@Node(coordinates = @TestAtlas.Loc(value = TEST_2)) },
// Edges
edges = { @Edge(id = "1000000", coordinates = { @Loc(value = TEST_1),
@Loc(value = TEST_2) }, tags = { "highway=service", "man_made=pier" }) })
private Atlas linearPierWithHighwayTagAtlas;

@TestAtlas(
// Nodes
nodes = { @Node(coordinates = @Loc(value = TEST_3)),
@Node(coordinates = @Loc(value = TEST_4)),
@Node(coordinates = @Loc(value = TEST_5)),
@Node(coordinates = @Loc(value = TEST_6)) },
// Edges
edges = { @Edge(id = "1000000", coordinates = { @Loc(value = TEST_3),
@Loc(value = TEST_4), @Loc(value = TEST_5) }, tags = { "man_made=pier" }),
@Edge(id = "2000000", coordinates = { @Loc(value = TEST_5),
@Loc(value = TEST_6) }, tags = { "route=ferry" }), })
private Atlas linearPierConnectedToFerryAtlas;

@TestAtlas(loadFromTextResource = "InvalidPiersCheck/polygonalPierOverlappingHighwayAtlas.txt")
private Atlas polygonalPierOverlappingHighwayAtlas;

@TestAtlas(loadFromTextResource = "InvalidPiersCheck/linearPierConnectedToBuildingAtlas.txt")
private Atlas linearPierConnectedToBuildingAtlas;

@TestAtlas(loadFromTextResource = "InvalidPiersCheck/polygonalPierConnectedToBuildingAtlas.txt")
private Atlas polygonalPierConnectedToBuildingAtlas;

@TestAtlas(loadFromTextResource = "InvalidPiersCheck/validPolygonalPier.txt")
private Atlas validPierAtlas;

public Atlas getLinearPierConnectedToBuildingAtlas()
{
return this.linearPierConnectedToBuildingAtlas;
}

public Atlas getLinearPierConnectedToFerryAtlas()
{
return this.linearPierConnectedToFerryAtlas;
}

public Atlas getLinearPierWithHighwayTag()
{
return this.linearPierWithHighwayTagAtlas;
}

public Atlas getPolygonalPierConnectedToBuildingAtlas()
{
return this.polygonalPierConnectedToBuildingAtlas;
}

public Atlas getPolygonalPierOverlappingHighwayAtlas()
{
return this.polygonalPierOverlappingHighwayAtlas;
}

public Atlas getValidPierAtlas()
{
return this.validPierAtlas;
}
}
Loading

0 comments on commit 7245262

Please sign in to comment.