From 88d1f7118f3628fb103504bf9019eeb87cd43548 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Fri, 3 Apr 2020 13:30:52 +0100 Subject: [PATCH 1/2] #2002 Stop pre allocation of downloads and instead check currently downloading books to calculate available space --- app/detekt_baseline.xml | 3 +- .../settings/KiwixPrefsFragment.kt | 4 +- .../library_view/AvailableSpaceCalculator.kt | 53 +++++++++++++++++++ .../library_view/LibraryFragment.kt | 32 +++++------ .../kiwixmobile/core/dao/FetchDownloadDao.kt | 3 ++ .../core/di/modules/DownloaderModule.kt | 8 ++- .../core/downloader/model/DownloadModel.kt | 1 + .../core/settings/StorageCalculator.kt | 11 ++-- .../core/utils/SharedPreferenceUtil.java | 19 ++++--- 9 files changed, 98 insertions(+), 36 deletions(-) create mode 100644 app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/AvailableSpaceCalculator.kt diff --git a/app/detekt_baseline.xml b/app/detekt_baseline.xml index e8547da19..a2dff3391 100644 --- a/app/detekt_baseline.xml +++ b/app/detekt_baseline.xml @@ -7,12 +7,12 @@ EmptyFunctionBlock:ZimHostActivity.kt$ZimHostActivity.<no name provided>${} ForbiddenComment:KiwixMainActivity.kt$KiwixMainActivity$// TODO: Show to user LongParameterList:ZimManageViewModel.kt$ZimManageViewModel$( booksOnFileSystem: List<BookOnDisk>, activeDownloads: List<DownloadModel>, allLanguages: List<Language>, libraryNetworkEntity: LibraryNetworkEntity, filter: String, fileSystemState: FileSystemState ) - MagicNumber:LibraryFragment.kt$LibraryFragment$1024f MagicNumber:LibraryListItem.kt$LibraryListItem.LibraryDownloadItem$1000L MagicNumber:ShareFiles.kt$ShareFiles$24 MagicNumber:ZimManageViewModel.kt$ZimManageViewModel$5 MagicNumber:ZimManageViewModel.kt$ZimManageViewModel$500 MagicNumber:ZimManageViewModel.kt$ZimManageViewModel$60 + PackageNaming:AvailableSpaceCalculator.kt$package org.kiwix.kiwixmobile.zim_manager.library_view PackageNaming:ConnectivityBroadcastReceiver.kt$package org.kiwix.kiwixmobile.zim_manager PackageNaming:DefaultLanguageProvider.kt$package org.kiwix.kiwixmobile.zim_manager PackageNaming:DeleteFiles.kt$package org.kiwix.kiwixmobile.zim_manager.fileselect_view.effects @@ -42,7 +42,6 @@ PackageNaming:ZimManageViewModel.kt$package org.kiwix.kiwixmobile.zim_manager ReturnCount:Fat32Checker.kt$Fat32Checker$private fun canCreate4GbFile(storage: String): Boolean ReturnCount:LanguageActivity.kt$LanguageActivity$override fun onOptionsItemSelected(item: MenuItem): Boolean - ReturnCount:LibraryFragment.kt$LibraryFragment$private fun onBookItemClick(item: BookItem) TooGenericExceptionCaught:FileWritingFileSystemChecker.kt$FileWritingFileSystemChecker$e: Exception TooGenericExceptionCaught:KiwixMainActivity.kt$KiwixMainActivity$e: Exception TooGenericExceptionThrown:ActivityExtensions.kt$throw RuntimeException( """ applicationContext is ${applicationContext::class.java.simpleName} application is ${application::class.java.simpleName} """.trimIndent() ) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt index dcdcdcb0a..f745636d2 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/settings/KiwixPrefsFragment.kt @@ -23,7 +23,6 @@ import android.os.Environment import org.kiwix.kiwixmobile.core.settings.CorePrefsFragment import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil.PREF_LANG import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil.PREF_STORAGE -import java.io.File class KiwixPrefsFragment : CorePrefsFragment() { override fun onCreate(savedInstanceState: Bundle?) { @@ -37,7 +36,6 @@ class KiwixPrefsFragment : CorePrefsFragment() { } else { findPreference(PREF_STORAGE).title = sharedPreferenceUtil.getPrefStorageTitle("External") } - findPreference(PREF_STORAGE).summary = - storageCalculator.calculateAvailableSpace(File(sharedPreferenceUtil.prefStorage)) + findPreference(PREF_STORAGE).summary = storageCalculator.calculateAvailableSpace() } } diff --git a/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/AvailableSpaceCalculator.kt b/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/AvailableSpaceCalculator.kt new file mode 100644 index 000000000..b0a5de365 --- /dev/null +++ b/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/AvailableSpaceCalculator.kt @@ -0,0 +1,53 @@ +/* + * Kiwix Android + * Copyright (c) 2020 Kiwix + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package org.kiwix.kiwixmobile.zim_manager.library_view + +import eu.mhutti1.utils.storage.Bytes +import eu.mhutti1.utils.storage.Kb +import io.reactivex.android.schedulers.AndroidSchedulers +import io.reactivex.schedulers.Schedulers +import org.kiwix.kiwixmobile.core.dao.FetchDownloadDao +import org.kiwix.kiwixmobile.core.downloader.model.DownloadModel +import org.kiwix.kiwixmobile.core.settings.StorageCalculator +import org.kiwix.kiwixmobile.zim_manager.library_view.adapter.LibraryListItem +import javax.inject.Inject + +class AvailableSpaceCalculator @Inject constructor( + private val downloadDao: FetchDownloadDao, + private val storageCalculator: StorageCalculator +) { + fun hasAvailableSpaceFor( + bookItem: LibraryListItem.BookItem, + successAction: (LibraryListItem.BookItem) -> Unit, + failureAction: (String) -> Unit + ) { + downloadDao.allDownloads() + .map { it.map(DownloadModel::bytesRemaining).sum() } + .map { bytesToBeDownloaded -> storageCalculator.availableBytes() - bytesToBeDownloaded } + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe { trueAvailableBytes -> + if (bookItem.book.size.toLong() * Kb < trueAvailableBytes) { + successAction.invoke(bookItem) + } else { + failureAction.invoke(Bytes(trueAvailableBytes).humanReadable) + } + } + } +} diff --git a/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/LibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/LibraryFragment.kt index a47f72845..6707954ef 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/LibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/zim_manager/library_view/LibraryFragment.kt @@ -42,7 +42,6 @@ import org.kiwix.kiwixmobile.core.downloader.Downloader import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity.Book import org.kiwix.kiwixmobile.core.extensions.ActivityExtensions.viewModel import org.kiwix.kiwixmobile.core.extensions.snack -import org.kiwix.kiwixmobile.core.settings.StorageCalculator import org.kiwix.kiwixmobile.core.utils.BookUtils import org.kiwix.kiwixmobile.core.utils.DialogShower import org.kiwix.kiwixmobile.core.utils.KiwixDialog.YesNoDialog.StopDownload @@ -60,7 +59,6 @@ import org.kiwix.kiwixmobile.zim_manager.library_view.adapter.LibraryDelegate.Di import org.kiwix.kiwixmobile.zim_manager.library_view.adapter.LibraryDelegate.DownloadDelegate import org.kiwix.kiwixmobile.zim_manager.library_view.adapter.LibraryListItem import org.kiwix.kiwixmobile.zim_manager.library_view.adapter.LibraryListItem.BookItem -import java.io.File import javax.inject.Inject class LibraryFragment : BaseFragment() { @@ -71,7 +69,7 @@ class LibraryFragment : BaseFragment() { @Inject lateinit var dialogShower: DialogShower @Inject lateinit var viewModelFactory: ViewModelProvider.Factory @Inject lateinit var bookUtils: BookUtils - @Inject lateinit var storageCalculator: StorageCalculator + @Inject lateinit var availableSpaceCalculator: AvailableSpaceCalculator private val zimManageViewModel by lazy { activity!!.viewModel(viewModelFactory) @@ -87,9 +85,6 @@ class LibraryFragment : BaseFragment() { ) } - private val spaceAvailable: Long - get() = storageCalculator.availableBytes(File(sharedPreferenceUtil.prefStorage)) - private val noWifiWithWifiOnlyPreferenceSet get() = sharedPreferenceUtil.prefWifiOnly && !NetworkUtils.isWiFi(context!!) @@ -192,16 +187,6 @@ class LibraryFragment : BaseFragment() { private fun onBookItemClick(item: BookItem) { when { - notEnoughSpaceAvailable(item) -> { - libraryList.snack( - getString(R.string.download_no_space) + - "\n" + getString(R.string.space_available) + " " + - storageCalculator.calculateAvailableSpace(File(sharedPreferenceUtil.prefStorage)), - R.string.download_change_storage, - ::showStorageSelectDialog - ) - return - } isNotConnected -> { noInternetSnackbar() return @@ -213,13 +198,20 @@ class LibraryFragment : BaseFragment() { }) return } - else -> downloadFile(item.book) + else -> availableSpaceCalculator.hasAvailableSpaceFor(item, + { downloadFile(item.book) }, + { + libraryList.snack( + getString(R.string.download_no_space) + + "\n" + getString(R.string.space_available) + " " + + it, + R.string.download_change_storage, + ::showStorageSelectDialog + ) + }) } } - private fun notEnoughSpaceAvailable(item: BookItem) = - spaceAvailable < item.book.size.toLong() * 1024f - private fun showStorageSelectDialog() = StorageSelectDialog() .apply { onSelectAction = ::storeDeviceInPreferences diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/FetchDownloadDao.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/FetchDownloadDao.kt index e3f9dcf3d..2a6045e80 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/FetchDownloadDao.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/FetchDownloadDao.kt @@ -23,6 +23,7 @@ import io.objectbox.Box import io.objectbox.kotlin.equal import io.objectbox.kotlin.query import io.reactivex.Flowable +import io.reactivex.Single import org.kiwix.kiwixmobile.core.dao.entities.FetchDownloadEntity import org.kiwix.kiwixmobile.core.dao.entities.FetchDownloadEntity_ import org.kiwix.kiwixmobile.core.downloader.DownloadRequester @@ -43,6 +44,8 @@ class FetchDownloadDao @Inject constructor( .doOnNext(::moveCompletedToBooksOnDiskDao) .map { it.map(::DownloadModel) } + fun allDownloads() = Single.fromCallable { box.all.map(::DownloadModel) } + private fun moveCompletedToBooksOnDiskDao(downloadEntities: List) { downloadEntities.filter { it.status == COMPLETED }.takeIf { it.isNotEmpty() }?.let { box.store.callInTx { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/DownloaderModule.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/DownloaderModule.kt index 32d83660b..6b608ce53 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/DownloaderModule.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/DownloaderModule.kt @@ -73,13 +73,19 @@ object DownloaderModule { enableLogging(BuildConfig.DEBUG) enableRetryOnNetworkGain(true) setHttpDownloader(okHttpDownloader) + preAllocateFileOnCreation(false) setNotificationManager(fetchNotificationManager) }.build().also(Impl::setDefaultInstanceConfiguration) @JvmStatic @Provides @Singleton - fun provideOkHttpDownloader() = OkHttpDownloader(OkHttpClient.Builder().build()) + fun provideOkHttpDownloader() = OkHttpDownloader( + OkHttpClient.Builder() + .followRedirects(true) + .followSslRedirects(true) + .build() + ) @JvmStatic @Provides diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt index 6c1c518e3..a1261029a 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt @@ -35,6 +35,7 @@ data class DownloadModel( val progress: Int, val book: Book ) { + val bytesRemaining: Long by lazy { totalSizeOfDownload - bytesDownloaded } val fileNameFromUrl: String by lazy { StorageUtils.getFileNameFromUrl(book.url) } constructor(downloadEntity: FetchDownloadEntity) : this( diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/StorageCalculator.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/StorageCalculator.kt index 4f9e441fc..cee87f469 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/StorageCalculator.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/StorageCalculator.kt @@ -19,18 +19,21 @@ package org.kiwix.kiwixmobile.core.settings import eu.mhutti1.utils.storage.Bytes +import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import java.io.File import javax.inject.Inject -class StorageCalculator @Inject constructor() { +class StorageCalculator @Inject constructor( + private val sharedPreferenceUtil: SharedPreferenceUtil +) { - fun calculateAvailableSpace(file: File): String = + fun calculateAvailableSpace(file: File = File(sharedPreferenceUtil.prefStorage)): String = Bytes(availableBytes(file)).humanReadable - fun calculateTotalSpace(file: File): String = + fun calculateTotalSpace(file: File = File(sharedPreferenceUtil.prefStorage)): String = Bytes(totalBytes(file)).humanReadable - fun availableBytes(file: File) = + fun availableBytes(file: File = File(sharedPreferenceUtil.prefStorage)) = if (file.exists()) file.freeSpace else 0L diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.java b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.java index b041e5d87..7a34f1111 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.java +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/utils/SharedPreferenceUtil.java @@ -110,17 +110,24 @@ public class SharedPreferenceUtil { } public String getPrefStorage() { - String storage = sharedPreferences.getString(PREF_STORAGE, null); + final String storage = sharedPreferences.getString(PREF_STORAGE, null); if (storage == null) { - final File externalFilesDir = - ContextCompat.getExternalFilesDirs(CoreApp.getInstance(), null)[0]; - storage = externalFilesDir != null ? externalFilesDir.getPath() - : CoreApp.getInstance().getFilesDir().getPath(); // workaround for emulators - putPrefStorage(storage); + final String defaultStorage = defaultStorage(); + putPrefStorage(defaultStorage); + return defaultStorage; + } else if (!new File(storage).exists()) { + return defaultStorage(); } return storage; } + private String defaultStorage() { + final File externalFilesDir = + ContextCompat.getExternalFilesDirs(CoreApp.getInstance(), null)[0]; + return externalFilesDir != null ? externalFilesDir.getPath() + : CoreApp.getInstance().getFilesDir().getPath(); // workaround for emulators + } + public String getPrefStorageTitle(String defaultTitle) { return sharedPreferences.getString(PREF_STORAGE_TITLE, defaultTitle); } From 3e0c77059a89eacd4fe60d9202b90cb8eef95037 Mon Sep 17 00:00:00 2001 From: Sean Mac Gillicuddy Date: Fri, 3 Apr 2020 14:22:08 +0100 Subject: [PATCH 2/2] #2002 Stop pre allocation of downloads and instead check currently downloading books to calculate available space - fix unit test compilation --- .../kiwix/kiwixmobile/core/settings/StorageCalculatorTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/settings/StorageCalculatorTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/settings/StorageCalculatorTest.kt index 245ad967a..dadced942 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/settings/StorageCalculatorTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/settings/StorageCalculatorTest.kt @@ -26,7 +26,7 @@ import java.io.File internal class StorageCalculatorTest { - private val storageCalculator = StorageCalculator() + private val storageCalculator = StorageCalculator(mockk()) private val file: File = mockk() @Test