Skip to content

Commit

Permalink
Duplicate Point Check (osmlab#37)
Browse files Browse the repository at this point in the history
* Initial logic, documentation and tests for DuplicatePoint

* Added configuration

* Add missing curly brace in config

* Updates to unit tests to use TestAtlas

* Gradle build

* Added missing comma to config

* Use pointsAt instead of pointsWithin

* Update documentation and flagging

* Update for gradle

* Only flag one point location
  • Loading branch information
savannahostrowski authored and mgcuthbert committed Apr 16, 2018
1 parent 1893f74 commit cd37d95
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 0 deletions.
8 changes: 8 additions & 0 deletions config/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@
"difficulty": "EASY"
}
},
"DuplicatePointCheck": {
"challenge": {
"description": "Tasks contain node locations where duplicate points of interest are found.",
"blurb": "Duplicate Points",
"instruction": "Open your favorite editor and remove one or more of the duplicate points of interest until only one remains.",
"difficulty": "EASY"
}
},
"DuplicateWaysCheck": {
"challenge": {
"description": "Tasks contain Ways which have been partially or completely duplicated.",
Expand Down
60 changes: 60 additions & 0 deletions docs/checks/duplicatePointCheck.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Duplicate Point Check

#### Description
The Duplicate Point check flags Points in OSM that share the same location. Whether it's a street
light, viewpoint, or tree, a Point represents something that exists at that location, therefore, we
do not want points on top of each other. Also, as per the [One feature, one OSM element principle](https://wiki.openstreetmap.org/wiki/One_feature,_one_OSM_element),
we don't want instances of duplicate Points that represent the same feature.

#### Live Example
The following examples illustrate two cases where there are two or more Points in the exact same location.
1) This Point [id:538464430](https://www.openstreetmap.org/node/538464430) has two Points in the same
location.
2) This Point [id:768590227](https://www.openstreetmap.org/node/768590227) has two Points in the same
location.


#### 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 [Points](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Point.java).
In OpenStreetMap, the concept of Points does not exist, and are referred to as [Ways](https://wiki.openstreetmap.org/wiki/Node).

Our first goal in the Duplicate Point check is to validate the incoming Atlas Object. The object:
* Must be a valid Point
* Location of the Point object must not have been seen before

```java
@Override
public boolean validCheckForObject(final AtlasObject object)
{
return object instanceof Point && !this.isFlagged(((Point) object).getLocation());
}

```

Next, we get all Points at that Point's location. If there is more than 1 Point object found to be
at that location, we mark the Points as flagged, get all the duplicate Point identifiers at
that location, and flag them. Otherwise, we do not flag the Point.

```java
@Override
protected Optional<CheckFlag> flag(final AtlasObject object)
{
final Point point = (Point) object;

final List<Point> duplicates = Iterables
.asList(object.getAtlas().pointsAt(point.getLocation()));
if (duplicates.size() > 1)
{
duplicates.forEach(duplicate -> this.markAsFlagged(point.getLocation()));
final List<Long> duplicateIdentifiers = duplicates.stream()
.map(AtlasEntity::getOsmIdentifier).collect(Collectors.toList());
return Optional.of(this.createFlag(object,
this.getLocalizedInstruction(0, duplicateIdentifiers, point.getLocation())));
}

return Optional.empty();
}
```
To learn more about the code, please look at the comments in the source code for the check.
[DuplicatePointCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/points/DuplicatePointCheck.java)
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package org.openstreetmap.atlas.checks.validation.points;

import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

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.AtlasEntity;
import org.openstreetmap.atlas.geography.atlas.items.AtlasObject;
import org.openstreetmap.atlas.geography.atlas.items.Point;
import org.openstreetmap.atlas.utilities.collections.Iterables;
import org.openstreetmap.atlas.utilities.configuration.Configuration;

/**
* This check looks for two or more {@link Point}s that are in the exact same location.
*
* @author savannahostrowski
*/
public class DuplicatePointCheck extends BaseCheck<Location>
{
private static final List<String> FALLBACK_INSTRUCTIONS = Arrays
.asList("Nodes {0} are duplicates at {1}");
private static final long serialVersionUID = 8624313405718452123L;

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

/**
* Default constructor
*
* @param configuration
* the JSON configuration for this check
*/
public DuplicatePointCheck(final Configuration configuration)
{
super(configuration);
}

@Override
public boolean validCheckForObject(final AtlasObject object)
{

return object instanceof Point && !this.isFlagged(((Point) object).getLocation());
}

@Override
protected Optional<CheckFlag> flag(final AtlasObject object)
{
final Point point = (Point) object;

final List<Point> duplicates = Iterables
.asList(object.getAtlas().pointsAt(point.getLocation()));
if (duplicates.size() > 1)
{
this.markAsFlagged(point.getLocation());
final List<Long> duplicateIdentifiers = duplicates.stream()
.map(AtlasEntity::getOsmIdentifier).collect(Collectors.toList());
return Optional.of(this.createFlag(object,
this.getLocalizedInstruction(0, duplicateIdentifiers, point.getLocation())));
}

return Optional.empty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.openstreetmap.atlas.checks.validation.points;

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;

/**
* Tests for {@link DuplicatePointCheck}
*
* @author savannahostrowski
*/
public class DuplicatePointCheckTest
{
@Rule
public DuplicatePointCheckTestRule setup = new DuplicatePointCheckTestRule();

@Rule
public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier();

@Test
public void singlePoint()
{
this.verifier.actual(this.setup.singlePointAtlas(),
new DuplicatePointCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
}

@Test
public void duplicatePoint()
{
this.verifier.actual(this.setup.duplicatePointAtlas(),
new DuplicatePointCheck(ConfigurationResolver.emptyConfiguration()));
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package org.openstreetmap.atlas.checks.validation.points;

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.Loc;
import org.openstreetmap.atlas.utilities.testing.TestAtlas.Point;

/**
* {@link DuplicatePointCheckTest} data generator
*
* @author savannahostrowski
*/
public class DuplicatePointCheckTestRule extends CoreTestRule
{
private static final String TEST_1 = "37.3314171,-122.0304871";

@TestAtlas(
// points
points = { @Point(id = "1", coordinates = @Loc(value = TEST_1)) })
private Atlas singlePointAtlas;
@TestAtlas(
// points
points = { @Point(id = "2", coordinates = @Loc(value = TEST_1)),
@Point(id = "3", coordinates = @Loc(value = TEST_1)) })
private Atlas duplicatePointAtlas;

public Atlas singlePointAtlas()
{
return this.singlePointAtlas;
}

public Atlas duplicatePointAtlas()
{
return this.duplicatePointAtlas;
}

}

0 comments on commit cd37d95

Please sign in to comment.