Feature/#25 - Project CRUD ORM과 Secret-Project 링크 메서드 구현#27
Conversation
|
Warning Review limit reached
Next review available in: 14 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)
WalkthroughProject 도메인 엔티티, 저장소 인터페이스/구현체(SwiftData), 유스케이스(CRUD)를 신규 도입하고, Secret의 secretType/subType을 문자열에서 열거형(SecretType/SecretSubType)으로 전환했습니다. Secret-Project 연결/해제 유스케이스와 데모 UI, 앱 와이어링이 함께 추가·수정되었습니다. ChangesProject CRUD 및 Secret-Project 연결
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DemoView as ProjectSecretRelationDemoView
participant CreateSecretUC as CreateSecretUseCaseImpl
participant SecretRepo as SecretRepositoryImpl
participant ProjectRepo as ProjectRepositoryImpl
DemoView->>CreateSecretUC: execute(draft, payload, projectIDs)
CreateSecretUC->>SecretRepo: create(secret)
loop projectIDs
CreateSecretUC->>SecretRepo: linkProject(secretID, projectID)
SecretRepo->>ProjectRepo: fetch(id) 존재 확인
end
alt 연결 실패
CreateSecretUC->>SecretRepo: delete(id) 롤백
else 성공
CreateSecretUC-->>DemoView: 생성된 Secret 반환
end
Possibly related PRs
Suggested reviewers:
🚥 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: 5
🧹 Nitpick comments (7)
Projects/DVPresentation/Sources/ProjectSecretRelationDemoView.swift (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win임포트 순서 수정 필요
내장 프레임워크(
SwiftUI)가 먼저, 서드파티/내부 모듈(DVDomain)이 빈 줄로 구분되어 뒤에 와야 합니다. 현재는 순서가 반대이고 구분 줄도 없습니다.🔧 제안
-import DVDomain -import SwiftUI +import SwiftUI + +import DVDomainAs per path instructions, "모듈 임포트가 알파벳 순으로 정렬되어 있는지 확인하세요. (내장 프레임워크 먼저, 빈 줄로 서드파티 구분)".
🤖 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 `@Projects/DVPresentation/Sources/ProjectSecretRelationDemoView.swift` around lines 3 - 4, Adjust the import ordering in ProjectSecretRelationDemoView so SwiftUI comes first as the built-in framework, then add a blank line before the internal module import DVDomain. Keep the imports alphabetically ordered within their groups and preserve the framework-first, separated-by-empty-line convention.Source: Path instructions
Projects/Devault/Sources/ContentView.swift (1)
17-38: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
body에서 UseCase를 매번 새로 생성 — init으로 이동 권장
body는 SwiftUI에 의해 재평가될 수 있는 지점입니다. 5개의 UseCase 인스턴스를 여기서 매번 생성하지 말고,init에서 한 번만 구성해private let으로 보관하는 편이 할당 낭비를 줄이고 의존성 구성 위치도 명확해집니다.🔧 제안
struct ContentView: View { private let secretRepository: SecretRepositoryImpl private let projectRepository: ProjectRepositoryImpl private let cryptoService = SecretCryptoServiceImpl() private let authenticationService = LocalUserAuthenticationServiceImpl() + private let createProjectUseCase: any CreateProjectUseCase + private let fetchProjectUseCase: any FetchProjectUseCase + private let createSecretUseCase: any CreateSecretUseCase + private let fetchSecretUseCase: any FetchSecretUseCase + private let secretProjectRelationUseCase: any SecretProjectRelationUseCase init(storage: LocalStorage) { self.secretRepository = SecretRepositoryImpl(modelContainer: storage.modelContainer) self.projectRepository = ProjectRepositoryImpl(modelContainer: storage.modelContainer) + self.createProjectUseCase = CreateProjectUseCaseImpl(repository: projectRepository) + self.fetchProjectUseCase = FetchProjectUseCaseImpl(repository: projectRepository) + self.createSecretUseCase = CreateSecretUseCaseImpl(repository: secretRepository, cryptoService: cryptoService) + self.fetchSecretUseCase = FetchSecretUseCaseImpl(repository: secretRepository, cryptoService: cryptoService, authenticationService: authenticationService) + self.secretProjectRelationUseCase = SecretProjectRelationUseCaseImpl(repository: secretRepository) } var body: some View { ProjectSecretRelationDemoView( - createProjectUseCase: CreateProjectUseCaseImpl(repository: projectRepository), - fetchProjectUseCase: FetchProjectUseCaseImpl(repository: projectRepository), - createSecretUseCase: CreateSecretUseCaseImpl(repository: secretRepository, cryptoService: cryptoService), - fetchSecretUseCase: FetchSecretUseCaseImpl(repository: secretRepository, cryptoService: cryptoService, authenticationService: authenticationService), - secretProjectRelationUseCase: SecretProjectRelationUseCaseImpl(repository: secretRepository) + createProjectUseCase: createProjectUseCase, + fetchProjectUseCase: fetchProjectUseCase, + createSecretUseCase: createSecretUseCase, + fetchSecretUseCase: fetchSecretUseCase, + secretProjectRelationUseCase: secretProjectRelationUseCase ) } }🤖 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 `@Projects/Devault/Sources/ContentView.swift` around lines 17 - 38, body에서 CreateProjectUseCaseImpl, FetchProjectUseCaseImpl, CreateSecretUseCaseImpl, FetchSecretUseCaseImpl, SecretProjectRelationUseCaseImpl를 매번 새로 만드는 구조를 제거하고, ContentView의 init에서 한 번만 생성해 private let 프로퍼티로 보관하도록 변경하세요. 그런 다음 body에서는 ProjectSecretRelationDemoView에 미리 준비된 use case들을 전달만 하게 바꿔서, SwiftUI 재평가 시 불필요한 할당이 반복되지 않도록 정리하세요.Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift (1)
29-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value두
update오버로드 로직 중복 — 공통 헬퍼 추출 권장metadata 유무만 다를 뿐 payload 암호화 → patch 세팅 →
repository.patch→reconcileProjects흐름이 거의 동일합니다. 공통 부분을 private 헬퍼로 추출하면 향후 흐름 변경(예: 에러 매핑, 롤백 로직 추가) 시 한 곳만 수정하면 됩니다.🤖 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 `@Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift` around lines 29 - 68, The two `update` overloads in `PatchSecretUseCaseImpl` duplicate the same flow for encrypting payload, building `fullPatch`, calling `repository.patch`, and then `reconcileProjects`. Extract the shared logic into a private helper method and have both overloads delegate to it, keeping only the metadata-specific encoding in the `update(id:patch:payload:metadata:projectIDs:)` path. This will centralize future changes in `PatchSecretUseCaseImpl` and reduce the risk of the two code paths drifting apart.Projects/DVDomain/Sources/Entity/SecretType.swift (1)
3-42: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winrawValue를 case 이름에 암묵적으로 의존하지 마세요.
SecretType/SecretSubType의 rawValue가 별도 지정 없이 case 이름 그대로 사용되고 있는데, 이 값이SwiftDataModel.Secret.secretType/subType에 문자열로 직접 영속화됩니다. 추후 case 이름을 리팩터링(오타 수정, 네이밍 개선 등)하면 rawValue도 함께 바뀌어 기존 저장 데이터의 디코딩이 깨질 수 있습니다. 저장되는 값은 case 이름과 별개로 명시적으로 고정해 두는 게 안전합니다.🛡️ 제안
public enum SecretType: String, Codable, CaseIterable, Sendable { - case apiKeyToken - case oauth - case database - case sshAndCredentials - case environmentVariableSet - case etc + case apiKeyToken = "apiKeyToken" + case oauth = "oauth" + case database = "database" + case sshAndCredentials = "sshAndCredentials" + case environmentVariableSet = "environmentVariableSet" + case etc = "etc"🤖 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 `@Projects/DVDomain/Sources/Entity/SecretType.swift` around lines 3 - 42, The rawValue of SecretType and SecretSubType is currently inferred from case names, which makes SwiftDataModel.Secret.secretType and subType persistence fragile. Update the SecretType and SecretSubType enums to use explicit, stable raw string values instead of relying on implicit case-name defaults. Keep the existing mapping behavior in availableSubTypes and secretType unchanged, but ensure the persisted strings remain fixed even if case names are later renamed.Projects/DVDomain/Sources/UseCase/Impl/Project/FetchProjectUseCaseImpl.swift (1)
12-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
do/catch → map패턴 중복, 헬퍼로 추출 고려
fetch,fetchAll둘 다 동일한do { ... } catch { throw ProjectUseCaseError.map(error) }골격을 반복합니다.DeleteProjectUseCaseImpl에도 같은 패턴이 있어 Project UseCase 계층 전반에 걸쳐 반복될 가능성이 높습니다. 작은 제네릭 헬퍼로 묶으면 유지보수가 편해집니다.♻️ 제안: 공통 헬퍼 추출
private func mapped<T: Sendable>(_ operation: () async throws -> T) async throws -> T { do { return try await operation() } catch { throw ProjectUseCaseError.map(error) } } public func fetch(id: UUID) async throws -> Project? { try await mapped { try await repository.fetch(id: id) } } public func fetchAll() async throws -> [Project] { try await mapped { try await repository.fetchAll() } }🤖 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 `@Projects/DVDomain/Sources/UseCase/Impl/Project/FetchProjectUseCaseImpl.swift` around lines 12 - 26, Both FetchProjectUseCaseImpl.fetch(id:) and fetchAll() repeat the same do/catch mapping to ProjectUseCaseError.map(error), so extract that logic into a small private generic helper in FetchProjectUseCaseImpl and have both methods call it. Keep the helper focused on wrapping an async throwing operation and returning its result, while preserving the existing error mapping behavior for repository.fetch(id:) and repository.fetchAll().Projects/DVData/Sources/RepositoryImpl/Project/ProjectRepositoryImpl.swift (2)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value임포트 순서가 지침과 반대입니다.
DVDomain(서드파티 모듈)이Foundation/SwiftData(내장 프레임워크)보다 앞에 있습니다. 내장 프레임워크를 먼저, 빈 줄로 구분 후 서드파티를 배치해야 합니다.📐 제안 수정
-import DVDomain import Foundation import SwiftData + +import DVDomainAs per path instructions, "모듈 임포트가 알파벳 순으로 정렬되어 있는지 확인하세요. (내장 프레임워크 먼저, 빈 줄로 서드파티 구분)".
🤖 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 `@Projects/DVData/Sources/RepositoryImpl/Project/ProjectRepositoryImpl.swift` around lines 3 - 5, The import ordering in ProjectRepositoryImpl is reversed relative to the module import rule. Reorder the imports so Foundation and SwiftData appear first, then add a blank line, and place DVDomain after that; keep the import block alphabetized within each group and use the import list in ProjectRepositoryImpl as the reference point.Source: Path instructions
132-144: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff중복 이름 체크가 전체 테이블 스캔.
containsProjectName이 매create/patch호출마다 전체 Project를 fetch해서 메모리에서 소문자 비교합니다. SwiftData#Predicate가 대소문자 무관 비교를 기본 지원하지 않아 현재 방식이 실용적인 우회이긴 하나, Project 수가 늘어나면 매 생성/수정마다 O(n) 스캔 비용이 커집니다. 정규화된 이름을 별도 인덱싱 필드로 저장해두면 조회 비용을 줄일 수 있습니다.🤖 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 `@Projects/DVData/Sources/RepositoryImpl/Project/ProjectRepositoryImpl.swift` around lines 132 - 144, containsProjectName is doing a full Project fetch and in-memory duplicate-name scan on every create/patch, which will not scale. Update ProjectRepositoryImpl to persist a normalized name field on SwiftDataModel.Project and keep it in sync in the create/patch flow, then query by that indexed field instead of fetching all projects; keep the existing excludedID logic in containsProjectName while moving the lookup to a targeted fetch/predicate.
🤖 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 @.github/PULL_REQUEST_TEMPLATE.md:
- Around line 2-3: PR 제목 예시가 안내된 컨벤션과 형식이 서로 다릅니다.
.github/PULL_REQUEST_TEMPLATE.md의 PR 제목 예시 문구를 수정해, “타입/#이슈 번호 - 작업 요약” 형식이 실제
예시와 정확히 일치하도록 맞추세요. 특히 예시 문자열에서 Feature 제목의 구분 형식을 하이픈 포함 형태로 바꿔 설명과 동일하게 유지하세요.
In `@Projects/DVData/Sources/Storage/Local/Models/Secret.swift`:
- Around line 83-84: `Secret`의 복원 로직에서 잘못된 `rawValue`를
`DVDomain.SecretType(rawValue:) ?? .etc`와
`DVDomain.SecretSubType.init(rawValue:)`의 fallback으로 숨기지 말고,
`secretType`/`subType`가 enum으로 변환되지 않으면 저장소 손상으로 간주하도록 `Secret` 초기화 또는 매핑 경로를
수정하세요. `DVDomain.SecretType`과 `DVDomain.SecretSubType` 복원 실패 시에는 기본값을 넣지 말고 실패를
전파하거나 별도 오류를 발생시켜, 잘못된 데이터가 정상 도메인 값으로 유입되지 않게 하세요.
In `@Projects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swift`:
- Around line 87-95: The rollback in createAndLink(_:projectIDs:) is currently
swallowing delete failures with try?, which can leave the newly created Secret
orphaned after a linkProject failure. Update
CreateSecretUseCaseImpl.createAndLink to either make the create/link/delete
sequence atomic via a repository transaction or explicitly propagate rollback
failure by throwing a distinct error when repository.delete(id:) fails, instead
of ignoring it.
- Around line 90-91: `CreateSecretUseCaseImpl`의 `projectIDs`를 순회하기 전에 중복을 제거하도록
수정하세요. 현재 `for projectID in projectIDs`에서 동일한 Project ID가 두 번 들어오면
`repository.linkProject(secretID:projectID:)`가 중복 링크 에러를 내고 전체 생성이 롤백될 수 있으니, 링크
전에 중복 없는 ID 목록으로 정리한 뒤 그 목록만 사용하도록 `create` 흐름을 바꾸세요.
In `@Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift`:
- Around line 72-82: `reconcileProjects(secretID:desiredIDs:)` can leave Secret
metadata updated but project links only partially reconciled if `linkProject` or
`unlinkProject` fails after `repository.patch`; add compensation so the secret
is restored to its previous state or the project changes are rolled back before
rethrowing. Update both `update` overloads in `PatchSecretUseCaseImpl` to treat
patch + reconciliation as one atomic operation at the use-case level, and guard
against stale reads/TOCTOU by reloading or validating the current project set
before applying `fetchProjects`-driven changes.
---
Nitpick comments:
In `@Projects/Devault/Sources/ContentView.swift`:
- Around line 17-38: body에서 CreateProjectUseCaseImpl, FetchProjectUseCaseImpl,
CreateSecretUseCaseImpl, FetchSecretUseCaseImpl,
SecretProjectRelationUseCaseImpl를 매번 새로 만드는 구조를 제거하고, ContentView의 init에서 한 번만
생성해 private let 프로퍼티로 보관하도록 변경하세요. 그런 다음 body에서는 ProjectSecretRelationDemoView에
미리 준비된 use case들을 전달만 하게 바꿔서, SwiftUI 재평가 시 불필요한 할당이 반복되지 않도록 정리하세요.
In `@Projects/DVData/Sources/RepositoryImpl/Project/ProjectRepositoryImpl.swift`:
- Around line 3-5: The import ordering in ProjectRepositoryImpl is reversed
relative to the module import rule. Reorder the imports so Foundation and
SwiftData appear first, then add a blank line, and place DVDomain after that;
keep the import block alphabetized within each group and use the import list in
ProjectRepositoryImpl as the reference point.
- Around line 132-144: containsProjectName is doing a full Project fetch and
in-memory duplicate-name scan on every create/patch, which will not scale.
Update ProjectRepositoryImpl to persist a normalized name field on
SwiftDataModel.Project and keep it in sync in the create/patch flow, then query
by that indexed field instead of fetching all projects; keep the existing
excludedID logic in containsProjectName while moving the lookup to a targeted
fetch/predicate.
In `@Projects/DVDomain/Sources/Entity/SecretType.swift`:
- Around line 3-42: The rawValue of SecretType and SecretSubType is currently
inferred from case names, which makes SwiftDataModel.Secret.secretType and
subType persistence fragile. Update the SecretType and SecretSubType enums to
use explicit, stable raw string values instead of relying on implicit case-name
defaults. Keep the existing mapping behavior in availableSubTypes and secretType
unchanged, but ensure the persisted strings remain fixed even if case names are
later renamed.
In
`@Projects/DVDomain/Sources/UseCase/Impl/Project/FetchProjectUseCaseImpl.swift`:
- Around line 12-26: Both FetchProjectUseCaseImpl.fetch(id:) and fetchAll()
repeat the same do/catch mapping to ProjectUseCaseError.map(error), so extract
that logic into a small private generic helper in FetchProjectUseCaseImpl and
have both methods call it. Keep the helper focused on wrapping an async throwing
operation and returning its result, while preserving the existing error mapping
behavior for repository.fetch(id:) and repository.fetchAll().
In `@Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift`:
- Around line 29-68: The two `update` overloads in `PatchSecretUseCaseImpl`
duplicate the same flow for encrypting payload, building `fullPatch`, calling
`repository.patch`, and then `reconcileProjects`. Extract the shared logic into
a private helper method and have both overloads delegate to it, keeping only the
metadata-specific encoding in the
`update(id:patch:payload:metadata:projectIDs:)` path. This will centralize
future changes in `PatchSecretUseCaseImpl` and reduce the risk of the two code
paths drifting apart.
In `@Projects/DVPresentation/Sources/ProjectSecretRelationDemoView.swift`:
- Around line 3-4: Adjust the import ordering in ProjectSecretRelationDemoView
so SwiftUI comes first as the built-in framework, then add a blank line before
the internal module import DVDomain. Keep the imports alphabetically ordered
within their groups and preserve the framework-first, separated-by-empty-line
convention.
🪄 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: c4c9de40-bfba-44fc-b1ba-affc76daf3fa
📒 Files selected for processing (42)
.github/PULL_REQUEST_TEMPLATE.mdProjects/DVData/Sources/RepositoryImpl/Project/ProjectRepositoryImpl.swiftProjects/DVData/Sources/RepositoryImpl/Secret/InMemorySecretQueryFilter.swiftProjects/DVData/Sources/RepositoryImpl/Secret/SecretRepositoryImpl.swiftProjects/DVData/Sources/Storage/Local/Models/Project.swiftProjects/DVData/Sources/Storage/Local/Models/Secret.swiftProjects/DVDomain/Sources/Entity/Project.swiftProjects/DVDomain/Sources/Entity/Secret.swiftProjects/DVDomain/Sources/Entity/SecretType.swiftProjects/DVDomain/Sources/Repository/Error/ProjectRepositoryError.swiftProjects/DVDomain/Sources/Repository/Error/SecretRepositoryError.swiftProjects/DVDomain/Sources/Repository/Interface/ProjectRepository.swiftProjects/DVDomain/Sources/Repository/Interface/SecretRepository.swiftProjects/DVDomain/Sources/Repository/PatchField.swiftProjects/DVDomain/Sources/Repository/ProjectPatch.swiftProjects/DVDomain/Sources/Repository/SecretPatch.swiftProjects/DVDomain/Sources/Service/Interface/SecretCryptoService.swiftProjects/DVDomain/Sources/Service/Interface/UserAuthenticationService.swiftProjects/DVDomain/Sources/UseCase/Draft/SecretDraft.swiftProjects/DVDomain/Sources/UseCase/Error/ProjectUseCaseError.swiftProjects/DVDomain/Sources/UseCase/Impl/Project/CreateProjectUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Project/DeleteProjectUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Project/FetchProjectUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Project/ProjectUseCaseHelper.swiftProjects/DVDomain/Sources/UseCase/Impl/Project/RenameProjectUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/FetchSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/SecretProjectRelationUseCaseImpl.swiftProjects/DVDomain/Sources/UseCase/Impl/Secret/SecretUseCaseHelper.swiftProjects/DVDomain/Sources/UseCase/Interface/Project/CreateProjectUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Project/DeleteProjectUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Project/FetchProjectUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Project/RenameProjectUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/CreateSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/DeleteSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/FetchSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/PatchSecretUseCase.swiftProjects/DVDomain/Sources/UseCase/Interface/Secret/SecretProjectRelationUseCase.swiftProjects/DVPresentation/Sources/ProjectSecretRelationDemoView.swiftProjects/DVPresentation/Sources/SecretUseCaseDemoView.swiftProjects/Devault/Sources/ContentView.swift
| secretType: DVDomain.SecretType(rawValue: secretType) ?? .etc, | ||
| subType: subType.flatMap(DVDomain.SecretSubType.init(rawValue:)), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
잘못된 rawValue를 조용히 .etc/nil로 숨기지 마세요.
저장된 secretType/subType이 enum으로 복원되지 않으면 저장소 손상으로 처리해야 합니다. 지금처럼 fallback하면 잘못된 타입이 정상 데이터처럼 도메인으로 유입됩니다.
🐛 제안 수정
guard let payload else {
throw SecretRepositoryError.corruptedStorage
}
+ guard let domainSecretType = DVDomain.SecretType(rawValue: secretType) else {
+ throw SecretRepositoryError.corruptedStorage
+ }
+ let domainSubType: DVDomain.SecretSubType?
+ if let subType {
+ guard let parsedSubType = DVDomain.SecretSubType(rawValue: subType) else {
+ throw SecretRepositoryError.corruptedStorage
+ }
+ domainSubType = parsedSubType
+ } else {
+ domainSubType = nil
+ }
return DVDomain.Secret(
id: id,
name: name,
- secretType: DVDomain.SecretType(rawValue: secretType) ?? .etc,
- subType: subType.flatMap(DVDomain.SecretSubType.init(rawValue:)),
+ secretType: domainSecretType,
+ subType: domainSubType,📝 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.
| secretType: DVDomain.SecretType(rawValue: secretType) ?? .etc, | |
| subType: subType.flatMap(DVDomain.SecretSubType.init(rawValue:)), | |
| guard let payload else { | |
| throw SecretRepositoryError.corruptedStorage | |
| } | |
| guard let domainSecretType = DVDomain.SecretType(rawValue: secretType) else { | |
| throw SecretRepositoryError.corruptedStorage | |
| } | |
| let domainSubType: DVDomain.SecretSubType? | |
| if let subType { | |
| guard let parsedSubType = DVDomain.SecretSubType(rawValue: subType) else { | |
| throw SecretRepositoryError.corruptedStorage | |
| } | |
| domainSubType = parsedSubType | |
| } else { | |
| domainSubType = nil | |
| } | |
| return DVDomain.Secret( | |
| id: id, | |
| name: name, | |
| secretType: domainSecretType, | |
| subType: domainSubType, |
🤖 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 `@Projects/DVData/Sources/Storage/Local/Models/Secret.swift` around lines 83 -
84, `Secret`의 복원 로직에서 잘못된 `rawValue`를 `DVDomain.SecretType(rawValue:) ?? .etc`와
`DVDomain.SecretSubType.init(rawValue:)`의 fallback으로 숨기지 말고,
`secretType`/`subType`가 enum으로 변환되지 않으면 저장소 손상으로 간주하도록 `Secret` 초기화 또는 매핑 경로를
수정하세요. `DVDomain.SecretType`과 `DVDomain.SecretSubType` 복원 실패 시에는 기본값을 넣지 말고 실패를
전파하거나 별도 오류를 발생시켜, 잘못된 데이터가 정상 도메인 값으로 유입되지 않게 하세요.
There was a problem hiding this comment.
The PR description image shows a screenshot of the "Devault" macOS app's demo screen, "Project ↔ Secret Relation Demo," with panels for Projects, All Secrets, and Relation Detail — this is unrelated to the current code comment and just provides feature context.
@doyeonk429 동의합니다, P0로 보는 게 맞습니다.
원인: toDomain()에서 rawValue 파싱 실패 시 .etc/nil로 조용히 대체하면, 마이그레이션 누락이나 DB 손상으로 잘못된 rawValue가 들어와도 정상 Secret처럼 UI/로직에 노출됩니다. Swift의 안전성 원칙상 이런 불변식 위반은 즉시 실패시켜야 디버깅이 쉬워집니다.
개선안: 위에 제안된 대로 파싱 실패 시 SecretRepositoryError.corruptedStorage를 던지도록 변경하면 됩니다.
guard let payload else {
throw SecretRepositoryError.corruptedStorage
}
guard let domainSecretType = DVDomain.SecretType(rawValue: secretType) else {
throw SecretRepositoryError.corruptedStorage
}
let domainSubType: DVDomain.SecretSubType?
if let subType {
guard let parsedSubType = DVDomain.SecretSubType(rawValue: subType) else {
throw SecretRepositoryError.corruptedStorage
}
domainSubType = parsedSubType
} else {
domainSubType = nil
}
return DVDomain.Secret(
id: id,
name: name,
secretType: domainSecretType,
subType: domainSubType,
...
)toDomain()이 이미 throws로 선언돼 있어 호출부 변경 없이 적용 가능합니다. @dlguszoo 님이 반영해주시면 좋을 것 같습니다 🙂
| func createAndLink(_ secret: Secret, projectIDs: [UUID]) async throws -> Secret { | ||
| let created = try await repository.create(secret) | ||
| do { | ||
| for projectID in projectIDs { | ||
| try await repository.linkProject(secretID: created.id, projectID: projectID) | ||
| } | ||
| } catch { | ||
| try? await repository.delete(id: created.id) | ||
| throw error |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
rollback 실패를 try?로 숨기지 마세요.
Line 94에서 삭제 실패를 삼키면 link 실패 후 생성된 Secret이 남을 수 있습니다. rollback을 저장소 트랜잭션으로 원자화하거나, rollback 실패를 별도 에러로 표면화하세요.
🤖 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 `@Projects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swift`
around lines 87 - 95, The rollback in createAndLink(_:projectIDs:) is currently
swallowing delete failures with try?, which can leave the newly created Secret
orphaned after a linkProject failure. Update
CreateSecretUseCaseImpl.createAndLink to either make the create/link/delete
sequence atomic via a repository transaction or explicitly propagate rollback
failure by throwing a distinct error when repository.delete(id:) fails, instead
of ignoring it.
There was a problem hiding this comment.
@doyeonk429 확인했습니다. 롤백 실패를 침묵시키지 않도록 개선 방향을 제안드립니다.
원인: createAndLink에서 linkProject 실패 시 try? await repository.delete(id:)로 삭제를 시도하는데, 이 삭제마저 실패하면 원인 파악이 불가능한 채로 orphan Secret이 DB에 남습니다.
개선안: rollback 실패를 별도 에러로 표면화해서 최소한 로그/에러 체인으로 추적 가능하게 만드세요.
private extension CreateSecretUseCaseImpl {
func createAndLink(_ secret: Secret, projectIDs: [UUID]) async throws -> Secret {
let created = try await repository.create(secret)
do {
for projectID in projectIDs {
try await repository.linkProject(secretID: created.id, projectID: projectID)
}
} catch let linkError {
do {
try await repository.delete(id: created.id)
} catch let rollbackError {
// 원본 link 실패 원인과 rollback 실패 원인을 모두 보존
throw SecretUseCaseError.rollbackFailed(
original: linkError,
rollback: rollbackError
)
}
throw linkError
}
return created
}
}SecretUseCaseError에 rollbackFailed(original:rollback:) 케이스를 추가하면, 최소한 롤백 실패 시 시스템이 조용히 넘어가지 않고 명확한 에러로 알려줄 수 있습니다. 완전한 원자성이 필요하다면 SwiftData modelContext의 트랜잭션 경계를 활용해 create + link를 하나의 저장 단위로 묶는 방법도 고려해보세요 (ProjectRepositoryImpl/SecretRepositoryImpl이 이미 같은 modelContext를 공유한다면 가능).
| for projectID in projectIDs { | ||
| try await repository.linkProject(secretID: created.id, projectID: projectID) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
projectIDs를 링크 전에 중복 제거하세요.
같은 Project ID가 두 번 들어오면 두 번째 linkProject가 중복 링크 에러를 던져 생성 전체가 rollback됩니다.
🐛 제안 수정
func createAndLink(_ secret: Secret, projectIDs: [UUID]) async throws -> Secret {
+ var seenProjectIDs = Set<UUID>()
+ let uniqueProjectIDs = projectIDs.filter { seenProjectIDs.insert($0).inserted }
let created = try await repository.create(secret)
do {
- for projectID in projectIDs {
+ for projectID in uniqueProjectIDs {
try await repository.linkProject(secretID: created.id, projectID: projectID)
}📝 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.
| for projectID in projectIDs { | |
| try await repository.linkProject(secretID: created.id, projectID: projectID) | |
| var seenProjectIDs = Set<UUID>() | |
| let uniqueProjectIDs = projectIDs.filter { seenProjectIDs.insert($0).inserted } | |
| let created = try await repository.create(secret) | |
| do { | |
| for projectID in uniqueProjectIDs { | |
| try await repository.linkProject(secretID: created.id, projectID: projectID) |
🤖 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 `@Projects/DVDomain/Sources/UseCase/Impl/Secret/CreateSecretUseCaseImpl.swift`
around lines 90 - 91, `CreateSecretUseCaseImpl`의 `projectIDs`를 순회하기 전에 중복을 제거하도록
수정하세요. 현재 `for projectID in projectIDs`에서 동일한 Project ID가 두 번 들어오면
`repository.linkProject(secretID:projectID:)`가 중복 링크 에러를 내고 전체 생성이 롤백될 수 있으니, 링크
전에 중복 없는 ID 목록으로 정리한 뒤 그 목록만 사용하도록 `create` 흐름을 바꾸세요.
| private extension PatchSecretUseCaseImpl { | ||
| func reconcileProjects(secretID: UUID, desiredIDs: [UUID]) async throws { | ||
| let currentIDs = Set(try await repository.fetchProjects(secretID: secretID).map(\.id)) | ||
| let desiredIDs = Set(desiredIDs) | ||
| for projectID in desiredIDs.subtracting(currentIDs) { | ||
| try await repository.linkProject(secretID: secretID, projectID: projectID) | ||
| } | ||
| for projectID in currentIDs.subtracting(desiredIDs) { | ||
| try await repository.unlinkProject(secretID: secretID, projectID: projectID) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
reconcileProjects 부분 실패 시 데이터 정합성 깨짐 — 롤백/보상 로직 필요
repository.patch가 성공한 뒤 reconcileProjects에서 linkProject/unlinkProject 중 하나라도 실패하면, Secret의 payload/필드는 이미 갱신됐지만 Project 연결은 일부만 반영된 채로 에러가 올라갑니다. 호출자는 전체 실패로 인지하지만 실제로는 부분 성공 상태라 재시도 시 이미 연결된 프로젝트에서 duplicateProjectLink 에러가 재발할 수 있습니다. 또한 fetchProjects(읽기) → link/unlink(쓰기) 사이에 TOCTOU 윈도우가 있어 동일 secret에 대한 동시 patch 호출 시 경합이 발생할 수 있습니다.
두 update 오버로드(29-46, 48-68) 모두 동일한 위험을 그대로 물려받습니다.
🔧 개선 방향 제안
private extension PatchSecretUseCaseImpl {
func reconcileProjects(secretID: UUID, desiredIDs: [UUID]) async throws {
let currentIDs = Set(try await repository.fetchProjects(secretID: secretID).map(\.id))
let desiredIDs = Set(desiredIDs)
- for projectID in desiredIDs.subtracting(currentIDs) {
- try await repository.linkProject(secretID: secretID, projectID: projectID)
- }
- for projectID in currentIDs.subtracting(desiredIDs) {
- try await repository.unlinkProject(secretID: secretID, projectID: projectID)
- }
+ var linked: [UUID] = []
+ do {
+ for projectID in desiredIDs.subtracting(currentIDs) {
+ try await repository.linkProject(secretID: secretID, projectID: projectID)
+ linked.append(projectID)
+ }
+ for projectID in currentIDs.subtracting(desiredIDs) {
+ try await repository.unlinkProject(secretID: secretID, projectID: projectID)
+ }
+ } catch {
+ // 실패 시 이번 호출에서 새로 연결한 항목만 되돌려 부분 성공 상태를 방지
+ for projectID in linked {
+ try? await repository.unlinkProject(secretID: secretID, projectID: projectID)
+ }
+ throw error
+ }
}
}이상적으로는 SwiftData modelContext 트랜잭션 수준에서 patch + link/unlink를 하나로 묶는 저장소 API를 노출하는 게 더 견고하지만, 리포지토리 계층 변경 없이 우선 이 정도 보상 로직만이라도 도입하는 걸 권장합니다.
Also applies to: 29-46, 48-68
🤖 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 `@Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift`
around lines 72 - 82, `reconcileProjects(secretID:desiredIDs:)` can leave Secret
metadata updated but project links only partially reconciled if `linkProject` or
`unlinkProject` fails after `repository.patch`; add compensation so the secret
is restored to its previous state or the project changes are rolled back before
rethrowing. Update both `update` overloads in `PatchSecretUseCaseImpl` to treat
patch + reconciliation as one atomic operation at the use-case level, and guard
against stale reads/TOCTOU by reloading or validating the current project set
before applying `fetchProjects`-driven changes.
There was a problem hiding this comment.
이거 부분 실패 시의 문제만이 아니라,
projectIDs: [UUID]? -> nil이면 재조정 skip (unchanged), []이면 전부 제거 같은 식으로 조정해야할 것 같은데,
아니면 PatchField<[UUID]> -> .unchanged/.set(...) 로 명시하거나 오버로드를 update vs updateAndReconcile 로 분리하거나
There was a problem hiding this comment.
@doyeonk429 좋은 지적입니다. 말씀하신 대로 [UUID]만으로는 "변경 없음"과 "전체 해제([])"를 구분할 수 없어서 API가 모호합니다. 이 프로젝트에서 이미 PatchField<Value> (.unchanged / .set(Value)) 패턴을 SecretPatch/ProjectPatch에서 일관되게 쓰고 있으니, 별도 오버로드 분리보다는 **projectIDs: PatchField<[UUID]>**로 통일하는 게 가장 자연스럽습니다.
public func update<Payload: SecretPayloadData>(
id: UUID,
patch: SecretPatch,
payload: Payload,
projectIDs: PatchField<[UUID]> = .unchanged
) async throws -> Secret {
do {
let encryptedPayload = try await cryptoService.encryptPayload(payload)
var fullPatch = patch
fullPatch.payload = .set(encryptedPayload)
fullPatch.updatedAt = .set(dateProvider())
let updated = try await repository.patch(id: id, with: fullPatch)
if case .set(let ids) = projectIDs {
try await reconcileProjects(secretID: id, desiredIDs: ids)
}
return updated
} catch {
throw SecretUseCaseError.map(error)
}
}장점:
.unchanged→ 프로젝트 연결에 전혀 손대지 않음 (현재 대부분의 patch 호출 케이스).set([])→ 명시적으로 전체 unlink.set([...])→ 명시적으로 지정한 목록으로 reconcile
update/updateAndReconcile 오버로드 분리보다 코드 중복이 적고, SecretPatch의 다른 필드들과 시맨틱이 통일돼서 호출부에서 실수할 여지도 줄어듭니다. 다만 이전에 지적한 부분 실패 시 롤백 문제는 별개 이슈이므로, reconcileProjects 보상 로직은 그대로 함께 반영하시는 걸 권장합니다.
CreateSecretUseCaseImpl의 projectIDs: [UUID]도 생성 시점엔 "unchanged" 개념이 없어 그대로 두셔도 무방하지만, PatchSecretUseCaseImpl/PatchSecretUseCase 프로토콜 시그니처는 함께 변경이 필요합니다.
✏️ Learnings added
Learnt from: doyeonk429
Repo: DevaultProject/Devault-macOS PR: 27
File: Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift:72-82
Timestamp: 2026-07-05T03:13:51.735Z
Learning: In `Projects/DVDomain/Sources/UseCase/Interface/Secret/PatchSecretUseCase.swift` and its implementation `PatchSecretUseCaseImpl` (Projects/DVDomain/Sources/UseCase/Impl/Secret/PatchSecretUseCaseImpl.swift), the `update` methods' `projectIDs: [UUID]` parameter is ambiguous — it cannot distinguish "no change to project links" from "clear all links" (both look like reconciling to an empty/unspecified set). The preferred fix per maintainer doyeonk429 is to change the parameter to `PatchField<[UUID]>` (the existing `.unchanged`/`.set(Value)` enum already used in `SecretPatch`/`ProjectPatch`), where `.unchanged` skips `reconcileProjects` entirely, and `.set([...])` (including `.set([])`) triggers reconciliation to that exact set.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
doyeonk429
left a comment
doyeonk429
left a comment
There was a problem hiding this comment.
GoodPoint ⭐
- PatchField를 별도 파일로 분리해서 SecretPatch/ProjectPatch가 공유
- Repository↔UseCase 에러 매핑(ProjectUseCaseError.map)이 SecretUseCaseError.map와 같은 패턴(일관성 굿)
- linkProject 시 duplicateProjectLink 사전 확인 후 삽입 -> SwiftData unique constraint 실패에 의존하지 않고 명시적으로 처리해서 에러 매핑이 깔끔해짐.
| secretType: DVDomain.SecretType(rawValue: secretType) ?? .etc, | ||
| subType: subType.flatMap(DVDomain.SecretSubType.init(rawValue:)), |
| @@ -0,0 +1,42 @@ | |||
| // Copyright © 2026 Devault. All rights reserved | |||
|
|
|||
| public enum SecretType: String, Codable, CaseIterable, Sendable { | |||
There was a problem hiding this comment.
P0) SecretQuery의 secretType은 그대로 String? type으로 정의되어있음. SecretType의 callers 체크 필요해보임!
| metadata: Metadata, | ||
| projectIDs: [UUID] | ||
| ) async throws -> Secret | ||
| func removeMetadata(id: UUID) async throws -> Secret |
There was a problem hiding this comment.
P1) metadata를 nil로 만드는 명시적 API가 사라지고 patch(id:with:)에 metadata: .set(nil)을 넘기는 방식으로 대체된 것으로 보이는데, 인터페이스 문서에 이 대체 경로가 명시되어있지 않음.
-> 기존 updateMetadata처럼 payload는 그대로 두고 metadata만 갱신하고 싶은 케이스에서 원본 복호화가 강제되는데, metadata만 갱신하는 오버로드 또는 updateMetadata(id:metadata:), removeMetadata(id:)가 있으면 어떨까 싶음.
2 participants
✨ What’s this PR?
📌 관련 이슈 (Related Issue)
🧶 주요 변경 내용 (Summary)
ProjectDomain entity 추가ProjectRepositoryinterface 정의 및 SwiftData 기반ProjectRepositoryImpl구현createfetch(id:)fetchAllpatchdeleteProjectPatch와ProjectRepositoryError추가ProjectUseCaseInterface 정의 및 구현CreateProjectUseCaseFetchProjectUseCaseRenameProjectUseCaseDeleteProjectUseCaseProject모델을 DomainProject로 변환하는 mapper 구현SecretRepository에 Project 관계 메서드를 추가fetchProjects(secretID:)linkProject(secretID:projectID:)unlinkProject(secretID:projectID:)SecretRepositoryError에 Project 관계 관련 error 추가projectNotFoundduplicateProjectLinkprojectLinkNotFoundSecretRepositoryImpl에서SecretProjectLink기반 link/unlink를 구현SecretQuery.Collection.project(id:)를 통해 Project 기준 Secret 조회 흐름 유지📸 스크린샷 (Optional)
🧪 테스트 / 검증 내역
tuist build Devault성공💬 기타 공유 사항
Project.secrets,Secret.projects같은 관계 배열을 두지 않았습니다.니다.
lowercased()기준으로 비교합니다.ProjectPatch와patch(id:with:)구조를 유지했습니다.
ProjectRepository에도 중복 정의하지 않고SecretRepository로 단일화했습니다.🙇🏻♀️ 리뷰 가이드 (선택)
update 시 patch-reconcile 간 일치성 보장 문제
PatchSecretUseCaseImpl.update는 내부적으로 두 단계로 실행됩니다.
두 호출이 별개의 ModelContext.save()로 분리되어 있어, patch 성공 후 reconcileProjects가 실패하면 필드는 수정됐지만 Project 연결은 이전 상태로 남는 partial state가 발생할 수 있습니다.
UseCase 레벨 rollback을 채택하지 않은 이유
rollback용 SecretPatch(from: originalSecret) 역변환 로직을 별도로 설계해야 하는 비용이 있고, rollback 자체도 실패할 경우 오히려 상태를 알 수 없게 됩니다. SwiftData 로컬 환경에서 patch 성공 후 reconcileProjects만 실패하는 케이스가 현실적으로 거의 없습니다.
근본적 해결 방향 (추후 논의)
patch와 link/unlink를 하나의 ModelContext.save() 트랜잭션으로 묶는 것이 근본적인 해결책인것 같습니다. 현재 Repository 인터페이스가 개별 작업 단위로 설계되어 있어 트랜잭션 묶음을 지원하려면 인터페이스 재설계가 필요하므로, Repository 계층 고도화 시 함께 논의해야할 것 같습니다.
Summary by CodeRabbit
New Features
Bug Fixes