Merge pull request #3052 from adobe/maintenance-for-5.1.0

afranken · web-flow · commit 630ea6eb5c0a · 2026-05-26T23:23:19.000+02:00
Maintenance for 5.1.0
diff --git a/.claude/commands/document.md b/.claude/commands/document.md
@@ -0,0 +1 @@
+@.agents/skills/document/SKILL.md
diff --git a/.claude/commands/implement.md b/.claude/commands/implement.md
@@ -0,0 +1 @@
+@.agents/skills/implement/SKILL.md
diff --git a/.claude/commands/lint.md b/.claude/commands/lint.md
@@ -0,0 +1 @@
+@.agents/skills/lint/SKILL.md
diff --git a/.claude/commands/refactor.md b/.claude/commands/refactor.md
@@ -0,0 +1 @@
+@.agents/skills/refactor/SKILL.md
diff --git a/.claude/commands/review.md b/.claude/commands/review.md
@@ -0,0 +1 @@
+@.agents/skills/review/SKILL.md
diff --git a/.claude/commands/test.md b/.claude/commands/test.md
@@ -0,0 +1 @@
+@.agents/skills/test/SKILL.md
diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md
@@ -31,13 +31,7 @@ make format            # Format Kotlin code
 
 ## Adding a New S3 Operation
 
