Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
09db4a3
Add transactional support and status update logic to `TicketService`
simonforsberg Apr 10, 2026
c10d85f
Add `assignInvestigator` method to `TicketService` with investigator …
simonforsberg Apr 10, 2026
cbddbaa
Add `unassignInvestigator` method to `TicketService` with investigato…
simonforsberg Apr 10, 2026
9029f6c
Add role validation TODOs for admin and investigator actions in `Tick…
simonforsberg Apr 10, 2026
8b2435f
Refine and expand role validation and audit logging TODO placeholders…
simonforsberg Apr 10, 2026
d99e30f
Add `internalNote` field to `CommentCreateDTO` with default value
simonforsberg Apr 10, 2026
b8d591e
Refine role validation logic in `updateTicketStatus` and improve stat…
simonforsberg Apr 10, 2026
2432aee
Introduce `TicketCommentService` for managing ticket comments and imp…
simonforsberg Apr 10, 2026
e4769b8
Enforce role-based validation for creating internal notes in `TicketC…
simonforsberg Apr 10, 2026
3638f20
Replace `RuntimeException` with `ResponseStatusException` for consist…
simonforsberg Apr 10, 2026
efb3655
Add `@Transactional(readOnly=true)` to `getComments` in `TicketCommen…
simonforsberg Apr 10, 2026
8f15adc
Replace `RuntimeException` with `ResponseStatusException` for consist…
simonforsberg Apr 10, 2026
e13640b
Enforce investigator assignment validation when transitioning ticket …
simonforsberg Apr 10, 2026
d650ba0
Enforce role and status validation for investigator assignment and un…
simonforsberg Apr 10, 2026
2916e7e
Refine role validation in `TicketCommentService` to handle null actor…
simonforsberg Apr 10, 2026
e15e1c9
Replace `IllegalStateException` and `IllegalArgumentException` with `…
simonforsberg Apr 10, 2026
d5c48b7
Enhance status validation for investigator assignment and unassignmen…
simonforsberg Apr 10, 2026
e799ad4
Replace `IllegalStateException` with `ResponseStatusException` for co…
simonforsberg Apr 10, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ public class CommentCreateDTO {

@NotBlank
private String message;
}

private boolean internalNote = false;

}
26 changes: 26 additions & 0 deletions src/main/java/org/example/alfs/mapper/TicketCommentMapper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.example.alfs.mapper;

import org.example.alfs.dto.comment.CommentViewDTO;
import org.example.alfs.entities.TicketComment;
import org.springframework.stereotype.Component;

@Component
public class TicketCommentMapper {
public CommentViewDTO entityToViewDTO(TicketComment comment) {
CommentViewDTO dto = new CommentViewDTO();

dto.setId(comment.getId());
dto.setMessage(comment.getMessage());
dto.setCreatedAt(comment.getCreatedAt());

if (comment.getAuthor() != null) {
dto.setAuthor(comment.getAuthor().getUsername());
dto.setRole(comment.getAuthor().getRole().name());
} else {
dto.setAuthor("Anonymous");
dto.setRole(null);
}

return dto;
}
}
75 changes: 75 additions & 0 deletions src/main/java/org/example/alfs/services/TicketCommentService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.example.alfs.services;

import org.example.alfs.dto.comment.CommentCreateDTO;
import org.example.alfs.dto.comment.CommentViewDTO;
import org.example.alfs.entities.Ticket;
import org.example.alfs.entities.TicketComment;
import org.example.alfs.entities.User;
import org.example.alfs.enums.Role;
import org.example.alfs.mapper.TicketCommentMapper;
import org.example.alfs.repositories.TicketCommentRepository;
import org.example.alfs.repositories.TicketRepository;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.server.ResponseStatusException;

import java.util.List;

