Skip to content

fix(spi-transfer): ExportResult.END carried ResultType.CONTINUE - #1510

Merged
alexeyqu merged 1 commit into
masterfrom
fix/export-result-end-constant
Aug 18, 2026
Merged

alexeyqu merged 1 commit into
masterfrom
fix/export-result-end-constant

Conversation

@alexeyqu

Copy link
Copy Markdown
Collaborator

Summary

ExportResult exposes two convenience constants. The second one is wrong:

public static final ExportResult CONTINUE = new ExportResult(ResultType.CONTINUE);
public static final ExportResult END = new ExportResult(ResultType.CONTINUE);  // <-- END, built with CONTINUE

One character: END now carries ResultType.END.

Why it matters, and why it doesn't (yet)

The constant has no call sites anywhere in the repo — every exporter constructs new ExportResult<>(ResultType.END, ...) directly — so this is latent, not a live bug. Nothing changes for existing code.

It is worth fixing anyway because it is the obvious thing for a new exporter to reach for, and the resulting failure is quiet rather than loud. PortabilityInMemoryDataCopier#copyHelper terminates on continuationData == null, not on ResultType:

ContinuationData continuationData = exportResult.getContinuationData();
if (null != continuationData) { ... recurse ... }

So an exporter returning ExportResult.END would not spin — the constant also carries null exportedData, and copyIteration skips the import entirely when that is null. The job would run to COMPLETE having imported nothing at all. That is a much harder thing to notice than a hang.

I hit this while writing an exporter for the offline-demo module (#1509) and worked around it there by using the enum directly. Splitting the fix out so it lands as a fix: in its own right rather than being buried in a feat: — this repo squash-merges, so the PR title is what commitizen sees.

Test plan

  • docker compose run --rm test --no-daemon build — full repo compiles and tests pass
  • grep -rn "ExportResult.END" --include=*.java . — confirms the constant has no consumers, so no behaviour changes for existing callers

Note for downstream consumers

portability-spi-transfer is published to Maven Central. Any external caller relying on the current value of ExportResult.END would be relying on it reporting CONTINUE, which is a bug rather than a contract — but it is a behaviour change for them, so it is worth a line in the release notes.

The END constant was built with ResultType.CONTINUE, so an exporter
returning it would report CONTINUE. It has no call sites in the repo,
so this is latent rather than live -- but it is the obvious thing for a
new exporter to reach for, and the failure is quiet: copyHelper
terminates on null continuation data rather than on ResultType, so the
job would complete having imported nothing.
@alexeyqu

Copy link
Copy Markdown
Collaborator Author

Surprisingly, this typo existed since Jan 22, 2018 #29 cc @lisad 😮

@alexeyqu
alexeyqu merged commit 2e24062 into master Aug 18, 2026
7 checks passed
@alexeyqu
alexeyqu deleted the fix/export-result-end-constant branch August 18, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants