properly close the getAllTiles iterator

onthegomap · Jan 6, 2024 · f73fa1f · f73fa1f
1 parent d749f8f
commit f73fa1f
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 12 deletions.
diff --git a/planetiler-core/src/main/java/com/onthegomap/planetiler/stream/ReadableStreamArchive.java b/planetiler-core/src/main/java/com/onthegomap/planetiler/stream/ReadableStreamArchive.java
@@ -26,19 +26,27 @@ abstract class ReadableStreamArchive<E> implements ReadableTileArchive {
 
   @Override
   public final byte[] getTile(TileCoord coord) {
-    return getAllTiles().stream().filter(c -> c.coord().equals(coord)).map(Tile::bytes).findFirst().orElse(null);
+    try (var tiles = getAllTiles(); var s = tiles.stream()) {
+      return s.filter(c -> c.coord().equals(coord)).map(Tile::bytes).findFirst().orElse(null);
+    }
   }
 
   @Override
   public final byte[] getTile(int x, int y, int z) {
     return getTile(TileCoord.ofXYZ(x, y, z));
   }
 
+  /**
+   * Callers MUST make sure to close the iterator/derived stream!
+   */
   @Override
   public final CloseableIterator<TileCoord> getAllTileCoords() {
     return getAllTiles().map(Tile::coord);
   }
 
+  /**
+   * Callers MUST make sure to close the iterator/derived stream!
+   */
   @Override
   public final CloseableIterator<Tile> getAllTiles() {
     return createIterator()
@@ -53,8 +61,8 @@ public final TileArchiveMetadata metadata() {
   }
 
   private TileArchiveMetadata loadMetadata() {
-    try (var it = createIterator()) {
-      return it.stream().map(this::mapEntryToMetadata).flatMap(Optional::stream).findFirst().orElse(null);
+    try (var i = createIterator(); var s = i.stream()) {
+      return s.map(this::mapEntryToMetadata).flatMap(Optional::stream).findFirst().orElse(null);
     }
   }
 

diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/TestUtils.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/TestUtils.java
@@ -294,7 +294,9 @@ public static Geometry decodeSilently(VectorTile.VectorGeometry geom) {
   }
 
   public static Set<Tile> getTiles(ReadableTileArchive db) {
-    return db.getAllTiles().stream().collect(Collectors.toSet());
+    try (var t = db.getAllTiles(); var s = t.stream()) {
+      return s.collect(Collectors.toUnmodifiableSet());
+    }
   }
 
   public static int getTilesDataCount(Mbtiles db) throws SQLException {

diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/files/ReadableFilesArchiveTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/files/ReadableFilesArchiveTest.java
@@ -49,8 +49,12 @@ void testRead(@TempDir Path tempDir) throws IOException {
       Files.write(files.get(i), new byte[]{(byte) i});
     }
 
-    try (var reader = ReadableFilesArchive.newReader(tilesDir, Arguments.of())) {
-      final List<Tile> tiles = reader.getAllTiles().stream().sorted().toList();
+    try (
+      var reader = ReadableFilesArchive.newReader(tilesDir, Arguments.of());
+      var t = reader.getAllTiles();
+      var s = t.stream()
+    ) {
+      final List<Tile> tiles = s.sorted().toList();
       assertEquals(
         List.of(
           new Tile(TileCoord.ofXYZ(0, 0, 0), new byte[]{0}),

diff --git a/...ler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableCsvStreamArchiveTest.java b/...ler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableCsvStreamArchiveTest.java
@@ -63,8 +63,12 @@ void testSimple(String columnSeparator, String lineSeparator, boolean pad, CsvBi
     );
 
     try (var reader = ReadableCsvArchive.newReader(TileArchiveConfig.Format.CSV, csvFile, config)) {
-      assertEquals(expectedTiles, reader.getAllTiles().stream().toList());
-      assertEquals(expectedTiles, reader.getAllTiles().stream().toList());
+      try (var s = reader.getAllTiles().stream()) {
+        assertEquals(expectedTiles, s.toList());
+      }
+      try (var s = reader.getAllTiles().stream()) {
+        assertEquals(expectedTiles, s.toList());
+      }
       assertNull(reader.metadata());
       assertNull(reader.metadata());
       assertArrayEquals(expectedTiles.get(1).bytes(), reader.getTile(TileCoord.ofXYZ(1, 2, 3)));

diff --git a/...er-core/src/test/java/com/onthegomap/planetiler/stream/ReadableJsonStreamArchiveTest.java b/...er-core/src/test/java/com/onthegomap/planetiler/stream/ReadableJsonStreamArchiveTest.java
@@ -36,8 +36,12 @@ void testSimple(@TempDir Path tempDir) throws IOException {
       new Tile(TileCoord.ofXYZ(1, 2, 3), new byte[]{1})
     );
     try (var reader = ReadableJsonStreamArchive.newReader(jsonFile, config)) {
-      assertEquals(expectedTiles, reader.getAllTiles().stream().toList());
-      assertEquals(expectedTiles, reader.getAllTiles().stream().toList());
+      try (var s = reader.getAllTiles().stream()) {
+        assertEquals(expectedTiles, s.toList());
+      }
+      try (var s = reader.getAllTiles().stream()) {
+        assertEquals(expectedTiles, s.toList());
+      }
       assertEquals(TestUtils.MAX_METADATA_DESERIALIZED, reader.metadata());
       assertEquals(TestUtils.MAX_METADATA_DESERIALIZED, reader.metadata());
       assertArrayEquals(expectedTiles.get(1).bytes(), reader.getTile(TileCoord.ofXYZ(1, 2, 3)));

diff --git a/...r-core/src/test/java/com/onthegomap/planetiler/stream/ReadableProtoStreamArchiveTest.java b/...r-core/src/test/java/com/onthegomap/planetiler/stream/ReadableProtoStreamArchiveTest.java
@@ -52,8 +52,13 @@ void testSimple(boolean maxMetaData, @TempDir Path tempDir) throws IOException {
       new Tile(TileCoord.ofXYZ(1, 2, 3), new byte[]{1})
     );
     try (var reader = ReadableProtoStreamArchive.newReader(p, new StreamArchiveConfig(false, Arguments.of()))) {
-      assertEquals(expectedTiles, reader.getAllTiles().stream().toList());
-      assertEquals(expectedTiles, reader.getAllTiles().stream().toList());
+      try (var s = reader.getAllTiles().stream()) {
+        assertEquals(expectedTiles, s.toList());
+      }
+      try (var s = reader.getAllTiles().stream()) {
+        assertEquals(expectedTiles, s.toList());
+      }
+      assertEquals(metadataDeserialized, reader.metadata());
       assertEquals(metadataDeserialized, reader.metadata());
     }
   }