diff --git a/app/src/androidTest/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java b/app/src/androidTest/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java index b57cd1034..e084fe6db 100644 --- a/app/src/androidTest/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java +++ b/app/src/androidTest/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java @@ -22,6 +22,7 @@ import static org.junit.Assert.assertTrue; /** * This test cannot run on Robolectric unfortunately since it does not support + * getting the timestamps from the files completely. *

* This is marked with {@link LargeTest} because it always fails on the emulator * tests on GitLab CI. That excludes it from the test run there. @@ -86,4 +87,24 @@ public class CleanCacheWorkerTest { CleanCacheWorker.clearOldFiles(nonexistent, 1); CleanCacheWorker.clearOldFiles(null, 1); } + + /* + // TODO enable this once getImageCacheDir() can be mocked or provide a writable dir in the test + @Test + public void testDeleteOldIcons() throws IOException { + Context context = InstrumentationRegistry.getInstrumentation().getContext(); + File imageCacheDir = Utils.getImageCacheDir(context); + imageCacheDir.mkdirs(); + assertTrue(imageCacheDir.isDirectory()); + File oldIcon = new File(imageCacheDir, "old.png"); + assertTrue(oldIcon.createNewFile()); + Assume.assumeTrue("test environment must be able to set LastModified time", + oldIcon.setLastModified(System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 370))); + File currentIcon = new File(imageCacheDir, "current.png"); + assertTrue(currentIcon.createNewFile()); + CleanCacheWorker.deleteOldIcons(context); + assertTrue(currentIcon.exists()); + assertFalse(oldIcon.exists()); + } + */ } diff --git a/app/src/basic/java/org/fdroid/fdroid/nearby/LocalRepoManager.java b/app/src/basic/java/org/fdroid/fdroid/nearby/LocalRepoManager.java new file mode 100644 index 000000000..b1cb50b71 --- /dev/null +++ b/app/src/basic/java/org/fdroid/fdroid/nearby/LocalRepoManager.java @@ -0,0 +1,5 @@ +package org.fdroid.fdroid.nearby; + +public class LocalRepoManager { + public static final String[] WEB_ROOT_ASSET_FILES = {}; +} diff --git a/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java b/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java index ff6ac545d..14d653613 100644 --- a/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java +++ b/app/src/full/java/org/fdroid/fdroid/nearby/LocalRepoManager.java @@ -10,10 +10,10 @@ import android.graphics.Bitmap.Config; import android.graphics.Canvas; import android.graphics.drawable.BitmapDrawable; import android.graphics.drawable.Drawable; -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import android.text.TextUtils; import android.util.Log; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import org.fdroid.fdroid.FDroidApp; import org.fdroid.fdroid.Hasher; import org.fdroid.fdroid.IndexUpdater; @@ -56,7 +56,6 @@ import java.util.jar.JarOutputStream; * This class deals specifically with the webroot side of things, ensuring we have a valid index.jar * and the relevant .apk and icon files available. */ -@SuppressWarnings("LineLength") public final class LocalRepoManager { private static final String TAG = "LocalRepoManager"; @@ -65,7 +64,7 @@ public final class LocalRepoManager { private final AssetManager assetManager; private final String fdroidPackageName; - private static final String[] WEB_ROOT_ASSET_FILES = { + public static final String[] WEB_ROOT_ASSET_FILES = { "swap-icon.png", "swap-tick-done.png", "swap-tick-not-done.png", @@ -348,7 +347,8 @@ public final class LocalRepoManager { serializer = XmlPullParserFactory.newInstance().newSerializer(); } - public void build(Context context, Map apps, OutputStream output) throws IOException, LocalRepoKeyStore.InitException { + public void build(Context context, Map apps, OutputStream output) + throws IOException, LocalRepoKeyStore.InitException { serializer.setOutput(output, "UTF-8"); serializer.startDocument(null, null); serializer.startTag("", "fdroid"); @@ -356,12 +356,14 @@ public final class LocalRepoManager { // block serializer.startTag("", "repo"); serializer.attribute("", "icon", "blah.png"); - serializer.attribute("", "name", Preferences.get().getLocalRepoName() + " on " + FDroidApp.ipAddressString); + serializer.attribute("", "name", Preferences.get().getLocalRepoName() + + " on " + FDroidApp.ipAddressString); serializer.attribute("", "pubkey", Hasher.hex(LocalRepoKeyStore.get(context).getCertificate())); long timestamp = System.currentTimeMillis() / 1000L; serializer.attribute("", "timestamp", String.valueOf(timestamp)); serializer.attribute("", "version", "10"); - tag("description", "A local FDroid repo generated from apps installed on " + Preferences.get().getLocalRepoName()); + tag("description", "A local FDroid repo generated from apps installed on " + + Preferences.get().getLocalRepoName()); serializer.endTag("", "repo"); // blocks diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java b/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java index e84cd52eb..b0dd65514 100644 --- a/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java +++ b/app/src/main/java/org/fdroid/fdroid/installer/ApkFileProvider.java @@ -42,13 +42,16 @@ import java.io.IOException; * "/Android/data/[app_package_name]/cache/apks" (if card is mounted and app has * appropriate permission) or on device's file system depending incoming parameters *

  • Before installation, the APK is copied into the private data directory of the F-Droid, - * "/data/data/[app_package_name]/files/install-$random.apk"
    • + * "/data/data/[app_package_name]/files/install-$random.apk" so that the install + * process broken if the user clears the cache while it is running. *
  • The hash of the file is checked against the expected hash from the repository
    • *
  • For {@link Build.VERSION_CODES#M < android-23}, a {@code file://} {@link Uri} * pointing to the {@link File} is returned, for {@link Build.VERSION_CODES#M >= android-23}, * a {@code content://} {@code Uri} is returned using support lib's * {@link FileProvider}
    • * + * + * @see org.fdroid.fdroid.work.CleanCacheWorker */ public class ApkFileProvider extends FileProvider { diff --git a/app/src/main/java/org/fdroid/fdroid/receiver/DeviceStorageReceiver.java b/app/src/main/java/org/fdroid/fdroid/receiver/DeviceStorageReceiver.java index 08b33d394..7aaa04019 100644 --- a/app/src/main/java/org/fdroid/fdroid/receiver/DeviceStorageReceiver.java +++ b/app/src/main/java/org/fdroid/fdroid/receiver/DeviceStorageReceiver.java @@ -18,9 +18,8 @@ public class DeviceStorageReceiver extends BroadcastReceiver { if (Intent.ACTION_DEVICE_STORAGE_LOW.equals(action)) { int percentageFree = Utils.getPercent(Utils.getImageCacheDirAvailableMemory(context), Utils.getImageCacheDirTotalMemory(context)); - if (percentageFree > 2) { - CleanCacheWorker.schedule(context); - } else { + CleanCacheWorker.force(context); + if (percentageFree <= 2) { DeleteCacheService.deleteAll(context); } } diff --git a/app/src/main/java/org/fdroid/fdroid/views/main/SettingsView.java b/app/src/main/java/org/fdroid/fdroid/views/main/SettingsView.java index 4c779c99e..5aa71206c 100644 --- a/app/src/main/java/org/fdroid/fdroid/views/main/SettingsView.java +++ b/app/src/main/java/org/fdroid/fdroid/views/main/SettingsView.java @@ -2,17 +2,14 @@ package org.fdroid.fdroid.views.main; import android.annotation.TargetApi; import android.content.Context; -import androidx.appcompat.app.AppCompatActivity; import android.util.AttributeSet; import android.widget.FrameLayout; - +import androidx.appcompat.app.AppCompatActivity; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentTransaction; - import org.fdroid.fdroid.R; import org.fdroid.fdroid.views.PreferencesFragment; -@SuppressWarnings("LineLength") /** * When attached to the window, the {@link PreferencesFragment} will be added. When detached from * the window, the fragment will be removed. @@ -52,9 +49,6 @@ public class SettingsView extends FrameLayout { super.onAttachedToWindow(); AppCompatActivity activity = (AppCompatActivity) getContext(); - if (activity == null) { - throw new IllegalArgumentException("Cannot add a SettingsView to activities which are not an AppCompatActivity"); - } if (currentTransaction == null) { currentTransaction = activity.getSupportFragmentManager().beginTransaction(); @@ -71,9 +65,6 @@ public class SettingsView extends FrameLayout { super.onDetachedFromWindow(); AppCompatActivity activity = (AppCompatActivity) getContext(); - if (activity == null) { - throw new IllegalArgumentException("Cannot add a SettingsView to activities which are not an AppCompatActivity"); - } Fragment existingFragment = activity.getSupportFragmentManager().findFragmentByTag("preferences-fragment"); if (existingFragment == null) { diff --git a/app/src/main/java/org/fdroid/fdroid/work/CleanCacheWorker.java b/app/src/main/java/org/fdroid/fdroid/work/CleanCacheWorker.java index c28f37925..584b01afa 100644 --- a/app/src/main/java/org/fdroid/fdroid/work/CleanCacheWorker.java +++ b/app/src/main/java/org/fdroid/fdroid/work/CleanCacheWorker.java @@ -10,6 +10,8 @@ import androidx.annotation.NonNull; import androidx.annotation.RequiresApi; import androidx.work.Constraints; import androidx.work.ExistingPeriodicWorkPolicy; +import androidx.work.ExistingWorkPolicy; +import androidx.work.OneTimeWorkRequest; import androidx.work.PeriodicWorkRequest; import androidx.work.WorkManager; import androidx.work.Worker; @@ -18,10 +20,25 @@ import org.apache.commons.io.FileUtils; import org.fdroid.fdroid.Preferences; import org.fdroid.fdroid.Utils; import org.fdroid.fdroid.installer.ApkCache; +import org.fdroid.fdroid.nearby.LocalRepoManager; import java.io.File; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.TimeUnit; +/** + * Deletes the built-up cruft left over from various processes. + *

    + * The installation process downloads APKs to the cache, then when an APK is being + * installed, it is copied into files for running the install. Installs can happen + * fully in the background, so the user might clear the cache at any time, so the + * APK cannot be installed from the cache. Also, F-Droid is not guaranteed to get + * an event after the APK is installed, so that can't be used to delete the APK + * from files when it is no longer needed. That's where CleanCacheWorker comes in, + * it runs regularly to ensure things are cleaned up. If something blocks it from + * running, then APKs can remain in {@link org.fdroid.fdroid.installer.ApkFileProvider} + */ public class CleanCacheWorker extends Worker { public static final String TAG = "CleanCacheWorker"; @@ -30,7 +47,7 @@ public class CleanCacheWorker extends Worker { } /** - * Schedule or cancel a work request to update the app index, according to the + * Schedule or cancel a work request to clean up caches, 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. */ @@ -56,15 +73,28 @@ public class CleanCacheWorker extends Worker { Utils.debugLog(TAG, "Scheduled periodic work for cleaning the cache."); } + /** + * Force a cache cleanup. Since {@link #deleteOldInstallerFiles(Context)} + * only deletes files older than an hour, any ongoing APK install processes + * should not have their APKs are deleted out from under them. + */ + public static void force(@NonNull final Context context) { + OneTimeWorkRequest cleanCache = new OneTimeWorkRequest.Builder(CleanCacheWorker.class).build(); + WorkManager workManager = WorkManager.getInstance(context); + workManager.enqueueUniqueWork(TAG + ".force", ExistingWorkPolicy.KEEP, cleanCache); + Utils.debugLog(TAG, "Enqueued forced run for cleaning the cache."); + } + @NonNull @Override public Result doWork() { Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST); try { - deleteExpiredApksFromCache(); - deleteStrayIndexFiles(); - deleteOldInstallerFiles(); - deleteOldIcons(); + final Context context = getApplicationContext(); + deleteExpiredApksFromCache(context); + deleteStrayIndexFiles(context); + deleteOldInstallerFiles(context); + deleteOldIcons(context); return Result.success(); } catch (Exception e) { return Result.failure(); @@ -76,17 +106,20 @@ public class CleanCacheWorker extends Worker { * specified by the user in the "Keep Cache Time" preference. This removes * any APK in the cache that is older than that preference specifies. */ - private void deleteExpiredApksFromCache() { - File cacheDir = ApkCache.getApkCacheDir(getApplicationContext()); + static void deleteExpiredApksFromCache(@NonNull Context context) { + File cacheDir = ApkCache.getApkCacheDir(context); clearOldFiles(cacheDir, Preferences.get().getKeepCacheTime()); } /** * {@link org.fdroid.fdroid.installer.Installer} instances copy the APK into - * a safe place before installing. It doesn't clean up them reliably yet. + * a safe place before installing. This only deletes files older than an + * hour to avoid deleting APKs while they are still being installed. This + * also avoids deleting the nearby swap repo files since that might be + * actively in use. */ - private void deleteOldInstallerFiles() { - File filesDir = getApplicationContext().getFilesDir(); + static void deleteOldInstallerFiles(@NonNull Context context) { + File filesDir = context.getFilesDir(); if (filesDir == null) { Utils.debugLog(TAG, "The files directory doesn't exist."); return; @@ -98,8 +131,9 @@ public class CleanCacheWorker extends Worker { return; } + final List webRootAssetFiles = Arrays.asList(LocalRepoManager.WEB_ROOT_ASSET_FILES); for (File f : files) { - if (f.getName().endsWith(".apk")) { + if (f.isFile() && !f.getName().endsWith(".html") && !webRootAssetFiles.contains(f.getName())) { clearOldFiles(f, TimeUnit.HOURS.toMillis(1)); } } @@ -117,8 +151,8 @@ public class CleanCacheWorker extends Worker { * This also deletes temp files that are created by * {@link org.fdroid.fdroid.net.DownloaderFactory#create(Context, String)}, e.g. "dl-*" */ - private void deleteStrayIndexFiles() { - File cacheDir = getApplicationContext().getCacheDir(); + static void deleteStrayIndexFiles(@NonNull Context context) { + File cacheDir = context.getCacheDir(); if (cacheDir == null) { Utils.debugLog(TAG, "The cache directory doesn't exist."); return; @@ -143,8 +177,8 @@ public class CleanCacheWorker extends Worker { /** * Delete cached icons that have not been accessed in over a year. */ - private void deleteOldIcons() { - clearOldFiles(Utils.getImageCacheDir(getApplicationContext()), TimeUnit.DAYS.toMillis(365)); + static void deleteOldIcons(@NonNull Context context) { + clearOldFiles(Utils.getImageCacheDir(context), TimeUnit.DAYS.toMillis(365)); } /** diff --git a/app/src/test/java/org/fdroid/fdroid/TestUtils.java b/app/src/test/java/org/fdroid/fdroid/TestUtils.java index a24c8d7b0..478c8ca1f 100644 --- a/app/src/test/java/org/fdroid/fdroid/TestUtils.java +++ b/app/src/test/java/org/fdroid/fdroid/TestUtils.java @@ -8,6 +8,7 @@ import android.content.ContextWrapper; import android.content.pm.ProviderInfo; import android.net.Uri; import androidx.test.core.app.ApplicationProvider; +import org.apache.commons.io.IOUtils; import org.fdroid.fdroid.data.Apk; import org.fdroid.fdroid.data.ApkProvider; import org.fdroid.fdroid.data.App; @@ -190,4 +191,17 @@ public class TestUtils { modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); field.set(null, newValue); } + + public static void ls(File dir) { + Process p = null; + try { + p = Runtime.getRuntime().exec("ls -l " + dir.getAbsolutePath()); + p.waitFor(); + for (String line : IOUtils.readLines(p.getInputStream())) { + System.out.println(line); + } + } catch (IOException | InterruptedException e) { + e.printStackTrace(); + } + } } diff --git a/app/src/test/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java b/app/src/test/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java new file mode 100644 index 000000000..97783d7f7 --- /dev/null +++ b/app/src/test/java/org/fdroid/fdroid/work/CleanCacheWorkerTest.java @@ -0,0 +1,93 @@ +package org.fdroid.fdroid.work; + +import android.content.Context; +import androidx.test.core.app.ApplicationProvider; +import org.fdroid.fdroid.BuildConfig; +import org.fdroid.fdroid.Preferences; +import org.fdroid.fdroid.nearby.LocalRepoManager; +import org.fdroid.fdroid.shadows.ShadowLog; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * Test non-time-based cache deletion methods. Robolectric is lacking full + * support for getting time from files, so methods that rely on time must be + * tested in {@code CleanCacheWorkerTest} in {@code androidTest}. + */ +@RunWith(RobolectricTestRunner.class) +public class CleanCacheWorkerTest { + public static final String TAG = "CleanCacheWorkerTest"; + + private static final Context CONTEXT = ApplicationProvider.getApplicationContext(); + private static final File FILES_DIR = CONTEXT.getFilesDir(); + + @Before + public void setUp() { + ShadowLog.stream = System.out; + Preferences.setupForTests(CONTEXT); + } + + @Test + public void testDeleteOldInstallerFiles() throws IOException { + Assume.assumeTrue(BuildConfig.FLAVOR.startsWith("full")); + ArrayList webRootAssetFiles = new ArrayList<>(); + File indexHtml = new File(FILES_DIR, "index.html"); + assertTrue(indexHtml.createNewFile()); + webRootAssetFiles.add(indexHtml); + for (String name : LocalRepoManager.WEB_ROOT_ASSET_FILES) { + File f = new File(FILES_DIR, name); + assertTrue(f.createNewFile()); + webRootAssetFiles.add(f); + } + File apk = new File(FILES_DIR, "fake.apk"); + assertTrue(apk.createNewFile()); + File giantblob = new File(FILES_DIR, "giantblob"); + assertTrue(giantblob.createNewFile()); + File obf = new File(FILES_DIR, "fake.obf"); + assertTrue(obf.createNewFile()); + File zip = new File(FILES_DIR, "fake.zip"); + assertTrue(zip.createNewFile()); + CleanCacheWorker.deleteOldInstallerFiles(CONTEXT); + assertFalse(apk.exists()); + assertFalse(giantblob.exists()); + assertFalse(obf.exists()); + assertFalse(zip.exists()); + for (File f : webRootAssetFiles) { + assertTrue(f.exists()); + } + } + + /** + * Pure smoke check, Robolectric does not support file times fully. + */ + @Test + public void testDeleteExpiredApksFromCache() { + CleanCacheWorker.deleteExpiredApksFromCache(CONTEXT); + } + + /** + * Pure smoke check, Robolectric does not support file times fully. + */ + @Test + public void testDeleteStrayIndexFiles() { + CleanCacheWorker.deleteStrayIndexFiles(CONTEXT); + } + + /** + * Pure smoke check, Robolectric does not support file times fully. + */ + @Test + public void testDeleteOldIcons() { + CleanCacheWorker.deleteOldIcons(CONTEXT); + } +}