Repository: graphhopper/graphhopper
Pull request #2198 (2020-12-01 22:57:31)

Stricter naming of EncodedValue 
A stricter naming schema for EncodedValue so we can later safely use an EncodedValue as a variable in expressions.

Incl. minor bug fix in BooleanToValueEntry.


Patch:
diff --git a/NOTICE.md b/NOTICE.md
index add7e0a37..f2fb150ca 100644
--- a/NOTICE.md
+++ b/NOTICE.md
@@ -17,6 +17,7 @@ The core module includes the following software:
  * com.fasterxml.jackson.core:jackson-annotations (Apache License)
  * org.locationtech:jts (EDL), see #1039
  * AngleCalc.atan2 from Jim Shima, 1999 (public domain)
+ * list of Java keywords in EncodingManager from janino compiler (BSD-3-Clause license)
 
 reader-osm:
 
@@ -44,12 +45,6 @@ web:
  * com.google.inject (Apache license)
  * some images from mapbox https://www.mapbox.com/maki/, BSD License, see core/files
 
-android:
-
- * android (Apache license)
- * org.mapsforge, LGPL
- * VTM, LGPL
-
 ## Data
 
 |source | license | used as default | included in repo |
diff --git a/core/files/changelog.txt b/core/files/changelog.txt
index 912ba8900..19ae42b9b 100644
--- a/core/files/changelog.txt
+++ b/core/files/changelog.txt
@@ -1,4 +1,5 @@
 3.0
+    the name of an encoded value can only contain lower letters, underscore or numbers. It has to start with a lower letter
     default for GraphHopperMatrixWeb (client for Matrix API) is now the sync POST request without the artificial polling delay in most cases
     refactored TransportationMode to better reflect the usage in source data parsing only
     CustomWeighting requires LinkedHashMap throw exception if this is not provided
