mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-14 10:34:06 +02:00
PAYG B-3 / S-3: cucumber suite for shadow-mode flows + CI workflow (#6522)
## What this PR is End-to-end cucumber coverage for the PAYG shadow charging engine (the filter + interceptor stack from #6519), wired into CI via a new `docker-compose-tests-saas.yml` workflow that runs only on PAYG-touching PRs. Stacked on #6519. ## Automated scenarios (run by `docker-compose-tests-saas.yml`) See [`testing/cucumber/features/payg/shadow_charges.feature`](../tree/payg-s3-cucumber/testing/cucumber/features/payg/shadow_charges.feature): | Scenario | Validates | |---|---| | First tool call writes a CHARGED row | Filter + interceptor fire end-to-end | | Lineage join — second call on output | `JobService.joinOrOpen` matching; no new shadow row | | 4xx leaves the row CHARGED | "Customer paid for the attempt" semantics | | ZIP-returning tool records per-PDF OUTPUT | `PaygOutputExtractor` unpacks + records signatures | | Multi-file input writes a single shadow row | Multi-input group sizing | | `X-Stirling-Automation` sets PIPELINE source | Header → `JobSource` detection | All 6 run locally via `./testing/test-payg.sh` and will run on CI for any PR that touches `app/saas/**`, the PAYG cucumber features, the saas compose stack, or the workflow itself. ## Manual-only scenarios — documented in design doc, not in this suite Two parts of the shadow engine are deliberately not automated; the engine paths are unit-tested in `PaygChargeInterceptorTest.afterCompletion_5xx_opened_*`, and the manual procedures (which require a temporary throw endpoint or a container restart with a flag flipped) live in [`notes/PAYG_DESIGN.md` §7.5.2 "PAYG cucumber: manual-only scenarios"](../tree/payg-s3-cucumber/notes/PAYG_DESIGN.md). - **5xx first-step failure → REFUNDED + CLOSED.** No reliably-5xx-ing endpoint exists; manual procedure adds a throw endpoint, runs, asserts, removes. - **Kill-switch (`PAYG_FILTER_ENABLED=false`).** Needs a container restart mid-suite; manual procedure tears down, flips env, brings up, asserts zero shadow rows. If either gets a hot-reload path (test-only throw endpoint shipped behind a profile gate, or admin endpoint for the kill switch), automate it in a follow-up and drop the manual procedure. ## CI workflow `.github/workflows/docker-compose-tests-saas.yml` (new) — self-contained, not wired into `build.yml`'s `files-changed` matrix so the saas-cucumber job fails and succeeds independently. Triggers only on PAYG-relevant paths. No JaCoCo coverage in v1 (saas compose doesn't have the coverage override; can add later). ## Test infrastructure (recap) - **`testing/compose/docker-compose-saas.yml`** — Stirling-PDF backend with `STIRLING_FLAVOR=saas` + Postgres holding the `stirling_pdf` schema. Supabase JWT auto-config disabled; API-key auth via `SECURITY_CUSTOMGLOBALAPIKEY` is the live path the cucumber tests exercise. - **`testing/compose/payg/saas-init.sql`** + **`saas-seed.sql`** — schema bootstrap + idempotent seed (team / user / wallet_policy). - **`testing/cucumber/features/payg/shadow_charges.feature`** — the 6 scenarios above. - **`testing/cucumber/features/steps/payg_step_definitions.py`** — step defs using `requests` (HTTP) + `psycopg` (direct DB inspection). Direct DB reads are deliberate — we want to see the filter's side effects, not relay them through another API layer. - **`testing/test-payg.sh`** — companion runner to `testing/test.sh`. Brings up the saas compose, waits for health, seeds, runs behave, tears down. - **`behave.ini`** excludes `features/payg` from the default behave run (the saas-cucumber CI job invokes it explicitly). ## Why a separate harness from `testing/test.sh` The existing `test.sh` covers the proprietary-flavour stack (no PAYG tables, no saas profile). Coupling two CI matrices that fail and succeed independently into one script is asking for trouble. Keep the saas-cucumber job focused on its own concerns; once the harness is mature, the wider team can decide whether to merge them. ## Tracked in `notes/PAYG_DESIGN.md` §7.5 (PR-S3) + §7.5.2 (manual scenarios).
This commit is contained in:
@@ -49,8 +49,12 @@ public class PaygOutputExtractor {
|
||||
/** {@code %PDF-} in ASCII. */
|
||||
private static final byte[] PDF_MAGIC = {0x25, 0x50, 0x44, 0x46, 0x2D};
|
||||
|
||||
/** {@code PK\x03\x04} — local file header for any non-empty ZIP. */
|
||||
private static final byte[] ZIP_MAGIC = {0x50, 0x4B, 0x03, 0x04};
|
||||
|
||||
private static final String ZIP_CONTENT_TYPE = "application/zip";
|
||||
private static final String PDF_CONTENT_TYPE = "application/pdf";
|
||||
private static final String OCTET_STREAM_CONTENT_TYPE = "application/octet-stream";
|
||||
|
||||
private final TempFileManager tempFileManager;
|
||||
|
||||
@@ -74,11 +78,31 @@ public class PaygOutputExtractor {
|
||||
return List.of();
|
||||
}
|
||||
String mediaType = stripParameters(contentType);
|
||||
if (PDF_CONTENT_TYPE.equalsIgnoreCase(mediaType)) {
|
||||
|
||||
// Stirling-PDF tool endpoints sometimes set Content-Type to
|
||||
// application/octet-stream (or no header at all) even when the body
|
||||
// is a real PDF or ZIP — Spring's default StreamingResponseBody path
|
||||
// doesn't always negotiate content type. When the declared
|
||||
// Content-Type is missing or generic, sniff magic bytes to recover
|
||||
// lineage capture.
|
||||
boolean missingOrGeneric =
|
||||
mediaType == null || OCTET_STREAM_CONTENT_TYPE.equalsIgnoreCase(mediaType);
|
||||
boolean isPdf = PDF_CONTENT_TYPE.equalsIgnoreCase(mediaType);
|
||||
boolean isZip = ZIP_CONTENT_TYPE.equalsIgnoreCase(mediaType);
|
||||
|
||||
if (missingOrGeneric) {
|
||||
if (isMagic(bodyPath, PDF_MAGIC)) {
|
||||
isPdf = true;
|
||||
} else if (isMagic(bodyPath, ZIP_MAGIC)) {
|
||||
isZip = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (isPdf) {
|
||||
// Wrapper-owned path. Don't claim ownership.
|
||||
return List.of(new ExtractedPdf(bodyPath, null));
|
||||
}
|
||||
if (ZIP_CONTENT_TYPE.equalsIgnoreCase(mediaType)) {
|
||||
if (isZip) {
|
||||
return extractZip(bodyPath);
|
||||
}
|
||||
return List.of();
|
||||
@@ -126,20 +150,25 @@ public class PaygOutputExtractor {
|
||||
}
|
||||
|
||||
private boolean isPdfMagic(Path path) {
|
||||
byte[] head = new byte[PDF_MAGIC.length];
|
||||
return isMagic(path, PDF_MAGIC);
|
||||
}
|
||||
|
||||
/** Returns true if the first {@code magic.length} bytes of {@code path} equal {@code magic}. */
|
||||
private boolean isMagic(Path path, byte[] magic) {
|
||||
byte[] head = new byte[magic.length];
|
||||
try (InputStream in = Files.newInputStream(path)) {
|
||||
int read = in.read(head);
|
||||
if (read != PDF_MAGIC.length) {
|
||||
if (read != magic.length) {
|
||||
return false;
|
||||
}
|
||||
for (int i = 0; i < PDF_MAGIC.length; i++) {
|
||||
if (head[i] != PDF_MAGIC[i]) {
|
||||
for (int i = 0; i < magic.length; i++) {
|
||||
if (head[i] != magic[i]) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
} catch (IOException e) {
|
||||
log.debug("PDF magic-byte check failed for {}", path, e);
|
||||
log.debug("Magic-byte check failed for {}", path, e);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
+78
@@ -0,0 +1,78 @@
|
||||
package stirling.software.saas.payg.test;
|
||||
|
||||
import org.springframework.context.annotation.Profile;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
import org.springframework.web.bind.annotation.RequestParam;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
import org.springframework.web.multipart.MultipartFile;
|
||||
|
||||
import io.swagger.v3.oas.annotations.Hidden;
|
||||
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
import stirling.software.common.annotations.AutoJobPostMapping;
|
||||
import stirling.software.common.enumeration.ResourceWeight;
|
||||
|
||||
/**
|
||||
* Cucumber-only force-5xx endpoint. Gated behind the {@code payg-cucumber} Spring profile so the
|
||||
* bean never registers in production — only the PAYG cucumber compose stack activates the profile
|
||||
* (via {@code SPRING_PROFILES_ACTIVE=saas,payg-cucumber} in {@code docker-compose-saas.yml}).
|
||||
*
|
||||
* <p>Purpose: drive the PAYG filter+interceptor's 5xx-first-step branch end-to-end. No reliably-
|
||||
* 5xx-ing real tool endpoint exists in current Stirling — every malformed input is caught as 4xx by
|
||||
* {@code GlobalExceptionHandler}. Without this stub the only way to exercise the refund path was a
|
||||
* manual procedure (a temporary throw endpoint added, run, removed) documented in {@code
|
||||
* notes/PAYG_DESIGN.md} §7.5.2 M1. This controller replaces that procedure with a profile- gated
|
||||
* automated scenario.
|
||||
*
|
||||
* <p>{@link AutoJobPostMapping} consumes {@code multipart/form-data} so the filter's {@code
|
||||
* MultipartHttpServletRequest} cast runs and the input lineage hash is computed before the
|
||||
* controller throws. The PAYG filter chain therefore sees: preHandle → openProcess (CHARGED row
|
||||
* written) → controller throws → afterCompletion observes status 500 → markFirstStepFailed (row →
|
||||
* REFUNDED, job → CLOSED).
|
||||
*
|
||||
* <p>The thrown {@link IllegalStateException} is unwrapped by {@code GlobalExceptionHandler}'s
|
||||
* RuntimeException handler — it has no IOException / IllegalArgument / BaseAppException cause, so
|
||||
* it falls through to the "Unexpected RuntimeException" branch which sets HTTP 500. Don't use
|
||||
* {@code ResponseStatusException} here — its dedicated handler is reached before the unwrapping
|
||||
* branch but the upstream test still passes; {@link IllegalStateException} is the more honest mimic
|
||||
* of a real bug-driven 500.
|
||||
*
|
||||
* <p>{@code @Hidden} keeps this endpoint out of OpenAPI / Swagger output even when the profile is
|
||||
* active, so no docs leak to anyone running the cucumber stack interactively.
|
||||
*
|
||||
* <p><b>Return type matters:</b> the method declares {@link ResponseEntity}{@code <Void>}, not
|
||||
* {@code void}. Spring's {@code InvocableHandlerMethod} picks a return-value handler based on the
|
||||
* <i>declared</i> return type; {@code void} matches the "no body, status 200" handler regardless of
|
||||
* what the {@code @Around} advice on {@link AutoJobPostMapping} actually returns at runtime. The
|
||||
* first version of this stub declared {@code void} and the 500 ResponseEntity from {@code
|
||||
* JobExecutorService}'s catch block was silently discarded — the client saw a 200 with an empty
|
||||
* body. Declaring {@code ResponseEntity<Void>} matches the real runtime type and lets the advice's
|
||||
* 500 reach the wire.
|
||||
*/
|
||||
@Slf4j
|
||||
@RestController
|
||||
@Profile("payg-cucumber")
|
||||
@RequestMapping("/api/v1/payg-cucumber")
|
||||
@Hidden
|
||||
public class PaygCucumberThrowController {
|
||||
|
||||
@AutoJobPostMapping(
|
||||
value = "/throw-500",
|
||||
consumes = MediaType.MULTIPART_FORM_DATA_VALUE,
|
||||
resourceWeight = ResourceWeight.SMALL_WEIGHT)
|
||||
public ResponseEntity<Void> throw500(
|
||||
@RequestParam(value = "fileInput", required = false) MultipartFile fileInput) {
|
||||
// The file is read by the PAYG filter via getMultiFileMap() before we get here; the
|
||||
// controller param is just to keep Spring's multipart binding happy. We don't touch it.
|
||||
log.warn(
|
||||
"PAYG cucumber forced 500 (fileInput name='{}' size={} bytes)",
|
||||
fileInput != null ? fileInput.getOriginalFilename() : null,
|
||||
fileInput != null ? fileInput.getSize() : 0);
|
||||
throw new IllegalStateException("PAYG cucumber forced 500");
|
||||
// unreachable — kept as a type signature so AutoJobAspect's @Around return value (the
|
||||
// 500 ResponseEntity from JobExecutorService) actually reaches the wire.
|
||||
}
|
||||
}
|
||||
+55
@@ -102,6 +102,61 @@ class PaygOutputExtractorTest {
|
||||
assertThat(extractor.extract("application/zip", zip)).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
void octetStreamWithPdfMagic_treatedAsPdf(@TempDir Path tmp) throws IOException {
|
||||
// Stirling tool endpoints sometimes set Content-Type to
|
||||
// application/octet-stream for streamed responses even when the body
|
||||
// is a real PDF. The extractor must sniff magic bytes when the
|
||||
// declared Content-Type is generic.
|
||||
Path body = tmp.resolve("body.bin");
|
||||
Files.write(body, pdfBytes("real-pdf-content"));
|
||||
List<PaygOutputExtractor.ExtractedPdf> out =
|
||||
extractor.extract("application/octet-stream", body);
|
||||
assertThat(out).hasSize(1);
|
||||
assertThat(out.get(0).path()).isEqualTo(body);
|
||||
}
|
||||
|
||||
@Test
|
||||
void octetStreamWithZipMagic_unpackedAsZip(@TempDir Path tmp) throws IOException {
|
||||
Path zip = tmp.resolve("body.bin");
|
||||
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zip))) {
|
||||
writeEntry(zos, "page1.pdf", pdfBytes("a"));
|
||||
writeEntry(zos, "page2.pdf", pdfBytes("b"));
|
||||
}
|
||||
List<PaygOutputExtractor.ExtractedPdf> out =
|
||||
extractor.extract("application/octet-stream", zip);
|
||||
try {
|
||||
assertThat(out).hasSize(2);
|
||||
} finally {
|
||||
for (PaygOutputExtractor.ExtractedPdf p : out) {
|
||||
p.close();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void nullContentTypeWithZipMagic_unpackedAsZip(@TempDir Path tmp) throws IOException {
|
||||
Path zip = tmp.resolve("body.bin");
|
||||
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zip))) {
|
||||
writeEntry(zos, "page1.pdf", pdfBytes("a"));
|
||||
}
|
||||
List<PaygOutputExtractor.ExtractedPdf> out = extractor.extract(null, zip);
|
||||
try {
|
||||
assertThat(out).hasSize(1);
|
||||
} finally {
|
||||
for (PaygOutputExtractor.ExtractedPdf p : out) {
|
||||
p.close();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void octetStreamWithoutMagic_returnsEmpty(@TempDir Path tmp) throws IOException {
|
||||
Path body = tmp.resolve("body.bin");
|
||||
Files.write(body, "neither pdf nor zip".getBytes(StandardCharsets.UTF_8));
|
||||
assertThat(extractor.extract("application/octet-stream", body)).isEmpty();
|
||||
}
|
||||
|
||||
private static void writeEntry(ZipOutputStream zos, String name, byte[] data)
|
||||
throws IOException {
|
||||
zos.putNextEntry(new ZipEntry(name));
|
||||
|
||||
Reference in New Issue
Block a user