Merge branch 'cleancacheservice' into 'master'

CleanCacheService

This creates `CleanCacheService` to do all of the cache clean up at the lowest possible priority.  It also adds a preference to set how long to keep cached APKs.

See merge request !260

app/src/androidTest/java/org/fdroid/fdroid/UtilsTest.java

@@ -1,13 +1,18 @@
 
 package org.fdroid.fdroid;
 
+import android.app.Instrumentation;
 import android.content.Context;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.runner.AndroidJUnit4;
 
+import org.apache.commons.io.FileUtils;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.io.File;
+import java.io.IOException;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -137,4 +142,34 @@ public class UtilsTest {
     }
 
     // TODO write tests that work with a Certificate
+
+    @Test
+    public void testClearOldFiles() throws IOException, InterruptedException {
+        Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
+        File dir = new File(TestUtils.getWriteableDir(instrumentation), "clearOldFiles");
+        FileUtils.deleteQuietly(dir);
+        dir.mkdirs();
+        assertTrue(dir.isDirectory());
+
+        File first = new File(dir, "first");
+        File second = new File(dir, "second");
+        assertFalse(first.exists());
+        assertFalse(second.exists());
+
+        first.createNewFile();
+        assertTrue(first.exists());
+
+        Thread.sleep(7000);
+        second.createNewFile();
+        assertTrue(second.exists());
+
+        Utils.clearOldFiles(dir, 3);
+        assertFalse(first.exists());
+        assertTrue(second.exists());
+
+        Thread.sleep(7000);
+        Utils.clearOldFiles(dir, 3);
+        assertFalse(first.exists());
+        assertFalse(second.exists());
+    }
 }

app/src/main/AndroidManifest.xml

@@ -445,6 +445,9 @@
         <service
             android:name=".net.DownloadCompleteService"
             android:exported="false" />
+        <service
+            android:name=".CleanCacheService"
+            android:exported="false" />
         <service android:name=".net.WifiStateChangeService" />
         <service android:name=".localrepo.SwapService" />
     </application>

app/src/main/java/org/fdroid/fdroid/CleanCacheService.java

@@ -0,0 +1,84 @@
+package org.fdroid.fdroid;
+
+import android.app.AlarmManager;
+import android.app.IntentService;
+import android.app.PendingIntent;
+import android.content.Context;
+import android.content.Intent;
+import android.os.Process;
+import android.os.SystemClock;
+import android.util.Log;
+
+import org.apache.commons.io.FileUtils;
+
+import java.io.File;
+
+/**
+ * Handles cleaning up caches files that are not going to be used, and do not
+ * block the operation of the app itself.  For things that must happen before
+ * F-Droid starts normal operation, that should go into
+ * {@link FDroidApp#onCreate()}
+ */
+public class CleanCacheService extends IntentService {
+    public static final String TAG = "CleanCacheService";
+
+    /**
+     * Schedule or cancel this service to update the app index, according to the
+     * current preferences. Should be called a) at boot, b) if the preference
+     * is changed, or c) on startup, in case we get upgraded.
+     */
+    public static void schedule(Context context) {
+        long keepTime = Preferences.get().getKeepCacheTime();
+        long interval = 604800000; // 1 day
+        if (keepTime < interval) {
+            interval = keepTime * 1000;
+        }
+        Log.i(TAG, "schedule " + keepTime + " " + interval);
+
+        Intent intent = new Intent(context, CleanCacheService.class);
+        PendingIntent pending = PendingIntent.getService(context, 0, intent, 0);
+
+        AlarmManager alarm = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);
+        alarm.cancel(pending);
+        alarm.setInexactRepeating(AlarmManager.ELAPSED_REALTIME,
+                SystemClock.elapsedRealtime() + 5000, interval, pending);
+    }
+
+    public CleanCacheService() {
+        super("CleanCacheService");
+    }
+
+    @Override
+    protected void onHandleIntent(Intent intent) {
+        Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST);
+        Utils.clearOldFiles(Utils.getApkCacheDir(this), Preferences.get().getKeepCacheTime());
+        deleteStrayIndexFiles();
+    }
+
+    /**
+     * Delete index files which were downloaded, but not removed (e.g. due to F-Droid being
+     * force closed during processing of the file, before getting a chance to delete). This
+     * may include both "index-*-downloaded" and "index-*-extracted.xml" files.
+     * <p/>
+     * Note that if the SD card is not ready, then the cache directory will probably not be
+     * available. In this situation no files will be deleted (and thus they may still exist
+     * after the SD card becomes available).
+     */
+    private void deleteStrayIndexFiles() {
+        File cacheDir = getCacheDir();
+        if (cacheDir == null) {
+            return;
+        }
+
+        final File[] files = cacheDir.listFiles();
+        if (files == null) {
+            return;
+        }
+
+        for (File f : files) {
+            if (f.getName().startsWith("index-")) {
+                FileUtils.deleteQuietly(f);
+            }
+        }
+    }
+}

