Cucumber concurrency validation plus fix (#7379)

# Description of Changes

cucumber tests to run multiple threads of commands at same time 

---

## Checklist

### General

- [ ] I have read the [Contribution
Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md)
- [ ] I have read the [Stirling-PDF Developer
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md)
(if applicable)
- [ ] I have read the [How to add new languages to
Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md)
(if applicable)
- [ ] I have performed a self-review of my own code
- [ ] My changes generate no new warnings

### Documentation

- [ ] I have updated relevant docs on [Stirling-PDF's doc
repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/)
(if functionality has heavily changed)
- [ ] I have read the section [Add New Translation
Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags)
(for new translation tags only)

### Translations (if applicable)

- [ ] I ran
[`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md)

### UI Changes (if applicable)

- [ ] Screenshots or videos demonstrating the UI changes are attached
(e.g., as comments or direct attachments in the PR)

### Testing (if applicable)

- [ ] I have run `task check` to verify linters, typechecks, and tests
pass
- [ ] I have tested my changes locally. Refer to the [Testing
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing)
for more details.
This commit is contained in:
Anthony Stirling
2026-08-14 14:01:45 +01:00
committed by GitHub
parent 55087313b7
commit 0be10b2dff
46 changed files with 2422 additions and 58 deletions
@@ -2,7 +2,9 @@ package stirling.software.common.aop;
import java.io.IOException;
import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
@@ -273,6 +275,7 @@ public class AutoJobAspect {
// Store the fileId for later reference
pdfFile.setFileId(fileId);
recordPendingInputFile(fileId);
// Replace the original MultipartFile with our persistent copy
MultipartFile persistentFile = fileStorage.retrieveFile(fileId);
@@ -290,6 +293,29 @@ public class AutoJobAspect {
return originalArgs;
}
/**
* Queue an input copy for attribution to the job. The job id does not exist yet at this point,
* so {@link JobExecutorService} drains this list once it mints one.
*/
@SuppressWarnings("unchecked")
private void recordPendingInputFile(String fileId) {
try {
Object existing = request.getAttribute(JobExecutorService.PENDING_INPUT_FILE_IDS_ATTR);
List<String> ids;
if (existing instanceof List<?> list) {
ids = (List<String>) list;
} else {
ids = new ArrayList<>();
request.setAttribute(JobExecutorService.PENDING_INPUT_FILE_IDS_ATTR, ids);
}
ids.add(fileId);
} catch (RuntimeException ex) {
// Without a bound request the copy cannot be attributed; the periodic sweep is the
// only backstop, so make the miss visible rather than silently leaking the file.
log.warn("Could not record input copy {} for cleanup: {}", fileId, ex.getMessage());
}
}
private String getJobIdFromContext() {
try {
return (String) request.getAttribute("jobId");
@@ -52,6 +52,13 @@ public class JobResult {
/** Key/value metadata that survives the write-through into the shared job store. */
private final Map<String, String> metadata = new ConcurrentHashMap<>();
/**
* File ids of the persistent input copies made for this job. An async submit copies the upload
* into FileStorage so the job can still read it after the request returns; without tracking
* them here nothing would ever delete those copies.
*/
@JsonIgnore private final List<String> inputFileIds = new CopyOnWriteArrayList<>();
/**
* Create a new JobResult with the given job ID
*
@@ -167,6 +174,22 @@ public class JobResult {
return Collections.unmodifiableList(notes);
}
/** Record a persistent input copy so job cleanup deletes it alongside the results. */
public void addInputFileId(String fileId) {
if (fileId != null && !fileId.isBlank() && !inputFileIds.contains(fileId)) {
this.inputFileIds.add(fileId);
}
}
/**
* File ids of this job's persistent input copies.
*
* @return An unmodifiable view of the input file ids
*/
public List<String> getInputFileIds() {
return Collections.unmodifiableList(inputFileIds);
}
/** Attach a metadata value, e.g. a policy id so cluster peers can identify a policy run. */
public void putMetadata(String key, String value) {
if (key != null && value != null) {
@@ -179,6 +179,21 @@ public class FileStorage {
return fileStore.delete(fileId);
}
/**
* Delete a stored file without the per-file ownership check.
*
* <p>Job cleanup authorises at the job level and then deletes that job's own files, so the
* deleter is legitimately not their owner - an admin sweeping every user's jobs, or the
* unauthenticated scheduled task. Routing those through {@link #deleteFile(String)} makes the
* ownership check throw and silently orphans the files on disk.
*
* <p>Only ever pass file ids read back off a job that the caller has already been authorised
* for; never a caller-supplied id.
*/
public boolean deleteFileAsSystem(String fileId) {
return fileStore.delete(fileId);
}
public boolean fileExists(String fileId) {
enforceOwnership(fileId);
return fileStore.exists(fileId);
@@ -1,6 +1,7 @@
package stirling.software.common.service;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
@@ -33,6 +34,14 @@ import stirling.software.common.util.RegexPatternUtils;
@Slf4j
public class JobExecutorService {
/**
* Request attribute holding the FileStorage ids of persistent input copies made for the job
* about to be created. Populated before the job id exists (the aspect copies the upload while
* processing arguments), drained onto the JobResult as soon as the task is created so cleanup
* can delete them.
*/
public static final String PENDING_INPUT_FILE_IDS_ATTR = "autoJobPendingInputFileIds";
private final TaskManager taskManager;
private final FileStorage fileStorage;
private final HttpServletRequest request;
@@ -133,6 +142,7 @@ public class JobExecutorService {
resourceWeight);
taskManager.createTask(jobId);
registerPendingInputFiles(jobId);
final String capturedJobIdForQueue = jobId;
Supplier<Object> wrappedWork =
@@ -163,6 +173,7 @@ public class JobExecutorService {
return ResponseEntity.ok().body(new JobResponse<>(true, jobId, null));
} else if (async) {
taskManager.createTask(jobId);
registerPendingInputFiles(jobId);
final String capturedJobId = jobId;
@@ -484,4 +495,30 @@ public class JobExecutorService {
}
return baseJobId;
}
/**
* Hand the input copies made while processing arguments to the freshly created job, so job
* cleanup deletes them. Drains the attribute so a retry cannot attribute the same ids twice.
*/
@SuppressWarnings("unchecked")
private void registerPendingInputFiles(String jobId) {
if (request == null) {
return;
}
Object pending;
try {
pending = request.getAttribute(PENDING_INPUT_FILE_IDS_ATTR);
request.removeAttribute(PENDING_INPUT_FILE_IDS_ATTR);
} catch (RuntimeException ex) {
// No request bound to this thread (e.g. an internally dispatched job).
log.debug("Could not read pending input file ids: {}", ex.getMessage());
return;
}
if (!(pending instanceof List<?> ids)) {
return;
}
for (String fileId : (List<String>) ids) {
taskManager.registerInputFile(jobId, fileId);
}
}
}
@@ -17,6 +17,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
@@ -234,6 +235,24 @@ public class TaskManager {
return false;
}
/**
* Record a persistent input copy against a job so cleanup deletes it with the results.
*
* @param jobId The job ID
* @param fileId The FileStorage id of the input copy
* @return true if the job exists and the id was recorded
*/
public boolean registerInputFile(String jobId, String fileId) {
JobResult jobResult = jobResults.get(jobId);
if (jobResult == null) {
log.warn("Attempted to register an input file against non-existent job ID: {}", jobId);
return false;
}
jobResult.addInputFileId(fileId);
log.debug("Registered input file {} for job {}", fileId, jobId);
return true;
}
/** Attach metadata to a job and write it through to the shared store for cluster peers. */
public boolean putMetadata(String jobId, String key, String value) {
JobResult jobResult = jobResults.get(jobId);
@@ -329,25 +348,59 @@ public class TaskManager {
return jobResults.computeIfAbsent(jobId, JobResult::createNew);
}
/**
* What a cleanup pass removed. Returned by the on-demand cleanup so callers can assert on it.
*/
public record CleanupSummary(int jobsRemoved, int filesDeleted, int jobsRetained) {}
/** Clean up old completed job results. No-op in cluster mode; the backplane TTL owns expiry. */
public void cleanupOldJobs() {
public CleanupSummary cleanupOldJobs() {
if (clusterBackplane != null && !clusterBackplane.shouldRunLocalCleanup()) {
return;
return new CleanupSummary(0, 0, jobResults.size());
}
return cleanupJobs(false, jobId -> true);
}
/**
* Force-expire this node's finished jobs now, ignoring the age threshold. Jobs still running
* are left alone - deleting their files mid-flight would break them - and are reported as
* retained.
*
* <p>Unlike {@link #cleanupOldJobs()} this always runs locally: it is an explicit request to
* release this node's storage, not the scheduled sweep the backplane TTL owns.
*
* @param jobIdFilter Only jobs whose id passes this predicate are considered, so a caller can
* restrict the sweep to jobs the requester is allowed to touch
* @return What was removed
*/
public CleanupSummary cleanupFinishedJobsNow(Predicate<String> jobIdFilter) {
return cleanupJobs(true, jobIdFilter);
}
private CleanupSummary cleanupJobs(boolean force, Predicate<String> filter) {
LocalDateTime expiryThreshold =
LocalDateTime.now().minus(jobResultExpiryMinutes, ChronoUnit.MINUTES);
LocalDateTime pendingExpiryThreshold =
LocalDateTime.now().minus(pendingJobExpiryMinutes, ChronoUnit.MINUTES);
int removedCount = 0;
int filesDeleted = 0;
int retainedCount = 0;
try {
for (Map.Entry<String, JobResult> entry : jobResults.entrySet()) {
JobResult result = entry.getValue();
if (!filter.test(entry.getKey())) {
retainedCount++;
continue;
}
boolean expiredCompletedJob =
result.isComplete()
&& result.getCompletedAt() != null
&& result.getCompletedAt().isBefore(expiryThreshold);
&& (force
|| (result.getCompletedAt() != null
&& result.getCompletedAt()
.isBefore(expiryThreshold)));
boolean abandonedPendingJob =
!result.isComplete()
&& result.getCreatedAt() != null
@@ -360,7 +413,7 @@ public class TaskManager {
// Clean up file results
if (expiredCompletedJob) {
cleanupJobFiles(result, entry.getKey());
filesDeleted += cleanupJobFiles(result, entry.getKey());
}
// Remove the job result
@@ -369,15 +422,22 @@ public class TaskManager {
jobStore.delete(entry.getKey());
}
removedCount++;
} else {
retainedCount++;
}
}
if (removedCount > 0) {
log.info("Cleaned up {} expired job results", removedCount);
log.info(
"Cleaned up {} {} job results ({} files deleted)",
removedCount,
force ? "finished" : "expired",
filesDeleted);
}
} catch (Exception e) {
log.error("Error during job cleanup: {}", e.getMessage(), e);
}
return new CleanupSummary(removedCount, filesDeleted, retainedCount);
}
/** Mirror the in-memory {@code JobResult} into the cluster-visible {@link JobStore}. */
@@ -525,22 +585,43 @@ public class TaskManager {
}
}
/** Clean up files associated with a job result */
private void cleanupJobFiles(JobResult result, String jobId) {
/**
* Clean up files associated with a job result: both the results and the persistent input copy
* an async submit made of the upload.
*
* @return The number of files actually deleted
*/
private int cleanupJobFiles(JobResult result, String jobId) {
int deleted = 0;
// Clean up all result files
if (result.hasFiles()) {
for (ResultFile resultFile : result.getAllResultFiles()) {
try {
fileStorage.deleteFile(resultFile.getFileId());
} catch (Exception e) {
log.warn(
"Failed to delete file {} for job {}: {}",
resultFile.getFileId(),
jobId,
e.getMessage());
if (deleteJobFile(resultFile.getFileId(), jobId)) {
deleted++;
}
}
}
for (String inputFileId : result.getInputFileIds()) {
if (deleteJobFile(inputFileId, jobId)) {
deleted++;
}
}
return deleted;
}
/**
* Deletes as the system, not as the caller: an admin sweeping another user's jobs, or the
* scheduled task running with no security context, is not the file's owner, and the
* ownership-checked delete would throw and leave the file orphaned on disk. The job itself is
* already authorised by the time we get here, and these ids come off that job, not the request.
*/
private boolean deleteJobFile(String fileId, String jobId) {
try {
return fileStorage.deleteFileAsSystem(fileId);
} catch (Exception e) {
log.warn("Failed to delete file {} for job {}: {}", fileId, jobId, e.getMessage());
return false;
}
}
/** Find the ResultFile metadata for a given file ID by searching through all job results */
@@ -12,6 +12,7 @@ import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
import java.security.MessageDigest;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -54,6 +55,10 @@ public class GeneralUtils {
private final String DEFAULT_WEBUI_CONFIGS_DIR = "defaultWebUIConfigs";
private final String PYTHON_SCRIPTS_DIR = "python";
// Extracted once per run. Rewriting a script while another request is exec-ing it
// races wherever rename is not atomic, such as 9p or NFS bind mounts.
private final Map<String, Path> EXTRACTED_SCRIPTS = new ConcurrentHashMap<>();
private final RegexPatternUtils patternCache = RegexPatternUtils.getInstance();
// Valid size units used for convertSizeToBytes validation and parsing
private final Set<String> VALID_SIZE_UNITS = Set.of("B", "KB", "MB", "GB", "TB");
@@ -1025,17 +1030,30 @@ public class GeneralUtils {
}
Path scriptsDir = Path.of(InstallationPathConfig.getScriptsPath(), PYTHON_SCRIPTS_DIR);
Files.createDirectories(scriptsDir);
Path target = scriptsDir.resolve(scriptName);
ClassPathResource res =
new ClassPathResource("static/" + PYTHON_SCRIPTS_DIR + "/" + scriptName);
if (!res.exists()) {
log.error("Resource not found: {}", res.getPath());
throw new IOException("Resource not found: " + res.getPath());
Path cached = EXTRACTED_SCRIPTS.get(scriptName);
if (cached != null && Files.isRegularFile(cached)) {
return cached;
}
synchronized (EXTRACTED_SCRIPTS) {
cached = EXTRACTED_SCRIPTS.get(scriptName);
if (cached != null && Files.isRegularFile(cached)) {
return cached;
}
Files.createDirectories(scriptsDir);
ClassPathResource res =
new ClassPathResource("static/" + PYTHON_SCRIPTS_DIR + "/" + scriptName);
if (!res.exists()) {
log.error("Resource not found: {}", res.getPath());
throw new IOException("Resource not found: " + res.getPath());
}
copyResourceToFile(res, target);
EXTRACTED_SCRIPTS.put(scriptName, target);
return target;
}
copyResourceToFile(res, target);
return target;
}
/*
@@ -2,6 +2,7 @@ package stirling.software.common.service;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -65,6 +66,24 @@ class FileStorageOwnershipTest {
assertThrows(SecurityException.class, () -> fs.deleteFile(id));
}
@Test
void systemDeleteOfAnotherUsersFile_allowed_soJobCleanupDoesNotOrphanIt(@TempDir Path tempDir)
throws IOException {
// An admin sweeping every user's finished jobs is not the owner of their files. The
// ownership-checked delete throws there, which used to drop the job record and leave the
// files stranded on disk with nothing left able to reference them.
AtomicReference<String> user = new AtomicReference<>("alice");
FileStorage fs = newStorageWithCurrentUser(tempDir, user);
String id = fs.storeBytes("alice's file".getBytes(), "x.bin");
user.set("admin");
assertThrows(SecurityException.class, () -> fs.deleteFile(id));
assertTrue(fs.deleteFileAsSystem(id), "System delete must not be blocked by ownership");
user.set("alice");
assertThrows(IOException.class, () -> fs.retrieveBytes(id), "File should really be gone");
}
@Test
void anonymousRetrieveOfOwnedFile_allowed_noCurrentUserMeansNoCompare(@TempDir Path tempDir)
throws IOException {
@@ -0,0 +1,263 @@
package stirling.software.common.service;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.*;
import java.time.LocalDateTime;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.http.MediaType;
import org.springframework.test.util.ReflectionTestUtils;
import stirling.software.common.cluster.ClusterBackplane;
import stirling.software.common.cluster.JobStore;
import stirling.software.common.model.job.JobResult;
import stirling.software.common.model.job.ResultFile;
/**
* Covers the on-demand cleanup path and the input-copy tracking that makes it complete. An async
* submit persists a copy of the upload as well as its results; before both were tracked, only the
* results were ever deleted and the input copy stayed on disk indefinitely.
*/
class TaskManagerCleanupTest {
@Mock private FileStorage fileStorage;
@Mock private JobStore jobStore;
@Mock private ClusterBackplane clusterBackplane;
@InjectMocks private TaskManager taskManager;
private AutoCloseable closeable;
@BeforeEach
void setUp() {
closeable = MockitoAnnotations.openMocks(this);
lenient().when(clusterBackplane.localNodeId()).thenReturn("test-node");
lenient().when(clusterBackplane.shouldRunLocalCleanup()).thenReturn(true);
lenient().when(fileStorage.deleteFileAsSystem(anyString())).thenReturn(true);
ReflectionTestUtils.setField(taskManager, "jobResultExpiryMinutes", 30);
ReflectionTestUtils.setField(taskManager, "pendingJobExpiryMinutes", 1440);
}
@AfterEach
void tearDown() throws Exception {
closeable.close();
}
@SuppressWarnings("unchecked")
private Map<String, JobResult> jobResults() {
return (Map<String, JobResult>) ReflectionTestUtils.getField(taskManager, "jobResults");
}
/** Complete a job with a single result file, as an async file-producing job would. */
private void completeWithFile(String jobId, String fileId) {
taskManager.setFileResult(jobId, fileId, "out.pdf", MediaType.APPLICATION_PDF_VALUE);
taskManager.setComplete(jobId);
}
@Test
void forcedCleanupRemovesFinishedJobsRegardlessOfAge() {
String jobId = "fresh-job";
taskManager.createTask(jobId);
completeWithFile(jobId, "result-file");
// The scheduled sweep leaves it alone: it completed well inside the retention window.
taskManager.cleanupOldJobs();
assertTrue(jobResults().containsKey(jobId), "Scheduled cleanup should respect the expiry");
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(1, summary.jobsRemoved());
assertEquals(1, summary.filesDeleted());
assertEquals(0, summary.jobsRetained());
assertFalse(jobResults().containsKey(jobId));
verify(fileStorage).deleteFileAsSystem("result-file");
verify(jobStore).delete(jobId);
}
@Test
void forcedCleanupDeletesThePersistedInputCopy() {
String jobId = "job-with-input";
taskManager.createTask(jobId);
assertTrue(taskManager.registerInputFile(jobId, "input-file"));
completeWithFile(jobId, "result-file");
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(1, summary.jobsRemoved());
assertEquals(2, summary.filesDeleted(), "Both the result and the input copy must go");
verify(fileStorage).deleteFileAsSystem("result-file");
verify(fileStorage).deleteFileAsSystem("input-file");
}
@Test
void scheduledCleanupAlsoDeletesThePersistedInputCopy() {
String jobId = "expired-job";
taskManager.createTask(jobId);
taskManager.registerInputFile(jobId, "input-file");
completeWithFile(jobId, "result-file");
JobResult result = taskManager.getJobResult(jobId);
ReflectionTestUtils.setField(result, "completedAt", LocalDateTime.now().minusHours(1));
taskManager.cleanupOldJobs();
assertFalse(jobResults().containsKey(jobId));
verify(fileStorage).deleteFileAsSystem("result-file");
verify(fileStorage).deleteFileAsSystem("input-file");
}
@Test
void forcedCleanupLeavesRunningJobsAlone() {
String running = "running-job";
taskManager.createTask(running);
taskManager.registerInputFile(running, "in-flight-input");
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(0, summary.jobsRemoved());
assertEquals(0, summary.filesDeleted());
assertEquals(1, summary.jobsRetained());
assertTrue(jobResults().containsKey(running));
// Deleting a running job's input mid-flight would break it.
verify(fileStorage, never()).deleteFileAsSystem(anyString());
}
@Test
void forcedCleanupSkipsJobsTheFilterRejects() {
taskManager.createTask("alice:job");
completeWithFile("alice:job", "alice-file");
taskManager.createTask("bob:job");
completeWithFile("bob:job", "bob-file");
TaskManager.CleanupSummary summary =
taskManager.cleanupFinishedJobsNow(id -> id.startsWith("alice:"));
assertEquals(1, summary.jobsRemoved());
assertEquals(1, summary.jobsRetained());
assertFalse(jobResults().containsKey("alice:job"));
assertTrue(jobResults().containsKey("bob:job"), "Another user's job must survive");
verify(fileStorage).deleteFileAsSystem("alice-file");
verify(fileStorage, never()).deleteFileAsSystem("bob-file");
}
@Test
void forcedCleanupIsIdempotent() {
String jobId = "job-to-clean";
taskManager.createTask(jobId);
taskManager.registerInputFile(jobId, "input-file");
completeWithFile(jobId, "result-file");
taskManager.cleanupFinishedJobsNow(id -> true);
TaskManager.CleanupSummary second = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(0, second.jobsRemoved());
assertEquals(0, second.filesDeleted());
}
@Test
void forcedCleanupRunsEvenWhenTheBackplaneOwnsScheduledExpiry() {
// The scheduled sweep defers to the backplane TTL in cluster mode, but an explicit
// request to release this node's storage still has to do something.
when(clusterBackplane.shouldRunLocalCleanup()).thenReturn(false);
String jobId = "clustered-job";
taskManager.createTask(jobId);
completeWithFile(jobId, "result-file");
taskManager.cleanupOldJobs();
assertTrue(jobResults().containsKey(jobId));
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(1, summary.jobsRemoved());
assertFalse(jobResults().containsKey(jobId));
}
@Test
void cleanupCountsOnlyFilesThatWereActuallyDeleted() {
// A file already gone (a retry deleted it, say) must not be counted as freed.
String jobId = "partially-cleaned";
taskManager.createTask(jobId);
taskManager.registerInputFile(jobId, "already-gone");
completeWithFile(jobId, "result-file");
when(fileStorage.deleteFileAsSystem("already-gone")).thenReturn(false);
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(1, summary.filesDeleted());
}
@Test
void cleanupSurvivesAFileStorageFailure() {
String jobId = "job-with-unhappy-storage";
taskManager.createTask(jobId);
taskManager.registerInputFile(jobId, "input-file");
completeWithFile(jobId, "result-file");
when(fileStorage.deleteFileAsSystem("result-file"))
.thenThrow(new RuntimeException("disk on fire"));
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
// The job is still released and the remaining file still deleted.
assertEquals(1, summary.jobsRemoved());
assertEquals(1, summary.filesDeleted());
assertFalse(jobResults().containsKey(jobId));
verify(fileStorage).deleteFileAsSystem("input-file");
}
@Test
void registerInputFileRejectsAnUnknownJob() {
assertFalse(taskManager.registerInputFile("no-such-job", "input-file"));
}
@Test
void registerInputFileIgnoresDuplicatesAndBlanks() {
String jobId = "dedupe-job";
taskManager.createTask(jobId);
taskManager.registerInputFile(jobId, "input-file");
taskManager.registerInputFile(jobId, "input-file");
taskManager.registerInputFile(jobId, " ");
taskManager.registerInputFile(jobId, null);
List<String> inputFileIds = taskManager.getJobResult(jobId).getInputFileIds();
assertEquals(List.of("input-file"), inputFileIds);
}
@Test
void multiFileResultsAndTheInputCopyAreAllDeleted() {
String jobId = "split-job";
taskManager.createTask(jobId);
taskManager.registerInputFile(jobId, "input-file");
JobResult result = taskManager.getJobResult(jobId);
result.completeWithFiles(
List.of(
ResultFile.builder()
.fileId("page-1")
.fileName("1.pdf")
.contentType(MediaType.APPLICATION_PDF_VALUE)
.fileSize(10L)
.build(),
ResultFile.builder()
.fileId("page-2")
.fileName("2.pdf")
.contentType(MediaType.APPLICATION_PDF_VALUE)
.fileSize(10L)
.build()));
TaskManager.CleanupSummary summary = taskManager.cleanupFinishedJobsNow(id -> true);
assertEquals(3, summary.filesDeleted());
verify(fileStorage).deleteFileAsSystem("page-1");
verify(fileStorage).deleteFileAsSystem("page-2");
verify(fileStorage).deleteFileAsSystem("input-file");
}
}
@@ -290,7 +290,8 @@ class TaskManagerMoreTest {
ReflectionTestUtils.setField(job, "complete", true);
ReflectionTestUtils.setField(job, "completedAt", LocalDateTime.now().minusHours(2));
when(fileStorage.deleteFile("doomed")).thenThrow(new RuntimeException("locked"));
when(fileStorage.deleteFileAsSystem("doomed"))
.thenThrow(new RuntimeException("locked"));
// Must not propagate; the job is still removed afterwards.
taskManager.cleanupOldJobs();
@@ -258,7 +258,7 @@ class TaskManagerTest {
.build();
ReflectionTestUtils.setField(oldJob, "resultFiles", java.util.List.of(resultFile));
when(fileStorage.deleteFile("file-id")).thenReturn(true);
when(fileStorage.deleteFileAsSystem("file-id")).thenReturn(true);
// Obtain access to the private jobResults map
Map<String, JobResult> jobResultsMap =
@@ -281,7 +281,7 @@ class TaskManagerTest {
assertFalse(jobResultsMap.containsKey(oldJobId));
assertTrue(jobResultsMap.containsKey(recentJobId));
assertTrue(jobResultsMap.containsKey(activeJobId));
verify(fileStorage).deleteFile("file-id");
verify(fileStorage).deleteFileAsSystem("file-id");
}
@Test
@@ -308,7 +308,7 @@ class TaskManagerTest {
// Assert: nothing was removed locally, and no jobStore.delete was issued.
assertTrue(jobResultsMap.containsKey(oldJobId));
verify(jobStore, never()).delete(anyString());
verify(fileStorage, never()).deleteFile(anyString());
verify(fileStorage, never()).deleteFileAsSystem(anyString());
}
@Test
@@ -12,6 +12,7 @@ import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
@@ -213,6 +214,35 @@ public class JobController {
}
}
/**
* Self-service counterpart to the admin-only {@code POST /api/v1/admin/job/cleanup}: that one
* sweeps every user's jobs and needs ROLE_ADMIN, this one releases only the caller's own and so
* is safe for any authenticated user. Both run the same sweep inside {@link TaskManager}.
*/
@PostMapping("/jobs/cleanup")
@Operation(
summary = "Release finished jobs and their stored files now",
description =
"Force-expires this node's finished jobs instead of waiting out the retention"
+ " window, deleting their result files and the persistent copies made of"
+ " their inputs. Only jobs the caller may access are touched, and jobs"
+ " still running are left alone. Admins can sweep every user's jobs"
+ " with POST /api/v1/admin/job/cleanup?force=true.")
public ResponseEntity<?> cleanupFinishedJobs() {
TaskManager.CleanupSummary summary =
taskManager.cleanupFinishedJobsNow(this::validateJobAccess);
log.info(
"On-demand job cleanup removed {} job(s) and {} file(s), retained {} job(s)",
summary.jobsRemoved(),
summary.filesDeleted(),
summary.jobsRetained());
return ResponseEntity.ok(
Map.of(
"jobsRemoved", summary.jobsRemoved(),
"filesDeleted", summary.filesDeleted(),
"jobsRetained", summary.jobsRetained()));
}
@GetMapping("/job/{jobId}/result/files")
@Operation(summary = "Get job result files")
public ResponseEntity<?> getJobFiles(@PathVariable("jobId") String jobId) {
@@ -61,7 +61,10 @@ class ToolIODeclarationCoverageTest {
// signing tool itself is /api/v1/security/cert-sign, which is declared.
"/api/v1/security/cert-sign/sessions",
"/api/v1/security/cert-sign/validate-certificate",
"/api/v1/security/cert-sign/hardware");
"/api/v1/security/cert-sign/hardware",
// Releases finished jobs and their stored files; server maintenance, takes and
// returns no document.
"/api/v1/general/jobs/cleanup");
private record Scan(Set<String> required, Map<String, ToolIOSpec> declared) {}
@@ -0,0 +1,78 @@
package stirling.software.SPDF.util;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import stirling.software.common.util.GeneralUtils;
/**
* Lives in core because the python resources it extracts ship in this module.
*
* <p>Callers hand the returned path straight to python3, so the file has to stay readable while
* other requests are extracting it. Re-writing it per call used to break that wherever rename is
* not atomic, such as a 9p or NFS bind mount.
*/
class ExtractScriptConcurrencyTest {
@Test
@DisplayName("concurrent callers always get a readable script")
void concurrentCallersSeeAReadableScript() throws Exception {
int threads = 16;
CountDownLatch start = new CountDownLatch(1);
CountDownLatch done = new CountDownLatch(threads);
AtomicInteger unreadable = new AtomicInteger();
AtomicInteger errors = new AtomicInteger();
for (int i = 0; i < threads; i++) {
Thread.ofVirtual()
.start(
() -> {
try {
start.await();
for (int n = 0; n < 25; n++) {
Path script = GeneralUtils.extractScript("png_to_webp.py");
if (!Files.isReadable(script)) {
unreadable.incrementAndGet();
}
}
} catch (Exception e) {
errors.incrementAndGet();
} finally {
done.countDown();
}
});
}
start.countDown();
assertTrue(done.await(60, TimeUnit.SECONDS), "extractScript threads did not finish");
assertEquals(0, errors.get(), "extractScript threw under concurrency");
assertEquals(0, unreadable.get(), "script was missing while another caller held it");
}
@Test
@DisplayName("repeated calls return the same path without rewriting the file")
void repeatedCallsAreStable() throws Exception {
Path first = GeneralUtils.extractScript("png_to_webp.py");
long modified = Files.getLastModifiedTime(first).toMillis();
long size = Files.size(first);
// Any rewrite after this pause lands on a later timestamp, so mtime is a
// reliable signal rather than a same-millisecond coin flip.
Thread.sleep(50);
for (int i = 0; i < 5; i++) {
assertEquals(first, GeneralUtils.extractScript("png_to_webp.py"));
}
assertEquals(modified, Files.getLastModifiedTime(first).toMillis(), "script was rewritten");
assertEquals(size, Files.size(first));
}
}
@@ -5,9 +5,11 @@ import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.*;
import java.util.Map;
import java.util.function.Predicate;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -442,4 +444,52 @@ class JobControllerTest {
assertEquals(HttpStatus.FORBIDDEN, response.getStatusCode());
verify(fileStorage, never()).getFileSize(eq(fileId));
}
@Test
void testCleanupFinishedJobs_ReportsWhatWasReleased() {
when(taskManager.cleanupFinishedJobsNow(any()))
.thenReturn(new TaskManager.CleanupSummary(2, 5, 1));
ResponseEntity<?> response = controller.cleanupFinishedJobs();
assertEquals(HttpStatus.OK, response.getStatusCode());
@SuppressWarnings("unchecked")
Map<String, Object> body = (Map<String, Object>) response.getBody();
assertEquals(2, body.get("jobsRemoved"));
assertEquals(5, body.get("filesDeleted"));
assertEquals(1, body.get("jobsRetained"));
}
@Test
void testCleanupFinishedJobs_OnlySweepsJobsTheCallerOwns() {
ReflectionTestUtils.setField(controller, "jobOwnershipService", jobOwnershipService);
when(jobOwnershipService.validateJobAccess("me:job")).thenReturn(true);
when(jobOwnershipService.validateJobAccess("someone-else:job"))
.thenThrow(new SecurityException("not yours"));
when(taskManager.cleanupFinishedJobsNow(any()))
.thenReturn(new TaskManager.CleanupSummary(1, 1, 1));
controller.cleanupFinishedJobs();
@SuppressWarnings("unchecked")
ArgumentCaptor<Predicate<String>> filter = ArgumentCaptor.forClass(Predicate.class);
verify(taskManager).cleanupFinishedJobsNow(filter.capture());
assertTrue(filter.getValue().test("me:job"));
assertFalse(
filter.getValue().test("someone-else:job"),
"A job the caller cannot access must be left in place");
}
@Test
void testCleanupFinishedJobs_SweepsEverythingWhenSecurityIsDisabled() {
when(taskManager.cleanupFinishedJobsNow(any()))
.thenReturn(new TaskManager.CleanupSummary(3, 3, 0));
controller.cleanupFinishedJobs();
@SuppressWarnings("unchecked")
ArgumentCaptor<Predicate<String>> filter = ArgumentCaptor.forClass(Predicate.class);
verify(taskManager).cleanupFinishedJobsNow(filter.capture());
assertTrue(filter.getValue().test("any-job-id"));
}
}
@@ -7,6 +7,7 @@ import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import io.swagger.v3.oas.annotations.Operation;
@@ -66,28 +67,40 @@ public class AdminJobController {
}
/**
* Manually trigger cleanup of old jobs (admin only)
* Manually trigger cleanup of old jobs (admin only). Covers every user's jobs, unlike the
* self-service {@code POST /api/v1/general/jobs/cleanup}, which only releases the caller's own.
*
* @return A response indicating how many jobs were cleaned up
* @param force Ignore the retention window and release every finished job now, rather than only
* those already past it
* @return A response indicating how many jobs and files were cleaned up
*/
@PostMapping("/job/cleanup")
@Operation(summary = "Cleanup old jobs")
@Operation(
summary = "Cleanup old jobs",
description =
"Runs the job retention sweep now across all users. With force=true the"
+ " retention window is ignored and every finished job is released"
+ " immediately.")
@PreAuthorize("hasRole('ADMIN')")
public ResponseEntity<?> cleanupOldJobs() {
int beforeCount = taskManager.getJobStats().getTotalJobs();
taskManager.cleanupOldJobs();
int afterCount = taskManager.getJobStats().getTotalJobs();
int removedCount = beforeCount - afterCount;
public ResponseEntity<?> cleanupOldJobs(
@RequestParam(name = "force", defaultValue = "false") boolean force) {
TaskManager.CleanupSummary summary =
force
? taskManager.cleanupFinishedJobsNow(jobId -> true)
: taskManager.cleanupOldJobs();
log.info(
"Admin triggered job cleanup: removed {} jobs, {} remaining",
removedCount,
afterCount);
"Admin triggered job cleanup (force={}): removed {} jobs and {} files, {} remaining",
force,
summary.jobsRemoved(),
summary.filesDeleted(),
summary.jobsRetained());
return ResponseEntity.ok(
Map.of(
"message", "Cleanup complete",
"removedJobs", removedCount,
"remainingJobs", afterCount));
"removedJobs", summary.jobsRemoved(),
"filesDeleted", summary.filesDeleted(),
"remainingJobs", summary.jobsRetained()));
}
}
@@ -0,0 +1,89 @@
package stirling.software.proprietary.controller.api;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import stirling.software.common.service.JobQueue;
import stirling.software.common.service.TaskManager;
/**
* The admin sweep covers every user's jobs, so it must stay distinct from the self-service endpoint
* on JobController - and both must run the same TaskManager sweep rather than each growing their
* own cleanup logic.
*/
class AdminJobControllerTest {
@Mock private TaskManager taskManager;
@Mock private JobQueue jobQueue;
@InjectMocks private AdminJobController controller;
private AutoCloseable closeable;
@BeforeEach
void setUp() {
closeable = MockitoAnnotations.openMocks(this);
}
@Test
void cleanupWithoutForceRunsTheRetentionSweep() throws Exception {
when(taskManager.cleanupOldJobs()).thenReturn(new TaskManager.CleanupSummary(2, 4, 3));
ResponseEntity<?> response = controller.cleanupOldJobs(false);
assertEquals(HttpStatus.OK, response.getStatusCode());
@SuppressWarnings("unchecked")
Map<String, Object> body = (Map<String, Object>) response.getBody();
assertEquals(2, body.get("removedJobs"));
assertEquals(4, body.get("filesDeleted"));
assertEquals(3, body.get("remainingJobs"));
verify(taskManager).cleanupOldJobs();
verify(taskManager, never()).cleanupFinishedJobsNow(any());
closeable.close();
}
@Test
void cleanupWithForceIgnoresTheRetentionWindow() throws Exception {
when(taskManager.cleanupFinishedJobsNow(any()))
.thenReturn(new TaskManager.CleanupSummary(5, 9, 0));
ResponseEntity<?> response = controller.cleanupOldJobs(true);
assertEquals(HttpStatus.OK, response.getStatusCode());
@SuppressWarnings("unchecked")
Map<String, Object> body = (Map<String, Object>) response.getBody();
assertEquals(5, body.get("removedJobs"));
assertEquals(9, body.get("filesDeleted"));
verify(taskManager).cleanupFinishedJobsNow(any());
verify(taskManager, never()).cleanupOldJobs();
closeable.close();
}
@Test
void forcedAdminCleanupSweepsEveryUsersJobs() throws Exception {
when(taskManager.cleanupFinishedJobsNow(any()))
.thenReturn(new TaskManager.CleanupSummary(1, 1, 0));
controller.cleanupOldJobs(true);
@SuppressWarnings("unchecked")
org.mockito.ArgumentCaptor<java.util.function.Predicate<String>> filter =
org.mockito.ArgumentCaptor.forClass(java.util.function.Predicate.class);
verify(taskManager).cleanupFinishedJobsNow(filter.capture());
// Unlike the self-service endpoint, the admin sweep is not scoped to one caller.
assertTrue(filter.getValue().test("alice:job"));
assertTrue(filter.getValue().test("bob:job"));
closeable.close();
}
}