fix: 커스텀 시간표 변경사항 워치 동기화#219
Merged
Merged
Conversation
- push updated custom timetable payload from iPhone on toggle/save - apply watch payloads via application context and replace stale local rows - reload watch main view when sync payload arrives
Contributor
🛠️ 이슈와 PR의 Labels 동기화를 스킵했어요. |
Contributor
WalkthroughiOS 앱의 AppDelegate가 Watch 동기화 알림을 구독하여 시간표 변경 신호를 감지하고, 현재 데이터를 Watch에 푸시합니다. watchOS의 WatchSessionManager가 푸시된 페이로드를 수신하여 로컬 저장소에 반영하고 동기화 버전을 증가시키며, MainView가 이 버전 변화를 감지하여 시간표를 재로드하는 단방향 동기화 흐름이 완성됩니다. ChangesWatch 동기화 알림 기반 데이터 푸시 및 상태 관리
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
✅ PR의 Assign 자동 지정을 성공했어요! |
baekteun
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the watchOS synchronization logic by consolidating payload building and data pushing into helper methods, utilizing updateApplicationContext alongside sendMessage, and adding a notification-based sync trigger. Feedback highlights two key issues: first, attempting to update the application context when WCSession is unsupported or inactive can flood logs with errors; second, simultaneous message and context updates can cause redundant database writes and UI reloads on the watch, which can be mitigated by adding a local state equality check.
Comment on lines
+187
to
+203
| private func pushCurrentWatchData() { | ||
| guard let payload = buildWatchPayload() else { return } | ||
|
|
||
| do { | ||
| try session.updateApplicationContext(payload) | ||
| } catch { | ||
| TWLog.error(error) | ||
| } | ||
|
|
||
| guard session.activationState == .activated, session.isWatchAppInstalled, session.isReachable else { | ||
| return | ||
| } | ||
|
|
||
| session.sendMessage(payload, replyHandler: nil) { error in | ||
| TWLog.error(error.localizedDescription) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
On devices where WCSession is not supported (such as iPads) or when the session is not yet activated, calling updateApplicationContext will throw an error and flood the logs with WCErrorDomain Code=7001 ("Session is not activated.").
To prevent this, guard against WCSession.isSupported() and check that session.activationState == .activated before attempting to update the application context.
Suggested change
| private func pushCurrentWatchData() { | |
| guard let payload = buildWatchPayload() else { return } | |
| do { | |
| try session.updateApplicationContext(payload) | |
| } catch { | |
| TWLog.error(error) | |
| } | |
| guard session.activationState == .activated, session.isWatchAppInstalled, session.isReachable else { | |
| return | |
| } | |
| session.sendMessage(payload, replyHandler: nil) { error in | |
| TWLog.error(error.localizedDescription) | |
| } | |
| } | |
| private func pushCurrentWatchData() { | |
| guard WCSession.isSupported(), session.activationState == .activated else { | |
| return | |
| } | |
| guard let payload = buildWatchPayload() else { return } | |
| do { | |
| try session.updateApplicationContext(payload) | |
| } catch { | |
| TWLog.error(error) | |
| } | |
| guard session.isWatchAppInstalled, session.isReachable else { | |
| return | |
| } | |
| session.sendMessage(payload, replyHandler: nil) { error in | |
| TWLog.error(error.localizedDescription) | |
| } | |
| } |
Comment on lines
+95
to
+128
| private func applyIncomingItems(_ items: [String: Any]) { | ||
| guard | ||
| let code = items["code"] as? String, | ||
| let orgCode = items["orgCode"] as? String, | ||
| let grade = items["grade"] as? Int, | ||
| let `class` = items["class"] as? Int, | ||
| let type = items["type"] as? String, | ||
| let isOnModifiedTimeTable = items["isOnModifiedTimeTable"] as? Bool, | ||
| let timeTablesData = items["timeTables"] as? Data | ||
| else { | ||
| return | ||
| } | ||
|
|
||
| let timeTables = decodeTimeTables(data: timeTablesData) | ||
| let dict: [UserDefaultsKeys: Any] = [ | ||
| .grade: grade, | ||
| .class: `class`, | ||
| .schoolType: type, | ||
| .orgCode: orgCode, | ||
| .schoolCode: code, | ||
| .isOnModifiedTimeTable: isOnModifiedTimeTable | ||
| ] | ||
| dict.forEach { key, value in | ||
| self.userDefaultsClient.setValue(key, value) | ||
| } | ||
| if let major = items["major"] as? String { | ||
| self.userDefaultsClient.setValue(.major, major) | ||
| } | ||
| try? self.localDatabaseClient.deleteAll(record: ModifiedTimeTableLocalEntity.self) | ||
| try? self.localDatabaseClient.save(records: timeTables) | ||
| DispatchQueue.main.async { | ||
| self.syncVersion += 1 | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Because watch sync payloads are delivered via both updateApplicationContext and sendMessage simultaneously, applyIncomingItems will be called twice in rapid succession when the watch app is in the foreground.
This causes redundant database deletions/writes and triggers duplicate UI reloads. Adding an equality check to compare the incoming payload with the current local state will prevent this redundancy and improve performance on the Apple Watch.
private func applyIncomingItems(_ items: [String: Any]) {
guard
let code = items["code"] as? String,
let orgCode = items["orgCode"] as? String,
let grade = items["grade"] as? Int,
let classValue = items["class"] as? Int,
let type = items["type"] as? String,
let isOnModifiedTimeTable = items["isOnModifiedTimeTable"] as? Bool,
let timeTablesData = items["timeTables"] as? Data
else {
return
}
let timeTables = decodeTimeTables(data: timeTablesData)
let currentGrade = userDefaultsClient.getValue(.grade) as? Int
let currentClass = userDefaultsClient.getValue(.class) as? Int
let currentSchoolType = userDefaultsClient.getValue(.schoolType) as? String
let currentOrgCode = userDefaultsClient.getValue(.orgCode) as? String
let currentSchoolCode = userDefaultsClient.getValue(.schoolCode) as? String
let currentIsOnModifiedTimeTable = userDefaultsClient.getValue(.isOnModifiedTimeTable) as? Bool
let currentMajor = userDefaultsClient.getValue(.major) as? String
let incomingMajor = items["major"] as? String
let currentLocalTimeTables = (try? localDatabaseClient.readRecords(as: ModifiedTimeTableLocalEntity.self)) ?? []
if currentGrade == grade,
currentClass == classValue,
currentSchoolType == type,
currentOrgCode == orgCode,
currentSchoolCode == code,
currentIsOnModifiedTimeTable == isOnModifiedTimeTable,
currentMajor == incomingMajor,
currentLocalTimeTables == timeTables {
return
}
let dict: [UserDefaultsKeys: Any] = [
.grade: grade,
.class: classValue,
.schoolType: type,
.orgCode: orgCode,
.schoolCode: code,
.isOnModifiedTimeTable: isOnModifiedTimeTable
]
dict.forEach { key, value in
self.userDefaultsClient.setValue(key, value)
}
if let major = items["major"] as? String {
self.userDefaultsClient.setValue(.major, major)
}
try? self.localDatabaseClient.deleteAll(record: ModifiedTimeTableLocalEntity.self)
try? self.localDatabaseClient.save(records: timeTables)
DispatchQueue.main.async {
self.syncVersion += 1
}
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Projects/App/iOS/Sources/Application/AppDelegate.swift (1)
19-19: ⚡ Quick win워치 동기화 Notification 이름을 공용 상수로 통합해 주세요.
현재 이 파일은 상수를 쓰지만 다른 레이어는 문자열 리터럴을 직접 사용합니다. 오탈자/드리프트 방지를 위해 shared contract로 올리는 편이 안전합니다.
Also applies to: 49-54
🤖 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/App/iOS/Sources/Application/AppDelegate.swift` at line 19, 현재 AppDelegate.swift의 private let watchSyncNotification = Notification.Name("TodayWhatWatchSyncDidRequest") 상수가 앱 전체에서 일부는 상수, 일부는 문자열 리터럴로 중복되어 있어 통합이 필요합니다; 새로운 공용 상수(예: public extension Notification.Name or enum NotificationNames)에 TodayWhatWatchSyncDidRequest를 정의하고 기존 AppDelegate의 watchSyncNotification을 이 공용 상수로 교체한 뒤 프로젝트 내 다른 파일들에서 직접 문자열 대신 새 공용 상수(예: .todayWhatWatchSyncDidRequest)를 사용하도록 변경하세요; 이때 접근 제어를 public으로 설정해 모듈 경계를 넘겨 사용할 수 있게 하고, 기존 문자열 리터럴(파일 내외부, 특히 언급된 49-54 라인 영역)을 모두 찾아 교체해 오탈자 위험을 제거하세요.
🤖 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 `@Projects/App/watchOS/Sources/Manager/WatchSessionManager.swift`:
- Line 108: The call to decodeTimeTables using force-unwrap (try!) is unsafe and
can crash when incoming watch payloads are corrupt; change uses of try!
(including the similar calls around lines 90-93) to a do-catch or try? pattern
so decoding errors are caught, log the decoding error (use the existing logger
or os_log) and handle failure by returning an empty/default value or
early-returning gracefully instead of crashing; specifically update the call
site that invokes decodeTimeTables(...) and any other decoding calls in
WatchSessionManager to call the throwing decoder safely and handle the thrown
error in the catch block.
- Around line 123-127: The current code in WatchSessionManager silently ignores
failures from localDatabaseClient.deleteAll(record:
ModifiedTimeTableLocalEntity.self) and localDatabaseClient.save(records:
timeTables) using try?, which can advance syncVersion even if the write failed;
change this to perform the delete and save inside a do/catch (or use the
clients' completion callbacks) and only on successful save increment syncVersion
on the main queue (DispatchQueue.main.async { self.syncVersion += 1 }); if save
fails, log/propagate the error and do not change syncVersion (optionally
rollback or handle partial state as appropriate).
---
Nitpick comments:
In `@Projects/App/iOS/Sources/Application/AppDelegate.swift`:
- Line 19: 현재 AppDelegate.swift의 private let watchSyncNotification =
Notification.Name("TodayWhatWatchSyncDidRequest") 상수가 앱 전체에서 일부는 상수, 일부는 문자열
리터럴로 중복되어 있어 통합이 필요합니다; 새로운 공용 상수(예: public extension Notification.Name or enum
NotificationNames)에 TodayWhatWatchSyncDidRequest를 정의하고 기존 AppDelegate의
watchSyncNotification을 이 공용 상수로 교체한 뒤 프로젝트 내 다른 파일들에서 직접 문자열 대신 새 공용 상수(예:
.todayWhatWatchSyncDidRequest)를 사용하도록 변경하세요; 이때 접근 제어를 public으로 설정해 모듈 경계를 넘겨
사용할 수 있게 하고, 기존 문자열 리터럴(파일 내외부, 특히 언급된 49-54 라인 영역)을 모두 찾아 교체해 오탈자 위험을 제거하세요.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f81455e3-6965-4536-b5cd-56a0f0c77d37
📒 Files selected for processing (5)
Projects/App/iOS/Sources/Application/AppDelegate.swiftProjects/App/watchOS/Sources/Manager/WatchSessionManager.swiftProjects/App/watchOS/Sources/Scenes/Main/MainView.swiftProjects/Feature/ModifyTimeTableFeature/Sources/ModifyTimeTableCore.swiftProjects/Feature/SettingsFeature/Sources/SettingsCore.swift
| return | ||
| } | ||
|
|
||
| let timeTables = decodeTimeTables(data: timeTablesData) |
Contributor
There was a problem hiding this comment.
외부 페이로드 디코딩에 try!를 사용하면 크래시 위험이 큽니다.
watch 통신 데이터가 손상되거나 스키마가 어긋나면 즉시 크래시 납니다. 안전 디코딩으로 바꿔야 합니다.
제안 패치
- private func decodeTimeTables(data: Data) -> [ModifiedTimeTableLocalEntity] {
- let entities = try! JSONDecoder().decode([ModifiedTimeTableLocalEntity].self, from: data)
- return entities
- }
+ private func decodeTimeTables(data: Data) -> [ModifiedTimeTableLocalEntity]? {
+ try? JSONDecoder().decode([ModifiedTimeTableLocalEntity].self, from: data)
+ }
...
- let timeTables = decodeTimeTables(data: timeTablesData)
+ guard let timeTables = decodeTimeTables(data: timeTablesData) else { return }Also applies to: 90-93
🤖 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/App/watchOS/Sources/Manager/WatchSessionManager.swift` at line 108,
The call to decodeTimeTables using force-unwrap (try!) is unsafe and can crash
when incoming watch payloads are corrupt; change uses of try! (including the
similar calls around lines 90-93) to a do-catch or try? pattern so decoding
errors are caught, log the decoding error (use the existing logger or os_log)
and handle failure by returning an empty/default value or early-returning
gracefully instead of crashing; specifically update the call site that invokes
decodeTimeTables(...) and any other decoding calls in WatchSessionManager to
call the throwing decoder safely and handle the thrown error in the catch block.
Comment on lines
+123
to
+127
| try? self.localDatabaseClient.deleteAll(record: ModifiedTimeTableLocalEntity.self) | ||
| try? self.localDatabaseClient.save(records: timeTables) | ||
| DispatchQueue.main.async { | ||
| self.syncVersion += 1 | ||
| } |
Contributor
There was a problem hiding this comment.
삭제/저장 실패를 무시하면 데이터 유실 후에도 syncVersion이 증가할 수 있습니다.
try?로 실패를 삼키지 말고, 저장 성공 시점에만 버전을 올리도록 처리해 주세요.
제안 패치
- try? self.localDatabaseClient.deleteAll(record: ModifiedTimeTableLocalEntity.self)
- try? self.localDatabaseClient.save(records: timeTables)
- DispatchQueue.main.async {
- self.syncVersion += 1
- }
+ do {
+ try self.localDatabaseClient.deleteAll(record: ModifiedTimeTableLocalEntity.self)
+ try self.localDatabaseClient.save(records: timeTables)
+ DispatchQueue.main.async {
+ self.syncVersion += 1
+ }
+ } catch {
+ // 필요 시 로깅 추가
+ return
+ }🤖 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/App/watchOS/Sources/Manager/WatchSessionManager.swift` around lines
123 - 127, The current code in WatchSessionManager silently ignores failures
from localDatabaseClient.deleteAll(record: ModifiedTimeTableLocalEntity.self)
and localDatabaseClient.save(records: timeTables) using try?, which can advance
syncVersion even if the write failed; change this to perform the delete and save
inside a do/catch (or use the clients' completion callbacks) and only on
successful save increment syncVersion on the main queue
(DispatchQueue.main.async { self.syncVersion += 1 }); if save fails,
log/propagate the error and do not change syncVersion (optionally rollback or
handle partial state as appropriate).
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.
요약
updateApplicationContext와sendMessage를 함께 사용해 백그라운드 최신 상태 반영과 포그라운드 즉시 반영을 모두 보장테스트
xcodebuild build -workspace TodayWhat.xcworkspace -scheme TodayWhat-DEV -destination 'generic/platform=iOS' CODE_SIGN_IDENTITY='' CODE_SIGNING_REQUIRED=NO CODE_SIGNING_ALLOWED=NOSummary by CodeRabbit
릴리스 노트