-1. Add DTO(s) in `server/dto/` — XML names must match the [AWS S3 API spec](https://docs.aws.amazon.com/AmazonS3/latest/API/Welcome.html) exactly
-2. Add store method(s) in `server/store/` — acquire the per-object or per-bucket lock
-3. Add service method(s) in `server/service/` — business logic only, throw `S3Exception` constants
-4. Add controller method(s) in `server/controller/` — HTTP mapping only, no logic, no exception catching
-5. Add integration test(s) in `integration-tests/` — use real AWS SDK v2 against Docker container
-6. Run `make integration-tests` to verify XML serialization
-7. Update `CHANGELOG.md` and `AGENTS.md` configuration tables if new properties are added
+See the full implementation checklist in **[server/AGENTS.md § Implementation Flow](../server/AGENTS.md)** — it is the authoritative step-by-step guide (DTO → Store → Service → Controller → IT → docs).
 
 ## Code Review Focus
 
@@ -110,7 +104,7 @@ Before flagging a blocker, verify:
 - [ ] `make test` passes (unit tests)
 - [ ] The change you're making is within the scope requested
 
-If an integration test fails and you cannot determine why without running the full Docker stack, say so explicitly — do not claim success based on compilation alone.
+If `make integration-tests` fails, first verify Docker is available: run `docker info`. If that command fails, Docker is not running — escalate to the human rather than debugging the test failure. Do not claim success based on compilation alone.
 
 ### What not to do without approval
 
diff --git a/AGENTS.md b/AGENTS.md
@@ -76,6 +76,49 @@ Filesystem layout:
 <root>/<bucket>/multiparts/<upload-id>/<part>.part
 ```
 
+**`bucketMetadata.json`** fields (`BucketMetadata`):
+
+| Field | Type | Notes |
+|---|---|---|
+| `name` | `String` | Bucket name |
+| `creationDate` | `String` | ISO-8601 timestamp |
+| `bucketRegion` | `String` | AWS region string |
+| `objects` | `Map<String, UUID>` | key → object UUID mapping |
+| `versioningConfiguration` | `VersioningConfiguration?` | null until versioning is configured |
+| `objectLockConfiguration` | `ObjectLockConfiguration?` | null until object lock is enabled |
+| `bucketLifecycleConfiguration` | `BucketLifecycleConfiguration?` | null until lifecycle rules are set |
+| `objectOwnership` | `ObjectOwnership?` | null until ownership is set |
+| `bucketInfo` | `BucketInfo?` | bucket type/data-redundancy info |
+| `locationInfo` | `LocationInfo?` | bucket location info |
+| `path` | `Path` | filesystem path to the bucket folder (not serialized for cross-host use) |
+
+**`objectMetadata.json`** fields (`S3ObjectMetadata`):
+
+| Field | Type | Notes |
+|---|---|---|
+| `id` | `UUID` | object identity (matches the folder name) |
+| `key` | `String` | S3 object key |
+| `size` | `String` | content length as string |
+| `contentType` | `String?` | MIME type |
+| `etag` | `String?` | ETag value |
+| `modificationDate` | `String` | formatted date string |
+| `lastModified` | `Long` | epoch millis |
+| `dataPath` | `Path` | path to the `binaryData` file |
+| `userMetadata` | `Map<String, String>?` | `x-amz-meta-*` headers |
+| `storeHeaders` | `Map<String, String>?` | headers persisted verbatim (e.g. `Content-Encoding`) |
+| `encryptionHeaders` | `Map<String, String>?` | SSE headers |
+| `tags` | `List<Tag>?` | object tags |
+| `checksumAlgorithm` | `ChecksumAlgorithm?` | CRC32 / SHA-256 / etc. |
+| `checksum` | `String?` | computed checksum value |
+| `checksumType` | `ChecksumType?` | FULL\_OBJECT or COMPOSITE |
+| `storageClass` | `StorageClass?` | STANDARD, GLACIER, etc. |
+| `owner` | `Owner` | object owner |
+| `legalHold` | `LegalHold?` | WORM legal hold status |
+| `retention` | `Retention?` | WORM retention mode + until-date |
+| `policy` | `AccessControlPolicy?` | ACL policy |
+| `versionId` | `String?` | non-null when versioning is enabled |
+| `deleteMarker` | `Boolean` | true for versioned delete markers |
+
 ## Configuration
 
 Environment variables:
@@ -111,6 +154,8 @@ See **[docs/TESTING.md](docs/TESTING.md)** for the full testing strategy, base c
 
 ## Build
 
+Always use `make` targets. Never invoke `./mvnw` directly.
+
 ```bash
 make install              # Full build
 make skip-docker          # Skip Docker
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -154,11 +154,19 @@ Version 5.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Jav
   * TBD
 * Version updates (deliverable dependencies)
   * Bump alpine from 3.23.3 to 3.23.4 in /docker
+  * Bump spring-boot.version from 4.0.5 to 4.0.6
+  * Bump software.amazon.awssdk:bom from 2.42.29 to 2.43.0
+  * Bump aws.sdk.kotlin:s3-jvm from 1.6.52 to 1.6.65
+  * Bump org.jetbrains.kotlin:kotlin-bom from 2.3.10 to 2.3.21
+  * Bump org.jetbrains.kotlinx:kotlinx-coroutines-bom from 1.10.2 to 1.11.0
+  * Bump org.testcontainers:testcontainers-bom from 2.0.2 to 2.0.5
 * Version updates (build dependencies)
-  * Bump com.github.gantsign.maven:ktlint-maven-plugin from 3.7.0 to 3.7.1.
+  * Bump com.github.gantsign.maven:ktlint-maven-plugin from 3.7.0 to 3.7.1
+  * Bump com.puppycrawl.tools:checkstyle from 13.4.0 to 13.4.2
   * Bump actions/upload-artifact from 7.0.0 to 7.0.1
-  * Bump github/codeql-action from 4.35.1 to 4.35.2
-  * Bump step-security/harden-runner from 2.16.1 to 2.18.0
+  * Bump actions/dependency-review-action from 4.9.0 to 5.0.0
+  * Bump github/codeql-action from 4.35.1 to 4.35.4
+  * Bump step-security/harden-runner from 2.16.1 to 2.19.1
 
 ## 5.0.0
 
diff --git a/INVARIANTS.md b/INVARIANTS.md
@@ -65,3 +65,18 @@ No performance SLAs are defined for S3Mock. It is a local testing tool, not a th
 ✅ Never use JUnit 4 — it was removed in 5.x — *Enforced by: human review*
 
 ✅ Never mock AWS SDK clients in integration tests (`*IT.kt`) — use actual SDK v2 clients against a live S3Mock Docker container — *Enforced by: human review*
+
+✅ Every code change must include unit tests (`*Test.kt` in `server/`) covering the new or modified logic — *Enforced by: human review*
+
+✅ Every behavior observable via the S3 HTTP API must be covered by an integration test (`*IT.kt` in `integration-tests/`) — *Enforced by: human review*
+
+---
+
+## Definition of Done
+
+A task is not complete until all of the following are true:
+
+- Unit tests cover the new or modified logic (`*Test.kt` in the module the change was made)
+- Integration tests cover the observable HTTP/S3 behavior (`*IT.kt` in `integration-tests/`)
+- `CHANGELOG.md` has an entry under the current version for any user-facing bug fix or feature
+- `make format` passes (ktlint + Checkstyle)
diff --git a/Makefile b/Makefile
@@ -16,7 +16,7 @@
 
 # Agents: run only make targets listed here. No direct shell commands.
 
-.PHONY: build verify install skip-docker format fmt lint typecheck check integration-tests run test sort help
+.PHONY: build verify install skip-docker format fmt lint typecheck check integration-tests run test test-class sort help
 .DEFAULT_GOAL := build
 
 build: verify
@@ -51,6 +51,10 @@ typecheck:
 test:
 	./mvnw -B -V -Dstyle.color=always test -pl server
 
+# Run a single test class: make test-class CLASS=BucketServiceTest
+test-class:
+	./mvnw -B -V -Dstyle.color=always test -pl server -Dtest=$(CLASS)
+
 integration-tests:
 	./mvnw -B -V -Dstyle.color=always verify -pl integration-tests
 
@@ -82,6 +86,7 @@ help:
 	@echo ""
 	@echo "Testing"
 	@echo "  test               Unit tests only (server/ module)"
+	@echo "  test-class         Run one test class: make test-class CLASS=BucketServiceTest"
 	@echo "  integration-tests  Integration tests against a live Docker container"
 	@echo ""
 	@echo "Development"
diff --git a/docs/TESTING.md b/docs/TESTING.md
@@ -115,7 +115,7 @@ make integration-tests                                                       # A
 
 ## Troubleshooting
 
-- **Docker not running**: Integration tests will fail — start Docker first
+- **Docker not running**: Run `docker info` — if it fails, Docker is not running; escalate to the human rather than debugging the test failure
 - **Port conflict**: Check `lsof -i :9090`
 - **Flaky test**: Look for shared state or ordering dependencies
 - **Compilation error**: Run `./mvnw clean install -DskipDocker -DskipTests` first
diff --git a/server/AGENTS.md b/server/AGENTS.md
@@ -32,12 +32,40 @@ server/src/main/kotlin/com/adobe/testing/s3mock/
 
 ## Implementation Flow
 
-**Adding S3 operation**: Follow **DTO → Store → Service → Controller**:
+**Adding S3 operation**: Follow **DTO → Store → Service → Controller → IT**:
 
 1. **DTO** (`dto/`): Data classes with Jackson annotations — see root `AGENTS.md` § XML Serialization for the correct `tools.jackson` annotations and namespace. Verify element names against [AWS S3 API docs](https://docs.aws.amazon.com/AmazonS3/latest/API/Welcome.html).
-2. **Store** (`store/`): Filesystem path resolution, binary storage, metadata JSON. Key classes: `BucketStore`, `ObjectStore`, `BucketMetadata`, `S3ObjectMetadata`.
+2. **Store** (`store/`): Filesystem path resolution, binary storage, metadata JSON. Key classes: `BucketStore`, `ObjectStore`, `BucketMetadata`, `S3ObjectMetadata`. Acquire the appropriate lock (see Locking section below).
 3. **Service** (`service/`): Validation, store coordination. Throw **`S3Exception` constants** (e.g., `S3Exception.NO_SUCH_BUCKET`) — see **[docs/SPRING.md](../docs/SPRING.md)** for exception handling rules.
 4. **Controller** (`controller/`): HTTP mapping only — delegate all logic to services. Controllers never catch exceptions.
+5. **Integration test** (`integration-tests/`): Real AWS SDK v2 against the Docker container — see **[integration-tests/AGENTS.md](../integration-tests/AGENTS.md)**. Run `make integration-tests` to verify XML serialization against the AWS S3 API.
+6. **Update docs**: `CHANGELOG.md` (user-facing entry) and root `AGENTS.md` Configuration table if new properties are added.
+
+## Locking
+
+Each store uses a `ConcurrentHashMap` keyed by entity identity to hold one plain `Any()` lock object per entity. All reads **and** writes of metadata files must hold the corresponding lock.
+
+| Store | Lock key type | Lock map field | Where lock is registered |
+|---|---|---|---|
+| `BucketStore` | `String` (bucket name) | `lockStore` | `createBucket` / `loadBuckets` via `lockStore.putIfAbsent(bucketName, Any())` |
+| `ObjectStore` | `UUID` (object ID) | `lockStore` | Before first write via `lockStore.putIfAbsent(id, Any())` |
+| `MultipartStore` | `UUID` (upload ID) | `lockStore` | On upload creation via `lockStore.putIfAbsent(uploadId, Any())` |
+
+**Pattern for adding a store method that reads or writes metadata:**
+
+```kotlin
+// Register lock lazily (writes only — reads rely on the lock already existing)
+lockStore.putIfAbsent(id, Any())
+// Acquire lock
+synchronized(lockStore[id]!!) {
+    // read or write metadata here
+}
+```
+
+**Rules:**
+- Never skip the lock for reads — `getBucketMetadata` and `getS3ObjectMetadata` are also synchronized.
+- Never acquire more than one lock in a single call path — there is no established ordering, so taking two locks risks deadlock.
+- Do not introduce `ReentrantLock`, `ReadWriteLock`, or other lock types — the existing `synchronized`/`Any()` pattern is intentional and consistent throughout all stores.
 
 ## Testing
 
