Files
clean-architecture-backend-…/docs/superpowers/specs/2026-08-02-warning-zero-build-design.md
T
DongHyeonkaandClaude Opus 5 5f10b791d3 chore: record pre-existing uncommitted repository state
Snapshot of the in-flight state that already existed, identically, in both
this worktree and the main checkout before this session began: the initial
HTTP Client platform implementation (previously untracked), the redis-lab
removal, and the JPA / object-storage / notification integration work.

Kept separate from this session's HTTP Client review response, which lands
in the following commit, so the two bodies of work stay reviewable apart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-11 16:48:43 +09:00

449 lines
22 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Warning-Zero Build Refactoring Design
**Date:** 2026-08-02
**Status:** Approved design, pending written-spec review
**Scope:** Java compilation, Error Prone, Checkstyle, SpotBugs, test JVM diagnostics, expected-negative
shell-contract output, and intentional legacy/architecture-test compatibility seams.
## Goal
Make the standard repository build both functionally green and warning-clean. A successful build
must no longer conceal compiler warnings, test-source SpotBugs findings, ignored Checkstyle
findings, deprecated third-party API calls, or expected-negative subprocess diagnostics that look
like real failures.
The final local proof is a fresh `./gradlew clean build --warning-mode=all --no-daemon
--console=plain` with:
- exit code zero;
- zero compiler/Error Prone warnings;
- zero Checkstyle and SpotBugs findings in every executed source set;
- zero `SpotBugs ended with exit code 1` messages;
- zero OpenJDK CDS warnings from test JVMs;
- no successful Redis lab contract printing its expected-negative child diagnostics;
- only the five currently intentional optional-adapter/TestKit skips, with no qualification lane
silently skipped.
## Baseline Evidence
The fresh pre-change command completed successfully in 20 minutes 26 seconds with 283 of 283 tasks
executed. Success did not mean warning-clean:
- 123 compiler warning diagnostics across 19 warning rules (122 distinct file-line/rule
coordinates because one line emits two separate removal diagnostics);
- one test-source SpotBugs `DMI_RANDOM_USED_ONLY_ONCE` finding;
- ten OpenJDK CDS warning lines from Mockito-using test JVMs;
- 82 `redis-lab:` expected-negative stderr lines;
- five intentional skipped tests;
- no test failure, compiler error, Checkstyle finding, SpotBugs analysis error, or missing analysis
class.
The Gradle Problems report is an informational index over compiler diagnostics, not a separate
defect. It must become empty as a consequence of removing the underlying warnings; it must not be
hidden.
### Warning inventory traceability
| Rule | Diagnostic instances | Required resolution |
| --- | ---: | --- |
| `removal` | 46 | Exact legacy lifecycle/suppression policy in section 4 |
| `MissingOverride` | 16 | Add annotations to the implementing test fakes in section 2 |
| `StringCaseLocaleUsage` | 10 | `Locale.ROOT` behavior fixes and test cleanup in sections 12 |
| `SameNameButDifferent` | 9 | Qualify the two Redis nested enum types in section 2 |
| `DefaultCharset` | 9 | Explicit UTF-8 test data in sections 12 |
| `ArrayRecordComponent` | 7 | Exact record policies and copy regressions in section 2 |
| `CanonicalDuration` | 5 | `Duration.ofDays(3)` in section 2 |
| `StringSplitter` | 4 | ETag scanner plus three grammar-specific test fixes in sections 12 |
| `EmptyCatch` | 4 | Cleanup failure propagation in section 1 |
| `StringConcatToTextBlock` | 2 | Byte-identical text blocks in section 2 |
| `InvalidBlockTag` | 2 | Inline-code annotation names in section 2 |
| `BigDecimalLiteralDouble` | 2 | Method-only intentional-fixture suppressions in section 5 |
| `TypeParameterUnusedInFormals` | 1 | Spring Session method-only suppression in section 2 |
| `ThreadLocalUsage` | 1 | Instance-isolation regression and field-only suppression in section 2 |
| `ReferenceEquality` | 1 | Redis catalog identity regression and constructor-only suppression in section 2 |
| `MissingSummary` | 1 | Public Javadoc summary in section 2 |
| `JavaTimeDefaultTimeZone` | 1 | Fixed date/explicit zone in section 1 |
| `FutureReturnValueIgnored` | 1 | Observe the future in section 1 |
| `BooleanLiteral` | 1 | Literal assertion cleanup in section 2 |
This table accounts for all 123 Error Prone/compiler-warning diagnostics. The separate
`-Xlint:deprecation,unchecked` inventory is covered by the third-party migrations and exact legacy
seam policy below; it is not allowed to disappear through a source-set suppression.
## Non-Goals
- Do not remove the legacy poster-image endpoint, `StoredObjectResponse`, raw-key compatibility
data, or legacy object-storage adapters during warning cleanup.
- Do not switch the sample runtime from legacy to publication mode without the separately required
API, data-adoption, dual-read, and external-consumer approvals.
- Do not apply module-wide or task-wide suppression for `removal`, `deprecation`, `unchecked`, or
Error Prone rules.
- Do not weaken architecture rules or change deliberately forbidden bytecode merely to silence a
fixture warning.
- Do not make quarantine tests blocking; their separate sunset and reporting policy remains
unchanged.
## Design Principles
1. Fix behavior defects at their source before applying any suppression.
2. Use suppression only where a framework signature, identity invariant, intentional violation
fixture, or approved compatibility seam makes the warning inapplicable.
3. Scope every suppression to the smallest class, method, field, constructor, or fixture that
explains it, with a nearby rationale.
4. Replace deprecated third-party APIs with their typed current equivalents and verify behavior,
not only compilation.
5. Capture expected-negative diagnostics and assert them exactly; never discard stderr globally.
6. Add blocking gates only after the current warning inventory is clean.
## Component Design
### 1. Real behavior defects
#### Locale-independent identifiers
Use `Locale.ROOT` for security roles, notification configuration keys, repository ACL names, and
test comparisons. Add Turkish-default-locale regressions that restore the original default locale
in `finally`:
- `JwtToAuthenticatedPrincipalConverter`: `admin` must always become `ROLE_ADMIN`;
- `RoutingNotifier`: diagnostic keys for `EMAIL` must remain `app.notification.routes.email...`;
- `RepoStatsAclMapper`: `IDEA/Repo` must normalize to `idea/repo`.
This is a correctness fix: the current code can generate dotless/dotted Turkish-I variants in
authorization and operational identifiers.
#### Quote-aware ETag list parsing
Do not replace `String.split(",")` with another delimiter-only splitter. A comma is legal inside a
quoted opaque entity tag. `ETags` will use a small scanner that:
- splits only on commas outside a quoted string;
- preserves weak-tag prefixes and the existing trimming behavior;
- treats malformed/unclosed quotes as non-matching input rather than guessing a token;
- preserves wildcard and ordinary multi-value behavior.
Regressions cover a single comma-bearing tag, a mixed list containing a weak comma-bearing tag,
ordinary lists, wildcard, stale values, blank input, and malformed quoting.
#### Asynchronous and cleanup failures
- `AsyncGracefulShutdownBehaviorTest` retains the returned `Future<?>` and observes `get()` so a
background assertion or exception cannot disappear.
- Outbox test cleanup methods propagate or wrap resource-destruction failures with the original
cause instead of using empty catches.
- Tests use fixed dates, UTF-8, and explicit locale rather than host defaults.
### 2. Production warning cleanup with preserved invariants
#### Redis primitive ownership
`RedisPrimitiveInvocation` intentionally requires descriptor object identity. Value equality would
admit a descriptor created by another catalog and weaken the closed-catalog invariant. Keep the
reference comparison, add an exact constructor-level `ReferenceEquality` suppression, and add a
regression proving value-equal but non-identical cross-catalog descriptors are rejected.
Qualify both nested `ExpectedKind` types with their enclosing record names rather than renaming the
types. This removes `SameNameButDifferent` without changing bytecode or package-local consumers.
#### Framework-owned generic signature
`RedisVersionedSession.<T>getAttribute(String)` must retain Spring Session's inherited signature.
Apply a method-only `TypeParameterUnusedInFormals` suppression with the interface-contract reason.
#### Instance-owned retry context
`OutboundRetryPolicy` keeps its instance `ThreadLocal`. Making it static would leak call context
between policy instances on the same thread. Add a field-only `ThreadLocalUsage` suppression and a
regression proving policy A's context is invisible to policy B and is cleared by `endCall()`.
#### Array-bearing records
- `NotificationCiphertext` retains its public array components because it already clones inputs and
accessors, implements content-based equality/hash code, and redacts `toString`. Add focused
defensive-copy/equality/redaction tests and an exact record-level suppression.
- The four internal session command/outcome records in `VersionedRedisSessionStore` remain internal
transport envelopes. Preserve defensive copies, document that generated record equality is not
their contract, add constructor/accessor copy tests, and suppress `ArrayRecordComponent` on each
exact record.
- The private test fake in `RedisVersionedSessionRepositoryTest` receives the same exact nested-type
treatment; no public type is changed.
#### Mechanical behavior-neutral fixes
- Express 72 hours as `Duration.ofDays(3)` in application/bootstrap/sample settings and matching
tests.
- Add the missing public Javadoc summary in `TracingSampleRateResolver`, and render annotation names
such as `@WebMvcTest` as inline `{@code ...}` rather than accidental block tags.
- Add missing `@Override` annotations in sample test fakes.
- Replace readability-only string concatenations with text blocks where the literal bytes remain
identical.
- Replace Boolean wrapper comparisons with boolean literals.
- For the three test-only delimiter warnings, preserve each existing grammar explicitly: retain CSV
empty-token filtering with a limit-bearing split or scanner, scan mapping-path segments without
changing leading/trailing-empty behavior, and parse the single HTTP byte-range hyphen with an
asserted `indexOf` boundary. These are not allowed to inherit the ETag scanner because their
grammars differ.
### 3. Third-party API migration
#### Jackson 3
In `LocalJsonSchemaRegistry`, replace deprecated `JsonNode.isTextual()`/`textValue()` with
`isString()`/`stringValue()`. Existing type guards remain, and JSON schema identity/reference/value
tests prove identical acceptance and rejection behavior.
In `DeterministicEnvelopeWriter`, replace the deprecated convenience call with
`jsonFactory.createGenerator(ObjectWriteContext.empty(), output, JsonEncoding.UTF8)`, the
non-deprecated Jackson 3.0.2 overload. Preserve canonical byte output; the existing deterministic
envelope golden tests are the behavior gate.
#### Lettuce
Convert both finite canonical scores to `BigDecimal`, build one inclusive
`Range<? extends Number>` for each invocation, and call the typed `zcount(key, range)` and
`zrangebyscoreWithScores(key, range, Limit.create(offset, count))` overloads. Preserve inclusive
bounds, offset, count, and exact reply mapping. A dynamic-proxy regression verifies both typed
overloads are selected; sorted-set primitive contract tests verify results.
#### AWS SDK retry
Replace old `RetryPolicy` and core `EqualJitterBackoffStrategy` with `StandardRetryStrategy`, the
retries API half-jitter exponential backoff, `maxAttempts`, and
`ClientOverrideConfiguration.Builder.retryStrategy`. Tests assert maximum attempts and normal versus
throttling backoff configuration. The focused object-storage check must cover provider assembly;
compile-only success is insufficient.
#### Testcontainers Toxiproxy
Use the Testcontainers 2 toxiproxy package and a typed `ToxiproxyClient`/`Proxy` with an explicit
exposed proxy port. Fault tests must still prove cut and restore behavior against MinIO. Dependency
and lock changes stay inside the object-storage leaf.
#### Remaining JDK/generic deprecations
- Replace deprecated `new URL(String)` test construction with `URI.create(...).toURL()`.
- Replace the varargs `thenReturn(firstFuture, secondFuture)` stub in
`S3ConditionalObjectControlStoreTest` with two chained single-value `thenReturn(...)` calls, so
Mockito does not create the unchecked generic `CompletableFuture<PutObjectResponse>[]` array.
- Resolve every `-Xlint:deprecation,unchecked` location individually; do not suppress the source
set.
### 4. Legacy object-storage compatibility seam
The canonical object-storage ports and sample publication path already exist. The legacy runtime is
still selected in local/test configuration and cannot be deleted solely to silence warnings.
Keep `@Deprecated(forRemoval = true)` on the genuinely replaced whole-byte contracts:
- `ObjectStoragePort`;
- `StoredObject`;
- `ObjectStorageSettings`.
Apply `removal` suppression only to exact compatibility owners:
- `ObjectStoragePort` for its legacy receipt return type;
- `FilesystemObjectStorageAdapter` and `S3ObjectStorageAdapter`;
- `UploadPosterImageUseCase`;
- the legacy bean method in `PosterImageApiConfig`;
- `LegacyPosterImageController`;
- `PosterWebMapper.toStoredObjectResponse`;
- named legacy characterization test classes and single legacy-receipt test methods.
The six `application.storage.migration` types and `AdoptLegacyPosterImageUseCase` are the mechanism
used to complete data adoption and currently have no replacement. Change their lifecycle marker
from `@Deprecated(forRemoval = true)` to plain `@Deprecated`; use exact `deprecation` suppression
only inside adoption implementation/configuration. Keep the application-core architecture contract
requiring `forRemoval=true` only for `ObjectStoragePort` and `StoredObject`. Keep the adapter-owned
`ObjectStorageSettings` marker and add its lifecycle assertion in the object-storage leaf.
This keeps migration debt visible without falsely claiming that the migration mechanism itself is
ready for removal.
### 5. Test/static-analysis/output cleanup
#### SpotBugs
Reuse one static `SecureRandom` in `RedisPrimitiveRuntimeServiceTest` rather than constructing a
one-shot generator. After all test reports are clean, make every ordinary and custom test-source
SpotBugs task included by `check` blocking. SpotBugs analysis errors and missing classes remain
separately fail-closed.
#### Intentional architecture fixtures
Keep prohibited `BigDecimal(double/float)` constructor bytecode and apply method-only
`BigDecimalLiteralDouble` suppressions. Fix unrelated warnings in allowed fixtures normally. A
suppression must never replace the forbidden operation the ArchUnit test is supposed to detect.
#### Redis lab expected failures
Change `assert_fails` to capture stdout/stderr per case, assert a non-zero exit and the exact expected
diagnostic, reject extra lines, and print the capture only when the assertion fails. Do not redirect
to `/dev/null` and do not silence the Gradle `Exec` task globally.
#### Mockito/CDS
Provide `mockito-core` to test JVMs as an explicit startup `-javaagent` through a relocatable Gradle
argument provider. This removes reliance on Java 21+ runtime self-attachment. Add test-JVM-only
`-Xshare:off` because Mockito's bootstrap append otherwise prints the harmless CDS warning. No
production JVM argument changes.
#### Skips
Retain exactly these five intentional app-bootstrap contract skips:
- `emailNotificationAdapterRunsOnlyWhenConfigured()`;
- `slackNotificationAdapterRunsOnlyWhenConfigured()`;
- `redisCacheAdapterRunsOnlyWhenEnabled()`;
- `messagingBrokerAdapterRunsOnlyWhenConfigured()`;
- `DisabledOptionalAdapterFixture.wouldFailIfItEverRan()`.
Qualification tasks continue to require positive discovery, at least one executed test, zero skips,
and fresh XML, so this policy cannot turn a selected qualification lane green without execution.
Any additional skip, or any of these five moving outside its named optional-adapter contract, fails
the inventory check.
### 6. Warning-zero enforcement
After all existing warnings are removed:
- configure every leaf `JavaCompile` task with `-Werror`, `-Xlint:deprecation`, and
`-Xlint:unchecked` in the root build policy;
- retain Error Prone on the same compile tasks so its warnings are promoted by `-Werror`;
- remove the root `checkstyleTest`/`spotbugsTest` warning-only policy and the app-bootstrap
`sampleOffTest`, `functionalTest`, and `conditionalTransportTest` Checkstyle/SpotBugs
`ignoreFailures` overrides, making every such task included by `check` blocking;
- retain exact suppression comments as the only approved exception mechanism;
- run Gradle with `--warning-mode=fail` in the warning-clean CI lane so Gradle API deprecations also
fail rather than print.
`quarantineTest` remains non-blocking by design. Protected AWS/Docker qualifications remain separate
environment evidence and are not converted into local unit tests.
## File Ownership and Expected Change Groups
### Root build policy
- `src/build.gradle`
- `src/gradle/test-jvm-agents.gradle`, defining the relocatable Mockito `-javaagent` argument
provider and test-only `-Xshare:off` policy, applied once by the root build
- `.github/workflows/ci-quality-gates.yml`, adding `--warning-mode=fail` to the blocking
`quality-gates` Gradle invocation
### Production leaves
- `src/application-core`
- `src/adapter/inbound/web`
- `src/adapter/outbound/cache-redis`
- `src/adapter/outbound/fileserver`
- `src/adapter/outbound/httpclient`
- `src/adapter/outbound/identifier`
- `src/adapter/outbound/messaging`
- `src/adapter/outbound/notification`
- `src/adapter/outbound/objectstorage`
- `src/adapter/outbound/persistence-jpa`
- `src/app-bootstrap`
- `src/sample-portfolio`
- `src/shared-contract`
Every focused command is derived from the owning leaf's `gradle_path` in
`src/config/architecture/modules.json`; no production dependency edge changes are permitted unless
the registry is deliberately updated and its architecture verifier passes.
### Tests and shell contract
- owning leaf tests adjacent to every behavior change
- exact architecture violation fixtures under app-bootstrap test sources
- `infra/redis-lab/test/redis-lab-contract.sh`
## Implementation Sequence
1. Add failing behavioral regressions for locale, ETag parsing, async exception observation,
cleanup propagation, Redis descriptor identity, and retry-context isolation.
2. Implement those behavior fixes and run owner-focused tests.
3. Remove behavior-neutral compiler/Error Prone warnings per leaf, using only exact justified
suppressions.
4. Migrate Jackson, Lettuce, AWS SDK, Testcontainers, URL, and generic stubs; run their focused
behavior/qualification tests.
5. Correct legacy lifecycle markers and exact compatibility suppressions; run application-core,
object-storage, sample, and architecture contracts.
6. Clean test-only warnings, SpotBugs, Mockito/CDS, and Redis-lab output.
7. Enable blocking compiler, Checkstyle, SpotBugs, and Gradle warning gates.
8. Run focused checks, architecture validators, dependency locks, full tests, full check, and the
fresh warning-clean build.
9. Update the LLM Wiki branch note and the warning-debt error note with resolved evidence or exact
remaining environmental blockers.
## Verification Strategy
### Focused verification
- Each behavior change follows RED → GREEN with the owning leaf test.
- Static-only warning fixes use the exact `compileJava`, `compileTestJava`, Checkstyle, or SpotBugs
task as the failing/passing executable contract.
- Third-party API migrations run behavior tests that exercise request mapping, retry/backoff,
sorted-set bounds, schema parsing, or network-fault cut/restore semantics.
- Legacy suppressions are checked by architecture tests that reject old imports outside the named
compatibility surface.
### Repository verification
Run from `src/`:
```bash
./gradlew test --no-daemon --console=plain
./gradlew check --no-daemon --console=plain
./gradlew build --warning-mode=fail --no-daemon --console=plain
./gradlew clean build --warning-mode=all --no-daemon --console=plain
./gradlew verifyCleanArchitectureDependencies verifyRuntimeModuleMembership \
verifyDependencyLocks verifyPublicPathSnapshot verifyEnvKeys \
--no-daemon --console=plain
```
Also verify the real gate matrix, wrapper contract, shell syntax, warning-report XML, skipped-test
inventory, and `git diff --check`.
## Failure Handling
- If a suggested warning fix changes a public signature or weakens an identity/security invariant,
retain the behavior and use an exact documented suppression backed by a regression.
- If three attempted fixes in one warning family fail or expose cross-module coupling, stop that
family and revisit the design instead of stacking suppressions.
- If the AWS retry or Toxiproxy migration cannot reproduce old behavior, report that qualification
as blocked; do not claim warning-zero by suppressing the deprecation.
- If a warning originates only in generated code, prove the generated source owner and configure
that exact generated boundary; do not disable warnings for handwritten sources.
## Risks and Mitigations
- **ETag grammar regression:** use quote-aware focused tests before replacing the parser.
- **Authorization drift:** test role normalization under Turkish locale.
- **Redis catalog weakening:** retain identity comparison and test cross-catalog rejection.
- **AWS retry semantic drift:** assert maximum attempts and backoff classes/policies, then run the
object-storage provider tests.
- **Legacy data stranding:** preserve legacy activation and characterization until the separate
data/API migration gates are approved.
- **Hidden diagnostics:** capture-and-assert expected stderr; never discard it.
- **Suppression creep:** exact annotations plus architecture/import checks prevent module-wide
exemptions.
- **Build duration:** use owner-focused RED/GREEN loops and reserve full clean builds for integration
checkpoints and final proof.
## Acceptance Criteria
The work is complete only when:
1. All behavior regressions and focused owner checks pass.
2. Every compiler task passes with `-Werror`, deprecation lint, unchecked lint, and Error Prone.
3. Every ordinary/custom Checkstyle and SpotBugs task included by `check` is blocking and clean.
4. Legacy warnings are limited to no output because exact compatibility code is explicitly and
locally justified; no module/task-wide suppression exists.
5. The Redis lab successful contract prints only its success summary and unexpected child
diagnostics still fail the test with captured evidence.
6. Test JVMs print no CDS/self-attachment warning.
7. Full test, check, build, dependency, architecture, runtime-membership, env, public-path, wrapper,
gate-matrix, shell, and diff validators pass.
8. The final fresh clean-build log contains no `warning:`, deprecated/unchecked `Note:`, SpotBugs
non-zero message, OpenJDK warning, or leaked expected-negative Redis diagnostic.
9. LLM Wiki capture records commands, results, resolved warning counts, suppressions, and any
environment-only qualification not executed locally.