Merge pull request #5225 from XiangRongLin/extract_settings_import

Extract settings import

app/build.gradle

@@ -102,6 +102,7 @@ ext {
     groupieVersion = '2.8.1'
     markwonVersion = '4.6.0'
     googleAutoServiceVersion = '1.0-rc7'
+    mockitoVersion = '3.6.0'
 }
 
 configurations {
@@ -235,7 +236,8 @@ dependencies {
     implementation "org.ocpsoft.prettytime:prettytime:4.0.6.Final"
 
     testImplementation 'junit:junit:4.13.1'
-    testImplementation 'org.mockito:mockito-core:3.6.0'
+    testImplementation "org.mockito:mockito-core:${mockitoVersion}"
+    testImplementation "org.mockito:mockito-inline:${mockitoVersion}"
 
     androidTestImplementation "androidx.test.ext:junit:1.1.2"
     androidTestImplementation "androidx.room:room-testing:${androidxRoomVersion}"

app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java

@@ -32,14 +32,9 @@ import org.schabi.newpipe.util.FilePickerActivityHelper;
 import org.schabi.newpipe.util.ZipHelper;
 
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.io.ObjectInputStream;
 import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.Locale;
-import java.util.Map;
-import java.util.zip.ZipFile;
 
 import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage;
 
@@ -49,13 +44,6 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
 
     private ContentSettingsManager manager;
 
-    private File databasesDir;
-    private File newpipeDb;
-    private File newpipeDbJournal;
-    private File newpipeDbShm;
-    private File newpipeDbWal;
-    private File newpipeSettings;
-
     private String thumbnailLoadToggleKey;
     private String youtubeRestrictedModeEnabledKey;
 
@@ -120,16 +108,8 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
     @Override
     public void onCreatePreferences(final Bundle savedInstanceState, final String rootKey) {
         final File homeDir = ContextCompat.getDataDir(requireContext());
-        databasesDir = new File(homeDir, "/databases");
+        manager = new ContentSettingsManager(new NewPipeFileLocator(homeDir));
-        newpipeDb = new File(homeDir, "/databases/newpipe.db");
+        manager.deleteSettingsFile();
-        newpipeDbJournal = new File(homeDir, "/databases/newpipe.db-journal");
-        newpipeDbShm = new File(homeDir, "/databases/newpipe.db-shm");
-        newpipeDbWal = new File(homeDir, "/databases/newpipe.db-wal");
-
-        newpipeSettings = new File(homeDir, "/databases/newpipe.settings");
-        newpipeSettings.delete();
-
-        manager = new ContentSettingsManager(homeDir);
 
         addPreferencesFromResource(R.xml.content_settings);
 
@@ -224,33 +204,24 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
 
     private void importDatabase(final String filePath) {
         // check if file is supported
-        try (ZipFile zipFile = new ZipFile(filePath)) {
+        if (!ZipHelper.isValidZipFile(filePath)) {
-        } catch (final IOException ioe) {
             Toast.makeText(getContext(), R.string.no_valid_zip_file, Toast.LENGTH_SHORT)
-                    .show();
+                .show();
             return;
         }
 
         try {
-            if (!databasesDir.exists() && !databasesDir.mkdir()) {
+            if (!manager.ensureDbDirectoryExists()) {
                 throw new Exception("Could not create databases dir");
             }
 
-            final boolean isDbFileExtracted = ZipHelper.extractFileFromZip(filePath,
+            if (!manager.extractDb(filePath)) {
-                    newpipeDb.getPath(), "newpipe.db");
-
-            if (isDbFileExtracted) {
-                newpipeDbJournal.delete();
-                newpipeDbWal.delete();
-                newpipeDbShm.delete();
-            } else {
                 Toast.makeText(getContext(), R.string.could_not_import_all_files, Toast.LENGTH_LONG)
-                        .show();
+                    .show();
             }
 
             //If settings file exist, ask if it should be imported.
-            if (ZipHelper.extractFileFromZip(filePath, newpipeSettings.getPath(),
+            if (manager.extractSettings(filePath)) {
-                    "newpipe.settings")) {
                 final AlertDialog.Builder alert = new AlertDialog.Builder(getContext());
                 alert.setTitle(R.string.import_settings);
 
@@ -261,7 +232,8 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
                 });
                 alert.setPositiveButton(getString(R.string.finish), (dialog, which) -> {
                     dialog.dismiss();
-                    loadSharedPreferences(newpipeSettings);
+                    manager.loadSharedPreferences(PreferenceManager
+                        .getDefaultSharedPreferences(requireContext()));
                     // restart app to properly load db
                     System.exit(0);
                 });
@@ -275,34 +247,6 @@ public class ContentSettingsFragment extends BasePreferenceFragment {
         }
     }
 
-    private void loadSharedPreferences(final File src) {
-        try (ObjectInputStream input = new ObjectInputStream(new FileInputStream(src))) {
-            final SharedPreferences.Editor prefEdit = PreferenceManager
-                    .getDefaultSharedPreferences(requireContext()).edit();
-            prefEdit.clear();
-            final Map<String, ?> entries = (Map<String, ?>) input.readObject();
-            for (final Map.Entry<String, ?> entry : entries.entrySet()) {
-                final Object v = entry.getValue();
-                final String key = entry.getKey();
-
-                if (v instanceof Boolean) {
-                    prefEdit.putBoolean(key, (Boolean) v);
-                } else if (v instanceof Float) {
-                    prefEdit.putFloat(key, (Float) v);
-                } else if (v instanceof Integer) {
-                    prefEdit.putInt(key, (Integer) v);
-                } else if (v instanceof Long) {
-                    prefEdit.putLong(key, (Long) v);
-                } else if (v instanceof String) {
-                    prefEdit.putString(key, (String) v);
-                }
-            }
-            prefEdit.commit();
-        } catch (final IOException | ClassNotFoundException e) {
-            e.printStackTrace();
-        }
-    }
-
     /*//////////////////////////////////////////////////////////////////////////
     // Error
     //////////////////////////////////////////////////////////////////////////*/

app/src/main/java/org/schabi/newpipe/settings/ContentSettingsManager.kt

@@ -3,22 +3,14 @@ package org.schabi.newpipe.settings
 import android.content.SharedPreferences
 import org.schabi.newpipe.util.ZipHelper
 import java.io.BufferedOutputStream
-import java.io.File
+import java.io.FileInputStream
 import java.io.FileOutputStream
 import java.io.IOException
+import java.io.ObjectInputStream
 import java.io.ObjectOutputStream
-import java.lang.Exception
 import java.util.zip.ZipOutputStream
 
-class ContentSettingsManager(
+class ContentSettingsManager(private val fileLocator: NewPipeFileLocator) {
-    private val newpipeDb: File,
-    private val newpipeSettings: File
-) {
-
-    constructor(homeDir: File) : this(
-        File(homeDir, "databases/newpipe.db"),
-        File(homeDir, "databases/newpipe.settings")
-    )
 
     /**
      * Exports given [SharedPreferences] to the file in given outputPath.
@@ -28,10 +20,10 @@ class ContentSettingsManager(
     fun exportDatabase(preferences: SharedPreferences, outputPath: String) {
         ZipOutputStream(BufferedOutputStream(FileOutputStream(outputPath)))
             .use { outZip ->
-                ZipHelper.addFileToZip(outZip, newpipeDb.path, "newpipe.db")
+                ZipHelper.addFileToZip(outZip, fileLocator.db.path, "newpipe.db")
 
                 try {
-                    ObjectOutputStream(FileOutputStream(newpipeSettings)).use { output ->
+                    ObjectOutputStream(FileOutputStream(fileLocator.settings)).use { output ->
                         output.writeObject(preferences.all)
                         output.flush()
                     }
@@ -39,7 +31,72 @@ class ContentSettingsManager(
                     e.printStackTrace()
                 }
 
-                ZipHelper.addFileToZip(outZip, newpipeSettings.path, "newpipe.settings")
+                ZipHelper.addFileToZip(outZip, fileLocator.settings.path, "newpipe.settings")
             }
     }
+
+    fun deleteSettingsFile() {
+        fileLocator.settings.delete()
+    }
+
+    /**
+     * Tries to create database directory if it does not exist.
+     *
+     * @return Whether the directory exists afterwards.
+     */
+    fun ensureDbDirectoryExists(): Boolean {
+        return fileLocator.dbDir.exists() || fileLocator.dbDir.mkdir()
+    }
+
+    fun extractDb(filePath: String): Boolean {
+        val success = ZipHelper.extractFileFromZip(filePath, fileLocator.db.path, "newpipe.db")
+        if (success) {
+            fileLocator.dbJournal.delete()
+            fileLocator.dbWal.delete()
+            fileLocator.dbShm.delete()
+        }
+
+        return success
+    }
+
+    fun extractSettings(filePath: String): Boolean {
+        return ZipHelper
+            .extractFileFromZip(filePath, fileLocator.settings.path, "newpipe.settings")
+    }
+
+    fun loadSharedPreferences(preferences: SharedPreferences) {
+        try {
+            val preferenceEditor = preferences.edit()
+
+            ObjectInputStream(FileInputStream(fileLocator.settings)).use { input ->
+                preferenceEditor.clear()
+                @Suppress("UNCHECKED_CAST")
+                val entries = input.readObject() as Map<String, *>
+                for ((key, value) in entries) {
+                    when (value) {
+                        is Boolean -> {
+                            preferenceEditor.putBoolean(key, value)
+                        }
+                        is Float -> {
+                            preferenceEditor.putFloat(key, value)
+                        }
+                        is Int -> {
+                            preferenceEditor.putInt(key, value)
+                        }
+                        is Long -> {
+                            preferenceEditor.putLong(key, value)
+                        }
+                        is String -> {
+                            preferenceEditor.putString(key, value)
+                        }
+                    }
+                }
+                preferenceEditor.commit()
+            }
+        } catch (e: IOException) {
+            e.printStackTrace()
+        } catch (e: ClassNotFoundException) {
+            e.printStackTrace()
+        }
+    }
 }

app/src/main/java/org/schabi/newpipe/settings/NewPipeFileLocator.kt

@@ -0,0 +1,21 @@
+package org.schabi.newpipe.settings
+
+import java.io.File
+
+/**
+ * Locates specific files of NewPipe based on the home directory of the app.
+ */
+class NewPipeFileLocator(private val homeDir: File) {
+
+    val dbDir by lazy { File(homeDir, "/databases") }
+
+    val db by lazy { File(homeDir, "/databases/newpipe.db") }
+
+    val dbJournal by lazy { File(homeDir, "/databases/newpipe.db-journal") }
+
+    val dbShm by lazy { File(homeDir, "/databases/newpipe.db-shm") }
+
+    val dbWal by lazy { File(homeDir, "/databases/newpipe.db-wal") }
+
+    val settings by lazy { File(homeDir, "/databases/newpipe.settings") }
+}

app/src/main/java/org/schabi/newpipe/util/ZipHelper.java

@@ -4,7 +4,9 @@ import java.io.BufferedInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
 import java.util.zip.ZipInputStream;
 import java.util.zip.ZipOutputStream;
 
@@ -99,4 +101,12 @@ public final class ZipHelper {
             return found;
         }
     }
+
+    public static boolean isValidZipFile(final String filePath) {
+        try (ZipFile ignored = new ZipFile(filePath)) {
+            return true;
+        } catch (final IOException ioe) {
+            return false;
+        }
+    }
 }

app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt

@@ -1,76 +1,184 @@
 package org.schabi.newpipe.settings
 
 import android.content.SharedPreferences
-import org.junit.Assert
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Assume
 import org.junit.Before
-import org.junit.BeforeClass
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.junit.runners.Suite
 import org.mockito.Mockito
 import org.mockito.Mockito.`when`
+import org.mockito.Mockito.anyBoolean
+import org.mockito.Mockito.anyInt
+import org.mockito.Mockito.anyString
+import org.mockito.Mockito.atLeastOnce
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.withSettings
 import org.mockito.junit.MockitoJUnitRunner
-import org.schabi.newpipe.settings.ContentSettingsManagerTest.ExportTest
 import java.io.File
 import java.io.ObjectInputStream
+import java.nio.file.Files
 import java.util.zip.ZipFile
 
-@RunWith(Suite::class)
+@RunWith(MockitoJUnitRunner::class)
-@Suite.SuiteClasses(ExportTest::class)
 class ContentSettingsManagerTest {
 
-    @RunWith(MockitoJUnitRunner::class)
+    companion object {
-    class ExportTest {
+        private val classloader = ContentSettingsManager::class.java.classLoader!!
+    }
 
-        companion object {
+    private lateinit var fileLocator: NewPipeFileLocator
-            private lateinit var newpipeDb: File
-            private lateinit var newpipeSettings: File
 
-            @JvmStatic
+    @Before
-            @BeforeClass
+    fun setupFileLocator() {
-            fun setupFiles() {
+        fileLocator = Mockito.mock(NewPipeFileLocator::class.java, withSettings().stubOnly())
-                val dbPath = ExportTest::class.java.classLoader?.getResource("settings/newpipe.db")?.file
+    }
-                val settingsPath = ExportTest::class.java.classLoader?.getResource("settings/newpipe.settings")?.path
-                Assume.assumeNotNull(dbPath)
-                Assume.assumeNotNull(settingsPath)
 
-                newpipeDb = File(dbPath!!)
+    @Test
-                newpipeSettings = File(settingsPath!!)
+    fun `The settings must be exported successfully in the correct format`() {
+        val db = File(classloader.getResource("settings/newpipe.db")!!.file)
+        val newpipeSettings = File.createTempFile("newpipe_", "")
+        `when`(fileLocator.db).thenReturn(db)
+        `when`(fileLocator.settings).thenReturn(newpipeSettings)
+
+        val expectedPreferences = mapOf("such pref" to "much wow")
+        val sharedPreferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly())
+        `when`(sharedPreferences.all).thenReturn(expectedPreferences)
+
+        val output = File.createTempFile("newpipe_", "")
+        ContentSettingsManager(fileLocator).exportDatabase(sharedPreferences, output.absolutePath)
+
+        val zipFile = ZipFile(output)
+        val entries = zipFile.entries().toList()
+        assertEquals(2, entries.size)
+
+        zipFile.getInputStream(entries.first { it.name == "newpipe.db" }).use { actual ->
+            db.inputStream().use { expected ->
+                assertEquals(expected.reader().readText(), actual.reader().readText())
             }
         }
 
-        private lateinit var preferences: SharedPreferences
+        zipFile.getInputStream(entries.first { it.name == "newpipe.settings" }).use { actual ->
-
+            val actualPreferences = ObjectInputStream(actual).readObject()
-        @Before
+            assertEquals(expectedPreferences, actualPreferences)
-        fun setupMocks() {
-            preferences = Mockito.mock(SharedPreferences::class.java, Mockito.withSettings().stubOnly())
-        }
-
-        @Test
-        fun `The settings must be exported successfully in the correct format`() {
-            val expectedPreferences = mapOf("such pref" to "much wow")
-            `when`(preferences.all).thenReturn(expectedPreferences)
-
-            val manager = ContentSettingsManager(newpipeDb, newpipeSettings)
-
-            val output = File.createTempFile("newpipe_", "")
-            manager.exportDatabase(preferences, output.absolutePath)
-
-            val zipFile = ZipFile(output.absoluteFile)
-            val entries = zipFile.entries().toList()
-            Assert.assertEquals(2, entries.size)
-
-            zipFile.getInputStream(entries.first { it.name == "newpipe.db" }).use { actual ->
-                newpipeDb.inputStream().use { expected ->
-                    Assert.assertEquals(expected.reader().readText(), actual.reader().readText())
-                }
-            }
-
-            zipFile.getInputStream(entries.first { it.name == "newpipe.settings" }).use { actual ->
-                val actualPreferences = ObjectInputStream(actual).readObject()
-                Assert.assertEquals(expectedPreferences, actualPreferences)
-            }
         }
     }
+
+    @Test
+    fun `Settings file must be deleted`() {
+        val settings = File.createTempFile("newpipe_", "")
+        `when`(fileLocator.settings).thenReturn(settings)
+
+        ContentSettingsManager(fileLocator).deleteSettingsFile()
+
+        assertFalse(settings.exists())
+    }
+
+    @Test
+    fun `Deleting settings file must do nothing if none exist`() {
+        val settings = File("non_existent")
+        `when`(fileLocator.settings).thenReturn(settings)
+
+        ContentSettingsManager(fileLocator).deleteSettingsFile()
+
+        assertFalse(settings.exists())
+    }
+
+    @Test
+    fun `Ensuring db directory existence must work`() {
+        val dir = Files.createTempDirectory("newpipe_").toFile()
+        Assume.assumeTrue(dir.delete())
+        `when`(fileLocator.dbDir).thenReturn(dir)
+
+        ContentSettingsManager(fileLocator).ensureDbDirectoryExists()
+        assertTrue(dir.exists())
+    }
+
+    @Test
+    fun `Ensuring db directory existence must work when the directory already exists`() {
+        val dir = Files.createTempDirectory("newpipe_").toFile()
+        `when`(fileLocator.dbDir).thenReturn(dir)
+
+        ContentSettingsManager(fileLocator).ensureDbDirectoryExists()
+        assertTrue(dir.exists())
+    }
+
+    @Test
+    fun `The database must be extracted from the zip file`() {
+        val db = File.createTempFile("newpipe_", "")
+        val dbJournal = File.createTempFile("newpipe_", "")
+        val dbWal = File.createTempFile("newpipe_", "")
+        val dbShm = File.createTempFile("newpipe_", "")
+        `when`(fileLocator.db).thenReturn(db)
+        `when`(fileLocator.dbJournal).thenReturn(dbJournal)
+        `when`(fileLocator.dbShm).thenReturn(dbShm)
+        `when`(fileLocator.dbWal).thenReturn(dbWal)
+
+        val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!)
+        val success = ContentSettingsManager(fileLocator).extractDb(zip.path)
+
+        assertTrue(success)
+        assertFalse(dbJournal.exists())
+        assertFalse(dbWal.exists())
+        assertFalse(dbShm.exists())
+        assertTrue("database file size is zero", Files.size(db.toPath()) > 0)
+    }
+
+    @Test
+    fun `Extracting the database from an empty zip must not work`() {
+        val db = File.createTempFile("newpipe_", "")
+        val dbJournal = File.createTempFile("newpipe_", "")
+        val dbWal = File.createTempFile("newpipe_", "")
+        val dbShm = File.createTempFile("newpipe_", "")
+        `when`(fileLocator.db).thenReturn(db)
+
+        val emptyZip = File(classloader.getResource("settings/empty.zip")?.file!!)
+        val success = ContentSettingsManager(fileLocator).extractDb(emptyZip.path)
+
+        assertFalse(success)
+        assertTrue(dbJournal.exists())
+        assertTrue(dbWal.exists())
+        assertTrue(dbShm.exists())
+        assertEquals(0, Files.size(db.toPath()))
+    }
+
+    @Test
+    fun `Contains setting must return true if a settings file exists in the zip`() {
+        val settings = File.createTempFile("newpipe_", "")
+        `when`(fileLocator.settings).thenReturn(settings)
+
+        val zip = File(classloader.getResource("settings/newpipe.zip")?.file!!)
+        val contains = ContentSettingsManager(fileLocator).extractSettings(zip.path)
+
+        assertTrue(contains)
+    }
+
+    @Test
+    fun `Contains setting must return false if a no settings file exists in the zip`() {
+        val settings = File.createTempFile("newpipe_", "")
+        `when`(fileLocator.settings).thenReturn(settings)
+
+        val emptyZip = File(classloader.getResource("settings/empty.zip")?.file!!)
+        val contains = ContentSettingsManager(fileLocator).extractSettings(emptyZip.path)
+
+        assertFalse(contains)
+    }
+
+    @Test
+    fun `Preferences must be set from the settings file`() {
+        val settings = File(classloader.getResource("settings/newpipe.settings")!!.path)
+        `when`(fileLocator.settings).thenReturn(settings)
+
+        val preferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly())
+        val editor = Mockito.mock(SharedPreferences.Editor::class.java)
+        `when`(preferences.edit()).thenReturn(editor)
+
+        ContentSettingsManager(fileLocator).loadSharedPreferences(preferences)
+
+        verify(editor, atLeastOnce()).putBoolean(anyString(), anyBoolean())
+        verify(editor, atLeastOnce()).putString(anyString(), anyString())
+        verify(editor, atLeastOnce()).putInt(anyString(), anyInt())
+    }
 }