Fix/작업 추가 및 의존성 추가 시 id 설정 by GoGradually · Pull Request #26 · Pinit-Scheduler/pinit-task

GoGradually · 2026-01-22T15:11:09Z

변경된 점

TaskUpdate와 TaskCreate DTO 분리
TaskCreate에서 0 사용 시 자기 자신을 의미하도록 설정

Copilot

Pull request overview

이 PR은 작업 생성과 수정 시 의존성 관계 설정의 문제를 해결합니다. TaskRequest DTO를 TaskCreateRequest와 TaskUpdateRequest로 분리하고, 작업 생성 시 아직 생성되지 않은 작업 자신을 참조하기 위해 0을 플레이스홀더로 사용하는 기능을 구현했습니다.

Changes:

TaskRequest를 TaskCreateRequest와 TaskUpdateRequest로 분리하여 생성과 수정의 검증 로직을 분리
작업 생성 시 의존성에 0 사용 시 자기 자신의 ID로 치환하는 로직 추가
작업 수정 시 0 플레이스홀더 사용을 금지하는 검증 로직 추가

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
TaskCreateRequest.java	작업 생성 전용 DTO 추가, 의존성에 0 플레이스홀더 사용 필수 검증
TaskUpdateRequest.java	TaskRequest에서 리네임, 의존성에 0 사용 금지 검증 추가
TaskControllerV1.java	생성/수정 엔드포인트에서 분리된 DTO 사용
TaskDependencyAdjustCommand.java	0을 자기 자신의 ID로 치환하는 로직 및 수정 시 0 검증 로직 추가
TaskAdjustmentService.java	수정 시 플레이스홀더 검증 호출 추가
DependencyRequest.java	Schema 설명에 0 사용 규칙 문서화
README.md	Task 의존성 페이로드 사용 규칙 문서화
TaskCreateRequestTest.java	생성 시 검증 로직 테스트 추가
TaskDependencyAdjustCommandTest.java	플레이스홀더 치환 및 검증 테스트 추가
TaskAdjustmentServiceTest.java	수정 시 플레이스홀더 거부 테스트 추가
TaskControllerV1IntegrationTest.java	DTO 변경에 따른 테스트 업데이트

Comments suppressed due to low confidence (1)

src/main/java/me/gg/pinit/pinittask/interfaces/dto/TaskUpdateRequest.java:69

(1) 문제점: validateNoPlaceholder 메서드에서 dependencies가 null일 때 불필요하게 빈 ArrayList를 생성합니다. Optional.ofNullable().orElseGet(ArrayList::new).forEach()는 null일 때도 빈 리스트를 생성하지만, forEach는 아무 작업도 수행하지 않으므로 리스트 생성이 낭비입니다.

(2) 영향: 성능에 미미한 영향이 있지만, 불필요한 객체 생성으로 인해 코드 효율성이 떨어집니다.

(3) 수정 제안: Optional.ofNullable(dependencies).ifPresent(deps -> deps.forEach(...))로 변경하거나, dependencies가 null인 경우 early return하도록 수정하세요.

Copilot · 2026-01-22T15:16:35Z

+                    if (dep.fromId() != 0L && dep.toId() != 0L) {
+                        throw new IllegalArgumentException("작업 생성 시 의존 관계에는 fromId 또는 toId 중 하나가 0이어야 합니다.");


(1) 문제점: validateMustContainSelfPlaceholder의 로직이 부정확합니다. 현재 코드는 "fromId와 toId가 모두 0이 아닌 경우" 예외를 던지지만, 이는 "둘 다 0인 경우"를 허용하게 됩니다.

(2) 영향: fromId=0, toId=0인 의존성이 허용되어 자기 자신에서 자기 자신으로의 의존성이 생성될 수 있습니다. 이는 의미 없는 관계를 생성하게 됩니다.

(3) 수정 제안: 로직을 수정하여 "정확히 하나만 0이어야 함"을 검증하도록 변경하세요. 조건을 (dep.fromId() == 0L) XOR (dep.toId() == 0L)로 검증하거나, 둘 다 0인 경우와 둘 다 0이 아닌 경우 모두 예외를 던지도록 수정하세요.

Suggested change

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이어야 합니다.");

Copilot · 2026-01-22T15:16:35Z

+        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)


(1) 문제점: TaskCreateRequest와 TaskUpdateRequest 사이에 중복된 코드가 존재합니다. toDependencyDtos 메서드가 두 클래스에서 동일하게 구현되어 있습니다.

