Skip to content
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

change PrimaryKey attribute - handle with ObjectID #15

Closed
spotlessicode opened this issue Aug 16, 2017 · 8 comments
Closed

change PrimaryKey attribute - handle with ObjectID #15

spotlessicode opened this issue Aug 16, 2017 · 8 comments

Comments

@spotlessicode
Copy link

spotlessicode commented Aug 16, 2017

If primaryKey attribute changes, SyncKit can't identify the origin Object in CloudKit. Now QSSyncedEntity has an attribute originObjectID, which keep the origin ObjectID, or the primaryKey. It should be 2 different attributes, one for the objectID originObjectID which handle the primaryKey or the origin ObjectID as now, and one for keeping the origin ObejcID originManagedObjectID.

And if primaryKey attribute is not nil, than ChangeManager should use it as now here:

- (QSSyncedEntity *)createSyncedEntityForRecord:(CKRecord *)record 
       NSManagedObject *object = [self insertManagedObjectWithEntityName:entityName];
        if ([self useUniqueIdentifierForEntityWithType:entityName]) {
            objectID = [self uniqueIdentifierForObjectFromRecord:record];
            [object setValue:objectID forKey:[self identifierFieldNameForEntityOfType:entityName]];
        } else {
            objectID = [object.objectID.URIRepresentation absoluteString];
        }
 syncedEntity.originObjectID = objectID;

but it should add the new attribute, to not losing the origin ObjectID:

syncedEntity.originManagedObjectID = [object.objectID.URIRepresentation absoluteString];

At the updating method, we could create 2 NSMutableArray for the IDs. The first, as now for the originObjectIDs, and the second for the originManagedObjectID.

- (void)targetContextDidSave:(NSNotification *)notification
        ....
        NSMutableArray *updatedIDs = [NSMutableArray array];
        NSMutableArray *updatedID2s = [NSMutableArray array];
        for (NSManagedObject *updatedObject in updated) {
            NSString *identifier = [self uniqueIdentifierForObject:updatedObject];
            [updatedIDs addObject:identifier];
            [updatedID2s addObject:updatedObject.originManagedObjectID] ;
        }

And if the entity is nil with primaryKey enumeration, then we can call the updatedID2s:

- (QSSyncedEntity *)syncedEntityWithOriginManagedObjectIdentifier:(NSString *)objectIdentifier
{
    NSError *error = nil;
    NSArray *fetchedObjects = [self.privateContext executeFetchRequestWithEntityName:@"QSSyncedEntity"
                                                                           predicate:[NSPredicate predicateWithFormat:@"originManagedObjectID == %@", objectIdentifier]
                                                                               error:&error];
    return [fetchedObjects firstObject];
}
[updatedIDs enumerateObjectsUsingBlock:^(NSString * _Nonnull objectIdentifier, NSUInteger idx, BOOL * _Nonnull stop) {
                QSSyncedEntity *entity = [self syncedEntityWithOriginObjectIdentifier:objectIdentifier];
                DLog(@"entity %@", entity);
                if(!entity){
                    DLog(@"entity nil");
                    [updatedID2s enumerateObjectsUsingBlock:^(NSString * _Nonnull objectIdentifier, NSUInteger idx, BOOL * _Nonnull stop) {
                        QSSyncedEntity *entity = [self syncedEntityWithOriginManagedObjectIdentifier:objectIdentifier];
                        DLog(@"updatedID2s entity %@", entity);
                        if ([entity.state integerValue] == QSSyncedEntityStateSynced && entity.changedKeys.length) {
                            entity.state = @(QSSyncedEntityStateChanged);
                        }
                        entity.updated = [NSDate date];
                    }];

                }else{
                    DLog(@"entity not nil");
                    if ([entity.state integerValue] == QSSyncedEntityStateSynced && entity.changedKeys.length) {
                        entity.state = @(QSSyncedEntityStateChanged);
                    }
                    entity.updated = [NSDate date];
                }
            }];

But with this method, we should change the QSSyncedEntity CroeData Property list. And I'm not sure how this will affect the entire database in Cloudkit.

@spotlessicode
Copy link
Author

and one more statement could be useful: if updatedID2s entity is nil, then the object should be inserted instead of being updated.

@mentrena
Copy link
Owner

If primaryKey attribute changes

Primary keys are not expected to change. If you need to change its value for some object, then that's an indication that the field shouldn't be used as the primary key field. Instead, add another field to your object and assign a unique value to it.
If you use an API your objects probably already get a unique value. Otherwise you might want to use [[NSUUID UUID] UUIDString];
The documentation should probably be more explicit about this, I will update it.

Regarding use of NSManagedObjectID: we can't rely on it because it will change as a result of a model update, so it's not reliable when it comes to tracking objects. SyncKit originally used NSManagedObjectID but when this limitation became apparent the primaryKey solution was added. I would actually like to completely drop the old behaviour and force users to provide a primaryKey field for their classes, it would avoid several problems.

@spotlessicode
Copy link
Author

the problem is with this, that you can add a primaryKey for the object, but that will be just an identifier, as ObjectID, and doesn't avoid duplications in CloudKit with the Name attribute if the user creates them on different devices... that was what primaryKey as a unique identifier meant to me. but ok, I'll try to handle it locally.

@mentrena
Copy link
Owner

Assigning a NSUUID as your identifier when you create objects would fix that issue --it's what I do in my own app

@spotlessicode
Copy link
Author

spotlessicode commented Aug 17, 2017 via email

@spotlessicode
Copy link
Author

I made modifications and test with it. my final opinion: The NSUUID just an identifier, and unfortunately, it can happen, that objects creates and upload to CloudKit with the same attribute (in my case it's important with one Name attribute just one object can exist, what I can manage, when the user create them, but I can't if there was a chaos with this nil primaryKey values at objects.. finally, of course, I can that too with a final deduplication check - but for those, who had same Problem - must to check the relationship at that objects too, and delete the good one!).

With proper scanning, synchronize database before creating new objects can be duplication avoided. However, if the user's iCloud account and/or Drive is unavailable or the user is not online and does not have the latest database, such problems may occur... and I don't think so that a good idea to handle it just with an easy deletion, rather than ask the user what should do. But in some situation, the user can't decide it, because the user can't see, which one of the same name objects has which relationships with other objects.. this issue needs some workaround.

@mentrena
Copy link
Owner

I get how it's an inconvenience for your use case, but I think de-duplication might be the only solution in this case.

One suggestion would be to check the importContext for goals with the same name in changeManager:didImportChanges:completion:, then use the modified field to work out which one to keep. That way you ensure that de-duplication happens in the child context, before the main context gets updated.

@spotlessicode
Copy link
Author

spotlessicode commented Aug 17, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants