refactor: 콘텐츠 배치 리펙토링 및 콘텐츠 API 인가 적용#260
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough이번 PR은 두 축으로 나뉩니다. 배치 수집에서는 TMDB/ SportsDB Processor의 DB 중복 확인을 제거하고, ContentRepository에 externalId 일괄 조회 쿼리를 추가한 뒤, ContentItemWriter가 DB 기준으로 신규 항목만 저장하도록 바꿨습니다. 동시에 관련 테스트와 BatchConfig가 갱신됐습니다. 보안 쪽에서는 Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/config/SecurityConfig.java`:
- Around line 65-69: The ADMIN authorization is duplicated between
SecurityConfig’s URL matchers and the controllers’ `@PreAuthorize` annotations, so
choose a single source of truth and align the other layer accordingly. Update
the relevant access rules in SecurityConfig and the affected controller methods
so /api/contents and /api/admin/** follow one consistent authorization strategy,
using the existing matcher setup and `@PreAuthorize` usage as the reference
points.
In
`@src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java`:
- Around line 36-53: Add an early return in ContentItemWriter.write for empty
chunks before accessing items.get(0), since Spring Batch can call write() with a
filtered/skip-only Chunk that has no items. Update the logic around the initial
items List and source extraction so that items.isEmpty() returns immediately,
then only compute ContentSource source and query
contentRepository.findExternalIdsBySourceAndExternalIdIn when there is at least
one item.
In
`@src/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.java`:
- Around line 20-26: Remove the unused existsBySourceAndExternalId method from
ContentRepository if there are no remaining call sites, and keep the repository
API centered on findExternalIdsBySourceAndExternalIdIn for the batch flow.
Verify any references in the content persistence path still compile after
deleting the method, and update any related tests or usages tied to
ContentRepository accordingly.
In
`@src/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java`:
- Around line 98-118: Add a regression test for empty chunks in
ContentItemWriterTest to cover the items.get(0) path inside
ContentItemWriter.write(). Create a test that calls writer.write(new
Chunk<>(List.of())) and asserts it completes without throwing, so the empty
input case is safely handled. Use the existing
write_allExistingInDb_skipsAllSaves pattern as a guide and keep the focus on the
write() method behavior rather than repository interactions.
🪄 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: 9e65c0a5-7910-4cce-b234-fbc4b59ec63f
📒 Files selected for processing (13)
src/main/java/com/codeit/team5/mopl/config/BatchConfig.javasrc/main/java/com/codeit/team5/mopl/config/SecurityConfig.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/writer/ContentItemWriter.javasrc/main/java/com/codeit/team5/mopl/content/controller/ContentCollectionController.javasrc/main/java/com/codeit/team5/mopl/content/controller/ContentController.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java
💤 Files with no reviewable changes (3)
- src/main/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessor.java
- src/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessor.java
- src/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessor.java
metDaisy
previously approved these changes
Jul 3, 2026
conradrado
previously approved these changes
Jul 3, 2026
conradrado
left a comment
conradrado
left a comment
Collaborator
There was a problem hiding this comment.
크게 문제없어 보입니다~ 고생하셨습니다~
- ProcessorTest: ContentRepository mock 및 중복 검증 제거 - ContentItemWriterTest: IN 쿼리 stub 추가 및 DB 기반 중복 제거, early return 케이스 추가
- SecurityConfig에 경로 규칙 추가 - EnableMethodSecurity 활성화 - ContentController 각 메서드에 PreAuthorize 적용 - ContentCollectionController 클래스 레벨에 PreAuthorize 적용
- 빈 청크 반환처리 추가 및 테스트 코드 추가 - 사용되지 않는 ContentRepository 메서드 제거
plzslp
force-pushed
the
refactor/253-content-batch
branch
from
a9773d7 to
fe08805
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
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/controller/ContentController.java (1)
59-100: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winADMIN 인가에 대한 403 테스트를 추가하세요
@AutoConfigureMockMvc(addFilters = false)상태에서는 성공 케이스만으로@PreAuthorize("hasRole('ADMIN')")가 비-ADMIN 요청을 막는지 드러나지 않습니다.@WithMockUser(roles = "USER")로 403,roles = "ADMIN"으로 201/200을 각각 넣으면 이번 변경의 권한 규칙을 더 안전하게 보호할 수 있습니다.🤖 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/controller/ContentController.java` around lines 59 - 100, Add authorization coverage for the ADMIN-only content endpoints in the controller tests, since MockMvc with filters disabled does not prove `@PreAuthorize`("hasRole('ADMIN')") is enforced. Update the tests around ContentController to include a non-admin case using `@WithMockUser`(roles = "USER") that expects 403 for postContent and patchContent, and keep/admin-add equivalent success cases with `@WithMockUser`(roles = "ADMIN") for the same ContentController methods.
🤖 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/config/SecurityConfig.java`:
- Line 90: Open access is currently too broad because
SecurityConfig.anyRequest().permitAll() leaves unspecified API routes public,
including the missing /api/contents/{contentId}/watching-sessions path. Update
the SecurityConfig authorization rules so the default for anyRequest is
authenticated(), and keep only the truly public endpoints explicitly marked with
permitAll(); use the existing SecurityConfig configuration block to locate and
adjust the matcher order.
In
`@src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java`:
- Around line 43-57: `ContentItemWriter` currently filters only against IDs
already in the database, so duplicate `externalId` values inside the same chunk
can still reach `saveAll` and violate the `contents(source, external_id)`
uniqueness rule. After the existing `findExternalIdsBySourceAndExternalIdIn`
filter in `write`, add a second de-duplication pass over `deduplicatedItems`
using a stable key-based approach such as `LinkedHashMap` so only the first
occurrence of each `item.content().getExternalId()` is kept while preserving
order. Keep the existing DB-based filtering and the early return/logging path
intact.
In
`@src/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java`:
- Around line 76-129: 현재 추가된 DB 기준 테스트들은 좋지만, `ContentItemWriter`의 청크 내부 중복 처리에
대한 회귀 테스트가 빠져 있습니다. `write_deduplicatesAgainstDb`,
`write_emptyChunk_doesNothing`, `write_allExistingInDb_skipsAllSaves`와 같은 스타일로,
같은 청크에 동일한 externalId를 가진 `ContentWithMetaData` 두 개가 들어오고 DB에는 없을 때
`ContentItemWriter.write`가 중복 저장을 막는지 검증하는 테스트를 다시 추가하세요. 이 테스트는
`ContentItemWriter`의 청크 내 deduplication 로직을 직접 검증하도록 구성하면 됩니다.
---
Outside diff comments:
In
`@src/main/java/com/codeit/team5/mopl/content/controller/ContentController.java`:
- Around line 59-100: Add authorization coverage for the ADMIN-only content
endpoints in the controller tests, since MockMvc with filters disabled does not
prove `@PreAuthorize`("hasRole('ADMIN')") is enforced. Update the tests around
ContentController to include a non-admin case using `@WithMockUser`(roles =
"USER") that expects 403 for postContent and patchContent, and keep/admin-add
equivalent success cases with `@WithMockUser`(roles = "ADMIN") for the same
ContentController methods.
🪄 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: 322f3629-1e23-4616-890e-d6f280621ce2
📒 Files selected for processing (13)
src/main/java/com/codeit/team5/mopl/config/BatchConfig.javasrc/main/java/com/codeit/team5/mopl/config/SecurityConfig.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/writer/ContentItemWriter.javasrc/main/java/com/codeit/team5/mopl/content/controller/ContentCollectionController.javasrc/main/java/com/codeit/team5/mopl/content/controller/ContentController.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java
💤 Files with no reviewable changes (3)
- src/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessor.java
- src/main/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessor.java
- src/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessor.java
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
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/controller/ContentController.java (1)
59-100: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winADMIN 인가에 대한 403 테스트를 추가하세요
@AutoConfigureMockMvc(addFilters = false)상태에서는 성공 케이스만으로@PreAuthorize("hasRole('ADMIN')")가 비-ADMIN 요청을 막는지 드러나지 않습니다.@WithMockUser(roles = "USER")로 403,roles = "ADMIN"으로 201/200을 각각 넣으면 이번 변경의 권한 규칙을 더 안전하게 보호할 수 있습니다.🤖 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/controller/ContentController.java` around lines 59 - 100, Add authorization coverage for the ADMIN-only content endpoints in the controller tests, since MockMvc with filters disabled does not prove `@PreAuthorize`("hasRole('ADMIN')") is enforced. Update the tests around ContentController to include a non-admin case using `@WithMockUser`(roles = "USER") that expects 403 for postContent and patchContent, and keep/admin-add equivalent success cases with `@WithMockUser`(roles = "ADMIN") for the same ContentController methods.
🤖 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/config/SecurityConfig.java`:
- Line 90: Open access is currently too broad because
SecurityConfig.anyRequest().permitAll() leaves unspecified API routes public,
including the missing /api/contents/{contentId}/watching-sessions path. Update
the SecurityConfig authorization rules so the default for anyRequest is
authenticated(), and keep only the truly public endpoints explicitly marked with
permitAll(); use the existing SecurityConfig configuration block to locate and
adjust the matcher order.
In
`@src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java`:
- Around line 43-57: `ContentItemWriter` currently filters only against IDs
already in the database, so duplicate `externalId` values inside the same chunk
can still reach `saveAll` and violate the `contents(source, external_id)`
uniqueness rule. After the existing `findExternalIdsBySourceAndExternalIdIn`
filter in `write`, add a second de-duplication pass over `deduplicatedItems`
using a stable key-based approach such as `LinkedHashMap` so only the first
occurrence of each `item.content().getExternalId()` is kept while preserving
order. Keep the existing DB-based filtering and the early return/logging path
intact.
In
`@src/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java`:
- Around line 76-129: 현재 추가된 DB 기준 테스트들은 좋지만, `ContentItemWriter`의 청크 내부 중복 처리에
대한 회귀 테스트가 빠져 있습니다. `write_deduplicatesAgainstDb`,
`write_emptyChunk_doesNothing`, `write_allExistingInDb_skipsAllSaves`와 같은 스타일로,
같은 청크에 동일한 externalId를 가진 `ContentWithMetaData` 두 개가 들어오고 DB에는 없을 때
`ContentItemWriter.write`가 중복 저장을 막는지 검증하는 테스트를 다시 추가하세요. 이 테스트는
`ContentItemWriter`의 청크 내 deduplication 로직을 직접 검증하도록 구성하면 됩니다.
---
Outside diff comments:
In
`@src/main/java/com/codeit/team5/mopl/content/controller/ContentController.java`:
- Around line 59-100: Add authorization coverage for the ADMIN-only content
endpoints in the controller tests, since MockMvc with filters disabled does not
prove `@PreAuthorize`("hasRole('ADMIN')") is enforced. Update the tests around
ContentController to include a non-admin case using `@WithMockUser`(roles =
"USER") that expects 403 for postContent and patchContent, and keep/admin-add
equivalent success cases with `@WithMockUser`(roles = "ADMIN") for the same
ContentController methods.
🪄 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: 322f3629-1e23-4616-890e-d6f280621ce2
📒 Files selected for processing (13)
src/main/java/com/codeit/team5/mopl/config/BatchConfig.javasrc/main/java/com/codeit/team5/mopl/config/SecurityConfig.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/writer/ContentItemWriter.javasrc/main/java/com/codeit/team5/mopl/content/controller/ContentCollectionController.javasrc/main/java/com/codeit/team5/mopl/content/controller/ContentController.javasrc/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessorTest.javasrc/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java
💤 Files with no reviewable changes (3)
- src/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbMovieItemProcessor.java
- src/main/java/com/codeit/team5/mopl/content/batch/processor/SportsDbEventItemProcessor.java
- src/main/java/com/codeit/team5/mopl/content/batch/processor/TmdbTvSeriesItemProcessor.java
🛑 Comments failed to post (3)
src/main/java/com/codeit/team5/mopl/config/SecurityConfig.java (1)
90-90: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # 컨트롤러 매핑과 SecurityConfig 화이트리스트 비교: 누락된 공개/보호 경로 탐지 rg -nP '@(Get|Post|Put|Patch|Delete|Request)Mapping' --type=java -g '!**/test/**' -C1 echo '--- SecurityConfig matchers ---' rg -nP 'requestMatchers|anyRequest' src/main/java/com/codeit/team5/mopl/config/SecurityConfig.javaRepository: sb10-part4-team5/sb10-mopl-team5
Length of output: 170
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate relevant files printf '== SecurityConfig ==\n' fd -a 'SecurityConfig.java' . printf '\n== Controller files ==\n' fd -a '.java' src/main/java | rg '/controller/|Controller\.java$|/web/'Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 3483
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect SecurityConfig and nearby routing definitions SEC="$(fd -a 'SecurityConfig.java' src/main/java | head -n 1)" printf 'SecurityConfig: %s\n' "$SEC" wc -l "$SEC" cat -n "$SEC" | sed -n '1,220p' printf '\n== Controllers with mappings ==\n' rg -n '@(Get|Post|Put|Patch|Delete|Request)Mapping' src/main/java -g '*.java' -C 2Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 29373
🏁 Script executed:
#!/bin/bash set -euo pipefail # List request matcher usages to see whether anyRequest().permitAll() is intentional rg -n 'requestMatchers|anyRequest|csrf|authorizeHttpRequests' src/main/java -g '*.java' -C 3Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 14506
🏁 Script executed:
#!/bin/bash set -euo pipefail SEC="$(fd -a 'SecurityConfig.java' src/main/java | head -n 1)" printf '== %s ==\n' "$SEC" wc -l "$SEC" cat -n "$SEC" | sed -n '1,220p' printf '\n== Controller mappings ==\n' rg -n '@(Get|Post|Put|Patch|Delete|Request)Mapping' src/main/java -g '*.java' -C 2 printf '\n== Security matchers ==\n' rg -n 'requestMatchers|anyRequest|csrf|authorizeHttpRequests' src/main/java -g '*.java' -C 3Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 43718
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect watching-session endpoints to see whether they are intended to be protected FILE="$(fd -a 'WatchingSessionController.java' src/main/java | head -n 1)" printf '== %s ==\n' "$FILE" cat -n "$FILE" | sed -n '1,220p' printf '\n== Interface / API contract ==\n' API="$(fd -a 'WatchingSessionRestApi.java' src/main/java | head -n 1)" printf '%s\n' "$API" cat -n "$API" | sed -n '1,220p'Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 4975
🏁 Script executed:
#!/bin/bash set -euo pipefail # Extract all request mappings and compare them with SecurityConfig patterns python3 - <<'PY' import re, pathlib root = pathlib.Path('src/main/java') patterns = [] security = root / 'com/codeit/team5/mopl/config/SecurityConfig.java' text = security.read_text() for line in text.splitlines(): m = re.search(r'requestMatchers\((.*)\)', line) if m: patterns.append(line.strip()) print("SECURITY_MATCHER_LINES") for p in patterns: print(p) print("\nREQUEST_MAPPINGS") for path in root.rglob('*.java'): s = path.read_text() if '`@RequestMapping`' in s or '`@GetMapping`' in s or '`@PostMapping`' in s or '`@PatchMapping`' in s or '`@DeleteMapping`' in s: for m in re.finditer(r'@(RequestMapping|GetMapping|PostMapping|PatchMapping|DeleteMapping)\s*(?:\(([^)]*)\))?', s): annot = m.group(1) args = m.group(2) or '' if '/api/' in args or '/api' in args: print(f"{path}: @{annot}({args})") PYRepository: sb10-part4-team5/sb10-mopl-team5
Length of output: 2715
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show whether the watching-session routes are already covered by existing matchers FILE="$(fd -a 'WatchingSessionController.java' src/main/java | head -n 1)" API="$(fd -a 'WatchingSessionRestApi.java' src/main/java | head -n 1)" printf '== %s ==\n' "$FILE" cat -n "$FILE" | sed -n '1,220p' printf '\n== %s ==\n' "$API" cat -n "$API" | sed -n '1,220p'Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 4950
🏁 Script executed:
#!/bin/bash set -euo pipefail # Identify any controller endpoints that are not matched by SecurityConfig and may fall through to permitAll() python3 - <<'PY' import re, pathlib, os sec = pathlib.Path('src/main/java/com/codeit/team5/mopl/config/SecurityConfig.java').read_text() # collect matcher entries roughly matchers = [] for line in sec.splitlines(): line = line.strip() if '.requestMatchers(' in line or '.anyRequest()' in line: matchers.append(line) print("MATCHER_LINES") for m in matchers: print(m) print("\nPOTENTIAL_API_PATHS") for path in pathlib.Path('src/main/java').rglob('*.java'): txt = path.read_text() if '/api/' not in txt: continue if any(a in txt for a in ['`@GetMapping`', '`@PostMapping`', '`@PatchMapping`', '`@DeleteMapping`', '`@RequestMapping`']): # print request mapping annotations with /api paths for line in txt.splitlines(): if '`@RequestMapping`' in line or '`@GetMapping`' in line or '`@PostMapping`' in line or '`@PatchMapping`' in line or '`@DeleteMapping`' in line: if '/api/' in line or '/api"' in line or "/api'" in line: print(f"{path}: {line.strip()}") PYRepository: sb10-part4-team5/sb10-mopl-team5
Length of output: 2427
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="$(fd -a 'WatchingSessionController.java' src/main/java | head -n 1)" API="$(fd -a 'WatchingSessionRestApi.java' src/main/java | head -n 1)" printf '== %s ==\n' "$FILE" cat -n "$FILE" | sed -n '1,220p' printf '\n== %s ==\n' "$API" cat -n "$API" | sed -n '1,220p'Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 4950
anyRequest().permitAll()은 이 API에서 누락된 경로를 그대로 공개합니다.
현재 명시 규칙에 없는/api/contents/{contentId}/watching-sessions가 인증 없이 열립니다. 기본값을authenticated()로 잠그고, 정말 공개가 필요한 경로만permitAll()로 풀어두는 쪽이 안전합니다.🤖 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/config/SecurityConfig.java` at line 90, Open access is currently too broad because SecurityConfig.anyRequest().permitAll() leaves unspecified API routes public, including the missing /api/contents/{contentId}/watching-sessions path. Update the SecurityConfig authorization rules so the default for anyRequest is authenticated(), and keep only the truly public endpoints explicitly marked with permitAll(); use the existing SecurityConfig configuration block to locate and adjust the matcher order.src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java (1)
43-57: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '== file list ==\n' git ls-files | rg 'ContentItemWriter|ContentSource|ContentWithMetaData|ContentRepository|batch/writer|content/repository|content/.+Entity|content/.+java$' printf '\n== outline: writer ==\n' ast-grep outline src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java --view expanded || true printf '\n== outline: repository/entity candidates ==\n' for f in $(git ls-files | rg 'ContentRepository|ContentSource|Content.*Entity|Content.*Repository'); do echo "--- $f" ast-grep outline "$f" --view expanded || true done printf '\n== relevant searches ==\n' rg -n "findExternalIdsBySourceAndExternalIdIn|saveAll\\(|externalId|unique|UNIQUE|`@Column`|`@Table`|`@Index`|`@UniqueConstraint`|source.*externalId|externalId.*source" src/main/java src/test/javaRepository: sb10-part4-team5/sb10-mopl-team5
Length of output: 31122
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '== ContentItemWriter.java ==\n' cat -n src/main/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriter.java | sed -n '1,220p' printf '\n== Content.java ==\n' cat -n src/main/java/com/codeit/team5/mopl/content/entity/Content.java | sed -n '1,180p' printf '\n== ContentRepository.java ==\n' cat -n src/main/java/com/codeit/team5/mopl/content/repository/ContentRepository.java | sed -n '1,120p' printf '\n== ContentItemWriterTest.java ==\n' cat -n src/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java | sed -n '1,340p'Repository: sb10-part4-team5/sb10-mopl-team5
Length of output: 28010
청크 내부
externalId중복도 다시 제거해야 합니다지금 로직은 DB에 이미 있는 값만 걸러서, 같은 청크 안의 신규 중복은 그대로
saveAll로 들어갑니다.contents(source, external_id)유니크 제약이 있으니 여기서 충돌하면 청크 전체가 롤백될 수 있습니다.
DB 필터 뒤에LinkedHashMap으로 한 번 더 묶으면 순서도 유지됩니다. 순서가 중요하지 않다면distinct()도 가능하지만, 중복 제거 기준이 명확한LinkedHashMap쪽이 더 안전합니다.🤖 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 43 - 57, `ContentItemWriter` currently filters only against IDs already in the database, so duplicate `externalId` values inside the same chunk can still reach `saveAll` and violate the `contents(source, external_id)` uniqueness rule. After the existing `findExternalIdsBySourceAndExternalIdIn` filter in `write`, add a second de-duplication pass over `deduplicatedItems` using a stable key-based approach such as `LinkedHashMap` so only the first occurrence of each `item.content().getExternalId()` is kept while preserving order. Keep the existing DB-based filtering and the early return/logging path intact.src/test/java/com/codeit/team5/mopl/content/batch/writer/ContentItemWriterTest.java (1)
76-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
DB 기준 시나리오 테스트는 탄탄합니다.
write_deduplicatesAgainstDb,write_emptyChunk_doesNothing,write_allExistingInDb_skipsAllSaves모두 새 로직을 잘 검증하고 있습니다. 다만 이전에 있던 "청크 내부 externalId 중복" 테스트가 대체 없이 삭제되었는데,ContentItemWriter.java에 남긴 코멘트처럼 이 시나리오(같은 청크에 동일 externalId 두 건, 둘 다 DB에 없음)에 대한 회귀 테스트를 하나 추가해 두면 향후 재발을 조기에 잡을 수 있습니다.🤖 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/content/batch/writer/ContentItemWriterTest.java` around lines 76 - 129, 현재 추가된 DB 기준 테스트들은 좋지만, `ContentItemWriter`의 청크 내부 중복 처리에 대한 회귀 테스트가 빠져 있습니다. `write_deduplicatesAgainstDb`, `write_emptyChunk_doesNothing`, `write_allExistingInDb_skipsAllSaves`와 같은 스타일로, 같은 청크에 동일한 externalId를 가진 `ContentWithMetaData` 두 개가 들어오고 DB에는 없을 때 `ContentItemWriter.write`가 중복 저장을 막는지 검증하는 테스트를 다시 추가하세요. 이 테스트는 `ContentItemWriter`의 청크 내 deduplication 로직을 직접 검증하도록 구성하면 됩니다.
metDaisy
approved these changes
Jul 3, 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
3 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.
관련 이슈
작업 내용
변경 사항
테스트 내용
체크리스트
리뷰 포인트
스크린샷 / 참고 자료
🔗 관련 이슈
#253#147📝 작업 내용
이번 PR은 (1) 콘텐츠 수집 배치의 중복 체크로 인해 발생하던 과도한 DB 조회와 책임 혼선을 줄이고, (2) 콘텐츠 API 및 관리자용 콘텐츠 수집 경로에 대한 인가를 명확히 적용하는 데 목적이 있습니다.
기존에는 배치의
Processor가 아이템 단위로existsBySourceAndExternalId()조회를 수행하며 중복을 판단했습니다. 이 방식은 청크 크기만큼 쿼리가 늘어날 수 있고,Processor가 “매핑” 외에 “영속성 조회/중복 판단”까지 떠안게 되어 역할이 흐려졌습니다. 또한 중복 반환(스킵) 방식이 바뀌면서filter_count기록 흐름도 함께 정리할 필요가 있었습니다.또한 콘텐츠 생성/수정/삭제 및 관리자용 수집 API는 ADMIN 전용으로 제한되어야 했지만, 권한 규칙이 충분히 반영되지 않은 상태였던 부분을 보완했습니다.
주요 변경사항
배치 중복 체크 책임 이동 (Processor → Writer)
Processor에서는 DTO → Entity/메타데이터 변환만 수행하도록 단순화했고,ContentRepository기반 존재 조회/스킵 분기 로직을 제거했습니다.Writer에서는 청크 전체의externalId를 모은 뒤 source + externalId 목록 기준IN쿼리로 기존 콘텐츠를 한 번에 조회하고, 이미 존재하는 항목은 제외한 뒤 신규 데이터만 저장하도록 흐름을 변경했습니다.filter_count는 더 이상 기록되지 않게 정리했습니다.Repository 확장: 일괄 조회용 쿼리 추가
ContentRepository에findExternalIdsBySourceAndExternalIdIn(...)(source + externalId 목록IN조회) 메서드를 추가하고, 아이템 단건 존재 여부 확인 메서드는 제거했습니다.콘텐츠 API 인가 적용(ADMIN 제한)
SecurityConfig에 요청 경로 기반 인가 규칙을 추가하고, 메서드 수준 보안 활성화를 위해@EnableMethodSecurity를 추가했습니다.ContentController의 생성/수정/삭제(POST/PATCH/DELETE /api/contents*)는@PreAuthorize("hasRole('ADMIN')")로 제한했습니다.ContentCollectionController의 관리자용 수집 엔드포인트(/api/admin/**하위)는 클래스 레벨@PreAuthorize("hasRole('ADMIN')")로 제한했습니다.📌 리뷰 시 특히 주의할 변경/트레이드오프
Processor에서Writer로 이동하면서, 청크 단위로 조회 후 필터링이 이루어집니다. 따라서 기존처럼 아이템 처리 시점에 즉시 스킵/카운트가 발생하는 패턴은 사라집니다(예:filter_count기록 중단).SecurityConfig의 경로 매칭 규칙과@PreAuthorize메서드 제한이 함께 적용됩니다. 실제 역할(ROLE_prefix 포함 여부 등)이 기대와 일치하는지 확인이 필요합니다.📊 변경 효과 요약 (가능한 경우에만)