Do not allow modifications after EncodingManager.Builder.build, improves on graphhopper#1546

Author: karussell

core/files/changelog.txt

@@ -1,6 +1,7 @@
 0.12
     access refactoring #1436 that moves AccessValue into SpatialRule.Access
     refactoring of EncodingManager to use builder pattern. Migration should be simple. Replace new EncodingManager with EncodingManager.create
+	The methods GraphHopper.setEnableInstructions/setPreferredLanguage is now in EncodingManager.Builder
     EncodingManager.supports renames to hasEncoder
     big refactoring #1447: to increase 64bit limit of flags, make reverse direction handling easier, to allow shared EncodedValues,
         remove reverseFlags method, much simpler property access, simplify FlagEncoder (maybe even deprecate this interface at a later stage)

core/src/main/java/com/graphhopper/GraphHopper.java

@@ -75,7 +75,6 @@ public class GraphHopper implements GraphHopperAPI {
     // utils
     private final TranslationMap trMap = new TranslationMap().doImport();
     boolean removeZipped = true;
-    boolean enableInstructions = true;
     // for graph:
     private GraphHopperStorage ghStorage;
     private EncodingManager encodingManager;
@@ -86,7 +85,6 @@ public class GraphHopper implements GraphHopperAPI {
     private boolean elevation = false;
     private LockFactory lockFactory = new NativeFSLockFactory();
     private boolean allowWrites = true;
-    private String preferredLanguage = "";
     private boolean fullyLoaded = false;
     private boolean smoothElevation = false;
     // for routing
@@ -359,44 +357,6 @@ public GraphHopper setElevation(boolean includeElevation) {
         return this;
     }
 
-    public boolean isEnableInstructions() {
-        return enableInstructions;
-    }
-
-    /**
-     * This method specifies if the import should include way names to be able to return
-     * instructions for a route.
-     */
-    public GraphHopper setEnableInstructions(boolean b) {
-        ensureNotLoaded();
-        enableInstructions = b;
-        return this;
-    }
-
-    public String getPreferredLanguage() {
-        return preferredLanguage;
-    }
-
-    /**
-     * This method specifies the preferred language for way names during import.
-     * <p>
-     * Language code as defined in ISO 639-1 or ISO 639-2.
-     * <ul>
-     * <li>If no preferred language is specified, only the default language with no tag will be
-     * imported.</li>
-     * <li>If a language is specified, it will be imported if its tag is found, otherwise fall back
-     * to default language.</li>
-     * </ul>
-     */
-    public GraphHopper setPreferredLanguage(String preferredLanguage) {
-        ensureNotLoaded();
-        if (preferredLanguage == null)
-            throw new IllegalArgumentException("preferred language cannot be null");
-
-        this.preferredLanguage = preferredLanguage;
-        return this;
-    }
-
     /**
      * This methods enables gps point calculation. If disabled only distance will be calculated.
      */
@@ -547,8 +507,12 @@ public GraphHopper init(CmdArgs args) {
         removeZipped = args.getBool("graph.remove_zipped", removeZipped);
         int bytesForFlags = args.getInt("graph.bytes_for_flags", 4);
         String flagEncodersStr = args.get("graph.flag_encoders", "");
-        if (!flagEncodersStr.isEmpty())
-            setEncodingManager(EncodingManager.create(flagEncoderFactory, flagEncodersStr, bytesForFlags));
+        if (!flagEncodersStr.isEmpty()) {
+            EncodingManager.Builder emBuilder = EncodingManager.createBuilder(flagEncoderFactory, flagEncodersStr, bytesForFlags);
+            emBuilder.setEnableInstructions(args.getBool("datareader.instructions", true));
+            emBuilder.setPreferredLanguage(args.get("datareader.preferred_language", ""));
+            setEncodingManager(emBuilder.build());
+        }
 
         if (args.get("graph.locktype", "native").equals("simple"))
             lockFactory = new SimpleFSLockFactory();
@@ -609,8 +573,6 @@ public GraphHopper init(CmdArgs args) {
         dataReaderWayPointMaxDistance = args.getDouble(Routing.INIT_WAY_POINT_MAX_DISTANCE, dataReaderWayPointMaxDistance);
 
         dataReaderWorkerThreads = args.getInt("datareader.worker_threads", dataReaderWorkerThreads);
-        enableInstructions = args.getBool("datareader.instructions", enableInstructions);
-        preferredLanguage = args.get("datareader.preferred_language", preferredLanguage);
 
         // index
         preciseIndexResolution = args.getInt("index.high_resolution", preciseIndexResolution);
@@ -687,8 +649,6 @@ protected DataReader importData() throws IOException {
             throw new IllegalStateException("Couldn't load from existing folder: " + ghLocation
                     + " but also cannot use file for DataReader as it wasn't specified!");
 
-        encodingManager.setEnableInstructions(enableInstructions);
-        encodingManager.setPreferredLanguage(preferredLanguage);
         DataReader reader = createReader(ghStorage);
         logger.info("using " + ghStorage.toString() + ", memory:" + getMemInfo());
         reader.readGraph();
@@ -1074,7 +1034,7 @@ else if (ALT_ROUTE.equalsIgnoreCase(algoStr))
                 // do the actual route calculation !
                 altPaths = routingTemplate.calcPaths(queryGraph, tmpAlgoFactory, algoOpts);
 
-                boolean tmpEnableInstructions = hints.getBool(Routing.INSTRUCTIONS, enableInstructions);
+                boolean tmpEnableInstructions = hints.getBool(Routing.INSTRUCTIONS, getEncodingManager().isEnableInstructions());
                 boolean tmpCalcPoints = hints.getBool(Routing.CALC_POINTS, calcPoints);
                 double wayPointMaxDistance = hints.getDouble(Routing.WAY_POINT_MAX_DISTANCE, 1d);
 

core/src/main/java/com/graphhopper/routing/util/EncodingManager.java

@@ -73,7 +73,11 @@ public static EncodingManager create(String flagEncodersStr, int bytesForEdgeFla
     }
 
     public static EncodingManager create(FlagEncoderFactory factory, String flagEncodersStr, int bytesForEdgeFlags) {
-        return create(parseEncoderString(factory, flagEncodersStr), bytesForEdgeFlags);
+        return createBuilder(factory, flagEncodersStr, bytesForEdgeFlags).build();
+    }
+
+    public static EncodingManager.Builder createBuilder(FlagEncoderFactory factory, String flagEncodersStr, int bytesForEdgeFlags) {
+        return createBuilder(parseEncoderString(factory, flagEncodersStr), bytesForEdgeFlags);
     }
 
     /**
@@ -95,11 +99,15 @@ public static EncodingManager create(List<? extends FlagEncoder> flagEncoders) {
     }
 
     public static EncodingManager create(List<? extends FlagEncoder> flagEncoders, int bytesForEdgeFlags) {
+        return createBuilder(flagEncoders, bytesForEdgeFlags).build();
+    }
+
+    private static EncodingManager.Builder createBuilder(List<? extends FlagEncoder> flagEncoders, int bytesForEdgeFlags) {
         Builder builder = new Builder(bytesForEdgeFlags);
         for (FlagEncoder flagEncoder : flagEncoders) {
             builder.add(flagEncoder);
         }
-        return builder.build();
+        return builder;
     }
 
     /**
@@ -126,7 +134,7 @@ public static EncodingManager create(FlagEncoderFactory factory, String ghLoc) {
             bytesForFlags = Integer.parseInt(properties.get("graph.bytes_for_flags"));
         } catch (NumberFormatException ex) {
         }
-        return create(factory, acceptStr, bytesForFlags);
+        return createBuilder(factory, acceptStr, bytesForFlags).build();
     }
 
     /**
@@ -145,8 +153,7 @@ private EncodingManager(int bytes) {
     }
 
     public static class Builder {
-        private final EncodingManager em;
-        private boolean buildCalled = false;
+        private EncodingManager em;
 
         public Builder(int bytes) {
             em = new EncodingManager(bytes);
@@ -164,6 +171,33 @@ public IntsRef handleWayTags(IntsRef edgeFlags, ReaderWay way, Access access, lo
             });
         }
 
+        /**
+         * This method specifies the preferred language for way names during import.
+         * <p>
+         * Language code as defined in ISO 639-1 or ISO 639-2.
+         * <ul>
+         * <li>If no preferred language is specified, only the default language with no tag will be
+         * imported.</li>
+         * <li>If a language is specified, it will be imported if its tag is found, otherwise fall back
+         * to default language.</li>
+         * </ul>
+         */
+        public Builder setPreferredLanguage(String language) {
+            check();
+            em.setPreferredLanguage(language);
+            return this;
+        }
+
+        /**
+         * This method specifies if the import should include way names to be able to return
+         * instructions for a route.
+         */
+        public Builder setEnableInstructions(boolean enable) {
+            check();
+            em.setEnableInstructions(enable);
+            return this;
+        }
+
         /**
          * For backward compatibility provide a way to add multiple FlagEncoders
          */
@@ -175,11 +209,13 @@ public Builder addAll(FlagEncoderFactory factory, String flagEncodersStr) {
         }
 
         public Builder add(FlagEncoder encoder) {
+            check();
             em.addEncoder((AbstractFlagEncoder) encoder);
             return this;
         }
 
         public Builder add(EncodedValue encodedValue) {
+            check();
             em.addEncodedValue(encodedValue);
             return this;
         }
@@ -190,14 +226,19 @@ public Builder put(EncodedValue encodedValue, TagParser tagParser) {
             return this;
         }
 
+        private void check() {
+            if (em == null)
+                throw new IllegalStateException("Cannot call method after Builder.build() was called");
+        }
+
         public EncodingManager build() {
-            if (buildCalled)
-                throw new IllegalStateException("Cannot call Builder.build() twice");
+            check();
             if (em.encodedValueMap.isEmpty())
                 throw new IllegalStateException("No EncodedValues found");
 
-            buildCalled = true;
-            return em;
+            EncodingManager tmp = em;
+            em = null;
+            return tmp;
         }
     }
 
@@ -244,6 +285,25 @@ public int getBytesForFlags() {
         return bitsForEdgeFlags / 8;
     }
 
+    private void setEnableInstructions(boolean enableInstructions) {
+        this.enableInstructions = enableInstructions;
+    }
+
+    public boolean isEnableInstructions() {
+        return enableInstructions;
+    }
+
+    private void setPreferredLanguage(String preferredLanguage) {
+        if (preferredLanguage == null)
+            throw new IllegalArgumentException("preferred language cannot be null");
+
+        this.preferredLanguage = preferredLanguage;
+    }
+
+    public String getPreferredLanguage() {
+        return preferredLanguage;
+    }
+
     private void addEncoder(AbstractFlagEncoder encoder) {
         if (encoder.isRegistered())
             throw new IllegalStateException("You must not register a FlagEncoder (" + encoder.toString() + ") twice!");
@@ -498,19 +558,6 @@ public long handleNodeTags(ReaderNode node) {
         return flags;
     }
 
-    public EncodingManager setEnableInstructions(boolean enableInstructions) {
-        this.enableInstructions = enableInstructions;
-        return this;
-    }
-
-    public EncodingManager setPreferredLanguage(String preferredLanguage) {
-        if (preferredLanguage == null)
-            throw new IllegalArgumentException("preferred language cannot be null");
-
-        this.preferredLanguage = preferredLanguage;
-        return this;
-    }
-
     public void applyWayTags(ReaderWay way, EdgeIteratorState edge) {
         // storing the road name does not yet depend on the flagEncoder so manage it directly
         if (enableInstructions) {

core/src/test/java/com/graphhopper/reader/dem/AbstractEdgeElevationInterpolatorTest.java

@@ -17,11 +17,6 @@
  */
 package com.graphhopper.reader.dem;
 
-import java.util.Arrays;
-
-import org.junit.After;
-import org.junit.Before;
-
 import com.graphhopper.coll.GHBitSetImpl;
 import com.graphhopper.coll.GHIntHashSet;
 import com.graphhopper.reader.ReaderWay;
@@ -33,6 +28,10 @@
 import com.graphhopper.storage.RAMDirectory;
 import com.graphhopper.util.EdgeIteratorState;
 import com.graphhopper.util.Helper;
+import org.junit.After;
+import org.junit.Before;
+
+import java.util.Arrays;
 
 /**
  * @author Alexey Valikov
@@ -52,8 +51,7 @@ public abstract class AbstractEdgeElevationInterpolatorTest {
     public void setUp() {
         dataFlagEncoder = new DataFlagEncoder();
         graph = new GraphHopperStorage(new RAMDirectory(),
-                EncodingManager.create(Arrays.asList(dataFlagEncoder, new FootFlagEncoder()),
-                        8),
+                EncodingManager.create(Arrays.asList(dataFlagEncoder, new FootFlagEncoder()), 8),
                 true, new GraphExtension.NoOpExtension()).create(100);
 
         edgeElevationInterpolator = createEdgeElevationInterpolator();

reader-osm/src/test/java/com/graphhopper/reader/osm/GraphHopperOSMTest.java

@@ -306,14 +306,14 @@ public void testSortedGraph_noCH() {
         assertEquals(new GHPoint(51.199999850988384, 9.39999970197677), rsp.getPoints().toGHPoint(2));
 
         GHRequest req = new GHRequest(51.2492152, 9.4317166, 51.2, 9.4);
-        boolean old = instance.isEnableInstructions();
+        boolean old = instance.getEncodingManager().isEnableInstructions();
         req.getHints().put("instructions", true);
         instance.route(req);
-        assertEquals(old, instance.isEnableInstructions());
+        assertEquals(old, instance.getEncodingManager().isEnableInstructions());
 
         req.getHints().put("instructions", false);
         instance.route(req);
-        assertEquals("route method should not change instance field", old, instance.isEnableInstructions());
+        assertEquals("route method should not change instance field", old, instance.getEncodingManager().isEnableInstructions());
     }
 
     @Test

reader-osm/src/test/java/com/graphhopper/reader/osm/OSMReaderTest.java

@@ -495,7 +495,7 @@ public void testRelation() {
 
     @Test
     public void testTurnRestrictions() {
-        GraphHopper hopper = new GraphHopperFacade(fileTurnRestrictions, true).
+        GraphHopper hopper = new GraphHopperFacade(fileTurnRestrictions, true, "").
                 importOrLoad();
 
         Graph graph = hopper.getGraphHopperStorage();
@@ -796,14 +796,14 @@ public Collection<OSMTurnRelation.TurnCostTableEntry> analyzeTurnRelation(FlagEn
 
     @Test
     public void testPreferredLanguage() {
-        GraphHopper hopper = new GraphHopperFacade(file1).setPreferredLanguage("de").importOrLoad();
+        GraphHopper hopper = new GraphHopperFacade(file1, false, "de").importOrLoad();
         GraphHopperStorage graph = hopper.getGraphHopperStorage();
         int n20 = AbstractGraphStorageTester.getIdOf(graph, 52);
         EdgeIterator iter = carOutExplorer.setBaseNode(n20);
         assertTrue(iter.next());
         assertEquals("straße 123, B 122", iter.getName());
 
-        hopper = new GraphHopperFacade(file1).setPreferredLanguage("el").importOrLoad();
+        hopper = new GraphHopperFacade(file1, false, "el").importOrLoad();
         graph = hopper.getGraphHopperStorage();
         n20 = AbstractGraphStorageTester.getIdOf(graph, 52);
         iter = carOutExplorer.setBaseNode(n20);
@@ -854,10 +854,10 @@ public void testRoutingRequestFails_issue665() {
 
     class GraphHopperFacade extends GraphHopperOSM {
         public GraphHopperFacade(String osmFile) {
-            this(osmFile, false);
+            this(osmFile, false, "");
         }
 
-        public GraphHopperFacade(String osmFile, boolean turnCosts) {
+        public GraphHopperFacade(String osmFile, boolean turnCosts, String prefLang) {
             setStoreOnFlush(false);
             setOSMFile(osmFile);
             setGraphHopperLocation(dir);
@@ -872,8 +872,8 @@ public GraphHopperFacade(String osmFile, boolean turnCosts) {
             }
 
             footEncoder = new FootFlagEncoder();
-
-            setEncodingManager(EncodingManager.create(footEncoder, carEncoder, bikeEncoder));
+            setEncodingManager(new EncodingManager.Builder(4).add(footEncoder).add(carEncoder).add(bikeEncoder).
+                    setPreferredLanguage(prefLang).build());
             carAccessEnc = carEncoder.getAccessEnc();
         }
 
@@ -884,7 +884,6 @@ protected DataReader createReader(GraphHopperStorage tmpGraph) {
 
         @Override
         protected DataReader importData() throws IOException {
-            getEncodingManager().setPreferredLanguage(getPreferredLanguage());
             GraphHopperStorage tmpGraph = newGraph(dir, getEncodingManager(), hasElevation(),
                     getEncodingManager().needsTurnCostsSupport());
             setGraphHopperStorage(tmpGraph);