diff --git a/.agents/skills/lint/SKILL.md b/.agents/skills/lint/SKILL.md index 2e0132b15..f99704585 100644 --- a/.agents/skills/lint/SKILL.md +++ b/.agents/skills/lint/SKILL.md @@ -22,7 +22,7 @@ Use this skill when: ## Execution Steps 1. **`make format`** — auto-formats all Kotlin files with ktlint. Fixes the vast majority of Kotlin style issues. -2. **`./mvnw checkstyle:check`** — reports Checkstyle violations for Java and XML files. Fix violations manually. +2. **`make lint`** — reports ktlint and Checkstyle violations. Fix Checkstyle (Java/XML) violations manually. 3. **`make sort`** — run this if any `pom.xml` was added or modified during this task. 4. Fix any remaining violations manually (see resources below for style guides). 5. **`make install`** — confirm all linting CI gates pass before finishing. @@ -37,7 +37,7 @@ Use this skill when: - Missing trailing newline - Line too long (max 120 characters) -To check without modifying files: `./mvnw ktlint:check` +To check without modifying files: `make lint` ### Checkstyle (Java / XML) @@ -50,7 +50,7 @@ Fix these manually: ## Completion Criteria - [ ] `make format` run (Kotlin auto-fixed) -- [ ] `./mvnw checkstyle:check` passes with no violations +- [ ] `make lint` passes with no violations - [ ] `make sort` run if any `pom.xml` was modified - [ ] `make install` passes all CI gates diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index e797b9ef2..aa589c4fd 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -18,7 +18,7 @@ All third-party contributions to this project must be accompanied by a signed co **Prerequisites:** - Java 25 (compile/run; JVM target 17 with Kotlin language/api 2.2, per Spring Boot 4.x guidance) -- Maven 3.9+ (use the included `./mvnw` wrapper) +- Maven 3.9+ (use make targets) - Docker (for Docker build and integration tests) **Build and verify:** diff --git a/.github/workflows/maven-ci-and-prb.yml b/.github/workflows/maven-ci-and-prb.yml index 17ee08b7f..974f411c9 100644 --- a/.github/workflows/maven-ci-and-prb.yml +++ b/.github/workflows/maven-ci-and-prb.yml @@ -1,5 +1,5 @@ # -# Copyright 2017-2020 Adobe. +# Copyright 2017-2026 Adobe. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -52,4 +52,4 @@ jobs: distribution: 'oracle' cache: 'maven' - name: Build with Maven - run: ./mvnw -B -V -Dstyle.color=always clean verify + run: make verify diff --git a/.github/workflows/maven-release.yml b/.github/workflows/maven-release.yml index 42ecac821..6ceeedb4d 100644 --- a/.github/workflows/maven-release.yml +++ b/.github/workflows/maven-release.yml @@ -87,4 +87,4 @@ jobs: ADOBE_BOT_GITHUB_PASSWORD: ${{ secrets.ADOBE_BOT_GITHUB_PASSWORD }} SONATYPE_USERNAME: ${{ secrets.S3MOCK_AFRANKEN_SONATYPE_USERNAME }} SONATYPE_PASSWORD: ${{ secrets.S3MOCK_AFRANKEN_SONATYPE_PASSWORD }} - run: ./mvnw -B -V release:prepare release:perform + run: make release diff --git a/AGENTS.md b/AGENTS.md index c581aeacc..71127187b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,59 +65,7 @@ See `dto/ListBucketResult.kt` for a representative example. XML names must match ## Storage -Filesystem layout: -``` -//bucketMetadata.json -///binaryData -///objectMetadata.json -///-binaryData # versioning -///-objectMetadata.json # versioning -//multiparts//multipartMetadata.json -//multiparts//.part -``` - -**`bucketMetadata.json`** fields (`BucketMetadata`): - -| Field | Type | Notes | -|---|---|---| -| `name` | `String` | Bucket name | -| `creationDate` | `String` | ISO-8601 timestamp | -| `bucketRegion` | `String` | AWS region string | -| `objects` | `Map` | 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?` | `x-amz-meta-*` headers | -| `storeHeaders` | `Map?` | headers persisted verbatim (e.g. `Content-Encoding`) | -| `encryptionHeaders` | `Map?` | SSE headers | -| `tags` | `List?` | 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 | +Filesystem layout and metadata schemas (`BucketMetadata`, `S3ObjectMetadata` fields): see **[server/AGENTS.md](server/AGENTS.md) § Storage Schema**. ## Configuration @@ -161,7 +109,11 @@ make install # Full build make skip-docker # Skip Docker make test # Unit tests only make integration-tests # Run integration tests -make format # Format Kotlin code (ktlint) +make integration-test-class CLASS=BucketIT # Run one IT class (or CLASS=BucketIT#methodName) +make format # Format Kotlin code (ktlint, auto-fix) +make lint # Check style without auto-fixing (ktlint + Checkstyle) +make typecheck # Compile all modules without running tests +make check # lint + typecheck + test combined make run # Run S3Mock from source (Spring Boot) make sort # Sort POM files (sortpom) ``` @@ -173,7 +125,7 @@ Use the **`lint` skill** to fix formatting and verify style gates (ktlint + Chec All PRs and pushes are validated by the `maven-ci-and-prb.yml` GitHub Actions workflow. **Required gates** (all must pass before merge): -1. Compilation and build (`./mvnw clean install`) +1. Compilation and build (`make verify`) 2. Unit tests (`*Test.kt` in each module) 3. Integration tests (`*IT.kt` against Docker container) 4. ktlint (Kotlin code style) diff --git a/INVARIANTS.md b/INVARIANTS.md index 1e79d3c0b..52526a532 100644 --- a/INVARIANTS.md +++ b/INVARIANTS.md @@ -79,4 +79,4 @@ 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) +- `make lint` passes (ktlint + Checkstyle) diff --git a/Makefile b/Makefile index ec67ddb09..8ac9b7b51 100644 --- 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 test-class sort help +.PHONY: build verify install skip-docker format fmt lint typecheck check integration-tests integration-test-class run test test-class sort release help .DEFAULT_GOAL := build build: verify @@ -58,6 +58,11 @@ test-class: integration-tests: ./mvnw -B -V -Dstyle.color=always verify -pl integration-tests +# Run a single integration test class or method: make integration-test-class CLASS=BucketIT +# To run a specific method: make integration-test-class CLASS=BucketIT#shouldCreateBucket +integration-test-class: + ./mvnw -B -V -Dstyle.color=always verify -pl integration-tests -Dit.test=$(CLASS) + # Master validation target: lint + typecheck + unit tests. check: lint typecheck test @@ -67,6 +72,9 @@ run: sort: ./mvnw -B -V -Dstyle.color=always com.github.ekryd.sortpom:sortpom-maven-plugin:sort +release: + ./mvnw -B -V release:prepare release:perform + help: @echo "" @echo "Usage: make " @@ -88,9 +96,11 @@ help: @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 " integration-test-class Run one integration test: make integration-test-class CLASS=BucketIT" @echo "" @echo "Development" @echo " run Run S3Mock from source via Spring Boot" @echo " sort Sort POM files with sortpom" + @echo " release Prepare and perform a Maven release (CI use)" @echo " help Show this message" @echo "" diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 98ba90b92..10cd33651 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -44,10 +44,17 @@ graph TD TC -->|path-style HTTP/HTTPS| CONN JU5 -->|in-process| CONN TNG -->|in-process| CONN - CONN --> KF --> Controller - Controller --> Service - Service --> Store - Store --> FS + CONN --> KF + KF --> BC & OC & MC & FC + KF --> KMS + BC --> BS + OC --> OS + MC --> MS + OS --> BKS + BS --> BKS + OS --> OBS + MS --> MPS + BKS & OBS & MPS --> FS ``` ## Data Flow diff --git a/docs/SETUP.md b/docs/SETUP.md index 599dc019a..47bafdf70 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -4,11 +4,11 @@ How to build, configure, and run S3Mock locally. ## Prerequisites -| Tool | Minimum version | Notes | -|---|---|---| +| Tool | Minimum version | Notes | +|---|---|------------------------------------------------| | JDK | 25 | Build toolchain only — bytecode targets JDK 17 | -| Maven | 3.9+ | Or use the included `./mvnw` wrapper | -| Docker | Any recent version | Required for integration tests and `make run` | +| Maven | 3.9+ | Use make targets | +| Docker | Any recent version | Required for integration tests and `make run` | Verify: ```bash @@ -78,8 +78,8 @@ make check # lint + typecheck + unit tests For a specific integration test: ```bash -./mvnw verify -pl integration-tests -Dit.test=BucketIT -./mvnw verify -pl integration-tests -Dit.test=BucketIT#shouldCreateBucket +make integration-test-class CLASS=BucketIT +make integration-test-class CLASS=BucketIT#shouldCreateBucket ``` ## Running Validation diff --git a/docs/TESTING.md b/docs/TESTING.md index 9754cd497..a2d5ac3cc 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -106,9 +106,8 @@ See **[docs/KOTLIN.md](KOTLIN.md)** for Kotlin naming conventions (backtick test ```bash make test # Unit tests only make integration-tests # All integration tests -./mvnw verify -pl integration-tests -Dit.test=BucketIT # Specific class -./mvnw verify -pl integration-tests -Dit.test=BucketIT#shouldCreateBucket # Specific method -./mvnw test -pl server -DskipDocker # Skip Docker +make integration-test-class CLASS=BucketIT # Specific class +make integration-test-class CLASS=BucketIT#shouldCreateBucket # Specific method ``` > Integration tests require Docker to be running. @@ -118,7 +117,7 @@ make integration-tests # A - **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 +- **Compilation error**: Run `make typecheck` first ## Checklist diff --git a/pom.xml b/pom.xml index ed61f9d69..70db7f4e4 100644 --- a/pom.xml +++ b/pom.xml @@ -64,9 +64,10 @@ https://github.com/adobe/S3Mock/tree/main - + 1.4.2 + 1.6.76 2.44.8 @@ -228,6 +229,12 @@ xmlunit-assertj3 ${xmlunit-assertj3.version} + + com.tngtech.archunit + archunit-junit5 + ${archunit.version} + test + digital.pragmatech.testing spring-test-profiler @@ -238,7 +245,6 @@ - src/main/kotlin @@ -431,8 +437,8 @@ ${java.version} - - + + @@ -662,6 +668,7 @@ central-publishing-maven-plugin + src/main/kotlin diff --git a/server/AGENTS.md b/server/AGENTS.md index 1a2091c7f..df0013db7 100644 --- a/server/AGENTS.md +++ b/server/AGENTS.md @@ -67,6 +67,62 @@ synchronized(lockStore[id]!!) { - 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. +## Storage Schema + +Filesystem layout: +``` +//bucketMetadata.json +///binaryData +///objectMetadata.json +///-binaryData # versioning +///-objectMetadata.json # versioning +//multiparts//multipartMetadata.json +//multiparts//.part +``` + +**`bucketMetadata.json`** fields (`BucketMetadata`): + +| Field | Type | Notes | +|---|---|---| +| `name` | `String` | Bucket name | +| `creationDate` | `String` | ISO-8601 timestamp | +| `bucketRegion` | `String` | AWS region string | +| `objects` | `Map` | 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?` | `x-amz-meta-*` headers | +| `storeHeaders` | `Map?` | headers persisted verbatim (e.g. `Content-Encoding`) | +| `encryptionHeaders` | `Map?` | SSE headers | +| `tags` | `List?` | 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 | + ## Testing See **[docs/TESTING.md](../docs/TESTING.md)** for the full strategy. Service and store tests use `@SpringBootTest` with `@MockitoBean`; controller tests use `@WebMvcTest` with `@MockitoBean` and `BaseControllerTest`. Always extend the appropriate base class (`ServiceTestBase`, `StoreTestBase`, `BaseControllerTest`). diff --git a/server/pom.xml b/server/pom.xml index e20da245d..48ea18347 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -101,14 +101,18 @@ jackson-module-kotlin - digital.pragmatech.testing - spring-test-profiler + com.code-intelligence + jazzer-junit test - - com.code-intelligence - jazzer-junit + com.tngtech.archunit + archunit-junit5 + test + + + digital.pragmatech.testing + spring-test-profiler test diff --git a/server/src/test/kotlin/com/adobe/testing/s3mock/ArchitectureTest.kt b/server/src/test/kotlin/com/adobe/testing/s3mock/ArchitectureTest.kt new file mode 100644 index 000000000..62411fb57 --- /dev/null +++ b/server/src/test/kotlin/com/adobe/testing/s3mock/ArchitectureTest.kt @@ -0,0 +1,79 @@ +/* + * Copyright 2017-2026 Adobe. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.adobe.testing.s3mock + +import com.tngtech.archunit.base.DescribedPredicate +import com.tngtech.archunit.core.domain.JavaClass +import com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAPackage +import com.tngtech.archunit.core.importer.ImportOption +import com.tngtech.archunit.junit.AnalyzeClasses +import com.tngtech.archunit.junit.ArchTest +import com.tngtech.archunit.lang.ArchRule +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noCodeUnits + +@AnalyzeClasses(packages = ["com.adobe.testing.s3mock"], importOptions = [ImportOption.DoNotIncludeTests::class]) +class ArchitectureTest { + // Data classes (BucketMetadata, S3ObjectMetadata, etc.) live in the store package and are + // legitimately used by controllers as service return types. The invariant is that controllers + // must not call *Store methods directly (bypassing the service layer). + private val storeOperationClasses: DescribedPredicate = + resideInAPackage("..store..").and( + DescribedPredicate.describe("have simple name ending with 'Store'") { cls -> + cls.simpleName.endsWith("Store") + }, + ) + + @ArchTest + val storesMayNotAccessUpperLayers: ArchRule = + noClasses() + .that() + .resideInAPackage("..store..") + .should() + .accessClassesThat() + .resideInAPackage("..controller..") + .orShould() + .accessClassesThat() + .resideInAPackage("..service..") + .because("stores are the bottom layer and must not depend on controllers or services") + + @ArchTest + val noLegacyJacksonXmlAnnotations: ArchRule = + noCodeUnits() + .should() + .beAnnotatedWith("com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement") + .orShould() + .beAnnotatedWith("com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty") + .orShould() + .beAnnotatedWith("com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper") + .because("use tools.jackson annotations, not the legacy com.fasterxml packages") + + @ArchTest + val controllersMayNotAccessStoreDirect: ArchRule = + noClasses() + .that() + .resideInAPackage("..controller..") + .and() + .doNotHaveSimpleName("KmsValidationFilter") + .and() + .doNotHaveSimpleName("ControllerConfiguration") + .should() + .accessClassesThat(storeOperationClasses) + .because( + "controllers must delegate to services, not call store methods directly; " + + "KmsValidationFilter and ControllerConfiguration are the intentional exceptions", + ) +}