From ddf03f89870aafd9283ecfd704b0c944081892fd Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Sat, 25 Jul 2026 08:59:52 +0100 Subject: [PATCH] fix(output): make file rendering non-destructive and the PPTX target explicit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rendering to a file opened the destination before the backend produced a byte, so the file was truncated first and written second. A render that then failed — an atomic node taller than the page, a missing backend, a full disk — replaced a published document with an empty one. That is the exact shape of the library's main use case: a server re-rendering a document that is already being served. Route both file paths through a scratch file in the destination's own directory, moved onto the destination only after the render returns normally. The move is atomic where the filesystem supports it, so a concurrent reader never sees a half-written document, and the scratch file is discarded on failure without masking the render's own exception. Two details the naive form gets wrong are handled explicitly: a temp file is created owner-only and `Files.move` carries that mode onto the destination, which would silently narrow a served file, so POSIX permissions are aligned to the destination's existing mode (or rw-r--r-- for a new file) before the move; and replacing the destination entry replaces a symlink rather than writing through it, which the contract now states. Remove the no-arg `DocumentSession.buildPptx()`. The session holds one configured output path, shared with `buildPdf()`, so the no-arg form wrote deck bytes into whatever that path was — its own Javadoc had to warn that a `.pdf` default would receive a PPTX. Naming the destination is also what lets one session emit both formats. The surface is `@Beta` and unpublished, so nothing released can depend on it; `graph-compose-core` at the 2.0.0 baseline does not declare the method at all, which is why the binary-compatibility gate stays green. Make duplicate backend registrations fail instead of resolving by classpath order: a third-party provider declaring an already-registered format silently decided which backend rendered the document. Both the by-format lookup and the no-arg default now share one resolver that throws `IllegalStateException` naming the competing classes. Tests: the qa regression writes a sentinel document, fails a render on an oversized node, and asserts the sentinel survives — it fails on the previous implementation with `expected: "previously published document" but was: ""`. Helper-level tests cover mid-stream failure, scratch cleanup, first-time creation, a missing parent directory, and the permission alignment (skipped off POSIX). Two ServiceLoader fakes sharing one format cover the ambiguity, with a companion test proving unrelated formats still resolve. Verified: full reactor `clean verify` green (1518 tests, 0 failures, 2 skipped off POSIX); japicmp against the 2.0.0 baseline green; `javadoc:javadoc` green on the six published modules and on examples, where two `{@link}` references to the removed overload lived. --- CHANGELOG.md | 24 ++- .../java/com/demcha/compose/GraphCompose.java | 6 +- .../document/api/AtomicFileOutput.java | 144 ++++++++++++++++++ .../document/api/DocumentRenderingFacade.java | 4 +- .../compose/document/api/DocumentSession.java | 28 +--- .../document/api/MultiSectionDocument.java | 4 +- .../backend/fixed/BackendProviders.java | 41 ++++- .../exceptions/MissingBackendException.java | 2 +- .../document/api/FileOutputSafetyTest.java | 119 +++++++++++++++ .../fixed/BackendProvidersSelectionTest.java | 17 +++ .../fixed/DuplicateFormatFakeProviders.java | 50 ++++++ ...t.backend.fixed.FixedLayoutBackendProvider | 2 + examples/README.md | 4 +- .../flagships/EngineDeckPptxExample.java | 4 +- .../flagships/MavenBannerPptxExample.java | 4 +- .../api/DocumentOutputFileSafetyTest.java | 115 ++++++++++++++ .../pptx/PptxSessionConvenienceTest.java | 24 ++- 17 files changed, 528 insertions(+), 64 deletions(-) create mode 100644 core/src/main/java/com/demcha/compose/document/api/AtomicFileOutput.java create mode 100644 core/src/test/java/com/demcha/compose/document/api/FileOutputSafetyTest.java create mode 100644 core/src/test/java/com/demcha/compose/document/backend/fixed/DuplicateFormatFakeProviders.java create mode 100644 qa/src/test/java/com/demcha/compose/document/api/DocumentOutputFileSafetyTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c12c0fd0..8a76c794 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,26 @@ follow semantic versioning; release dates are ISO 8601. ### Public API +- **A failed render no longer destroys the document it was replacing.** + `buildPdf(Path)`, `buildPptx(Path)` and multi-section `buildPdf(Path)` render into + a scratch file in the destination's own directory and move it onto the destination + only after the render returns. Previously the destination was opened — and therefore + truncated — before the backend produced a byte, so an oversized node, a missing + backend, or a full disk left an empty file where a published document used to be. + The move is atomic where the filesystem supports it, so a concurrent reader never + observes a half-written document; on POSIX the destination keeps the permissions it + already had, or gets `rw-r--r--` when it is new. +- **`DocumentSession.buildPptx()` (no-arg) is removed**; use `buildPptx(Path)`. The + session has a single configured output path, shared with `buildPdf()`, so the no-arg + form wrote deck bytes into whatever that path was — including a file named `.pdf`. + PPTX output now always names its destination, which is also what lets one session + emit both formats. The PPTX surface is `@Beta` (Experimental) and was never published, + so no released code can depend on the removed overload. +- **Two backends registered for one format now fail loudly.** + `BackendProviders.fixedLayout(format)` and the no-arg default previously took the + first `ServiceLoader` match, so a third-party provider declaring an existing format + silently decided the renderer by classpath order. Both entry points now throw + `IllegalStateException` naming the competing provider classes. - **Keep a heading with its content** — `SectionBuilder.keepWithNext()`. A section marked keep-with-next is never left stranded as the last block on a page apart from the content it introduces: when the section plus the first slice of the following @@ -184,7 +204,7 @@ follow semantic versioning; release dates are ISO 8601. documents that custom handlers do not apply inside rasterized clip composites. - `DocumentSession.toPptxBytes()` / `writePptx(OutputStream)` / - `buildPptx()` / `buildPptx(Path)` — the PPTX counterparts of the PDF + `buildPptx(Path)` / `buildPptx(Path)` — the PPTX counterparts of the PDF convenience trio. The backend resolves through the format-keyed provider (`BackendProviders.fixedLayout("pptx")`), so the core stays free of a PPTX dependency: with `graph-compose-render-pptx` on the classpath the session's @@ -243,7 +263,7 @@ follow semantic versioning; release dates are ISO 8601. AUTO-PAGINATION` tags and a `code → layout → PDF/PPTX` diagram whose amber connectors branch to both backends — composed as one full-bleed `CanvasLayerNode` and emitted as an editable PowerPoint slide via - `buildPptx()`. Registered in `GenerateAllExamples` and listed in the + `buildPptx(Path)`. Registered in `GenerateAllExamples` and listed in the examples gallery with committed PDF/PPTX previews; a native-shape guard (`MavenBannerNativeShapeTest`) pins the slide to a single picture (the badge checkmark) with the panels, tags, code and diagram all native. diff --git a/core/src/main/java/com/demcha/compose/GraphCompose.java b/core/src/main/java/com/demcha/compose/GraphCompose.java index ebb68d74..7beb842d 100644 --- a/core/src/main/java/com/demcha/compose/GraphCompose.java +++ b/core/src/main/java/com/demcha/compose/GraphCompose.java @@ -85,9 +85,11 @@ public static DocumentBuilder document() { /** * Starts the canonical semantic document composition flow with a default output target - * used by {@link DocumentSession#buildPdf()} and {@link DocumentSession#buildPptx()}. + * used by {@link DocumentSession#buildPdf()}. PPTX output always takes an explicit + * path — see {@link DocumentSession#buildPptx(Path)} — so one session can emit both + * formats without the deck overwriting the PDF. * - * @param outputFile default output path for the no-arg build methods + * @param outputFile default output path for {@code buildPdf()} * @return builder for creating a semantic document session */ public static DocumentBuilder document(Path outputFile) { diff --git a/core/src/main/java/com/demcha/compose/document/api/AtomicFileOutput.java b/core/src/main/java/com/demcha/compose/document/api/AtomicFileOutput.java new file mode 100644 index 00000000..88a6c448 --- /dev/null +++ b/core/src/main/java/com/demcha/compose/document/api/AtomicFileOutput.java @@ -0,0 +1,144 @@ +package com.demcha.compose.document.api; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Set; + +/** + * Writes a rendered document to a file without destroying the previous + * contents when the render fails. + * + *

