Skip to content

Conversation

@eric-jy-park
Copy link
Member

@eric-jy-park eric-jy-park commented Aug 3, 2025

📌 PR 제목

📋 작업 내용

  • 영상 청크 업로드 기능 구현

✅ 체크리스트

  • 테스트를 완료했나요?
  • 코드 컨벤션을 지켰나요?
  • 관련 문서를 업데이트 했나요?

🚨 주의사항

  • 리뷰어가 주의깊게 봐야할 부분이 있다면 적어주세요.

@eric-jy-park eric-jy-park requested a review from Copilot August 3, 2025 07:54
@eric-jy-park eric-jy-park self-assigned this Aug 3, 2025
@claude
Copy link

claude bot commented Aug 3, 2025

Claude finished @eric-jy-park's task —— View job


영상 청크 업로드 기능 구현 코드 리뷰

리뷰 요약

이번 PR은 영상 청크 업로드 기능을 구현한 작업입니다. 새로운 엔티티(UploadSession, Chunk)와 관련 서비스 로직을 추가하여 대용량 영상 파일의 분할 업로드를 지원합니다. 전반적인 아키텍처는 적절하나 몇 가지 중요한 문제점들이 발견되었습니다. 특히 데이터 타입 불일치, N+1 쿼리 문제, 예외 클래스명 오타 등이 수정이 필요합니다.

발견된 문제점

UploadSession.java:30 - [issue:] @Enumerated(EnumType.ORDINAL) 사용 시 순서 변경으로 인한 데이터 오류 가능성 - EnumType.STRING 사용 권장

UploadSession.java:48 - [issue:] FetchType.EAGER 사용으로 인한 N+1 쿼리 문제 - FetchType.LAZY로 변경하고 필요시 batch fetch 적용

UploadStatusChunkResponse.java:20 - [issue:] getReceivedChunkCount() 반환타입 longtotalReceived 필드타입 Long 불일치 - 타입 통일 필요

AttemptServiceImpl.java:135 - [suggestion:] 너무 긴 빌더 패턴 체인으로 가독성 저하 - 줄바꿈과 들여쓰기로 개선 필요

UploadSessionNotfoundException.java:1 - [issue:] 클래스명 오타 NotfoundNotFound로 수정 필요

AttemptController.java:303 - [question:] 청크 업로드 메서드가 빈 응답만 반환 - 실제 업로드 로직 구현 계획이 있는지 확인 필요

Chunk.java:28 - [suggestion:] boolean 필드 isCompleted 컬럼명이 is_completed로 매핑되어 있음 - JPA 네이밍 전략과의 일관성 확인 필요

This comment was marked as outdated.

@claude
Copy link

claude bot commented Aug 3, 2025

Claude finished @eric-jy-park's task —— View job


영상 청크 업로드 기능 구현 코드 리뷰

이번 PR은 영상 파일의 분할 업로드를 위한 청크 업로드 기능을 구현했습니다. UploadSessionChunk 엔티티를 새로 추가하여 업로드 세션 관리와 청크 추적이 가능하도록 구현되었습니다. 전반적인 아키텍처는 적절하나 데이터 타입 불일치와 성능 관련 몇 가지 문제점이 발견되었습니다. 특히 컨트롤러에서 실제 업로드 로직이 구현되지 않은 점은 추가 작업이 필요합니다.

발견된 문제점

UploadStatusChunkResponse.java:20 - [issue:] getReceivedChunkCount() 반환타입 LongtotalReceived 필드타입 Long 간 타입 불일치 - UploadSession.getReceivedChunkCount()Long을 반환하지만 실제로는 count() 결과는 long

AttemptServiceImpl.java:135 - [suggestion:] 긴 빌더 패턴 체인으로 가독성 저하 - 줄바꿈과 들여쓰기 적용으로 가독성 개선 필요

AttemptController.java:302 - [question:] 청크 업로드 메서드가 빈 응답만 반환하고 실제 업로드 로직 없음 - 실제 청크 업로드 처리 로직 구현 계획 확인 필요

AttemptController.java:333 - [question:] 업로드 세션 마무리 메서드도 빈 응답만 반환 - 실제 파일 병합 및 완료 처리 로직 구현 계획 확인 필요


@eric-jy-park eric-jy-park force-pushed the feat/42-chunk-upload branch from 42d7bc8 to cb0a568 Compare August 3, 2025 15:55
@claude
Copy link

claude bot commented Aug 3, 2025

Claude finished @eric-jy-park's task —— View job


