Skip to content

Commit dfcce63

Browse files
authored
[Feature] Add optimistic locking based on versions for tasks and task assignments (#571)
* feat: add new migration for task and assignment versioning * feat: add new exception for stale task versions * feat: add version field to TaskBody entity * feat: add version field to dtos * feat: add version check into service methods * feat: add new exception for stale assignment version * feat: add version field to TaskAssignment entity * feat: add version field to dtos * feat: add version check to service methods * test: update unit tests * docs: update OpenAPI documentation * fix: remove unused import * fix: audit owner changes with updated_at and updated_by fields * test: update unit tests * fix: coderabbitai issue * fix: coderabbitai issue
1 parent c997ef3 commit dfcce63

22 files changed

Lines changed: 268 additions & 70 deletions

src/main/java/com/itasocialacademy/oitassist/task/config/file.txt

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/main/java/com/itasocialacademy/oitassist/task/controller/TaskController.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ public ResponseEntity<PageResponse<TaskResponseDTO>> getMyTasks(
127127
schema = @Schema(implementation = ErrorResponse.class))),
128128
@ApiResponse(responseCode = "404", description = "Task not found",
129129
content = @Content(mediaType = "application/json",
130-
schema = @Schema(implementation = ErrorResponse.class)))
130+
schema = @Schema(implementation = ErrorResponse.class))),
131+
@ApiResponse(responseCode = "409",
132+
description = "Conflict — the entity was modified by another request since it was last read "
133+
+ "(stale version)",
134+
content = @Content(mediaType = "application/json", schema = @Schema(implementation = ErrorResponse.class)))
131135
})
132136
@PutMapping("/{taskId}")
133137
@PreAuthorize("hasAnyRole('ADMIN', 'ORG')")
@@ -152,7 +156,11 @@ public ResponseEntity<TaskResponseDTO> updateTask(@PathVariable Long taskId,
152156
schema = @Schema(implementation = ErrorResponse.class))),
153157
@ApiResponse(responseCode = "404", description = "Task or user not found",
154158
content = @Content(mediaType = "application/json",
155-
schema = @Schema(implementation = ErrorResponse.class)))
159+
schema = @Schema(implementation = ErrorResponse.class))),
160+
@ApiResponse(responseCode = "409",
161+
description = "Conflict — the entity was modified by another request since it was last read "
162+
+ "(stale version)",
163+
content = @Content(mediaType = "application/json", schema = @Schema(implementation = ErrorResponse.class)))
156164
})
157165
@PatchMapping("/{taskId}/add-owner")
158166
@PreAuthorize("hasRole('ADMIN')")
@@ -177,7 +185,11 @@ public ResponseEntity<TaskResponseDTO> addOwner(@PathVariable Long taskId,
177185
schema = @Schema(implementation = ErrorResponse.class))),
178186
@ApiResponse(responseCode = "404", description = "Task or user not found",
179187
content = @Content(mediaType = "application/json",
180-
schema = @Schema(implementation = ErrorResponse.class)))
188+
schema = @Schema(implementation = ErrorResponse.class))),
189+
@ApiResponse(responseCode = "409",
190+
description = "Conflict — the entity was modified by another request since it was last read "
191+
+ "(stale version)",
192+
content = @Content(mediaType = "application/json", schema = @Schema(implementation = ErrorResponse.class)))
181193
})
182194
@PatchMapping("/{taskId}/remove-owner")
183195
@PreAuthorize("hasRole('ADMIN')")

src/main/java/com/itasocialacademy/oitassist/task/dao/model/TaskBody.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ public class TaskBody {
4646
@Column(name = "updated_at")
4747
private Instant updatedAt;
4848

49+
@Version
50+
@Column(name = "version", nullable = false)
51+
private Long version;
52+
4953
@OneToMany(
5054
mappedBy = "task",
5155
cascade = CascadeType.ALL,

src/main/java/com/itasocialacademy/oitassist/task/dto/request/AddOwnerRequestDTO.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
import io.swagger.v3.oas.annotations.media.Schema;
44
import jakarta.validation.constraints.Email;
55
import jakarta.validation.constraints.NotBlank;
6+
import jakarta.validation.constraints.NotNull;
67

78
@Schema(description = "DTO for adding task owner")
89
public record AddOwnerRequestDTO(
910
@Schema(
1011
description = "Email address of the new task owner. The user must have ADMIN or ORG role",
1112
example = "example@mail.com",
12-
requiredMode = Schema.RequiredMode.REQUIRED) @NotBlank @Email String newOwnerEmail) {
13+
requiredMode = Schema.RequiredMode.REQUIRED) @NotBlank @Email String newOwnerEmail,
14+
15+
@Schema(description = "Optimistic locking version; must be echoed back on updates",
16+
requiredMode = Schema.RequiredMode.REQUIRED) @NotNull Long version) {
1317
}

src/main/java/com/itasocialacademy/oitassist/task/dto/request/RemoveOwnerRequestDTO.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33
import io.swagger.v3.oas.annotations.media.Schema;
44
import jakarta.validation.constraints.Email;
55
import jakarta.validation.constraints.NotBlank;
6+
import jakarta.validation.constraints.NotNull;
67

78
@Schema(description = "DTO for removing task owner")
89
public record RemoveOwnerRequestDTO(
910
@Schema(
1011
description = "Email address of the task owner.",
1112
example = "example@mail.com",
12-
requiredMode = Schema.RequiredMode.REQUIRED) @NotBlank @Email String ownerEmail) {
13+
requiredMode = Schema.RequiredMode.REQUIRED) @NotBlank @Email String ownerEmail,
14+
15+
@Schema(description = "Optimistic locking version; must be echoed back on updates",
16+
requiredMode = Schema.RequiredMode.REQUIRED) @NotNull Long version) {
1317
}

