From 9213169ef951632f1ef42f4a95568e4746ac2d3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elvis=20Nu=C3=B1ez?= <3lvis@users.noreply.github.com> Date: Fri, 26 Aug 2016 14:09:55 +0200 Subject: [PATCH] Fix bug 225 (#266) * Add unit tests * Add initial unit test * Simplify test case * Add support for clearing removed references * Remove unused parameter * Fix tests * Added more tests * Update comments * Check for null children * Improve consistency between empty and null * Add comment --- Demo.xcodeproj/project.pbxproj | 36 ++++++- Source/NSManagedObject+Sync.swift | 93 ++++++++++++++++--- Tests/JSONs/225-a-empty.json | 6 ++ Tests/JSONs/225-a-null.json | 6 ++ Tests/JSONs/225-a-replaced.json | 10 ++ Tests/JSONs/225-a.json | 10 ++ .../Models/225.xcdatamodeld/.xccurrentversion | 8 ++ .../151-many-to-many.xcdatamodel/contents | 15 +++ Tests/SyncTests.swift | 72 ++++++++++++++ 9 files changed, 241 insertions(+), 15 deletions(-) create mode 100755 Tests/JSONs/225-a-empty.json create mode 100755 Tests/JSONs/225-a-null.json create mode 100755 Tests/JSONs/225-a-replaced.json create mode 100755 Tests/JSONs/225-a.json create mode 100755 Tests/Models/225.xcdatamodeld/.xccurrentversion create mode 100644 Tests/Models/225.xcdatamodeld/151-many-to-many.xcdatamodel/contents diff --git a/Demo.xcodeproj/project.pbxproj b/Demo.xcodeproj/project.pbxproj index 849dc9c9..c331ab26 100644 --- a/Demo.xcodeproj/project.pbxproj +++ b/Demo.xcodeproj/project.pbxproj @@ -74,6 +74,11 @@ 1469B7FB1C68D7D50080FD71 /* notes_with_user_id_custom.json in Resources */ = {isa = PBXBuildFile; fileRef = 1469B7FA1C68D7D50080FD71 /* notes_with_user_id_custom.json */; }; 146F2EB31CE1F27200543F83 /* id.xcdatamodeld in Sources */ = {isa = PBXBuildFile; fileRef = 146F2EB11CE1F27200543F83 /* id.xcdatamodeld */; }; 146F2EB51CE1F2C500543F83 /* id.json in Resources */ = {isa = PBXBuildFile; fileRef = 146F2EB41CE1F2C500543F83 /* id.json */; }; + 147A3A0F1D704D860049C22A /* 225-a-empty.json in Resources */ = {isa = PBXBuildFile; fileRef = 147A3A0D1D704D860049C22A /* 225-a-empty.json */; }; + 147A3A101D704D860049C22A /* 225-a-null.json in Resources */ = {isa = PBXBuildFile; fileRef = 147A3A0E1D704D860049C22A /* 225-a-null.json */; }; + 1491123C1D70272D00167D26 /* 225-a.json in Resources */ = {isa = PBXBuildFile; fileRef = 149112351D70272D00167D26 /* 225-a.json */; }; + 1491123D1D70272D00167D26 /* 225-a-replaced.json in Resources */ = {isa = PBXBuildFile; fileRef = 149112361D70272D00167D26 /* 225-a-replaced.json */; }; + 149112451D70279200167D26 /* 225.xcdatamodeld in Sources */ = {isa = PBXBuildFile; fileRef = 149112431D70279200167D26 /* 225.xcdatamodeld */; }; 14959D141C065350004EF790 /* notes_with_user_id.json in Resources */ = {isa = PBXBuildFile; fileRef = 14959D131C065350004EF790 /* notes_with_user_id.json */; }; 14A644111CD77A4C00F51E23 /* Bug202.xcdatamodeld in Sources */ = {isa = PBXBuildFile; fileRef = 14A6440F1CD77A4C00F51E23 /* Bug202.xcdatamodeld */; }; 14A644151CD77C7A00F51E23 /* bug-202-a.json in Resources */ = {isa = PBXBuildFile; fileRef = 14A644121CD77C7A00F51E23 /* bug-202-a.json */; }; @@ -166,6 +171,11 @@ 146D72B11AB782920058798C /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 146F2EB21CE1F27200543F83 /* Demo.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = Demo.xcdatamodel; sourceTree = ""; }; 146F2EB41CE1F2C500543F83 /* id.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = id.json; sourceTree = ""; }; + 147A3A0D1D704D860049C22A /* 225-a-empty.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "225-a-empty.json"; sourceTree = ""; }; + 147A3A0E1D704D860049C22A /* 225-a-null.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "225-a-null.json"; sourceTree = ""; }; + 149112351D70272D00167D26 /* 225-a.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "225-a.json"; sourceTree = ""; }; + 149112361D70272D00167D26 /* 225-a-replaced.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "225-a-replaced.json"; sourceTree = ""; }; + 149112441D70279200167D26 /* 151-many-to-many.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "151-many-to-many.xcdatamodel"; sourceTree = ""; }; 14959D131C065350004EF790 /* notes_with_user_id.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = notes_with_user_id.json; sourceTree = ""; }; 14A644101CD77A4C00F51E23 /* Demo.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = Demo.xcdatamodel; sourceTree = ""; }; 14A644121CD77C7A00F51E23 /* bug-202-a.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "bug-202-a.json"; sourceTree = ""; }; @@ -228,6 +238,10 @@ 14D7321E1CE9A6C400C28D40 /* 157-cities.json */, 140B0DEE1CEAFAFC00D0EE3E /* 157-locations-update.json */, 141D9E711CE9A4830041A7FC /* 157-locations.json */, + 149112351D70272D00167D26 /* 225-a.json */, + 149112361D70272D00167D26 /* 225-a-replaced.json */, + 147A3A0D1D704D860049C22A /* 225-a-empty.json */, + 147A3A0E1D704D860049C22A /* 225-a-null.json */, 1418940B1BDD62F600EE52CE /* bug-113-comments-no-id.json */, 1418940C1BDD62F600EE52CE /* bug-113-custom_relationship_key_to_one.json */, 1418940D1BDD62F600EE52CE /* bug-113-stories-comments-no-ids.json */, @@ -279,6 +293,7 @@ 14096A3E1CEAF483002690D6 /* 151-ordered-many-to-many.xcdatamodeld */, 14096A401CEAF483002690D6 /* 151-ordered-to-many.xcdatamodeld */, 14096A3A1CEAF40E002690D6 /* 151-to-many.xcdatamodeld */, + 149112431D70279200167D26 /* 225.xcdatamodeld */, 1418942B1BDD62F600EE52CE /* Bug84.xcdatamodeld */, 141894251BDD62F600EE52CE /* Bug113.xcdatamodeld */, 141894291BDD62F600EE52CE /* Bug125.xcdatamodeld */, @@ -308,11 +323,11 @@ 1425D59A1CC65BEB00EC49D4 /* Source */ = { isa = PBXGroup; children = ( - 1425D59B1CC65BEB00EC49D4 /* NSArray+Sync.swift */, - 1425D59C1CC65BEB00EC49D4 /* NSEntityDescription+Sync.swift */, + 1425D59F1CC65BEB00EC49D4 /* Sync.swift */, 1425D59D1CC65BEB00EC49D4 /* NSManagedObject+Sync.swift */, + 1425D59C1CC65BEB00EC49D4 /* NSEntityDescription+Sync.swift */, 1425D59E1CC65BEB00EC49D4 /* NSManagedObjectContext+Sync.swift */, - 1425D59F1CC65BEB00EC49D4 /* Sync.swift */, + 1425D59B1CC65BEB00EC49D4 /* NSArray+Sync.swift */, ); path = Source; sourceTree = ""; @@ -457,6 +472,7 @@ files = ( B0B637171C6E517F00229B03 /* bug-179-places.json in Resources */, 14A644151CD77C7A00F51E23 /* bug-202-a.json in Resources */, + 1491123D1D70272D00167D26 /* 225-a-replaced.json in Resources */, 1418944C1BDD62F600EE52CE /* numbers.json in Resources */, 14959D141C065350004EF790 /* notes_with_user_id.json in Resources */, 14A644161CD77C7A00F51E23 /* bug-202-b.json in Resources */, @@ -474,10 +490,12 @@ 141894581BDD62F600EE52CE /* users_notes.json in Resources */, 14BF14DA1D6ED307008F5915 /* to-one-camelcase.json in Resources */, 141894531BDD62F600EE52CE /* unique.json in Resources */, + 1491123C1D70272D00167D26 /* 225-a.json in Resources */, B0B637181C6E517F00229B03 /* bug-179-routes.json in Resources */, 141894401BDD62F600EE52CE /* bug-113-comments-no-id.json in Resources */, 141894431BDD62F600EE52CE /* bug-125-light.json in Resources */, 1418944A1BDD62F600EE52CE /* markets_items.json in Resources */, + 147A3A101D704D860049C22A /* 225-a-null.json in Resources */, 144EFF4C1D6CF3EC003EA935 /* bug-254.json in Resources */, 146F2EB51CE1F2C500543F83 /* id.json in Resources */, 141894561BDD62F600EE52CE /* users_c.json in Resources */, @@ -496,6 +514,7 @@ 14096A371CEAF329002690D6 /* 151-to-many-users.json in Resources */, 140FBF1E1CB91C2D0026484E /* camelcase.json in Resources */, 141894501BDD62F600EE52CE /* stories-comments-no-ids.json in Resources */, + 147A3A0F1D704D860049C22A /* 225-a-empty.json in Resources */, 1418944F1BDD62F600EE52CE /* patients.json in Resources */, 141894471BDD62F600EE52CE /* custom_relationship_key_to_many.json in Resources */, 14BF14DB1D6ED307008F5915 /* to-one-snakecase.json in Resources */, @@ -579,6 +598,7 @@ 141D9E751CE9A4970041A7FC /* Bug157.xcdatamodeld in Sources */, 141894621BDD62F600EE52CE /* Recursive.xcdatamodeld in Sources */, 14E298431C563EE500B68B72 /* OrderedSocial.xcdatamodeld in Sources */, + 149112451D70279200167D26 /* 225.xcdatamodeld in Sources */, 14A644111CD77A4C00F51E23 /* Bug202.xcdatamodeld in Sources */, 1425D5A31CC65BEB00EC49D4 /* NSManagedObjectContext+Sync.swift in Sources */, 141894591BDD62F600EE52CE /* Bug113.xcdatamodeld in Sources */, @@ -957,6 +977,16 @@ sourceTree = ""; versionGroupType = wrapper.xcdatamodel; }; + 149112431D70279200167D26 /* 225.xcdatamodeld */ = { + isa = XCVersionGroup; + children = ( + 149112441D70279200167D26 /* 151-many-to-many.xcdatamodel */, + ); + currentVersion = 149112441D70279200167D26 /* 151-many-to-many.xcdatamodel */; + path = 225.xcdatamodeld; + sourceTree = ""; + versionGroupType = wrapper.xcdatamodel; + }; 14A6440F1CD77A4C00F51E23 /* Bug202.xcdatamodeld */ = { isa = XCVersionGroup; children = ( diff --git a/Source/NSManagedObject+Sync.swift b/Source/NSManagedObject+Sync.swift index 9dbf7838..8b9749b1 100644 --- a/Source/NSManagedObject+Sync.swift +++ b/Source/NSManagedObject+Sync.swift @@ -71,8 +71,7 @@ public extension NSManagedObject { setValue(nil, forKey: relationship.name) } } else { - guard let localPrimaryKey = localPrimaryKey as? NSArray else { return } - let remoteItems = localPrimaryKey + guard let remoteItems = localPrimaryKey as? NSArray else { return } let localRelationship: NSSet if relationship.ordered { let value = self.valueForKey(relationship.name) as? NSOrderedSet ?? NSOrderedSet() @@ -141,23 +140,93 @@ public extension NSManagedObject { - parameter dataStack: The DATAStack instance. */ func sync_toManyRelationship(relationship: NSRelationshipDescription, dictionary: [String : AnyObject], parent: NSManagedObject?, parentRelationship: NSRelationshipDescription?, dataStack: DATAStack, operations: DATAFilter.Operation) { - guard let managedObjectContext = managedObjectContext, destinationEntity = relationship.destinationEntity, childEntityName = destinationEntity.name else { abort() } - - let inverseIsToMany = relationship.inverseRelationship?.toMany ?? false var children: [[String : AnyObject]]? + var isNull = false - if let customRelationshipName = relationship.userInfo?[SYNCCustomRemoteKey] as? String { - children = dictionary[customRelationshipName] as? [[String : AnyObject]] - } else if let result = dictionary[relationship.name.hyp_remoteString()] as? [[String : AnyObject]] { - children = result - } else if let result = dictionary[relationship.name] as? [[String : AnyObject]] { - children = result + if relationship.userInfo?[SYNCCustomRemoteKey] is NSNull { + isNull = true + } else if dictionary[relationship.name.hyp_remoteString()] is NSNull { + isNull = true + } else if dictionary[relationship.name] is NSNull { + isNull = true } + if isNull { + children = [[String : AnyObject]]() + + if valueForKey(relationship.name) != nil { + setValue(nil, forKey: relationship.name) + } + } else { + if let customRelationshipName = relationship.userInfo?[SYNCCustomRemoteKey] as? String { + children = dictionary[customRelationshipName] as? [[String : AnyObject]] + } else if let result = dictionary[relationship.name.hyp_remoteString()] as? [[String : AnyObject]] { + children = result + } else if let result = dictionary[relationship.name] as? [[String : AnyObject]] { + children = result + } + } + + let inverseIsToMany = relationship.inverseRelationship?.toMany ?? false + guard let managedObjectContext = managedObjectContext else { abort() } + guard let destinationEntity = relationship.destinationEntity else { abort() } + guard let childEntityName = destinationEntity.name else { abort() } + if let children = children { - var childPredicate: NSPredicate? let childIDs = (children as NSArray).valueForKey(entity.sync_remotePrimaryKey()) + if childIDs is NSNull { + if let _ = valueForKey(relationship.name) { + setValue(nil, forKey: relationship.name) + } + } else { + guard let destinationEntityName = destinationEntity.name else { fatalError("entityName not found in entity: \(destinationEntity)") } + if let remoteItems = childIDs as? NSArray { + let localRelationship: NSSet + if relationship.ordered { + let value = self.valueForKey(relationship.name) as? NSOrderedSet ?? NSOrderedSet() + localRelationship = value.set + } else { + localRelationship = self.valueForKey(relationship.name) as? NSSet ?? NSSet() + } + let localItems = localRelationship.valueForKey(entity.sync_localPrimaryKey()) as? NSSet ?? NSSet() + + let deletedItems = NSMutableArray(array: localItems.allObjects) + deletedItems.removeObjectsInArray(remoteItems as [AnyObject]) + + if deletedItems.count > 0 { + let request = NSFetchRequest(entityName: destinationEntityName) + + do { + let safeLocalObjects = try managedObjectContext.executeFetchRequest(request) as? [NSManagedObject] ?? [NSManagedObject]() + for safeObject in safeLocalObjects { + let currentID = safeObject.valueForKey(entity.sync_localPrimaryKey())! + for deleted in deletedItems { + if currentID.isEqual(deleted) { + if relationship.ordered { + let relatedObjects = mutableOrderedSetValueForKey(relationship.name) + if relatedObjects.containsObject(safeObject) { + relatedObjects.removeObject(safeObject) + setValue(relatedObjects, forKey: relationship.name) + } + } else { + let relatedObjects = mutableSetValueForKey(relationship.name) + if relatedObjects.containsObject(safeObject) { + relatedObjects.removeObject(safeObject) + setValue(relatedObjects, forKey: relationship.name) + } + } + } + } + } + } catch { + fatalError() + } + } + } + } + + var childPredicate: NSPredicate? let manyToMany = inverseIsToMany && relationship.toMany if manyToMany { if childIDs.count > 0 { diff --git a/Tests/JSONs/225-a-empty.json b/Tests/JSONs/225-a-empty.json new file mode 100755 index 00000000..e59f15ad --- /dev/null +++ b/Tests/JSONs/225-a-empty.json @@ -0,0 +1,6 @@ +[ + { + "id": 1, + "tags": [] + } +] diff --git a/Tests/JSONs/225-a-null.json b/Tests/JSONs/225-a-null.json new file mode 100755 index 00000000..130b63a5 --- /dev/null +++ b/Tests/JSONs/225-a-null.json @@ -0,0 +1,6 @@ +[ + { + "id": 1, + "tags": null + } +] diff --git a/Tests/JSONs/225-a-replaced.json b/Tests/JSONs/225-a-replaced.json new file mode 100755 index 00000000..5a675570 --- /dev/null +++ b/Tests/JSONs/225-a-replaced.json @@ -0,0 +1,10 @@ +[ + { + "id": 1, + "tags": [ + { + "id": 20 + } + ] + } +] diff --git a/Tests/JSONs/225-a.json b/Tests/JSONs/225-a.json new file mode 100755 index 00000000..0b5fae8d --- /dev/null +++ b/Tests/JSONs/225-a.json @@ -0,0 +1,10 @@ +[ + { + "id": 1, + "tags": [ + { + "id": 10 + } + ] + } +] diff --git a/Tests/Models/225.xcdatamodeld/.xccurrentversion b/Tests/Models/225.xcdatamodeld/.xccurrentversion new file mode 100755 index 00000000..2477a35b --- /dev/null +++ b/Tests/Models/225.xcdatamodeld/.xccurrentversion @@ -0,0 +1,8 @@ + + + + + _XCCurrentVersionName + 151-many-to-many.xcdatamodel + + diff --git a/Tests/Models/225.xcdatamodeld/151-many-to-many.xcdatamodel/contents b/Tests/Models/225.xcdatamodeld/151-many-to-many.xcdatamodel/contents new file mode 100644 index 00000000..9f497085 --- /dev/null +++ b/Tests/Models/225.xcdatamodeld/151-many-to-many.xcdatamodel/contents @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/Tests/SyncTests.swift b/Tests/SyncTests.swift index 1667f2ff..71cdb31e 100644 --- a/Tests/SyncTests.swift +++ b/Tests/SyncTests.swift @@ -1124,4 +1124,76 @@ class SyncTests: XCTestCase { try! dataStack.drop() } + + // MARK: - https://github.com/hyperoslo/Sync/issues/225 + + func test225ReplacedTag() { + let dataStack = Helper.dataStackWithModelName("225") + + let usersA = Helper.objectsFromJSON("225-a.json") as! [[String : AnyObject]] + Sync.changes(usersA, inEntityNamed: "User", dataStack: dataStack, completion: nil) + XCTAssertEqual(Helper.countForEntity("User", inContext:dataStack.mainContext), 1) + XCTAssertEqual(Helper.countForEntity("Tag", inContext:dataStack.mainContext), 1) + + // This should remove the old tag reference to the user and insert this new one. + let usersB = Helper.objectsFromJSON("225-a-replaced.json") as! [[String : AnyObject]] + Sync.changes(usersB, inEntityNamed: "User", dataStack: dataStack, completion: nil) + XCTAssertEqual(Helper.countForEntity("User", inContext:dataStack.mainContext), 1) + XCTAssertEqual(Helper.countForEntity("Tag", inContext:dataStack.mainContext), 2) + + let user = Helper.fetchEntity("User", inContext: dataStack.mainContext).first! + let predicate = NSPredicate(format: "ANY users IN %@", [user]) + let tags = Helper.fetchEntity("Tag", predicate: predicate, inContext: dataStack.mainContext) + XCTAssertEqual(tags.count, 1) + + try! dataStack.drop() + } + + func test225RemovedTagsWithEmptyArray() { + let dataStack = Helper.dataStackWithModelName("225") + + let usersA = Helper.objectsFromJSON("225-a.json") as! [[String : AnyObject]] + Sync.changes(usersA, inEntityNamed: "User", dataStack: dataStack, completion: nil) + XCTAssertEqual(Helper.countForEntity("User", inContext:dataStack.mainContext), 1) + XCTAssertEqual(Helper.countForEntity("Tag", inContext:dataStack.mainContext), 1) + + // This should remove all the references. + let usersB = Helper.objectsFromJSON("225-a-empty.json") as! [[String : AnyObject]] + Sync.changes(usersB, inEntityNamed: "User", dataStack: dataStack, completion: nil) + XCTAssertEqual(Helper.countForEntity("User", inContext:dataStack.mainContext), 1) + + // WARNING: Maybe this shouldn't be 0, but should be 1 instead, since it shouldn't delete the + // object, but instead, it should just remove the reference. + XCTAssertEqual(Helper.countForEntity("Tag", inContext:dataStack.mainContext), 0) + + let user = Helper.fetchEntity("User", inContext: dataStack.mainContext).first! + let predicate = NSPredicate(format: "ANY users IN %@", [user]) + let tags = Helper.fetchEntity("Tag", predicate: predicate, inContext: dataStack.mainContext) + XCTAssertEqual(tags.count, 0) + + try! dataStack.drop() + } + + func test225RemovedTagsWithNull() { + let dataStack = Helper.dataStackWithModelName("225") + + let usersA = Helper.objectsFromJSON("225-a.json") as! [[String : AnyObject]] + Sync.changes(usersA, inEntityNamed: "User", dataStack: dataStack, completion: nil) + XCTAssertEqual(Helper.countForEntity("User", inContext:dataStack.mainContext), 1) + XCTAssertEqual(Helper.countForEntity("Tag", inContext:dataStack.mainContext), 1) + + // WARNING: Maybe this shouldn't be 0, but should be 1 instead, since it shouldn't delete the + // object, but instead, it should just remove the reference. + let usersB = Helper.objectsFromJSON("225-a-null.json") as! [[String : AnyObject]] + Sync.changes(usersB, inEntityNamed: "User", dataStack: dataStack, completion: nil) + XCTAssertEqual(Helper.countForEntity("User", inContext:dataStack.mainContext), 1) + XCTAssertEqual(Helper.countForEntity("Tag", inContext:dataStack.mainContext), 0) + + let user = Helper.fetchEntity("User", inContext: dataStack.mainContext).first! + let predicate = NSPredicate(format: "ANY users IN %@", [user]) + let tags = Helper.fetchEntity("Tag", predicate: predicate, inContext: dataStack.mainContext) + XCTAssertEqual(tags.count, 0) + + try! dataStack.drop() + } }