Merge release/3.2.2 into develop#174
Merged
Merged
Conversation
* 🐛 [fix] CloudKit 동기화 데이터 유실 방지 강화 및 동기화 상태 표시 - 기존 유저 플래그(hasEverHadUserCollection) 도입하여 앱 업데이트/iCloud 재구성 시 빈 UC 자동 생성으로 인한 클라우드 데이터 오염 차단 - Grace period 30초 → 120초 연장 (대용량 re-import 대응) - DailyTask 기본값 자동 생성에 shouldSuppressDataCreation 통합 보호 적용 - 의도적 데이터 초기화 시 hasEverHadUserCollection 리셋 (clearHasEverHadUserCollection) - 설정 화면에 동기화 상태 표시 (로컬 레코드 수, 마지막 동기화 시각, 동기화 진행 상태) * ♻️ [refactor] Storage 에러 로깅 debugPrint → os_log 교체 Release 빌드에서도 에러 추적이 가능하도록 모든 CoreData Storage 클래스와 Items.swift, TasksEditReactor의 debugPrint(error)를 os_log(.error)로 교체 * 📝 [docs] iCloud 동기화 문서 업데이트 - Known User Protection (hasEverHadUserCollection) 섹션 추가 - shouldSuppressDataCreation 통합 프로퍼티 설명 - UC 생성 억제 조건 4가지 → 5가지로 업데이트 (grace period 120초) - Sync Status Display 섹션 추가 - Error Logging 강화 섹션 추가 * 🎨 [refactor] 동기화 상태 표시 문구 개선 및 한 줄 UI로 변경 - "로컬 레코드: N개" → "N개 저장됨" (사용자 친화적 문구) - 동기화 시각 있을 때 "3분 전 동기화 · N개 저장됨" 형태로 통합 - 여러 줄 → 한 줄 표시 (numberOfLines=1, textAlignment=.right) * 🎨 [refactor] 앱 설정 Row 간격 조정 (4 → 16) * ♻️ [refactor] 리뷰 반영 — 중복 제거 및 성능 개선 - grace period 상수(120s) + isWithinGracePeriod 추출 (DRY) - entityCounts(in:) 헬퍼 추출하여 logSyncDiagnostics/fetchSyncStatus 공용화 - hasEverHadUserCollection 메모리 캐싱 (매 호출 UserDefaults 읽기 제거) - RelativeDateTimeFormatter를 DateFormatters.syncRelativeDate로 캐싱 * 📝 [docs] icloud-sync.md 리팩터링 반영 (entityCounts, isWithinGracePeriod, 메모리 캐싱)
📝 WalkthroughWalkthroughThis PR introduces iCloud sync status display functionality alongside infrastructure improvements. Changes include version bump to 3.2.2, new sync status UI with localization, updated Core Data storage with persistent user tracking and modified grace-period logic, systematic replacement of debug logging with OS-level logging across storage modules, and supporting documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant View as AppSettingView
participant Reactor as AppSettingReactor
participant Storage as CoreDataStorage
participant CoreData as Core Data
View->>Reactor: bind(to:) starts
View->>Reactor: sendAction(.loadSyncStatus)
Reactor->>Storage: fetchSyncStatus(completion:)
Storage->>CoreData: fetch UserCollectionEntity count
Storage->>CoreData: fetch total item entities
CoreData-->>Storage: entity counts
Storage->>Storage: gather timestamps<br/>(lastImport/Export)
Storage->>Storage: check isSyncing state
Storage-->>Reactor: SyncStatusInfo
Reactor->>Reactor: reduce mutation<br/>setSyncStatus(info)
Reactor-->>View: emit newState
View->>View: updateSyncStatusLabel(status)
View->>View: determine display message<br/>(Syncing/Waiting/Synced/Saved)
View->>View: update UI label
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/UserInfoStorage/CoreDataUserInfoStorage.swift (2)
49-49: Minor style suggestion per SwiftLint.SwiftLint's
conditional_returns_on_newlinerule suggests placingreturnon a new line for better readability.♻️ Optional style fix
- guard let self else { return } + guard let self else { + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/UserInfoStorage/CoreDataUserInfoStorage.swift` at line 49, Replace the single-line guard with a multiline guard so the return is on its own line; in CoreDataUserInfoStorage (the closure that currently has "guard let self else { return }"), change it to "guard let self else {\n return\n}" to satisfy SwiftLint's conditional_returns_on_newline rule.
50-50: Redundant cast toNSManagedObject.
UserCollectionEntityalready inherits fromNSManagedObject, so theas? NSManagedObjectcast is unnecessary. TheflatMapon line 51 handles thenilcase fromtry?.♻️ Suggested simplification
- let object = try? self.coreDataStorage.getUserCollection(context) as? NSManagedObject + let object = try? self.coreDataStorage.getUserCollection(context)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/UserInfoStorage/CoreDataUserInfoStorage.swift` at line 50, The code performs an unnecessary cast when assigning 'object'—change the line using coreDataStorage.getUserCollection(context) so it does not cast to NSManagedObject; treat the result as the expected UserCollectionEntity (or optional thereof) from getUserCollection and let the existing flatMap handle nils; update references to 'object' accordingly (function/method: coreDataStorage.getUserCollection, type: UserCollectionEntity, variable: object) to remove the redundant "as? NSManagedObject" cast.Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/CoreDataStorage.swift (1)
90-112: Thread safety consideration forhasEverHadUserCollection.The
hasEverHadUserCollectionproperty uses a cached value (_hasEverHadUserCollectionCached) without synchronization, unlike other state properties that useOSAllocatedUnfairLock. While the getter and setter are straightforward, concurrent reads and writes from different threads (e.g., background Core Data tasks) could theoretically race.In practice, the property is:
- Read frequently in
getUserCollection()(background contexts)- Written rarely (only when UC is first found or cleared)
The current pattern is likely safe given Swift's memory model for simple Bool assignments, but for consistency with other state properties, consider wrapping it in a lock.
🔧 Optional: Add thread-safe wrapper for consistency
- private var _hasEverHadUserCollectionCached = UserDefaults.standard.bool( - forKey: CoreDataStorage.hasEverHadUserCollectionKey - ) - private(set) var hasEverHadUserCollection: Bool { - get { _hasEverHadUserCollectionCached } - set { - guard _hasEverHadUserCollectionCached != newValue else { return } - _hasEverHadUserCollectionCached = newValue - UserDefaults.standard.set(newValue, forKey: Self.hasEverHadUserCollectionKey) - } - } + private let _hasEverHadUserCollection = OSAllocatedUnfairLock( + initialState: UserDefaults.standard.bool(forKey: CoreDataStorage.hasEverHadUserCollectionKey) + ) + private(set) var hasEverHadUserCollection: Bool { + get { _hasEverHadUserCollection.withLock { $0 } } + set { + _hasEverHadUserCollection.withLock { cached in + guard cached != newValue else { return } + cached = newValue + UserDefaults.standard.set(newValue, forKey: Self.hasEverHadUserCollectionKey) + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/CoreDataStorage.swift` around lines 90 - 112, The cached Bool _hasEverHadUserCollectionCached and computed property hasEverHadUserCollection are not synchronized and should be guarded like other state in CoreDataStorage; wrap both the getter and setter (and clearHasEverHadUserCollection) with the same OSAllocatedUnfairLock used elsewhere in this type to avoid races: acquire the lock before reading or writing _hasEverHadUserCollectionCached and before calling UserDefaults.standard.set, and release after; keep the existing semantics (early-return on equal value) and continue to log in clearHasEverHadUserCollection.Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersHouseStorage/CoreDataVillagersHouseStorage.swift (2)
10-10: Consider usingimport OSLoginstead ofimport os.The
OSLogmodule is the modern, recommended import that provides access to both the legacyos_logfunction and the newerLoggerAPI. This offers better forward compatibility and aligns with Apple's current logging recommendations.📦 Suggested import change
-import os +import OSLog🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersHouseStorage/CoreDataVillagersHouseStorage.swift` at line 10, Replace the legacy module import by changing the top-level import of "os" to "OSLog" in CoreDataVillagersHouseStorage.swift; update any references that rely on the old module (e.g., uses of os_log or Logger) to continue working under the OSLog import (no other API changes needed if you're already using Logger), ensuring the file now imports OSLog instead of os.
44-44: Consider modernizing to Logger API and using a custom log category.While the current
os_logusage is functionally correct, there are two observability improvements to consider:
- Modern Logger API: Apple recommends the
Loggertype (introduced in iOS 14) over the legacyos_logfunction for improved ergonomics and type safety.- Custom log category: Using
.defaultlimits your ability to filter logs in Console.app. A custom log with a specific subsystem and category (e.g.,"VillagersHouseStorage") would make debugging significantly easier.To verify this is consistent with the broader refactoring and check deployment target compatibility:
#!/bin/bash # Check iOS deployment target and logging patterns across storage modules # Check deployment target from project file fd -e pbxproj . | head -1 | xargs grep -A5 "IPHONEOS_DEPLOYMENT_TARGET" # Check if other storage files use the same os_log pattern rg -n "os_log\(.error" --type swift -g '*Storage.swift' -C2 # Check if there's a centralized Logger or OSLog definition ast-grep --pattern 'let $_ = Logger(subsystem: $_, category: $_)'🔄 Suggested refactor using Logger API
If the deployment target is iOS 14+, consider this modern approach:
-import os +import OSLog + +fileprivate let logger = Logger(subsystem: "com.yourapp.acnhwiki", category: "VillagersHouseStorage")Then at line 44:
- os_log(.error, log: .default, "VillagersHouseStorage error: %{public}@", error.localizedDescription) + logger.error("VillagersHouseStorage error: \(error.localizedDescription, privacy: .public)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersHouseStorage/CoreDataVillagersHouseStorage.swift` at line 44, Replace the legacy os_log call in CoreDataVillagersHouseStorage with a Logger instance: add a private static Logger (e.g., let logger = Logger(subsystem: Bundle.main.bundleIdentifier ?? "com.yourapp", category: "VillagersHouseStorage")) inside the CoreDataVillagersHouseStorage type and change the os_log(.error, ...) invocation to use logger.error with the error message (marking it public if needed). Ensure you import os and swift-logging APIs as required and only apply this change when the deployment target supports Logger (iOS 14+/macOS 11+).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/CoreDataStorage.swift`:
- Around line 90-112: The cached Bool _hasEverHadUserCollectionCached and
computed property hasEverHadUserCollection are not synchronized and should be
guarded like other state in CoreDataStorage; wrap both the getter and setter
(and clearHasEverHadUserCollection) with the same OSAllocatedUnfairLock used
elsewhere in this type to avoid races: acquire the lock before reading or
writing _hasEverHadUserCollectionCached and before calling
UserDefaults.standard.set, and release after; keep the existing semantics
(early-return on equal value) and continue to log in
clearHasEverHadUserCollection.
In
`@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/UserInfoStorage/CoreDataUserInfoStorage.swift`:
- Line 49: Replace the single-line guard with a multiline guard so the return is
on its own line; in CoreDataUserInfoStorage (the closure that currently has
"guard let self else { return }"), change it to "guard let self else {\n
return\n}" to satisfy SwiftLint's conditional_returns_on_newline rule.
- Line 50: The code performs an unnecessary cast when assigning 'object'—change
the line using coreDataStorage.getUserCollection(context) so it does not cast to
NSManagedObject; treat the result as the expected UserCollectionEntity (or
optional thereof) from getUserCollection and let the existing flatMap handle
nils; update references to 'object' accordingly (function/method:
coreDataStorage.getUserCollection, type: UserCollectionEntity, variable: object)
to remove the redundant "as? NSManagedObject" cast.
In
`@Animal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersHouseStorage/CoreDataVillagersHouseStorage.swift`:
- Line 10: Replace the legacy module import by changing the top-level import of
"os" to "OSLog" in CoreDataVillagersHouseStorage.swift; update any references
that rely on the old module (e.g., uses of os_log or Logger) to continue working
under the OSLog import (no other API changes needed if you're already using
Logger), ensuring the file now imports OSLog instead of os.
- Line 44: Replace the legacy os_log call in CoreDataVillagersHouseStorage with
a Logger instance: add a private static Logger (e.g., let logger =
Logger(subsystem: Bundle.main.bundleIdentifier ?? "com.yourapp", category:
"VillagersHouseStorage")) inside the CoreDataVillagersHouseStorage type and
change the os_log(.error, ...) invocation to use logger.error with the error
message (marking it public if needed). Ensure you import os and swift-logging
APIs as required and only apply this change when the deployment target supports
Logger (iOS 14+/macOS 11+).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 659c0e70-e900-4b37-ab6e-e45005d9b530
📒 Files selected for processing (17)
Animal-Crossing-Wiki/Configurations/TargetVersion.xcconfigAnimal-Crossing-Wiki/Projects/App/Resources/en.lproj/Localizable.stringsAnimal-Crossing-Wiki/Projects/App/Resources/ko.lproj/Localizable.stringsAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/CoreDataStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/DailyTaskStorage/CoreDataDailyTaskStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/ItemsStorage/CoreDataItemsStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/UserInfoStorage/CoreDataUserInfoStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VariantsStorage/CoreDataVariantsStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersHouseStorage/CoreDataVillagersHouseStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersLikeStorage/CoreDataNPCLikeStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/CoreDataStorage/VillagersLikeStorage/CoreDataVillagersLikeStorage.swiftAnimal-Crossing-Wiki/Projects/App/Sources/Extension/DateFormatters.swiftAnimal-Crossing-Wiki/Projects/App/Sources/Presentation/Dashboard/ViewModels/AppSettingReactor.swiftAnimal-Crossing-Wiki/Projects/App/Sources/Presentation/Dashboard/ViewModels/TasksEditReactor.swiftAnimal-Crossing-Wiki/Projects/App/Sources/Presentation/Dashboard/Views/AppSettingView.swiftAnimal-Crossing-Wiki/Projects/App/Sources/Utility/Items.swiftdocs/features/icloud-sync.md
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.
Summary by CodeRabbit
New Features
Localization
Documentation