refactor(import): replace importSingleDocument boolean return with ImportResult enum #620

New Issue

marcel · 2026-05-18T15:20:58+02:00

marcel commented

2026-05-18 15:20:58 +02:00

Context

importSingleDocument currently returns boolean:

false when the document already exists and is not a PLACEHOLDER (intentional skip)
false when the S3 upload fails (error)
true when the document was successfully imported

A caller reading if (imported) cannot distinguish between these two very different failure modes. This makes adding per-category counting (processed / skipped / failed) error-prone.

Proposed change

Replace the boolean return with a package-private enum:

enum ImportResult { IMPORTED, SKIPPED_EXISTING, FAILED_S3, FAILED_READ }

Update processRows to switch on the result and route each case to the appropriate counter/list.

Acceptance criteria

All existing MassImportServiceTest tests pass unchanged (behaviour is identical)
The three counters in ProcessResult — processed, skippedFiles, and (future) failedFiles — are populated from distinct ImportResult values with no ambiguity

Surfaced during review of #618. The S3 failure surfacing work (#[s3 issue]) depends on this enum being in place first.

## Context `importSingleDocument` currently returns `boolean`: - `false` when the document already exists and is not a PLACEHOLDER (intentional skip) - `false` when the S3 upload fails (error) - `true` when the document was successfully imported A caller reading `if (imported)` cannot distinguish between these two very different failure modes. This makes adding per-category counting (processed / skipped / failed) error-prone. ## Proposed change Replace the `boolean` return with a package-private enum: ```java enum ImportResult { IMPORTED, SKIPPED_EXISTING, FAILED_S3, FAILED_READ } ``` Update `processRows` to switch on the result and route each case to the appropriate counter/list. ## Acceptance criteria - All existing `MassImportServiceTest` tests pass unchanged (behaviour is identical) - The three counters in `ProcessResult` — `processed`, `skippedFiles`, and (future) `failedFiles` — are populated from distinct `ImportResult` values with no ambiguity ## Related Surfaced during review of #618. The S3 failure surfacing work (#[s3 issue]) depends on this enum being in place first.

marcel added the P3-later refactor labels 2026-05-18 15:21:12 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#620