Rendering straight into the destination truncates it the moment the + * stream opens, long before the backend has produced a single byte. A render + * that then fails — a missing backend, an unsupported payload, a full disk — + * leaves the caller with an empty or half-written file where a good document + * used to be. That matters most for the case this library is built for: a + * server that overwrites a previously published document in place.

+ * + *

So the bytes go to a temporary file in the destination's own directory + * and are moved onto the destination only after the writer returns normally. + * The move is atomic where the filesystem supports it, which also means a + * concurrent reader never observes a partially written document. On failure + * the temporary file is removed and the destination is left untouched.

+ * + *

Permissions. {@link Files#createTempFile} deliberately creates an + * owner-only file, and {@link Files#move} carries those permissions onto the + * destination — which would silently narrow a served file from world-readable + * to owner-only. On POSIX filesystems the temporary file is therefore widened + * before the move: to the permissions the destination already had when it + * exists, and to {@code rw-r--r--} when it does not. Filesystems without POSIX + * support (Windows) are unaffected.

+ * + *

Symbolic links. Publishing replaces the destination entry. + * When the destination is a symlink, the link itself is replaced by the rendered + * file rather than the render being written through it to the link's target. + * Render to the real path when a symlink must survive.

+ * + * @author Artem Demchyshyn + */ +final class AtomicFileOutput { + + private static final Logger LOG = LoggerFactory.getLogger("com.demcha.compose.document.lifecycle"); + + private static final Set DEFAULT_PERMISSIONS = + Set.copyOf(PosixFilePermissions.fromString("rw-r--r--")); + + private AtomicFileOutput() { + } + + /** + * Writes bytes into a caller-owned stream. + */ + @FunctionalInterface + interface StreamWriter { + /** + * @param output stream to write to; closed by the caller, not the writer + * @throws Exception if rendering fails + */ + void writeTo(OutputStream output) throws Exception; + } + + /** + * Renders through {@code writer} and publishes the result at {@code target}. + * + * @param target destination file, replaced only after a successful render + * @param writer produces the document bytes + * @throws NoSuchFileException if the destination's parent directory does not exist + * @throws Exception whatever the writer throws, with {@code target} untouched + */ + static void write(Path target, StreamWriter writer) throws Exception { + Path directory = target.toAbsolutePath().getParent(); + if (directory == null || !Files.isDirectory(directory)) { + throw new NoSuchFileException( + target.toString(), + null, + "The parent directory does not exist. Create it before rendering."); + } + + Path temporary = Files.createTempFile(directory, ".graphcompose-", ".tmp"); + boolean published = false; + try { + try (OutputStream output = Files.newOutputStream(temporary)) { + writer.writeTo(output); + } + alignPermissions(temporary, target); + move(temporary, target); + published = true; + } finally { + if (!published) { + discard(temporary); + } + } + } + + /** + * Removes the scratch file without masking the failure that caused it to be + * abandoned. A cleanup {@link IOException} — a scanner still holding the + * handle, a revoked mount — must not replace the render diagnostic the + * caller actually needs. + */ + private static void discard(Path temporary) { + try { + Files.deleteIfExists(temporary); + } catch (IOException ex) { + LOG.debug("document.output.scratch-cleanup-failed path={}", temporary, ex); + } + } + + private static void move(Path temporary, Path target) throws IOException { + try { + Files.move(temporary, target, + StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException ex) { + // Some network and FUSE filesystems reject ATOMIC_MOVE. Replacing + // without the atomicity guarantee is still strictly better than + // truncating the destination up front. + Files.move(temporary, target, StandardCopyOption.REPLACE_EXISTING); + } + } + + private static void alignPermissions(Path temporary, Path target) { + if (!temporary.getFileSystem().supportedFileAttributeViews().contains("posix")) { + return; + } + try { + Set permissions = Files.exists(target) + ? Files.getPosixFilePermissions(target) + : DEFAULT_PERMISSIONS; + Files.setPosixFilePermissions(temporary, permissions); + } catch (IOException | UnsupportedOperationException ex) { + // A render that succeeded must not fail over file metadata; the + // document is still published, just with the temp file's mode. + } + } +} diff --git a/core/src/main/java/com/demcha/compose/document/api/DocumentRenderingFacade.java b/core/src/main/java/com/demcha/compose/document/api/DocumentRenderingFacade.java index 97cb083f..a891377e 100644 --- a/core/src/main/java/com/demcha/compose/document/api/DocumentRenderingFacade.java +++ b/core/src/main/java/com/demcha/compose/document/api/DocumentRenderingFacade.java @@ -189,8 +189,8 @@ private void buildFixedLayout(String format, Path outputFile) throws Exception { long startNanos = System.nanoTime(); LIFECYCLE_LOG.debug("document.{}.build.start sessionId={} revision={} roots={}", format, context.sessionId(), context.revision(), context.rootCount()); - try (OutputStream output = Files.newOutputStream(target)) { - writeFixedLayout(format, output); + try { + AtomicFileOutput.write(target, output -> writeFixedLayout(format, output)); LIFECYCLE_LOG.debug( "document.{}.build.end sessionId={} revision={} durationMs={}", format, diff --git a/core/src/main/java/com/demcha/compose/document/api/DocumentSession.java b/core/src/main/java/com/demcha/compose/document/api/DocumentSession.java index 281f3c2d..1cb74ec1 100644 --- a/core/src/main/java/com/demcha/compose/document/api/DocumentSession.java +++ b/core/src/main/java/com/demcha/compose/document/api/DocumentSession.java @@ -54,7 +54,7 @@ *
  • inspect {@link #layoutGraph()} / {@link #layoutSnapshot()} as needed
    • *
  • render with {@link #writePdf(OutputStream)}, {@link #toPdfBytes()}, {@link #buildPdf()}, * their PPTX counterparts ({@link #writePptx(OutputStream)}, {@link #toPptxBytes()}, - * {@link #buildPptx()}), or a custom backend
    • + * {@link #buildPptx(Path)}), or a custom backend * * *

    Thread-safety: this type is mutable and not thread-safe.

    @@ -989,32 +989,6 @@ public void writePptx(OutputStream output) throws DocumentRenderingException { }); } - /** - * Builds the current document as a .pptx deck into the default output file - * configured on the builder. See {@link #toPptxBytes()} for the geometry - * and classpath contract. - * - *

    The default file is shared with {@link #buildPdf()}: the session has - * one configured output path, and this method writes deck bytes to it - * as-is — pass an explicit {@code .pptx} path to - * {@link #buildPptx(Path)} when the default is named for PDF output.

    - * - * @throws IllegalStateException if no default output file was configured - * @throws DocumentRenderingException if PPTX rendering fails - * @since 2.1.0 - */ - @Beta - public void buildPptx() throws DocumentRenderingException { - ensureOpen(); - if (defaultOutputFile == null) { - throw new IllegalStateException("No default output file was configured for this document session."); - } - wrapRendering("build PPTX at '" + defaultOutputFile + "'", () -> { - renderingFacade.buildPptx(defaultOutputFile); - return null; - }); - } - /** * Builds the current document as a .pptx deck into the supplied output * file. See {@link #toPptxBytes()} for the geometry and classpath contract. diff --git a/core/src/main/java/com/demcha/compose/document/api/MultiSectionDocument.java b/core/src/main/java/com/demcha/compose/document/api/MultiSectionDocument.java index 8f6b9b31..5ffd524e 100644 --- a/core/src/main/java/com/demcha/compose/document/api/MultiSectionDocument.java +++ b/core/src/main/java/com/demcha/compose/document/api/MultiSectionDocument.java @@ -140,9 +140,7 @@ public void buildPdf() throws DocumentRenderingException { public void buildPdf(Path outputFile) throws DocumentRenderingException { Objects.requireNonNull(outputFile, "outputFile"); render("build PDF at '" + outputFile + "'", () -> { - try (OutputStream output = Files.newOutputStream(outputFile)) { - backend.writeSections(renderUnits(), output); - } + AtomicFileOutput.write(outputFile, output -> backend.writeSections(renderUnits(), output)); return null; }); } diff --git a/core/src/main/java/com/demcha/compose/document/backend/fixed/BackendProviders.java b/core/src/main/java/com/demcha/compose/document/backend/fixed/BackendProviders.java index 3bc72016..0b6db3e6 100644 --- a/core/src/main/java/com/demcha/compose/document/backend/fixed/BackendProviders.java +++ b/core/src/main/java/com/demcha/compose/document/backend/fixed/BackendProviders.java @@ -11,6 +11,7 @@ import java.util.ServiceLoader; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; +import java.util.stream.Collectors; /** * Locates the fixed-layout backend service providers registered on the @@ -76,13 +77,16 @@ public static FixedLayoutBackendProvider fixedLayout() { * format, resolving and caching it on first use. * *

    Matching against {@link FixedLayoutBackendProvider#format()} is - * case-insensitive. When several providers declare the same format the - * first one enumerated by {@link ServiceLoader} wins.

    + * case-insensitive. Two providers claiming the same format is a classpath + * mistake rather than a preference, so it fails loudly instead of letting + * {@link ServiceLoader} enumeration order silently decide which backend + * renders the document.

    * * @param format output format identifier such as {@code "pdf"} or {@code "pptx"} * @return the provider rendering that format * @throws MissingBackendException if no provider for the format is registered * on the classpath + * @throws IllegalStateException if more than one provider declares the format * @since 2.1.0 */ public static FixedLayoutBackendProvider fixedLayout(String format) { @@ -92,11 +96,31 @@ public static FixedLayoutBackendProvider fixedLayout(String format) { if (cached != null) { return cached; } - FixedLayoutBackendProvider resolved = fixedLayoutProviders().stream() + return cacheByFormat(key, resolveExactlyOne(key)); + } + + /** + * Resolves the single provider declaring {@code key}, refusing an ambiguous + * classpath. Shared by the by-format lookup and the default lookup so both + * entry points agree about what "registered" means. + */ + private static FixedLayoutBackendProvider resolveExactlyOne(String key) { + List matches = fixedLayoutProviders().stream() .filter(provider -> key.equals(normalizedFormat(provider))) - .findFirst() - .orElseThrow(() -> new MissingBackendException(missingFormatMessage(key))); - return cacheByFormat(key, resolved); + .toList(); + if (matches.isEmpty()) { + throw new MissingBackendException(missingFormatMessage(key)); + } + if (matches.size() > 1) { + String names = matches.stream() + .map(provider -> provider.getClass().getName()) + .collect(Collectors.joining(", ")); + throw new IllegalStateException( + "Multiple fixed-layout backends are registered for format \"" + key + "\": " + names + + ". Remove the duplicate artifact from the classpath, or select a backend " + + "explicitly instead of resolving it by format."); + } + return matches.get(0); } /** @@ -122,7 +146,10 @@ private static FixedLayoutBackendProvider defaultFixedLayout() { !DEFAULT_FIXED_LAYOUT_FORMAT.equals(normalizedFormat(provider))) .thenComparing(BackendProviders::normalizedFormat)) .orElseThrow(missingBackend()); - return cacheByFormat(normalizedFormat(chosen), chosen); + // The comparator picks a format deterministically, but two providers + // claiming that format would still make the winner arbitrary. + String key = normalizedFormat(chosen); + return cacheByFormat(key, resolveExactlyOne(key)); } /** diff --git a/core/src/main/java/com/demcha/compose/document/exceptions/MissingBackendException.java b/core/src/main/java/com/demcha/compose/document/exceptions/MissingBackendException.java index b56d2e18..bf5bf4b8 100644 --- a/core/src/main/java/com/demcha/compose/document/exceptions/MissingBackendException.java +++ b/core/src/main/java/com/demcha/compose/document/exceptions/MissingBackendException.java @@ -10,7 +10,7 @@ * {@code io.github.demchaav:graph-compose-render-pptx}) discovered at * runtime through {@link java.util.ServiceLoader}. Calling a convenience output * method — {@code toPdfBytes()}, {@code buildPdf()}, {@code toImages()}, - * {@code toPptxBytes()}, {@code buildPptx()} — or requesting + * {@code toPptxBytes()}, {@code buildPptx(Path)} — or requesting * {@code layoutSnapshot()} without the matching artifact on the classpath * fails with this exception. The message names the artifact to add.

    * diff --git a/core/src/test/java/com/demcha/compose/document/api/FileOutputSafetyTest.java b/core/src/test/java/com/demcha/compose/document/api/FileOutputSafetyTest.java new file mode 100644 index 00000000..495272f3 --- /dev/null +++ b/core/src/test/java/com/demcha/compose/document/api/FileOutputSafetyTest.java @@ -0,0 +1,119 @@ +package com.demcha.compose.document.api; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.List; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * A render that fails must not destroy the document that was already there. + * + *

    Rendering used to open the destination directly, which truncates it before + * the backend has produced a byte — so a mid-render failure replaced a good + * published document with an empty file. These tests pin the contract of the + * {@link AtomicFileOutput} helper in isolation; the session-level path that a + * caller actually reaches is covered by {@code DocumentOutputFileSafetyTest} in + * the qa module, which needs a real backend to get as far as rendering.

    + */ +class FileOutputSafetyTest { + + private static final String SENTINEL = "previously published document"; + + @Test + void aSuccessfulWriteReplacesTheDestinationAndRemovesTheScratchFile(@TempDir Path tempDir) throws Exception { + Path published = tempDir.resolve("report.bin"); + Files.writeString(published, SENTINEL); + + AtomicFileOutput.write(published, output -> output.write("fresh".getBytes(StandardCharsets.UTF_8))); + + assertThat(Files.readString(published)).isEqualTo("fresh"); + assertThat(listFiles(tempDir)).containsExactly(published); + } + + @Test + void aWriterFailingMidStreamPublishesNothing(@TempDir Path tempDir) throws Exception { + Path published = tempDir.resolve("report.bin"); + Files.writeString(published, SENTINEL); + + assertThatThrownBy(() -> AtomicFileOutput.write(published, output -> { + output.write("partial".getBytes(StandardCharsets.UTF_8)); + throw new IllegalStateException("backend blew up halfway"); + })) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("halfway"); + + assertThat(Files.readString(published)) + .describedAs("half-written bytes must never reach the destination") + .isEqualTo(SENTINEL); + assertThat(listFiles(tempDir)).containsExactly(published); + } + + @Test + void writingCreatesTheDestinationWhenItDoesNotExistYet(@TempDir Path tempDir) throws Exception { + Path published = tempDir.resolve("new-report.bin"); + + AtomicFileOutput.write(published, output -> output.write("fresh".getBytes(StandardCharsets.UTF_8))); + + assertThat(Files.readString(published)).isEqualTo("fresh"); + } + + @Test + void publishingKeepsThePermissionsTheDestinationAlreadyHad(@TempDir Path tempDir) throws Exception { + assumePosix(tempDir); + Path published = tempDir.resolve("report.bin"); + Files.writeString(published, SENTINEL); + Set readableByAll = PosixFilePermissions.fromString("rw-r--r--"); + Files.setPosixFilePermissions(published, readableByAll); + + AtomicFileOutput.write(published, output -> output.write("fresh".getBytes(StandardCharsets.UTF_8))); + + assertThat(Files.getPosixFilePermissions(published)) + .describedAs("re-rendering a served document must not narrow it to owner-only") + .isEqualTo(readableByAll); + } + + @Test + void aBrandNewDocumentIsNotPublishedAsOwnerOnly(@TempDir Path tempDir) throws Exception { + assumePosix(tempDir); + Path published = tempDir.resolve("new-report.bin"); + + AtomicFileOutput.write(published, output -> output.write("fresh".getBytes(StandardCharsets.UTF_8))); + + assertThat(Files.getPosixFilePermissions(published)) + .describedAs("the scratch file's owner-only mode must not leak onto the destination") + .contains(PosixFilePermission.OTHERS_READ); + } + + private static void assumePosix(Path directory) { + assumeTrue(directory.getFileSystem().supportedFileAttributeViews().contains("posix"), + "POSIX permissions are not a concept on this filesystem"); + } + + @Test + void aMissingParentDirectoryFailsWithAnActionableMessage(@TempDir Path tempDir) { + Path published = tempDir.resolve("nested").resolve("report.bin"); + + assertThatThrownBy(() -> AtomicFileOutput.write(published, OutputStream::flush)) + .isInstanceOf(NoSuchFileException.class) + .hasMessageContaining("parent directory"); + } + + private static List listFiles(Path directory) throws IOException { + try (var entries = Files.list(directory)) { + return entries.sorted().toList(); + } + } +} diff --git a/core/src/test/java/com/demcha/compose/document/backend/fixed/BackendProvidersSelectionTest.java b/core/src/test/java/com/demcha/compose/document/backend/fixed/BackendProvidersSelectionTest.java index 8a74b1a0..bc26c698 100644 --- a/core/src/test/java/com/demcha/compose/document/backend/fixed/BackendProvidersSelectionTest.java +++ b/core/src/test/java/com/demcha/compose/document/backend/fixed/BackendProvidersSelectionTest.java @@ -39,4 +39,21 @@ void pdfStaysMissingDespiteOtherRegisteredProviders() { .hasMessageContaining("graph-compose-render-pdf") .hasMessageContaining("graph-compose-bundle"); } + + @Test + void twoProvidersClaimingOneFormatFailInsteadOfLettingClasspathOrderDecide() { + assertThatThrownBy(() -> BackendProviders.fixedLayout(DuplicateFormatFakeProviders.FORMAT)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("\"dup\"") + .hasMessageContaining("DuplicateFormatFakeProviders$One") + .hasMessageContaining("DuplicateFormatFakeProviders$Two"); + } + + @Test + void anAmbiguousFormatDoesNotDisturbTheUnambiguousOnes() { + // The duplicate registration must stay contained: resolving it throws, + // but every other format still resolves, and the default is unchanged. + assertThat(BackendProviders.fixedLayout("aaa").format()).isEqualTo("aaa"); + assertThat(BackendProviders.fixedLayout().format()).isEqualTo("aaa"); + } } diff --git a/core/src/test/java/com/demcha/compose/document/backend/fixed/DuplicateFormatFakeProviders.java b/core/src/test/java/com/demcha/compose/document/backend/fixed/DuplicateFormatFakeProviders.java new file mode 100644 index 00000000..8e75135c --- /dev/null +++ b/core/src/test/java/com/demcha/compose/document/backend/fixed/DuplicateFormatFakeProviders.java @@ -0,0 +1,50 @@ +package com.demcha.compose.document.backend.fixed; + +import com.demcha.compose.document.output.DocumentDebugOptions; +import com.demcha.compose.document.output.DocumentOutputOptions; + +/** + * Two ServiceLoader fakes that both declare format {@code "dup"} — the shape a + * classpath takes when a third-party plugin ships a backend for a format the + * official artifact already provides. Selection must refuse the ambiguity + * rather than let enumeration order pick the renderer. + * + *

    The format is deliberately not {@code "aaa"} or {@code "zzz"}: those drive + * {@link BackendProvidersSelectionTest}, and {@code "aaa"} sorts before + * {@code "dup"}, so the default-selection contract there is unaffected.

    + */ +final class DuplicateFormatFakeProviders { + + static final String FORMAT = "dup"; + + private DuplicateFormatFakeProviders() { + } + + /** First registered provider for {@link #FORMAT}. */ + public static final class One implements FixedLayoutBackendProvider { + + @Override + public String format() { + return FORMAT; + } + + @Override + public FixedLayoutRenderer create(DocumentOutputOptions chrome, DocumentDebugOptions debug) { + throw new UnsupportedOperationException("selection-test fake; never renders"); + } + } + + /** Second registered provider for {@link #FORMAT}. */ + public static final class Two implements FixedLayoutBackendProvider { + + @Override + public String format() { + return FORMAT; + } + + @Override + public FixedLayoutRenderer create(DocumentOutputOptions chrome, DocumentDebugOptions debug) { + throw new UnsupportedOperationException("selection-test fake; never renders"); + } + } +} diff --git a/core/src/test/resources/META-INF/services/com.demcha.compose.document.backend.fixed.FixedLayoutBackendProvider b/core/src/test/resources/META-INF/services/com.demcha.compose.document.backend.fixed.FixedLayoutBackendProvider index 733c62e3..72bc0f7b 100644 --- a/core/src/test/resources/META-INF/services/com.demcha.compose.document.backend.fixed.FixedLayoutBackendProvider +++ b/core/src/test/resources/META-INF/services/com.demcha.compose.document.backend.fixed.FixedLayoutBackendProvider @@ -1,2 +1,4 @@ com.demcha.compose.document.backend.fixed.ZuluFakeFixedLayoutBackendProvider com.demcha.compose.document.backend.fixed.AlphaFakeFixedLayoutBackendProvider +com.demcha.compose.document.backend.fixed.DuplicateFormatFakeProviders$One +com.demcha.compose.document.backend.fixed.DuplicateFormatFakeProviders$Two diff --git a/examples/README.md b/examples/README.md index 5b95962f..89f87e4b 100644 --- a/examples/README.md +++ b/examples/README.md @@ -61,9 +61,9 @@ are with the canonical DSL, then jump to its detailed section below. | [Cover Letter](#cover-letter) | One-page `BusinessTheme.modern()` cover letter with section presets | [PDF](../assets/readme/examples/cover-letter.pdf) · [Source](src/main/java/com/demcha/examples/templates/coverletter/CoverLetterFileExample.java) | | [Module-first Profile](#module-first-profile) | Authoring directly against `DocumentSession.module(...).paragraph(...)` — DSL-direct, no template | [PDF](../assets/readme/examples/module-first-profile.pdf) · [Source](src/main/java/com/demcha/examples/flagships/ModuleFirstFileExample.java) | | **Engine Showcase** | Single-page cinematic brand promo — semantic-graph → polished-PDFs visual metaphor with rounded clip frame, magazine headline lockup, KPI cards, capability columns; source of the README hero image | [Source](src/main/java/com/demcha/examples/flagships/EngineShowcase.java) | -| **Engine Deck** | Multi-page **landscape** capability deck — page 1 is a banner infographic (DSL code → engine → backends → **real rendered-document thumbnails**), then an authoring-pipeline walkthrough, and two pages of **real benchmark data** (GraphCompose vs iText 9 vs JasperReports) loaded from a bundled result file and drawn as tables + native charts; the landscape companion to Engine Showcase. The same composition also renders as a **geometry-identical PowerPoint deck** (one page = one editable slide) through `buildPptx()` | [PDF](../assets/readme/examples/engine-deck.pdf) · [Source](src/main/java/com/demcha/examples/flagships/EngineDeckExample.java) · [PPTX source](src/main/java/com/demcha/examples/flagships/EngineDeckPptxExample.java) | +| **Engine Deck** | Multi-page **landscape** capability deck — page 1 is a banner infographic (DSL code → engine → backends → **real rendered-document thumbnails**), then an authoring-pipeline walkthrough, and two pages of **real benchmark data** (GraphCompose vs iText 9 vs JasperReports) loaded from a bundled result file and drawn as tables + native charts; the landscape companion to Engine Showcase. The same composition also renders as a **geometry-identical PowerPoint deck** (one page = one editable slide) through `buildPptx(Path)` | [PDF](../assets/readme/examples/engine-deck.pdf) · [Source](src/main/java/com/demcha/examples/flagships/EngineDeckExample.java) · [PPTX source](src/main/java/com/demcha/examples/flagships/EngineDeckPptxExample.java) | | **Twin Output** | The dual-output hook stated by the artifact itself — a single 16:9 page written once and emitted **twice from the same session**: `buildPdf()` and `buildPptx(...)` produce a print-ready PDF and a PowerPoint slide with identical geometry where text, panels, and vectors stay native, editable shapes (only the clip-masked logo art lands as a picture); the root README shows PowerPoint's own render of the slide next to the PDF | [PDF](../assets/readme/examples/twin-output.pdf) · [PPTX](../assets/readme/examples/twin-output.pptx) · [Source](src/main/java/com/demcha/examples/flagships/TwinOutputExample.java) | -| **Maven Central Banner** | Single 16:9 "Available on Maven Central" brand slide — the `GraphCompose` wordmark, an `io.github.demchaav:graph-compose` coordinate card, `JAVA 17+ / PDF / PPTX / AUTO-PAGINATION` tags, and a `code → layout → PDF/PPTX` diagram whose amber connectors branch to both backends — composed as one full-bleed `CanvasLayerNode` and emitted as an editable PowerPoint slide via `buildPptx()`; panels and text stay native shapes, only the badge checkmark rasterises | [PDF](../assets/readme/examples/maven-banner.pdf) · [PPTX](../assets/readme/examples/maven-banner.pptx) · [Source](src/main/java/com/demcha/examples/flagships/MavenBannerPptxExample.java) | +| **Maven Central Banner** | Single 16:9 "Available on Maven Central" brand slide — the `GraphCompose` wordmark, an `io.github.demchaav:graph-compose` coordinate card, `JAVA 17+ / PDF / PPTX / AUTO-PAGINATION` tags, and a `code → layout → PDF/PPTX` diagram whose amber connectors branch to both backends — composed as one full-bleed `CanvasLayerNode` and emitted as an editable PowerPoint slide via `buildPptx(Path)`; panels and text stay native shapes, only the badge checkmark rasterises | [PDF](../assets/readme/examples/maven-banner.pdf) · [PPTX](../assets/readme/examples/maven-banner.pptx) · [Source](src/main/java/com/demcha/examples/flagships/MavenBannerPptxExample.java) | ### 🧱 Core DSL diff --git a/examples/src/main/java/com/demcha/examples/flagships/EngineDeckPptxExample.java b/examples/src/main/java/com/demcha/examples/flagships/EngineDeckPptxExample.java index 6cdb64d6..7eb7b66c 100644 --- a/examples/src/main/java/com/demcha/examples/flagships/EngineDeckPptxExample.java +++ b/examples/src/main/java/com/demcha/examples/flagships/EngineDeckPptxExample.java @@ -11,7 +11,7 @@ * The Engine Deck flagship rendered as a PowerPoint deck: the exact * composition of {@link EngineDeckExample} — banner infographic, authoring * pipeline, and two benchmark pages — built once and emitted through the - * fixed-layout PPTX backend via {@link DocumentSession#buildPptx()}. One + * fixed-layout PPTX backend via {@link DocumentSession#buildPptx(java.nio.file.Path)}. One * resolved page becomes one identically-sized slide with the same geometry * the PDF carries, so the .pptx opens in PowerPoint as a slide-per-page copy * of the PDF deck — editable shapes and text frames, except clipped regions, @@ -35,7 +35,7 @@ public static Path generate() throws Exception { .margin(16, 16, 30, 16) .create()) { EngineDeckExample.compose(document); - document.buildPptx(); + document.buildPptx(outputFile); } return outputFile; } diff --git a/examples/src/main/java/com/demcha/examples/flagships/MavenBannerPptxExample.java b/examples/src/main/java/com/demcha/examples/flagships/MavenBannerPptxExample.java index 41dbf9d4..44b7be5e 100644 --- a/examples/src/main/java/com/demcha/examples/flagships/MavenBannerPptxExample.java +++ b/examples/src/main/java/com/demcha/examples/flagships/MavenBannerPptxExample.java @@ -32,7 +32,7 @@ /** * Single-slide "Available on Maven Central" brand banner, composed with the * canonical document DSL and emitted through the fixed-layout PPTX backend via - * {@link DocumentSession#buildPptx()}. One resolved 16:9 page becomes one + * {@link DocumentSession#buildPptx(java.nio.file.Path)}. One resolved 16:9 page becomes one * identically-sized slide carrying the same geometry, so the {@code .pptx} * opens in PowerPoint as an editable copy of the banner — gradient, rounded * panels, native paths and text frames. @@ -99,7 +99,7 @@ public static Path generate() throws Exception { .margin(DocumentInsets.zero()) .create()) { compose(document); - document.buildPptx(); + document.buildPptx(outputFile); } return outputFile; } diff --git a/qa/src/test/java/com/demcha/compose/document/api/DocumentOutputFileSafetyTest.java b/qa/src/test/java/com/demcha/compose/document/api/DocumentOutputFileSafetyTest.java new file mode 100644 index 00000000..c7b84ed2 --- /dev/null +++ b/qa/src/test/java/com/demcha/compose/document/api/DocumentOutputFileSafetyTest.java @@ -0,0 +1,115 @@ +package com.demcha.compose.document.api; + +import com.demcha.compose.GraphCompose; +import com.demcha.compose.document.exceptions.AtomicNodeTooLargeException; +import com.demcha.compose.document.image.DocumentImageData; +import com.demcha.compose.document.node.ImageNode; +import com.demcha.compose.document.style.DocumentInsets; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.ByteArrayOutputStream; +import java.awt.image.BufferedImage; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import javax.imageio.ImageIO; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * The session-level half of the non-destructive output contract: a render that + * fails must leave the document already at that path exactly as it was. + * + *

    The failure is triggered the way a real one arrives — an atomic node taller + * than the page, which fails while the layout graph is being compiled, i.e. + * after the destination would have been opened by the old implementation and + * before a single output byte exists. Anything published to a server and then + * re-rendered with bad input hits this exact window.

    + * + *

    Lives in qa rather than core because reaching the render path at all needs + * a font-metrics provider, which the lean core has no backend to supply.

    + */ +class DocumentOutputFileSafetyTest { + + private static final String SENTINEL = "previously published document"; + + @Test + void aFailedRenderLeavesTheExistingDocumentUntouched(@TempDir Path tempDir) throws Exception { + Path published = tempDir.resolve("report.pdf"); + Files.writeString(published, SENTINEL); + + try (DocumentSession session = oversizedDocument()) { + assertThatThrownBy(() -> session.buildPdf(published)) + .isInstanceOf(AtomicNodeTooLargeException.class); + } + + assertThat(Files.readString(published)) + .describedAs("a failed render must not truncate the document it was replacing") + .isEqualTo(SENTINEL); + } + + @Test + void aFailedRenderLeavesNoScratchFileBehind(@TempDir Path tempDir) throws Exception { + Path published = tempDir.resolve("report.pdf"); + Files.writeString(published, SENTINEL); + + try (DocumentSession session = oversizedDocument()) { + assertThatThrownBy(() -> session.buildPdf(published)) + .isInstanceOf(AtomicNodeTooLargeException.class); + } + + assertThat(listFiles(tempDir)) + .describedAs("the scratch file must be removed when the render fails") + .containsExactly(published); + } + + @Test + void aSuccessfulRenderReplacesTheDocumentAndLeavesNoScratchFile(@TempDir Path tempDir) throws Exception { + Path published = tempDir.resolve("report.pdf"); + Files.writeString(published, SENTINEL); + + try (DocumentSession session = GraphCompose.document() + .pageSize(200, 200) + .margin(DocumentInsets.of(12)) + .create()) { + session.pageFlow(page -> page.module("m", module -> module.paragraph("hello"))); + session.buildPdf(published); + } + + assertThat(Files.readAllBytes(published)).startsWith('%', 'P', 'D', 'F'); + assertThat(listFiles(tempDir)).containsExactly(published); + } + + /** A document whose only node is taller than the page can ever be. */ + private static DocumentSession oversizedDocument() throws Exception { + DocumentSession session = GraphCompose.document() + .pageSize(180, 180) + .margin(DocumentInsets.of(12)) + .create(); + session.add(new ImageNode( + "TooTallImage", + DocumentImageData.fromBytes(onePixelPng()), + 96.0, + 240.0, + DocumentInsets.zero(), + DocumentInsets.zero())); + return session; + } + + private static byte[] onePixelPng() throws IOException { + BufferedImage image = new BufferedImage(1, 1, BufferedImage.TYPE_INT_ARGB); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + ImageIO.write(image, "png", bytes); + return bytes.toByteArray(); + } + + private static List listFiles(Path directory) throws IOException { + try (var entries = Files.list(directory)) { + return entries.sorted().toList(); + } + } +} diff --git a/render-pptx/src/test/java/com/demcha/compose/document/backend/fixed/pptx/PptxSessionConvenienceTest.java b/render-pptx/src/test/java/com/demcha/compose/document/backend/fixed/pptx/PptxSessionConvenienceTest.java index 71fab6c0..0f853427 100644 --- a/render-pptx/src/test/java/com/demcha/compose/document/backend/fixed/pptx/PptxSessionConvenienceTest.java +++ b/render-pptx/src/test/java/com/demcha/compose/document/backend/fixed/pptx/PptxSessionConvenienceTest.java @@ -102,31 +102,27 @@ void buildPptxWritesTheSuppliedFile(@TempDir Path tempDir) throws Exception { } @Test - void buildPptxUsesTheConfiguredDefaultOutputFile(@TempDir Path tempDir) throws Exception { - Path target = tempDir.resolve("default-deck.pptx"); - try (DocumentSession session = GraphCompose.document(target) + void buildPptxNeverWritesDeckBytesIntoTheSessionsDefaultPdfFile(@TempDir Path tempDir) throws Exception { + // The session's default output file is a PDF target. PPTX output takes an + // explicit path, so the deck can never land in it by omission. + Path pdfTarget = tempDir.resolve("report.pdf"); + Path deckTarget = tempDir.resolve("report.pptx"); + try (DocumentSession session = GraphCompose.document(pdfTarget) .pageSize(400, 300) .margin(DocumentInsets.of(24)) .create()) { session.add(session.dsl().shape().name("Card").size(140, 40) .fillColor(DocumentColor.ROYAL_BLUE) .build()); - session.buildPptx(); - try (XMLSlideShow show = new XMLSlideShow(Files.newInputStream(target))) { + session.buildPptx(deckTarget); + + assertThat(pdfTarget).doesNotExist(); + try (XMLSlideShow show = new XMLSlideShow(Files.newInputStream(deckTarget))) { assertThat(show.getSlides()).hasSize(1); } } } - @Test - void buildPptxWithoutADefaultOutputFileFails() { - try (DocumentSession session = composeWithChrome()) { - assertThatThrownBy(session::buildPptx) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("default output file"); - } - } - @Test void anEmptySessionRefusesToRender() { try (DocumentSession session = GraphCompose.document()