From e2cf9104d7eeb59ff913811060a0cedc2316454d Mon Sep 17 00:00:00 2001 From: kl3jvi Date: Thu, 17 Oct 2024 21:50:20 +0200 Subject: [PATCH 1/7] fix: Ensure proper resource management by using .use for file operations --- .../wire/android/di/ViewModelScopedPreview.kt | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt b/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt index deb7b367c5e..800e50b73da 100644 --- a/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt +++ b/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt @@ -27,6 +27,8 @@ import com.google.devtools.ksp.processing.SymbolProcessorProvider import com.google.devtools.ksp.symbol.ClassKind import com.google.devtools.ksp.symbol.KSAnnotated import com.google.devtools.ksp.symbol.KSClassDeclaration +import com.google.devtools.ksp.symbol.KSFunctionDeclaration +import com.google.devtools.ksp.symbol.KSPropertyDeclaration import com.google.devtools.ksp.validate import java.io.OutputStream @@ -42,26 +44,21 @@ internal class ViewModelScopedPreviewProcessor(private val codeGenerator: CodeGe .filterIsInstance() .toList() if (!viewModelScopedPreviews.iterator().hasNext()) return emptyList() - viewModelScopedPreviews.forEach { - if (it.classKind != ClassKind.INTERFACE) { - throw IllegalArgumentException( + viewModelScopedPreviews.forEach { preview -> + require(preview.classKind == ClassKind.INTERFACE) { "ViewModelScopedPreview can only be applied to interfaces, " + - "but ${it.qualifiedName?.asString()} is a ${it.classKind}" - ) + "but ${preview.qualifiedName?.asString()} is a ${preview.classKind}" } - if (it.getAllFunctions().any { it.isAbstract }) { - throw IllegalArgumentException( + require(!preview.getAllFunctions().any(KSFunctionDeclaration::isAbstract)) { "ViewModelScopedPreview can only be applied to interfaces with default implementations, " + - "but ${it.qualifiedName?.asString()} is abstract" - ) + "but ${preview.qualifiedName?.asString()} is abstract" + } - if (it.getAllProperties().any { it.isAbstract() }) { - throw IllegalArgumentException( + require(!preview.getAllProperties().any(KSPropertyDeclaration::isAbstract)) { "ViewModelScopedPreview can only be applied to interfaces with default implementations, " + - "but ${it.qualifiedName?.asString()} is abstract" - ) + "but ${preview.qualifiedName?.asString()} is abstract" } - createObjectFile(it) + createObjectFile(preview) } createListFile(viewModelScopedPreviews) return (viewModelScopedPreviews).filterNot { it.validate() }.toList() @@ -76,9 +73,8 @@ internal class ViewModelScopedPreviewProcessor(private val codeGenerator: CodeGe "import ${item.qualifiedName?.asString()}\n\n" + "data object $name : ${item.simpleName.asString()}" val dependencies = Dependencies(aggregating = true, *listOfNotNull(item.containingFile).toTypedArray()) - val file: OutputStream = codeGenerator.createNewFile(dependencies, packageName, name, "kt") - file.write(content.toByteArray()) - file.close() + codeGenerator.createNewFile(dependencies, packageName, name, "kt") + .use { it.write(content.toByteArray()) } } private fun createListFile(items: List) { From d04ae831a9cc67afa77e128dce249b7fde5bf582 Mon Sep 17 00:00:00 2001 From: Klejvi Kapaj <40796367+kl3jvi@users.noreply.github.com> Date: Wed, 23 Oct 2024 09:35:43 +0200 Subject: [PATCH 2/7] fix: Ensure proper resource management by using .use for file operations --- .../kotlin/com/wire/android/di/ViewModelScopedPreview.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt b/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt index 800e50b73da..49e859a1993 100644 --- a/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt +++ b/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt @@ -30,7 +30,6 @@ import com.google.devtools.ksp.symbol.KSClassDeclaration import com.google.devtools.ksp.symbol.KSFunctionDeclaration import com.google.devtools.ksp.symbol.KSPropertyDeclaration import com.google.devtools.ksp.validate -import java.io.OutputStream class ViewModelScopedPreviewProcessorProvider : SymbolProcessorProvider { override fun create(environment: SymbolProcessorEnvironment): SymbolProcessor = @@ -85,8 +84,7 @@ internal class ViewModelScopedPreviewProcessor(private val codeGenerator: CodeGe items.joinToString("\n") { "import ${it.packageName.asString()}.${it.previewName()}" } + "\n\n" + "val $name = listOf(\n\t" + items.joinToString(",\n\t") { it.previewName() } + "\n)" val dependencies = Dependencies(aggregating = true, *items.mapNotNull { it.containingFile }.toTypedArray()) - val file: OutputStream = codeGenerator.createNewFile(dependencies, packageName, name, "kt") - file.write(content.toByteArray()) - file.close() + codeGenerator.createNewFile(dependencies, packageName, name, "kt") + .use { it.write(content.toByteArray()) } } } From cb2218f26e4bd61f8c1fa276614e68b62e9c38ef Mon Sep 17 00:00:00 2001 From: kl3jvi Date: Fri, 25 Oct 2024 11:39:36 +0200 Subject: [PATCH 3/7] fix: Ensure proper resource management by using .use for file operations --- .../main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt b/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt index 49e859a1993..c7b18043658 100644 --- a/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt +++ b/ksp/src/main/kotlin/com/wire/android/di/ViewModelScopedPreview.kt @@ -51,7 +51,6 @@ internal class ViewModelScopedPreviewProcessor(private val codeGenerator: CodeGe require(!preview.getAllFunctions().any(KSFunctionDeclaration::isAbstract)) { "ViewModelScopedPreview can only be applied to interfaces with default implementations, " + "but ${preview.qualifiedName?.asString()} is abstract" - } require(!preview.getAllProperties().any(KSPropertyDeclaration::isAbstract)) { "ViewModelScopedPreview can only be applied to interfaces with default implementations, " + From 1e10eb8750b724f8a972aa43692ed3cea037f64e Mon Sep 17 00:00:00 2001 From: Klejvi Kapaj <40796367+kl3jvi@users.noreply.github.com> Date: Thu, 7 Nov 2024 19:43:12 +0100 Subject: [PATCH 4/7] refactor: AudioMediaRecorder to improve readability and resource management --- .../recordaudio/AudioMediaRecorder.kt | 336 +++++++++--------- 1 file changed, 161 insertions(+), 175 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt index 292b63b77c1..ca6344a67f9 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt @@ -121,19 +121,21 @@ class AudioMediaRecorder @Inject constructor( val blockAlign = channels * (bitsPerSample / BITS_PER_BYTE) // We use buffer() to correctly write the string values. - bufferedSink.writeUtf8(CHUNK_ID_RIFF) // Chunk ID - bufferedSink.writeIntLe(PLACEHOLDER_SIZE) // Placeholder for Chunk Size (will be updated later) - bufferedSink.writeUtf8(FORMAT_WAVE) // Format - bufferedSink.writeUtf8(SUBCHUNK1_ID_FMT) // Subchunk1 ID - bufferedSink.writeIntLe(SUBCHUNK1_SIZE_PCM) // Subchunk1 Size (PCM) - bufferedSink.writeShortLe(AUDIO_FORMAT_PCM) // Audio Format (PCM) - bufferedSink.writeShortLe(channels) // Number of Channels - bufferedSink.writeIntLe(sampleRate) // Sample Rate - bufferedSink.writeIntLe(byteRate) // Byte Rate - bufferedSink.writeShortLe(blockAlign) // Block Align - bufferedSink.writeShortLe(bitsPerSample) // Bits Per Sample - bufferedSink.writeUtf8(SUBCHUNK2_ID_DATA) // Subchunk2 ID - bufferedSink.writeIntLe(PLACEHOLDER_SIZE) // Placeholder for Subchunk2 Size (will be updated later) + with(bufferedSink) { + writeUtf8(CHUNK_ID_RIFF) // Chunk ID + writeIntLe(PLACEHOLDER_SIZE) // Placeholder for Chunk Size (will be updated later) + writeUtf8(FORMAT_WAVE) // Format + writeUtf8(SUBCHUNK1_ID_FMT) // Subchunk1 ID + writeIntLe(SUBCHUNK1_SIZE_PCM) // Subchunk1 Size (PCM) + writeShortLe(AUDIO_FORMAT_PCM) // Audio Format (PCM) + writeShortLe(channels) // Number of Channels + writeIntLe(sampleRate) // Sample Rate + writeIntLe(byteRate) // Byte Rate + writeShortLe(blockAlign) // Block Align + writeShortLe(bitsPerSample) // Bits Per Sample + writeUtf8(SUBCHUNK2_ID_DATA) // Subchunk2 ID + writeIntLe(PLACEHOLDER_SIZE) // Placeholder for Subchunk2 Size (will be updated later) + } } private fun updateWavHeader(filePath: Path) { @@ -149,17 +151,14 @@ class AudioMediaRecorder @Inject constructor( dataSizeBuffer.order(ByteOrder.LITTLE_ENDIAN) dataSizeBuffer.putInt(dataSize) - val randomAccessFile = java.io.RandomAccessFile(file, "rw") - - // Update Chunk Size - randomAccessFile.seek(CHUNK_SIZE_OFFSET.toLong()) - randomAccessFile.write(chunkSizeBuffer.array()) - - // Update Subchunk2 Size - randomAccessFile.seek(SUBCHUNK2_SIZE_OFFSET.toLong()) - randomAccessFile.write(dataSizeBuffer.array()) - - randomAccessFile.close() + java.io.RandomAccessFile(file, "rw").use { randomAccessFile -> + // Update Chunk Size + randomAccessFile.seek(CHUNK_SIZE_OFFSET.toLong()) + randomAccessFile.write(chunkSizeBuffer.array()) + // Update Subchunk2 Size + randomAccessFile.seek(SUBCHUNK2_SIZE_OFFSET.toLong()) + randomAccessFile.write(dataSizeBuffer.array()) + } appLogger.i("Updated WAV Header: Chunk Size = ${fileSize - CHUNK_ID_SIZE}, Data Size = $dataSize") } @@ -175,48 +174,41 @@ class AudioMediaRecorder @Inject constructor( audioRecorder = null } + @Suppress("NestedBlockDepth") private fun writeAudioDataToFile() { val data = ByteArray(BUFFER_SIZE) - var sink: okio.Sink? = null try { - sink = kaliumFileSystem.sink(originalOutputPath!!) - val bufferedSink = sink.buffer() - - // Write WAV header - writeWavHeader(bufferedSink, SAMPLING_RATE, AUDIO_CHANNELS, BITS_PER_SAMPLE) - - while (isRecording) { - val read = audioRecorder?.read(data, 0, BUFFER_SIZE) ?: 0 - if (read > 0) { - bufferedSink.write(data, 0, read) - } + kaliumFileSystem.sink(originalOutputPath!!).use { sink -> + sink.buffer() + .use { + writeWavHeader(it, SAMPLING_RATE, AUDIO_CHANNELS, BITS_PER_SAMPLE) + while (isRecording) { + val read = audioRecorder?.read(data, 0, BUFFER_SIZE) ?: 0 + if (read > 0) { + it.write(data, 0, read) + } - // Check if the file size exceeds the limit - val currentSize = originalOutputPath!!.toFile().length() - if (currentSize > (assetLimitInMB * SIZE_OF_1MB)) { - isRecording = false - scope.launch { - _maxFileSizeReached.emit( - RecordAudioDialogState.MaxFileSizeReached( - maxSize = assetLimitInMB / SIZE_OF_1MB - ) - ) + // Check if the file size exceeds the limit + val currentSize = originalOutputPath!!.toFile().length() + if (currentSize > (assetLimitInMB * SIZE_OF_1MB)) { + isRecording = false + scope.launch { + _maxFileSizeReached.emit( + RecordAudioDialogState.MaxFileSizeReached( + maxSize = assetLimitInMB / SIZE_OF_1MB + ) + ) + } + break + } + } + updateWavHeader(originalOutputPath!!) } - break - } } - - // Close buffer to ensure all data is written - bufferedSink.close() - - // Update WAV header with final file size - updateWavHeader(originalOutputPath!!) } catch (e: IOException) { e.printStackTrace() appLogger.e("[RecordAudio] writeAudioDataToFile: IOException - ${e.message}") - } finally { - sink?.close() } } @@ -224,147 +216,141 @@ class AudioMediaRecorder @Inject constructor( suspend fun convertWavToMp4(inputFilePath: String): Boolean = withContext(Dispatchers.IO) { var codec: MediaCodec? = null var muxer: MediaMuxer? = null - var fileInputStream: FileInputStream? = null - var parcelFileDescriptor: ParcelFileDescriptor? = null - var success = true try { - val inputFile = File(inputFilePath) - fileInputStream = FileInputStream(inputFile) - - val outputFile = mp4OutputPath?.toFile() - parcelFileDescriptor = ParcelFileDescriptor.open( - outputFile, - ParcelFileDescriptor.MODE_READ_WRITE or ParcelFileDescriptor.MODE_CREATE - ) - - val mediaExtractor = MediaExtractor() - mediaExtractor.setDataSource(inputFilePath) - - val format = MediaFormat.createAudioFormat( - MediaFormat.MIMETYPE_AUDIO_AAC, - SAMPLING_RATE, - AUDIO_CHANNELS - ) - format.setInteger(MediaFormat.KEY_BIT_RATE, BIT_RATE) - format.setInteger(MediaFormat.KEY_AAC_PROFILE, MediaCodecInfo.CodecProfileLevel.AACObjectLC) - - codec = MediaCodec.createEncoderByType(MediaFormat.MIMETYPE_AUDIO_AAC) - codec.configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE) - codec.start() - - val bufferInfo = MediaCodec.BufferInfo() - muxer = MediaMuxer(parcelFileDescriptor.fileDescriptor, MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4) - var trackIndex = -1 - var sawInputEOS = false - var sawOutputEOS = false - - var retryCount = 0 - var presentationTimeUs = 0L - val bytesPerSample = (BITS_PER_SAMPLE / BITS_PER_BYTE) * AUDIO_CHANNELS - - while (!sawOutputEOS && retryCount < MAX_RETRY_COUNT) { - if (!sawInputEOS) { - val inputBufferIndex = codec.dequeueInputBuffer(TIMEOUT_US) - if (inputBufferIndex >= 0) { - val inputBuffer = codec.getInputBuffer(inputBufferIndex) - inputBuffer?.clear() - - val sampleSize = fileInputStream.channel.read(inputBuffer!!) - if (sampleSize < 0) { - codec.queueInputBuffer(inputBufferIndex, 0, 0, 0, MediaCodec.BUFFER_FLAG_END_OF_STREAM) - sawInputEOS = true - } else { - val numSamples = sampleSize / bytesPerSample - val bufferDurationUs = (numSamples * MICROSECONDS_PER_SECOND) / SAMPLING_RATE - codec.queueInputBuffer(inputBufferIndex, 0, sampleSize, presentationTimeUs, 0) - - presentationTimeUs += bufferDurationUs - } - } - } - - val outputBufferIndex = codec.dequeueOutputBuffer(bufferInfo, TIMEOUT_US) - - when { - outputBufferIndex == MediaCodec.INFO_OUTPUT_FORMAT_CHANGED -> { - val newFormat = codec.outputFormat - trackIndex = muxer.addTrack(newFormat) - muxer.start() - retryCount = 0 - } - - outputBufferIndex >= 0 -> { - val outputBuffer = codec.getOutputBuffer(outputBufferIndex) - - if (bufferInfo.flags and MediaCodec.BUFFER_FLAG_CODEC_CONFIG != 0) { - bufferInfo.size = 0 - } - - if (bufferInfo.size != 0 && outputBuffer != null) { - outputBuffer.position(bufferInfo.offset) - outputBuffer.limit(bufferInfo.offset + bufferInfo.size) + FileInputStream(File(inputFilePath)).use { fileInputStream -> + mp4OutputPath?.toFile()?.let { outputFile -> + ParcelFileDescriptor.open( + outputFile, + ParcelFileDescriptor.MODE_READ_WRITE or ParcelFileDescriptor.MODE_CREATE + ).use { parcelFileDescriptor -> + + val mediaExtractor = MediaExtractor() + mediaExtractor.setDataSource(inputFilePath) + + val format = MediaFormat.createAudioFormat( + MediaFormat.MIMETYPE_AUDIO_AAC, + SAMPLING_RATE, + AUDIO_CHANNELS + ) + format.setInteger(MediaFormat.KEY_BIT_RATE, BIT_RATE) + format.setInteger(MediaFormat.KEY_AAC_PROFILE, MediaCodecInfo.CodecProfileLevel.AACObjectLC) + + codec = MediaCodec.createEncoderByType(MediaFormat.MIMETYPE_AUDIO_AAC) + val mediaCodec = codec!! + mediaCodec.configure(format, null, null, MediaCodec.CONFIGURE_FLAG_ENCODE) + mediaCodec.start() + + val bufferInfo = MediaCodec.BufferInfo() + muxer = MediaMuxer(parcelFileDescriptor.fileDescriptor, MediaMuxer.OutputFormat.MUXER_OUTPUT_MPEG_4) + val mediaMuxer = muxer!! + + var trackIndex = -1 + var sawInputEOS = false + var sawOutputEOS = false + + var retryCount = 0 + var presentationTimeUs = 0L + val bytesPerSample = (BITS_PER_SAMPLE / BITS_PER_BYTE) * AUDIO_CHANNELS + + while (!sawOutputEOS && retryCount < MAX_RETRY_COUNT) { + if (!sawInputEOS) { + val inputBufferIndex = mediaCodec.dequeueInputBuffer(TIMEOUT_US) + if (inputBufferIndex >= 0) { + val inputBuffer = mediaCodec.getInputBuffer(inputBufferIndex) + inputBuffer?.clear() + + val sampleSize = fileInputStream.channel.read(inputBuffer!!) + if (sampleSize < 0) { + mediaCodec.queueInputBuffer(inputBufferIndex, 0, 0, 0, MediaCodec.BUFFER_FLAG_END_OF_STREAM) + sawInputEOS = true + } else { + val numSamples = sampleSize / bytesPerSample + val bufferDurationUs = (numSamples * MICROSECONDS_PER_SECOND) / SAMPLING_RATE + mediaCodec.queueInputBuffer(inputBufferIndex, 0, sampleSize, presentationTimeUs, 0) + + presentationTimeUs += bufferDurationUs + } + } + } - if (trackIndex >= 0) { - muxer.writeSampleData(trackIndex, outputBuffer, bufferInfo) - } else { - appLogger.e("Track index is not set. Skipping writeSampleData.") + val outputBufferIndex = mediaCodec.dequeueOutputBuffer(bufferInfo, TIMEOUT_US) + + when { + outputBufferIndex == MediaCodec.INFO_OUTPUT_FORMAT_CHANGED -> { + val newFormat = mediaCodec.outputFormat + trackIndex = mediaMuxer.addTrack(newFormat) + mediaMuxer.start() + retryCount = 0 + } + + outputBufferIndex >= 0 -> { + val outputBuffer = mediaCodec.getOutputBuffer(outputBufferIndex) + + if (bufferInfo.flags and MediaCodec.BUFFER_FLAG_CODEC_CONFIG != 0) { + bufferInfo.size = 0 + } + + if (bufferInfo.size != 0 && outputBuffer != null) { + outputBuffer.position(bufferInfo.offset) + outputBuffer.limit(bufferInfo.offset + bufferInfo.size) + + if (trackIndex >= 0) { + mediaMuxer.writeSampleData(trackIndex, outputBuffer, bufferInfo) + } else { + appLogger.e("Track index is not set. Skipping writeSampleData.") + } + } + + mediaCodec.releaseOutputBuffer(outputBufferIndex, false) + retryCount = 0 + + if (bufferInfo.flags and MediaCodec.BUFFER_FLAG_END_OF_STREAM != 0) { + sawOutputEOS = true + } + } + + outputBufferIndex == MediaCodec.INFO_TRY_AGAIN_LATER -> { + retryCount++ + delay(RETRY_DELAY_IN_MILLIS) + } } } - - codec.releaseOutputBuffer(outputBufferIndex, false) - retryCount = 0 - - if (bufferInfo.flags and MediaCodec.BUFFER_FLAG_END_OF_STREAM != 0) { - sawOutputEOS = true + if (retryCount >= MAX_RETRY_COUNT) { + appLogger.e("Reached maximum retries without receiving output from codec.") + return@withContext false } } - - outputBufferIndex == MediaCodec.INFO_TRY_AGAIN_LATER -> { - retryCount++ - delay(RETRY_DELAY_IN_MILLIS) - } + } ?: run { + appLogger.e("[RecordAudio] convertWavToMp4: mp4OutputPath is null") + return@withContext false } } - if (retryCount >= MAX_RETRY_COUNT) { - appLogger.e("Reached maximum retries without receiving output from codec.") - success = false - } } catch (e: Exception) { appLogger.e("Could not convert wav to mp4: ${e.message}", throwable = e) - success = false + return@withContext false } finally { try { - fileInputStream?.close() - } catch (e: Exception) { - appLogger.e("Could not close FileInputStream: ${e.message}", throwable = e) - success = false - } - - try { - muxer?.stop() - muxer?.release() + muxer?.let { safeMuxer -> + safeMuxer.stop() + safeMuxer.release() + } } catch (e: Exception) { appLogger.e("Could not stop or release MediaMuxer: ${e.message}", throwable = e) - success = false + return@withContext false } try { - codec?.stop() - codec?.release() + codec?.let { safeCodec -> + safeCodec.stop() + safeCodec.release() + } } catch (e: Exception) { appLogger.e("Could not stop or release MediaCodec: ${e.message}", throwable = e) - success = false - } - - try { - parcelFileDescriptor?.close() - } catch (e: Exception) { - appLogger.e("Could not close ParcelFileDescriptor: ${e.message}", throwable = e) - success = false + return@withContext false } } - success + return@withContext true } companion object { From 36a60c4857260c84c66d46bf9b5ea5e666d9759c Mon Sep 17 00:00:00 2001 From: Klejvi Kapaj <40796367+kl3jvi@users.noreply.github.com> Date: Thu, 7 Nov 2024 20:38:57 +0100 Subject: [PATCH 5/7] refactor: AudioMediaRecorder to improve readability and resource management --- .../recordaudio/AudioMediaRecorder.kt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt index ca6344a67f9..71e2a9fc5ca 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/messagecomposer/recordaudio/AudioMediaRecorder.kt @@ -48,6 +48,7 @@ import okio.buffer import java.io.File import java.io.FileInputStream import java.io.IOException +import java.io.RandomAccessFile import java.nio.ByteBuffer import java.nio.ByteOrder import javax.inject.Inject @@ -151,7 +152,7 @@ class AudioMediaRecorder @Inject constructor( dataSizeBuffer.order(ByteOrder.LITTLE_ENDIAN) dataSizeBuffer.putInt(dataSize) - java.io.RandomAccessFile(file, "rw").use { randomAccessFile -> + RandomAccessFile(file, "rw").use { randomAccessFile -> // Update Chunk Size randomAccessFile.seek(CHUNK_SIZE_OFFSET.toLong()) randomAccessFile.write(chunkSizeBuffer.array()) @@ -216,6 +217,7 @@ class AudioMediaRecorder @Inject constructor( suspend fun convertWavToMp4(inputFilePath: String): Boolean = withContext(Dispatchers.IO) { var codec: MediaCodec? = null var muxer: MediaMuxer? = null + var success = true try { FileInputStream(File(inputFilePath)).use { fileInputStream -> @@ -318,17 +320,16 @@ class AudioMediaRecorder @Inject constructor( } if (retryCount >= MAX_RETRY_COUNT) { appLogger.e("Reached maximum retries without receiving output from codec.") - return@withContext false + success = false } } } ?: run { appLogger.e("[RecordAudio] convertWavToMp4: mp4OutputPath is null") - return@withContext false + success = false } } } catch (e: Exception) { appLogger.e("Could not convert wav to mp4: ${e.message}", throwable = e) - return@withContext false } finally { try { muxer?.let { safeMuxer -> @@ -337,7 +338,7 @@ class AudioMediaRecorder @Inject constructor( } } catch (e: Exception) { appLogger.e("Could not stop or release MediaMuxer: ${e.message}", throwable = e) - return@withContext false + success = false } try { @@ -347,10 +348,10 @@ class AudioMediaRecorder @Inject constructor( } } catch (e: Exception) { appLogger.e("Could not stop or release MediaCodec: ${e.message}", throwable = e) - return@withContext false + success = false } } - return@withContext true + success } companion object { From eb153c59b2b4c724cbe757b194736a6faab8ed0b Mon Sep 17 00:00:00 2001 From: kl3jvi Date: Sat, 23 Nov 2024 23:48:10 +0100 Subject: [PATCH 6/7] fix: backup and restore progress updates in a more meaningful way. --- .../backup/BackupAndRestoreViewModel.kt | 26 ++-- .../android/framework/FakeKaliumFileSystem.kt | 1 + .../home/BackupAndRestoreViewModelTest.kt | 127 +++++++++--------- 3 files changed, 76 insertions(+), 78 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/ui/home/settings/backup/BackupAndRestoreViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/home/settings/backup/BackupAndRestoreViewModel.kt index 357a47fb1d5..7b690fd2990 100644 --- a/app/src/main/kotlin/com/wire/android/ui/home/settings/backup/BackupAndRestoreViewModel.kt +++ b/app/src/main/kotlin/com/wire/android/ui/home/settings/backup/BackupAndRestoreViewModel.kt @@ -104,21 +104,11 @@ class BackupAndRestoreViewModel } fun createBackup() = viewModelScope.launch { - // TODO: Find a way to update the creation progress more faithfully. For now we will just show this small delays to mimic the - // progress also for small backups - updateCreationProgress(PROGRESS_25) - delay(SMALL_DELAY) - updateCreationProgress(PROGRESS_50) - delay(SMALL_DELAY) - - when (val result = createBackupFile(createBackupPasswordState.text.toString())) { + when (val result = createBackupFile(createBackupPasswordState.text.toString(), ::updateCreationProgress)) { is CreateBackupResult.Success -> { state = state.copy(backupCreationProgress = BackupCreationProgress.Finished(result.backupFileName)) latestCreatedBackup = BackupAndRestoreState.CreatedBackup( - result.backupFilePath, - result.backupFileName, - result.backupFileSize, - createBackupPasswordState.text.isNotEmpty() + result.backupFilePath, result.backupFileName, result.backupFileSize, createBackupPasswordState.text.isNotEmpty() ) createBackupPasswordState.clearText() } @@ -223,8 +213,7 @@ class BackupAndRestoreViewModel is RestoreBackupResult.Failure -> { appLogger.e("Error when restoring the backup db file caused by: ${result.failure.cause}") state = state.copy( - restoreFileValidation = RestoreFileValidation.IncompatibleBackup, - backupRestoreProgress = BackupRestoreProgress.Failed + restoreFileValidation = RestoreFileValidation.IncompatibleBackup, backupRestoreProgress = BackupRestoreProgress.Failed ) AnonymousAnalyticsManagerImpl.sendEvent(event = AnalyticsEvent.BackupRestoreFailed) } @@ -243,8 +232,7 @@ class BackupAndRestoreViewModel when (val result = importBackup(latestImportedBackupTempPath, restoreBackupPasswordState.text.toString())) { RestoreBackupResult.Success -> { state = state.copy( - backupRestoreProgress = BackupRestoreProgress.Finished, - restorePasswordValidation = PasswordValidation.Valid + backupRestoreProgress = BackupRestoreProgress.Finished, restorePasswordValidation = PasswordValidation.Valid ) restoreBackupPasswordState.clearText() AnonymousAnalyticsManagerImpl.sendEvent(event = AnalyticsEvent.BackupRestoreSucceeded) @@ -317,8 +305,10 @@ class BackupAndRestoreViewModel } } - private suspend fun updateCreationProgress(progress: Float) = withContext(dispatcher.main()) { - state = state.copy(backupCreationProgress = BackupCreationProgress.InProgress(progress)) + private fun updateCreationProgress(progress: Float) { + viewModelScope.launch(dispatcher.main()) { + state = state.copy(backupCreationProgress = BackupCreationProgress.InProgress(progress)) + } } internal companion object { diff --git a/app/src/test/kotlin/com/wire/android/framework/FakeKaliumFileSystem.kt b/app/src/test/kotlin/com/wire/android/framework/FakeKaliumFileSystem.kt index 71c4b1febe4..a6fc7ef8570 100644 --- a/app/src/test/kotlin/com/wire/android/framework/FakeKaliumFileSystem.kt +++ b/app/src/test/kotlin/com/wire/android/framework/FakeKaliumFileSystem.kt @@ -105,5 +105,6 @@ class FakeKaliumFileSystem( override fun selfUserAvatarPath(): Path = providePersistentAssetPath("self_user_avatar.jpg") override suspend fun listDirectories(dir: Path): List = fakeFileSystem.list(dir) + override fun fileSize(path: Path): Long = fakeFileSystem.metadata(path).size ?: 0 } diff --git a/app/src/test/kotlin/com/wire/android/ui/home/settings/home/BackupAndRestoreViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/home/settings/home/BackupAndRestoreViewModelTest.kt index 98e7efef6b0..7ebf0d1ebc7 100644 --- a/app/src/test/kotlin/com/wire/android/ui/home/settings/home/BackupAndRestoreViewModelTest.kt +++ b/app/src/test/kotlin/com/wire/android/ui/home/settings/home/BackupAndRestoreViewModelTest.kt @@ -94,10 +94,7 @@ class BackupAndRestoreViewModelTest { fun givenAnEmptyPassword_whenCreatingABackup_thenItCreatesItSuccessfully() = runTest { // Given val emptyPassword = "" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withValidPassword() - .withSuccessfulCreation(emptyPassword) - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withValidPassword().withSuccessfulCreation(emptyPassword).arrange() backupAndRestoreViewModel.createBackupPasswordState.setTextAndPlaceCursorAtEnd(emptyPassword) // When @@ -107,17 +104,14 @@ class BackupAndRestoreViewModelTest { // Then assert(backupAndRestoreViewModel.state.backupCreationProgress is BackupCreationProgress.Finished) assertFalse(backupAndRestoreViewModel.latestCreatedBackup?.isEncrypted!!) - coVerify(exactly = 1) { arrangement.createBackupFile(password = emptyPassword) } + coVerify(exactly = 1) { arrangement.createBackupFile(password = emptyPassword, any()) } } @Test fun givenANonEmptyPassword_whenCreatingABackup_thenItCreatesItSuccessfully() = runTest(dispatcher.default()) { // Given val password = "mayTh3ForceBeWIthYou" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withValidPassword() - .withSuccessfulCreation(password) - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withValidPassword().withSuccessfulCreation(password).arrange() backupAndRestoreViewModel.createBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -127,16 +121,14 @@ class BackupAndRestoreViewModelTest { // Then assertInstanceOf(BackupCreationProgress.Finished::class.java, backupAndRestoreViewModel.state.backupCreationProgress) assertTrue(backupAndRestoreViewModel.latestCreatedBackup?.isEncrypted!!) - coVerify(exactly = 1) { arrangement.createBackupFile(password = password) } + coVerify(exactly = 1) { arrangement.createBackupFile(password = password, any()) } } @Test fun givenAnEmptyPassword_whenValidating_thenItUpdatePasswordStateToValid() = runTest(dispatcher.default()) { // Given val password = "" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withInvalidPassword() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withInvalidPassword().arrange() // When backupAndRestoreViewModel.validateBackupCreationPassword(password) @@ -151,9 +143,7 @@ class BackupAndRestoreViewModelTest { fun givenANonEmptyPassword_whenItIsInvalid_thenItUpdatePasswordValidationState() = runTest(dispatcher.default()) { // Given val password = "mayTh3ForceBeWIthYou" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withInvalidPassword() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withInvalidPassword().arrange() // When backupAndRestoreViewModel.validateBackupCreationPassword(password) @@ -167,9 +157,7 @@ class BackupAndRestoreViewModelTest { fun givenANonEmptyPassword_whenItIsValid_thenItUpdatePasswordValidationState() = runTest(dispatcher.default()) { // Given val password = "mayTh3ForceBeWIthYou_" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withValidPassword() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withValidPassword().arrange() // When backupAndRestoreViewModel.validateBackupCreationPassword(password) @@ -183,10 +171,7 @@ class BackupAndRestoreViewModelTest { fun givenANonEmptyPassword_whenCreatingABackupWithAGivenError_thenItReturnsAFailure() = runTest { // Given val password = "mayTh3ForceBeWIthYou" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withValidPassword() - .withFailedCreation(password) - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withValidPassword().withFailedCreation(password).arrange() backupAndRestoreViewModel.createBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -196,16 +181,14 @@ class BackupAndRestoreViewModelTest { // Then assertEquals(backupAndRestoreViewModel.state.backupCreationProgress, BackupCreationProgress.Failed) assert(backupAndRestoreViewModel.latestCreatedBackup == null) - coVerify(exactly = 1) { arrangement.createBackupFile(password = password) } + coVerify(exactly = 1) { arrangement.createBackupFile(password = password, any()) } } @Test fun givenACreatedBackup_whenSharingIt_thenTheStateIsResetButKeepsTheLastBackupDate() = runTest { // Given val storedBackup = BackupAndRestoreState.CreatedBackup("backupFilePath".toPath(), "backupName.zip", 100L, true) - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withPreviouslyCreatedBackup(storedBackup) - .withUpdateLastBackupData() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withPreviouslyCreatedBackup(storedBackup).withUpdateLastBackupData() .arrange() // When @@ -290,9 +273,7 @@ class BackupAndRestoreViewModelTest { fun givenAStoredEncryptedBackup_whenChoosingIt_thenTheRequirePasswordDialogIsShown() = runTest(dispatcher.default()) { // Given val isBackupEncrypted = true - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withSuccessfulDBImport(isBackupEncrypted) - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withSuccessfulDBImport(isBackupEncrypted).arrange() val backupUri = "some-backup".toUri() // When @@ -310,9 +291,7 @@ class BackupAndRestoreViewModelTest { @Test fun givenAStoredBackup_whenThereIsAnErrorVerifyingItsEncryption_thenTheRightErrorDialogIsShown() = runTest(dispatcher.default()) { // Given - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withFailedBackupVerification() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withFailedBackupVerification().arrange() val backupUri = "some-backup".toUri() // When @@ -352,9 +331,7 @@ class BackupAndRestoreViewModelTest { fun givenARestoreDialogShown_whenDismissingIt_thenTheTempImportedBackupPathIsDeleted() = runTest(dispatcher.default()) { // Given val mockUri = "some-backup" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withSuccessfulDBImport(false) - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withSuccessfulDBImport(false).arrange() val backupUri = mockUri.toUri() // When @@ -377,10 +354,7 @@ class BackupAndRestoreViewModelTest { fun givenAPasswordEncryptedBackup_whenRestoringIt_thenTheCorrectSuccessDialogIsShown() = runTest(dispatcher.default()) { // Given val password = "some-password" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withSuccessfulBackupRestore() - .withRequestedPasswordDialog() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withSuccessfulBackupRestore().withRequestedPasswordDialog().arrange() backupAndRestoreViewModel.restoreBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -400,10 +374,8 @@ class BackupAndRestoreViewModelTest { fun givenAPasswordEncryptedBackup_whenRestoringWithWrongPassword_thenTheCorrectErrorDialogIsShown() = runTest(dispatcher.default()) { // Given val password = "some-password" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withFailedDBImport(Failure(InvalidPassword)) - .withRequestedPasswordDialog() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withFailedDBImport(Failure(InvalidPassword)) + .withRequestedPasswordDialog().arrange() backupAndRestoreViewModel.restoreBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -424,10 +396,8 @@ class BackupAndRestoreViewModelTest { runTest(dispatcher.default()) { // Given val password = "some-password" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withFailedDBImport(Failure(InvalidUserId)) - .withRequestedPasswordDialog() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withFailedDBImport(Failure(InvalidUserId)) + .withRequestedPasswordDialog().arrange() backupAndRestoreViewModel.restoreBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -447,10 +417,8 @@ class BackupAndRestoreViewModelTest { fun givenAPasswordEncryptedBackup_whenRestoringAnIncompatibleBackup_thenTheCorrectErrorDialogIsShown() = runTest(dispatcher.default()) { // Given val password = "some-password" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withFailedDBImport(Failure(IncompatibleBackup("old format backup"))) - .withRequestedPasswordDialog() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withFailedDBImport(Failure(IncompatibleBackup("old format backup"))) + .withRequestedPasswordDialog().arrange() backupAndRestoreViewModel.restoreBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -470,11 +438,8 @@ class BackupAndRestoreViewModelTest { fun givenAPasswordEncryptedBackup_whenRestoringABackupWithAnIOError_thenTheCorrectErrorDialogIsShown() = runTest(dispatcher.default()) { // Given val password = "some-password" - val (arrangement, backupAndRestoreViewModel) = Arrangement() - .withFailedDBImport(Failure(BackupIOFailure("IO error"))) - .withRequestedPasswordDialog() - .withValidPassword() - .arrange() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withFailedDBImport(Failure(BackupIOFailure("IO error"))) + .withRequestedPasswordDialog().withValidPassword().arrange() backupAndRestoreViewModel.restoreBackupPasswordState.setTextAndPlaceCursorAtEnd(password) // When @@ -490,6 +455,42 @@ class BackupAndRestoreViewModelTest { } } + @Test + fun givenBackupCreation_whenProgressUpdates_thenStateIsUpdatedCorrectly() = runTest { + // Given + val password = "blackAndRedFl4g" + val (arrangement, backupAndRestoreViewModel) = Arrangement().withValidPassword().withSuccessfulCreation(password).arrange() + backupAndRestoreViewModel.createBackupPasswordState.setTextAndPlaceCursorAtEnd(password) + + // When + backupAndRestoreViewModel.createBackup() + advanceUntilIdle() + + // Then + assert(backupAndRestoreViewModel.state.backupCreationProgress is BackupCreationProgress.Finished) + assertTrue(backupAndRestoreViewModel.latestCreatedBackup?.isEncrypted!!) + coVerify(exactly = 1) { arrangement.createBackupFile(password = password, any()) } + } + + @Test + fun givenBackupRestore_whenProgressUpdates_thenStateIsUpdatedCorrectly() = runTest { + // Given + val backupUri = "some-backup".toUri() + val (arrangement, backupAndRestoreViewModel) = Arrangement().withSuccessfulDBImport(false).arrange() + + // When + backupAndRestoreViewModel.chooseBackupFileToRestore(backupUri) + advanceUntilIdle() + + // Then + assert(backupAndRestoreViewModel.state.backupRestoreProgress == BackupRestoreProgress.Finished) + assert(backupAndRestoreViewModel.state.restoreFileValidation == RestoreFileValidation.ValidNonEncryptedBackup) + assert(arrangement.fakeKaliumFileSystem.exists(backupAndRestoreViewModel.latestImportedBackupTempPath)) + coVerify(exactly = 1) { + arrangement.fileManager.copyToPath(backupUri, backupAndRestoreViewModel.latestImportedBackupTempPath, any()) + } + } + private inner class Arrangement { init { @@ -500,7 +501,7 @@ class BackupAndRestoreViewModelTest { withGetLastBackupDateSeconds() every { Uri.parse("some-backup") } returns mockUri coEvery { importBackup(any(), any()) } returns RestoreBackupResult.Success - coEvery { createBackupFile(any()) } returns CreateBackupResult.Success("".toPath(), 0L, "") + coEvery { createBackupFile(any(), any()) } returns CreateBackupResult.Success("".toPath(), 0L, "") coEvery { verifyBackup(any()) } returns VerifyBackupResult.Success.Encrypted } @@ -539,11 +540,17 @@ class BackupAndRestoreViewModelTest { val backupFilePath = "some-file-path".toPath() val backupSize = 1000L val backupName = "some-backup.zip" - coEvery { createBackupFile(eq(password)) } returns CreateBackupResult.Success(backupFilePath, backupSize, backupName) + coEvery { + createBackupFile(eq(password), any()) + } returns CreateBackupResult.Success(backupFilePath, backupSize, backupName) } fun withFailedCreation(password: String) = apply { - coEvery { createBackupFile(eq(password)) } returns CreateBackupResult.Failure(CoreFailure.Unknown(IOException("Some db error"))) + coEvery { + createBackupFile( + eq(password), any() + ) + } returns CreateBackupResult.Failure(CoreFailure.Unknown(IOException("Some db error"))) } fun withPreviouslyCreatedBackup(backup: BackupAndRestoreState.CreatedBackup) = apply { From 841b4725259225283b79b2e664881c6886f0609b Mon Sep 17 00:00:00 2001 From: kl3jvi Date: Sat, 23 Nov 2024 23:48:49 +0100 Subject: [PATCH 7/7] fix: backup and restore progress updates in a more meaningful way. --- kalium | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kalium b/kalium index 08c0cffe74b..36981da06e1 160000 --- a/kalium +++ b/kalium @@ -1 +1 @@ -Subproject commit 08c0cffe74b9523c7464e3645eb79f2ca7d59d3f +Subproject commit 36981da06e1d005a3d7d6f377a086668bd2d1b02