feat: 콘텐츠 관리 - 사전 클래스 작성 및 콘텐츠 등록#111
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:
📝 WalkthroughWalkthroughContent 엔티티, Tag, ContentTag, ContentStats, ContentType/ContentSource 열거형을 새로 정의하여 콘텐츠 도메인 기반을 구성합니다. Content는 admin/external 두 팩토리로 생성 규칙을 분리하고, addTag()로 중복 제거 연관을 지원합니다. 예외 계층(ContentException, TagException 및 각 특화 예외), DTO/Mapper, Repository, OpenAPI 계약을 구현한 후, multipart POST 엔드포인트( Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 주요 리뷰 포인트 및 개선 고려사항1. Content 팩토리 메서드: 책임 분리의 명확화현재 설계의 강점:
향후 확장성 관점: 현재는 두 가지 경로가 명확하지만, 만약 파트너별 규칙(예: "넷플릭스 콘텐츠는 특정 타임존 적용", "유튜브 콘텐츠는 메타데이터 형식 다름")이 추가된다면 선형적 증가 위험이 있습니다. 개선 제안:
/**
* ADMIN이 직접 등록하는 콘텐츠용 팩토리.
* - source는 항상 ADMIN으로 설정됨 (변경 불가)
* - externalId는 UUID 기반으로 자동 생성됨
*
* 외부 API(TMDB, 스포츠DB 등)에서 주입된 콘텐츠는
* createByExternalSource()를 사용하세요.
*/
public static Content createByAdmin(ContentType type, String title, String description)향후 요구사항이 증가하면 그때 빌더 패턴 도입을 검토하면 충분합니다. 2. ContentTag 복합 키 생성: 불완전한 상태의 위험현재 코드: public static ContentTag create(Content content, Tag tag) {
ContentTagId id = new ContentTagId(); // ← no-arg 생성
// ... 이후 setter로 필드 채움
contentTag.setId(id);
contentTag.setContent(content);
contentTag.setTag(tag);
return contentTag;
}우려사항:
더 견고한 방식: 이미 public static ContentTag create(Content content, Tag tag) {
ContentTagId id = new ContentTagId(content.getId(), tag.getId());
ContentTag contentTag = new ContentTag();
contentTag.setId(id);
contentTag.setContent(content);
contentTag.setTag(tag);
return contentTag;
}이렇게 하면 CompleteTagId가 생성되는 순간부터 모든 필드가 설정된 상태가 되어, 중간 상태를 걱정할 필요가 없습니다. 3. Content.addTag() 중복 검사: 이원적 판정의 부작용현재 로직: boolean isDuplicate = contentTags.stream().anyMatch(ct -> {
if (ct.getTag().getId() != null) {
return ct.getTag().getId().equals(tag.getId()); // id로 비교
} else {
return ct.getTag().getName().equals(tag.getName()); // name으로 비교
}
});문제점: 잠재적 시나리오:
개선 선택지: 선택지 1: 항상 ID로 비교 (권장) boolean isDuplicate = contentTags.stream()
.anyMatch(ct -> ct.getTag().getId() != null &&
ct.getTag().getId().equals(tag.getId()));
선택지 2: 항상 Name으로 비교 boolean isDuplicate = contentTags.stream()
.anyMatch(ct -> ct.getTag().getName().equalsIgnoreCase(tag.getName()));
선택지 3: 현재 방식 유지 + 명문화 현재 코드는 실제로 안전합니다. 왜냐하면 다만 Javadoc을 명확히 하면 좋습니다: /**
* 태그를 콘텐츠에 추가합니다.
*
* 중복 검사 규칙:
* - Tag.id가 할당되었으면 id로 중복 판정
* - Tag.id가 null이면 name으로 중복 판정
*
* 주의: 기존 콘텐츠의 태그는 반드시 저장된 상태여야 합니다.
* (즉, 항상 id가 할당된 상태에서 호출될 것을 전제)
*/
public void addTag(Tag tag) { ... }권장사항: 4. ContentService.create(): thumbnail 파라미터의 역할 불명현재 상황: public ContentResponse create(ContentCreateRequest request, MultipartFile thumbnail) {
// ... 콘텐츠 및 태그 처리
// TODO: thumbnail 업로드 및 URL 설정
return contentMapper.toDto(content, content.getContentTags(), stats);
}명확히 해야 할 점:
평가: 5. 태그 정규화 로직: 도메인 규칙인가, 입력 처리인가?현재 설계:
List<String> normalizedTags = request.getTags().stream()
.map(String::trim)
.map(String::toLowerCase)
.distinct()
.collect(toList());설계 의도 검토:
문제점:
더 나은 설계: 방식 1: 도메인 규칙으로 격상 (권장) public class Tag {
public static String normalize(String name) {
return name != null ? name.trim().toLowerCase() : "";
}
public static Tag create(String name) {
String normalized = normalize(name);
if (normalized.isBlank() || normalized.length() > 50) {
throw new InvalidTagNameException(...);
}
return new Tag(normalized); // 정규화된 name 저장
}
}
// ContentService에서:
List<String> tagNames = request.getTags().stream()
.map(Tag::normalize)
.distinct()
.collect(toList());방식 2: 유틸 클래스 분리 public class TagNormalizer {
public static String normalize(String name) {
return name != null ? name.trim().toLowerCase() : "";
}
}방식 3: 현재 유지
권장사항: 6. ContentMapper null 안전성: 명문화의 가치현재 구현: default List<String> toTagNames(List<ContentTag> contentTags) {
return contentTags == null ? Collections.emptyList() : ...;
}
default double toAverageRating(ContentStats stats) {
return stats == null ? 0.0 : ...; // ← `@OneToOne`(optional=false)인데 null 체크?
}설계 평가:
개선: 의도를 Javadoc으로 명문화하면 향후 리팩토링이 수월합니다: /**
* 통계에서 평균 평점을 계산합니다.
*
* `@param` stats 콘텐츠 통계 (선택사항, 방어 프로그래밍)
* `@return` 평균 평점. stats 미존재 또는 리뷰 0개이면 0.0
*/
default double toAverageRating(ContentStats stats) {
if (stats == null || stats.getReviewCount() == 0) {
return 0.0;
}
return (double) stats.getRatingSum() / stats.getReviewCount();
}평가: 7. 테스트 커버리지: 충실하나, 엔티티 경계 케이스 보충 가능현재 테스트의 강점:
보충할 가치 있는 경계 케이스:
평가: 종합 의견이 PR의 주요 강점
현 단계에서 개선할 여지
최종 평가이 PR은 매우 건실한 기초를 제공합니다. 설계가 명확하고, 위의 개선 항목들은 모두 선택사항입니다. 긴급한 버그나 구조적 결함은 없으며, 오히려 좋은 학습 기회가 될 수 있는 코드입니다. 코드 리뷰 중에 위의 고려사항들을 팀과 논의하면서 설계 의도를 공유하면, 향후 유지보수 시 같은 결정을 일관되게 적용할 수 있을 것입니다. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 14
🤖 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/content/controller/ContentController.java`:
- Around line 27-36: The postContent method in ContentController is missing
authorization checks despite being documented as an admin-only endpoint. Add the
`@PreAuthorize`("hasRole('ADMIN')") annotation to the postContent method to
enforce admin-only access at the method level. Additionally, update the
SecurityConfig class to add path-based authorization rules that restrict access
to the /api/contents/** endpoints to users with the ADMIN role, ensuring that
unauthorized requests are rejected before reaching the controller. This
dual-layer approach (path-based and method-level) prevents security gaps when
new endpoints are added.
In `@src/main/java/com/codeit/team5/mopl/content/entity/Content.java`:
- Around line 68-79: The externalId field is declared with nullable = false but
the create() factory method assigns null to externalId when the ContentSource is
not ADMIN (for TMDB and SPORTS_DB sources), which violates the database
constraint. To fix this, modify the create() method signature to accept an
externalId parameter and use the provided value for non-ADMIN sources, or
alternatively create a separate factory method such as createAdminContent() that
generates a random UUID for the ADMIN source while requiring externalId to be
passed explicitly for external sources like TMDB and SPORTS_DB. Either approach
ensures all Content instances have a non-null externalId that satisfies both the
database constraint and the uk_contents_source_external_id unique constraint.
- Around line 81-98: The addTag() method's duplicate detection logic only uses
reference equality when Tag IDs are null, which allows Tags with the same name
but different object instances to be added as duplicates. Enhance the duplicate
check in the anyMatch() stream operation to also compare Tag names when IDs are
null, so that two Tag instances with identical names are correctly identified as
duplicates regardless of whether they have been persisted yet. Update the
condition to check name equality (using equals()) alongside the reference
equality check when both Tags have null IDs.
In `@src/main/java/com/codeit/team5/mopl/content/entity/ContentStats.java`:
- Around line 34-35: The `ratingSum` field in the `ContentStats` class is
declared as `double`, which can accumulate floating-point precision errors when
many ratings are aggregated together. Evaluate whether your application's scale
and precision requirements justify changing this field: if you expect large
numbers of ratings to be accumulated, consider using an integer-based
accumulation approach by storing ratings as integers (for example, multiplying
by 10 to preserve one decimal place) and converting back when calculating
averages, or alternatively use `BigDecimal` for arbitrary precision. If the
current use case involves a limited dataset where precision loss is negligible,
you may keep the `double` type, but document this decision. Update the
`ratingSum` field type and adjust any related calculation logic in methods that
update or retrieve this value accordingly.
- Around line 40-41: The updatedAt field in the ContentStats class is not
automatically refreshed when the entity is updated, only when it's first
created. Add a `@PreUpdate` annotated method (a JPA lifecycle callback) to the
ContentStats class that automatically sets updatedAt to Instant.now() whenever
the entity is persisted or updated. This ensures that every time statistics
change, the updatedAt timestamp is refreshed without requiring manual updates in
each method that modifies the statistics.
In `@src/main/java/com/codeit/team5/mopl/content/entity/ContentTag.java`:
- Around line 38-44: The create() method in ContentTag is manually assigning the
id field using new ContentTagId(content.getId(), tag.getId()), which conflicts
with the `@MapsId` annotation pattern that is meant to automatically map the IDs
from the associated content and tag entities. Remove the manual id assignment
line and only set the content and tag fields, allowing Hibernate to
automatically populate the ContentTagId through the `@MapsId` mechanism when the
entity is persisted. This aligns with the standard JPA pattern and ensures
single responsibility for ID management.
In
`@src/main/java/com/codeit/team5/mopl/content/exception/InvalidContentTypeException.java`:
- Around line 7-9: The InvalidContentTypeException constructor receives a value
parameter but does not pass it to the parent constructor, resulting in loss of
the specific invalid value information in error responses. Modify the
constructor to pass the value parameter as a detail message to the parent class
constructor call via super(), using the appropriate parent constructor signature
that accepts both ErrorCode and detailMessage parameters to preserve the invalid
content type value for debugging and user feedback.
In `@src/main/java/com/codeit/team5/mopl/content/mapper/ContentMapper.java`:
- Around line 27-30: The toAverageRating method in ContentMapper has a precision
loss issue where integer division may occur if getRatingSum() returns an integer
type. To fix this, cast at least one operand to double in the division operation
on line 29 where stats.getRatingSum() is divided by stats.getReviewCount(),
ensuring floating-point division is performed instead of integer division. This
will preserve decimal values like 2.5 instead of truncating to 2.0.
In `@src/main/java/com/codeit/team5/mopl/content/service/ContentService.java`:
- Line 39: The method in the ContentService class is accepting a MultipartFile
thumbnail parameter but the actual storage upload logic is missing, with only a
TODO comment present at line 39. Either implement the complete thumbnail upload
functionality by integrating with your storage solution (S3, local filesystem,
etc.) to handle the MultipartFile thumbnail parameter and set the thumbnailUrl
in the ContentResponse, or remove the thumbnail parameter from the method
signature and the TODO comment if this feature is not yet ready for this PR.
Ensure consistency between the API contract and the actual implementation.
- Line 41: The distinct() operation on request.tags().stream() at line 41 does
not normalize tag names, allowing duplicate tags like "Action", "action", and "
Action " to be treated as separate tags. Before calling distinct() in the stream
pipeline, add map operations to normalize the tag names by trimming whitespace
with String::trim and converting to lowercase with String::toLowerCase. This
ensures semantically identical tags are recognized as duplicates regardless of
case or surrounding whitespace, improving data consistency for tag search and
filtering operations.
- Around line 41-45: The tag processing in ContentService uses stream().map()
with individual findByName() calls for each tag, causing an N+1 query problem
where each tag lookup executes a separate SELECT query. To fix this, add a
findByNameIn(List<String> names) method to TagRepository that fetches all tags
matching the given names in a single query using an IN clause, then refactor the
stream processing to first collect all tag names, call findByNameIn once to get
existing tags, filter for missing tags, save them in batch, and finally add all
tags to the content. This reduces the query count from N individual selects plus
M inserts to just one select and one batch insert operation.
In `@src/main/java/com/codeit/team5/mopl/tag/entity/Tag.java`:
- Around line 27-31: Add parameter validation to the create() method in the Tag
class to check the name parameter before creating the Tag object. The validation
should check that name is not null, not empty, and does not exceed 50 characters
in length. If any of these conditions fail, throw an appropriate exception (such
as IllegalArgumentException) with a clear message that describes the validation
requirement. This ensures invalid data is rejected at the domain layer rather
than failing later at the database constraint level, providing clearer error
messages and easier debugging.
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentControllerTest.java`:
- Line 98: Remove the TODO comment at line 98 in the ContentControllerTest class
that mentions the missing thumbnail processing logic. Instead of leaving inline
TODO comments, create a separate issue in your issue tracking system to track
the implementation of the thumbnail processing feature. This approach keeps the
codebase clean and ensures future work items are properly prioritized and
managed in a centralized location rather than scattered as comments throughout
the code.
In `@src/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.java`:
- Line 153: The issue is that `thenReturn(any())` is incorrect Mockito usage
since `any()` is an ArgumentMatcher and cannot be used as a return value. In the
`when(contentMapper.toDto(any(Content.class), any(),
any(ContentStats.class))).thenReturn(any())` statement, replace
`thenReturn(any())` with either `thenReturn(null)` if the return value is not
used by the test, or with `thenReturn()` followed by creating an actual response
object that represents what the mocked method should return. Apply this same fix
to the identical pattern that appears on lines 177 and 201 as well.
🪄 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: f0d50dad-0c54-4c93-a7d0-3c7503c262ce
📒 Files selected for processing (23)
build.gradlesrc/main/java/com/codeit/team5/mopl/content/controller/ContentController.javasrc/main/java/com/codeit/team5/mopl/content/controller/api/ContentApi.javasrc/main/java/com/codeit/team5/mopl/content/dto/request/ContentCreateRequest.javasrc/main/java/com/codeit/team5/mopl/content/dto/response/ContentResponse.javasrc/main/java/com/codeit/team5/mopl/content/entity/Content.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentSource.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentStats.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentTag.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentType.javasrc/main/java/com/codeit/team5/mopl/content/exception/ContentException.javasrc/main/java/com/codeit/team5/mopl/content/exception/ContentNotFoundException.javasrc/main/java/com/codeit/team5/mopl/content/exception/InvalidContentTypeException.javasrc/main/java/com/codeit/team5/mopl/content/mapper/ContentMapper.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentStatsRepository.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/tag/entity/Tag.javasrc/main/java/com/codeit/team5/mopl/tag/repository/TagRepository.javasrc/main/resources/db/migration/V3__modify_watcher_count_type.sqlsrc/main/resources/db/migration/V4__add_admin_content_source.sqlsrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerTest.javasrc/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.java
plzslp
force-pushed
the
feature/5-content-posting
branch
from
ed94d85 to
fe9fcfc
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/main/java/com/codeit/team5/mopl/content/service/ContentService.java (1)
41-41: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift썸네일 파라미터가 계약에 포함됐지만 실제 반영 로직이 없습니다.
현재는 업로드 파일을 받아도 저장/URL 매핑이 없어 응답
thumbnailUrl이 항상 비게 됩니다.
선택지는 두 가지입니다:
- 이번 PR에서 구현 완료: 스펙과 동작 일치(장점), 구현/연동 비용 큼(단점)
- API 계약에서 임시 제외: 릴리즈 리스크 축소(장점), 기능 범위 축소 합의 필요(단점)
🤖 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/content/service/ContentService.java` at line 41, The TODO comment in ContentService indicates that thumbnail storage upload and URL reflection functionality is incomplete, yet the thumbnailUrl parameter is part of the API contract, resulting in empty thumbnail URLs in responses. To fix this, you must either: (1) implement the complete thumbnail storage upload logic by capturing the uploaded file, storing it to the designated storage service, retrieving the generated URL, and mapping it to the thumbnailUrl field in the response object, or (2) remove the thumbnailUrl parameter from the API contract/DTO if deferring this feature to a future release. Coordinate with your team to decide which approach aligns with the sprint goals, then either complete the ContentService implementation or remove the thumbnail-related fields from the API specification and relevant DTOs.
🤖 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/content/dto/request/ContentCreateRequest.java`:
- Around line 18-20: The `tags` field in the ContentCreateRequest DTO only
validates the list itself but not individual tag elements, allowing
whitespace-only tags like [" "] to pass validation. This breaks the API
contract when the service trims and removes empty tags. Add element-level
validation by annotating each String element in the List with `@NotBlank` to
ensure each tag is not null, empty, or just whitespace. This separates
validation responsibility at the DTO layer from normalization logic that should
happen in the service layer, preventing the API contract violation.
In `@src/main/java/com/codeit/team5/mopl/content/entity/Content.java`:
- Around line 78-88: The createByExternalSource() method does not validate that
the source parameter is a valid external source (not ADMIN) and that the
externalId parameter is not null or blank, which violates the database schema
constraints and delays error detection until persistence time. Add guard clauses
at the beginning of createByExternalSource() to validate that source is
appropriate for external sources (excluding ADMIN or other internal sources) and
that externalId is not null and not blank, throwing an appropriate exception
(such as IllegalArgumentException) if either constraint is violated to fail fast
and clearly indicate the problem at the call site.
In `@src/main/java/com/codeit/team5/mopl/content/service/ContentService.java`:
- Around line 43-48: After the tag normalization stream operations that create
the tagNames list (which includes trim, filter empty, lowercase, and distinct
operations), add validation to ensure that tagNames is not empty. If tagNames is
empty after normalization, throw an appropriate exception immediately to prevent
content creation with zero tags. This validation should occur in the service
layer right after the tagNames assignment to enforce the domain rule that
content must have at least one valid tag, even if the input initially appears to
contain tags before normalization.
In
`@src/test/java/com/codeit/team5/mopl/content/controller/ContentControllerTest.java`:
- Around line 192-216: Add a new test method to validate that tags containing
only whitespace strings (e.g., List.of(" ")) are properly rejected. Create a
separate test similar to postContent_emptyTags_returnsBadRequest but with a
ContentCreateRequest containing whitespace-only tags instead of an empty list,
and verify it returns the same 400 bad request response with the appropriate
error message about empty tag lists. This ensures the validation catches both
completely empty tag lists and lists where tags normalize to empty after
whitespace trimming.
In `@src/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.java`:
- Around line 101-133: The test method create_withoutThumbnail_success expects
content creation to succeed with a null thumbnail, but this conflicts with the
PR requirement that thumbnail should be a mandatory input field. Either update
the requirement documentation to explicitly mark thumbnail as optional, or
refactor this test to verify that null thumbnail fails with an appropriate
exception or validation error. If choosing the latter, update the
ContentCreateRequest validation logic, the contentService.create method, and
modify the test expectations to verify the failure behavior instead of success.
- Around line 159-205: Add a new test method following the pattern of
create_newTag_savesTag and create_duplicateTags_savesOnce that validates tag
normalization behavior. The test should create a ContentCreateRequest with tags
containing leading/trailing whitespace and mixed case characters (for example, "
액션 " and "액션"), set up the mocks similar to existing tests, and then verify that
the tagRepository.save method is called only once with the normalized tag. This
ensures that the trim and lowercase normalization logic in the content service
is properly tested and prevents regressions.
---
Duplicate comments:
In `@src/main/java/com/codeit/team5/mopl/content/service/ContentService.java`:
- Line 41: The TODO comment in ContentService indicates that thumbnail storage
upload and URL reflection functionality is incomplete, yet the thumbnailUrl
parameter is part of the API contract, resulting in empty thumbnail URLs in
responses. To fix this, you must either: (1) implement the complete thumbnail
storage upload logic by capturing the uploaded file, storing it to the
designated storage service, retrieving the generated URL, and mapping it to the
thumbnailUrl field in the response object, or (2) remove the thumbnailUrl
parameter from the API contract/DTO if deferring this feature to a future
release. Coordinate with your team to decide which approach aligns with the
sprint goals, then either complete the ContentService implementation or remove
the thumbnail-related fields from the API specification and relevant DTOs.
🪄 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: b1ccfa9c-3e10-49ca-b040-583d9175438e
📒 Files selected for processing (28)
build.gradlesrc/main/java/com/codeit/team5/mopl/content/controller/ContentController.javasrc/main/java/com/codeit/team5/mopl/content/controller/api/ContentApi.javasrc/main/java/com/codeit/team5/mopl/content/dto/request/ContentCreateRequest.javasrc/main/java/com/codeit/team5/mopl/content/dto/response/ContentResponse.javasrc/main/java/com/codeit/team5/mopl/content/entity/Content.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentSource.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentStats.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentTag.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentType.javasrc/main/java/com/codeit/team5/mopl/content/exception/ContentException.javasrc/main/java/com/codeit/team5/mopl/content/exception/ContentNotFoundException.javasrc/main/java/com/codeit/team5/mopl/content/exception/InvalidContentTypeException.javasrc/main/java/com/codeit/team5/mopl/content/mapper/ContentMapper.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentStatsRepository.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/global/exception/ErrorCode.javasrc/main/java/com/codeit/team5/mopl/tag/entity/Tag.javasrc/main/java/com/codeit/team5/mopl/tag/exception/InvalidTagNameException.javasrc/main/java/com/codeit/team5/mopl/tag/exception/TagException.javasrc/main/java/com/codeit/team5/mopl/tag/repository/TagRepository.javasrc/main/resources/db/migration/V3__modify_watcher_count_type.sqlsrc/main/resources/db/migration/V4__add_admin_content_source.sqlsrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerTest.javasrc/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.javasrc/test/java/com/codeit/team5/mopl/watcher/repository/WatchingSessionRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/watcher/service/WatchingSessionServiceTest.java
There was a problem hiding this comment.
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/content/dto/request/ContentCreateRequest.java (1)
13-14: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
title길이 검증을 DTO에 추가해 DB 컬럼 제약과 맞춰 주세요.현재는
@NotBlank만 있어 500자 초과 제목이 검증을 통과할 수 있고, 이후 DB 저장 단계에서 실패합니다. 입력 계약을 API 단계에서 명확히 막는 게 장애/500 전파를 줄이는 데 유리합니다.선택지는 두 가지입니다.
- 옵션 1 (권장): DTO에
@Size(max = 500)추가 → 가장 빠르고 일관된 400 응답.- 옵션 2: 엔티티 팩토리에서 길이 검증 → 도메인 불변식 강도는 높지만, API 입력 계약 가시성은 상대적으로 낮아짐.
🔧 제안 diff
import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; import java.util.List; public record ContentCreateRequest( `@NotNull`(message = "콘텐츠 타입은 필수입니다.") ContentType type, `@NotBlank`(message = "제목은 필수입니다.") + `@Size`(max = 500, message = "제목은 500자 이하여야 합니다.") String title,🤖 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/content/dto/request/ContentCreateRequest.java` around lines 13 - 14, The title field in the ContentCreateRequest DTO currently only validates that it is not blank, but does not enforce the maximum length constraint that exists in the database schema. Add the `@Size`(max = 500) annotation to the title field (alongside the existing `@NotBlank` annotation) to validate the length constraint at the API input layer. This will ensure that titles exceeding 500 characters are rejected with a 400 Bad Request response before reaching the database layer, preventing cascading failures.
♻️ Duplicate comments (1)
src/main/java/com/codeit/team5/mopl/content/entity/Content.java (1)
81-86: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
createByExternalSource()에서source == null을 즉시 차단해 주세요.Line 81 조건은
ADMIN만 막고 있어서source == null이 통과됩니다. 이 경우 Line 60의nullable = false제약과 충돌해 저장 시점 예외로 늦게 터지고, API 레이어에서는 500으로 번질 수 있습니다.선택지는 두 가지입니다.
- 옵션 1 (권장): 현재 가드에서
null까지 함께 검증 → 도메인 예외로 빠르게 실패.- 옵션 2:
Objects.requireNonNull(source)+ 기존ADMIN체크 분리 → NPE 계열 예외가 섞여 예외 일관성은 떨어짐.🔧 제안 diff
public static Content createByExternalSource(ContentType type, String title, String description, ContentSource source, String externalId, Instant releasedAt, String metadata) { - if (source == ContentSource.ADMIN) { + if (source == null || source == ContentSource.ADMIN) { throw new InvalidContentSourceException("외부 소스 생성에 ADMIN 소스는 사용할 수 없습니다."); } if (externalId == null || externalId.isBlank()) { throw new InvalidContentSourceException("외부 소스 콘텐츠는 externalId가 필수입니다."); }🤖 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/content/entity/Content.java` around lines 81 - 86, In the createByExternalSource() method, the current validation at line 81 only checks if source equals ContentSource.ADMIN but does not check if source itself is null. Add a null check for the source parameter to validate that it is both not null and not ADMIN before the method proceeds, ensuring the domain exception is thrown early during validation rather than later at database save time due to the nullable = false constraint violation.
🤖 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.
Outside diff comments:
In
`@src/main/java/com/codeit/team5/mopl/content/dto/request/ContentCreateRequest.java`:
- Around line 13-14: The title field in the ContentCreateRequest DTO currently
only validates that it is not blank, but does not enforce the maximum length
constraint that exists in the database schema. Add the `@Size`(max = 500)
annotation to the title field (alongside the existing `@NotBlank` annotation) to
validate the length constraint at the API input layer. This will ensure that
titles exceeding 500 characters are rejected with a 400 Bad Request response
before reaching the database layer, preventing cascading failures.
---
Duplicate comments:
In `@src/main/java/com/codeit/team5/mopl/content/entity/Content.java`:
- Around line 81-86: In the createByExternalSource() method, the current
validation at line 81 only checks if source equals ContentSource.ADMIN but does
not check if source itself is null. Add a null check for the source parameter to
validate that it is both not null and not ADMIN before the method proceeds,
ensuring the domain exception is thrown early during validation rather than
later at database save time due to the nullable = false constraint violation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a7d476a6-d829-4c67-bd76-31e841ec9e0a
📒 Files selected for processing (6)
src/main/java/com/codeit/team5/mopl/content/dto/request/ContentCreateRequest.javasrc/main/java/com/codeit/team5/mopl/content/entity/Content.javasrc/main/java/com/codeit/team5/mopl/content/exception/InvalidContentSourceException.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/global/exception/ErrorCode.javasrc/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.java
plzslp
changed the title
There was a problem hiding this comment.
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/content/entity/Content.java (1)
84-93: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win외부 ID 정규화 누락으로 동일 콘텐츠 중복 적재 위험이 있습니다.
Line 84-85에서 blank 검증은 하고 있지만, Line 92에서
externalId를 정규화 없이 저장해"tt123"와" tt123 "가 다른 값으로 들어갈 수 있습니다. 현재 Line 29-33의(source, external_id)유니크 제약이 있어도 공백 변형은 막지 못해 연동 데이터 무결성이 깨질 수 있습니다.선택지는 두 가지입니다.
- 옵션 A (권장): 팩토리에서
trim()후 저장
장점: 변경 범위가 작고 즉시 효과가 큼.
단점: 대소문자/포맷 규칙까지는 별도 합의가 필요.- 옵션 B: 외부 ID 전용 값 객체 도입
장점: 정규화/검증 규칙을 강하게 일원화 가능.
단점: 적용 범위가 넓어 PR 범위를 키움.🔧 최소 변경 제안
if (externalId == null || externalId.isBlank()) { throw new InvalidContentSourceException("외부 소스 콘텐츠는 externalId가 필수입니다."); } + String normalizedExternalId = externalId.trim(); + if (normalizedExternalId.isEmpty()) { + throw new InvalidContentSourceException("외부 소스 콘텐츠는 externalId가 필수입니다."); + } Content content = new Content(); content.type = type; content.title = title; content.description = description; content.source = source; - content.externalId = externalId; + content.externalId = normalizedExternalId; content.releasedAt = releasedAt; content.metadata = metadata; return content;🤖 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/content/entity/Content.java` around lines 84 - 93, The externalId is being stored without normalization, allowing whitespace variations like "tt123" and " tt123 " to be treated as different values and create duplicates despite the unique constraint. Normalize the externalId by calling trim() on the parameter before assigning it to content.externalId in the Content factory method to ensure consistent storage and prevent data integrity issues from external source integration.
🤖 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.
Outside diff comments:
In `@src/main/java/com/codeit/team5/mopl/content/entity/Content.java`:
- Around line 84-93: The externalId is being stored without normalization,
allowing whitespace variations like "tt123" and " tt123 " to be treated as
different values and create duplicates despite the unique constraint. Normalize
the externalId by calling trim() on the parameter before assigning it to
content.externalId in the Content factory method to ensure consistent storage
and prevent data integrity issues from external source integration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b6a08dfa-91d0-455d-bf47-93320d687d74
📒 Files selected for processing (2)
src/main/java/com/codeit/team5/mopl/content/dto/request/ContentCreateRequest.javasrc/main/java/com/codeit/team5/mopl/content/entity/Content.java
metDaisy
previously approved these changes
Jun 24, 2026
# Conflicts: # src/main/java/com/codeit/team5/mopl/content/entity/Content.java
- source 타입에 'ADMIN' 추가
- ContentMapper: singleton empty list 반환 - ContentService: 태그 목록 saveAll로 수정 - ContentControllerTest: DTO 검증(Captor) 추가
plzslp
force-pushed
the
feature/5-content-posting
branch
from
2c7cbd5 to
6556afb
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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`:
- Line 31: The springdoc-openapi-starter-webmvc-ui dependency at version 2.8.8
has compatibility issues with Spring Boot 3.5.15, causing potential
NoSuchMethodError exceptions and may include outdated security patches in
dependent libraries. Update the version number in the
springdoc-openapi-starter-webmvc-ui dependency declaration in build.gradle from
2.8.8 to 2.8.17, which provides full Spring Boot 3.5.x compatibility and
includes the latest security patches for all transitive dependencies.
In
`@src/main/java/com/codeit/team5/mopl/content/controller/ContentController.java`:
- Around line 27-36: The request log in the postContent method currently only
logs a simple entry message without any parameter details, making debugging and
monitoring difficult. Enhance the log.info statement to include useful debugging
information by adding parameters such as the content type and title from the
request object, and whether a thumbnail file was provided. Modify the log.info
call to use a parametrized format string that includes request.type(),
request.title(), and a check for thumbnail != null to provide better visibility
into what content is being created.
In `@src/main/java/com/codeit/team5/mopl/content/entity/ContentType.java`:
- Around line 23-31: The from() method in ContentType enum throws
InvalidContentTypeException during deserialization, but Jackson wraps it in
JsonMappingException which Spring then wraps in HttpMessageNotReadableException.
Since GlobalExceptionHandler only handles BusinessException and
MethodArgumentNotValidException, this results in a 500 error instead of the
expected 400. Add a new handler method in GlobalExceptionHandler that catches
HttpMessageNotReadableException, inspects the cause chain for BusinessException
to extract the appropriate ErrorCode, and returns a 400 response with
INVALID_INPUT when no BusinessException is found. Additionally, add a test case
to ContentControllerTest that sends an invalid enum value (not just missing the
field) to verify the correct error response is returned.
In
`@src/main/java/com/codeit/team5/mopl/content/exception/InvalidContentTypeException.java`:
- Around line 7-9: The InvalidContentTypeException constructor currently uses
the generic INVALID_INPUT error code, which is inconsistent with similar domain
exceptions like InvalidContentSourceException (uses INVALID_CONTENT_SOURCE) and
InvalidTagNameException (uses INVALID_TAG_NAME). To fix this, first add a new
INVALID_CONTENT_TYPE error code to the ErrorCode enum in the Content section,
then update the InvalidContentTypeException constructor to use this new specific
error code instead of INVALID_INPUT. This ensures all content domain validation
exceptions follow a consistent pattern with specific, self-documenting error
codes.
In `@src/main/java/com/codeit/team5/mopl/content/mapper/ContentMapper.java`:
- Around line 21-26: Based on the review comment, the current implementation in
the toTagNames method is already correct and efficient. The method properly uses
Collections.emptyList() for the null case (an immutable singleton that avoids
unnecessary object creation) and .toList() for the stream case (which also
returns an immutable list). Both approaches are consistent and memory-efficient,
so no code changes are required. The current implementation follows good
practices and aligns with your team's coding standards.
In `@src/main/java/com/codeit/team5/mopl/content/service/ContentService.java`:
- Around line 56-65: The tag creation logic has a race condition where multiple
concurrent requests can try to create the same new tag, causing a unique
constraint violation when saveAll is called on the newTags list. Fix this by
wrapping the tagRepository.saveAll(newTags) call in a try-catch block to handle
integrity constraint exceptions, then when an exception is caught, re-query
tagRepository.findByNameIn() for the tags that failed to insert and add them to
the existingTags map so the operation completes successfully instead of failing
with a 500 error.
In `@src/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.java`:
- Around line 80-81: The test mocks in ContentServiceTest are set up with
tagRepository.save() stubs, but the ContentService implementation uses
tagRepository.saveAll() for batch tag operations. Update all mock setups (at
lines 80-81, 97-99, 158-159, 173-175, 182-183, 198-199, 206-207) to replace
when(tagRepository.save(any(Tag.class))) with
when(tagRepository.saveAll(any(List.class))) to match the actual service
contract. Consider using ArgumentCaptor with List<Tag> type to capture and
verify the exact tags and counts being saved in each scenario (new tags vs.
duplicate tags), ensuring the test validates the actual batch save behavior
rather than individual saves.
🪄 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: 18b613d1-6307-41f6-a56a-b9ab87b2328d
📒 Files selected for processing (29)
build.gradlesrc/main/java/com/codeit/team5/mopl/content/controller/ContentController.javasrc/main/java/com/codeit/team5/mopl/content/controller/api/ContentApi.javasrc/main/java/com/codeit/team5/mopl/content/dto/request/ContentCreateRequest.javasrc/main/java/com/codeit/team5/mopl/content/dto/response/ContentResponse.javasrc/main/java/com/codeit/team5/mopl/content/entity/Content.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentSource.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentStats.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentTag.javasrc/main/java/com/codeit/team5/mopl/content/entity/ContentType.javasrc/main/java/com/codeit/team5/mopl/content/exception/ContentException.javasrc/main/java/com/codeit/team5/mopl/content/exception/ContentNotFoundException.javasrc/main/java/com/codeit/team5/mopl/content/exception/InvalidContentSourceException.javasrc/main/java/com/codeit/team5/mopl/content/exception/InvalidContentTypeException.javasrc/main/java/com/codeit/team5/mopl/content/mapper/ContentMapper.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentStatsRepository.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/global/exception/ErrorCode.javasrc/main/java/com/codeit/team5/mopl/tag/entity/Tag.javasrc/main/java/com/codeit/team5/mopl/tag/exception/InvalidTagNameException.javasrc/main/java/com/codeit/team5/mopl/tag/exception/TagException.javasrc/main/java/com/codeit/team5/mopl/tag/repository/TagRepository.javasrc/main/resources/db/migration/V3__modify_watcher_count_type.sqlsrc/main/resources/db/migration/V4__add_admin_content_source.sqlsrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerTest.javasrc/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.javasrc/test/java/com/codeit/team5/mopl/watcher/repository/WatchingSessionRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/watcher/service/WatchingSessionServiceTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.java
metDaisy
approved these changes
Jun 24, 2026
This was referenced Jun 24, 2026
Merged
Merged
This was referenced Jun 26, 2026
This was referenced Jun 30, 2026
Merged
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.
관련 이슈
작업 내용
변경 사항
테스트 내용
체크리스트
리뷰 포인트
스크린샷 / 참고 자료
🔗 관련 이슈
#97#5#100📝 작업 내용
기존에는 “콘텐츠 생성 요청 → DB 저장 → 태그 매핑 → 통계 초기화 → 응답 반환”까지 이어지는 계층(도메인/DTO/매퍼/리포지토리/컨트롤러/서비스)과 그에 대한 테스트 기준이 없어, 기능을 실제로 완성하기 위한 기반이 부족했습니다. 이 PR은 콘텐츠 관리의 프리퀴지트 모델을 먼저 정리한 뒤, 관리자 전용 콘텐츠 등록 API와 end-to-end 데이터 흐름(태그/통계 포함)을 구현하고 테스트로 보완하는 것을 목표로 했습니다.
주요 변경사항
도메인 모델을 JPA 엔티티로 분리해 책임을 고정
Content,Tag,ContentTag(조인/복합키),ContentStats(1:1 통계)로 역할을 나누고 연관관계를 명확히 했습니다.Content의type/source는 문자열 대신ContentType,ContentSourceenum으로 정합성을 맞추고(EnumType.STRING),metadata는 JSONB 저장을 위해String+@JdbcTypeCode로 변경했습니다.Content에서는 기존updatedAt를 제거하고, 통계 갱신 시각은ContentStats.updatedAt으로 일원화했습니다.contents테이블에source + external_id유니크 제약을 두어 외부/관리자 소스 데이터의 중복 가능성을 제한했습니다.멀티파트 기반 API 계약을 확정하고, 서비스에서 “생성→태그 정규화→매핑→통계”를 한 번에 처리
/api/contentsPOST를ContentController에서 제공하고,ContentCreateRequest(JSON 파트)와thumbnail(선택 파트)을 함께 받도록 계약을 구성했습니다.ContentApi를 추가하고, 요청 파트(request,thumbnail)의 노출을 숨기도록 설계했습니다.ContentService.create()는 다음 흐름으로 동작합니다.Content.createByAdmin(...)으로 콘텐츠 먼저 저장trim → 소문자화 → distinct로 정규화하고, 정규화 후 공백/빈 값만 남으면 즉시 입력 오류로 중단TagRepository.findByNameIn(...)로 기존 태그를 재사용하고, 없으면Tag.create(...)로 생성 후 저장Content.addTag()를 통해ContentTag연관(복합키) 구성ContentStats.create(content)로 통계 레코드를 초기화해 저장ContentMapper로ContentResponse(태그명/평균평점 계산 포함) 생성예외/에러코드 체계를 확장하고, 컨트롤러/서비스 테스트로 계약을 고정
ContentException계층,InvalidContentTypeException,InvalidContentSourceException,ContentNotFoundExceptionTagException계층,InvalidTagNameExceptionErrorCode에INVALID_TAG_NAME을 추가해 400 응답 메시지 체계를 보강했습니다.ContentControllerTest,ContentServiceTest를 추가/확장해:2개(컨트롤러/서비스) 추가되었습니다.📊 변경 효과 요약 (가능한 경우에만)
2개파일 (ContentControllerTest,ContentServiceTest)2개(V3,V4)springdoc-openapi-starter-webmvc-ui버전 업데이트 (2.6.0 → 2.8.8)4종+ DTO2종+ 매퍼/리포지토리/서비스/컨트롤러/예외/에러코드 확장