diff --git a/core/src/main/java/com/graphhopper/routing/weighting/AbstractWeighting.java b/core/src/main/java/com/graphhopper/routing/weighting/AbstractWeighting.java index 9479207f3b5..573176d5b91 100644 --- a/core/src/main/java/com/graphhopper/routing/weighting/AbstractWeighting.java +++ b/core/src/main/java/com/graphhopper/routing/weighting/AbstractWeighting.java @@ -59,7 +59,6 @@ protected AbstractWeighting(FlagEncoder encoder, TurnCostProvider turnCostProvid */ public abstract double calcEdgeWeight(EdgeIteratorState edgeState, boolean reverse); - @Override public long calcEdgeMillis(EdgeIteratorState edgeState, boolean reverse) { // special case for loop edges: since they do not have a meaningful direction we always need to read them in diff --git a/core/src/main/java/com/graphhopper/storage/CHGraph.java b/core/src/main/java/com/graphhopper/storage/CHGraph.java index fd5d82df387..edbf3863156 100644 --- a/core/src/main/java/com/graphhopper/storage/CHGraph.java +++ b/core/src/main/java/com/graphhopper/storage/CHGraph.java @@ -83,7 +83,6 @@ public interface CHGraph extends Graph { @Override AllCHEdgesIterator getAllEdges(); - void disconnectEdge(int edge, int adjNode, int prevEdge); /** diff --git a/core/src/main/java/com/graphhopper/storage/CHProfile.java b/core/src/main/java/com/graphhopper/storage/CHProfile.java index 5e8a359242d..2405973a3f4 100644 --- a/core/src/main/java/com/graphhopper/storage/CHProfile.java +++ b/core/src/main/java/com/graphhopper/storage/CHProfile.java @@ -4,7 +4,6 @@ import com.graphhopper.routing.weighting.AbstractWeighting; import com.graphhopper.routing.weighting.Weighting; -import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -15,22 +14,35 @@ * @author easbar */ public class CHProfile { + private final String profileName; private final Weighting weighting; private final boolean edgeBased; public static CHProfile nodeBased(Weighting weighting) { - return new CHProfile(weighting, TraversalMode.NODE_BASED); + return nodeBased(defaultName(weighting, false), weighting); + } + + public static CHProfile nodeBased(String profileName, Weighting weighting) { + return new CHProfile(profileName, weighting, false); } public static CHProfile edgeBased(Weighting weighting) { - return new CHProfile(weighting, TraversalMode.EDGE_BASED); + return edgeBased(defaultName(weighting, true), weighting); } - public CHProfile(Weighting weighting, TraversalMode traversalMode) { - this(weighting, traversalMode.isEdgeBased()); + public static CHProfile edgeBased(String profileName, Weighting weighting) { + return new CHProfile(profileName, weighting, true); } public CHProfile(Weighting weighting, boolean edgeBased) { + this(defaultName(weighting, edgeBased), weighting, edgeBased); + } + + public CHProfile(String profileName, Weighting weighting, boolean edgeBased) { + if (!profileName.matches("^[a-z0-9_\\-]*$")) { + throw new IllegalArgumentException("CH profile names may only contain lower case letters, numbers, underscores and dashs, given: " + profileName); + } + this.profileName = profileName; this.weighting = weighting; this.edgeBased = edgeBased; } @@ -48,20 +60,7 @@ public TraversalMode getTraversalMode() { } public String toFileName() { - String result = AbstractWeighting.weightingToFileName(weighting); - // keeping legacy file names for now, like fastest_edge_utc40 (instead of fastest_40_edge), because we will - // most likely use profile names soon: #1708 - Pattern pattern = Pattern.compile("-?\\d+"); - Matcher matcher = pattern.matcher(result); - if (matcher.find()) { - String turnCostPostfix = matcher.group(); - result = matcher.replaceAll(""); - result += edgeBased ? "edge" : "node"; - result += "_utc" + turnCostPostfix; - } else { - result += edgeBased ? "_edge" : "_node"; - } - return result; + return profileName; } public String toString() { @@ -78,17 +77,36 @@ public String toString() { return result; } + public String getName() { + return profileName; + } + @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; CHProfile chProfile = (CHProfile) o; - return edgeBased == chProfile.edgeBased && - Objects.equals(weighting, chProfile.weighting); + return getName().equals(chProfile.getName()); } @Override public int hashCode() { - return Objects.hash(weighting, edgeBased); + return getName().hashCode(); + } + + private static String defaultName(Weighting weighting, boolean edgeBased) { + String result = AbstractWeighting.weightingToFileName(weighting); + // this is how we traditionally named the files, something like 'fastest_edge_utc40' + Pattern pattern = Pattern.compile("-?\\d+"); + Matcher matcher = pattern.matcher(result); + if (matcher.find()) { + String turnCostPostfix = matcher.group(); + result = matcher.replaceAll(""); + result += edgeBased ? "edge" : "node"; + result += "_utc" + turnCostPostfix; + } else { + result += edgeBased ? "_edge" : "_node"; + } + return result; } } diff --git a/core/src/main/java/com/graphhopper/storage/GraphHopperStorage.java b/core/src/main/java/com/graphhopper/storage/GraphHopperStorage.java index fb1ca951e9b..31bd3435cd5 100644 --- a/core/src/main/java/com/graphhopper/storage/GraphHopperStorage.java +++ b/core/src/main/java/com/graphhopper/storage/GraphHopperStorage.java @@ -117,24 +117,25 @@ public CHGraph getCHGraph() { } } + public CHGraph getCHGraph(CHProfile chProfile) { + return getCHGraph(chProfile.getName()); + } + /** - * @return the {@link CHGraph} for the specified {@link CHProfile} + * @return the {@link CHGraph} for the specified profile name */ - public CHGraph getCHGraph(CHProfile profile) { + public CHGraph getCHGraph(String profileName) { if (chGraphs.isEmpty()) throw new IllegalStateException("There is no CHGraph"); - if (profile == null) - throw new IllegalStateException("Cannot find CHGraph with null CHProfile"); - - List existing = new ArrayList<>(); + List existing = new ArrayList<>(); for (CHGraphImpl cg : chGraphs) { - if (cg.getCHProfile().equals(profile)) + if (cg.getCHProfile().getName().equals(profileName)) return cg; - existing.add(cg.getCHProfile()); + existing.add(cg.getCHProfile().getName()); } - throw new IllegalStateException("Cannot find CHGraph for the specified profile: " + profile + ", existing:" + existing); + throw new IllegalStateException("Cannot find CHGraph for the specified profile: " + profileName + ", existing:" + existing); } public boolean isCHPossible() { diff --git a/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmTest.java b/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmTest.java index 39b27e94257..9f1f0a92609 100644 --- a/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmTest.java +++ b/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmTest.java @@ -1003,7 +1003,7 @@ private GraphHopperStorage createGHStorage() { private GraphHopperStorage createGHStorage(boolean is3D, Weighting... weightings) { CHProfile[] chProfiles = new CHProfile[weightings.length]; for (int i = 0; i < weightings.length; i++) { - chProfiles[i] = new CHProfile(weightings[i], traversalMode); + chProfiles[i] = new CHProfile(weightings[i], traversalMode.isEdgeBased()); } return new GraphBuilder(encodingManager).set3D(is3D) .setCHProfiles(chProfiles) @@ -1156,7 +1156,7 @@ public String toString() { private static abstract class CHCalculator implements PathCalculator { @Override public Path calcPath(GraphHopperStorage graph, Weighting weighting, TraversalMode traversalMode, int maxVisitedNodes, int from, int to) { - CHProfile chProfile = new CHProfile(weighting, traversalMode); + CHProfile chProfile = new CHProfile(weighting, traversalMode.isEdgeBased()); PrepareContractionHierarchies pch = PrepareContractionHierarchies.fromGraphHopperStorage(graph, chProfile); CHGraph chGraph = graph.getCHGraph(chProfile); if (chGraph.getEdges() == chGraph.getOriginalEdges()) { @@ -1182,7 +1182,7 @@ public Path calcPath(GraphHopperStorage graph, Weighting weighting, TraversalMod @Override public Path calcPath(GraphHopperStorage graph, Weighting weighting, TraversalMode traversalMode, int maxVisitedNodes, QueryResult from, QueryResult to) { - CHProfile chProfile = new CHProfile(weighting, traversalMode); + CHProfile chProfile = new CHProfile(weighting, traversalMode.isEdgeBased()); PrepareContractionHierarchies pch = PrepareContractionHierarchies.fromGraphHopperStorage(graph, chProfile); CHGraph chGraph = graph.getCHGraph(chProfile); if (chGraph.getEdges() == chGraph.getOriginalEdges()) { diff --git a/core/src/test/java/com/graphhopper/storage/CHProfileTest.java b/core/src/test/java/com/graphhopper/storage/CHProfileTest.java index 3e32fb9f068..46b285da972 100644 --- a/core/src/test/java/com/graphhopper/storage/CHProfileTest.java +++ b/core/src/test/java/com/graphhopper/storage/CHProfileTest.java @@ -20,10 +20,12 @@ public void filename() { EncodingManager.create(encoder); TurnCostStorage tcs = new TurnCostStorage(null, null); assertEquals("fastest_car_edge_utc30", CHProfile.edgeBased(new FastestWeighting(encoder, new DefaultTurnCostProvider(encoder, tcs, 30))).toFileName()); + assertEquals("my_profile_name", CHProfile.edgeBased("my_profile_name", new FastestWeighting(encoder, new DefaultTurnCostProvider(encoder, tcs, 30))).toFileName()); assertEquals("shortest_car_edge_utc-1", CHProfile.edgeBased(new ShortestWeighting(encoder, new DefaultTurnCostProvider(encoder, tcs, INFINITE_U_TURN_COSTS))).toFileName()); assertEquals("shortest_car_edge", CHProfile.edgeBased(new ShortestWeighting(encoder, NO_TURN_COST_PROVIDER)).toFileName()); assertEquals("fastest_car_node", CHProfile.nodeBased(new FastestWeighting(encoder)).toFileName()); assertEquals("short_fastest_car_node", CHProfile.nodeBased(new ShortFastestWeighting(encoder, 0.1)).toFileName()); + assertEquals("your_profile_name", CHProfile.nodeBased("your_profile_name", new ShortFastestWeighting(encoder, 0.1)).toFileName()); } } \ No newline at end of file diff --git a/core/src/test/java/com/graphhopper/storage/GraphHopperStorageCHTest.java b/core/src/test/java/com/graphhopper/storage/GraphHopperStorageCHTest.java index d2ff0ddba07..7a09912016c 100644 --- a/core/src/test/java/com/graphhopper/storage/GraphHopperStorageCHTest.java +++ b/core/src/test/java/com/graphhopper/storage/GraphHopperStorageCHTest.java @@ -633,6 +633,43 @@ public void testLoadingWithLessWeightings_nodeAndEdge_works() { mixedStorage.flush(); } + @Test + public void testCHProfilesWithDifferentNames() { + FastestWeighting weighting = new FastestWeighting(carEncoder); + // creating multiple profiles with the same name is an error + { + try { + new GraphBuilder(encodingManager) + .setCHProfiles( + CHProfile.nodeBased("a", weighting), + CHProfile.nodeBased("b", weighting), + CHProfile.nodeBased("a", weighting) + ) + .create(); + fail("creating mulitple profiles with the same name should be an error"); + } catch (Exception e) { + assertTrue("unexpected error: " + e.getMessage(), e.getMessage().contains("a CHGraph already exists")); + } + } + // ... but using multiple profiles with different names is fine even when their properties/weighting are the same + { + GraphHopperStorage storage = new GraphBuilder(encodingManager) + .setCHProfiles( + CHProfile.nodeBased("a", weighting), + CHProfile.nodeBased("b", weighting), + CHProfile.nodeBased("c", weighting) + ) + .create(); + assertSame(storage.getCHGraph("a"), storage.getCHGraph("a")); + assertNotNull(storage.getCHGraph("a")); + assertNotNull(storage.getCHGraph("b")); + assertNotNull(storage.getCHGraph("c")); + assertNotSame(storage.getCHGraph("a"), storage.getCHGraph("b")); + assertNotSame(storage.getCHGraph("b"), storage.getCHGraph("c")); + assertNotSame(storage.getCHGraph("a"), storage.getCHGraph("c")); + } + } + private GraphHopperStorage createStorageWithWeightings(String... profileStrings) { return new GraphBuilder(encodingManager) .setCHProfileStrings(profileStrings)