From 4abcc610d5838c6b75180ab6b83d587eb055e946 Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:17:17 +0200 Subject: [PATCH] refactor(retention): JarRetentionJob deletes via ArtifactStore --- .../server/app/retention/JarRetentionJob.java | 56 +++++++----------- .../app/retention/JarRetentionJobTest.java | 57 +++++++++++++++++++ 2 files changed, 76 insertions(+), 37 deletions(-) create mode 100644 cameleer-server-app/src/test/java/com/cameleer/server/app/retention/JarRetentionJobTest.java diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/retention/JarRetentionJob.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/retention/JarRetentionJob.java index bbaeed83..a0b465d7 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/retention/JarRetentionJob.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/retention/JarRetentionJob.java @@ -1,15 +1,13 @@ package com.cameleer.server.app.retention; import com.cameleer.server.core.runtime.*; +import com.cameleer.server.core.storage.ArtifactStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Comparator; import java.util.List; import java.util.Set; import java.util.UUID; @@ -29,15 +27,18 @@ public class JarRetentionJob { private final AppService appService; private final AppVersionRepository versionRepo; private final DeploymentRepository deploymentRepo; + private final ArtifactStore store; public JarRetentionJob(EnvironmentService environmentService, AppService appService, AppVersionRepository versionRepo, - DeploymentRepository deploymentRepo) { + DeploymentRepository deploymentRepo, + ArtifactStore store) { this.environmentService = environmentService; this.appService = appService; this.versionRepo = versionRepo; this.deploymentRepo = deploymentRepo; + this.store = store; } @Scheduled(cron = "0 0 3 * * *") // 03:00 every day @@ -51,7 +52,6 @@ public class JarRetentionJob { log.debug("Environment {} has unlimited retention, skipping", env.slug()); continue; } - for (App app : appService.listByEnvironment(env.id())) { totalDeleted += cleanupApp(app, retentionCount); } @@ -64,49 +64,31 @@ public class JarRetentionJob { List versions = versionRepo.findByAppId(app.id()); // ordered DESC by version if (versions.size() <= retentionCount) return 0; - // Find version IDs that are currently deployed (any status) Set deployedVersionIds = deploymentRepo.findByAppId(app.id()).stream() - .map(Deployment::appVersionId) - .collect(Collectors.toSet()); + .map(Deployment::appVersionId).collect(Collectors.toSet()); int deleted = 0; - // versions is sorted DESC — skip the first retentionCount, delete the rest for (int i = retentionCount; i < versions.size(); i++) { AppVersion version = versions.get(i); if (deployedVersionIds.contains(version.id())) { - log.debug("Skipping deployed version v{} of app {} ({})", version.version(), app.slug(), version.id()); + log.debug("Skipping deployed version v{} of app {} ({})", + version.version(), app.slug(), version.id()); continue; } - - // Delete JAR from disk - deleteJarFile(version); - - // Delete DB record + try { + store.delete(appService.coordinatesFor(version)); + } catch (IOException e) { + // Don't abort the sweep for one app's IO error — match the legacy + // log-and-continue behavior. The DB row still gets cleaned up since + // the JAR is no longer pointed at by anything (FilesystemArtifactStore.delete + // already handles the racy parent-sweep gracefully). + log.warn("Failed to delete artifact for version {}: {}", version.id(), e.getMessage()); + } versionRepo.delete(version.id()); deleted++; - log.info("Deleted version v{} of app {} ({}) — JAR: {}", version.version(), app.slug(), version.id(), version.jarPath()); + log.info("Deleted version v{} of app {} ({})", + version.version(), app.slug(), version.id()); } - return deleted; } - - private void deleteJarFile(AppVersion version) { - try { - Path jarPath = Path.of(version.jarPath()); - if (Files.exists(jarPath)) { - Files.delete(jarPath); - // Try to remove the empty version directory - Path versionDir = jarPath.getParent(); - if (versionDir != null && Files.isDirectory(versionDir)) { - try (var entries = Files.list(versionDir)) { - if (entries.findFirst().isEmpty()) { - Files.delete(versionDir); - } - } - } - } - } catch (IOException e) { - log.warn("Failed to delete JAR file for version {}: {}", version.id(), e.getMessage()); - } - } } diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/retention/JarRetentionJobTest.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/retention/JarRetentionJobTest.java new file mode 100644 index 00000000..0c9cb14f --- /dev/null +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/retention/JarRetentionJobTest.java @@ -0,0 +1,57 @@ +package com.cameleer.server.app.retention; + +import com.cameleer.server.core.runtime.App; +import com.cameleer.server.core.runtime.AppService; +import com.cameleer.server.core.runtime.AppVersion; +import com.cameleer.server.core.runtime.AppVersionRepository; +import com.cameleer.server.core.runtime.DeploymentRepository; +import com.cameleer.server.core.runtime.Environment; +import com.cameleer.server.core.runtime.EnvironmentService; +import com.cameleer.server.core.storage.ArtifactCoordinates; +import com.cameleer.server.core.storage.ArtifactStore; +import org.junit.jupiter.api.Test; + +import java.time.Instant; +import java.util.List; +import java.util.UUID; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +class JarRetentionJobTest { + + @Test + void cleanupDeletesViaArtifactStore() throws Exception { + EnvironmentService envSvc = mock(EnvironmentService.class); + AppService appSvc = mock(AppService.class); + AppVersionRepository versionRepo = mock(AppVersionRepository.class); + DeploymentRepository deployRepo = mock(DeploymentRepository.class); + ArtifactStore store = mock(ArtifactStore.class); + + UUID envId = UUID.randomUUID(); + UUID appId = UUID.randomUUID(); + Instant now = Instant.now(); + Environment env = new Environment(envId, "dev", "Dev", false, true, + null, 2, "slate", now, 7, 7, 7); + App app = new App(appId, envId, "demo", "Demo", null, now, now); + // versions ordered DESC by version + AppVersion v3 = new AppVersion(UUID.randomUUID(), appId, 3, "ignored", "h", "a.jar", 1L, null, null, null); + AppVersion v2 = new AppVersion(UUID.randomUUID(), appId, 2, "ignored", "h", "a.jar", 1L, null, null, null); + AppVersion v1 = new AppVersion(UUID.randomUUID(), appId, 1, "ignored", "h", "a.jar", 1L, null, null, null); + + when(envSvc.listAll()).thenReturn(List.of(env)); + when(appSvc.listByEnvironment(envId)).thenReturn(List.of(app)); + when(versionRepo.findByAppId(appId)).thenReturn(List.of(v3, v2, v1)); + when(deployRepo.findByAppId(appId)).thenReturn(List.of()); // none deployed + when(appSvc.coordinatesFor(v1)).thenReturn(new ArtifactCoordinates("default", appId, 1)); + + JarRetentionJob job = new JarRetentionJob(envSvc, appSvc, versionRepo, deployRepo, store); + job.cleanupOldVersions(); + + verify(store).delete(new ArtifactCoordinates("default", appId, 1)); + verify(versionRepo).delete(v1.id()); + verifyNoMoreInteractions(store); + } +}