refactor: BianryContent 엔티티 및 Tag 추출 로직 리펙토링#305
Merged
Conversation
📝 WalkthroughWalkthroughBinaryContent 엔티티에서 externalUrl 팩토리 메서드를 제거하고 completed로 통합했으며, 이를 사용하던 ContentCollectionUtils와 ContentItemWriter 및 관련 테스트를 수정했습니다. 태그명 정규화 및 조회/생성 로직을 신규 TagResolver 클래스로 추출해 ContentService와 ContentItemWriter의 중복 로직을 대체하고, ContentCollectionUtils 패키지를 service.util에서 batch.util로 이동했습니다. TagResolver에 대한 단위 테스트가 추가되었습니다. Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
리뷰 시 참고할 만한 몇 가지 포인트를 짚어드릴게요.
Comments failed to post (3) 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 5
🤖 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/batch/writer/ContentItemWriter.java`:
- Around line 101-115: 태그 처리 로직에서 정규화가 `TagResolver.normalizeNames`와
`deduplicatedItems.forEach` 내부에 중복 구현되어 있고, item 단위 중복 제거도 빠져 있습니다.
`ContentItemWriter`의 태그 저장 블록에서 `tagName.trim().toLowerCase()` 재구현을 제거하고
`TagResolver.normalizeNames(item.tagNames())`를 재사용해 정규화 정책을 한 곳으로 통일하세요. 또한 각
item의 태그 목록은 distinct된 결과만 순회하도록 바꿔 `ContentTag.create(...)`가 동일 태그에 대해 여러 번
호출되지 않게 처리하세요.
In `@src/main/java/com/codeit/team5/mopl/tag/util/TagResolver.java`:
- Around line 33-47: `TagResolver.resolve()`는 정규화된 중복 없는 입력을 전제로 하지만 이를 코드상으로
보장하지 않아 동일 태그가 반복 생성될 수 있습니다. `resolve`, `Tag.create`,
`tagRepository.findByNameIn` 주변에서 `tagNames`에 대해 방어적으로 중복 제거를 추가하거나, 메서드
Javadoc에 입력 전제(정규화/중복 제거 필요)를 명시해 `ContentService`와 `ContentItemWriter` 외의 호출에서도
안전하게 사용되도록 수정하세요.
- Around line 33-47: TagResolver.resolve() currently does a read-then-save flow
for missing tag names, which can race with the tags.name unique constraint under
concurrent use. Update the resolve method so new tag creation is atomic in the
TagResolver/TagRepository path, either by using a DB upsert-style save or by
catching saveAll() conflicts and reloading the existing tags before merging them
into existingTags. Keep the existing resolve() contract intact while ensuring
duplicate tag names from overlapping controller/scheduler/job executions do not
fail with DataIntegrityViolationException.
In
`@src/test/java/com/codeit/team5/mopl/content/service/util/ContentCollectionUtilsTest.java`:
- Line 9: The test for ContentCollectionUtils is still located under the old
content.service.util package path even though the utility moved to
content.batch.util. Move ContentCollectionUtilsTest to the matching
content.batch.util test package, update its package declaration to align with
the new location, and keep the ContentCollectionUtils import consistent so the
test structure mirrors the production class.
In `@src/test/java/com/codeit/team5/mopl/tag/util/TagResolverTest.java`:
- Around line 87-102: TagResolverTest의 resolve() 커버리지에 중복된 신규 입력 시나리오가 빠져 있습니다.
TagResolver.resolve(List<String>, TagRepository)로 List.of("액션", "액션") 같은 중복 이름을
넣었을 때 saveAll에 동일 이름의 Tag가 2건 전달되는지, 그리고 반환된 Map이 기대대로 동작하는지 검증하는 테스트 케이스를
추가하세요. 기존 resolve_allNew_createsAndSavesAll 패턴을 참고하되,
tagRepository.findByNameIn과 tagRepository.saveAll의 동작을 중복 입력 기준으로 설정하고
ArgumentCaptor로 전달된 Tag 목록을 확인하면 됩니다.
🪄 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: 21b85262-483e-4c6f-8792-c37ac6c3e8c4
📒 Files selected for processing (16)
src/main/java/com/codeit/team5/mopl/binarycontent/entity/BinaryContent.javasrc/main/java/com/codeit/team5/mopl/binarycontent/service/BinaryContentService.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/util/ContentCollectionUtils.javasrc/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/tag/util/TagResolver.javasrc/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/repository/ContentRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.javasrc/test/java/com/codeit/team5/mopl/content/service/util/ContentCollectionUtilsTest.javasrc/test/java/com/codeit/team5/mopl/tag/util/TagResolverTest.javasrc/test/java/com/codeit/team5/mopl/user/service/UserServiceTest.java
plzslp
force-pushed
the
refactor/271-binary-content-tag-refactor
branch
from
3e7a878 to
36bb0a7
Compare
Collaborator
Author
|
@coderabbitai 지금은 리뷰 가능해? |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/main/java/com/codeit/team5/mopl/tag/util/TagResolver.java (1)
47-63: 🎯 Functional Correctness | 🔴 Critical신규 태그 생성 원자성 문제, 여전히 미해결입니다.
resolve()는 조회 후 없는 이름만saveAll()하는 구조를 그대로 유지하고 있어, 이전 리뷰에서 지적된tags.name유니크 제약 위반 위험이 그대로 남아있습니다. 이 프로젝트는TaskExecutorJobLauncher로 배치를 비동기 실행하고 컨트롤러/스케줄러도 같은 경로를 재사용하므로, 겹친 실행에서 동일 태그명이 동시에 들어오면 한쪽이DataIntegrityViolationException으로 실패할 수 있습니다.Javadoc에 정규화/중복 제거 전제를 명시하고 내부
distinct()를 추가한 부분은 잘 반영됐지만, 이 원자성 문제 자체는 이번 커밋 범위에 포함되지 않은 것으로 보입니다.saveAll()실패를 캐치해 재조회 후 병합하거나, DB 레벨 upsert로 원자화하는 방향을 권장합니다.🤖 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/tag/util/TagResolver.java` around lines 47 - 63, The atomicity issue in TagResolver.resolve is still present: it checks existing tags and then calls saveAll on only the missing ones, which can still race on the unique tags.name constraint. Update resolve to handle concurrent inserts safely by either using a DB-level upsert/merge path or catching the persistence conflict from TagRepository.saveAll, then re-querying and merging the persisted tags back into the existingTags map. Keep the duplicate filtering in resolve, but make the TagResolver flow resilient so concurrent callers don’t fail with DataIntegrityViolationException.
🤖 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.
Duplicate comments:
In `@src/main/java/com/codeit/team5/mopl/tag/util/TagResolver.java`:
- Around line 47-63: The atomicity issue in TagResolver.resolve is still
present: it checks existing tags and then calls saveAll on only the missing
ones, which can still race on the unique tags.name constraint. Update resolve to
handle concurrent inserts safely by either using a DB-level upsert/merge path or
catching the persistence conflict from TagRepository.saveAll, then re-querying
and merging the persisted tags back into the existingTags map. Keep the
duplicate filtering in resolve, but make the TagResolver flow resilient so
concurrent callers don’t fail with DataIntegrityViolationException.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bc2b7dd2-b7e7-4d3f-aabe-74c61b98d3f2
⛔ Files ignored due to path filters (1)
src/main/resources/static/placeholder-movie.pngis excluded by!**/*.png
📒 Files selected for processing (16)
src/main/java/com/codeit/team5/mopl/binarycontent/entity/BinaryContent.javasrc/main/java/com/codeit/team5/mopl/binarycontent/service/BinaryContentService.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/util/ContentCollectionUtils.javasrc/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/tag/util/TagResolver.javasrc/test/java/com/codeit/team5/mopl/content/batch/util/ContentCollectionUtilsTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.javasrc/test/java/com/codeit/team5/mopl/content/controller/ContentControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/content/repository/ContentRepositoryTest.javasrc/test/java/com/codeit/team5/mopl/content/service/ContentServiceTest.javasrc/test/java/com/codeit/team5/mopl/tag/util/TagResolverTest.javasrc/test/java/com/codeit/team5/mopl/user/service/UserServiceTest.java
metDaisy
reviewed
Jul 7, 2026
metDaisy
approved these changes
Jul 7, 2026
- 연관된 BinaryContentService, ContentItemWriter, ContentCollectionUtils 수정 - 기존 BinaryContent 엔티티 생성 로직 사용 테스트 코드 수정
- of() -> completed()
plzslp
force-pushed
the
refactor/271-binary-content-tag-refactor
branch
from
36bb0a7 to
b75b619
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java (1)
101-116: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win정규화 로직 통일은 됐지만, item 내부 중복 태그명 dedup은 여전히 빠져있어요.
이전 리뷰에서 지적된 두 가지 중 "정규화 로직 중복 구현"은
TagResolver.normalize()로 잘 통일됐습니다. 하지만 두 번째 지적사항인 item 단위 dedup 누락은 그대로 남아있습니다.
item.tagNames().forEach(...)는distinct()없이 원본 리스트를 그대로 순회합니다. 만약 한 item의 태그 목록에"Action"과" action "처럼 정규화 후 동일한 값이 되는 항목이 섞여 있으면, 같은(content, tag)쌍에 대해ContentTag.create()가 두 번 호출되고, 두 개의ContentTag가content.addTag()로 중복 추가됩니다. 저장 시점에ContentTagId복합키 유니크 제약을 위반해 청크 전체가 실패할 위험이 있습니다.
TagResolver.normalizeNames(item.tagNames())로 바꾸면 정규화와 dedup을 한 번에 해결할 수 있습니다.♻️ 제안하는 수정
deduplicatedItems.forEach(item -> item.tagNames().forEach(rawTagName -> { - String normalized = TagResolver.normalize(rawTagName); - Tag tag = normalized == null ? null : existingTags.get(normalized); - if (tag != null) { + normalized -> { + Tag tag = existingTags.get(normalized); + if (tag != null) { item.content().addTag(ContentTag.create(item.content(), tag)); } }));또는 더 간단히:
- deduplicatedItems.forEach(item -> item.tagNames().forEach(rawTagName -> { - String normalized = TagResolver.normalize(rawTagName); - Tag tag = normalized == null ? null : existingTags.get(normalized); - if (tag != null) { - item.content().addTag(ContentTag.create(item.content(), tag)); - } - })); + deduplicatedItems.forEach(item -> TagResolver.normalizeNames(item.tagNames()).forEach(normalized -> { + Tag tag = existingTags.get(normalized); + if (tag != null) { + item.content().addTag(ContentTag.create(item.content(), tag)); + } + }));item별 tagNames에 실제로 정규화 후 중복될 수 있는 값이 유입되는지는 TMDB/SportsDB 응답 구조에 달려있으니, 한 번 확인해보시는 걸 권장합니다.
🤖 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/batch/writer/ContentItemWriter.java` around lines 101 - 116, The per-item tag loop in ContentItemWriter still iterates raw tagNames without deduplicating after normalization, so the same tag can be added twice to one content item. Update the inner processing in ContentItemWriter to normalize and dedupe each item’s tags before calling ContentTag.create() and content.addTag(), preferably by reusing TagResolver.normalizeNames(item.tagNames()) or an equivalent distinct pass over normalized names, while keeping TagResolver.resolve() for existing tag lookup.src/main/java/com/codeit/team5/mopl/tag/util/TagResolver.java (1)
35-63: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift동시 저장 시 유니크 제약 위반 위험이 아직 남아있습니다.
resolve()는 여전히 "조회 → 없는 것만 생성/저장"하는 read-then-write 패턴입니다. 이전 리뷰에서 이미 지적된 것처럼, 이 프로젝트는 배치 잡을 비동기로 띄우고(TaskExecutorJobLauncher) 컨트롤러/스케줄러가 같은 경로를 공유하므로, 두 실행이 겹쳐서 같은 신규 태그명을 동시에 처리하면 한쪽의saveAll()이tags.name유니크 제약에 걸려DataIntegrityViolationException으로 실패할 수 있습니다.선택지는 두 가지입니다:
- 재조회 후 병합:
saveAll()에서 제약 위반이 발생하면 캐치하고, 실패한 이름들을 다시findByNameIn으로 조회해 기존 태그로 병합. 구현이 상대적으로 간단하지만 예외 기반 흐름 제어라 다소 지저분함.- DB 레벨 upsert(예:
INSERT ... ON CONFLICT DO NOTHING후 재조회): 원자적으로 처리 가능하지만 벤더 종속 네이티브 쿼리가 필요해 구현 비용이 더 큼.지금 당장 재현 빈도가 낮을 수 있지만, 배치 동시 실행 시나리오가 실제로 존재한다면 반드시 짚고 넘어가야 할 데이터 정합성 문제입니다.
🤖 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/tag/util/TagResolver.java` around lines 35 - 63, `TagResolver.resolve()` still uses a non-atomic read-then-write flow, so concurrent callers can both miss the same new tag and then fail on the `tags.name` unique constraint during `saveAll()`. Update the resolution logic to be concurrency-safe: either catch `DataIntegrityViolationException` around `tagRepository.saveAll(...)`, re-query the conflicting names with `findByNameIn`, and merge the existing entities, or replace the current lookup/create path with an atomic upsert-based approach. Keep the existing `resolve`, `findByNameIn`, and `Tag::create` flow as the entry points, but ensure duplicate concurrent inserts cannot surface as a failure.
🤖 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.
Duplicate comments:
In
`@src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java`:
- Around line 101-116: The per-item tag loop in ContentItemWriter still iterates
raw tagNames without deduplicating after normalization, so the same tag can be
added twice to one content item. Update the inner processing in
ContentItemWriter to normalize and dedupe each item’s tags before calling
ContentTag.create() and content.addTag(), preferably by reusing
TagResolver.normalizeNames(item.tagNames()) or an equivalent distinct pass over
normalized names, while keeping TagResolver.resolve() for existing tag lookup.
In `@src/main/java/com/codeit/team5/mopl/tag/util/TagResolver.java`:
- Around line 35-63: `TagResolver.resolve()` still uses a non-atomic
read-then-write flow, so concurrent callers can both miss the same new tag and
then fail on the `tags.name` unique constraint during `saveAll()`. Update the
resolution logic to be concurrency-safe: either catch
`DataIntegrityViolationException` around `tagRepository.saveAll(...)`, re-query
the conflicting names with `findByNameIn`, and merge the existing entities, or
replace the current lookup/create path with an atomic upsert-based approach.
Keep the existing `resolve`, `findByNameIn`, and `Tag::create` flow as the entry
points, but ensure duplicate concurrent inserts cannot surface as a failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3ed9973c-c32a-4700-8919-a61976faee8e
⛔ Files ignored due to path filters (1)
src/main/resources/static/placeholder-movie.pngis excluded by!**/*.png
📒 Files selected for processing (11)
src/main/java/com/codeit/team5/mopl/binarycontent/entity/BinaryContent.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessor.javasrc/main/java/com/codeit/team5/mopl/content/batch/util/ContentCollectionUtils.javasrc/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.javasrc/main/java/com/codeit/team5/mopl/content/service/ContentService.javasrc/main/java/com/codeit/team5/mopl/tag/util/TagResolver.javasrc/test/java/com/codeit/team5/mopl/content/batch/util/ContentCollectionUtilsTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.javasrc/test/java/com/codeit/team5/mopl/tag/util/TagResolverTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/codeit/team5/mopl/binarycontent/entity/BinaryContent.java
metDaisy
approved these changes
Jul 7, 2026
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.
관련 이슈
작업 내용
변경 사항
테스트 내용
체크리스트
리뷰 포인트
스크린샷 / 참고 자료
🔗 관련 이슈
#측정하지않음📝 작업 내용
BinaryContent생성 방식과 태그 처리 흐름에서 중복되던 책임을 정리해, 콘텐츠 저장 로직이 더 일관된 공용 경로를 사용하도록 개선했습니다.TagResolver로 모아,ContentService와 배치 writer가 같은 규칙으로 태그를 처리하게 했습니다.BinaryContent생성 계약을 더 명확하게 맞췄습니다.주요 변경사항
BinaryContent의 중복 팩토리 경로를 정리하고, 썸네일/외부 URL 생성 흐름을 하나의 기준 메서드로 통합했습니다.TagResolver로 분리해 데이터 흐름을 단순화했습니다.ContentService와 배치 writer가 동일한 태그 해석 규칙을 사용하도록 바꾸고, 관련 JavaDoc도 현재 동작에 맞게 갱신했습니다.📊 변경 효과 요약
TagResolver테스트 1개 클래스 추가BinaryContent생성 기대값을 새 팩토리 기준으로 갱신