feat: 플레이리스트 구현#262
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough플레이리스트와 구독 도메인에 대한 REST API, DTO, 엔티티, QueryDSL 저장소, 서비스가 신규 추가되었습니다. 공용 커서 응답의 정렬 방향 정규화, MapStruct 공용 설정, 사용자 요약 DTO/매퍼가 추가되었고, Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
Changes이번 변경은 기능을 크게 두 갈래로 나눠 볼 수 있습니다. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/codeit/team5/mopl/global/dto/CursorResponse.java`:
- Around line 13-27: The normalization in CursorResponse is only applied in the
overridden sortDirection() accessor, which leaves the record’s stored field
unchanged and can make equals(), hashCode(), and toString() inconsistent. Move
the normalization into CursorResponse’s compact constructor so the private final
sortDirection value is always canonicalized when the record is created. Keep the
accessor as the default if possible, and ensure all inputs like
"ASC"/"ASCENDING" and "DESC"/"DESCENDING" are converted before assignment.
In `@src/main/java/com/codeit/team5/mopl/global/mapper/GlobalMapperConfig.java`:
- Around line 9-12: The global MapStruct config currently suppresses unmapped
target warnings via GlobalMapperConfig, which can hide newly added DTO/entity
fields that are not mapped. Update the unmappedTargetPolicy in
GlobalMapperConfig from IGNORE to WARN so missing mappings surface in build
logs, while keeping the existing componentModel,
nullValuePropertyMappingStrategy, and injectionStrategy settings unchanged.
In `@src/main/java/com/codeit/team5/mopl/playlist/constant/PlaylistSortBy.java`:
- Around line 28-35: `PlaylistSortBy.parse` is discarding the original parsing
failure, so update the exception flow to preserve the cause. When
`parser.apply(cursor)` throws, pass `ex` into `PlaylistSortByMismatchException`
(or add a matching constructor there if needed) so the original stack trace from
failures like `NumberFormatException` or `DateTimeParseException` is chained and
visible.
In
`@src/main/java/com/codeit/team5/mopl/playlist/controller/api/PlaylistControllerApi.java`:
- Around line 29-33: `PlaylistControllerApi.create` is missing request
validation, so the `PlaylistCreateRequest` constraints on `title` and
`description` will not run. Add `@Valid` to the `create` method’s request body
parameter in `PlaylistControllerApi` (or the matching implementation method if
you prefer one place only); no change is needed for `update` since it has no
required validation here.
In
`@src/main/java/com/codeit/team5/mopl/playlist/controller/PlaylistController.java`:
- Around line 50-64: The create endpoint in PlaylistController currently accepts
PlaylistCreateRequest without triggering bean validation, so add `@Valid` to the
`@RequestBody` parameter in create to enforce the existing `@NotEmpty` constraints
before service.create is called. Also review update in PlaylistController: if
PlaylistUpdateRequest is meant to be validated as well, add `@Valid` there only
after defining the intended field-level constraints on PlaylistUpdateRequest.
In
`@src/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCursorRequest.java`:
- Line 13: `PlaylistCursorRequest`의 `limit`은 `@Positive`만으로는 상한이 없어 과도한 조회를
허용합니다. `limit` 선언부에 상한 검증을 추가해 최대 허용값을 명시하고, 필요하면 관련 DTO 검증 규칙이 일관되게 적용되는지
확인하세요. 특히 `PlaylistCursorRequest`의 `limit` 필드와 이를 사용하는 커서 조회 흐름에서 큰 값이 들어오지 않도록
제한을 걸어주세요.
In `@src/main/java/com/codeit/team5/mopl/playlist/entity/Playlist.java`:
- Around line 38-46: The validation in Playlist currently uses Assert.hasText in
the private constructor, which throws IllegalArgumentException and is being
handled as a server error. Update the Playlist creation validation to use the
existing BusinessException pattern or add explicit 400 handling for
IllegalArgumentException in GlobalExceptionHandler, and make sure the
owner/title/description creation path in Playlist is still enforced through the
same validation point.
- Around line 57-70: The update flow in PlaylistController.update() and
PlaylistUpdateRequest currently lets blank title/description values be silently
ignored by Playlist.updateIfChanged(), which hides invalid input. Either enforce
validation at the API boundary with `@Valid` and `@NotBlank` on
PlaylistUpdateRequest so empty strings fail fast, or explicitly separate null
from blank handling if partial updates are intended. Update the
controller/request contract and keep Playlist.updateTitle(),
updateDescription(), and updateIfChanged() aligned with that chosen behavior.
In `@src/main/java/com/codeit/team5/mopl/playlist/entity/PlaylistItem.java`:
- Around line 27-34: The nullability is only enforced by Bean Validation on
PlaylistItem fields, so the database schema may still allow nulls. Update the
PlaylistItem mapping by adding nullable = false to both the `@Column` on
playlistId and the `@JoinColumn` on content so the NOT NULL constraint is enforced
at the schema level as well.
In
`@src/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistNotFoundException.java`:
- Around line 10-13: `PlaylistNotFoundException` is using a 400 status while the
other “not found” exceptions in this PR use 404; update the constructor in
`PlaylistNotFoundException` to align with the same not-found semantics as
`SubscriptionNotFoundException` and `SubscriptionPlaylistNotFoundException` by
returning `HttpStatus.NOT_FOUND` instead of `HttpStatus.BAD_REQUEST`, keeping
the existing message and `Map.of("playlistId", id)` payload unchanged.
In
`@src/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistSortByMismatchException.java`:
- Around line 9-13: Add a constructor overload to
PlaylistSortByMismatchException that accepts a Throwable cause and forwards it
to BusinessException, so PlaylistSortBy.parse can preserve the original parsing
exception. Keep the existing constructor intact, and update the new overload to
call the appropriate super(...) signature while still passing the same
BAD_REQUEST status, message, and metadata (sortBy/cursor).
In
`@src/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistUserNotFoundException.java`:
- Around line 9-12: `PlaylistUserNotFoundException`의 상태 코드가
`SubscriptionUserNotFoundException`과 같은 “유저를 찾을 수 없음” 상황과 불일치하므로,
`PlaylistUserNotFoundException(String email)` 생성자에서 `HttpStatus.BAD_REQUEST`를
`HttpStatus.NOT_FOUND`로 맞춰 playlist/subscription 예외 컨벤션을 통일하세요. 이 변경은
`PlaylistUserNotFoundException`와 함께 `PlaylistNotFoundException`,
`SubscriptionUserNotFoundException`의 설계를 기준으로 확인하면 됩니다.
In
`@src/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.java`:
- Around line 115-124: The cursor handling in cursorQuery currently returns null
when either cursor or idAfter is missing, which silently drops the pagination
filter. Add request validation on PlaylistCursorCommand so cursor and idAfter
are enforced as an all-or-none pair before
PlaylistQueryRepositoryImpl.cursorQuery is used, and ensure the validation is
triggered via `@Valid` or equivalent at the controller/service boundary.
- Around line 93-102: `fetchPlaylistItemsWithContents`에서 `playlistItem ->
content -> contentTags -> tag`의 연속 fetch join 때문에 SQL row가 태그 수만큼 곱해질 수 있으니, 이
체인을 끊어 카티션 곱을 줄이도록 수정하세요. `PlaylistQueryRepositoryImpl`의 `findByCursor` 경로에 맞춰
`contentTags`(및 `tag`)를 별도 배치 조회로 분리하거나 `@BatchSize` 기반 지연 로딩으로 전환하고,
`distinct()`에만 의존하지 않도록 정리하세요.
- Around line 39-41: The SORT_PATH_MAP field in PlaylistQueryRepositoryImpl has
a naming/convention mismatch because it is an instance final field using
constant-style uppercase. Either make the map static final if it does not depend
on instance state and keep the current constant naming, or keep it as an
instance field and rename it to camelCase such as sortPathMap; update any
references in PlaylistQueryRepositoryImpl accordingly.
- Around line 126-138: subscriberQuery is currently filtering by
playlist.owner.id, so it duplicates ownerQuery and returns owned playlists
instead of subscribed ones. Update subscriberQuery in
PlaylistQueryRepositoryImpl to filter through Subscription(playlist, subscriber)
using an exists subquery (or equivalent) keyed by subscriberIdEqual, and keep
ownerQuery only for ownerIdEqual. Make sure findByCursor and countByCommand use
the corrected subscriber predicate so “subscribed playlists” are selected
without introducing duplicate rows.
In `@src/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.java`:
- Around line 40-46: `PlaylistService.create` is creating a `Playlist` via
`Playlist.of(...)` but never persisting it before mapping to `PlaylistResponse`.
Update the `create` method to call `playlistRepository.save(...)` on the new
`Playlist` and then pass the saved entity to `mapper.toDto(...)` so the returned
response includes the generated `id` and the row is actually stored. Use the
existing `PlaylistService.create`, `Playlist.of`, and `mapper.toDto` flow as the
place to apply the fix.
In
`@src/main/java/com/codeit/team5/mopl/subscription/controller/api/SubscriptionControllerApi.java`:
- Around line 12-13: The Swagger tag on SubscriptionControllerApi is mislabeled
as playlist management even though this interface defines subscription APIs.
Update the `@Tag` name on SubscriptionControllerApi to a subscription-specific
label such as 구독 관리 so the generated API docs match the actual domain and are
not grouped with playlist features.
In `@src/main/java/com/codeit/team5/mopl/subscription/entity/Subscription.java`:
- Around line 20-42: The Subscription entity allows duplicate
playlist/subscriber pairs because it only maps the two `@ManyToOne` fields without
a database-level uniqueness rule. Update Subscription’s `@Table` mapping to add a
composite unique constraint on playlist_id and subscriber_id, and keep
Subscription.of() unchanged so duplicates are prevented even under concurrent
calls from SubscriptionService.create().
In
`@src/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionNotFoundException.java`:
- Around line 10-13: Update SubscriptionNotFoundException so its constructor
message matches the actual failure in SubscriptionService.delete(): change the
current “유저의 플레이리스트를 찾을 수 없습니다.” text to a clear subscription-specific message
like “구독 정보를 찾을 수 없습니다.” and keep the existing payload fields (playlistId,
email) unchanged. Make sure this exception stays distinct from
SubscriptionPlaylistNotFoundException so the two error cases are not confused.
In
`@src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java`:
- Around line 26-35: The SubscriptionService.create() flow currently saves a
Subscription without checking whether the same subscriber already follows the
same playlist, so add a pre-save duplicate check using
repository.existsBySubscriberEmailAndPlaylistId before calling repository.save
and reject duplicates with an appropriate exception. Also make sure the
Subscription entity has a unique constraint for the (subscriber, playlist) pair
and handle constraint-violation cases in the create() path to cover concurrent
requests; use the create() method, repository, and Subscription.of() as the main
touchpoints.
In
`@src/test/java/com/codeit/team5/mopl/playlist/service/PlaylistServiceTest.java`:
- Around line 109-123: The find_success test only checks the playlist title and
uses an empty contents list, so it does not verify the new response content
mapping. Add a separate test around PlaylistService.find and PlaylistMapper
behavior that builds a PlaylistContentsDto with at least one Content item, stubs
playlistRepository.findByIdWithContents, and asserts the returned
PlaylistResponse includes the expected contents via response.contents() (or the
equivalent field). Keep the existing title assertion, but make sure the new case
validates that contents are preserved end-to-end.
- Around line 81-95: `create_success` 테스트에 저장 호출 검증이 빠져 있어 서비스가 `save()`를 누락해도
통과할 수 있습니다. `PlaylistServiceTest.create_success`에서 `playlistService.create(...)`
이후 `playlistRepository.save(...)`가 호출됐는지 `verify`로 확인하도록 추가하세요.
`delete_success`, `addContent_success`, `removeContent_success`처럼 상호작용 검증 패턴을 맞춰
`PlaylistService`의 생성 흐름이 실제로 영속화되는지 보장하면 됩니다.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6ca789b3-6629-434f-945f-e38a38670e81
📒 Files selected for processing (45)
.gitignoresrc/main/java/com/codeit/team5/mopl/global/dto/CursorResponse.javasrc/main/java/com/codeit/team5/mopl/global/mapper/GlobalMapperConfig.javasrc/main/java/com/codeit/team5/mopl/playlist/constant/PlaylistSortBy.javasrc/main/java/com/codeit/team5/mopl/playlist/controller/PlaylistController.javasrc/main/java/com/codeit/team5/mopl/playlist/controller/api/PlaylistControllerApi.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/PlaylistContentsDto.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/PlaylistCursorCommand.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCreateRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCursorRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistUpdateRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/response/PlaylistResponse.javasrc/main/java/com/codeit/team5/mopl/playlist/entity/Playlist.javasrc/main/java/com/codeit/team5/mopl/playlist/entity/PlaylistItem.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistAccessDeniedException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistContentNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistIncorrectSortByException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistItemNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistSortByMismatchException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistUserNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/mapper/PlaylistMapper.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistItemRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.javasrc/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.javasrc/main/java/com/codeit/team5/mopl/subscription/controller/SubscriptionController.javasrc/main/java/com/codeit/team5/mopl/subscription/controller/api/SubscriptionControllerApi.javasrc/main/java/com/codeit/team5/mopl/subscription/entity/Subscription.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionPlaylistNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionUserNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/repository/SubscriptionRepository.javasrc/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.javasrc/main/java/com/codeit/team5/mopl/user/dto/response/UserSummary.javasrc/main/java/com/codeit/team5/mopl/user/mapper/UserSummaryMapper.javasrc/main/java/com/codeit/team5/mopl/user/repository/UserRepository.javasrc/main/java/com/codeit/team5/mopl/watcher/command/subscribe/ContentChatSubscribeHandler.javasrc/main/resources/db/migration/V17__remove_playlist_items_col.sqlsrc/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistItemRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistQueryRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/service/PlaylistServiceTest.java
This was
linked to
issues
Jul 3, 2026
Closed
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
--- 본문 --- - 임시로 작성된 플레이리스트 엔티티를 정식으로 구현하고, 객체 생성 방식을 개선했습니다. - `@Setter` 대신 `@Builder`와 `protected` 기본 생성자를 도입하여 객체 생성을 제어하고 불변성을 강화했습니다. - 정적 팩토리 메서드 `of`를 추가하여 플레이리스트 생성 시 소유자, 제목, 설명을 필수로 받고 구독자 수를 0으로 초기화하도록 했습니다. - `@NotNull`, `@Size`와 같은 JPA 유효성 검증 어노테이션 대신 생성자 내에서 `Assert`를 사용하여 객체 생성 시점에 필수 필드 유효성을 검증하도록 변경했습니다. - `title`, `description`, `subscriberCount` 컬럼 정의를 명확히 하고, `subscriberCount`의 기본값 설정을 생성 로직으로 이동했습니다.
--- 본문 --- - 사용자 요약 정보를 담는 UserSummary 레코드 DTO를 추가했습니다. - User 엔티티를 UserSummary DTO로 변환하는 UserSummaryMapper 인터페이스를 MapStruct 기반으로 구현했습니다. - MapStruct의 전역 설정을 정의하는 GlobalMapperConfig 인터페이스를 추가하여 일관된 매핑 전략을 적용했습니다. - 사용자 정보 중 핵심적인 요약 데이터를 효율적으로 제공하기 위함입니다. - 엔티티-DTO 변환 로직의 가독성과 유지보수성을 향상시키기 위해 MapStruct를 도입했습니다.
--- 본문 --- - findByEmail 메서드에 @EntityGraph를 적용하여 User 엔티티 조회 시 profileImage를 즉시 로딩하도록 변경했습니다. - 사용자 정보 조회 시 발생하던 N+1 쿼리 문제를 해결하고 애플리케이션 성능을 개선합니다.
--- 본문 --- - 플레이리스트 도메인에서 발생할 수 있는 비즈니스 예외 처리를 위한 클래스 3종을 추가했습니다. - `PlaylistNotFoundException`: 특정 ID의 플레이리스트를 찾을 수 없을 때 발생합니다. - `PlaylistUserNotFoundException`: 플레이리스트 작업 시 해당 유저를 찾을 수 없을 때 발생합니다. - `PlaylistAccessDeniedException`: 플레이리스트에 대한 접근 권한이 없을 때 발생합니다. - 모든 예외 클래스는 `BusinessException`을 상속받아 일관된 방식으로 예외를 처리하도록 구현했습니다.
--- 본문 --- - `PlaylistService` 클래스를 추가하여 플레이리스트의 핵심 비즈니스 로직을 구현했습니다. - 플레이리스트 생성, 단건 조회, 수정, 삭제 기능을 제공합니다. - 플레이리스트 생성 시 사용자 존재 여부를 확인하고, 수정 및 삭제 시에는 플레이리스트 존재 여부와 소유자 검증을 수행하여 데이터 무결성과 보안을 강화했습니다. - 관련 예외 처리 로직을 포함하여 서비스 안정성을 높였습니다.
--- 본문 --- - `PlaylistResponse` DTO를 추가하여 플레이리스트 정보를 클라이언트에 전달하기 위한 응답 형식을 정의했습니다. - `PlaylistCreateRequest` 및 `PlaylistUpdateRequest` DTO를 추가하여 플레이리스트 생성 및 수정을 위한 요청 형식을 정의했습니다. - `PlaylistMapper` 인터페이스를 추가하여 MapStruct를 이용해 `Playlist` 엔티티를 `PlaylistResponse` DTO로 변환하는 매퍼를 정의했습니다. - 이 변경사항들은 향후 플레이리스트 CRUD 기능 구현의 기반을 마련합니다.
--- 본문 --- - PlaylistRepository 인터페이스를 추가하여 플레이리스트 엔티티의 영속성 계층을 정의했습니다. - JpaRepository를 상속받아 기본적인 CRUD 기능을 제공합니다. - @EntityGraph를 사용하여 Playlist 조회 시 owner 및 owner.profileImage를 즉시 로딩하는 findOwnerById 메서드를 추가했습니다. - 특정 ID와 소유자 이메일로 플레이리스트 존재 여부를 확인하는 existsByIdAndOwnerEmail 메서드를 추가했습니다. - @Modifying과 @query를 사용하여 ID로 플레이리스트를 직접 삭제하는 deleteByIdDirectly 메서드를 추가했습니다.
--- 본문 --- - Playlist 엔티티에 `updateTitle`, `updateDescription` 메서드를 추가하여 필드 업데이트 로직을 캡슐화했습니다. - `updateIfChanged` 헬퍼 메서드를 통해 새로운 값이 유효하고 기존 값과 다를 경우에만 업데이트가 수행되도록 하여 불필요한 변경을 방지합니다. - 엔티티의 불변성을 강화하고, 필드 변경에 대한 제어권을 높여 데이터 무결성을 확보합니다. - 기존 `lombok.Setter` 사용을 지양하고, 엔티티의 비즈니스 로직을 엔티티 내부에 응집시켰습니다. - 엔티티에 직접 적용되었던 `@NotNull`, `@Size` 등의 유효성 검사 어노테이션을 제거했습니다.
--- 본문 ---
- PlaylistController를 추가하여 플레이리스트 관련 REST API 엔드포인트를 정의했습니다.
- 플레이리스트 생성, 조회, 수정, 삭제 기능을 제공합니다.
- GET /{id}: 특정 플레이리스트를 ID로 조회합니다.
- POST /: 새로운 플레이리스트를 생성합니다.
- PATCH /{id}: 기존 플레이리스트의 정보를 수정합니다.
- DELETE /{id}: 특정 플레이리스트를 삭제합니다.
- Principal 객체를 활용하여 사용자 인증 정보를 기반으로 작업을 수행합니다.
--- 본문 --- - 플레이리스트 구독 기능 구현을 위한 핵심 엔티티 및 예외 클래스를 추가했습니다. - Subscription 엔티티는 Playlist와 User 간의 구독 관계를 나타내며, 불변(immutable) 객체로 설계되었습니다. - SubscriptionPlaylistNotFoundException과 SubscriptionUserNotFoundException은 구독 대상(플레이리스트 또는 유저)을 찾지 못했을 때 발생하는 비즈니스 예외를 처리합니다.
--- 본문 --- - 플레이리스트 구독 생성 API를 구현했습니다. - 구독 생성 시 플레이리스트와 사용자를 확인하는 로직을 추가했습니다. - 구독 생성에 필요한 Repository, Service, Controller를 구현했습니다.
--- 본문 --- - 플레이리스트 구독 취소 API를 추가했습니다. - 구독 취소 시 해당 구독 정보를 삭제합니다. - 구독 정보가 존재하지 않으면 `SubscriptionNotFoundException`을 발생시킵니다.
--- - 새로운 플레이리스트 목록 조회 API를 추가했습니다. - 기존 오프셋 기반 페이지네이션의 한계를 극복하고 대규모 데이터셋 조회에 효율적인 커서 기반 페이지네이션을 도입합니다. - 정렬 기준(`PlaylistSortBy`) 및 커서 값의 유효성을 검증하는 로직과 관련 예외를 정의하여 API의 안정성을 높였습니다. - 이를 통해 클라이언트는 더 빠르고 일관된 방식으로 플레이리스트 목록을 탐색할 수 있습니다.
--- 본문 --- - 플레이리스트 목록을 효율적으로 조회하기 위한 커서 기반 페이지네이션을 구현했습니다. - QueryDSL을 활용하여 동적으로 정렬 및 커서 조건을 적용하는 `PlaylistQueryRepository`를 추가했습니다. - `PlaylistService`에 `findByCursor` 메서드를 추가하여 커서 조회 로직을 처리하고, `PlaylistMapper`를 통해 `CursorResponse`를 생성하도록 했습니다. - `PlaylistMapper`에 `List<Playlist>`를 `List<PlaylistResponse>`로 변환하는 메서드와 `CursorResponse`를 생성하는 헬퍼 메서드를 추가했습니다.
--- 본문 --- - CursorResponse 레코드의 sortDirection() 메서드를 오버라이드하여 정렬 방향 값을 정규화했습니다. - "ASC", "ASCENDING", "DESC", "DESCENDING" 등 다양한 대소문자 및 약어 형태의 입력에 대해 일관된 "ASCENDING" 또는 "DESCENDING" 값을 반환하도록 하여, 정렬 방향 처리의 견고성과 일관성을 높였습니다. - null 값에 대한 처리 로직도 추가되었습니다.
--- 본문 --- - 플레이리스트에 포함될 콘텐츠를 관리하기 위한 PlaylistItem 엔티티를 추가했습니다. - PlaylistItem은 불변(Immutable) 엔티티로 설계되었으며, 콘텐츠가 플레이리스트에 추가된 시점(added_at)을 기록합니다. - 특정 플레이리스트와 해당 플레이리스트에 속한 모든 콘텐츠 목록을 함께 전달하기 위한 PlaylistContentsDto 레코드를 추가했습니다. - 이는 플레이리스트 상세 조회 시 필요한 데이터를 효율적으로 제공하기 위함입니다.
--- 본문 --- - 플레이리스트 단건 및 목록 조회 시 응답 DTO에 해당 플레이리스트에 포함된 콘텐츠 목록을 추가했습니다. - 이를 위해 `PlaylistContentsDto`를 도입하여 플레이리스트 엔티티와 콘텐츠 목록을 함께 묶어 처리하도록 변경했습니다. - `PlaylistMapper`와 `PlaylistService`를 업데이트하여 새로운 DTO를 사용하고, `PlaylistQueryRepository`에서 콘텐츠 정보를 함께 조회하도록 쿼리를 수정했습니다. - `PlaylistResponse`에 `List<ContentResponse> contents` 필드를 추가했습니다.
--- 본문 --- - IDE(예: VS Code) 관련 설정 파일 및 빌드 결과물(bin/)이 Git에 의해 추적되지 않도록 .gitignore에 항목을 추가했습니다. - 이를 통해 개발 환경 간 불필요한 충돌을 방지하고 저장소를 깔끔하게 유지할 수 있습니다.
--- 본문 --- - `PlaylistQueryRepositoryImpl`에서 플레이리스트 아이템 및 연관 콘텐츠 조회 로직을 `fetchPlaylistItemsWithContents` 헬퍼 메서드로 통합하고 최적화했습니다. - 콘텐츠 썸네일, 통계, 태그 정보를 `fetchJoin`을 사용하여 한 번의 쿼리로 함께 로드하도록 개선하여 N+1 쿼리 문제를 방지하고 성능을 향상시켰습니다. - `Subscription` 엔티티의 `playlist` 및 `subscriber` 필드에 `@NotNull` 어노테이션을 추가하여 JPA 표준에 맞는 방식으로 필수 연관 관계를 명시했습니다. 기존 `@JoinColumn(nullable = false)`를 대체합니다. - 사용되지 않는 임포트 문을 제거하고 임포트 순서를 정리하여 코드 가독성을 높였습니다.
--- 본문 --- - 플레이리스트에 콘텐츠를 추가하는 API를 구현했습니다. - 플레이리스트에서 콘텐츠를 삭제하는 API를 구현했습니다. - 각 기능은 플레이리스트 소유자만 접근 가능하도록 권한 검증을 추가했습니다. - 존재하지 않는 콘텐츠나 플레이리스트 아이템에 대한 예외 처리를 추가했습니다.
--- 본문 --- - PlaylistItemRepository 인터페이스를 추가하여 플레이리스트 항목의 존재 여부 확인 및 직접 삭제 기능을 구현했습니다. - PlaylistContentNotFoundException 및 PlaylistItemNotFoundException 클래스를 추가하여 콘텐츠 및 플레이리스트 항목을 찾을 수 없을 때 발생하는 예외 처리를 정의했습니다. - PlaylistControllerApi 인터페이스에 플레이리스트 관련 API 명세를 추가했습니다.
--- 본문 --- - PlaylistController가 PlaylistControllerApi 인터페이스를 구현하도록 변경했습니다. - 이를 통해 API 명세에 따른 컨트롤러 구현을 명확히 하고, 향후 확장성을 고려했습니다. - 또한, PlaylistService에 PlaylistItemNotFoundException을 추가하여 관련 예외 처리를 개선했습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java (1)
41-48: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
delete()도 동일한 exists-then-act 패턴으로 레이스 컨디션 가능성이 있습니다.동시에 삭제 요청이 두 번 들어오면 두 번째 요청이
existsBySubscriberEmailAndPlaylistId통과 후 실제 삭제 시점에는 이미 레코드가 없어deleteBySubscriberEmailAndPlaylistIdDirectly가 아무 것도 지우지 못하고 조용히 성공 처리될 수 있습니다. 삭제 실패가 사용자에게 노출되지 않는 정도라 영향은 제한적이지만,delete쿼리의 영향받은 행 수(affected rows)를 확인해 0이면 예외를 던지는 방식으로 더 견고하게 만들 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java` around lines 41 - 48, `SubscriptionService.delete()` uses an exists-then-act flow that can miss concurrent deletes; make the operation rely on the delete result instead. In `delete(UUID playlistId, String email)`, remove the separate `existsBySubscriberEmailAndPlaylistId` check and have `repository.deleteBySubscriberEmailAndPlaylistIdDirectly(...)` return the affected row count so the service can throw `SubscriptionNotFoundException` when it is 0. This keeps the behavior in `delete()` consistent even under race conditions and uses the existing `SubscriptionNotFoundException`, `repository`, and `deleteBySubscriberEmailAndPlaylistIdDirectly` symbols to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.java`:
- Around line 39-41: The `PlaylistQueryRepositoryImpl` has a readability and
safety issue because the `cursorBuilder` field name collides with the existing
`cursorBuilder(Expression<?> path)` helper method, and the `Map.of()` approach
can miss new `PlaylistSortBy` values at compile time. Prefer option A: replace
the map-based lookup with a `switch`-driven selection like `orderSpecifiers`
that covers all `PlaylistSortBy` cases, then pass the chosen path into
`cursorBuilder(...)` to avoid the field/method name conflict and gain
compile-time exhaustiveness. If you keep the map, rename the field to something
distinct like `cursorConditionBuilders` to remove the ambiguity. Update the
`cursorQuery` usage accordingly so the chosen condition builder remains clear
and safe.
In
`@src/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionAlreadyExistsException.java`:
- Around line 13-16: SubscriptionAlreadyExistsException currently maps a
duplicate-subscription conflict to HttpStatus.BAD_REQUEST, which should be
changed to HttpStatus.CONFLICT. Update the constructor in
SubscriptionAlreadyExistsException so the superclass call uses 409 semantics
while keeping the existing message and Map.of("email", email, "playlistId",
playlistId) payload unchanged.
---
Outside diff comments:
In
`@src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java`:
- Around line 41-48: `SubscriptionService.delete()` uses an exists-then-act flow
that can miss concurrent deletes; make the operation rely on the delete result
instead. In `delete(UUID playlistId, String email)`, remove the separate
`existsBySubscriberEmailAndPlaylistId` check and have
`repository.deleteBySubscriberEmailAndPlaylistIdDirectly(...)` return the
affected row count so the service can throw `SubscriptionNotFoundException` when
it is 0. This keeps the behavior in `delete()` consistent even under race
conditions and uses the existing `SubscriptionNotFoundException`, `repository`,
and `deleteBySubscriberEmailAndPlaylistIdDirectly` symbols to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dd31e344-498f-4385-9cd4-a743b44c9e9b
📒 Files selected for processing (18)
src/main/java/com/codeit/team5/mopl/global/mapper/GlobalMapperConfig.javasrc/main/java/com/codeit/team5/mopl/playlist/controller/PlaylistController.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCursorRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/entity/Playlist.javasrc/main/java/com/codeit/team5/mopl/playlist/entity/PlaylistItem.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistCreataionViolationException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistUserNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.javasrc/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.javasrc/main/java/com/codeit/team5/mopl/subscription/controller/api/SubscriptionControllerApi.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionAlreadyExistsException.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.javasrc/main/java/com/codeit/team5/mopl/user/repository/UserRepository.javasrc/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.javasrc/test/java/com/codeit/team5/mopl/playlist/service/PlaylistServiceTest.java
💤 Files with no reviewable changes (2)
- src/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepository.java
- src/main/java/com/codeit/team5/mopl/playlist/entity/Playlist.java
--- 본문 --- - 플레이리스트 정렬 기준에 따른 CursorConditionBuilder 생성 로직을 Map에서 Switch 문으로 변경했습니다. - 불필요한 contentTag와 tag 조인을 제거하여 쿼리 성능을 개선했습니다.
--- 본문 --- - SubscriptionRepositoryTest: 구독 존재 여부 확인 및 삭제 테스트 추가 - SubscriptionControllerTest: 구독 생성 및 삭제 API 테스트 추가 - SubscriptionServiceTest: 구독 생성 및 삭제 로직 테스트 추가
- 플레이리스트 단건 조회 및 목록 조회 시, 현재 사용자가 해당 플레이리스트를 구독 중인지 여부를 반환하도록 수정했습니다. - 이를 위해 `PlaylistContentsDto`에 `subscribedByMe` 필드를 추가하고, QueryDSL 쿼리에서 구독 여부를 확인하는 로직을 추가했습니다. - `PlaylistQueryRepository` 및 `PlaylistService`의 관련 메서드 시그니처를 변경하여 `userId`를 받도록 수정했습니다. - 컨트롤러에서 `AuthenticationPrincipal`을 통해 현재 사용자 ID를 가져와 서비스로 전달하도록 수정했습니다. - `PlaylistRepository`의 `existsByIdAndOwnerEmail` 메서드를 `existsByIdAndOwnerId`로 변경하여 사용자 ID 기반으로 소유자 여부를 확인하도록 수정했습니다. - `PlaylistAccessDeniedException` 및 `PlaylistUserNotFoundException`의 생성자 파라미터를 `email`에서 `userId`로 변경했습니다.
- 구독 취소 시 해당 플레이리스트의 구독자 수를 감소시키는 로직을 추가했습니다. - 이를 통해 플레이리스트의 구독자 수를 정확하게 관리할 수 있습니다.
--- 본문 --- - 플레이리스트 관련 기능에서 소유자 인증 시 이메일 대신 사용자 ID를 사용하도록 변경했습니다. - 이는 사용자 ID가 더 고유하고 안정적인 식별자이기 때문입니다. - 관련 테스트 코드(Repository, Controller, Service, QueryRepository)를 수정하여 변경 사항을 반영했습니다.
--- 본문 --- - 커서 기반 플레이리스트 조회 시, 여러 페이지에 걸쳐 연속적으로 데이터를 조회하는 시나리오에 대한 테스트 코드를 추가했습니다. - 이를 통해 페이징 로직의 정확성과 안정성을 검증합니다.
- ReflectionTestUtils 대신 직접 createdAt 필드를 설정하는 헬퍼 메소드 사용 - 테스트 코드의 가독성과 유지보수성 향상
--- 본문 --- - QueryDSL 라이브러리 의존성을 `com.querydsl`에서 `io.github.openfeign.querydsl`로 변경했습니다. - `.env` 파일 로딩을 위한 `spring-dotenv` 라이브러리를 추가했습니다. - JaCoCo 테스트 리포트 생성 시 제외할 클래스 패턴을 정의하고, 리포트 자동 열림 기능을 추가했습니다.
--- 본문 --- - 플레이리스트 단건 조회 시 콘텐츠 목록까지 함께 조회하는 경우에 대한 테스트를 추가했습니다. - 이를 통해 플레이리스트 조회 기능의 정확성과 안정성을 높였습니다.
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (2)
src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java (1)
27-40: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift중복 체크는 추가됐지만 TOCTOU 경쟁 조건은 여전히 남아있음
이전 리뷰에서 지적된 중복 구독 체크 누락은
existsBySubscriberEmailAndPlaylistId체크(Line 34)로 반영됐습니다. 다만exists체크와save(Line 34, 38) 사이에 원자성이 보장되지 않아, 동시에 같은 (email, playlistId) 조합으로 요청이 들어오면 두 요청 모두 exists 체크를 통과해 중복 레코드가 생성될 수 있습니다.이 문제를 근본적으로 막으려면
Subscription엔티티에 (subscriber, playlist) 유니크 제약을 추가하고, 저장 시 발생하는 제약 위반 예외(DataIntegrityViolationException)를SubscriptionAlreadyExistsException으로 매핑하는 처리가 함께 필요합니다. 이번 리뷰 배치에는Subscription엔티티 파일이 포함되지 않아 유니크 제약 존재 여부를 확인할 수 없습니다.#!/bin/bash # Subscription 엔티티에 유니크 제약이 존재하는지 확인 fd Subscription.java --exec cat -n {} \; | rg -n -A5 -B5 'UniqueConstraint|`@Table`'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java` around lines 27 - 40, The current duplicate-subscription guard in SubscriptionService.create still has a TOCTOU race because the existsBySubscriberEmailAndPlaylistId check and repository.save are not atomic. Add a database-level unique constraint on the Subscription entity for the subscriber/playlist pair, and update SubscriptionService.create to catch DataIntegrityViolationException around repository.save and convert it to SubscriptionAlreadyExistsException. Keep the existing existence check only as a fast pre-check, but rely on the unique constraint plus exception mapping as the source of truth.src/main/java/com/codeit/team5/mopl/subscription/entity/Subscription.java (1)
17-42: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win구독 중복 방지를 위한 유니크 제약이 여전히 없습니다.
playlist_id와subscriber_id조합에 대한 DB 레벨 유니크 제약이 없어, 동시 요청 시 TOCTOU 문제로 중복 구독 레코드가 쌓일 수 있습니다. 이전 리뷰에서 지적된 사항이 이번 커밋에서도 반영되지 않았습니다.🛡️ 유니크 제약 추가 제안
+import jakarta.persistence.UniqueConstraint; + `@Getter` `@NoArgsConstructor`(access = AccessLevel.PROTECTED) `@Immutable` `@Entity` -@Table(name = "playlist_subscriptions") +@Table(name = "playlist_subscriptions", + uniqueConstraints = `@UniqueConstraint`(columnNames = {"playlist_id", "subscriber_id"})) public class Subscription extends BaseEntity {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/codeit/team5/mopl/subscription/entity/Subscription.java` around lines 17 - 42, Add a DB-level uniqueness guard to Subscription so duplicate playlist/subscriber pairs cannot be inserted under concurrent requests. Update the Subscription entity to define a composite unique constraint on the playlist and subscriber columns in addition to the existing `@Table` mapping, and keep the existing Subscription.of factory unchanged so callers still create subscriptions the same way.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.gradle`:
- Around line 87-133: The JaCoCo classDirectories exclusion setup is duplicated
in multiple afterEvaluate blocks, so refactor the repeated
classDirectories.setFrom(fileTree(... exclude: jacocoExcludePatterns)) logic
into a shared helper or reusable closure in build.gradle. Update the
jacocoTestReport configuration to call that single helper from each place it is
currently repeated, keeping the behavior the same while making the configuration
easier to maintain.
In
`@src/main/java/com/codeit/team5/mopl/playlist/controller/api/PlaylistControllerApi.java`:
- Around line 39-40: The delete API signature in PlaylistControllerApi uses a
path variable name that does not match the implementation in PlaylistController,
which can cause inconsistent API documentation. Update the interface method to
use the same path variable name as the controller’s delete mapping and
`@PathVariable` parameter (the id symbol), so springdoc and the actual route stay
aligned.
In `@src/main/java/com/codeit/team5/mopl/playlist/dto/PlaylistCursorCommand.java`:
- Around line 9-11: The PlaylistCursorCommand record currently uses Object for
cursor, which removes type safety and forces casting in
PlaylistQueryRepositoryImpl. Replace the generic cursor field with a type-safe
design tied to PlaylistSortBy, such as a sealed interface hierarchy or separate
cursor fields per sort type, and update any callers/consumers of
PlaylistCursorCommand to construct and read the new strongly typed cursor value.
In `@src/main/java/com/codeit/team5/mopl/playlist/entity/Playlist.java`:
- Around line 45-52: Playlist.of() currently accepts title and description
as-is, so whitespace-only values can still be persisted even though the request
DTO uses `@NotEmpty`. Update the validation to enforce non-blank text by using
`@NotBlank` on PlaylistCreateRequest and/or keeping equivalent entity-level
validation in Playlist.of or the Playlist fields, and verify the
owner/title/description flow still routes through the Playlist builder with the
stricter constraint.
In
`@src/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistContentNotFoundException.java`:
- Around line 11-15: Update the NotFound-style exception to use a 404 status
instead of 400: in PlaylistContentNotFoundException, change the
BusinessException call to align with PlaylistNotFoundException and
PlaylistUserNotFoundException by using HttpStatus.NOT_FOUND while keeping the
existing message and contentId details. Also check PlaylistItemNotFoundException
in the same playlist/exception package and make the same status-code correction
so all “찾을 수 없음” exceptions are consistent.
In
`@src/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistCreataionViolationException.java`:
- Line 7: The class name contains a typo in PlaylistCreataionViolationException;
rename it to use Creation instead of Creataion and update any references that
use this exception type so they match the corrected symbol name. Check all
usages of the exception class and any imports or throw/catch sites to ensure the
rename is consistent across the codebase.
In
`@src/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistItemRepository.java`:
- Around line 13-15: Bulk delete method deleteByPlaylistIdAndContentIdDirectly
can leave stale PlaylistItem entities in the persistence context because
`@Modifying` does not clear it automatically. Update the `@Modifying` annotation on
PlaylistItemRepository.deleteByPlaylistIdAndContentIdDirectly to clear the
persistence context after execution so subsequent reads in the same transaction
don’t return outdated entities.
In
`@src/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepository.java`:
- Around line 18-24: The bulk update methods in PlaylistRepository directly
change the database but leave the persistence context stale, so update the
`@Modifying` queries on increaseSubscribeCount and decreaseSubscribeCount to clear
the context automatically. Apply the fix on these repository methods so any
Playlist entity or proxy obtained earlier in the same transaction, such as in
SubscriptionService.create(), will not keep outdated subscriberCount state when
later used for mapping or response building.
In `@src/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.java`:
- Around line 77-86: `PlaylistService.addContent` is missing duplicate-content
protection before saving a `PlaylistItem`. Add a pre-check using
`playlistItemRepository.existsByPlaylistIdAndContentId(playlistId, contentId)`
(or equivalent) after `validateOwner(...)` and before
`contentRepository.getReferenceById(...)`/`save(...)`, and throw
`PlaylistCreataionViolationException` when the pair already exists. Keep the
existing `PlaylistContentNotFoundException` flow for missing content, and ensure
the new guard prevents hitting the database unique constraint on `(playlist_id,
content_id)`.
In
`@src/main/java/com/codeit/team5/mopl/subscription/repository/SubscriptionRepository.java`:
- Around line 13-15: The bulk delete in
SubscriptionRepository.deleteBySubscriberEmailAndPlaylistIdDirectly uses
`@Modifying` but does not clear the persistence context, so managed Subscription
entities can remain stale after the delete. Update the repository method
annotation to enable automatic context cleanup for this direct delete, using the
same method name and `@Query` target so the change is localized and future calls
stay consistent within the transaction.
In
`@src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java`:
- Around line 42-50: In SubscriptionService.delete, the separate
existsBySubscriberEmailAndPlaylistId check and
deleteBySubscriberEmailAndPlaylistIdDirectly call are not atomic, so concurrent
deletes can decrement the subscribe count twice. Update the flow to rely on the
delete result from repository.deleteBySubscriberEmailAndPlaylistIdDirectly
(change it to return the affected row count) and call
playlistRepository.decreaseSubscribeCount only when that count is greater than
zero; keep throwing SubscriptionNotFoundException when no row was deleted.
In `@src/main/java/com/codeit/team5/mopl/user/repository/UserRepository.java`:
- Around line 17-18: The `UserRepository.findByEmail` method is applying
`@EntityGraph` globally, causing unnecessary `profileImage` joins for callers
like `MoplUserDetailsService`, `PasswordResetService`, and `AdminInitializer`
that do not need it. Remove the eager fetch from `findByEmail` and introduce a
separate repository method for the profile-image-including lookup, then update
only the code paths that actually need `profileImage` to use that new method.
In
`@src/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.java`:
- Around line 105-119: The test setup in PlaylistControllerTest leaves
authentication in SecurityContextHolder without cleanup, which can leak state
into other tests. Add an `@AfterEach` cleanup method that clears the
SecurityContextHolder after each test, and keep the existing authentication
setup in setUp() unchanged except for making sure the context is only used per
test. Use the existing setUp() and any new tearDown()/cleanup method in
PlaylistControllerTest to locate the fix.
- Around line 175-204: The validation test in
PlaylistControllerTest.findCursor_fail_validation is missing coverage for the
new `@Max`(50) constraint on PlaylistCursorRequest.limit. Add one or more
parameterized cases with limit values above 50 (for example 51 or 1000) and
assert the same INVALID_INPUT bad request response and limit field error
details, alongside the existing lower-bound and non-numeric checks.
In
`@src/test/java/com/codeit/team5/mopl/playlist/service/PlaylistServiceTest.java`:
- Line 104: Replace the repeated fully qualified references in
PlaylistServiceTest with the already imported symbols to keep the test
consistent and readable. In the test methods that use
userRepository.findWithProfileImageById, PlaylistContentsDto, and Collections,
switch from org.mockito.ArgumentMatchers.any/eq and
com.codeit.team5.mopl.playlist.dto.PlaylistContentsDto/java.util.Collections to
the existing static and regular imports already declared at the top of
PlaylistServiceTest. This should be applied anywhere the same pattern appears in
the test class so the code relies on the imported any, eq, PlaylistContentsDto,
and Collections symbols instead of repeating fully qualified names.
---
Duplicate comments:
In `@src/main/java/com/codeit/team5/mopl/subscription/entity/Subscription.java`:
- Around line 17-42: Add a DB-level uniqueness guard to Subscription so
duplicate playlist/subscriber pairs cannot be inserted under concurrent
requests. Update the Subscription entity to define a composite unique constraint
on the playlist and subscriber columns in addition to the existing `@Table`
mapping, and keep the existing Subscription.of factory unchanged so callers
still create subscriptions the same way.
In
`@src/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.java`:
- Around line 27-40: The current duplicate-subscription guard in
SubscriptionService.create still has a TOCTOU race because the
existsBySubscriberEmailAndPlaylistId check and repository.save are not atomic.
Add a database-level unique constraint on the Subscription entity for the
subscriber/playlist pair, and update SubscriptionService.create to catch
DataIntegrityViolationException around repository.save and convert it to
SubscriptionAlreadyExistsException. Keep the existing existence check only as a
fast pre-check, but rely on the unique constraint plus exception mapping as the
source of truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2db7f64f-17bb-4725-a711-35422fd089c3
📒 Files selected for processing (52)
.gitignorebuild.gradlesrc/main/java/com/codeit/team5/mopl/global/dto/CursorResponse.javasrc/main/java/com/codeit/team5/mopl/global/mapper/GlobalMapperConfig.javasrc/main/java/com/codeit/team5/mopl/playlist/constant/PlaylistSortBy.javasrc/main/java/com/codeit/team5/mopl/playlist/controller/PlaylistController.javasrc/main/java/com/codeit/team5/mopl/playlist/controller/api/PlaylistControllerApi.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/PlaylistContentsDto.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/PlaylistCursorCommand.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCreateRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCursorRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistUpdateRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/dto/response/PlaylistResponse.javasrc/main/java/com/codeit/team5/mopl/playlist/entity/Playlist.javasrc/main/java/com/codeit/team5/mopl/playlist/entity/PlaylistItem.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistAccessDeniedException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistContentNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistCreataionViolationException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistIncorrectSortByException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistItemNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistSortByMismatchException.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistUserNotFoundException.javasrc/main/java/com/codeit/team5/mopl/playlist/mapper/PlaylistMapper.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistItemRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepository.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.javasrc/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.javasrc/main/java/com/codeit/team5/mopl/subscription/controller/SubscriptionController.javasrc/main/java/com/codeit/team5/mopl/subscription/controller/api/SubscriptionControllerApi.javasrc/main/java/com/codeit/team5/mopl/subscription/entity/Subscription.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionAlreadyExistsException.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionPlaylistNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/exception/SubscriptionUserNotFoundException.javasrc/main/java/com/codeit/team5/mopl/subscription/repository/SubscriptionRepository.javasrc/main/java/com/codeit/team5/mopl/subscription/service/SubscriptionService.javasrc/main/java/com/codeit/team5/mopl/user/dto/response/UserSummary.javasrc/main/java/com/codeit/team5/mopl/user/mapper/UserSummaryMapper.javasrc/main/java/com/codeit/team5/mopl/user/repository/UserRepository.javasrc/main/java/com/codeit/team5/mopl/watcher/command/subscribe/ContentChatSubscribeHandler.javasrc/main/resources/db/migration/V17__remove_playlist_items_col.sqlsrc/test/java/com/codeit/team5/mopl/notification/repository/NotificationRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistItemRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistQueryRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/playlist/service/PlaylistServiceTest.javasrc/test/java/com/codeit/team5/mopl/subscription/controller/SubscriptionControllerTest.javasrc/test/java/com/codeit/team5/mopl/subscription/repository/SubscriptionRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/subscription/service/SubscriptionServiceTest.java
--- 본문 --- - 플레이리스트에 컨텐츠를 추가할 때 이미 존재하는 컨텐츠인지 확인하는 로직을 추가했습니다. - 중복된 컨텐츠가 추가될 경우 `PlaylistItemAlreadyExistsException`을 발생시키도록 수정했습니다. - `PlaylistCreataionViolationException` 클래스가 삭제되어 해당 예외를 사용하지 않도록 변경했습니다. - `PlaylistCreateRequest`의 `title`과 `description` 필드에 `@NotBlank` 어노테이션을 적용하여 빈 문자열 입력을 방지하도록 수정했습니다. - Jacoco 테스트 리포트 생성 시 제외 패턴을 적용하도록 `build.gradle` 파일을 수정했습니다.
--- 본문 --- - MockMvc 테스트에서 AuthenticationPrincipalArgumentResolver를 사용하기 위한 설정 추가 - UUID 관련 임포트 정리 및 코드 가독성 개선 - 테스트 코드 내에서 사용되는 UUID 리터럴을 상수로 분리하여 관리 - Mockito의 given, verify, willDoNothing 등의 메서드 호출 시 인자 매칭 방식 통일
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.java (1)
115-378: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win하드코딩된 UUID 문자열을 상수로 추출하는 것을 권장합니다
"11111111-1111-1111-1111-111111111111"리터럴이 파일 전체에 15회 이상 반복됩니다. 커밋 메시지에서는 "UUID 상수화"로 테스트 코드를 정리했다고 언급하지만, 실제 코드에는 상수 필드 없이 동일한 문자열이 계속 하드코딩되어 있습니다. 값이 하나라도 바뀌면 여러 곳을 동시에 수정해야 하고, 오탈자로 인한 테스트 실패 위험도 커집니다.♻️ 개선 제안
+ private static final UUID USER_ID = UUID.fromString("11111111-1111-1111-1111-111111111111"); + private MoplPrincipal principal; private Authentication auth;이후 파일 내
UUID.fromString("11111111-1111-1111-1111-111111111111")호출을USER_ID로 일괄 치환하면 가독성과 유지보수성이 개선됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.java` around lines 115 - 378, The test class repeatedly hardcodes the same user UUID literal instead of using a shared constant. Add a single reusable UUID field in PlaylistControllerTest for the authenticated user ID and replace every UUID.fromString("11111111-1111-1111-1111-111111111111") usage in the setup and test methods with that constant, so the repeated calls are centralized and easier to maintain.
♻️ Duplicate comments (1)
src/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.java (1)
78-90: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win중복 검사 추가는 좋으나,
exists체크와save사이에 경쟁 조건(TOCTOU)이 남아 있습니다.과거 지적된 중복 콘텐츠 방지 로직(84-86번 라인)이 추가된 점은 좋습니다. 다만
existsByPlaylistIdAndContentId체크와 실제save()호출이 원자적이지 않아, 동시에 같은 콘텐츠를 추가하는 두 요청이 들어오면 둘 다 체크를 통과한 뒤 하나는 DB 유니크 제약(playlist_id,content_id) 위반으로DataIntegrityViolationException이 그대로 컨트롤러까지 전파될 수 있습니다. 이 경우 사용자에게는 도메인 예외(PlaylistItemAlreadyExistsException)가 아니라 500 에러 혹은 처리되지 않은 스택 트레이스가 노출될 위험이 있습니다.
save()를 감싸서 무결성 위반 예외를 도메인 예외로 변환하면 근본적으로 안전해집니다.🛡️ 제안: 저장 시점 예외 변환
Content content = contentRepository.getReferenceById(contentId); PlaylistItem playlistItem = PlaylistItem.of(playlistId, content); - playlistItemRepository.save(playlistItem); + try { + playlistItemRepository.save(playlistItem); + } catch (DataIntegrityViolationException e) { + throw new PlaylistItemAlreadyExistsException(playlistId, contentId); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.java` around lines 78 - 90, `PlaylistService.addContent` still has a TOCTOU gap between `existsByPlaylistIdAndContentId` and `playlistItemRepository.save`, so concurrent duplicate inserts can surface as a raw `DataIntegrityViolationException`. Keep the current pre-checks, but wrap the `save(playlistItem)` call in `addContent` and translate any unique-constraint violation into `PlaylistItemAlreadyExistsException` so callers always get the domain-level duplicate error. Use the existing `PlaylistItemAlreadyExistsException`, `playlistItemRepository`, and `addContent` method as the main points to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistItemAlreadyExistsException.java`:
- Around line 14-15: `PlaylistItemAlreadyExistsException` is using
`HttpStatus.BAD_REQUEST` for a duplicate-resource case; update the constructor
call in this exception to use `HttpStatus.CONFLICT` instead. Keep the existing
message and metadata map intact, and make sure the change is applied only in
`PlaylistItemAlreadyExistsException` unless you intentionally want to align
related duplicate-mapping exceptions with the same status convention.
---
Outside diff comments:
In
`@src/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.java`:
- Around line 115-378: The test class repeatedly hardcodes the same user UUID
literal instead of using a shared constant. Add a single reusable UUID field in
PlaylistControllerTest for the authenticated user ID and replace every
UUID.fromString("11111111-1111-1111-1111-111111111111") usage in the setup and
test methods with that constant, so the repeated calls are centralized and
easier to maintain.
---
Duplicate comments:
In `@src/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.java`:
- Around line 78-90: `PlaylistService.addContent` still has a TOCTOU gap between
`existsByPlaylistIdAndContentId` and `playlistItemRepository.save`, so
concurrent duplicate inserts can surface as a raw
`DataIntegrityViolationException`. Keep the current pre-checks, but wrap the
`save(playlistItem)` call in `addContent` and translate any unique-constraint
violation into `PlaylistItemAlreadyExistsException` so callers always get the
domain-level duplicate error. Use the existing
`PlaylistItemAlreadyExistsException`, `playlistItemRepository`, and `addContent`
method as the main points to update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3f2e7489-15b2-4ab8-a02c-e448984fc47b
📒 Files selected for processing (6)
build.gradlesrc/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCreateRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/exception/PlaylistItemAlreadyExistsException.javasrc/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.javasrc/test/java/com/codeit/team5/mopl/playlist/controller/PlaylistControllerTest.javasrc/test/java/com/codeit/team5/mopl/subscription/repository/SubscriptionRepositoryTest.java
dstle
reviewed
Jul 4, 2026
dstle
left a comment
dstle
left a comment
Contributor
There was a problem hiding this comment.
플레이리스트 구현 고생하셨습니다!!
별개로 유저가 삭제되면 플레이리스트 구독 카운트도 같이 차감되어야하는데
관련 로직이 없는 것 같아서 확인 부탁드립니다!!
- `PlaylistCreateRequest`의 `title` 필드에 `@Size(max = 255)` 제약 조건을 추가하여 유효성 검사를 강화했습니다. - `PlaylistQueryRepositoryImpl`에서 `subscribedByMeSubquery`에 `IS_SUBSCRIBED` 별칭을 적용하여 쿼리 가독성을 높이고 중복 로직을 제거했습니다. - `PlaylistService.removeContent` 메서드에서 불필요한 `contentRepository.existsById` 호출을 제거하여 성능을 최적화했습니다. - `PlaylistQueryRepositoryTest`에 `subscribedByMe` 필드 검증 로직을 추가하여 플레이리스트 구독 여부 조회 기능의 정확성을 보장했습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.java`:
- Line 42: `IS_SUBSCRIBED` is a class-level constant-like QueryDSL path and
should not be an instance field. Update `PlaylistQueryRepositoryImpl` so
`IS_SUBSCRIBED` is declared as `static final` alongside the other shared query
symbols, keeping it consistent with the `UPPER_SNAKE_CASE` naming and avoiding
per-instance allocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1b78b600-10b0-4592-97bf-e8f4eac134f3
📒 Files selected for processing (4)
src/main/java/com/codeit/team5/mopl/playlist/dto/request/PlaylistCreateRequest.javasrc/main/java/com/codeit/team5/mopl/playlist/repository/query/PlaylistQueryRepositoryImpl.javasrc/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.javasrc/test/java/com/codeit/team5/mopl/playlist/repository/PlaylistQueryRepositoryTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/codeit/team5/mopl/playlist/service/PlaylistService.java
- 플레이리스트에서 콘텐츠를 삭제할 때, 콘텐츠 자체의 존재 여부를 확인하는 로직을 제거했습니다. - 플레이리스트에 해당 콘텐츠가 존재하는지만 확인하면 충분하므로, 불필요한 `contentRepository.existsById` 호출을 제거하여 로직을 간소화했습니다.
Collaborator
Author
감사합니다. |
--- 본문 --- - `com.codeit.team5.mopl.playlist.entity.event` 패키지를 `com.codeit.team5.mopl.playlist.event`로 변경했습니다. - `com.codeit.team5.mopl.playlist.entity.exception` 패키지를 `com.codeit.team5.mopl.playlist.exception`으로 변경했습니다. - 변경된 패키지 경로에 맞춰 관련 클래스의 import 문을 수정했습니다. - 이는 플레이리스트 관련 엔티티와 이벤트, 예외 클래스들을 좀 더 명확하게 분리하기 위한 구조 변경입니다.
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.
관련 이슈
작업 내용
변경 사항
테스트 내용
체크리스트
리뷰 포인트
스크린샷 / 참고 자료
🔗 관련 이슈
#54#59#231📝 작업 내용
이 PR은 플레이리스트 기능을 본격적으로 도입하기 위한 기반 작업입니다.
기존에는 플레이리스트/구독/플레이리스트-콘텐츠 연결을 처리하는 전용 도메인 구조가 부족했고, 조회·생성·수정·삭제·구독 흐름이 분리되지 않아 책임 경계가 불명확했습니다. 이번 변경으로 플레이리스트를 중심으로 한 API, 서비스, 저장소, 예외 처리, 응답 DTO가 정리되어 전체 흐름이 일관되게 구성되었습니다.
주요 변경사항
title,description빈값 방지를 위한 검증을 추가했습니다.📊 변경 효과 요약 (가능한 경우에만)