From 25e37e9c10139fdea892ea3c4c2a4e4503704cae Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Tue, 1 Oct 2024 14:52:58 +0200 Subject: [PATCH 01/18] Select files from file picker --- .../importfiles/ImportFilesScreen.kt | 53 +++++++++++++------ .../UploadSourceChoiceBottomSheet.kt | 26 ++++++--- 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index 74c759a0d..ec7262441 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -17,11 +17,17 @@ */ package com.infomaniak.swisstransfer.ui.screen.newtransfer.importfiles -import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf +import android.net.Uri +import androidx.activity.compose.rememberLauncherForActivityResult +import androidx.activity.result.contract.ActivityResultContracts +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.material3.Text +import androidx.compose.runtime.* import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.components.* import com.infomaniak.swisstransfer.ui.images.AppImages.AppIcons @@ -36,7 +42,17 @@ import com.infomaniak.swisstransfer.ui.utils.PreviewSmallWindow fun ImportFilesScreen(navigateToTransferTypeScreen: () -> Unit, closeActivity: () -> Unit) { var showUploadSourceChoiceBottomSheet by rememberSaveable { mutableStateOf(true) } - var isNextButtonEnabled by rememberSaveable { mutableStateOf(false) } + + var fileNames by rememberSaveable { mutableStateOf(listOf()) } + val isNextButtonEnabled by remember { derivedStateOf { fileNames.isNotEmpty() } } + + val filePickerLauncher = rememberLauncherForActivityResult( + contract = ActivityResultContracts.OpenMultipleDocuments() + ) { uris: List -> + fileNames = uris.map { uri -> + uri.lastPathSegment?.split("/")?.last() ?: "Unknown file" + } + } BottomStickyButtonScaffold( topBar = { @@ -52,10 +68,7 @@ fun ImportFilesScreen(navigateToTransferTypeScreen: () -> Unit, closeActivity: ( titleRes = R.string.buttonAddFiles, imageVector = AppIcons.Add, style = ButtonType.TERTIARY, - onClick = { - showUploadSourceChoiceBottomSheet = true - isNextButtonEnabled = true // TODO: Move this where it should be - }, + onClick = { showUploadSourceChoiceBottomSheet = true }, ) }, bottomButton = { modifier -> @@ -67,14 +80,24 @@ fun ImportFilesScreen(navigateToTransferTypeScreen: () -> Unit, closeActivity: ( ) }, content = { - EmptyState( - icon = AppIllus.MascotWithMagnifyingGlass, - title = R.string.noFileTitle, - description = R.string.noFileDescription, - ) + if (fileNames.isNotEmpty()) { + LazyColumn { + items(fileNames) { fileName -> + Text(text = fileName, modifier = Modifier.padding(8.dp)) + } + } + } else { + EmptyState( + icon = AppIllus.MascotWithMagnifyingGlass, + title = R.string.noFileTitle, + description = R.string.noFileDescription, + ) + } + UploadSourceChoiceBottomSheet( isBottomSheetVisible = { showUploadSourceChoiceBottomSheet }, - onDismissRequest = { showUploadSourceChoiceBottomSheet = false }, + onFilePickerClicked = { filePickerLauncher.launch(arrayOf("*/*")) }, + closeBottomSheet = { showUploadSourceChoiceBottomSheet = false }, ) }, ) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/UploadSourceChoiceBottomSheet.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/UploadSourceChoiceBottomSheet.kt index 1efa2919d..7c762f125 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/UploadSourceChoiceBottomSheet.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/UploadSourceChoiceBottomSheet.kt @@ -38,30 +38,40 @@ import com.infomaniak.swisstransfer.ui.utils.PreviewSmallWindow @Composable fun UploadSourceChoiceBottomSheet( isBottomSheetVisible: () -> Boolean, - onDismissRequest: () -> Unit, + onFilePickerClicked: () -> Unit, + closeBottomSheet: () -> Unit, ) { if (isBottomSheetVisible()) { SwissTransferBottomSheet( - onDismissRequest = onDismissRequest, + onDismissRequest = closeBottomSheet, titleRes = R.string.transferUploadSourceChoiceTitle, content = { Column { BottomSheetItem( imageVector = AppIcons.Camera, titleRes = R.string.transferUploadSourceChoiceCamera, - onClick = { /* TODO */ }, + onClick = { + closeBottomSheet() + /* TODO */ + }, ) HorizontalDivider(Modifier.padding(horizontal = Margin.Medium)) BottomSheetItem( imageVector = AppIcons.PolaroidLandscape, titleRes = R.string.transferUploadSourceChoiceGallery, - onClick = { /* TODO */ }, + onClick = { + closeBottomSheet() + /* TODO */ + }, ) HorizontalDivider(Modifier.padding(horizontal = Margin.Medium)) BottomSheetItem( imageVector = AppIcons.Folder, titleRes = R.string.transferUploadSourceChoiceFiles, - onClick = { /* TODO */ }, + onClick = { + closeBottomSheet() + onFilePickerClicked() + }, ) } }, @@ -75,7 +85,11 @@ fun UploadSourceChoiceBottomSheet( private fun UploadSourceChoiceBottomSheetPreview() { SwissTransferTheme { Surface { - UploadSourceChoiceBottomSheet(isBottomSheetVisible = { true }, onDismissRequest = {}) + UploadSourceChoiceBottomSheet( + isBottomSheetVisible = { true }, + onFilePickerClicked = {}, + closeBottomSheet = {} + ) } } } From e9109b8ae9f9895d2c13362192ca4a467da36b76 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Tue, 1 Oct 2024 16:23:49 +0200 Subject: [PATCH 02/18] Use view model to store files and read their name/size --- .../newtransfer/NewTransferViewModel.kt | 94 +++++++++++++++++++ .../importfiles/ImportFilesScreen.kt | 37 +++++--- 2 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt new file mode 100644 index 000000000..6d946d1ea --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -0,0 +1,94 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * 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 com.infomaniak.swisstransfer.ui.screen.newtransfer + +import android.app.Application +import android.content.ContentResolver +import android.content.Context +import android.database.Cursor +import android.net.Uri +import android.provider.OpenableColumns +import androidx.lifecycle.AndroidViewModel +import androidx.lifecycle.viewModelScope +import com.infomaniak.swisstransfer.ui.MainApplication +import kotlinx.coroutines.flow.* + +class NewTransferViewModel(application: Application) : AndroidViewModel(application) { + private val context get() = getApplication() as Context + + private val _fileUris = MutableStateFlow>(emptyList()) + val fileUris: StateFlow> = _fileUris + + private val _transferFiles: Flow> = _fileUris + .map { uris -> uris.map { getFileNameAndSize(it) } } + + val transferFiles: StateFlow> = _transferFiles + .map { files -> files.filter { file -> file.isSuccessfullyImported } } + .stateIn( + scope = viewModelScope, + started = SharingStarted.Lazily, + initialValue = emptyList() + ) + + val failedTransferFileCount: StateFlow = _transferFiles.map { files -> files.count { !it.isSuccessfullyImported } } + .stateIn( + scope = viewModelScope, + started = SharingStarted.Lazily, + initialValue = 0 + ) + + data class TransferFile(private val _metadata: Metadata?, val uri: Uri) { + val isSuccessfullyImported = _metadata != null + val metadata = _metadata!! + + data class Metadata(val fileName: String, val size: Long) + } + + + private fun getFileNameAndSize(uri: Uri): TransferFile { + var fileName: String? = null + var fileSize: Long? = null + + val contentResolver: ContentResolver = context.contentResolver + val cursor: Cursor? = contentResolver.query(uri, null, null, null, null) + + val isSuccess = cursor?.use { + if (it.moveToFirst()) { + val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return@use false + fileName = it.getString(displayNameColumnIndex) + + val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return@use false + fileSize = it.getLong(fileSizeColumnIndex) + + return@use true + } else { + return@use false + } + } ?: false + + return if (isSuccess) TransferFile(TransferFile.Metadata(fileName!!, fileSize!!), uri) else TransferFile(null, uri) + } + + private fun Cursor.getColumnIndexOrNull(column: String): Int? { + return getColumnIndex(column).takeIf { it != -1 } + } + + fun setFiles(uris: List) { + _fileUris.value = uris + } +} diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index ec7262441..7d6fd1a58 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -20,38 +20,47 @@ package com.infomaniak.swisstransfer.ui.screen.newtransfer.importfiles import android.net.Uri import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts -import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material3.Text import androidx.compose.runtime.* import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.ui.Modifier -import androidx.compose.ui.unit.dp +import androidx.lifecycle.viewmodel.compose.viewModel import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.components.* import com.infomaniak.swisstransfer.ui.images.AppImages.AppIcons import com.infomaniak.swisstransfer.ui.images.AppImages.AppIllus import com.infomaniak.swisstransfer.ui.images.icons.Add import com.infomaniak.swisstransfer.ui.images.illus.MascotWithMagnifyingGlass +import com.infomaniak.swisstransfer.ui.screen.newtransfer.NewTransferViewModel import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.PreviewLargeWindow import com.infomaniak.swisstransfer.ui.utils.PreviewSmallWindow @Composable -fun ImportFilesScreen(navigateToTransferTypeScreen: () -> Unit, closeActivity: () -> Unit) { +fun ImportFilesScreen( + newTransferViewModel: NewTransferViewModel = viewModel(), + navigateToTransferTypeScreen: () -> Unit, + closeActivity: () -> Unit, +) { + val transferFiles by newTransferViewModel.transferFiles.collectAsState() + ImportFilesScreen({ transferFiles }, newTransferViewModel::setFiles, closeActivity, navigateToTransferTypeScreen) +} +@Composable +private fun ImportFilesScreen( + transferFiles: () -> List, + setFiles: (List) -> Unit, + closeActivity: () -> Unit, + navigateToTransferTypeScreen: () -> Unit +) { var showUploadSourceChoiceBottomSheet by rememberSaveable { mutableStateOf(true) } - - var fileNames by rememberSaveable { mutableStateOf(listOf()) } - val isNextButtonEnabled by remember { derivedStateOf { fileNames.isNotEmpty() } } + val isNextButtonEnabled by remember { derivedStateOf { transferFiles().isNotEmpty() } } val filePickerLauncher = rememberLauncherForActivityResult( contract = ActivityResultContracts.OpenMultipleDocuments() ) { uris: List -> - fileNames = uris.map { uri -> - uri.lastPathSegment?.split("/")?.last() ?: "Unknown file" - } + setFiles(uris) } BottomStickyButtonScaffold( @@ -80,10 +89,10 @@ fun ImportFilesScreen(navigateToTransferTypeScreen: () -> Unit, closeActivity: ( ) }, content = { - if (fileNames.isNotEmpty()) { + if (transferFiles().isNotEmpty()) { LazyColumn { - items(fileNames) { fileName -> - Text(text = fileName, modifier = Modifier.padding(8.dp)) + items(items = transferFiles(), key = { it.uri }) { file -> + Text(text = "${file.metadata.fileName} - ${file.metadata.size}") } } } else { @@ -108,6 +117,6 @@ fun ImportFilesScreen(navigateToTransferTypeScreen: () -> Unit, closeActivity: ( @Composable private fun ImportFilesScreenPreview() { SwissTransferTheme { - ImportFilesScreen({}, {}) + ImportFilesScreen({ emptyList() }, {}, navigateToTransferTypeScreen = {}, closeActivity = {}) } } From a78734962b235dffe9d4f9c807fcb45a8c73b49b Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Tue, 1 Oct 2024 16:48:49 +0200 Subject: [PATCH 03/18] Add picked files to the existing ones --- .../ui/screen/newtransfer/NewTransferViewModel.kt | 4 ++-- .../ui/screen/newtransfer/importfiles/ImportFilesScreen.kt | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 6d946d1ea..77fa7e469 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -88,7 +88,7 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat return getColumnIndex(column).takeIf { it != -1 } } - fun setFiles(uris: List) { - _fileUris.value = uris + fun addFiles(uris: List) { + _fileUris.value += uris } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index 7d6fd1a58..16252bf1f 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -44,13 +44,13 @@ fun ImportFilesScreen( closeActivity: () -> Unit, ) { val transferFiles by newTransferViewModel.transferFiles.collectAsState() - ImportFilesScreen({ transferFiles }, newTransferViewModel::setFiles, closeActivity, navigateToTransferTypeScreen) + ImportFilesScreen({ transferFiles }, newTransferViewModel::addFiles, closeActivity, navigateToTransferTypeScreen) } @Composable private fun ImportFilesScreen( transferFiles: () -> List, - setFiles: (List) -> Unit, + addFiles: (List) -> Unit, closeActivity: () -> Unit, navigateToTransferTypeScreen: () -> Unit ) { @@ -60,7 +60,7 @@ private fun ImportFilesScreen( val filePickerLauncher = rememberLauncherForActivityResult( contract = ActivityResultContracts.OpenMultipleDocuments() ) { uris: List -> - setFiles(uris) + addFiles(uris) } BottomStickyButtonScaffold( From 5104ea01da6250ddc346c64e67ff6342210135f6 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Wed, 2 Oct 2024 11:24:46 +0200 Subject: [PATCH 04/18] Clean code --- .../ui/screen/newtransfer/NewTransferViewModel.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 77fa7e469..2ad155202 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -81,7 +81,8 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat } } ?: false - return if (isSuccess) TransferFile(TransferFile.Metadata(fileName!!, fileSize!!), uri) else TransferFile(null, uri) + val metadata = if (isSuccess) TransferFile.Metadata(customFileName, fileSize!!) else null + return TransferFile(metadata, uri) } private fun Cursor.getColumnIndexOrNull(column: String): Int? { From 7ffdeb2d7c3709714a5674fa4f08f0abd4e03e89 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Wed, 2 Oct 2024 11:25:41 +0200 Subject: [PATCH 05/18] Postfix already existing filenames with a number --- .../newtransfer/NewTransferViewModel.kt | 15 +++++ .../swisstransfer/ui/utils/FileNameUtils.kt | 62 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 2ad155202..c256281f0 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -26,6 +26,7 @@ import android.provider.OpenableColumns import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.infomaniak.swisstransfer.ui.MainApplication +import com.infomaniak.swisstransfer.ui.utils.FileNameUtils.postfixExistingFileNames import kotlinx.coroutines.flow.* class NewTransferViewModel(application: Application) : AndroidViewModel(application) { @@ -45,6 +46,18 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat initialValue = emptyList() ) + private val alreadyUsedFileNames: StateFlow> = transferFiles + .map { files -> + buildSet { + files.forEach { add(it.metadata.fileName) } + } + } + .stateIn( + scope = viewModelScope, + started = SharingStarted.Lazily, + initialValue = emptySet(), + ) + val failedTransferFileCount: StateFlow = _transferFiles.map { files -> files.count { !it.isSuccessfullyImported } } .stateIn( scope = viewModelScope, @@ -81,6 +94,8 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat } } ?: false + val customFileName = postfixExistingFileNames(fileName!!, alreadyUsedFileNames.value) + val metadata = if (isSuccess) TransferFile.Metadata(customFileName, fileSize!!) else null return TransferFile(metadata, uri) } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt new file mode 100644 index 000000000..0ef0bcee9 --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt @@ -0,0 +1,62 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * 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 com.infomaniak.swisstransfer.ui.utils + +object FileNameUtils { + private val ALREADY_POSTFIXED_REGEX = Regex("""(.*\()(\d)(\))$""") + + fun postfixExistingFileNames(wholeFileName: String, existingFileNames: Set): String { + return if (wholeFileName in existingFileNames) { + val (name, ext) = splitNameAndExtension(wholeFileName) + + var (nameStart, namePostfixNumber, nameEnd) = ALREADY_POSTFIXED_REGEX.find(name)?.let { + Triple(it.groupValues[1], (it.groupValues[2]).toInt(), it.groupValues[3]) + } ?: Triple("$name(", 0, ")") + + fun incrementFileNamePostfix() = incrementFileNamePostfix(nameStart, namePostfixNumber, nameEnd, ext) + + var newName = incrementFileNamePostfix() + while (newName in existingFileNames) { + namePostfixNumber += 1 + newName = incrementFileNamePostfix() + } + + newName + } else { + wholeFileName + } + } + + private fun incrementFileNamePostfix( + nameStart: String, + previousPostfixNumber: Int, + nameEnd: String, + ext: String, + ) = "$nameStart${(previousPostfixNumber + 1)}$nameEnd$ext" + + private fun splitNameAndExtension(fileName: String): Pair { + val dotIndex = fileName.lastIndexOf('.') + + // If there's no dot or it's the first/last character, return the whole name and an empty extension + return if (dotIndex == -1 || dotIndex == 0 || dotIndex == fileName.length - 1) { + fileName to "" + } else { + fileName.substring(0, dotIndex) to fileName.substring(dotIndex) + } + } +} From 146e83610dd1ea80a1947e46af11b012ef450cdd Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Wed, 2 Oct 2024 15:29:36 +0200 Subject: [PATCH 06/18] Better split postfixed file names' logic and do not detect already postfixed file names by the user --- .../swisstransfer/ui/utils/FileNameUtils.kt | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt index 0ef0bcee9..090d7918c 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt @@ -18,45 +18,54 @@ package com.infomaniak.swisstransfer.ui.utils object FileNameUtils { - private val ALREADY_POSTFIXED_REGEX = Regex("""(.*\()(\d)(\))$""") - fun postfixExistingFileNames(wholeFileName: String, existingFileNames: Set): String { return if (wholeFileName in existingFileNames) { - val (name, ext) = splitNameAndExtension(wholeFileName) - - var (nameStart, namePostfixNumber, nameEnd) = ALREADY_POSTFIXED_REGEX.find(name)?.let { - Triple(it.groupValues[1], (it.groupValues[2]).toInt(), it.groupValues[3]) - } ?: Triple("$name(", 0, ")") - - fun incrementFileNamePostfix() = incrementFileNamePostfix(nameStart, namePostfixNumber, nameEnd, ext) + val postfixedFileName = PostfixedFileName.fromFileName(wholeFileName) - var newName = incrementFileNamePostfix() - while (newName in existingFileNames) { - namePostfixNumber += 1 - newName = incrementFileNamePostfix() + while (postfixedFileName.assembleFileName() in existingFileNames) { + postfixedFileName.incrementPostfix() } - newName + postfixedFileName.assembleFileName() } else { wholeFileName } } - private fun incrementFileNamePostfix( - nameStart: String, - previousPostfixNumber: Int, - nameEnd: String, - ext: String, - ) = "$nameStart${(previousPostfixNumber + 1)}$nameEnd$ext" + private data class PostfixedFileName( + val start: String, + var postfixNumber: Int, + val end: String, + val extension: String, + ) { + fun incrementPostfix() { + postfixNumber += 1 + } - private fun splitNameAndExtension(fileName: String): Pair { - val dotIndex = fileName.lastIndexOf('.') + fun assembleFileName(): String = "$start$postfixNumber$end$extension" - // If there's no dot or it's the first/last character, return the whole name and an empty extension - return if (dotIndex == -1 || dotIndex == 0 || dotIndex == fileName.length - 1) { - fileName to "" - } else { - fileName.substring(0, dotIndex) to fileName.substring(dotIndex) + companion object { + fun fromFileName(wholeFileName: String): PostfixedFileName { + val (name, ext) = splitNameAndExtension(wholeFileName) + + return PostfixedFileName( + start = "$name(", + postfixNumber = 1, + end = ")", + extension = ext, + ) + } + + private fun splitNameAndExtension(fileName: String): Pair { + val dotIndex = fileName.lastIndexOf('.') + + // If there's no dot or it's the first/last character, return the whole name and an empty extension + return if (dotIndex == -1 || dotIndex == 0 || dotIndex == fileName.length - 1) { + fileName to "" + } else { + fileName.substring(0, dotIndex) to fileName.substring(dotIndex) + } + } } } } From 95c97afdfe03753d8288b7e8bd38dcde8b4322dd Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 08:43:15 +0200 Subject: [PATCH 07/18] Add unit tests for postfixing already used filenames with a number --- .../PostifxedFileNameUnitTest.kt | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt diff --git a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt new file mode 100644 index 000000000..e0e870980 --- /dev/null +++ b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt @@ -0,0 +1,89 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * 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 com.infomaniak.swisstransfer + +import com.infomaniak.swisstransfer.ui.utils.FileNameUtils.postfixExistingFileNames +import org.junit.Assert.assertEquals +import org.junit.Test + +class PostifxedFileNameUnitTest { + @Test + fun unusedName_isUnchanged() { + val inputFileName = "world.txt" + val newName = postfixExistingFileNames(inputFileName, alreadyUsedFileNames) + assertEquals(newName, inputFileName) + } + + @Test + fun usedName_isPostfixed() { + val newName = postfixExistingFileNames("hello.txt", alreadyUsedFileNames) + assertEquals(newName, "hello(1).txt") + } + + @Test + fun alreadyPostfixedName_isPostfixedAgain() { + val newName = postfixExistingFileNames("test(1).txt", alreadyUsedFileNames) + assertEquals(newName, "test(1)(1).txt") + } + + @Test + fun postfixedNameThatCollidesWithAnotherName_isPostfixedUntilNoMoreCollision() { + val newName = postfixExistingFileNames("test.txt", alreadyUsedFileNames) + assertEquals(newName, "test(3).txt") + } + + @Test + fun unusedEmptyFileName_isUnchanged() { + assertNewFileNameIsUnchanged(inputFileName = "") + } + + @Test + fun usedEmptyFileName_isPostfixed() { + assertAlreadyExistingFileName(inputFileName = "", expectedFileName = "(1)") + } + + @Test + fun unusedNoExtensionFileName_isUnchanged() { + assertNewFileNameIsUnchanged(inputFileName = "file") + } + + @Test + fun usedNoExtensionFileName_isPostfixed() { + assertAlreadyExistingFileName(inputFileName = "file", expectedFileName = "file(1)") + } + + @Test + fun multipleExtensionFileName_isPostfixedBeforeLastOne() { + assertAlreadyExistingFileName(inputFileName = "file.abc.def.ghi", expectedFileName = "file.abc.def(1).ghi") + } + + private fun assertAlreadyExistingFileName(inputFileName: String, expectedFileName: String) { + val newName = postfixExistingFileNames(inputFileName, setOf(inputFileName)) + assertEquals(newName, expectedFileName) + } + + private fun assertNewFileNameIsUnchanged(inputFileName: String) { + val newName = postfixExistingFileNames(inputFileName, emptySet()) + assertEquals(newName, inputFileName) + } + + companion object { + // Used for tests that require to check existing filenames among multiple edge case already existing filenames + val alreadyUsedFileNames = setOf("test.txt", "test(1).txt", "test(2).txt", "hello.txt") + } +} From 2eb1f38d3a419f33805b6fa63fad6e1023a6e9e2 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 08:55:27 +0200 Subject: [PATCH 08/18] Rearrange code --- .../newtransfer/NewTransferViewModel.kt | 45 ++++++++----------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index c256281f0..4f69e635f 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -26,14 +26,12 @@ import android.provider.OpenableColumns import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.infomaniak.swisstransfer.ui.MainApplication -import com.infomaniak.swisstransfer.ui.utils.FileNameUtils.postfixExistingFileNames import kotlinx.coroutines.flow.* class NewTransferViewModel(application: Application) : AndroidViewModel(application) { private val context get() = getApplication() as Context private val _fileUris = MutableStateFlow>(emptyList()) - val fileUris: StateFlow> = _fileUris private val _transferFiles: Flow> = _fileUris .map { uris -> uris.map { getFileNameAndSize(it) } } @@ -65,46 +63,41 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat initialValue = 0 ) - data class TransferFile(private val _metadata: Metadata?, val uri: Uri) { - val isSuccessfullyImported = _metadata != null - val metadata = _metadata!! - - data class Metadata(val fileName: String, val size: Long) + fun addFiles(uris: List) { + _fileUris.value += uris } - private fun getFileNameAndSize(uri: Uri): TransferFile { - var fileName: String? = null - var fileSize: Long? = null - val contentResolver: ContentResolver = context.contentResolver val cursor: Cursor? = contentResolver.query(uri, null, null, null, null) - val isSuccess = cursor?.use { + return TransferFile(getFileMetadata(cursor), uri) + } + + private fun getFileMetadata(cursor: Cursor?): TransferFile.Metadata? { + cursor?.use { if (it.moveToFirst()) { - val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return@use false - fileName = it.getString(displayNameColumnIndex) + val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return null + val fileName = it.getString(displayNameColumnIndex) - val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return@use false - fileSize = it.getLong(fileSizeColumnIndex) + val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return null + val fileSize = it.getLong(fileSizeColumnIndex) - return@use true + return TransferFile.Metadata(fileName, fileSize) } else { - return@use false + return null } - } ?: false - - val customFileName = postfixExistingFileNames(fileName!!, alreadyUsedFileNames.value) - - val metadata = if (isSuccess) TransferFile.Metadata(customFileName, fileSize!!) else null - return TransferFile(metadata, uri) + } ?: return null } private fun Cursor.getColumnIndexOrNull(column: String): Int? { return getColumnIndex(column).takeIf { it != -1 } } - fun addFiles(uris: List) { - _fileUris.value += uris + data class TransferFile(private val _metadata: Metadata?, val uri: Uri) { + val isSuccessfullyImported = _metadata != null + val metadata = _metadata!! + + data class Metadata(val fileName: String, val size: Long) } } From fee60b7b4a84b990db9f675777c2dd6790e0bf90 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 09:49:49 +0200 Subject: [PATCH 09/18] Support importing the same file twice inside the ImportFilesScreen and NewTransferViewModel --- .../newtransfer/NewTransferViewModel.kt | 55 +++++++++++++------ .../importfiles/ImportFilesScreen.kt | 4 +- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 4f69e635f..b4cb6bb7e 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -26,45 +26,63 @@ import android.provider.OpenableColumns import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.infomaniak.swisstransfer.ui.MainApplication +import com.infomaniak.swisstransfer.ui.utils.FileNameUtils.postfixExistingFileNames +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.* +import kotlinx.coroutines.launch +@OptIn(ExperimentalCoroutinesApi::class) class NewTransferViewModel(application: Application) : AndroidViewModel(application) { private val context get() = getApplication() as Context - private val _fileUris = MutableStateFlow>(emptyList()) + // TODO: replay = 1 not needed for now because addFiles() cannot be called before we start listening to the sharedflow in the + // viewmodel + private val addedFilesUri = MutableSharedFlow>(replay = 1) + private val allTransferFiles = MutableStateFlow>(emptyList()) - private val _transferFiles: Flow> = _fileUris - .map { uris -> uris.map { getFileNameAndSize(it) } } - - val transferFiles: StateFlow> = _transferFiles - .map { files -> files.filter { file -> file.isSuccessfullyImported } } + val successfulTransferFiles: StateFlow> = allTransferFiles + .mapLatest { files -> files.filter { file -> file.isSuccessfullyImported } } .stateIn( scope = viewModelScope, - started = SharingStarted.Lazily, + started = SharingStarted.Eagerly, initialValue = emptyList() ) - private val alreadyUsedFileNames: StateFlow> = transferFiles - .map { files -> + private val alreadyUsedFileNames: StateFlow> = successfulTransferFiles + .mapLatest { files -> buildSet { files.forEach { add(it.metadata.fileName) } } } .stateIn( scope = viewModelScope, - started = SharingStarted.Lazily, + started = SharingStarted.Eagerly, initialValue = emptySet(), ) - val failedTransferFileCount: StateFlow = _transferFiles.map { files -> files.count { !it.isSuccessfullyImported } } - .stateIn( - scope = viewModelScope, - started = SharingStarted.Lazily, - initialValue = 0 - ) + val failedTransferFileCount: StateFlow = + allTransferFiles.mapLatest { files -> files.count { !it.isSuccessfullyImported } } + .stateIn( + scope = viewModelScope, + started = SharingStarted.Lazily, // TODO: Decide of the SharingStarted strategy once this value is used + initialValue = 0 + ) + + init { + viewModelScope.launch { + addedFilesUri.collect { uris -> + uris.forEach { uri -> + val transferFile = getFileNameAndSize(uri) + allTransferFiles.value += transferFile + } + } + } + } fun addFiles(uris: List) { - _fileUris.value += uris + viewModelScope.launch { + addedFilesUri.emit(uris) + } } private fun getFileNameAndSize(uri: Uri): TransferFile { @@ -83,7 +101,8 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return null val fileSize = it.getLong(fileSizeColumnIndex) - return TransferFile.Metadata(fileName, fileSize) + val customName = postfixExistingFileNames(fileName, alreadyUsedFileNames.value) + return TransferFile.Metadata(customName, fileSize) } else { return null } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index 16252bf1f..7b14b52a6 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -43,7 +43,7 @@ fun ImportFilesScreen( navigateToTransferTypeScreen: () -> Unit, closeActivity: () -> Unit, ) { - val transferFiles by newTransferViewModel.transferFiles.collectAsState() + val transferFiles by newTransferViewModel.successfulTransferFiles.collectAsState() ImportFilesScreen({ transferFiles }, newTransferViewModel::addFiles, closeActivity, navigateToTransferTypeScreen) } @@ -91,7 +91,7 @@ private fun ImportFilesScreen( content = { if (transferFiles().isNotEmpty()) { LazyColumn { - items(items = transferFiles(), key = { it.uri }) { file -> + items(items = transferFiles(), key = { it.metadata.fileName }) { file -> Text(text = "${file.metadata.fileName} - ${file.metadata.size}") } } From 19595c812c93203c563addc564a60124dfbf6c3d Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 14:34:40 +0200 Subject: [PATCH 10/18] Apply suggestions from code review by rewriting some parts of the code --- .../newtransfer/NewTransferViewModel.kt | 99 +++---------------- .../ui/screen/newtransfer/TransferFile.kt | 22 +++++ .../screen/newtransfer/TransferFileUtils.kt | 57 +++++++++++ .../importfiles/ImportFilesScreen.kt | 24 ++--- .../swisstransfer/ui/utils/FileNameUtils.kt | 29 +++--- .../PostifxedFileNameUnitTest.kt | 2 +- 6 files changed, 123 insertions(+), 110 deletions(-) create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFile.kt create mode 100644 app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index b4cb6bb7e..28d6678ac 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -18,105 +18,36 @@ package com.infomaniak.swisstransfer.ui.screen.newtransfer import android.app.Application -import android.content.ContentResolver import android.content.Context -import android.database.Cursor import android.net.Uri -import android.provider.OpenableColumns import androidx.lifecycle.AndroidViewModel import androidx.lifecycle.viewModelScope import com.infomaniak.swisstransfer.ui.MainApplication -import com.infomaniak.swisstransfer.ui.utils.FileNameUtils.postfixExistingFileNames -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.* +import com.infomaniak.swisstransfer.ui.screen.newtransfer.TransferFileUtils.getTransferFile +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch -@OptIn(ExperimentalCoroutinesApi::class) class NewTransferViewModel(application: Application) : AndroidViewModel(application) { private val context get() = getApplication() as Context - // TODO: replay = 1 not needed for now because addFiles() cannot be called before we start listening to the sharedflow in the - // viewmodel - private val addedFilesUri = MutableSharedFlow>(replay = 1) - private val allTransferFiles = MutableStateFlow>(emptyList()) + val transferFiles = MutableStateFlow>(emptyList()) + val failedTransferFileCount = MutableSharedFlow() - val successfulTransferFiles: StateFlow> = allTransferFiles - .mapLatest { files -> files.filter { file -> file.isSuccessfullyImported } } - .stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptyList() - ) - - private val alreadyUsedFileNames: StateFlow> = successfulTransferFiles - .mapLatest { files -> - buildSet { - files.forEach { add(it.metadata.fileName) } - } - } - .stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptySet(), - ) - - val failedTransferFileCount: StateFlow = - allTransferFiles.mapLatest { files -> files.count { !it.isSuccessfullyImported } } - .stateIn( - scope = viewModelScope, - started = SharingStarted.Lazily, // TODO: Decide of the SharingStarted strategy once this value is used - initialValue = 0 - ) - - init { + fun addFiles(uris: List) { viewModelScope.launch { - addedFilesUri.collect { uris -> - uris.forEach { uri -> - val transferFile = getFileNameAndSize(uri) - allTransferFiles.value += transferFile + val alreadyUsedFileNames = buildSet { transferFiles.value.forEach { add(it.fileName) } } + + var failedFileCount = 0 + uris.forEach { uri -> + context.getTransferFile(uri, alreadyUsedFileNames)?.let { transferFile -> + transferFiles.value += transferFile + } ?: run { + failedFileCount++ } } - } - } - fun addFiles(uris: List) { - viewModelScope.launch { - addedFilesUri.emit(uris) + failedTransferFileCount.emit(failedFileCount) } } - - private fun getFileNameAndSize(uri: Uri): TransferFile { - val contentResolver: ContentResolver = context.contentResolver - val cursor: Cursor? = contentResolver.query(uri, null, null, null, null) - - return TransferFile(getFileMetadata(cursor), uri) - } - - private fun getFileMetadata(cursor: Cursor?): TransferFile.Metadata? { - cursor?.use { - if (it.moveToFirst()) { - val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return null - val fileName = it.getString(displayNameColumnIndex) - - val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return null - val fileSize = it.getLong(fileSizeColumnIndex) - - val customName = postfixExistingFileNames(fileName, alreadyUsedFileNames.value) - return TransferFile.Metadata(customName, fileSize) - } else { - return null - } - } ?: return null - } - - private fun Cursor.getColumnIndexOrNull(column: String): Int? { - return getColumnIndex(column).takeIf { it != -1 } - } - - data class TransferFile(private val _metadata: Metadata?, val uri: Uri) { - val isSuccessfullyImported = _metadata != null - val metadata = _metadata!! - - data class Metadata(val fileName: String, val size: Long) - } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFile.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFile.kt new file mode 100644 index 000000000..e1e98d815 --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFile.kt @@ -0,0 +1,22 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * 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 com.infomaniak.swisstransfer.ui.screen.newtransfer + +import android.net.Uri + +data class TransferFile(val fileName: String, val size: Long, val uri: Uri) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt new file mode 100644 index 000000000..3f00a3fa3 --- /dev/null +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt @@ -0,0 +1,57 @@ +/* + * Infomaniak SwissTransfer - Android + * Copyright (C) 2024 Infomaniak Network SA + * + * 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 com.infomaniak.swisstransfer.ui.screen.newtransfer + +import android.content.ContentResolver +import android.content.Context +import android.database.Cursor +import android.net.Uri +import android.provider.OpenableColumns +import com.infomaniak.swisstransfer.ui.utils.FileNameUtils + +object TransferFileUtils { + fun Context.getTransferFile(uri: Uri, alreadyUsedFileNames: Set): TransferFile? { + val contentResolver: ContentResolver = contentResolver + val cursor: Cursor? = contentResolver.query(uri, null, null, null, null) + + return cursor?.getFileNameAndSize()?.let { (name, size) -> + val uniqueName = FileNameUtils.postfixExistingFileNames(name, alreadyUsedFileNames) + TransferFile(uniqueName, size, uri) + } + } + + private fun Cursor.getFileNameAndSize(): Pair? { + use { + if (it.moveToFirst()) { + val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return null + val fileName = it.getString(displayNameColumnIndex) + + val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return null + val fileSize = it.getLong(fileSizeColumnIndex) + + return fileName to fileSize + } else { + return null + } + } + } + + private fun Cursor.getColumnIndexOrNull(column: String): Int? { + return getColumnIndex(column).takeIf { it != -1 } + } +} diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index 7b14b52a6..efac6f36b 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -25,6 +25,7 @@ import androidx.compose.foundation.lazy.items import androidx.compose.material3.Text import androidx.compose.runtime.* import androidx.compose.runtime.saveable.rememberSaveable +import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.components.* @@ -33,6 +34,7 @@ import com.infomaniak.swisstransfer.ui.images.AppImages.AppIllus import com.infomaniak.swisstransfer.ui.images.icons.Add import com.infomaniak.swisstransfer.ui.images.illus.MascotWithMagnifyingGlass import com.infomaniak.swisstransfer.ui.screen.newtransfer.NewTransferViewModel +import com.infomaniak.swisstransfer.ui.screen.newtransfer.TransferFile import com.infomaniak.swisstransfer.ui.theme.SwissTransferTheme import com.infomaniak.swisstransfer.ui.utils.PreviewLargeWindow import com.infomaniak.swisstransfer.ui.utils.PreviewSmallWindow @@ -43,16 +45,16 @@ fun ImportFilesScreen( navigateToTransferTypeScreen: () -> Unit, closeActivity: () -> Unit, ) { - val transferFiles by newTransferViewModel.successfulTransferFiles.collectAsState() - ImportFilesScreen({ transferFiles }, newTransferViewModel::addFiles, closeActivity, navigateToTransferTypeScreen) + val transferFiles by newTransferViewModel.transferFiles.collectAsStateWithLifecycle() + ImportFilesScreen({ transferFiles }, newTransferViewModel::addFiles, navigateToTransferTypeScreen, closeActivity) } @Composable private fun ImportFilesScreen( - transferFiles: () -> List, + transferFiles: () -> List, addFiles: (List) -> Unit, + navigateToTransferTypeScreen: () -> Unit, closeActivity: () -> Unit, - navigateToTransferTypeScreen: () -> Unit ) { var showUploadSourceChoiceBottomSheet by rememberSaveable { mutableStateOf(true) } val isNextButtonEnabled by remember { derivedStateOf { transferFiles().isNotEmpty() } } @@ -89,18 +91,18 @@ private fun ImportFilesScreen( ) }, content = { - if (transferFiles().isNotEmpty()) { - LazyColumn { - items(items = transferFiles(), key = { it.metadata.fileName }) { file -> - Text(text = "${file.metadata.fileName} - ${file.metadata.size}") - } - } - } else { + if (transferFiles().isEmpty()) { EmptyState( icon = AppIllus.MascotWithMagnifyingGlass, title = R.string.noFileTitle, description = R.string.noFileDescription, ) + } else { + LazyColumn { + items(items = transferFiles(), key = { it.fileName }) { file -> + Text(text = "${file.fileName} - ${file.size}") + } + } } UploadSourceChoiceBottomSheet( diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt index 090d7918c..878b01097 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/utils/FileNameUtils.kt @@ -18,35 +18,36 @@ package com.infomaniak.swisstransfer.ui.utils object FileNameUtils { - fun postfixExistingFileNames(wholeFileName: String, existingFileNames: Set): String { - return if (wholeFileName in existingFileNames) { - val postfixedFileName = PostfixedFileName.fromFileName(wholeFileName) - while (postfixedFileName.assembleFileName() in existingFileNames) { + fun postfixExistingFileNames(fileName: String, existingFileNames: Set): String { + return if (fileName in existingFileNames) { + val postfixedFileName = PostfixedFileName.fromFileName(fileName) + + while (postfixedFileName.fullName() in existingFileNames) { postfixedFileName.incrementPostfix() } - postfixedFileName.assembleFileName() + postfixedFileName.fullName() } else { - wholeFileName + fileName } } private data class PostfixedFileName( - val start: String, - var postfixNumber: Int, - val end: String, - val extension: String, + private val start: String, + private var postfixNumber: Int, + private val end: String, + private val extension: String, ) { fun incrementPostfix() { postfixNumber += 1 } - fun assembleFileName(): String = "$start$postfixNumber$end$extension" + fun fullName(): String = "$start$postfixNumber$end$extension" companion object { - fun fromFileName(wholeFileName: String): PostfixedFileName { - val (name, ext) = splitNameAndExtension(wholeFileName) + fun fromFileName(fileName: String): PostfixedFileName { + val (name, ext) = splitNameAndExtension(fileName) return PostfixedFileName( start = "$name(", @@ -60,7 +61,7 @@ object FileNameUtils { val dotIndex = fileName.lastIndexOf('.') // If there's no dot or it's the first/last character, return the whole name and an empty extension - return if (dotIndex == -1 || dotIndex == 0 || dotIndex == fileName.length - 1) { + return if (dotIndex == -1) { fileName to "" } else { fileName.substring(0, dotIndex) to fileName.substring(dotIndex) diff --git a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt index e0e870980..2f024595a 100644 --- a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt +++ b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt @@ -84,6 +84,6 @@ class PostifxedFileNameUnitTest { companion object { // Used for tests that require to check existing filenames among multiple edge case already existing filenames - val alreadyUsedFileNames = setOf("test.txt", "test(1).txt", "test(2).txt", "hello.txt") + private val alreadyUsedFileNames = setOf("test.txt", "test(1).txt", "test(2).txt", "hello.txt") } } From d0f86de72860c56c9761a6bd19b9025ba3682d88 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 14:42:16 +0200 Subject: [PATCH 11/18] Inject NewTransferViewModel using Hilt --- .../ui/screen/newtransfer/NewTransferViewModel.kt | 13 +++++++------ .../newtransfer/importfiles/ImportFilesScreen.kt | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 28d6678ac..d8092856d 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -17,19 +17,20 @@ */ package com.infomaniak.swisstransfer.ui.screen.newtransfer -import android.app.Application import android.content.Context import android.net.Uri -import androidx.lifecycle.AndroidViewModel +import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.infomaniak.swisstransfer.ui.MainApplication import com.infomaniak.swisstransfer.ui.screen.newtransfer.TransferFileUtils.getTransferFile +import dagger.hilt.android.lifecycle.HiltViewModel +import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch +import javax.inject.Inject -class NewTransferViewModel(application: Application) : AndroidViewModel(application) { - private val context get() = getApplication() as Context +@HiltViewModel +class NewTransferViewModel @Inject constructor(@ApplicationContext private val appContext: Context) : ViewModel() { val transferFiles = MutableStateFlow>(emptyList()) val failedTransferFileCount = MutableSharedFlow() @@ -40,7 +41,7 @@ class NewTransferViewModel(application: Application) : AndroidViewModel(applicat var failedFileCount = 0 uris.forEach { uri -> - context.getTransferFile(uri, alreadyUsedFileNames)?.let { transferFile -> + appContext.getTransferFile(uri, alreadyUsedFileNames)?.let { transferFile -> transferFiles.value += transferFile } ?: run { failedFileCount++ diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt index efac6f36b..ba400fa79 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/importfiles/ImportFilesScreen.kt @@ -25,8 +25,8 @@ import androidx.compose.foundation.lazy.items import androidx.compose.material3.Text import androidx.compose.runtime.* import androidx.compose.runtime.saveable.rememberSaveable +import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle -import androidx.lifecycle.viewmodel.compose.viewModel import com.infomaniak.swisstransfer.R import com.infomaniak.swisstransfer.ui.components.* import com.infomaniak.swisstransfer.ui.images.AppImages.AppIcons @@ -41,7 +41,7 @@ import com.infomaniak.swisstransfer.ui.utils.PreviewSmallWindow @Composable fun ImportFilesScreen( - newTransferViewModel: NewTransferViewModel = viewModel(), + newTransferViewModel: NewTransferViewModel = hiltViewModel(), navigateToTransferTypeScreen: () -> Unit, closeActivity: () -> Unit, ) { From 50b444cd4a426b6019d5710e4cae19ef874e3791 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 15:05:09 +0200 Subject: [PATCH 12/18] Add missing PostifxedFileName tests --- .../swisstransfer/PostifxedFileNameUnitTest.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt index 2f024595a..43bba2161 100644 --- a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt +++ b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt @@ -72,6 +72,16 @@ class PostifxedFileNameUnitTest { assertAlreadyExistingFileName(inputFileName = "file.abc.def.ghi", expectedFileName = "file.abc.def(1).ghi") } + @Test + fun unusedDotEndingFileName_isUnchanged() { + assertNewFileNameIsUnchanged(inputFileName = "file.") + } + + @Test + fun usedDotEndingFileName_isPostfixed() { + assertAlreadyExistingFileName(inputFileName = "file.", expectedFileName = "file(1).") + } + private fun assertAlreadyExistingFileName(inputFileName: String, expectedFileName: String) { val newName = postfixExistingFileNames(inputFileName, setOf(inputFileName)) assertEquals(newName, expectedFileName) From 403d8322a5a69afb7b513269c92d0157eaf8c0fd Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 16:22:03 +0200 Subject: [PATCH 13/18] Inject TransferFileUtils using Hilt --- .../ui/screen/newtransfer/NewTransferViewModel.kt | 7 ++----- .../ui/screen/newtransfer/TransferFileUtils.kt | 10 +++++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index d8092856d..d6db1d821 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -17,20 +17,17 @@ */ package com.infomaniak.swisstransfer.ui.screen.newtransfer -import android.content.Context import android.net.Uri import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.infomaniak.swisstransfer.ui.screen.newtransfer.TransferFileUtils.getTransferFile import dagger.hilt.android.lifecycle.HiltViewModel -import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel -class NewTransferViewModel @Inject constructor(@ApplicationContext private val appContext: Context) : ViewModel() { +class NewTransferViewModel @Inject constructor(private val transferFileUtils: TransferFileUtils) : ViewModel() { val transferFiles = MutableStateFlow>(emptyList()) val failedTransferFileCount = MutableSharedFlow() @@ -41,7 +38,7 @@ class NewTransferViewModel @Inject constructor(@ApplicationContext private val a var failedFileCount = 0 uris.forEach { uri -> - appContext.getTransferFile(uri, alreadyUsedFileNames)?.let { transferFile -> + transferFileUtils.getTransferFile(uri, alreadyUsedFileNames)?.let { transferFile -> transferFiles.value += transferFile } ?: run { failedFileCount++ diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt index 3f00a3fa3..93f284ac3 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt @@ -23,10 +23,14 @@ import android.database.Cursor import android.net.Uri import android.provider.OpenableColumns import com.infomaniak.swisstransfer.ui.utils.FileNameUtils +import dagger.hilt.android.qualifiers.ApplicationContext +import javax.inject.Inject +import javax.inject.Singleton -object TransferFileUtils { - fun Context.getTransferFile(uri: Uri, alreadyUsedFileNames: Set): TransferFile? { - val contentResolver: ContentResolver = contentResolver +@Singleton +class TransferFileUtils @Inject constructor(@ApplicationContext private val appContext: Context) { + fun getTransferFile(uri: Uri, alreadyUsedFileNames: Set): TransferFile? { + val contentResolver: ContentResolver = appContext.contentResolver val cursor: Cursor? = contentResolver.query(uri, null, null, null, null) return cursor?.getFileNameAndSize()?.let { (name, size) -> From 5458490b29bb4b784f6afcf262f501ac2111dafa Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Thu, 3 Oct 2024 16:34:23 +0200 Subject: [PATCH 14/18] Move TransferFile creation logic based on Uri from view model to utils --- .../screen/newtransfer/NewTransferViewModel.kt | 12 +++--------- .../ui/screen/newtransfer/TransferFileUtils.kt | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index d6db1d821..74a05b321 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -36,16 +36,10 @@ class NewTransferViewModel @Inject constructor(private val transferFileUtils: Tr viewModelScope.launch { val alreadyUsedFileNames = buildSet { transferFiles.value.forEach { add(it.fileName) } } - var failedFileCount = 0 - uris.forEach { uri -> - transferFileUtils.getTransferFile(uri, alreadyUsedFileNames)?.let { transferFile -> - transferFiles.value += transferFile - } ?: run { - failedFileCount++ - } - } + val newTransferFiles = transferFileUtils.getTransferFiles(uris, alreadyUsedFileNames) - failedTransferFileCount.emit(failedFileCount) + transferFiles.value += newTransferFiles + failedTransferFileCount.emit(uris.count() - newTransferFiles.count()) } } } diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt index 93f284ac3..5f5051dc0 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt @@ -29,7 +29,21 @@ import javax.inject.Singleton @Singleton class TransferFileUtils @Inject constructor(@ApplicationContext private val appContext: Context) { - fun getTransferFile(uri: Uri, alreadyUsedFileNames: Set): TransferFile? { + fun getTransferFiles(uris: List, alreadyUsedFileNames: Set): MutableSet { + val currentUsedFileNames = alreadyUsedFileNames.toMutableSet() + val transferFiles = mutableSetOf() + + uris.forEach { uri -> + getTransferFile(uri, currentUsedFileNames)?.let { transferFile -> + currentUsedFileNames += transferFile.fileName + transferFiles += transferFile + } + } + + return transferFiles + } + + private fun getTransferFile(uri: Uri, alreadyUsedFileNames: Set): TransferFile? { val contentResolver: ContentResolver = appContext.contentResolver val cursor: Cursor? = contentResolver.query(uri, null, null, null, null) From dc3538562ad330beef0c069964da3a88519c4cfb Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Mon, 7 Oct 2024 13:57:29 +0200 Subject: [PATCH 15/18] Only expose non mutable flow --- .../ui/screen/newtransfer/NewTransferViewModel.kt | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 74a05b321..5ddc3445e 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -23,14 +23,19 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel class NewTransferViewModel @Inject constructor(private val transferFileUtils: TransferFileUtils) : ViewModel() { - val transferFiles = MutableStateFlow>(emptyList()) - val failedTransferFileCount = MutableSharedFlow() + private val _transferFiles = MutableStateFlow>(emptyList()) + val transferFiles: StateFlow> = _transferFiles + + private val _failedTransferFileCount = MutableSharedFlow() + val failedTransferFileCount: SharedFlow = _failedTransferFileCount fun addFiles(uris: List) { viewModelScope.launch { @@ -38,8 +43,8 @@ class NewTransferViewModel @Inject constructor(private val transferFileUtils: Tr val newTransferFiles = transferFileUtils.getTransferFiles(uris, alreadyUsedFileNames) - transferFiles.value += newTransferFiles - failedTransferFileCount.emit(uris.count() - newTransferFiles.count()) + _transferFiles.value += newTransferFiles + _failedTransferFileCount.emit(uris.count() - newTransferFiles.count()) } } } From 2754e914bfacf1d9dbe3eaec144f24fec60ce86b Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Mon, 7 Oct 2024 14:46:17 +0200 Subject: [PATCH 16/18] Rename TransferFileUtils into TransferFilesManager --- .../ui/screen/newtransfer/NewTransferViewModel.kt | 4 ++-- .../{TransferFileUtils.kt => TransferFilesManager.kt} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/{TransferFileUtils.kt => TransferFilesManager.kt} (96%) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt index 5ddc3445e..bc9fecf33 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/NewTransferViewModel.kt @@ -29,7 +29,7 @@ import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel -class NewTransferViewModel @Inject constructor(private val transferFileUtils: TransferFileUtils) : ViewModel() { +class NewTransferViewModel @Inject constructor(private val transferFilesManager: TransferFilesManager) : ViewModel() { private val _transferFiles = MutableStateFlow>(emptyList()) val transferFiles: StateFlow> = _transferFiles @@ -41,7 +41,7 @@ class NewTransferViewModel @Inject constructor(private val transferFileUtils: Tr viewModelScope.launch { val alreadyUsedFileNames = buildSet { transferFiles.value.forEach { add(it.fileName) } } - val newTransferFiles = transferFileUtils.getTransferFiles(uris, alreadyUsedFileNames) + val newTransferFiles = transferFilesManager.getTransferFiles(uris, alreadyUsedFileNames) _transferFiles.value += newTransferFiles _failedTransferFileCount.emit(uris.count() - newTransferFiles.count()) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt similarity index 96% rename from app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt rename to app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt index 5f5051dc0..0d545a769 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFileUtils.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt @@ -28,7 +28,7 @@ import javax.inject.Inject import javax.inject.Singleton @Singleton -class TransferFileUtils @Inject constructor(@ApplicationContext private val appContext: Context) { +class TransferFilesManager @Inject constructor(@ApplicationContext private val appContext: Context) { fun getTransferFiles(uris: List, alreadyUsedFileNames: Set): MutableSet { val currentUsedFileNames = alreadyUsedFileNames.toMutableSet() val transferFiles = mutableSetOf() From 5c27afd80efd4a044fa8b15486ec517f9d76bfef Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Mon, 7 Oct 2024 16:24:32 +0200 Subject: [PATCH 17/18] Gain a level of indent --- .../newtransfer/TransferFilesManager.kt | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt index 0d545a769..71ee71b60 100644 --- a/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt +++ b/app/src/main/java/com/infomaniak/swisstransfer/ui/screen/newtransfer/TransferFilesManager.kt @@ -53,19 +53,17 @@ class TransferFilesManager @Inject constructor(@ApplicationContext private val a } } - private fun Cursor.getFileNameAndSize(): Pair? { - use { - if (it.moveToFirst()) { - val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return null - val fileName = it.getString(displayNameColumnIndex) + private fun Cursor.getFileNameAndSize(): Pair? = use { + if (it.moveToFirst()) { + val displayNameColumnIndex = it.getColumnIndexOrNull(OpenableColumns.DISPLAY_NAME) ?: return null + val fileName = it.getString(displayNameColumnIndex) - val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return null - val fileSize = it.getLong(fileSizeColumnIndex) + val fileSizeColumnIndex = it.getColumnIndexOrNull(OpenableColumns.SIZE) ?: return null + val fileSize = it.getLong(fileSizeColumnIndex) - return fileName to fileSize - } else { - return null - } + fileName to fileSize + } else { + null } } From ba3df97b02837ebe2713b0bcbb7ac90ce1e1ae49 Mon Sep 17 00:00:00 2001 From: Gibran Chevalley Date: Mon, 7 Oct 2024 16:31:09 +0200 Subject: [PATCH 18/18] Avoid companion being instantiated if it doesn't expose anything --- .../com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt index 43bba2161..6ad50852d 100644 --- a/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt +++ b/app/src/test/java/com/infomaniak/swisstransfer/PostifxedFileNameUnitTest.kt @@ -92,8 +92,8 @@ class PostifxedFileNameUnitTest { assertEquals(newName, inputFileName) } - companion object { + private companion object { // Used for tests that require to check existing filenames among multiple edge case already existing filenames - private val alreadyUsedFileNames = setOf("test.txt", "test(1).txt", "test(2).txt", "hello.txt") + val alreadyUsedFileNames = setOf("test.txt", "test(1).txt", "test(2).txt", "hello.txt") } }