From 19c834fb5d94260cad7b2d1768a7fb94cc24a38e Mon Sep 17 00:00:00 2001 From: bbilger Date: Sat, 6 Jan 2024 22:16:59 +0100 Subject: [PATCH] properly close the getAllTiles iterator --- .../planetiler/stream/ReadableStreamArchive.java | 14 +++++++++++--- .../java/com/onthegomap/planetiler/TestUtils.java | 4 +++- .../planetiler/files/ReadableFilesArchiveTest.java | 8 ++++++-- .../stream/ReadableCsvStreamArchiveTest.java | 8 ++++++-- .../stream/ReadableJsonStreamArchiveTest.java | 8 ++++++-- .../stream/ReadableProtoStreamArchiveTest.java | 9 +++++++-- 6 files changed, 39 insertions(+), 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 index f6f9ecff..2ea88215 100644 --- 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,7 +26,9 @@ abstract class ReadableStreamArchive 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 @@ -34,11 +36,17 @@ abstract class ReadableStreamArchive implements ReadableTileArchive { return getTile(TileCoord.ofXYZ(x, y, z)); } + /** + * Callers MUST make sure to close the iterator/derived stream! + */ @Override public final CloseableIterator getAllTileCoords() { return getAllTiles().map(Tile::coord); } + /** + * Callers MUST make sure to close the iterator/derived stream! + */ @Override public final CloseableIterator getAllTiles() { return createIterator() @@ -53,8 +61,8 @@ abstract class ReadableStreamArchive implements ReadableTileArchive { } 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 index 1d11e550..8d847228 100644 --- 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 class TestUtils { } public static Set 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 index ad9d0038..127030bf 100644 --- 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 @@ class ReadableFilesArchiveTest { Files.write(files.get(i), new byte[]{(byte) i}); } - try (var reader = ReadableFilesArchive.newReader(tilesDir, Arguments.of())) { - final List tiles = reader.getAllTiles().stream().sorted().toList(); + try ( + var reader = ReadableFilesArchive.newReader(tilesDir, Arguments.of()); + var t = reader.getAllTiles(); + var s = t.stream() + ) { + final List tiles = s.sorted().toList(); assertEquals( List.of( new Tile(TileCoord.ofXYZ(0, 0, 0), new byte[]{0}), diff --git a/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableCsvStreamArchiveTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableCsvStreamArchiveTest.java index 31258995..0f08420a 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableCsvStreamArchiveTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableCsvStreamArchiveTest.java @@ -63,8 +63,12 @@ class ReadableCsvStreamArchiveTest { ); 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/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableJsonStreamArchiveTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableJsonStreamArchiveTest.java index ab728f8e..f95145b9 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableJsonStreamArchiveTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableJsonStreamArchiveTest.java @@ -36,8 +36,12 @@ class ReadableJsonStreamArchiveTest { 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/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableProtoStreamArchiveTest.java b/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableProtoStreamArchiveTest.java index 7681f98e..6ffd685c 100644 --- a/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableProtoStreamArchiveTest.java +++ b/planetiler-core/src/test/java/com/onthegomap/planetiler/stream/ReadableProtoStreamArchiveTest.java @@ -52,8 +52,13 @@ class ReadableProtoStreamArchiveTest { 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()); } }