Thread Management, Singleton Double Init, Save Document Bug about iclouddocumentsync HOT 5 CLOSED

I think I'm a little confused by the first part of the issue. Could you clarify? If I understand correctly, the ubiquityContainer should be returned on the same thread that is was retrieved on. So it should look more like this:

- (NSURL *)ubiquitousContainerURL {
    dispatch_async (dispatch_get_global_queue (DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void) {
        ubiquityContainer = [fileManager URLForUbiquityContainerIdentifier:nil];
        return ubiquityContainer;
    });
}

It's possible this would make a difference. Is this what you're thinking?

Great idea with the completion handler on the initialization! I hadn't thought of that. It'll take some work, but I'll start developing it. Maybe a separate initWithCompletion: method in addition to the regular init method would work.

And remember, version 7.0 makes it a lot easier to make contributions - so if you have a good idea (like that one) and know how to implement / write it then you can do it! Take a look at Contributing.md for more details.

from iclouddocumentsync.

Yes, exactly. The way it is now, it looks as if the method returns ubiquityContainer without waiting for the completion of [fileManager URLForUbiquityContainerIdentifier:nil];.
When calling dispatch_async I thought the thread just continues without waiting for the completion handler to finish.
But I don't think it would work out just returning from within the completion handler, because it - as stated above - will just continue without waiting for the handler to finish and so there is no valid return value. (It throws an error clarifying this too.)

I'll gladly look at your instructions to make contributions - but for now I hope you bear with me, when I post my stuff here :-)

I've changed the cloud.m-init method to

- (id)init {
    // Setup Starter Sync
    self = [super init];
    
    NSLog(@"cloud init ...");
    
    if (self) {
        // Setup the File Manager
        if (fileManager == nil) fileManager = [NSFileManager defaultManager];
        
        // Setup the Notification Center
        if (notificationCenter == nil) notificationCenter = [NSNotificationCenter defaultCenter];
        
        // Initialize file lists, results, and queries
        if (fileList == nil) fileList = [NSMutableArray array];
        if (previousQueryResults == nil) previousQueryResults = [NSMutableArray array];
        if (query == nil) query = [[NSMetadataQuery alloc] init];
        
        // Log the setup
        NSLog(@"[iCloud] Initialized");
        
        // Check the iCloud Ubiquity Container
        dispatch_async (dispatch_get_global_queue (DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void) {
            ubiquityContainer = [[NSFileManager defaultManager] URLForUbiquityContainerIdentifier: nil];
            if (ubiquityContainer != nil) {
                // We can write to the ubiquity container
                
                // Check iCloud Availability
                id cloudToken = [fileManager ubiquityIdentityToken];
                
                // Sync and Update Documents List
                [self enumerateCloudDocuments];
                
                dispatch_async (dispatch_get_main_queue (), ^(void) {                   
                    // Subscribe to changes in iCloud availability (placed here because it should run on the main thread)
                    [notificationCenter addObserver:self selector:@selector(checkCloudAvailability) name:NSUbiquityIdentityDidChangeNotification object:nil];
                    
                    if ([delegate respondsToSelector:@selector(iCloudDidFinishInitializingWitUbiquityToken: withUbiquityContainer:)])
                        [delegate iCloudDidFinishInitializingWitUbiquityToken:cloudToken withUbiquityContainer:ubiquityContainer];
                });
            }
        });
    }
    
    return self;
}

Here I implemented another delegate-method to be called when the initializing has finished :
iCloudDidFinishInitializingWitUbiquityToken: withUbiquityContainer:.
This of course has to be added to cloud.h as well.

- (BOOL)checkCloudUbiquityContainer became obsolete with the above.

To avoid the same thread-issue as above in the method - (NSURL *)ubiquitousContainerURL I changed it to use the instance variable ubiquityContainer:

- (NSURL *)ubiquitousContainerURL {    
    return ubiquityContainer;
}

because I think it suffice to set this when initializing the iCloud (and maybe updating it when a change to the availability is detected?)

Further on, I used this same instance variable in the - (NSURL *)ubiquitousDocumentsDirectoryURL method directly:

NSURL *documentsDirectory = [ubiquityContainer URLByAppendingPathComponent:DOCUMENT_DIRECTORY];

BTW, are you sure, you are initializing the singleton correctly? As I see it, it becomes initialized twice, when you explicitly call the init method. I think it will suffice, initializing the whole thing like

