phonegap / phonegap-plugin-contentsync

Download and cache remotely hosted content
Apache License 2.0
206 stars 98 forks source link

Can't reliably get/set the root path for sync'ed content #44

Open denisbabineau opened 9 years ago

denisbabineau commented 9 years ago

This issue is related to issue #40 but I felt was different enough to warrant a separate issue;

I'm using this plugin to sync/update my app; on start-up I may (or may not) attempt a remote sync (i.e. depending on connection state, may want to avoid syncs OTA). Past this I need to find out if the app is in cache or not (wether it was sync'ed this start-up or not). Since the sync location is 'unknown' I have to rely on sync({type:'local'}) to return me the path if it's been sync'ed or fail if not. The documentation states:

options.src is not required if cached content actually exists.

But validation forces me to include this parameter which I define as "null" (I DON'T want it to download, I have a fallback location in my bundled assets). On iOS at least, this faults (SIGABRT) in startDownload: . I would rather not have to call sync() to get the location but since it's in an abitrary location I'd rather not hardcode it (why is it nested in /files/files/ ?). If it was rooted in one of the cordova.file location at least it would make more sense but it's currently in cordova.file.dataDirectory + 'files' for Android (resulting in .../files/files/ ) and on iOS it's not even in dataDirectory (it's ../files relative to dataDirectory) which might be getting sync'ed to the cloud which would likely not be desired in most cases.

In short it would be ideal if we could specify the destination location either as a full path when sync'ing or having a global root value on ContentSync itself that can be user-defined.

Thanks

revolunet commented 9 years ago

From my tests, you can set the location with the id parameter. the issue is that the iOS plugin currently arbitrary stores files in "files" subfolder which result in "files/[sync-id]/". i think the Api should let the user set the full path. btw currently your extracted zip content is at files/[sync-id]/ what is the "official" folder to store files without icloud on ios ?

macdonst commented 9 years ago

@denisbabineau If we did ContentSync.get('appId') which returns the path to the sync'ed content if it exists or null/error callback if it doesn't make life easier?

denisbabineau commented 9 years ago

@macdonst that certainly would be helpful and would probably be the least disruptive to the API. However I strongly feel that lacking the ability to set the root/destination path will still be a limiting factor for this plugin given various usage scenarios, to name a few:

imhotep commented 9 years ago

@denisbabineau I don't understand some of your questions (are they questions)? but let me know if any of the things below help you.

I updated the plugin for iOS to:


ContentSync.sync({"id": "appId", "type": "local"});
revolunet commented 9 years ago

does the UUIDs change when we upgrade the app ? or it is constant for a given bundle id ?

denisbabineau commented 9 years ago

@imhotep thanks for the updates. Looks like that should take care of things. I had to move to another task but will dive into this again later on. My main concern seems to be solved so I think we're good to close this issue with a small addition; I still get a segfault in iOS when attempting to get a path with sync local of a non-existant package. This is due to src being NSNull and not nil, I modified the check in startDownload: on my local copy to the following and it works, I'm not sure if the check for nil is even valid anymore in this case:

    if((src != nil) && (src != (id)[NSNull null])) {
        NSLog(@"startDownload from %@", src);

Including that change, I think we can close this issue. If there's anything that comes up I'll re-open as needed.

Concerning the setting of the path, to me this would still be a nice to have but I think I can live without it. My idea would not be to "hardcode" the paths (as you mentioned the UUIDs change) but rather rely on predefined constants such as those defined by cordova.file. Alternatively there could be extra options such as "tmp", "cloud/nocloud", "documents", etc but again, that would be an extra feature and perhaps not warranted unless there's enough demand.

Thanks Denis

imhotep commented 9 years ago

@revolunet the UUID changes on every app launch. We can only rely on the iOS SDK to get the full path to the app's Library/ folder. That is also the reason why options.type="local" exists.

revolunet commented 9 years ago

thanks for clarifications ! :+1:

tripodsan commented 9 years ago

so in order to work together with the file-plugin, we need to set the appid to 'files/' + id ?

imhotep commented 9 years ago

@tripodsan Yes I believe so on Android. I don't think you need to on iOS according to the cordova-plugin-file README

tripodsan commented 9 years ago

but now I think it is inconsistent with android. they still add the 'files' automatically: https://github.com/phonegap/phonegap-plugin-contentsync/blob/master/src/android/Sync.java#L546