[Feat] 컬렉션 관리 API 구현 - #31
Merged
Merged
Conversation
…TE /collections/{id}/documents/{documentId})
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: 53 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 (2)
📝 WalkthroughWalkthrough컬렉션 목록 조회, 컬렉션 soft delete, 컬렉션 내 문서 제거 API가 추가되었다. 삭제 작업은 소유자 검증과 USER 권한 캐시 무효화를 수행하며, 관련 리포지토리·오류 코드·단위 테스트와 설계 문서가 함께 보강되었다. Changes컬렉션 관리 기능
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CollectionController
participant CollectionCommandService
participant UserDocumentAccessCacheService
participant CollectionDocumentRepository
Client->>CollectionController: DELETE /collections/{collectionId}/documents/{documentId}
CollectionController->>CollectionCommandService: removeDocument(collectionId, documentId, userId)
CollectionCommandService->>UserDocumentAccessCacheService: bulkRevokeBySourcesForDocument(...)
UserDocumentAccessCacheService-->>CollectionCommandService: cache invalidated
CollectionCommandService->>CollectionDocumentRepository: delete(collectionDocument)
CollectionController-->>Client: 204 No Content
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/test/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandServiceTest.java (2)
301-303: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win구체적인 Mock 인자 검증을 통해 테스트 정확도를 높이세요.
bulkRevokeBySourcesForDocument호출 시any()를 사용하고 있어 의도하지 않은 파라미터가 전달되어도 테스트가 통과할 위험이 있습니다.Mock 사용법 지침에 따라 실제 사용되는 값을 명시하여 검증을 강화하는 것을 제안합니다.
💡 제안하는 수정안
- then(cacheService).should().bulkRevokeBySourcesForDocument(any(), any(), any()); - then(collectionDocumentRepository).should().delete(cd); + then(cacheService).should().bulkRevokeBySourcesForDocument(eq(AccessSourceType.DIRECT_COLLECTION_PERMISSION), eq(List.of(userPermission.getId())), eq(document.getId())); + then(collectionDocumentRepository).should().delete(cd);🤖 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/collection/service/command/CollectionCommandServiceTest.java` around lines 301 - 303, Update the verification for cacheService.bulkRevokeBySourcesForDocument in CollectionCommandServiceTest to assert the exact arguments used by the test scenario instead of any() matchers. Reuse the concrete source, document, and remaining parameter values established in the test setup, while leaving the collectionDocumentRepository.delete(cd) verification unchanged.Source: Path instructions
236-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSoft delete 상태 검증 및 구체적인 Mock 인자 검증을 추가하세요.
@DisplayName에는 "soft delete 처리되고"라고 명시되어 있으나, 실제 테스트 코드에는collection객체가 soft delete 상태(deletedAt,status등)로 변경되었는지 검증하는 Assertion이 누락되어 있습니다. 또한, Mock 객체의 행위 검증 시any()대신 구체적인 값을 사용하면 테스트의 신뢰도를 높일 수 있습니다.테스트 커버리지와 mock 사용법 규칙을 준수하기 위해 아래와 같이 개선할 것을 제안합니다.
💡 제안하는 수정안
`@Test` `@DisplayName`("소유자가 컬렉션을 삭제하면 soft delete 처리되고 권한 및 캐시가 무효화된다") void deleteCollection_succeeds_when_owner() { User owner = CollectionFixture.createOwner(); DocumentCollection collection = CollectionFixture.createCollection(owner); CollectionPermission userPermission = PermissionFixture.createCollectionPermission(collection, owner); given(collectionRepository.findById(CollectionFixture.COLLECTION_ID)).willReturn(Optional.of(collection)); given(collectionPermissionRepository.findAllByCollectionId(CollectionFixture.COLLECTION_ID)) .willReturn(List.of(userPermission)); collectionCommandService.deleteCollection(CollectionFixture.COLLECTION_ID, CollectionFixture.USER_ID); - then(cacheService).should().bulkRevokeBySource(any(), any()); - then(collectionPermissionRepository).should().deleteAll(any()); + then(cacheService).should().bulkRevokeBySource(eq(AccessSourceType.DIRECT_COLLECTION_PERMISSION), eq(userPermission.getId())); + then(collectionPermissionRepository).should().deleteAll(List.of(userPermission)); + + // Soft delete 상태 검증 (실제 도메인 객체의 메서드명에 맞게 조정) + assertThat(collection.getDeletedAt()).isNotNull(); }🤖 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/collection/service/command/CollectionCommandServiceTest.java` around lines 236 - 251, Update deleteCollection_succeeds_when_owner to assert that collection is soft-deleted by verifying its deletedAt/status state after the service call. Replace broad any() arguments in cacheService.bulkRevokeBySource and collectionPermissionRepository.deleteAll verifications with the specific collection and permission values expected from this scenario.Source: Path instructions
src/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java (2)
113-117: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winN+1 쿼리 방지를 위한 성능 최적화 제안
deleteAll(permissions)는 내부적으로 각 엔티티마다 단건DELETE쿼리를 수행하여 N+1 문제를 유발할 수 있습니다. 성능 최적화를 위해deleteAllInBatch(permissions)사용을 권장합니다.또한,
cacheService.bulkRevokeBySource를 반복문 내에서 호출하면 권한 수만큼 업데이트 쿼리가 발생하므로, 향후 성능 확장을 위해 다건의sourceId를 한 번에 무효화하는 벌크 처리 메서드 도입을 고려해 보는 것도 좋습니다.💡 제안하는 수정안
List<CollectionPermission> permissions = collectionPermissionRepository.findAllByCollectionId(collectionId); permissions.stream() .filter(p -> p.getTargetType() == PermissionTargetType.USER) .forEach(p -> cacheService.bulkRevokeBySource(AccessSourceType.DIRECT_COLLECTION_PERMISSION, p.getId())); - collectionPermissionRepository.deleteAll(permissions); + collectionPermissionRepository.deleteAllInBatch(permissions);🤖 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/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java` around lines 113 - 117, Update the permission deletion flow in CollectionCommandService to use collectionPermissionRepository.deleteAllInBatch(permissions) instead of deleteAll(permissions). Preserve the existing filtering and cache invalidation behavior; the bulk cache API is only a future consideration and does not need to be introduced here.
135-139: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win메모리 필터링 대신 DB 레벨 필터링 제안
현재 컬렉션에 속한 모든 권한 엔티티를 영속성 컨텍스트로 불러온 뒤 메모리에서
USER타입만 필터링하여 ID를 추출하고 있습니다. 컬렉션에 연결된 부서나 역할 권한 데이터가 많아질 경우 불필요한 메모리 및 I/O 오버헤드가 발생할 수 있습니다.성능 향상을 위해 Repository 단에서 특정 타입의 권한 ID만 직접 조회하는 전용 쿼리를 추가하는 것을 권장합니다.
💡 제안하는 접근 방식
CollectionPermissionRepository에 기존 방식처럼 정규화된 Enum 패키지명을 활용한 쿼리를 추가할 수 있습니다:`@Query`(""" SELECT cp.id FROM CollectionPermission cp WHERE cp.collection.id = :collectionId AND cp.targetType = com.opensource.docgrid.domain.permission.enums.PermissionTargetType.USER """) List<Long> findUserPermissionIdsByCollectionId(`@Param`("collectionId") Long collectionId);이후
CollectionCommandService의 호출부를 다음과 같이 간결하게 수정할 수 있습니다:- List<Long> userPermissionIds = collectionPermissionRepository.findAllByCollectionId(collectionId) - .stream() - .filter(p -> p.getTargetType() == PermissionTargetType.USER) - .map(CollectionPermission::getId) - .toList(); + List<Long> userPermissionIds = collectionPermissionRepository.findUserPermissionIdsByCollectionId(collectionId);🤖 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/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java` around lines 135 - 139, Replace the in-memory filtering in CollectionCommandService with a repository-level query that directly returns USER-target permission IDs. Add a dedicated method such as findUserPermissionIdsByCollectionId to CollectionPermissionRepository, filtering by collectionId and PermissionTargetType.USER, then call it from the existing command-service flow.
🤖 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 `@CLAUDE.md`:
- Around line 74-78: Update the filename examples in the docs/ section of
CLAUDE.md to use the {github아이디}-#{이슈번호}-{설명}.md convention for both PR design
documents and test-results documents, matching .claude/rules/deploy.md and the
existing documentation naming pattern.
In `@docs/chelung-`#24-document-permission-check.md:
- Around line 204-212: 완료 기준의 PUBLIC 문서 권한 항목에서 잘못 표기된 `PUBLILC`를 API 응답과 일치하는
`PUBLIC`으로 수정하세요.
---
Nitpick comments:
In
`@src/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.java`:
- Around line 113-117: Update the permission deletion flow in
CollectionCommandService to use
collectionPermissionRepository.deleteAllInBatch(permissions) instead of
deleteAll(permissions). Preserve the existing filtering and cache invalidation
behavior; the bulk cache API is only a future consideration and does not need to
be introduced here.
- Around line 135-139: Replace the in-memory filtering in
CollectionCommandService with a repository-level query that directly returns
USER-target permission IDs. Add a dedicated method such as
findUserPermissionIdsByCollectionId to CollectionPermissionRepository, filtering
by collectionId and PermissionTargetType.USER, then call it from the existing
command-service flow.
In
`@src/test/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandServiceTest.java`:
- Around line 301-303: Update the verification for
cacheService.bulkRevokeBySourcesForDocument in CollectionCommandServiceTest to
assert the exact arguments used by the test scenario instead of any() matchers.
Reuse the concrete source, document, and remaining parameter values established
in the test setup, while leaving the collectionDocumentRepository.delete(cd)
verification unchanged.
- Around line 236-251: Update deleteCollection_succeeds_when_owner to assert
that collection is soft-deleted by verifying its deletedAt/status state after
the service call. Replace broad any() arguments in
cacheService.bulkRevokeBySource and collectionPermissionRepository.deleteAll
verifications with the specific collection and permission values expected from
this scenario.
🪄 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: 4f22cbd7-87a6-4a77-9fc3-820c154e25b9
📒 Files selected for processing (17)
.claude/rules/deploy.mdCLAUDE.mddocs/chelung-#16-collection-crud.mddocs/chelung-#18-permission-grant-revoke.mddocs/chelung-#21-permission-query-service.mddocs/chelung-#24-document-permission-check.mddocs/chelung-#29-collection-management.mdsrc/main/java/com/opensource/docgrid/domain/collection/controller/CollectionController.javasrc/main/java/com/opensource/docgrid/domain/collection/repository/CollectionDocumentRepository.javasrc/main/java/com/opensource/docgrid/domain/collection/repository/CollectionRepository.javasrc/main/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandService.javasrc/main/java/com/opensource/docgrid/domain/collection/service/query/CollectionQueryService.javasrc/main/java/com/opensource/docgrid/domain/permission/repository/CollectionPermissionRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/repository/UserDocumentAccessCacheRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/service/command/UserDocumentAccessCacheService.javasrc/main/java/com/opensource/docgrid/global/exception/ErrorCode.javasrc/test/java/com/opensource/docgrid/domain/collection/service/command/CollectionCommandServiceTest.java
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
🔍 작업 내용
✨ 상세 설명
컬렉션 목록 조회, 컬렉션 삭제(soft delete), 컬렉션에서 문서 제거 API를 구현했습니다.
GET /collections— 로그인한 사용자가 소유한 ACTIVE 상태 컬렉션 목록 반환DELETE /collections/{collectionId}— 소유자만 가능, soft delete + 관련 권한 및 USER 캐시 일괄 무효화DELETE /collections/{collectionId}/documents/{documentId}— 소유자만 가능, 해당 문서에 대한 캐시만 선택적으로 무효화컬렉션 삭제와 문서 제거 시 캐시 무효화 범위를 다르게 처리했습니다.
설계 문서:
docs/chelung-#29-collection-management.md🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
bulkRevokeBySourcesForDocument)로 분리한 구조가 괜찮은지 봐주세요.Summary by CodeRabbit
새 기능
문서화
버그 수정