Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.commonjava.service.storage.util.Utils.getDuration;
import static org.commonjava.service.storage.util.Utils.sort;
import static org.commonjava.service.storage.util.Utils.depth;
import static org.commonjava.service.storage.util.Utils.getParentPath;
import static org.commonjava.service.storage.util.Utils.getAllCandidates;
import static org.commonjava.service.storage.util.Utils.normalizeFolderPath;
import static org.commonjava.storage.pathmapped.util.PathMapUtils.ROOT_DIR;

@Startup
Expand Down Expand Up @@ -269,6 +273,77 @@ public void purgeEmptyFilesystems()
ret.forEach( filesystem -> fileManager.purgeFilesystem( filesystem ));
}

/**
* Cleans up (deletes) the given empty folders and, if possible, their parent folders up to the root.
* <p>
* Optimization details:
* <ul>
* <li>Collects all input folders and their ancestors as candidates for deletion.</li>
* <li>Sorts candidates by depth (deepest first) to ensure children are processed before parents.</li>
* <li>Attempts to delete each folder only once (tracked in the processed set).</li>
* <li>If a folder is not empty, marks it and all its ancestors as processed, since their parents must also be non-empty.</li>
* <li>Tracks succeeded and failed deletions in the result object.</li>
* <li>Returns a BatchDeleteResult with all attempted deletions for client inspection.</li>
* </ul>
* This approach avoids redundant deletion attempts and is efficient for overlapping directory trees.
*/
public BatchDeleteResult cleanupEmptyFolders(String filesystem, Collection<String> paths) {
Set<String> allCandidates = getAllCandidates(paths);
// Sort by depth, deepest first
List<String> sortedCandidates = new ArrayList<>(allCandidates);
sortedCandidates.sort((a, b) -> Integer.compare(depth(b), depth(a)));
logger.debug("Sorted candidate folders for cleanup (deepest first): {}", sortedCandidates);

Set<String> succeeded = new HashSet<>();
Set<String> failed = new HashSet<>();
Set<String> processed = new HashSet<>();
for (String folder : sortedCandidates) {
if (processed.contains(folder)) {
continue;
}
processed.add(folder);
try {
if (!fileManager.isDirectory(filesystem, folder)) {
logger.debug("Path is not a directory or does not exist, skipping: {} in filesystem: {}", folder, filesystem);
continue;
}
String[] contents = fileManager.list(filesystem, folder);
if (contents == null || contents.length == 0) {
boolean deleted = fileManager.delete(filesystem, normalizeFolderPath(folder));
if (deleted) {
succeeded.add(folder);
logger.debug("Folder deleted: {} in filesystem: {}", folder, filesystem);
} else {
failed.add(folder);
}
} else {
logger.debug("Folder not empty, skipping cleanup: {} in filesystem: {}, contents: {}", folder, filesystem, Arrays.toString(contents));
// Mark this folder and all its ancestors as processed
markAncestorsProcessed(folder, processed);
}
} catch (Exception e) {
logger.warn("Failed to clean up folder: {} in filesystem: {}. Error: {}", folder, filesystem, e.getMessage());
failed.add(folder);
markAncestorsProcessed(folder, processed);
}
}
logger.info("Cleanup empty folders result: succeeded={}, failed={}", succeeded, failed);
BatchDeleteResult result = new BatchDeleteResult();
result.setFilesystem(filesystem);
result.setSucceeded(succeeded);
result.setFailed(failed);
return result;
}

// Mark the given folder and all its ancestors as processed
private void markAncestorsProcessed(String folder, Set<String> processed) {
String current = folder;
while (current != null && !current.isEmpty() && !current.equals("/")) {
processed.add(current);
current = getParentPath(current);
}
}