(2) 영향: 코드 중복으로 인해 유지보수성이 저하되고, 한쪽만 수정될 경우 불일치가 발생할 수 있습니다.

(3) 수정 제안: 공통 유틸리티 클래스나 헬퍼 메서드로 추출하여 재사용하도록 리팩토링하세요.

Suggested change

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 · 2026-01-22T15:16:36Z

+    @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()));
+    }


(1) 문제점: 테스트가 fromId와 toId가 모두 0인 경우를 검증하지 않습니다. 현재 validateMustContainSelfPlaceholder 로직의 버그로 인해 이 케이스가 허용될 수 있습니다.

(2) 영향: 자기 자신에서 자기 자신으로의 의존성이 생성되는 잘못된 케이스를 탐지하지 못합니다.

(3) 수정 제안: fromId=0, toId=0인 의존성에 대한 테스트 케이스를 추가하여 예외가 발생하는지 검증하세요.

Copilot · 2026-01-22T15:16:36Z

+  * Task 의존성 페이로드: **생성 시** 각 의존 관계마다 `fromId` 또는 `toId` 중 하나는 반드시 `0`(새로 생성될 자기 자신)이어야 하며, `removeDependencies`는 비워두어야
+    합니다. **수정 시에는 0을 사용할 수 없습니다.**
 * **작업 → 일정 복사**: `POST /v1/tasks/{taskId}/schedules`로 기존 작업을 지정 시각에 일정으로 등록할 수 있습니다.


(1) 문제점: 문서의 설명이 불완전합니다. "removeDependencies는 비워두어야"로 끝나서 문장이 완결되지 않았습니다.

(2) 영향: API 사용자가 removeDependencies 필드의 정확한 사용 방법을 이해하기 어렵습니다.

(3) 수정 제안: 문장을 완성하여 "removeDependencies는 비워두어야 합니다" 또는 "removeDependencies는 생성 시 지원되지 않습니다"와 같이 명확하게 작성하세요.

Suggested change

* Task 의존성 페이로드: **생성 시** 각 의존 관계마다 `fromId` 또는 `toId` 중 하나는 반드시 `0`(새로 생성될 자기 자신)이어야 하며, `removeDependencies`는 비워두어야

합니다. **수정 시에는 0을 사용할 수 없습니다.**

* **작업 → 일정 복사**: `POST /v1/tasks/{taskId}/schedules`로 기존 작업을 지정 시각에 일정으로 등록할 수 있습니다.

* Task 의존성 페이로드: **생성 시** 각 의존 관계마다 `fromId` 또는 `toId` 중 하나는 반드시 `0`(새로 생성될 자기 자신)이어야 하며, `removeDependencies`는 비워두어야 합니다. **수정 시에는 0을 사용할 수 없습니다.**

* **작업 → 일정 복사**: `POST /v1/tasks/{taskId}/schedules`로 기존 작업을 지정 시각에 일정으로 등록할 수 있습니다.

Copilot · 2026-01-22T15:16:36Z

    private Long resolveId(Long rawId, Long selfId) {
-        if (rawId == null) {
+        if (rawId == 0L) {
            return selfId;
        }
        return rawId;


(1) 문제점: null 값 처리가 누락되어 있습니다. rawId가 null인 경우 NullPointerException이 발생할 수 있습니다.

(2) 영향: DependencyDto의 fromId 또는 toId가 null일 경우 런타임 예외가 발생하여 애플리케이션이 중단될 수 있습니다.

(3) 수정 제안: null 체크를 추가해야 합니다. rawId가 null인 경우를 먼저 확인한 후 0L과 비교하도록 수정하세요.

Copilot · 2026-01-22T15:16:37Z

+    @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()));
+    }


(1) 문제점: 테스트가 생성 시 의존성이 비어있는 경우를 검증하지 않습니다. addDependencies가 빈 리스트이거나 null인 경우에도 정상 동작해야 하는지 확인이 필요합니다.

(2) 영향: 엣지 케이스에 대한 검증이 부족하여 예상치 못한 동작이 발생할 수 있습니다.

(3) 수정 제안: addDependencies가 비어있거나 null인 경우의 테스트 케이스를 추가하여 정상 동작을 검증하세요.

Copilot · 2026-01-22T15:16:37Z

