From 0be10b2dfffdaa9956fe96057bf31f188a0937c1 Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:01:45 +0100 Subject: [PATCH] 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. --- .github/workflows/nightly.yml | 79 +++ .gitignore | 1 + .taskfiles/cucumber.yml | 44 ++ Taskfile.yml | 3 + .../software/common/aop/AutoJobAspect.java | 26 + .../software/common/model/job/JobResult.java | 23 + .../software/common/service/FileStorage.java | 15 + .../common/service/JobExecutorService.java | 37 ++ .../software/common/service/TaskManager.java | 113 ++++- .../software/common/util/GeneralUtils.java | 36 +- .../service/FileStorageOwnershipTest.java | 19 + .../service/TaskManagerCleanupTest.java | 263 ++++++++++ .../common/service/TaskManagerMoreTest.java | 3 +- .../common/service/TaskManagerTest.java | 6 +- .../common/controller/JobController.java | 30 ++ .../config/ToolIODeclarationCoverageTest.java | 5 +- .../util/ExtractScriptConcurrencyTest.java | 78 +++ .../common/controller/JobControllerTest.java | 50 ++ .../controller/api/AdminJobController.java | 39 +- .../api/AdminJobControllerTest.java | 89 ++++ engine/scripts/generate_tool_models.py | 3 + testing/cucumber/behave.ini | 1 + testing/cucumber/features/analysis.feature | 8 + testing/cucumber/features/async_jobs.feature | 143 ++++++ testing/cucumber/features/attachments.feature | 5 + .../features/bookmarks_chapters.feature | 2 + .../cucumber/features/convert_nightly.feature | 53 ++ testing/cucumber/features/environment.py | 61 ++- testing/cucumber/features/examples.feature | 7 + testing/cucumber/features/external.feature | 3 + testing/cucumber/features/filter.feature | 6 + .../cucumber/features/form_advanced.feature | 1 + testing/cucumber/features/forms.feature | 35 ++ testing/cucumber/features/general.feature | 3 + testing/cucumber/features/general_new.feature | 60 +++ testing/cucumber/features/info_config.feature | 134 +++++ .../cucumber/features/merge_overlay.feature | 2 + testing/cucumber/features/misc_new.feature | 32 ++ .../cucumber/features/security_new.feature | 37 ++ .../features/steps/job_step_definitions.py | 171 +++++++ .../cucumber/features/steps/job_support.py | 22 + .../steps/parallel_step_definitions.py | 47 ++ .../features/steps/parallel_support.py | 478 ++++++++++++++++++ .../features/steps/step_definitions.py | 43 +- testing/cucumber/run-parallel.sh | 143 ++++++ testing/test.sh | 21 +- 46 files changed, 2422 insertions(+), 58 deletions(-) create mode 100644 .taskfiles/cucumber.yml create mode 100644 app/common/src/test/java/stirling/software/common/service/TaskManagerCleanupTest.java create mode 100644 app/core/src/test/java/stirling/software/SPDF/util/ExtractScriptConcurrencyTest.java create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/controller/api/AdminJobControllerTest.java create mode 100644 testing/cucumber/features/async_jobs.feature create mode 100644 testing/cucumber/features/convert_nightly.feature create mode 100644 testing/cucumber/features/info_config.feature create mode 100644 testing/cucumber/features/steps/job_step_definitions.py create mode 100644 testing/cucumber/features/steps/job_support.py create mode 100644 testing/cucumber/features/steps/parallel_step_definitions.py create mode 100644 testing/cucumber/features/steps/parallel_support.py create mode 100644 testing/cucumber/run-parallel.sh diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index ba4054f190..9981226acf 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -4,6 +4,11 @@ on: schedule: - cron: "0 2 * * *" # 2 AM UTC every night workflow_dispatch: + pull_request: + paths: + - .github/workflows/nightly.yml + - testing/cucumber/** + - docker/embedded/compose/test_cicd.yml concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -99,8 +104,13 @@ jobs: # Builds all desktop platforms on a schedule so the Rust dependency cache is # written on main, where PR and merge-queue tauri builds can restore it. + # + # The only job here still pinned to schedule/main: it primes a cache rather than + # testing anything, and Actions scopes a cache written on a PR branch to that PR + # alone, so a PR run costs three platform builds and produces nothing reusable. warm-tauri-cache: name: Warm Tauri Rust cache + if: github.event_name == 'schedule' || github.ref == 'refs/heads/main' permissions: contents: read pull-requests: write @@ -109,3 +119,72 @@ jobs: platform: all sign: false secrets: inherit + + # Runs the @nightly tag (conversion scenarios) plus a 10-shard concurrency run + # of every other feature. + cucumber-nightly: + name: Cucumber (nightly scenarios + full concurrency) + runs-on: ubuntu-latest + # Fork pull requests get no MAVEN_* secrets, so the image build cannot work. + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository + permissions: + contents: read + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Set up JDK 25 + uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0 + with: + java-version: "25" + distribution: "temurin" + + - name: Install uv + uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + enable-cache: true + cache-dependency-glob: | + engine/pyproject.toml + engine/uv.lock + + - name: Install Task + uses: go-task/setup-task@01a4adf9db2d14c1de7a560f09170b6e0df736aa # v2.1.0 + + - name: Start the fat image with login and storage enabled + run: docker compose -f docker/embedded/compose/test_cicd.yml up -d --build + env: + MAVEN_USER: ${{ secrets.MAVEN_USER }} + MAVEN_PASSWORD: ${{ secrets.MAVEN_PASSWORD }} + MAVEN_PUBLIC_URL: ${{ secrets.MAVEN_PUBLIC_URL }} + + - name: Wait for the server + # Throwaway key from test_cicd.yml; out of the header literal for gitleaks. + env: + TEST_API_KEY: "123456789" + run: | + curl --retry 90 --retry-delay 3 --retry-connrefused --retry-all-errors \ + -sf -H "X-API-KEY: $TEST_API_KEY" http://localhost:8080/api/v1/info/status + + # Heavy LibreOffice/Calibre/Ghostscript conversions, excluded from the PR run. + # Both tasks install the behave deps themselves, so there is no separate uv sync step. + - name: Run @nightly scenarios + run: task cucumber:nightly + + # Genuinely different payloads contending on one backend. + - name: Sharded concurrency validation + run: task cucumber:parallel SHARDS=10 + + - name: Container logs on failure + if: failure() + run: docker compose -f docker/embedded/compose/test_cicd.yml logs --tail 400 + + - name: Tear down + if: always() + run: docker compose -f docker/embedded/compose/test_cicd.yml down -v diff --git a/.gitignore b/.gitignore index a064973a3b..d2b52ea6f8 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ exampleYmlFiles/stirling/ /testing/file_snapshots /testing/cucumber/junit/ /testing/cucumber/report.html +/testing/cucumber/.parallel/ /testing/.failed_tests /.test-state/ SwaggerDoc.json diff --git a/.taskfiles/cucumber.yml b/.taskfiles/cucumber.yml new file mode 100644 index 0000000000..cb630abf85 --- /dev/null +++ b/.taskfiles/cucumber.yml @@ -0,0 +1,44 @@ +version: '3' + +tasks: + install: + desc: "Sync the Python environment with the cucumber test dependencies" + run: once + # Deliberately no sources/status fingerprint: the engine venv is shared, so it can + # already exist while synced to a different dependency group. uv no-ops when correct. + cmds: + - uv sync --project ../../engine --locked --group cucumber + + run: + desc: "Run the cucumber suite against a running server (BASE_URL, default localhost:8080)" + deps: [install] + cmds: + - uv run --project ../../engine --locked --group cucumber python -m behave --no-capture -f plain {{.CLI_ARGS}} + + nightly: + desc: "Run the @nightly cucumber scenarios, excluded from the default run" + summary: | + Heavy LibreOffice/Calibre/Ghostscript conversions. behave.ini excludes @nightly, + so this opts back in explicitly. + + Pass extra behave flags via -- : + task cucumber:nightly -- --tags=@convert + deps: [install] + cmds: + - uv run --project ../../engine --locked --group cucumber python -m behave --tags=@nightly --no-capture -f plain {{.CLI_ARGS}} + + parallel: + desc: "Run the cucumber suite as concurrent shards against one server (SHARDS, default 10)" + summary: | + Splits the feature files across SHARDS concurrent behave processes hitting a single + backend, to shake out cross-request interference. Auth-coupled features are pinned + to one shard because they change the admin password mid-scenario. + + task cucumber:parallel + task cucumber:parallel SHARDS=4 + BASE_URL=http://localhost:8081 task cucumber:parallel + deps: [install] + vars: + SHARDS: '{{.SHARDS | default "10"}}' + cmds: + - bash run-parallel.sh {{.SHARDS}} {{if .CLI_ARGS}}-- {{.CLI_ARGS}}{{end}} diff --git a/Taskfile.yml b/Taskfile.yml index 304a031ac0..fc7a564032 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -25,6 +25,9 @@ includes: e2e: taskfile: .taskfiles/e2e.yml dir: . + cucumber: + taskfile: .taskfiles/cucumber.yml + dir: testing/cucumber pre-commit: taskfile: .taskfiles/pre-commit.yml dir: . diff --git a/app/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java b/app/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java index adfad7704b..96293e9c3f 100644 --- a/app/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java +++ b/app/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java @@ -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 ids; + if (existing instanceof List list) { + ids = (List) 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"); diff --git a/app/common/src/main/java/stirling/software/common/model/job/JobResult.java b/app/common/src/main/java/stirling/software/common/model/job/JobResult.java index aa43431a15..b3f1f31472 100644 --- a/app/common/src/main/java/stirling/software/common/model/job/JobResult.java +++ b/app/common/src/main/java/stirling/software/common/model/job/JobResult.java @@ -52,6 +52,13 @@ public class JobResult { /** Key/value metadata that survives the write-through into the shared job store. */ private final Map 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 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 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) { diff --git a/app/common/src/main/java/stirling/software/common/service/FileStorage.java b/app/common/src/main/java/stirling/software/common/service/FileStorage.java index c5c1ddb5db..700a6e5a65 100644 --- a/app/common/src/main/java/stirling/software/common/service/FileStorage.java +++ b/app/common/src/main/java/stirling/software/common/service/FileStorage.java @@ -179,6 +179,21 @@ public class FileStorage { return fileStore.delete(fileId); } + /** + * Delete a stored file without the per-file ownership check. + * + *

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. + * + *

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); diff --git a/app/common/src/main/java/stirling/software/common/service/JobExecutorService.java b/app/common/src/main/java/stirling/software/common/service/JobExecutorService.java index f283f65763..eca4413350 100644 --- a/app/common/src/main/java/stirling/software/common/service/JobExecutorService.java +++ b/app/common/src/main/java/stirling/software/common/service/JobExecutorService.java @@ -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 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) ids) { + taskManager.registerInputFile(jobId, fileId); + } + } } diff --git a/app/common/src/main/java/stirling/software/common/service/TaskManager.java b/app/common/src/main/java/stirling/software/common/service/TaskManager.java index f504b39395..0d0f8c23aa 100644 --- a/app/common/src/main/java/stirling/software/common/service/TaskManager.java +++ b/app/common/src/main/java/stirling/software/common/service/TaskManager.java @@ -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. + * + *

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 jobIdFilter) { + return cleanupJobs(true, jobIdFilter); + } + + private CleanupSummary cleanupJobs(boolean force, Predicate 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 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 */ diff --git a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java index 52f79f733a..1d7320ca84 100644 --- a/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java +++ b/app/common/src/main/java/stirling/software/common/util/GeneralUtils.java @@ -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 EXTRACTED_SCRIPTS = new ConcurrentHashMap<>(); private final RegexPatternUtils patternCache = RegexPatternUtils.getInstance(); // Valid size units used for convertSizeToBytes validation and parsing private final Set 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; } /* diff --git a/app/common/src/test/java/stirling/software/common/service/FileStorageOwnershipTest.java b/app/common/src/test/java/stirling/software/common/service/FileStorageOwnershipTest.java index 861efa8509..9bf78b6af3 100644 --- a/app/common/src/test/java/stirling/software/common/service/FileStorageOwnershipTest.java +++ b/app/common/src/test/java/stirling/software/common/service/FileStorageOwnershipTest.java @@ -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 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 { diff --git a/app/common/src/test/java/stirling/software/common/service/TaskManagerCleanupTest.java b/app/common/src/test/java/stirling/software/common/service/TaskManagerCleanupTest.java new file mode 100644 index 0000000000..050004dc1f --- /dev/null +++ b/app/common/src/test/java/stirling/software/common/service/TaskManagerCleanupTest.java @@ -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 jobResults() { + return (Map) 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 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"); + } +} diff --git a/app/common/src/test/java/stirling/software/common/service/TaskManagerMoreTest.java b/app/common/src/test/java/stirling/software/common/service/TaskManagerMoreTest.java index 0a02039ab3..1871e31a3f 100644 --- a/app/common/src/test/java/stirling/software/common/service/TaskManagerMoreTest.java +++ b/app/common/src/test/java/stirling/software/common/service/TaskManagerMoreTest.java @@ -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(); diff --git a/app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java b/app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java index 9d880d3451..cb423d977c 100644 --- a/app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java +++ b/app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java @@ -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 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 diff --git a/app/core/src/main/java/stirling/software/common/controller/JobController.java b/app/core/src/main/java/stirling/software/common/controller/JobController.java index ef9e81873e..c6fec4b92b 100644 --- a/app/core/src/main/java/stirling/software/common/controller/JobController.java +++ b/app/core/src/main/java/stirling/software/common/controller/JobController.java @@ -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) { diff --git a/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java b/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java index f6fe52f2a6..d7d211a7ce 100644 --- a/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java +++ b/app/core/src/test/java/stirling/software/SPDF/config/ToolIODeclarationCoverageTest.java @@ -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 required, Map declared) {} diff --git a/app/core/src/test/java/stirling/software/SPDF/util/ExtractScriptConcurrencyTest.java b/app/core/src/test/java/stirling/software/SPDF/util/ExtractScriptConcurrencyTest.java new file mode 100644 index 0000000000..f0c3ff6f95 --- /dev/null +++ b/app/core/src/test/java/stirling/software/SPDF/util/ExtractScriptConcurrencyTest.java @@ -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. + * + *

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)); + } +} diff --git a/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java b/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java index 53212a2e65..2083e7339b 100644 --- a/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java +++ b/app/core/src/test/java/stirling/software/common/controller/JobControllerTest.java @@ -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 body = (Map) 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> 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> filter = ArgumentCaptor.forClass(Predicate.class); + verify(taskManager).cleanupFinishedJobsNow(filter.capture()); + assertTrue(filter.getValue().test("any-job-id")); + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AdminJobController.java b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AdminJobController.java index ef941c603f..fa9d6db141 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AdminJobController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/controller/api/AdminJobController.java @@ -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())); } } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/controller/api/AdminJobControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/controller/api/AdminJobControllerTest.java new file mode 100644 index 0000000000..95d2e097cf --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/controller/api/AdminJobControllerTest.java @@ -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 body = (Map) 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 body = (Map) 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> 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(); + } +} diff --git a/engine/scripts/generate_tool_models.py b/engine/scripts/generate_tool_models.py index 7587d8c62f..c3cd9e7267 100644 --- a/engine/scripts/generate_tool_models.py +++ b/engine/scripts/generate_tool_models.py @@ -149,6 +149,9 @@ class ToolDiscovery: "/api/v1/misc/add-image", "/api/v1/misc/add-attachments", "/api/v1/general/overlay-pdfs", + # 5. Server maintenance, not a document operation: releases finished jobs and + # their stored files. Nothing an edit agent should ever call on its own. + "/api/v1/general/jobs/cleanup", ) def _is_excluded(self, path: str) -> bool: diff --git a/testing/cucumber/behave.ini b/testing/cucumber/behave.ini index 0b2a9e4bac..f838c8dad2 100644 --- a/testing/cucumber/behave.ini +++ b/testing/cucumber/behave.ini @@ -3,6 +3,7 @@ # Run either directly, e.g. `python -m behave features/enterprise`. exclude_re = features/(enterprise|multinode) tags = ~@manual + ~@nightly [behave.formatters] # Registers the html report formatter (behave-html-formatter) used by run-multinode-regression.sh. diff --git a/testing/cucumber/features/analysis.feature b/testing/cucumber/features/analysis.feature index 0f7ed17b32..039dc31d76 100644 --- a/testing/cucumber/features/analysis.feature +++ b/testing/cucumber/features/analysis.feature @@ -15,6 +15,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains pages When I send the API request to the endpoint "/api/v1/analysis/page-count" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -35,6 +36,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 4 pages When I send the API request to the endpoint "/api/v1/analysis/basic-info" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -66,6 +68,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/analysis/document-properties" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -104,6 +107,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 2 pages with random text When I send the API request to the endpoint "/api/v1/analysis/page-dimensions" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -118,6 +122,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/analysis/form-fields" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -141,6 +146,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/analysis/annotation-info" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -164,6 +170,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/analysis/font-info" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -187,6 +194,7 @@ Feature: Analysis API Endpoints Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/analysis/security-info" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 diff --git a/testing/cucumber/features/async_jobs.feature b/testing/cucumber/features/async_jobs.feature new file mode 100644 index 0000000000..b0181bbe94 --- /dev/null +++ b/testing/cucumber/features/async_jobs.feature @@ -0,0 +1,143 @@ +@jobs +Feature: Asynchronous job API + + # Any tool endpoint accepts ?async=true and returns a jobId instead of the file. + # No parallel step here: every step after the submit depends on that one jobId. + + @positive + Scenario: An async job runs to completion and returns its result + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + And the request data includes + | parameter | value | + | angle | 90 | + When I send the API request to the endpoint "/api/v1/general/rotate-pdf?async=true" + Then the response status code should be 200 + And the response JSON field "async" should be true + + When I store the job id from the response + And I wait for the job to complete + Then the job should be reported complete + + When I request the job result + Then the response status code should be 200 + And the response content type should be "application/pdf" + And the response PDF should contain 3 pages + + + @positive + Scenario: Async job results are listed and downloadable as individual files + Given I generate a PDF file as "fileInput" + And the pdf contains 4 pages + And the request data includes + | parameter | value | + | pageNumbers | all | + When I send the API request to the endpoint "/api/v1/general/split-pages?async=true" + Then the response status code should be 200 + + When I store the job id from the response + And I wait for the job to complete + And I request the job result file list + Then the response status code should be 200 + And the job result file list should contain at least 4 file(s) + + When I request the first job result file metadata + Then the response status code should be 200 + And the response JSON field "fileName" should not be empty + + When I download the first job result file + Then the response status code should be 200 + And the response file should have size greater than 100 + + + @negative + Scenario: Cancelling an already-finished job is rejected + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + And the request data includes + | parameter | value | + | angle | 180 | + When I send the API request to the endpoint "/api/v1/general/rotate-pdf?async=true" + And I store the job id from the response + And I wait for the job to complete + And I cancel the job + Then the response status code should be 400 + + + @negative + Scenario: Polling a job id the caller does not own is forbidden + When I send a GET request to "/api/v1/general/job/does-not-exist-1234" + Then the response status code should be 403 + + + @negative + Scenario: Downloading an unknown file id is rejected + When I send a GET request to "/api/v1/general/files/does-not-exist-1234" + Then the response status code should be 404 + + + # An async submit persists a copy of the upload and its results under the server's + # file store. Those outlive the request by design, so the only proof they are not a + # leak is that cleanup actually removes them. These scenarios exercise that, and + # environment.after_all sweeps the rest so the post-run temp-file diff stays honest. + + @positive @cleanup + Scenario: Downloading a result does not consume it + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + And the request data includes + | parameter | value | + | angle | 90 | + When I send the API request to the endpoint "/api/v1/general/rotate-pdf?async=true" + And I store the job id from the response + And I wait for the job to complete + And I request the job result file list + Then the response status code should be 200 + + When I download the first job result file + Then the response status code should be 200 + And the response file should have size greater than 100 + # A download is a read, not a take: the file survives for a retry or a second client. + And the job result file should still be downloadable + + + @positive @cleanup + Scenario: Cleanup releases a finished job and deletes its stored files + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + And the request data includes + | parameter | value | + | pageNumbers | all | + When I send the API request to the endpoint "/api/v1/general/split-pages?async=true" + And I store the job id from the response + And I wait for the job to complete + And I request the job result file list + Then the response status code should be 200 + And the job result file list should contain at least 3 file(s) + + When I trigger the async job cleanup + Then the response status code should be 200 + # 3 split results plus the persisted copy of the upload. + And the cleanup should report at least 1 job(s) removed + And the cleanup should report at least 4 file(s) deleted + And the job should no longer exist + And the job result file should no longer be downloadable + + + @positive @cleanup + Scenario: A second cleanup finds nothing left behind + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + And the request data includes + | parameter | value | + | angle | 180 | + When I send the API request to the endpoint "/api/v1/general/rotate-pdf?async=true" + And I store the job id from the response + And I wait for the job to complete + And I trigger the async job cleanup + Then the response status code should be 200 + And the cleanup should report at least 1 job(s) removed + + When I trigger the async job cleanup + Then the response status code should be 200 + And the cleanup should report nothing left to remove diff --git a/testing/cucumber/features/attachments.feature b/testing/cucumber/features/attachments.feature index 6bb9cfd6ea..582abadda0 100644 --- a/testing/cucumber/features/attachments.feature +++ b/testing/cucumber/features/attachments.feature @@ -7,6 +7,7 @@ Feature: Attachments API Validation And the pdf contains 2 pages with random text And I also generate a PDF file as "attachments" When I send the API request to the endpoint "/api/v1/misc/add-attachments" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -30,6 +31,7 @@ Feature: Attachments API Validation And the pdf contains 2 pages And the pdf has an attachment named "test_doc.txt" When I send the API request to the endpoint "/api/v1/misc/list-attachments" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -49,6 +51,7 @@ Feature: Attachments API Validation And the pdf contains 2 pages And the pdf has an attachment named "report.txt" When I send the API request to the endpoint "/api/v1/misc/extract-attachments" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response file should have size greater than 0 @@ -62,6 +65,7 @@ Feature: Attachments API Validation | attachmentName | original.txt | | newName | renamed.txt | When I send the API request to the endpoint "/api/v1/misc/rename-attachment" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -76,6 +80,7 @@ Feature: Attachments API Validation | parameter | value | | attachmentName | to_delete.txt | When I send the API request to the endpoint "/api/v1/misc/delete-attachment" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 diff --git a/testing/cucumber/features/bookmarks_chapters.feature b/testing/cucumber/features/bookmarks_chapters.feature index 8bda991728..02fb33fb23 100644 --- a/testing/cucumber/features/bookmarks_chapters.feature +++ b/testing/cucumber/features/bookmarks_chapters.feature @@ -7,6 +7,7 @@ Feature: Bookmarks and Chapter Splitting API Validation And the pdf contains 3 pages with random text And the pdf has bookmarks When I send the API request to the endpoint "/api/v1/general/extract-bookmarks" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -31,6 +32,7 @@ Feature: Bookmarks and Chapter Splitting API Validation | includeMetadata | false | | allowDuplicates | false | When I send the API request to the endpoint "/api/v1/general/split-pdf-by-chapters" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response file should have size greater than 0 diff --git a/testing/cucumber/features/convert_nightly.feature b/testing/cucumber/features/convert_nightly.feature new file mode 100644 index 0000000000..eb30bdbbac --- /dev/null +++ b/testing/cucumber/features/convert_nightly.feature @@ -0,0 +1,53 @@ +@nightly @convert +Feature: Heavy conversion endpoints + + # Too slow for every PR: these shell out to LibreOffice, Calibre or Ghostscript. + # behave.ini excludes @nightly; the nightly job opts back in with --tags=@nightly. + + @pdf-to-xlsx @positive + Scenario: Convert a PDF containing tables to XLSX + Given I use an example file at "exampleFiles/tables.pdf" as parameter "fileInput" + When I send the API request to the endpoint "/api/v1/convert/pdf/xlsx" + And this operation is run 3 times in parallel + Then the response status code should be 200 + And the response file should have size greater than 1000 + And the response file should have extension ".xlsx" + + + @pdf-to-xlsx @positive + Scenario: A PDF with no tables converts without producing a spreadsheet + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages with random text + When I send the API request to the endpoint "/api/v1/convert/pdf/xlsx" + Then the response status code should be 204 + + + @text-editor @positive + Scenario: text-editor metadata describes the document for the editor + Given I use an example file at "exampleFiles/tables.pdf" as parameter "fileInput" + When I send the API request to the endpoint "/api/v1/convert/pdf/text-editor/metadata" + And this operation is run 3 times in parallel + Then the response status code should be 200 + And the response content type should be "application/json" + And the response JSON field "fonts" should not be empty + + + @vector @negative + Scenario: vector conversion rejects an input format Ghostscript cannot read + Given I generate an SVG file as "fileInput" + When I send the API request to the endpoint "/api/v1/convert/vector/pdf" + Then the response status code should be 400 + And the response JSON error should contain "Unsupported" + + + @ebook @positive + Scenario: An EPUB produced by Stirling converts back to PDF + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages with random text + And the request data includes + | parameter | value | + | outputFormat | EPUB | + When I send the API request to the endpoint "/api/v1/convert/pdf/epub" + And this operation is run 3 times in parallel + Then the response status code should be 200 + And the response file should have size greater than 200 diff --git a/testing/cucumber/features/environment.py b/testing/cucumber/features/environment.py index 208e45b23b..84d30a50bc 100644 --- a/testing/cucumber/features/environment.py +++ b/testing/cucumber/features/environment.py @@ -1,8 +1,13 @@ import os import subprocess +import sys import requests +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "steps")) +import job_support # noqa: E402 +import parallel_support # noqa: E402 + _BASE_URL = "http://localhost:8080" _CONTAINER_NAME = os.environ.get("TEST_CONTAINER_NAME", "") _REPORT_DIR = os.environ.get("TEST_REPORT_DIR", "") @@ -142,8 +147,17 @@ def before_all(context): def before_scenario(context, scenario): """Reset all per-scenario state before each scenario runs.""" - # Skip scenarios that require JWT Bearer auth when it is not functional. scenario_tags = set(scenario.effective_tags) + + # Concurrency is opted into by a step in the feature, never by configuration. + context.parallel_repeat = 1 + context.parallel_decoy = False + context.parallel_validated = False + context.parallel_ran_at = 0 + context.parallel_request = None + context.parallel_get = None + + # Skip scenarios that require JWT Bearer auth when it is not functional. if _JWT_DEPENDENT_TAGS & scenario_tags and not context.jwt_available: scenario.skip( "JWT Bearer authentication not available in this environment (V2 disabled). " @@ -217,3 +231,48 @@ def after_scenario(context, scenario): context.jwt_token = None context.original_jwt_token = None context._status_ok = False + context.parallel_request = None + context.parallel_get = None + + +def _cleanup_async_job_files(): + """Release every async job result the run left on the server. + + An async submit persists a copy of the upload plus its results, and both are held + for the job retention window (30 minutes by default) - far longer than a test run. + The regression check that diffs the container filesystem before and after this suite + would otherwise flag them as leaked temp files. Sweeping them here keeps that check + strict: anything it still reports afterwards is a genuine leak. + """ + try: + response = job_support.trigger_cleanup() + except Exception as exc: + print(f"\n[CLEANUP] Async job cleanup request failed: {exc}") + return + if response.status_code == 404: + print( + "\n[CLEANUP] Async job cleanup endpoint not available on this build; " + "async job files will age out on their own." + ) + return + if response.status_code != 200: + print( + f"\n[CLEANUP] Async job cleanup returned {response.status_code}: " + f"{response.text[:200]}" + ) + return + try: + summary = response.json() + except ValueError: + print("\n[CLEANUP] Async job cleanup returned a non-JSON body") + return + print( + f"\n[CLEANUP] Released {summary.get('jobsRemoved', '?')} async job(s) and " + f"{summary.get('filesDeleted', '?')} stored file(s); " + f"{summary.get('jobsRetained', '?')} retained." + ) + + +def after_all(context): + _cleanup_async_job_files() + parallel_support.print_summary() diff --git a/testing/cucumber/features/examples.feature b/testing/cucumber/features/examples.feature index e0ea72024e..186d882e1c 100644 --- a/testing/cucumber/features/examples.feature +++ b/testing/cucumber/features/examples.feature @@ -10,6 +10,7 @@ Feature: API Validation | parameter | value | | password | password123 | When I send the API request to the endpoint "/api/v1/security/remove-password" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response file should have size greater than 0 And the response PDF is not passworded @@ -31,6 +32,7 @@ Feature: API Validation Scenario: Get info Given I generate a PDF file as "fileInput" When I send the API request to the endpoint "/api/v1/security/get-info-on-pdf" + And this operation is run 5 times in parallel Then the response content type should be "application/json" And the response file should have size greater than 100 And the response status code should be 200 @@ -43,6 +45,7 @@ Feature: API Validation | parameter | value | | password | password123 | When I send the API request to the endpoint "/api/v1/security/add-password" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response file should have size greater than 100 And the response PDF is passworded @@ -81,6 +84,7 @@ Feature: API Validation | alphabet | roman | | customColor | #d3d3d3 | When I send the API request to the endpoint "/api/v1/security/add-watermark" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response file should have size greater than 100 And the response status code should be 200 @@ -94,6 +98,7 @@ Feature: API Validation | threshold | 90 | | whitePercent | 99.9 | When I send the API request to the endpoint "/api/v1/misc/remove-blanks" + And this operation is run 5 times in parallel Then the response content type should be "application/octet-stream" And the response file should have extension ".zip" And the response ZIP should contain 1 files @@ -106,6 +111,7 @@ Feature: API Validation | parameter | value | | flattenOnlyForms | false | When I send the API request to the endpoint "/api/v1/misc/flatten" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response file should have size greater than 0 And the response status code should be 200 @@ -121,6 +127,7 @@ Feature: API Validation | keywords | sample, test | | producer | Test Producer | When I send the API request to the endpoint "/api/v1/misc/update-metadata" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response file should have size greater than 0 And the response PDF metadata should include "Author" as "John Doe" diff --git a/testing/cucumber/features/external.feature b/testing/cucumber/features/external.feature index e8e8d85272..3257657068 100644 --- a/testing/cucumber/features/external.feature +++ b/testing/cucumber/features/external.feature @@ -174,6 +174,7 @@ Feature: API Validation | dpi | 300 | | imageFormat | | When I send the API request to the endpoint "/api/v1/convert/pdf/img" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response file should have size greater than 100 And the response file should have extension ".zip" @@ -231,6 +232,7 @@ Feature: API Validation Given I generate a PDF file as "fileInput" And the pdf contains 3 pages with random text When I send the API request to the endpoint "/api/v1/convert/pdf/markdown" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response file should have size greater than 100 And the response file should have extension ".md" @@ -244,6 +246,7 @@ Feature: API Validation | outputFormat | csv | | pageNumbers | all | When I send the API request to the endpoint "/api/v1/convert/pdf/csv" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response file should have size greater than 200 And the response file should have extension ".zip" diff --git a/testing/cucumber/features/filter.feature b/testing/cucumber/features/filter.feature index d0f34c8731..2caf1eacf1 100644 --- a/testing/cucumber/features/filter.feature +++ b/testing/cucumber/features/filter.feature @@ -18,6 +18,7 @@ Feature: Filter API Endpoints | pageCount | | | comparator | | When I send the API request to the endpoint "/api/v1/filter/filter-page-count" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -59,6 +60,7 @@ Feature: Filter API Endpoints | fileSize | | | comparator | | When I send the API request to the endpoint "/api/v1/filter/filter-file-size" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -93,6 +95,7 @@ Feature: Filter API Endpoints | rotation | 0 | | comparator | Equal | When I send the API request to the endpoint "/api/v1/filter/filter-page-rotation" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -170,6 +173,7 @@ Feature: Filter API Endpoints | standardPageSize | LETTER | | comparator | Equal | When I send the API request to the endpoint "/api/v1/filter/filter-page-size" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -242,6 +246,7 @@ Feature: Filter API Endpoints | text | FINDME | | pageNumbers | all | When I send the API request to the endpoint "/api/v1/filter/filter-contains-text" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -281,6 +286,7 @@ Feature: Filter API Endpoints | parameter | value | | pageNumbers | all | When I send the API request to the endpoint "/api/v1/filter/filter-contains-image" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 diff --git a/testing/cucumber/features/form_advanced.feature b/testing/cucumber/features/form_advanced.feature index bb02a453b4..c66e7df2e0 100644 --- a/testing/cucumber/features/form_advanced.feature +++ b/testing/cucumber/features/form_advanced.feature @@ -8,6 +8,7 @@ Feature: Advanced Forms API Validation (JSON data parts) And the pdf has form fields And the request includes a JSON part "data" with content "{}" When I send the API request to the endpoint "/api/v1/form/fill" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 diff --git a/testing/cucumber/features/forms.feature b/testing/cucumber/features/forms.feature index c0c1b0f7ff..05ece6f99d 100644 --- a/testing/cucumber/features/forms.feature +++ b/testing/cucumber/features/forms.feature @@ -6,6 +6,7 @@ Feature: Forms API Validation Given I generate a PDF file as "file" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/form/fields" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -24,6 +25,7 @@ Feature: Forms API Validation Given I generate a PDF file as "file" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/form/fields-with-coordinates" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 0 @@ -67,6 +69,7 @@ Feature: Forms API Validation Given I generate a PDF file as "file" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/form/modify-fields" + And this operation is run 5 times in parallel Then the response status code should be 400 @modify-fields @negative @@ -81,6 +84,7 @@ Feature: Forms API Validation Given I generate a PDF file as "file" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/form/delete-fields" + And this operation is run 5 times in parallel Then the response status code should be 400 @delete-fields @negative @@ -89,3 +93,34 @@ Feature: Forms API Validation And the pdf contains 4 pages When I send the API request to the endpoint "/api/v1/form/delete-fields" Then the response status code should be 400 + + + @extract-csv @positive + Scenario: extract-csv returns CSV for a form PDF + Given I generate a PDF file as "file" + And the pdf contains 2 pages + And the pdf has form fields + When I send the API request to the endpoint "/api/v1/form/extract-csv" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "text/csv" + + + @extract-xlsx @positive + Scenario: extract-xlsx returns a spreadsheet for a form PDF + Given I generate a PDF file as "file" + And the pdf contains 2 pages + And the pdf has form fields + When I send the API request to the endpoint "/api/v1/form/extract-xlsx" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response file should have size greater than 200 + + + @extract-csv @negative + Scenario: extract-csv rejects a upload sent under the wrong part name + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + When I send the API request to the endpoint "/api/v1/form/extract-csv" + Then the response status code should be 400 + And the response JSON error should contain "file" diff --git a/testing/cucumber/features/general.feature b/testing/cucumber/features/general.feature index 6ede9bb4fc..747788a9b5 100644 --- a/testing/cucumber/features/general.feature +++ b/testing/cucumber/features/general.feature @@ -12,6 +12,7 @@ Feature: API Validation | verticalDivisions | | | merge | true | When I send the API request to the endpoint "/api/v1/general/split-pdf-by-sections" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response file should have size greater than 200 And the response status code should be 200 @@ -34,6 +35,7 @@ Feature: API Validation | fileInput | fileInput | | pageNumbers | | When I send the API request to the endpoint "/api/v1/general/split-pages" + And this operation is run 5 times in parallel Then the response content type should be "application/octet-stream" And the response status code should be 200 And the response file should have size greater than 200 @@ -57,6 +59,7 @@ Feature: API Validation | splitType | | | splitValue | | When I send the API request to the endpoint "/api/v1/general/split-by-size-or-count" + And this operation is run 5 times in parallel Then the response content type should be "application/octet-stream" And the response status code should be 200 And the response file should have size greater than 200 diff --git a/testing/cucumber/features/general_new.feature b/testing/cucumber/features/general_new.feature index 9d07bb1749..cd25b09650 100644 --- a/testing/cucumber/features/general_new.feature +++ b/testing/cucumber/features/general_new.feature @@ -10,6 +10,7 @@ Feature: General PDF Operations API Validation | parameter | value | | angle | | When I send the API request to the endpoint "/api/v1/general/rotate-pdf" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -41,6 +42,7 @@ Feature: General PDF Operations API Validation | parameter | value | | pageNumbers | 3 | When I send the API request to the endpoint "/api/v1/general/remove-pages" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -76,6 +78,7 @@ Feature: General PDF Operations API Validation | parameter | value | | customMode | | When I send the API request to the endpoint "/api/v1/general/rearrange-pages" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -103,6 +106,7 @@ Feature: General PDF Operations API Validation | parameter | value | | pageSize | | When I send the API request to the endpoint "/api/v1/general/scale-pages" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -127,6 +131,7 @@ Feature: General PDF Operations API Validation | width | 50 | | height | 50 | When I send the API request to the endpoint "/api/v1/general/crop" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -155,6 +160,7 @@ Feature: General PDF Operations API Validation Given I generate a PDF file as "fileInput" And the pdf contains 5 pages When I send the API request to the endpoint "/api/v1/general/pdf-to-single-page" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -199,6 +205,7 @@ Feature: General PDF Operations API Validation | parameter | value | | pagesPerSheet | 9 | When I send the API request to the endpoint "/api/v1/general/multi-page-layout" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -213,6 +220,7 @@ Feature: General PDF Operations API Validation | parameter | value | | pagesPerSheet | 2 | When I send the API request to the endpoint "/api/v1/general/booklet-imposition" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -249,3 +257,55 @@ Feature: General PDF Operations API Validation # Then the response content type should be "application/pdf" # And the response status code should be 200 # And the response file should have size greater than 0 + + + @edit-table-of-contents @positive + Scenario: edit-table-of-contents rewrites the outline + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + And the pdf has bookmarks + And the request data includes + | parameter | value | + | bookmarkData | [{"title":"Intro","pageNumber":1,"children":[]}] | + When I send the API request to the endpoint "/api/v1/general/edit-table-of-contents" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/pdf" + And the response PDF should contain 3 pages + + + @edit-text @positive + Scenario: edit-text applies a find and replace across the document + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + And the pdf pages all contain the text "Hello world" + And the request data includes + | parameter | value | + | edits | [{"find":"Hello","replace":"Goodbye"}] | + When I send the API request to the endpoint "/api/v1/general/edit-text" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/pdf" + + + @edit-text @negative + Scenario: edit-text without any operations returns 400 + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + When I send the API request to the endpoint "/api/v1/general/edit-text" + Then the response status code should be 400 + And the response JSON error should contain "find/replace" + + + @split-for-poster-print @positive + Scenario: split-for-poster-print tiles each page into an archive + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + And the request data includes + | parameter | value | + | horizontalDivisions | 1 | + | verticalDivisions | 1 | + When I send the API request to the endpoint "/api/v1/general/split-for-poster-print" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response file should have size greater than 200 diff --git a/testing/cucumber/features/info_config.feature b/testing/cucumber/features/info_config.feature new file mode 100644 index 0000000000..3559110c29 --- /dev/null +++ b/testing/cucumber/features/info_config.feature @@ -0,0 +1,134 @@ +@info @config +Feature: Config, info and UI-data read APIs + + # Cheap read-only JSON endpoints behind the frontend and admin surfaces. + + @app-config @positive + Scenario: app-config returns the frontend configuration + When I send a GET request to "/api/v1/config/app-config" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/json" + + + @endpoints-availability @positive + Scenario: endpoints-availability reports per-endpoint availability + When I send a GET request to "/api/v1/config/endpoints-availability" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/json" + + + @endpoints-enabled @positive + Scenario: endpoints-enabled reports the status of a named endpoint + When I send a GET request to "/api/v1/config/endpoints-enabled" with parameters + | parameter | value | + | endpoints | merge-pdfs | + Then the response status code should be 200 + And the response content type should be "application/json" + + + @endpoints-enabled @negative + Scenario: endpoints-enabled without the endpoints parameter returns 400 + When I send a GET request to "/api/v1/config/endpoints-enabled" + Then the response status code should be 400 + + + @login-disclaimer @positive + Scenario: login-disclaimer returns its configuration + When I send a GET request to "/api/v1/config/login-disclaimer" + Then the response status code should be 200 + And the response JSON field "enabled" should be false + + + @health @positive + Scenario: info health reports the running version + When I send a GET request to "/api/v1/info/health" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response JSON field "status" should equal "UP" + And the response JSON field "version" should not be empty + + + @metrics @positive + Scenario Outline: metrics endpoints return a numeric aggregate + When I send a GET request to "" + Then the response status code should be 200 + And the response content type should be "application/json" + And the response should match the regex "^[0-9.]+$" + + Examples: + | endpoint | + | /api/v1/info/load/unique | + | /api/v1/info/requests/unique | + + + @metrics @positive + Scenario Outline: metrics list endpoints return a JSON list + When I send a GET request to "" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response JSON should be a list + + Examples: + | endpoint | + | /api/v1/info/load/all/unique | + | /api/v1/info/requests/all/unique | + + + @metrics @positive + Scenario: weekly active users reports tracking metadata + When I send a GET request to "/api/v1/info/wau" + Then the response status code should be 200 + And the response JSON field "trackingSince" should not be empty + + + @settings @positive + Scenario: get-endpoints-status returns the endpoint toggle map + When I send a GET request to "/api/v1/settings/get-endpoints-status" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/json" + + + @ui-data @positive + Scenario Outline: UI data endpoints return JSON for the frontend + When I send a GET request to "" + Then the response status code should be 200 + And the response content type should be "application/json" + + Examples: + | endpoint | + | /api/v1/ui-data/footer-info | + | /api/v1/ui-data/home | + | /api/v1/ui-data/licenses | + | /api/v1/ui-data/pipeline | + + + @ui-data @positive + Scenario: OCR UI data lists the installed tesseract languages + When I send a GET request to "/api/v1/ui-data/ocr-pdf" + Then the response status code should be 200 + And the response JSON field "languages" should be a list + + + @ui-data @positive + Scenario: Sign UI data lists the available signature fonts + When I send a GET request to "/api/v1/ui-data/sign" + Then the response status code should be 200 + And the response JSON field "fonts" should be a list + + + @hardware-signing @positive + Scenario: Hardware signing capabilities are reported for the server build + When I send a GET request to "/api/v1/security/cert-sign/hardware/capabilities" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response JSON field "desktop" should be false + + + @hardware-signing @negative + Scenario: Windows certificate store is rejected outside the desktop app + When I send a GET request to "/api/v1/security/cert-sign/hardware/windows-certificates" + Then the response status code should be 400 + And the response JSON error should contain "desktop" diff --git a/testing/cucumber/features/merge_overlay.feature b/testing/cucumber/features/merge_overlay.feature index 9e62d1ae99..2f7abe06bc 100644 --- a/testing/cucumber/features/merge_overlay.feature +++ b/testing/cucumber/features/merge_overlay.feature @@ -7,6 +7,7 @@ Feature: Merge and Overlay PDF API Validation And the pdf contains 2 pages with random text And I also generate a PDF file as "fileInput" When I send the API request to the endpoint "/api/v1/general/merge-pdfs" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -51,6 +52,7 @@ Feature: Merge and Overlay PDF API Validation | overlayMode | SequentialOverlay | | overlayPosition | 0 | When I send the API request to the endpoint "/api/v1/general/overlay-pdfs" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 diff --git a/testing/cucumber/features/misc_new.feature b/testing/cucumber/features/misc_new.feature index c10f0601d7..528ed6391d 100644 --- a/testing/cucumber/features/misc_new.feature +++ b/testing/cucumber/features/misc_new.feature @@ -48,6 +48,7 @@ Feature: Miscellaneous PDF Operations API Validation | overrideX | -1 | | overrideY | -1 | When I send the API request to the endpoint "/api/v1/misc/add-stamp" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -92,6 +93,7 @@ Feature: Miscellaneous PDF Operations API Validation | position | 2 | | fontSize | 14 | When I send the API request to the endpoint "/api/v1/misc/add-page-numbers" + And this operation is run 5 times in parallel against decoy traffic Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 200 @@ -103,6 +105,7 @@ Feature: Miscellaneous PDF Operations API Validation Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/misc/unlock-pdf-forms" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 0 @@ -138,6 +141,7 @@ Feature: Miscellaneous PDF Operations API Validation | replaceAndInvertOption | HIGH_CONTRAST_COLOR | | highContrastColorCombination | WHITE_TEXT_ON_BLACK | When I send the API request to the endpoint "/api/v1/misc/replace-invert-pdf" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 0 @@ -163,6 +167,7 @@ Feature: Miscellaneous PDF Operations API Validation Given I generate a PDF file as "fileInput" And the pdf contains 3 pages When I send the API request to the endpoint "/api/v1/misc/decompress-pdf" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 0 @@ -188,6 +193,7 @@ Feature: Miscellaneous PDF Operations API Validation | parameter | value | | useFirstTextAsFallback | true | When I send the API request to the endpoint "/api/v1/misc/auto-rename" + And this operation is run 5 times in parallel Then the response content type should be "application/pdf" And the response status code should be 200 And the response file should have size greater than 0 @@ -212,6 +218,7 @@ Feature: Miscellaneous PDF Operations API Validation Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/misc/show-javascript" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response file should have size greater than 0 @@ -222,3 +229,28 @@ Feature: Miscellaneous PDF Operations API Validation And the pdf contains 5 pages with random text When I send the API request to the endpoint "/api/v1/misc/show-javascript" Then the response status code should be 200 + + + @auto-rotate-pdf @positive + Scenario: auto-rotate-pdf returns a PDF with the page count preserved + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages with random text + When I send the API request to the endpoint "/api/v1/misc/auto-rotate-pdf" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/pdf" + And the response PDF should contain 3 pages + + + @add-comments @positive + Scenario: add-comments annotates a page without changing the page count + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + And the request data includes + | parameter | value | + | comments | [{"pageNumber":1,"x":100,"y":100,"text":"review me","author":"qa"}] | + When I send the API request to the endpoint "/api/v1/misc/add-comments" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/pdf" + And the response PDF should contain 3 pages diff --git a/testing/cucumber/features/security_new.feature b/testing/cucumber/features/security_new.feature index 7058cf2fdd..13763e302f 100644 --- a/testing/cucumber/features/security_new.feature +++ b/testing/cucumber/features/security_new.feature @@ -14,6 +14,7 @@ Feature: Security API Validation | removeLinks | true | | removeFonts | false | When I send the API request to the endpoint "/api/v1/security/sanitize-pdf" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -54,6 +55,7 @@ Feature: Security API Validation | wholeWordSearch| true | | convertPDFToImage | false | When I send the API request to the endpoint "/api/v1/security/auto-redact" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -96,6 +98,7 @@ Feature: Security API Validation | pageNumbers | 2,4 | | pageRedactionColor | #000000 | When I send the API request to the endpoint "/api/v1/security/redact" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -130,6 +133,7 @@ Feature: Security API Validation Scenario: Verify PDF-A compliance Given I use an example file at "exampleFiles/pdfa1.pdf" as parameter "fileInput" When I send the API request to the endpoint "/api/v1/security/verify-pdf" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/json" And the response file should have size greater than 2 @@ -148,6 +152,7 @@ Feature: Security API Validation Given I generate a PDF file as "fileInput" And the pdf contains 2 pages When I send the API request to the endpoint "/api/v1/security/remove-cert-sign" + And this operation is run 5 times in parallel Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 @@ -161,3 +166,35 @@ Feature: Security API Validation Then the response status code should be 200 And the response content type should be "application/pdf" And the response file should have size greater than 0 + + + @validate-signature @positive + Scenario: validate-signature reports no signatures on an unsigned PDF + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + When I send the API request to the endpoint "/api/v1/security/validate-signature" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response JSON should be a list + + + @redact-execute @positive + Scenario: redact-execute removes the targeted text + Given I generate a PDF file as "fileInput" + And the pdf contains 3 pages + And the pdf pages all contain the text "Hello world" + And the request data includes + | parameter | value | + | textValues | Hello | + When I send the API request to the endpoint "/api/v1/security/redact-execute" + And this operation is run 5 times in parallel + Then the response status code should be 200 + And the response content type should be "application/pdf" + + + @redact-execute @negative + Scenario: redact-execute without any targets returns 400 + Given I generate a PDF file as "fileInput" + And the pdf contains 2 pages + When I send the API request to the endpoint "/api/v1/security/redact-execute" + Then the response status code should be 400 diff --git a/testing/cucumber/features/steps/job_step_definitions.py b/testing/cucumber/features/steps/job_step_definitions.py new file mode 100644 index 0000000000..44c695b7a4 --- /dev/null +++ b/testing/cucumber/features/steps/job_step_definitions.py @@ -0,0 +1,171 @@ +"""Steps for the async job API. DELETE is a cancel, so it 400s once the job finishes.""" +import time + +import requests +from behave import then, when +from job_support import API_HEADERS, BASE_URL, trigger_cleanup + +POLL_TIMEOUT_SECONDS = 60 + + +@when("I store the job id from the response") +def step_store_job_id(context): + payload = context.response.json() + context.job_id = payload.get("jobId") + assert context.job_id, f"No jobId in async submit response: {payload}" + + +@when("I wait for the job to complete") +def step_wait_for_job(context): + deadline = time.time() + POLL_TIMEOUT_SECONDS + while time.time() < deadline: + response = requests.get( + f"{BASE_URL}/api/v1/general/job/{context.job_id}", + headers=API_HEADERS, timeout=30, + ) + assert response.status_code == 200, ( + f"Job status returned {response.status_code}: {response.text}" + ) + context.response = response + payload = response.json() + if payload.get("complete"): + context.job_status = payload + return + time.sleep(0.2) + raise AssertionError( + f"Job {context.job_id} did not complete within {POLL_TIMEOUT_SECONDS}s" + ) + + +@when("I request the job result") +def step_request_job_result(context): + context.response = requests.get( + f"{BASE_URL}/api/v1/general/job/{context.job_id}/result", + headers=API_HEADERS, timeout=60, + ) + + +@when("I request the job result file list") +def step_request_job_result_files(context): + context.response = requests.get( + f"{BASE_URL}/api/v1/general/job/{context.job_id}/result/files", + headers=API_HEADERS, timeout=60, + ) + files = context.response.json().get("files") or [] + context.job_files = files + if files: + context.job_file_id = files[0].get("fileId") + + +@when("I download the first job result file") +def step_download_job_file(context): + assert getattr(context, "job_file_id", None), "No fileId captured from the result file list" + context.response = requests.get( + f"{BASE_URL}/api/v1/general/files/{context.job_file_id}", + headers=API_HEADERS, timeout=60, + ) + + +@when("I request the first job result file metadata") +def step_job_file_metadata(context): + assert getattr(context, "job_file_id", None), "No fileId captured from the result file list" + context.response = requests.get( + f"{BASE_URL}/api/v1/general/files/{context.job_file_id}/metadata", + headers=API_HEADERS, timeout=60, + ) + + +@when("I cancel the job") +def step_cancel_job(context): + context.response = requests.delete( + f"{BASE_URL}/api/v1/general/job/{context.job_id}", + headers=API_HEADERS, timeout=30, + ) + + +@then("the job result file list should contain at least {count:d} file(s)") +def step_check_job_file_count(context, count): + files = context.response.json().get("files") or [] + assert len(files) >= count, f"Expected at least {count} result file(s), got {len(files)}" + + +@then("the job should be reported complete") +def step_check_job_complete(context): + payload = context.response.json() + assert payload.get("complete") is True, f"Job not complete: {payload}" + assert not payload.get("error"), f"Job reported an error: {payload.get('error')}" + + +# --- Cleanup: the async job files must actually go away, not just age out --- + + +@when("I trigger the async job cleanup") +def step_trigger_cleanup(context): + context.response = trigger_cleanup() + try: + context.cleanup_summary = context.response.json() + except ValueError: + context.cleanup_summary = {} + + +@then("the cleanup should report at least {count:d} job(s) removed") +def step_check_cleanup_jobs(context, count): + removed = context.cleanup_summary.get("jobsRemoved") + assert removed is not None, f"No jobsRemoved in cleanup response: {context.cleanup_summary}" + assert removed >= count, f"Expected at least {count} job(s) removed, got {removed}" + + +@then("the cleanup should report at least {count:d} file(s) deleted") +def step_check_cleanup_files(context, count): + deleted = context.cleanup_summary.get("filesDeleted") + assert deleted is not None, f"No filesDeleted in cleanup response: {context.cleanup_summary}" + assert deleted >= count, f"Expected at least {count} file(s) deleted, got {deleted}" + + +@then("the cleanup should report nothing left to remove") +def step_check_cleanup_idempotent(context): + removed = context.cleanup_summary.get("jobsRemoved") + deleted = context.cleanup_summary.get("filesDeleted") + assert removed == 0 and deleted == 0, ( + "A repeat cleanup still found work to do, so the first pass did not fully clean up: " + f"{context.cleanup_summary}" + ) + + +@then("the job should no longer exist") +def step_check_job_gone(context): + response = requests.get( + f"{BASE_URL}/api/v1/general/job/{context.job_id}", + headers=API_HEADERS, timeout=30, + ) + assert response.status_code == 404, ( + f"Job {context.job_id} still exists after cleanup: " + f"{response.status_code} {response.text[:200]}" + ) + + +@then("the job result file should no longer be downloadable") +def step_check_job_file_gone(context): + assert getattr(context, "job_file_id", None), "No fileId captured from the result file list" + response = requests.get( + f"{BASE_URL}/api/v1/general/files/{context.job_file_id}", + headers=API_HEADERS, timeout=30, + ) + assert response.status_code == 404, ( + f"File {context.job_file_id} still downloadable after cleanup: " + f"{response.status_code} {response.text[:200]}" + ) + + +@then("the job result file should still be downloadable") +def step_check_job_file_still_there(context): + assert getattr(context, "job_file_id", None), "No fileId captured from the result file list" + response = requests.get( + f"{BASE_URL}/api/v1/general/files/{context.job_file_id}", + headers=API_HEADERS, timeout=60, + ) + assert response.status_code == 200, ( + f"File {context.job_file_id} was not retrievable a second time: " + f"{response.status_code} {response.text[:200]}" + ) + assert len(response.content) > 0, "Second download returned an empty body" diff --git a/testing/cucumber/features/steps/job_support.py b/testing/cucumber/features/steps/job_support.py new file mode 100644 index 0000000000..027bdcb9eb --- /dev/null +++ b/testing/cucumber/features/steps/job_support.py @@ -0,0 +1,22 @@ +"""Shared helpers for the async job API. + +Support module, not a step module: behave execs everything under features/steps as +step definitions, so anything environment.py needs to import has to live apart from +the @when/@then decorators or they would register twice. +""" +import requests + +BASE_URL = "http://localhost:8080" +API_HEADERS = {"X-API-KEY": "123456789"} +CLEANUP_URL = f"{BASE_URL}/api/v1/general/jobs/cleanup" + + +def trigger_cleanup(): + """Force-expire finished jobs so their stored files are released now. + + An async submit persists a copy of the upload plus its results and holds them for + the job retention window (30 minutes by default), which far outlasts a test run. + Calling this keeps the post-run temp-file check strict: anything it still reports + afterwards is a genuine leak rather than a file that simply had not aged out. + """ + return requests.post(CLEANUP_URL, headers=API_HEADERS, timeout=60) diff --git a/testing/cucumber/features/steps/parallel_step_definitions.py b/testing/cucumber/features/steps/parallel_step_definitions.py new file mode 100644 index 0000000000..d832005fba --- /dev/null +++ b/testing/cucumber/features/steps/parallel_step_definitions.py @@ -0,0 +1,47 @@ +"""Concurrency steps, usable either before the request or after it.""" + +from behave import given, then, when + +import parallel_support + + +def _set_repeat(context, count, decoy=False): + context.parallel_decoy = decoy + if count < 2: + context.parallel_repeat = 1 + return + context.parallel_repeat = count + + # An asked-for level higher than one already run wins. + if count > getattr(context, "parallel_ran_at", 0): + context.parallel_validated = False + if getattr(context, "parallel_validated", False): + return + + # Placed after the request: replay whichever request was just sent. + pending = getattr(context, "parallel_request", None) + if pending is not None: + url, spec, headers, label = pending + parallel_support.validate(context, url, spec, headers, context.response, label) + return + + pending_get = getattr(context, "parallel_get", None) + if pending_get is not None: + url, params, headers, label = pending_get + parallel_support.validate_get(context, url, params, headers, context.response, label) + + +@given("this operation is run {count:d} times in parallel") +@when("this operation is run {count:d} times in parallel") +@then("this operation is run {count:d} times in parallel") +def step_operation_run_in_parallel(context, count): + _set_repeat(context, count) + + +# Decoy traffic is an equal number of concurrent requests carrying different page +# text, so a response that came from the wrong request becomes visible. +@given("this operation is run {count:d} times in parallel against decoy traffic") +@when("this operation is run {count:d} times in parallel against decoy traffic") +@then("this operation is run {count:d} times in parallel against decoy traffic") +def step_operation_run_in_parallel_with_decoy(context, count): + _set_repeat(context, count, decoy=True) diff --git a/testing/cucumber/features/steps/parallel_support.py b/testing/cucumber/features/steps/parallel_support.py new file mode 100644 index 0000000000..d84d8354de --- /dev/null +++ b/testing/cucumber/features/steps/parallel_support.py @@ -0,0 +1,478 @@ +"""Re-issues a request concurrently and asserts every response matches the baseline.""" + +import io +import json as json_module +import os +import re +import sys +import zipfile +from concurrent.futures import ThreadPoolExecutor +from hashlib import sha256 + +import requests +from pypdf import PdfReader + +_UUID_RE = re.compile(r"[0-9a-fA-F]{8}-(?:[0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}") +_LONG_NUM_RE = re.compile(r"\d{10,}") +_DATE_RE = re.compile(r"\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}(?:\.\d+)?Z?") +_VOLATILE_KEY_RE = re.compile( + r"^(id|uuid|.*Id|.*_id|.*[Tt]ime|.*[Dd]ate|timestamp|created.*|modified.*|" + r"updated.*|expires.*|token|traceId|requestId|duration.*|elapsed.*)$" +) + +# Only hash text for reasonably sized documents so the check stays cheap. +_MAX_TEXT_PAGES = 25 + +# Sequential samples taken to separate inherent nondeterminism from a concurrency bug. +NOISE_PROBE_SAMPLES = 3 + +# Response size is allowed to drift this far before it counts as a difference. +SIZE_TOLERANCE = 0.05 + +# Collected per-scenario results, printed as a summary in after_all. +VALIDATIONS = [] + + +# A spec replaces open file handles with bytes so it can be replayed from many threads. + + +def materialize(spec): + """Turn a captured spec into a fresh `files=` list for one request.""" + parts = [] + for key, filename, payload, mime in spec: + if filename is None: + parts.append((key, (None, payload) if mime is None else (None, payload, mime))) + else: + parts.append((key, (filename, io.BytesIO(payload), mime))) + return parts + + +def send(url, spec, headers, timeout=300): + return requests.post(url, files=materialize(spec), headers=headers, timeout=timeout) + + +def _normalize_name(name): + name = _UUID_RE.sub("", name) + return _LONG_NUM_RE.sub("", name) + + +def _strip_volatile(value): + """Drop keys whose values legitimately differ between two identical requests.""" + if isinstance(value, dict): + return { + k: _strip_volatile(v) + for k, v in sorted(value.items()) + if not _VOLATILE_KEY_RE.match(k) + } + if isinstance(value, list): + return [_strip_volatile(v) for v in value] + if isinstance(value, str): + return _normalize_name(value) + return value + + +def _pdf_fingerprint(body, prefix=""): + parts = {} + try: + reader = PdfReader(io.BytesIO(body)) + except Exception as exc: + parts[prefix + "pdf"] = f"unreadable: {type(exc).__name__}" + return parts + parts[prefix + "encrypted"] = reader.is_encrypted + if reader.is_encrypted: + return parts + try: + pages = reader.pages + parts[prefix + "pages"] = len(pages) + except Exception as exc: + parts[prefix + "pdf"] = f"unreadable pages: {type(exc).__name__}" + return parts + if len(pages) <= _MAX_TEXT_PAGES: + try: + text = "\n".join((page.extract_text() or "") for page in pages) + parts[prefix + "text_sha"] = sha256(text.encode("utf-8")).hexdigest()[:16] + except Exception: + pass + return parts + + +def _entry_sha(entry): + """Hash an archive entry, normalizing the ids and timestamps EPUB/ODF restamp each run.""" + try: + text = entry.decode("utf-8") + if "\x00" not in text: + entry = _DATE_RE.sub("", _normalize_name(text)).encode("utf-8") + except UnicodeDecodeError: + pass + return sha256(entry).hexdigest()[:16] + + +def _zip_fingerprint(body): + parts = {} + try: + with zipfile.ZipFile(io.BytesIO(body)) as archive: + names = sorted(_normalize_name(n) for n in archive.namelist()) + parts["zip_entries"] = len(names) + parts["zip_names"] = names + for index, name in enumerate(sorted(archive.namelist())): + entry = archive.read(name) + if entry[:5] == b"%PDF-": + parts.update(_pdf_fingerprint(entry, prefix=f"zip[{index}].")) + else: + parts[f"zip[{index}].sha"] = _entry_sha(entry) + except Exception as exc: + parts["zip"] = f"unreadable: {type(exc).__name__}" + return parts + + +def fingerprint(response): + """Structural signature of a response, ignoring benign per-request variance.""" + body = response.content + content_type = (response.headers.get("Content-Type") or "").split(";")[0].strip() + parts = {"status": response.status_code, "content_type": content_type, "size": len(body)} + + + if "json" in content_type: + try: + parts["json"] = _strip_volatile(json_module.loads(body.decode("utf-8"))) + except Exception: + parts["body_sha"] = sha256(body).hexdigest()[:16] + elif body[:5] == b"%PDF-": + parts.update(_pdf_fingerprint(body)) + elif body[:2] == b"PK": + parts.update(_zip_fingerprint(body)) + elif content_type.startswith("text/") or response.status_code >= 400: + try: + parts["text"] = _normalize_name(body.decode("utf-8", "replace"))[:2000] + except Exception: + parts["body_sha"] = sha256(body).hexdigest()[:16] + else: + parts["body_sha"] = sha256(body).hexdigest()[:16] + return parts + + +def differing_keys(baseline, other): + return { + key + for key in set(baseline) | set(other) + if key != "size" and baseline.get(key) != other.get(key) + } + + +def size_differs(baseline, other): + base_size, other_size = baseline.get("size", 0), other.get("size", 0) + return abs(other_size - base_size) > max(64, base_size * SIZE_TOLERANCE) + + +def compare(baseline, other, ignore=frozenset(), ignore_size=False): + """Return a list of human-readable differences between two fingerprints.""" + diffs = [] + for key in sorted(differing_keys(baseline, other) - set(ignore)): + diffs.append( + f"{key}: baseline={_short(baseline.get(key))} parallel={_short(other.get(key))}" + ) + if not ignore_size and size_differs(baseline, other): + diffs.append( + f"size: baseline={baseline.get('size', 0)} parallel={other.get('size', 0)} " + f"(differs by more than {SIZE_TOLERANCE:.0%})" + ) + return diffs + + +def _short(value): + text = repr(value) + return text if len(text) <= 160 else text[:157] + "..." + + +def build_decoy_spec(spec): + """Clone a spec with each PDF stamped with unique text, or None if not possible. + + Structure is preserved so parameters stay valid; only the text differs, which + is what makes bleed visible. + """ + try: + from pypdf import PdfWriter + from reportlab.pdfgen import canvas + except ImportError: + return None + + decoy = [] + stamped_any = False + for key, filename, payload, mime in spec: + if filename is None or not isinstance(payload, bytes) or payload[:5] != b"%PDF-": + decoy.append((key, filename, payload, mime)) + continue + try: + reader = PdfReader(io.BytesIO(payload)) + if reader.is_encrypted: + return None + writer = PdfWriter() + for index, page in enumerate(reader.pages): + box = page.mediabox + width, height = float(box.width), float(box.height) + overlay_buffer = io.BytesIO() + overlay_canvas = canvas.Canvas(overlay_buffer, pagesize=(width, height)) + overlay_canvas.drawString( + 20, max(20.0, height - 20), f"DECOY-MARKER-{index}-do-not-mix" + ) + overlay_canvas.showPage() + overlay_canvas.save() + overlay_buffer.seek(0) + page.merge_page(PdfReader(overlay_buffer).pages[0]) + writer.add_page(page) + out = io.BytesIO() + writer.write(out) + decoy.append((key, filename, out.getvalue(), mime)) + stamped_any = True + except Exception: + return None + return decoy if stamped_any else None + + +def _run_concurrently(url, specs, headers, timeout): + """Fire every spec at once and return (response, error) in submission order.""" + results = [None] * len(specs) + + def _worker(index): + try: + results[index] = (send(url, specs[index], headers, timeout=timeout), None) + except Exception as exc: + results[index] = (None, exc) + + with ThreadPoolExecutor(max_workers=len(specs)) as pool: + list(pool.map(_worker, range(len(specs)))) + return results + + +def validate(context, url, spec, headers, baseline, label, timeout=300): + """Re-issue the request concurrently; no-op unless the repeat count is above 1.""" + repeat = getattr(context, "parallel_repeat", 1) + if repeat < 2 or getattr(context, "parallel_validated", False): + return + context.parallel_validated = True + context.parallel_ran_at = repeat + + decoy_spec = build_decoy_spec(spec) if getattr(context, "parallel_decoy", False) else None + specs = [spec] * repeat + ([decoy_spec] * repeat if decoy_spec else []) + results = _run_concurrently(url, specs, headers, timeout) + + main_results = results[:repeat] + decoy_results = results[repeat:] + baseline_fp = fingerprint(baseline) + + noise, noisy_size = frozenset(), False + failures = _collect_failures( + main_results, decoy_results, baseline_fp, repeat, noise, noisy_size + ) + if failures: + # Some endpoints are inherently nondeterministic (embedded ids, timestamps, + # deliberate randomness). Re-run sequentially to tell that apart from a real bug. + noise, noisy_size = _probe_noise(url, spec, headers, baseline_fp, timeout) + failures = _collect_failures( + main_results, decoy_results, baseline_fp, repeat, noise, noisy_size + ) + + VALIDATIONS.append( + { + "label": label, + "repeat": repeat, + "decoy": bool(decoy_spec), + "failed": bool(failures), + "noise": sorted(noise) + (["size"] if noisy_size else []), + } + ) + + if failures: + raise AssertionError( + f"Parallel consistency failed for {label} at concurrency {repeat}.\n" + f"Two sequential runs agreed on these fields, so the differences below are " + f"caused by running the same operation concurrently.\n" + f"Baseline fingerprint: {_short(baseline_fp)}\n - " + "\n - ".join(failures) + ) + + +def _collect_failures(main_results, decoy_results, baseline_fp, repeat, noise, noisy_size): + decoy_fp = _decoy_reference(decoy_results, baseline_fp, noise, noisy_size) + failures = [] + + for index, (response, error) in enumerate(main_results): + if error is not None: + failures.append(f"copy {index + 1}/{repeat} raised {type(error).__name__}: {error}") + continue + actual_fp = fingerprint(response) + diffs = compare(baseline_fp, actual_fp, noise, noisy_size) + if not diffs: + continue + if decoy_fp is not None and not compare( + decoy_fp, actual_fp, noise, noisy_size + ): + failures.append( + f"copy {index + 1}/{repeat} returned the CONCURRENT DECOY REQUEST'S response " + f"(cross-request bleed)" + ) + else: + failures.append(f"copy {index + 1}/{repeat} diverged: " + "; ".join(diffs)) + + failures.extend(_check_decoys(decoy_results, repeat, noise, noisy_size)) + return failures + + +def _probe_noise(url, spec, headers, baseline_fp, timeout, samples=NOISE_PROBE_SAMPLES): + """Fields that already vary between uncontended runs, so they prove nothing. + + Several samples: high-variance output can look stable across any single pair. + """ + probes = [] + for _ in range(samples): + try: + probes.append(fingerprint(send(url, spec, headers, timeout=timeout))) + except Exception: + break + return _noise_from_samples(baseline_fp, probes) + + +def _noise_from_samples(baseline_fp, probes): + noise = set() + noisy_size = False + for index, probe in enumerate(probes): + for other in [baseline_fp] + probes[:index]: + noise |= differing_keys(other, probe) + noisy_size = noisy_size or size_differs(other, probe) + return frozenset(noise), noisy_size + + +def validate_get(context, url, params, headers, baseline, label, timeout=60): + """Concurrency check for read-only GET endpoints.""" + repeat = getattr(context, "parallel_repeat", 1) + if repeat < 2 or getattr(context, "parallel_validated", False): + return + context.parallel_validated = True + context.parallel_ran_at = repeat + + results = [None] * repeat + + def _worker(index): + try: + results[index] = ( + requests.get(url, params=params, headers=headers, timeout=timeout), + None, + ) + except Exception as exc: + results[index] = (None, exc) + + with ThreadPoolExecutor(max_workers=repeat) as pool: + list(pool.map(_worker, range(repeat))) + + baseline_fp = fingerprint(baseline) + + def _failures(noise, noisy_size): + found = [] + for index, (response, error) in enumerate(results): + if error is not None: + found.append(f"copy {index + 1}/{repeat} raised {type(error).__name__}: {error}") + continue + diffs = compare( + baseline_fp, + fingerprint(response), + noise, + noisy_size, + ) + if diffs: + found.append(f"copy {index + 1}/{repeat} diverged: " + "; ".join(diffs)) + return found + + noise, noisy_size = frozenset(), False + failures = _failures(noise, noisy_size) + if failures: + probes = [] + for _ in range(NOISE_PROBE_SAMPLES): + try: + probes.append( + fingerprint( + requests.get(url, params=params, headers=headers, timeout=timeout) + ) + ) + except Exception: + break + noise, noisy_size = _noise_from_samples(baseline_fp, probes) + failures = _failures(noise, noisy_size) + + VALIDATIONS.append( + { + "label": label, + "repeat": repeat, + "decoy": False, + "failed": bool(failures), + "noise": sorted(noise) + (["size"] if noisy_size else []), + } + ) + if failures: + raise AssertionError( + f"Parallel consistency failed for GET {label} at concurrency {repeat}.\n - " + + "\n - ".join(failures) + ) + + +def _decoy_reference(decoy_results, baseline_fp, noise, noisy_size): + """Fingerprint of the decoy response, or None when it is not distinguishable.""" + live = [r for r, _e in decoy_results if r is not None] + if not live: + return None + decoy_fp = fingerprint(live[0]) + # Some endpoints ignore page content, so the decoy cannot prove anything there. + if not compare(baseline_fp, decoy_fp, noise, noisy_size): + return None + return decoy_fp + + +def _check_decoys(decoy_results, repeat, noise, noisy_size): + """Assert the decoy load stayed self-consistent while contending with the main copies.""" + if not decoy_results: + return [] + live = [(i, r) for i, (r, _e) in enumerate(decoy_results) if r is not None] + if not live: + return ["every decoy request failed to complete"] + + failures = [] + reference_fp = fingerprint(live[0][1]) + for index, response in live[1:]: + diffs = compare( + reference_fp, + fingerprint(response), + noise, + noisy_size, + ) + if diffs: + failures.append(f"decoy {index + 1}/{repeat} diverged: " + "; ".join(diffs)) + return failures + + +def print_summary(): + if not VALIDATIONS: + return + total = len(VALIDATIONS) + failed = sum(1 for v in VALIDATIONS if v["failed"]) + max_repeat = max(v["repeat"] for v in VALIDATIONS) + requests_sent = sum(v["repeat"] * (2 if v["decoy"] else 1) for v in VALIDATIONS) + + lines = [ + f"\n[PARALLEL] {total - failed}/{total} operations stayed consistent under " + f"concurrency (up to {max_repeat} at once, {requests_sent} concurrent requests sent)." + ] + noisy = {} + for entry in VALIDATIONS: + if entry["noise"]: + noisy.setdefault(entry["label"], set()).update(entry["noise"]) + if noisy: + lines.append( + f"[PARALLEL] {len(noisy)} endpoint(s) produce nondeterministic output. Those " + f"fields were excluded only after confirming they also vary across " + f"{NOISE_PROBE_SAMPLES} sequential runs:" + ) + for label, fields in sorted(noisy.items()): + lines.append(f" {label} -> {', '.join(sorted(fields))}") + + # behave's --junit reporter swallows after_all stdout, so bypass any capture. + stream = getattr(sys, "__stderr__", None) or sys.stderr + stream.write("\n".join(lines) + "\n") + stream.flush() diff --git a/testing/cucumber/features/steps/step_definitions.py b/testing/cucumber/features/steps/step_definitions.py index aa1d45cac0..5abefa049a 100644 --- a/testing/cucumber/features/steps/step_definitions.py +++ b/testing/cucumber/features/steps/step_definitions.py @@ -15,6 +15,8 @@ import zipfile import re from PIL import Image, ImageDraw +import parallel_support + API_HEADERS = {"X-API-KEY": "123456789"} ######### @@ -585,6 +587,8 @@ def step_send_get_request(context, endpoint): full_url = f"{base_url}{endpoint}" response = requests.get(full_url, headers=API_HEADERS, timeout=60) context.response = response + context.parallel_get = (full_url, None, API_HEADERS, endpoint) + parallel_support.validate_get(context, full_url, None, API_HEADERS, response, endpoint) @when('I send a GET request to "{endpoint}" with parameters') @@ -594,17 +598,18 @@ def step_send_get_request_with_params(context, endpoint): full_url = f"{base_url}{endpoint}" response = requests.get(full_url, params=params, headers=API_HEADERS, timeout=60) context.response = response + context.parallel_get = (full_url, params, API_HEADERS, endpoint) + parallel_support.validate_get(context, full_url, params, API_HEADERS, response, endpoint) -@when('I send the API request to the endpoint "{endpoint}"') -def step_send_api_request(context, endpoint): - url = f"http://localhost:8080{endpoint}" +def _build_request_spec(context): + """Capture the multipart payload as replayable bytes rather than file handles.""" files = context.files if hasattr(context, "files") else {} if not hasattr(context, "request_data") or context.request_data is None: context.request_data = {} - form_data = [] + spec = [] for key, value in context.request_data.items(): # Handle list parameters (like 'languages') - send multiple form fields # Split comma-separated values or treat single values as single-item lists @@ -612,32 +617,50 @@ def step_send_api_request(context, endpoint): # Split by comma if present, otherwise treat as single value values = [v.strip() for v in value.split(",")] if "," in value else [value] for val in values: - form_data.append((key, (None, val))) + spec.append((key, None, val, None)) else: - form_data.append((key, (None, value))) + spec.append((key, None, value, None)) + + def _read(file): + file.seek(0) + payload = file.read() + file.seek(0) + return payload for key, file in files.items(): mime_type, _ = mimetypes.guess_type(file.name) mime_type = mime_type or "application/octet-stream" print(f"form_data {file.name} with {mime_type}") - form_data.append((key, (file.name, file, mime_type))) + spec.append((key, file.name, _read(file), mime_type)) # Multi-file entries (duplicate keys for MultipartFile[] endpoints, e.g. merge-pdfs) for key, file in getattr(context, "multi_files", []): mime_type, _ = mimetypes.guess_type(file.name) mime_type = mime_type or "application/octet-stream" print(f"form_data (multi) {file.name} with {mime_type}") - form_data.append((key, (file.name, file, mime_type))) + spec.append((key, file.name, _read(file), mime_type)) # JSON multipart parts for @RequestPart endpoints (e.g. /form/fill) for part_name, json_content in getattr(context, "json_parts", {}).items(): - form_data.append((part_name, (None, json_content, "application/json"))) + spec.append((part_name, None, json_content, "application/json")) + + return spec + + +@when('I send the API request to the endpoint "{endpoint}"') +def step_send_api_request(context, endpoint): + url = f"http://localhost:8080{endpoint}" + spec = _build_request_spec(context) # Set timeout to 300 seconds (5 minutes) to prevent infinite hangs print(f"Sending POST request to {endpoint} with timeout=300s") - response = requests.post(url, files=form_data, headers=API_HEADERS, timeout=300) + response = parallel_support.send(url, spec, API_HEADERS, timeout=300) context.response = response + # Remembered so a later "run N times in parallel" step can replay this request. + context.parallel_request = (url, spec, API_HEADERS, endpoint) + parallel_support.validate(context, url, spec, API_HEADERS, response, endpoint) + ######## # THEN # diff --git a/testing/cucumber/run-parallel.sh b/testing/cucumber/run-parallel.sh new file mode 100644 index 0000000000..40d30b3c40 --- /dev/null +++ b/testing/cucumber/run-parallel.sh @@ -0,0 +1,143 @@ +#!/bin/bash +# Run the behave suite as N concurrent shards against one server. +# Usage: ./run-parallel.sh [SHARDS] [-- behave args] Env: BASE_URL + +set -uo pipefail + +SHARDS="${1:-10}" +if [[ "$SHARDS" =~ ^[0-9]+$ ]]; then + shift +else + SHARDS=10 +fi +[[ "${1:-}" == "--" ]] && shift + +BASE_URL="${BASE_URL:-http://localhost:8080}" +CUCUMBER_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +WORK_ROOT="$CUCUMBER_DIR/.parallel" +REPORT_DIR="${PARALLEL_REPORT_DIR:-$WORK_ROOT/reports}" + +cd "$CUCUMBER_DIR" || exit 1 + +if ! curl -sf --retry 30 --retry-delay 2 --retry-connrefused --retry-all-errors \ + "$BASE_URL/api/v1/info/status" >/dev/null; then + echo "ERROR: no server responding at $BASE_URL" + exit 1 +fi + +mapfile -t ALL_FEATURES < <( + find features -name '*.feature' \ + -not -path 'features/enterprise/*' \ + -not -path 'features/multinode/*' | sort +) + +if [ "${#ALL_FEATURES[@]}" -eq 0 ]; then + echo "ERROR: no feature files found under $CUCUMBER_DIR/features" + exit 1 +fi + +# user_management.feature changes the admin password mid-scenario, so a concurrent +# shard logging in then gets a 401. Keep all admin-auth features in one shard. +AUTH_RE='logged in as admin|I login with username|JWT authentication|stored JWT token' +declare -a AUTH_FEATURES=() FEATURES=() +for feature in "${ALL_FEATURES[@]}"; do + if grep -qE "$AUTH_RE" "$feature"; then + AUTH_FEATURES+=("$feature") + else + FEATURES+=("$feature") + fi +done + +if [ "${#AUTH_FEATURES[@]}" -gt 0 ]; then + echo "Pinning ${#AUTH_FEATURES[@]} auth-coupled feature(s) to a single shard:" + printf ' %s\n' "${AUTH_FEATURES[@]##*/}" + SHARDS=$((SHARDS - 1)) +fi + +# The pin above can take SHARDS to 0 or below, which makes the shard loop run zero +# times: every shardable feature is skipped and the run still reports success. +[ "$SHARDS" -lt 0 ] && SHARDS=0 +[ "${#FEATURES[@]}" -gt 0 ] && [ "$SHARDS" -lt 1 ] && SHARDS=1 + +if [ "$SHARDS" -gt "${#FEATURES[@]}" ]; then + echo "Only ${#FEATURES[@]} shardable feature files, reducing shards from $SHARDS" + SHARDS="${#FEATURES[@]}" +fi + +rm -rf "$WORK_ROOT" +mkdir -p "$WORK_ROOT" "$REPORT_DIR" + +TOTAL_SHARDS=$SHARDS +[ "${#AUTH_FEATURES[@]}" -gt 0 ] && TOTAL_SHARDS=$((SHARDS + 1)) +echo "Running ${#ALL_FEATURES[@]} feature files across $TOTAL_SHARDS concurrent shards against $BASE_URL" + +declare -a PIDS=() + +start_shard() { + local shard=$1 + shift + local SHARD_DIR="$WORK_ROOT/shard-$shard" + mkdir -p "$SHARD_DIR" + # Feature files reference exampleFiles/ and behave.ini relative to the CWD. + cp -r "$CUCUMBER_DIR/exampleFiles" "$SHARD_DIR/exampleFiles" + cp "$CUCUMBER_DIR/behave.ini" "$SHARD_DIR/behave.ini" + + local assigned=("$@") + ( + cd "$SHARD_DIR" || exit 1 + uv run --project "$CUCUMBER_DIR/../../engine" --locked --group cucumber \ + python -m behave "${assigned[@]}" \ + --junit --junit-directory "$REPORT_DIR/shard-$shard" \ + --no-capture -f plain "${BEHAVE_EXTRA[@]}" \ + >"$REPORT_DIR/shard-$shard.log" 2>&1 + ) & + PIDS+=($!) +} + +declare -a BEHAVE_EXTRA=("$@") + +for ((shard = 0; shard < SHARDS; shard++)); do + assigned=() + for ((i = shard; i < ${#FEATURES[@]}; i += SHARDS)); do + assigned+=("$CUCUMBER_DIR/${FEATURES[$i]}") + done + start_shard "$shard" "${assigned[@]}" +done + +if [ "${#AUTH_FEATURES[@]}" -gt 0 ]; then + assigned=() + for feature in "${AUTH_FEATURES[@]}"; do + assigned+=("$CUCUMBER_DIR/$feature") + done + start_shard "$SHARDS" "${assigned[@]}" +fi + +FAILED=0 +for ((shard = 0; shard < TOTAL_SHARDS; shard++)); do + if wait "${PIDS[$shard]}"; then + echo "shard $shard PASSED" + else + echo "shard $shard FAILED" + FAILED=$((FAILED + 1)) + fi +done + +echo "" +echo "=== Parallel shard summary ===" +grep -h '^[0-9]* scenarios passed' "$REPORT_DIR"/shard-*.log 2>/dev/null || true +grep -h '^\[PARALLEL\] [0-9]*/' "$REPORT_DIR"/shard-*.log 2>/dev/null || true + +if [ "$FAILED" -gt 0 ]; then + echo "" + echo "$FAILED of $TOTAL_SHARDS shards failed. Failing scenarios:" + sed -n '/^Failing scenarios:/,/^[0-9]* features/p' "$REPORT_DIR"/shard-*.log 2>/dev/null | + grep 'feature:' | sort -u + echo "" + echo "Parallel consistency failures:" + grep -h -A4 'Parallel consistency failed' "$REPORT_DIR"/shard-*.log 2>/dev/null | head -40 + echo "" + echo "Full logs: $REPORT_DIR/shard-*.log" + exit 1 +fi + +echo "All $TOTAL_SHARDS shards passed." diff --git a/testing/test.sh b/testing/test.sh index 95c0c77edd..254d566763 100644 --- a/testing/test.sh +++ b/testing/test.sh @@ -391,6 +391,17 @@ capture_file_list() { gha_endgroup } +# Tail of container logs shown when a leak is detected. The full log is captured as a +# job artifact anyway, and dumping all of it here buried the actual finding. +TEMP_FILE_LOG_TAIL=${TEMP_FILE_LOG_TAIL:-200} + +# Print the log context for a detected temp-file leak, bounded so the finding stays readable. +print_temp_file_leak_logs() { + local container_name=$1 + echo "Last $TEMP_FILE_LOG_TAIL lines of container logs (full log is in the uploaded artifacts):" + docker logs --tail "$TEMP_FILE_LOG_TAIL" "$container_name" 2>&1 || true +} + # Function to compare before and after file lists compare_file_lists() { local before_file=$1 @@ -417,8 +428,7 @@ compare_file_lists() { if [ -s "${diff_file}.tmp" ]; then echo "WARNING: Temporary files found:" cat "${diff_file}.tmp" - echo "Printing docker logs due to temporary file detection:" - docker logs "$container_name" # Print logs when temp files are found + print_temp_file_leak_logs "$container_name" gha_endgroup return 1 else @@ -450,8 +460,11 @@ compare_file_lists() { if [ -s "${diff_file}.tmp" ]; then echo "WARNING: Temporary files detected:" cat "${diff_file}.tmp" - echo "Printing docker logs due to temporary file detection:" - docker logs "$container_name" # Print logs when temp files are found + echo "These files were still present after the suite finished. Async job results" + echo "and their input copies live under the server's file store and are released" + echo "by the cleanup that features/environment.py runs in after_all - if they show" + echo "up here, that cleanup did not cover them." + print_temp_file_leak_logs "$container_name" return 1 fi fi