From 82f43ac6a64c1754e023aed6c5cd7a117f1a99dc Mon Sep 17 00:00:00 2001 From: Alireza Tofighi Date: Mon, 17 May 2021 21:57:27 +0430 Subject: [PATCH 1/6] Save backup import/export location for feature import/exports --- .../settings/ContentSettingsFragment.java | 39 ++++++++++++++++++- app/src/main/res/values/settings_keys.xml | 1 + 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java index 3fd44c4d5..e4850d7fa 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java @@ -41,6 +41,8 @@ public class ContentSettingsFragment extends BasePreferenceFragment { private ContentSettingsManager manager; + private String importExportDataPathKey; + private String thumbnailLoadToggleKey; private String youtubeRestrictedModeEnabledKey; @@ -56,6 +58,7 @@ public class ContentSettingsFragment extends BasePreferenceFragment { addPreferencesFromResource(R.xml.content_settings); + importExportDataPathKey = getString(R.string.import_export_data_path); final Preference importDataPreference = findPreference(getString(R.string.import_data)); importDataPreference.setOnPreferenceClickListener(p -> { final Intent i = new Intent(getActivity(), FilePickerActivityHelper.class) @@ -63,6 +66,10 @@ public class ContentSettingsFragment extends BasePreferenceFragment { .putExtra(FilePickerActivityHelper.EXTRA_ALLOW_CREATE_DIR, false) .putExtra(FilePickerActivityHelper.EXTRA_MODE, FilePickerActivityHelper.MODE_FILE); + final String path = defaultPreferences.getString(importExportDataPathKey, ""); + if (isValidPath(path)) { + i.putExtra(FilePickerActivityHelper.EXTRA_START_PATH, path); + } startActivityForResult(i, REQUEST_IMPORT_PATH); return true; }); @@ -74,6 +81,10 @@ public class ContentSettingsFragment extends BasePreferenceFragment { .putExtra(FilePickerActivityHelper.EXTRA_ALLOW_CREATE_DIR, true) .putExtra(FilePickerActivityHelper.EXTRA_MODE, FilePickerActivityHelper.MODE_DIR); + final String path = defaultPreferences.getString(importExportDataPathKey, ""); + if (isValidPath(path)) { + i.putExtra(FilePickerActivityHelper.EXTRA_START_PATH, path); + } startActivityForResult(i, REQUEST_EXPORT_PATH); return true; }); @@ -164,7 +175,10 @@ public class ContentSettingsFragment extends BasePreferenceFragment { if ((requestCode == REQUEST_IMPORT_PATH || requestCode == REQUEST_EXPORT_PATH) && resultCode == Activity.RESULT_OK && data.getData() != null) { - final String path = Utils.getFileForUri(data.getData()).getAbsolutePath(); + final File file = Utils.getFileForUri(data.getData()); + final String path = file.getAbsolutePath(); + setImportExportDataPath(file); + if (requestCode == REQUEST_EXPORT_PATH) { final SimpleDateFormat sdf = new SimpleDateFormat("yyyyMMdd_HHmmss", Locale.US); exportDatabase(path + "/NewPipeData-" + sdf.format(new Date()) + ".zip"); @@ -239,4 +253,27 @@ public class ContentSettingsFragment extends BasePreferenceFragment { ErrorActivity.reportUiErrorInSnackbar(this, "Importing database", e); } } + + private boolean isValidPath(final String path) { + if (path == null || path.isEmpty()) { + return false; + } + final File file = new File(path); + return file.exists() && file.isDirectory(); + } + + private void setImportExportDataPath(final File file) { + final String directoryPath; + if (!file.isDirectory()) { + final File parentFile = file.getParentFile(); + if (parentFile != null) { + directoryPath = parentFile.getAbsolutePath(); + } else { + directoryPath = ""; + } + } else { + directoryPath = file.getAbsolutePath(); + } + defaultPreferences.edit().putString(importExportDataPathKey, directoryPath).apply(); + } } diff --git a/app/src/main/res/values/settings_keys.xml b/app/src/main/res/values/settings_keys.xml index fd6cc7251..c23e81fbe 100644 --- a/app/src/main/res/values/settings_keys.xml +++ b/app/src/main/res/values/settings_keys.xml @@ -265,6 +265,7 @@ feed_use_dedicated_fetch_method + import_export_data_path import_data export_data From fa2b11b7685d989d1e28e62f65ff1f80d890a9d1 Mon Sep 17 00:00:00 2001 From: Alireza Tofighi Date: Fri, 21 May 2021 20:21:58 +0430 Subject: [PATCH 2/6] Move ContentSettingsFragment.isValidPath to helpers and add unit test for it. --- .../settings/ContentSettingsFragment.java | 13 ++------ .../schabi/newpipe/util/FilePathUtils.java | 22 +++++++++++++ .../newpipe/util/FilePathHelperTest.java | 32 +++++++++++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/util/FilePathUtils.java create mode 100644 app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java diff --git a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java index e4850d7fa..3464f1081 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java @@ -26,6 +26,7 @@ import org.schabi.newpipe.extractor.NewPipe; import org.schabi.newpipe.extractor.localization.ContentCountry; import org.schabi.newpipe.extractor.localization.Localization; import org.schabi.newpipe.util.FilePickerActivityHelper; +import org.schabi.newpipe.util.FilePathUtils; import org.schabi.newpipe.util.ZipHelper; import java.io.File; @@ -67,7 +68,7 @@ public class ContentSettingsFragment extends BasePreferenceFragment { .putExtra(FilePickerActivityHelper.EXTRA_MODE, FilePickerActivityHelper.MODE_FILE); final String path = defaultPreferences.getString(importExportDataPathKey, ""); - if (isValidPath(path)) { + if (FilePathUtils.isValidDirectoryPath(path)) { i.putExtra(FilePickerActivityHelper.EXTRA_START_PATH, path); } startActivityForResult(i, REQUEST_IMPORT_PATH); @@ -82,7 +83,7 @@ public class ContentSettingsFragment extends BasePreferenceFragment { .putExtra(FilePickerActivityHelper.EXTRA_MODE, FilePickerActivityHelper.MODE_DIR); final String path = defaultPreferences.getString(importExportDataPathKey, ""); - if (isValidPath(path)) { + if (FilePathUtils.isValidDirectoryPath(path)) { i.putExtra(FilePickerActivityHelper.EXTRA_START_PATH, path); } startActivityForResult(i, REQUEST_EXPORT_PATH); @@ -254,14 +255,6 @@ public class ContentSettingsFragment extends BasePreferenceFragment { } } - private boolean isValidPath(final String path) { - if (path == null || path.isEmpty()) { - return false; - } - final File file = new File(path); - return file.exists() && file.isDirectory(); - } - private void setImportExportDataPath(final File file) { final String directoryPath; if (!file.isDirectory()) { diff --git a/app/src/main/java/org/schabi/newpipe/util/FilePathUtils.java b/app/src/main/java/org/schabi/newpipe/util/FilePathUtils.java new file mode 100644 index 000000000..4162e563a --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/util/FilePathUtils.java @@ -0,0 +1,22 @@ +package org.schabi.newpipe.util; + +import java.io.File; + +public final class FilePathUtils { + private FilePathUtils() { } + + + /** + * Check that the path is a valid directory path and it exists. + * + * @param path full path of directory, + * @return is path valid or not + */ + public static boolean isValidDirectoryPath(final String path) { + if (path == null || path.isEmpty()) { + return false; + } + final File file = new File(path); + return file.exists() && file.isDirectory(); + } +} diff --git a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java new file mode 100644 index 000000000..7cdc5f855 --- /dev/null +++ b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java @@ -0,0 +1,32 @@ +package org.schabi.newpipe.util; + +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class FilePathHelperTest { + @Test + public void testIsValidDirectoryPath() throws IOException { + // path that exists + final File dir1 = Files.createTempDirectory("dir1").toFile(); + assertTrue(FilePathUtils.isValidDirectoryPath(dir1.getAbsolutePath())); + + // a directory in above path that exists + final File subDir = Files.createDirectory(dir1.toPath().resolve("subdir")).toFile(); + assertTrue(FilePathUtils.isValidDirectoryPath(subDir.getAbsolutePath())); + + // a directory in above path that doesn't exist + assertFalse(FilePathUtils.isValidDirectoryPath(dir1.toPath().resolve("not-exists-subdir"). + toFile().getAbsolutePath())); + + // file is not a valid direcotry path + final File tempFile = Files.createFile(dir1.toPath().resolve("simple_file")).toFile(); + assertFalse(FilePathUtils.isValidDirectoryPath(tempFile.getAbsolutePath())); + } + +} From 92ab9cae27620337d21e4f634b918a919aa5d830 Mon Sep 17 00:00:00 2001 From: Alireza Tofighi Date: Fri, 21 May 2021 20:24:57 +0430 Subject: [PATCH 3/6] Invert if condition in ContentSettingsFragment.setImportExportDataPath for better readability --- .../schabi/newpipe/settings/ContentSettingsFragment.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java index 3464f1081..ab6ff7414 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/ContentSettingsFragment.java @@ -257,15 +257,15 @@ public class ContentSettingsFragment extends BasePreferenceFragment { private void setImportExportDataPath(final File file) { final String directoryPath; - if (!file.isDirectory()) { + if (file.isDirectory()) { + directoryPath = file.getAbsolutePath(); + } else { final File parentFile = file.getParentFile(); if (parentFile != null) { directoryPath = parentFile.getAbsolutePath(); } else { directoryPath = ""; } - } else { - directoryPath = file.getAbsolutePath(); } defaultPreferences.edit().putString(importExportDataPathKey, directoryPath).apply(); } From 067528211f0274922a92a3940155f67e71c66776 Mon Sep 17 00:00:00 2001 From: Alireza Tofighi Date: Fri, 21 May 2021 20:28:42 +0430 Subject: [PATCH 4/6] Add more tests for FilePathUtils.isValidDirectoryPath for better coverage --- .../java/org/schabi/newpipe/util/FilePathHelperTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java index 7cdc5f855..b81a06e30 100644 --- a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java +++ b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java @@ -12,6 +12,12 @@ import static org.junit.Assert.assertTrue; public class FilePathHelperTest { @Test public void testIsValidDirectoryPath() throws IOException { + // empty path is not valid + assertFalse(FilePathUtils.isValidDirectoryPath("")); + + // null path is not valid + assertFalse(FilePathUtils.isValidDirectoryPath(null)); + // path that exists final File dir1 = Files.createTempDirectory("dir1").toFile(); assertTrue(FilePathUtils.isValidDirectoryPath(dir1.getAbsolutePath())); From e8ad947d37dbbd6b0ba0de0f9df40e90d2321b8e Mon Sep 17 00:00:00 2001 From: Alireza Tofighi Date: Fri, 21 May 2021 22:44:38 +0430 Subject: [PATCH 5/6] Split up FilePathHelperTest tests in simpler methods --- .../newpipe/util/FilePathHelperTest.java | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java index b81a06e30..1e6761603 100644 --- a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java +++ b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java @@ -1,5 +1,6 @@ package org.schabi.newpipe.util; +import org.junit.Before; import org.junit.Test; import java.io.File; @@ -10,28 +11,44 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class FilePathHelperTest { + + private File dir; + + @Before + public void setUp() throws IOException { + dir = Files.createTempDirectory("dir1").toFile(); + } + @Test - public void testIsValidDirectoryPath() throws IOException { - // empty path is not valid + public void testIsValidDirectoryPathWithEmptyString() { assertFalse(FilePathUtils.isValidDirectoryPath("")); + } - // null path is not valid + @Test + public void testIsValidDirectoryPathWithNullString() { assertFalse(FilePathUtils.isValidDirectoryPath(null)); + } - // path that exists - final File dir1 = Files.createTempDirectory("dir1").toFile(); - assertTrue(FilePathUtils.isValidDirectoryPath(dir1.getAbsolutePath())); + @Test + public void testIsValidDirectoryPathWithValidPath() { + assertTrue(FilePathUtils.isValidDirectoryPath(dir.getAbsolutePath())); + } - // a directory in above path that exists - final File subDir = Files.createDirectory(dir1.toPath().resolve("subdir")).toFile(); + @Test + public void testIsValidDirectoryPathWithDeepValidDirectory() throws IOException { + final File subDir = Files.createDirectory(dir.toPath().resolve("subdir")).toFile(); assertTrue(FilePathUtils.isValidDirectoryPath(subDir.getAbsolutePath())); + } - // a directory in above path that doesn't exist - assertFalse(FilePathUtils.isValidDirectoryPath(dir1.toPath().resolve("not-exists-subdir"). + @Test + public void testIsValidDirectoryPathWithNotExistDirectory() { + assertFalse(FilePathUtils.isValidDirectoryPath(dir.toPath().resolve("not-exists-subdir"). toFile().getAbsolutePath())); + } - // file is not a valid direcotry path - final File tempFile = Files.createFile(dir1.toPath().resolve("simple_file")).toFile(); + @Test + public void testIsValidDirectoryPathWithFile() throws IOException { + final File tempFile = Files.createFile(dir.toPath().resolve("simple_file")).toFile(); assertFalse(FilePathUtils.isValidDirectoryPath(tempFile.getAbsolutePath())); } From 376e5c1546641a10c654475d303b8d298461430a Mon Sep 17 00:00:00 2001 From: XiangRongLin <41164160+XiangRongLin@users.noreply.github.com> Date: Fri, 21 May 2021 20:24:11 +0200 Subject: [PATCH 6/6] Remove unnecessary conversion between file and path --- .../schabi/newpipe/util/FilePathHelperTest.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java index 1e6761603..3c9f12720 100644 --- a/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java +++ b/app/src/test/java/org/schabi/newpipe/util/FilePathHelperTest.java @@ -6,17 +6,18 @@ import org.junit.Test; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class FilePathHelperTest { - private File dir; + private Path dir; @Before public void setUp() throws IOException { - dir = Files.createTempDirectory("dir1").toFile(); + dir = Files.createTempDirectory("dir1"); } @Test @@ -31,24 +32,24 @@ public class FilePathHelperTest { @Test public void testIsValidDirectoryPathWithValidPath() { - assertTrue(FilePathUtils.isValidDirectoryPath(dir.getAbsolutePath())); + assertTrue(FilePathUtils.isValidDirectoryPath(dir.toAbsolutePath().toString())); } @Test public void testIsValidDirectoryPathWithDeepValidDirectory() throws IOException { - final File subDir = Files.createDirectory(dir.toPath().resolve("subdir")).toFile(); + final File subDir = Files.createDirectory(dir.resolve("subdir")).toFile(); assertTrue(FilePathUtils.isValidDirectoryPath(subDir.getAbsolutePath())); } @Test public void testIsValidDirectoryPathWithNotExistDirectory() { - assertFalse(FilePathUtils.isValidDirectoryPath(dir.toPath().resolve("not-exists-subdir"). - toFile().getAbsolutePath())); + assertFalse(FilePathUtils.isValidDirectoryPath(dir.resolve("not-exists-subdir"). + toFile().getAbsolutePath())); } @Test public void testIsValidDirectoryPathWithFile() throws IOException { - final File tempFile = Files.createFile(dir.toPath().resolve("simple_file")).toFile(); + final File tempFile = Files.createFile(dir.resolve("simple_file")).toFile(); assertFalse(FilePathUtils.isValidDirectoryPath(tempFile.getAbsolutePath())); }