[feature] 구글 드라이브 업로드 기능 구현#369
Merged
PororoAndFriends merged 6 commits intodevelop/befrom May 6, 2025
Merged
Conversation
Contributor
|
""" Walkthrough이번 변경 사항은 백엔드에서 Google Drive를 활용한 동아리 이미지 관리 기능을 도입하는 데 초점을 맞추고 있습니다. Google Drive API 연동을 위한 의존성이 추가되었고, 클럽 이미지 업로드, 삭제, 갱신을 위한 서비스 인터페이스와 구현체가 도입되었습니다. 기존 GCS 기반 서비스는 인터페이스화되어 Google Drive 및 GCS 구현체가 공존할 수 있도록 구조가 변경되었습니다. 또한, 한글 파일명 처리 유틸리티와 Google Drive API 클라이언트 설정을 위한 Spring Configuration이 추가되었습니다. 오류 코드 및 예외 처리도 보강되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ClubImageController
participant ClubImageService (인터페이스)
participant GoogleDriveClubImageService
participant ClubRepository
participant GoogleDriveAPI
Client->>ClubImageController: 클럽 로고/피드 이미지 업로드 요청
ClubImageController->>ClubImageService: uploadLogo/uploadFeed 호출
ClubImageService->>GoogleDriveClubImageService: (구현체) 메서드 실행
GoogleDriveClubImageService->>ClubRepository: 클럽 엔티티 조회/수정
GoogleDriveClubImageService->>GoogleDriveAPI: 파일 업로드/삭제
GoogleDriveAPI-->>GoogleDriveClubImageService: 업로드 결과(파일 ID/URL)
GoogleDriveClubImageService-->>ClubRepository: 업로드/삭제 결과 반영
GoogleDriveClubImageService-->>ClubImageController: 결과 반환
ClubImageController-->>Client: 응답 반환
Possibly related issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
Test Results1 tests 1 ✅ 0s ⏱️ Results for commit aecaa78. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
backend/src/main/java/moadong/media/service/GcsClubImageService.java (2)
155-158: 🛠️ Refactor suggestionsplitPath 메서드의 취약성
splitPath메서드는 하드코딩된 인덱스(예:path.split("/",5)[4])를 사용하여 URL을 파싱합니다. 이 방식은 URL 구조가 변경되면 오류가 발생할 수 있어 취약합니다.private String splitPath(String path) { // https://storage.googleapis.com/{bucketName}/{clubId}/{fileType}/{filePath} -> {filePath} - return path.split("/",5)[4]; + String prefix = "https://storage.googleapis.com/" + bucketName + "/"; + if (path.startsWith(prefix)) { + return path.substring(prefix.length()); + } + // URL 형식이 다른 경우 대비 + String[] parts = path.split("/"); + if (parts.length >= 5) { + return String.join("/", Arrays.copyOfRange(parts, 4, parts.length)); + } + throw new RestApiException(ErrorCode.IMAGE_DELETE_FAILED); }
132-133: 🛠️ Refactor suggestionURL 경로 파싱 논리의 취약성
filePath.split("/")[5]와 같은 하드코딩된 인덱스를 사용하여 파일 타입을 추출하는 방식은 URL 구조가 변경될 경우 오류가 발생할 위험이 있습니다.// https://storage.googleapis.com/{bucketName}/{clubId}/{fileType}/{filePath} -> {fileType} - String fileType = filePath.split("/")[5]; + String fileType; + try { + String[] parts = filePath.split("/"); + fileType = parts.length > 5 ? parts[5] : ""; + } catch (ArrayIndexOutOfBoundsException e) { + throw new RestApiException(ErrorCode.IMAGE_DELETE_FAILED); + }
🧹 Nitpick comments (12)
backend/build.gradle (1)
47-50: 의존성 버전 관리: Google API Client BOM 사용 권장
현재 Google Drive 관련 라이브러리를 개별 버전으로 관리하고 있으나, 향후 호환성 확보 및 일관성 유지를 위해 BOM(Bill of Materials) 사용을 고려해주세요.dependencyManagement { imports { + mavenBom "com.google.api-client:google-api-client-bom:2.0.0" } }backend/src/main/java/moadong/media/controller/ClubImageController.java (1)
54-60: 리소스 업데이트 엔드포인트 시맨틱 검토
@PostMapping을 사용해 피드 이미지 설정을 업데이트하고 있지만, REST 관점에서 리소스 수정을 나타내는@PutMapping사용을 고려해보세요.- @PostMapping(value = "/{clubId}/feeds") + @PutMapping(value = "/{clubId}/feeds", consumes = MediaType.APPLICATION_JSON_VALUE)backend/src/main/java/moadong/global/exception/ErrorCode.java (1)
16-16: 문구 스타일 일관성 유지 제안
IMAGE_DELETE_FAILED메시지에 마침표를 추가해 다른 항목들과 스타일을 통일하는 것이 좋습니다.- IMAGE_DELETE_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, "601-4", "이미지 삭제에 실패하였습니다"), + IMAGE_DELETE_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, "601-4", "이미지 삭제에 실패하였습니다."),backend/src/main/java/moadong/media/util/ClubImageUtil.java (1)
1-12: 유틸리티 클래스 구현이 깔끔합니다!한글 문자 감지를 위한 유틸리티 메서드가 적절하게 구현되었습니다. Normalizer를 사용하여 텍스트를 NFC 형식으로 정규화한 후 정규식 패턴을 적용하는 접근 방식이 효과적입니다.
다음과 같은 개선사항을 고려해 보세요:
- 클래스와 메서드에 JavaDoc 주석을 추가하여 목적과 동작 방식을 명확히 설명
- 정규식 패턴을 상수로 추출하여 코드 가독성 향상
- 유닛 테스트 추가 고려
package moadong.media.util; import java.text.Normalizer; import java.util.regex.Pattern; +/** + * 클럽 이미지 관련 유틸리티 클래스 + */ public class ClubImageUtil { + private static final String KOREAN_PATTERN = ".*[ㄱ-ㅎㅏ-ㅣ가-힣]+.*"; + + /** + * 주어진 텍스트에 한글이 포함되어 있는지 확인합니다. + * + * @param text 검사할 텍스트 + * @return 한글 포함 여부 (true: 포함, false: 미포함) + */ public static boolean containsKorean(String text) { text = Normalizer.normalize(text, Normalizer.Form.NFC); - return Pattern.matches(".*[ㄱ-ㅎㅏ-ㅣ가-힣]+.*", text); + return Pattern.matches(KOREAN_PATTERN, text); } }backend/src/main/java/moadong/media/util/GoogleDriveConfig.java (2)
3-10: 미사용 임포트 제거 필요
GoogleCredential클래스가 임포트되었지만 실제로 사용되지 않습니다. 코드 정리를 위해 제거하는 것이 좋습니다.package moadong.media.util; -import com.google.api.client.googleapis.auth.oauth2.GoogleCredential; import com.google.api.client.googleapis.javanet.GoogleNetHttpTransport; import com.google.api.client.json.jackson2.JacksonFactory; import com.google.api.services.drive.Drive; import com.google.api.services.drive.DriveScopes; import com.google.auth.http.HttpCredentialsAdapter; import com.google.auth.oauth2.GoogleCredentials;
17-42: 구글 드라이브 설정 구현 개선 제안현재 구현은 기능적으로는 문제가 없으나 몇 가지 개선이 가능합니다:
- 예외 처리가 불충분합니다. 인증 파일 로드 실패나 API 연결 문제 시 발생하는 예외에 대한 명시적인 처리가 필요합니다.
- DriveScopes가 하드코딩되어 있습니다. 필요한 접근 범위를 설정 파일에서 가져오도록 수정하면 더 유연하게 관리할 수 있습니다.
- JavaDoc 주석이 없어 각 메서드의 목적과 동작 방식이 명확하지 않습니다.
- 클래스 끝에 불필요한 빈 줄이 있습니다.
@Configuration +/** + * Google Drive API 클라이언트 설정 클래스 + */ public class GoogleDriveConfig { @Value("${google.drive.credentials.file}") private String credentialsLocation; @Value("${google.application.name}") private String applicationName; + @Value("${google.drive.scopes:https://www.googleapis.com/auth/drive}") + private String driveScope; + /** + * Google Drive 서비스 클라이언트 빈을 생성합니다. + * + * @return 구성된 Drive 서비스 객체 + * @throws Exception 인증 파일 로드 실패 또는 HTTP 전송 초기화 오류 시 + */ @Bean public Drive googleDriveService() throws Exception { - InputStream in = new ClassPathResource(credentialsLocation).getInputStream(); - GoogleCredentials credentials = GoogleCredentials.fromStream(in) - .createScoped(Collections.singleton(DriveScopes.DRIVE)); + try { + InputStream in = new ClassPathResource(credentialsLocation).getInputStream(); + GoogleCredentials credentials = GoogleCredentials.fromStream(in) + .createScoped(Collections.singleton(driveScope)); - return new Drive.Builder( - GoogleNetHttpTransport.newTrustedTransport(), - JacksonFactory.getDefaultInstance(), - new HttpCredentialsAdapter(credentials)) - .setApplicationName(applicationName) - .build(); + return new Drive.Builder( + GoogleNetHttpTransport.newTrustedTransport(), + JacksonFactory.getDefaultInstance(), + new HttpCredentialsAdapter(credentials)) + .setApplicationName(applicationName) + .build(); + } catch (IOException e) { + throw new IllegalStateException("Google Drive 인증 파일을 로드할 수 없습니다.", e); + } } - }backend/src/main/java/moadong/media/service/ClubImageService.java (1)
1-19: 인터페이스 설계 개선 제안인터페이스 설계가 명확하게 구현되었으나, 메서드 문서화와 반환 값 및 예외 처리에 대한 명세가 부족합니다. 또한 파일 존재 여부 확인 메서드 추가를 고려해 볼 수 있습니다.
package moadong.media.service; import java.util.List; import moadong.club.entity.Club; import org.springframework.web.multipart.MultipartFile; +/** + * 클럽 이미지 관리를 위한 서비스 인터페이스 + * 로고 및 피드 이미지의 업로드, 수정, 삭제 기능을 제공합니다. + */ public interface ClubImageService { + /** + * 클럽 로고를 업로드합니다. + * + * @param clubId 클럽 ID + * @param file 업로드할 이미지 파일 + * @return 업로드된 이미지의 URL + * @throws RestApiException 클럽을 찾을 수 없거나 파일 업로드에 실패한 경우 + */ String uploadLogo(String clubId, MultipartFile file); + /** + * 클럽 로고를 삭제합니다. + * + * @param clubId 클럽 ID + * @throws RestApiException 클럽을 찾을 수 없거나 파일 삭제에 실패한 경우 + */ void deleteLogo(String clubId); + /** + * 클럽 피드 이미지를 업로드합니다. + * + * @param clubId 클럽 ID + * @param file 업로드할 이미지 파일 + * @return 업로드된 이미지의 URL + * @throws RestApiException 클럽을 찾을 수 없거나, 최대 피드 개수를 초과하거나, 파일 업로드에 실패한 경우 + */ String uploadFeed(String clubId, MultipartFile file); + /** + * 클럽 피드 이미지 목록을 업데이트합니다. + * + * @param clubId 클럽 ID + * @param newFeedImageList 새로운 피드 이미지 URL 목록 + * @throws RestApiException 클럽을 찾을 수 없거나, 최대 피드 개수를 초과하거나, 파일 업데이트에 실패한 경우 + */ void updateFeeds(String clubId, List<String> newFeedImageList); + /** + * 특정 클럽 파일을 삭제합니다. + * + * @param club 클럽 엔티티 + * @param filePath 삭제할 파일 경로 + * @throws RestApiException 파일 삭제에 실패한 경우 + */ void deleteFile(Club club, String filePath); + /** + * 파일이 존재하는지 확인합니다. + * + * @param filePath 확인할 파일 경로 + * @return 파일 존재 여부 + */ + boolean fileExists(String filePath); }backend/src/main/java/moadong/media/service/GcsClubImageService.java (3)
36-50: uploadLogo 메서드 구현은 적절하나 문서화가 필요합니다.메서드 구현은 적절하게 되어 있습니다:
- 클럽 ID 유효성 검사 및 클럽 조회
- 기존 로고가 있는 경우 삭제
- 새 로고 업로드 및 클럽 정보 업데이트
다만, 다음과 같은 개선을 권장합니다:
- 메서드에 JavaDoc 주석 추가
- 클럽 ID가 유효하지 않을 경우와 파일 업로드 실패 시 발생하는 예외 명시
+ /** + * 클럽 로고를 업로드합니다. + * + * @param clubId 클럽 ID + * @param file 업로드할 이미지 파일 + * @return 업로드된 이미지의 URL + * @throws RestApiException CLUB_NOT_FOUND: 클럽을 찾을 수 없는 경우 + * CLUB_ID_INVALID: 클럽 ID가 유효하지 않은 경우 + * IMAGE_UPLOAD_FAILED: 이미지 업로드에 실패한 경우 + */ @Override public String uploadLogo(String clubId, MultipartFile file) { ObjectId objectId = ObjectIdConverter.convertString(clubId); Club club = clubRepository.findClubById(objectId) .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); if (club.getClubRecruitmentInformation().getLogo() != null) { deleteFile(club, club.getClubRecruitmentInformation().getLogo()); } String filePath = uploadFile(clubId, file, FileType.LOGO); club.updateLogo(filePath); clubRepository.save(club); return filePath; }
144-146: 유틸리티 메서드 사용 방식 개선이 코드는 이제
ClubImageUtil클래스에서 가져온containsKorean메서드를 사용하고 있습니다. 하지만 정적 임포트 형태로 사용하는 것보다 클래스명을 명시적으로 사용하여 호출하는 것이 코드의 출처를 더 명확하게 할 수 있습니다.- import static moadong.media.util.ClubImageUtil.containsKorean; + import moadong.media.util.ClubImageUtil; // ... - if (containsKorean(originalFileName)) { + if (ClubImageUtil.containsKorean(originalFileName)) { originalFileName = RandomStringUtil.generateRandomString(10) + "." + contentType; }
93-100: 전체 코드 개선사항 제안전반적인 코드는 잘 구현되어 있지만, 다음과 같은 개선 사항을 고려해 보세요:
- 모든 public 메서드에 JavaDoc 주석 추가
- 오류 발생 시 로깅 강화 (현재는 예외 발생 시 상세 정보가 로그에 기록되지 않음)
- 테스트 코드 작성 (특히 파일 경로 파싱과 관련된 코드)
- 리팩토링된 인터페이스에 맞게
fileExists메서드 구현 (인터페이스에 제안한 경우)이러한 개선 사항에 대한 구현 도움이 필요하시면 알려주세요. 특히 테스트 코드나 로깅 개선에 대한 예제 코드를 제공해 드릴 수 있습니다.
backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java (2)
25-36: 의존성 주입 및 클래스 구조 검토클래스 구조와 의존성 주입 방식은 잘 구현되어 있습니다. Lombok을 활용한
@RequiredArgsConstructor와 Spring의@Value주입이 적절히 사용되었습니다.그러나
shareFileId와MAX_FEED_COUNT변수의 가시성 일관성이 필요합니다. 현재shareFileId는 접근 제한자가 없고MAX_FEED_COUNT는 private입니다.- @Value("${google.drive.share-file-id}") - String shareFileId; + @Value("${google.drive.share-file-id}") + private String shareFileId;
97-103: 피드 이미지 삭제 헬퍼 메서드 검토
deleteFeedImages메서드의 로직은 간단하고 명확하게 구현되어 있습니다. 다만 변수명의 일관성을 위해 수정이 필요합니다.private void deleteFeedImages(Club club, List<String> feedImages, List<String> newFeedImages) { - for (String feedsImage : feedImages) { + for (String feedImage : feedImages) { - if (!newFeedImages.contains(feedsImage)) { - deleteFile(club, feedsImage); + if (!newFeedImages.contains(feedImage)) { + deleteFile(club, feedImage); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/.gitignore(1 hunks)backend/build.gradle(1 hunks)backend/src/main/java/moadong/global/exception/ErrorCode.java(1 hunks)backend/src/main/java/moadong/media/controller/ClubImageController.java(1 hunks)backend/src/main/java/moadong/media/domain/FileType.java(1 hunks)backend/src/main/java/moadong/media/dto/FeedUpdateRequest.java(1 hunks)backend/src/main/java/moadong/media/service/ClubImageService.java(1 hunks)backend/src/main/java/moadong/media/service/GcsClubImageService.java(5 hunks)backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java(1 hunks)backend/src/main/java/moadong/media/util/ClubImageUtil.java(1 hunks)backend/src/main/java/moadong/media/util/GoogleDriveConfig.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/src/main/java/moadong/media/controller/ClubImageController.java (1)
backend/src/main/java/moadong/media/service/GcsClubImageService.java (1)
RequiredArgsConstructor(23-160)
backend/src/main/java/moadong/media/service/GcsClubImageService.java (4)
backend/src/main/java/moadong/media/util/ClubImageUtil.java (1)
ClubImageUtil(6-12)backend/src/main/java/moadong/global/util/ObjectIdConverter.java (1)
ObjectIdConverter(7-17)backend/src/main/java/moadong/global/util/RandomStringUtil.java (1)
RandomStringUtil(5-22)backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java (1)
Service(25-171)
🔇 Additional comments (9)
backend/.gitignore (1)
41-41: 파일 끝에 개행 추가로 POSIX 호환성 보장
.gitignore마지막 줄에 개행 문자가 추가되어, POSIX 표준을 준수하고 Git diff 시 불필요한 변경 표시를 방지합니다.backend/src/main/java/moadong/media/dto/FeedUpdateRequest.java (1)
1-1: 패키지 구조 조정 확인
FeedUpdateRequest클래스가moadong.media.dto로 이동된 것이 적절합니다. 내부 로직에 변경이 없으므로 정상 동작을 보장합니다.backend/src/main/java/moadong/media/domain/FileType.java (1)
1-1: 패키지 구조 조정 확인
FileTypeEnum이moadong.media.domain로 옮겨진 것이 일관적이며 인터페이스 분리에도 잘 부합합니다.backend/src/main/java/moadong/media/controller/ClubImageController.java (3)
1-1: 패키지 경로 변경 확인
컨트롤러 패키지가moadong.media.controller로 올바르게 조정되었습니다.
7-7: DTO import 경로 수정 확인
FeedUpdateRequest임포트가moadong.media.dto로 올바르게 변경되었습니다.
9-9: Service 인터페이스 import 확인
ClubImageService임포트가moadong.media.service로 올바르게 변경되었습니다.backend/src/main/java/moadong/global/exception/ErrorCode.java (1)
18-18: 파일 전송 오류 코드 추가 확인
FILE_TRANSFER_ERROR(HttpStatus.BAD_REQUEST, "601-6", "파일을 올바른 형식으로 변경할 수 없습니다.")추가가 적절합니다.backend/src/main/java/moadong/media/service/GcsClubImageService.java (1)
31-32: 하드코딩된 상수가 설정 값으로 대체됨하드코딩된
MAX_FEED_COUNT상수를 설정 파일에서 주입받는 방식으로 변경한 것은 좋은 개선입니다. 이렇게 하면 코드 변경 없이 설정만으로 값을 조정할 수 있습니다.backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java (1)
1-24: 패키지 구조 및 임포트 검토패키지 구조와 임포트 구문은 잘 정리되어 있습니다. 필요한 클래스들이 적절하게 임포트되어 있습니다.
Comment on lines
+22
to
23
| //@Service | ||
| @RequiredArgsConstructor |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
@service 어노테이션이 주석 처리된 이유 확인 필요
@Service 어노테이션이 주석 처리되어 있습니다. 이렇게 되면 Spring이 이 클래스를 서비스 빈으로 자동 등록하지 않아 의존성 주입에 문제가 발생할 수 있습니다.
가능한 이유:
- 구글 드라이브 서비스와 GCS 서비스 중 하나만 사용하기 위한 의도일 수 있습니다.
- 다른 설정 방식(예: Java 설정 클래스)으로 빈을 등록하려는 의도일 수 있습니다.
의도적인 경우가 아니라면 주석을 제거하거나, 조건부 빈 등록(@ConditionalOnProperty)을 사용하는 것이 좋습니다.
다음 스크립트로 ClubImageService를 주입받는 곳이 있는지 확인해보세요:
🏁 Script executed:
#!/bin/bash
# 의존성 주입 방식 확인
rg "ClubImageService" --type javaLength of output: 659
GcsClubImageService에 @service 주석 해제 및 조건부 빈 등록 검토
현재 GcsClubImageService 클래스 상단의 @Service 어노테이션이 주석 처리되어 있어 Spring 컨텍스트에 빈으로 등록되지 않습니다. 이로 인해 ClubImageController에서 ClubImageService를 주입받을 때 GCS 구현체가 등록되지 않아 의도와 다른 구현체가 선택되거나 주입 오류가 발생할 수 있습니다.
검증 결과:
ClubImageController(backend/src/main/java/moadong/media/controller/ClubImageController.java)에서private final ClubImageService clubImageService;를 통해 주입받고 있습니다.GoogleDriveClubImageService는 빈으로 등록되나,GcsClubImageService는 등록되지 않습니다.
수정 제안:
- 의도대로 GCS 구현체를 사용하려면:
GcsClubImageService(backend/src/main/java/moadong/media/service/GcsClubImageService.java:22–23)에서@Service주석을 제거- 환경별 선택적 빈 등록이 필요하다면
@ConditionalOnProperty("club.image.storage","gcs")등을 추가
- GCS 구현체를 현재 사용하지 않으려면:
- 해당 클래스를 제거하거나, 주석 처리 상태를 유지하되 불필요한 코드는 삭제
Comment on lines
+79
to
+95
| @Override | ||
| public void updateFeeds(String clubId, List<String> newFeedImageList) { | ||
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | ||
| Club club = clubRepository.findClubById(objectId) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | ||
|
|
||
| if (newFeedImageList.size() > MAX_FEED_COUNT) { | ||
| throw new RestApiException(ErrorCode.TOO_MANY_FILES); | ||
| } | ||
|
|
||
| List<String> feedImages = club.getClubRecruitmentInformation().getFeedImages(); | ||
| if (feedImages != null && !feedImages.isEmpty()) { | ||
| deleteFeedImages(club, feedImages, newFeedImageList); | ||
| } | ||
| club.updateFeedImages(newFeedImageList); | ||
| clubRepository.save(club); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
피드 이미지 업데이트 메서드 검토
피드 이미지 업데이트 로직은 전반적으로 잘 구현되어 있습니다. 트랜잭션 관리가 필요합니다.
+ @Transactional
@Override
public void updateFeeds(String clubId, List<String> newFeedImageList) {📝 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.
Suggested change
| @Override | |
| public void updateFeeds(String clubId, List<String> newFeedImageList) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (newFeedImageList.size() > MAX_FEED_COUNT) { | |
| throw new RestApiException(ErrorCode.TOO_MANY_FILES); | |
| } | |
| List<String> feedImages = club.getClubRecruitmentInformation().getFeedImages(); | |
| if (feedImages != null && !feedImages.isEmpty()) { | |
| deleteFeedImages(club, feedImages, newFeedImageList); | |
| } | |
| club.updateFeedImages(newFeedImageList); | |
| clubRepository.save(club); | |
| } | |
| @Transactional | |
| @Override | |
| public void updateFeeds(String clubId, List<String> newFeedImageList) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (newFeedImageList.size() > MAX_FEED_COUNT) { | |
| throw new RestApiException(ErrorCode.TOO_MANY_FILES); | |
| } | |
| List<String> feedImages = club.getClubRecruitmentInformation().getFeedImages(); | |
| if (feedImages != null && !feedImages.isEmpty()) { | |
| deleteFeedImages(club, feedImages, newFeedImageList); | |
| } | |
| club.updateFeedImages(newFeedImageList); | |
| clubRepository.save(club); | |
| } |
Comment on lines
+66
to
+77
| @Override | ||
| public String uploadFeed(String clubId, MultipartFile file) { | ||
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | ||
| int feedImagesCount = clubRepository.findClubById(objectId) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)) | ||
| .getClubRecruitmentInformation().getFeedImages().size(); | ||
|
|
||
| if (feedImagesCount + 1 > MAX_FEED_COUNT) { | ||
| throw new RestApiException(ErrorCode.TOO_MANY_FILES); | ||
| } | ||
| return uploadFile(clubId, file, FileType.FEED); | ||
| } |
Contributor
There was a problem hiding this comment.
피드 업로드 메서드 검토
피드 이미지 업로드 로직은 잘 구현되었으나, 클럽 엔티티 저장 과정이 누락되어 있습니다. 현재 코드는 파일을 업로드하지만 클럽 엔티티의 피드 이미지 리스트를 업데이트하지 않고 있습니다.
피드 이미지 URL을 클럽 엔티티에 추가하고 저장하는 로직을 추가해야 합니다:
@Override
+ @Transactional
public String uploadFeed(String clubId, MultipartFile file) {
ObjectId objectId = ObjectIdConverter.convertString(clubId);
- int feedImagesCount = clubRepository.findClubById(objectId)
+ Club club = clubRepository.findClubById(objectId)
.orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND));
- .getClubRecruitmentInformation().getFeedImages().size();
+ int feedImagesCount = club.getClubRecruitmentInformation().getFeedImages().size();
if (feedImagesCount + 1 > MAX_FEED_COUNT) {
throw new RestApiException(ErrorCode.TOO_MANY_FILES);
}
- return uploadFile(clubId, file, FileType.FEED);
+ String filePath = uploadFile(clubId, file, FileType.FEED);
+ List<String> feedImages = new ArrayList<>(club.getClubRecruitmentInformation().getFeedImages());
+ feedImages.add(filePath);
+ club.updateFeedImages(feedImages);
+ clubRepository.save(club);
+ return filePath;
}📝 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.
Suggested change
| @Override | |
| public String uploadFeed(String clubId, MultipartFile file) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| int feedImagesCount = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)) | |
| .getClubRecruitmentInformation().getFeedImages().size(); | |
| if (feedImagesCount + 1 > MAX_FEED_COUNT) { | |
| throw new RestApiException(ErrorCode.TOO_MANY_FILES); | |
| } | |
| return uploadFile(clubId, file, FileType.FEED); | |
| } | |
| @Override | |
| @Transactional | |
| public String uploadFeed(String clubId, MultipartFile file) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| int feedImagesCount = club.getClubRecruitmentInformation().getFeedImages().size(); | |
| if (feedImagesCount + 1 > MAX_FEED_COUNT) { | |
| throw new RestApiException(ErrorCode.TOO_MANY_FILES); | |
| } | |
| String filePath = uploadFile(clubId, file, FileType.FEED); | |
| List<String> feedImages = new ArrayList<>(club.getClubRecruitmentInformation().getFeedImages()); | |
| feedImages.add(filePath); | |
| club.updateFeedImages(feedImages); | |
| clubRepository.save(club); | |
| return filePath; | |
| } |
Comment on lines
+53
to
+64
| @Override | ||
| public void deleteLogo(String clubId) { | ||
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | ||
| Club club = clubRepository.findClubById(objectId) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | ||
|
|
||
| if (club.getClubRecruitmentInformation().getLogo() != null) { | ||
| deleteFile(club, club.getClubRecruitmentInformation().getLogo()); | ||
| } | ||
| club.updateLogo(null); | ||
| clubRepository.save(club); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
로고 삭제 메서드 구현 검토
deleteLogo 메서드 역시 클럽 정보 조회와 업데이트 작업을 하므로 트랜잭션 관리가 필요합니다.
+ @Transactional
@Override
public void deleteLogo(String clubId) {📝 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.
Suggested change
| @Override | |
| public void deleteLogo(String clubId) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (club.getClubRecruitmentInformation().getLogo() != null) { | |
| deleteFile(club, club.getClubRecruitmentInformation().getLogo()); | |
| } | |
| club.updateLogo(null); | |
| clubRepository.save(club); | |
| } | |
| @Transactional | |
| @Override | |
| public void deleteLogo(String clubId) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (club.getClubRecruitmentInformation().getLogo() != null) { | |
| deleteFile(club, club.getClubRecruitmentInformation().getLogo()); | |
| } | |
| club.updateLogo(null); | |
| clubRepository.save(club); | |
| } |
Comment on lines
+37
to
+51
| @Override | ||
| public String uploadLogo(String clubId, MultipartFile file) { | ||
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | ||
| Club club = clubRepository.findClubById(objectId) | ||
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | ||
|
|
||
| if (club.getClubRecruitmentInformation().getLogo() != null) { | ||
| deleteFile(club, club.getClubRecruitmentInformation().getLogo()); | ||
| } | ||
|
|
||
| String filePath = uploadFile(clubId, file, FileType.LOGO); | ||
| club.updateLogo(filePath); | ||
| clubRepository.save(club); | ||
| return filePath; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
로고 업로드 메서드 구현 검토
로고 업로드 로직은 잘 구현되어 있으나, 트랜잭션 관리가 필요할 수 있습니다. 현재 파일 업로드와 클럽 정보 업데이트가 별도 작업으로 진행되고 있어, 한 작업이 실패할 경우 일관성 문제가 발생할 수 있습니다.
메서드에 @Transactional 어노테이션을 추가하여 트랜잭션 관리를 개선하세요:
+ @Transactional
@Override
public String uploadLogo(String clubId, MultipartFile file) {📝 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.
Suggested change
| @Override | |
| public String uploadLogo(String clubId, MultipartFile file) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (club.getClubRecruitmentInformation().getLogo() != null) { | |
| deleteFile(club, club.getClubRecruitmentInformation().getLogo()); | |
| } | |
| String filePath = uploadFile(clubId, file, FileType.LOGO); | |
| club.updateLogo(filePath); | |
| clubRepository.save(club); | |
| return filePath; | |
| } | |
| @Transactional | |
| @Override | |
| public String uploadLogo(String clubId, MultipartFile file) { | |
| ObjectId objectId = ObjectIdConverter.convertString(clubId); | |
| Club club = clubRepository.findClubById(objectId) | |
| .orElseThrow(() -> new RestApiException(ErrorCode.CLUB_NOT_FOUND)); | |
| if (club.getClubRecruitmentInformation().getLogo() != null) { | |
| deleteFile(club, club.getClubRecruitmentInformation().getLogo()); | |
| } | |
| String filePath = uploadFile(clubId, file, FileType.LOGO); | |
| club.updateLogo(filePath); | |
| clubRepository.save(club); | |
| return filePath; | |
| } |
Comment on lines
+105
to
+117
| @Override | ||
| public void deleteFile(Club club, String filePath) { | ||
| //"https://drive.google.com/file/d/{fileId}/view" -> {fileId} | ||
| String fileId = filePath.split("/")[5]; | ||
| try { | ||
| googleDrive.files() | ||
| .delete(fileId) | ||
| .setSupportsAllDrives(true) // 공유 드라이브(Shared Drive)도 지원할 경우 | ||
| .execute(); | ||
| } catch (IOException e) { | ||
| throw new RestApiException(ErrorCode.IMAGE_DELETE_FAILED); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
파일 삭제 메서드 구현 관련 중요 이슈
현재 파일 ID 추출 방식은 Google Drive URL 형식에 의존하고 있어 Google Drive가 URL 형식을 변경하면 문제가 발생할 수 있습니다. 또한 split 인덱스를 하드코딩하는 것은 안전하지 않습니다.
정규식 또는 URL 파싱 라이브러리를 사용하여 더 안정적으로 fileId를 추출하도록 개선하세요:
- //"https://drive.google.com/file/d/{fileId}/view" -> {fileId}
- String fileId = filePath.split("/")[5];
+ // "https://drive.google.com/file/d/{fileId}/view" -> {fileId}
+ String fileId;
+ try {
+ java.util.regex.Pattern pattern = java.util.regex.Pattern.compile("https://drive\\.google\\.com/file/d/([^/]+)/view");
+ java.util.regex.Matcher matcher = pattern.matcher(filePath);
+ if (matcher.find()) {
+ fileId = matcher.group(1);
+ } else {
+ throw new RestApiException(ErrorCode.INVALID_FILE_URL);
+ }
+ } catch (Exception e) {
+ throw new RestApiException(ErrorCode.INVALID_FILE_URL);
+ }추가로 ErrorCode.INVALID_FILE_URL 오류 코드를 정의해야 합니다.
Comment on lines
+119
to
+169
| private String uploadFile(String clubId, MultipartFile file, FileType fileType) { | ||
| if (file == null) { | ||
| throw new RestApiException(ErrorCode.FILE_NOT_FOUND); | ||
| } | ||
| // MultipartFile → java.io.File 변환 | ||
| java.io.File tempFile; | ||
| try { | ||
| tempFile = java.io.File.createTempFile("upload-", file.getOriginalFilename()); | ||
| file.transferTo(tempFile); | ||
| } catch (IOException e) { | ||
| throw new RestApiException(ErrorCode.FILE_TRANSFER_ERROR); | ||
| } | ||
|
|
||
| // 메타데이터 생성 | ||
| File fileMetadata = new File(); | ||
| String fileName = file.getOriginalFilename(); | ||
| if (containsKorean(fileName)) { | ||
| fileName = RandomStringUtil.generateRandomString(10); | ||
| } | ||
|
|
||
| fileMetadata.setName(clubId + "/" + fileType + "/" + fileName); | ||
| fileMetadata.setMimeType(file.getContentType()); | ||
| // 공유 ID 설정 | ||
| fileMetadata.setParents(Collections.singletonList(shareFileId)); | ||
|
|
||
| // 파일 업로드 | ||
| FileContent mediaContent = new FileContent(file.getContentType(), tempFile); | ||
| // 전체 공개 권한 설정 | ||
| Permission publicPermission = new Permission() | ||
| .setType("anyone") // 누구나 | ||
| .setRole("reader"); // 읽기 권한 | ||
|
|
||
| File uploadedFile; | ||
| try { | ||
| uploadedFile= googleDrive.files().create(fileMetadata, mediaContent) | ||
| .setFields("id") | ||
| .execute(); | ||
|
|
||
| googleDrive.permissions().create(uploadedFile.getId(), publicPermission) | ||
| .setFields("id") | ||
| .execute(); | ||
| } catch (Exception e) { | ||
| throw new RestApiException(ErrorCode.IMAGE_UPLOAD_FAILED); | ||
| }finally { | ||
| // 임시 파일 삭제 | ||
| tempFile.delete(); | ||
| } | ||
| // 공유 링크 반환 | ||
| String publicUrl = "https://drive.google.com/file/d/" + uploadedFile.getId() + "/view"; | ||
| return publicUrl; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
파일 업로드 메서드 구현 검토
파일 업로드 메서드는 전반적으로 잘 구현되어 있지만, 몇 가지 개선 사항이 있습니다:
- 파일 크기나 형식에 대한 검증이 없습니다.
- 임시 파일 생성 시 try-with-resources 패턴을 사용하지 않았습니다.
- 파일 이름에서 한글만 확인하지만 다른 비ASCII 문자에 대한 처리도 필요할 수 있습니다.
- 모든 파일에 공개 권한을 부여하는 것은 보안 이슈가 될 수 있습니다.
파일 크기 및 형식 검증을 추가하고, 임시 파일 관리를 개선하세요:
private String uploadFile(String clubId, MultipartFile file, FileType fileType) {
if (file == null) {
throw new RestApiException(ErrorCode.FILE_NOT_FOUND);
}
+
+ // 파일 크기 및 형식 검증
+ if (file.getSize() > 10 * 1024 * 1024) { // 예: 10MB 제한
+ throw new RestApiException(ErrorCode.FILE_SIZE_EXCEEDED);
+ }
+
+ // 지원되는 형식 검증 (이미지 파일만 허용하는 예시)
+ String contentType = file.getContentType();
+ if (contentType == null || !contentType.startsWith("image/")) {
+ throw new RestApiException(ErrorCode.UNSUPPORTED_FILE_TYPE);
+ }
+
// MultipartFile → java.io.File 변환
- java.io.File tempFile;
+ java.io.File tempFile = null;
try {
tempFile = java.io.File.createTempFile("upload-", file.getOriginalFilename());
file.transferTo(tempFile);또한 파일명 처리 로직을 개선하세요:
// 메타데이터 생성
File fileMetadata = new File();
String fileName = file.getOriginalFilename();
- if (containsKorean(fileName)) {
+ // 한글뿐만 아니라 모든 특수문자에 대비하여 파일명 정리
+ if (fileName == null || !fileName.matches("[a-zA-Z0-9._-]+")) {
fileName = RandomStringUtil.generateRandomString(10);
}
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.
#️⃣연관된 이슈
📝작업 내용
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링
기타