\uccad\ud06c \uc5c5\ub85c\ub4dc \uae30\ub2a5 \ucf54\ub4dc \ub9ac\ubdf0

  • \uc8fc\uc694 \ubcc0\uacbd \ud30c\uc77c\ub4e4\uc744 \uc77d\uc5b4\uc11c \uccad\ud06c \uc5c5\ub85c\ub4dc \uad6c\ud604 \ub0b4\uc6a9 \ud30c\uc545
  • \ubc84\uadf8, \uc131\ub2a5 \ubb38\uc81c, \ubd88\ubd84\uba85\ud55c \ub85c\uc9c1\uc5d0 \uc9d1\uc911\ud558\uc5ec \ucf54\ub4dc \ubd84\uc11d
  • \ud55c\uad6d\uc5b4\ub85c \uac04\uacb0\ud55c \ucf54\ub4dc \ub9ac\ubdf0 \uc791\uc131 (\uc9c0\uc815\ub41c \ud615\uc2dd \uc900\uc218)

\uc774\ubc88 PR\uc740 \ub300\uc6a9\ub7c9 \uc601\uc0c1 \ud30c\uc77c\uc758 \ubd84\ud560 \uc5c5\ub85c\ub4dc\ub97c \uc704\ud55c \uccad\ud06c \uc5c5\ub85c\ub4dc \uae30\ub2a5\uc744 \uad6c\ud604\ud588\uc2b5\ub2c8\ub2e4. UploadSession\uacfc Chunk \uc5d4\ud2f0\ud2f0\ub97c \uc0c8\ub85c \ucd94\uac00\ud558\uc5ec \uc5c5\ub85c\ub4dc \uc138\uc158 \uad00\ub9ac\uc640 \uccad\ud06c \ucd94\uc801\uc774 \uac00\ub2a5\ud558\ub3c4\ub85d \uad6c\ud604\ub418\uc5c8\uc2b5\ub2c8\ub2e4. \uc804\ubc18\uc801\uc778 \uc544\ud0a4\ud14d\ucc98\ub294 \uc801\uc808\ud558\ub098 \ub370\uc774\ud130 \ud0c0\uc785 \ubd88\uc77c\uce58\uc640 \uc131\ub2a5 \uad00\ub828 \uba87 \uac00\uc9c0 \ubb38\uc81c\uc810\uc774 \ubc1c\uacac\ub418\uc5c8\uc2b5\ub2c8\ub2e4. \ud2b9\ud788 \ucef4\ud2b8\ub864\ub7ec\uc5d0\uc11c \uc2e4\uc81c \uc5c5\ub85c\ub4dc \ub85c\uc9c1\uc774 \uad6c\ud604\ub418\uc9c0 \uc54a\uc740 \uc810\uc740 \ucd94\uac00 \uc791\uc5c5\uc774 \ud544\uc694\ud569\ub2c8\ub2e4.

\ubc1c\uacac\ub41c \ubb38\uc81c\uc810

UploadStatusChunkResponse.java:20 - [issue:] getReceivedChunkCount() \ubc18\ud658\ud0c0\uc785 Long\uacfc totalReceived \ud544\ub4dc\ud0c0\uc785 Long \uac04 \ud0c0\uc785 \ubd88\uc77c\uce58 - UploadSession.getReceivedChunkCount()\uac00 Long\uc744 \ubc18\ud658\ud558\uc9c0\ub9cc \uc2e4\uc81c\ub85c\ub294 count() \uacb0\uacfc\ub294 long\uc784

AttemptServiceImpl.java:135 - [suggestion:] \uae34 \ube4c\ub354 \ud328\ud134 \uccb4\uc778\uc73c\ub85c \uac00\ub3c5\uc131 \uc800\ud558 - \uc904\ubc14\uafc8\uacfc \ub4e4\uc5ec\uc4f0\uae30 \uc801\uc6a9\uc73c\ub85c \uac00\ub3c5\uc131 \uac1c\uc120 \ud544\uc694

AttemptController.java:154 - [question:] \uccad\ud06c \uc5c5\ub85c\ub4dc \uba54\uc11c\ub4dc\uac00 \ube48 \uc751\ub2f5\ub9cc \ubc18\ud658\ud558\uace0 \uc2e4\uc81c \uc5c5\ub85c\ub4dc \ub85c\uc9c1 \uc5c6\uc74c - \uc2e4\uc81c \uccad\ud06c \uc5c5\ub85c\ub4dc \ucc98\ub9ac \ub85c\uc9c1 \uad6c\ud604 \uacc4\ud68d \ud655\uc778 \ud544\uc694

