[DDING-47] media convert job status 업데이트 API 구현 #203
Conversation
|
Warning Rate limit exceeded@5uhwann has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough이 풀 리퀘스트는 비디오 처리 작업(Vod Processing Job)과 관련된 여러 컴포넌트에 대한 변경 사항을 포함하고 있습니다. 주요 변경 사항은 새로운 작업 상태 업데이트 기능 추가, 보안 구성 수정, 그리고 관련된 DTO와 엔티티의 유효성 검사 및 상태 열거형 확장입니다. 이러한 변경은 비디오 처리 작업의 상태 관리 및 엔드포인트 접근성을 개선하는 데 초점을 맞추고 있습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
participant Entity
Client->>Controller: PATCH /job-status
Controller->>Service: updateVodProcessingJobStatus(command)
Service->>Repository: getByConvertJobId()
Repository-->>Service: VodProcessingJob
Service->>Entity: updateConvertJobStatus()
Entity-->>Service: Updated Job
Service-->>Controller: Job Status Updated
Controller-->>Client: 200 OK
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/entity/ConvertJobStatus.java (1)
4-6: 각 상태값에 대한 문서화 필요각 상태값(
PENDING,COMPLETE,ERROR)의 의미와 사용 시점을 명확히 하기 위해 JavaDoc 문서화가 필요합니다.public enum ConvertJobStatus { + /** 작업이 대기 중인 상태 */ PENDING, + /** 작업이 성공적으로 완료된 상태 */ COMPLETE, + /** 작업 처리 중 오류가 발생한 상태 */ ERROR }또한, 상태 전이 유효성 검사 로직 추가를 고려해 보시기 바랍니다. 예를 들어:
COMPLETE상태에서 다른 상태로의 전환 방지ERROR상태에서COMPLETE로의 직접 전환 방지src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/service/FacadeVodProcessingJobService.java (1)
10-11: 메서드 문서화 및 반환 타입 개선 필요상태 업데이트 메서드에 대한 개선사항:
- JavaDoc을 추가하여 메서드의 목적, 파라미터, 예외 상황 등을 명시해 주세요.
- 작업 성공/실패 여부를 확인할 수 있도록
boolean또는 커스텀 응답 객체 반환을 고려해 보세요.+ /** + * VOD 처리 작업의 상태를 업데이트합니다. + * + * @param command 상태 업데이트 명령 객체 + * @throws JobNotFoundException 지정된 작업을 찾을 수 없는 경우 + * @throws IllegalStateException 유효하지 않은 상태 전이인 경우 + * @return 업데이트 성공 여부 + */ - void updateVodProcessingJobStatus(UpdateVodProcessingJobStatusCommand command); + boolean updateVodProcessingJobStatus(UpdateVodProcessingJobStatusCommand command);src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/controller/dto/request/CreatePendingVodProcessingJobRequest.java (1)
8-12: 추가 유효성 검사 고려
userId필드에 대한 추가 제약조건을 고려해 보세요:
- 최소/최대 길이 제한
- 허용되는 문자 패턴 정의
@NotNull @UUID String convertJobId, @NotNull + @Size(min = 1, max = 50) + @Pattern(regexp = "^[a-zA-Z0-9_-]+$", message = "영문자, 숫자, 밑줄, 하이픈만 허용됩니다") String userId
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/ddingdong/ddingdongBE/common/config/SecurityConfig.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/controller/VodProcessingJobController.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/controller/dto/request/CreatePendingVodProcessingJobRequest.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/controller/dto/request/UpdateVodProcessingJobStatusRequest.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/entity/ConvertJobStatus.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/entity/VodProcessingJob.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/service/FacadeVodProcessingJobService.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/service/FacadeVodProcessingJobServiceImpl.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/service/dto/command/UpdateVodProcessingJobStatusCommand.java(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/vodprocessing/service/FacadeGeneralVodProcessingJobServiceTest.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (2)
src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/controller/dto/request/UpdateVodProcessingJobStatusRequest.java (1)
16-18: 예외 처리 보완 필요
toCommand()메서드에서 발생할 수 있는 예외 상황에 대한 처리가 필요합니다.public UpdateVodProcessingJobStatusCommand toCommand() { + Objects.requireNonNull(convertJobId, "convertJobId must not be null"); + Objects.requireNonNull(status, "status must not be null"); return new UpdateVodProcessingJobStatusCommand(convertJobId, status); }src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/service/FacadeVodProcessingJobServiceImpl.java (1)
23-28: 🛠️ Refactor suggestion존재하지 않는 convertJobId에 대한 예외 처리 추가 필요
getByConvertJobId메서드 호출 시 해당 작업이 존재하지 않을 경우에 대한 예외 처리가 명시적으로 보이지 않습니다. 적절한 예외 처리를 추가하는 것이 좋겠습니다.다음과 같은 예외 처리를 추가하는 것을 제안드립니다:
@Transactional public void updateVodProcessingJobStatus(UpdateVodProcessingJobStatusCommand command) { VodProcessingJob vodProcessingJob = vodProcessingJobService.getByConvertJobId(command.convertJobId()); + if (vodProcessingJob == null) { + throw new EntityNotFoundException("존재하지 않는 VOD 처리 작업입니다. convertJobId: " + command.convertJobId()); + } vodProcessingJob.updateConvertJobStatus(command.status()); }
| public record UpdateVodProcessingJobStatusRequest( | ||
| @NotNull | ||
| @UUID | ||
| String convertJobId, | ||
| @NotNull | ||
| ConvertJobStatus status | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 전이 유효성 검사 추가 필요
현재 상태값의 유효성만 검사하고 있습니다. 상태 전이의 유효성을 검사하는 커스텀 validator를 추가하는 것이 좋습니다.
public record UpdateVodProcessingJobStatusRequest(
@NotNull
@UUID
String convertJobId,
@NotNull
+ @ValidStatusTransition(message = "유효하지 않은 상태 전이입니다")
ConvertJobStatus status
) {다음과 같은 커스텀 validator 구현을 제안드립니다:
@Target({ElementType.FIELD})
@Retention(RetentionPolicy.RUNTIME)
@Constraint(validatedBy = StatusTransitionValidator.class)
public @interface ValidStatusTransition {
String message() default "유효하지 않은 상태 전이입니다";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}| @PatchMapping("/job-status") | ||
| public void updateJobStatus(@RequestBody @Valid UpdateVodProcessingJobStatusRequest request) { | ||
| facadeVodProcessingJobService.updateVodProcessingJobStatus(request.toCommand()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
내부 API에 대한 보안 검토 및 문서화 개선 필요
- 내부 API임에도 추가적인 보안 계층이 보이지 않습니다.
- Swagger 문서화가 누락되어 있습니다.
다음과 같은 개선을 제안드립니다:
+ @Operation(summary = "VOD 처리 작업 상태 업데이트", description = "내부 시스템에서 VOD 처리 작업의 상태를 업데이트합니다.")
+ @ApiResponses(value = {
+ @ApiResponse(responseCode = "200", description = "상태 업데이트 성공"),
+ @ApiResponse(responseCode = "404", description = "존재하지 않는 작업"),
+ @ApiResponse(responseCode = "400", description = "잘못된 요청")
+ })
@PatchMapping("/job-status")
+ @InternalApiAuth
public void updateJobStatus(@RequestBody @Valid UpdateVodProcessingJobStatusRequest request) {
facadeVodProcessingJobService.updateVodProcessingJobStatus(request.toCommand());
}Committable suggestion skipped: line range outside the PR's diff.
| public void updateConvertJobStatus(ConvertJobStatus convertJobStatus) { | ||
| this.convertJobStatus = convertJobStatus; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 전이 유효성 검증 로직 필요
현재 구현은 모든 상태 전이를 허용하고 있습니다. 예를 들어, COMPLETE 상태에서 PENDING 상태로의 변경도 가능합니다. 비즈니스 로직에 맞는 상태 전이 규칙을 추가하는 것이 좋겠습니다.
다음과 같은 개선을 제안드립니다:
public void updateConvertJobStatus(ConvertJobStatus convertJobStatus) {
+ validateStatusTransition(convertJobStatus);
this.convertJobStatus = convertJobStatus;
}
+
+ private void validateStatusTransition(ConvertJobStatus newStatus) {
+ if (!this.convertJobStatus.canTransitionTo(newStatus)) {
+ throw new IllegalStateException(
+ String.format("잘못된 상태 전이입니다: %s -> %s", this.convertJobStatus, newStatus)
+ );
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
| @DisplayName("CodProcessingJob 상태를 변경한다.") | ||
| @Test | ||
| void updateVodProcessingJobStatus() { | ||
| //given | ||
| VodProcessingJob savedVodProcessingJob = vodProcessingJobRepository.save(VodProcessingJob.builder() | ||
| .convertJobStatus(PENDING) | ||
| .userId("1") | ||
| .convertJobId("test") | ||
| .build()); | ||
|
|
||
| UpdateVodProcessingJobStatusCommand command = new UpdateVodProcessingJobStatusCommand("test", COMPLETE); | ||
|
|
||
| //when | ||
| facadeVodProcessingJobService.updateVodProcessingJobStatus(command); | ||
|
|
||
| //then | ||
| Optional<VodProcessingJob> result = vodProcessingJobRepository.findByConvertJobId("test"); | ||
| assertThat(result).isPresent(); | ||
| assertThat(result.get().getUserId()).isEqualTo("1"); | ||
| assertThat(result.get().getConvertJobStatus()).isEqualTo(COMPLETE); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 케이스 보완 필요
현재 테스트는 기본적인 성공 케이스만 다루고 있습니다. 다음과 같은 테스트 케이스를 추가하는 것이 좋겠습니다:
- 존재하지 않는 convertJobId로 상태 업데이트 시도
- 잘못된 상태 전이 시도 (예: COMPLETE -> PENDING)
- 동일한 상태로의 업데이트
예시 테스트 케이스:
@Test
void updateVodProcessingJobStatus_WithInvalidJobId_ShouldThrowException() {
// given
UpdateVodProcessingJobStatusCommand command =
new UpdateVodProcessingJobStatusCommand("invalid-id", COMPLETE);
// when & then
assertThatThrownBy(() ->
facadeVodProcessingJobService.updateVodProcessingJobStatus(command))
.isInstanceOf(EntityNotFoundException.class);
}| .requestMatchers(API_PREFIX + "/internal/**") | ||
| .permitAll() |
There was a problem hiding this comment.
💡 Codebase verification
내부 API 엔드포인트에 대한 보안 강화 필요
VOD 처리 작업을 위한 내부 API(/server/internal/trigger/vod-processing-job)가 현재 모든 요청에 대해 완전히 개방되어 있어 보안 위험이 있습니다.
필요한 보안 조치:
- POST(작업 생성)와 PATCH(상태 업데이트) 메소드만 명시적으로 허용하도록 제한
- 내부 서비스의 IP 주소만 허용하는 필터 추가
- 서비스 간 인증 메커니즘 구현 (예: API 키, 상호 TLS)
🔗 Analysis chain
내부 API 엔드포인트에 대한 보안 검토 필요
내부 API 엔드포인트(/internal/**)에 대해 모든 HTTP 메소드를 허용하도록 변경되었습니다. 이는 필요 이상의 권한을 부여할 수 있으며 보안 위험을 초래할 수 있습니다.
다음 사항들을 고려해주세요:
- 필요한 HTTP 메소드만 명시적으로 허용
- IP 기반 필터링 추가
- 내부 API 호출에 대한 추가 인증 메커니즘 구현
다음 스크립트로 현재 내부 API 엔드포인트들을 확인해보세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all internal API endpoints to assess security impact
# Search for all controller methods with internal API paths
rg -t java "(@GetMapping|@PostMapping|@PutMapping|@PatchMapping|@DeleteMapping).*internal"
Length of output: 92
Script:
#!/bin/bash
# Search for controller classes and mappings related to internal endpoints
echo "=== Searching for controller classes with internal endpoints ==="
rg -t java "@(Rest)?Controller.*internal" -B 2 -A 5
echo -e "\n=== Searching for RequestMapping with internal paths ==="
rg -t java "@RequestMapping.*internal" -B 2 -A 2
echo -e "\n=== Checking security configurations for internal paths ==="
fd "Security.*\.java" --type f --exec cat {}
Length of output: 7947
Script:
#!/bin/bash
# Check the implementation of the VodProcessingJobController
echo "=== VodProcessingJobController Implementation ==="
cat src/main/java/ddingdong/ddingdongBE/domain/vodprocessing/controller/VodProcessingJobController.java
echo -e "\n=== Checking for any other internal paths or configurations ==="
rg -t java "internal" -g "!{test,Test}*.java" -B 2 -A 2
Length of output: 3173
| public record UpdateVodProcessingJobStatusCommand( | ||
| String convertJobId, | ||
| ConvertJobStatus status | ||
| ) { | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
커맨드 객체에 대한 유효성 검사 및 문서화 개선 필요
현재 구현에서 다음 사항들을 개선하면 좋을 것 같습니다:
convertJobId와status필드에 대한 유효성 검사 추가- 상태 전이(state transition)에 대한 제약 조건 정의
- 각 필드에 대한 JavaDoc 문서화
다음과 같이 개선해보세요:
+/**
+ * VOD 처리 작업의 상태를 업데이트하기 위한 커맨드 객체
+ *
+ * @param convertJobId 변환 작업 식별자
+ * @param status 업데이트할 작업 상태
+ */
public record UpdateVodProcessingJobStatusCommand(
+ @NotNull(message = "변환 작업 ID는 필수입니다")
String convertJobId,
+ @NotNull(message = "상태는 필수입니다")
ConvertJobStatus status
) {
+ public UpdateVodProcessingJobStatusCommand {
+ Objects.requireNonNull(convertJobId, "convertJobId must not be null");
+ Objects.requireNonNull(status, "status must not be null");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public record UpdateVodProcessingJobStatusCommand( | |
| String convertJobId, | |
| ConvertJobStatus status | |
| ) { | |
| } | |
| /** | |
| * VOD 처리 작업의 상태를 업데이트하기 위한 커맨드 객체 | |
| * | |
| * @param convertJobId 변환 작업 식별자 | |
| * @param status 업데이트할 작업 상태 | |
| */ | |
| public record UpdateVodProcessingJobStatusCommand( | |
| @NotNull(message = "변환 작업 ID는 필수입니다") | |
| String convertJobId, | |
| @NotNull(message = "상태는 필수입니다") | |
| ConvertJobStatus status | |
| ) { | |
| public UpdateVodProcessingJobStatusCommand { | |
| Objects.requireNonNull(convertJobId, "convertJobId must not be null"); | |
| Objects.requireNonNull(status, "status must not be null"); | |
| } | |
| } |
| @@ -0,0 +1,10 @@ | |||
| package ddingdong.ddingdongBE.domain.vodprocessing.service.dto.command; | |||
|
|
|||
| import ddingdong.ddingdongBE.domain.vodprocessing.entity.ConvertJobStatus; | |||
There was a problem hiding this comment.
💡 Codebase verification
상태 전이 규칙이 누락되어 있습니다
ConvertJobStatus 열거형에 상태 전이 규칙이 정의되어 있지 않아 잠재적인 문제가 발생할 수 있습니다:
- 현재 구현에서는 어떤 상태에서든 다른 상태로의 전환이 가능합니다 (예: ERROR → COMPLETE)
VodProcessingJob.updateConvertJobStatus()메서드에서 상태 전이 유효성 검증이 이루어지지 않습니다- 비즈니스 규칙에 따른 올바른 상태 전이 보장이 필요합니다
🔗 Analysis chain
상태 전이 검증을 위한 ConvertJobStatus 확인 필요
ConvertJobStatus 열거형에 대한 상태 전이 규칙이 명확하게 정의되어 있는지 확인이 필요합니다.
다음 스크립트로 ConvertJobStatus 구현을 확인해보세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find ConvertJobStatus implementation and usage
# Search for ConvertJobStatus enum definition
ast-grep --pattern 'enum ConvertJobStatus {
$$$
}'
# Search for status transition validations
rg -t java "ConvertJobStatus.*validate"
Length of output: 96
Script:
#!/bin/bash
# Find all occurrences of ConvertJobStatus
rg -l "ConvertJobStatus"
# Find enum definition with more flexible pattern
ast-grep --pattern 'enum $_ {
$$$
}'
# Look for methods involving ConvertJobStatus
rg -A 5 "ConvertJobStatus"
Length of output: 11061
...dongBE/domain/vodprocessing/controller/dto/request/CreatePendingVodProcessingJobRequest.java
Outdated
Show resolved
Hide resolved
|
🚀 작업 내용
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
보안 설정
새로운 기능
유효성 검사