Core backend (Spring Boot)
Type: ReferenceCreated: Team: Platform
draft
About this prompt
This is the exact review prompt the AI reviewer runs for core backend pull requests, applied on top of the shared review rules. Skim it before you raise a PR — most of what it flags is something you can catch and fix yourself first.
# Skapp Backend Review Guidelines
> Spring Boot 4 + Java 21 + Spring Security + JPA/Hibernate + Liquibase + MapStruct + Lombok + MySQL
## Architecture
Layered monolith with module-based domain organization. Each feature module (under `com.skapp.community` and `com.skapp.enterprise`) follows the same layered structure. Enterprise features live in git submodules at `src/main/java/com/skapp/enterprise` (source), `src/main/resources/enterprise` (resources), and `src/main/resources/config` (config), and can conditionally override community beans.
### Layer flow
```
Controller (v1/) → Service (interface) → ServiceImpl → Dao (JpaRepository + JpaSpecificationExecutor + custom *Repository)
↳ *RepositoryImpl (Criteria API / EntityManager)
```
ServiceImpl classes also inject other service interfaces, MapStruct mappers, validators, and utility classes.
### Package structure per module
```
module/
controller/v1/ → REST endpoints
service/ → Service interfaces
service/impl/ → Service implementations
repository/ → Dao (JPA), custom Repository interface
repository/impl/ → Custom repository implementations (Criteria API)
model/ → JPA entities (extend Auditable<String>)
payload/ → DTOs (mix of flat DTOs + request/, response/, email/ subdirs)
mapper/ → MapStruct mappers
type/ → Enums
constant/ → Message constants
util/ → Utility classes
```
The `common` module additionally has: `component/`, `config/`, `exception/`, `security/`.
### Key conventions
- **Constructor injection** via Lombok `@RequiredArgsConstructor` — never field injection
- **Interface-first services** — every service has an interface, implementation is `*ServiceImpl`
- **Dao naming** — Spring Data repositories are `*Dao`, custom query interfaces are `*Repository` with `*RepositoryImpl`
- **All entities extend `Auditable<String>`** — provides createdBy, createdDate, lastModifiedBy, lastModifiedDate
- **Enums persisted as strings** — `@Enumerated(EnumType.STRING)` always
---
## What to flag
### Architecture violations
- Business logic in controllers — controllers must only delegate to service methods and return `ResponseEntity<ResponseEntityDto>`
- Direct repository calls from controllers — always go through the service layer
- Service implementations without a corresponding interface
- Cross-module service calls that bypass the service interface (e.g., calling `ServiceImpl` directly instead of the interface)
- Missing `@Transactional` on service methods that perform writes or multiple reads that must be consistent
- `@Transactional` on read-only methods without `readOnly = true`
### Entity & JPA issues
- Entities not extending `Auditable<String>` — all entities must have audit fields
- `@Enumerated(EnumType.ORDINAL)` — always use `EnumType.STRING` to avoid breakage on enum reordering
- `CascadeType.ALL` or `CascadeType.REMOVE` without careful justification — can cause accidental deletions
- `FetchType.EAGER` on collections (OneToMany, ManyToMany) — causes N+1 queries
- Missing `@MapsId` on OneToOne relationships that share the primary key (e.g., Employee → User pattern)
- New entities without Liquibase changeset — schema changes go through `community/db/changelog/`, never `ddl-auto`
- Direct SQL or native queries when Criteria API or derived queries would work
### DTO & mapping issues
- Entities exposed directly in controller responses — always use DTOs
- Manual field-by-field mapping when MapStruct should be used — all mappers are `@Mapper(componentModel = "spring")`
- Missing `@Mapping` annotations for fields that don't match by name between entity and DTO
- Response DTOs containing sensitive fields (password, tokens, internal IDs that shouldn't be public)
### Security
- Controllers missing `@PreAuthorize` on endpoints that require authentication
- Overly broad role checks — verify the minimum required role (ROLE_SUPER_ADMIN, ROLE_PEOPLE_ADMIN, ROLE_LEAVE_ADMIN, ROLE_ATTENDANCE_ADMIN, ROLE_PEOPLE_MANAGER, ROLE_LEAVE_MANAGER, ROLE_PEOPLE_EMPLOYEE, ROLE_LEAVE_EMPLOYEE)
- Endpoints in the permit-all list (`/v1/auth/**`, `/swagger-ui/**`, `/health`) that shouldn't be public
- Missing `@Valid` on `@RequestBody` parameters — bean validation must be triggered
- User input used in JPA queries without parameterization (SQL injection risk)
- Token or password data logged at INFO level — sensitive data must never appear in logs
### Validation
- Missing bean validation annotations on DTO fields (`@NotNull`, `@Email`, `@NotBlank`, etc.)
- Custom validation in service layer that duplicates what bean validation already catches
- Validation error messages using hardcoded strings instead of message constants from `*MessageConstant` classes
- New validation patterns that ignore the existing `Validation` utility class (`com.skapp.community.common.util.Validation`)
### Exception handling
- Catching generic `Exception` instead of specific exception types
- Throwing raw `RuntimeException` instead of `ModuleException` with a message constant key
- Swallowing exceptions silently (empty catch blocks)
- Not using the established `ErrorResponse` format — all errors must flow through `GlobalExceptionHandler`
- Missing message constant for new error scenarios — add to the appropriate `*MessageConstant` class and `*-messages.properties`
### Repository patterns
- Complex queries built with string concatenation — use Criteria API with JPA metamodel (`Employee_`, `LeaveType_`, etc.)
- N+1 query patterns — verify JOIN FETCH or `@EntityGraph` for relationships accessed in loops
- Unbounded queries without pagination (`findAll()` on large tables)
- Custom repository implementations not following the `*Repository` interface + `*RepositoryImpl` naming convention
### Logging
- Missing `@Slf4j` on service/component classes that need logging
- `System.out.println` instead of SLF4J logger
- Logging sensitive data (passwords, tokens, personal information)
- Missing entry logging on service methods — convention is `log.info("methodName: execution started")`
- Exception stack traces at INFO level — use `log.error("message", exception)` for errors
### Testing
- Service changes without corresponding test updates
- Integration tests missing `@SpringBootTest` + `@AutoConfigureMockMvc`
- Test data setup that relies on production database state
- Mocking repository returns without verifying the actual query behavior
### Naming & conventions
- Classes not following naming convention: `*Controller`, `*Service`, `*ServiceImpl`, `*Dao`, `*Repository`, `*RepositoryImpl`, `*Dto`, `*RequestDto`, `*ResponseDto`, `*Mapper`, `*Constant`
- Controller endpoints not versioned under `/v1/` or `/v2/`
- Methods not following verb patterns: `get*`, `create*`, `update*`, `delete*` for CRUD
- Package placement — new classes must go in the correct module and layer package
### Configuration
- Hardcoded values that should be in `application.yml` or environment variables
- Missing `@ConditionalOnMissingBean` when enterprise might override a community bean
- New scheduled jobs not using Quartz (`@EnableScheduling` + Quartz is the pattern)
---
## What NOT to flag (false positives)
- Lombok usage (`@Getter`, `@Setter`, `@RequiredArgsConstructor`, `@Slf4j`, `@Builder`) — this is standard
- `ResponseEntityDto` wrapper pattern — this is the established response format
- `Auditable<String>` using String for user ID — this is the design choice
- `Dao` naming for Spring Data repositories — this is the convention (not `Repository` which is reserved for custom query interfaces)
- Checkstyle issues (formatting, imports) — these are caught by `checkstyle.xml` at build time
---
## Checklist
Verify each item for every changed file:
1. **Exception handling**: Every `Long.valueOf()`, `Integer.parseInt()`, `URLDecoder.decode()`, `encryptionService.decrypt()`, `JSON.parse()` — is failure caught and wrapped in `ModuleException`? Unhandled → 500 to the client
2. **Null safety**: Every method chain `a.getB().getC()` — can `getB()` return null? Check JPA `@JoinColumn` nullable settings and `Optional` returns from repositories
3. **Security on public endpoints**: Any path added to `permitAll` or JWT bypass lists — what prevents abuse? Check for: link expiry validation, rate limiting, input size bounds. The unauthenticated path should do equivalent validation to its authenticated counterpart (e.g., `validateTokenFlows`)
4. **Behavioral preservation in refactors**: When code is extracted into helpers — does the new version preserve ALL behaviors? Check: same parameters passed, same validations run, same logging present, same return values, same error paths
5. **Dead code**: Null checks on `String.split()` results (never null), impossible enum branches, unused variables, conditions that are always true/false
6. **DTO validation annotations**: `@NotBlank` (not `@NotNull`) for String fields that must be non-empty, `@Valid` on nested DTOs, `@Email` on email fields, `@NotNull` on enum/object fields
7. **Naming conventions**: Controllers end in `*Controller`, services in `*Service`/`*ServiceImpl`, endpoints under `/v1/` or `/v2/`, CRUD methods use `get*`/`create*`/`update*`/`delete*`
8. **Missing service logging**: New service methods must have entry logging: `log.info("methodName: execution started")`. Errors logged with stack trace: `log.error("message", exception)`
9. **Code duplication**: 2+ new DTOs with identical fields → consolidate into shared base or single DTO. 2+ methods with near-identical bodies → extract shared helper
10. **Symmetric auth/unauth endpoints**: If both authenticated and unauthenticated versions of an operation exist — do they call the same validation and return the same response structure? Flag any unexplained asymmetry