Files

12 KiB

Fileserver — deviations from the design

The design specification and the implementation plan are frozen documents. Where implementation found them under-specified or self-contradicting, the resolution is recorded here rather than by editing the specification, and every entry names the test that pins the decision.

Resolved inconsistencies in the state machine and contracts

1. CREATED → FAILED has no edge in the transition table

A create that fails after the record exists must end in FAILED, but the table has no direct edge. The record therefore walks CREATED → UPLOADING → FAILED, which is also the honest reading: the upload had been admitted before it failed.

Pinned by UploadApplicationServiceTest (application-core).

2. ContentKey's alphabet admits a leading separator

The design's key pattern [a-z0-9/_-]{16,200} matches /etc/passwd/.... Rejecting an absolute path at the value type would change a design-fixed contract, so the stricter shape check lives in PhysicalPathResolver, per §12.2 rule 1 — the only place that turns an identifier into a path.

Pinned by PhysicalPathResolverTest.

3. VERIFYING → DELETING has no edge

Deleting a file that is mid-verification has no legal transition. The lifecycle service refuses it with 409 FILE_NOT_READY rather than inventing an edge, which matches the allowed-state list the JPA markDeleting statement already enforced.

Pinned by FileLifecycleServiceTest.

4. If-Match is specified as an ETag but the lifecycle was designed around the row version

The HTTP contract sends an entity tag; the metadata store guards on a numeric version. The service takes Optional<String> expectedEtag and compares against the record's strong validator, so the precondition a client sends is the precondition that is checked.

Pinned by FileLifecycleServiceTest.

5. The filename policy left : intact

C:\Windows\system.ini sanitized to C:Windowssystem.ini — a drive-qualified name surviving into display text and headers. : joined the structural strip set.

Pinned by FileserverHardeningContractTest and AmbiguousFilesystemOperationDetectorTest.

Additions the design implies but does not specify

6. fs_recovery_item

§10.2 lists five core tables and none of them can hold the recovery queue, yet §29.3 requires one: reconciliation reports files whose bytes and metadata disagree, and holding that list in memory would lose exactly the cases a restart interrupted. Added in V2__fileserver_recovery_and_staging_cleanup.sql with one open item per file, so repeated sweeps update a worklist rather than accumulating a log.

Pinned by PostgreSqlFileserverReclamationIntegrationTest.

7. fs_cleanup_item.upload_id

A staging object is addressed by upload, not by file. Without this column a queued staging cleanup could name only already-published content, so a cancelled or expired upload left bytes nothing could find. Added in the same migration, with a check constraint that an item names exactly one target.

Pinned by PostgreSqlFileserverReclamationIntegrationTest.

8. ContentReferenceLedger and StagingUploadLocator

The orphan scan must ask whether a record still claims a physical object, and reconciliation must map a file back to the upload that last staged it. Neither question is answerable through the design's FileMetadataStore or UploadSessionStore as written. Rather than widen those design-fixed interfaces, both are narrow single-method ports.

Pinned by PostgreSqlFileserverReclamationIntegrationTest and LocalOrphanScanAdapterTest.

Interpretations

9. Quota settlement is FIFO within a scope

Nothing links a reservation row to the upload that took it, and the design deliberately reclaims stragglers by TTL and the STALE_QUOTA_RESERVATION cleanup type rather than threading a reservation id through the upload session. QuotaCommitGateway therefore settles the oldest live reservation in the file's namespace.

Which row closes does not change any quota decision: enforcement sums reserved and committed bytes per scope and never reads an individual row. Concurrent uploads of different sizes can leave the reserved total transiently high or low, and it converges as each settles. Durable usage with no live reservation behind it — an upload that outlived its TTL — is still recorded, because a ledger that silently under-counts is worse than one that is briefly imprecise.

Pinned by PostgreSqlFileserverReclamationIntegrationTest.

10. Zero copy is a channel transfer, not a file handoff

Task 22 asks for zero copy on local files; §19 forbids a Path leaving the storage adapter, and §5 of the plan forbids adding Path to the content store. WebFlux's zero-copy API takes a Path, so that route is closed.

The servlet path takes the other one: ZeroCopyDownloadGateway receives a WritableByteChannel from the transport and the storage adapter performs FileChannel.transferTo into it. That is a genuine kernel-level transfer with no filesystem concept leaving storage. The reactive path continues to stream with bounded demand.

Zero copy is an optimization with no observable difference: when storage declines, the response is streamed and is byte-identical.

Pinned by LocalStorageGatewayContractTest and ZeroCopyEligibilityTest.

11. The Fileserver JPA stores are gated on the capability switch

The metadata store, session store, quota service, queues, ledger, and staging locator carry @ConditionalOnProperty(app.fileserver-platform.enabled) even though the rest of adapter:outbound:persistence-jpa is unconditional.