+        return dto.getFromId() != null && dto.getFromId() == 0L
+                || dto.getToId() != null && dto.getToId() == 0L;


(1) 문제점: 연산자 우선순위 문제로 인해 의도와 다르게 동작할 수 있습니다. || 연산자가 && 연산자보다 낮은 우선순위를 가지므로, 현재 코드는 (dto.getFromId() != null && dto.getFromId() == 0L) || (dto.getToId() != null && dto.getToId() == 0L)로 해석되지만 가독성이 떨어집니다.

(2) 영향: 코드 리뷰어나 유지보수 담당자가 의도를 오해할 수 있으며, 향후 수정 시 논리 오류가 발생할 가능성이 있습니다.

(3) 수정 제안: 명확성을 위해 괄호를 추가하여 의도를 명확히 표현하세요. 예: return (dto.getFromId() != null && dto.getFromId() == 0L) || (dto.getToId() != null && dto.getToId() == 0L);

Suggested change

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);

Copilot · 2026-01-22T15:16:37Z

+        TaskCreateRequest createRequest = new TaskCreateRequest(
                "리포트 작성",
                "주간 리포트 초안 작성",
                new DateTimeWithZone(LocalDateTime.of(2024, 4, 1, 18, 0), MEMBER_ZONE),
                5,
                3,
-                List.of(),
                List.of()
        );


(1) 문제점: 통합 테스트가 작업 생성 시 의존성에 0을 사용하는 핵심 기능을 검증하지 않습니다. 현재 테스트는 빈 의존성 리스트만 사용하고 있습니다.

(2) 영향: PR의 주요 기능인 "작업 생성 시 0을 자기 자신의 플레이스홀더로 사용"하는 기능이 통합 테스트로 검증되지 않아, 실제 환경에서의 동작을 보장할 수 없습니다.

(3) 수정 제안: addDependencies에 fromId 또는 toId가 0인 의존성을 포함하는 테스트 케이스를 추가하여 실제로 생성된 작업의 ID로 치환되는지 검증하세요.

Copilot · 2026-01-22T15:16:37Z

+    @Test
+    void resolvesSelfPlaceholderForZeroIds() {
+        Long ownerId = 1L;
+        Long selfId = 99L;


The variable 'selfId' is only assigned values of primitive type and is never 'null', but it is declared with the boxed type 'Long'.

Suggested change

Long selfId = 99L;

long selfId = 99L;

GoGradually added 9 commits January 23, 2026 00:08

feat: 작업 생성 및 수정 요청 DTO 추가

b5aee49

fix: updateTask에서 플레이스홀더 0 사용 시 예외 처리 추가

7fc59c5

fix: 업데이트 요청에서 0 플레이스홀더 사용 금지 로직 추가

a8b9771

test: TaskDependencyAdjustCommand에 대한 테스트 추가

b382825

test: TaskCreateRequest의 의존성에 대한 플레이스홀더 0 예외 처리 테스트 추가

378c3f9

feat: 작업 생성 및 수정 요청 DTO 변경

d968073

fix: 의존성 요청의 작업 ID 설명 수정

28b3cae

feat: 작업 생성 요청 DTO를 TaskCreateRequest로 변경

9f255b5

docs: Task 의존성 페이로드에 대한 설명 추가

9cf4c3a

Copilot AI review requested due to automatic review settings January 22, 2026 15:11

GoGradually merged commit f64b8a7 into master Jan 22, 2026
3 checks passed

GoGradually deleted the fix/작업-추가-및-의존성-추가-시-ID-설정 branch January 22, 2026 15:11

Copilot started reviewing on behalf of GoGradually January 22, 2026 15:11 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/작업 추가 및 의존성 추가 시 id 설정#26

Hidden character warning

Fix/작업 추가 및 의존성 추가 시 id 설정#26
GoGradually merged 9 commits into
masterfrom
fix/작업-추가-및-의존성-추가-시-ID-설정

GoGradually commented Jan 22, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if (dep.fromId() != 0L && dep.toId() != 0L) {
		throw new IllegalArgumentException("작업 생성 시 의존 관계에는 fromId 또는 toId 중 하나가 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이어야 합니다.");

		return dto.getFromId() != null && dto.getFromId() == 0L
		\|\| dto.getToId() != null && dto.getToId() == 0L;

Hidden character warning

Conversation

GoGradually commented Jan 22, 2026

변경된 점

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants