-
Notifications
You must be signed in to change notification settings - Fork 0
Loggers #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Loggers #31
Changes from all commits
c74eb3a
5b9b030
bd047b1
4a2924b
f3f68e2
f883fa5
2b75ad4
cb3d3e9
0c73fdc
5d5a7b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,3 +35,4 @@ out/ | |
|
|
||
| ### VS Code ### | ||
| .vscode/ | ||
| target/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,33 @@ | ||
| package org.example.crimearchive.Entity.försvare; | ||
| import jakarta.persistence.*; | ||
| import org.example.crimearchive.audit.Auditable; | ||
| import java.time.LocalDate; | ||
| import java.time.ZonedDateTime; | ||
|
|
||
| public class Advocat { | ||
| @Entity | ||
| @Table(name = "advocat") | ||
| public class Advocat extends Auditable { | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(nullable = false) | ||
| private String email; | ||
|
|
||
| @Column(nullable = false) | ||
| private String company; | ||
|
|
||
| @Column(nullable = false) | ||
| private String telephone; | ||
|
|
||
| @Column(nullable = false) | ||
| private String name; | ||
|
|
||
| private String knumber; | ||
|
|
||
| private LocalDate appointedTime; | ||
|
|
||
| private ZonedDateTime crimeTime; | ||
|
Comment on lines
+26
to
+30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent column nullability declarations.
Decide which is authoritative and align them (typically: add 🤖 Prompt for AI Agents |
||
|
|
||
| // Getters and Setters if needed, or use Lombok if available | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing getters/setters / accessors — entity is effectively unusable. The trailing comment Either generate explicit getters/setters, or add Lombok annotations (and confirm Lombok is on the classpath in Want me to generate a Lombok-annotated version (or plain getters/setters) and verify Lombok is configured in 🤖 Prompt for AI Agents |
||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package org.example.crimearchive.audit; | ||
|
|
||
|
|
||
| import jakarta.persistence.*; | ||
| import org.hibernate.envers.Audited; | ||
| import org.springframework.data.annotation.*; | ||
| import org.springframework.data.jpa.domain.support.AuditingEntityListener; | ||
|
|
||
| import java.time.LocalDateTime; | ||
|
|
||
| @MappedSuperclass | ||
| @EntityListeners(AuditingEntityListener.class) | ||
| @Audited | ||
| public abstract class Auditable { | ||
|
|
||
| @CreatedBy | ||
| protected String createdBy; | ||
|
|
||
| @CreatedDate | ||
| protected LocalDateTime createdDate; | ||
|
|
||
| @LastModifiedBy | ||
| protected String lastModifiedBy; | ||
|
|
||
| @LastModifiedDate | ||
| protected LocalDateTime lastModifiedDate; | ||
|
|
||
| // Getters och Setters | ||
|
|
||
| public String getCreatedBy() { | ||
| return createdBy; | ||
| } | ||
| public void setCreatedBy(String createdBy) { | ||
| this.createdBy = createdBy; | ||
| } | ||
| public LocalDateTime getCreatedDate() { | ||
| return createdDate; | ||
| } | ||
| public void setCreatedDate(LocalDateTime createdDate) { | ||
| this.createdDate = createdDate; | ||
| } | ||
| public String getLastModifiedBy() { | ||
| return lastModifiedBy; | ||
| } | ||
| public void setLastModifiedBy(String lastModifiedBy) { | ||
| this.lastModifiedBy = lastModifiedBy; | ||
| } | ||
| public LocalDateTime getLastModifiedDate() { | ||
| return lastModifiedDate; | ||
| } | ||
| public void setLastModifiedDate(LocalDateTime lastModifiedDate) { | ||
| this.lastModifiedDate = lastModifiedDate; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,13 @@ | |
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
| import org.springframework.web.server.ResponseStatusException; | ||
| import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.CopyOnWriteArrayList; | ||
|
|
||
| @Service | ||
| public class CaseLifecycleService { | ||
|
|
@@ -14,6 +19,8 @@ public class CaseLifecycleService { | |
| private final CaseEventRepository eventRepository; | ||
| private final CaseCommentRepository commentRepository; | ||
|
|
||
| private final ConcurrentHashMap<String, CopyOnWriteArrayList<SseEmitter>> emitters = new ConcurrentHashMap<>(); | ||
|
|
||
| public CaseLifecycleService(CasesRepository casesRepository, | ||
| CaseEventRepository eventRepository, | ||
| CaseCommentRepository commentRepository) { | ||
|
|
@@ -22,16 +29,25 @@ public CaseLifecycleService(CasesRepository casesRepository, | |
| this.commentRepository = commentRepository; | ||
| } | ||
|
|
||
| public SseEmitter subscribe(String caseNumber) { | ||
| SseEmitter emitter = new SseEmitter(Long.MAX_VALUE); | ||
| emitters.computeIfAbsent(caseNumber, k -> new CopyOnWriteArrayList<>()).add(emitter); | ||
| emitter.onCompletion(() -> removeEmitter(caseNumber, emitter)); | ||
| emitter.onTimeout(() -> removeEmitter(caseNumber, emitter)); | ||
| emitter.onError(e -> removeEmitter(caseNumber, emitter)); | ||
| return emitter; | ||
| } | ||
|
|
||
| @Transactional | ||
| public void initCase(Cases cases, String performedBy) { | ||
| cases.setStatus(CaseStatus.OPEN); | ||
| eventRepository.save(new CaseEvent(cases, CaseEventType.CASE_CREATED, | ||
| saveAndBroadcast(new CaseEvent(cases, CaseEventType.CASE_CREATED, | ||
| "Ärende " + cases.getCaseNumber() + " skapades", performedBy)); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void onHandlerAssigned(Cases cases, String handlerName, String performedBy) { | ||
| eventRepository.save(new CaseEvent(cases, CaseEventType.HANDLER_ASSIGNED, | ||
| saveAndBroadcast(new CaseEvent(cases, CaseEventType.HANDLER_ASSIGNED, | ||
| "Handläggare " + handlerName + " tilldelades ärendet", performedBy)); | ||
| if (cases.getStatus() == CaseStatus.OPEN) { | ||
| updateStatus(cases, CaseStatus.ASSIGNED, performedBy); | ||
|
|
@@ -40,7 +56,7 @@ public void onHandlerAssigned(Cases cases, String handlerName, String performedB | |
|
|
||
| @Transactional | ||
| public void onHandlerRemoved(Cases cases, String handlerName, String performedBy) { | ||
| eventRepository.save(new CaseEvent(cases, CaseEventType.HANDLER_REMOVED, | ||
| saveAndBroadcast(new CaseEvent(cases, CaseEventType.HANDLER_REMOVED, | ||
| "Handläggare " + handlerName + " togs bort från ärendet", performedBy)); | ||
| // Only revert to OPEN from ASSIGNED — later states (IN_PROGRESS, CLOSED) are not disturbed by handler removal. | ||
| if (cases.getAccounts().isEmpty() && cases.getStatus() == CaseStatus.ASSIGNED) { | ||
|
|
@@ -50,10 +66,19 @@ public void onHandlerRemoved(Cases cases, String handlerName, String performedBy | |
|
|
||
| @Transactional | ||
| public void onDocumentUploaded(Cases cases, String filename, String performedBy) { | ||
| eventRepository.save(new CaseEvent(cases, CaseEventType.DOCUMENT_UPLOADED, | ||
| saveAndBroadcast(new CaseEvent(cases, CaseEventType.DOCUMENT_UPLOADED, | ||
| "Dokument laddades upp: " + filename, performedBy)); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void onRoleChanged(Long accountId, String username, List<String> oldRoles, List<String> newRoles, String performedBy) { | ||
| List<Cases> cases = casesRepository.findByAccountsId(accountId); | ||
| String description = "Roller för " + username + " ändrades från [" + String.join(", ", oldRoles) + "] till [" + String.join(", ", newRoles) + "]"; | ||
| for (Cases c : cases) { | ||
| saveAndBroadcast(new CaseEvent(c, CaseEventType.ROLE_CHANGED, description, performedBy)); | ||
| } | ||
| } | ||
|
|
||
| @Transactional | ||
| public CaseComment addComment(String caseNumber, String content, String author) { | ||
| if (content == null || content.isBlank()) { | ||
|
|
@@ -64,7 +89,7 @@ public CaseComment addComment(String caseNumber, String content, String author) | |
| } | ||
| Cases cases = findCase(caseNumber); | ||
| CaseComment comment = commentRepository.save(new CaseComment(cases, content, author)); | ||
| eventRepository.save(new CaseEvent(cases, CaseEventType.COMMENT_ADDED, | ||
| saveAndBroadcast(new CaseEvent(cases, CaseEventType.COMMENT_ADDED, | ||
| author + " lade till en kommentar", author)); | ||
| return comment; | ||
| } | ||
|
|
@@ -93,10 +118,36 @@ public CaseStatus getStatus(String caseNumber) { | |
| private void updateStatus(Cases cases, CaseStatus newStatus, String performedBy) { | ||
| CaseStatus previous = cases.getStatus(); | ||
| cases.setStatus(newStatus); | ||
| eventRepository.save(new CaseEvent(cases, CaseEventType.STATUS_CHANGED, | ||
| saveAndBroadcast(new CaseEvent(cases, CaseEventType.STATUS_CHANGED, | ||
| "Status ändrades från " + previous + " till " + newStatus, performedBy)); | ||
| } | ||
|
|
||
| private CaseEvent saveAndBroadcast(CaseEvent event) { | ||
| CaseEvent saved = eventRepository.save(event); | ||
| broadcast(event.getCaseEntity().getCaseNumber(), saved); | ||
| return saved; | ||
| } | ||
|
|
||
| private void broadcast(String caseNumber, CaseEvent event) { | ||
| CopyOnWriteArrayList<SseEmitter> list = emitters.get(caseNumber); | ||
| if (list == null || list.isEmpty()) return; | ||
| CaseEventResponse dto = CaseEventResponse.from(event); | ||
| List<SseEmitter> dead = new ArrayList<>(); | ||
| for (SseEmitter emitter : list) { | ||
| try { | ||
| emitter.send(SseEmitter.event().name("case-event").data(dto)); | ||
| } catch (IOException e) { | ||
| dead.add(emitter); | ||
| } | ||
| } | ||
| list.removeAll(dead); | ||
| } | ||
|
Comment on lines
+125
to
+144
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Major: SSE events are broadcast before the transaction commits — risk of "ghost events" on rollback. Every call site of A second consequence is that Recommended fix: publish a Spring application event from ♻️ Sketch of the after-commit pattern+import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.transaction.event.TransactionalEventListener;
+import org.springframework.transaction.event.TransactionPhase;
@@
- public CaseLifecycleService(CasesRepository casesRepository,
+ private final ApplicationEventPublisher publisher;
+
+ public CaseLifecycleService(CasesRepository casesRepository,
CaseEventRepository eventRepository,
- CaseCommentRepository commentRepository) {
+ CaseCommentRepository commentRepository,
+ ApplicationEventPublisher publisher) {
this.casesRepository = casesRepository;
this.eventRepository = eventRepository;
this.commentRepository = commentRepository;
+ this.publisher = publisher;
}
@@
private CaseEvent saveAndBroadcast(CaseEvent event) {
CaseEvent saved = eventRepository.save(event);
- broadcast(event.getCaseEntity().getCaseNumber(), saved);
+ publisher.publishEvent(new CaseEventCommitted(saved.getCaseEntity().getCaseNumber(), saved));
return saved;
}
+
+ record CaseEventCommitted(String caseNumber, CaseEvent event) {}
+
+ `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT)
+ void onCommitted(CaseEventCommitted ev) {
+ broadcast(ev.caseNumber(), ev.event());
+ }🤖 Prompt for AI Agents |
||
|
|
||
| private void removeEmitter(String caseNumber, SseEmitter emitter) { | ||
| CopyOnWriteArrayList<SseEmitter> list = emitters.get(caseNumber); | ||
| if (list != null) list.remove(emitter); | ||
| } | ||
|
|
||
| private Cases findCase(String caseNumber) { | ||
| return casesRepository.findFirstByCaseNumber(caseNumber) | ||
| .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package org.example.crimearchive.config; | ||
|
|
||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.data.domain.AuditorAware; | ||
| import org.springframework.data.jpa.repository.config.EnableJpaAuditing; | ||
| import org.springframework.security.core.Authentication; | ||
| import org.springframework.security.core.context.SecurityContextHolder; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @Configuration | ||
| @EnableJpaAuditing(auditorAwareRef = "auditorProvider") | ||
| public class AuditConfig { | ||
|
|
||
| @Bean | ||
| public AuditorAware<String> auditorProvider() { | ||
| return () -> { | ||
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | ||
| if (authentication == null || !authentication.isAuthenticated()) { | ||
| return Optional.of("SYSTEM"); | ||
| } | ||
| return Optional.of(authentication.getName()); | ||
| }; | ||
| } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,4 +73,4 @@ public ApplicationRunner createBucket(S3Client s3Client) { | |
| } | ||
| }; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long idhere vslong idinDTOAdvocat— auto-unboxing NPE risk.Entity uses boxed
Long id(correct for JPA, sincenullindicates "not yet persisted"), butDTOAdvocat(src/main/java/org/example/crimearchive/dto/defense/DTOAdvocat.javaline 8) declareslong id(primitive). If a mapper ever copiesentity.getId()into the DTO before the entity is flushed/saved, auto-unboxingnull → longwill throwNullPointerException. Recommend changingDTOAdvocat.idtoLongfor consistency, or guarantee mappers only run post-persist.🤖 Prompt for AI Agents