Skip to content

Commit 1a44241

Browse files
authored
Merge pull request #2976 from adobe/copilot/extract-spring-guidance-doc
Remove duplicated Spring/Kotlin/Testing guidance from AGENTS.md files
2 parents 7783a3d + ee03595 commit 1a44241

File tree

4 files changed

+152
-27
lines changed

4 files changed

+152
-27
lines changed

AGENTS.md

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,37 +29,21 @@ docker/ # Docker image build
2929

3030
## DO / DON'T
3131

32+
> For Kotlin idioms and naming conventions, see **[docs/KOTLIN.md](docs/KOTLIN.md)**.
33+
> For Spring Boot patterns and testing setup, see **[docs/SPRING.md](docs/SPRING.md)**.
34+
> For testing conventions and commands, see **[docs/TESTING.md](docs/TESTING.md)**.
35+
3236
### DO
33-
- Use **constructor injection** for all Spring beans (in production code)
3437
- Use **data classes** for DTOs with Jackson XML annotations
35-
- Use **Kotlin stdlib** and built-in language features over third-party utilities
3638
- Use **AWS SDK v2** for all new integration tests
3739
- Use **JUnit 5** for all new tests
38-
- Use **`@SpringBootTest`** with **`@MockitoBean`** for unit tests — this is the project's standard mocking approach
39-
- Use **expression bodies** for simple functions
40-
- Use **null safety** (`?`, `?.`, `?:`) instead of null checks
41-
- **Name the `it` parameter** in nested lambdas, loops, and scope functions to avoid shadowing: `.map { part -> ... }` instead of `.map { it.name }`
42-
- Match **AWS S3 API naming exactly** in Jackson XML annotations (`localName = "..."`)
43-
- Keep tests **independent** — each test creates its own resources (UUID bucket names)
44-
- Use **backtick test names** with descriptive sentences: `` fun `should create bucket successfully`() ``
45-
- Mark test classes as **`internal`**: `internal class ObjectServiceTest`
46-
- **Refactor** legacy `testSomething` camelCase names to backtick style when touching existing tests
4740
- **Update the copyright year** in the file's license header to the current year whenever you modify an existing file
4841
- Validate XML serialization against [AWS S3 API documentation](https://docs.aws.amazon.com/AmazonS3/latest/API/Welcome.html)
4942

5043
### DON'T
51-
- DON'T use `@Autowired` or field injection in production code — always use constructor injection
52-
- DON'T use `var` for public API properties — prefer `val` (immutability)
5344
- DON'T use AWS SDK v1 — it has been removed in 5.x
5445
- DON'T use JUnit 4 — it has been removed in 5.x
55-
- DON'T use `@ExtendWith(MockitoExtension::class)` or `@Mock` / `@InjectMocks` — use `@SpringBootTest` with `@MockitoBean` instead
56-
- DON'T add Apache Commons dependencies — use Kotlin stdlib equivalents
57-
- DON'T put business logic in controllers — controllers only map HTTP, delegate to services
58-
- DON'T return raw strings from controllers — use typed DTOs for XML/JSON responses
5946
- DON'T declare dependency versions in sub-module POMs — all versions are managed in root `pom.xml`
60-
- DON'T share mutable state between tests — each test must be self-contained
61-
- DON'T hardcode bucket names in tests — use `UUID.randomUUID()` for uniqueness
62-
- DON'T use legacy `testSomething` camelCase naming for new tests — use backtick names instead
6347
- DON'T update copyright years in files you haven't modified — copyright is only bumped when a file is actually changed
6448

6549
## Code Style
@@ -68,7 +52,7 @@ See **[docs/KOTLIN.md](docs/KOTLIN.md)** for Kotlin idioms, naming conventions,
6852

6953
See **[docs/JAVA.md](docs/JAVA.md)** for Java idioms, naming conventions, common anti-patterns, and Javadoc guidelines.
7054

71-
**Spring**: `@RestController`, `@Service`, `@Component`, constructor injection over field injection
55+
See **[docs/SPRING.md](docs/SPRING.md)** for Spring Boot patterns, bean registration, dependency injection, controller guidelines, configuration properties, exception handling, and testing.
7256

7357
## XML Serialization
7458

@@ -105,7 +89,7 @@ Environment variables (prefix: `COM_ADOBE_TESTING_S3MOCK_STORE_`):
10589

10690
Services throw `S3Exception` constants (`NO_SUCH_BUCKET`, `NO_SUCH_KEY`, `INVALID_BUCKET_NAME`, etc.).
10791
Spring exception handlers convert them to XML `ErrorResponse` with the correct HTTP status.
108-
See `server/AGENTS.md` for details.
92+
See **[docs/SPRING.md](docs/SPRING.md)** for exception handling patterns and `server/AGENTS.md` for the concrete handler classes.
10993

11094
## Testing
11195

docs/SPRING.md

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
# Spring Guidelines — S3Mock
2+
3+
Canonical reference for Spring Boot idioms, patterns, and code quality standards used across this project.
4+
5+
## Bean Registration
6+
7+
Register beans explicitly in `@Configuration` classes using `@Bean` factory methods — not via component scanning or class-level `@Service`/`@Component` annotations.
8+
9+
```kotlin
10+
@Configuration
11+
class ServiceConfiguration {
12+
@Bean
13+
fun bucketService(bucketStore: BucketStore, objectStore: ObjectStore): BucketService =
14+
BucketService(bucketStore, objectStore)
15+
}
16+
```
17+
18+
Three configuration layers mirror the architecture:
19+
- `StoreConfiguration` — store beans
20+
- `ServiceConfiguration` — service beans (imported in `@SpringBootTest`)
21+
- `ControllerConfiguration` — controller, filter, and exception-handler beans
22+
23+
## Dependency Injection
24+
25+
Always use **constructor injection**. Never use `@Autowired` on fields or setters in production code.
26+
27+
```kotlin
28+
// DO — constructor injection
29+
class BucketService(
30+
private val bucketStore: BucketStore,
31+
private val objectStore: ObjectStore,
32+
)
33+
34+
// DON'T — field injection
35+
class BucketService {
36+
@Autowired private lateinit var bucketStore: BucketStore
37+
}
38+
```
39+
40+
## Controllers
41+
42+
- `@RestController` classes map HTTP only — never contain business logic
43+
- All logic is delegated to a `@Service`; controllers call the service and return the result
44+
- Return typed DTOs, never raw strings
45+
- Controllers never catch exceptions — exception handlers in `ControllerConfiguration` do that
46+
47+
```kotlin
48+
// DO
49+
@GetMapping("/{bucketName}")
50+
fun getBucket(@PathVariable bucketName: String): ResponseEntity<ListBucketResult> =
51+
ResponseEntity.ok(bucketService.listObjects(bucketName))
52+
53+
// DON'T
54+
@GetMapping("/{bucketName}")
55+
fun getBucket(@PathVariable bucketName: String): String {
56+
// business logic here ...
57+
return "<ListBucketResult>...</ListBucketResult>"
58+
}
59+
```
60+
61+
## Configuration Properties
62+
63+
Bind configuration via `@ConfigurationProperties` data classes — never inject individual values with `@Value` in production code.
64+
65+
```kotlin
66+
@JvmRecord
67+
@ConfigurationProperties("com.adobe.testing.s3mock.store")
68+
data class StoreProperties(
69+
@param:DefaultValue("false") val retainFilesOnExit: Boolean,
70+
@param:DefaultValue("us-east-1") val region: Region,
71+
)
72+
```
73+
74+
Enable each properties class with `@EnableConfigurationProperties` in the matching `@Configuration`:
75+
76+
```kotlin
77+
@Configuration
78+
@EnableConfigurationProperties(StoreProperties::class)
79+
class StoreConfiguration { ... }
80+
```
81+
82+
## Exception Handling
83+
84+
- Services throw `S3Exception` constants (e.g., `S3Exception.NO_SUCH_BUCKET`) — never create new exception classes
85+
- Exception handlers are `@ControllerAdvice` classes registered as `@Bean`s in `ControllerConfiguration`
86+
- `S3MockExceptionHandler` converts `S3Exception` → XML `ErrorResponse` with the correct HTTP status
87+
- `IllegalStateExceptionHandler` converts unexpected errors → `500 InternalError`
88+
89+
```kotlin
90+
@ControllerAdvice
91+
class S3MockExceptionHandler : ResponseEntityExceptionHandler() {
92+
@ExceptionHandler(S3Exception::class)
93+
fun handleS3Exception(s3Exception: S3Exception): ResponseEntity<ErrorResponse> { ... }
94+
}
95+
```
96+
97+
## Testing
98+
99+
### Service and Store Tests — `@SpringBootTest`
100+
101+
Use `@SpringBootTest` scoped to the relevant `@Configuration` class with `@MockitoBean` for dependencies.
102+
103+
```kotlin
104+
@SpringBootTest(classes = [ServiceConfiguration::class], webEnvironment = SpringBootTest.WebEnvironment.NONE)
105+
@MockitoBean(types = [MultipartService::class, MultipartStore::class])
106+
internal class BucketServiceTest : ServiceTestBase() {
107+
@Autowired private lateinit var iut: BucketService
108+
109+
@Test
110+
fun `should return no such bucket`() { ... }
111+
}
112+
```
113+
114+
Always extend the appropriate base class:
115+
- `ServiceTestBase` — service layer tests
116+
- `StoreTestBase` — store layer tests
117+
118+
### Controller Tests — `@WebMvcTest`
119+
120+
Use `@WebMvcTest` scoped to the controller under test with `@MockitoBean` for services and `BaseControllerTest` for shared fixtures.
121+
122+
```kotlin
123+
@WebMvcTest(controllers = [BucketController::class], ...
124+
@MockitoBean(types = [BucketService::class])
125+
internal class BucketControllerTest : BaseControllerTest() {
126+
@Autowired private lateinit var mockMvc: MockMvc
127+
@Autowired private lateinit var bucketService: BucketService
128+
129+
@Test
130+
fun `should list buckets`() { ... }
131+
}
132+
```
133+
134+
### Common Anti-Patterns
135+
136+
| Anti-Pattern | Refactor To |
137+
|---|---|
138+
| `@ExtendWith(MockitoExtension::class)` + `@Mock` + `@InjectMocks` | `@SpringBootTest` + `@MockitoBean` + `@Autowired` |
139+
| `@Autowired` field injection in production code | Constructor injection |
140+
| Business logic in controller method | Delegate to a service class |
141+
| Returning a raw `String` from a controller | Return a typed DTO wrapped in `ResponseEntity` |
142+
| `@Value("${property}")` scattered throughout beans | `@ConfigurationProperties` data class |
143+
| New exception class for S3 errors | `S3Exception` constant |

docs/TESTING.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ Access `serviceEndpoint`, `serviceEndpointHttp`, and `serviceEndpointHttps` from
6767

6868
See **[docs/KOTLIN.md](KOTLIN.md)** for Kotlin naming conventions (backtick test names, `internal` visibility, naming patterns).
6969

70-
- **Naming**: Backtick names with descriptive sentences — `` fun `should create bucket successfully`() ``
71-
- **Visibility**: Mark test classes `internal`
7270
- **Pattern**: Arrange-Act-Assert
7371
- **Assertions**: AssertJ (`assertThat(...)`) — use specific matchers, not just `isNotNull()`
7472
- **Error cases**: Use AssertJ, not JUnit `assertThrows`:
@@ -78,7 +76,6 @@ See **[docs/KOTLIN.md](KOTLIN.md)** for Kotlin naming conventions (backtick test
7876
.hasMessageContaining("Status Code: 409")
7977
```
8078
- **Independence**: Each test creates its own resources — no shared state, UUID-based bucket names
81-
- **Legacy names**: Refactor `testSomething` camelCase names to backtick style when touching existing tests
8279

8380
## Running Tests
8481

server/AGENTS.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,15 @@ server/src/main/kotlin/com/adobe/testing/s3mock/
3636

3737
1. **DTO** (`dto/`): Data classes with Jackson XML annotations (`@JacksonXmlRootElement`, `@JacksonXmlProperty`, `@JacksonXmlElementWrapper(useWrapping = false)`). Verify element names against [AWS S3 API docs](https://docs.aws.amazon.com/AmazonS3/latest/API/Welcome.html).
3838
2. **Store** (`store/`): Filesystem path resolution, binary storage, metadata JSON. Key classes: `BucketStore`, `ObjectStore`, `BucketMetadata`, `S3ObjectMetadata`.
39-
3. **Service** (`service/`): Validation, store coordination. Throw **`S3Exception` constants** (e.g., `S3Exception.NO_SUCH_BUCKET`) — don't create new exception classes.
39+
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.
4040
4. **Controller** (`controller/`): HTTP mapping only — delegate all logic to services. Controllers never catch exceptions.
4141

4242
## Error Handling
4343

4444
- `S3MockExceptionHandler` converts `S3Exception` → XML `ErrorResponse` with the correct HTTP status
4545
- `IllegalStateExceptionHandler` converts unexpected errors → `500 InternalError`
46-
- Add new error types as constants in `S3Exception` — DON'T create new exception classes
46+
47+
See **[docs/SPRING.md](../docs/SPRING.md)** for exception handling patterns and rules.
4748

4849
## Testing
4950

0 commit comments

Comments
 (0)