-
Notifications
You must be signed in to change notification settings - Fork 0
Fix/작업 추가 및 의존성 추가 시 id 설정 #26
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
The head ref may contain hidden characters: "fix/\uC791\uC5C5-\uCD94\uAC00-\uBC0F-\uC758\uC874\uC131-\uCD94\uAC00-\uC2DC-ID-\uC124\uC815"
Changes from all commits
b5aee49
7fc59c5
a8b9771
b382825
378c3f9
d968073
28b3cae
9f255b5
9cf4c3a
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -64,6 +64,21 @@ public TaskPatch getTaskPatch() { | |||||||||
| .setDifficulty(difficulty); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * 업데이트 요청에서는 0 플레이스홀더 사용을 금지한다. | ||||||||||
| * (0은 새 Task 생성 시 자기 자신을 의미하므로 create 전용) | ||||||||||
| */ | ||||||||||
| public void validateNoPlaceholderForUpdate() { | ||||||||||
| if (taskId == null) { | ||||||||||
| return; | ||||||||||
| } | ||||||||||
| boolean hasPlaceholder = addDependencies.stream().anyMatch(this::hasPlaceholder) | ||||||||||
| || removeDependencies.stream().anyMatch(this::hasPlaceholder); | ||||||||||
| if (hasPlaceholder) { | ||||||||||
| throw new IllegalArgumentException("수정 요청에서는 fromId/toId에 0을 사용할 수 없습니다."); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public List<Dependency> getRemoveDependencies(Long selfId) { | ||||||||||
| return removeDependencies.stream() | ||||||||||
| .map(d -> new Dependency(ownerId, resolveId(d.getFromId(), selfId), resolveId(d.getToId(), selfId))) | ||||||||||
|
|
@@ -77,9 +92,14 @@ public List<Dependency> getAddDependencies(Long selfId) { | |||||||||
| } | ||||||||||
|
|
||||||||||
| private Long resolveId(Long rawId, Long selfId) { | ||||||||||
| if (rawId == null) { | ||||||||||
| if (rawId == 0L) { | ||||||||||
| return selfId; | ||||||||||
| } | ||||||||||
| return rawId; | ||||||||||
|
Comment on lines
94
to
98
|
||||||||||
| } | ||||||||||
|
|
||||||||||
| private boolean hasPlaceholder(DependencyDto dto) { | ||||||||||
| return dto.getFromId() != null && dto.getFromId() == 0L | ||||||||||
| || dto.getToId() != null && dto.getToId() == 0L; | ||||||||||
|
Comment on lines
+102
to
+103
|
||||||||||
| return dto.getFromId() != null && dto.getFromId() == 0L | |
| || dto.getToId() != null && dto.getToId() == 0L; | |
| return (dto.getFromId() != null && dto.getFromId() == 0L) | |
| || (dto.getToId() != null && dto.getToId() == 0L); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,76 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| package me.gg.pinit.pinittask.interfaces.dto; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import io.swagger.v3.oas.annotations.media.Schema; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.validation.Valid; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.Max; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.Min; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.NotBlank; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.validation.constraints.NotNull; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import me.gg.pinit.pinittask.application.datetime.DateTimeUtils; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import me.gg.pinit.pinittask.application.schedule.dto.DependencyDto; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import me.gg.pinit.pinittask.application.task.dto.TaskDependencyAdjustCommand; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import me.gg.pinit.pinittask.interfaces.utils.FibonacciDifficulty; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Optional; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| public record TaskCreateRequest( | ||||||||||||||||||||||||||||||||||||||||||||||||
| @NotBlank | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Schema(description = "작업 제목", example = "스터디 준비") | ||||||||||||||||||||||||||||||||||||||||||||||||
| String title, | ||||||||||||||||||||||||||||||||||||||||||||||||
| @NotBlank | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Schema(description = "작업 설명", example = "다음 주 발표 자료 정리") | ||||||||||||||||||||||||||||||||||||||||||||||||
| String description, | ||||||||||||||||||||||||||||||||||||||||||||||||
| @NotNull | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Schema(description = "마감 기한", example = "{\"dateTime\":\"2024-03-01T18:00:00\",\"zoneId\":\"Asia/Seoul\"}") | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Valid | ||||||||||||||||||||||||||||||||||||||||||||||||
| DateTimeWithZone dueDate, | ||||||||||||||||||||||||||||||||||||||||||||||||
| @NotNull | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Min(1) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Max(9) | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Schema(description = "중요도 (1~9)", example = "5") | ||||||||||||||||||||||||||||||||||||||||||||||||
| Integer importance, | ||||||||||||||||||||||||||||||||||||||||||||||||
| @NotNull | ||||||||||||||||||||||||||||||||||||||||||||||||
| @FibonacciDifficulty | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Schema(description = "난이도 (피보나치 수: 1,2,3,5,8,13,21)", example = "5") | ||||||||||||||||||||||||||||||||||||||||||||||||
| Integer difficulty, | ||||||||||||||||||||||||||||||||||||||||||||||||
| @Schema(description = "추가할 의존 관계 목록 (생성 시 각 항목에 fromId 또는 toId 중 하나는 0)") | ||||||||||||||||||||||||||||||||||||||||||||||||
| List<@Valid DependencyRequest> addDependencies | ||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| public TaskDependencyAdjustCommand toCommand(Long taskId, Long ownerId, DateTimeUtils dateTimeUtils) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| validateMustContainSelfPlaceholder(addDependencies); | ||||||||||||||||||||||||||||||||||||||||||||||||
| List<DependencyDto> remove = List.of(); // 생성 시 remove는 허용하지 않음 | ||||||||||||||||||||||||||||||||||||||||||||||||
| List<DependencyDto> add = toDependencyDtos(addDependencies); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return new TaskDependencyAdjustCommand( | ||||||||||||||||||||||||||||||||||||||||||||||||
| taskId, | ||||||||||||||||||||||||||||||||||||||||||||||||
| ownerId, | ||||||||||||||||||||||||||||||||||||||||||||||||
| title, | ||||||||||||||||||||||||||||||||||||||||||||||||
| description, | ||||||||||||||||||||||||||||||||||||||||||||||||
| dateTimeUtils.toZonedDateTime(dueDate.dateTime(), dueDate.zoneId()), | ||||||||||||||||||||||||||||||||||||||||||||||||
| importance, | ||||||||||||||||||||||||||||||||||||||||||||||||
| difficulty, | ||||||||||||||||||||||||||||||||||||||||||||||||
| remove, | ||||||||||||||||||||||||||||||||||||||||||||||||
| add | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private List<DependencyDto> toDependencyDtos(List<DependencyRequest> requests) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return Optional.ofNullable(requests) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .orElseGet(ArrayList::new) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .stream() | ||||||||||||||||||||||||||||||||||||||||||||||||
| .map(request -> new DependencyDto(null, request.fromId(), request.toId())) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .toList(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| private void validateMustContainSelfPlaceholder(List<DependencyRequest> dependencies) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Optional.ofNullable(dependencies) | ||||||||||||||||||||||||||||||||||||||||||||||||
| .orElseGet(ArrayList::new) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
+68
|
||||||||||||||||||||||||||||||||||||||||||||||||
| return Optional.ofNullable(requests) | |
| .orElseGet(ArrayList::new) | |
| .stream() | |
| .map(request -> new DependencyDto(null, request.fromId(), request.toId())) | |
| .toList(); | |
| } | |
| private void validateMustContainSelfPlaceholder(List<DependencyRequest> dependencies) { | |
| Optional.ofNullable(dependencies) | |
| .orElseGet(ArrayList::new) | |
| return emptyIfNull(requests) | |
| .stream() | |
| .map(request -> new DependencyDto(null, request.fromId(), request.toId())) | |
| .toList(); | |
| } | |
| private static <T> List<T> emptyIfNull(List<T> list) { | |
| return Optional.ofNullable(list) | |
| .orElseGet(ArrayList::new); | |
| } | |
| private void validateMustContainSelfPlaceholder(List<DependencyRequest> dependencies) { | |
| emptyIfNull(dependencies) |
Copilot
AI
Jan 22, 2026
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.
(1) 문제점: validateMustContainSelfPlaceholder의 로직이 부정확합니다. 현재 코드는 "fromId와 toId가 모두 0이 아닌 경우" 예외를 던지지만, 이는 "둘 다 0인 경우"를 허용하게 됩니다.
(2) 영향: fromId=0, toId=0인 의존성이 허용되어 자기 자신에서 자기 자신으로의 의존성이 생성될 수 있습니다. 이는 의미 없는 관계를 생성하게 됩니다.
(3) 수정 제안: 로직을 수정하여 "정확히 하나만 0이어야 함"을 검증하도록 변경하세요. 조건을 (dep.fromId() == 0L) XOR (dep.toId() == 0L)로 검증하거나, 둘 다 0인 경우와 둘 다 0이 아닌 경우 모두 예외를 던지도록 수정하세요.
| if (dep.fromId() != 0L && dep.toId() != 0L) { | |
| throw new IllegalArgumentException("작업 생성 시 의존 관계에는 fromId 또는 toId 중 하나가 0이어야 합니다."); | |
| boolean hasFromPlaceholder = dep.fromId() == 0L; | |
| boolean hasToPlaceholder = dep.toId() == 0L; | |
| if (!(hasFromPlaceholder ^ hasToPlaceholder)) { | |
| throw new IllegalArgumentException("작업 생성 시 의존 관계에는 fromId 또는 toId 중 정확히 하나만 0이어야 합니다."); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||
| package me.gg.pinit.pinittask.application.task.dto; | ||||||
|
|
||||||
| import me.gg.pinit.pinittask.application.schedule.dto.DependencyDto; | ||||||
| import me.gg.pinit.pinittask.domain.dependency.model.Dependency; | ||||||
| import org.junit.jupiter.api.Test; | ||||||
|
|
||||||
| import java.time.ZonedDateTime; | ||||||
| import java.util.List; | ||||||
|
|
||||||
| import static org.assertj.core.api.Assertions.assertThat; | ||||||
|
|
||||||
| class TaskDependencyAdjustCommandTest { | ||||||
|
|
||||||
| @Test | ||||||
| void resolvesSelfPlaceholderForZeroIds() { | ||||||
| Long ownerId = 1L; | ||||||
| Long selfId = 99L; | ||||||
|
||||||
| Long selfId = 99L; | |
| long selfId = 99L; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package me.gg.pinit.pinittask.interfaces.dto; | ||
|
|
||
| import me.gg.pinit.pinittask.application.datetime.DateTimeUtils; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.time.ZoneId; | ||
| import java.util.List; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
|
||
| class TaskCreateRequestTest { | ||
|
|
||
| @Test | ||
| void toCommand_requiresPlaceholderZeroForEachDependency() { | ||
| TaskCreateRequest req = new TaskCreateRequest( | ||
| "t", | ||
| "d", | ||
| new DateTimeWithZone(LocalDateTime.now(), ZoneId.of("UTC")), | ||
| 5, | ||
| 3, | ||
| List.of(new DependencyRequest(10L, 20L)) // neither is 0 | ||
| ); | ||
|
|
||
| assertThrows(IllegalArgumentException.class, | ||
| () -> req.toCommand(null, 1L, new DateTimeUtils())); | ||
| } | ||
|
Comment on lines
+14
to
+27
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| import me.gg.pinit.pinittask.domain.schedule.model.ScheduleType; | ||
| import me.gg.pinit.pinittask.infrastructure.events.RabbitEventPublisher; | ||
| import me.gg.pinit.pinittask.interfaces.dto.DateTimeWithZone; | ||
| import me.gg.pinit.pinittask.interfaces.dto.TaskRequest; | ||
| import me.gg.pinit.pinittask.interfaces.dto.TaskCreateRequest; | ||
| import me.gg.pinit.pinittask.interfaces.dto.TaskScheduleRequest; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
@@ -59,13 +59,12 @@ void setUpMember() { | |
|
|
||
| @Test | ||
| void taskLifecycle_create_retrieve_list_cursor_complete_reopen_delete() throws Exception { | ||
| TaskRequest createRequest = new TaskRequest( | ||
| TaskCreateRequest createRequest = new TaskCreateRequest( | ||
| "리포트 작성", | ||
| "주간 리포트 초안 작성", | ||
| new DateTimeWithZone(LocalDateTime.of(2024, 4, 1, 18, 0), MEMBER_ZONE), | ||
| 5, | ||
| 3, | ||
| List.of(), | ||
| List.of() | ||
| ); | ||
|
Comment on lines
+62
to
69
|
||
|
|
||
|
|
@@ -168,7 +167,6 @@ void createTask_validationErrors_returnDetailedErrors() throws Exception { | |
| "dueDate": null, | ||
| "importance": 0, | ||
| "difficulty": 4, | ||
| "removeDependencies": [], | ||
| "addDependencies": [] | ||
| } | ||
| """; | ||
|
|
||
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.
(1) 문제점: 문서의 설명이 불완전합니다. "removeDependencies는 비워두어야"로 끝나서 문장이 완결되지 않았습니다.
(2) 영향: API 사용자가 removeDependencies 필드의 정확한 사용 방법을 이해하기 어렵습니다.
(3) 수정 제안: 문장을 완성하여 "removeDependencies는 비워두어야 합니다" 또는 "removeDependencies는 생성 시 지원되지 않습니다"와 같이 명확하게 작성하세요.