src/main/java/com/itasocialacademy/oitassist/task/dto/request/UpdateTaskRequestDTO.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import io.swagger.v3.oas.annotations.media.Schema;
44
import jakarta.validation.constraints.NotBlank;
5+
import jakarta.validation.constraints.NotNull;
56
import java.util.List;
67

78
@Schema(description = "DTO for updating an already existing task")
@@ -21,5 +22,7 @@ public record UpdateTaskRequestDTO(
2122
@Schema(
2223
description = "File ids to be detached from task",
2324
example = "[52]",
24-
requiredMode = Schema.RequiredMode.NOT_REQUIRED) List<Long> removedFileIds) {
25+
requiredMode = Schema.RequiredMode.NOT_REQUIRED) List<Long> removedFileIds,
26+
@Schema(description = "Optimistic locking version; must be echoed back on updates",
27+
requiredMode = Schema.RequiredMode.REQUIRED) @NotNull Long version) {
2528
}

src/main/java/com/itasocialacademy/oitassist/task/dto/response/TaskResponseDTO.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,7 @@ public record TaskResponseDTO(
3434

3535
@Schema(
3636
description = "Ids of task's current owners",
37-
example = "[1,2,3]") Set<Long> ownerIds) {
37+
example = "[1,2,3]") Set<Long> ownerIds,
38+
39+
@Schema(description = "Optimistic locking version; must be echoed back on updates") Long version) {
3840
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package com.itasocialacademy.oitassist.task.exceptions;
2+
3+
import com.itasocialacademy.oitassist.core.enums.ErrorCode;
4+
import com.itasocialacademy.oitassist.core.exceptions.BusinessException;
5+
6+
public class StaleTaskVersionException extends BusinessException {
7+
public StaleTaskVersionException(Long taskId) {
8+
super("Task with id %d has been modified by another user. ".formatted(taskId),
9+
ErrorCode.ENTITY_VERSION_CONFLICT);
10+
}
11+
}

src/main/java/com/itasocialacademy/oitassist/task/service/TaskServiceImpl.java

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.itasocialacademy.oitassist.task.dto.request.RemoveOwnerRequestDTO;
2323
import com.itasocialacademy.oitassist.task.dto.request.UpdateTaskRequestDTO;
2424
import com.itasocialacademy.oitassist.task.dto.response.TaskResponseDTO;
25+
import com.itasocialacademy.oitassist.task.exceptions.StaleTaskVersionException;
2526
import com.itasocialacademy.oitassist.task.exceptions.TaskAccessRestrictedException;
2627
import com.itasocialacademy.oitassist.task.exceptions.TaskNotFoundException;
2728
import com.itasocialacademy.oitassist.task.mapper.TaskBodyMapper;
@@ -30,6 +31,7 @@
3031
import com.itasocialacademy.oitassist.user.api.interfaces.UserFacade;
3132
import com.itasocialacademy.oitassist.user.dao.enums.Role;
3233
import com.itasocialacademy.oitassist.user.exceptions.UserNotFoundException;
34+
import java.time.Instant;
3335
import java.util.*;
3436
import java.util.stream.Collectors;
3537
import lombok.RequiredArgsConstructor;
@@ -128,10 +130,12 @@ public TaskResponseDTO updateTask(Long taskId, UpdateTaskRequestDTO requestDTO)
128130
.map(o -> o.getId().getOwnerId()).collect(Collectors.toSet()),
129131
existingTask.getId());
130132

133+
checkTaskVersion(existingTask.getVersion(), requestDTO.version(), existingTask.getId());
134+
131135
existingTask.setTitle(requestDTO.title());
132136
existingTask.setDescription(requestDTO.description());
133137