AttemptController.java:167 - [question:] \uc5c5\ub85c\ub4dc \uc138\uc158 \ub9c8\ubb34\ub9ac \uba54\uc11c\ub4dc\ub3c4 \ube48 \uc751\ub2f5\ub9cc \ubc18\ud658 - \uc2e4\uc81c \ud30c\uc77c \ubcd1\ud569 \ubc0f \uc644\ub8cc \ucc98\ub9ac \ub85c\uc9c1 \uad6c\ud604 \uacc4\ud68d \ud655\uc778 \ud544\uc694


@claude
Copy link

claude bot commented Aug 3, 2025

Claude finished @eric-jy-park's task —— View job


청크 업로드 기능 코드 리뷰

  • 주요 변경 파일들을 읽어서 청크 업로드 구현 내용 파악
  • 버그, 성능 문제, 불분명한 로직에 집중하여 코드 분석
  • 한국어로 간결한 코드 리뷰 작성 (지정된 형식 준수)

이번 PR은 대용량 영상 파일의 분할 업로드를 위한 청크 업로드 기능을 구현했습니다. UploadSessionChunk 엔티티를 새로 추가하여 업로드 세션 관리와 청크 추적이 가능하도록 구현되었습니다. 전반적인 아키텍처는 적절하나 데이터 타입 불일치와 성능 관련 몇 가지 문제점이 발견되었습니다. 특히 컨트롤러에서 실제 업로드 로직이 구현되지 않은 점은 추가 작업이 필요합니다.

발견된 문제점

UploadSession.java:48 - [issue:] getReceivedChunkCount() 메서드가 count() 결과를 Long으로 반환하지만 실제로는 long 타입임 - 반환 타입을 long으로 변경하여 타입 불일치 해결 필요

AttemptServiceImpl.java:161 - [suggestion:] 긴 빌더 패턴 체인으로 가독성 저하 - 줄바꿈과 들여쓰기 적용으로 가독성 개선 필요

AttemptController.java:167 - [question:] 업로드 세션 마무리 메서드가 빈 응답만 반환하고 실제 로직 없음 - 실제 파일 병합 및 완료 처리 로직 구현 계획 확인 필요

AttemptServiceImpl.java:233 - [question:] 청크 결합 기능이 주석으로만 표시되어 있고 실제 구현 없음 - 파일 병합 로직 구현 일정 확인 필요


@claude
Copy link

claude bot commented Aug 3, 2025

Claude finished @eric-jy-park's task —— View job


청크 업로드 기능 코드 리뷰

  • 주요 변경 파일들을 읽어서 청크 업로드 구현 내용 파악
  • 버그, 성능 문제, 불분명한 로직에 집중하여 코드 분석
  • 한국어로 간결한 코드 리뷰 작성 (지정된 형식 준수)

이번 PR은 대용량 영상 파일의 분할 업로드를 위한 청크 업로드 기능을 구현했습니다. UploadSessionChunk 엔티티를 새로 추가하여 업로드 세션 관리와 청크 추적이 가능하도록 구현되었습니다. 전반적인 아키텍처는 적절하나 데이터 타입 불일치와 성능 관련 몇 가지 문제점이 발견되었습니다. 특히 컨트롤러에서 실제 업로드 로직이 구현되지 않은 점은 추가 작업이 필요합니다.

발견된 문제점

UploadSession.java:48 - [issue:] getReceivedChunkCount() 메서드가 count() 결과를 Long으로 반환하지만 실제로는 long 타입임 - 반환 타입을 long으로 변경하여 타입 불일치 해결 필요

AttemptServiceImpl.java:200 - [suggestion:] 긴 빌더 패턴 체인으로 가독성 저하 - 줄바꿈과 들여쓰기 적용으로 가독성 개선 필요

AttemptController.java:167 - [question:] 업로드 세션 마무리 메서드가 빈 응답만 반환하고 실제 로직 없음 - 실제 파일 병합 및 완료 처리 로직 구현 계획 확인 필요

AttemptServiceImpl.java:272 - [question:] 청크 결합 기능이 주석으로만 표시되어 있고 실제 구현 없음 - 파일 병합 로직 구현 일정 확인 필요


@eric-jy-park eric-jy-park changed the title (WIP) feat: 영상 청크 업로드 기능 구현 feat: 영상 청크 업로드 기능 구현 Aug 3, 2025
@eric-jy-park eric-jy-park requested a review from Copilot August 3, 2025 17:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements video chunk upload functionality for route mission attempts, enabling users to upload large video files in chunks for better reliability and performance.

  • Adds complete upload session and chunk management system with entities, repositories, and services
  • Implements chunked video upload API endpoints for initializing sessions, uploading chunks, and finalizing uploads
  • Creates comprehensive error handling for upload-related scenarios