/**
* By default, the pathmap storage will override existing paths. Here we must check:
* 1. all paths exist in source
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.commonjava.service.storage.dto.BatchDeleteResult;
import org.commonjava.service.storage.util.ResponseHelper;
import org.commonjava.storage.pathmapped.model.Filesystem;
import org.commonjava.service.storage.dto.BatchDeleteRequest;
import org.eclipse.microprofile.openapi.annotations.Operation;
import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
import org.eclipse.microprofile.openapi.annotations.responses.APIResponses;
Expand Down Expand Up @@ -94,4 +95,26 @@ public Response purgeFilesystem( final @PathParam( "filesystem" ) String filesys
logger.debug( "Purge filesystem result: {}", result );
return responseHelper.formatOkResponseWithJsonEntity( result );
}

/**
* Cleans up multiple empty folders in a filesystem.
*
* @param request BatchDeleteRequest containing the filesystem and the set of folder paths to attempt to clean up.
* Only empty folders will be deleted; non-empty folders will be skipped.
* If a parent folder becomes empty as a result, it will also be deleted recursively up to the root.
*/
@Operation( summary = "Cleanup multiple empty folders in a filesystem. Always returns 200 OK for easier client handling; "
+ "failures are reported in the result object." )
@APIResponses( { @APIResponse( responseCode = "200",
description = "Cleanup done (some folders may have failed, see result object)." ) } )
@Consumes( APPLICATION_JSON )
@Produces( APPLICATION_JSON )
@DELETE
@Path( "folders/empty" )
public Response cleanupEmptyFolders( BatchDeleteRequest request )
{
logger.info( "Cleanup empty folders: filesystem={}, paths={}", request.getFilesystem(), request.getPaths() );
BatchDeleteResult result = controller.cleanupEmptyFolders( request.getFilesystem(), request.getPaths() );
return Response.ok(result).build();
}
}
67 changes: 67 additions & 0 deletions src/main/java/org/commonjava/service/storage/util/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import java.util.HashSet;
import java.util.Collection;

