Support for file URI destinations + last modified timestamp
Steve Howard [Tue, 13 Jul 2010 00:24:17 +0000 (17:24 -0700)]
File URI destinations:
* permission checking in DownloadProvider
* implementation in Helpers.generateSaveFile().  it's a fairly small
  amount of logic added here, but I did some significant method
  extraction to simplify this change and clean up the code in general.
* added test case

Last modified timestamp:
* made DownloadProvider use a SystemFacade for getting system time, so I could properly test timestamps
* updated test cases to cover last modified time + handle new ordering

src/com/android/providers/downloads/DownloadProvider.java
src/com/android/providers/downloads/Helpers.java
tests/src/com/android/providers/downloads/AbstractDownloadManagerFunctionalTest.java
tests/src/com/android/providers/downloads/PublicApiFunctionalTest.java

index d7c24f9..7070f31 100644 (file)
@@ -16,6 +16,8 @@
 
 package com.android.providers.downloads;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import android.content.ContentProvider;
 import android.content.ContentValues;
 import android.content.Context;
@@ -110,6 +112,9 @@ public final class DownloadProvider extends ContentProvider {
     private int mSystemUid = -1;
     private int mDefContainerUid = -1;
 
+    @VisibleForTesting
+    SystemFacade mSystemFacade;
+
     /**
      * Creates and updated database on demand when opening it.
      * Helper class to create database the first time the provider is
@@ -172,6 +177,10 @@ public final class DownloadProvider extends ContentProvider {
      */
     @Override
     public boolean onCreate() {
+        if (mSystemFacade == null) {
+            mSystemFacade = new RealSystemFacade();
+        }
+
         mOpenHelper = new DatabaseHelper(getContext());
         // Initialize the system uid
         mSystemUid = Process.SYSTEM_UID;
@@ -294,9 +303,16 @@ public final class DownloadProvider extends ContentProvider {
             if (getContext().checkCallingPermission(Downloads.Impl.PERMISSION_ACCESS_ADVANCED)
                     != PackageManager.PERMISSION_GRANTED
                     && dest != Downloads.Impl.DESTINATION_EXTERNAL
-                    && dest != Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) {
+                    && dest != Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE
+                    && dest != Downloads.Impl.DESTINATION_FILE_URI) {
                 throw new SecurityException("unauthorized destination code");
             }
+            if (dest == Downloads.Impl.DESTINATION_FILE_URI) {
+                getContext().enforcePermission(
+                        android.Manifest.permission.WRITE_EXTERNAL_STORAGE,
+                        Binder.getCallingPid(), Binder.getCallingUid(),
+                        "need WRITE_EXTERNAL_STORAGE permission to use DESTINATION_FILE_URI");
+            }
             filteredValues.put(Downloads.Impl.COLUMN_DESTINATION, dest);
         }
         Integer vis = values.getAsInteger(Downloads.Impl.COLUMN_VISIBILITY);
@@ -313,7 +329,8 @@ public final class DownloadProvider extends ContentProvider {
         }
         copyInteger(Downloads.Impl.COLUMN_CONTROL, values, filteredValues);
         filteredValues.put(Downloads.Impl.COLUMN_STATUS, Downloads.Impl.STATUS_PENDING);
-        filteredValues.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, System.currentTimeMillis());
+        filteredValues.put(Downloads.Impl.COLUMN_LAST_MODIFICATION,
+                           mSystemFacade.currentTimeMillis());
         String pckg = values.getAsString(Downloads.Impl.COLUMN_NOTIFICATION_PACKAGE);
         String clazz = values.getAsString(Downloads.Impl.COLUMN_NOTIFICATION_CLASS);
         if (pckg != null && clazz != null) {
@@ -733,7 +750,7 @@ public final class DownloadProvider extends ContentProvider {
             throw new FileNotFoundException("couldn't open file");
         } else {
             ContentValues values = new ContentValues();
-            values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, System.currentTimeMillis());
+            values.put(Downloads.Impl.COLUMN_LAST_MODIFICATION, mSystemFacade.currentTimeMillis());
             update(uri, values, null, null);
         }
         return ret;
index 6bb5093..2705a7c 100644 (file)
@@ -35,7 +35,7 @@ import android.util.Config;
 import android.util.Log;
 import android.webkit.MimeTypeMap;
 
-import java.io.File; 
+import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.util.Random;
@@ -76,6 +76,17 @@ public class Helpers {
     }
 
     /**
+     * Exception thrown from methods called by generateSaveFile() for any fatal error.
+     */
+    private static class GenerateSaveFileError extends Exception {
+        int mStatus;
+
+        public GenerateSaveFileError(int status) {
+            mStatus = status;
+        }
+    }
+
+    /**
      * Creates a filename (where the file should be saved) from a uri.
      */
     public static DownloadFileInfo generateSaveFile(
@@ -88,16 +99,80 @@ public class Helpers {
             int destination,
             int contentLength) throws FileNotFoundException {
 
-        /*
-         * Don't download files that we won't be able to handle
-         */
+        if (!canHandleDownload(context, mimeType, destination)) {
+            return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE);
+        }
+
+        String fullFilename;
+        try {
+            if (destination == Downloads.Impl.DESTINATION_FILE_URI) {
+                fullFilename = getPathForFileUri(hint);
+            } else {
+                fullFilename = chooseFullPath(context, url, hint, contentDisposition,
+                                              contentLocation, mimeType, destination,
+                                              contentLength);
+            }
+        } catch (GenerateSaveFileError exc) {
+            return new DownloadFileInfo(null, null, exc.mStatus);
+        }
+
+        return new DownloadFileInfo(fullFilename, new FileOutputStream(fullFilename), 0);
+    }
+
+    private static String getPathForFileUri(String hint) throws GenerateSaveFileError {
+        Uri uri = Uri.parse(hint);
+        if (!uri.getScheme().equals("file")) {
+            Log.d(Constants.TAG, "Not a file URI: " + hint);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR);
+        }
+
+        String path = uri.getSchemeSpecificPart();
+        if (new File(path).exists()) {
+            Log.d(Constants.TAG, "File already exists: " + path);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR);
+        }
+
+        return path;
+    }
+
+    private static String chooseFullPath(Context context, String url, String hint,
+                                         String contentDisposition, String contentLocation,
+                                         String mimeType, int destination, int contentLength)
+            throws GenerateSaveFileError {
+        File base = locateDestinationDirectory(context, mimeType, destination, contentLength);
+        String filename = chooseFilename(url, hint, contentDisposition, contentLocation,
+                                         destination);
+
+        // Split filename between base and extension
+        // Add an extension if filename does not have one
+        String extension = null;
+        int dotIndex = filename.indexOf('.');
+        if (dotIndex < 0) {
+            extension = chooseExtensionFromMimeType(mimeType, true);
+        } else {
+            extension = chooseExtensionFromFilename(mimeType, destination, filename, dotIndex);
+            filename = filename.substring(0, dotIndex);
+        }
+
+        boolean recoveryDir = Constants.RECOVERY_DIRECTORY.equalsIgnoreCase(filename + extension);
+
+        filename = base.getPath() + File.separator + filename;
+
+        if (Constants.LOGVV) {
+            Log.v(Constants.TAG, "target file: " + filename + extension);
+        }
+
+        return chooseUniqueFilename(destination, filename, extension, recoveryDir);
+    }
+
+    private static boolean canHandleDownload(Context context, String mimeType, int destination) {
         if (destination == Downloads.Impl.DESTINATION_EXTERNAL
                 || destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE) {
             if (mimeType == null) {
                 if (Config.LOGD) {
                     Log.d(Constants.TAG, "external download with no mime type not allowed");
                 }
-                return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE);
+                return false;
             }
             if (!DrmRawContent.DRM_MIMETYPE_MESSAGE_STRING.equalsIgnoreCase(mimeType)) {
                 // Check to see if we are allowed to download this file. Only files
@@ -121,32 +196,19 @@ public class Helpers {
                     if (Config.LOGD) {
                         Log.d(Constants.TAG, "no handler found for type " + mimeType);
                     }
-                    return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_NOT_ACCEPTABLE);
+                    return false;
                 }
             }
         }
