Skip to content

Commit 76d489e

Browse files
committed
Fix ReadOnlyZipStoreTest failures due to inconsistent ZIP entry paths and directory existence
- Problem: * Some tests failed because ReadOnlyZipStore could not locate ZIP entries when the store was created by simply zipping a directory. Tools differ: some produce entry names with a leading slash, others without. This caused getInputStream(), read(), and getSize() to return null. * testExists() failed for root keys because exists() included directories, but by design it should only test file existence. - Solution: * Added resolvePathWithLeadingSlashFromKeys() to try a secondary lookup with a leading slash. Primary lookup uses the standard key without leading slash; secondary is only for compatibility. * Modified exists(String[] keys) to check only fileSizeIndex, ignoring directories, to match the intended design. - Effect: * ReadOnlyZipStoreTest passes regardless of whether ZIP entries have leading slashes or not. * Test logic now clearly distinguishes between file existence and directory entries.
1 parent 8c6511f commit 76d489e

1 file changed

Lines changed: 24 additions & 3 deletions

File tree

src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,12 @@ String resolveKeys(String[] keys) {
210210
return String.join("/", keys);
211211
}
212212

213+
private String resolvePathWithLeadingSlashFromKeys(String[] keys) {
214+
// Join the keys with "/" and add a leading slash
215+
String path = "/" + String.join("/", keys);
216+
return path;
217+
}
218+
213219
String[] resolveEntryKeys(String entryKey) {
214220
return entryKey.split("/");
215221
}
@@ -218,7 +224,9 @@ String[] resolveEntryKeys(String entryKey) {
218224
public boolean exists(String[] keys) {
219225
ensureCache();
220226
String key = resolveKeys(keys);
221-
return fileSizeIndex.containsKey(key) || directoryToChildrenDirectoriesIndex.containsKey(key);
227+
return fileSizeIndex.containsKey(key);
228+
//This tests for existence of files not directories by design for testing dirs and files use the following line
229+
//return fileSizeIndex.containsKey(key) || directoryToChildrenDirectoriesIndex.containsKey(key);
222230
}
223231

224232
@Nullable
@@ -245,7 +253,11 @@ public ByteBuffer get(String[] keys, long start, long end) {
245253
}
246254

247255
try (ZipFile zf = new ZipFile(zipStorePath.toFile())) {
248-
ZipEntry entry = zf.getEntry(key);
256+
ZipEntry entry = zf.getEntry(key);
257+
if (entry == null) {
258+
String pathInZip = resolvePathWithLeadingSlashFromKeys(keys);// Sometimes paths in zip start with leading slash
259+
entry = zf.getEntry(pathInZip);
260+
}
249261
if (entry == null || entry.isDirectory()) {
250262
return null;
251263
}
@@ -423,6 +435,10 @@ public InputStream getInputStream(String[] keys, long start, long end) {
423435
try {
424436
ZipFile zf = new ZipFile(zipStorePath.toFile());
425437
ZipEntry entry = zf.getEntry(key);
438+
if (entry == null) {
439+
String pathInZip = resolvePathWithLeadingSlashFromKeys(keys);// Sometimes paths in zip start with leading slash
440+
entry = zf.getEntry(pathInZip);
441+
}
426442
if (entry == null || entry.isDirectory()) {
427443
zf.close();
428444
return null;
@@ -527,9 +543,14 @@ public long getSize(String[] keys) {
527543
return cachedSize;
528544
}
529545

530-
// if size is not in header/cache, we fallback to reading the entry to determine size (inefficient but necessary for some zip anomalies)
546+
// if size is not in header/cache, we fallback to reading the entry to determine size (inefficient and probably required only for some zip anomalies)
531547
try (ZipFile zf = new ZipFile(zipStorePath.toFile())) {
532548
ZipEntry entry = zf.getEntry(key);
549+
if (entry == null) {
550+
String pathInZip = resolvePathWithLeadingSlashFromKeys(keys);// Sometimes paths in zip start with leading slash
551+
logger.log(Level.WARNING, "ZipEntry is null for key: {0} when attempting to get size, trying with leading slash", key);
552+
entry = zf.getEntry(pathInZip);
553+
}
533554
if (entry == null || entry.isDirectory()) {
534555
return -1;
535556
}

0 commit comments

Comments
 (0)