Skip to content

Suggestions: Backend Services for Item Image Upload

Improvement suggestions, technical debt, and documentation gaps identified during implementation.

  1. Network I/O inside DB transaction. ItemValidator.validateImageUrl() performs CDN validation and S3 HEAD inside the DBIO returned by the ScopingValidator pattern, which executes within a DB transaction. The HEAD call is fast (sub-100ms) but this is architecturally impure. Suggestion: Investigate a pre-validation hook that runs before the transaction starts, or a two-phase commit pattern where validation happens outside the transaction and the result is passed in.

  2. CdnUrlResolver does not validate UUID in filename segment. The validator checks host, tenant prefix, segment count, and file extension presence — but does not verify the filename part is a valid UUID. Defense in depth only (keys are server-generated and policy-signed). Suggestion: Add UUID validation in the filename segment for stricter URL verification. Low priority.

  3. URL type in domain model. Item.Entity.imageUrl uses java.net.URL which has DNS-resolving equals()/hashCode(). Per coding standards, URI is preferred. The field should be migrated to URI in a future refactoring. Wire format is unchanged (JSON string).

  4. Endpoint path deviation from TD-13. The spec called for POST /v1/item/<itemEId>/image-upload-url (entity-scoped). The implementation is POST /v1/item/image-upload/request-upload-credentials (module-level). The presigned POST is not scoped to a specific item EId since the upload happens before entity persistence. The phasing document and spec should be updated to reflect the actual path.

  1. Agent team-lead reliability. The coord-run1-cm team-lead agent went idle without acting (3 attempts). Direct coordination (spawning agents from the parent session) was more reliable. Suggestion: For sequential single-agent work, skip the team-lead and coordinate directly.

  2. Agent test fixture updates are error-prone. Changing a constructor signature (e.g., ItemValidator object → class, CsvS3BucketDirectAccess factory) required updating 26+ test files. Agents using relaxed = true mocks caused ClassCastException at runtime. Suggestion: Create shared test fixture factories (e.g., TestFixtures.itemValidator(), TestFixtures.s3AssetService()) that centralize mock construction. A single change point instead of 26 files.

  3. includeBuild management. Commenting/uncommenting includeBuild in settings.gradle.kts was a recurring manual step that risked accidental commits. Suggestion: Use the conditional pattern from the release lifecycle guide: if (file("../common-module").exists()) includeBuild(...).

  1. Kotlin coding standards now cover !!, single exit, DI, and flatInApplicationContext — added during this project. These should be reviewed and merged to main via the documentation PR.

  2. Release lifecycle now covers CHANGELOG category selection — added during this project. Same documentation PR.

  3. common-module knowledge-base has s3-asset-service-patterns.md with 9 patterns. This should be maintained as new patterns emerge.

  4. operations knowledge-base is empty. Patterns from this project (validator DI, facade injection, shared constants) should be documented there.


Copyright: (c) Arda Systems 2025-2026, All rights reserved