Skip to content

Commit 90dfc37

Browse files
easbarkarussell
authored andcommitted
Allows using less ch weightings than loaded, fixes graphhopper#1413. (graphhopper#1488)
* Allows using less ch weightings than loaded, fixes graphhopper#1413. * Fixes test (do not allow loading CH graph without CH weightings).
1 parent 77aedb5 commit 90dfc37

File tree

3 files changed

+91
-13
lines changed

3 files changed

+91
-13
lines changed

core/src/main/java/com/graphhopper/storage/GraphHopperStorage.java

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,7 @@
2525
import com.graphhopper.util.EdgeIteratorState;
2626
import com.graphhopper.util.shapes.BBox;
2727

28-
import java.util.ArrayList;
29-
import java.util.Collection;
30-
import java.util.Collections;
31-
import java.util.List;
28+
import java.util.*;
3229

3330
/**
3431
* This class manages all storage related methods and delegates the calls to the associated graphs.
@@ -248,10 +245,7 @@ public boolean loadExisting() {
248245
String dim = properties.get("graph.dimension");
249246
baseGraph.loadExisting(dim);
250247

251-
String loadedCHWeightings = properties.get("graph.ch.weightings");
252-
String configuredCHWeightings = getCHWeightings().toString();
253-
if (!loadedCHWeightings.equals(configuredCHWeightings))
254-
throw new IllegalStateException("Configured graph.ch.weightings: " + configuredCHWeightings + " is not equal to loaded " + loadedCHWeightings);
248+
checkIfConfiguredAndLoadedWeightingsCompatible();
255249

256250
for (CHGraphImpl cg : chGraphs) {
257251
if (!cg.loadExisting())
@@ -263,6 +257,37 @@ public boolean loadExisting() {
263257
return false;
264258
}
265259

260+
private void checkIfConfiguredAndLoadedWeightingsCompatible() {
261+
String loadedStr = properties.get("graph.ch.weightings");
262+
List<String> loaded = parseList(loadedStr);
263+
List<Weighting> configured = getCHWeightings();
264+
if (configured.isEmpty() && !loaded.isEmpty()) {
265+
throw new IllegalStateException("You loaded a CH graph, but you did not specify graph.ch.weightings");
266+
}
267+
for (Weighting w : configured) {
268+
if (!loaded.contains(w.toString())) {
269+
throw new IllegalStateException("Configured weighting: " + w.toString() + " is not contained in loaded weightings " + loadedStr + ".\n" +
270+
"You configured graph.ch.weightings: " + configured);
271+
}
272+
}
273+
}
274+
275+
/**
276+
* parses a string like [a,b,c]
277+
*/
278+
private List<String> parseList(String listStr) {
279+
String trimmed = listStr.trim();
280+
String[] items = trimmed.substring(1, trimmed.length() - 1).split(",");
281+
List<String> result = new ArrayList<>();
282+
for (String item : items) {
283+
String s = item.trim();
284+
if (!s.isEmpty()) {
285+
result.add(s);
286+
}
287+
}
288+
return result;
289+
}
290+
266291
@Override
267292
public void flush() {
268293
for (CHGraphImpl cg : chGraphs) {

core/src/test/java/com/graphhopper/storage/GraphHopperStorageCHTest.java

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.graphhopper.routing.ch.PrepareEncoder;
2222
import com.graphhopper.routing.util.*;
2323
import com.graphhopper.routing.weighting.FastestWeighting;
24+
import com.graphhopper.routing.weighting.ShortestWeighting;
2425
import com.graphhopper.routing.weighting.Weighting;
2526
import com.graphhopper.storage.index.QueryResult;
2627
import com.graphhopper.util.*;
@@ -252,7 +253,7 @@ public void testQueryGraph() {
252253
EdgeExplorer explorer = baseGraph.createEdgeExplorer();
253254

254255
assertTrue(chGraph.getNodes() < qGraph.getNodes());
255-
assertTrue(baseGraph.getNodes() == qGraph.getNodes());
256+
assertEquals(baseGraph.getNodes(), qGraph.getNodes());
256257

257258
// traverse virtual edges and normal edges but no shortcuts!
258259
assertEquals(GHUtility.asSet(fromRes.getClosestNode()), GHUtility.getNeighbors(explorer.setBaseNode(0)));
@@ -414,7 +415,7 @@ public void testShortcutCreationAndAccessForManyVehicles() {
414415
// throw exception for wrong encoder
415416
try {
416417
assertFalse(carCHGraph.getEdgeIteratorState(carSC02.getEdge(), 2).isForward(tmpBike));
417-
assertTrue(false);
418+
fail();
418419
} catch (AssertionError ex) {
419420
}
420421

@@ -425,8 +426,60 @@ public void testShortcutCreationAndAccessForManyVehicles() {
425426
// throw exception for wrong encoder
426427
try {
427428
assertFalse(bikeCHGraph.getEdgeIteratorState(bikeSC02.getEdge(), 2).isBackward(tmpCar));
428-
assertTrue(false);
429+
fail();
429430
} catch (AssertionError ex) {
430431
}
431432
}
433+
434+
@Test(expected = IllegalStateException.class)
435+
public void testLoadingWithWrongWeighting_throws() {
436+
// we start with one weighting
437+
GraphHopperStorage ghStorage = newGHStorage(new GHDirectory(defaultGraphLoc, DAType.RAM_STORE), false);
438+
ghStorage.create(defaultSize);
439+
ghStorage.flush();
440+
441+
// but then configure another weighting and try to load the graph from disk -> error
442+
GraphHopperStorage newGHStorage = createStorageWithWeightings(new ShortestWeighting(carEncoder));
443+
newGHStorage.loadExisting();
444+
}
445+
446+
@Test(expected = IllegalStateException.class)
447+
public void testLoadingWithExtraWeighting_throws() {
448+
// we start with one weighting
449+
GraphHopperStorage ghStorage = newGHStorage(new GHDirectory(defaultGraphLoc, DAType.RAM_STORE), false);
450+
ghStorage.create(defaultSize);
451+
ghStorage.flush();
452+
453+
// but then add an additional weighting and try to load the graph from disk -> error
454+
GraphHopperStorage newGHStorage = createStorageWithWeightings(
455+
new FastestWeighting(carEncoder), new ShortestWeighting(carEncoder));
456+
newGHStorage.loadExisting();
457+
}
458+
459+
@Test
460+
public void testLoadingWithLessWeightings_works() {
461+
// we start with a gh storage with two ch weightings and flush it to disk
462+
FastestWeighting weighting1 = new FastestWeighting(carEncoder);
463+
ShortestWeighting weighting2 = new ShortestWeighting(carEncoder);
464+
GraphHopperStorage originalStorage = createStorageWithWeightings(weighting1, weighting2);
465+
originalStorage.create(defaultSize);
466+
originalStorage.flush();
467+
468+
// now we create a new storage but only use one of the weightings, which should be ok
469+
GraphHopperStorage smallStorage = createStorageWithWeightings(weighting1);
470+
smallStorage.loadExisting();
471+
assertEquals(1, smallStorage.getCHWeightings().size());
472+
smallStorage.flush();
473+
474+
// now we create yet another storage that uses both weightings again, which still works
475+
GraphHopperStorage fullStorage = createStorageWithWeightings(weighting1, weighting2);
476+
fullStorage.loadExisting();
477+
assertEquals(2, fullStorage.getCHWeightings().size());
478+
fullStorage.flush();
479+
}
480+
481+
private GraphHopperStorage createStorageWithWeightings(Weighting... weightings) {
482+
return new GraphHopperStorage(Arrays.asList(weightings), new GHDirectory(defaultGraphLoc, DAType.RAM_STORE),
483+
encodingManager, false, new GraphExtension.NoOpExtension());
484+
}
432485
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public void testLoadingWithDifferentCHConfig_issue471() {
168168
gh.load(ghLoc);
169169
fail();
170170
} catch (Exception ex) {
171-
assertTrue(ex.getMessage(), ex.getMessage().startsWith("Configured graph.ch.weightings:"));
171+
assertTrue(ex.getMessage(), ex.getMessage().startsWith("You loaded a CH graph, but you did not specify graph.ch.weightings"));
172172
}
173173

174174
Helper.removeDir(new File(ghLoc));
@@ -190,7 +190,7 @@ public void testLoadingWithDifferentCHConfig_issue471() {
190190
gh.load(ghLoc);
191191
fail();
192192
} catch (Exception ex) {
193-
assertTrue(ex.getMessage(), ex.getMessage().startsWith("Configured graph.ch.weightings:"));
193+
assertTrue(ex.getMessage(), ex.getMessage().contains("is not contained in loaded weightings"));
194194
}
195195
}
196196

0 commit comments

Comments
 (0)