@Service
public class TicketCommentService {

private final TicketRepository ticketRepository;
private final TicketCommentRepository ticketCommentRepository;
private final TicketCommentMapper ticketCommentMapper;

public TicketCommentService(TicketRepository ticketRepository,
TicketCommentRepository ticketCommentRepository,
TicketCommentMapper ticketCommentMapper) {
this.ticketRepository = ticketRepository;
this.ticketCommentRepository = ticketCommentRepository;
this.ticketCommentMapper = ticketCommentMapper;
}

@Transactional
public CommentViewDTO addComment(Long ticketId, CommentCreateDTO dto, User author) {
Ticket ticket = ticketRepository.findById(ticketId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

TicketComment comment = new TicketComment();
comment.setTicket(ticket);
comment.setAuthor(author);
comment.setMessage(dto.getMessage());

boolean internalNote = dto.isInternalNote();
if (internalNote && (author == null || (author.getRole() != Role.ADMIN && author.getRole() != Role.INVESTIGATOR))) {
throw new ResponseStatusException(HttpStatus.FORBIDDEN, "Only investigators/admins can create internal notes");
}
comment.setInternalNote(internalNote);

TicketComment savedComment = ticketCommentRepository.save(comment);

return ticketCommentMapper.entityToViewDTO(savedComment);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

@Transactional(readOnly = true)
public List<CommentViewDTO> getComments(Long ticketId, User actor) {
List<TicketComment> all = ticketCommentRepository.findByTicketIdOrderByCreatedAtAsc(ticketId);

if (all.isEmpty()) {
ticketRepository.findById(ticketId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));
}

if (actor == null || actor.getRole() == Role.REPORTER) {
return all.stream()
.filter(comment -> !comment.isInternalNote())
.map(ticketCommentMapper::entityToViewDTO)
.toList();
}

return all.stream()
.map(ticketCommentMapper::entityToViewDTO)
.toList();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
185 changes: 153 additions & 32 deletions src/main/java/org/example/alfs/services/TicketService.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,56 +3,60 @@
import org.example.alfs.dto.ticket.TicketCreateDTO;
import org.example.alfs.dto.ticket.TicketViewDTO;
import org.example.alfs.entities.Ticket;
import org.example.alfs.entities.User;
import org.example.alfs.enums.Role;
import org.example.alfs.enums.TicketStatus;
import org.example.alfs.mapper.TicketMapper;
import org.example.alfs.repositories.TicketRepository;
import org.example.alfs.repositories.UserRepository;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.server.ResponseStatusException;

import java.util.List;
import java.util.Optional;
import java.util.Map;
import java.util.Set;

@Service
public class TicketService {


private final TicketRepository ticketRepository;
private final TicketMapper ticketMapper;
private final UserRepository userRepository;

public TicketService(TicketRepository ticketRepository, TicketMapper ticketMapper) {
public TicketService(TicketRepository ticketRepository, TicketMapper ticketMapper, UserRepository userRepository) {
this.ticketRepository = ticketRepository;
this.ticketMapper = ticketMapper;
this.userRepository = userRepository;
}

//createNewTicket
public TicketViewDTO createNewTicket(TicketCreateDTO ticketCreateDTO) {

Ticket ticket = new Ticket();
Ticket ticket = new Ticket();

ticket.setTitle(ticketCreateDTO.getTitle());
ticket.setDescription(ticketCreateDTO.getDescription());

Ticket save = ticketRepository.save(ticket);
Ticket savedTicket = ticketRepository.save(ticket);

return ticketMapper.entityToViewDTO(save);
return ticketMapper.entityToViewDTO(savedTicket);
}

// View by token
public TicketViewDTO getTicketByToken(String token) {

Ticket ticket = ticketRepository.findByReporterToken(token).
orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

return ticketMapper.entityToViewDTO(ticket);
}

//findById
public TicketViewDTO getTicketById(Long id) {

Ticket ticket = ticketRepository.findById(id).
orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));
return ticketMapper.entityToViewDTO(ticket);

return ticketMapper.entityToViewDTO(ticket);
}

//findByReporterId
Expand All @@ -64,32 +68,149 @@ public List<TicketViewDTO> getTicketsByReporterId(Long reporterId) {
}

//findByInvestigatorId
public List<TicketViewDTO> getTicketsByInvestigatorId(Long investigatorId){
return ticketRepository.findByInvestigatorId(investigatorId)
.stream()
.map(ticketMapper::entityToViewDTO)
.toList();
}
public List<TicketViewDTO> getTicketsByInvestigatorId(Long investigatorId) {
return ticketRepository.findByInvestigatorId(investigatorId)
.stream()
.map(ticketMapper::entityToViewDTO)
.toList();
}

//findByStatus
public List<TicketViewDTO> getTicketsByStatus(TicketStatus status) {
return ticketRepository.findByStatus(status)
.stream()
.map(ticketMapper::entityToViewDTO)
.toList();
}
public List<TicketViewDTO> getTicketsByStatus(TicketStatus status) {
return ticketRepository.findByStatus(status)
.stream()
.map(ticketMapper::entityToViewDTO)
.toList();
}

//findByStatusAndInvestigatorId
public List<TicketViewDTO> getTicketsByStatusAndInvestigator(
TicketStatus status,
Long investigatorId) {

return ticketRepository
.findByStatusAndInvestigatorId(status, investigatorId)
.stream()
.map(ticketMapper::entityToViewDTO)
.toList();
}
public List<TicketViewDTO> getTicketsByStatusAndInvestigator(
TicketStatus status,
Long investigatorId) {
return ticketRepository
.findByStatusAndInvestigatorId(status, investigatorId)
.stream()
.map(ticketMapper::entityToViewDTO)
.toList();
}

//findAll (pageable)

@Transactional
public TicketViewDTO updateTicketStatus(Long id, TicketStatus newStatus) {
// TODO: Check role? Is user is Admin or Investigator?
// Typ/Placeholder:
// if (user.getRole() != Role.ADMIN && user.getRole() != Role.INVESTIGATOR) {
// throw new AccessDeniedException("Only admins or investigators can update ticket status");
// }
// */

Ticket ticket = ticketRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

TicketStatus oldStatus = ticket.getStatus();

// No-op check
if (oldStatus == newStatus) {
return ticketMapper.entityToViewDTO(ticket);
}

Set<TicketStatus> allowedTransitions = ALLOWED_TRANSITIONS.getOrDefault(ticket.getStatus(), Set.of());

if (!allowedTransitions.contains(newStatus)) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Invalid ticket status transition: Cannot transition from " + ticket.getStatus() + " to " + newStatus);
}

if (newStatus == TicketStatus.IN_PROGRESS && ticket.getInvestigator() == null) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Cannot move ticket to IN_PROGRESS without an assigned investigator");
}

ticket.setStatus(newStatus);
Ticket savedTicket = ticketRepository.save(ticket);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +108 to +129

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e

echo "=== Locking/version support ==="
rg -n -C2 '@Version|@Lock|LockModeType|version\b' \
  src/main/java/org/example/alfs/entities \
  src/main/java/org/example/alfs/repositories

echo
echo "=== Ticket mutation methods ==="
rg -n -C3 'updateTicketStatus|assignInvestigator|unassignInvestigator' \
  src/main/java/org/example/alfs/services/TicketService.java

Repository: ithsjava25/project-backend-alfs

Length of output: 106


🏁 Script executed:

#!/bin/bash
set -e

echo "=== Ticket entity structure ==="
fd 'Ticket\.java$' src/main/java/org/example/alfs/entities --type f | head -5 | xargs -I {} sh -c 'echo "File: {}"; cat -n "{}"'

echo
echo "=== TicketRepository ==="
fd 'TicketRepository\.java$' src/main/java/org/example/alfs/repositories --type f | head -5 | xargs -I {} sh -c 'echo "File: {}"; cat -n "{}"'

echo
echo "=== Full TicketService.updateTicketStatus method ==="
rg -n -A 20 'public.*updateTicketStatus' src/main/java/org/example/alfs/services/TicketService.java

echo
echo "=== Full TicketService.assignInvestigator method ==="
rg -n -A 20 'public.*assignInvestigator' src/main/java/org/example/alfs/services/TicketService.java

echo
echo "=== Full TicketService.unassignInvestigator method ==="
rg -n -A 20 'public.*unassignInvestigator' src/main/java/org/example/alfs/services/TicketService.java

Repository: ithsjava25/project-backend-alfs

Length of output: 8064


These ticket mutations are race-prone and need concurrency control.

All three mutation methods (updateTicketStatus, assignInvestigator, unassignInvestigator) perform read-check-write sequences on the same Ticket row without concurrency guards. Two concurrent requests can both pass precondition checks and then overwrite each other—last-write-wins. For example, two calls to assignInvestigator() can both observe ticket.getInvestigator() == null, both pass the guard at line 159, and then both attempt to save, with the second request overwriting the first.

The Ticket entity has no @Version field for optimistic locking, and TicketRepository exposes no locked query methods, so there is currently no mechanism to prevent these collisions.

Add optimistic locking (@Version on Ticket) or use pessimistic locking when fetching the row in these mutation flows.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/alfs/services/TicketService.java` around lines 108
- 129, The three mutation flows (updateTicketStatus, assignInvestigator,
unassignInvestigator) perform read-check-write on the same Ticket and are
race-prone; fix by adding concurrency control: either add an optimistic lock
field to the Ticket entity (a long/int field annotated with `@Version`) and let
JPA detect concurrent updates on ticketRepository.save, or change the repository
access in those methods to fetch the row with a pessimistic lock (e.g., a new
repository method or using EntityManager.find with
LockModeType.PESSIMISTIC_WRITE) so the precondition checks and the subsequent
save happen under a lock; update the code paths in updateTicketStatus,
assignInvestigator, and unassignInvestigator to use the chosen locking strategy
and handle OptimisticLockException / lock-related exceptions appropriately.


// TODO: Audit log(-service?), auditLogService.log()
// Typ/Placeholder:
// auditLogService.log(ticket.getId(), user, "STATUS_CHANGED",
// "Status changed from " + oldStatus + " to " + newStatus);

return ticketMapper.entityToViewDTO(savedTicket);
}

// Bestäm vilka övergångar/transitions som är tillåtna
private static final Map<TicketStatus, Set<TicketStatus>> ALLOWED_TRANSITIONS = Map.of(
TicketStatus.OPEN, Set.of(TicketStatus.IN_PROGRESS),
TicketStatus.IN_PROGRESS, Set.of(TicketStatus.RESOLVED),
TicketStatus.RESOLVED, Set.of(TicketStatus.CLOSED, TicketStatus.IN_PROGRESS),
TicketStatus.CLOSED, Set.of()
);

@Transactional
public TicketViewDTO assignInvestigator(Long id, Long investigatorId) {
// TODO Check if user is admin?
// Typ/Placeholder:
// if (user.getRole() != Role.ADMIN) {
// throw new AccessDeniedException("Only admins can assign handlers");
// }
// */

Ticket ticket = ticketRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

if (ticket.getInvestigator() != null) {
throw new ResponseStatusException(HttpStatus.CONFLICT, ("Ticket already has an investigator assigned"));
}

if (ticket.getStatus() != TicketStatus.OPEN) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, ("Can only assign investigator to an OPEN ticket, current status: " + ticket.getStatus()));
}

User investigator = userRepository.findById(investigatorId)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Investigator not found"));

if (investigator.getRole() != Role.INVESTIGATOR) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, ("User is not an investigator"));
}

ticket.setInvestigator(investigator);
ticket.setStatus(TicketStatus.IN_PROGRESS);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Ticket savedTicket = ticketRepository.save(ticket);
// TODO: auditLogService.log()
// Typ/Placeholder:
// auditLogService.log(ticket.getId(), user, "ASSIGNED",
// "Ticket assigned to " + investigatorId);

return ticketMapper.entityToViewDTO(savedTicket);
}

@Transactional
public TicketViewDTO unassignInvestigator(Long id) {
// TODO: Check if user is admin?
// Typ/Placeholder:
// if (actor.getRole() != Role.ADMIN) {
// throw new AccessDeniedException("Only admins can unassign handlers");
// }
// */

Ticket ticket = ticketRepository.findById(id)
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Ticket not found"));

if (ticket.getInvestigator() == null) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, ("Ticket does not have an investigator assigned"));
}

if (ticket.getStatus() != TicketStatus.IN_PROGRESS) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, ("Can only unassign investigator from an IN_PROGRESS ticket, current status: " + ticket.getStatus()));
}

ticket.setInvestigator(null);
ticket.setStatus(TicketStatus.OPEN);
Ticket savedTicket = ticketRepository.save(ticket);
// TODO: auditLogService.log()
// Typ/Placeholder:
// auditLogService.log(ticket.getId(), user, "UNASSIGNED",
// "Ticket unassigned from " + investigatorId);

return ticketMapper.entityToViewDTO(savedTicket);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

}
Loading