Fix a just-deleted server being readded using old info (#2093)

The cache for server infos was inserting the old 'fallback' value when read after deletion, which caused it to be used if the server was re-added while the app was still in memory afterwards.
home-assistant · Feb 27, 2022 · f88fea3 · f88fea3
1 parent 326d9a2
commit f88fea3
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 3 deletions.
diff --git a/Sources/Shared/API/ServerManager.swift b/Sources/Shared/API/ServerManager.swift
@@ -93,8 +93,18 @@ private extension Identifier where ObjectType == Server {
 private struct ServerCache {
     var restrictCaching: Bool = false
     var deletedServers: Set<Identifier<Server>> = .init()
-    var info: [Identifier<Server>: ServerInfo] = [:]
-    var server: [Identifier<Server>: Server] = [:]
+    var info: [Identifier<Server>: ServerInfo] = [:] {
+        didSet {
+            precondition(deletedServers.isDisjoint(with: info.keys))
+        }
+    }
+
+    var server: [Identifier<Server>: Server] = [:] {
+        didSet {
+            precondition(deletedServers.isDisjoint(with: server.keys))
+        }
+    }
+
     var all: [Server]?
 
     mutating func remove(identifier: Identifier<Server>) {
@@ -208,6 +218,7 @@ internal final class ServerManagerImpl: ServerManager {
         let result = cache.mutate { cache -> Server in
             cache.deletedServers.remove(identifier)
             keychain.set(serverInfo: setValue, key: identifier.keychainKey, encoder: encoder)
+            cache.info[identifier] = setValue
             cache.all = nil
 
             return server(key: identifier.keychainKey, value: setValue, currentCache: &cache)
@@ -261,7 +272,9 @@ internal final class ServerManagerImpl: ServerManager {
                     return info
                 } else {
                     let info = keychain.getServerInfo(key: identifier.keychainKey, decoder: decoder) ?? fallback
-                    cache.info[identifier] = info
+                    if !cache.deletedServers.contains(identifier) {
+                        cache.info[identifier] = info
+                    }
                     return info
                 }
             }

diff --git a/Tests/Shared/ServerManager.test.swift b/Tests/Shared/ServerManager.test.swift
@@ -132,6 +132,9 @@ class ServerManagerTests: XCTestCase {
         }
         try XCTAssertNil(keychain.getData("fake1"))
 
+        // grab it, which may also side-effect insert into cache, if buggy
+        _ = server1.info
+
         expectingObserver {
             // we just deleted it, so we re-add it to make sure _that_ works
             let tempFake1 = with(info1) {