diff --git a/app/build.gradle b/app/build.gradle index 87df0ce89..debf072a6 100644 --- a/app/build.gradle +++ b/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}" diff --git a/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsFragment.java b/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsFragment.java index 43c6aad25..c788d1748 100644 --- a/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsFragment.java @@ -32,14 +32,9 @@ import org.schabi.newpipelegacy.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.newpipelegacy.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 boolean onPreferenceTreeClick(final Preference preference) { @Override public void onCreatePreferences(final Bundle savedInstanceState, final String rootKey) { final File homeDir = ContextCompat.getDataDir(requireContext()); - databasesDir = new File(homeDir, "/databases"); - newpipeDb = new File(homeDir, "/databases/newpipe.db"); - 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); + manager = new ContentSettingsManager(new NewPipeFileLocator(homeDir)); + manager.deleteSettingsFile(); addPreferencesFromResource(R.xml.content_settings); @@ -224,33 +204,24 @@ private void exportDatabase(final String path) { private void importDatabase(final String filePath) { // check if file is supported - try (ZipFile zipFile = new ZipFile(filePath)) { - } catch (final IOException ioe) { + if (!ZipHelper.isValidZipFile(filePath)) { 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, - newpipeDb.getPath(), "newpipe.db"); - - if (isDbFileExtracted) { - newpipeDbJournal.delete(); - newpipeDbWal.delete(); - newpipeDbShm.delete(); - } else { + if (!manager.extractDb(filePath)) { 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(), - "newpipe.settings")) { + if (manager.extractSettings(filePath)) { final AlertDialog.Builder alert = new AlertDialog.Builder(getContext()); alert.setTitle(R.string.import_settings); @@ -261,7 +232,8 @@ private void importDatabase(final String filePath) { }); 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 @@ private void importDatabase(final String filePath) { } } - 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 entries = (Map) input.readObject(); - for (final Map.Entry 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 //////////////////////////////////////////////////////////////////////////*/ diff --git a/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsManager.kt b/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsManager.kt index 2b947bd2d..cfe473cfa 100644 --- a/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsManager.kt +++ b/app/src/main/java/org/schabi/newpipelegacy/settings/ContentSettingsManager.kt @@ -3,22 +3,14 @@ package org.schabi.newpipelegacy.settings import android.content.SharedPreferences import org.schabi.newpipelegacy.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( - private val newpipeDb: File, - private val newpipeSettings: File -) { - - constructor(homeDir: File) : this( - File(homeDir, "databases/newpipe.db"), - File(homeDir, "databases/newpipe.settings") - ) +class ContentSettingsManager(private val fileLocator: NewPipeFileLocator) { /** * 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 + 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() + } } } diff --git a/app/src/main/java/org/schabi/newpipelegacy/settings/NewPipeFileLocator.kt b/app/src/main/java/org/schabi/newpipelegacy/settings/NewPipeFileLocator.kt new file mode 100644 index 000000000..339bcc52e --- /dev/null +++ b/app/src/main/java/org/schabi/newpipelegacy/settings/NewPipeFileLocator.kt @@ -0,0 +1,21 @@ +package org.schabi.newpipelegacy.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") } +} diff --git a/app/src/main/java/org/schabi/newpipelegacy/util/ZipHelper.java b/app/src/main/java/org/schabi/newpipelegacy/util/ZipHelper.java index d820ebd1c..15ed80c25 100644 --- a/app/src/main/java/org/schabi/newpipelegacy/util/ZipHelper.java +++ b/app/src/main/java/org/schabi/newpipelegacy/util/ZipHelper.java @@ -4,7 +4,9 @@ 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 static boolean extractFileFromZip(final String filePath, final String fil return found; } } + + public static boolean isValidZipFile(final String filePath) { + try (ZipFile ignored = new ZipFile(filePath)) { + return true; + } catch (final IOException ioe) { + return false; + } + } } diff --git a/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt b/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt index b577cbd70..a3d4e5163 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt +++ b/app/src/test/java/org/schabi/newpipe/settings/ContentSettingsManagerTest.kt @@ -1,76 +1,184 @@ package org.schabi.newpipelegacy.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.newpipelegacy.settings.ContentSettingsManagerTest.ExportTest import java.io.File import java.io.ObjectInputStream +import java.nio.file.Files import java.util.zip.ZipFile -@RunWith(Suite::class) -@Suite.SuiteClasses(ExportTest::class) +@RunWith(MockitoJUnitRunner::class) class ContentSettingsManagerTest { - @RunWith(MockitoJUnitRunner::class) - class ExportTest { + companion object { + private val classloader = ContentSettingsManager::class.java.classLoader!! + } + + private lateinit var fileLocator: NewPipeFileLocator + + @Before + fun setupFileLocator() { + fileLocator = Mockito.mock(NewPipeFileLocator::class.java, withSettings().stubOnly()) + } + + @Test + 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) - companion object { - private lateinit var newpipeDb: File - private lateinit var newpipeSettings: File + val expectedPreferences = mapOf("such pref" to "much wow") + val sharedPreferences = Mockito.mock(SharedPreferences::class.java, withSettings().stubOnly()) + `when`(sharedPreferences.all).thenReturn(expectedPreferences) - @JvmStatic - @BeforeClass - fun setupFiles() { - 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) + val output = File.createTempFile("newpipe_", "") + ContentSettingsManager(fileLocator).exportDatabase(sharedPreferences, output.absolutePath) - newpipeDb = File(dbPath!!) - newpipeSettings = File(settingsPath!!) + 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 - - @Before - fun setupMocks() { - preferences = Mockito.mock(SharedPreferences::class.java, Mockito.withSettings().stubOnly()) + zipFile.getInputStream(entries.first { it.name == "newpipe.settings" }).use { actual -> + val actualPreferences = ObjectInputStream(actual).readObject() + assertEquals(expectedPreferences, actualPreferences) } + } - @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) + @Test + fun `Settings file must be deleted`() { + val settings = File.createTempFile("newpipe_", "") + `when`(fileLocator.settings).thenReturn(settings) - val manager = ContentSettingsManager(newpipeDb, newpipeSettings) + ContentSettingsManager(fileLocator).deleteSettingsFile() - val output = File.createTempFile("newpipe_", "") - manager.exportDatabase(preferences, output.absolutePath) + assertFalse(settings.exists()) + } - val zipFile = ZipFile(output.absoluteFile) - val entries = zipFile.entries().toList() - Assert.assertEquals(2, entries.size) + @Test + fun `Deleting settings file must do nothing if none exist`() { + val settings = File("non_existent") + `when`(fileLocator.settings).thenReturn(settings) - zipFile.getInputStream(entries.first { it.name == "newpipe.db" }).use { actual -> - newpipeDb.inputStream().use { expected -> - Assert.assertEquals(expected.reader().readText(), actual.reader().readText()) - } - } + ContentSettingsManager(fileLocator).deleteSettingsFile() - zipFile.getInputStream(entries.first { it.name == "newpipe.settings" }).use { actual -> - val actualPreferences = ObjectInputStream(actual).readObject() - Assert.assertEquals(expectedPreferences, actualPreferences) - } - } + 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()) } } diff --git a/app/src/test/resources/settings/empty.zip b/app/src/test/resources/settings/empty.zip new file mode 100644 index 000000000..15cb0ecb3 Binary files /dev/null and b/app/src/test/resources/settings/empty.zip differ diff --git a/app/src/test/resources/settings/newpipe.settings b/app/src/test/resources/settings/newpipe.settings index e69de29bb..56e6c5d5d 100644 Binary files a/app/src/test/resources/settings/newpipe.settings and b/app/src/test/resources/settings/newpipe.settings differ diff --git a/app/src/test/resources/settings/newpipe.zip b/app/src/test/resources/settings/newpipe.zip new file mode 100644 index 000000000..1ce8431fe Binary files /dev/null and b/app/src/test/resources/settings/newpipe.zip differ