# 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.