-        String filename = chooseFilename(
-                url, hint, contentDisposition, contentLocation, destination);
-
-        // Split filename between base and extension
-        // Add an extension if filename does not have one
-        String extension = null;
-        int dotIndex = filename.indexOf('.');
-        if (dotIndex < 0) {
-            extension = chooseExtensionFromMimeType(mimeType, true);
-        } else {
-            extension = chooseExtensionFromFilename(
-                    mimeType, destination, filename, dotIndex);
-            filename = filename.substring(0, dotIndex);
-        }
-
-        /*
-         *  Locate the directory where the file will be saved
-         */
+        return true;
+    }
 
+    private static File locateDestinationDirectory(Context context, String mimeType,
+                                                   int destination, int contentLength)
+            throws GenerateSaveFileError {
         File base = null;
         StatFs stat = null;
-        // DRM messages should be temporarily stored internally and then passed to 
+        // DRM messages should be temporarily stored internally and then passed to
         // the DRM content provider
         if (destination == Downloads.Impl.DESTINATION_CACHE_PARTITION
                 || destination == Downloads.Impl.DESTINATION_CACHE_PARTITION_PURGEABLE
@@ -170,8 +232,7 @@ public class Helpers {
                         Log.d(Constants.TAG,
                                 "download aborted - not enough free space in internal storage");
                     }
-                    return new DownloadFileInfo(null, null,
-                            Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
+                    throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
                 } else {
                     // Recalculate available space and try again.
                     stat.restat(base.getPath());
@@ -192,8 +253,7 @@ public class Helpers {
                 if (Config.LOGD) {
                     Log.d(Constants.TAG, "download aborted - not enough free space");
                 }
-                return new DownloadFileInfo(null, null,
-                        Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
+                throw new GenerateSaveFileError(Downloads.Impl.STATUS_INSUFFICIENT_SPACE_ERROR);
             }
 
             base = new File(root + Constants.DEFAULT_DL_SUBDIR);
@@ -204,35 +264,17 @@ public class Helpers {
                     Log.d(Constants.TAG, "download aborted - can't create base directory "
                             + base.getPath());
                 }
-                return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_FILE_ERROR);
+                throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR);
             }
         } else {
             // No SD card found.
             if (Config.LOGD) {
                 Log.d(Constants.TAG, "download aborted - no external storage");
             }
-            return new DownloadFileInfo(null, null,
-                    Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR);
+            throw new GenerateSaveFileError(Downloads.Impl.STATUS_DEVICE_NOT_FOUND_ERROR);
         }
 