app/src/main/java/org/fdroid/fdroid/FDroidApp.java

@@ -136,6 +136,9 @@ public class FDroidApp extends Application {
         }
     }
 
+    /**
+     * Initialize the settings needed to run a local swap repo.
+     */
     public static void initWifiSettings() {
         port = 8888;
         ipAddressString = null;
@@ -182,26 +185,14 @@ public class FDroidApp extends Application {
         updateLanguage();
         ACRA.init(this);
 
-        // Needs to be setup before anything else tries to access it.
-        // Perhaps the constructor is a better place, but then again,
-        // it is more deterministic as to when this gets called...
-        Preferences.setup(this);
-        curTheme = Preferences.get().getTheme();
-
-        // Apply the Google PRNG fixes to properly seed SecureRandom
         PRNGFixes.apply();
 
-        // Check that the installed app cache hasn't gotten out of sync somehow.
+        Preferences.setup(this);
-        // e.g. if we crashed/ran out of battery half way through responding
+        curTheme = Preferences.get().getTheme();
-        // to a package installed intent. It doesn't really matter where
-        // we put this in the bootstrap process, because it runs on a different
-        // thread, which will be delayed by some seconds to avoid an error where
-        // the database is locked due to the database updater.
-        InstalledAppCacheUpdater.updateInBackground(getApplicationContext());
-
-        // make sure the current proxy stuff is configured
         Preferences.get().configureProxy();
 
+        InstalledAppCacheUpdater.updateInBackground(getApplicationContext());
+
         // If the user changes the preference to do with filtering rooted apps,
         // it is easier to just notify a change in the app provider,
         // so that the newly updated list will correctly filter relevant apps.
@@ -230,29 +221,7 @@ public class FDroidApp extends Application {
             }
         });
 
-        // Clear cached apk files. We used to just remove them after they'd
+        CleanCacheService.schedule(this);
-        // been installed, but this causes problems for proprietary gapps
-        // users since the introduction of verification (on pre-4.2 Android),
-        // because the install intent says it's finished when it hasn't.
-        if (!Preferences.get().shouldCacheApks()) {
-            Utils.deleteFiles(Utils.getApkCacheDir(this), null, ".apk");
-        }
-
-        // Index files which downloaded, but were not removed (e.g. due to F-Droid being force
-        // closed during processing of the file, before getting a chance to delete). This may
-        // include both "index-*-downloaded" and "index-*-extracted.xml" files. The first is from
-        // either signed or unsigned repos, and the later is from signed repos.
-        Utils.deleteFiles(getCacheDir(), "index-", null);
-
-        // As above, but for legacy F-Droid clients that downloaded under a different name, and
-        // extracted to the files directory rather than the cache directory.
-        // TODO: This can be removed in a a few months or a year (e.g. 2016) because people will
-        // have upgraded their clients, this code will have executed, and they will not have any
-        // left over files any more. Even if they do hold off upgrading until this code is removed,
-        // the only side effect is that they will have a few more MiB of storage taken up on their
-        // device until they uninstall and re-install F-Droid.
-        Utils.deleteFiles(getCacheDir(), "dl-", null);
-        Utils.deleteFiles(getFilesDir(), "index-", null);
 
         UpdateService.schedule(getApplicationContext());
         bluetoothAdapter = getBluetoothAdapter();