Reviewed Changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
application.yaml Changes Hibernate DDL mode from create to update
ErrorCode.java Adds upload-related error codes
ErrorCategory.java Adds UPLOAD_SESSION error category
UploadSession.java Defines upload session entity with chunk management methods
Chunk.java Defines chunk entity for individual file pieces
AttemptServiceImpl.java Implements core upload logic including chunk storage and file combination
AttemptController.java Adds REST endpoints for upload operations
UserMissionAttempt.java Links attempts to upload sessions

Comment on lines 86 to 88
try {
FileOutputStream fos = new FileOutputStream(finalFilePath.toFile());
BufferedOutputStream bos = new BufferedOutputStream(fos);
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resources are not properly closed. The FileOutputStream and BufferedOutputStream should be used within try-with-resources blocks to ensure they are closed even if an exception occurs.

Suggested change
try {
FileOutputStream fos = new FileOutputStream(finalFilePath.toFile());
BufferedOutputStream bos = new BufferedOutputStream(fos);
try (FileOutputStream fos = new FileOutputStream(finalFilePath.toFile());
BufferedOutputStream bos = new BufferedOutputStream(fos)) {

Copilot uses AI. Check for mistakes.
byte[] chunkData = Files.readAllBytes(chunkPath);
bos.write(chunkData);
}
bos.flush();
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flush() and close() operations are outside the try block but the streams are declared inside. This will cause compilation errors as the variables are not in scope.

Copilot uses AI. Check for mistakes.

private final AttemptService attemptService;
private final RecommendationService recommendationService;
private final UploadSessionRepository uploadSessionRepository;
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UploadSessionRepository is injected but never used in the controller. Controllers should delegate business logic to services rather than directly accessing repositories.

Suggested change
private final UploadSessionRepository uploadSessionRepository;

Copilot uses AI. Check for mistakes.

chunkRepository.save(chunk);

uploadSession.getChunks().add(chunk);
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modifying the chunks collection directly can cause issues with JPA entity state management. The addChunk() method defined in UploadSession should be used instead, or the relationship should be properly managed by setting both sides of the bidirectional relationship.

Suggested change
uploadSession.getChunks().add(chunk);
uploadSession.addChunk(chunk);

Copilot uses AI. Check for mistakes.
return this.chunks.stream().anyMatch(chunk -> chunk.getChunkIndex() == index);
}

public void addChunk(Chunk chunk) {
Copy link

Copilot AI Aug 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addChunk method only sets one side of the bidirectional relationship. It should also call chunk.setUploadSession(this) to properly maintain the relationship integrity.

Suggested change
public void addChunk(Chunk chunk) {
public void addChunk(Chunk chunk) {
chunk.setUploadSession(this);

Copilot uses AI. Check for mistakes.
@claude
Copy link

claude bot commented Aug 4, 2025

Claude finished @eric-jy-park's task —— View job


청크 업로드 기능 코드 리뷰

  • 주요 변경 파일들을 읽어서 청크 업로드 구현 내용 파악
  • 버그, 성능 문제, 불분명한 로직에 집중하여 코드 분석
  • 한국어로 간결한 코드 리뷰 작성 (지정된 형식 준수)

이번 PR은 대용량 영상 파일의 분할 업로드를 위한 청크 업로드 기능을 구현했습니다. UploadSessionChunk 엔티티를 새로 추가하여 업로드 세션 관리와 청크 추적이 가능하도록 구현되었습니다. 전반적인 아키텍처는 적절하나 데이터 타입 불일치와 성능 관련 몇 가지 문제점이 발견되었습니다. 특히 컨트롤러에서 실제 업로드 로직이 구현되지 않은 점은 추가 작업이 필요합니다.

발견된 문제점

UploadSession.java:48 - [issue:] getReceivedChunkCount() 메서드가 count() 결과를 Long으로 반환하지만 실제로는 long 타입임 - 반환 타입을 long으로 변경하여 타입 불일치 해결 필요

AttemptServiceImpl.java:199 - [suggestion:] 긴 빌더 패턴 체인으로 가독성 저하 - 줄바꿈과 들여쓰기 적용으로 가독성 개선 필요

AttemptController.java:45 - [suggestion:] UploadSessionRepository가 주입되었지만 사용되지 않음 - 불필요한 의존성 제거 필요

AttemptServiceImpl.java:278 - [question:] NCP 스토리지 업로드 로직이 주석으로만 표시되어 있고 실제 구현 없음 - 파일 업로드 로직 구현 일정 확인 필요


@eric-jy-park eric-jy-park merged commit 03625f8 into main Aug 4, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants