Skip to content

Commit

Permalink
CheckFlag Fix Suggestions (#361)
Browse files Browse the repository at this point in the history 
* fix suggestions

* fix tag

* comment and constant

* comment and constant
  • Loading branch information
@Bentleysb
Bentleysb authored Sep 23, 2020
1 parent bbba79b commit 91b7ec1
Show file tree
Hide file tree
Showing 7 changed files with 514 additions and 30 deletions.
21 changes: 20 additions & 1 deletion src/main/java/org/openstreetmap/atlas/checks/event/CheckFlagEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.function.Consumer;
import java.util.stream.Collectors;

import org.apache.commons.lang3.StringUtils;
import org.openstreetmap.atlas.checks.base.Check;
import org.openstreetmap.atlas.checks.flag.CheckFlag;
import org.openstreetmap.atlas.checks.flag.FlaggedObject;
Expand All @@ -34,7 +35,8 @@
* Wraps a {@link CheckFlag} for submission to the
* {@link org.openstreetmap.atlas.event.EventService} for handling {@link Check} results
*
* @author mkalender, bbreithaupt
* @author mkalender
* @author bbreithaupt
*/
public final class CheckFlagEvent extends Event
{
Expand All @@ -45,6 +47,7 @@ public final class CheckFlagEvent extends Event
private static final String FEATURE_COLLECTION = "FeatureCollection";
private static final String INSTRUCTIONS = "instructions";
private static final String IDENTIFIERS = "identifiers";
private static final String FIX_SUGGESTIONS = "fix_suggestions";

private static final Gson GSON = new Gson();

Expand Down Expand Up @@ -149,6 +152,7 @@ else if (flaggedRelations.size() != 1 && !feature.has(GEOMETRY))
flagProperties.add("feature_osmids", uniqueFeatureOsmIds);
flagProperties.addProperty("feature_count", featureProperties.size());
flagProperties.add(IDENTIFIERS, GSON.toJsonTree(flag.getUniqueIdentifiers()));
flagProperties.add(FIX_SUGGESTIONS, getFixSuggestionDescriptions(flag));

feature.addProperty("id", flag.getIdentifier());
feature.add("properties", flagProperties);
Expand Down Expand Up @@ -202,6 +206,10 @@ public static JsonObject flagToJson(final CheckFlag flag,

// Add properties to the previously generate geojson
flagJson.add("properties", flagPropertiesJson);

// Add fix suggestions as their own foreign object in the geojson
flagJson.add(FIX_SUGGESTIONS, getFixSuggestionDescriptions(flag));

return flagJson;
}

Expand Down Expand Up @@ -235,6 +243,17 @@ private static Optional<String> featureDecorator(final JsonArray featureProperti
.map(tag -> String.format("%s=%s", HighwayTag.KEY, tag.getTagValue()));
}

private static JsonObject getFixSuggestionDescriptions(final CheckFlag flag)
{
final JsonObject fixSuggestionObject = new JsonObject();
flag.getFixSuggestions()
.forEach(suggestion -> fixSuggestionObject.add(
StringUtils.capitalize(suggestion.getItemType().toString().toLowerCase())
+ suggestion.getIdentifier(),
suggestion.explain().toJsonElement()));
return fixSuggestionObject;
}

/**
* Get geometry of flagged relation feature.
*
Expand Down
67 changes: 67 additions & 0 deletions src/main/java/org/openstreetmap/atlas/checks/flag/CheckFlag.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import java.io.Serializable;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
Expand All @@ -23,6 +25,7 @@
import org.openstreetmap.atlas.geography.Location;
import org.openstreetmap.atlas.geography.PolyLine;
import org.openstreetmap.atlas.geography.Rectangle;
import org.openstreetmap.atlas.geography.atlas.change.FeatureChange;
import org.openstreetmap.atlas.geography.atlas.items.AtlasItem;
import org.openstreetmap.atlas.geography.atlas.items.AtlasObject;
import org.openstreetmap.atlas.geography.atlas.items.LocationItem;
Expand Down Expand Up @@ -61,6 +64,7 @@ public class CheckFlag implements Iterable<Location>, Located, Serializable
private Set<FlaggedObject> flaggedObjects = new LinkedHashSet<>();
private final String identifier;
private final List<String> instructions = new ArrayList<>();
private final Set<FeatureChange> fixSuggestions = new HashSet<>();

/**
* A basic constructor that simply flags some identifying value
Expand Down Expand Up @@ -105,11 +109,66 @@ public CheckFlag(final String identifier, final Set<? extends AtlasObject> objec
*/
public CheckFlag(final String identifier, final Set<? extends AtlasObject> objects,
final List<String> instructions, final List<Location> points)
{
this(identifier, objects, instructions, points, new HashSet<>());
}

/**
* Creates a {@link CheckFlag} with the addition of a list of {@code point} {@link Location}s
* that highlight specific points and a {@link Set} of {@link FeatureChange}s that suggest how
* to fix the flagged objects.
*
* @param identifier
* the identifying value to flag
* @param objects
* {@link AtlasObject}s to flag
* @param instructions
* a list of free form instructions
* @param points
* {@code point} {@link Location}s to highlight
* @param fixSuggestions
* {@link Set} of {@link FeatureChange}s representing suggested fixes for flagged
* features
*/
public CheckFlag(final String identifier, final Set<? extends AtlasObject> objects,
final List<String> instructions, final List<Location> points,
final Set<FeatureChange> fixSuggestions)
{
this.addObjects(objects);
this.addPoints(points);
this.addInstructions(instructions);
this.identifier = identifier;
this.addFixSuggestions(fixSuggestions);
}

/**
* Add a single {@link FeatureChange} to the fix suggestions. Fix suggestions should be
* {@link FeatureChange}s of
* {@link org.openstreetmap.atlas.geography.atlas.complete.CompleteEntity}s created from flagged
* {@link org.openstreetmap.atlas.geography.atlas.items.AtlasEntity}s with changes suggesting
* how the flagged feature can be fixed.
*
* @param suggestion
* {@link FeatureChange} with suggested alterations to a flagged feature
* @return this {@link CheckFlag}
*/
public CheckFlag addFixSuggestion(final FeatureChange suggestion)
{
this.fixSuggestions.add(suggestion);
return this;
}

/**
* Add a {@link Set} of {@link FeatureChange}s to the fix suggestions.
*
* @param suggestions
* {@link Collection} of {@link FeatureChange} fix suggestions
* @return this {@link CheckFlag}
*/
public CheckFlag addFixSuggestions(final Collection<FeatureChange> suggestions)
{
this.fixSuggestions.addAll(suggestions);
return this;
}

/**
Expand Down Expand Up @@ -302,6 +361,14 @@ public String getCountryISO()
return FlaggedObject.COUNTRY_MISSING;
}

/**
* @return a {@link Set} of {@link FeatureChange} fix suggestions
*/
public Set<FeatureChange> getFixSuggestions()
{
return this.fixSuggestions;
}

/**
* @return a set of flagged {@link AtlasObject}s
*/
Expand Down
74 changes: 48 additions & 26 deletions src/main/java/org/openstreetmap/atlas/checks/validation/areas/AreasWithHighwayTagCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
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.change.FeatureChange;
import org.openstreetmap.atlas.geography.atlas.complete.CompleteEntity;
import org.openstreetmap.atlas.geography.atlas.items.Area;
import org.openstreetmap.atlas.geography.atlas.items.AtlasEntity;
import org.openstreetmap.atlas.geography.atlas.items.AtlasObject;
import org.openstreetmap.atlas.geography.atlas.items.Edge;
import org.openstreetmap.atlas.geography.atlas.walker.EdgeWalker;
import org.openstreetmap.atlas.tags.AreaTag;
import org.openstreetmap.atlas.tags.HighwayTag;
import org.openstreetmap.atlas.tags.annotations.validation.Validators;
Expand All @@ -25,6 +26,7 @@
*
* @author matthieun
* @author cuthbertm
* @author bbreithaupt
*/
public class AreasWithHighwayTagCheck extends BaseCheck<Long>
{
Expand Down Expand Up @@ -71,33 +73,41 @@ protected Optional<CheckFlag> flag(final AtlasObject object)
// If the tag isn't one of the VALID_HIGHWAY_TAGS, we want to flag it.
.filter(tag -> isUnacceptableAreaHighwayTagCombination(object, tag)).map(tag ->
{
final String instruction;
this.markAsFlagged(object.getOsmIdentifier());

if (tag.equals(HighwayTag.FOOTWAY))
{
instruction = this.getLocalizedInstruction(0, object.getOsmIdentifier(),
tag, HighwayTag.PEDESTRIAN);
}
else
{
instruction = this.getLocalizedInstruction(1, object.getOsmIdentifier(),
tag.getTagValue());
}
final Set<AtlasObject> results;
if (object instanceof Edge)
{
final EdgeWalker walker = new AreasWithHighwayTagCheckWalker((Edge) object);
final Set<Edge> connectedBadEdges = walker.collectEdges().stream()
.filter(Edge::isMainEdge).collect(Collectors.toSet());
connectedBadEdges
.forEach(badEdge -> this.markAsFlagged(badEdge.getOsmIdentifier()));
results = new HashSet<>(connectedBadEdges);
}
else
{
results = Collections.singleton(object);
this.markAsFlagged(object.getOsmIdentifier());
final Set<AtlasObject> objectsToFlag = this.getObjectsToFlag(object);
return this
.createFlag(objectsToFlag,
this.getLocalizedInstruction(0, object.getOsmIdentifier(),
tag, HighwayTag.PEDESTRIAN))
.addFixSuggestions(objectsToFlag.stream()
.map(toFlag -> FeatureChange.add(
(AtlasEntity) ((CompleteEntity) CompleteEntity
.shallowFrom((AtlasEntity) toFlag))
.withTags(toFlag.getTags())
.withReplacedTag(HighwayTag.KEY,
HighwayTag.KEY,
HighwayTag.PEDESTRIAN
.getTagValue()),
object.getAtlas()))
.collect(Collectors.toSet()));
}
return this.createFlag(results, instruction);

final Set<AtlasObject> objectsToFlag = this.getObjectsToFlag(object);
return this
.createFlag(objectsToFlag,
this.getLocalizedInstruction(1, object.getOsmIdentifier(), tag,
tag.getTagValue()))
.addFixSuggestions(objectsToFlag.stream()
.map(toFlag -> FeatureChange.add(
(AtlasEntity) ((CompleteEntity) CompleteEntity
.shallowFrom((AtlasEntity) toFlag))
.withTags(toFlag.getTags())
.withRemovedTag(AreaTag.KEY),
object.getAtlas()))
.collect(Collectors.toSet()));
});
}

Expand All @@ -106,4 +116,16 @@ protected List<String> getFallbackInstructions()
{
return FALLBACK_INSTRUCTIONS;
}

private Set<AtlasObject> getObjectsToFlag(final AtlasObject object)
{
if (object instanceof Edge)
{
return new AreasWithHighwayTagCheckWalker((Edge) object).collectEdges().stream()
.filter(Edge::isMainEdge).collect(Collectors.toSet());
}

return Collections.singleton(object);

}
}
Loading

0 comments on commit 91b7ec1

Please sign in to comment.