[[iCloud sharedCloud] setDelegate:self]; // Set this if you plan to use the delegate
[[iCloud sharedCloud] setVerboseLogging:YES]; // We want detailed feedback about what's going on with iCloud, this is OFF by default

because the init method already got a run when the singleton got initialized with + (id)sharedCloud.

Now there is still a problem, I can't resolve; when saving a document using

[[iCloud sharedCloud] saveAndCloseDocumentWithName:fileName withContent:content completion:^(UIDocument *cloudDocument, NSData *documentData, NSError *error) {
            if (error == nil) {
                NSLog(@"##### saved %@", fileName);
            }
            else
            {
                NSLog(@"Error saving %@ to iCloud: %@", fileName, [error localizedDescription]);
            }
        }];

The code in the completion handler just never gets called - although when I double checked, I found the data was written to the cloud just fine...

Du you have any hint here?

Now I'll go looking into the contribution thing :-)

from iclouddocumentsync.

OK, I've uploaded a fork.

I've investigated my problem when saving and found it to be related to an issue with saveAndCloseDocumentWithName. It just does not complete when the document in question do already exist. You're using document closeWithCompletionHandler: after checking if the document exists. And that's where it stalls (obviously after saving everything as it should but without running the completion hanlder....).

But when either bypassing the check or using the saveChangesToDocumentWithName all is fine...

Maybe the document is closed already and therefor nothing happens here?!

Why do you distinguish between these two states at all? It works out just fine when using document saveToURL: even if the document exists. Is there really any need for using document closeWithCompletionHandler: here?

I'm a bit confused about the different ways to save the document, i.e.
document closeWithCompletionHandler:
document saveToURL: and even
document updateChangeCount:

from iclouddocumentsync.

Well, I think the point here in fact is, that the document already is closed, when you're calling closeWithCompletionHandler

I've inserted a check for the state of the document in - (void)saveAndCloseDocumentWithName:...:

...
// If the file exists, close it; otherwise, create it.
    if ([fileManager fileExistsAtPath:[fileURL path]]) {
        // Log closing
        if (verboseLogging == YES) NSLog(@"[iCloud] Document exists at %@ with state %@, saving and closing", fileURL, document.stateDescription);
        
        // Save and close the document
        [document closeWithCompletionHandler:^(BOOL success) {
            if (success) {
...

and as expected it reports the document state as "document closed" so the attempt to "reclose" it will do no good.

Do you see any harm done by skipping the whole checking for an existing file

    // If the file exists, close it; otherwise, create it.
    if ([fileManager fileExistsAtPath:[fileURL path]]) {

and just proceed with the saveToURL in the else-block? I've just removed the block and so far it runs fine. But it would feel better, if you could confirm this :-)

Update: According to the UIDocument docs a file just created with initWithFileURL is expected to be in a closed state. I updated it by enclosing the closeWithCompletionHandler statements with [document saveToURL:document.fileURL forSaveOperation:UIDocumentSaveForOverwriting completionHandler:

(BTW, your example app will not run into this issue because it will not save an existing file using this block. But my app now and then saves data to an existing file in the cloud and therefor runs into this...)

from iclouddocumentsync.

Wow! That's a lot of work! I'll try to go through and respond to everything one at a time. Also, when you do finish with all your changes - don't forget to submit a Pull Request so I can merge the changes into the main repo.

All of your changes with the init method, the thread management, the new delegate method, the correction of the use of init on the singleton, and the use of the ubiquityContainer instance variable is spot on perfect! The double init of iCloud was something I had overlooked - I had just been used to using [[iCloud alloc] init] for so long!

From what I previously understood (I now see that I am incorrect) overwriting a document using saveToURL: forSaveOperation: completionHandler: would also overwrite any recorded changes made to the document. It turns out that it only overwrites the current version. A major focal point of UIDocument is to have file versioning, change tracking, conflict handling, etc. This is why I moved to closeWithCompletionhandler:. The closeWithCompletionHandler: method both saves and closes the UIDocument. I overlooked the fact that a file created with initWithFileURL: is expected to be in a closed state.

The document updateChangeCount: method allows you to register changes made to a document (for change tracking), it also lets UIDocument know that there may be unsaved changes.

So, your changes are correct. Saving a document explicitly opens the document. Inside of the completion handler for saveToURL: forSaveOperation: completionHandler:, call the closeDocumentWithCompletionHandler: to close the document. Previous to version 7.0, this was the implemented behavior.

Everything on your fork looks good! Can't wait for the pull request so that I can merge the changes!

from iclouddocumentsync.