feat: refactor DTOs, Entities, and Services to use Lombok annotations#51
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactors many DTOs, entities, mappers, controllers, and services to use Lombok and builder-style construction. Several JPA UUID generation annotations were removed from entity Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/org/example/projektarendehantering/presentation/dto/CreateCaseForm.java (1)
13-16: Prefer@Getter/@Setterover@Datafor mutable form input objects.Form classes receiving user input should include only the necessary accessors and constructors.
@Dataunnecessarily generatestoString,equals, andhashCodemethods. For a web form carrying sensitive medical case data and patient IDs, removing these unnecessary methods follows the principle of minimizing the surface area exposed through generated code.Suggested diff
import lombok.AllArgsConstructor; import lombok.Builder; -import lombok.Data; +import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.Setter; @@ -@Data +@Getter +@Setter `@Builder` `@NoArgsConstructor` `@AllArgsConstructor` public class CreateCaseForm {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/presentation/dto/CreateCaseForm.java` around lines 13 - 16, Replace the Lombok `@Data` on the CreateCaseForm class with explicit `@Getter` and `@Setter` to avoid generating toString/equals/hashCode; keep the existing `@NoArgsConstructor`, `@AllArgsConstructor` and `@Builder` annotations so constructors and builder remain available while preventing automatic generation of sensitive toString/equals/hashCode methods for CreateCaseForm.src/main/java/org/example/projektarendehantering/presentation/dto/CaseDTO.java (1)
13-17: Avoid@Dataon DTOs with mutable collections (notes).On
CaseDTO, generatedequals/hashCode/toStringfrom@Dataincludesnotes, which can become costly and brittle for mutable object graphs.♻️ Proposed change
import lombok.AllArgsConstructor; import lombok.Builder; -import lombok.Data; +import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.Setter; @@ -@Data +@Getter +@Setter `@Builder` `@NoArgsConstructor` `@AllArgsConstructor` public class CaseDTO {Also applies to: 26-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/presentation/dto/CaseDTO.java` around lines 13 - 17, Replace the class-level Lombok `@Data` on CaseDTO because it generates equals/hashCode/toString that include the mutable notes collection; instead use the explicit Lombok annotations that keep getters/setters but exclude notes from equality and toString (for example, replace `@Data` with `@Getter`, `@Setter`, `@Builder`, `@NoArgsConstructor`, `@AllArgsConstructor` and add `@EqualsAndHashCode`(exclude = "notes") and `@ToString`(exclude = "notes")). Update the annotations on the CaseDTO class and verify the notes field remains unchanged; this keeps generated accessors but avoids expensive or brittle equality/toString behavior involving the notes collection.src/main/java/org/example/projektarendehantering/presentation/dto/AuditEventDTO.java (1)
11-15: Replace@Datawith explicit annotations and limittoString()output for security hardening.
@DatageneratestoString()over all fields. This DTO contains sensitive audit information (principalName,queryString,clientIp,userAgent) that could be unexpectedly logged if the object is serialized or used in error handling. Use@ToString(onlyExplicitlyIncluded = true)to restricttoString()to only non-sensitive fields.♻️ Proposed change
import lombok.AllArgsConstructor; import lombok.Builder; -import lombok.Data; +import lombok.EqualsAndHashCode; +import lombok.Getter; import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; @@ -@Data +@Getter +@Setter +@EqualsAndHashCode +@ToString(onlyExplicitlyIncluded = true) `@Builder` `@NoArgsConstructor` `@AllArgsConstructor` public class AuditEventDTO { + `@ToString.Include` private UUID id; + `@ToString.Include` private Instant occurredAt;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/presentation/dto/AuditEventDTO.java` around lines 11 - 15, Remove the Lombok `@Data` on class AuditEventDTO and replace it with explicit annotations: add `@Getter`, `@Setter`, `@Builder`, `@NoArgsConstructor` and `@AllArgsConstructor`, plus `@ToString`(onlyExplicitlyIncluded = true); then mark only non-sensitive fields with `@ToString.Include` so that sensitive fields (principalName, queryString, clientIp, userAgent) are excluded from toString() output. Ensure any equals/hashCode needs are handled explicitly (e.g., add `@EqualsAndHashCode` if desired) rather than relying on `@Data`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/projektarendehantering/infrastructure/persistence/EmployeeEntity.java`:
- Around line 30-32: Remove the JPA-driven ID generation so the app-controlled
UUIDs are respected: in EmployeeEntity delete the `@GeneratedValue`(strategy =
GenerationType.UUID) annotation so Hibernate won't override IDs that
EmployeeMapper.toEntity() sets via UUID.nameUUIDFromBytes(...) or the fallback
set in EmployeeService.createEmployee() (UUID.randomUUID()). Ensure only the `@Id`
and the UUID field remain so application-assigned IDs persist.
---
Nitpick comments:
In
`@src/main/java/org/example/projektarendehantering/presentation/dto/AuditEventDTO.java`:
- Around line 11-15: Remove the Lombok `@Data` on class AuditEventDTO and replace
it with explicit annotations: add `@Getter`, `@Setter`, `@Builder`, `@NoArgsConstructor`
and `@AllArgsConstructor`, plus `@ToString`(onlyExplicitlyIncluded = true); then
mark only non-sensitive fields with `@ToString.Include` so that sensitive fields
(principalName, queryString, clientIp, userAgent) are excluded from toString()
output. Ensure any equals/hashCode needs are handled explicitly (e.g., add
`@EqualsAndHashCode` if desired) rather than relying on `@Data`.
In
`@src/main/java/org/example/projektarendehantering/presentation/dto/CaseDTO.java`:
- Around line 13-17: Replace the class-level Lombok `@Data` on CaseDTO because it
generates equals/hashCode/toString that include the mutable notes collection;
instead use the explicit Lombok annotations that keep getters/setters but
exclude notes from equality and toString (for example, replace `@Data` with
`@Getter`, `@Setter`, `@Builder`, `@NoArgsConstructor`, `@AllArgsConstructor` and add
`@EqualsAndHashCode`(exclude = "notes") and `@ToString`(exclude = "notes")). Update
the annotations on the CaseDTO class and verify the notes field remains
unchanged; this keeps generated accessors but avoids expensive or brittle
equality/toString behavior involving the notes collection.
In
`@src/main/java/org/example/projektarendehantering/presentation/dto/CreateCaseForm.java`:
- Around line 13-16: Replace the Lombok `@Data` on the CreateCaseForm class with
explicit `@Getter` and `@Setter` to avoid generating toString/equals/hashCode; keep
the existing `@NoArgsConstructor`, `@AllArgsConstructor` and `@Builder` annotations so
constructors and builder remain available while preventing automatic generation
of sensitive toString/equals/hashCode methods for CreateCaseForm.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38466995-29fb-4b59-8607-a721d8434ff2
📒 Files selected for processing (23)
src/main/java/org/example/projektarendehantering/application/service/AuditEventMapper.javasrc/main/java/org/example/projektarendehantering/application/service/AuditService.javasrc/main/java/org/example/projektarendehantering/application/service/CaseMapper.javasrc/main/java/org/example/projektarendehantering/application/service/CaseNoteMapper.javasrc/main/java/org/example/projektarendehantering/application/service/CaseService.javasrc/main/java/org/example/projektarendehantering/application/service/EmployeeMapper.javasrc/main/java/org/example/projektarendehantering/application/service/EmployeeService.javasrc/main/java/org/example/projektarendehantering/application/service/PatientMapper.javasrc/main/java/org/example/projektarendehantering/application/service/PatientService.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/AuditEventEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/EmployeeEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/PatientEntity.javasrc/main/java/org/example/projektarendehantering/presentation/dto/AuditEventDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CaseAssignmentDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CaseDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CaseNoteDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CreateCaseForm.javasrc/main/java/org/example/projektarendehantering/presentation/dto/EmployeeCreateDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/EmployeeDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/PatientCreateDTO.javasrc/main/java/org/example/projektarendehantering/presentation/dto/PatientDTO.java
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/org/example/projektarendehantering/application/service/CaseService.java (1)
64-71:⚠️ Potential issue | 🟠 MajorDon't assign managers as
ownerIdduring creation.
assignUsers()later enforces at Line 136 thatownerIdmust reference a doctor, but this branch stores a manager'suserId()when a manager creates the case. That leaves newly created cases in a state your own validation rejects and makes doctor-scoped queries/ACL checks miss them until somebody reassigns the case.Suggested fix
- if (isDoctor(actor) || isManager(actor)) { + if (isDoctor(actor)) { entity.setOwnerId(actor.userId()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/application/service/CaseService.java` around lines 64 - 71, The creation path in CaseService sets entity.setOwnerId(actor.userId()) for managers which later violates assignUsers() validation that ownerId must be a doctor; change the logic in the case creation method (where CaseEntity entity = caseMapper.toEntity(caseDTO) and actor is checked with isDoctor/isManager) so that entity.setOwnerId(...) is only called when isDoctor(actor) is true (do not set ownerId for managers), leaving ownerId null for manager-created cases so assignUsers()/doctor-scoped ACLs remain consistent.src/main/java/org/example/projektarendehantering/application/service/AuditService.java (1)
62-71:⚠️ Potential issue | 🔴 CriticalAlways generate a fresh audit-event ID here.
The conditional check allows caller-supplied IDs to pass through to
save(). SinceAuditEventEntityuses an application-assigned ID strategy (no@GeneratedValue), Spring Data JPA'ssave()will perform an UPDATE (merge) instead of INSERT when a non-null ID is present, potentially overwriting existing audit records and breaking the append-only audit trail.Suggested fix
- if (event.getId() == null) { - event.setId(UUID.randomUUID()); - } + event.setId(UUID.randomUUID());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/projektarendehantering/application/service/AuditService.java` around lines 62 - 71, The record method in AuditService currently lets caller-supplied IDs through which can cause save() to perform UPDATEs; always overwrite the event ID to a fresh UUID before persisting (i.e., call event.setId(UUID.randomUUID()) unconditionally in AuditService.record), keep the existing occurredAt fallback behavior (set if null), continue to sanitize via sanitizeAuditPayload(event.getQueryString()), and then call auditEventRepository.save(event) so every audit record is inserted with a new application-assigned ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@src/main/java/org/example/projektarendehantering/application/service/AuditService.java`:
- Around line 62-71: The record method in AuditService currently lets
caller-supplied IDs through which can cause save() to perform UPDATEs; always
overwrite the event ID to a fresh UUID before persisting (i.e., call
event.setId(UUID.randomUUID()) unconditionally in AuditService.record), keep the
existing occurredAt fallback behavior (set if null), continue to sanitize via
sanitizeAuditPayload(event.getQueryString()), and then call
auditEventRepository.save(event) so every audit record is inserted with a new
application-assigned ID.
In
`@src/main/java/org/example/projektarendehantering/application/service/CaseService.java`:
- Around line 64-71: The creation path in CaseService sets
entity.setOwnerId(actor.userId()) for managers which later violates
assignUsers() validation that ownerId must be a doctor; change the logic in the
case creation method (where CaseEntity entity = caseMapper.toEntity(caseDTO) and
actor is checked with isDoctor/isManager) so that entity.setOwnerId(...) is only
called when isDoctor(actor) is true (do not set ownerId for managers), leaving
ownerId null for manager-created cases so assignUsers()/doctor-scoped ACLs
remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbed68c6-7582-422b-988b-f5c39623e28e
📒 Files selected for processing (8)
src/main/java/org/example/projektarendehantering/application/service/AuditService.javasrc/main/java/org/example/projektarendehantering/application/service/CaseService.javasrc/main/java/org/example/projektarendehantering/application/service/PatientService.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/AuditEventEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseNoteEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/EmployeeEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/PatientEntity.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/example/projektarendehantering/application/service/PatientService.java
- src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java
… consistency across project
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.java`:
- Around line 24-25: The test is creating a CaseEntity without an ID which
causes save() to fail; in DocumentIntegrationTest where CaseEntity is
instantiated, call caseEntity.setId(UUID.randomUUID()) (or
UUID.nameUUIDFromBytes(...) if you need deterministic IDs) on the CaseEntity
before invoking caseRepository.save(caseEntity) so the entity has an ID prior to
persistence; locate the instantiation of CaseEntity in DocumentIntegrationTest
and add the setId(...) call before the save() invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2235f8f-f662-4bc2-bef5-61ce171423bf
📒 Files selected for processing (10)
src/main/java/org/example/projektarendehantering/application/service/CaseMapper.javasrc/main/java/org/example/projektarendehantering/application/service/DocumentService.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/CaseEntity.javasrc/main/java/org/example/projektarendehantering/infrastructure/persistence/DocumentEntity.javasrc/main/java/org/example/projektarendehantering/presentation/dto/CaseDTO.javasrc/main/java/org/example/projektarendehantering/presentation/rest/AuditController.javasrc/main/java/org/example/projektarendehantering/presentation/rest/CaseController.javasrc/main/java/org/example/projektarendehantering/presentation/rest/EmployeeController.javasrc/main/java/org/example/projektarendehantering/presentation/rest/PatientController.javasrc/main/java/org/example/projektarendehantering/presentation/web/DocumentUiController.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/org/example/projektarendehantering/presentation/rest/AuditController.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/example/projektarendehantering/application/service/CaseMapper.java
closes #15
Summary by CodeRabbit