Feature/#25 - Project CRUD ORM과 Secret-Project 링크 메서드 구현#27
Open
dlguszoo wants to merge 24 commits into
Open
Conversation
6 tasks
|
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
Comment on lines
+83
to
+84
| 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.
Suggested change
| 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` 복원 실패 시에는 기본값을 넣지 말고 실패를
전파하거나 별도 오류를 발생시켜, 잘못된 데이터가 정상 도메인 값으로 유입되지 않게 하세요.
Comment on lines
+87
to
+95
| 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.
Comment on lines
+90
to
+91
| 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.
Suggested change
| 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` 흐름을 바꾸세요.
Comment on lines
+72
to
82
| 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.
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.
✨ 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