Skip to content

Commit

Permalink
One Member Relation Check (#40)
Browse files Browse the repository at this point in the history
* Initial logic and unit tests for OMR

* Add configuration, documentation

* Add information to documentation

* Added live examples of relations with one member to documentation

* Update documentation; simplified code

* Added functionality for Multipolygon

* Update to comments

* Added multipolygon unit tests

* Update to add flagging to all relation entities + gradle formatting

* Updates to documentation, and use Method Reference

* Fix some small nits

* Refactored to remove redundant work

* Remove flagging

* Handle multi-polygon relation
  • Loading branch information
savannahostrowski authored and mgcuthbert committed May 2, 2018
1 parent 526220b commit a8247f2
Show file tree
Hide file tree
Showing 5 changed files with 335 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 @@ -116,6 +116,14 @@
"tags":"highway"
}
},
"OneMemberRelationCheck": {
"challenge": {
"description": "Tasks containing relations with only one member.",
"blurb": "One Member Relations",
"instruction": "Correct the relation to add necessary members",
"difficulty": "HARD"
}
},
"RoundaboutClosedLoopCheck": {
"challenge": {
"description": "A roundabout should be formed by one-way edges with no dead-end nodes.",
Expand Down
64 changes: 64 additions & 0 deletions docs/checks/oneMemberRelationCheck.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# One Member Relation Check

#### Description

This check identifies relations that only contain one member. In OSM, a [Relation](https://wiki.openstreetmap.org/wiki/Elements#Relation)
is a multi-purpose data structure that documents a relationship between two or more data elements
(nodes, ways, and/or other relations).

#### Live Example
The following examples illustrate two cases where a Relation contains only one member.
1) This relation [id:4646212](https://www.openstreetmap.org/relation/4646212) has only one member.
2) This relation [id:7691824](https://www.openstreetmap.org/relation/7691824) has only one member.

#### 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 [Relations](https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/geography/atlas/items/Relation.java).

Our first goal is to validate the incoming Atlas Object. We use some preliminary filtering to target
Relation objects. Therefore, we use:
* Must be a Relation
* Must not have been flagged before

```java
@Override
public boolean validCheckForObject(final AtlasObject object)
{
return object instanceof Relation;
}
```

After the preliminary filtering of features, we get the members of that relation. If the number of
members in the relation is 1, then we flag the relation as problematic. One thing to note is that
multi-polygon relations with one member are flagged with a more specific instruction than the
regular one member relation.

```java
@Override
protected Optional<CheckFlag> flag(final AtlasObject object)
{
final Relation relation = (Relation) object;
final RelationMemberList members = relation.members();

// If the number of members in the relation is 1
if (members.size() == 1)
{
// If the relation is a multi-polygon,
if (relation.isMultiPolygon()) {
return Optional.of(createFlag(
relation.members().stream().map(RelationMember::getEntity)
.collect(Collectors.toSet()),
this.getLocalizedInstruction(1, relation.getOsmIdentifier())));
}
return Optional.of(createFlag(
relation.members().stream().map(RelationMember::getEntity)
.collect(Collectors.toSet()),
this.getLocalizedInstruction(0, relation.getOsmIdentifier())));
}
return Optional.empty();
}
```

To learn more about the code, please look at the comments in the source code for the check.
[OneMemberRelationCheck.java](../../src/main/java/org/openstreetmap/atlas/checks/validation/relations/OneMemberRelationCheck.java)
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package org.openstreetmap.atlas.checks.validation.relations;

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.atlas.items.AtlasObject;
import org.openstreetmap.atlas.geography.atlas.items.Relation;
import org.openstreetmap.atlas.geography.atlas.items.RelationMember;
import org.openstreetmap.atlas.geography.atlas.items.RelationMemberList;
import org.openstreetmap.atlas.utilities.configuration.Configuration;

/**
* This check flags {@link Relation}s which contain only one member.
*
* @author savannahostrowski
*/
public class OneMemberRelationCheck extends BaseCheck
{
public static final String OMR_INSTRUCTIONS = "This relation, {0,number,#}, contains only "
+ "one member.";

public static final String MULTIPOLYGON_OMR_INSTRUCTIONS = "This relation, {0,number,#}, contains only "
+ "one member. Multi-polygon relations need multiple polygons.";

private static final List<String> FALLBACK_INSTRUCTIONS = Arrays.asList(OMR_INSTRUCTIONS,
MULTIPOLYGON_OMR_INSTRUCTIONS);

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

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

@Override
public boolean validCheckForObject(final AtlasObject object)
{
return object instanceof Relation;
}

@Override
protected Optional<CheckFlag> flag(final AtlasObject object)
{
final Relation relation = (Relation) object;
final RelationMemberList members = relation.members();

// If the number of members in the relation is 1
if (members.size() == 1)
{
// If the relation is a multi-polygon,
if (relation.isMultiPolygon())
{
return Optional.of(createFlag(
relation.members().stream().map(RelationMember::getEntity)
.collect(Collectors.toSet()),
this.getLocalizedInstruction(1, relation.getOsmIdentifier())));
}

return Optional.of(createFlag(
relation.members().stream().map(RelationMember::getEntity)
.collect(Collectors.toSet()),
this.getLocalizedInstruction(0, relation.getOsmIdentifier())));
}
return Optional.empty();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package org.openstreetmap.atlas.checks.validation.relations;

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 OneMemberRelationCheck}
*
* @author savannahostrowski
*/
public class OneMemberRelationCheckTest
{
private static final OneMemberRelationCheck check = new OneMemberRelationCheck(
ConfigurationResolver.emptyConfiguration());

@Rule
public OneMemberRelationCheckTestRule setup = new OneMemberRelationCheckTestRule();

@Rule
public ConsumerBasedExpectedCheckVerifier verifier = new ConsumerBasedExpectedCheckVerifier();

@Test
public void testValidRelation()
{
this.verifier.actual(this.setup.getValidRelation(), check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
}

@Test
public void testOneMemberRelation()
{
this.verifier.actual(this.setup.getOneMemberRelation(), check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testOneMemberRelationMultipolygonInner()
{
this.verifier.actual(this.setup.getOneMemberRelationMultipolygonInner(), check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testOneMemberRelationMultipolygonOuter()
{
this.verifier.actual(this.setup.getOneMemberRelationMultipolygonOuter(), check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(1, flags.size()));
}

@Test
public void testValidRelationMultipolygon()
{
this.verifier.actual(this.setup.getValidRelationMultipolygon(), check);
this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
}

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

import org.openstreetmap.atlas.geography.atlas.Atlas;
import org.openstreetmap.atlas.tags.RelationTypeTag;
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;
import org.openstreetmap.atlas.utilities.testing.TestAtlas.Relation;
import org.openstreetmap.atlas.utilities.testing.TestAtlas.Relation.Member;

/**
* {@link OneMemberRelationCheckTest} data generator
*
* @author savannahostrowski
*/
public class OneMemberRelationCheckTestRule extends CoreTestRule
{
private static final String ONE = "18.4360044, -71.7194204";
private static final String TWO = "18.4360737, -71.6970306";
private static final String THREE = "18.4273807, -71.7052283";

@TestAtlas(
// nodes
nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@Node(id = "2", coordinates = @Loc(value = TWO)),
@Node(id = "3", coordinates = @Loc(value = THREE)) },
// edges
edges = {
@Edge(id = "12", coordinates = { @Loc(value = ONE),
@Loc(value = TWO) }, tags = { "highway=road" }),
@Edge(id = "23", coordinates = { @Loc(value = TWO),
@Loc(value = THREE) }, tags = { "highway=road" }),
@Edge(id = "31", coordinates = { @Loc(value = THREE),
@Loc(value = ONE) }, tags = { "highway=road" }) },
// relations
relations = { @Relation(id = "123", members = {
@Member(id = "12", type = "edge", role = RelationTypeTag.RESTRICTION_ROLE_FROM),
@Member(id = "2", type = "node", role = RelationTypeTag.RESTRICTION_ROLE_VIA),
@Member(id = "23", type = "edge", role = RelationTypeTag.RESTRICTION_ROLE_TO) }, tags = {
"restriction=no_u_turn" }) })
private Atlas validRelation;

@TestAtlas(
// nodes
nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@Node(id = "2", coordinates = @Loc(value = TWO)) },
// edges
edges = { @Edge(id = "12", coordinates = { @Loc(value = ONE),
@Loc(value = TWO) }, tags = { "highway=road" }) },
// relations
relations = { @Relation(id = "123", members = {
@Member(id = "12", type = "edge", role = RelationTypeTag.RESTRICTION_ROLE_FROM) }, tags = {
"restriction=no_u_turn" }) })
private Atlas oneMemberRelation;

@TestAtlas(
// nodes
nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@Node(id = "2", coordinates = @Loc(value = TWO)) },
// edges
edges = { @Edge(id = "12", coordinates = { @Loc(value = ONE),
@Loc(value = TWO) }, tags = { "highway=road" }) },
// relations
relations = { @Relation(id = "123", members = {
@Member(id = "12", type = "edge", role = RelationTypeTag.MULTIPOLYGON_ROLE_INNER) }, tags = {
"restriction=no_u_turn", "type=multipolygon" }) })
private Atlas oneMemberRelationMultipolygonInner;

@TestAtlas(
// nodes
nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@Node(id = "2", coordinates = @Loc(value = TWO)) },
// edges
edges = { @Edge(id = "12", coordinates = { @Loc(value = ONE),
@Loc(value = TWO) }, tags = { "highway=road" }) },
// relations
relations = { @Relation(id = "123", members = {
@Member(id = "12", type = "edge", role = RelationTypeTag.MULTIPOLYGON_ROLE_OUTER) }, tags = {
"restriction=no_u_turn", "type=multipolygon" }) })
private Atlas oneMemberRelationMultipolygonOuter;

@TestAtlas(
// nodes
nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@Node(id = "2", coordinates = @Loc(value = TWO)),
@Node(id = "3", coordinates = @Loc(value = THREE)) },
// edges
edges = {
@Edge(id = "12", coordinates = { @Loc(value = ONE),
@Loc(value = TWO) }, tags = { "highway=road" }),
@Edge(id = "23", coordinates = { @Loc(value = TWO),
@Loc(value = THREE) }, tags = { "highway=road" }),
@Edge(id = "31", coordinates = { @Loc(value = THREE),
@Loc(value = ONE) }, tags = { "highway=road" }) },
// relations
relations = { @Relation(id = "123", members = {
@Member(id = "12", type = "edge", role = RelationTypeTag.MULTIPOLYGON_ROLE_INNER),
@Member(id = "2", type = "node", role = RelationTypeTag.MULTIPOLYGON_ROLE_OUTER),
@Member(id = "23", type = "edge", role = RelationTypeTag.MULTIPOLYGON_ROLE_OUTER) }, tags = {
"restriction=no_u_turn", "type=multipolygon" }) })
private Atlas validRelationMultipolygon;

public Atlas getValidRelation()
{
return this.validRelation;
}

public Atlas getOneMemberRelation()
{
return this.oneMemberRelation;
}

public Atlas getOneMemberRelationMultipolygonInner()
{
return this.oneMemberRelationMultipolygonInner;
}

public Atlas getOneMemberRelationMultipolygonOuter()
{
return this.oneMemberRelationMultipolygonOuter;
}

public Atlas getValidRelationMultipolygon()
{
return this.validRelationMultipolygon;
}
}

0 comments on commit a8247f2

Please sign in to comment.