CB-7253: requestFileSystem fails when no external storage is present There were 2 issues behind this bug: 1) DirectoryManager.getFreeDiskSpace() used an incorrect way of checking the free internal storage space: using a path of "/" would always result in 0 reported free space. 2) When checking whether requested space was available, the requestFileSystem() method would always check the external storage space and fallback to internal storage space, regardless of what type of filesystem (external, internal, or other) was being requested.
To fix both of these issues, I've added a new getFreeSpaceInBytes() method on the FileSystem base class, which is called on the actual filesystem instance being retrieved by requestFileSystem(). The new method for getting free space always uses the filesystem's correct root path, so it works for internal, external or any other Android storage filesystem. Project: http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/repo Commit: http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/commit/4097a985 Tree: http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/tree/4097a985 Diff: http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/diff/4097a985 Branch: refs/heads/master Commit: 4097a9855653aaf7f62806c06f1ae0502a29fbad Parents: 769eead Author: Jason Ginchereau <jason...@microsoft.com> Authored: Fri Oct 30 13:15:47 2015 -0700 Committer: Jason Ginchereau <jason...@microsoft.com> Committed: Fri Oct 30 13:51:19 2015 -0700 ---------------------------------------------------------------------- src/android/DirectoryManager.java | 45 ++++++++------- src/android/FileUtils.java | 102 ++++++++++++++++++--------------- src/android/Filesystem.java | 8 ++- src/android/LocalFilesystem.java | 21 ++++--- tests/tests.js | 8 ++- 5 files changed, 104 insertions(+), 80 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/blob/4097a985/src/android/DirectoryManager.java ---------------------------------------------------------------------- diff --git a/src/android/DirectoryManager.java b/src/android/DirectoryManager.java index c2d1278..07af5ea 100644 --- a/src/android/DirectoryManager.java +++ b/src/android/DirectoryManager.java @@ -56,45 +56,46 @@ public class DirectoryManager { } /** - * Get the free disk space - * + * Get the free space in external storage + * * @return Size in KB or -1 if not available */ - public static long getFreeDiskSpace(boolean checkInternal) { + public static long getFreeExternalStorageSpace() { String status = Environment.getExternalStorageState(); - long freeSpace = 0; + long freeSpaceInBytes = 0; - // If SD card exists + // Check if external storage exists if (status.equals(Environment.MEDIA_MOUNTED)) { - freeSpace = freeSpaceCalculation(Environment.getExternalStorageDirectory().getPath()); - } - else if (checkInternal) { - freeSpace = freeSpaceCalculation("/"); - } - // If no SD card and we haven't been asked to check the internal directory then return -1 - else { + freeSpaceInBytes = getFreeSpaceInBytes(Environment.getExternalStorageDirectory().getPath()); + } else { + // If no external storage then return -1 return -1; } - return freeSpace; + return freeSpaceInBytes / 1024; } /** - * Given a path return the number of free KB - * + * Given a path return the number of free bytes in the filesystem containing the path. + * * @param path to the file system - * @return free space in KB + * @return free space in bytes */ - private static long freeSpaceCalculation(String path) { - StatFs stat = new StatFs(path); - long blockSize = stat.getBlockSize(); - long availableBlocks = stat.getAvailableBlocks(); - return availableBlocks * blockSize / 1024; + public static long getFreeSpaceInBytes(String path) { + try { + StatFs stat = new StatFs(path); + long blockSize = stat.getBlockSize(); + long availableBlocks = stat.getAvailableBlocks(); + return availableBlocks * blockSize; + } catch (IllegalArgumentException e) { + // The path was invalid. Just return 0 free bytes. + return 0; + } } /** * Determine if SD card exists. - * + * * @return T=exists, F=not found */ public static boolean testSaveLocationExists() { http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/blob/4097a985/src/android/FileUtils.java ---------------------------------------------------------------------- diff --git a/src/android/FileUtils.java b/src/android/FileUtils.java index 931e2ee..97a1409 100644 --- a/src/android/FileUtils.java +++ b/src/android/FileUtils.java @@ -77,7 +77,7 @@ public class FileUtils extends CordovaPlugin { public static final int WRITE_PERM = 1; public static int UNKNOWN_ERR = 1000; - + private boolean configured = false; private String lastRawArgs; @@ -97,7 +97,7 @@ public class FileUtils extends CordovaPlugin { private interface FileOp { void run(JSONArray args) throws Exception; } - + private ArrayList<Filesystem> filesystems; public void registerFilesystem(Filesystem fs) { @@ -105,7 +105,7 @@ public class FileUtils extends CordovaPlugin { this.filesystems.add(fs); } } - + private Filesystem filesystemForName(String name) { for (Filesystem fs:filesystems) { if (fs != null && fs.name != null && fs.name.equals(name)) { @@ -141,7 +141,7 @@ public class FileUtils extends CordovaPlugin { } } } - + protected HashMap<String, String> getAvailableFileSystems(Activity activity) { Context context = activity.getApplicationContext(); HashMap<String, String> availableFileSystems = new HashMap<String,String>(); @@ -226,7 +226,7 @@ public class FileUtils extends CordovaPlugin { activity.finish(); } } - + public static FileUtils getFilePlugin() { return filePlugin; } @@ -235,7 +235,7 @@ public class FileUtils extends CordovaPlugin { if (localURL == null) return null; return filesystemForName(localURL.fsName); } - + @Override public Uri remapUri(Uri uri) { // Remap only cdvfile: URLs (not content:). @@ -276,7 +276,10 @@ public class FileUtils extends CordovaPlugin { else if (action.equals("getFreeDiskSpace")) { threadhelper( new FileOp( ){ public void run(JSONArray args) { - long l = DirectoryManager.getFreeDiskSpace(false); + // The getFreeDiskSpace plugin API is not documented, but some apps call it anyway via exec(). + // For compatibility it always returns free space in the primary external storage, and + // does NOT fallback to internal store if external storage is unavailable. + long l = DirectoryManager.getFreeExternalStorageSpace(); callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.OK, l)); } }, rawArgs, callbackContext); @@ -393,15 +396,10 @@ public class FileUtils extends CordovaPlugin { ); } else if (action.equals("requestFileSystem")) { threadhelper( new FileOp( ){ - public void run(JSONArray args) throws IOException, JSONException { - int fstype=args.getInt(0); - long size = args.optLong(1); - if (size != 0 && size > (DirectoryManager.getFreeDiskSpace(true) * 1024)) { - callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, FileUtils.QUOTA_EXCEEDED_ERR)); - } else { - JSONObject obj = requestFileSystem(fstype); - callbackContext.success(obj); - } + public void run(JSONArray args) throws JSONException { + int fstype = args.getInt(0); + long requiredSize = args.optLong(1); + requestFileSystem(fstype, requiredSize, callbackContext); } }, rawArgs, callbackContext); } @@ -690,15 +688,15 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("Unrecognized filesystem URL"); } throw new FileNotFoundException(); - } - + } + /** * Read the list of files from this directory. * * @return a JSONArray containing JSONObjects that represent Entry objects. * @throws FileNotFoundException if the directory is not found. * @throws JSONException - * @throws MalformedURLException + * @throws MalformedURLException */ private JSONArray readEntries(String baseURLstr) throws FileNotFoundException, JSONException, MalformedURLException { try { @@ -708,7 +706,7 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("No installed handlers for this URL"); } return fs.readEntriesAtLocalURL(inputURL); - + } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } @@ -755,8 +753,8 @@ public class FileUtils extends CordovaPlugin { * * @return a boolean representing success of failure * @throws FileExistsException - * @throws NoModificationAllowedException - * @throws MalformedURLException + * @throws NoModificationAllowedException + * @throws MalformedURLException */ private boolean removeRecursively(String baseURLstr) throws FileExistsException, NoModificationAllowedException, MalformedURLException { try { @@ -771,7 +769,7 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("No installed handlers for this URL"); } return fs.recursiveRemoveFileAtLocalURL(inputURL); - + } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } @@ -785,7 +783,7 @@ public class FileUtils extends CordovaPlugin { * @return a boolean representing success of failure * @throws NoModificationAllowedException * @throws InvalidModificationException - * @throws MalformedURLException + * @throws MalformedURLException */ private boolean remove(String baseURLstr) throws NoModificationAllowedException, InvalidModificationException, MalformedURLException { try { @@ -801,7 +799,7 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("No installed handlers for this URL"); } return fs.removeFileAtLocalURL(inputURL); - + } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } @@ -829,7 +827,7 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("No installed handlers for this URL"); } return fs.getFileForLocalURL(inputURL, path, options, directory); - + } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } @@ -848,7 +846,7 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("No installed handlers for this URL"); } return fs.getParentForLocalURL(inputURL); - + } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } @@ -867,7 +865,7 @@ public class FileUtils extends CordovaPlugin { throw new MalformedURLException("No installed handlers for this URL"); } return fs.getFileMetadataForLocalURL(inputURL); - + } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } @@ -877,27 +875,37 @@ public class FileUtils extends CordovaPlugin { * Requests a filesystem in which to store application data. * * @param type of file system requested - * @return a JSONObject representing the file system - * @throws IOException + * @param requiredSize required free space in the file system in bytes + * @param callbackContext context for returning the result or error * @throws JSONException */ - private JSONObject requestFileSystem(int type) throws IOException, JSONException { - JSONObject fs = new JSONObject(); + private void requestFileSystem(int type, long requiredSize, final CallbackContext callbackContext) throws JSONException { Filesystem rootFs = null; try { - rootFs = this.filesystems.get(type); + rootFs = this.filesystems.get(type); } catch (ArrayIndexOutOfBoundsException e) { - // Pass null through + // Pass null through } if (rootFs == null) { - throw new IOException("No filesystem of type requested"); + callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, FileUtils.NOT_FOUND_ERR)); + } else { + // If a nonzero required size was specified, check that the retrieved filesystem has enough free space. + long availableSize = 0; + if (requiredSize > 0) { + availableSize = rootFs.getFreeSpaceInBytes(); + } + + if (availableSize < requiredSize) { + callbackContext.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, FileUtils.QUOTA_EXCEEDED_ERR)); + } else { + JSONObject fs = new JSONObject(); + fs.put("name", rootFs.name); + fs.put("root", rootFs.getRootEntry()); + callbackContext.success(fs); + } } - fs.put("name", rootFs.name); - fs.put("root", rootFs.getRootEntry()); - return fs; } - /** * Requests a filesystem in which to store application data. * @@ -914,7 +922,7 @@ public class FileUtils extends CordovaPlugin { private static String toDirUrl(File f) { return Uri.fromFile(f).toString() + '/'; } - + private JSONObject requestAllPaths() throws JSONException { Context context = cordova.getActivity(); JSONObject ret = new JSONObject(); @@ -993,23 +1001,23 @@ public class FileUtils extends CordovaPlugin { if (fs == null) { throw new MalformedURLException("No installed handlers for this URL"); } - + fs.readFileAtURL(inputURL, start, end, new Filesystem.ReadFileCallback() { public void handleData(InputStream inputStream, String contentType) { try { ByteArrayOutputStream os = new ByteArrayOutputStream(); final int BUFFER_SIZE = 8192; byte[] buffer = new byte[BUFFER_SIZE]; - + for (;;) { int bytesRead = inputStream.read(buffer, 0, BUFFER_SIZE); - + if (bytesRead <= 0) { break; } os.write(buffer, 0, bytesRead); } - + PluginResult result; switch (resultType) { case PluginResult.MESSAGE_TYPE_STRING: @@ -1062,12 +1070,12 @@ public class FileUtils extends CordovaPlugin { if (fs == null) { throw new MalformedURLException("No installed handlers for this URL"); } - + long x = fs.writeToFileAtURL(inputURL, data, offset, isBinary); Log.d("TEST",srcURLstr + ": "+x); return x; } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); } - + } /** @@ -1080,7 +1088,7 @@ public class FileUtils extends CordovaPlugin { if (fs == null) { throw new MalformedURLException("No installed handlers for this URL"); } - + return fs.truncateFileAtURL(inputURL, size); } catch (IllegalArgumentException e) { throw new MalformedURLException("Unrecognized filesystem URL"); http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/blob/4097a985/src/android/Filesystem.java ---------------------------------------------------------------------- diff --git a/src/android/Filesystem.java b/src/android/Filesystem.java index faf31d2..c69d3bd 100644 --- a/src/android/Filesystem.java +++ b/src/android/Filesystem.java @@ -185,7 +185,13 @@ public abstract class Filesystem { } } - + /** + * Gets the free space in bytes available on this filesystem. + * Subclasses may override this method to return nonzero free space. + */ + public long getFreeSpaceInBytes() { + return 0; + } public abstract Uri toNativeUri(LocalFilesystemURL inputURL); public abstract LocalFilesystemURL toLocalUri(Uri inputURL); http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/blob/4097a985/src/android/LocalFilesystem.java ---------------------------------------------------------------------- diff --git a/src/android/LocalFilesystem.java b/src/android/LocalFilesystem.java index 2c77596..153d8e3 100644 --- a/src/android/LocalFilesystem.java +++ b/src/android/LocalFilesystem.java @@ -50,7 +50,7 @@ public class LocalFilesystem extends Filesystem { public String filesystemPathForFullPath(String fullPath) { return new File(rootUri.getPath(), fullPath).toString(); } - + @Override public String filesystemPathForURL(LocalFilesystemURL url) { return filesystemPathForFullPath(url.path); @@ -99,7 +99,7 @@ public class LocalFilesystem extends Filesystem { } return LocalFilesystemURL.parse(b.build()); } - + @Override public LocalFilesystemURL URLforFilesystemPath(String path) { return localUrlforFullPath(fullPathForFilesystemPath(path)); @@ -124,7 +124,7 @@ public class LocalFilesystem extends Filesystem { } LocalFilesystemURL requestedURL; - + // Check whether the supplied path is absolute or relative if (directory && !path.endsWith("/")) { path += "/"; @@ -134,7 +134,7 @@ public class LocalFilesystem extends Filesystem { } else { requestedURL = localUrlforFullPath(normalizePath(inputURL.path + "/" + path)); } - + File fp = new File(this.filesystemPathForURL(requestedURL)); if (create) { @@ -189,11 +189,16 @@ public class LocalFilesystem extends Filesystem { } @Override + public long getFreeSpaceInBytes() { + return DirectoryManager.getFreeSpaceInBytes(rootUri.getPath()); + } + + @Override public boolean recursiveRemoveFileAtLocalURL(LocalFilesystemURL inputURL) throws FileExistsException { File directory = new File(filesystemPathForURL(inputURL)); return removeDirRecursively(directory); } - + protected boolean removeDirRecursively(File directory) throws FileExistsException { if (directory.isDirectory()) { for (File file : directory.listFiles()) { @@ -329,7 +334,7 @@ public class LocalFilesystem extends Filesystem { // The destination does not exist so we should fail. throw new FileNotFoundException("The source does not exist"); } - + // Figure out where we should be copying to final LocalFilesystemURL destinationURL = makeDestinationURL(newName, srcURL, destURL, srcURL.isDirectory); @@ -364,7 +369,7 @@ public class LocalFilesystem extends Filesystem { } return makeEntryForURL(destinationURL); } - + @Override public long writeToFileAtURL(LocalFilesystemURL inputURL, String data, int offset, boolean isBinary) throws IOException, NoModificationAllowedException { @@ -439,7 +444,7 @@ public class LocalFilesystem extends Filesystem { if (!file.exists()) { throw new FileNotFoundException("File at " + inputURL.uri + " does not exist."); } - + RandomAccessFile raf = new RandomAccessFile(filesystemPathForURL(inputURL), "rw"); try { if (raf.length() >= size) { http://git-wip-us.apache.org/repos/asf/cordova-plugin-file/blob/4097a985/tests/tests.js ---------------------------------------------------------------------- diff --git a/tests/tests.js b/tests/tests.js index 2cb3fd5..34200fa 100644 --- a/tests/tests.js +++ b/tests/tests.js @@ -30,7 +30,7 @@ exports.defineAutoTests = function () { var isIndexedDBShim = isBrowser && !isChrome; // Firefox and IE for example var isWindows = (cordova.platformId === "windows" || cordova.platformId === "windows8"); - + var MEDIUM_TIMEOUT = 15000; describe('File API', function () { @@ -231,8 +231,12 @@ exports.defineAutoTests = function () { expect(fileSystem.root.toURL()).toMatch(/\/$/); done(); }; + + // Request a little bit of space on the filesystem, unless we're running in a browser where that could cause a prompt. + var spaceRequired = isBrowser ? 0 : 1024; + // retrieve PERSISTENT file system - window.requestFileSystem(LocalFileSystem.PERSISTENT, 0, win, failed.bind(null, done, 'window.requestFileSystem - Error retrieving PERSISTENT file system')); + window.requestFileSystem(LocalFileSystem.PERSISTENT, spaceRequired, win, failed.bind(null, done, 'window.requestFileSystem - Error retrieving PERSISTENT file system')); }); it("file.spec.5 should be able to retrieve a TEMPORARY file system", function (done) { var win = function (fileSystem) { --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cordova.apache.org For additional commands, e-mail: commits-h...@cordova.apache.org
