From 22cc534f4f5578bf89c3f26961f9ef5342ef3ca2 Mon Sep 17 00:00:00 2001 From: vishwab1 Date: Tue, 20 Oct 2026 10:03:22 +0530 Subject: [PATCH 1/3] chore: set Common-API version to 3.12.0 Co-Authored-By: Claude Opus 5.5 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index ffe74404..4c727f1b 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.iemr.common-API common-api - 3.8.2 + 3.12.0 war Common-API From 2701b015faf89cea53ac120b324a7618fb5762ab Mon Sep 17 00:00:00 2001 From: vishwab1 Date: Tue, 20 Oct 2026 10:03:22 +0530 Subject: [PATCH 2/3] fix(user): do not wait for the CTI server when a password is set setForgetPassword called updateCTIPasswordForUserV1 from the same class, so its @Async was not applied: an unreachable CTI server held the password screen until the network timeout, before the password was saved (camp laptops cannot reach the call-centre CTI server). The CTI update now runs in the background; the password is saved and the user continues immediately. Co-Authored-By: Claude Opus 5.5 --- .../users/IEMRAdminUserServiceImpl.java | 14 ++++++++++-- .../users/IEMRAdminUserServiceImplTest.java | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java b/src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java index e411b9ef..3b60c78b 100644 --- a/src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java +++ b/src/main/java/com/iemr/common/service/users/IEMRAdminUserServiceImpl.java @@ -26,6 +26,7 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.security.spec.InvalidKeySpecException; +import java.util.concurrent.CompletableFuture; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -469,7 +470,7 @@ public int setForgetPassword(User user, String loginpass, String transactionId, int count = 0; try { if (isAdmin != null && isAdmin == true) { - updateCTIPasswordForUserV1(user.getUserID(), loginpass); + updateCTIPasswordInBackground(user.getUserID(), loginpass); loginpass = securePassword.generateStrongPassword(loginpass); count = iEMRUserRepositoryCustom.updateSetForgetPassword(user.getUserID(), loginpass); } else { @@ -477,7 +478,7 @@ public int setForgetPassword(User user, String loginpass, String transactionId, .getSessionObjectForChangePassword((user.getUserID().toString() + user.getUserName())); if (tokenFromRedis.equalsIgnoreCase(transactionId)) { - updateCTIPasswordForUserV1(user.getUserID(), loginpass); + updateCTIPasswordInBackground(user.getUserID(), loginpass); loginpass = securePassword.generateStrongPassword(loginpass); count = iEMRUserRepositoryCustom.updateSetForgetPassword(user.getUserID(), loginpass); // Deleting transaction Id @@ -600,6 +601,15 @@ public void updateCTIPasswordForUser(Long userID, String password) { } } + /** + * Sends the new password to the CTI server without making the user wait: called from this class, so the + * {@code @Async} of updateCTIPasswordForUserV1 is not applied (no proxy) and an unreachable CTI server held the + * password screen until the network timeout - before the password was even saved. + */ + void updateCTIPasswordInBackground(Long userID, String password) { + CompletableFuture.runAsync(() -> updateCTIPasswordForUserV1(userID, password)); + } + @Async public void updateCTIPasswordForUserV1(Long userID, String password) { try { diff --git a/src/test/java/com/iemr/common/service/users/IEMRAdminUserServiceImplTest.java b/src/test/java/com/iemr/common/service/users/IEMRAdminUserServiceImplTest.java index de282b52..a896327f 100644 --- a/src/test/java/com/iemr/common/service/users/IEMRAdminUserServiceImplTest.java +++ b/src/test/java/com/iemr/common/service/users/IEMRAdminUserServiceImplTest.java @@ -374,6 +374,28 @@ void testSetForgetPassword_ValidTransaction() throws Exception { assertEquals(1, result); } + @Test + void testSetForgetPassword_DoesNotWaitForCtiServer() throws Exception { + // Given: the CTI server does not answer until released + java.util.concurrent.CountDownLatch ctiReleased = new java.util.concurrent.CountDownLatch(1); + when(iEMRUserRepositoryCustom.findUserByUserID(1L)).thenReturn(testUser); + when(ctiService.addUpdateUserData(anyString(), anyString())).thenAnswer(invocation -> { + ctiReleased.await(10, java.util.concurrent.TimeUnit.SECONDS); + return new OutputResponse(); + }); + when(securePassword.generateStrongPassword("newpassword")).thenReturn("hashedNewPassword"); + when(iEMRUserRepositoryCustom.updateSetForgetPassword(1L, "hashedNewPassword")).thenReturn(1); + + // When: the password is saved without waiting for CTI + int result = assertTimeoutPreemptively(java.time.Duration.ofSeconds(2), + () -> userService.setForgetPassword(testUser, "newpassword", "token123", true)); + + // Then: saved, and CTI is still updated in the background + assertEquals(1, result); + ctiReleased.countDown(); + verify(ctiService, timeout(2000)).addUpdateUserData(anyString(), eq("127.0.0.1")); + } + @Test void testSetForgetPassword_InvalidTransaction() throws Exception { // Given From 4dface84a073456c5e49142229fdf45dfadcb5b7 Mon Sep 17 00:00:00 2001 From: Sehjot Singh Pannu Date: Mon, 2 Nov 2026 15:59:12 +0530 Subject: [PATCH 3/3] Removed claude files --- .claude/commands/new-bugfix.md | 46 -- .claude/commands/new-feature.md | 45 -- .claude/commands/new-hotfix.md | 52 -- .claude/commands/pr-checklist.md | 167 ----- .claude/skills/java-code-reviewer/SKILL.md | 247 -------- .../agents/architecture-reviewer.md | 181 ------ .../agents/security-auditor.md | 145 ----- .../agents/test-reviewer.md | 226 ------- .../references/common-issues.md | 397 ------------ .../references/review-checklist.md | 157 ----- .claude/skills/java-code-writer/SKILL.md | 156 ----- .../java-code-writer/references/exceptions.md | 158 ----- .../references/factory-patterns.md | 364 ----------- .../references/security-patterns.md | 197 ------ .../references/spring-patterns.md | 584 ------------------ 15 files changed, 3122 deletions(-) delete mode 100644 .claude/commands/new-bugfix.md delete mode 100644 .claude/commands/new-feature.md delete mode 100644 .claude/commands/new-hotfix.md delete mode 100644 .claude/commands/pr-checklist.md delete mode 100644 .claude/skills/java-code-reviewer/SKILL.md delete mode 100644 .claude/skills/java-code-reviewer/agents/architecture-reviewer.md delete mode 100644 .claude/skills/java-code-reviewer/agents/security-auditor.md delete mode 100644 .claude/skills/java-code-reviewer/agents/test-reviewer.md delete mode 100644 .claude/skills/java-code-reviewer/references/common-issues.md delete mode 100644 .claude/skills/java-code-reviewer/references/review-checklist.md delete mode 100644 .claude/skills/java-code-writer/SKILL.md delete mode 100644 .claude/skills/java-code-writer/references/exceptions.md delete mode 100644 .claude/skills/java-code-writer/references/factory-patterns.md delete mode 100644 .claude/skills/java-code-writer/references/security-patterns.md delete mode 100644 .claude/skills/java-code-writer/references/spring-patterns.md diff --git a/.claude/commands/new-bugfix.md b/.claude/commands/new-bugfix.md deleted file mode 100644 index a3ab0ad1..00000000 --- a/.claude/commands/new-bugfix.md +++ /dev/null @@ -1,46 +0,0 @@ -# /new-bugfix - -Create a new bugfix branch targeting the current release branch. - -## Steps - -1. Ask the user for: - - The Jira ticket ID (e.g., `AMRIT-456`) - - A short description of the bug being fixed (3–5 words, kebab-case) - - Which release branch to target (default: latest `release-X.Y.Z`) - -2. Determine available release branches: - ```bash - git branch -r | grep 'release-' | sort -V - ``` - -3. Ensure the target release branch is up to date: - ```bash - git fetch origin - git checkout - git pull origin - ``` - -4. Create and push the bugfix branch: - ```bash - git checkout -b bugfix/- - git push -u origin bugfix/- - ``` - -5. Confirm the branch was created: - ```bash - git branch --show-current - ``` - -## Branch Naming Rules - -- Pattern: `bugfix/AMRIT--` -- Slug: lowercase kebab-case, max 5 words, describes the bug (not the fix) -- Example: `bugfix/AMRIT-456-null-pointer-in-anc-save` - -## Notes - -- Bugfix branches target `release-X.Y.Z`, NOT `main`. -- If the fix is urgent and needs to go to `main` immediately, use `/new-hotfix` instead. -- If the user has uncommitted changes, warn them and ask whether to stash before switching. -- After branching, remind the user to write a failing test that reproduces the bug before implementing the fix. diff --git a/.claude/commands/new-feature.md b/.claude/commands/new-feature.md deleted file mode 100644 index 1fbab454..00000000 --- a/.claude/commands/new-feature.md +++ /dev/null @@ -1,45 +0,0 @@ -# /new-feature - -Create a new feature branch following the AMRIT branching strategy. - -## Steps - -1. Ask the user for: - - The Jira ticket ID (e.g., `AMRIT-123`) - - A short description (3–5 words, kebab-case) - -2. Determine the current release branch: - ```bash - git branch -r | grep 'release-' | sort -V | tail -1 - ``` - If no release branch exists, use `main` and warn the user. - -3. Ensure the local release branch is up to date: - ```bash - git fetch origin - git checkout - git pull origin - ``` - -4. Create and push the feature branch: - ```bash - git checkout -b feature/- - git push -u origin feature/- - ``` - -5. Confirm the branch was created: - ```bash - git branch --show-current - ``` - -## Branch Naming Rules - -- Pattern: `feature/AMRIT--` -- Slug: lowercase kebab-case, max 5 words -- Example: `feature/AMRIT-123-add-cbac-screening` - -## Notes - -- Feature branches MUST be cut from the current `release-X.Y.Z` branch, not from `main`. -- If the user has uncommitted changes, warn them and ask whether to stash before switching. -- Never push directly to `main` or `release-X.Y.Z`. \ No newline at end of file diff --git a/.claude/commands/new-hotfix.md b/.claude/commands/new-hotfix.md deleted file mode 100644 index 46234c7b..00000000 --- a/.claude/commands/new-hotfix.md +++ /dev/null @@ -1,52 +0,0 @@ -# /new-hotfix - -Create a hotfix branch from `main` for urgent production fixes. - -## Steps - -1. Ask the user for: - - The Jira ticket ID (e.g., `AMRIT-789`) - - A short description of the production issue (3–5 words, kebab-case) - - Confirmation that this is a production incident (not a regular bugfix) - -2. Ensure `main` is up to date: - ```bash - git fetch origin - git checkout main - git pull origin main - ``` - -3. Create and push the hotfix branch: - ```bash - git checkout -b hotfix/- - git push -u origin hotfix/- - ``` - -4. Confirm the branch was created: - ```bash - git branch --show-current - git log --oneline -3 - ``` - -## Branch Naming Rules - -- Pattern: `hotfix/AMRIT--` -- Slug: lowercase kebab-case, max 5 words -- Example: `hotfix/AMRIT-789-fix-jwt-expiry-crash` - -## Post-Merge Reminders - -After the hotfix PR merges to `main`, remind the user to: -1. Tag the release: `git tag -a vX.Y.Z+1 -m "Hotfix: "` -2. Back-merge `main` into the current `release-X.Y.Z` branch so the fix is not lost: - ```bash - git checkout release-X.Y.Z - git merge main - git push origin release-X.Y.Z - ``` - -## Notes - -- Hotfix branches MUST be cut from `main`, not from a release branch. -- Use this only for critical production issues. For non-urgent fixes use `/new-bugfix`. -- If the user has uncommitted changes, warn them and ask whether to stash before switching. diff --git a/.claude/commands/pr-checklist.md b/.claude/commands/pr-checklist.md deleted file mode 100644 index 0799185a..00000000 --- a/.claude/commands/pr-checklist.md +++ /dev/null @@ -1,167 +0,0 @@ -# /pr-checklist - -Audit the current branch against AMRIT PR standards before opening a pull request. Report every finding — do not skip items that cannot be auto-checked. - -## Execution Plan - -Run each check in order. For each check, print: `[PASS]`, `[FAIL]`, or `[WARN]` followed by a one-line explanation. - ---- - -### 1. Branch Name - -```bash -git branch --show-current -``` - -- PASS if: matches `feature/AMRIT-\d+-.+`, `bugfix/AMRIT-\d+-.+`, or `hotfix/AMRIT-\d+-.+` -- FAIL if: does not include a Jira ticket ID or uses `main`/`release-*` - ---- - -### 2. Target Branch - -```bash -git log --oneline origin/main..HEAD | wc -l -git branch -r | grep 'release-' | sort -V | tail -1 -``` - -- Confirm the PR targets the correct branch (feature/bugfix → latest release branch; hotfix → main). -- WARN if the branch appears to have diverged by more than 50 commits. - ---- - -### 3. Build Passes - -```bash -mvn clean package -Denv=example -q 2>&1 | tail -5 -``` - -- PASS if build exits 0. -- FAIL if there are compilation errors. - ---- - -### 4. Tests Pass - -```bash -mvn test -DskipTests=false 2>&1 | grep -E 'Tests run|BUILD|FAILURE|ERROR' | tail -20 -``` - -- PASS if `BUILD SUCCESS` and 0 failures/errors. -- FAIL if any test fails or the build errors. - ---- - -### 5. No Hardcoded Secrets - -```bash -git diff origin/main...HEAD -- '*.java' '*.properties' '*.yml' '*.yaml' | \ - grep -E '(?i)(password|secret|token|apikey|api_key)\s*=\s*["\x27][^$\{]' || echo "CLEAN" -``` - -- PASS if output is `CLEAN`. -- FAIL if any matches found — list the file and line. - ---- - -### 6. No System.out / printStackTrace - -```bash -git diff origin/main...HEAD -- '*.java' | grep -E '(System\.out\.|e\.printStackTrace)' | head -10 || echo "CLEAN" -``` - -- PASS if `CLEAN`. -- FAIL if matches found. - ---- - -### 7. No PII in Logs - -```bash -git diff origin/main...HEAD -- '*.java' | \ - grep -E 'log\.(info|debug|warn|error).*\b(name|aadhaar|phone|mobile|address|dob)\b' | head -10 || echo "CLEAN" -``` - -- PASS if `CLEAN`. -- WARN if matches found — manual review required. - ---- - -### 8. @Valid Present on Controller Endpoints - -```bash -git diff origin/main...HEAD -- 'src/main/java/**/controller/**/*.java' | \ - grep -E '(@PostMapping|@PutMapping|@PatchMapping)' -A3 | grep -v '@Valid' | head -20 || echo "CLEAN" -``` - -- WARN if any mutation endpoint parameter is missing `@Valid`. - ---- - -### 9. No Direct Repository Calls in Controllers - -```bash -git diff origin/main...HEAD -- 'src/main/java/**/controller/**/*.java' | \ - grep -E 'Repository|\.save\(|\.findBy' | head -10 || echo "CLEAN" -``` - -- FAIL if controllers import or call repositories directly. - ---- - -### 10. No JPA Entities Returned from Controllers - -```bash -git diff origin/main...HEAD -- 'src/main/java/**/controller/**/*.java' | \ - grep -E 'ResponseEntity<(List<)?(AncCare|DiseaseAESJE|CbacDetails|ChildRegister|[A-Z][a-z]+Entity)' | head -10 || echo "CLEAN" -``` - -- FAIL if an entity type appears as a controller return type. - ---- - -### 11. Tests Added for New Code - -```bash -git diff --name-only origin/main...HEAD | grep 'src/main/java' | sed 's|src/main/java/||;s|\.java$||' | \ - while read class; do - base=$(basename "$class") - find src/test -name "${base}Test.java" | head -1 - done -``` - -- WARN for every changed production class that has no corresponding test class. - ---- - -### 12. Constructor Injection Only - -```bash -git diff origin/main...HEAD -- '*.java' | grep -E '@Autowired\s+private' | head -10 || echo "CLEAN" -``` - -- FAIL if field-level `@Autowired` is used. - ---- - -## Summary - -After all checks, print a summary table: - -| # | Check | Status | -|---|---|---| -| 1 | Branch name | | -| 2 | Target branch | | -| 3 | Build passes | | -| 4 | Tests pass | | -| 5 | No hardcoded secrets | | -| 6 | No System.out | | -| 7 | No PII in logs | | -| 8 | @Valid on endpoints | | -| 9 | No repo calls in controller | | -| 10 | No entities from controller | | -| 11 | Tests added | | -| 12 | Constructor injection | | - -Then print: **"Ready to open PR"** if all checks PASS/WARN, or **"PR blocked — fix FAIL items first"** if any FAIL. diff --git a/.claude/skills/java-code-reviewer/SKILL.md b/.claude/skills/java-code-reviewer/SKILL.md deleted file mode 100644 index a342b18a..00000000 --- a/.claude/skills/java-code-reviewer/SKILL.md +++ /dev/null @@ -1,247 +0,0 @@ -# java-code-reviewer - -Orchestrator skill for reviewing Java / Spring Boot code in the AMRIT / FLW-API project. - -## Triggers - -Activate this skill when the user says any of: -- "review this", "check my code", "audit this", "PR review", "code review" -- "is this correct", "what's wrong with this", "look at this file" -- Pastes raw Java code or a file path to a `.java` file -- Shares a diff or lists 2+ changed files - ---- - -## Routing Decision - -### Single file or pasted snippet -Run a full inline review yourself. Work through all 6 dimensions below in order. Produce the structured report defined in **Report Format**. - -### PR with 3 or more changed files -Spawn the three specialized agents **in parallel**: -1. `agents/security-auditor.md` — security scan -2. `agents/architecture-reviewer.md` — layering and SOLID check -3. `agents/test-reviewer.md` — test coverage and quality - -Wait for all three JSON outputs. Then synthesize them into one final **Synthesized PR Report** (format defined below). Do not produce a partial report while agents are running. - ---- - -## 6 Review Dimensions - -Work through every dimension for every file. Skip a dimension only if it is genuinely not applicable (e.g., no tests exist yet and the task is not about testing). - -### 1. Architecture & Layering -Enforce the strict AMRIT layer contract: - -``` -Controller → Service Interface → Service Impl → Repository → Entity - ↕ - Mapper (DTO ↔ Entity) -``` - -Check: -- Controllers import only: DTOs, service interfaces, `ApiResponse`, Spring MVC types, validation annotations. No repository or entity imports. -- Service implementations own all business logic. No Spring MVC types (`HttpServletRequest`, etc.). -- Repositories are Spring Data JPA interfaces only. No business logic. -- DTOs cross the Controller ↔ Service boundary. Entities never leave the service layer. -- Mappers (MapStruct) handle all DTO ↔ Entity conversion. No manual field copying. -- Dependency direction never points upward (lower layer must not import higher layer). - -### 2. Security -This project uses **custom JWT auth** (not Spring Security method security). There are no `@PreAuthorize` annotations in the current codebase. The security model is: -- `JwtUserIdValidationFilter` validates the JWT and returns 401 if invalid -- `HTTPRequestInterceptor` validates the token exists in Redis via `Validator.checkKeyExists()` -- Role data lives in `v_userservicerolemapping` (read via `UserServiceRoleRepo`) - -Check for: -- Missing `Authorization` header requirement on controller endpoints (all non-public endpoints must declare `@RequestHeader("Authorization")` or have `headers = "Authorization"` on the `@RequestMapping`) -- Missing `@Valid` on any `@RequestBody` parameter -- `@RequestParam` / `@PathVariable` accepting IDs without `@Positive`/`@Min(1)` -- Hardcoded passwords, API keys, tokens, or connection strings in any `.java` or `.properties` file -- PII in log statements: names, Aadhaar, phone numbers, addresses, dates of birth -- `@Query` values built with string concatenation (SQL/JPQL injection) -- Sensitive fields stored without `CryptoUtil.encrypt()` / `CryptoUtil.decrypt()` -- `System.out.println` or `e.printStackTrace()` in production code - -### 3. Code Quality -- Field injection (`@Autowired` on fields) — must be constructor injection -- `System.out.println` or `e.printStackTrace()` instead of SLF4J logger -- Empty `catch` blocks (exception swallowing) -- Generic `RuntimeException` thrown instead of domain exceptions (`ResourceNotFoundException`, etc.) -- Overly long methods (> 50 lines) or deeply nested code (> 4 levels) -- Dead code, commented-out blocks -- Naming: classes PascalCase, methods/fields camelCase, constants SCREAMING_SNAKE - -### 4. Spring Boot Best Practices -- Write methods missing `@Transactional` -- Read methods missing `@Transactional(readOnly = true)` -- `@OneToMany` / `@ManyToMany` collections using `FetchType.EAGER` (default for `@ManyToMany` is EAGER — must be overridden to LAZY) -- `@OneToMany` without `@BatchSize` or `@EntityGraph` where collections are iterated -- Returning raw entity from a controller instead of a DTO -- `new` construction of Spring beans (bypasses the container) -- `@Value` used on non-`String` fields without type conversion (silent null) - -### 5. Testing -- Every public service method must have at least one test -- Tests must use JUnit 5 (`@ExtendWith(MockitoExtension.class)`) — not JUnit 4 `@RunWith` -- `@Mock` for dependencies, `@InjectMocks` for the class under test -- AAA pattern: Arrange / Act / Assert clearly separated by blank lines -- `@DisplayName` on test class and every test method -- Happy path + not-found path + exception path coverage per method -- Assertions must be specific — `assertNotNull` alone is a warning -- `verify()` to confirm side effects (saves, deletes) -- No `Thread.sleep`, no real PII in fixtures - -### 6. Performance -- N+1 queries: iterating a collection and calling a repo method per element -- Missing `@Transactional` on multi-step writes (separate commits on failure) -- `findAll()` on a table without a `WHERE` clause on large tables -- `String` concatenation in loops (use `StringBuilder`) -- Repeated `.getSessionObject()` calls inside loops instead of one call outside - ---- - -## Report Format — Single File - -``` -## Code Review — - -### Summary -<2–3 sentences: what the code does, overall quality verdict, biggest risk> - ---- - -### 🔴 Critical Issues - -**[ARCH-1] ** — `FileName.java:<line>` -> <What is wrong and why it matters> -> Fix: <specific action> - -*(None)* — if no criticals - ---- - -### 🟡 Warnings -**[SEC-1] <Title>** — `FileName.java:<line>` -> <What is wrong> -> Fix: <specific action> - -*(None)* — if no warnings - ---- - -### 🟢 Suggestions -**[QUAL-1] <Title>** — `FileName.java:<line>` -> <What could be improved> - -*(None)* — if no suggestions - ---- - -### ✅ What's Done Well -- <Specific correct pattern observed — e.g., "Constructor injection used throughout"> -- <max 3 items> - ---- - -### Verdict - -| Dimension | Status | -|----------------------|-------------------------| -| Architecture | ✅ / 🔴 N critical / 🟡 N warnings | -| Security | ✅ / 🔴 N critical / 🟡 N warnings | -| Code Quality | ✅ / 🟡 N warnings / 🟢 N suggestions | -| Spring Boot | ✅ / 🟡 N warnings | -| Testing | ✅ / 🟡 N warnings | -| Performance | ✅ / 🟢 N suggestions | - -**APPROVED** — all dimensions pass -**APPROVED WITH COMMENTS** — only warnings/suggestions, no criticals -**CHANGES REQUESTED** — one or more critical issues present -``` - ---- - -## Report Format — Synthesized PR Report (3+ files) - -After receiving JSON from all three agents, produce: - -``` -## PR Review — <branch or PR title> - -### Summary -<3–4 sentences: what the PR does, total finding counts, merge recommendation> - ---- - -### 🔴 Critical Issues (<N total>) -<Merge all CRITICAL findings from all three agent outputs.> -**[<ID>] <title>** — `<file>:<line>` -> <description> -> Fix: <recommendation> - ---- - -### 🟡 Warnings (<N total>) -<Merge all WARNING findings from all three agent outputs.> - ---- - -### 🟢 Suggestions (<N total>) -<Merge all INFO/SUGGESTION findings.> - ---- - -### Coverage Summary (from test-reviewer) -| Class | Methods | Fully Covered | Partial | Not Covered | -|---|---|---|---|---| -| <ClassName> | N | N | N | N | - -Overall coverage: X / Y methods fully tested (Z%) - ---- - -### Verdict - -| Dimension | Agent | Status | -|----------------|---------------------|----------------| -| Security | security-auditor | 🔴 / 🟡 / ✅ | -| Architecture | architecture-reviewer | 🔴 / 🟡 / ✅ | -| Testing | test-reviewer | 🟡 / ✅ | - -**APPROVED** / **APPROVED WITH COMMENTS** / **CHANGES REQUESTED** -``` - ---- - -## Finding ID Prefixes - -| Prefix | Dimension | -|---|---| -| `ARCH-n` | Architecture & Layering | -| `SEC-n` | Security | -| `QUAL-n` | Code Quality | -| `BOOT-n` | Spring Boot Best Practice | -| `TEST-n` | Testing | -| `PERF-n` | Performance | - ---- - -## Verdict Rules - -| Condition | Verdict | -|---|---| -| Zero criticals, zero warnings | APPROVED | -| Zero criticals, one or more warnings or suggestions | APPROVED WITH COMMENTS | -| One or more criticals | CHANGES REQUESTED | - ---- - -## Reference Files - -- `references/review-checklist.md` — per-class-type checklist -- `references/common-issues.md` — 10 AMRIT anti-patterns with wrong/correct examples -- `agents/security-auditor.md` — deep security scan agent -- `agents/architecture-reviewer.md` — layering and SOLID agent -- `agents/test-reviewer.md` — test coverage and quality agent diff --git a/.claude/skills/java-code-reviewer/agents/architecture-reviewer.md b/.claude/skills/java-code-reviewer/agents/architecture-reviewer.md deleted file mode 100644 index 99527896..00000000 --- a/.claude/skills/java-code-reviewer/agents/architecture-reviewer.md +++ /dev/null @@ -1,181 +0,0 @@ -# Architecture Reviewer Agent - -You are an architecture reviewer for the AMRIT / FLW-API Java Spring Boot project. Verify that code respects the AMRIT layered architecture, SOLID principles, and dependency direction rules. Return all findings as structured JSON only. - -## AMRIT Architecture Model - -The mandatory dependency direction is: - -``` -Controller → Service Interface → Service Impl → Repository → Entity - ↕ - Mapper (DTO ↔ Entity) -``` - -**Nothing in a lower layer imports from a higher layer.** - -### Layer Identification Rules - -Determine which layer a file belongs to from its package: - -| Package pattern | Layer | -|---|---| -| `com.iemr.flw.controller` | CONTROLLER | -| `com.iemr.flw.service` (interface) | SERVICE_INTERFACE | -| `com.iemr.flw.service.impl` | SERVICE_IMPL | -| `com.iemr.flw.repo.iemr` or `repo.identity` | REPOSITORY | -| `com.iemr.flw.domain.iemr` or `domain.identity` | ENTITY | -| `com.iemr.flw.dto` | DTO | -| `com.iemr.flw.mapper` | MAPPER | -| `com.iemr.flw.config` | CONFIG | -| `com.iemr.flw.utils` | UTIL | - ---- - -## Checks to Perform - -### CHECK-1: Business Logic in Controller -A controller method contains business logic if it has any of: -- `if/else` beyond a null-guard on the return value -- Arithmetic, date calculations, or status derivation -- Direct calls to a `@Repository` or `JpaRepository` bean -- A `new Entity()` construction with field assignment - -Flag: file, line number, the specific line(s) of logic found. - -Severity: **CRITICAL** - -### CHECK-2: Direct Repository Import or Call in Controller -If a controller class imports any class from `repo.iemr` or `repo.identity`, or calls a method on a repository bean, that is a direct layering violation. - -Severity: **CRITICAL** - -### CHECK-3: JPA Entity Returned from Controller -If a controller method's return type includes an entity class (i.e., a type from `domain.iemr` or `domain.identity`), or if a `ResponseEntity<EntityClass>` is declared, flag it. - -Severity: **CRITICAL** - -### CHECK-4: Field Injection Instead of Constructor Injection -Scan for `@Autowired` applied directly to a field (not a constructor or setter). This is the `@Autowired\n private SomeType field;` pattern. - -Also flag: `@Autowired` setter methods (warn, prefer constructor). - -Severity: **WARNING** - -### CHECK-5: Service Implementation Importing Spring MVC Types -A `service.impl` class that imports `jakarta.servlet.http.HttpServletRequest`, `jakarta.servlet.http.HttpServletResponse`, `org.springframework.web.context.request.WebRequest`, or `org.springframework.mock.web.*` has a dependency pointing upward toward the web layer. - -Severity: **WARNING** - -### CHECK-6: Entity Containing Business Logic -Flag any entity class that: -- Calls a Spring bean (would require `@Autowired` inside entity — also a separate violation) -- Implements a non-trivial `@PrePersist` / `@PreUpdate` lifecycle method (> 3 lines, contains conditionals) -- Has a method with business semantics (not just getters/setters/`equals`/`hashCode`/`toString`) - -Severity: **WARNING** - -### CHECK-7: SOLID — Single Responsibility -Flag any service implementation method that: -- Exceeds 50 lines -- Performs more than one distinct business operation (e.g., saves record AND sends notification AND updates another entity — should be decomposed) - -Report the method name and line count. - -Severity: **WARNING** - -### CHECK-8: SOLID — Interface Segregation -Flag any service interface with more than 15 declared methods. Large interfaces force implementors to implement methods they don't need and are a sign the interface should be split. - -Severity: **WARNING** - -### CHECK-9: SOLID — Dependency Inversion -Flag any class that instantiates a concrete service or repository implementation with `new`: -```java -SomeServiceImpl svc = new SomeServiceImpl(...); -``` -Spring-managed beans must be injected, not constructed inline. - -Severity: **WARNING** - -### CHECK-10: Mapper Calling a Service or Repository -A mapper class in `com.iemr.flw.mapper` must not import or invoke any service or repository bean. Mappers are pure transformation utilities. - -Severity: **WARNING** - -### CHECK-11: Wrong Package Placement -Flag files that exist in a package inconsistent with their role: -- A class annotated `@Service` in a `controller` package -- A class annotated `@RestController` in a `service` package -- A `JpaRepository` in a `service` or `controller` package - -Severity: **WARNING** - -### CHECK-12: Circular Service Dependencies -Flag if Service A injects Service B which injects Service A (detected from `@Autowired` / constructor signatures). Spring will fail to start with a circular dependency error. - -Severity: **CRITICAL** - ---- - -## Output Format - -Return **only** valid JSON. No prose before or after the JSON object. - -```json -{ - "auditTarget": "<filename(s) or PR title>", - "summary": { - "criticalCount": 0, - "warningCount": 0, - "infoCount": 0, - "totalChecks": 12, - "passedChecks": 0, - "layeringViolations": 0, - "solidViolations": 0 - }, - "findings": [ - { - "id": "ARCH-1", - "severity": "CRITICAL", - "check": "CHECK-1", - "category": "BUSINESS_LOGIC_IN_CONTROLLER", - "file": "com/iemr/flw/controller/CoupleController.java", - "line": 55, - "affectedLayer": "CONTROLLER", - "title": "Business logic in controller: age validation", - "description": "Lines 55–60 implement age validation (if husbandAge < 18) directly in the controller method saveEligibleCouple(). Controllers must delegate all validation beyond @Valid to the service layer.", - "codeSnippet": "if (request.getHusbandAge() < 18 || request.getWifeAge() < 18) {\n return ResponseEntity.badRequest()...\n}", - "recommendation": "Move the age validation to EligibleCoupleServiceImpl. The controller should call coupleService.save(request) and return the result." - } - ], - "passedChecks": [ - "CHECK-2: No direct repository imports found in any controller", - "CHECK-3: No entity types found in controller return signatures", - "CHECK-12: No circular service dependencies detected" - ] -} -``` - -### Severity Values -- `"CRITICAL"` — direct layering violation, circular dependency, entity exposed at API boundary -- `"WARNING"` — SOLID violation, wrong injection style, wrong package, upward dependency -- `"INFO"` — style or naming concern with no correctness impact - -### Category Values -`BUSINESS_LOGIC_IN_CONTROLLER`, `DIRECT_REPO_IN_CONTROLLER`, `ENTITY_FROM_CONTROLLER`, `FIELD_INJECTION`, `UPWARD_DEPENDENCY`, `BUSINESS_LOGIC_IN_ENTITY`, `SOLID_SINGLE_RESPONSIBILITY`, `SOLID_INTERFACE_SEGREGATION`, `SOLID_DEPENDENCY_INVERSION`, `MAPPER_CALLS_SERVICE`, `WRONG_PACKAGE`, `CIRCULAR_DEPENDENCY`, `OTHER` - -### affectedLayer Values -`CONTROLLER`, `SERVICE_INTERFACE`, `SERVICE_IMPL`, `REPOSITORY`, `ENTITY`, `MAPPER`, `DTO`, `CONFIG`, `UTIL` - ---- - -## Execution Instructions - -1. For each provided file, determine its layer from the package name using the table above. -2. Run all 12 checks applicable to that layer. -3. For each issue found: one entry in `findings[]` with a specific file path and line number. -4. For each check that passes cleanly across all files: one brief entry in `passedChecks[]`. -5. Never hallucinate — only report what can be pointed to in the actual code. -6. Count totals accurately in `summary`. -7. Return only the JSON object — no explanation, no markdown fences, no surrounding text. diff --git a/.claude/skills/java-code-reviewer/agents/security-auditor.md b/.claude/skills/java-code-reviewer/agents/security-auditor.md deleted file mode 100644 index 8d5e52ee..00000000 --- a/.claude/skills/java-code-reviewer/agents/security-auditor.md +++ /dev/null @@ -1,145 +0,0 @@ -# Security Auditor Agent - -You are a security auditor for the AMRIT / FLW-API Java Spring Boot project. Perform a deep security scan of the provided code and return findings as structured JSON only. - -## Project Security Context - -This project does NOT use Spring Security method-level annotations (`@PreAuthorize`, `@Secured`). The current auth model is: -- `JwtUserIdValidationFilter` — validates JWT from cookie (`Jwttoken`) or header (`JwtToken`); returns 401 if missing/invalid -- `HTTPRequestInterceptor` — validates the `Authorization` token exists in Redis via `Validator.checkKeyExists()` -- Skipped paths: `/user/userAuthenticate`, `/swagger-ui/**`, `/v3/api-docs/**`, `/public/**`, `/health`, `/version` -- Role data lives in `v_userservicerolemapping` via `UserServiceRoleRepo` — but role enforcement is **not** currently wired into the request pipeline - -Evaluate security relative to this actual model — do not flag missing `@PreAuthorize` as a finding unless Spring Security has been added. DO flag endpoints that lack `@RequestMapping(headers = "Authorization")` or `@RequestHeader("Authorization")`, as that bypasses the interceptor chain. - ---- - -## Checks to Perform - -### CHECK-1: Missing Authorization Header Declaration -Every controller that handles protected data must declare the authorization requirement at the class or method level. Flag endpoints where: -- The `@RequestMapping` on the class does not include `headers = "Authorization"` -- AND the method does not declare `@RequestHeader("Authorization") String Authorization` -- AND the path is not in the public skip list - -Severity: **CRITICAL** - -### CHECK-2: Missing @Valid on @RequestBody -Every mutation endpoint (`@PostMapping`, `@PutMapping`, `@PatchMapping`) that accepts a `@RequestBody` must have `@Valid` on that parameter. Without it, all Bean Validation annotations on the DTO are silently bypassed. - -Severity: **CRITICAL** - -### CHECK-3: Path Variables / Request Params Without Constraints -`@PathVariable` and `@RequestParam` values accepting IDs must use `@Positive` or `@Min(1)` to prevent negative/zero IDs from reaching the DB. - -Severity: **WARNING** - -### CHECK-4: Hardcoded Credentials, Tokens, or Keys -Scan for: -- `password`, `secret`, `token`, `apiKey`, `api_key`, `key` assigned a string literal (not `${...}`) -- Long Base64-looking strings (> 20 chars) assigned to a field -- Connection strings with embedded credentials -- Property files committing real values for `password`, `token`, `secret` keys (not `<placeholder>` or `${ENV_VAR}`) - -Severity: **CRITICAL** - -### CHECK-5: PII or Health Data in Log Statements -Scan every `log.info`, `log.debug`, `log.warn`, `log.error`, `logger.info`, `logger.debug`, `logger.warn`, `logger.error`, and `System.out.println` statement for: -- Field names or variables suggesting PII: `name`, `aadhaarNo`, `aadhaar`, `phoneNo`, `phone`, `mobile`, `address`, `dob`, `dateOfBirth`, `motherName`, `fatherName` -- Health data: `diagnosis`, `tbResult`, `hivStatus`, `bloodGroup` - -Flag the log statement, the field/variable name referenced, and the file+line. - -Severity: **CRITICAL** (health data in a patient-facing system is a regulatory breach) - -### CHECK-6: SQL Injection via String Concatenation -Scan `@Query` annotation values for `+` concatenation or `String.format()` used to build the query string. Also scan for `EntityManager.createNativeQuery(string)` where `string` contains a variable. - -Severity: **CRITICAL** - -### CHECK-7: Plain Text Password Storage -Scan for `passwordEncoder` absent where user passwords are being set/stored. Also flag MD5, SHA-1, SHA-256 (without bcrypt) used as password hashing. - -Severity: **CRITICAL** - -### CHECK-8: Sensitive Fields Stored Without Encryption -In service implementations: flag any `.set<SensitiveField>(rawValue)` where the field name suggests sensitivity (`aadhaarNo`, `phoneNo`, `address`) and the value is not wrapped in `cryptoUtil.encrypt(...)` or `CryptoUtil.encrypt(...)`. - -Severity: **WARNING** (would be CRITICAL if CryptoUtil usage is established project standard) - -### CHECK-9: System.out / printStackTrace in Production Code -Flag any `System.out.println`, `System.err.println`, or `e.printStackTrace()` in files under `src/main/`. - -Severity: **WARNING** - -### CHECK-10: Sensitive Data in Response DTOs -Flag controller methods that return types or response DTOs containing fields like `aadhaarNo`, `password`, `rawPassword`, `secretKey`, or similar — fields that should never be serialized to an API response. - -Severity: **CRITICAL** - ---- - -## Output Format - -Return **only** valid JSON. No prose before or after the JSON object. - -```json -{ - "auditTarget": "<filename(s) or PR title>", - "summary": { - "criticalCount": 0, - "warningCount": 0, - "infoCount": 0, - "totalChecks": 10, - "passedChecks": 0 - }, - "findings": [ - { - "id": "SEC-1", - "severity": "CRITICAL", - "check": "CHECK-2", - "category": "INPUT_VALIDATION", - "file": "com/iemr/flw/controller/CbacController.java", - "line": 34, - "title": "Missing @Valid on @RequestBody", - "description": "The POST /saveCbacDetails endpoint accepts CbacDetailsRequest without @Valid. Bean Validation annotations on the DTO are not enforced. A null beneficiaryId will cause a NullPointerException in the service layer.", - "codeSnippet": "@PostMapping(\"/saveCbacDetails\")\npublic ResponseEntity<?> saveCbacDetails(@RequestBody CbacDetailsRequest request) {", - "recommendation": "Change to: @Valid @RequestBody CbacDetailsRequest request", - "owaspCategory": "A04:2021 Insecure Design" - } - ], - "passedChecks": [ - "CHECK-1: All controllers declare headers = \"Authorization\" at class level", - "CHECK-6: No string concatenation found in @Query values" - ] -} -``` - -### Severity Values -- `"CRITICAL"` — exploitable, data breach risk, or regulatory violation -- `"WARNING"` — weakens security posture, violates project standards -- `"INFO"` — informational observation, no immediate risk - -### Category Values -`MISSING_AUTH_HEADER`, `INPUT_VALIDATION`, `HARDCODED_SECRET`, `PII_LOGGING`, `SQL_INJECTION`, `WEAK_CRYPTO`, `PLAIN_TEXT_PASSWORD`, `MISSING_ENCRYPTION`, `SENSITIVE_EXPOSURE`, `DEBUG_OUTPUT`, `OTHER` - -### OWASP Top 10 2021 Reference -- A01: Broken Access Control → CHECK-1 -- A02: Cryptographic Failures → CHECK-7, CHECK-8 -- A03: Injection → CHECK-6 -- A04: Insecure Design → CHECK-2, CHECK-3, CHECK-10 -- A05: Security Misconfiguration → CHECK-9 -- A07: Identification and Authentication Failures → CHECK-1 -- A09: Security Logging and Monitoring Failures → CHECK-5 - ---- - -## Execution Instructions - -1. Read every `.java` file provided. -2. Run all 10 checks against every file. -3. For each issue found: one entry in `findings[]` with specific file path and line number. -4. For each check that passes cleanly on all files: one entry in `passedChecks[]`. -5. Never hallucinate a finding — only report what can be pointed to in the actual code. -6. Count totals accurately in `summary`. -7. Return only the JSON object — no explanation, no markdown fences. diff --git a/.claude/skills/java-code-reviewer/agents/test-reviewer.md b/.claude/skills/java-code-reviewer/agents/test-reviewer.md deleted file mode 100644 index 5fca8cbe..00000000 --- a/.claude/skills/java-code-reviewer/agents/test-reviewer.md +++ /dev/null @@ -1,226 +0,0 @@ -# Test Reviewer Agent - -You are a test quality reviewer for the AMRIT / FLW-API Java Spring Boot project. Map test coverage per service method, assess test quality across multiple dimensions, and return findings as structured JSON only. - -## Project Test Context - -- Framework: **JUnit 5** (`@ExtendWith(MockitoExtension.class)`) — not JUnit 4 -- Mocking: **Mockito** (`@Mock`, `@InjectMocks`) -- Pattern: **AAA** — Arrange / Act / Assert -- Style: `@DisplayName` on every class and method describing behavior -- Test class naming convention: `<ProductionClass>Test` in the mirrored package under `src/test/` - ---- - -## Phase 1: Coverage Mapping - -For every **public method** in each changed service implementation (classes in `service.impl`): - -1. Locate the corresponding test class: same class name + `Test` suffix in `src/test/`. -2. For each production method, scan all `@Test` methods in the test class to determine which ones exercise it (the method name or a `when(...)` stub for it appears in the test body). -3. Classify coverage: - -| Coverage | Definition | -|---|---| -| `COVERED_FULL` | Happy-path test + at least one failure/not-found test + at least one exception test all present | -| `HAPPY_PATH_ONLY` | At least one test calls this method but no failure/exception path is tested | -| `MISSING` | No test method calls this production method at all | - -4. If no test class exists for a production class, mark **all** its public methods as `MISSING`. - ---- - -## Phase 2: Quality Checks - -For every `@Test` method in the test class, run these checks: - -### CHECK-T1: JUnit Version -`@ExtendWith(MockitoExtension.class)` must be present on the test class. Flag `@RunWith(MockitoJUnitRunner.class)` or `@RunWith(SpringRunner.class)` as JUnit 4 usage. - -Severity: **WARNING** - -### CHECK-T2: @DisplayName on Test Class -The class must have `@DisplayName("...")`. Content should describe the class under test in plain English, e.g. `"AncServiceImpl"` or `"ANC Care Service"`. - -Severity: **INFO** - -### CHECK-T3: @DisplayName on Every Test Method -Each `@Test` method must have `@DisplayName("should <behaviour> when <condition>")`. Flag any method missing it or using only the method name pattern like `testSaveAnc()`. - -Severity: **WARNING** - -### CHECK-T4: AAA Pattern -Each test method should have exactly three logical sections: -- **Arrange**: mock setup (`when(...).thenReturn(...)`, test data creation) -- **Act**: single call to the method under test -- **Assert**: assertion(s) on the result and/or `verify(...)` calls - -Flag tests where: -- Assertions appear before the Act call -- Multiple Act calls exist in one test (likely testing two things) -- Arrange and Assert are interleaved with no clear separation - -Severity: **WARNING** - -### CHECK-T5: Mock Setup -- All dependencies injected into the class under test must be declared with `@Mock` -- The class under test must be declared with `@InjectMocks` -- Flag: `Mockito.mock(SomeClass.class)` called inside a test body (not a field) when `@Mock` would suffice -- Flag: `@Spy` used where `@Mock` is appropriate (and no partial mock is needed) - -Severity: **WARNING** - -### CHECK-T6: Failure Path Coverage -For every public method that calls `orElseThrow(...)` or has a `findBy...` repository call, check that there is a test exercising the path where the repository returns `Optional.empty()` or an empty list. - -The test should use `assertThrows(ResourceNotFoundException.class, () -> ...)`. - -Severity: **WARNING** - -### CHECK-T7: Exception Message Assertion -When `assertThrows` is used, the test should also assert the exception message: -```java -Exception ex = assertThrows(ResourceNotFoundException.class, () -> service.getById(99L)); -assertThat(ex.getMessage()).contains("99"); -``` - -Flag tests that only assert the exception type without checking the message. - -Severity: **INFO** - -### CHECK-T8: Weak Assertions -Flag test methods where the only assertion is: -- `assertNotNull(result)` — passes even if result has all null fields -- `assertTrue(result != null)` — same problem -- `assertEquals(result, result)` — tautology - -These must be supplemented with field-level assertions (`assertEquals(expected, result.getField())`). - -Severity: **WARNING** - -### CHECK-T9: Side Effect Verification -When the production method calls `repository.save(...)`, `repository.delete(...)`, or any void method with a side effect, the test should have a `verify(mockRepo).save(any())` or similar call. Flag tests that call a write method but never `verify` it. - -Severity: **WARNING** - -### CHECK-T10: Thread.sleep -Any `Thread.sleep(...)` in a test is an anti-pattern — it makes tests slow and flaky. Flag it. - -Severity: **CRITICAL** - -### CHECK-T11: Real PII in Fixtures -Scan string literals in test files for: -- Patterns matching 12-digit Aadhaar numbers (e.g. `"123456789012"`) -- Indian phone number patterns (`"9876543210"`, `"+919876543210"`) -- Strings that look like real person names combined with health data - -Severity: **CRITICAL** - -### CHECK-T12: Real Database or Redis Access -Flag `@SpringBootTest`, `@DataJpaTest` without `replace = Replace.ANY`, or any test that does not mock its repository dependencies (i.e., uses real `JpaRepository` calls). Service unit tests must not touch the DB. - -Severity: **CRITICAL** - ---- - -## Output Format - -Return **only** valid JSON. No prose before or after the JSON object. - -```json -{ - "auditTarget": "<filename(s) or PR title>", - "coverageSummary": { - "totalProductionMethods": 0, - "fullyCovered": 0, - "happyPathOnly": 0, - "missing": 0, - "coveragePercent": 0.0 - }, - "coverageMap": [ - { - "productionClass": "com.iemr.flw.service.impl.AncServiceImpl", - "method": "saveAncCare(AncCareRequest)", - "coverage": "COVERED_FULL", - "testClass": "com.iemr.flw.service.impl.AncServiceImplTest", - "testMethods": [ - "should save ANC record and return mapped response", - "should throw ResourceNotFoundException when beneficiary not found", - "should throw InvalidOperationException when gestational age is out of range" - ] - }, - { - "productionClass": "com.iemr.flw.service.impl.AncServiceImpl", - "method": "getAllByBeneficiary(Long)", - "coverage": "MISSING", - "testClass": "com.iemr.flw.service.impl.AncServiceImplTest", - "testMethods": [] - } - ], - "qualityFindings": [ - { - "id": "TEST-1", - "severity": "WARNING", - "check": "CHECK-T3", - "category": "MISSING_DISPLAY_NAME", - "file": "com/iemr/flw/service/impl/AncServiceImplTest.java", - "line": 45, - "title": "Test method missing @DisplayName", - "description": "Method testSaveAnc() at line 45 has no @DisplayName annotation. Describe the expected behaviour, e.g. 'should save ANC record and return mapped response'.", - "recommendation": "Add @DisplayName(\"should save ANC record and return mapped response when beneficiary exists\")" - }, - { - "id": "TEST-2", - "severity": "WARNING", - "check": "CHECK-T8", - "category": "WEAK_ASSERTION", - "file": "com/iemr/flw/service/impl/AncServiceImplTest.java", - "line": 62, - "title": "Assertion too weak — assertNotNull only", - "description": "The test at line 62 only calls assertNotNull(result). This passes even if the returned AncResponse has null beneficiaryId or null visitDate. Add field-level assertions.", - "recommendation": "Add: assertEquals(request.getBeneficiaryId(), result.getBeneficiaryId()); assertEquals(request.getVisitDate(), result.getVisitDate());" - } - ], - "qualitySummary": { - "totalTestMethods": 0, - "criticalIssues": 0, - "warningIssues": 0, - "infoIssues": 0, - "passedChecks": [] - } -} -``` - -### Severity Values -- `"CRITICAL"` — test uses real PII, accesses real DB/Redis, or `Thread.sleep` present -- `"WARNING"` — missing coverage path, weak assertions, JUnit 4 usage, no `verify()` on side effects -- `"INFO"` — `@DisplayName` missing, minor style issue - -### Coverage Values -`"COVERED_FULL"`, `"HAPPY_PATH_ONLY"`, `"MISSING"` - -### Category Values -`MISSING_DISPLAY_NAME`, `POOR_ASSERTION`, `WEAK_ASSERTION`, `MISSING_FAILURE_PATH`, `MISSING_EXCEPTION_ASSERTION`, `WRONG_MOCK_SETUP`, `JUNIT4_ANNOTATION`, `THREAD_SLEEP`, `PRODUCTION_DATA_IN_FIXTURE`, `MISSING_VERIFY`, `REAL_DB_ACCESS`, `AAA_VIOLATION`, `OTHER` - ---- - -## coveragePercent Formula - -``` -coveragePercent = (fullyCovered / totalProductionMethods) * 100 -``` - -Round to one decimal place. If `totalProductionMethods` is 0, set `coveragePercent` to 100.0. - ---- - -## Execution Instructions - -1. Read every changed production service implementation file (`service.impl`). -2. List all public methods (exclude `private`, `protected`, `static` helper methods). -3. Read the corresponding test file (class name + `Test`). If not found, mark all methods `MISSING`. -4. For each production method, scan all `@Test` methods to determine coverage category. -5. For each `@Test` method, run all 12 quality checks. -6. Compute coverage summary counts. -7. For quality checks that pass cleanly across all tests, add a brief entry to `passedChecks[]`. -8. Return only the JSON object — no explanation, no markdown fences, no surrounding text. diff --git a/.claude/skills/java-code-reviewer/references/common-issues.md b/.claude/skills/java-code-reviewer/references/common-issues.md deleted file mode 100644 index 5ab9898b..00000000 --- a/.claude/skills/java-code-reviewer/references/common-issues.md +++ /dev/null @@ -1,397 +0,0 @@ -# 10 Common AMRIT Anti-Patterns - -Each pattern shows the wrong code found in the wild, the correct replacement, and the rule it violates. - ---- - -## 1. Business Logic in Controller - -**Rule:** Controllers are thin. They validate input, call one service method, and return `ApiResponse`. All `if/else`, calculations, and DB access belong in the service. - -**Wrong:** -```java -@PostMapping("/saveEligibleCouple") -public ResponseEntity<?> saveEligibleCouple( - @RequestBody EligibleCoupleRequest request, - @RequestHeader("Authorization") String auth) { - - // Business logic directly in controller - if (request.getHusbandAge() < 18 || request.getWifeAge() < 18) { - return ResponseEntity.badRequest() - .body(new ApiResponse(false, "Age below minimum", null)); - } - EligibleCoupleRegister entity = new EligibleCoupleRegister(); - entity.setBeneficiaryId(request.getBeneficiaryId()); - entity.setHusbandAge(request.getHusbandAge()); - coupleRepo.save(entity); // direct repo call - return ResponseEntity.ok(new ApiResponse(true, "Saved", null)); -} -``` - -**Correct:** -```java -@PostMapping("/saveEligibleCouple") -public ResponseEntity<ApiResponse> saveEligibleCouple( - @Valid @RequestBody EligibleCoupleRequest request, - @RequestHeader("Authorization") String Authorization) { - try { - EligibleCoupleResponse saved = coupleService.saveEligibleCouple(request); - return ResponseEntity.ok(new ApiResponse(true, "Saved successfully", saved)); - } catch (Exception e) { - logger.error("Error saving eligible couple: {}", e.getMessage(), e); - return ResponseEntity.internalServerError() - .body(new ApiResponse(false, e.getMessage(), null)); - } -} -``` - ---- - -## 2. Field Injection Instead of Constructor Injection - -**Rule:** Use constructor injection. Field injection hides dependencies, prevents immutability, and breaks unit testing without a Spring context. - -**Wrong:** -```java -@Service -public class MaternalHealthServiceImpl implements MaternalHealthService { - - @Autowired - private AncCareRepo ancCareRepo; // field injection - - @Autowired - private BeneficiaryRepo beneficiaryRepo; - - @Autowired - private AncMapper ancMapper; -} -``` - -**Correct:** -```java -@Service -public class MaternalHealthServiceImpl implements MaternalHealthService { - - private final AncCareRepo ancCareRepo; - private final BeneficiaryRepo beneficiaryRepo; - private final AncMapper ancMapper; - - public MaternalHealthServiceImpl(AncCareRepo ancCareRepo, - BeneficiaryRepo beneficiaryRepo, - AncMapper ancMapper) { - this.ancCareRepo = ancCareRepo; - this.beneficiaryRepo = beneficiaryRepo; - this.ancMapper = ancMapper; - } -} -``` - -Or with Lombok: -```java -@Service -@RequiredArgsConstructor -public class MaternalHealthServiceImpl implements MaternalHealthService { - private final AncCareRepo ancCareRepo; - private final BeneficiaryRepo beneficiaryRepo; - private final AncMapper ancMapper; -} -``` - ---- - -## 3. Logging PII or Health Data - -**Rule:** Never log personally identifiable information or health data. Log entity IDs and status codes only. This is a regulatory requirement under India's DPDP Act and AMRIT's data governance policy. - -**Wrong:** -```java -logger.info("Saving ANC for beneficiary name={}, aadhaar={}, phone={}, dob={}", - beneficiary.getName(), - beneficiary.getAadhaarNo(), - beneficiary.getPhoneNo(), - beneficiary.getDob()); - -logger.debug("TB diagnosis: patient={}, result={}, testDate={}", - patient.getName(), diagnosisResult, testDate); -``` - -**Correct:** -```java -logger.debug("Saving ANC record for benefId={}", request.getBeneficiaryId()); - -logger.debug("TB diagnosis saved for benefId={}, recordId={}", - beneficiaryId, savedRecord.getId()); -``` - ---- - -## 4. Missing @Valid on Request Body - -**Rule:** Every `@RequestBody` parameter in a mutation endpoint must have `@Valid`. Without it, Bean Validation annotations on the DTO are silently ignored, allowing null/invalid data into the service layer. - -**Wrong:** -```java -@PostMapping("/saveCbacDetails") -public ResponseEntity<?> saveCbacDetails( - @RequestBody CbacDetailsRequest request, // no @Valid - @RequestHeader("Authorization") String auth) { - // request.getBeneficiaryId() could be null — NullPointerException downstream - return ResponseEntity.ok(cbacService.save(request)); -} -``` - -**Correct:** -```java -@PostMapping("/saveCbacDetails") -public ResponseEntity<ApiResponse> saveCbacDetails( - @Valid @RequestBody CbacDetailsRequest request, - @RequestHeader("Authorization") String Authorization) { - try { - return ResponseEntity.ok(new ApiResponse(true, "Saved", cbacService.save(request))); - } catch (Exception e) { - logger.error("Error saving CBAC details: {}", e.getMessage(), e); - return ResponseEntity.internalServerError() - .body(new ApiResponse(false, e.getMessage(), null)); - } -} -``` - -And the DTO must declare constraints: -```java -public class CbacDetailsRequest { - @NotNull(message = "beneficiaryId is required") - @Positive - private Long beneficiaryId; - - @NotNull(message = "providerServiceMapId is required") - private Integer providerServiceMapId; -} -``` - ---- - -## 5. Swallowing Exceptions Silently - -**Rule:** An empty `catch` block is always a bug. At minimum, log and re-throw. Prefer wrapping in a domain exception. - -**Wrong:** -```java -public String saveDeliveryOutcome(DeliveryOutcomeRequest request) { - try { - DeliveryOutcome entity = mapper.toEntity(request); - deliveryRepo.save(entity); - return "Saved"; - } catch (Exception e) { - // silent — caller never knows it failed - } - return null; -} -``` - -**Correct:** -```java -public DeliveryOutcomeResponse saveDeliveryOutcome(DeliveryOutcomeRequest request) { - try { - DeliveryOutcome entity = mapper.toEntity(request); - DeliveryOutcome saved = deliveryRepo.save(entity); - return mapper.toResponse(saved); - } catch (DataIntegrityViolationException e) { - logger.error("Duplicate delivery outcome for benefId={}", request.getBeneficiaryId(), e); - throw new DuplicateResourceException("DeliveryOutcome", "beneficiaryId", - request.getBeneficiaryId()); - } catch (Exception e) { - logger.error("Unexpected error saving delivery outcome for benefId={}", - request.getBeneficiaryId(), e); - throw new RuntimeException("Failed to save delivery outcome", e); - } -} -``` - ---- - -## 6. Returning Raw Entity from Controller - -**Rule:** JPA entities must never leave the service layer. Returning an entity from a controller exposes the DB schema, leaks lazy-loaded collections as serialization errors, and may expose sensitive fields. - -**Wrong:** -```java -@GetMapping("/getBeneficiary/{id}") -public ResponseEntity<?> getBeneficiary(@PathVariable Long id) { - BenRegDetail entity = benRepo.findById(id).orElse(null); - return ResponseEntity.ok(entity); // raw JPA entity in response -} -``` - -**Correct:** -```java -@GetMapping("/getBeneficiary/{id}") -public ResponseEntity<ApiResponse> getBeneficiary( - @PathVariable @Positive Long id, - @RequestHeader("Authorization") String Authorization) { - try { - BeneficiaryResponse response = beneficiaryService.getById(id); - return ResponseEntity.ok(new ApiResponse(true, "Fetched", response)); - } catch (Exception e) { - logger.error("Error fetching beneficiary id={}: {}", id, e.getMessage(), e); - return ResponseEntity.internalServerError() - .body(new ApiResponse(false, e.getMessage(), null)); - } -} -``` - ---- - -## 7. N+1 Query Problem - -**Rule:** Never call a repository method inside a loop over a collection. This fires one query per element (N+1 total). Use a `JOIN FETCH` or a batch query instead. - -**Wrong:** -```java -// SupervisorDashboardServiceImpl — N+1 pattern found in production -List<IncentiveActivityRecord> records = incentiveRepo.findByDistrictId(districtId); -for (IncentiveActivityRecord record : records) { - // One DB query per record to get the verifier's name - String verifierName = userServiceRoleRepo - .getUserRole(record.getVerifiedByUserId()) - .get(0) - .getName(); - record.setVerifiedByName(verifierName); -} -``` - -**Correct:** -```java -// Option 1: fetch all needed user IDs in one batch query -List<Integer> userIds = records.stream() - .map(IncentiveActivityRecord::getVerifiedByUserId) - .distinct() - .collect(Collectors.toList()); - -Map<Integer, String> userNameMap = userServiceRoleRepo.getUserNamesByIds(userIds) - .stream() - .collect(Collectors.toMap(UserServiceRoleDTO::getUserId, UserServiceRoleDTO::getName)); - -records.forEach(r -> r.setVerifiedByName(userNameMap.get(r.getVerifiedByUserId()))); - -// Option 2: use a JOIN in the original query to fetch the name alongside the record -@Query("SELECT new com.iemr.flw.dto.IncentiveWithVerifierDTO(i, u.name) " + - "FROM IncentiveActivityRecord i JOIN UserServiceRole u ON u.userId = i.verifiedByUserId " + - "WHERE i.districtId = :districtId AND u.userServciceRoleDeleted = false") -List<IncentiveWithVerifierDTO> findWithVerifierByDistrict(@Param("districtId") Integer districtId); -``` - ---- - -## 8. Missing @Transactional on Write Methods - -**Rule:** Any service method that performs two or more DB writes must be `@Transactional`. Without it, if the second write fails, the first is already committed — leaving data in a corrupted partial state. - -**Wrong:** -```java -public String saveAncAndUpdateBeneficiary(AncRequest request) { - // Two writes — no transaction. If second fails, first is committed. - ancCareRepo.save(ancMapper.toEntity(request)); - beneficiaryRepo.updateAncStatus(request.getBeneficiaryId(), "ANC_REGISTERED"); - return "Saved"; -} -``` - -**Correct:** -```java -@Transactional -public AncResponse saveAncAndUpdateBeneficiary(AncRequest request) { - AncCare saved = ancCareRepo.save(ancMapper.toEntity(request)); - beneficiaryRepo.updateAncStatus(request.getBeneficiaryId(), "ANC_REGISTERED"); - // Both roll back atomically if either fails - return ancMapper.toResponse(saved); -} -``` - -Also apply `@Transactional(readOnly = true)` to pure read methods: -```java -@Transactional(readOnly = true) -public List<AncResponse> getAllByBeneficiary(Long beneficiaryId) { - return ancCareRepo.findByBeneficiaryIdAndDeletedFalse(beneficiaryId) - .stream().map(ancMapper::toResponse).collect(Collectors.toList()); -} -``` - ---- - -## 9. Hardcoded Secrets or Credentials - -**Rule:** No credentials, API keys, tokens, or connection strings in source code. Use `@Value` referencing property keys that themselves reference environment variables in production. - -**Wrong:** -```java -private static final String TM_API_KEY = "amrit-tm-key-20240101"; -private static final String FHIR_TOKEN = "Bearer eyJhbGciOiJSUzI1NiJ9..."; - -// In properties file committed to repo: -// spring.datasource.password=Admin@123 -// fhir.api.token=Bearer eyJhbGciOiJSUzI1NiJ9... -``` - -**Correct:** -```java -@Value("${tm.api.key}") -private String tmApiKey; - -@Value("${fhir.api.token}") -private String fhirToken; -``` - -In `common_example.properties` (the committed template): -```properties -tm.api.key=<set-in-environment> -fhir.api.token=<set-in-environment> -spring.datasource.password=<set-in-environment> -``` - -In production deployment, values come from environment variables or a secrets manager — never from committed property files. - ---- - -## 10. Throwing Generic RuntimeException Instead of Custom Exceptions - -**Rule:** Use domain exceptions that communicate intent and map to the correct HTTP status. Raw `RuntimeException` gives a generic 500 to every caller regardless of the actual problem. - -**Wrong:** -```java -public AncResponse getAncById(Long id) { - AncCare record = ancCareRepo.findById(id) - .orElseThrow(() -> new RuntimeException("Not found")); // maps to 500 - return ancMapper.toResponse(record); -} - -public void validateAge(int age) { - if (age < 10 || age > 60) { - throw new RuntimeException("Invalid age"); // generic — no HTTP mapping - } -} -``` - -**Correct:** -```java -public AncResponse getAncById(Long id) { - AncCare record = ancCareRepo.findByIdAndDeletedFalse(id) - .orElseThrow(() -> new ResourceNotFoundException("AncCare", "id", id)); // 404 - return ancMapper.toResponse(record); -} - -public void validateAge(int age) { - if (age < 10 || age > 60) { - throw new InvalidOperationException( - "Age must be between 10 and 60, received: " + age); // 422 - } -} -``` - -Custom exception hierarchy: -``` -ResourceNotFoundException → 404 Not Found -DuplicateResourceException → 409 Conflict -InvalidOperationException → 422 Unprocessable Entity -``` - -All handled centrally by `GlobalExceptionHandler` — controllers never catch these. diff --git a/.claude/skills/java-code-reviewer/references/review-checklist.md b/.claude/skills/java-code-reviewer/references/review-checklist.md deleted file mode 100644 index 3ab4fbcf..00000000 --- a/.claude/skills/java-code-reviewer/references/review-checklist.md +++ /dev/null @@ -1,157 +0,0 @@ -# Per-Class Review Checklist — AMRIT / FLW-API - -Use this checklist when reviewing each file. Mark each item: ✅ PASS / ❌ FAIL / ➖ N/A - ---- - -## Controller - -> Package: `com.iemr.flw.controller` -> Naming: `<Domain>Controller` - -| # | Item | Severity if Failed | -|---|---|---| -| C-01 | Class annotated `@RestController` and `@RequestMapping(value = "/<path>", headers = "Authorization")` | 🟡 | -| C-02 | All dependencies injected via constructor — no `@Autowired` on fields | 🟡 | -| C-03 | Every method that accepts `@RequestBody` has `@Valid` on that parameter | 🔴 | -| C-04 | Every `@PathVariable` / `@RequestParam` accepting an ID has `@Positive` or `@Min(1)` | 🟡 | -| C-05 | Every method declares `@RequestHeader("Authorization") String Authorization` | 🟡 | -| C-06 | **Zero business logic** in the method body — at most: call one service method, return `ApiResponse` | 🔴 | -| C-07 | No direct repository imports or calls | 🔴 | -| C-08 | No JPA entity type in return signature — only DTOs or `ApiResponse<DTO>` | 🔴 | -| C-09 | Returns `ResponseEntity<ApiResponse<T>>` consistently (not `ResponseEntity<?>` or raw type) | 🟡 | -| C-10 | HTTP method semantics: `@PostMapping` for writes, `@GetMapping` for reads | 🟡 | -| C-11 | SLF4J logger: `private final Logger logger = LoggerFactory.getLogger(...)` | 🟡 | -| C-12 | No `System.out.println` or `e.printStackTrace()` | 🟡 | -| C-13 | No PII logged (no name, Aadhaar, phone, address in log statements) | 🔴 | -| C-14 | `catch` block logs the exception with `logger.error(...)` and returns an error `ApiResponse` — not empty | 🔴 | -| C-15 | `@Operation(summary = "...")` present for each endpoint (Swagger) | 🟢 | -| C-16 | No hardcoded String values used as role names or status codes — use constants | 🟢 | - ---- - -## Service Interface - -> Package: `com.iemr.flw.service` -> Naming: `<Domain>Service` - -| # | Item | Severity if Failed | -|---|---|---| -| S-01 | File is an `interface`, not a class | 🔴 | -| S-02 | Lives in `service/` package (not `service/impl/`) | 🟡 | -| S-03 | Name is `<Domain>Service` — not `I<Domain>Service` or `<Domain>ServiceInterface` | 🟢 | -| S-04 | All method parameters and return types are DTOs — never JPA entities | 🔴 | -| S-05 | No Spring annotations on the interface (`@Service`, `@Transactional`, etc.) | 🟡 | -| S-06 | No `throws Exception` — declare specific checked exceptions if any | 🟢 | - ---- - -## Service Implementation - -> Package: `com.iemr.flw.service.impl` -> Naming: `<Domain>ServiceImpl` - -| # | Item | Severity if Failed | -|---|---|---| -| SI-01 | Annotated `@Service` | 🟡 | -| SI-02 | Lives in `service/impl/` package | 🟡 | -| SI-03 | Declares `implements <Domain>Service` | 🔴 | -| SI-04 | All dependencies injected via constructor — no `@Autowired` on fields | 🟡 | -| SI-05 | Every write method annotated `@Transactional` | 🔴 | -| SI-06 | Every read-only method annotated `@Transactional(readOnly = true)` | 🟡 | -| SI-07 | No empty `catch` blocks — every catch logs and re-throws or wraps | 🔴 | -| SI-08 | Throws domain exceptions (`ResourceNotFoundException`, `DuplicateResourceException`, `InvalidOperationException`) — not raw `RuntimeException` | 🟡 | -| SI-09 | SLF4J logger declared `private static final Logger log = LoggerFactory.getLogger(...)` | 🟡 | -| SI-10 | Debug log on method entry with non-PII parameters (IDs only) | 🟢 | -| SI-11 | Error log includes exception object as last arg: `log.error("msg id={}", id, e)` | 🟡 | -| SI-12 | No PII in any log statement | 🔴 | -| SI-13 | No Spring MVC imports (`HttpServletRequest`, `HttpServletResponse`) | 🟡 | -| SI-14 | No direct calls to repositories belonging to the wrong DB (iemr vs identity) | 🟡 | -| SI-15 | Sensitive fields encrypted via `CryptoUtil.encrypt()` before save | 🔴 | -| SI-16 | Sensitive fields decrypted via `CryptoUtil.decrypt()` after load | 🔴 | -| SI-17 | No method longer than 50 lines | 🟡 | -| SI-18 | No nesting deeper than 4 levels | 🟢 | - ---- - -## Repository - -> Package: `com.iemr.flw.repo.iemr` or `com.iemr.flw.repo.identity` -> Naming: `<Domain>Repo` or `<Domain>Repository` - -| # | Item | Severity if Failed | -|---|---|---| -| R-01 | Declared as `interface` extending `JpaRepository<Entity, ID>` | 🔴 | -| R-02 | Annotated `@Repository` | 🟡 | -| R-03 | Lives in `repo/iemr/` for `db_iemr` entities; `repo/identity/` for `db_identity` | 🟡 | -| R-04 | Custom queries use `@Query` with JPQL or named native SQL — never string concatenation | 🔴 | -| R-05 | Named parameters used: `@Param("name") Type name` — not positional `?1` | 🟡 | -| R-06 | Soft-delete queries filter `deleted = false` or `Deleted = 0` where applicable | 🟡 | -| R-07 | No business logic in the repository (no `if`, no loops, no service calls) | 🔴 | -| R-08 | Read-only summary queries use projections or DTO constructors, not full entity loads | 🟢 | -| R-09 | `findAll()` never called without a `WHERE` clause on large tables — use paged queries | 🟡 | - ---- - -## DTO - -> Package: `com.iemr.flw.dto.iemr` or `com.iemr.flw.dto.identity` -> Naming: `<Domain>Request` / `<Domain>Response` / `<Domain>DTO` - -| # | Item | Severity if Failed | -|---|---|---| -| D-01 | No `@Entity`, `@Table`, or JPA annotations present | 🔴 | -| D-02 | Request DTOs: all required fields annotated `@NotNull` or `@NotBlank` | 🟡 | -| D-03 | `String` fields have `@Size(max = N)` to prevent oversized input | 🟡 | -| D-04 | ID fields annotated `@Positive` or `@Min(1)` | 🟢 | -| D-05 | Date-of-birth / visit-date fields annotated `@PastOrPresent` | 🟢 | -| D-06 | Separate request DTO and response DTO — not the same class for both directions | 🟢 | -| D-07 | No circular references (DTO → DTO → same DTO) | 🟡 | -| D-08 | Sensitive fields (`aadhaarNumber`, `phoneNumber`) not included in response DTOs that go to lower-privilege clients | 🔴 | -| D-09 | Annotated `@Data` (Lombok) or has explicit getters/setters | 🟡 | - ---- - -## Entity - -> Package: `com.iemr.flw.domain.iemr` or `com.iemr.flw.domain.identity` -> Naming: Plain domain noun (e.g., `AncCare`, `ChildRegister`) - -| # | Item | Severity if Failed | -|---|---|---| -| E-01 | Annotated `@Entity` and `@Table(name = "t_<domain>", schema = "db_iemr")` | 🟡 | -| E-02 | Lives in the correct package for its database | 🟡 | -| E-03 | Primary key: `@Id` + `@GeneratedValue(strategy = GenerationType.IDENTITY)` | 🟡 | -| E-04 | Soft-delete field present: `Boolean deleted = false` or `Deleted` | 🟡 | -| E-05 | Audit fields present: `createdDate`, `createdBy`, `modifiedDate`, `modifiedBy` | 🟢 | -| E-06 | `@OneToMany` collections use `fetch = FetchType.LAZY` | 🟡 | -| E-07 | `@ManyToMany` collections override the default to `fetch = FetchType.LAZY` | 🟡 | -| E-08 | No business logic in entity (no service calls, no derived compute, no `@PrePersist` with logic) | 🔴 | -| E-09 | No Spring annotations inside entity (no `@Autowired`, `@Value`) | 🔴 | -| E-10 | `toString()` does not include sensitive fields (`aadhaarNumber`, `phoneNumber`) | 🔴 | -| E-11 | `equals()` / `hashCode()` based on `id` only — not mutable fields | 🟡 | - ---- - -## Test Class - -> Package: mirrors production package under `src/test/` -> Naming: `<ProductionClass>Test` - -| # | Item | Severity if Failed | -|---|---|---| -| T-01 | `@ExtendWith(MockitoExtension.class)` on the class — not `@RunWith(MockitoJUnitRunner.class)` | 🟡 | -| T-02 | `@DisplayName("...")` on the test class — describes the class under test | 🟢 | -| T-03 | Every dependency is `@Mock`; class under test is `@InjectMocks` | 🟡 | -| T-04 | Every `@Test` method has `@DisplayName("should <behaviour> when <condition>")` | 🟢 | -| T-05 | AAA pattern: Arrange → blank line → Act → blank line → Assert | 🟡 | -| T-06 | Happy-path test exists for every public service method | 🟡 | -| T-07 | Not-found / empty path tested with `assertThrows(ResourceNotFoundException.class, ...)` | 🟡 | -| T-08 | Exception message also asserted inside `assertThrows` — not just the exception type | 🟢 | -| T-09 | Assertions are specific: `assertEquals`, `assertThat`, etc. — not just `assertNotNull` | 🟡 | -| T-10 | `verify(mockRepo).save(any())` used when a save side-effect must be confirmed | 🟢 | -| T-11 | No `Thread.sleep` | 🔴 | -| T-12 | No real PII in test fixtures (no real Aadhaar 12-digit numbers, no real phone numbers) | 🔴 | -| T-13 | Test does not access a real database or Redis — all dependencies are mocked | 🔴 | -| T-14 | No shared mutable state between test methods without `@BeforeEach` reset | 🟡 | -| T-15 | `@BeforeEach` used for common setup — not repeated inside each test | 🟢 | diff --git a/.claude/skills/java-code-writer/SKILL.md b/.claude/skills/java-code-writer/SKILL.md deleted file mode 100644 index 76b6beb0..00000000 --- a/.claude/skills/java-code-writer/SKILL.md +++ /dev/null @@ -1,156 +0,0 @@ -# java-code-writer - -Write production-ready Spring Boot code for the AMRIT / FLW-API platform. - -## Trigger - -Use this skill when asked to: -- Add a new API endpoint or module -- Implement a new service or repository -- Scaffold a full feature (DTO → Entity → Repo → Service → Controller + Test) -- Build an integration with an external system (ERP, LIMS, SMS, notification channel, report format, etc.) - ---- - -## Output Order - -Always produce artifacts in this sequence — never skip a layer: - -1. **Request/Response DTOs** (`dto/`) -2. **JPA Entity** (`domain/iemr/` or `domain/identity/`) -3. **MapStruct Mapper** (`mapper/`) -4. **Repository interface** (`repo/iemr/` or `repo/identity/`) -5. **Service interface** (`service/`) -6. **Service implementation** (`service/impl/`) -7. **Controller** (`controller/`) -8. **Unit test** for the service impl (`src/test/…`) - -Additionally, whenever the feature involves an external integration, multiple output formats, or any behaviour with foreseeable alternative implementations, produce these artifacts **before** the Service layer: - -8a. **Provider/Strategy enum** (`integration/` or relevant package) -8b. **Provider/Strategy interface** (same package) -8c. **Concrete implementation(s)** (`integration/impl/` or `service/impl/`) -8d. **Factory `@Component`** (`integration/` or relevant package) -8e. **Resolver `@Component`** (config-driven selector) - -See `references/factory-patterns.md` for full templates. - -If an artifact already exists, show only the diff (new methods / fields). - ---- - -## Architecture Rules - -- **Controllers** are thin: `@Valid` on request body, one service call, return `ApiResponse`. No `if/else` business logic. -- **Services** own all business logic. Implement a named interface. Never import Spring MVC types. -- **Repositories** are Spring Data JPA interfaces. Custom queries use `@Query` with named parameters — no string concatenation. -- **DTOs** cross the controller ↔ service boundary. Entities never leave the service layer. -- **Mappers** use MapStruct (`@Mapper(componentModel = "spring")`). No manual field copying elsewhere. - -### Factory Pattern Rules (mandatory for integrations and multi-implementation scenarios) - -Apply the Factory pattern whenever a feature has more than one possible implementation or communicates with an external system. **Never couple a service directly to a single concrete integration class.** - -| Scenario | Factory to create | -|---|---| -| External system integration (ERP, LIMS, IoT…) | `IntegrationServiceFactory` | -| Report generation (PDF, Excel, CSV…) | `ReportGeneratorFactory` | -| Notification dispatch (email, SMS, push…) | `NotificationProviderFactory` | -| Validation strategy (rule sets per plant/lab…) | `ValidatorFactory` | -| Storage backend (DB, S3, filesystem…) | `StorageServiceFactory` | - -Rules: -- **Interface declares `getType()`** — always typed with an enum, never a raw string. -- **Factory is `@Component`** injected with `List<ProviderInterface>` — Spring collects all implementations automatically. -- **Active provider is config-driven**: `@Value("${integration.provider}")` in the Resolver, never hard-coded. -- **`InvalidOperationException`** thrown on unknown key — never return `null`. -- **A `MOCK` implementation must always exist** — local dev and unit tests must never depend on a real external system. -- **Adding a new provider = 2 files only**: new enum constant + new `@Service` class. Factory, Resolver, and callers are untouched. - -See `references/factory-patterns.md` for full boilerplate and worked examples. - ---- - -## Injection Pattern - -Always use **constructor injection**. Never use `@Autowired` on fields. - -```java -private final SomeService someService; - -public SomeController(SomeService someService) { - this.someService = someService; -} -``` - -For service implementations with multiple dependencies, generate a `@RequiredArgsConstructor` (Lombok) or explicit constructor. - ---- - -## Logging Standards - -```java -private static final Logger log = LoggerFactory.getLogger(ClassName.class); -``` - -- Log method entry at `DEBUG` with non-PII parameters only. -- Log exceptions at `ERROR` with message and cause: `log.error("Failed to send to ERP for requestId={}", requestId, e)`. -- Never log: names, Aadhaar, phone numbers, addresses, or any field tagged sensitive. -- Never use `System.out.println` or `e.printStackTrace()`. - ---- - -## Security - -- Add `@Valid` to every `@RequestBody` / `@RequestParam` in controller methods. -- Add `@PreAuthorize("hasRole('ROLE_ASHA')")` (or the appropriate AMRIT role) to every controller method. Use role constants from `masterEnum/` — never inline strings. -- Never hardcode passwords, tokens, or keys. Reference `@Value("${property.name}")` or `Environment`. -- Sensitive entity fields must be encrypted/decrypted via `CryptoUtil`. -- Integration credentials (API keys, endpoints, auth tokens for ERP/LIMS/SMS providers) must be read from `@Value` and must reference environment variables in deployed environments. - ---- - -## Validation - -Add Bean Validation annotations to DTO fields: -- `@NotNull`, `@NotBlank`, `@Size`, `@Min`, `@Max`, `@Pattern` as appropriate. -- For ID fields: `@Positive`. -- For date fields: `@PastOrPresent` for date of birth / visit dates. - ---- - -## Exception Handling - -- Throw `ResourceNotFoundException` when a record is not found (maps to 404). -- Throw `DuplicateResourceException` for unique constraint violations (maps to 409). -- Throw `InvalidOperationException` for business rule violations and for unknown factory keys (maps to 422). -- Never swallow exceptions in an empty catch block. -- Wrap checked exceptions from external integrations (`IOException`, `HttpClientErrorException`, etc.) in `InvalidOperationException` with a meaningful message and the original cause. -- See `references/exceptions.md` for exception class definitions. - ---- - -## References - -| File | Contents | -|---|---| -| `references/spring-patterns.md` | Full boilerplate for every layer (DTO → Entity → Repo → Service → Controller → Test) | -| `references/exceptions.md` | Custom exception classes and HTTP mapping table | -| `references/security-patterns.md` | Role constants, `@PreAuthorize` patterns, JWT filter, CryptoUtil usage | -| `references/factory-patterns.md` | **Factory pattern templates** — IntegrationServiceFactory, ReportGeneratorFactory, NotificationProviderFactory, unit tests | - ---- - -## Checklist Before Finalizing Output - -- [ ] Every controller method has `@Valid` and `@PreAuthorize` -- [ ] No entity is returned from a controller -- [ ] Constructor injection used throughout -- [ ] SLF4J logger declared (no System.out) -- [ ] No PII logged -- [ ] Unit test covers happy path, not-found, and exception path -- [ ] `@DisplayName` on test class and every test method -- [ ] No hardcoded secrets -- [ ] **If an integration or multi-impl scenario:** Factory + Resolver + Mock impl present -- [ ] **Provider selection is config-driven** (`application.properties` → env var), not hard-coded -- [ ] **`InvalidOperationException` thrown** (not null returned) when factory key is missing diff --git a/.claude/skills/java-code-writer/references/exceptions.md b/.claude/skills/java-code-writer/references/exceptions.md deleted file mode 100644 index ef4a2bbc..00000000 --- a/.claude/skills/java-code-writer/references/exceptions.md +++ /dev/null @@ -1,158 +0,0 @@ -# Custom Exception Classes — AMRIT / FLW-API - -## Exception Hierarchy - -``` -RuntimeException -├── ResourceNotFoundException → HTTP 404 -├── DuplicateResourceException → HTTP 409 -└── InvalidOperationException → HTTP 422 -``` - ---- - -## ResourceNotFoundException - -Throw when a requested entity does not exist or has been soft-deleted. - -```java -package com.piramal.flwapi.exception; - -public class ResourceNotFoundException extends RuntimeException { - - private final String resourceName; - private final String fieldName; - private final Object fieldValue; - - public ResourceNotFoundException(String resourceName, String fieldName, Object fieldValue) { - super(String.format("%s not found with %s: '%s'", resourceName, fieldName, fieldValue)); - this.resourceName = resourceName; - this.fieldName = fieldName; - this.fieldValue = fieldValue; - } - - public String getResourceName() { return resourceName; } - public String getFieldName() { return fieldName; } - public Object getFieldValue() { return fieldValue; } -} -``` - -**Usage:** -```java -exampleRepository.findByIdAndDeletedFalse(id) - .orElseThrow(() -> new ResourceNotFoundException("AncCare", "id", id)); -``` - ---- - -## DuplicateResourceException - -Throw when a create/update would violate a uniqueness constraint. - -```java -package com.piramal.flwapi.exception; - -public class DuplicateResourceException extends RuntimeException { - - private final String resourceName; - private final String fieldName; - private final Object fieldValue; - - public DuplicateResourceException(String resourceName, String fieldName, Object fieldValue) { - super(String.format("%s already exists with %s: '%s'", resourceName, fieldName, fieldValue)); - this.resourceName = resourceName; - this.fieldName = fieldName; - this.fieldValue = fieldValue; - } - - public String getResourceName() { return resourceName; } - public String getFieldName() { return fieldName; } - public Object getFieldValue() { return fieldValue; } -} -``` - -**Usage:** -```java -if (coupleRepository.existsByBeneficiaryIdAndDeletedFalse(request.getBeneficiaryId())) { - throw new DuplicateResourceException("EligibleCoupleRegister", "beneficiaryId", - request.getBeneficiaryId()); -} -``` - ---- - -## InvalidOperationException - -Throw when a business rule prevents the requested operation. Use for semantic validation that cannot be expressed as Bean Validation annotations. - -```java -package com.piramal.flwapi.exception; - -public class InvalidOperationException extends RuntimeException { - - public InvalidOperationException(String message) { - super(message); - } - - public InvalidOperationException(String message, Throwable cause) { - super(message, cause); - } -} -``` - -**Usage:** -```java -if (deliveryOutcome.getDeliveryDate().isBefore(ancRecord.getLmpDate())) { - throw new InvalidOperationException( - "Delivery date cannot be before last menstrual period date"); -} - -if (tbTreatment.getStatus() == TreatmentStatus.COMPLETED) { - throw new InvalidOperationException( - "Cannot modify a completed TB treatment record"); -} -``` - ---- - -## HTTP Status Mapping - -| Exception | HTTP Status | Spring `@ResponseStatus` | Use case | -|---|---|---|---| -| `ResourceNotFoundException` | 404 Not Found | `HttpStatus.NOT_FOUND` | Entity not found / soft-deleted | -| `DuplicateResourceException` | 409 Conflict | `HttpStatus.CONFLICT` | Unique constraint violation | -| `InvalidOperationException` | 422 Unprocessable Entity | `HttpStatus.UNPROCESSABLE_ENTITY` | Business rule violation | -| `MethodArgumentNotValidException` (Spring) | 400 Bad Request | `HttpStatus.BAD_REQUEST` | Bean Validation failure | -| `AccessDeniedException` (Spring Security) | 403 Forbidden | `HttpStatus.FORBIDDEN` | Missing role/permission | -| `AuthenticationException` (Spring Security) | 401 Unauthorized | `HttpStatus.UNAUTHORIZED` | Invalid/missing JWT | - -All are handled in `GlobalExceptionHandler` — see `references/spring-patterns.md` section 9. - ---- - -## Exception Handling Rules - -1. **Never swallow exceptions.** An empty `catch` block is a bug. -2. **Always log at the right level:** - - `WARN` for 4xx errors (expected bad input, not-found). - - `ERROR` for 5xx errors (unexpected failures). -3. **Never log PII** in exception messages or log statements. -4. **Do not expose stack traces** to API consumers — let `GlobalExceptionHandler` normalize the response. -5. **Wrap checked exceptions** from external libraries (e.g., `IOException` from crypto) in `InvalidOperationException` with a meaningful message. - -```java -// Wrong -try { - return cryptoUtil.decrypt(value); -} catch (Exception e) { - // swallowed! -} - -// Correct -try { - return cryptoUtil.decrypt(value); -} catch (CryptoException e) { - log.error("Failed to decrypt field for record id={}", recordId, e); - throw new InvalidOperationException("Data decryption failed", e); -} -``` diff --git a/.claude/skills/java-code-writer/references/factory-patterns.md b/.claude/skills/java-code-writer/references/factory-patterns.md deleted file mode 100644 index 4605fdde..00000000 --- a/.claude/skills/java-code-writer/references/factory-patterns.md +++ /dev/null @@ -1,364 +0,0 @@ -# Factory Design Patterns — AMRIT / FLW-API - -Use Factory patterns wherever there are **multiple implementations** of a behaviour (integrations, report formats, notification channels, validators, etc.) so future providers can be plugged in without touching existing code. - ---- - -## When to Apply a Factory - -| Scenario | Factory type to use | -|---|---| -| Multiple external integrations (SMS, ERP, LIMS…) | `IntegrationServiceFactory` | -| Multiple report formats (PDF, Excel, CSV…) | `ReportGeneratorFactory` | -| Multiple notification channels (email, push, SMS…) | `NotificationProviderFactory` | -| Multiple validators with shared interface | `ValidatorFactory` | -| Multiple storage backends (DB, S3, local…) | `StorageServiceFactory` | - ---- - -## 1. Provider / Strategy Interface - -Every variant implements a common interface that declares a **type identifier** plus the operation contract. - -```java -package com.piramal.flwapi.integration; - -/** - * Common contract for all external integration providers. - * Implement this interface for every new provider — no other class needs to change. - */ -public interface IntegrationService { - - /** - * Returns the unique provider key used to look this implementation up in - * {@link IntegrationServiceFactory}. Must be UPPER_SNAKE_CASE and match an - * {@code integration.provider} config value. - */ - IntegrationProvider getProvider(); - - /** - * Sends {@code payload} to the external system. - * - * @param payload the data to transmit (never null) - * @return the raw response from the provider - * @throws IntegrationException on any provider-side error - */ - IntegrationResponse send(IntegrationPayload payload); -} -``` - -```java -package com.piramal.flwapi.integration; - -/** Enum of all supported integration providers. Add new values here when onboarding a provider. */ -public enum IntegrationProvider { - SAP_ERP, - ORACLE_ERP, - LIMS_CONNECT, - MOCK // for tests / local dev -} -``` - ---- - -## 2. Concrete Implementations - -Each implementation is a Spring `@Service` — the factory discovers them automatically through injection. - -```java -package com.piramal.flwapi.integration.impl; - -import com.piramal.flwapi.integration.IntegrationPayload; -import com.piramal.flwapi.integration.IntegrationProvider; -import com.piramal.flwapi.integration.IntegrationResponse; -import com.piramal.flwapi.integration.IntegrationService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Service; - -@Service -public class SapErpIntegrationService implements IntegrationService { - - private static final Logger log = LoggerFactory.getLogger(SapErpIntegrationService.class); - - @Override - public IntegrationProvider getProvider() { - return IntegrationProvider.SAP_ERP; - } - - @Override - public IntegrationResponse send(IntegrationPayload payload) { - log.debug("Sending payload to SAP ERP, requestId={}", payload.getRequestId()); - // SAP-specific HTTP/RFC call here - return IntegrationResponse.ok("SAP_ACK_" + payload.getRequestId()); - } -} -``` - -```java -package com.piramal.flwapi.integration.impl; - -import com.piramal.flwapi.integration.IntegrationPayload; -import com.piramal.flwapi.integration.IntegrationProvider; -import com.piramal.flwapi.integration.IntegrationResponse; -import com.piramal.flwapi.integration.IntegrationService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Service; - -/** Stub used in local / test environments — never wired in production. */ -@Service -public class MockIntegrationService implements IntegrationService { - - private static final Logger log = LoggerFactory.getLogger(MockIntegrationService.class); - - @Override - public IntegrationProvider getProvider() { - return IntegrationProvider.MOCK; - } - - @Override - public IntegrationResponse send(IntegrationPayload payload) { - log.debug("Mock integration — swallowing payload for requestId={}", payload.getRequestId()); - return IntegrationResponse.ok("MOCK_OK"); - } -} -``` - ---- - -## 3. The Factory - -The factory is injected with **all** `IntegrationService` beans by Spring and indexes them by provider. - -```java -package com.piramal.flwapi.integration; - -import com.piramal.flwapi.exception.InvalidOperationException; -import org.springframework.stereotype.Component; - -import java.util.List; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; - -/** - * Resolves the correct {@link IntegrationService} at runtime based on a configured - * {@link IntegrationProvider} key. - * - * <p>To add a new provider: create a class that implements {@link IntegrationService}, - * annotate it with {@code @Service}, and add its key to {@link IntegrationProvider}. - * No changes needed here or anywhere else. - */ -@Component -public class IntegrationServiceFactory { - - private final Map<IntegrationProvider, IntegrationService> registry; - - /** Spring injects every IntegrationService bean in the application context. */ - public IntegrationServiceFactory(List<IntegrationService> services) { - this.registry = services.stream() - .collect(Collectors.toMap(IntegrationService::getProvider, Function.identity())); - } - - /** - * Returns the integration service for the given provider. - * - * @param provider the target provider - * @return the matching service - * @throws InvalidOperationException if no implementation is registered for the provider - */ - public IntegrationService getService(IntegrationProvider provider) { - IntegrationService service = registry.get(provider); - if (service == null) { - throw new InvalidOperationException( - "No IntegrationService registered for provider: " + provider); - } - return service; - } -} -``` - ---- - -## 4. Driving the Factory from Config - -The active provider is read from `application.properties` — switching providers is a config change, not a code change. - -```properties -# application.properties -integration.provider=SAP_ERP -``` - -```java -package com.piramal.flwapi.integration; - -import org.springframework.beans.factory.annotation.Value; -import org.springframework.stereotype.Component; - -/** - * Resolves the configured provider once on startup and caches the service. - * Inject this wherever you need to send to the external system. - */ -@Component -public class IntegrationServiceResolver { - - private final IntegrationService activeService; - - public IntegrationServiceResolver( - IntegrationServiceFactory factory, - @Value("${integration.provider}") String providerKey) { - - IntegrationProvider provider = IntegrationProvider.valueOf(providerKey); - this.activeService = factory.getService(provider); - } - - public IntegrationService get() { - return activeService; - } -} -``` - -Usage in a service: -```java -@Service -@RequiredArgsConstructor -public class SampleDispatchService { - - private final IntegrationServiceResolver integrationResolver; - - public void dispatchToErp(Long requestId) { - IntegrationPayload payload = IntegrationPayload.of(requestId); - IntegrationResponse response = integrationResolver.get().send(payload); - log.debug("ERP dispatch completed, reference={}", response.getReference()); - } -} -``` - ---- - -## 5. Report Generator Factory (second example — same pattern) - -```java -// Interface -public interface ReportGenerator { - ReportFormat getFormat(); - byte[] generate(ReportData data); -} - -// Enum -public enum ReportFormat { PDF, EXCEL, CSV } - -// Implementation example -@Service -public class PdfReportGenerator implements ReportGenerator { - @Override public ReportFormat getFormat() { return ReportFormat.PDF; } - @Override public byte[] generate(ReportData data) { /* iText / JasperReports */ return new byte[0]; } -} - -// Factory — identical structure to IntegrationServiceFactory -@Component -public class ReportGeneratorFactory { - private final Map<ReportFormat, ReportGenerator> registry; - public ReportGeneratorFactory(List<ReportGenerator> generators) { - this.registry = generators.stream() - .collect(Collectors.toMap(ReportGenerator::getFormat, Function.identity())); - } - public ReportGenerator getGenerator(ReportFormat format) { - ReportGenerator generator = registry.get(format); - if (generator == null) throw new InvalidOperationException("No generator for format: " + format); - return generator; - } -} -``` - ---- - -## 6. Notification Provider Factory (third example) - -```java -// Interface -public interface NotificationProvider { - NotificationChannel getChannel(); - void send(NotificationPayload payload); -} - -// Enum -public enum NotificationChannel { EMAIL, SMS, PUSH } - -// EMAIL implementation -@Service -public class EmailNotificationProvider implements NotificationProvider { - @Override public NotificationChannel getChannel() { return NotificationChannel.EMAIL; } - @Override public void send(NotificationPayload payload) { /* SMTP / SES */ } -} - -// SMS implementation -@Service -public class SmsNotificationProvider implements NotificationProvider { - @Override public NotificationChannel getChannel() { return NotificationChannel.SMS; } - @Override public void send(NotificationPayload payload) { /* Twilio / AWS SNS */ } -} - -// Factory -@Component -public class NotificationProviderFactory { - private final Map<NotificationChannel, NotificationProvider> registry; - public NotificationProviderFactory(List<NotificationProvider> providers) { - this.registry = providers.stream() - .collect(Collectors.toMap(NotificationProvider::getChannel, Function.identity())); - } - public NotificationProvider getProvider(NotificationChannel channel) { - NotificationProvider provider = registry.get(channel); - if (provider == null) throw new InvalidOperationException("No provider for channel: " + channel); - return provider; - } -} -``` - ---- - -## 7. Unit Testing a Factory - -```java -@DisplayName("IntegrationServiceFactory") -class IntegrationServiceFactoryTest { - - private IntegrationServiceFactory factory; - - @BeforeEach - void setUp() { - factory = new IntegrationServiceFactory( - List.of(new SapErpIntegrationService(), new MockIntegrationService())); - } - - @Test - @DisplayName("returns SAP implementation when provider is SAP_ERP") - void returnsSapService() { - IntegrationService service = factory.getService(IntegrationProvider.SAP_ERP); - assertThat(service).isInstanceOf(SapErpIntegrationService.class); - } - - @Test - @DisplayName("throws InvalidOperationException for unknown provider") - void throwsForUnknownProvider() { - // Force removal to simulate missing implementation - IntegrationServiceFactory emptyFactory = new IntegrationServiceFactory(List.of()); - assertThatThrownBy(() -> emptyFactory.getService(IntegrationProvider.SAP_ERP)) - .isInstanceOf(InvalidOperationException.class) - .hasMessageContaining("SAP_ERP"); - } -} -``` - ---- - -## Factory Pattern Rules - -- **Interface declares `getType()`** — the key method the factory indexes by. Always use an enum, never a string. -- **Factory is `@Component`**, never `@Service` — it orchestrates, it does not implement business logic. -- **Implementations are `@Service`** — Spring auto-discovers them; the factory just collects what's already in the context. -- **Config drives selection** — use `@Value("${…}")` to pick the active provider; never hard-code it. -- **`InvalidOperationException` on missing key** — fail fast with a clear message rather than returning `null`. -- **Mock / stub implementation always exists** — ensures local dev and tests never need a real external system. -- **Adding a new provider = 2 files only**: a new enum constant + a new `@Service` class. Zero changes to factory, resolver, or callers. diff --git a/.claude/skills/java-code-writer/references/security-patterns.md b/.claude/skills/java-code-writer/references/security-patterns.md deleted file mode 100644 index c16735d0..00000000 --- a/.claude/skills/java-code-writer/references/security-patterns.md +++ /dev/null @@ -1,197 +0,0 @@ -# Security Patterns — AMRIT / FLW-API - -## AMRIT Role Constants - -These roles are used in `@PreAuthorize` annotations. Always reference constants — never inline strings. - -```java -package com.piramal.flwapi.masterEnum; - -public final class AmritRoles { - private AmritRoles() {} - - public static final String ASHA = "ROLE_ASHA"; - public static final String ANM = "ROLE_ANM"; - public static final String SUPERVISOR = "ROLE_SUPERVISOR"; - public static final String MO = "ROLE_MO"; // Medical Officer - public static final String ADMIN = "ROLE_ADMIN"; - public static final String VIEWER = "ROLE_VIEWER"; -} -``` - ---- - -## @PreAuthorize Patterns - -```java -// Single role -@PreAuthorize("hasRole('ROLE_ADMIN')") - -// Multiple roles (OR) -@PreAuthorize("hasAnyRole('ROLE_ASHA', 'ROLE_ANM')") - -// Role AND ownership check (use with Principal injection) -@PreAuthorize("hasRole('ROLE_SUPERVISOR') or #userId == authentication.principal.id") - -// Method-level expression using SpEL -@PreAuthorize("hasRole('ROLE_ADMIN') and #request.districtId != null") -``` - -**Recommended per endpoint type:** - -| Endpoint type | Minimum roles | -|---|---| -| Create / Update beneficiary data | `ROLE_ASHA`, `ROLE_ANM` | -| Read beneficiary data | `ROLE_ASHA`, `ROLE_ANM`, `ROLE_SUPERVISOR` | -| Delete / soft-delete | `ROLE_SUPERVISOR`, `ROLE_ADMIN` | -| Reports / dashboards | `ROLE_SUPERVISOR`, `ROLE_MO`, `ROLE_ADMIN` | -| Admin operations | `ROLE_ADMIN` only | - ---- - -## JWT Filter (reference — already implemented) - -The `JwtUserIdValidationFilter` intercepts every request, validates the Bearer token via `JwtUtil`, and populates `SecurityContextHolder`. You do not need to re-implement this. - -```java -// JwtUtil — key methods already available -String extractUsername(String token); -boolean validateToken(String token, UserDetails userDetails); -String extractUserId(String token); -``` - -To extract the current user in a service: - -```java -// Inject via controller and pass as parameter — preferred -public ExampleResponse create(CreateExampleRequest request, String currentUser) { ... } - -// Or read from SecurityContext directly in service (acceptable) -String currentUser = SecurityContextHolder.getContext() - .getAuthentication() - .getName(); -``` - ---- - -## Spring Security Configuration Template - -```java -package com.piramal.flwapi.config; - -import com.piramal.flwapi.utils.JwtUserIdValidationFilter; -import lombok.RequiredArgsConstructor; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; -import org.springframework.security.config.annotation.web.builders.HttpSecurity; -import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; -import org.springframework.security.config.http.SessionCreationPolicy; -import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; -import org.springframework.security.crypto.password.PasswordEncoder; -import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; - -@Configuration -@EnableWebSecurity -@EnableMethodSecurity // enables @PreAuthorize -@RequiredArgsConstructor -public class SecurityConfig { - - private final JwtUserIdValidationFilter jwtFilter; - - private static final String[] PUBLIC_PATHS = { - "/actuator/health", - "/swagger-ui/**", - "/v3/api-docs/**", - "/api/v1/auth/login", - "/api/v1/auth/refresh" - }; - - @Bean - public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { - return http - .csrf(csrf -> csrf.disable()) - .sessionManagement(sm -> sm.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) - .authorizeHttpRequests(auth -> auth - .requestMatchers(PUBLIC_PATHS).permitAll() - .anyRequest().authenticated() - ) - .addFilterBefore(jwtFilter, UsernamePasswordAuthenticationFilter.class) - .build(); - } - - @Bean - public PasswordEncoder passwordEncoder() { - return new BCryptPasswordEncoder(12); - } -} -``` - ---- - -## BCrypt Usage - -```java -// Hashing (e.g., on user creation) -String hashed = passwordEncoder.encode(rawPassword); - -// Verification (e.g., on login) -boolean matches = passwordEncoder.matches(rawPassword, storedHash); -``` - -Never store or log raw passwords. Never use MD5 or SHA-1 for password hashing. - ---- - -## Sensitive Data Encryption via CryptoUtil - -Fields containing PII (Aadhaar, phone, address) must be encrypted at rest using the existing `CryptoUtil`. - -```java -// In service layer — encrypt before save -entity.setAadhaarNumber(cryptoUtil.encrypt(request.getAadhaarNumber())); - -// Decrypt on read -response.setAadhaarNumber(cryptoUtil.decrypt(entity.getAadhaarNumber())); -``` - -Never log the raw (unencrypted) value of any PII field. - ---- - -## Input Validation Security - -Rely on Bean Validation (`@Valid`) at the controller boundary. For free-text fields that may be persisted or displayed: - -```java -// DTO field — prevent XSS via length limit and pattern -@Pattern(regexp = "^[a-zA-Z0-9 .,\\-]*$", message = "Invalid characters in remarks") -@Size(max = 500) -private String remarks; -``` - -For file uploads (not currently used but may be added): -- Always validate MIME type server-side, not just client-supplied `Content-Type`. -- Store uploads outside the web root. - ---- - -## No Hardcoded Secrets - -```java -// WRONG -private static final String SECRET_KEY = "my-super-secret-123"; - -// CORRECT — read from application.properties -@Value("${jwt.secret}") -private String jwtSecret; -``` - -Secrets in `application.properties` must reference environment variables in deployed environments: -```properties -jwt.secret=${JWT_SECRET} -spring.datasource.password=${DB_PASSWORD} -``` - -Never commit `common_local.properties` with real credentials. diff --git a/.claude/skills/java-code-writer/references/spring-patterns.md b/.claude/skills/java-code-writer/references/spring-patterns.md deleted file mode 100644 index 44a44cce..00000000 --- a/.claude/skills/java-code-writer/references/spring-patterns.md +++ /dev/null @@ -1,584 +0,0 @@ -# Spring Boot Code Templates — AMRIT / FLW-API - -## 1. Request DTO - -```java -package com.piramal.flwapi.dto; - -import jakarta.validation.constraints.*; -import lombok.Data; - -@Data -public class CreateExampleRequest { - - @NotNull(message = "beneficiaryId is required") - @Positive - private Long beneficiaryId; - - @NotBlank(message = "visitType is required") - @Size(max = 100) - private String visitType; - - @PastOrPresent(message = "visitDate must not be in the future") - private LocalDate visitDate; -} -``` - -## 2. Response DTO - -```java -package com.piramal.flwapi.dto; - -import lombok.Builder; -import lombok.Data; - -@Data -@Builder -public class ExampleResponse { - private Long id; - private Long beneficiaryId; - private String visitType; - private LocalDate visitDate; - private LocalDateTime createdAt; - private String createdBy; -} -``` - -## 3. JPA Entity (db_iemr) - -```java -package com.piramal.flwapi.domain.iemr; - -import jakarta.persistence.*; -import lombok.Data; -import org.hibernate.annotations.CreationTimestamp; -import org.hibernate.annotations.UpdateTimestamp; -import java.time.LocalDate; -import java.time.LocalDateTime; - -@Entity -@Table(name = "t_example") -@Data -public class ExampleRecord { - - @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) - private Long id; - - @Column(name = "beneficiary_id", nullable = false) - private Long beneficiaryId; - - @Column(name = "visit_type", length = 100) - private String visitType; - - @Column(name = "visit_date") - private LocalDate visitDate; - - @Column(name = "deleted", nullable = false) - private Boolean deleted = false; - - @CreationTimestamp - @Column(name = "created_date", updatable = false) - private LocalDateTime createdDate; - - @Column(name = "created_by", length = 50, updatable = false) - private String createdBy; - - @UpdateTimestamp - @Column(name = "last_modified_date") - private LocalDateTime lastModifiedDate; - - @Column(name = "modified_by", length = 50) - private String modifiedBy; -} -``` - -## 4. MapStruct Mapper - -```java -package com.piramal.flwapi.mapper; - -import com.piramal.flwapi.domain.iemr.ExampleRecord; -import com.piramal.flwapi.dto.CreateExampleRequest; -import com.piramal.flwapi.dto.ExampleResponse; -import org.mapstruct.Mapper; -import org.mapstruct.Mapping; - -@Mapper(componentModel = "spring") -public interface ExampleMapper { - - ExampleRecord toEntity(CreateExampleRequest request); - - @Mapping(source = "createdDate", target = "createdAt") - ExampleResponse toResponse(ExampleRecord entity); -} -``` - -## 5. Repository - -```java -package com.piramal.flwapi.repo.iemr; - -import com.piramal.flwapi.domain.iemr.ExampleRecord; -import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.data.jpa.repository.Query; -import org.springframework.data.repository.query.Param; -import org.springframework.stereotype.Repository; - -import java.util.List; -import java.util.Optional; - -@Repository -public interface ExampleRepository extends JpaRepository<ExampleRecord, Long> { - - List<ExampleRecord> findByBeneficiaryIdAndDeletedFalse(Long beneficiaryId); - - @Query("SELECT e FROM ExampleRecord e WHERE e.beneficiaryId = :benefId AND e.deleted = false ORDER BY e.visitDate DESC") - List<ExampleRecord> findLatestVisitsByBeneficiary(@Param("benefId") Long beneficiaryId); - - Optional<ExampleRecord> findByIdAndDeletedFalse(Long id); -} -``` - -## 6. Service Interface - -```java -package com.piramal.flwapi.service; - -import com.piramal.flwapi.dto.CreateExampleRequest; -import com.piramal.flwapi.dto.ExampleResponse; - -import java.util.List; - -public interface ExampleService { - - ExampleResponse create(CreateExampleRequest request, String createdBy); - - List<ExampleResponse> getAllByBeneficiary(Long beneficiaryId); - - ExampleResponse getById(Long id); - - ExampleResponse update(Long id, CreateExampleRequest request, String modifiedBy); - - void delete(Long id); -} -``` - -## 7. Service Implementation - -```java -package com.piramal.flwapi.service.impl; - -import com.piramal.flwapi.domain.iemr.ExampleRecord; -import com.piramal.flwapi.dto.CreateExampleRequest; -import com.piramal.flwapi.dto.ExampleResponse; -import com.piramal.flwapi.exception.ResourceNotFoundException; -import com.piramal.flwapi.mapper.ExampleMapper; -import com.piramal.flwapi.repo.iemr.ExampleRepository; -import com.piramal.flwapi.service.ExampleService; -import lombok.RequiredArgsConstructor; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Transactional; - -import java.util.List; -import java.util.stream.Collectors; - -@Service -@RequiredArgsConstructor -public class ExampleServiceImpl implements ExampleService { - - private static final Logger log = LoggerFactory.getLogger(ExampleServiceImpl.class); - - private final ExampleRepository exampleRepository; - private final ExampleMapper exampleMapper; - - @Override - @Transactional - public ExampleResponse create(CreateExampleRequest request, String createdBy) { - log.debug("Creating example record for benefId={}", request.getBeneficiaryId()); - ExampleRecord entity = exampleMapper.toEntity(request); - entity.setCreatedBy(createdBy); - ExampleRecord saved = exampleRepository.save(entity); - log.debug("Created example record id={}", saved.getId()); - return exampleMapper.toResponse(saved); - } - - @Override - @Transactional(readOnly = true) - public List<ExampleResponse> getAllByBeneficiary(Long beneficiaryId) { - log.debug("Fetching example records for benefId={}", beneficiaryId); - return exampleRepository.findByBeneficiaryIdAndDeletedFalse(beneficiaryId) - .stream() - .map(exampleMapper::toResponse) - .collect(Collectors.toList()); - } - - @Override - @Transactional(readOnly = true) - public ExampleResponse getById(Long id) { - ExampleRecord record = exampleRepository.findByIdAndDeletedFalse(id) - .orElseThrow(() -> new ResourceNotFoundException("ExampleRecord", "id", id)); - return exampleMapper.toResponse(record); - } - - @Override - @Transactional - public ExampleResponse update(Long id, CreateExampleRequest request, String modifiedBy) { - log.debug("Updating example record id={}", id); - ExampleRecord record = exampleRepository.findByIdAndDeletedFalse(id) - .orElseThrow(() -> new ResourceNotFoundException("ExampleRecord", "id", id)); - record.setVisitType(request.getVisitType()); - record.setVisitDate(request.getVisitDate()); - record.setModifiedBy(modifiedBy); - return exampleMapper.toResponse(exampleRepository.save(record)); - } - - @Override - @Transactional - public void delete(Long id) { - log.debug("Soft-deleting example record id={}", id); - ExampleRecord record = exampleRepository.findByIdAndDeletedFalse(id) - .orElseThrow(() -> new ResourceNotFoundException("ExampleRecord", "id", id)); - record.setDeleted(true); - exampleRepository.save(record); - } -} -``` - -## 8. Controller - -```java -package com.piramal.flwapi.controller; - -import com.piramal.flwapi.dto.CreateExampleRequest; -import com.piramal.flwapi.dto.ExampleResponse; -import com.piramal.flwapi.service.ExampleService; -import com.piramal.flwapi.utils.ApiResponse; -import jakarta.validation.Valid; -import lombok.RequiredArgsConstructor; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; -import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.web.bind.annotation.*; - -import java.util.List; - -@RestController -@RequestMapping("/api/v1/example") -@RequiredArgsConstructor -public class ExampleController { - - private final ExampleService exampleService; - - @PostMapping - @PreAuthorize("hasAnyRole('ROLE_ASHA', 'ROLE_ANM')") - public ResponseEntity<ApiResponse<ExampleResponse>> create( - @Valid @RequestBody CreateExampleRequest request, - @RequestHeader("Authorization") String authHeader) { - // Extract user from JWT via JwtUtil if needed, or inject Principal - ExampleResponse response = exampleService.create(request, "currentUser"); - return ResponseEntity.status(HttpStatus.CREATED) - .body(ApiResponse.success("Record created", response)); - } - - @GetMapping("/beneficiary/{beneficiaryId}") - @PreAuthorize("hasAnyRole('ROLE_ASHA', 'ROLE_ANM', 'ROLE_SUPERVISOR')") - public ResponseEntity<ApiResponse<List<ExampleResponse>>> getAllByBeneficiary( - @PathVariable @Positive Long beneficiaryId) { - List<ExampleResponse> records = exampleService.getAllByBeneficiary(beneficiaryId); - return ResponseEntity.ok(ApiResponse.success("Records fetched", records)); - } - - @GetMapping("/{id}") - @PreAuthorize("hasAnyRole('ROLE_ASHA', 'ROLE_ANM', 'ROLE_SUPERVISOR')") - public ResponseEntity<ApiResponse<ExampleResponse>> getById(@PathVariable @Positive Long id) { - return ResponseEntity.ok(ApiResponse.success("Record fetched", exampleService.getById(id))); - } - - @PostMapping("/{id}") - @PreAuthorize("hasAnyRole('ROLE_ASHA', 'ROLE_ANM')") - public ResponseEntity<ApiResponse<ExampleResponse>> update( - @PathVariable @Positive Long id, - @Valid @RequestBody CreateExampleRequest request) { - return ResponseEntity.ok(ApiResponse.success("Record updated", exampleService.update(id, request, "currentUser"))); - } - - @PostMapping("/{id}/delete") - @PreAuthorize("hasAnyRole('ROLE_SUPERVISOR', 'ROLE_ADMIN')") - public ResponseEntity<ApiResponse<Void>> delete(@PathVariable @Positive Long id) { - exampleService.delete(id); - return ResponseEntity.ok(ApiResponse.success("Record deleted", null)); - } -} -``` - -## 9. GlobalExceptionHandler - -```java -package com.piramal.flwapi.exception; - -import com.piramal.flwapi.utils.ApiResponse; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; -import org.springframework.validation.FieldError; -import org.springframework.web.bind.MethodArgumentNotValidException; -import org.springframework.web.bind.annotation.ExceptionHandler; -import org.springframework.web.bind.annotation.RestControllerAdvice; - -import java.util.Map; -import java.util.stream.Collectors; - -@RestControllerAdvice -public class GlobalExceptionHandler { - - private static final Logger log = LoggerFactory.getLogger(GlobalExceptionHandler.class); - - @ExceptionHandler(ResourceNotFoundException.class) - public ResponseEntity<ApiResponse<Void>> handleNotFound(ResourceNotFoundException ex) { - log.warn("Resource not found: {}", ex.getMessage()); - return ResponseEntity.status(HttpStatus.NOT_FOUND) - .body(ApiResponse.error(ex.getMessage())); - } - - @ExceptionHandler(DuplicateResourceException.class) - public ResponseEntity<ApiResponse<Void>> handleDuplicate(DuplicateResourceException ex) { - log.warn("Duplicate resource: {}", ex.getMessage()); - return ResponseEntity.status(HttpStatus.CONFLICT) - .body(ApiResponse.error(ex.getMessage())); - } - - @ExceptionHandler(InvalidOperationException.class) - public ResponseEntity<ApiResponse<Void>> handleInvalidOp(InvalidOperationException ex) { - log.warn("Invalid operation: {}", ex.getMessage()); - return ResponseEntity.status(HttpStatus.UNPROCESSABLE_ENTITY) - .body(ApiResponse.error(ex.getMessage())); - } - - @ExceptionHandler(MethodArgumentNotValidException.class) - public ResponseEntity<ApiResponse<Map<String, String>>> handleValidation( - MethodArgumentNotValidException ex) { - Map<String, String> errors = ex.getBindingResult().getFieldErrors().stream() - .collect(Collectors.toMap(FieldError::getField, FieldError::getDefaultMessage, - (a, b) -> a)); - log.warn("Validation failed: {}", errors); - return ResponseEntity.status(HttpStatus.BAD_REQUEST) - .body(ApiResponse.error("Validation failed", errors)); - } - - @ExceptionHandler(Exception.class) - public ResponseEntity<ApiResponse<Void>> handleGeneric(Exception ex) { - log.error("Unexpected error", ex); - return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) - .body(ApiResponse.error("An unexpected error occurred")); - } -} -``` - -## 10. Pagination Pattern - -```java -// Repository -Page<ExampleRecord> findByBeneficiaryIdAndDeletedFalse(Long beneficiaryId, Pageable pageable); - -// Service -public Page<ExampleResponse> getAllPaged(Long beneficiaryId, int page, int size) { - Pageable pageable = PageRequest.of(page, size, Sort.by("visitDate").descending()); - return exampleRepository.findByBeneficiaryIdAndDeletedFalse(beneficiaryId, pageable) - .map(exampleMapper::toResponse); -} - -// Controller -@GetMapping("/beneficiary/{beneficiaryId}/paged") -@PreAuthorize("hasAnyRole('ROLE_ASHA', 'ROLE_ANM', 'ROLE_SUPERVISOR')") -public ResponseEntity<ApiResponse<Page<ExampleResponse>>> getPaged( - @PathVariable Long beneficiaryId, - @RequestParam(defaultValue = "0") int page, - @RequestParam(defaultValue = "10") int size) { - return ResponseEntity.ok(ApiResponse.success("Records fetched", - exampleService.getAllPaged(beneficiaryId, page, size))); -} -``` - ---- - -## 11. Factory Pattern — Integration Service (canonical template) - -Use this structure for any feature that talks to an external system or has multiple possible implementations. - -### 11a. Provider Enum - -```java -package com.piramal.flwapi.integration; - -public enum IntegrationProvider { - SAP_ERP, - ORACLE_ERP, - LIMS_CONNECT, - MOCK // always present — used in tests and local dev -} -``` - -### 11b. Provider Interface - -```java -package com.piramal.flwapi.integration; - -public interface IntegrationService { - - /** Key used by the factory to index this implementation. */ - IntegrationProvider getProvider(); - - IntegrationResponse send(IntegrationPayload payload); -} -``` - -### 11c. Concrete Implementation - -```java -package com.piramal.flwapi.integration.impl; - -import com.piramal.flwapi.integration.*; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Service; - -@Service -public class SapErpIntegrationService implements IntegrationService { - - private static final Logger log = LoggerFactory.getLogger(SapErpIntegrationService.class); - - @Override - public IntegrationProvider getProvider() { return IntegrationProvider.SAP_ERP; } - - @Override - public IntegrationResponse send(IntegrationPayload payload) { - log.debug("Dispatching to SAP ERP, requestId={}", payload.getRequestId()); - // SAP-specific HTTP/RFC logic - return IntegrationResponse.ok("SAP_ACK_" + payload.getRequestId()); - } -} -``` - -### 11d. Mock Implementation (always required) - -```java -package com.piramal.flwapi.integration.impl; - -import com.piramal.flwapi.integration.*; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.stereotype.Service; - -@Service -public class MockIntegrationService implements IntegrationService { - - private static final Logger log = LoggerFactory.getLogger(MockIntegrationService.class); - - @Override - public IntegrationProvider getProvider() { return IntegrationProvider.MOCK; } - - @Override - public IntegrationResponse send(IntegrationPayload payload) { - log.debug("Mock integration — no-op for requestId={}", payload.getRequestId()); - return IntegrationResponse.ok("MOCK_OK"); - } -} -``` - -### 11e. Factory - -```java -package com.piramal.flwapi.integration; - -import com.piramal.flwapi.exception.InvalidOperationException; -import org.springframework.stereotype.Component; - -import java.util.List; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; - -@Component -public class IntegrationServiceFactory { - - private final Map<IntegrationProvider, IntegrationService> registry; - - public IntegrationServiceFactory(List<IntegrationService> services) { - this.registry = services.stream() - .collect(Collectors.toMap(IntegrationService::getProvider, Function.identity())); - } - - public IntegrationService getService(IntegrationProvider provider) { - IntegrationService service = registry.get(provider); - if (service == null) { - throw new InvalidOperationException( - "No IntegrationService registered for provider: " + provider); - } - return service; - } -} -``` - -### 11f. Config-driven Resolver - -```java -package com.piramal.flwapi.integration; - -import org.springframework.beans.factory.annotation.Value; -import org.springframework.stereotype.Component; - -@Component -public class IntegrationServiceResolver { - - private final IntegrationService activeService; - - public IntegrationServiceResolver( - IntegrationServiceFactory factory, - @Value("${integration.provider}") String providerKey) { - this.activeService = factory.getService(IntegrationProvider.valueOf(providerKey)); - } - - public IntegrationService get() { return activeService; } -} -``` - -`application.properties`: -```properties -integration.provider=${INTEGRATION_PROVIDER:MOCK} -``` - -### 11g. Factory Unit Test - -```java -@DisplayName("IntegrationServiceFactory") -class IntegrationServiceFactoryTest { - - private IntegrationServiceFactory factory; - - @BeforeEach - void setUp() { - factory = new IntegrationServiceFactory( - List.of(new SapErpIntegrationService(), new MockIntegrationService())); - } - - @Test - @DisplayName("returns correct implementation for SAP_ERP provider") - void returnsSapService() { - assertThat(factory.getService(IntegrationProvider.SAP_ERP)) - .isInstanceOf(SapErpIntegrationService.class); - } - - @Test - @DisplayName("throws InvalidOperationException when provider has no registered implementation") - void throwsForUnknownProvider() { - IntegrationServiceFactory empty = new IntegrationServiceFactory(List.of()); - assertThatThrownBy(() -> empty.getService(IntegrationProvider.SAP_ERP)) - .isInstanceOf(InvalidOperationException.class) - .hasMessageContaining("SAP_ERP"); - } -} -```