@@ -278,9 +247,6 @@ public class FDroidApp extends Application {
                 .build();
         ImageLoader.getInstance().init(config);
 
-        // TODO reintroduce PinningTrustManager and MemorizingTrustManager
-
-        // initialized the local repo information
         FDroidApp.initWifiSettings();
         startService(new Intent(this, WifiStateChangeService.class));
         // if the HTTPS pref changes, then update all affected things

app/src/main/java/org/fdroid/fdroid/Preferences.java

@@ -32,9 +32,11 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
 
     private static final String TAG = "Preferences";
 
+    private final Context context;
     private final SharedPreferences preferences;
 
     private Preferences(Context context) {
+        this.context = context;
         preferences = PreferenceManager.getDefaultSharedPreferences(context);
         preferences.registerOnSharedPreferenceChangeListener(this);
         if (preferences.getString(PREF_LOCAL_REPO_NAME, null) == null) {
@@ -52,7 +54,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
     public static final String PREF_INCOMP_VER = "incompatibleVersions";
     public static final String PREF_THEME = "theme";
     public static final String PREF_IGN_TOUCH = "ignoreTouchscreen";
-    public static final String PREF_CACHE_APK = "cacheDownloaded";
+    public static final String PREF_KEEP_CACHE_TIME = "keepCacheFor";
     public static final String PREF_UNSTABLE_UPDATES = "unstableUpdates";
     public static final String PREF_EXPERT = "expert";
     public static final String PREF_PRIVILEGED_INSTALLER = "privilegedInstaller";
@@ -72,7 +74,7 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
     private static final int DEFAULT_UPD_HISTORY = 14;
     private static final boolean DEFAULT_PRIVILEGED_INSTALLER = false;
     //private static final boolean DEFAULT_LOCAL_REPO_BONJOUR = true;
-    private static final boolean DEFAULT_CACHE_APK = false;
+    private static final long DEFAULT_KEEP_CACHE_SECONDS = 86400;  // one day
     private static final boolean DEFAULT_UNSTABLE_UPDATES = false;
     //private static final boolean DEFAULT_LOCAL_REPO_HTTPS = false;
     private static final boolean DEFAULT_INCOMP_VER = false;
@@ -139,8 +141,32 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
         PreferencesCompat.apply(preferences.edit().putBoolean(PREF_POST_PRIVILEGED_INSTALL, postInstall));
     }
 
-    public boolean shouldCacheApks() {
+    /**
-        return preferences.getBoolean(PREF_CACHE_APK, DEFAULT_CACHE_APK);
+     * Old preference replaced by {@link #PREF_KEEP_CACHE_TIME}
+     */
+    private static final String PREF_CACHE_APK = "cacheDownloaded";
+
+    /**
+     * Time in seconds to keep cached files.  Anything that has been around longer will be deleted
+     */
+    public long getKeepCacheTime() {
+        String value = preferences.getString(PREF_KEEP_CACHE_TIME, String.valueOf(DEFAULT_KEEP_CACHE_SECONDS));
+
+        if (preferences.contains(PREF_CACHE_APK)) {
+            if (preferences.getBoolean(PREF_CACHE_APK, false)) {
+                value = context.getString(R.string.keep_forever);
+            }
+            SharedPreferences.Editor editor = preferences.edit();
+            editor.remove(PREF_CACHE_APK);
+            editor.putString(PREF_KEEP_CACHE_TIME, value);
+            PreferencesCompat.apply(editor);
+        }
+
+        try {
+            return Long.parseLong(value);
+        } catch (NumberFormatException e) {
+            return DEFAULT_KEEP_CACHE_SECONDS;
+        }
     }
 
     public boolean getUnstableUpdates() {
@@ -347,6 +373,9 @@ public final class Preferences implements SharedPreferences.OnSharedPreferenceCh
 
     private static Preferences instance;
 
+    /**
+     * Needs to be setup before anything else tries to access it.
+     */
     public static void setup(Context context) {
         if (instance != null) {
             final String error = "Attempted to reinitialize preferences after it " +

app/src/main/java/org/fdroid/fdroid/RepoUpdater.java

@@ -92,15 +92,6 @@ public class RepoUpdater {
         return hasChanged;
     }
 
-    private static void cleanupDownloader(Downloader d) {
-        if (d == null || d.outputFile == null) {
-            return;
-        }
-        if (!d.outputFile.delete()) {
-            Log.w(TAG, "Couldn't delete file: " + d.outputFile.getAbsolutePath());
-        }
-    }
-
     private Downloader downloadIndex() throws UpdateException {
         Downloader downloader = null;
         try {
@@ -115,7 +106,11 @@ public class RepoUpdater {
             }
 
         } catch (IOException e) {
-            cleanupDownloader(downloader);
+            if (downloader != null && downloader.outputFile != null) {
+                if (!downloader.outputFile.delete()) {
+                    Log.w(TAG, "Couldn't delete file: " + downloader.outputFile.getAbsolutePath());
+                }
+            }
 
             throw new UpdateException(repo, "Error getting index file", e);
         } catch (InterruptedException e) {
@@ -202,8 +197,10 @@ public class RepoUpdater {
         } finally {
             FDroidApp.enableSpongyCastleOnLollipop();
             Utils.closeQuietly(indexInputStream);
-            if (downloadedFile != null && !downloadedFile.delete()) {
+            if (downloadedFile != null) {
-                Log.w(TAG, "Couldn't delete file: " + downloadedFile.getAbsolutePath());
+                if (!downloadedFile.delete()) {
+                    Log.w(TAG, "Couldn't delete file: " + downloadedFile.getAbsolutePath());
+                }
             }
         }
     }

app/src/main/java/org/fdroid/fdroid/UpdateService.java

@@ -101,9 +101,11 @@ public class UpdateService extends IntentService implements ProgressListener {
         context.startService(intent);
     }
 
-    // Schedule (or cancel schedule for) this service, according to the
+    /**
-    // current preferences. Should be called a) at boot, b) if the preference
+     * Schedule or cancel this service to update the app index, according to the
-    // is changed, or c) on startup, in case we get upgraded.
+     * current preferences. Should be called a) at boot, b) if the preference
+     * is changed, or c) on startup, in case we get upgraded.
+     */
     public static void schedule(Context ctx) {
 
         SharedPreferences prefs = PreferenceManager

app/src/main/java/org/fdroid/fdroid/Utils.java

@@ -37,6 +37,7 @@ import com.nostra13.universalimageloader.core.assist.ImageScaleType;
 import com.nostra13.universalimageloader.core.display.FadeInBitmapDisplayer;
 import com.nostra13.universalimageloader.utils.StorageUtils;
 
+import org.apache.commons.io.FileUtils;
 import org.fdroid.fdroid.compat.FileCompat;
 import org.fdroid.fdroid.data.Apk;
 import org.fdroid.fdroid.data.Repo;
@@ -325,14 +326,37 @@ public final class Utils {
      * Using {@link org.fdroid.fdroid.installer.Installer#installPackage(File, String, String)}
      * is fine since that does the right thing.
      */
-    public static SanitizedFile getApkCacheDir(Context context) {
+    public static File getApkCacheDir(Context context) {
-        final SanitizedFile apkCacheDir = new SanitizedFile(StorageUtils.getCacheDirectory(context, true), "apks");
+        File apkCacheDir = new File(StorageUtils.getCacheDirectory(context, true), "apks");
+        if (apkCacheDir.isFile()) {
+            apkCacheDir.delete();
+        }
         if (!apkCacheDir.exists()) {
             apkCacheDir.mkdir();
         }
         return apkCacheDir;
     }
 
+    /**
+     * Recursively delete files in {@code dir} that were last modified
+     * {@code secondsAgo} seconds ago, e.g. when it was downloaded.
+     *
+     * @param dir        The directory to recurse in
+     * @param secondsAgo The number of seconds old that marks a file for deletion.
+     */
+    public static void clearOldFiles(File dir, long secondsAgo) {
+        long olderThan = System.currentTimeMillis() - (secondsAgo * 1000L);
+        for (File f : dir.listFiles()) {
+            if (f.isDirectory()) {
+                clearOldFiles(f, olderThan);
+                f.delete();
+            }
+            if (FileUtils.isFileOlder(f, olderThan)) {
+                f.delete();
+            }
+        }
+    }
+
     public static String calcFingerprint(String keyHexString) {
         if (TextUtils.isEmpty(keyHexString)
                 || keyHexString.matches(".*[^a-fA-F0-9].*")) {
@@ -647,40 +671,6 @@ public final class Utils {
         }
     }
 
-    /**
-     * Remove all files from the {@param directory} either beginning with {@param startsWith}
-     * or ending with {@param endsWith}. Note that if the SD card is not ready, then the
-     * cache directory will probably not be available. In this situation no files will be
-     * deleted (and thus they may still exist after the SD card becomes available).
-     */
-    public static void deleteFiles(@Nullable File directory, @Nullable String startsWith, @Nullable String endsWith) {
-
-        if (directory == null) {
-            return;
-        }
-
-        final File[] files = directory.listFiles();
-        if (files == null) {
-            return;
-        }
-
-        if (startsWith != null) {
-            debugLog(TAG, "Cleaning up files in " + directory + " that start with \"" + startsWith + "\"");
-        }
-
-        if (endsWith != null) {
-            debugLog(TAG, "Cleaning up files in " + directory + " that end with \"" + endsWith + "\"");
-        }
-
-        for (File f : files) {
-            if (((startsWith != null && f.getName().startsWith(startsWith))
-                        || (endsWith != null && f.getName().endsWith(endsWith)))
-                    && !f.delete()) {
-                Log.w(TAG, "Couldn't delete cache file " + f);
-            }
-        }
-    }
-
     public static void debugLog(String tag, String msg) {
         if (BuildConfig.DEBUG) {
             Log.d(tag, msg);

app/src/main/java/org/fdroid/fdroid/compat/FileCompat.java

@@ -85,20 +85,21 @@ public class FileCompat {
         }
     }
 
+    /**
+     * Set a {@link SanitizedFile} readable by all if {@code readable} is {@code true}.
+     *
+     * @return {@code true} if the operation succeeded
+     */
     @TargetApi(9)
-    public static boolean setReadable(SanitizedFile file, boolean readable, boolean ownerOnly) {
+    public static boolean setReadable(SanitizedFile file, boolean readable) {
-
         if (Build.VERSION.SDK_INT >= 9) {
-            return file.setReadable(readable, ownerOnly);
+            return file.setReadable(readable, false);
         }
-        String mode;
         if (readable) {
-            mode = ownerOnly ? "0600" : "0644";
+            return setMode(file, "0644");
         } else {
-            mode = "0000";
+            return setMode(file, "0000");
         }
-        return setMode(file, mode);
-
     }
 
     private static boolean setMode(SanitizedFile file, String mode) {

app/src/main/java/org/fdroid/fdroid/data/InstalledAppCacheUpdater.java

@@ -51,8 +51,13 @@ public final class InstalledAppCacheUpdater {
 
     /**
      * Ensure our database of installed apps is in sync with what the PackageManager tells us is installed.
-     * Once completed, the relevant ContentProviders will be notified of any changes to installed statuses.
+     * The installed app cache hasn't gotten out of sync somehow, e.g. if we crashed/ran out of battery
-     * This method returns immediately, and will continue to work in an AsyncTask.
+     * half way through responding to a package installed {@link android.content.Intent}. Once completed,
+     * the relevant {@link android.content.ContentProvider}s will be notified of any changes to installed
+     * statuses. This method returns immediately, and will continue to work in an AsyncTask.  It doesn't
+     * really matter where we put this in the bootstrap process, because it runs on a different thread,
+     * which will be delayed by some seconds to avoid an error where the database is locked due to the
+     * database updater.
      */
     public static void updateInBackground(Context context) {
         InstalledAppCacheUpdater updater = new InstalledAppCacheUpdater(context);

app/src/main/java/org/fdroid/fdroid/installer/Installer.java

@@ -222,7 +222,7 @@ public abstract class Installer {
             // have access is insecure, because apps with permission to write to the external
             // storage can overwrite the app between F-Droid asking for it to be installed and
             // the installer actually installing it.
-            FileCompat.setReadable(apkToInstall, true, false);
+            FileCompat.setReadable(apkToInstall, true);
             installPackageInternal(apkToInstall);
 
             NotificationManager nm = (NotificationManager)

app/src/main/java/org/fdroid/fdroid/views/fragments/PreferencesFragment.java

@@ -39,7 +39,7 @@ public class PreferencesFragment extends PreferenceFragment
         Preferences.PREF_IGN_TOUCH,
         Preferences.PREF_LOCAL_REPO_NAME,
         Preferences.PREF_LANGUAGE,
-        Preferences.PREF_CACHE_APK,
+        Preferences.PREF_KEEP_CACHE_TIME,
         Preferences.PREF_EXPERT,
         Preferences.PREF_PRIVILEGED_INSTALLER,
         Preferences.PREF_ENABLE_PROXY,
@@ -143,8 +143,8 @@ public class PreferencesFragment extends PreferenceFragment
                 }
                 break;
 
-            case Preferences.PREF_CACHE_APK:
+            case Preferences.PREF_KEEP_CACHE_TIME:
-                checkSummary(key, R.string.cache_downloaded_on);
+                entrySummary(key);
                 break;
 
             case Preferences.PREF_EXPERT:

app/src/main/res/values/array.xml

@@ -10,6 +10,15 @@
         <item>@string/interval_2w</item>
     </string-array>
 
+    <string-array name="keepCacheNames">
+        <item>@string/keep_hour</item>
+        <item>@string/keep_day</item>
+        <item>@string/keep_week</item>
+        <item>@string/keep_month</item>
+        <item>@string/keep_year</item>
+        <item>@string/keep_forever</item>
+    </string-array>
+
     <string-array name="themeNames">
         <item>@string/theme_light</item>
         <item>@string/theme_dark</item>

app/src/main/res/values/donottranslate.xml

@@ -23,6 +23,15 @@
         <item>336</item>
     </string-array>
 
+    <string-array name="keepCacheValues">
+        <item>3600</item>
+        <item>86400</item>
+        <item>604800</item>
+        <item>2592000</item>
+        <item>31449600</item>
+        <item>2147483647</item>
+    </string-array>
+
     <string-array name="themeValues">
         <item>light</item>
         <item>dark</item>

app/src/main/res/values/strings.xml

@@ -10,8 +10,8 @@
     <string name="by_author">by</string>
     <string name="delete">Delete</string>
     <string name="enable_nfc_send">Enable NFC Send…</string>
-    <string name="cache_downloaded">Cache packages</string>
+    <string name="cache_downloaded">Keep cached apps</string>
-    <string name="cache_downloaded_on">Keep downloaded package files on device</string>
+    <string name="cache_downloaded_on">Keep downloaded APK files on device</string>
     <string name="updates">Updates</string>
     <string name="unstable_updates">Unstable updates</string>
     <string name="unstable_updates_summary">Suggest updates to unstable versions</string>
@@ -383,6 +383,13 @@
     <string name="interval_1w">Weekly</string>
     <string name="interval_2w">Every 2 weeks</string>
 
+    <string name="keep_hour">1 Hour</string>
+    <string name="keep_day">1 Day</string>
+    <string name="keep_week">1 Week</string>
+    <string name="keep_month">1 Month</string>
+    <string name="keep_year">1 Year</string>
+    <string name="keep_forever">Forever</string>
+
     <string name="theme_light">Light</string>
     <string name="theme_dark">Dark</string>
     <string name="theme_night">Night</string>

app/src/main/res/xml/preferences.xml

@@ -73,9 +73,10 @@
             android:dependency="enableProxy" />
     </PreferenceCategory>
     <PreferenceCategory android:title="@string/other">
-        <CheckBoxPreference android:title="@string/cache_downloaded"
+        <ListPreference android:title="@string/cache_downloaded"
-            android:defaultValue="false"
+            android:key="keepCacheFor"
-            android:key="cacheDownloaded" />
+            android:entries="@array/keepCacheNames"
+            android:entryValues="@array/keepCacheValues" />
         <CheckBoxPreference android:title="@string/expert"
             android:defaultValue="false"
             android:key="expert" />