public class Utils {
public static Duration getDuration( String timeout )
Expand Down Expand Up @@ -59,4 +62,68 @@ public static String[] sort(String[] list) {
Collections.sort(ret);
return ret.toArray(new String[0]);
}

/**
* Returns the depth (number of slashes) in the path, ignoring root.
*/
public static int depth(String path) {
if (path == null || path.isEmpty() || path.equals("/")) {
return 0;
}
String normalized = path.endsWith("/") && path.length() > 1 ? path.substring(0, path.length() - 1) : path;
int depth = 0;
for (char c : normalized.toCharArray()) {
if (c == '/') depth++;
}
return depth;
}

/**
* Returns the parent path of a given path, or null if at root.
*/
public static String getParentPath(String path) {
if (path == null || path.isEmpty() || path.equals("/")) {
return null;
}
String normalized = path.endsWith("/") && path.length() > 1 ? path.substring(0, path.length() - 1) : path;
int lastSlashIndex = normalized.lastIndexOf('/');
if (lastSlashIndex == -1) {
return null;
}
return normalized.substring(0, lastSlashIndex);
}

/**
* Given a collection of folder paths, returns a set of all those folders and their ancestors,
* up to but not including root.
*/
public static Set<String> getAllCandidates(Collection<String> paths) {
Set<String> allCandidates = new HashSet<>();
if (paths != null) {
for (String path : paths) {
String current = path;
while (current != null && !current.isEmpty() && !current.equals("/")) {
allCandidates.add(current);
current = getParentPath(current);
}
}
}
return allCandidates;
}

/**
* Normalize a folder path for deletion: ensures a trailing slash (except for root).
* This helps avoid issues with path handling in the underlying storage system.
*/
public static String normalizeFolderPath(String path) {
if (path == null || path.isEmpty() || "/".equals(path)) {
return "/";
}
// Add trailing slashes
String normalized = path;
while (!normalized.endsWith("/")) {
normalized = path + "/";
}
return normalized;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.commonjava.service.storage.controller.StorageController;
import org.commonjava.service.storage.dto.BatchCleanupResult;
import org.commonjava.service.storage.dto.FileInfoObj;
import org.commonjava.service.storage.dto.BatchDeleteResult;
import org.junit.jupiter.api.Test;

import jakarta.inject.Inject;
Expand All @@ -27,6 +28,7 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
import java.io.OutputStream;

import static org.junit.jupiter.api.Assertions.*;

Expand Down Expand Up @@ -80,4 +82,86 @@ public void testCleanup()
assertNull( result );
}

/**
* Test cleanupEmptyFolders for:
* - Deleting empty folders (should succeed)
* - Not deleting non-empty folders (should remain)
* - Recursive parent cleanup (parents deleted if they become empty)
* - Overlapping folder trees (shared parents handled efficiently)
* - Correct reporting in BatchDeleteResult (succeeded/failed)
* - Actual state of the storage after cleanup
*/
@Test
public void testCleanupEmptyFolders_recursiveAndOverlapping() throws Exception {
// Setup: create a nested directory structure
// Structure:
// root/
// a/
// b/ (empty)
// c/ (contains file)
// d/ (empty)
// e/f/g/ (empty)
String root = "test-root";
String a = root + "/a";
String b = a + "/b";
String c = a + "/c";
String d = root + "/d";
String e = root + "/e";
String f = e + "/f";
String g = f + "/g";
String fileInC = c + "/file.txt";

// Create directories (by writing and deleting a dummy file)
createEmptyDirectory(filesystem, b);
createEmptyDirectory(filesystem, c);
createEmptyDirectory(filesystem, d);
createEmptyDirectory(filesystem, g);
// Add a file to c (so c and a are not empty)
try (OutputStream out = fileManager.openOutputStream(filesystem, fileInC)) {
out.write("data".getBytes());
}

// Sanity check: all directories exist
assertTrue(fileManager.isDirectory(filesystem, b));
assertTrue(fileManager.isDirectory(filesystem, c));
assertTrue(fileManager.isDirectory(filesystem, d));
assertTrue(fileManager.isDirectory(filesystem, g));
assertTrue(fileManager.isDirectory(filesystem, f));
assertTrue(fileManager.isDirectory(filesystem, e));
assertTrue(fileManager.isDirectory(filesystem, a));
assertTrue(fileManager.isDirectory(filesystem, root));

// Call cleanupEmptyFolders on [b, d, g]
Set<String> toCleanup = new HashSet<>();
toCleanup.add(b); // should delete b, then a (if a becomes empty)
toCleanup.add(d); // should delete d
toCleanup.add(g); // should delete g, f, e (if they become empty)
BatchDeleteResult result = controller.cleanupEmptyFolders(filesystem, toCleanup);

// Check results
// b, d, g, f, e should be deleted (a and root remain because c is not empty)
assertTrue(result.getSucceeded().contains(b));
assertTrue(result.getSucceeded().contains(d));
assertTrue(result.getSucceeded().contains(g));
assertTrue(result.getSucceeded().contains(f));
assertTrue(result.getSucceeded().contains(e));
// a, c, root should not be deleted
assertFalse(result.getSucceeded().contains(a));
assertFalse(result.getSucceeded().contains(c));
assertFalse(result.getSucceeded().contains(root));
// No failures expected
assertTrue(result.getFailed().isEmpty());
// Check actual state
assertFalse(fileManager.isDirectory(filesystem, b));
assertFalse(fileManager.isDirectory(filesystem, d));
assertFalse(fileManager.isDirectory(filesystem, g));
assertFalse(fileManager.isDirectory(filesystem, f));
assertFalse(fileManager.isDirectory(filesystem, e));
assertTrue(fileManager.isDirectory(filesystem, a));
assertTrue(fileManager.isDirectory(filesystem, c));
assertTrue(fileManager.isDirectory(filesystem, root));
// File in c should still exist
assertTrue(fileManager.exists(filesystem, fileInC));
}

}
8 changes: 8 additions & 0 deletions src/test/java/org/commonjava/service/storage/StorageIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ protected void prepareFile( InputStream in, String filesystem, String path ) thr
}
}

protected void createEmptyDirectory(String filesystem, String dir) throws Exception {
String dummyFile = dir + "/.keep";
try (OutputStream out = fileManager.openOutputStream(filesystem, dummyFile)) {
out.write(0);
}
fileManager.delete(filesystem, dummyFile);
}

/**
* Override this if your test case don't need to prepare the PATH
* @return
Expand Down
Loading