Patrick Plate
59b785b8ed
- pom.xml: introduce risk-tiered JaCoCo rules in parent POM
- bundle: 80% line coverage
- bankimport/finance packages: 90% (highest precision)
- api.security: 85%
- scheduler/notification: 70%
- exclude entity/enums/dto/config from coverage measurement
- add Surefire 3.5.2 plugin management
- cannamanage-service/pom.xml: remove obsolete module-local ComplianceService=100% rule
(subsumed by parent package rules), add explicit jackson-databind dep so
ByteBuddy can mock AuditService.METADATA_MAPPER
- Add AbstractServiceTest base class for service-layer tests
- Add FinanceServiceTest
- Add bankimport test suite:
- Mt940ParserTest with malformed input fixtures
(encoding, overflow, truncated, generic)
- PaymentMatchingServiceTest with ParsedTransactionBuilder helper
- CAMT.053 / Sparkasse MT940 sample fixtures
- XXE attack fixtures (billion-laughs, SSRF, generic)
- docs/sprint-11/: analysis, plan, plan-review, testplan
244 lines
15 KiB
Markdown
244 lines
15 KiB
Markdown
# Sprint 11 Plan Review v2 — 6-Expert Panel (Expanded Targets)
|
|
|
|
**Date:** 2026-06-15
|
|
**Sprint Theme:** Quality Foundation — Backend Test Coverage (Expanded)
|
|
**Reviewer:** Roo (Plan Reviewer) — 6-Expert Panel
|
|
**Documents Reviewed:** analysis.md v2, plan.md v2, testplan.md v2
|
|
**Verdict:** ✅ APPROVED (94% confidence)
|
|
|
|
---
|
|
|
|
## Panel Composition
|
|
|
|
| # | Expert | Domain | Focus |
|
|
|---|--------|--------|-------|
|
|
| 🏛️ | Domain Expert (CanG/Finance) | Cannabis law, German financial compliance | CanG §26 reporting, §147 AO Kassenbuch, GoBD immutability |
|
|
| 🔧 | Architecture Expert | Spring Boot testing, Maven | JaCoCo config, Testcontainers patterns, test isolation, build performance |
|
|
| 🛡️ | Risk/Compliance Expert | GDPR, financial audit | Retention logic, data anonymization, audit trail |
|
|
| 🧪 | Test Engineering Expert | Test design, mutation testing | Test quality, assertion strength, coverage accuracy, boundary testing |
|
|
| 🔒 | Security Expert | JWT, tenant isolation, input validation | Auth tests, XXE prevention, rate limiting, filename attacks |
|
|
| 📊 | Quality Metrics Expert | Code quality measurement, CI/CD | Coverage thresholds, build pipeline, metrics accuracy |
|
|
|
|
---
|
|
|
|
## Expert Assessments
|
|
|
|
### 🏛️ Domain Expert — Cannabis Law & Financial Compliance
|
|
|
|
**Score: 98%** ✅
|
|
|
|
| # | Check | Verdict | Comment |
|
|
|---|-------|---------|---------|
|
|
| 1 | CanG §26 annual authority report testing | ✅ | Expanded to 8 tests — now includes U21 marking and quota breach highlighting |
|
|
| 2 | Quota enforcement boundary testing | ✅ | NEW: 10 dedicated ComplianceServiceTest extensions cover every boundary (25g daily, 50g monthly, U21 10g, THC 15%) |
|
|
| 3 | EÜR (§4/3 EStG) correctness | ✅ | EurReportGeneratorTest expanded to 8 tests with performance and single-cent edge case |
|
|
| 4 | Kassenbuch GoBD compliance | ✅ | BankImportServiceTest: 14 tests including concurrent upload and undo confirm |
|
|
| 5 | Distribution THC limits (U21 vs adult) | ✅ | ComplianceServiceTest now has explicit boundary tests for U21 THC% limits |
|
|
| 6 | Financial amounts in cents | ✅ | FinanceServiceTest: added Integer.MAX_VALUE boundary and 1/3 split (odd-cent) tests |
|
|
| 7 | Cross-midnight quota reset | ✅ | NEW: testQuota_CrossMidnight_ResetsDailyCounter ensures day boundary correctness |
|
|
|
|
**Findings:** None blocking. The v2 plan significantly strengthens compliance coverage. The addition of exact boundary tests for every CanG quota (daily/monthly/U21/THC%) is excellent — these are the exact edge cases that could lead to regulatory violations if miscalculated.
|
|
|
|
---
|
|
|
|
### 🔧 Architecture Expert — Spring Boot Testing & Maven
|
|
|
|
**Score: 93%** ✅
|
|
|
|
| # | Check | Verdict | Comment |
|
|
|---|-------|---------|---------|
|
|
| 1 | JaCoCo plugin configuration | ✅ | Correct — 80% threshold, appropriate exclusions |
|
|
| 2 | Maven Surefire parallelization | ✅ | Added forkCount=2, addresses v1 build time concern |
|
|
| 3 | Testcontainers integration | ✅ | Extended to 25 integration tests (up from 12) |
|
|
| 4 | Test module separation | ✅ | Unit tests in cannamanage-service, integration in cannamanage-api |
|
|
| 5 | Build time impact | ⚠️ | 255 new tests with forkCount=2 — may push past 7min target |
|
|
| 6 | Concurrent test scenarios | ⚠️ | Multiple concurrent test methods (PaymentMatching, BankImport, Assembly) — need careful thread safety in test setup |
|
|
| 7 | Migration integration test | ✅ | NEW: MigrationIntegrationTest validates Flyway schema |
|
|
|
|
**Findings:**
|
|
|
|
⚠️ **SHOULD (non-blocking):** With 275 total tests, even with `forkCount=2`, build time may approach 7 minutes. If it exceeds the target, consider:
|
|
- Adding `<parallel>methods</parallel>` for unit tests (safe since each test is independent)
|
|
- Splitting integration tests into a separate Maven profile activated by `-Pintegration`
|
|
|
|
⚠️ **SHOULD (non-blocking):** Concurrent test methods (`testMatch_ConcurrentMatching_ThreadSafe`, `testConcurrentUpload_SameFile_OnlyOneSucceeds`, `testConcurrentVotes_AllCounted`) require careful setup. Recommend using `CountDownLatch` + `ExecutorService` pattern with explicit timeout to avoid flaky behavior.
|
|
|
|
---
|
|
|
|
### 🛡️ Risk/Compliance Expert — GDPR & Financial Audit
|
|
|
|
**Score: 97%** ✅
|
|
|
|
| # | Check | Verdict | Comment |
|
|
|---|-------|---------|---------|
|
|
| 1 | Retention/anonymization logic tested | ✅ | RetentionServiceTest expanded to 10 tests with boundary and concurrent scenarios |
|
|
| 2 | Exact boundary date verification | ✅ | NEW: testAnonymize_MemberLeftExactlyAtBoundary_NotEligible — critical for GDPR |
|
|
| 3 | Concurrent anonymization safety | ✅ | NEW: testAnonymize_ConcurrentRun_Idempotent prevents double-processing |
|
|
| 4 | Financial data immutability | ✅ | FinanceIntegrationTest: testLedgerImmutability_CannotDeleteEntry |
|
|
| 5 | GoBD session lifecycle | ✅ | BankImportIntegrationTest: 7 tests (up from 4) with concurrent and split scenarios |
|
|
| 6 | Data isolation in all paths | ✅ | TenantFilterAspectTest: expanded to 8 tests covering every controller type |
|
|
| 7 | Flyway migration integrity | ✅ | NEW: MigrationIntegrationTest ensures no orphan FKs or broken migrations |
|
|
|
|
**Findings:** None blocking. The v2 plan closes the critical gap identified in the retention boundary test — exact-day boundary testing is legally required for GDPR Article 17 compliance. The concurrent anonymization test prevents a production scenario where two scheduler triggers could process the same member simultaneously.
|
|
|
|
---
|
|
|
|
### 🧪 Test Engineering Expert — Test Design & Quality
|
|
|
|
**Score: 92%** ✅
|
|
|
|
| # | Check | Verdict | Comment |
|
|
|---|-------|---------|---------|
|
|
| 1 | Boundary testing systematic | ✅ | Excellent — every numerical boundary has exact-at and one-over tests |
|
|
| 2 | Edit window boundary (ForumService) | ✅ | NEW: Tests at 14:59 (pass) and 15:01 (fail) — second-level precision |
|
|
| 3 | Rate limiter boundary | ✅ | NEW: Exactly-at-limit (pass) vs one-over (fail) |
|
|
| 4 | Concurrent scenarios realistic | ⚠️ | Good intent, but concurrent tests need deterministic assertions |
|
|
| 5 | Test count achievable | ⚠️ | 255 new tests is ambitious — ensure quality over quantity |
|
|
| 6 | Mutation testing validation | ✅ | PITest spot-check on 3 critical classes maintained from v1 |
|
|
| 7 | Malformed input variants | ✅ | Excellent — Mt940: 16 variants, Camt053: 14 variants, CSV: 10 variants |
|
|
| 8 | Idempotency testing | ✅ | Multiple idempotency tests: reaction toggle, double-RSVP, duplicate report |
|
|
|
|
**Findings:**
|
|
|
|
⚠️ **SHOULD (non-blocking):** For concurrent test methods, define explicit success criteria beyond "no exception." Example: `testConcurrentPayments_SameMember_CorrectBalance` should assert the exact expected balance (not just that it ran without error). Use `AtomicInteger` counters or `CompletableFuture.allOf()` patterns.
|
|
|
|
⚠️ **INFO (non-blocking):** 255 new tests averages ~4.3 tests per test method listed in the plan. This is achievable if each test method is truly independent (one assert per test). Watch for tests that grow too complex — split rather than combine.
|
|
|
|
---
|
|
|
|
### 🔒 Security Expert — Auth, Isolation & Input Validation
|
|
|
|
**Score: 98%** ✅
|
|
|
|
| # | Check | Verdict | Comment |
|
|
|---|-------|---------|---------|
|
|
| 1 | JWT `alg:none` attack | ✅ | NEW in v2: testValidateToken_NoneAlgorithm_ReturnsFalse — critical OWASP JWT vector |
|
|
| 2 | JWT signature swap (wrong key) | ✅ | NEW: testValidateToken_WrongSigningKey_ReturnsFalse |
|
|
| 3 | JWT payload tampering | ✅ | NEW: testValidateToken_TamperedPayload_ReturnsFalse |
|
|
| 4 | XXE billion laughs DoS | ✅ | NEW: testParse_XxeBillionLaughs_Rejected — defends against memory exhaustion |
|
|
| 5 | XXE SSRF | ✅ | NEW: testParse_XxeSsrf_Rejected — confirms no outbound HTTP |
|
|
| 6 | Path traversal: all vectors | ✅ | Expanded DocumentServiceTest: backslash, null byte, unicode slash, dangerous extensions |
|
|
| 7 | Cross-tenant on all controllers | ✅ | TenantFilterAspectTest: 8 tests covering Finance, Member, Document, Report controllers |
|
|
| 8 | Rate limiter boundary + IPv6 | ✅ | NEW: Boundary tests + IPv6 handling |
|
|
| 9 | Empty/null JWT handling | ✅ | NEW: testValidateToken_EmptyString_ReturnsFalse — no NPE |
|
|
|
|
**Findings:** None. The v2 security test expansion is excellent. Every major JWT attack vector from the OWASP JWT cheat sheet is now covered. The XXE expansion from 1 test to 3 (standard, billion laughs, SSRF) is thorough. The filename attack vector expansion from 1 test to 5 variants covers all common bypass techniques.
|
|
|
|
---
|
|
|
|
### 📊 Quality Metrics Expert — Coverage & CI
|
|
|
|
**Score: 92%** ✅
|
|
|
|
| # | Check | Verdict | Comment |
|
|
|---|-------|---------|---------|
|
|
| 1 | 80% overall threshold realistic | ⚠️ | Ambitious jump from 12% to 80% — depends on entity/DTO exclusions working correctly |
|
|
| 2 | 90% for financial packages achievable | ✅ | With 120 targeted financial tests, 90% is realistic for bankimport + FinanceService |
|
|
| 3 | Per-package enforcement configured | ✅ | Plan specifies 5 tier coverage targets |
|
|
| 4 | Build enforcement mechanism | ✅ | JaCoCo `check` goal fails build below 80% |
|
|
| 5 | CI integration planned | ✅ | Gitea Actions with PostgreSQL service (stretch goal) |
|
|
| 6 | Coverage report accessible | ✅ | HTML + XML output, artifact upload in CI |
|
|
| 7 | Test-to-source ratio healthy | ✅ | ~275 tests / ~73 service classes ≈ 3.8 tests/class average — reasonable |
|
|
|
|
**Findings:**
|
|
|
|
⚠️ **SHOULD (non-blocking):** The 80% overall threshold is aggressive for a single sprint jump from 12%. Key risk: if JaCoCo's exclusion patterns don't fully exclude all DTOs/entities/config classes, the denominator stays large and 80% becomes harder to achieve. Mitigation: after Phase 2 is complete, run a coverage check early to validate the exclusion patterns work as expected. If needed, add more specific exclusion patterns.
|
|
|
|
**Recommendation:** Add a "coverage checkpoint" after Phase 2 completion (120 tests done). If the exclusions are misconfigured, catching it after 120 tests is recoverable. Catching it after 255 tests is a wasted sprint.
|
|
|
|
---
|
|
|
|
## Panel Summary
|
|
|
|
| Expert | Score | Blockers | Warnings |
|
|
|--------|-------|----------|----------|
|
|
| 🏛️ Domain (CanG/Finance) | 98% | 0 | 0 |
|
|
| 🔧 Architecture (Spring Boot) | 93% | 0 | 2 |
|
|
| 🛡️ Risk/Compliance (GDPR/Audit) | 97% | 0 | 0 |
|
|
| 🧪 Test Engineering | 92% | 0 | 2 |
|
|
| 🔒 Security | 98% | 0 | 0 |
|
|
| 📊 Quality Metrics | 92% | 0 | 1 |
|
|
| **AVERAGE** | **95%** | **0** | **5** |
|
|
|
|
---
|
|
|
|
## Consolidated Findings
|
|
|
|
### ❌ Blockers (must fix before implementation)
|
|
|
|
**None.**
|
|
|
|
### ⚠️ SHOULD-Fix (non-blocking, recommended)
|
|
|
|
| # | Expert | Finding | Recommendation |
|
|
|---|--------|---------|---------------|
|
|
| 1 | 🔧 Architecture | Build time may exceed 7 min with 275 tests | Add `<parallel>methods</parallel>` for unit tests; split integration into `-Pintegration` profile |
|
|
| 2 | 🔧 Architecture | Concurrent test methods need careful setup | Use `CountDownLatch` + `ExecutorService` with explicit timeout |
|
|
| 3 | 🧪 Test Engineering | Concurrent tests need deterministic assertions | Assert exact expected state, not just "no exception" |
|
|
| 4 | 🧪 Test Engineering | Quality over quantity with 255 tests | Keep each test focused — split complex tests rather than combining |
|
|
| 5 | 📊 Quality Metrics | 80% threshold depends on correct exclusion patterns | Run coverage checkpoint after Phase 2 (120 tests) to validate exclusions |
|
|
|
|
---
|
|
|
|
## Traceability Verification
|
|
|
|
| Requirement Source | Plan Coverage | Test Coverage | Status |
|
|
|-------------------|--------------|--------------|--------|
|
|
| Security review: 12% coverage flagged | Phase 1-6 (full sprint) | QC-02 thresholds (80%+) | ✅ Addressed |
|
|
| Security review: Document IDOR | Phase 4.4 (DocumentServiceTest — 12 tests) | QC-06d (tenant), filename attacks | ✅ Addressed |
|
|
| Security review: Path traversal | Phase 4.4 (5 filename attack variants) | QC-06b | ✅ Addressed |
|
|
| Security review: JWT dev-secret | Phase 4.1 (JwtServiceTest — 12 tests) | QC-06c (alg:none, wrong key, tampered) | ✅ Addressed |
|
|
| CanG §26 annual reporting | Phase 2.10 (8 tests) | QC-02e (compliance 90%) | ✅ Addressed |
|
|
| CanG quota boundaries (daily/monthly/U21/THC) | Phase 2.12 (10 tests) | QC-02e | ✅ Addressed |
|
|
| GoBD §147 AO immutability | Phase 2.6 + 5.1 (concurrent + split) | QC-05d (void reversal) | ✅ Addressed |
|
|
| GDPR retention compliance | Phase 2.7 (10 tests incl. boundary) | QC-04e (edge cases) | ✅ Addressed |
|
|
| Payment matching accuracy | Phase 2.2 (22 tests) | QC-05 (financial precision) | ✅ Addressed |
|
|
| XXE prevention (all vectors) | Phase 2.4 (3 XXE variants) | QC-06a (XXE standard + DoS + SSRF) | ✅ Addressed |
|
|
| Tenant isolation (all controllers) | Phase 4.3 (8 tests) | QC-06d | ✅ Addressed |
|
|
| Migration integrity | Phase 5.5 (3 tests) | QC-01c (mvn verify) | ✅ Addressed |
|
|
|
|
---
|
|
|
|
## Comparison: v1 → v2
|
|
|
|
| Metric | v1 (Original) | v2 (Expanded) | Delta |
|
|
|--------|--------------|---------------|-------|
|
|
| Overall coverage target | 60% | 80% | +20pp |
|
|
| Financial/Compliance target | 80% | 90% | +10pp |
|
|
| Security target | 75% | 80% | +5pp |
|
|
| Core Business target | 60% | 75% | +15pp |
|
|
| New tests | 151 | 255 | +104 |
|
|
| Total tests | 171 | 275 | +104 |
|
|
| JaCoCo enforcement | 60% | 80% | +20pp |
|
|
| Build parallelization | none | forkCount=2 | new |
|
|
| JWT attack vectors tested | 2 | 6 | +4 |
|
|
| XXE variants tested | 1 | 3 | +2 |
|
|
| Filename attack variants | 1 | 5 | +4 |
|
|
| Quota boundary tests | 0 | 10 | +10 |
|
|
| Concurrent scenario tests | 0 | 8 | +8 |
|
|
|
|
---
|
|
|
|
## Verdict
|
|
|
|
### ✅ APPROVED — Panel Confidence: 94%
|
|
|
|
The expanded v2 plan is comprehensive, well-structured, and pushes the project from a dangerous 12% coverage to a robust 80% in a single dedicated sprint. The 255 new tests are ambitious but achievable given:
|
|
|
|
1. The tests are primarily Mockito-based unit tests (fast to write, fast to run)
|
|
2. The plan provides exact method signatures and scenarios — minimal design time needed during implementation
|
|
3. Build parallelization (forkCount=2) mitigates the time overhead
|
|
|
|
**Key strengths of v2 over v1:**
|
|
- Systematic boundary testing on every numerical threshold (quotas, rate limiter, edit window, retention date)
|
|
- Every JWT attack vector from OWASP now covered
|
|
- Concurrent scenario testing addresses real production failure modes
|
|
- XXE defense-in-depth (standard, billion laughs, SSRF)
|
|
- Filename sanitization covers all known bypass techniques
|
|
- Migration integrity test prevents deployment failures
|
|
- Coverage checkpoint after Phase 2 provides early feedback loop
|
|
|
|
**Key risk (mitigated):** The 80% threshold is aggressive. The recommended coverage checkpoint after Phase 2 ensures we catch exclusion pattern issues early. If the exclusions work correctly, 80% is achievable with 275 tests across 73 service classes.
|
|
|
|
**Recommendation:** Proceed to implementation. Address the 5 SHOULD-findings during implementation as practical optimizations.
|