134-
TaskBody updatedTask = taskBodyRepository.save(existingTask);
138+
TaskBody updatedTask = taskBodyRepository.saveAndFlush(existingTask);
135139
log.debug("Updated Task: Id {}, Title - {}", updatedTask.getId(), updatedTask.getTitle());
136140

137141
Long currentUserId = securityFacade.getCurrentUserId()
@@ -161,6 +165,8 @@ public TaskResponseDTO addTaskOwner(Long taskId, AddOwnerRequestDTO addOwnerRequ
161165
throw new ValidationException("Provided user is not ADMIN nor ORG", ErrorCode.COMMON_VALIDATION_FAILED);
162166
}
163167

168+
checkTaskVersion(task.getVersion(), addOwnerRequest.version(), task.getId());
169+
164170
if (task.getOwners().stream()
165171
.anyMatch(owner -> owner.getId().getOwnerId().equals(userDetails.id()))) {
166172
return getResponse(task);
@@ -171,6 +177,7 @@ public TaskResponseDTO addTaskOwner(Long taskId, AddOwnerRequestDTO addOwnerRequ
171177
.build();
172178

173179
task.addOwner(owner);
180+
auditOwnersUpdate(task);
174181

175182
log.debug("User {} added to task`s {} owners", userDetails.id(), task.getId());
176183

@@ -190,20 +197,24 @@ public TaskResponseDTO removeTaskOwner(Long taskId, RemoveOwnerRequestDTO remove
190197
UserAuthDetails userDetails = userFacade.findByEmail(removeOwnerRequest.ownerEmail())
191198
.orElseThrow(UserNotFoundException::new);
192199

200+
checkTaskVersion(task.getVersion(), removeOwnerRequest.version(), task.getId());
201+
193202
Optional<TaskOwner> toRemove = task.getOwners().stream()
194203
.filter(o -> o.getId().getOwnerId().equals(userDetails.id())).findFirst();
195204

196-
if (toRemove.isPresent()) {
197-
if (task.getOwners().size() == 1) {
198-
throw new ValidationException(
199-
"Cannot remove the last owner of a task",
200-
ErrorCode.COMMON_VALIDATION_FAILED);
201-
}
202-
task.removeOwner(toRemove.get());
203-
} else {
205+
if (toRemove.isEmpty()) {
204206
return getResponse(task);
205207
}
206208

209+
if (task.getOwners().size() == 1) {
210+
throw new ValidationException(
211+
"Cannot remove the last owner of a task",
212+
ErrorCode.COMMON_VALIDATION_FAILED);
213+
}
214+
215+
task.removeOwner(toRemove.get());
216+
auditOwnersUpdate(task);
217+
207218
log.debug("User {} removed from task`s {} owners", userDetails.id(), task.getId());
208219

209220
return getResponse(task);
@@ -333,4 +344,16 @@ private String getNormalizedSearch(String search) {
333344
.replace("%", "\\%")
334345
.replace("_", "\\_");
335346
}
347+
348+
private void checkTaskVersion(Long actualVersion, Long providedVersion, Long taskId) {
349+
if (!Objects.equals(actualVersion, providedVersion)) {
350+
throw new StaleTaskVersionException(taskId);
351+
}
352+
}
353+
354+
private void auditOwnersUpdate(TaskBody taskBody) {
355+
taskBody.setUpdatedAt(Instant.now());
356+
taskBody.setUpdatedBy(securityFacade.getCurrentUserId().orElseThrow(UserNotFoundException::new));
357+
taskBodyRepository.saveAndFlush(taskBody);
358+
}
336359
}

src/main/java/com/itasocialacademy/oitassist/taskassignment/controller/AssignmentController.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,11 @@ public ResponseEntity<List<LinkedToursResponseDTO>> getLinkedTours(@PathVariable
148148
schema = @Schema(implementation = ErrorResponse.class))),
149149
@ApiResponse(responseCode = "404", description = "Task assignment not found",
150150
content = @Content(mediaType = "application/json",
151-
schema = @Schema(implementation = ErrorResponse.class)))
151+
schema = @Schema(implementation = ErrorResponse.class))),
152+
@ApiResponse(responseCode = "409",
153+
description = "Conflict — the entity was modified by another request since it was last read "
154+
+ "(stale version)",
155+
content = @Content(mediaType = "application/json", schema = @Schema(implementation = ErrorResponse.class)))
152156
})
153157
@PatchMapping("/task-assignments/{assignmentId}")
154158
@PreAuthorize("hasAnyRole('ADMIN', 'ORG')")

0 commit comments

Comments
 (0)