-
-
Notifications
You must be signed in to change notification settings - Fork 134
Fix Shortcuts support for deleted homes with fallback mechanism #1005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,11 +32,23 @@ public actor OpenHABItemCache { | |||||
| } | ||||||
|
|
||||||
| public func getCachedItems(home: UUID) async -> [OpenHABItem]? { | ||||||
| // Validate home exists in storedHomes before trying to load | ||||||
| let storedHomes = await Preferences.shared.storedHomes | ||||||
| guard storedHomes[home] != nil else { | ||||||
| Logger.itemCache.error("Cannot get cached items: home \(home) not found in storedHomes. Available homes: \(storedHomes.keys)") | ||||||
| return nil | ||||||
| } | ||||||
| await reloadCacheIfNeeded(homes: [home]) | ||||||
| return items[home] | ||||||
| } | ||||||
|
|
||||||
| public func getCachedItem(name: String, home: UUID) async -> [OpenHABItem]? { | ||||||
| // Validate home exists in storedHomes before trying to load | ||||||
| let storedHomes = await Preferences.shared.storedHomes | ||||||
| guard storedHomes[home] != nil else { | ||||||
| Logger.itemCache.error("Cannot get cached item: home \(home) not found in storedHomes. Available homes: \(storedHomes.keys)") | ||||||
timbms marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| return nil | ||||||
| } | ||||||
| await reloadCacheIfNeeded(homes: [home]) | ||||||
| return items[home]?.filter { $0.name == name } | ||||||
| } | ||||||
|
|
@@ -86,11 +98,17 @@ public actor OpenHABItemCache { | |||||
| do { | ||||||
| let loadedItems = try await loadNonGroupItemsForHomes(homes) | ||||||
| Logger.itemCache.info("Store loaded items in cache") | ||||||
| homes.forEach { items[$0] = loadedItems[$0] } | ||||||
| // Update cache with loaded items, ensuring we don't write nil | ||||||
| for homeId in homes { | ||||||
| items[homeId] = loadedItems[homeId] ?? [] | ||||||
| } | ||||||
|
Comment on lines
+102
to
+104
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I liked the one liner more, I think it should work with the fallback, too. But feel free to write it according to your taste. |
||||||
| let now = Date.now | ||||||
| homes.forEach { lastLoad[$0] = now } | ||||||
| let itemCounts = items.map { ($0.key, $0.value.count) } | ||||||
| Logger.itemCache.info("Loaded \(itemCounts) items to cache") | ||||||
| // Log only the items that were just loaded, not the entire cache | ||||||
| let justLoadedCounts = loadedItems.map { ($0.key, $0.value.count) } | ||||||
| Logger.itemCache.info("Just loaded \(justLoadedCounts) items") | ||||||
| let totalCacheCount = items.map { ($0.key, $0.value.count) } | ||||||
| Logger.itemCache.info("Total cache now contains \(totalCacheCount) items") | ||||||
| } catch { | ||||||
| Logger.itemCache.error("Could not reload \(error.localizedDescription)") | ||||||
| } | ||||||
|
|
@@ -153,19 +171,34 @@ public actor OpenHABItemCache { | |||||
|
|
||||||
| private func loadItems(homeId: UUID) async -> [OpenHABItem]? { | ||||||
| guard let networkTracker = await assureNetworkTracker(homeId: homeId) else { | ||||||
| Logger.itemCache.error("Home \(homeId) not reachable") | ||||||
| Logger.itemCache.error("Home \(homeId) not reachable - not found in storedHomes") | ||||||
|
||||||
| Logger.itemCache.error("Home \(homeId) not reachable - not found in storedHomes") | |
| Logger.itemCache.error("Home \(homeId, privacy: .private) not reachable - not found in storedHomes") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure whether that new part in the log line is accurate, there may be other reasons like timeouts which cause the network tracker to be nil. Maybe you can replace the "-" with ", potentially because" or just log the failure in the assureNetworkTracker method if you don´t do that already anyway.
timbms marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Home UUID is being logged without privacy settings. Since this could be considered sensitive user data, consider adding privacy modifiers. For example:
Logger.itemCache.error("Failed to load items for home \(homeId, privacy: .private): \(error.localizedDescription)")| Logger.itemCache.error("Failed to load items for home \(homeId): \(error.localizedDescription)") | |
| Logger.itemCache.error("Failed to load items for home \(homeId, privacy: .private): \(error.localizedDescription)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the verbose do-catch syntax we can do a guard let items = try?... here with log and return in the else branch?
timbms marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Home UUIDs are being logged without privacy settings. Since these could be considered sensitive user data, consider adding privacy modifiers. For example:
Logger.itemCache.info("Creating network tracker for home \(homeId, privacy: .private)")
timbms marked this conversation as resolved.
Show resolved
Hide resolved
timbms marked this conversation as resolved.
Show resolved
Hide resolved
timbms marked this conversation as resolved.
Show resolved
Hide resolved
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Home UUIDs and storedHomes keys are being logged without privacy settings. Since these could be considered sensitive user data, consider adding privacy modifiers to these log statements. For example: