Conversation
## Walkthrough
`GoogleDriveClubImageService` 클래스에 Google Drive 파일 URL 구성용 상수 `PREFIX`와 `SUFFIX`가 추가되고, `uploadFile` 메서드가 이 상수들을 사용하도록 변경되었습니다. 클럽 검색 서비스와 Google Drive 이미지 서비스(로고, 피드) 관련 새로운 단위 테스트 클래스들이 추가되어 다양한 시나리오를 검증합니다.
## Changes
| 파일/경로 | 변경 요약 |
|---------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------|
| backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java | Google Drive 파일 URL 구성용 상수 `PREFIX`, `SUFFIX` 추가 및 `uploadFile` 메서드 내 URL 생성 로직 상수 사용으로 변경 |
| backend/src/test/java/moadong/club/service/ClubSearchServiceTest.java | 클럽 검색 서비스의 `searchClubsByKeyword` 메서드 정렬 및 필터링 로직 검증을 위한 단위 테스트 클래스 신규 추가 |
| backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceFeedTest.java<br>backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java | Google Drive 클럽 피드 이미지 및 로고 이미지 업로드/삭제 기능에 대한 단위 테스트 클래스 신규 추가, 예외 처리 및 상태 변경 검증 |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant ClubSearchService
participant ClubSearchRepository
User->>ClubSearchService: searchClubsByKeyword(criteria)
ClubSearchService->>ClubSearchRepository: findClubs(criteria)
ClubSearchRepository-->>ClubSearchService: List<Club>
ClubSearchService-->>User: Sorted/Filtered List<Club>sequenceDiagram
participant User
participant GoogleDriveClubImageService
participant ClubRepository
participant Drive
User->>GoogleDriveClubImageService: uploadClubLogo(clubId, file)
GoogleDriveClubImageService->>ClubRepository: findById(clubId)
alt Club exists
GoogleDriveClubImageService->>Drive: upload(file)
Drive-->>GoogleDriveClubImageService: fileId
GoogleDriveClubImageService->>ClubRepository: save(updatedClub)
GoogleDriveClubImageService-->>User: logoUrl
else Club not found
GoogleDriveClubImageService-->>User: throw CLUB_NOT_FOUND
end
sequenceDiagram
participant User
participant GoogleDriveClubImageService
participant ClubRepository
participant Drive
User->>GoogleDriveClubImageService: uploadClubFeedImage(clubId, file)
GoogleDriveClubImageService->>ClubRepository: findById(clubId)
alt Club exists
GoogleDriveClubImageService->>GoogleDriveClubImageService: checkFeedImageCount()
alt Count exceeded
GoogleDriveClubImageService-->>User: throw TOO_MANY_FILES
else
GoogleDriveClubImageService->>Drive: upload(file)
Drive-->>GoogleDriveClubImageService: fileId
GoogleDriveClubImageService->>ClubRepository: save(updatedClub)
GoogleDriveClubImageService-->>User: feedImageUrl
end
else Club not found
GoogleDriveClubImageService-->>User: throw CLUB_NOT_FOUND
end
Possibly related PRs
Suggested labels
Suggested reviewers
|
Test Results27 tests 27 ✅ 1s ⏱️ Results for commit ee3ad32. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
backend/src/test/java/moadong/club/service/ClubSearchServiceTest.java (2)
75-103: 2차, 3차 정렬 테스트의 검증 방식 개선 제안현재 각 항목별로 개별 assertEquals를 사용하고 있는데, 첫 번째 테스트처럼 assertIterableEquals를 사용하면 코드가 더 간결해질 수 있습니다.
다음과 같이 변경해 보세요:
- assertEquals("club1", sorted.get(0).name()); - assertEquals("club2", sorted.get(1).name()); - assertEquals("club3", sorted.get(2).name()); + List<ClubSearchResult> expected = List.of(club1, club2, club3); + assertIterableEquals(expected, sorted);
1-106: 테스트 코드의 전반적인 품질이 좋습니다테스트 메서드 이름이 한국어로 명확하게 테스트 의도를 전달하고 있으며, given-when-then 패턴을 따라 테스트 코드가 구조화되어 이해하기 쉽습니다. 다만 다음 사항을 고려해 보세요:
- 엣지 케이스 테스트 추가: null 또는 빈 키워드, 유효하지 않은 모집상태 등의 경계 조건에 대한 테스트를 추가하면 코드의 견고성을 더 높일 수 있습니다.
- 필터링 기능 테스트: 현재는 정렬 기능에 집중되어 있는데, 카테고리나 구역별 필터링 기능에 대한 테스트도 추가하면 좋을 것 같습니다.
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceFeedTest.java (1)
88-106: 테스트 케이스 개선 제안현재 테스트는 feedImages 목록 변경을 잘 검증하고 있습니다. 더 완벽한 테스트를 위해
deleteFile메서드가 삭제된 피드 이미지에 대해서만 호출되는지도 검증하면 좋을 것 같습니다.// then assertIterableEquals(newList, club.getClubRecruitmentInformation().getFeedImages()); +// 특정 이미지만 삭제되었는지 검증 +verify(clubImageService).deleteFile(eq(club), eq("old1.jpg")); +verify(clubImageService).deleteFile(eq(club), eq("old2.jpg")); +verify(clubImageService, never()).deleteFile(eq(club), eq("new1.jpg"));backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java (1)
1-134: 기존 로고가 있는 경우에 대한 테스트 추가 제안현재 로고 업로드 테스트는 로고가 없는 상태에서의 업로드만 테스트하고 있습니다. 이미 로고가 있는 경우에 대한 테스트 케이스를 추가하면 좋을 것 같습니다.
@Test void 기존_로고가_있는_경우_삭제_후_업데이트된다() { // given String oldLogo = "old logo link"; ClubRecruitmentInformation info = ClubRecruitmentInformation.builder() .logo(oldLogo) .build(); club = Club.builder().clubRecruitmentInformation(info).build(); when(clubRepository.findClubById(objectId)).thenReturn(Optional.of(club)); doNothing().when(clubImageService).deleteFile(club, oldLogo); String newLogo = "https://drive.google.com/file/d/newId/view"; doReturn(newLogo).when(clubImageService).uploadFile(any(), eq(mockFile), eq(FileType.LOGO)); // when String result = clubImageService.uploadLogo(objectId.toHexString(), mockFile); // then assertEquals(newLogo, result); assertEquals(newLogo, club.getClubRecruitmentInformation().getLogo()); verify(clubImageService).deleteFile(club, oldLogo); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java(2 hunks)backend/src/test/java/moadong/club/service/ClubSearchServiceTest.java(1 hunks)backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceFeedTest.java(1 hunks)backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java(1 hunks)
🔇 Additional comments (10)
backend/src/test/java/moadong/club/service/ClubSearchServiceTest.java (4)
1-19: 테스트 클래스를 적절하게 구성하였습니다클래스의 구조와 필요한 임포트가 잘 구성되어 있으며, Mockito 확장을 사용하여 테스트 프레임워크를 적절하게 설정하였습니다.
21-26: 의존성 주입 모의 객체 설정이 적절합니다@mock과 @Injectmocks 어노테이션을 적절히 사용하여 테스트 대상 서비스와 그 의존성을 설정하였습니다.
27-55: 모집상태 정렬 테스트 케이스가 적절합니다모집상태 순서대로 정렬되는 기능에 대한 테스트가 잘 구현되었습니다. 다양한 모집상태(ALWAYS, OPEN, CLOSED, UPCOMING)를 가진 클럽들이 올바른 순서로 정렬되는지 검증하고 있습니다.
57-73: 빈 결과 처리 테스트 케이스가 적절합니다검색 결과가 없을 때의 동작을 확인하는 테스트 케이스가 올바르게 구현되었습니다.
backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java (2)
119-119: 접근 제한자 변경이 적절합니다.
uploadFile메서드를 private에서 public으로 변경한 것은 테스트 코드에서 접근할 수 있게 해주므로 단위 테스트 작성을 위해 적절한 변경입니다.
167-167: 코드 간소화가 잘 되었습니다.URL 문자열을 직접 반환하도록 변경하여 코드가 더 간결해졌습니다.
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceFeedTest.java (2)
1-132: 피드 이미지 관련 테스트가 잘 작성되었습니다.클럽 피드 이미지 업로드 및 업데이트 기능에 대한 테스트 케이스들이 잘 구성되어 있습니다. 성공 케이스와 예외 케이스를 모두 다루고 있습니다.
65-80: 테스트 케이스가 적절합니다.MAX_FEED_COUNT 이상의 피드를 업로드할 때 TOO_MANY_FILES 예외가 발생하는 시나리오를 정확하게 테스트하고 있습니다.
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java (2)
69-82: 테스트 케이스가 적절합니다.로고 업데이트 기능에 대한 테스트가 잘 작성되었습니다. 특히 mockito를 활용하여
uploadFile메서드의 반환값을 적절하게 모킹하고 있습니다.
106-122: 로고 삭제 테스트가 잘 작성되었습니다.로고 삭제 기능이 제대로 동작하는지 검증하는 테스트가 잘 작성되었습니다. 로고 필드의 값이 null로 변경되는지 확인하고 있습니다.
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private String uploadFile(String clubId, MultipartFile file, FileType fileType) { | ||
| public String uploadFile(String clubId, MultipartFile file, FileType fileType) { |
There was a problem hiding this comment.
테스트를 위해 접근제어자를 public으로 바꿔도 괜찮을까요?
There was a problem hiding this comment.
전 테스트를 위해서 public 으로 바꾸는 것은 안좋다고 생각하는데 어떠신가요?
There was a problem hiding this comment.
테스트하는 메서드 내부에서 private메서드를 호출시키는 형태로 되어있어서 uploadFile메서드를 Mockito로 처리하고 반환값을 받고 싶었는데 그것보다는 정상적으로 uploadFile을 호출하고 에러를 반환한다는 식으로 테스트하는 것으로 수정해야겠습니다..
| } | ||
|
|
||
| private String uploadFile(String clubId, MultipartFile file, FileType fileType) { | ||
| public String uploadFile(String clubId, MultipartFile file, FileType fileType) { |
There was a problem hiding this comment.
전 테스트를 위해서 public 으로 바꾸는 것은 안좋다고 생각하는데 어떠신가요?
| // 공유 링크 반환 | ||
| String publicUrl = "https://drive.google.com/file/d/" + uploadedFile.getId() + "/view"; | ||
| return publicUrl; | ||
| return "https://drive.google.com/file/d/" + uploadedFile.getId() + "/view"; |
There was a problem hiding this comment.
재사용성을 위해서 상수로 변경하는 게 더 좋을 것 같아요
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java (3)
67-68: 주석 처리된 코드가 있습니다.테스트 코드에 주석 처리된 스텁 코드가 남아있습니다. 이는 테스트의 의도를 불분명하게 만들고 코드 가독성을 저하시킬 수 있습니다. 현재 테스트 로직에 필요하지 않다면 제거하는 것이 좋습니다.
-// doReturn( "https://drive.google.com/file/d/" + club.getId() + "/LOGO/" + mockFile.getOriginalFilename() + "/view" ) -// .when(clubImageService).uploadFile(any(), eq(mockFile), eq(FileType.LOGO));
54-60: setUp 메서드에서 불필요한 예외 선언이 있습니다.
setUp메서드에throws NoSuchMethodException예외를 선언했지만, 메서드 내부에서 이 예외가 발생할 수 있는 코드가 없습니다. 불필요한 예외 선언은 제거하는 것이 좋습니다.-void setUp() throws NoSuchMethodException { +void setUp() {
33-126: 테스트 메서드 이름의 일관성을 유지하세요.테스트 메서드 이름이 한글로 작성되어 있는데, 이는 테스트의 목적을 명확히 하는 좋은 방법입니다. 하지만 이름 지정 패턴에 일관성이 있어야 합니다. 일부 메서드는 '로고를_'으로 시작하고 일부는 'logo를_'으로 시작합니다. 한글 표기법을 일관되게 사용하는 것이 가독성을 높일 수 있습니다.
-void logo를_업로드할_club이_존재하지_않는다면_CLUB_NOT_FOUND를_반환한다() { +void 로고를_업로드할_클럽이_존재하지_않는다면_CLUB_NOT_FOUND를_반환한다() { -void logo를_삭제할_club이_존재하지_않는다면_CLUB_NOT_FOUND를_반환한다() { +void 로고를_삭제할_클럽이_존재하지_않는다면_CLUB_NOT_FOUND를_반환한다() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java(2 hunks)backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/main/java/moadong/media/service/GoogleDriveClubImageService.java
🔇 Additional comments (4)
backend/src/test/java/moadong/media/service/GoogleDriveClubImageServiceLogoTest.java (4)
36-38: 서비스 클래스 주입 방식이 적절합니다.
@Spy와@InjectMocks어노테이션을 함께 사용하여 실제 메서드를 호출하면서도 필요한 경우 특정 메서드만 목킹할 수 있도록 설정한 부분이 좋습니다.
88-97: 적절한 예외 테스트 구현입니다.파일이 null일 경우
FILE_NOT_FOUND예외를 발생시키는 테스트가 잘 구현되어 있습니다. 입력 유효성 검사에 대한 테스트는 중요합니다.
100-115: 로고 삭제 테스트가 명확합니다.로고 삭제 기능에 대한 테스트가 잘 구현되어 있습니다. 특히
doNothing().when(clubImageService).deleteFile(club, "test link");부분에서 삭제 메서드를 적절히 스텁하고, 삭제 후 로고 값이 null이 되는지 검증하는 로직이 명확합니다.
117-126: 존재하지 않는 클럽에 대한 예외 처리 테스트가 적절합니다.존재하지 않는 클럽의 로고를 삭제하려고 할 때
CLUB_NOT_FOUND예외가 발생하는지 테스트하는 로직이 잘 구현되어 있습니다.
Zepelown
left a comment
There was a problem hiding this comment.
고생하셨습니다. 코드레빗이 제시한 불필요한 import만 제거하면 될 듯합니다.
#️⃣연관된 이슈
📝작업 내용
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit