-
-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extract ZIP file walker out of ShapefileReader. #423
Conversation
https://github.com/onthegomap/planetiler/actions/runs/3813787964 ℹ️ Base Logs 71d0450
ℹ️ This Branch Logs 3b32a99
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall! One comment about extracting the glob stuff to FileUtils so we can have a test to show what it's doing.
@@ -752,6 +764,50 @@ private void ensureInputFilesExist() { | |||
} | |||
} | |||
|
|||
// Helper for globMatchPath which will not recurse into zip files. | |||
private List<Path> globMatchPath(Path basePath, String pattern) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: instead of inline comment, maybe rename this to something to indicate it doesn't recurse into nested zips?
planetiler-core/src/main/java/com/onthegomap/planetiler/Planetiler.java
Outdated
Show resolved
Hide resolved
// `shapefile` and `shapefile-glob-zip` both match only one file. | ||
assertEquals(fileCount, globZipCount); | ||
// `shapefile-glob` matches two input files, should have 2x number of features of `shapefile`. | ||
assertEquals(2 * fileCount, globCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a good e2e test to convince me this all works!
Kudos, SonarCloud Quality Gate passed! |
Looks great, thanks @erik! |
This PR pulls ZIP file handling out of
ShapefileReader
so that it can be reused for different readers. The changes here can be applied to other reader types as we add them.Added:
.shp
files from a single.zip
.zip
fileThere's a breaking change in naming:
addShapefileDirectorySource
becomesaddShapefileGlobSource
. I figuredaddShapefileDirectorySource
was new enough that the more accurate name was worth the churn, as the pattern can now be applied both to ZIP archives and directories.Another change in behavior: previously, if you had a
.zip
with two.shp
files, only one would be read. Now (unless a glob pattern filters one out), both will be read.