[Feat] 권한 부여·회수 API + USER 캐시 갱신 - #19
Merged
Merged
Conversation
- CollectionPermissionRepository, DocumentPermissionRepository, UserDocumentAccessCacheRepository 추가
- UserDocumentAccessCacheService: USER 권한 grant 시 upsert, revoke 시 invalidate
- CollectionPermissionCommandService: POST/DELETE /permissions/collections/{id}
- DocumentPermissionCommandService: POST/DELETE /permissions/documents/{id}
- PermissionController, PermissionConverter, GrantPermissionRequest, PermissionResponse DTO 추가
- UserDocumentAccessCache 엔티티에 grant(), invalidate() 메서드 추가
- CollectionDocumentRepository에 findAllByCollectionId() 추가
- ROLE/DEPARTMENT 권한은 캐시 미적용, USER 권한만 캐시 갱신
- ErrorCode: PERMISSION_DENIED 메시지 범용화, INVALID_TARGET_TYPE/COLLECTION_PERMISSION_NOT_FOUND/DOCUMENT_PERMISSION_NOT_FOUND 추가
- DTO 클래스 레벨 @Schema 추가
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rvice 단위 테스트 추가 - PermissionFixture: Role, Department, CollectionPermission, DocumentPermission 팩토리 메서드 - CollectionPermissionCommandServiceTest: USER 권한 부여(캐시 갱신), ROLE 권한 부여(캐시 없음), COLLECTION_NOT_FOUND, PERMISSION_DENIED, INVALID_TARGET_TYPE, 권한 회수(캐시 무효화), COLLECTION_PERMISSION_NOT_FOUND - DocumentPermissionCommandServiceTest: USER/ROLE 권한 부여, DOCUMENT_NOT_FOUND, PERMISSION_DENIED, INVALID_TARGET_TYPE, 권한 회수(캐시 무효화), DOCUMENT_PERMISSION_NOT_FOUND Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough컬렉션·문서 권한 부여 및 회수 API가 추가되었습니다. 사용자 대상 권한은 문서 접근 캐시에 반영·무효화되며, 요청 검증·응답 변환·오류 코드와 서비스 테스트가 함께 구현되었습니다. Changes권한 기능
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant PermissionController
participant PermissionCommandService
participant UserDocumentAccessCacheService
Client->>PermissionController: 권한 부여 또는 회수 요청
PermissionController->>PermissionCommandService: 권한 명령 호출
PermissionCommandService->>UserDocumentAccessCacheService: USER 캐시 갱신 또는 무효화
PermissionCommandService-->>PermissionController: 권한 결과 반환
PermissionController-->>Client: 201 또는 204 응답
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/opensource/docgrid/domain/permission/service/command/DocumentPermissionCommandService.java (1)
70-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winboolean 배열 대신 레코드(Record)나 명시적 객체를 사용해 가독성과 타입 안정성을 높이는 것을 권장합니다.
resolvePermissions가boolean[]을 반환하고 이를 인덱스(permissions[0],permissions[1])로 접근하는 방식은 클린 코드 관점에서 실수를 유발하기 쉽고 가독성이 떨어집니다. 반환 타입을 명시적인 Record로 변경하면 각 플래그의 의미가 명확해집니다.♻️ 레코드를 활용한 리팩토링 제안
- boolean[] permissions = resolvePermissions(request.permissionType()); + PermissionFlags flags = resolvePermissions(request.permissionType()); User grantor = userRepository.getReferenceById(grantorId); DocumentPermission permission = DocumentPermission.builder() .document(document) .targetType(request.targetType()) .user(targetUser) .role(targetRole) .department(targetDepartment) .permissionType(request.permissionType()) - .canRead(permissions[0]) - .canWrite(permissions[1]) - .canAdmin(permissions[2]) + .canRead(flags.canRead()) + .canWrite(flags.canWrite()) + .canAdmin(flags.canAdmin()) .grantedBy(grantor) .grantedAt(LocalDateTime.now()) .expiresAt(request.expiresAt()) .build();아래 캐시 서비스 호출과 메서드 선언부도 다음과 같이 변경합니다:
if (request.targetType() == PermissionTargetType.USER) { cacheService.grantUserPermission(targetUser, document, - permissions[0], permissions[1], permissions[2], + flags.canRead(), flags.canWrite(), flags.canAdmin(), AccessSourceType.DIRECT_DOCUMENT_PERMISSION, permission.getId(), request.expiresAt()); }+ private record PermissionFlags(boolean canRead, boolean canWrite, boolean canAdmin) {} + - private boolean[] resolvePermissions(PermissionType type) { + private PermissionFlags resolvePermissions(PermissionType type) { return switch (type) { - case READ -> new boolean[]{true, false, false}; - case WRITE -> new boolean[]{true, true, false}; - case ADMIN -> new boolean[]{true, true, true}; + case READ -> new PermissionFlags(true, false, false); + case WRITE -> new PermissionFlags(true, true, false); + case ADMIN -> new PermissionFlags(true, true, true); }; }Also applies to: 134-140
🤖 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/permission/service/command/DocumentPermissionCommandService.java` around lines 70 - 82, Replace the boolean[] contract returned by resolvePermissions with an explicit record or value object whose fields represent read, write, and admin permissions; update all callers, including the cache-service path, to use named accessors instead of permissions[0], permissions[1], and permissions[2].src/main/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandService.java (1)
141-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value불리언 배열 대신 레코드(Record) 반환 (가독성 향상)
boolean[]배열을 반환하여 인덱스([0],[1],[2])로 권한을 가져오는 방식은 매직 넘버를 유발하며 각 인덱스의 의미를 한눈에 파악하기 어렵게 만듭니다.의미가 명확하게 드러나도록 내부 레코드(Record)나 DTO를 정의하여 반환하는 방식을 권장합니다.
💡 레코드 적용 예시
private record ResolvedPermission(boolean canRead, boolean canWrite, boolean canAdmin) {} private ResolvedPermission resolvePermissions(PermissionType type) { return switch (type) { case READ -> new ResolvedPermission(true, false, false); case WRITE -> new ResolvedPermission(true, true, false); case ADMIN -> new ResolvedPermission(true, true, true); }; }호출부에서는
permissions.canRead()와 같이 직관적으로 속성에 접근할 수 있습니다.🤖 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/permission/service/command/CollectionPermissionCommandService.java` around lines 141 - 147, Update resolvePermissions to return a named internal record such as ResolvedPermission with canRead, canWrite, and canAdmin components instead of boolean[]. Adjust its callers to use the record accessors rather than positional array indexes, preserving the existing permission values for READ, WRITE, and ADMIN.
🤖 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/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandService.java`:
- Around line 152-157: Replace the per-document grantUserPermission loop in
CollectionPermissionCommandService with a bulk operation on
UserDocumentAccessCacheService that accepts the collection’s Document list and
shared permission metadata, performs a single IN-based lookup, and batch-updates
existing entries while inserting missing entries via saveAll.
- Around line 118-123: Replace the per-document lookup and loop in the
collection-permission revocation flow with a bulk invalidation operation on
UserDocumentAccessCacheRepository. Add or reuse a single `@Modifying` query that
updates invalidatedAt for records matching
AccessSourceType.DIRECT_COLLECTION_PERMISSION and permissionId, then invoke it
from CollectionPermissionCommandService without calling findAllByCollectionId or
cacheService.revokeUserPermission for each document.
---
Nitpick comments:
In
`@src/main/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandService.java`:
- Around line 141-147: Update resolvePermissions to return a named internal
record such as ResolvedPermission with canRead, canWrite, and canAdmin
components instead of boolean[]. Adjust its callers to use the record accessors
rather than positional array indexes, preserving the existing permission values
for READ, WRITE, and ADMIN.
In
`@src/main/java/com/opensource/docgrid/domain/permission/service/command/DocumentPermissionCommandService.java`:
- Around line 70-82: Replace the boolean[] contract returned by
resolvePermissions with an explicit record or value object whose fields
represent read, write, and admin permissions; update all callers, including the
cache-service path, to use named accessors instead of permissions[0],
permissions[1], and permissions[2].
🪄 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: 953d6d48-9821-40e8-a401-9e282502c704
📒 Files selected for processing (21)
src/main/java/com/opensource/docgrid/domain/collection/dto/request/AddDocumentRequest.javasrc/main/java/com/opensource/docgrid/domain/collection/dto/request/CreateCollectionRequest.javasrc/main/java/com/opensource/docgrid/domain/collection/dto/response/CollectionDocumentResponse.javasrc/main/java/com/opensource/docgrid/domain/collection/dto/response/CollectionResponse.javasrc/main/java/com/opensource/docgrid/domain/collection/repository/CollectionDocumentRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/controller/PermissionController.javasrc/main/java/com/opensource/docgrid/domain/permission/converter/PermissionConverter.javasrc/main/java/com/opensource/docgrid/domain/permission/dto/request/GrantPermissionRequest.javasrc/main/java/com/opensource/docgrid/domain/permission/dto/response/CollectionPermissionResponse.javasrc/main/java/com/opensource/docgrid/domain/permission/dto/response/DocumentPermissionResponse.javasrc/main/java/com/opensource/docgrid/domain/permission/entity/UserDocumentAccessCache.javasrc/main/java/com/opensource/docgrid/domain/permission/repository/CollectionPermissionRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/repository/DocumentPermissionRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/repository/UserDocumentAccessCacheRepository.javasrc/main/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandService.javasrc/main/java/com/opensource/docgrid/domain/permission/service/command/DocumentPermissionCommandService.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/permission/fixture/PermissionFixture.javasrc/test/java/com/opensource/docgrid/domain/permission/service/command/CollectionPermissionCommandServiceTest.javasrc/test/java/com/opensource/docgrid/domain/permission/service/command/DocumentPermissionCommandServiceTest.java
Comment on lines
+118
to
+123
| List<CollectionDocument> docs = collectionDocumentRepository.findAllByCollectionId(collectionId); | ||
| for (CollectionDocument cd : docs) { | ||
| cacheService.revokeUserPermission(targetUserId, cd.getDocument().getId(), | ||
| AccessSourceType.DIRECT_COLLECTION_PERMISSION, permissionId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
단일 쿼리로 캐시 무효화 (불필요한 조회 및 N+1 문제 개선)
컬렉션 권한을 회수할 때 컬렉션 내의 전체 문서를 조회하고 반복문을 돌며 개별적으로 캐시를 무효화하면 대량의 SELECT/UPDATE 쿼리가 발생(N+1 문제)하여 성능이 크게 저하됩니다.
sourceType과 sourceId(컬렉션 권한 ID)를 활용하면 해당 컬렉션 권한을 통해 생성된 모든 캐시 레코드를 고유하게 식별할 수 있습니다. 문서 목록을 DB에서 모두 조회할 필요 없이, UserDocumentAccessCacheRepository에 단일 @Modifying UPDATE 쿼리를 추가하여 일괄 무효화하는 방식으로 개선을 적극 권장합니다.
💡 단일 쿼리 무효화 리팩토링 예시
UserDocumentAccessCacheRepository.java 쿼리 추가:
`@Modifying`
`@Query`("UPDATE UserDocumentAccessCache c SET c.invalidatedAt = CURRENT_TIMESTAMP WHERE c.sourceType = :sourceType AND c.sourceId = :sourceId")
void invalidateBySource(AccessSourceType sourceType, Long sourceId);CollectionPermissionCommandService.java 적용:
- Long targetUserId = permission.getUser().getId();
- List<CollectionDocument> docs = collectionDocumentRepository.findAllByCollectionId(collectionId);
- for (CollectionDocument cd : docs) {
- cacheService.revokeUserPermission(targetUserId, cd.getDocument().getId(),
- AccessSourceType.DIRECT_COLLECTION_PERMISSION, permissionId);
- }
+ cacheService.revokeUserPermissionBySource(AccessSourceType.DIRECT_COLLECTION_PERMISSION, permissionId);🤖 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/permission/service/command/CollectionPermissionCommandService.java`
around lines 118 - 123, Replace the per-document lookup and loop in the
collection-permission revocation flow with a bulk invalidation operation on
UserDocumentAccessCacheRepository. Add or reuse a single `@Modifying` query that
updates invalidatedAt for records matching
AccessSourceType.DIRECT_COLLECTION_PERMISSION and permissionId, then invoke it
from CollectionPermissionCommandService without calling findAllByCollectionId or
cacheService.revokeUserPermission for each document.
Comment on lines
+152
to
+157
| List<CollectionDocument> docs = collectionDocumentRepository.findAllByCollectionId(collectionId); | ||
| for (CollectionDocument cd : docs) { | ||
| cacheService.grantUserPermission(targetUser, cd.getDocument(), | ||
| permissions[0], permissions[1], permissions[2], | ||
| AccessSourceType.DIRECT_COLLECTION_PERMISSION, sourceId, expiresAt); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
일괄 처리를 통한 캐시 갱신 (N+1 문제 개선)
컬렉션 권한 부여 시 반복문을 돌며 grantUserPermission을 호출하면 컬렉션 내의 문서 개수만큼 개별적인 SELECT와 INSERT/UPDATE가 발생하게 됩니다.
문서가 많은 컬렉션의 경우 심각한 DB I/O 병목이 발생할 수 있습니다. UserDocumentAccessCacheService에 대상 Document 리스트를 넘겨받아 일괄 처리(Bulk Update/Insert)를 수행하는 전용 메서드를 추가하여 사용하는 것을 권장합니다. (예: 기존 캐시는 IN 절로 한 번에 조회하여 상태를 갱신하고, 존재하지 않는 항목은 saveAll을 통해 배치로 삽입)
🤖 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/permission/service/command/CollectionPermissionCommandService.java`
around lines 152 - 157, Replace the per-document grantUserPermission loop in
CollectionPermissionCommandService with a bulk operation on
UserDocumentAccessCacheService that accepts the collection’s Document list and
shared permission metadata, performs a single IN-based lookup, and batch-updates
existing entries while inserting missing entries via saveAll.
This was referenced Jul 16, 2026
Merged
Merged
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
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.
🔍 작업 내용
✨ 상세 설명
구현 내용
POST/DELETE /permissions/collections/{collectionId}— 컬렉션 권한 부여·회수POST/DELETE /permissions/documents/{documentId}— 문서 예외 권한 부여·회수user_document_access_cache즉시 갱신invalidated_at설정)권한 판단 구조
targetType(USER/ROLE/DEPARTMENT)과 ID 필드 조합 유효성 검사 (INVALID_TARGET_TYPE)canAdminDocument()완성 후 대체 예정추가된 ErrorCode
INVALID_TARGET_TYPE,COLLECTION_PERMISSION_NOT_FOUND,DOCUMENT_PERMISSION_NOT_FOUNDPERMISSION_DENIED메시지 범용화 ("관리자만" → "접근 권한이 없습니다")🛠️ 추후 리팩토링 및 고도화 계획
PermissionQueryService완성 후 owner 직접 비교 →canAdminDocument()교체💬 리뷰 요구사항
UserDocumentAccessCacheServicegrant/revoke 흐름 (upsert 방식) 확인 부탁Summary by CodeRabbit