Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough앱 모듈 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
yubin012
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (22)
apps/api-user/src/main/java/com/ott/api_user/playlist/service/PlaylistPreferenceService.java (2)
84-85: 정렬을 위한 가변 리스트 래핑 - LGTM!
findAllById()가 반환하는 리스트가 불변일 수 있으므로ArrayList로 래핑하여sort()호출을 안전하게 처리한 점이 좋습니다.동일하게 변수명
tags를tagList로 변경하면 프로젝트 네이밍 컨벤션과 일관성을 유지할 수 있습니다.♻️ 변수명 변경 제안
- List<Tag> tags = new ArrayList<>(tagRepository.findAllById(topTagIds)); - tags.sort(Comparator.comparing(tag -> topTagIds.indexOf(tag.getId()))); + List<Tag> tagList = new ArrayList<>(tagRepository.findAllById(topTagIds)); + tagList.sort(Comparator.comparing(tag -> topTagIds.indexOf(tag.getId()))); - return tags; + return tagList;As per coding guidelines, "Collection variable names with
Listsuffix".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/playlist/service/PlaylistPreferenceService.java` around lines 84 - 85, The local variable name tags should be renamed to tagList to follow the project's naming convention; update the declaration using new ArrayList<>(tagRepository.findAllById(topTagIds)) to assign to tagList and update the subsequent sort call (tags.sort(...)) to tagList.sort(Comparator.comparing(tag -> topTagIds.indexOf(tag.getId()))), and ensure any other references in PlaylistPreferenceService (e.g., later uses of tags) are updated to tagList.
77-80: 변경 가능한 리스트로 래핑한 것은 좋은 접근입니다.JPA 리포지토리가 불변 컬렉션을 반환할 수 있어
Collections.shuffle()호출 시UnsupportedOperationException이 발생할 수 있는데,ArrayList로 래핑하여 이를 방지한 점이 좋습니다.다만, 코딩 가이드라인에 따라 컬렉션 변수명에
List접미사를 사용해야 합니다.allTags→allTagList로 변경을 권장합니다.♻️ 변수명 변경 제안
- List<Tag> allTags = new ArrayList<>(tagRepository.findAll()); - Collections.shuffle(allTags); - return allTags.stream().limit(3).collect(Collectors.toList()); + List<Tag> allTagList = new ArrayList<>(tagRepository.findAll()); + Collections.shuffle(allTagList); + return allTagList.stream().limit(3).collect(Collectors.toList());As per coding guidelines, "Collection variable names with
Listsuffix".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/playlist/service/PlaylistPreferenceService.java` around lines 77 - 80, The local collection variable allTags in PlaylistPreferenceService is correctly wrapped with new ArrayList<>(tagRepository.findAll()) to avoid UnsupportedOperationException, but it violates the naming guideline; rename the variable to allTagList (update its declaration and all usages in the method) so the list variable follows the "List" suffix convention while leaving the logic using tagRepository and Collections.shuffle intact.apps/api-user/src/test/java/com/ott/api_user/shortform/service/ClickEventServiceTest.java (1)
67-84: 예외 케이스에서 저장 부수효과도 함께 잠가두는 걸 권장합니다.현재는 예외 코드만 확인하므로,
clickRepository.save(...)미호출까지 검증하면 회귀 방지에 더 좋습니다.테스트 보강 예시
import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; @@ void saveClickEvent_throwsWhenMemberMissing() { when(memberRepository.findById(5L)).thenReturn(Optional.empty()); assertThatThrownBy(() -> clickEventService.saveClickEvent(5L, 1L, ClickType.SHORT_CLICK)) .isInstanceOf(BusinessException.class) .hasFieldOrPropertyWithValue("errorCode", ErrorCode.USER_NOT_FOUND); + verifyNoInteractions(clickRepository); } @@ void saveClickEvent_throwsWhenShortFormMissing() { Member member = Member.builder().id(6L).email("u@ott").nickname("u").provider(Provider.KAKAO).role(Role.MEMBER).build(); when(memberRepository.findById(6L)).thenReturn(Optional.of(member)); when(shortFormRepository.findById(99L)).thenReturn(Optional.empty()); assertThatThrownBy(() -> clickEventService.saveClickEvent(6L, 99L, ClickType.SHORT_CLICK)) .isInstanceOf(BusinessException.class) .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHORT_FORM_NOT_FOUND); + verifyNoInteractions(clickRepository); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/shortform/service/ClickEventServiceTest.java` around lines 67 - 84, The tests saveClickEvent_throwsWhenMemberMissing and saveClickEvent_throwsWhenShortFormMissing currently only assert the BusinessException and ERROR codes; also verify that the clickRepository.save(...) interaction does not occur to prevent side-effect regressions. In ClickEventServiceTest, after the assertThatThrownBy in both methods, add a Mockito verifyZeroInteractions or verify(clickRepository, never()).save(...) (or equivalent) against the mocked clickRepository to ensure save is not called when member or short form is missing.apps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.java (4)
43-88: 카테고리 조회 실패 케이스 테스트 추가 고려현재 테스트는 중복 태그, 카테고리 불일치, 정상 저장 시나리오를 잘 커버하고 있습니다. 추가로
categoryRepository.findByIdAndStatus가 빈Optional을 반환하는 경우(존재하지 않는 카테고리)에 대한 테스트도 고려해 보세요.💡 추가 테스트 케이스 예시
`@Test` void linkTags_throwsWhenCategoryNotFound() { Media media = Media.builder().id(1L).build(); when(categoryRepository.findByIdAndStatus(10L, Status.ACTIVE)) .thenReturn(java.util.Optional.empty()); assertThatThrownBy(() -> mediaTagLinker.linkTags(media, 10L, List.of(1L))) .isInstanceOf(BusinessException.class); // 적절한 ErrorCode 검증 추가 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.java` around lines 43 - 88, Add a test that verifies mediaTagLinker.linkTags throws when categoryRepository.findByIdAndStatus returns Optional.empty: create a Media (Media.builder().id(1L).build()), stub categoryRepository.findByIdAndStatus(10L, Status.ACTIVE) to return Optional.empty(), call mediaTagLinker.linkTags(media, 10L, List.of(1L)) and assert that a BusinessException is thrown and (if applicable) its errorCode equals the expected code (e.g., ErrorCode.CATEGORY_NOT_FOUND); reference mediaTagLinker.linkTags and categoryRepository.findByIdAndStatus when locating where to add the test.
19-20: 사용하지 않는 import 제거
java.util.Set이 import되어 있지만 테스트 코드에서 사용되지 않습니다.♻️ 수정 제안
import java.util.List; -import java.util.Set; import org.junit.jupiter.api.Test;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.java` around lines 19 - 20, Remove the unused import by deleting the line importing java.util.Set in MediaTagLinkerTest; update the imports block (or run automatic organize/imports) so only required imports like java.util.List remain, ensuring MediaTagLinkerTest no longer contains the unused Set import.
84-87: 컬렉션 변수명에List접미사 사용 필요코딩 가이드라인에 따라 컬렉션 변수명에는
List접미사를 사용해야 합니다.♻️ 수정 제안
- List<MediaTag> saved = captor.getValue(); - assertThat(saved).hasSize(1); - assertThat(saved.get(0).getTag()).isSameAs(tag); - assertThat(saved.get(0).getMedia()).isSameAs(media); + List<MediaTag> savedList = captor.getValue(); + assertThat(savedList).hasSize(1); + assertThat(savedList.get(0).getTag()).isSameAs(tag); + assertThat(savedList.get(0).getMedia()).isSameAs(media);As per coding guidelines, "Collection variable names with
Listsuffix".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.java` around lines 84 - 87, Rename the local collection variable in MediaTagLinkerTest from saved to savedList to follow the "List" suffix guideline; update the assignment (savedList = captor.getValue()), all subsequent uses and assertions (assertThat(savedList).hasSize(1); assertThat(savedList.get(0).getTag()).isSameAs(tag); assertThat(savedList.get(0).getMedia()).isSameAs(media)) to reference savedList, and ensure any imports or static references remain correct.
82-84:@Captor어노테이션 사용 권장
ArgumentCaptor.forClass(List.class)는 제네릭 타입 정보를 잃어 unchecked assignment 경고가 발생합니다.@Captor어노테이션을 사용하면 타입 안전성을 유지할 수 있습니다.♻️ 수정 제안
필드 선언 추가:
`@Captor` private ArgumentCaptor<List<MediaTag>> mediaTagCaptor;테스트 메서드 내 변경:
- ArgumentCaptor<List<MediaTag>> captor = ArgumentCaptor.forClass(List.class); - verify(mediaTagRepository).saveAll(captor.capture()); - List<MediaTag> saved = captor.getValue(); + verify(mediaTagRepository).saveAll(mediaTagCaptor.capture()); + List<MediaTag> savedList = mediaTagCaptor.getValue();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.java` around lines 82 - 84, Replace the raw ArgumentCaptor.forClass usage with a typed `@Captor` field to avoid unchecked warnings: add a field `@Captor private ArgumentCaptor<List<MediaTag>> mediaTagCaptor;` to MediaTagLinkerTest and then use `verify(mediaTagRepository).saveAll(mediaTagCaptor.capture()); List<MediaTag> saved = mediaTagCaptor.getValue();` so the capture targets the saveAll call in mediaTagRepository with full generic type safety.apps/api-user/src/test/java/com/ott/api_user/history/service/WatchHistoryServiceTest.java (2)
59-62: 이벤트 타입만 검증하면 회귀를 놓칠 수 있습니다.현재는
WatchHistoryCreatedEvent인스턴스 여부만 확인합니다.memberId,contentsId같은 핵심 필드까지 검증해 두면 이벤트 생성 로직 변경 시 더 빨리 감지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/history/service/WatchHistoryServiceTest.java` around lines 59 - 62, The test currently only checks the event type; update the assertion in WatchHistoryServiceTest to capture and cast the published event (using eventCaptor and verify(eventPublisher).publishEvent(eventCaptor.capture())), then assert the event is a WatchHistoryCreatedEvent and verify its key fields (e.g., getMemberId()/memberId and getContentsId()/contentsId) match the expected values from the test setup (use assertEquals or AssertJ assertions against the expected memberId and contentsId); this ensures publishEvent emitted the correct payload, not just the correct type.
76-77: Line 76의intValue()변환은 제거하는 편이 안전합니다.
Long값을int로 축소 변환했다가 다시long연산에 쓰고 있어 불필요하며, 큰 값에서 오버플로우 위험이 있습니다.🔧 제안 변경
- .id(100L + mediaId.intValue()) + .id(100L + mediaId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/history/service/WatchHistoryServiceTest.java` around lines 76 - 77, Replace the unnecessary int narrowing on mediaId in the test: change the builder call currently using id(100L + mediaId.intValue()) to use the Long directly (e.g., id(100L + mediaId)) so you avoid pointless intValue() conversion and potential overflow; update the expression in WatchHistoryServiceTest where id(...) is set for the Media builder.apps/api-user/src/test/java/com/ott/api_user/series/service/SeriesServiceTest.java (3)
130-141: 테스트 데이터의 series ID가 혼란스럽습니다.Line 134에서
createSeries(seriesId + 1, 10L)로 series ID가 401L인 시리즈를 생성하지만, mock은seriesId=400L로 설정되어 있습니다. 현재 테스트는 동작하지만, 이런 불일치는 코드 이해와 유지보수를 어렵게 만들 수 있습니다.♻️ 일관된 ID 사용 제안
`@Test` void getFirstEpisodeMediaId_returnsFirstEpisodeMediaId() { Long seriesId = 400L; Pageable limitOne = PageRequest.of(0, 1); - Contents episode = createContents(401L, createSeries(seriesId + 1, 10L), 180); + Contents episode = createContents(401L, createSeries(seriesId, 10L), 180); when(contentsRepository.findBySeriesIdAndStatusAndMedia_PublicStatusOrderByIdAsc(seriesId, Status.ACTIVE, PublicStatus.PUBLIC, limitOne)) .thenReturn(new PageImpl<>(List.of(episode), limitOne, 1));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/series/service/SeriesServiceTest.java` around lines 130 - 141, The test uses inconsistent series IDs: it sets seriesId = 400L but creates a Contents with createSeries(seriesId + 1, 10L) (401L), which is confusing; update the test so the created series uses the same ID as the mocked query (use createSeries(seriesId, 10L)) so that the mocked repository call and the Contents passed to PageImpl align, ensuring getFirstEpisodeMediaId (invoked on seriesService via ReflectionTestUtils) is validated against a consistent series ID.
117-128: Reflection을 통한 private 메서드 테스트는 구현 세부사항에 결합됩니다.
ReflectionTestUtils.invokeMethod()로 private 메서드를 테스트하면 내부 구현 변경 시 테스트가 깨질 수 있습니다. 가능하다면 public 메서드(getSeriesContents등)를 통해 간접적으로 해당 동작을 검증하는 방식을 고려해 주세요.다만, 해당 예외 케이스가 public API를 통해 테스트하기 어려운 경우라면 현재 방식도 수용 가능합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/series/service/SeriesServiceTest.java` around lines 117 - 128, The test uses ReflectionTestUtils.invokeMethod to call the private method getFirstEpisodeMediaId which couples the test to implementation details; change the test to exercise the equivalent public behavior (e.g., call getSeriesContents or another public method on seriesService that triggers the same EPISODE_NOT_REGISTERED path) and assert the BusinessException with ErrorCode.EPISODE_NOT_REGISTERED, or if that public path is not available, refactor getFirstEpisodeMediaId to package-private (or extract a small helper class) so it can be tested without reflection and update the test to call the new accessible method instead of ReflectionTestUtils.invokeMethod.
164-188: 파라미터 이름mediaId가 실제 용도와 맞지 않습니다.
createContents메서드에서mediaId파라미터가Contents.id(line 167)와Media.id(line 170) 모두에 사용되고 있습니다. 이 이름은 오해를 유발할 수 있으므로id또는entityId로 변경하거나, 주석으로 의도를 명확히 해주세요.♻️ 파라미터 이름 명확화 제안
- // 컨텐츠/미디어 엔티티를 묶어서 재생 시간 단위 테스트에 쓰기 위한 헬퍼 - private static Contents createContents(Long mediaId, Series series, Integer duration) { + // 컨텐츠/미디어 엔티티를 묶어서 재생 시간 단위 테스트에 쓰기 위한 헬퍼 + // id는 테스트 편의상 Contents와 Media에 동일하게 적용됩니다. + private static Contents createContents(Long id, Series series, Integer duration) { return Contents.builder() - .id(mediaId) + .id(id) .series(series) .media(Media.builder() - .id(mediaId) + .id(id) .uploader(createMember(200L)) - .title("episode-" + mediaId) + .title("episode-" + id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/series/service/SeriesServiceTest.java` around lines 164 - 188, The parameter name mediaId in the helper method createContents causes confusion because it is used for both Contents.id and Media.id; rename the parameter to a clearer name (e.g., id or entityId) in the createContents signature and update all internal usages (Contents.builder().id(...) and Media.builder().id(...)) and any test callers to use the new name, or alternatively add a short comment above createContents explaining that the same id is intentionally used for both Contents and Media; ensure method name and references (createContents, Contents.builder, Media.builder) remain consistent.apps/api-user/src/test/java/com/ott/api_user/playback/service/PlaybackServiceTest.java (2)
53-56: 예외 코드 검증을 문자열 필드명 기반 대신 타입 안전 방식으로 바꾸는 것을 권장합니다.
hasFieldOrPropertyWithValue("errorCode", ...)는 필드명 변경에 취약합니다.변경 예시
+import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ assertThatThrownBy(() -> playbackService.upsertPlayback(1L, 5L, 10)) - .isInstanceOf(BusinessException.class) - .hasFieldOrPropertyWithValue("errorCode", ErrorCode.CONTENTS_NOT_FOUND); + .isInstanceOfSatisfying(BusinessException.class, + ex -> assertThat(ex.getErrorCode()).isEqualTo(ErrorCode.CONTENTS_NOT_FOUND));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/playback/service/PlaybackServiceTest.java` around lines 53 - 56, The test in PlaybackServiceTest uses the fragile hasFieldOrPropertyWithValue("errorCode", ...) assertion; change it to a type-safe check by extracting the thrown BusinessException and asserting its getErrorCode() equals ErrorCode.CONTENTS_NOT_FOUND — for example, replace the hasFieldOrPropertyWithValue call after assertThatThrownBy(() -> playbackService.upsertPlayback(1L, 5L, 10)) with a satisfies block that casts the exception to BusinessException and asserts ((BusinessException) e).getErrorCode() == ErrorCode.CONTENTS_NOT_FOUND; this uses the BusinessException#getErrorCode method and ErrorCode enum directly instead of relying on a string field name.
40-41: 테스트 픽스처에서Contents.id와Media.id를 동일값으로 두면 회귀를 가릴 수 있습니다.두 ID를 분리하면 서비스가 올바른 식별자를 사용하는지 더 정확히 검증할 수 있습니다.
변경 예시
- when(contentsRepository.findByMediaIdAndStatusAndMedia_PublicStatus(mediaId, Status.ACTIVE, PublicStatus.PUBLIC)) - .thenReturn(Optional.of(createContents(mediaId))); + when(contentsRepository.findByMediaIdAndStatusAndMedia_PublicStatus(mediaId, Status.ACTIVE, PublicStatus.PUBLIC)) + .thenReturn(Optional.of(createContents(999L, mediaId))); @@ - private static Contents createContents(Long mediaId) { + private static Contents createContents(Long contentsId, Long mediaId) { return Contents.builder() - .id(mediaId) + .id(contentsId) .media(Media.builder() .id(mediaId)Also applies to: 58-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/playback/service/PlaybackServiceTest.java` around lines 40 - 41, The test fixture currently sets Contents.id and Media.id to the same value which can mask regressions; update the fixture used by PlaybackServiceTest (the createContents helper) to assign distinct identifiers for Contents.id and its embedded Media.id so the service logic must use the correct id; change the createContents call(s) used in the when(...) stubbing at the referenced test cases (and the similar block around lines 58-63) to return a Contents instance where Contents.id != Media.id, ensuring assertions validate the service uses the proper identifier.apps/api-user/src/test/java/com/ott/api_user/shortform/service/ShortFormFeedServiceTest.java (1)
64-67: 컬렉션 변수명에List접미사를 맞추는 편이 좋습니다.
recommendedMediaIds,recommendationIds는 리스트 타입이므로recommendedMediaIdList,recommendationIdList처럼 타입 의도가 드러나게 맞추면 가독성이 좋아집니다.As per coding guidelines, Enforce: Collection variable names with
Listsuffix.Also applies to: 119-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/shortform/service/ShortFormFeedServiceTest.java` around lines 64 - 67, Rename the collection variables to include the List suffix to match coding guidelines: change recommendedMediaIds to recommendedMediaIdList and recommendationIds to recommendationIdList in ShortFormFeedServiceTest (update the declaration where List.of(...) is assigned and all usages in the test methods), and update any imports or static references if necessary so compilation and tests continue to pass.apps/api-admin/src/test/java/com/ott/api_admin/click_event/service/BackOfficeClickEventServiceTest.java (1)
45-50: 테스트의 월 경계 시간 의존성으로 인한 간헐적 실패 가능성이 있습니다.테스트와 서비스 코드 모두
LocalDate.now()를 호출하는데, 월 변경 시점(예: 1월 31일 23:59:59)에 테스트 실행 중 두 호출 사이에 날짜가 바뀔 수 있습니다. 테스트는(2026, 1)로 모킹하지만 서비스는(2026, 2)로 쿼리할 수 있어 테스트가 실패합니다.Clock주입 또는 테스트 고정 시간 설정으로 결정적 테스트로 변경하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/click_event/service/BackOfficeClickEventServiceTest.java` around lines 45 - 50, The test and service call LocalDate.now() which can drift across month boundaries causing flaky failures; update BackOfficeClickEventServiceTest to use a deterministic Clock (e.g., Clock.fixed(Instant, ZoneId)) or otherwise set a fixed current date and construct now/last from that clock instead of calling LocalDate.now() directly so both the test and the service use the same fixed time; modify any code paths that call LocalDate.now() (symbols: LocalDate.now(), variables now, thisYear, thisMonth, last, lastYear, lastMonth) to accept or derive from an injected Clock so the test can control time.apps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.java (1)
64-119: 권한(인증/인가) 계약을 검증하는 테스트 케이스를 추가하는 것이 좋습니다.현재 테스트는 기능 경로만 확인하고 있어, 미인증(401)·권한 불일치(403) 시나리오가 보장되지 않습니다. 최소한 각 엔드포인트에 대해 권한 실패 케이스를 1개씩 추가해 보안 계약을 고정해 주세요.
As per coding guidelines, Strict USER/EDITOR/ADMIN authorization checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.java` around lines 64 - 119, Add negative auth tests for each existing happy-path test: for getShortFormFeed_returnsSuccessResponseWithPageInfo, recordShortFormView_callsShortClick, and recordCtaClick_callsCtaClickType add at least one case that performs the same request without a principal (expect 401) and one case with a principal lacking required role (expect 403); ensure the tests call mockMvc.perform on "/short-forms", "/short-forms/events", and "/short-forms/cta" respectively, assert the expected status (401 or 403), and verify that shortFormFeedService.getShortFormFeed or clickEventService.saveClickEvent are NOT invoked (use verifyNoInteractions/verify(..., never())).apps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.java (2)
96-126:hideRefreshCard의 not-found 예외 경로 테스트도 추가해 주세요.소유권 관련 테스트는 잘 작성되어 있지만,
REFRESH_CARD_NOT_FOUND경로가 빠져 있어 예외 계약 회귀를 놓칠 수 있습니다.추가 테스트 예시
+ `@Test` + void hideRefreshCard_throwsWhenCardNotFound() { + Long refreshId = 999L; + when(refreshRepository.findById(refreshId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> moodRefreshService.hideRefreshCard(1L, refreshId)) + .isInstanceOf(BusinessException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.REFRESH_CARD_NOT_FOUND); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.java` around lines 96 - 126, Add a test in MoodRefreshServiceTest covering the not-found path for hideRefreshCard: mock refreshRepository.findById(refreshId) to return Optional.empty() and assert that calling moodRefreshService.hideRefreshCard(memberId, refreshId) throws BusinessException with errorCode REFRESH_CARD_NOT_FOUND (use ErrorCode.REFRESH_CARD_NOT_FOUND), similar to the existing ownership test; reference hideRefreshCard, refreshRepository, BusinessException and ErrorCode to locate the code under test.
130-133: 컬렉션 변수명에List접미사를 맞춰 주세요.예:
histories,tags,targetTags,recommendedMedia→historyList,tagList,targetTagList,recommendedMediaList형태로 통일하면 가이드라인 일관성이 맞습니다.리네이밍 예시
- List<WatchHistory> histories = List.of( + List<WatchHistory> historyList = List.of( createWatchHistory(31L, 201L), createWatchHistory(32L, 202L) ); ... - List<String> tags = ReflectionTestUtils.invokeMethod(moodRefreshService, "extractTagsFromHistories", histories); - assertThat(tags).containsExactlyInAnyOrder("one", "two", "three"); + List<String> tagList = ReflectionTestUtils.invokeMethod(moodRefreshService, "extractTagsFromHistories", historyList); + assertThat(tagList).containsExactlyInAnyOrder("one", "two", "three");As per coding guidelines, "Collection variable names with
Listsuffix".Also applies to: 153-156, 171-173, 206-210, 230-234, 257-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.java` around lines 130 - 133, Rename collection variables in MoodRefreshServiceTest to use the "List" suffix to match coding guidelines: change histories to historyList, tags to tagList, targetTags to targetTagList, recommendedMedia to recommendedMediaList (and any other collection variables noted in the review). Update all usages and assertions that reference these variables (e.g., where createWatchHistory(...) results are assigned and consumed) so the test compiles and behavior is unchanged.apps/api-admin/src/test/java/com/ott/api_admin/upload/support/UploadHelperTest.java (2)
60-63: 예외 타입뿐 아니라 에러 코드까지 검증해 계약을 고정하세요.Line 60~63은
BusinessException타입만 검증하고 있어, 다른 원인으로 던져져도 통과할 수 있습니다.변경 제안
- assertThatThrownBy(() -> - uploadHelper.validateOriginObjectKey(objectKey, "https://wrong-url", ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH)) - .isInstanceOf(BusinessException.class); + assertThatThrownBy(() -> + uploadHelper.validateOriginObjectKey(objectKey, "https://wrong-url", ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH)) + .isInstanceOf(BusinessException.class) + .extracting("errorCode") + .isEqualTo(ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/UploadHelperTest.java` around lines 60 - 63, The test currently only asserts the thrown type from uploadHelper.validateOriginObjectKey(objectKey, "https://wrong-url", ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH);; update the assertion to also verify the BusinessException contains the expected ErrorCode (SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH) so the contract is fixed — catch or assertThatThrownBy should inspect the exception (from uploadHelper.validateOriginObjectKey) and assert the exception's getErrorCode()/getError() matches ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH.
67-74: 테스트 검증값이 너무 약해서 회귀를 놓칠 수 있습니다.현재는
isPositive()만 확인해서 내부 계산이 잘못되어도 통과할 수 있습니다. Line 72~73은 입력(1 byte) 기준 기대 파트 수를 정확히 검증하는 편이 안전합니다.변경 제안
- int partCount = uploadHelper.getMultipartPartCount(1); - assertThat(partCount).isPositive(); + int partCount = uploadHelper.getMultipartPartCount(1); + assertThat(partCount).isEqualTo(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/UploadHelperTest.java` around lines 67 - 74, The test UploadHelperTest#getMultipartPartCount_usesMultipartPlan currently only asserts isPositive() which is weak; change the assertion to verify the exact expected part count for a 1-byte input given the configured multipartDefaultPartSizeBytes (5L * 1024L * 1024L) and multipartMaxParts (2000). Specifically, call uploadHelper.getMultipartPartCount(1) and assertThat(partCount).isEqualTo(1) (since 1 byte with a 5MB part size should yield exactly 1 part); update any related assertion imports if needed.apps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormReaderTest.java (1)
10-10: 주석 처리된 코드 잔여물은 제거하는 것이 좋습니다.Line 10, Line 105의 주석 코드는 현재 테스트 의도와 무관해 가독성을 떨어뜨립니다.
Also applies to: 105-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormReaderTest.java` at line 10, Remove leftover commented-out code in the BackOfficeShortFormReaderTest test class: delete the commented import for MediaStatus and the unrelated commented code at the other location in the file (the remnants around the second commented block). Ensure the test file BackOfficeShortFormReaderTest contains only active, relevant test code and imports to improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormReaderTest.java`:
- Around line 77-79: The test BackOfficeShortFormReaderTest currently creates
authorities as Role.MEMBER and calls reader.getShortFormUploadInfo(shortFormId,
objectKey, auth), which masks real back-office role checks; update the positive
success scenario to use a back-office role with sufficient privileges (e.g.,
Role.EDITOR or Role.ADMIN) by changing the authorities/auth setup accordingly,
and move or add a separate test that asserts MEMBER is denied (throws or returns
forbidden) to cover the negative path; ensure you reference the same variables
(authorities, auth) and the method getShortFormUploadInfo so the tests validate
proper role-based access rather than allowing MEMBER to pass.
In `@apps/api-user/build.gradle`:
- Around line 44-48: The test task configuration currently sets ignoreFailures =
true in the test { ... } block which allows failing tests to still succeed
during builds (e.g., when running gradle :apps:api-user:bootJar); revert this to
the safe default by removing the ignoreFailures override or setting
ignoreFailures = false in the test task so unit test failures fail the build and
prevent producing a bootJar on broken tests.
In
`@apps/api-user/src/main/java/com/ott/api_user/moodrefresh/service/MoodRefreshService.java`:
- Line 182: In MoodRefreshService.java update the log message emitted via
log.info to insert a space between sentences so it reads "...간주합니다. 미디어 IDs: {}"
instead of "...간주합니다.미디어 IDs: {}", locate the call to log.info(...) that
references mediaIds in the MoodRefreshService class and modify the string
literal accordingly (keep the same parameter mediaIds).
In
`@apps/api-user/src/test/java/com/ott/api_user/event/MoodRefreshEventListenerTest.java`:
- Around line 31-39: The test
handleWatchHistoryCreated_swallowsServiceExceptions currently only checks that
no exception propagates and may pass if the listener never calls the service;
update the test to explicitly verify that
moodRefreshService.analyzeAndCreateRefreshCard(memberId) was invoked when
calling listener.handleWatchHistoryCreated(new
WatchHistoryCreatedEvent(memberId)). Keep the doThrow setup, then after invoking
listener.handleWatchHistoryCreated add a Mockito verification (e.g.,
verify(moodRefreshService).analyzeAndCreateRefreshCard(memberId)) to assert the
service call occurred.
In
`@apps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.java`:
- Around line 171-173: The test in MoodRefreshServiceTest fixes tag order
causing flakiness: change the assertion on the List<String> tags (result of
invoking extractTagsFromHistories) to an order-independent assertion (e.g.,
replace containsExactly(...) with AssertJ's containsExactlyInAnyOrder(...) or
containsAll(...) so the test only asserts membership not order) to make the test
stable.
In
`@apps/api-user/src/test/java/com/ott/api_user/playlist/controller/PlaylistControllerTest.java`:
- Around line 96-107: Replace all plain Java assert statements in
PlaylistControllerTest with AssertJ/JUnit assertions so tests run without JVM
-ea; specifically update the assertions around the captured PlaylistCondition
and Pageable values: use
assertThat(conditionCaptor.getValue().getContentSource()).isEqualTo(ContentSource.RECOMMEND),
assertThat(conditionCaptor.getValue().getExcludeMediaId()).isEqualTo(123L),
assertThat(pageableCaptor.getValue().getPageNumber()).isEqualTo(2) and
assertThat(pageableCaptor.getValue().getPageSize()).isEqualTo(4). Also remove
the duplicate captured variable assertions (captured) or convert them to
assertThat equivalents to avoid redundant checks. Ensure necessary static
imports for AssertJ (assertThat) are present.
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.java`:
- Around line 49-58: The custom HandlerMethodArgumentResolver in the test
currently intercepts all Long parameters and lacks null checks; update
supportsParameter to only return true when
parameter.getParameterType().equals(Long.class) AND
parameter.getParameterName().equals("memberId"), and update resolveArgument to
defensively handle a null Principal by checking webRequest.getUserPrincipal()
before calling getName() (return null or throw a clear exception as appropriate
for the test). Target the anonymous HandlerMethodArgumentResolver implementation
and the methods supportsParameter and resolveArgument for these changes.
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/service/ShortFormFeedServiceTest.java`:
- Around line 110-132: The test getShortFormFeed_deduplicatesSameMediaIds
currently asserts two results with the same media ID which conflicts with its
deduplication intent; update the assertions (or the test name) so they match:
call shortFormFeedService.getShortFormFeed(memberId, 0, 5) and assert
response.getDataList() has size 1 and that the single element's getShortFormId()
equals recommend.getMedia().getId(); alternatively, if you intended to allow
duplicates, rename the test from getShortFormFeed_deduplicatesSameMediaIds to
reflect that behavior so the method under test
(shortFormFeedService.getShortFormFeed) and the mock setup (recommend and
latestWithSame) are consistent with the assertion.
---
Nitpick comments:
In
`@apps/api-admin/src/test/java/com/ott/api_admin/click_event/service/BackOfficeClickEventServiceTest.java`:
- Around line 45-50: The test and service call LocalDate.now() which can drift
across month boundaries causing flaky failures; update
BackOfficeClickEventServiceTest to use a deterministic Clock (e.g.,
Clock.fixed(Instant, ZoneId)) or otherwise set a fixed current date and
construct now/last from that clock instead of calling LocalDate.now() directly
so both the test and the service use the same fixed time; modify any code paths
that call LocalDate.now() (symbols: LocalDate.now(), variables now, thisYear,
thisMonth, last, lastYear, lastMonth) to accept or derive from an injected Clock
so the test can control time.
In
`@apps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormReaderTest.java`:
- Line 10: Remove leftover commented-out code in the
BackOfficeShortFormReaderTest test class: delete the commented import for
MediaStatus and the unrelated commented code at the other location in the file
(the remnants around the second commented block). Ensure the test file
BackOfficeShortFormReaderTest contains only active, relevant test code and
imports to improve readability.
In
`@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.java`:
- Around line 43-88: Add a test that verifies mediaTagLinker.linkTags throws
when categoryRepository.findByIdAndStatus returns Optional.empty: create a Media
(Media.builder().id(1L).build()), stub categoryRepository.findByIdAndStatus(10L,
Status.ACTIVE) to return Optional.empty(), call mediaTagLinker.linkTags(media,
10L, List.of(1L)) and assert that a BusinessException is thrown and (if
applicable) its errorCode equals the expected code (e.g.,
ErrorCode.CATEGORY_NOT_FOUND); reference mediaTagLinker.linkTags and
categoryRepository.findByIdAndStatus when locating where to add the test.
- Around line 19-20: Remove the unused import by deleting the line importing
java.util.Set in MediaTagLinkerTest; update the imports block (or run automatic
organize/imports) so only required imports like java.util.List remain, ensuring
MediaTagLinkerTest no longer contains the unused Set import.
- Around line 84-87: Rename the local collection variable in MediaTagLinkerTest
from saved to savedList to follow the "List" suffix guideline; update the
assignment (savedList = captor.getValue()), all subsequent uses and assertions
(assertThat(savedList).hasSize(1);
assertThat(savedList.get(0).getTag()).isSameAs(tag);
assertThat(savedList.get(0).getMedia()).isSameAs(media)) to reference savedList,
and ensure any imports or static references remain correct.
- Around line 82-84: Replace the raw ArgumentCaptor.forClass usage with a typed
`@Captor` field to avoid unchecked warnings: add a field `@Captor private
ArgumentCaptor<List<MediaTag>> mediaTagCaptor;` to MediaTagLinkerTest and then
use `verify(mediaTagRepository).saveAll(mediaTagCaptor.capture());
List<MediaTag> saved = mediaTagCaptor.getValue();` so the capture targets the
saveAll call in mediaTagRepository with full generic type safety.
In
`@apps/api-admin/src/test/java/com/ott/api_admin/upload/support/UploadHelperTest.java`:
- Around line 60-63: The test currently only asserts the thrown type from
uploadHelper.validateOriginObjectKey(objectKey, "https://wrong-url",
ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH);; update the assertion to also
verify the BusinessException contains the expected ErrorCode
(SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH) so the contract is fixed — catch or
assertThatThrownBy should inspect the exception (from
uploadHelper.validateOriginObjectKey) and assert the exception's
getErrorCode()/getError() matches
ErrorCode.SHORTFORM_ORIGIN_OBJECT_KEY_MISMATCH.
- Around line 67-74: The test
UploadHelperTest#getMultipartPartCount_usesMultipartPlan currently only asserts
isPositive() which is weak; change the assertion to verify the exact expected
part count for a 1-byte input given the configured multipartDefaultPartSizeBytes
(5L * 1024L * 1024L) and multipartMaxParts (2000). Specifically, call
uploadHelper.getMultipartPartCount(1) and assertThat(partCount).isEqualTo(1)
(since 1 byte with a 5MB part size should yield exactly 1 part); update any
related assertion imports if needed.
In
`@apps/api-user/src/main/java/com/ott/api_user/playlist/service/PlaylistPreferenceService.java`:
- Around line 84-85: The local variable name tags should be renamed to tagList
to follow the project's naming convention; update the declaration using new
ArrayList<>(tagRepository.findAllById(topTagIds)) to assign to tagList and
update the subsequent sort call (tags.sort(...)) to
tagList.sort(Comparator.comparing(tag -> topTagIds.indexOf(tag.getId()))), and
ensure any other references in PlaylistPreferenceService (e.g., later uses of
tags) are updated to tagList.
- Around line 77-80: The local collection variable allTags in
PlaylistPreferenceService is correctly wrapped with new
ArrayList<>(tagRepository.findAll()) to avoid UnsupportedOperationException, but
it violates the naming guideline; rename the variable to allTagList (update its
declaration and all usages in the method) so the list variable follows the
"List" suffix convention while leaving the logic using tagRepository and
Collections.shuffle intact.
In
`@apps/api-user/src/test/java/com/ott/api_user/history/service/WatchHistoryServiceTest.java`:
- Around line 59-62: The test currently only checks the event type; update the
assertion in WatchHistoryServiceTest to capture and cast the published event
(using eventCaptor and
verify(eventPublisher).publishEvent(eventCaptor.capture())), then assert the
event is a WatchHistoryCreatedEvent and verify its key fields (e.g.,
getMemberId()/memberId and getContentsId()/contentsId) match the expected values
from the test setup (use assertEquals or AssertJ assertions against the expected
memberId and contentsId); this ensures publishEvent emitted the correct payload,
not just the correct type.
- Around line 76-77: Replace the unnecessary int narrowing on mediaId in the
test: change the builder call currently using id(100L + mediaId.intValue()) to
use the Long directly (e.g., id(100L + mediaId)) so you avoid pointless
intValue() conversion and potential overflow; update the expression in
WatchHistoryServiceTest where id(...) is set for the Media builder.
In
`@apps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.java`:
- Around line 96-126: Add a test in MoodRefreshServiceTest covering the
not-found path for hideRefreshCard: mock refreshRepository.findById(refreshId)
to return Optional.empty() and assert that calling
moodRefreshService.hideRefreshCard(memberId, refreshId) throws BusinessException
with errorCode REFRESH_CARD_NOT_FOUND (use ErrorCode.REFRESH_CARD_NOT_FOUND),
similar to the existing ownership test; reference hideRefreshCard,
refreshRepository, BusinessException and ErrorCode to locate the code under
test.
- Around line 130-133: Rename collection variables in MoodRefreshServiceTest to
use the "List" suffix to match coding guidelines: change histories to
historyList, tags to tagList, targetTags to targetTagList, recommendedMedia to
recommendedMediaList (and any other collection variables noted in the review).
Update all usages and assertions that reference these variables (e.g., where
createWatchHistory(...) results are assigned and consumed) so the test compiles
and behavior is unchanged.
In
`@apps/api-user/src/test/java/com/ott/api_user/playback/service/PlaybackServiceTest.java`:
- Around line 53-56: The test in PlaybackServiceTest uses the fragile
hasFieldOrPropertyWithValue("errorCode", ...) assertion; change it to a
type-safe check by extracting the thrown BusinessException and asserting its
getErrorCode() equals ErrorCode.CONTENTS_NOT_FOUND — for example, replace the
hasFieldOrPropertyWithValue call after assertThatThrownBy(() ->
playbackService.upsertPlayback(1L, 5L, 10)) with a satisfies block that casts
the exception to BusinessException and asserts ((BusinessException)
e).getErrorCode() == ErrorCode.CONTENTS_NOT_FOUND; this uses the
BusinessException#getErrorCode method and ErrorCode enum directly instead of
relying on a string field name.
- Around line 40-41: The test fixture currently sets Contents.id and Media.id to
the same value which can mask regressions; update the fixture used by
PlaybackServiceTest (the createContents helper) to assign distinct identifiers
for Contents.id and its embedded Media.id so the service logic must use the
correct id; change the createContents call(s) used in the when(...) stubbing at
the referenced test cases (and the similar block around lines 58-63) to return a
Contents instance where Contents.id != Media.id, ensuring assertions validate
the service uses the proper identifier.
In
`@apps/api-user/src/test/java/com/ott/api_user/series/service/SeriesServiceTest.java`:
- Around line 130-141: The test uses inconsistent series IDs: it sets seriesId =
400L but creates a Contents with createSeries(seriesId + 1, 10L) (401L), which
is confusing; update the test so the created series uses the same ID as the
mocked query (use createSeries(seriesId, 10L)) so that the mocked repository
call and the Contents passed to PageImpl align, ensuring getFirstEpisodeMediaId
(invoked on seriesService via ReflectionTestUtils) is validated against a
consistent series ID.
- Around line 117-128: The test uses ReflectionTestUtils.invokeMethod to call
the private method getFirstEpisodeMediaId which couples the test to
implementation details; change the test to exercise the equivalent public
behavior (e.g., call getSeriesContents or another public method on seriesService
that triggers the same EPISODE_NOT_REGISTERED path) and assert the
BusinessException with ErrorCode.EPISODE_NOT_REGISTERED, or if that public path
is not available, refactor getFirstEpisodeMediaId to package-private (or extract
a small helper class) so it can be tested without reflection and update the test
to call the new accessible method instead of ReflectionTestUtils.invokeMethod.
- Around line 164-188: The parameter name mediaId in the helper method
createContents causes confusion because it is used for both Contents.id and
Media.id; rename the parameter to a clearer name (e.g., id or entityId) in the
createContents signature and update all internal usages
(Contents.builder().id(...) and Media.builder().id(...)) and any test callers to
use the new name, or alternatively add a short comment above createContents
explaining that the same id is intentionally used for both Contents and Media;
ensure method name and references (createContents, Contents.builder,
Media.builder) remain consistent.
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.java`:
- Around line 64-119: Add negative auth tests for each existing happy-path test:
for getShortFormFeed_returnsSuccessResponseWithPageInfo,
recordShortFormView_callsShortClick, and recordCtaClick_callsCtaClickType add at
least one case that performs the same request without a principal (expect 401)
and one case with a principal lacking required role (expect 403); ensure the
tests call mockMvc.perform on "/short-forms", "/short-forms/events", and
"/short-forms/cta" respectively, assert the expected status (401 or 403), and
verify that shortFormFeedService.getShortFormFeed or
clickEventService.saveClickEvent are NOT invoked (use
verifyNoInteractions/verify(..., never())).
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/service/ClickEventServiceTest.java`:
- Around line 67-84: The tests saveClickEvent_throwsWhenMemberMissing and
saveClickEvent_throwsWhenShortFormMissing currently only assert the
BusinessException and ERROR codes; also verify that the
clickRepository.save(...) interaction does not occur to prevent side-effect
regressions. In ClickEventServiceTest, after the assertThatThrownBy in both
methods, add a Mockito verifyZeroInteractions or verify(clickRepository,
never()).save(...) (or equivalent) against the mocked clickRepository to ensure
save is not called when member or short form is missing.
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/service/ShortFormFeedServiceTest.java`:
- Around line 64-67: Rename the collection variables to include the List suffix
to match coding guidelines: change recommendedMediaIds to recommendedMediaIdList
and recommendationIds to recommendationIdList in ShortFormFeedServiceTest
(update the declaration where List.of(...) is assigned and all usages in the
test methods), and update any imports or static references if necessary so
compilation and tests continue to pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: da0b42e3-efb3-42bd-a1e4-e36920286150
📒 Files selected for processing (25)
apps/api-admin/build.gradleapps/api-admin/src/test/java/com/ott/api_admin/ApiAdminApplicationTests.javaapps/api-admin/src/test/java/com/ott/api_admin/click_event/service/BackOfficeClickEventServiceTest.javaapps/api-admin/src/test/java/com/ott/api_admin/content/service/BackOfficeContentsServiceTest.javaapps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormReaderTest.javaapps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormServiceTest.javaapps/api-admin/src/test/java/com/ott/api_admin/upload/support/MediaTagLinkerTest.javaapps/api-admin/src/test/java/com/ott/api_admin/upload/support/UploadHelperTest.javaapps/api-user/build.gradleapps/api-user/src/main/java/com/ott/api_user/moodrefresh/service/MoodRefreshService.javaapps/api-user/src/main/java/com/ott/api_user/playlist/service/PlaylistPreferenceService.javaapps/api-user/src/main/java/com/ott/api_user/shortform/service/ClickEventService.javaapps/api-user/src/test/java/com/ott/api_user/ApiUserApplicationTests.javaapps/api-user/src/test/java/com/ott/api_user/event/MoodRefreshEventListenerTest.javaapps/api-user/src/test/java/com/ott/api_user/history/service/WatchHistoryServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/playback/service/PlaybackServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/playlist/controller/PlaylistControllerTest.javaapps/api-user/src/test/java/com/ott/api_user/playlist/service/PlaylistPreferenceServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/playlist/service/PlaylistServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/playlist/service/PlaylistStrategyServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/series/service/SeriesServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.javaapps/api-user/src/test/java/com/ott/api_user/shortform/service/ClickEventServiceTest.javaapps/api-user/src/test/java/com/ott/api_user/shortform/service/ShortFormFeedServiceTest.java
Comment on lines
+77
to
+79
| List<GrantedAuthority> authorities = List.of((GrantedAuthority) () -> Role.MEMBER.getKey()); | ||
| var auth = new UsernamePasswordAuthenticationToken(55L, null, authorities); | ||
| int totalParts = reader.getShortFormUploadInfo(shortFormId, objectKey, auth); |
There was a problem hiding this comment.
관리자 모듈 성공 시나리오에 MEMBER 권한을 쓰면 권한 우회가 테스트로 고정됩니다.
apps/api-admin 경로의 성공 케이스라면 최소 EDITOR/ADMIN 권한으로 검증해야 합니다. Line 77~79는 MEMBER로도 정상 동작을 기대하게 만들어 보안 정책을 약화시킵니다. MEMBER 거부(예외) 케이스를 별도 테스트로 분리해 주세요.
변경 제안
- List<GrantedAuthority> authorities = List.of((GrantedAuthority) () -> Role.MEMBER.getKey());
+ List<GrantedAuthority> authorities = List.of((GrantedAuthority) () -> Role.ADMIN.getKey());
var auth = new UsernamePasswordAuthenticationToken(55L, null, authorities);As per coding guidelines apps/api-admin/**: Check back-office authorization and role constraints. Flag missing route protection or accidental permitAll as P0. Ensure EDITOR restrictions are not bypassable.
📝 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
| List<GrantedAuthority> authorities = List.of((GrantedAuthority) () -> Role.MEMBER.getKey()); | |
| var auth = new UsernamePasswordAuthenticationToken(55L, null, authorities); | |
| int totalParts = reader.getShortFormUploadInfo(shortFormId, objectKey, auth); | |
| List<GrantedAuthority> authorities = List.of((GrantedAuthority) () -> Role.ADMIN.getKey()); | |
| var auth = new UsernamePasswordAuthenticationToken(55L, null, authorities); | |
| int totalParts = reader.getShortFormUploadInfo(shortFormId, objectKey, auth); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-admin/src/test/java/com/ott/api_admin/shortform/service/BackOfficeShortFormReaderTest.java`
around lines 77 - 79, The test BackOfficeShortFormReaderTest currently creates
authorities as Role.MEMBER and calls reader.getShortFormUploadInfo(shortFormId,
objectKey, auth), which masks real back-office role checks; update the positive
success scenario to use a back-office role with sufficient privileges (e.g.,
Role.EDITOR or Role.ADMIN) by changing the authorities/auth setup accordingly,
and move or add a separate test that asserts MEMBER is denied (throws or returns
forbidden) to cover the negative path; ensure you reference the same variables
(authorities, auth) and the method getShortFormUploadInfo so the tests validate
proper role-based access rather than allowing MEMBER to pass.
Comment on lines
+44
to
+48
| test { | ||
| useJUnitPlatform() | ||
| ignoreFailures = true | ||
| finalizedBy(jacocoTestReport) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 검증 목적:
# 1) test task에서 실패 무시 설정 존재 여부 확인
# 2) CI workflow가 어떤 Gradle task(test/check/build)를 호출하는지 확인
rg -n --type=gradle '^\s*ignoreFailures\s*=\s*true'
fd -e yml -e yaml .github/workflows | xargs -r rg -n '(\./gradlew|gradle).*(test|check|build)'Repository: OpenTheTaste/backend
Length of output: 561
🏁 Script executed:
#!/bin/bash
# 1) CI workflow 파일 찾기 (올바른 방법)
echo "=== CI Workflows ==="
fd . '.github/workflows' -e yml -e yaml 2>/dev/null || echo "No .github/workflows found"
# 2) apps/api-user/build.gradle 전체 내용 확인
echo -e "\n=== apps/api-user/build.gradle (full) ==="
wc -l apps/api-user/build.gradle
# 3) test task 주변 맥락 확인
echo -e "\n=== test task context ==="
sed -n '40,60p' apps/api-user/build.gradle | cat -n
# 4) check task 또는 다른 품질 게이트 확인
echo -e "\n=== check/verify tasks in api-user ==="
rg -n 'task\s+(check|verify|test)' apps/api-user/build.gradleRepository: OpenTheTaste/backend
Length of output: 1043
🏁 Script executed:
#!/bin/bash
# 5) 전체 Gradle 플러그인 및 구성 확인
echo "=== Gradle plugins in api-user ==="
head -30 apps/api-user/build.gradle | cat -nRepository: OpenTheTaste/backend
Length of output: 1455
🏁 Script executed:
#!/bin/bash
# 6) CI workflow 파일 검색 (gitlab-ci, workflow 등 다양한 위치)
echo "=== All workflow/CI config files ==="
find . -maxdepth 3 \( -name "*.yml" -o -name "*.yaml" -o -name ".gitlab-ci.yml" \) 2>/dev/null | head -20Repository: OpenTheTaste/backend
Length of output: 531
🏁 Script executed:
#!/bin/bash
# 1) 주요 배포 워크플로우 파일 확인
echo "=== deploy-ec2-docker.yml (주요 배포 워크플로우) ==="
cat .github/workflows/deploy-ec2-docker.ymlRepository: OpenTheTaste/backend
Length of output: 12089
🏁 Script executed:
#!/bin/bash
# 2) qodana.yaml 확인 (코드 품질 관련)
echo "=== qodana.yaml ==="
cat qodana.yamlRepository: OpenTheTaste/backend
Length of output: 2026
🏁 Script executed:
#!/bin/bash
# 3) 최상위 build.gradle 또는 settings.gradle 확인
echo "=== Root build.gradle or settings.gradle ==="
if [ -f build.gradle ]; then
echo "=== build.gradle exists ==="
wc -l build.gradle
head -50 build.gradle | cat -n
else
echo "No root build.gradle"
fi
if [ -f settings.gradle ]; then
echo -e "\n=== settings.gradle ==="
cat settings.gradle
fiRepository: OpenTheTaste/backend
Length of output: 1765
🏁 Script executed:
#!/bin/bash
# 4) apps/api-admin/build.gradle에서 동일 설정 확인
echo "=== api-admin test task context ==="
sed -n '28,40p' apps/api-admin/build.gradle | cat -nRepository: OpenTheTaste/backend
Length of output: 367
🏁 Script executed:
#!/bin/bash
# 1) Dockerfile 확인
echo "=== Dockerfiles ==="
find . -name "Dockerfile" -type f | head -10Repository: OpenTheTaste/backend
Length of output: 193
🏁 Script executed:
#!/bin/bash
# 2) api-user Dockerfile 확인
echo "=== apps/api-user/Dockerfile ==="
if [ -f apps/api-user/Dockerfile ]; then
cat apps/api-user/Dockerfile
else
echo "Not found"
fiRepository: OpenTheTaste/backend
Length of output: 466
🏁 Script executed:
#!/bin/bash
# 3) 다른 workflow 파일들 확인 (test, check 관련)
echo "=== Checking other workflow files for test/check tasks ==="
ls -la .github/workflows/
rg -l 'gradlew|gradle' .github/workflows/ 2>/dev/nullRepository: OpenTheTaste/backend
Length of output: 657
bootJar 빌드 시 테스트 실패가 무시되어 배포 품질이 저하됩니다
Line 46의 ignoreFailures = true 설정으로 인해, Dockerfile에서 gradle :apps:api-user:bootJar를 실행할 때 테스트가 실패해도 빌드가 성공 처리됩니다. 단위 테스트는 회귀 방지의 기본 안전장치이므로 기본값(false)으로 유지해야 합니다.
🔧 제안 수정안
test {
useJUnitPlatform()
- ignoreFailures = true
finalizedBy(jacocoTestReport)
}📝 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
| test { | |
| useJUnitPlatform() | |
| ignoreFailures = true | |
| finalizedBy(jacocoTestReport) | |
| } | |
| test { | |
| useJUnitPlatform() | |
| finalizedBy(jacocoTestReport) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api-user/build.gradle` around lines 44 - 48, The test task configuration
currently sets ignoreFailures = true in the test { ... } block which allows
failing tests to still succeed during builds (e.g., when running gradle
:apps:api-user:bootJar); revert this to the safe default by removing the
ignoreFailures override or setting ignoreFailures = false in the test task so
unit test failures fail the build and prevent producing a bootJar on broken
tests.
| // 태그가 누락된 영상이 하나라도 있다면 환기 불가로 간주 | ||
| if (primaryTags.size() < mediaIds.size()) { | ||
| log.info("[Mood Refresh]: 해당 영상에는 태그가 없어 누락으로 간주합니다."); | ||
| log.info("🚨 [Mood Refresh]: 해당 영상에는 태그가 없어 누락으로 간주합니다.미디어 IDs: {}", mediaIds); |
There was a problem hiding this comment.
로그 메시지 문장 공백 누락을 수정해 주세요.
"간주합니다.미디어"는 로그 검색/가독성을 떨어뜨립니다. 문장 사이 공백을 넣어 주세요.
수정 예시
- log.info("🚨 [Mood Refresh]: 해당 영상에는 태그가 없어 누락으로 간주합니다.미디어 IDs: {}", mediaIds);
+ log.info("🚨 [Mood Refresh]: 해당 영상에는 태그가 없어 누락으로 간주합니다. 미디어 IDs: {}", mediaIds);📝 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
| log.info("🚨 [Mood Refresh]: 해당 영상에는 태그가 없어 누락으로 간주합니다.미디어 IDs: {}", mediaIds); | |
| log.info("🚨 [Mood Refresh]: 해당 영상에는 태그가 없어 누락으로 간주합니다. 미디어 IDs: {}", mediaIds); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-user/src/main/java/com/ott/api_user/moodrefresh/service/MoodRefreshService.java`
at line 182, In MoodRefreshService.java update the log message emitted via
log.info to insert a space between sentences so it reads "...간주합니다. 미디어 IDs: {}"
instead of "...간주합니다.미디어 IDs: {}", locate the call to log.info(...) that
references mediaIds in the MoodRefreshService class and modify the string
literal accordingly (keep the same parameter mediaIds).
Comment on lines
+31
to
+39
| @Test | ||
| void handleWatchHistoryCreated_swallowsServiceExceptions() { | ||
| Long memberId = 22L; | ||
| doThrow(new RuntimeException("boom")).when(moodRefreshService).analyzeAndCreateRefreshCard(memberId); | ||
|
|
||
| listener.handleWatchHistoryCreated(new WatchHistoryCreatedEvent(memberId)); | ||
|
|
||
| // 예외가 외부로 전파되지 않음을 검증하기 위해 별도 검증은 없음 | ||
| } |
There was a problem hiding this comment.
예외 경로 테스트에 호출 단언이 없어 거짓 양성이 가능합니다.
현재는 예외 전파 여부만 간접 확인하고 있어, 리스너가 서비스를 호출하지 않아도 테스트가 통과할 수 있습니다. Line [38] 상태를 유지하지 말고 호출 검증을 추가해 주세요.
제안 수정안
`@Test`
void handleWatchHistoryCreated_swallowsServiceExceptions() {
Long memberId = 22L;
doThrow(new RuntimeException("boom")).when(moodRefreshService).analyzeAndCreateRefreshCard(memberId);
listener.handleWatchHistoryCreated(new WatchHistoryCreatedEvent(memberId));
+ verify(moodRefreshService).analyzeAndCreateRefreshCard(memberId);
- // 예외가 외부로 전파되지 않음을 검증하기 위해 별도 검증은 없음
+ // 예외 전파 없음 + 서비스 호출 시도 여부 검증
}📝 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
| @Test | |
| void handleWatchHistoryCreated_swallowsServiceExceptions() { | |
| Long memberId = 22L; | |
| doThrow(new RuntimeException("boom")).when(moodRefreshService).analyzeAndCreateRefreshCard(memberId); | |
| listener.handleWatchHistoryCreated(new WatchHistoryCreatedEvent(memberId)); | |
| // 예외가 외부로 전파되지 않음을 검증하기 위해 별도 검증은 없음 | |
| } | |
| `@Test` | |
| void handleWatchHistoryCreated_swallowsServiceExceptions() { | |
| Long memberId = 22L; | |
| doThrow(new RuntimeException("boom")).when(moodRefreshService).analyzeAndCreateRefreshCard(memberId); | |
| listener.handleWatchHistoryCreated(new WatchHistoryCreatedEvent(memberId)); | |
| verify(moodRefreshService).analyzeAndCreateRefreshCard(memberId); | |
| // 예외 전파 없음 + 서비스 호출 시도 여부 검증 | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-user/src/test/java/com/ott/api_user/event/MoodRefreshEventListenerTest.java`
around lines 31 - 39, The test
handleWatchHistoryCreated_swallowsServiceExceptions currently only checks that
no exception propagates and may pass if the listener never calls the service;
update the test to explicitly verify that
moodRefreshService.analyzeAndCreateRefreshCard(memberId) was invoked when
calling listener.handleWatchHistoryCreated(new
WatchHistoryCreatedEvent(memberId)). Keep the doThrow setup, then after invoking
listener.handleWatchHistoryCreated add a Mockito verification (e.g.,
verify(moodRefreshService).analyzeAndCreateRefreshCard(memberId)) to assert the
service call occurred.
Comment on lines
+171
to
+173
| List<String> tags = ReflectionTestUtils.invokeMethod(moodRefreshService, "extractTagsFromHistories", histories); | ||
| assertThat(tags).containsExactly("one", "two", "three"); | ||
| } |
There was a problem hiding this comment.
태그 순서 고정 단정은 flaky 테스트를 만들 수 있습니다.
현재 단정은 순서를 강제(containsExactly)하고 있어, 내부 수집 순서가 달라지면 간헐 실패할 수 있습니다. 순서 비의존 단정으로 바꾸는 편이 안전합니다.
수정 예시
- assertThat(tags).containsExactly("one", "two", "three");
+ assertThat(tags).containsExactlyInAnyOrder("one", "two", "three");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-user/src/test/java/com/ott/api_user/moodrefresh/service/MoodRefreshServiceTest.java`
around lines 171 - 173, The test in MoodRefreshServiceTest fixes tag order
causing flakiness: change the assertion on the List<String> tags (result of
invoking extractTagsFromHistories) to an order-independent assertion (e.g.,
replace containsExactly(...) with AssertJ's containsExactlyInAnyOrder(...) or
containsAll(...) so the test only asserts membership not order) to make the test
stable.
Comment on lines
+96
to
+107
| ArgumentCaptor<PlaylistCondition> conditionCaptor = ArgumentCaptor.forClass(PlaylistCondition.class); | ||
| ArgumentCaptor<Pageable> pageableCaptor = ArgumentCaptor.forClass(Pageable.class); | ||
| verify(playlistStrategyService).getPlaylists(conditionCaptor.capture(), pageableCaptor.capture()); | ||
| PlaylistCondition capturedCondition = conditionCaptor.getValue(); | ||
| assert capturedCondition.getContentSource() == ContentSource.RECOMMEND; | ||
| assert capturedCondition.getExcludeMediaId().equals(123L); | ||
| Pageable capturedPageable = pageableCaptor.getValue(); | ||
| assert capturedPageable.getPageNumber() == 2; | ||
| assert capturedPageable.getPageSize() == 4; | ||
| PlaylistCondition captured = conditionCaptor.getValue(); | ||
| assert captured.getContentSource() == ContentSource.RECOMMEND; | ||
| assert captured.getExcludeMediaId().equals(123L); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 목적: 테스트 코드에서 Java assert 사용 위치 확인
# 기대 결과: 단위 테스트 파일에서 assert 문이 0건(또는 AssertJ/JUnit으로 대체)
rg -nP --type=java '^\s*assert\s+' -g '**/*Test.java'Repository: OpenTheTaste/backend
Length of output: 1332
Java assert 대신 AssertJ/JUnit 어썰션을 사용하세요.
Java assert는 -ea JVM 옵션 없이는 실행되지 않아 테스트가 검증되지 않은 채로 통과할 수 있습니다. 현재 코드의 모든 assert 문(라인 100-101, 103-104, 106-107, 142-143)을 AssertJ의 assertThat()으로 교체하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-user/src/test/java/com/ott/api_user/playlist/controller/PlaylistControllerTest.java`
around lines 96 - 107, Replace all plain Java assert statements in
PlaylistControllerTest with AssertJ/JUnit assertions so tests run without JVM
-ea; specifically update the assertions around the captured PlaylistCondition
and Pageable values: use
assertThat(conditionCaptor.getValue().getContentSource()).isEqualTo(ContentSource.RECOMMEND),
assertThat(conditionCaptor.getValue().getExcludeMediaId()).isEqualTo(123L),
assertThat(pageableCaptor.getValue().getPageNumber()).isEqualTo(2) and
assertThat(pageableCaptor.getValue().getPageSize()).isEqualTo(4). Also remove
the duplicate captured variable assertions (captured) or convert them to
assertThat equivalents to avoid redundant checks. Ensure necessary static
imports for AssertJ (assertThat) are present.
Comment on lines
+49
to
+58
| .setCustomArgumentResolvers( | ||
| new HandlerMethodArgumentResolver() { // memberId(Principal) 자동 해석기 | ||
| @Override | ||
| public boolean supportsParameter(MethodParameter parameter) { | ||
| return parameter.getParameterType().equals(Long.class); | ||
| } | ||
| @Override | ||
| public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) { | ||
| return Long.valueOf(webRequest.getUserPrincipal().getName()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
controller_file="$(fd 'ShortFormController.java' | head -n 1)"
echo "[1] 대상 컨트롤러: ${controller_file}"
sed -n '1,260p' "${controller_file}"
echo
echo "[2] Long/long 파라미터 사용 위치 확인"
rg -nP --type=java '\b(Long|long)\s+\w+' "${controller_file}"
echo
echo "[3] 권한 어노테이션 존재 여부 확인"
rg -n --type=java '@PreAuthorize|@Secured|@RolesAllowed' "${controller_file}"Repository: OpenTheTaste/backend
Length of output: 6795
🏁 Script executed:
find apps/api-user -name "ShortFormController.java" -type fRepository: OpenTheTaste/backend
Length of output: 154
🏁 Script executed:
find apps/api-user -name "ShortFormControllerTest.java" -type fRepository: OpenTheTaste/backend
Length of output: 158
🏁 Script executed:
cat apps/api-user/src/main/java/com/ott/api_user/shortform/controller/ShortFormController.javaRepository: OpenTheTaste/backend
Length of output: 2601
🏁 Script executed:
cat apps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.javaRepository: OpenTheTaste/backend
Length of output: 5377
커스텀 인자 해석기가 모든 Long 타입을 무분별하게 가로채고 있으며, null 방어가 필요합니다.
현재 supportsParameter 메서드가 Long.class 타입 전체를 처리하므로, 컨트롤러에 다른 Long 파라미터가 추가될 경우 테스트가 실제 바인딩 오류를 가릴 수 있습니다. 파라미터명을 memberId로 명시적으로 제한하고, getUserPrincipal() null 방어를 추가해야 합니다.
수정 예시
.setCustomArgumentResolvers(
new HandlerMethodArgumentResolver() {
`@Override`
public boolean supportsParameter(MethodParameter parameter) {
- return parameter.getParameterType().equals(Long.class);
+ return parameter.getParameterType().equals(Long.class)
+ && "memberId".equals(parameter.getParameterName());
}
`@Override`
public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {
- return Long.valueOf(webRequest.getUserPrincipal().getName());
+ var principal = webRequest.getUserPrincipal();
+ if (principal == null) {
+ throw new IllegalStateException("Principal is required");
+ }
+ return Long.valueOf(principal.getName());
}
}
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/controller/ShortFormControllerTest.java`
around lines 49 - 58, The custom HandlerMethodArgumentResolver in the test
currently intercepts all Long parameters and lacks null checks; update
supportsParameter to only return true when
parameter.getParameterType().equals(Long.class) AND
parameter.getParameterName().equals("memberId"), and update resolveArgument to
defensively handle a null Principal by checking webRequest.getUserPrincipal()
before calling getName() (return null or throw a clear exception as appropriate
for the test). Target the anonymous HandlerMethodArgumentResolver implementation
and the methods supportsParameter and resolveArgument for these changes.
Comment on lines
+110
to
+132
| void getShortFormFeed_deduplicatesSameMediaIds() { | ||
| Long memberId = 6L; | ||
| when(playlistPreferenceService.getTotalTagScores(memberId)).thenReturn(Map.of(2L, 5)); | ||
|
|
||
| ShortForm recommend = createShortForm(201L, 2001L, "중복추천", true); | ||
| when(shortFormRepository.findRecommendedShortForms(eq(Map.of(2L, 5)), eq(3), eq(0L))) | ||
| .thenReturn(List.of(recommend)); | ||
|
|
||
| ShortForm latestWithSame = createShortForm(201L, 2001L, "중복최신", false); | ||
| List<Long> recommendationIds = List.of(recommend.getMedia().getId()); | ||
| when(shortFormRepository.findLatestShortForms(eq(2), eq(0L), eq(recommendationIds))) | ||
| .thenReturn(List.of(latestWithSame)); | ||
|
|
||
| ArgumentCaptor<List<Long>> likedCaptor = ArgumentCaptor.forClass(List.class); | ||
| ArgumentCaptor<List<Long>> bookmarkedCaptor = ArgumentCaptor.forClass(List.class); | ||
|
|
||
| when(likesRepository.findLikedMediaIds(eq(memberId), any())).thenReturn(Set.of()); | ||
| when(bookmarkRepository.findBookmarkedMediaIds(eq(memberId), any())).thenReturn(Set.of()); | ||
|
|
||
| PageResponse<ShortFormFeedResponse> response = shortFormFeedService.getShortFormFeed(memberId, 0, 5); | ||
| assertThat(response.getDataList()).hasSize(2); | ||
| assertThat(response.getDataList()).allMatch(dto -> dto.getShortFormId().equals(recommend.getMedia().getId())); | ||
|
|
There was a problem hiding this comment.
중복 제거 테스트 의도와 검증식이 서로 충돌합니다.
Line 110의 테스트명은 dedup 검증인데, Line 130-131은 중복이 남은 결과(2개, 동일 ID)를 기대합니다. 현재 상태로는 중복 제거 회귀를 놓칠 수 있습니다.
수정 예시
- assertThat(response.getDataList()).hasSize(2);
- assertThat(response.getDataList()).allMatch(dto -> dto.getShortFormId().equals(recommend.getMedia().getId()));
+ assertThat(response.getDataList()).hasSize(1);
+ assertThat(response.getDataList())
+ .extracting(ShortFormFeedResponse::getShortFormId)
+ .containsExactly(recommend.getMedia().getId());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/api-user/src/test/java/com/ott/api_user/shortform/service/ShortFormFeedServiceTest.java`
around lines 110 - 132, The test getShortFormFeed_deduplicatesSameMediaIds
currently asserts two results with the same media ID which conflicts with its
deduplication intent; update the assertions (or the test name) so they match:
call shortFormFeedService.getShortFormFeed(memberId, 0, 5) and assert
response.getDataList() has size 1 and that the single element's getShortFormId()
equals recommend.getMedia().getId(); alternatively, if you intended to allow
duplicates, rename the test from getShortFormFeed_deduplicatesSameMediaIds to
reflect that behavior so the method under test
(shortFormFeedService.getShortFormFeed) and the mock setup (recommend and
latestWithSame) are consistent with the assertion.
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
2 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.
📝 작업 내용
api-user및api-admin모듈의 주요 서비스와 컨트롤러에 대한 단위 테스트를 구축테스트 커버리지 측정을 위한 JaCoCo 설정 완료
api-user:
MoodRefresh,Playlist,ShortForm,WatchHistory등 핵심 서비스의 단위 테스트를 Mockito를 활용해 작성. (커버리지 90% 이상 확보)api-admin:
UploadHelper,MediaTagLinker,ClickEvent등 관리자 핵심 기능에 대한 검증 완료.api-user: 39 tests completed, 0 failed
api-admin: 13 tests completed, 0 failed
일단은 DB·인프라 없이 단위로 돌릴 수 있는 코드들에 대해 순수하게 목(Mock) 데이터로만 단위 테스트
📷 스크린샷
☑️ 체크 리스트
#️⃣ 연관된 이슈
💬 리뷰 요구사항
Summary by CodeRabbit
릴리스 노트
테스트 인프라 개선
버그 수정
로깅 개선