Skip to content

Commit

Permalink
Add support for exceptions to the normal tag reversing behaviour
Browse files Browse the repository at this point in the history 
This allows per key exceptions to the usual way/node tag reversal rules
by specifying the key and a JosmFilter expression that has to match.

See openstreetmap/iD#10128
  • Loading branch information
@simonpoole
simonpoole committed Feb 26, 2024
1 parent 48ad773 commit 8dd5279
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 68 deletions.
4 changes: 2 additions & 2 deletions src/androidTest/java/de/blau/android/osm/GeometryEditsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -887,15 +887,15 @@ public void reverseWay() {

// reverse "one way" too now
java.util.Map<String, String> dirTags = Reverse.getDirectionDependentTags(w);
if (dirTags != null) {
if (!dirTags.isEmpty()) {
Reverse.reverseDirectionDependentTags(w, dirTags, true);
} else {
fail("no direction dependent tags found");
}
assertEquals("-1", w.getTagWithKey(Tags.KEY_ONEWAY));

dirTags = Reverse.getDirectionDependentTags(w);
if (dirTags != null) {
if (!dirTags.isEmpty()) {
Reverse.reverseDirectionDependentTags(w, dirTags, true);
} else {
fail("no direction dependent tags found");
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/de/blau/android/osm/MergeAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public List<Result> mergeWays() throws OsmIllegalOperationException {
atBeginning = true;
// check for direction dependent tags
Map<String, String> dirTags = Reverse.getDirectionDependentTags(w2);
if (dirTags != null) {
if (!dirTags.isEmpty()) {
Reverse.reverseDirectionDependentTags(w2, dirTags, true);
mergeResult.addIssue(ReverseIssue.TAGSREVERSED);
}
Expand All @@ -197,7 +197,7 @@ public List<Result> mergeWays() throws OsmIllegalOperationException {
atBeginning = false;
// check for direction dependent tags
Map<String, String> dirTags = Reverse.getDirectionDependentTags(w2);
if (dirTags != null) {
if (!dirTags.isEmpty()) {
Reverse.reverseDirectionDependentTags(w2, dirTags, true);
mergeResult.addIssue(ReverseIssue.TAGSREVERSED);
}
Expand Down Expand Up @@ -348,7 +348,7 @@ public List<Result> mergeSimplePolygons(@NonNull de.blau.android.Map map) throws
if (!otherInputRing.isEmpty() && winding(otherInputRing) == COUNTERCLOCKWISE) {
// check for direction dependent tags
Map<String, String> dirTags = Reverse.getDirectionDependentTags(p2);
if (dirTags != null) {
if (!dirTags.isEmpty()) {
Reverse.reverseDirectionDependentTags(p2, dirTags, true);
mergeResult.addIssue(ReverseIssue.TAGSREVERSED);
}
Expand Down
85 changes: 61 additions & 24 deletions src/main/java/de/blau/android/osm/Reverse.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package de.blau.android.osm;

import java.io.ByteArrayInputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
Expand All @@ -14,6 +16,10 @@
import android.util.Log;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import ch.poole.osm.josmfilterparser.Condition;
import ch.poole.osm.josmfilterparser.JosmFilterParseException;
import ch.poole.osm.josmfilterparser.JosmFilterParser;
import de.blau.android.search.Wrapper;

/**
* Logic for reversing direction dependent tags, this is one of the more arcane things about the OSM data model
Expand All @@ -22,7 +28,8 @@
*
*/
final class Reverse {
private static final String DEBUG_TAG = "Reverse";

private static final String DEBUG_TAG = Reverse.class.getSimpleName();

private static final String LEFT_INFIX = ":left:";
private static final String RIGHT_INFIX = ":right:";
Expand All @@ -40,6 +47,15 @@ final class Reverse {
private static Set<String> directionDependentValues = Collections
.unmodifiableSet(new HashSet<>(Arrays.asList(Tags.VALUE_RIGHT, Tags.VALUE_LEFT, Tags.VALUE_FORWARD, Tags.VALUE_BACKWARD)));

private static Map<String, Condition> reverseExceptions = new HashMap<>();
static {
try {
reverseExceptions.put(Tags.KEY_SIDE, compilePattern(Tags.KEY_HIGHWAY + "=" + Tags.VALUE_CYCLIST_WAITING_AID));
} catch (JosmFilterParseException e) {
Log.e(DEBUG_TAG, e.getMessage());
}
}

/**
* Private constructor
*/
Expand All @@ -51,33 +67,43 @@ private Reverse() {
* Return the direction dependent tags and associated values oneway, *:left, *:right, *:backward, *:forward from the
* element
*
* Probably we should check for issues with relation membership too
*
* @param e element that we extract the direction dependent tags from
* @return map containing the tags
*/
@Nullable
@NonNull
public static Map<String, String> getDirectionDependentTags(@NonNull OsmElement e) {
Map<String, String> result = null;
Map<String, String> result = new TreeMap<>();
Map<String, String> tags = e.getTags();
if (tags != null) {
for (Entry<String, String> entry : tags.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
if ((Tags.KEY_HIGHWAY.equals(key) && (Tags.VALUE_MOTORWAY.equals(value) || Tags.VALUE_MOTORWAY_LINK.equals(value)))
|| directionDependentKeys.contains(key) || key.endsWith(LEFT_POSTFIX) || key.endsWith(RIGHT_POSTFIX) || key.endsWith(BACKWARD_POSTFIX)
|| key.endsWith(FORWARD_POSTFIX) || key.contains(FORWARD_INFIX) || key.contains(BACKWARD_INFIX) || key.contains(RIGHT_INFIX)
|| key.contains(LEFT_INFIX) || directionDependentValues.contains(value)) {
if (result == null) {
result = new TreeMap<>();
}
result.put(key, value);
}
for (Entry<String, String> entry : tags.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
if (((Tags.KEY_HIGHWAY.equals(key) && (Tags.VALUE_MOTORWAY.equals(value) || Tags.VALUE_MOTORWAY_LINK.equals(value)))
|| directionDependentKeys.contains(key) || key.endsWith(LEFT_POSTFIX) || key.endsWith(RIGHT_POSTFIX) || key.endsWith(BACKWARD_POSTFIX)
|| key.endsWith(FORWARD_POSTFIX) || key.contains(FORWARD_INFIX) || key.contains(BACKWARD_INFIX) || key.contains(RIGHT_INFIX)
|| key.contains(LEFT_INFIX) || directionDependentValues.contains(value)) && !matchExceptions(e, key)) {
result.put(key, value);
}
}
return result;
}

/**
* Check if a key is in the exceptions map and the object matches the pattern
*
* @param e the OsmElement
* @param key the key to check
* @return true if this is an exception we should not reverse
*/
private static boolean matchExceptions(@NonNull OsmElement e, @NonNull String key) {
Condition c = reverseExceptions.get(key);
if (c != null) {
Wrapper meta = new Wrapper();
meta.setElement(e);
return c.eval(Wrapper.toJosmFilterType(e), meta, e.getTags());
}
return false;
}

/**
* Return a list of (route) relations that the element is a member of with a direction dependent role
*
Expand Down Expand Up @@ -232,9 +258,9 @@ private static String reverseIncline(@NonNull final String value) {
* @return reversed value
*/
private static String reverseOneway(@NonNull final String value) {
if (Tags.VALUE_YES.equalsIgnoreCase(value) || Tags.VALUE_TRUE.equalsIgnoreCase(value) || "1".equals(value)) {
return "-1";
} else if (Tags.VALUE_REVERSE.equalsIgnoreCase(value) || "-1".equals(value)) {
if (Tags.VALUE_YES.equalsIgnoreCase(value) || Tags.VALUE_TRUE.equalsIgnoreCase(value) || Tags.VALUE_ONE.equals(value)) {
return Tags.VALUE_MINUS_ONE;
} else if (Tags.VALUE_REVERSE.equalsIgnoreCase(value) || Tags.VALUE_MINUS_ONE.equals(value)) {
return Tags.VALUE_YES;
}
return value;
Expand All @@ -256,7 +282,7 @@ public static void reverseDirectionDependentTags(@NonNull OsmElement e, @NonNull
}
Map<String, String> tags = new TreeMap<>(e.getTags());

// remove all dir dependent key first
// remove all dir dependent keys first
for (String key : dirTags.keySet()) {
tags.remove(key);
}
Expand All @@ -271,7 +297,7 @@ public static void reverseDirectionDependentTags(@NonNull OsmElement e, @NonNull
if (reverseOneway && Tags.KEY_HIGHWAY.equals(key) && (Tags.VALUE_MOTORWAY.equals(value) || Tags.VALUE_MOTORWAY_LINK.equals(value))) {
tags.put(key, value); // don't have to change anything
if (!dirTags.containsKey(Tags.KEY_ONEWAY)) {
tags.put(Tags.KEY_ONEWAY, "-1");
tags.put(Tags.KEY_ONEWAY, Tags.VALUE_MINUS_ONE);
}
continue;
}
Expand Down Expand Up @@ -329,7 +355,7 @@ public static void reverseDirectionDependentTags(@NonNull OsmElement e, @NonNull
continue;
}
if (Tags.VALUE_FORWARD.equals(value)) {
tags.put(key, "backward");
tags.put(key, Tags.VALUE_BACKWARD);
continue;
}
if (Tags.VALUE_BACKWARD.equals(value)) {
Expand Down Expand Up @@ -359,4 +385,15 @@ private static String floatToString(float f) {
return String.format(Locale.US, "%s", f);
}
}

/**
* Compile a JosmFilter pattern
*
* @param pattern the pattern to compile
* @return the compiled pattern in a Condition object
* @throws JosmFilterParseException if parsing fails
*/
private static Condition compilePattern(@NonNull String pattern) throws JosmFilterParseException {
return new JosmFilterParser(new ByteArrayInputStream(pattern.getBytes())).condition(false);
}
}
4 changes: 2 additions & 2 deletions src/main/java/de/blau/android/osm/StorageDelegator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,7 @@ public List<Result> reverseWay(@NonNull final Way way) {
undo.save(way);
// check for direction dependent tags
Map<String, String> dirTags = Reverse.getDirectionDependentTags(way);
if (dirTags != null) {
if (!dirTags.isEmpty()) {
Result wayResult = new Result();
wayResult.setElement(way);
wayResult.addIssue(ReverseIssue.TAGSREVERSED);
Expand Down Expand Up @@ -1806,7 +1806,7 @@ List<Result> reverseWayNodeTags(List<Node> nodes) {
List<Result> result = new ArrayList<>();
for (Node n : nodes) {
Map<String, String> nodeDirTags = Reverse.getDirectionDependentTags(n);
if (nodeDirTags != null) {
if (!nodeDirTags.isEmpty()) {
undo.save(n);
Result nodeResult = new Result();
nodeResult.setElement(n);
Expand Down
69 changes: 37 additions & 32 deletions src/main/java/de/blau/android/osm/Tags.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,38 +126,43 @@ public static boolean isLikeAName(@NonNull String key) {
return false;
}

public static final String KEY_NAME_LEFT = "name:left";
public static final String KEY_NAME_RIGHT = "name:right";
public static final String KEY_REF = "ref";
public static final String KEY_NONAME = "noname";
public static final String KEY_VALIDATE_NO_NAME = "validate:no_name";
public static final String KEY_NOREF = "noref";
public static final String KEY_HIGHWAY = "highway";
public static final String VALUE_ROAD = "road";
public static final String VALUE_MOTORWAY = "motorway";
public static final String VALUE_MOTORWAY_LINK = "motorway_link";
public static final String VALUE_FOOTWAY = "footway";
public static final String VALUE_CYCLEWAY = "cycleway";
public static final String VALUE_PATH = "path";
public static final String VALUE_STEPS = "steps";
public static final String VALUE_TRACK = "track";
public static final String KEY_TRACKTYPE = "tracktype";
public static final String KEY_SIDEWALK = "sidewalk";
public static final String KEY_BRIDGE = "bridge";
public static final String KEY_TUNNEL = "tunnel";
public static final String VALUE_CULVERT = "culvert";
public static final String KEY_BARRIER = "barrier";
public static final String VALUE_RETAINING_WALL = "retaining_wall";
public static final String VALUE_KERB = "kerb";
public static final String VALUE_GUARD_RAIL = "guard_rail";
public static final String VALUE_CITY_WALL = "city_wall";
public static final String KEY_TWO_SIDED = "two_sided";
public static final String KEY_MAN_MADE = "man_made";
public static final String VALUE_EMBANKMENT = "embankment";
public static final String KEY_ONEWAY = "oneway";
public static final String VALUE_REVERSE = "reverse";
public static final String KEY_WATERWAY = "waterway";
public static final String VALUE_RIVERBANK = "riverbank";
public static final String KEY_NAME_LEFT = "name:left";
public static final String KEY_NAME_RIGHT = "name:right";
public static final String KEY_REF = "ref";
public static final String KEY_NONAME = "noname";
public static final String KEY_VALIDATE_NO_NAME = "validate:no_name";
public static final String KEY_NOREF = "noref";
public static final String KEY_HIGHWAY = "highway";
public static final String VALUE_ROAD = "road";
public static final String VALUE_MOTORWAY = "motorway";
public static final String VALUE_MOTORWAY_LINK = "motorway_link";
public static final String VALUE_FOOTWAY = "footway";
public static final String VALUE_CYCLEWAY = "cycleway";
public static final String VALUE_PATH = "path";
public static final String VALUE_STEPS = "steps";
public static final String VALUE_TRACK = "track";
public static final String KEY_TRACKTYPE = "tracktype";
public static final String KEY_SIDEWALK = "sidewalk";
public static final String KEY_BRIDGE = "bridge";
public static final String KEY_TUNNEL = "tunnel";
public static final String VALUE_CULVERT = "culvert";
public static final String KEY_BARRIER = "barrier";
public static final String VALUE_RETAINING_WALL = "retaining_wall";
public static final String VALUE_KERB = "kerb";
public static final String VALUE_GUARD_RAIL = "guard_rail";
public static final String VALUE_CITY_WALL = "city_wall";
public static final String KEY_TWO_SIDED = "two_sided";
public static final String KEY_MAN_MADE = "man_made";
public static final String VALUE_EMBANKMENT = "embankment";
public static final String KEY_ONEWAY = "oneway";
public static final String VALUE_REVERSE = "reverse";
public static final String VALUE_MINUS_ONE = "-1";
public static final String VALUE_ONE = "1";
public static final String VALUE_CYCLIST_WAITING_AID = "cyclist_waiting_aid";
public static final String KEY_SIDE = "side";

public static final String KEY_WATERWAY = "waterway";
public static final String VALUE_RIVERBANK = "riverbank";

public static final String KEY_LANDUSE = "landuse";
public static final String VALUE_CONSTRUCTION = "construction";
Expand Down
35 changes: 31 additions & 4 deletions src/main/java/de/blau/android/search/Wrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import android.content.Context;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import ch.poole.osm.josmfilterparser.Condition;
import ch.poole.osm.josmfilterparser.ElementState.State;
import ch.poole.osm.josmfilterparser.Meta;
Expand Down Expand Up @@ -48,12 +49,20 @@ public class Wrapper implements Meta {
final Context context;
final Logic logic;

/**
* Create a new wrapper object
*
*/
public Wrapper() {
this(null);
}

/**
* Create a new wrapper object
*
* @param context an Android Context
*/
public Wrapper(@NonNull Context context) {
public Wrapper(@Nullable Context context) {
this.context = context;
this.logic = App.getLogic();
}
Expand Down Expand Up @@ -93,7 +102,18 @@ public static Type toJosmFilterType(@NonNull OsmElement element) {

@Override
public String getUser() {
throw new IllegalArgumentException(context.getString(R.string.search_objects_unsupported, "user"));
throw unsupported("user");
}

/**
* Construct an IllegalArgumentException
*
* @param expression the unsupported expression
* @return an IllegalArgumentException
*/
private IllegalArgumentException unsupported(@NonNull String expression) {
return new IllegalArgumentException(
context != null ? context.getString(R.string.search_objects_unsupported, expression) : "Unsupported expression \"" + expression + "\"");
}

@Override
Expand All @@ -108,7 +128,7 @@ public long getVersion() {

@Override
public long getChangeset() {
throw new IllegalArgumentException(context.getString(R.string.search_objects_unsupported, "changeset"));
throw unsupported("changeset");
}

@Override
Expand Down Expand Up @@ -176,7 +196,7 @@ private int getMemberTypeCount(@NonNull String type) {

@Override
public int getAreaSize() {
throw new IllegalArgumentException(context.getString(R.string.search_objects_unsupported, "areasize"));
throw unsupported("areasize");
}

@Override
Expand Down Expand Up @@ -227,6 +247,9 @@ public boolean hasRole(@NonNull String role) {

@Override
public Object getPreset(@NonNull String presetPath) {
if (context == null) {
throw unsupported("preset:");
}
if (presetPath.endsWith("*")) {
presetPath = presetPath.substring(0, presetPath.length() - 1);
}
Expand Down Expand Up @@ -412,6 +435,7 @@ public boolean isEmpty() {
* @param c the Condition to check
* @return a SearchResult object
*/
@NonNull
SearchResult getMatchingElementsInternal(@NonNull Condition c) {
StorageDelegator delegator = App.getDelegator();
SearchResult result = new SearchResult();
Expand Down Expand Up @@ -439,6 +463,9 @@ SearchResult getMatchingElementsInternal(@NonNull Condition c) {

@Override
public @NotNull Meta wrap(Object arg0) {
if (context == null) {
throw unsupported("unknown");
}
Wrapper wrapper = new Wrapper(context);
wrapper.setElement((OsmElement) arg0);
return wrapper;
Expand Down

0 comments on commit 8dd5279

Please sign in to comment.