-        boolean recoveryDir = Constants.RECOVERY_DIRECTORY.equalsIgnoreCase(filename + extension);
-
-        filename = base.getPath() + File.separator + filename;
-
-        /*
-         * Generate a unique filename, create the file, return it.
-         */
-        if (Constants.LOGVV) {
-            Log.v(Constants.TAG, "target file: " + filename + extension);
-        }
-
-        String fullFilename = chooseUniqueFilename(
-                destination, filename, extension, recoveryDir);
-        if (fullFilename != null) {
-            return new DownloadFileInfo(fullFilename, new FileOutputStream(fullFilename), 0);
-        } else {
-            return new DownloadFileInfo(null, null, Downloads.Impl.STATUS_FILE_ERROR);
-        }
+        return base;
     }
 
     private static String chooseFilename(String url, String hint, String contentDisposition,
@@ -383,7 +425,7 @@ public class Helpers {
     }
 
     private static String chooseUniqueFilename(int destination, String filename,
-            String extension, boolean recoveryDir) {
+            String extension, boolean recoveryDir) throws GenerateSaveFileError {
         String fullFilename = filename + extension;
         if (!new File(fullFilename).exists()
                 && (!recoveryDir ||
@@ -420,7 +462,7 @@ public class Helpers {
                 sequence += sRandom.nextInt(magnitude) + 1;
             }
         }
-        return null;
+        throw new GenerateSaveFileError(Downloads.Impl.STATUS_FILE_ERROR);
     }
 
     /**
index 1394a17..06dd52a 100644 (file)
@@ -17,7 +17,6 @@
 package com.android.providers.downloads;
 
 import android.content.ComponentName;
-import android.content.ContentProvider;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
@@ -138,6 +137,7 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     protected void setUp() throws Exception {
         super.setUp();
 
+        mSystemFacade = new FakeSystemFacade();
         Context realContext = getContext();
         mTestContext = new TestContext(realContext);
         setupProviderAndResolver();
@@ -146,7 +146,6 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
         mTestContext.setResolver(mResolver);
         setContext(mTestContext);
         setupService();
-        mSystemFacade = new FakeSystemFacade();
         getService().mSystemFacade = mSystemFacade;
 
         mServer = new MockWebServer();
@@ -169,7 +168,8 @@ public abstract class AbstractDownloadManagerFunctionalTest extends
     }
 
     void setupProviderAndResolver() {
-        ContentProvider provider = new DownloadProvider();
+        DownloadProvider provider = new DownloadProvider();
+        provider.mSystemFacade = mSystemFacade;
         provider.attachInfo(mTestContext, null);
         mResolver = new MockContentResolver();
         mResolver.addProvider(PROVIDER_AUTHORITY, provider);
index 1927545..e3b278b 100644 (file)
@@ -22,14 +22,12 @@ import android.net.Uri;
 import android.os.Environment;
 import tests.http.RecordedRequest;
 
+import java.io.File;
 import java.io.FileInputStream;
 import java.io.InputStream;
 import java.net.MalformedURLException;
 
 public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTest {
-    /**
-     *
-     */
     private static final String REQUEST_PATH = "/path";
 
     class Download implements StatusReader {
@@ -87,11 +85,34 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
     }
 
     private DownloadManager mManager;
+    private File mTestDirectory;
 
     @Override
     protected void setUp() throws Exception {
         super.setUp();
         mManager = new DownloadManager(mResolver);
+
+        mTestDirectory = new File(Environment.getExternalStorageDirectory() + File.separator
+                                  + "download_manager_functional_test");
+        if (mTestDirectory.exists()) {
+            throw new RuntimeException(
+                    "Test directory on external storage already exists, cannot run");
+        }
+        if (!mTestDirectory.mkdir()) {
+            throw new RuntimeException("Couldn't create test directory: "
+                                       + mTestDirectory.getPath());
+        }
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        if (mTestDirectory != null) {
+            for (File file : mTestDirectory.listFiles()) {
+                file.delete();
+            }
+            mTestDirectory.delete();
+        }
+        super.tearDown();
     }
 
     public void testBasicRequest() throws Exception {
@@ -103,20 +124,25 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         assertEquals(getServerUri(REQUEST_PATH),
                      download.getStringField(DownloadManager.COLUMN_URI));
         assertEquals(download.mId, download.getLongField(DownloadManager.COLUMN_ID));
+        assertEquals(mSystemFacade.currentTimeMillis(),
+                     download.getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP));
 
+        mSystemFacade.incrementTimeMillis(10);
         RecordedRequest request = download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
         assertEquals("GET", request.getMethod());
         assertEquals(REQUEST_PATH, request.getPath());
 
         Uri localUri = Uri.parse(download.getStringField(DownloadManager.COLUMN_LOCAL_URI));
         assertEquals("file", localUri.getScheme());
-        assertStartsWith(Environment.getDownloadCacheDirectory().getPath(),
+        assertStartsWith("//" + Environment.getDownloadCacheDirectory().getPath(),
                          localUri.getSchemeSpecificPart());
         assertEquals("text/plain", download.getStringField(DownloadManager.COLUMN_MEDIA_TYPE));
 
         int size = FILE_CONTENT.length();
         assertEquals(size, download.getLongField(DownloadManager.COLUMN_TOTAL_SIZE_BYTES));
         assertEquals(size, download.getLongField(DownloadManager.COLUMN_BYTES_DOWNLOADED_SO_FAR));
+        assertEquals(mSystemFacade.currentTimeMillis(),
+                     download.getLongField(DownloadManager.COLUMN_LAST_MODIFIED_TIMESTAMP));
 
         assertEquals(FILE_CONTENT, download.getContents());
     }
@@ -176,12 +202,16 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         Download download1 = enqueueRequest(getRequest());
         download1.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
         enqueueEmptyResponse(HTTP_NOT_FOUND);
+
+        mSystemFacade.incrementTimeMillis(1); // ensure downloads are correctly ordered by time
         Download download2 = enqueueRequest(getRequest());
         download2.runUntilStatus(DownloadManager.STATUS_FAILED);
+
+        mSystemFacade.incrementTimeMillis(1);
         Download download3 = enqueueRequest(getRequest());
 
         Cursor cursor = mManager.query(new DownloadManager.Query());
-        checkAndCloseCursor(cursor, download1, download2, download3);
+        checkAndCloseCursor(cursor, download3, download2, download1);
 
         cursor = mManager.query(new DownloadManager.Query().setFilterById(download2.mId));
         checkAndCloseCursor(cursor, download2);
@@ -193,7 +223,7 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         cursor = mManager.query(new DownloadManager.Query()
                                 .setFilterByStatus(DownloadManager.STATUS_FAILED
                                               | DownloadManager.STATUS_SUCCESSFUL));
-        checkAndCloseCursor(cursor, download1, download2);
+        checkAndCloseCursor(cursor, download2, download1);
 
         cursor = mManager.query(new DownloadManager.Query()
                                 .setFilterByStatus(DownloadManager.STATUS_RUNNING));
@@ -224,6 +254,23 @@ public class PublicApiFunctionalTest extends AbstractDownloadManagerFunctionalTe
         fail("No exception thrown for invalid URI");
     }
 
+    public void testDestination() throws Exception {
+        enqueueResponse(HTTP_OK, FILE_CONTENT);
+        Uri destination = Uri.fromFile(mTestDirectory).buildUpon().appendPath("testfile").build();
+        Download download = enqueueRequest(getRequest().setDestinationUri(destination));
+        download.runUntilStatus(DownloadManager.STATUS_SUCCESSFUL);
+
+        Uri localUri = Uri.parse(download.getStringField(DownloadManager.COLUMN_LOCAL_URI));
+        assertEquals(destination, localUri);
+
+        InputStream stream = new FileInputStream(destination.getSchemeSpecificPart());
+        try {
+            assertEquals(FILE_CONTENT, readStream(stream));
+        } finally {
+            stream.close();
+        }
+    }
+
     private DownloadManager.Request getRequest() throws MalformedURLException {
         return getRequest(getServerUri(REQUEST_PATH));
     }