Merged
Conversation
can019
reviewed
Sep 27, 2025
Comment on lines
+92
to
+97
| public Schedule toEntity(Long workflowId, Long userId) { | ||
| return Schedule.builder() | ||
| .workflowId(workflowId) | ||
| .cronExpression(this.cronExpression) | ||
| .scheduleText(this.scheduleText) | ||
| .isActive(this.isActive != null ? this.isActive : true) |
Collaborator
There was a problem hiding this comment.
정적 팩토리 메서드로 변경해주시면 좋을 것 같습니다.
- 객체 책임 명확화
- DTO의 역할
- DTO 내부 상태를 변경하는 작업이 아니기 때문에 인스턴스 메서드가 아닌 클래스 레벨로 두는 것이 좋을 것 같습니다.
public static Schedule toEntity(ScheduleRequestDto dto, Long workflowId, Long userId) {
return Schedule.builder()
.workflowId(workflowId)
// 인스턴스가 아닌 정적 메서드의 인수로 받은 dto를 사용
.cronExpression(dto.cronExpression)
.scheduleText(dto.scheduleText)
.isActive(dto.isActive != null ? dto.isActive : true)
.parameters(dto.parameters)
.createdBy(userId)
.updatedBy(userId)
.build();
}
can019
reviewed
Sep 27, 2025
| * @return 스케줄이 1개 이상 있으면 true | ||
| */ | ||
| public boolean hasSchedules() { | ||
| return schedules != null && !schedules.isEmpty(); |
Collaborator
There was a problem hiding this comment.
여담으로 이런 방법도 있긴 합니다.
import org.springframework.util.CollectionUtils;
public boolean hasSchedules() {
return !CollectionUtils.isEmpty(schedules);
}
can019
reviewed
Sep 27, 2025
| // 4. 워크플로우 이름 중복 체크 | ||
| if (workflowMapper.existsByName(dto.getName())) { | ||
| throw new IllegalArgumentException("이미 존재하는 워크플로우 이름입니다 : " + dto.getName()); | ||
| throw new IllegalArgumentException("이미 존재하는 워크플로우 이름입니다: " + dto.getName()); |
Collaborator
There was a problem hiding this comment.
common/execption/DuplicateDataException.class 사용해주세요
can019
reviewed
Sep 27, 2025
Comment on lines
+249
to
+302
| private void registerSchedules( | ||
| Long workflowId, List<ScheduleCreateDto> scheduleCreateDtos, Long userId) { | ||
| if (scheduleCreateDtos == null || scheduleCreateDtos.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| log.info("스케줄 등록 시작: Workflow ID {} - {}개", workflowId, scheduleCreateDtos.size()); | ||
|
|
||
| int successCount = 0; | ||
| int failCount = 0; | ||
|
|
||
| for (ScheduleCreateDto dto : scheduleCreateDtos) { | ||
| try { | ||
| // 1. DTO → Model 변환 | ||
| Schedule schedule = dto.toEntity(workflowId, userId); | ||
|
|
||
| // 2. DB 중복 체크 (같은 워크플로우 + 같은 크론식) | ||
| if (scheduleMapper.existsByWorkflowIdAndCronExpression( | ||
| workflowId, schedule.getCronExpression())) { | ||
| throw new DuplicateDataException( | ||
| "이미 동일한 크론식의 스케줄이 존재합니다: " + schedule.getCronExpression()); | ||
| } | ||
|
|
||
| // 3. DB 저장 | ||
| int insertResult = scheduleMapper.insertSchedule(schedule); | ||
| if (insertResult != 1) { | ||
| log.error("스케줄 DB 저장 실패: Workflow ID {} - {}", workflowId, schedule.getCronExpression()); | ||
| failCount++; | ||
| continue; | ||
| } | ||
|
|
||
| // 4. Quartz 등록 (실시간 반영) | ||
| quartzScheduleService.addOrUpdateSchedule(schedule); | ||
|
|
||
| log.info( | ||
| "스케줄 등록 완료: Workflow ID {} - {} ({})", | ||
| workflowId, | ||
| schedule.getCronExpression(), | ||
| schedule.getScheduleText()); | ||
| successCount++; | ||
|
|
||
| } catch (DuplicateDataException e) { | ||
| log.warn("스케줄 중복으로 등록 건너뜀: Workflow ID {} - {}", workflowId, dto.getCronExpression()); | ||
| failCount++; | ||
| // 중복은 경고만 하고 계속 진행 | ||
| } catch (Exception e) { | ||
| log.error("스케줄 등록 실패: Workflow ID {} - {}", workflowId, dto.getCronExpression(), e); | ||
| failCount++; | ||
| // 스케줄 등록 실패해도 워크플로우는 유지 | ||
| } | ||
| } | ||
|
|
||
| log.info("스케줄 등록 완료: Workflow ID {} - 성공 {}개, 실패 {}개", workflowId, successCount, failCount); | ||
| } |
Collaborator
There was a problem hiding this comment.
이 메서드에서 사용하는 로거는 execution log 테이블에 들어가야 하나요?
만약 그렇다면 아래와 같이 해주시면 됩니다.
private static final Logger workflowLogger = LoggerFactory.getLogger("WORKFLOW_HISTORY");
workflowLogger.info();- 사용법은 아래 클래스 확인 부탁드립니다.
site.icebang.domain.workflow.service.TaskExecutionService
can019
reviewed
Sep 27, 2025
Collaborator
There was a problem hiding this comment.
@bwnfo3 확인해주시면 감사하겠습니다.
추가로 WorkflowController에서 아래 부분 수정 부탁드립니다.
@PostMapping("")
@ResponseStatus(HttpStatus.CREATED)
public ApiResponse<Void> createWorkflow(
@Valid @RequestBody WorkflowCreateDto workflowCreateDto,
@AuthenticationPrincipal AuthCredential authCredential) {
// 인증 체크
if (authCredential == null) {
throw new IllegalArgumentException("로그인이 필요합니다");
}참조: https://github.com/Kernel180-BE12/Final-4team-icebang/pull/211#discussion_r2379011039
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 작업 내용
schedule 생성 api와 생성할 때 관련된 메서드들 구현입니다.
Quartz에 실시간으로 연동은 구현중이고 잡, 태스크 생성과 연결하기 위해서 1차 pr입니다.
🔗 관련 이슈
💬 추가 요청사항
✅ 체크리스트
코드 품질
테스트
배포 준비