Bug fix: Correctly save objectId of Installation (#116)

* Bug fix: Save objectId of Installation to Keychain

* Changelog entry

* nit

* Disable test that doesn't work on Linux

* Update .codecov.yml

* Update .codecov.yml

* Update changelog

* Improve codecov

* Update .codecov.yml

* Remove requirement of being logged in to save current installation and config to keychain

* regression in playgrounds. Add directions for addressing update in changelog

* Migrate old installations automatically

* Testing Linux

* Don't run migration test on Linux

* nits

Author: cbaker6

.codecov.yml

@@ -5,7 +5,7 @@ coverage:
   status:
     patch:
       default:
-        target: auto
+        target: 58
     changes: false
     project:
       default:

CHANGELOG.md

@@ -10,6 +10,9 @@
 __Improvements__
 - (Breaking Change) No longer require dispatch to main queue when using ParseInstallation. The side effect of this is bade is no longer retrieved by the SDK. The developer should retrieve the badge count on their own and save it to `ParseInstallation` if they require badge ([#114](https://github.com/parse-community/Parse-Swift/pull/114)), thanks to [Corey Baker](https://github.com/cbaker6).
 
+__Fixes__
+- (Breaking Change) Correctly saves objectId of ParseInstallation to Keychain when saving to server. Also fixes issue when using deleteAll with current ParseUser and ParseInstallation. Old installations will automatically be migrated to the new one. If you end up having issues you can delete all of the installations in your ParseDashboard that were created with Parse-Swift < 1.30. If you are not able to do this, you can all log out of devices using Parse-Swift < 1.30 and then log back in ([#116](https://github.com/parse-community/Parse-Swift/pull/116)), thanks to [Corey Baker](https://github.com/cbaker6).
+
 ### 1.2.6
 [Full Changelog](https://github.com/parse-community/Parse-Swift/compare/1.2.5...1.2.6)
 
@@ -58,14 +61,12 @@ __Fixes__
 ### 1.2.0
 [Full Changelog](https://github.com/parse-community/Parse-Swift/compare/1.1.6...1.2.0)
 
-__Breaking changes__
-- Allows return types to be specified for `ParseCloud`, query `hint`, and `explain` (see playgrounds for examples). Changed functionality of synchronous `query.first()`. It use to return nil if no values are found. Now it will throw an error if none are found. ([#92](https://github.com/parse-community/Parse-Swift/pull/92)), thanks to [Corey Baker](https://github.com/cbaker6).
-
 __New features__
 - Add transaction support to batch saveAll and deleteAll ([#89](https://github.com/parse-community/Parse-Swift/pull/89)), thanks to [Corey Baker](https://github.com/cbaker6).
 - Add modifiers to containsString, hasPrefix, hasSuffix ([#85](https://github.com/parse-community/Parse-Swift/pull/85)), thanks to [Corey Baker](https://github.com/cbaker6).
 
 __Improvements__
+- (Breaking Change) Allows return types to be specified for `ParseCloud`, query `hint`, and `explain` (see playgrounds for examples). Changed functionality of synchronous `query.first()`. It use to return nil if no values are found. Now it will throw an error if none are found. ([#92](https://github.com/parse-community/Parse-Swift/pull/92)), thanks to [Corey Baker](https://github.com/cbaker6).
 - Better error reporting when decode errors occur ([#92](https://github.com/parse-community/Parse-Swift/pull/92)), thanks to [Corey Baker](https://github.com/cbaker6).
 - Can use a variadic version of exclude. Added examples of select and exclude query in playgrounds ([#88](https://github.com/parse-community/Parse-Swift/pull/88)), thanks to [Corey Baker](https://github.com/cbaker6).
 

ParseSwift.playground/Pages/2 - Finding Objects.xcplaygroundpage/Contents.swift

@@ -21,7 +21,7 @@ struct GameScore: ParseObject {
 
     //: Your own properties.
     var score: Int?
-    var timeStamp = Date()
+    var timeStamp: Date? = Date()
     var oldScore: Int?
 }
 

ParseSwift.playground/Pages/4 - User - Continued.xcplaygroundpage/Contents.swift

@@ -131,7 +131,7 @@ do {
     print("Error logging out: \(error)")
 }
 
-//: Password Reset Request - synchronously.
+//: Verification Email - synchronously.
 do {
     try User.verificationEmail(email: "[email protected]")
     print("Successfully requested verification email be sent")

ParseSwift.playground/Pages/6 - Installation.xcplaygroundpage/Contents.swift

@@ -53,5 +53,21 @@ Installation.current?.save { results in
     }
 }
 
+/*: Update your `ParseInstallation` `customKey` value.
+    Performs work on background queue and returns to designated on
+    designated callbackQueue. If no callbackQueue is specified it
+    returns to main queue.
+ */
+Installation.current?.customKey = "updatedValue"
+Installation.current?.save { results in
+
+    switch results {
+    case .success(let updatedInstallation):
+        print("Successfully save myCustomInstallationKey to ParseServer: \(updatedInstallation)")
+    case .failure(let error):
+        print("Failed to update installation: \(error)")
+    }
+}
+
 PlaygroundPage.current.finishExecution()
 //: [Next](@next)

ParseSwift.playground/Pages/7 - GeoPoint.xcplaygroundpage/Contents.swift

@@ -58,7 +58,6 @@ score.save { result in
     case .failure(let error):
         assertionFailure("Error saving: \(error)")
     }
-
 }
 
 //: Now we will show how to query based on the `ParseGeoPoint`.

ParseSwift.xcodeproj/project.pbxproj

@@ -295,6 +295,9 @@
 		70D1BE7425BB43EB00A42E7C /* BaseConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70D1BE7225BB43EB00A42E7C /* BaseConfig.swift */; };
 		70D1BE7525BB43EB00A42E7C /* BaseConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70D1BE7225BB43EB00A42E7C /* BaseConfig.swift */; };
 		70D1BE7625BB43EB00A42E7C /* BaseConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70D1BE7225BB43EB00A42E7C /* BaseConfig.swift */; };
+		70DFEA8A2618E77800F8EB4B /* MigrateOldParseInstallationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70DFEA892618E77800F8EB4B /* MigrateOldParseInstallationTests.swift */; };
+		70DFEA8B2618E77800F8EB4B /* MigrateOldParseInstallationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70DFEA892618E77800F8EB4B /* MigrateOldParseInstallationTests.swift */; };
+		70DFEA8C2618E77800F8EB4B /* MigrateOldParseInstallationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70DFEA892618E77800F8EB4B /* MigrateOldParseInstallationTests.swift */; };
 		70F2E255254F247000B2EA5C /* ParseSwift.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 4AFDA7121F26D9A5002AE4FC /* ParseSwift.framework */; };
 		70F2E2B3254F283000B2EA5C /* ParseUserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70C7DC1D24D20E530050419B /* ParseUserTests.swift */; };
 		70F2E2B4254F283000B2EA5C /* ParseQueryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70C7DC1F24D20F180050419B /* ParseQueryTests.swift */; };
@@ -632,6 +635,7 @@
 		70D1BDB925BB17A600A42E7C /* ParseConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ParseConfig.swift; sourceTree = "<group>"; };
 		70D1BE0625BB2BF400A42E7C /* ParseConfigTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ParseConfigTests.swift; sourceTree = "<group>"; };
 		70D1BE7225BB43EB00A42E7C /* BaseConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BaseConfig.swift; sourceTree = "<group>"; };
+		70DFEA892618E77800F8EB4B /* MigrateOldParseInstallationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MigrateOldParseInstallationTests.swift; sourceTree = "<group>"; };
 		70F2E23E254F246000B2EA5C /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
 		70F2E250254F247000B2EA5C /* ParseSwiftTestsmacOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = ParseSwiftTestsmacOS.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
 		70F2E254254F247000B2EA5C /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
@@ -810,6 +814,7 @@
 				7003957525A0EE770052CB31 /* BatchUtilsTests.swift */,
 				705726ED2592C91C00F0ADD5 /* HashTests.swift */,
 				4AA8076E1F794C1C008CD551 /* KeychainStoreTests.swift */,
+				70DFEA892618E77800F8EB4B /* MigrateOldParseInstallationTests.swift */,
 				9194657724F16E330070296B /* ParseACLTests.swift */,
 				7044C22C25C5E4E90011F6E7 /* ParseAnonymousCombineTests.swift */,
 				70A2D86A25B3ADB6001BEB7D /* ParseAnonymousTests.swift */,
@@ -1668,6 +1673,7 @@
 				70732C5A2606CCAD000CAB81 /* ParseObjectCustomObjectIdTests.swift in Sources */,
 				911DB12C24C3F7720027F3C7 /* MockURLResponse.swift in Sources */,
 				7044C24325C5EA360011F6E7 /* ParseAppleCombineTests.swift in Sources */,
+				70DFEA8A2618E77800F8EB4B /* MigrateOldParseInstallationTests.swift in Sources */,
 				7044C1DF25C5C70D0011F6E7 /* ParseObjectCombine.swift in Sources */,
 				89899D9F26045998002E2043 /* ParseTwitterCombineTests.swift in Sources */,
 				70C5504625B40D5200B5DBC2 /* ParseSessionTests.swift in Sources */,
@@ -1819,6 +1825,7 @@
 				70732C5C2606CCAD000CAB81 /* ParseObjectCustomObjectIdTests.swift in Sources */,
 				709B984D2556ECAA00507778 /* AnyDecodableTests.swift in Sources */,
 				7044C24525C5EA360011F6E7 /* ParseAppleCombineTests.swift in Sources */,
+				70DFEA8C2618E77800F8EB4B /* MigrateOldParseInstallationTests.swift in Sources */,
 				7044C1E125C5C70D0011F6E7 /* ParseObjectCombine.swift in Sources */,
 				89899DA126045998002E2043 /* ParseTwitterCombineTests.swift in Sources */,
 				70C5504825B40D5200B5DBC2 /* ParseSessionTests.swift in Sources */,
@@ -1873,6 +1880,7 @@
 				70732C5B2606CCAD000CAB81 /* ParseObjectCustomObjectIdTests.swift in Sources */,
 				70F2E2C2254F283000B2EA5C /* APICommandTests.swift in Sources */,
 				7044C24425C5EA360011F6E7 /* ParseAppleCombineTests.swift in Sources */,
+				70DFEA8B2618E77800F8EB4B /* MigrateOldParseInstallationTests.swift in Sources */,
 				7044C1E025C5C70D0011F6E7 /* ParseObjectCombine.swift in Sources */,
 				89899DA026045998002E2043 /* ParseTwitterCombineTests.swift in Sources */,
 				70C5504725B40D5200B5DBC2 /* ParseSessionTests.swift in Sources */,

README.md

@@ -44,7 +44,7 @@ import PackageDescription
 let package = Package(
     name: "YOUR_PROJECT_NAME",
     dependencies: [
-        .package(url: "https://github.com/parse-community/Parse-Swift", from: "1.1.2"),
+        .package(url: "https://github.com/parse-community/Parse-Swift", from: "1.3.0"),
     ]
 )
 ```

Sources/ParseSwift/Objects/ParseInstallation.swift

@@ -100,6 +100,10 @@ extension ParseInstallation {
         }
     }
 
+    func hasSameInstallationId<T: ParseInstallation>(as other: T) -> Bool {
+        return other.className == className && other.installationId == installationId && installationId != nil
+    }
+
     /**
      Sets the device token string property from an `Data`-encoded token.
      - parameter data: A token that identifies the device.
@@ -303,23 +307,22 @@ extension ParseInstallation {
 
 // MARK: Fetchable
 extension ParseInstallation {
-    internal static func updateKeychainIfNeeded(_ results: [Self], deleting: Bool = false) {
-        guard BaseParseUser.current != nil,
-              let currentInstallation = BaseParseInstallation.current else {
+    internal static func updateKeychainIfNeeded(_ results: [Self], deleting: Bool = false) throws {
+        guard let currentInstallation = BaseParseInstallation.current else {
             return
         }
 
-        var saveInstallation: Self?
-        let foundCurrentInstallationObjects = results.filter { $0.hasSameObjectId(as: currentInstallation) }
+        var foundCurrentInstallationObjects = results.filter { $0.hasSameInstallationId(as: currentInstallation) }
+        foundCurrentInstallationObjects = try foundCurrentInstallationObjects.sorted(by: {
+            if $0.updatedAt == nil || $1.updatedAt == nil {
+                throw ParseError(code: .unknownError,
+                                 message: "Objects from the server should always have an 'updatedAt'")
+            }
+            return $0.updatedAt!.compare($1.updatedAt!) == .orderedDescending
+        })
         if let foundCurrentInstallation = foundCurrentInstallationObjects.first {
-            saveInstallation = foundCurrentInstallation
-        } else {
-            saveInstallation = results.first
-        }
-
-        if saveInstallation != nil {
             if !deleting {
-                Self.current = saveInstallation
+                Self.current = foundCurrentInstallation
                 Self.saveCurrentContainerToKeychain()
             } else {
                 Self.deleteCurrentContainerFromKeychain()
@@ -341,7 +344,7 @@ extension ParseInstallation {
                       options: API.Options = []) throws -> Self {
         let result: Self = try fetchCommand(include: includeKeys)
             .execute(options: options, callbackQueue: .main)
-        Self.updateKeychainIfNeeded([result])
+        try Self.updateKeychainIfNeeded([result])
         return result
     }
 
@@ -369,9 +372,21 @@ extension ParseInstallation {
                               callbackQueue: callbackQueue) { result in
                     callbackQueue.async {
                         if case .success(let foundResult) = result {
-                            Self.updateKeychainIfNeeded([foundResult])
+                            do {
+                                try Self.updateKeychainIfNeeded([foundResult])
+                                completion(.success(foundResult))
+                            } catch {
+                                let returnError: ParseError!
+                                if let parseError = error as? ParseError {
+                                    returnError = parseError
+                                } else {
+                                    returnError = ParseError(code: .unknownError, message: error.localizedDescription)
+                                }
+                                completion(.failure(returnError))
+                            }
+                        } else {
+                            completion(result)
                         }
-                        completion(result)
                     }
                 }
          } catch {
@@ -438,7 +453,7 @@ extension ParseInstallation {
                      callbackQueue: .main,
                      childObjects: childObjects,
                      childFiles: childFiles)
-        Self.updateKeychainIfNeeded([result])
+        try Self.updateKeychainIfNeeded([result])
         return result
     }
 
@@ -466,10 +481,22 @@ extension ParseInstallation {
                                       childFiles: savedChildFiles) { result in
                             callbackQueue.async {
                                 if case .success(let foundResults) = result {
-                                    Self.updateKeychainIfNeeded([foundResults])
+                                    do {
+                                        try Self.updateKeychainIfNeeded([foundResults])
+                                        completion(.success(foundResults))
+                                    } catch {
+                                        let returnError: ParseError!
+                                        if let parseError = error as? ParseError {
+                                            returnError = parseError
+                                        } else {
+                                            returnError = ParseError(code: .unknownError,
+                                                                     message: error.localizedDescription)
+                                        }
+                                        completion(.failure(returnError))
+                                    }
+                                } else {
+                                    completion(result)
                                 }
-
-                                completion(result)
                             }
                         }
                 } catch {
@@ -533,7 +560,7 @@ extension ParseInstallation {
     */
     public func delete(options: API.Options = []) throws {
         _ = try deleteCommand().execute(options: options)
-        Self.updateKeychainIfNeeded([self], deleting: true)
+        try Self.updateKeychainIfNeeded([self], deleting: true)
     }
 
     /**
@@ -558,8 +585,18 @@ extension ParseInstallation {
                         switch result {
 
                         case .success:
-                            Self.updateKeychainIfNeeded([self], deleting: true)
-                            completion(.success(()))
+                            do {
+                                try Self.updateKeychainIfNeeded([self], deleting: true)
+                                completion(.success(()))
+                            } catch {
+                                let returnError: ParseError!
+                                if let parseError = error as? ParseError {
+                                    returnError = parseError
+                                } else {
+                                    returnError = ParseError(code: .unknownError, message: error.localizedDescription)
+                                }
+                                completion(.failure(returnError))
+                            }
                         case .failure(let error):
                             completion(.failure(error))
                         }
@@ -678,7 +715,7 @@ public extension Sequence where Element: ParseInstallation {
                          childFiles: childFiles)
             returnBatch.append(contentsOf: currentBatch)
         }
-        Self.Element.updateKeychainIfNeeded(returnBatch.compactMap {try? $0.get()})
+        try Self.Element.updateKeychainIfNeeded(returnBatch.compactMap {try? $0.get()})
         return returnBatch
     }
 
@@ -779,7 +816,7 @@ public extension Sequence where Element: ParseInstallation {
                             returnBatch.append(contentsOf: saved)
                             if completed == (batches.count - 1) {
                                 callbackQueue.async {
-                                    Self.Element.updateKeychainIfNeeded(returnBatch.compactMap {try? $0.get()})
+                                    try? Self.Element.updateKeychainIfNeeded(returnBatch.compactMap {try? $0.get()})
                                     completion(.success(returnBatch))
                                 }
                             }
@@ -842,7 +879,7 @@ public extension Sequence where Element: ParseInstallation {
                                                                       message: "objectId \"\(uniqueObjectId)\" was not found in className \"\(Self.Element.className)\"")))
                 }
             }
-            Self.Element.updateKeychainIfNeeded(fetchedObjects)
+            try Self.Element.updateKeychainIfNeeded(fetchedObjects)
             return fetchedObjectsToReturn
         } else {
             throw ParseError(code: .unknownError, message: "all items to fetch must be of the same class")
@@ -891,7 +928,7 @@ public extension Sequence where Element: ParseInstallation {
                         }
                     }
                     callbackQueue.async {
-                        Self.Element.updateKeychainIfNeeded(fetchedObjects)
+                        try? Self.Element.updateKeychainIfNeeded(fetchedObjects)
                         completion(.success(fetchedObjectsToReturn))
                     }
                 case .failure(let error):
@@ -950,7 +987,8 @@ public extension Sequence where Element: ParseInstallation {
             returnBatch.append(contentsOf: currentBatch)
         }
 
-        Self.Element.updateKeychainIfNeeded(compactMap {$0})
+        try Self.Element.updateKeychainIfNeeded(compactMap {$0},
+                                                deleting: true)
         return returnBatch
     }
 
@@ -1006,7 +1044,8 @@ public extension Sequence where Element: ParseInstallation {
                         returnBatch.append(contentsOf: saved)
                         if completed == (batches.count - 1) {
                             callbackQueue.async {
-                                Self.Element.updateKeychainIfNeeded(self.compactMap {$0})
+                                try? Self.Element.updateKeychainIfNeeded(self.compactMap {$0},
+                                                                         deleting: true)
                                 completion(.success(returnBatch))
                             }
                         }