Suggestions: Backend Services for Item Image Upload
Improvement suggestions, technical debt, and documentation gaps identified during implementation.
Technical Debt
Section titled “Technical Debt”-
Network I/O inside DB transaction.
ItemValidator.validateImageUrl()performs CDN validation and S3 HEAD inside theDBIOreturned by theScopingValidatorpattern, 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. -
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.
-
URLtype in domain model.Item.Entity.imageUrlusesjava.net.URLwhich has DNS-resolvingequals()/hashCode(). Per coding standards,URIis preferred. The field should be migrated toURIin a future refactoring. Wire format is unchanged (JSON string). -
Endpoint path deviation from TD-13. The spec called for
POST /v1/item/<itemEId>/image-upload-url(entity-scoped). The implementation isPOST /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.
Process Improvements
Section titled “Process Improvements”-
Agent team-lead reliability. The
coord-run1-cmteam-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. -
Agent test fixture updates are error-prone. Changing a constructor signature (e.g.,
ItemValidatorobject → class,CsvS3BucketDirectAccessfactory) required updating 26+ test files. Agents usingrelaxed = truemocks causedClassCastExceptionat 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. -
includeBuildmanagement. Commenting/uncommentingincludeBuildinsettings.gradle.ktswas a recurring manual step that risked accidental commits. Suggestion: Use the conditional pattern from the release lifecycle guide:if (file("../common-module").exists()) includeBuild(...).
Documentation Gaps
Section titled “Documentation Gaps”-
Kotlin coding standards now cover
!!, single exit, DI, andflatInApplicationContext— added during this project. These should be reviewed and merged tomainvia the documentation PR. -
Release lifecycle now covers CHANGELOG category selection — added during this project. Same documentation PR.
-
common-module knowledge-base has
s3-asset-service-patterns.mdwith 9 patterns. This should be maintained as new patterns emerge. -
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
Copyright: © Arda Systems 2025-2026, All rights reserved