fix(output): make file rendering non-destructive and the PPTX target explicit#439
Merged
Conversation
…explicit
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.
DemchaAV
force-pushed
the
fix/output-api-safety
branch
from
July 25, 2026 08:21
daa79d4 to
ddf03f8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
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 it is already serving.
Separately,
DocumentSessionholds one configured output path shared withbuildPdf(), so the no-argbuildPptx()wrote deck bytes into whatever that path was. Its own Javadoc had to warn that a.pdfdefault would receive a PPTX.And
BackendProviders.fixedLayout(format)took the firstServiceLoadermatch, so a third-party provider declaring an already-registered format silently decided which backend rendered the document, depending on classpath order.What changed
Files.createTempFilecreates owner-only andFiles.movecarries that mode onto the destination — silently narrowing a served file — so POSIX permissions are aligned to the destination's existing mode (orrw-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.DocumentSession.buildPptx()(no-arg) removed;buildPptx(Path)stays. Naming the destination is also what lets one session emit both formats. The surface is@Betaand unpublished —graph-compose-coreat the 2.0.0 baseline does not declare the method, which is why the binary-compatibility gate stays green. The real risk wasjavadoc:javadoc: two{@link}references lived in theexamplesmodule.IllegalStateExceptionnaming the competing provider classes.Verification
./mvnw -B -ntp clean verify— BUILD SUCCESS, 1518 tests (from 1508), 0 failures, 2 skipped off POSIX.DocumentRenderingFacadetoFiles.newOutputStream(target)turnsDocumentOutputFileSafetyTestred withexpected: "previously published document" but was: ""../mvnw -P japicmp verify -pl :graph-compose-core— green;git show v2.0.0:…/DocumentSession.java | grep -c buildPptxis0.javadoc:javadocgreen on the six published modules and onexamples.ServiceLoaderfakes sharing one format with a companion test proving unrelated formats still resolve.