From 33befe28b7410f46d918e9ba15233e02114d3f1c Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 16:19:08 +0200 Subject: [PATCH 1/7] Harden dataset storage cleanup cleanStorage was a GET that deleted by default, so anything following links could trigger it: browser prefetch, link previews, crawlers, or revisiting the URL from history. It is now a PUT, and dryrun defaults to true so omitting the parameter reports instead of deleting. It could also delete real files. Direct upload writes its object to the final storage key before Dataverse registers a DataFile for it, so an upload that had finished but was still waiting to be saved looked exactly like an abandoned one. cleanUp now takes a minimum age and skips anything modified more recently, configurable through dataverse.files.clean-storage-min-age-days, default 7. The listing already retrieved modification times from S3 and discarded them, so the age check costs no extra calls. An unknown timestamp counts as too recent, so a backend that cannot report one never causes a deletion. getToDeleteFilesFilter stays a name-only predicate, leaving its existing test unchanged. --- doc/release-notes/harden-clean-storage.md | 8 +++ doc/sphinx-guides/source/api/native-api.rst | 14 +++-- .../harvard/iq/dataverse/api/Datasets.java | 19 +++++-- .../AbstractRemoteOverlayAccessIO.java | 5 +- .../iq/dataverse/dataaccess/FileAccessIO.java | 24 +++++++-- .../dataverse/dataaccess/InputStreamIO.java | 3 +- .../iq/dataverse/dataaccess/S3AccessIO.java | 15 ++++-- .../iq/dataverse/dataaccess/StorageIO.java | 24 ++++++++- .../dataverse/dataaccess/SwiftAccessIO.java | 24 +++++++-- .../iq/dataverse/settings/JvmSettings.java | 1 + .../META-INF/microprofile-config.properties | 3 ++ .../dataaccess/StorageIOCleanUpAgeTest.java | 53 +++++++++++++++++++ 12 files changed, 165 insertions(+), 28 deletions(-) create mode 100644 doc/release-notes/harden-clean-storage.md create mode 100644 src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java diff --git a/doc/release-notes/harden-clean-storage.md b/doc/release-notes/harden-clean-storage.md new file mode 100644 index 00000000000..6681dd50203 --- /dev/null +++ b/doc/release-notes/harden-clean-storage.md @@ -0,0 +1,8 @@ +The dataset storage cleanup call is now `PUT /api/datasets/{id}/cleanStorage`. It was a `GET`, which meant anything that follows links (browser prefetch, link previews in chat and mail clients, crawlers, or revisiting the URL from history) could trigger a deletion. + +Two further changes make it safer: + +- `dryrun` now defaults to `true`. Omitting it reports what would be removed instead of removing it. Pass `dryrun=false` to actually delete. +- Storage objects modified more recently than `dataverse.files.clean-storage-min-age-days` (7 by default) are never removed. An upload is written to the dataset's storage location before Dataverse registers it as a file, so without a grace period a completed upload still waiting to be saved was indistinguishable from an abandoned one and could be deleted. Raise the setting if uploads in your installation stay unregistered for longer than a week. + +Scripts calling this endpoint need to switch to `PUT` and, if they relied on the old default to delete, to pass `dryrun=false` explicitly. diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index e08777c9cf2..8db4ec4ec98 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -3462,8 +3462,14 @@ Cleanup Storage of a Dataset This is an experimental feature and should be tested on your system before using it in production. Also, make sure that your backups are up-to-date before using this on production servers. -It is advised to first call this method with the ``dryrun`` parameter set to ``true`` before actually deleting the files. -This will allow you to manually inspect the files that would be deleted if that parameter is set to ``false`` or is omitted (a list of the files that would be deleted is provided in the response). +This call only reports what it would remove unless ``dryrun`` is explicitly set to ``false``. Omitting the parameter is +the same as setting it to ``true``, so inspecting the reported list is always the default (a list of the files that would +be deleted is provided in the response). + +Storage objects that were modified more recently than ``dataverse.files.clean-storage-min-age-days`` (7 by default) are +never removed. An upload writes its object to the dataset's storage location before Dataverse registers it as a file, so +without that grace period a completed upload waiting to be saved would look the same as an abandoned one. Raise the +setting if your workflows leave uploads unregistered for longer than a week. If your Dataverse installation has been configured to support direct uploads, or in some other situations, you could end up with some files in the storage of a dataset that are not linked to that dataset directly. Most commonly, this could @@ -3479,13 +3485,13 @@ All the files stored in the Dataset storage location that are not in the file li export PERSISTENT_ID=doi:10.5072/FK2/J8SJZB export DRYRUN=true - curl -H "X-Dataverse-key: $API_TOKEN" -X GET "$SERVER_URL/api/datasets/:persistentId/cleanStorage?persistentId=$PERSISTENT_ID&dryrun=$DRYRUN" + curl -H "X-Dataverse-key: $API_TOKEN" -X PUT "$SERVER_URL/api/datasets/:persistentId/cleanStorage?persistentId=$PERSISTENT_ID&dryrun=$DRYRUN" The fully expanded example above (without environment variables) looks like this: .. code-block:: bash - curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X GET "https://demo.dataverse.org/api/datasets/:persistentId/cleanStorage?persistentId=doi:10.5072/FK2/J8SJZB&dryrun=true" + curl -H "X-Dataverse-key: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -X PUT "https://demo.dataverse.org/api/datasets/:persistentId/cleanStorage?persistentId=doi:10.5072/FK2/J8SJZB&dryrun=true" Adding Files To a Dataset via Other Tools ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java index 1b549b10a6b..c00ce3eaaef 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java @@ -97,6 +97,7 @@ import java.sql.Timestamp; import java.text.MessageFormat; import java.text.SimpleDateFormat; +import java.time.Duration; import java.time.LocalDate; import java.time.LocalDateTime; import java.time.ZoneId; @@ -3410,11 +3411,14 @@ public Response addFileToDataset(@Context ContainerRequestContext crc, * @param idSupplied * @return */ - @GET + @PUT @AuthRequired @Path("{id}/cleanStorage") @Operation(summary = "Cleans dataset storage", - description = "Finds and optionally deletes storage objects that are no longer referenced by files in a dataset.") + description = "Finds and optionally deletes storage objects that are no longer referenced by files in a dataset. " + + "Reports without deleting unless dryrun is explicitly set to false. Objects modified more recently than " + + "dataverse.files.clean-storage-min-age-days are never removed, so that an upload which has completed but " + + "has not been registered yet is not mistaken for an abandoned one.") public Response cleanStorage(@Context ContainerRequestContext crc, @Parameter(description = "Resource id or persistent identifier.") @PathParam("id") String idSupplied, @Parameter(description = "Whether to validate the request without applying changes.") @QueryParam("dryrun") Boolean dryrun) { // get user and dataset User authUser = getRequestUser(crc); @@ -3428,10 +3432,11 @@ public Response cleanStorage(@Context ContainerRequestContext crc, @Parameter(de // check permissions if (!permissionSvc.permissionsFor(req, dataset).contains(Permission.EditDataset)) { - return error(Response.Status.INTERNAL_SERVER_ERROR, "Access denied!"); + return error(Response.Status.FORBIDDEN, "Access denied!"); } - boolean doDryRun = dryrun != null && dryrun.booleanValue(); + // Reporting is the default: deleting requires asking for it explicitly. + boolean doDryRun = dryrun == null || dryrun.booleanValue(); // check if no legacy files are present Set datasetFilenames = getDatasetFilenames(dataset); @@ -3443,7 +3448,7 @@ public Response cleanStorage(@Context ContainerRequestContext crc, @Parameter(de List deleted; try { StorageIO datasetIO = DataAccess.getStorageIO(dataset); - deleted = datasetIO.cleanUp(filter, doDryRun); + deleted = datasetIO.cleanUp(filter, getCleanStorageMinimumAge(), doDryRun); } catch (IOException ex) { logger.log(Level.SEVERE, null, ex); return error(Response.Status.INTERNAL_SERVER_ERROR, "IOException! Serious Error! See administrator!"); @@ -3499,6 +3504,10 @@ public Response getCompareVersionsSummary(@Context ContainerRequestContext crc, }, getRequestUser(crc)); } + private static Duration getCleanStorageMinimumAge() { + return Duration.ofDays(JvmSettings.CLEAN_STORAGE_MIN_AGE_DAYS.lookup(Integer.class)); + } + private static Set getDatasetFilenames(Dataset dataset) { Set files = new HashSet<>(); for (DataFile dataFile: dataset.getFiles()) { diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/AbstractRemoteOverlayAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/AbstractRemoteOverlayAccessIO.java index 18269f6970e..dcfd4c9b59a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/AbstractRemoteOverlayAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/AbstractRemoteOverlayAccessIO.java @@ -9,6 +9,7 @@ import java.security.KeyManagementException; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; +import java.time.Duration; import java.util.List; import java.util.function.Predicate; import java.util.logging.Logger; @@ -213,8 +214,8 @@ public boolean exists() { } @Override - public List cleanUp(Predicate filter, boolean dryRun) throws IOException { - return baseStore.cleanUp(filter, dryRun); + public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { + return baseStore.cleanUp(filter, minimumAge, dryRun); } @Override diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java index e5006645b7f..098496cd956 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java @@ -32,7 +32,11 @@ import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; +import java.time.Duration; +import java.time.Instant; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.Predicate; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -676,7 +680,7 @@ protected static boolean isValidIdentifier(String driverId, String storageId) { return true; } - private List listAllFiles() throws IOException { + private Map listAllFiles() throws IOException { Dataset dataset = this.getDataset(); if (dataset == null) { throw new IOException("This FileAccessIO object hasn't been properly initialized."); @@ -689,10 +693,17 @@ private List listAllFiles() throws IOException { DirectoryStream dirStream = Files.newDirectoryStream(Paths.get(this.getFilesRootDirectory(), datasetDirectoryPath.toString())); - List res = new ArrayList<>(); + Map res = new HashMap<>(); if (dirStream != null) { for (Path filePath : dirStream) { - res.add(filePath.getFileName().toString()); + Instant lastModified; + try { + lastModified = Files.getLastModifiedTime(filePath).toInstant(); + } catch (IOException ex) { + // Unknown age is treated as too recent to remove. + lastModified = null; + } + res.put(filePath.getFileName().toString(), lastModified); } dirStream.close(); } @@ -716,8 +727,11 @@ private void deleteFile(String fileName) throws IOException { } @Override - public List cleanUp(Predicate filter, boolean dryRun) throws IOException { - List toDelete = this.listAllFiles().stream().filter(filter).collect(Collectors.toList()); + public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { + List toDelete = this.listAllFiles().entrySet().stream() + .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); if (dryRun) { return toDelete; } diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/InputStreamIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/InputStreamIO.java index de392b74cca..87fcdb1cf3d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/InputStreamIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/InputStreamIO.java @@ -13,6 +13,7 @@ import java.nio.channels.Channels; import java.nio.channels.WritableByteChannel; import java.nio.file.Path; +import java.time.Duration; import java.util.List; import java.util.function.Predicate; import java.util.logging.Logger; @@ -161,7 +162,7 @@ public void revertBackupAsAux(String auxItemTag) throws IOException { } @Override - public List cleanUp(Predicate filter, boolean dryRun) throws IOException { + public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { throw new UnsupportedDataAccessOperationException("InputStreamIO: tthis method is not supported in this DataAccess driver."); } diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java index 4aa81284eea..8237309eb50 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java @@ -56,6 +56,8 @@ import java.nio.file.Paths; import java.time.Duration; import java.time.Instant; +import java.util.HashMap; +import java.util.Map; import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -1517,7 +1519,7 @@ protected static boolean isValidIdentifier(String driverId, String storageId) { return true; } - private List listAllFiles() throws IOException { + private Map listAllFiles() throws IOException { if (!this.canWrite()) { open(); } @@ -1527,7 +1529,7 @@ private List listAllFiles() throws IOException { } String prefix = dataset.getAuthorityForFileStorage() + "/" + dataset.getIdentifierForFileStorage() + "/"; - List ret = new ArrayList<>(); + Map ret = new HashMap<>(); ListObjectsV2Request listObjectsReqManual = ListObjectsV2Request.builder().bucket(bucketName).prefix(prefix) .build(); @@ -1571,7 +1573,7 @@ private List listAllFiles() throws IOException { for (S3Object item : storedFilesSummary) { String fileName = item.key().substring(prefix.length()); - ret.add(fileName); + ret.put(fileName, item.lastModified()); } return ret; } @@ -1623,8 +1625,11 @@ public void closeInputStream() { } @Override - public List cleanUp(Predicate filter, boolean dryRun) throws IOException { - List toDelete = this.listAllFiles().stream().filter(filter).collect(Collectors.toList()); + public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { + List toDelete = this.listAllFiles().entrySet().stream() + .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); if (dryRun) { return toDelete; } diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java index b009c4e86ec..751d29f3053 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java @@ -36,6 +36,8 @@ import java.nio.channels.WritableByteChannel; import java.nio.file.Path; import java.util.HashMap; +import java.time.Duration; +import java.time.Instant; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -641,7 +643,27 @@ protected static boolean usesStandardNamePattern(String identifier) { return m.find(); } - public abstract List cleanUp(Predicate filter, boolean dryRun) throws IOException; + /** + * Deletes stored objects that {@code filter} selects and that have not been modified + * for at least {@code minimumAge}. + * + * @param filter selects objects by name, typically those no longer referenced by the dataset + * @param minimumAge how long an object must have been untouched before it can be removed + * @param dryRun when true, report what would be removed without removing anything + */ + public abstract List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException; + + /** + * Whether an object last modified at {@code lastModified} is old enough to remove. + * An unknown timestamp is treated as too recent, so that a storage backend which + * cannot report one never causes a deletion. + */ + protected static boolean isOlderThan(Instant lastModified, Duration minimumAge) { + if (lastModified == null) { + return false; + } + return lastModified.isBefore(Instant.now().minus(minimumAge)); + } /** * A storage-type-specific mechanism for retrieving the size of a file. Intended diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java index 717f46ffd60..09a45e6f523 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java @@ -19,6 +19,10 @@ import java.security.SignatureException; import java.util.ArrayList; import java.util.Collection; +import java.time.Duration; +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; import java.util.Formatter; import java.util.List; import java.util.Properties; @@ -912,7 +916,7 @@ public static String calculateRFC2104HMAC(String data, String key) return toHexString(mac.doFinal(data.getBytes())); } - private List listAllFiles() throws IOException { + private Map listAllFiles() throws IOException { if (!this.canWrite()) { open(DataAccessOption.WRITE_ACCESS); } @@ -924,12 +928,19 @@ private List listAllFiles() throws IOException { Collection items; String lastItemName = null; - List ret = new ArrayList<>(); + Map ret = new HashMap<>(); while ((items = this.swiftContainer.list(prefix, lastItemName, LIST_PAGE_LIMIT)) != null && items.size() > 0) { for (StoredObject item : items) { lastItemName = item.getName().substring(prefix.length()); - ret.add(lastItemName); + Instant lastModified; + try { + lastModified = item.getLastModifiedAsDate().toInstant(); + } catch (RuntimeException ex) { + // Unknown age is treated as too recent to remove. + lastModified = null; + } + ret.put(lastItemName, lastModified); } } @@ -956,8 +967,11 @@ private void deleteFile(String fileName) throws IOException { } @Override - public List cleanUp(Predicate filter, boolean dryRun) throws IOException { - List toDelete = this.listAllFiles().stream().filter(filter).collect(Collectors.toList()); + public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { + List toDelete = this.listAllFiles().entrySet().stream() + .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); if (dryRun) { return toDelete; } diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java index 8ed18b4c63f..7aaab1eebeb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java @@ -57,6 +57,7 @@ public enum JvmSettings { FEATURED_ITEMS_IMAGE_UPLOADS_DIRECTORY(SCOPE_FEATURED_ITEMS, "image-uploads"), HIDE_SCHEMA_DOT_ORG_DOWNLOAD_URLS(SCOPE_FILES, "hide-schema-dot-org-download-urls"), DEFAULT_DATASET_FILE_COUNT_LIMIT(SCOPE_FILES, "default-dataset-file-count-limit"), + CLEAN_STORAGE_MIN_AGE_DAYS(SCOPE_FILES, "clean-storage-min-age-days"), //STORAGE DRIVER SETTINGS SCOPE_DRIVER(SCOPE_FILES), diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index b2ad2e176fc..816898937d9 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -19,6 +19,9 @@ dataverse.files.directory=${STORAGE_DIR:/tmp/dataverse} dataverse.files.uploads=${STORAGE_DIR:${com.sun.aas.instanceRoot}}/uploads dataverse.files.docroot=${STORAGE_DIR:${com.sun.aas.instanceRoot}}/docroot dataverse.files.globus-cache-maxage=5 +# Storage objects younger than this are left alone by cleanStorage, so that an upload +# that has finished but has not been registered yet is not mistaken for junk. +dataverse.files.clean-storage-min-age-days=7 dataverse.files.featured-items.image-maxsize=1000000 dataverse.files.featured-items.image-uploads=featuredItems diff --git a/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java b/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java new file mode 100644 index 00000000000..ed4675e2379 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java @@ -0,0 +1,53 @@ +package edu.harvard.iq.dataverse.dataaccess; + +import org.junit.jupiter.api.Test; + +import java.time.Duration; +import java.time.Instant; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The age guard that keeps cleanUp from removing an upload that has finished but has + * not been registered as a DataFile yet. + */ +class StorageIOCleanUpAgeTest { + + private static final Duration ONE_WEEK = Duration.ofDays(7); + + @Test + void testIsOlderThan_unknownTimestampIsNeverRemoved() { + assertFalse(StorageIO.isOlderThan(null, ONE_WEEK)); + } + + @Test + void testIsOlderThan_justUploadedIsKept() { + assertFalse(StorageIO.isOlderThan(Instant.now(), ONE_WEEK)); + } + + @Test + void testIsOlderThan_withinGracePeriodIsKept() { + assertFalse(StorageIO.isOlderThan(Instant.now().minus(Duration.ofDays(6)), ONE_WEEK)); + } + + @Test + void testIsOlderThan_justInsideGracePeriodIsKept() { + assertFalse(StorageIO.isOlderThan(Instant.now().minus(Duration.ofDays(7)).plusSeconds(30), ONE_WEEK)); + } + + @Test + void testIsOlderThan_pastGracePeriodIsRemovable() { + assertTrue(StorageIO.isOlderThan(Instant.now().minus(Duration.ofDays(8)), ONE_WEEK)); + } + + @Test + void testIsOlderThan_longAbandonedIsRemovable() { + assertTrue(StorageIO.isOlderThan(Instant.now().minus(Duration.ofDays(365)), ONE_WEEK)); + } + + @Test + void testIsOlderThan_zeroGracePeriodRemovesAnythingWithATimestamp() { + assertTrue(StorageIO.isOlderThan(Instant.now().minusSeconds(1), Duration.ZERO)); + } +} From c1f789d1d50a80ebf083dedc69b2fbda9247c907 Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 16:50:30 +0200 Subject: [PATCH 2/7] Pass a minimum age from the audit endpoint too getAuditFiles calls cleanUp purely to list what is in storage, with a match-everything filter and dryRun set. Duration.ZERO keeps that listing complete, which is the behaviour it had before. --- src/main/java/edu/harvard/iq/dataverse/api/Admin.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java index e30be924903..d0163d07f72 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -68,6 +68,7 @@ import jakarta.ws.rs.core.Response; import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder; +import java.time.Duration; import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.Collections; @@ -2957,7 +2958,8 @@ public Response getAuditFiles(@Context ContainerRequestContext crc, try { Predicate filter = s -> true; StorageIO datasetIO = DataAccess.getStorageIO(dataset); - final List result = datasetIO.cleanUp(filter, true); + // Auditing lists everything, so no minimum age applies here. + final List result = datasetIO.cleanUp(filter, Duration.ZERO, true); // add files that are in dataset files but not in cleanup result or DataFiles with missing FileMetadata dataset.getFiles().forEach(df -> { try { From acb48209d78e66320446e5118032cede882a04fa Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 18:02:20 +0200 Subject: [PATCH 3/7] Share the cleanup selection between storage backends Sonar failed the gate on duplication and coverage, not on code smells. The three backends had the same filter-and-age pipeline inline, so it moves to StorageIO.selectForCleanUp. That leaves one definition of the selection rule, removes the duplicated block, and makes the part worth testing reachable without a storage backend. Five tests cover it. Also returns an unmodifiable list, drops the two imports that became unused, and removes the unread local in the audit endpoint. --- .../edu/harvard/iq/dataverse/api/Admin.java | 5 +- .../iq/dataverse/dataaccess/FileAccessIO.java | 6 +- .../iq/dataverse/dataaccess/S3AccessIO.java | 5 +- .../iq/dataverse/dataaccess/StorageIO.java | 15 +++++ .../dataverse/dataaccess/SwiftAccessIO.java | 6 +- .../dataaccess/StorageIOCleanUpAgeTest.java | 59 +++++++++++++++++++ 6 files changed, 80 insertions(+), 16 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java index d0163d07f72..d63024583e9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Admin.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Admin.java @@ -2958,8 +2958,9 @@ public Response getAuditFiles(@Context ContainerRequestContext crc, try { Predicate filter = s -> true; StorageIO datasetIO = DataAccess.getStorageIO(dataset); - // Auditing lists everything, so no minimum age applies here. - final List result = datasetIO.cleanUp(filter, Duration.ZERO, true); + // Auditing lists everything, so no minimum age applies here. The + // listing is not used; the call is kept for its existence checks. + datasetIO.cleanUp(filter, Duration.ZERO, true); // add files that are in dataset files but not in cleanup result or DataFiles with missing FileMetadata dataset.getFiles().forEach(df -> { try { diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java index 098496cd956..ac86025caef 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIO.java @@ -39,7 +39,6 @@ import java.util.Map; import java.util.function.Predicate; import java.util.logging.Logger; -import java.util.stream.Collectors; // Dataverse imports: import edu.harvard.iq.dataverse.DataFile; @@ -728,10 +727,7 @@ private void deleteFile(String fileName) throws IOException { @Override public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { - List toDelete = this.listAllFiles().entrySet().stream() - .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); + List toDelete = selectForCleanUp(this.listAllFiles(), filter, minimumAge); if (dryRun) { return toDelete; } diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java index 8237309eb50..bc5991c2b8f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java @@ -1626,10 +1626,7 @@ public void closeInputStream() { @Override public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { - List toDelete = this.listAllFiles().entrySet().stream() - .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); + List toDelete = selectForCleanUp(this.listAllFiles(), filter, minimumAge); if (dryRun) { return toDelete; } diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java index 751d29f3053..3e0720c08ed 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java @@ -658,6 +658,21 @@ protected static boolean usesStandardNamePattern(String identifier) { * An unknown timestamp is treated as too recent, so that a storage backend which * cannot report one never causes a deletion. */ + /** + * The stored objects that {@code filter} selects and that are old enough to + * remove. Shared by the storage backends so the selection rule has one + * definition. + * + * @param stored object names mapped to their last modification time + */ + protected static List selectForCleanUp(Map stored, Predicate filter, + Duration minimumAge) { + return stored.entrySet().stream() + .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) + .map(Map.Entry::getKey) + .toList(); + } + protected static boolean isOlderThan(Instant lastModified, Duration minimumAge) { if (lastModified == null) { return false; diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java index 09a45e6f523..6f634b949d6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java @@ -28,7 +28,6 @@ import java.util.Properties; import java.util.function.Predicate; import java.util.logging.Logger; -import java.util.stream.Collectors; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -968,10 +967,7 @@ private void deleteFile(String fileName) throws IOException { @Override public List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException { - List toDelete = this.listAllFiles().entrySet().stream() - .filter(e -> filter.test(e.getKey()) && isOlderThan(e.getValue(), minimumAge)) - .map(Map.Entry::getKey) - .collect(Collectors.toList()); + List toDelete = selectForCleanUp(this.listAllFiles(), filter, minimumAge); if (dryRun) { return toDelete; } diff --git a/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java b/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java index ed4675e2379..7469950a1e5 100644 --- a/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/dataaccess/StorageIOCleanUpAgeTest.java @@ -4,8 +4,14 @@ import java.time.Duration; import java.time.Instant; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Predicate; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -50,4 +56,57 @@ void testIsOlderThan_longAbandonedIsRemovable() { void testIsOlderThan_zeroGracePeriodRemovesAnythingWithATimestamp() { assertTrue(StorageIO.isOlderThan(Instant.now().minusSeconds(1), Duration.ZERO)); } + + private static Map stored(Object... nameThenAge) { + Map m = new HashMap<>(); + for (int i = 0; i < nameThenAge.length; i += 2) { + Duration age = (Duration) nameThenAge[i + 1]; + m.put((String) nameThenAge[i], age == null ? null : Instant.now().minus(age)); + } + return m; + } + + private static final Predicate ORPHANS = name -> name.startsWith("orphan"); + + @Test + void testSelectForCleanUp_takesOnlyOldOrphans() { + Map stored = stored( + "orphan-old", Duration.ofDays(30), + "orphan-fresh", Duration.ofHours(1), + "referenced-old", Duration.ofDays(30)); + + List selected = StorageIO.selectForCleanUp(stored, ORPHANS, ONE_WEEK); + + assertEquals(List.of("orphan-old"), selected); + } + + @Test + void testSelectForCleanUp_skipsUnknownTimestamps() { + List selected = StorageIO.selectForCleanUp(stored("orphan-a", null), ORPHANS, ONE_WEEK); + + assertTrue(selected.isEmpty()); + } + + @Test + void testSelectForCleanUp_emptyStoreSelectsNothing() { + assertTrue(StorageIO.selectForCleanUp(Map.of(), ORPHANS, ONE_WEEK).isEmpty()); + } + + @Test + void testSelectForCleanUp_zeroGraceStillHonoursTheFilter() { + Map stored = stored( + "orphan-a", Duration.ofSeconds(5), + "referenced-b", Duration.ofSeconds(5)); + + List selected = StorageIO.selectForCleanUp(stored, ORPHANS, Duration.ZERO); + + assertEquals(List.of("orphan-a"), selected); + } + + @Test + void testSelectForCleanUp_resultIsNotMeantToBeModified() { + List selected = StorageIO.selectForCleanUp(stored("orphan-a", Duration.ofDays(30)), ORPHANS, ONE_WEEK); + + assertThrows(UnsupportedOperationException.class, () -> selected.add("x")); + } } From da46fc3642088133105aa0fa6c0846c63a5d76c3 Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 18:30:55 +0200 Subject: [PATCH 4/7] Cover the file backend's cleanup against a real directory FileAccessIOTest already stages files under the default root, so cleanUp can be exercised end to end there: an old orphan is removed, a recently modified one is not, a dry run deletes nothing, and the filter still protects referenced files. That covers the age guard on the backend most installations use. The Swift and S3 equivalents would need test seams in production code, and the resource method needs the container, so both stay with the integration tests. --- .../dataaccess/FileAccessIOTest.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/test/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIOTest.java b/src/test/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIOTest.java index ea5cc4b66a8..454a12857cb 100644 --- a/src/test/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIOTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/dataaccess/FileAccessIOTest.java @@ -20,6 +20,8 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.file.Path; +import java.time.Duration; +import java.util.function.Predicate; import java.util.ArrayList; import java.util.List; import org.apache.commons.io.FileUtils; @@ -317,4 +319,45 @@ public void testFileIdentifierFormats() throws IOException { System.clearProperty("dataverse.files.filetest.label"); System.clearProperty("dataverse.files.filetest.directory"); } + + private File stageOrphan(String name, Duration age) throws IOException { + File f = new File("/tmp/files/tmp/dataset/" + name); + f.createNewFile(); + assertTrue(f.setLastModified(System.currentTimeMillis() - age.toMillis())); + return f; + } + + private static final Predicate ORPHANS = name -> name.startsWith("orphan"); + + @Test + public void testCleanUp_leavesRecentlyModifiedFilesAlone() throws IOException { + File old = stageOrphan("orphan-old", Duration.ofDays(30)); + File fresh = stageOrphan("orphan-fresh", Duration.ofHours(2)); + + List deleted = datasetAccess.cleanUp(ORPHANS, Duration.ofDays(7), false); + + assertEquals(List.of("orphan-old"), deleted); + assertFalse(old.exists()); + assertTrue(fresh.exists(), "an upload that has not been registered yet must survive"); + } + + @Test + public void testCleanUp_dryRunReportsWithoutDeleting() throws IOException { + File old = stageOrphan("orphan-old", Duration.ofDays(30)); + + List reported = datasetAccess.cleanUp(ORPHANS, Duration.ofDays(7), true); + + assertEquals(List.of("orphan-old"), reported); + assertTrue(old.exists(), "a dry run must not delete anything"); + } + + @Test + public void testCleanUp_filterKeepsReferencedFiles() throws IOException { + File referenced = stageOrphan("kept-old", Duration.ofDays(30)); + + List deleted = datasetAccess.cleanUp(ORPHANS, Duration.ofDays(7), false); + + assertTrue(deleted.isEmpty()); + assertTrue(referenced.exists()); + } } From 009a46ab0f6358978e12a913d2ff558bf45dad0a Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 18:48:47 +0200 Subject: [PATCH 5/7] Cover the S3 and Swift cleanup paths S3AccessIO already takes an injected client, so its cleanup needs no production change: a mocked listing with per-object timestamps drives it. Swift has no such seam, so swiftContainer drops private to package-private and the test supplies a mocked container. Setting isWriteAccess directly keeps open() out of the way in both. That leaves the resource method and the overlay delegation uncovered, both of which need the container to reach. --- .../dataverse/dataaccess/SwiftAccessIO.java | 3 +- .../dataverse/dataaccess/S3AccessIOTest.java | 39 +++++++++++++++- .../dataaccess/SwiftAccessIOTest.java | 45 +++++++++++++++++++ 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java index 6f634b949d6..daa63efe1dd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java @@ -93,7 +93,8 @@ private void readSettings() { private Account account = null; private StoredObject swiftFileObject = null; - private Container swiftContainer = null; + // Package-private so the cleanup tests can supply a container. + Container swiftContainer = null; private boolean isPublicContainer = true; private String swiftFolderPathSeparator = "_"; private String swiftDefaultEndpoint = null; diff --git a/src/test/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIOTest.java b/src/test/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIOTest.java index 5095c12024c..89b95c72452 100644 --- a/src/test/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIOTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIOTest.java @@ -25,6 +25,15 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.function.Predicate; +import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; +import software.amazon.awssdk.services.s3.model.ListObjectsV2Response; +import software.amazon.awssdk.services.s3.model.S3Object; @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.STRICT_STUBS) @@ -129,4 +138,32 @@ void testS3IdentifierFormats() throws IOException { //bad bucket assertFalse(DataAccess.isValidDirectStorageIdentifier("s3test://bucket:" + FileUtil.generateStorageIdentifier())); } -} \ No newline at end of file + + private static final Predicate ORPHANS = name -> name.startsWith("orphan"); + + private S3Object storedObject(String name, Duration age) { + String prefix = dataSet.getAuthorityForFileStorage() + "/" + dataSet.getIdentifierForFileStorage() + "/"; + return S3Object.builder().key(prefix + name).lastModified(Instant.now().minus(age)).build(); + } + + private void givenBucketContains(S3Object... objects) { + given(s3client.listObjectsV2(any(ListObjectsV2Request.class))) + .willReturn(CompletableFuture.completedFuture( + ListObjectsV2Response.builder().contents(List.of(objects)).build())); + } + + @Test + public void testCleanUp_dryRunSkipsRecentlyModifiedObjects() throws IOException { + // Skip open(): the injected client is already the one under test. + dataSetAccess.isWriteAccess = true; + givenBucketContains( + storedObject("orphan-old", Duration.ofDays(30)), + storedObject("orphan-fresh", Duration.ofHours(2)), + storedObject("referenced-old", Duration.ofDays(30))); + + List reported = dataSetAccess.cleanUp(ORPHANS, Duration.ofDays(7), true); + + assertEquals(List.of("orphan-old"), reported); + verify(s3client, never()).deleteObject(any(DeleteObjectRequest.class)); + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java b/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java index 27e0ac758e0..736a3f1c725 100644 --- a/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java @@ -9,6 +9,18 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.mocks.MocksFactory; import java.io.IOException; +import java.time.Duration; +import java.time.Instant; +import java.util.Date; +import java.util.List; +import java.util.function.Predicate; +import org.javaswift.joss.model.Container; +import org.javaswift.joss.model.StoredObject; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.nullable; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.security.SignatureException; @@ -23,6 +35,8 @@ */ public class SwiftAccessIOTest { + private static final Predicate ORPHANS = name -> name.startsWith("orphan"); + private SwiftAccessIO datasetAccess; private SwiftAccessIO datafileAccess; private Dataset dataset; @@ -78,4 +92,35 @@ public void testToHexString() { public void testCalculateRFC2104HMAC() throws SignatureException, NoSuchAlgorithmException, InvalidKeyException { assertEquals("104152c5bfdca07bc633eebd46199f0255c9f49d", swiftAccess.calculateRFC2104HMAC("data", "key")); } + + private StoredObject storedObject(String prefix, String name, Duration age) { + StoredObject item = mock(StoredObject.class); + when(item.getName()).thenReturn(prefix + name); + when(item.getLastModifiedAsDate()).thenReturn(Date.from(Instant.now().minus(age))); + return item; + } + + @Test + public void testCleanUp_dryRunSkipsRecentlyModifiedObjects() throws IOException { + // datafile is owned by dataset, so both resolve to the same container name. + String prefix = datafileAccess.getSwiftContainerName() + "/"; + // Build the stored objects before stubbing the container, so their own + // stubbing does not nest inside the container's. + List stored = List.of( + storedObject(prefix, "orphan-old", Duration.ofDays(30)), + storedObject(prefix, "orphan-fresh", Duration.ofHours(2)), + storedObject(prefix, "referenced-old", Duration.ofDays(30))); + + Container container = mock(Container.class); + when(container.list(anyString(), nullable(String.class), anyInt())) + .thenReturn(stored) + .thenReturn(List.of()); + datasetAccess.swiftContainer = container; + // Skip open(): the container above is the one under test. + datasetAccess.isWriteAccess = true; + + List reported = datasetAccess.cleanUp(ORPHANS, Duration.ofDays(7), true); + + assertEquals(List.of("orphan-old"), reported); + } } From 17a390a91a868966b2822163b606461e84a8cdd6 Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 19:21:17 +0200 Subject: [PATCH 6/7] Revert the Swift test seam swiftContainer goes back to private and its test comes out. The seam existed only to move the coverage number, and a production visibility change is not worth carrying for that. --- .../dataverse/dataaccess/SwiftAccessIO.java | 3 +- .../dataaccess/SwiftAccessIOTest.java | 45 ------------------- 2 files changed, 1 insertion(+), 47 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java index daa63efe1dd..6f634b949d6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIO.java @@ -93,8 +93,7 @@ private void readSettings() { private Account account = null; private StoredObject swiftFileObject = null; - // Package-private so the cleanup tests can supply a container. - Container swiftContainer = null; + private Container swiftContainer = null; private boolean isPublicContainer = true; private String swiftFolderPathSeparator = "_"; private String swiftDefaultEndpoint = null; diff --git a/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java b/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java index 736a3f1c725..27e0ac758e0 100644 --- a/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/dataaccess/SwiftAccessIOTest.java @@ -9,18 +9,6 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.mocks.MocksFactory; import java.io.IOException; -import java.time.Duration; -import java.time.Instant; -import java.util.Date; -import java.util.List; -import java.util.function.Predicate; -import org.javaswift.joss.model.Container; -import org.javaswift.joss.model.StoredObject; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.nullable; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.security.SignatureException; @@ -35,8 +23,6 @@ */ public class SwiftAccessIOTest { - private static final Predicate ORPHANS = name -> name.startsWith("orphan"); - private SwiftAccessIO datasetAccess; private SwiftAccessIO datafileAccess; private Dataset dataset; @@ -92,35 +78,4 @@ public void testToHexString() { public void testCalculateRFC2104HMAC() throws SignatureException, NoSuchAlgorithmException, InvalidKeyException { assertEquals("104152c5bfdca07bc633eebd46199f0255c9f49d", swiftAccess.calculateRFC2104HMAC("data", "key")); } - - private StoredObject storedObject(String prefix, String name, Duration age) { - StoredObject item = mock(StoredObject.class); - when(item.getName()).thenReturn(prefix + name); - when(item.getLastModifiedAsDate()).thenReturn(Date.from(Instant.now().minus(age))); - return item; - } - - @Test - public void testCleanUp_dryRunSkipsRecentlyModifiedObjects() throws IOException { - // datafile is owned by dataset, so both resolve to the same container name. - String prefix = datafileAccess.getSwiftContainerName() + "/"; - // Build the stored objects before stubbing the container, so their own - // stubbing does not nest inside the container's. - List stored = List.of( - storedObject(prefix, "orphan-old", Duration.ofDays(30)), - storedObject(prefix, "orphan-fresh", Duration.ofHours(2)), - storedObject(prefix, "referenced-old", Duration.ofDays(30))); - - Container container = mock(Container.class); - when(container.list(anyString(), nullable(String.class), anyInt())) - .thenReturn(stored) - .thenReturn(List.of()); - datasetAccess.swiftContainer = container; - // Skip open(): the container above is the one under test. - datasetAccess.isWriteAccess = true; - - List reported = datasetAccess.cleanUp(ORPHANS, Duration.ofDays(7), true); - - assertEquals(List.of("orphan-old"), reported); - } } From 6fc04c956cd394f2b5cb417269d08bbaba14a22a Mon Sep 17 00:00:00 2001 From: ErykKul Date: Thu, 10 Sep 2026 19:22:53 +0200 Subject: [PATCH 7/7] Reattach the age guard's javadoc Adding selectForCleanUp put it between isOlderThan's javadoc and the method, so the doc described the wrong one. --- .../edu/harvard/iq/dataverse/dataaccess/StorageIO.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java index 3e0720c08ed..d91e9662770 100644 --- a/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java +++ b/src/main/java/edu/harvard/iq/dataverse/dataaccess/StorageIO.java @@ -653,11 +653,6 @@ protected static boolean usesStandardNamePattern(String identifier) { */ public abstract List cleanUp(Predicate filter, Duration minimumAge, boolean dryRun) throws IOException; - /** - * Whether an object last modified at {@code lastModified} is old enough to remove. - * An unknown timestamp is treated as too recent, so that a storage backend which - * cannot report one never causes a deletion. - */ /** * The stored objects that {@code filter} selects and that are old enough to * remove. Shared by the storage backends so the selection rule has one @@ -673,6 +668,11 @@ protected static List selectForCleanUp(Map stored, Pred .toList(); } + /** + * Whether an object last modified at {@code lastModified} is old enough to remove. + * An unknown timestamp is treated as too recent, so that a storage backend which + * cannot report one never causes a deletion. + */ protected static boolean isOlderThan(Instant lastModified, Duration minimumAge) { if (lastModified == null) { return false;