[Feat] 권한 pre-filter 구성 - #55
Conversation
…vice 구현 — F-SEARCH-04 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 51 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.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough권한 조건을 UNION으로 결합한 문서 ID pre-filter 쿼리와 전체·컬렉션 범위 라우팅 서비스를 추가하고, 조회 결과 전달 및 빈 목록 처리를 단위 테스트와 설계 문서로 정리합니다. Changes권한 pre-filter
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AccessibleDocumentQueryService
participant DocumentRepository
participant Database
AccessibleDocumentQueryService->>DocumentRepository: userId와 collectionId 기준 조회 위임
DocumentRepository->>Database: 읽기 권한 UNION 쿼리 실행
Database-->>DocumentRepository: 접근 가능한 문서 ID 목록 반환
DocumentRepository-->>AccessibleDocumentQueryService: 문서 ID 목록 반환
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/design/kangcheolung-`#54-search-permission-pre-filter.md:
- Around line 57-61: Specify the fenced code block language as text for the
findReadableDocumentIds flow diagram to resolve the MD040 lint violation,
without changing its contents.
In
`@src/test/java/com/opensource/docgrid/domain/search/service/query/AccessibleDocumentQueryServiceTest.java`:
- Around line 32-76: Replace the mock-based tests in
AccessibleDocumentQueryServiceTest with a `@DataJpaTest` integration test setup
that executes the native DocumentRepository queries against an actual test
database. Add fixtures and assertions covering OWNER, PUBLIC, and USER cache
expiration/invalidation, document and collection ROLE/DEPARTMENT permissions,
unauthorized documents, and collection-intersection filtering, including
empty-result cases; retain clear scenario-based test names and verify both
global and collection query methods.
- Around line 19-21: 클래스 수준 주석이 없어 AccessibleDocumentQueryServiceTest의 역할과 책임
범위가 드러나지 않습니다. `@DisplayName` 위 또는 클래스 선언부에 주석을 추가해 이 테스트 클래스가
AccessibleDocumentQueryService를 단위 테스트하며 테스트 대상과 범위를 명확히 설명하도록 하세요.
🪄 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.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 43137fd2-7ba4-4be6-8f49-ccbd9eabd1f8
📒 Files selected for processing (4)
docs/design/kangcheolung-#54-search-permission-pre-filter.mdsrc/main/java/com/opensource/docgrid/domain/document/repository/DocumentRepository.javasrc/main/java/com/opensource/docgrid/domain/search/service/query/AccessibleDocumentQueryService.javasrc/test/java/com/opensource/docgrid/domain/search/service/query/AccessibleDocumentQueryServiceTest.java
| @ExtendWith(MockitoExtension.class) | ||
| @DisplayName("AccessibleDocumentQueryService 단위 테스트") | ||
| class AccessibleDocumentQueryServiceTest { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
테스트 클래스의 역할과 경계를 클래스 주석으로 명시하세요.
@DisplayName은 클래스 수준 주석을 대체하지 않습니다.
수정 예시
+/**
+ * AccessibleDocumentQueryService의 저장소 호출 분기를 단위 테스트한다.
+ * 네이티브 권한 SQL 검증은 저장소 통합 테스트 범위다.
+ */
`@ExtendWith`(MockitoExtension.class)
`@DisplayName`("AccessibleDocumentQueryService 단위 테스트")
class AccessibleDocumentQueryServiceTest {As per coding guidelines, "Every newly created class, interface, or record must have a class-level comment describing its role, responsibility, and boundary."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ExtendWith(MockitoExtension.class) | |
| @DisplayName("AccessibleDocumentQueryService 단위 테스트") | |
| class AccessibleDocumentQueryServiceTest { | |
| /** | |
| * AccessibleDocumentQueryService의 저장소 호출 분기를 단위 테스트한다. | |
| * 네이티브 권한 SQL 검증은 저장소 통합 테스트 범위다. | |
| */ | |
| `@ExtendWith`(MockitoExtension.class) | |
| `@DisplayName`("AccessibleDocumentQueryService 단위 테스트") | |
| class AccessibleDocumentQueryServiceTest { |
🤖 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/opensource/docgrid/domain/search/service/query/AccessibleDocumentQueryServiceTest.java`
around lines 19 - 21, 클래스 수준 주석이 없어 AccessibleDocumentQueryServiceTest의 역할과 책임
범위가 드러나지 않습니다. `@DisplayName` 위 또는 클래스 선언부에 주석을 추가해 이 테스트 클래스가
AccessibleDocumentQueryService를 단위 테스트하며 테스트 대상과 범위를 명확히 설명하도록 하세요.
Source: Coding guidelines
| @Test | ||
| @DisplayName("collectionId가 null이면 전체 범위 쿼리를 호출하고 결과를 반환한다") | ||
| void findReadableDocumentIds_withoutCollection_callsGlobalQuery() { | ||
| List<Long> expected = List.of(1L, 2L, 3L); | ||
| given(documentRepository.findReadableDocumentIds(USER_ID)).willReturn(expected); | ||
|
|
||
| List<Long> result = accessibleDocumentQueryService.findReadableDocumentIds(USER_ID, null); | ||
|
|
||
| assertThat(result).isEqualTo(expected); | ||
| then(documentRepository).should(times(1)).findReadableDocumentIds(USER_ID); | ||
| then(documentRepository).shouldHaveNoMoreInteractions(); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("collectionId가 있으면 컬렉션 범위 쿼리를 호출하고 결과를 반환한다") | ||
| void findReadableDocumentIds_withCollection_callsCollectionQuery() { | ||
| List<Long> expected = List.of(2L, 3L); | ||
| given(documentRepository.findReadableDocumentIdsInCollection(USER_ID, COLLECTION_ID)).willReturn(expected); | ||
|
|
||
| List<Long> result = accessibleDocumentQueryService.findReadableDocumentIds(USER_ID, COLLECTION_ID); | ||
|
|
||
| assertThat(result).isEqualTo(expected); | ||
| then(documentRepository).should(times(1)).findReadableDocumentIdsInCollection(USER_ID, COLLECTION_ID); | ||
| then(documentRepository).shouldHaveNoMoreInteractions(); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("접근 가능한 문서가 없으면 빈 목록을 반환한다") | ||
| void findReadableDocumentIds_noAccessible_returnsEmptyList() { | ||
| given(documentRepository.findReadableDocumentIds(USER_ID)).willReturn(List.of()); | ||
|
|
||
| List<Long> result = accessibleDocumentQueryService.findReadableDocumentIds(USER_ID, null); | ||
|
|
||
| assertThat(result).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| @DisplayName("컬렉션 범위에서 접근 가능한 문서가 없으면 빈 목록을 반환한다") | ||
| void findReadableDocumentIds_noAccessibleInCollection_returnsEmptyList() { | ||
| given(documentRepository.findReadableDocumentIdsInCollection(USER_ID, COLLECTION_ID)).willReturn(List.of()); | ||
|
|
||
| List<Long> result = accessibleDocumentQueryService.findReadableDocumentIds(USER_ID, COLLECTION_ID); | ||
|
|
||
| assertThat(result).isEmpty(); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
네이티브 권한 SQL을 검증하는 저장소 통합 테스트를 추가하세요.
현재 테스트는 DocumentRepository를 mock 처리해 라우팅만 확인합니다. 따라서 OWNER, PUBLIC, USER 캐시의 만료·무효화, 문서/컬렉션 ROLE·DEPARTMENT 권한, 무권한 및 컬렉션 교집합이 실제 SQL에서 올바르게 필터링되는지 검증하지 못합니다. @DataJpaTest로 각 권한 경로와 빈 결과를 실행 검증하세요.
As per path instructions, "src/test/**/*.java: 테스트 커버리지, 스프링 테스트 어노테이션, mock 사용법, 네이밍 규칙을 확인한다."
🤖 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/opensource/docgrid/domain/search/service/query/AccessibleDocumentQueryServiceTest.java`
around lines 32 - 76, Replace the mock-based tests in
AccessibleDocumentQueryServiceTest with a `@DataJpaTest` integration test setup
that executes the native DocumentRepository queries against an actual test
database. Add fixtures and assertions covering OWNER, PUBLIC, and USER cache
expiration/invalidation, document and collection ROLE/DEPARTMENT permissions,
unauthorized documents, and collection-intersection filtering, including
empty-result cases; retain clear scenario-based test names and verify both
global and collection query methods.
Source: Path instructions
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🔍️ 작업 내용
Closes #54
✨ 상세 설명
배경
벡터 검색 실행 전에 사용자가 읽을 수 있는 문서 ID 목록을 미리 확보해, 검색 결과에 접근 불가 문서가 노출되지 않도록 한다.
변경 내용
DocumentRepository— 7-branch UNION 네이티브 쿼리 2종 추가5가지 접근 경로(OWNER / PUBLIC / USER캐시 / ROLE live / DEPT live)를 UNION으로 합산해 한 번의 쿼리로 문서 ID를 반환한다.
findReadableDocumentIds(userId)findReadableDocumentIdsInCollection(userId, collectionId)collection_documents로 교집합AccessibleDocumentQueryService(신규)collectionIdnull 여부로 두 쿼리를 분기하는 단일 진입점. 빈 목록은 그대로 반환해 호출 측(Issue 5)에서 벡터 검색을 skip하도록 한다.🛠️ 추후 리팩토링 및 고도화 계획
AccessibleDocumentQueryService를 SearchFacade에 연결 예정status = 'INDEXED'+deleted_at IS NULL필터로 범위 제한)📸 스크린샷 (선택)
해당 없음
💬 리뷰 요구사항
collection_documents를 조인하는 방식 대비 적절한지Summary by CodeRabbit
새로운 기능
문서화