Bug 1373276 - Call POST /account/devices/notify in the send tab exten… (

#2852)

* Bug 1373276 - Call POST /account/devices/notify in the send tab extension

* Fixed tests.

Account/FirefoxAccount.swift

@@ -178,6 +178,18 @@ open class FirefoxAccount {
         }
     }
 
+    @discardableResult open func notify(deviceIDs: [GUID], collectionsChanged collections: [String]) -> Deferred<Maybe<FxANotifyResponse>> {
+        let cachedState = stateCache.value!
+        if let session = cachedState as? TokenState {
+            let client = FxAClient10(endpoint: self.configuration.authEndpointURL)
+            return client.notify(deviceIDs: deviceIDs, collectionsChanged: collections, withSessionToken: session.sessionToken as NSData)
+        }
+
+        let deferred = Deferred<Maybe<FxANotifyResponse>>()
+        deferred.fill(Maybe(failure: FxAClientError.local(NSError())))
+        return deferred
+    }
+
     @discardableResult open func advance() -> Deferred<FxAState> {
         OSSpinLockLock(&advanceLock)
         if let deferred = advanceDeferred {

Account/FxAClient10.swift

@@ -52,6 +52,10 @@ public struct FxADevicesResponse {
     let devices: [FxADevice]
 }
 
+public struct FxANotifyResponse {
+    let success: Bool
+}
+
 // fxa-auth-server produces error details like:
 //        {
 //            "code": 400, // matches the HTTP status code
@@ -235,6 +239,10 @@ open class FxAClient10 {
 
         return FxADevicesResponse(devices: devices)
     }
+
+    fileprivate class func notifyResponse(fromJSON json: JSON) -> FxANotifyResponse {
+        return FxANotifyResponse(success: json.error == nil)
+    }
 
     lazy fileprivate var alamofire: SessionManager = {
         let ua = UserAgent.fxaUserAgent
@@ -289,6 +297,34 @@ open class FxAClient10 {
 
         return makeRequest(mutableURLRequest, responseHandler: FxAClient10.devicesResponse)
     }
+
+    open func notify(deviceIDs: [GUID], collectionsChanged collections: [String], withSessionToken sessionToken: NSData) -> Deferred<Maybe<FxANotifyResponse>> {
+        let URL = self.URL.appendingPathComponent("/account/devices/notify")
+        var mutableURLRequest = URLRequest(url: URL)
+        mutableURLRequest.httpMethod = HTTPMethod.post.rawValue
+
+        mutableURLRequest.setValue("application/json", forHTTPHeaderField: "Content-Type")
+
+        let httpBody = JSON([
+            "to": deviceIDs,
+            "payload": [
+                "version": 1,
+                "command": "sync:collection_changed",
+                "data": [
+                    "collections": collections
+                ]
+            ]
+        ])
+
+        mutableURLRequest.httpBody = httpBody.stringValue()?.utf8EncodedData
+
+        let salt: Data = Data()
+        let contextInfo: Data = FxAClient10.KW("sessionToken")
+        let key = sessionToken.deriveHKDFSHA256Key(withSalt: salt, contextInfo: contextInfo, length: UInt(2 * KeyLength))!
+        mutableURLRequest.addAuthorizationHeader(forHKDFSHA256Key: key)
+
+        return makeRequest(mutableURLRequest, responseHandler: FxAClient10.notifyResponse)
+    }
 
     open func registerOrUpdate(device: FxADevice, withSessionToken sessionToken: NSData) -> Deferred<Maybe<FxADevice>> {
         let URL = self.URL.appendingPathComponent("/account/device")

ClientTests/ResetTests.swift

@@ -55,7 +55,7 @@ class ResetTests: XCTestCase {
 
         // Add a client.
         let tabs = profile.peekTabs
-        XCTAssertTrue(tabs.insertOrUpdateClient(RemoteClient(guid: "abcdefghijkl", name: "Remote", modified: Date.now(), type: "mobile", formfactor: "tablet", os: "Windows", version: "55.0.1a")).value.isSuccess)
+        XCTAssertTrue(tabs.insertOrUpdateClient(RemoteClient(guid: "abcdefghijkl", name: "Remote", modified: Date.now(), type: "mobile", formfactor: "tablet", os: "Windows", version: "55.0.1a", fxaDeviceId: nil)).value.isSuccess)
 
         // Verify that it's there.
         assertClientsHaveGUIDsFromStorage(tabs, expected: ["abcdefghijkl"])

Providers/Profile.swift

@@ -470,7 +470,21 @@ open class BrowserProfile: Profile {
         let commands = items.map { item in
             SyncCommand.displayURIFromShareItem(item, asClient: id)
         }
-        return self.remoteClientsAndTabs.insertCommands(commands, forClients: clients) >>> { self.syncManager.syncClients() }
+
+        func notifyClients() {
+            let deviceIDs = clients.flatMap { $0.fxaDeviceId }
+            guard let account = self.getAccount() else {
+                return
+            }
+
+            account.notify(deviceIDs: deviceIDs, collectionsChanged: ["clients"])
+        }
+
+        return self.remoteClientsAndTabs.insertCommands(commands, forClients: clients) >>> {
+            let syncStatus = self.syncManager.syncClients()
+            syncStatus >>> notifyClients
+            return syncStatus
+        }
     }
 
     lazy var logins: BrowserLogins & SyncableLogins & ResettableSyncStorage = {

Storage/Clients.swift

@@ -13,6 +13,7 @@ public struct RemoteClient: Equatable {
     public let type: String?
     public let os: String?
     public let version: String?
+    public let fxaDeviceId: String?
 
     let protocols: [String]?
 
@@ -35,16 +36,18 @@ public struct RemoteClient: Equatable {
         self.application = json["application"].string
         self.formfactor = json["formfactor"].string
         self.device = json["device"].string
+        self.fxaDeviceId = json["fxaDeviceId"].string
     }
 
-    public init(guid: GUID?, name: String, modified: Timestamp, type: String?, formfactor: String?, os: String?, version: String?) {
+    public init(guid: GUID?, name: String, modified: Timestamp, type: String?, formfactor: String?, os: String?, version: String?, fxaDeviceId: String?) {
         self.guid = guid
         self.name = name
         self.modified = modified
         self.type = type
         self.formfactor = formfactor
         self.os = os
         self.version = version
+        self.fxaDeviceId = fxaDeviceId
 
         self.device = nil
         self.appPackage = nil
@@ -61,11 +64,12 @@ public func ==(lhs: RemoteClient, rhs: RemoteClient) -> Bool {
         lhs.type == rhs.type &&
         lhs.formfactor == rhs.formfactor &&
         lhs.os == rhs.os &&
-        lhs.version == rhs.version
+        lhs.version == rhs.version &&
+        lhs.fxaDeviceId == rhs.fxaDeviceId
 }
 
 extension RemoteClient: CustomStringConvertible {
     public var description: String {
-        return "<RemoteClient GUID: \(guid ?? "nil"), name: \(name), modified: \(modified), type: \(type ?? "nil"), formfactor: \(formfactor ?? "nil"), OS: \(os ?? "nil"), version: \(version ?? "nil")>"
+        return "<RemoteClient GUID: \(guid ?? "nil"), name: \(name), modified: \(modified), type: \(type ?? "nil"), formfactor: \(formfactor ?? "nil"), OS: \(os ?? "nil"), version: \(version ?? "nil"), fxaDeviceId: \(fxaDeviceId ?? "nil")>"
     }
 }

Storage/SQL/RemoteTabsTable.swift

@@ -12,7 +12,7 @@ private let log = Logger.syncLogger
 
 class RemoteClientsTable<T>: GenericTable<RemoteClient> {
     override var name: String { return TableClients }
-    override var version: Int { return 2 }
+    override var version: Int { return 3 }
 
     // TODO: index on guid and last_modified.
     override var rows: String { return [
@@ -23,6 +23,7 @@ class RemoteClientsTable<T>: GenericTable<RemoteClient> {
             "formfactor TEXT",
             "os TEXT",
             "version TEXT",
+            "fxaDeviceId TEXT",
         ].joined(separator: ",")
     }
 
@@ -36,8 +37,9 @@ class RemoteClientsTable<T>: GenericTable<RemoteClient> {
             item.formfactor,
             item.os,
             item.version,
+            item.fxaDeviceId,
         ]
-        return ("INSERT INTO \(name) (guid, name, modified, type, formfactor, os, version) VALUES (?, ?, ?, ?, ?, ?, ?)", args)
+        return ("INSERT INTO \(name) (guid, name, modified, type, formfactor, os, version, fxaDeviceId) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", args)
     }
 
     override func getUpdateAndArgs(_ item: inout RemoteClient) -> (String, Args)? {
@@ -48,10 +50,11 @@ class RemoteClientsTable<T>: GenericTable<RemoteClient> {
             item.formfactor,
             item.os,
             item.version,
+            item.fxaDeviceId,
             item.guid,
         ]
 
-        return ("UPDATE \(name) SET name = ?, modified = ?, type = ?, formfactor = ?, os = ?, version = ? WHERE guid = ?", args)
+        return ("UPDATE \(name) SET name = ?, modified = ?, type = ?, formfactor = ?, os = ?, version = ?, fxaDeviceId = ? WHERE guid = ?", args)
     }
 
     override func getDeleteAndArgs(_ item: inout RemoteClient?) -> (String, Args)? {
@@ -71,7 +74,8 @@ class RemoteClientsTable<T>: GenericTable<RemoteClient> {
             let form = row["formfactor"] as? String
             let os = row["os"] as? String
             let version = row["version"] as? String
-            return RemoteClient(guid: guid, name: name, modified: mod, type: type, formfactor: form, os: os, version: version)
+            let fxaDeviceId = row["fxaDeviceId"] as? String
+            return RemoteClient(guid: guid, name: name, modified: mod, type: type, formfactor: form, os: os, version: version, fxaDeviceId: fxaDeviceId)
         }
     }
 
@@ -95,6 +99,16 @@ class RemoteClientsTable<T>: GenericTable<RemoteClient> {
                 return false
             }
         }
+
+        if from < 3 && to >= 3 {
+            let sql = "ALTER TABLE \(TableClients) ADD COLUMN fxaDeviceId TEXT"
+            let err = db.executeChange(sql)
+            if err != nil {
+                log.error("Error running SQL in RemoteClientsTable: \(err?.localizedDescription ?? "nil")")
+                log.error("SQL was \(sql)")
+                return false
+            }
+        }
 
         return true
     }

StorageTests/SyncCommandsTests.swift

@@ -43,9 +43,9 @@ class SyncCommandsTests: XCTestCase {
         let client2GUID = Bytes.generateGUID()
         let client3GUID = Bytes.generateGUID()
 
-        self.clients.append(RemoteClient(guid: client1GUID, name: "Test client 1", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1"))
-        self.clients.append(RemoteClient(guid: client2GUID, name: "Test client 2", modified: (now - OneHourInMilliseconds), type: "desktop", formfactor: "laptop", os: "Darwin", version: "55.0.1"))
-        self.clients.append(RemoteClient(guid: client3GUID, name: "Test local client", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1"))
+        self.clients.append(RemoteClient(guid: client1GUID, name: "Test client 1", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1", fxaDeviceId: nil))
+        self.clients.append(RemoteClient(guid: client2GUID, name: "Test client 2", modified: (now - OneHourInMilliseconds), type: "desktop", formfactor: "laptop", os: "Darwin", version: "55.0.1", fxaDeviceId: nil))
+        self.clients.append(RemoteClient(guid: client3GUID, name: "Test local client", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1", fxaDeviceId: nil))
         clientsAndTabs = SQLiteRemoteClientsAndTabs(db: db)
         clientsAndTabs.insertOrUpdateClients(clients).succeeded()
 

StorageTests/TestSQLiteRemoteClientsAndTabs.swift

@@ -27,10 +27,10 @@ open class MockRemoteClientsAndTabs: RemoteClientsAndTabs {
         let u22 = URL(string: "http://different.com/test2")!
         let tab22 = RemoteTab(clientGUID: client2GUID, URL: u22, title: "Different Test 2", history: [], lastUsed: now + OneHourInMilliseconds, icon: nil)
 
-        let client1 = RemoteClient(guid: client1GUID, name: "Test client 1", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1")
-        let client2 = RemoteClient(guid: client2GUID, name: "Test client 2", modified: (now - OneHourInMilliseconds), type: "desktop", formfactor: "laptop", os: "Darwin", version: "55.0.1")
+        let client1 = RemoteClient(guid: client1GUID, name: "Test client 1", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1", fxaDeviceId: nil)
+        let client2 = RemoteClient(guid: client2GUID, name: "Test client 2", modified: (now - OneHourInMilliseconds), type: "desktop", formfactor: "laptop", os: "Darwin", version: "55.0.1", fxaDeviceId: nil)
 
-        let localClient = RemoteClient(guid: nil, name: "Test local client", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1")
+        let localClient = RemoteClient(guid: nil, name: "Test local client", modified: (now - OneMinuteInMilliseconds), type: "mobile", formfactor: "largetablet", os: "iOS", version: "55.0.1", fxaDeviceId: nil)
         let localUrl1 = URL(string: "http://test.com/testlocal1")!
         let localTab1 = RemoteTab(clientGUID: nil, URL: localUrl1, title: "Local test 1", history: [], lastUsed: (now - OneMinuteInMilliseconds), icon: nil)
         let localUrl2 = URL(string: "http://test.com/testlocal2")!

Sync/Synchronizers/ClientsSynchronizer.swift

@@ -409,21 +409,19 @@ open class ClientsSynchronizer: TimestampedSingleCollectionSynchronizer, Synchro
 
         let clientsClient = storageClient.clientForCollection(self.collection, encrypter: encrypter!)
 
-        if !self.remoteHasChanges(info) {
-            log.debug("No remote changes for clients. (Last fetched \(self.lastFetched).)")
-            statsSession.start()
-            return self.maybeUploadOurRecord(false, ifUnmodifiedSince: nil, toServer: clientsClient)
-                >>> { self.uploadClientCommands(toLocalClients: localClients, withServer: clientsClient) }
-                >>> { deferMaybe(self.completedWithStats) }
-        }
-
         // TODO: some of the commands we process might involve wiping collections or the
         // entire profile. We should model this as an explicit status, and return it here
         // instead of .completed.
         statsSession.start()
-        return clientsClient.getSince(self.lastFetched)
+        // XXX: This is terrible. We always force a re-sync of the clients to work around
+        // the fact that `fxaDeviceId` may not have been populated if the list of clients
+        // hadn't changed since before the update to v8.0. To force a re-sync, we get all
+        // clients since the beginning of time instead of looking at `self.lastFetched`.
+        // We also call `localClients.wipeClients()` directly instead of relying on
+        // `self.wipeIfNecessary(localClients)` to force the list to always be current.
+        return clientsClient.getSince(0)
             >>== { response in
-                return self.wipeIfNecessary(localClients)
+                return localClients.wipeClients()
                     >>> { self.applyStorageResponse(response, toLocalClients: localClients, withServer: clientsClient) }
             }
             >>> { deferMaybe(self.completedWithStats) }