phonegap / phonegap-plugin-contentsync

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

download zip and output folder with type=local #40

Closed revolunet closed 9 years ago

revolunet commented 9 years ago

Hi,

I use this (great) plugin to download assets for an existing application and i've set the type param to local to prevent existing files to be re-downloaded.

var sync = ContentSync.sync({ src: url, id: 'programs/xxx-2', type: 'local', copyCordovaAssets: false, headers: false });

The zip gets extracted to Library/files/programs/xxx-2 but in the sync method, when you check if the file already exist, the checked path is Library/programs/xxx-2 (no "files" folder), so it download and extracts the zip file again.

Why this "files" subfolder when extracting ZIPs ?

Another non-related question : in the README, you say we should not use any other cordova File plugin, but in your sample, you use window.requestFileSystem... confusing :) Also note PERSISTENT is undefined in your example (https://github.com/phonegap/phonegap-plugin-contentsync/blob/master/sample/js/index.js#L120)

Thanks

imhotep commented 9 years ago

@revolunet thanks for reporting this.

We are extracting the archives under files because it's easier to access your app with

window.requestFileSystem(PERSISTENT, 1024 * 1024, function(fs) {
    fs.root.getDirectory("appId", ...)
})

fs.root points to the app's Library/files and I have no idea why that is.

Using the FileSystem plugin is not required for ContentSync.sync to work.

You are required to use the FileSystem plugin if you want break down the operations (download, extract/unzip) because you need to be able to manipulate the downloaded archive. The methods are available as a convenience only because this plugin is incompatible with cordova-plugin-zip. The API is exactly the same though so you don't need to install both.

revolunet commented 9 years ago

Thanks for the fix and explanations @imhotep :)

Shouldn't the output folder be defined by the user himself instead of the arbitrary "files" ? If the user want to output to files he can do id: "files/apps/app-xxx" ?

imhotep commented 9 years ago

@revolunet as I said above. window.requestFileSystem(PERSISTENT, ...) returns Library/files as the root. So if you specified id: "appId" you wouldn't be able to access the folder with window.requestFileSystem(PERSISTENT, ...). I am going to close the issue for now.

revolunet commented 9 years ago

I mean shouldn't the user be responsible of where to store files ? if one wants to access via window.requestFileSystem he can specify the files prefix in the id; but if i want to use "Cloud", "NoCloud" or anything i can do it too.

I think this "files" folder should not be set by the plugin but be the user, doesnt it ?

denisbabineau commented 9 years ago

@revolunet below are the paths I'm seeing on my platforms, see documentation for the file plugin for paths and their usage:

https://github.com/apache/cordova-plugin-file

iOS:

ContentSync:           /var/mobile/Containers/Data/Application/<uuid>/Library/files/
dataDirectory:  file:///var/mobile/Containers/Data/Application/<uuid>/Library/NoCloud/
TEMPORARY:      file:///var/mobile/Containers/Data/Application/<uuid>/tmp/
PERSISTENT:     file:///var/mobile/Containers/Data/Application/<uuid>/Documents/

Android:

ContentSync:    file:///data/data/<app-id>/files/files/
dataDirectory:  file:///data/data/<app-id>/files/
TEMPORARY:      file:///storage/emulated/0/Android/data/<app-id>/cache/
PERSISTENT:     file:///storage/emulated/0/                             (???)

Where:

dataDirectory = cordova.file.dataDirectory (when using the cordova file plugin)
TEMPORARY/PERSISTENT = paths returned by window.requestFileSystem() with associated arguments (NOTE: constant values are 0/1 respectively)

For iOS according to the documented layout, ContentSync on iOS looks like would be extracting in a cloud backed up location but it's unclear since Library/ itself is not defined as one of the cordova.file.* paths and shows "Yes?" under 'sync' column. I haven't verified this but I believe paths under iOS' Library path are backed up to the cloud by default, cordova has defined Cloud/ & NoCloud/ to specifically tag them as backed up/not backed up. So "Yes?" would be accurate. Looking at the iOS plugin code the following exists, note that no action is taken on libPathSync so the default must be to be cloud backed up:

    // Create the directories if necessary.
    [[NSFileManager defaultManager] createDirectoryAtPath:libPathSync withIntermediateDirectories:YES attributes:nil error:nil];
    [[NSFileManager defaultManager] createDirectoryAtPath:libPathNoSync withIntermediateDirectories:YES attributes:nil error:nil];
    // Mark NoSync as non-iCloud.
    [[NSURL fileURLWithPath:libPathNoSync] setResourceValue: [NSNumber numberWithBool: YES]
                                                     forKey: NSURLIsExcludedFromBackupKey error:nil];

In response to @imhotep in the other thread/bug I don't know if these paths have changed over time (with phonegap vs cordova?) but ContentSync is not going in the PERSISTENT filesystem on either platforms for myself. This lack of consistency coupled with the lack of a proper way to get the root path has forced me for the time being to modify the plugin code for both platform to point to my desired location.

I understand the plugin's requirement to not be dependant on the cordova.file plugin but completely ignoring the established paths (above and beyond TEMPORARY/PERSISTENT) defined by this defacto plugin and (in my case anyways) failing to be consistent even with the basic requestFileSystem() paths coupled with no means of getting (or setting) the storage path makes this plugin next to unuseable except in the simplest case. My understanding is that this plugin came from the PhoneGap Dev App where in a dev environment you don't really care where temporary modifications are sync'ed but in a production environment you'll normally want to control where content gets sync especially when it defines things like wether it gets backed up to the cloud and not. And in my case at least, storing in the external folder/sdcard location on Android is neither desired or consistent with the iOS platform.

Cordova version: 5.0.0 Installed platforms: android 4.0.0, browser 3.5.2, ios 3.8.0 cordova-plugin-file 2.0.0 "File" phonegap-plugin-contentsync 1.1.3 "content-sync"

Cheers Denis

revolunet commented 9 years ago

Thanks @denisbabineau for the detailed insights :)

What about adding/fixing the current implementation ?

My requirements :

This works currently except we cannot get of the "files" subfolder

What's yours ?

imhotep commented 9 years ago

@revolunet I think latest commit meets all your requirements! Give it a try and let me know so that I can close this issue. Thanks!

revolunet commented 9 years ago

:+1: Thanks @imhotep looks great, we can now decide exactly here to store the files and the fileExistsAtPath check is correctly executed.