From 72b67ab5d4992f9618fb81569290880e4d6f04fd Mon Sep 17 00:00:00 2001 From: Isira Seneviratne Date: Mon, 7 Jul 2025 07:52:54 +0530 Subject: [PATCH 1/4] Refactor zip import/export using Path --- .../BackupRestoreSettingsFragment.java | 12 ++---- .../settings/export/BackupFileLocator.kt | 19 ++++----- .../settings/export/ImportExportManager.kt | 29 ++++++------- .../org/schabi/newpipe/util/ZipHelper.java | 41 ++++++------------- .../settings/ImportAllCombinationsTest.kt | 16 ++++---- .../settings/ImportExportManagerTest.kt | 36 +++++++++------- 6 files changed, 67 insertions(+), 86 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java index 97df1549b..cc93e9227 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java @@ -35,7 +35,6 @@ import org.schabi.newpipe.streams.io.StoredFileHelper; import org.schabi.newpipe.util.NavigationHelper; import org.schabi.newpipe.util.ZipHelper; -import java.io.File; import java.io.IOException; import java.text.SimpleDateFormat; import java.util.Date; @@ -61,13 +60,12 @@ public class BackupRestoreSettingsFragment extends BasePreferenceFragment { @Override public void onCreatePreferences(@Nullable final Bundle savedInstanceState, @Nullable final String rootKey) { - final File homeDir = ContextCompat.getDataDir(requireContext()); - Objects.requireNonNull(homeDir); - manager = new ImportExportManager(new BackupFileLocator(homeDir)); + final var dbDir = Objects.requireNonNull(ContextCompat.getDataDir(requireContext())) + .toPath(); + manager = new ImportExportManager(new BackupFileLocator(dbDir)); importExportDataPathKey = getString(R.string.import_export_data_path); - addPreferencesFromResourceRegistry(); final Preference importDataPreference = requirePreference(R.string.import_data); @@ -183,9 +181,7 @@ public class BackupRestoreSettingsFragment extends BasePreferenceFragment { } try { - if (!manager.ensureDbDirectoryExists()) { - throw new IOException("Could not create databases dir"); - } + manager.ensureDbDirectoryExists(); // replace the current database if (!manager.extractDb(file)) { diff --git a/app/src/main/java/org/schabi/newpipe/settings/export/BackupFileLocator.kt b/app/src/main/java/org/schabi/newpipe/settings/export/BackupFileLocator.kt index c864e4a0d..0ce2d5f4d 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/export/BackupFileLocator.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/export/BackupFileLocator.kt @@ -1,11 +1,12 @@ package org.schabi.newpipe.settings.export -import java.io.File +import java.nio.file.Path +import kotlin.io.path.div /** * Locates specific files of NewPipe based on the home directory of the app. */ -class BackupFileLocator(private val homeDir: File) { +class BackupFileLocator(homeDir: Path) { companion object { const val FILE_NAME_DB = "newpipe.db" @Deprecated( @@ -16,13 +17,9 @@ class BackupFileLocator(private val homeDir: File) { const val FILE_NAME_JSON_PREFS = "preferences.json" } - val dbDir by lazy { File(homeDir, "/databases") } - - val db by lazy { File(dbDir, FILE_NAME_DB) } - - val dbJournal by lazy { File(dbDir, "$FILE_NAME_DB-journal") } - - val dbShm by lazy { File(dbDir, "$FILE_NAME_DB-shm") } - - val dbWal by lazy { File(dbDir, "$FILE_NAME_DB-wal") } + val dbDir = homeDir / "databases" + val db = homeDir / FILE_NAME_DB + val dbJournal = homeDir / "$FILE_NAME_DB-journal" + val dbShm = dbDir / "$FILE_NAME_DB-shm" + val dbWal = dbDir / "$FILE_NAME_DB-wal" } diff --git a/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt b/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt index 36e0b9ce1..04562bc77 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt @@ -12,6 +12,8 @@ import java.io.FileNotFoundException import java.io.IOException import java.io.ObjectOutputStream import java.util.zip.ZipOutputStream +import kotlin.io.path.createDirectories +import kotlin.io.path.deleteExisting class ImportExportManager(private val fileLocator: BackupFileLocator) { companion object { @@ -28,11 +30,8 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { // previous file size, the file will retain part of the previous content and be corrupted ZipOutputStream(SharpOutputStream(file.openAndTruncateStream()).buffered()).use { outZip -> // add the database - ZipHelper.addFileToZip( - outZip, - BackupFileLocator.FILE_NAME_DB, - fileLocator.db.path, - ) + val name = BackupFileLocator.FILE_NAME_DB + ZipHelper.addFileToZip(outZip, name, fileLocator.db) // add the legacy vulnerable serialized preferences (will be removed in the future) ZipHelper.addFileToZip( @@ -61,11 +60,10 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { /** * 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() + @Throws(IOException::class) + fun ensureDbDirectoryExists() { + fileLocator.dbDir.createDirectories() } /** @@ -75,16 +73,13 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { * @return true if the database was successfully extracted, false otherwise */ fun extractDb(file: StoredFileHelper): Boolean { - val success = ZipHelper.extractFileFromZip( - file, - BackupFileLocator.FILE_NAME_DB, - fileLocator.db.path, - ) + val name = BackupFileLocator.FILE_NAME_DB + val success = ZipHelper.extractFileFromZip(file, name, fileLocator.db) if (success) { - fileLocator.dbJournal.delete() - fileLocator.dbWal.delete() - fileLocator.dbShm.delete() + fileLocator.dbJournal.deleteExisting() + fileLocator.dbWal.deleteExisting() + fileLocator.dbShm.deleteExisting() } return success diff --git a/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java b/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java index b2aebac42..771452c89 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java @@ -6,12 +6,12 @@ import org.schabi.newpipe.streams.io.StoredFileHelper; import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; import java.util.zip.ZipOutputStream; @@ -55,17 +55,17 @@ public final class ZipHelper { /** - * This function helps to create zip files. Caution this will overwrite the original file. + * This function helps to create zip files. Caution, this will overwrite the original file. * * @param outZip the ZipOutputStream where the data should be stored in * @param nameInZip the path of the file inside the zip - * @param fileOnDisk the path of the file on the disk that should be added to zip + * @param path the path of the file on the disk that should be added to zip */ public static void addFileToZip(final ZipOutputStream outZip, final String nameInZip, - final String fileOnDisk) throws IOException { - try (FileInputStream fi = new FileInputStream(fileOnDisk)) { - addFileToZip(outZip, nameInZip, fi); + final Path path) throws IOException { + try (var inputStream = Files.newInputStream(path)) { + addFileToZip(outZip, nameInZip, inputStream); } } @@ -113,33 +113,18 @@ public final class ZipHelper { } /** - * This will extract data from ZipInputStream. Caution this will overwrite the original file. + * This will extract data from ZipInputStream. Caution, this will overwrite the original file. * * @param zipFile the zip file to extract from * @param nameInZip the path of the file inside the zip - * @param fileOnDisk the path of the file on the disk where the data should be extracted to + * @param path the path of the file on the disk where the data should be extracted to * @return will return true if the file was found within the zip file */ public static boolean extractFileFromZip(final StoredFileHelper zipFile, final String nameInZip, - final String fileOnDisk) throws IOException { - return extractFileFromZip(zipFile, nameInZip, input -> { - // delete old file first - final File oldFile = new File(fileOnDisk); - if (oldFile.exists()) { - if (!oldFile.delete()) { - throw new IOException("Could not delete " + fileOnDisk); - } - } - - final byte[] data = new byte[BUFFER_SIZE]; - try (FileOutputStream outFile = new FileOutputStream(fileOnDisk)) { - int count; - while ((count = input.read(data)) != -1) { - outFile.write(data, 0, count); - } - } - }); + final Path path) throws IOException { + return extractFileFromZip(zipFile, nameInZip, input -> + Files.copy(input, path, StandardCopyOption.REPLACE_EXISTING)); } /** diff --git a/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt b/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt index 862ac3b80..2d8a29acb 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt +++ b/app/src/test/java/org/schabi/newpipe/settings/ImportAllCombinationsTest.kt @@ -10,7 +10,9 @@ import org.schabi.newpipe.streams.io.StoredFileHelper import us.shandian.giga.io.FileStream import java.io.File import java.io.IOException -import java.nio.file.Files +import kotlin.io.path.createTempFile +import kotlin.io.path.exists +import kotlin.io.path.fileSize class ImportAllCombinationsTest { @@ -47,10 +49,10 @@ class ImportAllCombinationsTest { BackupFileLocator::class.java, Mockito.withSettings().stubOnly() ) - val db = File.createTempFile("newpipe_", "") - val dbJournal = File.createTempFile("newpipe_", "") - val dbWal = File.createTempFile("newpipe_", "") - val dbShm = File.createTempFile("newpipe_", "") + val db = createTempFile("newpipe_", "") + val dbJournal = createTempFile("newpipe_", "") + val dbWal = createTempFile("newpipe_", "") + val dbShm = createTempFile("newpipe_", "") Mockito.`when`(fileLocator.db).thenReturn(db) Mockito.`when`(fileLocator.dbJournal).thenReturn(dbJournal) Mockito.`when`(fileLocator.dbShm).thenReturn(dbShm) @@ -62,7 +64,7 @@ class ImportAllCombinationsTest { Assert.assertFalse(dbJournal.exists()) Assert.assertFalse(dbWal.exists()) Assert.assertFalse(dbShm.exists()) - Assert.assertTrue("database file size is zero", Files.size(db.toPath()) > 0) + Assert.assertTrue("database file size is zero", db.fileSize() > 0) } } else { runTest { @@ -70,7 +72,7 @@ class ImportAllCombinationsTest { Assert.assertTrue(dbJournal.exists()) Assert.assertTrue(dbWal.exists()) Assert.assertTrue(dbShm.exists()) - Assert.assertEquals(0, Files.size(db.toPath())) + Assert.assertEquals(0, db.fileSize()) } } diff --git a/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt b/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt index 5b8023561..f362963db 100644 --- a/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt +++ b/app/src/test/java/org/schabi/newpipe/settings/ImportExportManagerTest.kt @@ -25,8 +25,14 @@ import org.schabi.newpipe.streams.io.StoredFileHelper import us.shandian.giga.io.FileStream import java.io.File import java.io.ObjectInputStream -import java.nio.file.Files import java.util.zip.ZipFile +import kotlin.io.path.Path +import kotlin.io.path.createTempDirectory +import kotlin.io.path.createTempFile +import kotlin.io.path.deleteIfExists +import kotlin.io.path.exists +import kotlin.io.path.fileSize +import kotlin.io.path.inputStream @RunWith(MockitoJUnitRunner::class) class ImportExportManagerTest { @@ -46,7 +52,7 @@ class ImportExportManagerTest { @Test fun `The settings must be exported successfully in the correct format`() { - val db = File(classloader.getResource("settings/newpipe.db")!!.file) + val db = Path(classloader.getResource("settings/newpipe.db")!!.file) `when`(fileLocator.db).thenReturn(db) val expectedPreferences = mapOf("such pref" to "much wow") @@ -81,8 +87,8 @@ class ImportExportManagerTest { @Test fun `Ensuring db directory existence must work`() { - val dir = Files.createTempDirectory("newpipe_").toFile() - Assume.assumeTrue(dir.delete()) + val dir = createTempDirectory("newpipe_") + Assume.assumeTrue(dir.deleteIfExists()) `when`(fileLocator.dbDir).thenReturn(dir) ImportExportManager(fileLocator).ensureDbDirectoryExists() @@ -91,7 +97,7 @@ class ImportExportManagerTest { @Test fun `Ensuring db directory existence must work when the directory already exists`() { - val dir = Files.createTempDirectory("newpipe_").toFile() + val dir = createTempDirectory("newpipe_") `when`(fileLocator.dbDir).thenReturn(dir) ImportExportManager(fileLocator).ensureDbDirectoryExists() @@ -100,10 +106,10 @@ class ImportExportManagerTest { @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_", "") + val db = createTempFile("newpipe_", "") + val dbJournal = createTempFile("newpipe_", "") + val dbWal = createTempFile("newpipe_", "") + val dbShm = createTempFile("newpipe_", "") `when`(fileLocator.db).thenReturn(db) `when`(fileLocator.dbJournal).thenReturn(dbJournal) `when`(fileLocator.dbShm).thenReturn(dbShm) @@ -117,15 +123,15 @@ class ImportExportManagerTest { assertFalse(dbJournal.exists()) assertFalse(dbWal.exists()) assertFalse(dbShm.exists()) - assertTrue("database file size is zero", Files.size(db.toPath()) > 0) + assertTrue("database file size is zero", db.fileSize() > 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_", "") + val db = createTempFile("newpipe_", "") + val dbJournal = createTempFile("newpipe_", "") + val dbWal = createTempFile("newpipe_", "") + val dbShm = createTempFile("newpipe_", "") `when`(fileLocator.db).thenReturn(db) val emptyZip = File(classloader.getResource("settings/nodb_noser_nojson.zip")?.file!!) @@ -136,7 +142,7 @@ class ImportExportManagerTest { assertTrue(dbJournal.exists()) assertTrue(dbWal.exists()) assertTrue(dbShm.exists()) - assertEquals(0, Files.size(db.toPath())) + assertEquals(0, db.fileSize()) } @Test From 3e106b5e4feea5c3b54a3df06188507c4e10a329 Mon Sep 17 00:00:00 2001 From: Isira Seneviratne Date: Mon, 7 Jul 2025 07:53:20 +0530 Subject: [PATCH 2/4] Fix DB import/export issue --- .../newpipe/settings/BackupRestoreSettingsFragment.java | 6 ++---- .../schabi/newpipe/settings/export/ImportExportManager.kt | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java b/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java index cc93e9227..fca158c28 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java +++ b/app/src/main/java/org/schabi/newpipe/settings/BackupRestoreSettingsFragment.java @@ -17,7 +17,6 @@ import androidx.activity.result.ActivityResultLauncher; import androidx.activity.result.contract.ActivityResultContracts; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.core.content.ContextCompat; import androidx.preference.Preference; import androidx.preference.PreferenceManager; @@ -39,7 +38,6 @@ import java.io.IOException; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Locale; -import java.util.Objects; public class BackupRestoreSettingsFragment extends BasePreferenceFragment { @@ -60,8 +58,8 @@ public class BackupRestoreSettingsFragment extends BasePreferenceFragment { @Override public void onCreatePreferences(@Nullable final Bundle savedInstanceState, @Nullable final String rootKey) { - final var dbDir = Objects.requireNonNull(ContextCompat.getDataDir(requireContext())) - .toPath(); + final var dbDir = requireContext().getDatabasePath(BackupFileLocator.FILE_NAME_DB).toPath() + .getParent(); manager = new ImportExportManager(new BackupFileLocator(dbDir)); importExportDataPathKey = getString(R.string.import_export_data_path); diff --git a/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt b/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt index 04562bc77..6b0eb7eb9 100644 --- a/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt +++ b/app/src/main/java/org/schabi/newpipe/settings/export/ImportExportManager.kt @@ -13,7 +13,7 @@ import java.io.IOException import java.io.ObjectOutputStream import java.util.zip.ZipOutputStream import kotlin.io.path.createDirectories -import kotlin.io.path.deleteExisting +import kotlin.io.path.deleteIfExists class ImportExportManager(private val fileLocator: BackupFileLocator) { companion object { @@ -77,9 +77,9 @@ class ImportExportManager(private val fileLocator: BackupFileLocator) { val success = ZipHelper.extractFileFromZip(file, name, fileLocator.db) if (success) { - fileLocator.dbJournal.deleteExisting() - fileLocator.dbWal.deleteExisting() - fileLocator.dbShm.deleteExisting() + fileLocator.dbJournal.deleteIfExists() + fileLocator.dbWal.deleteIfExists() + fileLocator.dbShm.deleteIfExists() } return success From 225cb91105eb024cab54a3e76a545e5b1d7c6a1c Mon Sep 17 00:00:00 2001 From: Isira Seneviratne Date: Mon, 7 Jul 2025 08:06:52 +0530 Subject: [PATCH 3/4] Use InputStream#transferTo() --- .../org/schabi/newpipe/util/ZipHelper.java | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java b/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java index 771452c89..e53fbe52a 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java @@ -37,9 +37,6 @@ import java.util.zip.ZipOutputStream; */ public final class ZipHelper { - - private static final int BUFFER_SIZE = 2048; - @FunctionalInterface public interface InputStreamConsumer { void acceptStream(InputStream inputStream) throws IOException; @@ -80,13 +77,13 @@ public final class ZipHelper { final String nameInZip, final OutputStreamConsumer streamConsumer) throws IOException { final byte[] bytes; - try (ByteArrayOutputStream byteOutput = new ByteArrayOutputStream()) { + try (var byteOutput = new ByteArrayOutputStream()) { streamConsumer.acceptStream(byteOutput); bytes = byteOutput.toByteArray(); } - try (ByteArrayInputStream byteInput = new ByteArrayInputStream(bytes)) { - ZipHelper.addFileToZip(outZip, nameInZip, byteInput); + try (var byteInput = new ByteArrayInputStream(bytes)) { + addFileToZip(outZip, nameInZip, byteInput); } } @@ -97,19 +94,12 @@ public final class ZipHelper { * @param nameInZip the path of the file inside the zip * @param inputStream the content to put inside the file */ - public static void addFileToZip(final ZipOutputStream outZip, - final String nameInZip, - final InputStream inputStream) throws IOException { - final byte[] data = new byte[BUFFER_SIZE]; - try (BufferedInputStream bufferedInputStream = - new BufferedInputStream(inputStream, BUFFER_SIZE)) { - final ZipEntry entry = new ZipEntry(nameInZip); - outZip.putNextEntry(entry); - int count; - while ((count = bufferedInputStream.read(data, 0, BUFFER_SIZE)) != -1) { - outZip.write(data, 0, count); - } - } + private static void addFileToZip(final ZipOutputStream outZip, + final String nameInZip, + final InputStream inputStream) throws IOException { + final var entry = new ZipEntry(nameInZip); + outZip.putNextEntry(entry); + inputStream.transferTo(outZip); } /** From 4ffadc20577fce88a3ffad49fac78c0e1f1d90c0 Mon Sep 17 00:00:00 2001 From: Isira Seneviratne Date: Tue, 8 Jul 2025 06:21:42 +0530 Subject: [PATCH 4/4] Inline variable --- app/src/main/java/org/schabi/newpipe/util/ZipHelper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java b/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java index e53fbe52a..bccfc7f38 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ZipHelper.java @@ -97,8 +97,7 @@ public final class ZipHelper { private static void addFileToZip(final ZipOutputStream outZip, final String nameInZip, final InputStream inputStream) throws IOException { - final var entry = new ZipEntry(nameInZip); - outZip.putNextEntry(entry); + outZip.putNextEntry(new ZipEntry(nameInZip)); inputStream.transferTo(outZip); }