diff --git a/core/src/main/java/com/graphhopper/routing/ev/EncodedValueLookup.java b/core/src/main/java/com/graphhopper/routing/ev/EncodedValueLookup.java
index c36c635dc..52a8d2f0b 100644
--- a/core/src/main/java/com/graphhopper/routing/ev/EncodedValueLookup.java
+++ b/core/src/main/java/com/graphhopper/routing/ev/EncodedValueLookup.java
@@ -21,7 +21,7 @@ import java.util.List;
 
 public interface EncodedValueLookup {
 
-    List<EncodedValue> getAllShared();
+    List<EncodedValue> getEncodedValues();
 
     <T extends EncodedValue> T getEncodedValue(String key, Class<T> encodedValueType);
 
diff --git a/core/src/main/java/com/graphhopper/routing/ev/UnsignedIntEncodedValue.java b/core/src/main/java/com/graphhopper/routing/ev/UnsignedIntEncodedValue.java
index 281982a73..4fb107ba9 100644
--- a/core/src/main/java/com/graphhopper/routing/ev/UnsignedIntEncodedValue.java
+++ b/core/src/main/java/com/graphhopper/routing/ev/UnsignedIntEncodedValue.java
@@ -17,6 +17,7 @@
  */
 package com.graphhopper.routing.ev;
 
+import com.graphhopper.routing.util.EncodingManager;
 import com.graphhopper.storage.IntsRef;
 import com.graphhopper.util.Helper;
 
@@ -53,8 +54,8 @@ public class UnsignedIntEncodedValue implements IntEncodedValue {
      *                           direction.
      */
     public UnsignedIntEncodedValue(String name, int bits, boolean storeTwoDirections) {
-        if (!name.toLowerCase(Locale.ROOT).equals(name))
-            throw new IllegalArgumentException("EncodedValue name must be lower case but was " + name);
+        if (!EncodingManager.isValidEncodedValue(name))
+            throw new IllegalArgumentException("EncodedValue name wasn't valid: " + name + ". Use lower case letters, underscore and numbers only.");
         if (bits <= 0)
             throw new IllegalArgumentException(name + ": bits cannot be zero or negative");
         if (bits > 31)
diff --git a/core/src/main/java/com/graphhopper/routing/util/AbstractFlagEncoder.java b/core/src/main/java/com/graphhopper/routing/util/AbstractFlagEncoder.java
index c67fb142e..f34d772e1 100644
--- a/core/src/main/java/com/graphhopper/routing/util/AbstractFlagEncoder.java
+++ b/core/src/main/java/com/graphhopper/routing/util/AbstractFlagEncoder.java
@@ -398,8 +398,8 @@ public abstract class AbstractFlagEncoder implements FlagEncoder {
     }
 
     @Override
-    public List<EncodedValue> getAllShared() {
-        return encodedValueLookup.getAllShared();
+    public List<EncodedValue> getEncodedValues() {
+        return encodedValueLookup.getEncodedValues();
     }
 
     @Override
diff --git a/core/src/main/java/com/graphhopper/routing/util/EncodingManager.java b/core/src/main/java/com/graphhopper/routing/util/EncodingManager.java
index f235d9174..c9a8c8b94 100644
--- a/core/src/main/java/com/graphhopper/routing/util/EncodingManager.java
+++ b/core/src/main/java/com/graphhopper/routing/util/EncodingManager.java
@@ -475,9 +475,9 @@ public class EncodingManager implements EncodedValueLookup {
     private void addEncodedValue(EncodedValue ev, boolean withNamespace) {
         if (hasEncodedValue(ev.getName()))
             throw new IllegalStateException("EncodedValue " + ev.getName() + " already exists " + encodedValueMap.get(ev.getName()) + " vs " + ev);
-        if (!withNamespace && !isSharedEV(ev))
+        if (!withNamespace && !isSharedEncodedValues(ev))
             throw new IllegalArgumentException("EncodedValue " + ev.getName() + " must not contain namespace character '" + SPECIAL_SEPARATOR + "'");
-        if (withNamespace && isSharedEV(ev))
+        if (withNamespace && isSharedEncodedValues(ev))
             throw new IllegalArgumentException("EncodedValue " + ev.getName() + " must contain namespace character '" + SPECIAL_SEPARATOR + "'");
         ev.init(edgeConfig);
         encodedValueMap.put(ev.getName(), ev);
@@ -651,7 +651,7 @@ public class EncodingManager implements EncodedValueLookup {
     public String toEncodedValuesAsString() {
         StringBuilder str = new StringBuilder();
         for (EncodedValue ev : encodedValueMap.values()) {
-            if (!isSharedEV(ev))
+            if (!isSharedEncodedValues(ev))
                 continue;
 
             if (str.length() > 0)
@@ -739,7 +739,7 @@ public class EncodingManager implements EncodedValueLookup {
     }
 
     public List<FlagEncoder> fetchEdgeEncoders() {
-        return new ArrayList<FlagEncoder>(edgeEncoders);
+        return new ArrayList<>(edgeEncoders);
     }
 
     public boolean needsTurnCostsSupport() {
@@ -761,12 +761,8 @@ public class EncodingManager implements EncodedValueLookup {
     }
 
     @Override
-    public List<EncodedValue> getAllShared() {
-        List<EncodedValue> list = new ArrayList<>(encodedValueMap.size());
-        for (EncodedValue ev : encodedValueMap.values()) {
-            if (isSharedEV(ev)) list.add(ev);
-        }
-        return list;
+    public List<EncodedValue> getEncodedValues() {
+        return Collections.unmodifiableList(new ArrayList<>(encodedValueMap.values()));
     }
 
     @Override
@@ -800,14 +796,13 @@ public class EncodingManager implements EncodedValueLookup {
 
     private static final String SPECIAL_SEPARATOR = ".";
 
-    private boolean isSharedEV(EncodedValue ev) {
+    public static boolean isSharedEncodedValues(EncodedValue ev) {
         return !ev.getName().contains(SPECIAL_SEPARATOR);
     }
 
     /**
      * All EncodedValue names that are created from a FlagEncoder should use this method to mark them as
-     * "none-shared" across the other FlagEncoders. E.g. average_speed for the CarFlagEncoder will
-     * be named car.average_speed
+     * "none-shared" across the other FlagEncoders.
      */
     public static String getKey(FlagEncoder encoder, String str) {
         return getKey(encoder.toString(), str);
@@ -816,4 +811,50 @@ public class EncodingManager implements EncodedValueLookup {
     public static String getKey(String prefix, String str) {
         return prefix + SPECIAL_SEPARATOR + str;
     }
+
+    // copied from janino
+    private static final Set<String> KEYWORDS = new HashSet<>(Arrays.asList(
+            "first_match",
+            "abstract", "assert",
+            "boolean", "break", "byte",
+            "case", "catch", "char", "class", "const", "continue",
+            "default", "do", "double",
+            "else", "enum", "extends",
+            "final", "finally", "float", "for",
+            "goto",
+            "if", "implements", "import", "instanceof", "int", "interface",
+            "long",
+            "native", "new",
+            "package", "private", "protected", "public",
+            "return",
+            "short", "static", "strictfp", "super", "switch", "synchronized",
+            "this", "throw", "throws", "transient", "try",
+            "void", "volatile",
+            "while"
+    ));
+
+    public static boolean isValidEncodedValue(String name) {
+        // first character must be a lower case letter
+        if (name.isEmpty() || !isLowerLetter(name.charAt(0)) || KEYWORDS.contains(name)) return false;
+
+        int dotCount = 0;
+        for (int i = 1; i < name.length(); i++) {
+            char c = name.charAt(i);
+            if (c == '.') {
+                if (dotCount > 0) return false;
+                dotCount++;
+            } else if (!isLowerLetter(c) && !isNumber(c) && c != '_' && c != '$') {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private static boolean isNumber(char c) {
+        return c >= '0' && c <= '9';
+    }
+
+    private static boolean isLowerLetter(char c) {
+        return c >= 'a' && c <= 'z';
+    }
 }
\ No newline at end of file
diff --git a/core/src/main/java/com/graphhopper/routing/weighting/custom/BooleanToValueEntry.java b/core/src/main/java/com/graphhopper/routing/weighting/custom/BooleanToValueEntry.java
index 781438ef3..bcca3e470 100644
--- a/core/src/main/java/com/graphhopper/routing/weighting/custom/BooleanToValueEntry.java
+++ b/core/src/main/java/com/graphhopper/routing/weighting/custom/BooleanToValueEntry.java
@@ -75,7 +75,7 @@ final class BooleanToValueEntry implements EdgeToValueEntry {
     @Override
     public double getValue(EdgeIteratorState iter, boolean reverse) {
         if (Double.isNaN(value)) return elseValue; // special case if only catch-all key is present
-        return iter.get(bev) ? value : elseValue;
+        return (reverse ? iter.getReverse(bev) : iter.get(bev)) ? value : elseValue;
     }
 
     @Override
diff --git a/core/src/main/java/com/graphhopper/routing/weighting/custom/PriorityCalculator.java b/core/src/main/java/com/graphhopper/routing/weighting/custom/PriorityCalculator.java
index e64da469f..56cb6f60b 100644
--- a/core/src/main/java/com/graphhopper/routing/weighting/custom/PriorityCalculator.java
+++ b/core/src/main/java/com/graphhopper/routing/weighting/custom/PriorityCalculator.java
@@ -73,7 +73,7 @@ final class PriorityCalculator {
     static EncodedValue getEV(EncodedValueLookup lookup, String name, String key) {
         if (!lookup.hasEncodedValue(key))
             throw new IllegalArgumentException("Cannot find encoded value '" + key + "' specified in '" + name
-                    + "'. Available: " + names(lookup.getAllShared()));
+                    + "'. Available: " + names(lookup.getEncodedValues()));
         return lookup.getEncodedValue(key, EncodedValue.class);
     }
 
diff --git a/web-bundle/src/main/java/com/graphhopper/resources/InfoResource.java b/web-bundle/src/main/java/com/graphhopper/resources/InfoResource.java
index 83e285700..c6462918d 100644
--- a/web-bundle/src/main/java/com/graphhopper/resources/InfoResource.java
+++ b/web-bundle/src/main/java/com/graphhopper/resources/InfoResource.java
@@ -21,6 +21,7 @@ import com.graphhopper.GraphHopper;
 import com.graphhopper.GraphHopperConfig;
 import com.graphhopper.config.Profile;
 import com.graphhopper.routing.ev.*;
+import com.graphhopper.routing.util.EncodingManager;
 import com.graphhopper.storage.GraphHopperStorage;
 import com.graphhopper.util.Constants;
 import org.locationtech.jts.geom.Envelope;
@@ -96,11 +97,13 @@ public class InfoResource {
         info.data_date = storage.getProperties().get("datareader.data.date");
 
         // do not list all supported encoded values like the none-shared ones or *.turn_costs
-        List<EncodedValue> evList = storage.getEncodingManager().getAllShared();
+        List<EncodedValue> evList = storage.getEncodingManager().getEncodedValues();
         info.encoded_values = new LinkedHashMap<>();
         for (EncodedValue encodedValue : evList) {
             List<Object> possibleValueList = new ArrayList<>();
-            if (encodedValue instanceof EnumEncodedValue) {
+            if (!EncodingManager.isSharedEncodedValues(encodedValue)) {
+                // skip
+            } else if (encodedValue instanceof EnumEncodedValue) {
                 for (Object o : ((EnumEncodedValue) encodedValue).getValues()) {
                     possibleValueList.add(o.toString());
                 }


Test patch:
diff --git a/core/src/test/java/com/graphhopper/routing/util/EncodingManagerTest.java b/core/src/test/java/com/graphhopper/routing/util/EncodingManagerTest.java
index 0b86c69fe..718c405a5 100644
--- a/core/src/test/java/com/graphhopper/routing/util/EncodingManagerTest.java
+++ b/core/src/test/java/com/graphhopper/routing/util/EncodingManagerTest.java
@@ -26,6 +26,8 @@ import com.graphhopper.routing.ev.RouteNetwork;
 import com.graphhopper.storage.IntsRef;
 import org.junit.Test;
 
+import java.util.Arrays;
+
 import static org.junit.Assert.*;
 
 /**
@@ -275,4 +277,19 @@ public class EncodingManagerTest {
             assertTrue(encoder.toString(), roundaboutEnc.getBool(false, edgeFlags));
         }
     }
+
+    @Test
+    public void validEV() {
+        for (String str : Arrays.asList("blup_test", "test", "test12", "tes$0", "blup.test", "blup_te.st_")) {
+            assertTrue(str, EncodingManager.isValidEncodedValue(str));
+        }
+
+        for (String str : Arrays.asList("Test", "12test", "test|3", "test{34", "test,21", "täst", "blup.two.three", "blup..test")) {
+            assertFalse(str, EncodingManager.isValidEncodedValue(str));
+        }
+
+        for (String str : Arrays.asList("break", "switch")) {
+            assertFalse(str, EncodingManager.isValidEncodedValue(str));
+        }
+    }
 }
diff --git a/core/src/test/java/com/graphhopper/routing/weighting/CustomWeightingTest.java b/core/src/test/java/com/graphhopper/routing/weighting/CustomWeightingTest.java
index 9cea03c84..bdfa48ec9 100644
--- a/core/src/test/java/com/graphhopper/routing/weighting/CustomWeightingTest.java
+++ b/core/src/test/java/com/graphhopper/routing/weighting/CustomWeightingTest.java
@@ -179,6 +179,29 @@ public class CustomWeightingTest {
         assertEquals(5.5, weighting.calcEdgeWeight(edge.set(rcLinkEnc, true), false), 0.01);
     }
 
+    @Test
+    public void testBoolean() {
+        carFE = new CarFlagEncoder();
+        BooleanEncodedValue specialEnc = new SimpleBooleanEncodedValue("special", true);
+        encodingManager = new EncodingManager.Builder().add(carFE).add(specialEnc).build();
+        avSpeedEnc = carFE.getAverageSpeedEnc();
+        graphHopperStorage = new GraphBuilder(encodingManager).create();
+
+        BooleanEncodedValue accessEnc = carFE.getAccessEnc();
+        EdgeIteratorState edge = graphHopperStorage.edge(0, 1).set(accessEnc, true).setReverse(accessEnc, true).
+                set(avSpeedEnc, 15).set(specialEnc, false).setReverse(specialEnc, true).setDistance(10);
+
+        CustomModel vehicleModel = new CustomModel();
+        assertEquals(3.1, createWeighting(vehicleModel).calcEdgeWeight(edge, false), 0.01);
+        Map map = new LinkedHashMap<>();
+        map.put("false", 0.4);
+        map.put("true", 0.8);
+        vehicleModel.getPriority().put("special", map);
+        CustomWeighting weighting = createWeighting(vehicleModel);
+        assertEquals(6.7, weighting.calcEdgeWeight(edge, false), 0.01);
+        assertEquals(3.7, weighting.calcEdgeWeight(edge, true), 0.01);
+    }
+
     @Test
     public void testPriority() {
         CustomModel vehicleModel = new CustomModel();