Without the gate, every composition root that includes the persistence module built these beans — including sample-portfolio, which has no Fileserver — and each of them needs collaborators only the Fileserver configuration provides. That is the same rule the design states for the transport surface, applied to persistence: no surface appears merely because the dependency is present.

FileStateMachine is bound alongside them, in FileserverStorageConfiguration. It had no production binding at all before, which made the metadata store unconstructible in any component-scanned context.

Pinned by SampleApplicationContextTest (the capability off) and FileserverRuntimeAssemblyTest (the capability on).

12. Transaction boundaries are owned by the application services, and are deliberately narrow

The design does not say where a transaction begins. The repository does: adapter:outbound:persistence-jpa forbids a repository adapter from owning a @Transactional boundary, and application-core owns them through TransactionPort. The Fileserver follows that rule — every Jpa* store here declares no @Transactional of its own.

What is specific to this capability is how narrow the boundaries are. A boundary covers a contiguous run of metadata writes and stops before every storage call, because a filesystem operation inside a database transaction would hold a connection for the length of a byte transfer. The upload path therefore has three boundaries, not one: acquire the lease, transfer the bytes, commit the offset.

Where several stores must agree, they share one boundary:

Unit Why it is one boundary
reserve quota + insert record + create session a reservation that outlived a failed insert holds capacity for a file that never existed
READY transition + quota commit a finished file whose reservation was never converted holds capacity until the reservation expires
markDeleting + enqueue cleanup a file that stopped being reachable with nothing queued to reclaim it is never collected
content delete settlement: reclaim + retire record + close queue item half of it leaves the item to be retried against content that no longer exists

What this cannot make atomic is the storage/metadata seam itself — no database boundary could. That seam is exactly what the ambiguous-completion path and the reconciler exist for, and the one hand-written compensation that remains (staging creation failing after the records committed) is there for the same reason.

Pinned by FileserverRoundTripContractTest against real PostgreSQL; the application tests use DirectTransactions, which runs a boundary inline and counts it.

Not implemented

AsyncContentStore, CapacityAwareContentStore, CopyCapableContentStore, DelegatedDownloadStore

Four optional content-store SPIs are declared in application-core with no implementation. Each is an extension point for a backend this template does not ship:

  • AsyncContentStore — for a backend whose native client is non-blocking. The local platform is blocking, and the reactive transport bridges to it on a dedicated I/O scheduler.
  • CapacityAwareContentStore — capacity is reported through StorageHealthPort and StorageUsageProbe, which the local platform implements.
  • CopyCapableContentStore — server-side copy is delivered by CopyContentGateway; the local platform has no cheaper primitive than a streamed copy.
  • DelegatedDownloadStore — delegation is delivered at the transport boundary by the nginx X-Accel-Redirect strategy, which needs no store participation.

ContentStoreCapabilities reports what the running store actually supports, so no unimplemented SPI is advertised as available.

Known deviation from the repository's application-layer contract

5. Fileserver application services are not CommandUseCase / QueryUseCase

src/application-core/CLAUDE.md requires every inbound port implementation to extend CommandUseCase or QueryUseCase and to carry @UseCaseCapability, which declares its transaction mode, idempotency and repository access. The Fileserver instead exposes multi-method services — UploadApplicationService, DownloadApplicationService, FileLifecycleService, FileserverAdminService and their Default* implementations.

This is a real deviation, not an oversight, and it is unenforced: the ArchUnit rules inbound_port_implementations_end_with_use_case and inbound_port_implementations_declare_capability only match types that implement UseCase, so a service that never does is silently exempt. The capability contract that every other feature in this repository declares is therefore absent here.

Two things follow from it. The transaction mode of each operation is expressed only by which TransactionPort method the body happens to call, rather than declared and checked. And the application layer holds transport policy it would not hold if each operation were a use case with its own command: HTTP status codes on FileserverErrorCode, Range and conditional-request parsing in api.transfer, and Content-Disposition construction.

The status mapping in particular is a deliberate trade rather than an accident. It lives in application-core so the servlet transport, the reactive transport and the Nginx delegation path cannot answer the same failure with three different statuses. Moving it to the transport layer resolves the layering complaint and reintroduces exactly that drift, which is why this is an architecture decision rather than a cleanup.

Status: open, deliberately unresolved in this change set. Closing it means roughly thirty command/query use cases, a decision about where the shared status vocabulary lives, and a change to the ArchUnit rules so a service that bypasses the contract fails the build instead of being exempt from it. That belongs in its own ADR with its own review, and doing it inside a correctness patch would mix a large mechanical refactor into changes that need to be readable.

Nothing here is pinned by a test, because the deviation is the absence of a constraint. The next step is the ADR, not another test.