PM-41429: Chore: Decouple dependencies (#7238)

This commit is contained in:
David Perez authored and GitHub committed 2026-08-04 14:21:05 +00:00
1 parent d029ebb0bf
commit 33a8429c3e
11 files changed
+221 -82

No files matched your search

@@ -7,7 +7,6 @@ import com.bitwarden.core.data.util.asFailure
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.core.data.util.flatMap
import com.bitwarden.data.manager.file.FileManager
import com.bitwarden.data.manager.model.DownloadResult
import com.bitwarden.network.model.ArchiveCipherResponseJson
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
@@ -18,6 +17,7 @@ import com.bitwarden.network.model.UnarchiveCipherResponseJson
import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
import com.bitwarden.network.model.UpdateCipherResponseJson
import com.bitwarden.network.service.CiphersService
import com.bitwarden.network.service.DownloadService
import com.bitwarden.vault.AttachmentView
import com.bitwarden.vault.CipherView
import com.bitwarden.vault.EncryptionContext
@@ -57,6 +57,7 @@ import java.time.Clock
*/
@Suppress("TooManyFunctions", "LongParameterList", "LargeClass")
class CipherManagerImpl(
private val downloadService: DownloadService,
private val fileManager: FileManager,
private val authDiskSource: AuthDiskSource,
private val settingsDiskSource: SettingsDiskSource,
@@ -622,14 +623,13 @@ class CipherManagerImpl(
val url = attachmentData.url
?: return IllegalStateException("Attachment does not have a url").asFailure()
val encryptedFile = when (val result = fileManager.downloadFileToCache(url)) {
is DownloadResult.Failure -> {
return IllegalStateException("Download failed", result.error).asFailure()
val encryptedFile = downloadService
.getDataStream(url = url)
.flatMap { fileManager.streamFileToCache(stream = it.byteStream()) }
.getOrElse {
return IllegalStateException("Download failed", it).asFailure()
}
is DownloadResult.Success -> result.file
}
val decryptedFile = File(encryptedFile.path + "_decrypted")
return vaultSdkSource
.decryptFile(
@@ -7,6 +7,7 @@ import com.bitwarden.cxf.parser.CredentialExchangePayloadParser
import com.bitwarden.data.manager.appstate.AppStateManager
import com.bitwarden.data.manager.file.FileManager
import com.bitwarden.network.service.CiphersService
import com.bitwarden.network.service.DownloadService
import com.bitwarden.network.service.FolderService
import com.bitwarden.network.service.SendsService
import com.bitwarden.network.service.SyncService
@@ -98,6 +99,7 @@ object VaultManagerModule {
@Provides
@Singleton
fun provideCipherManager(
downloadService: DownloadService,
ciphersService: CiphersService,
settingsDiskSource: SettingsDiskSource,
vaultDiskSource: VaultDiskSource,
@@ -109,6 +111,7 @@ object VaultManagerModule {
dispatcherManager: DispatcherManager,
pushManager: PushManager,
): CipherManager = CipherManagerImpl(
downloadService = downloadService,
fileManager = fileManager,
settingsDiskSource = settingsDiskSource,
authDiskSource = authDiskSource,
@@ -7,7 +7,6 @@ import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
import com.bitwarden.core.data.util.asFailure
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.data.manager.file.FileManager
import com.bitwarden.data.manager.model.DownloadResult
import com.bitwarden.network.exception.CookieRedirectException
import com.bitwarden.network.model.ArchiveCipherResponseJson
import com.bitwarden.network.model.AttachmentJsonRequest
@@ -27,6 +26,7 @@ import com.bitwarden.network.model.createMockCipherJsonRequest
import com.bitwarden.network.model.createMockCollection
import com.bitwarden.network.model.createMockLogin
import com.bitwarden.network.service.CiphersService
import com.bitwarden.network.service.DownloadService
import com.bitwarden.vault.Attachment
import com.bitwarden.vault.AttachmentView
import com.bitwarden.vault.Cipher
@@ -77,12 +77,14 @@ import io.mockk.unmockkConstructor
import io.mockk.unmockkStatic
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.test.runTest
import okhttp3.ResponseBody
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import java.io.File
import java.io.InputStream
import java.time.Clock
import java.time.Instant
import java.time.ZoneOffset
@@ -112,8 +114,10 @@ class CipherManagerTest {
every { syncCipherDeleteFlow } returns mutableSyncCipherDeleteFlow
every { syncCipherUpsertFlow } returns mutableSyncCipherUpsertFlow
}
private val downloadService: DownloadService = mockk()
private val cipherManager: CipherManager = CipherManagerImpl(
downloadService = downloadService,
ciphersService = ciphersService,
settingsDiskSource = fakeSettingsDiskSource,
vaultDiskSource = vaultDiskSource,
@@ -1303,9 +1307,16 @@ class CipherManagerTest {
coEvery {
ciphersService.getCipherAttachment(cipherId = "mockId-1", attachmentId = "mockId-1")
} returns attachment.asSuccess()
val mockInputStream = mockk<InputStream>()
val mockResponseBody = mockk<ResponseBody> {
every { byteStream() } returns mockInputStream
}
coEvery {
fileManager.downloadFileToCache(url = "mockUrl-1")
} returns DownloadResult.Success(file = encryptedFile)
downloadService.getDataStream(url = "mockUrl-1")
} returns mockResponseBody.asSuccess()
coEvery {
fileManager.streamFileToCache(stream = mockInputStream)
} returns encryptedFile.asSuccess()
coEvery {
vaultSdkSource.decryptFile(
userId = userId,
@@ -2311,7 +2322,7 @@ class CipherManagerTest {
}
@Test
fun `downloadAttachment with failed download should return Failure`() = runTest {
fun `downloadAttachment with failed data stream request should return Failure`() = runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val attachmentId = "mockId-1"
@@ -2343,8 +2354,8 @@ class CipherManagerTest {
ciphersService.getCipherAttachment(cipherId = any(), attachmentId = any())
} returns response.asSuccess()
coEvery {
fileManager.downloadFileToCache(url = any())
} returns DownloadResult.Failure(error = Throwable("Fail!"))
downloadService.getDataStream(url = any())
} returns Throwable("Fail!").asFailure()
assertEquals(
DownloadAttachmentResult.Failure(IllegalStateException()),
@@ -2363,7 +2374,75 @@ class CipherManagerTest {
cipherId = requireNotNull(cipherView.id),
attachmentId = attachmentId,
)
fileManager.downloadFileToCache("https://bitwarden.com")
downloadService.getDataStream(url = "https://bitwarden.com")
}
coVerify(exactly = 0) {
fileManager.streamFileToCache(stream = any())
}
}
@Test
fun `downloadAttachment with failed stream to cache should return Failure`() = runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val attachmentId = "mockId-1"
val attachment = mockk<Attachment> {
every { id } returns attachmentId
}
val mockCipher = mockk<Cipher> {
every { key } returns "key"
every { attachments } returns listOf(attachment)
every { id } returns "mockId-1"
}
val mockEncryptionContext = mockk<EncryptionContext> {
every { encryptedFor } returns "mockEncryptedFor-1"
every { cipher } returns mockCipher
}
val cipherView = createMockCipherView(number = 1)
coEvery {
vaultSdkSource.encryptCipher(
userId = MOCK_USER_STATE.activeUserId,
cipherView = cipherView,
)
} returns mockEncryptionContext.asSuccess()
val response = mockk<SyncResponseJson.Cipher.Attachment> {
every { url } returns "https://bitwarden.com"
}
coEvery {
ciphersService.getCipherAttachment(cipherId = any(), attachmentId = any())
} returns response.asSuccess()
val mockInputStream = mockk<InputStream>()
val mockResponseBody = mockk<ResponseBody> {
every { byteStream() } returns mockInputStream
}
coEvery {
downloadService.getDataStream(url = any())
} returns mockResponseBody.asSuccess()
coEvery {
fileManager.streamFileToCache(stream = mockInputStream)
} returns Throwable("Fail!").asFailure()
assertEquals(
DownloadAttachmentResult.Failure(IllegalStateException()),
cipherManager.downloadAttachment(
cipherView = cipherView,
attachmentId = attachmentId,
),
)
coVerify(exactly = 1) {
vaultSdkSource.encryptCipher(
userId = MOCK_USER_STATE.activeUserId,
cipherView = cipherView,
)
ciphersService.getCipherAttachment(
cipherId = requireNotNull(cipherView.id),
attachmentId = attachmentId,
)
downloadService.getDataStream(url = "https://bitwarden.com")
fileManager.streamFileToCache(stream = mockInputStream)
}
}
@@ -2405,9 +2484,16 @@ class CipherManagerTest {
every { path } returns "path/to/encrypted/file"
}
coEvery { fileManager.delete(file) } just runs
val mockInputStream = mockk<InputStream>()
val mockResponseBody = mockk<ResponseBody> {
every { byteStream() } returns mockInputStream
}
coEvery {
fileManager.downloadFileToCache(url = any())
} returns DownloadResult.Success(file)
downloadService.getDataStream(url = any())
} returns mockResponseBody.asSuccess()
coEvery {
fileManager.streamFileToCache(stream = mockInputStream)
} returns file.asSuccess()
val error = Throwable("Fail")
coEvery {
vaultSdkSource.decryptFile(
@@ -2436,7 +2522,8 @@ class CipherManagerTest {
cipherId = requireNotNull(cipherView.id),
attachmentId = attachmentId,
)
fileManager.downloadFileToCache("https://bitwarden.com")
downloadService.getDataStream(url = "https://bitwarden.com")
fileManager.streamFileToCache(stream = mockInputStream)
vaultSdkSource.decryptFile(
userId = MOCK_USER_STATE.activeUserId,
cipher = mockCipher,
@@ -2488,9 +2575,16 @@ class CipherManagerTest {
every { path } returns "path/to/encrypted/file"
}
coEvery { fileManager.delete(file) } just runs
val mockInputStream = mockk<InputStream>()
val mockResponseBody = mockk<ResponseBody> {
every { byteStream() } returns mockInputStream
}
coEvery {
fileManager.downloadFileToCache(any())
} returns DownloadResult.Success(file)
downloadService.getDataStream(url = any())
} returns mockResponseBody.asSuccess()
coEvery {
fileManager.streamFileToCache(stream = mockInputStream)
} returns file.asSuccess()
coEvery {
vaultSdkSource.decryptFile(
@@ -2521,7 +2615,8 @@ class CipherManagerTest {
cipherId = requireNotNull(cipherView.id),
attachmentId = attachmentId,
)
fileManager.downloadFileToCache(url = "https://bitwarden.com")
downloadService.getDataStream(url = "https://bitwarden.com")
fileManager.streamFileToCache(stream = mockInputStream)
vaultSdkSource.decryptFile(
userId = MOCK_USER_STATE.activeUserId,
cipher = mockCipher,
@@ -24,7 +24,6 @@ import com.bitwarden.authenticator.data.platform.manager.lock.AppLockManagerImpl
import com.bitwarden.authenticator.data.platform.repository.DebugMenuRepository
import com.bitwarden.authenticator.data.platform.repository.SettingsRepository
import com.bitwarden.core.data.manager.UuidManager
import com.bitwarden.core.data.manager.UuidManagerImpl
import com.bitwarden.core.data.manager.dispatcher.DispatcherManager
import com.bitwarden.core.data.manager.dispatcher.DispatcherManagerImpl
import com.bitwarden.core.data.manager.realtime.RealtimeManager
@@ -128,10 +127,6 @@ object PlatformManagerModule {
@Singleton
fun provideEncodingManager(): BitwardenEncodingManager = BitwardenEncodingManagerImpl()
@Provides
@Singleton
fun provideUuidManager(): UuidManager = UuidManagerImpl()
@Provides
@Singleton
fun providesFeatureFlagManager(
@@ -1,6 +1,8 @@
package com.bitwarden.core.data.manager.di
import com.bitwarden.core.data.manager.BuildInfoManager
import com.bitwarden.core.data.manager.UuidManager
import com.bitwarden.core.data.manager.UuidManagerImpl
import com.bitwarden.core.data.manager.encryption.EncryptionManager
import com.bitwarden.core.data.manager.encryption.EncryptionManagerImpl
import com.bitwarden.core.data.manager.encryption.KeystoreManager
@@ -33,4 +35,8 @@ object CoreManagerModule {
): KeystoreManager = KeystoreManagerImpl(
buildInfoManager = buildInfoManager,
)
@Provides
@Singleton
fun provideUuidManager(): UuidManager = UuidManagerImpl()
}
@@ -3,7 +3,9 @@ package com.bitwarden.data.manager.di
import android.app.Application
import android.content.Context
import com.bitwarden.core.data.manager.BuildInfoManager
import com.bitwarden.core.data.manager.UuidManager
import com.bitwarden.core.data.manager.dispatcher.DispatcherManager
import com.bitwarden.data.datasource.disk.ConfigDiskSource
import com.bitwarden.data.datasource.disk.FlightRecorderDiskSource
import com.bitwarden.data.manager.BitwardenPackageManager
import com.bitwarden.data.manager.BitwardenPackageManagerImpl
@@ -17,8 +19,6 @@ import com.bitwarden.data.manager.flightrecorder.FlightRecorderManager
import com.bitwarden.data.manager.flightrecorder.FlightRecorderManagerImpl
import com.bitwarden.data.manager.flightrecorder.FlightRecorderWriter
import com.bitwarden.data.manager.flightrecorder.FlightRecorderWriterImpl
import com.bitwarden.data.repository.ServerConfigRepository
import com.bitwarden.network.service.DownloadService
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
@@ -50,11 +50,11 @@ object DataManagerModule {
@Singleton
fun provideFileManager(
@ApplicationContext context: Context,
downloadService: DownloadService,
uuidManager: UuidManager,
dispatcherManager: DispatcherManager,
): FileManager = FileManagerImpl(
context = context,
downloadService = downloadService,
uuidManager = uuidManager,
dispatcherManager = dispatcherManager,
)
@@ -81,13 +81,13 @@ object DataManagerModule {
fileManager: FileManager,
dispatcherManager: DispatcherManager,
buildInfoManager: BuildInfoManager,
serverConfigRepository: ServerConfigRepository,
configDiskSource: ConfigDiskSource,
): FlightRecorderWriter = FlightRecorderWriterImpl(
clock = clock,
fileManager = fileManager,
dispatcherManager = dispatcherManager,
buildInfoManager = buildInfoManager,
serverConfigRepository = serverConfigRepository,
configDiskSource = configDiskSource,
)
@Provides
@@ -2,9 +2,9 @@ package com.bitwarden.data.manager.file
import android.net.Uri
import com.bitwarden.annotation.OmitFromCoverage
import com.bitwarden.data.manager.model.DownloadResult
import com.bitwarden.data.manager.model.ZipFileResult
import java.io.File
import java.io.InputStream
/**
* Manages reading files.
@@ -28,10 +28,10 @@ interface FileManager {
suspend fun delete(vararg files: File)
/**
* Downloads a file temporarily to cache from [url]. A successful [DownloadResult] will contain
* Opens the provided [stream] and writes it to cache. A successful [Result] will contain
* the final file path.
*/
suspend fun downloadFileToCache(url: String): DownloadResult
suspend fun streamFileToCache(stream: InputStream): Result<File>
/**
* Writes an existing [file] to a [fileUri]. `true` will be returned if the file was
@@ -5,11 +5,12 @@ package com.bitwarden.data.manager.file
import android.content.Context
import android.net.Uri
import com.bitwarden.annotation.OmitFromCoverage
import com.bitwarden.core.data.manager.UuidManager
import com.bitwarden.core.data.manager.dispatcher.DispatcherManager
import com.bitwarden.core.data.util.asFailure
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.core.data.util.sdkAgnosticTransferTo
import com.bitwarden.data.manager.model.DownloadResult
import com.bitwarden.data.manager.model.ZipFileResult
import com.bitwarden.network.service.DownloadService
import kotlinx.coroutines.withContext
import java.io.BufferedInputStream
import java.io.BufferedOutputStream
@@ -18,7 +19,7 @@ import java.io.File
import java.io.FileInputStream
import java.io.FileOutputStream
import java.io.IOException
import java.util.UUID
import java.io.InputStream
import java.util.zip.ZipEntry
import java.util.zip.ZipOutputStream
@@ -32,7 +33,7 @@ private const val BUFFER_SIZE: Int = 1024
*/
internal class FileManagerImpl(
private val context: Context,
private val downloadService: DownloadService,
private val uuidManager: UuidManager,
private val dispatcherManager: DispatcherManager,
) : FileManager {
@@ -48,20 +49,10 @@ internal class FileManagerImpl(
}
}
@Suppress("NestedBlockDepth")
override suspend fun downloadFileToCache(url: String): DownloadResult {
val response = downloadService
.getDataStream(url)
.fold(
onSuccess = { it },
onFailure = { return DownloadResult.Failure(error = it) },
)
override suspend fun streamFileToCache(stream: InputStream): Result<File> {
// Create a temporary file in cache to write to
val file = File(context.cacheDir, UUID.randomUUID().toString())
withContext(dispatcherManager.io) {
val stream = response.byteStream()
val file = File(context.cacheDir, uuidManager.generateUuid())
return withContext(dispatcherManager.io) {
stream.use {
val buffer = ByteArray(BUFFER_SIZE)
var progress = 0
@@ -76,13 +67,12 @@ internal class FileManagerImpl(
}
fos.flush()
} catch (e: RuntimeException) {
return@withContext DownloadResult.Failure(error = e)
return@withContext e.asFailure()
}
}
}
file.asSuccess()
}
return DownloadResult.Success(file)
}
@Suppress("NestedBlockDepth")
@@ -8,9 +8,9 @@ import com.bitwarden.core.data.manager.BuildInfoManager
import com.bitwarden.core.data.manager.dispatcher.DispatcherManager
import com.bitwarden.core.data.manager.util.deviceData
import com.bitwarden.core.data.util.toFormattedPattern
import com.bitwarden.data.datasource.disk.ConfigDiskSource
import com.bitwarden.data.datasource.disk.model.FlightRecorderDataSet
import com.bitwarden.data.manager.file.FileManager
import com.bitwarden.data.repository.ServerConfigRepository
import com.bitwarden.network.util.redactHostnamesInMessage
import kotlinx.coroutines.withContext
import timber.log.Timber
@@ -34,12 +34,15 @@ internal class FlightRecorderWriterImpl(
private val fileManager: FileManager,
private val dispatcherManager: DispatcherManager,
private val buildInfoManager: BuildInfoManager,
private val serverConfigRepository: ServerConfigRepository,
private val configDiskSource: ConfigDiskSource,
) : FlightRecorderWriter {
private val configuredHosts: Set<String>
get() {
val environment = serverConfigRepository.serverConfigStateFlow.value
?.serverData?.environment ?: return emptySet()
val environment = configDiskSource
.serverConfig
?.serverData
?.environment
?: return emptySet()
return listOfNotNull(
environment.vaultUrl,
environment.apiUrl,
@@ -75,7 +78,7 @@ internal class FlightRecorderWriterImpl(
logFile.createNewFile()
val ciInfo = buildInfoManager.ciBuildInfo?.takeIf { it.isNotBlank() }
val serverData = serverConfigRepository.serverConfigStateFlow.value?.serverData
val serverData = configDiskSource.serverConfig?.serverData
val serverInfo = StringBuilder()
.append(serverData?.server?.name ?: "Bitwarden Cloud")
.apply {
@@ -1,20 +0,0 @@
package com.bitwarden.data.manager.model
import java.io.File
/**
* Represents a result from downloading a raw file.
*/
sealed class DownloadResult {
/**
* The download was a success, and was saved to [file].
*/
data class Success(val file: File) : DownloadResult()
/**
* The download failed.
*/
data class Failure(
val error: Throwable,
) : DownloadResult()
}
@@ -3,23 +3,27 @@ package com.bitwarden.data.manager.file
import android.content.ContentResolver
import android.content.Context
import android.net.Uri
import com.bitwarden.core.data.manager.UuidManager
import com.bitwarden.core.data.manager.dispatcher.FakeDispatcherManager
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.network.service.DownloadService
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertArrayEquals
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertInstanceOf
import java.io.File
import java.io.IOException
import java.io.InputStream
import java.io.OutputStream
import java.nio.file.Files
/**
* Test class for [FileManagerImpl].
@@ -28,18 +32,25 @@ class FileManagerTest {
private val fakeDispatcherManager = FakeDispatcherManager()
private val mockContentResolver = mockk<ContentResolver>()
private val downloadService = mockk<DownloadService>()
private val cacheDirectory: File = Files.createTempDirectory("cache").toFile()
private val mockContext = mockk<Context> {
every { contentResolver } returns mockContentResolver
every { cacheDir } returns cacheDirectory
}
private val uuidManager: UuidManager = mockk()
private val mockUri = mockk<Uri>()
private val fileManager = FileManagerImpl(
context = mockContext,
uuidManager = uuidManager,
dispatcherManager = fakeDispatcherManager,
downloadService = downloadService,
)
@AfterEach
fun tearDown() {
cacheDirectory.deleteRecursively()
}
//region stringToUri Tests
@Test
@@ -258,6 +269,62 @@ class FileManagerTest {
//endregion
//region streamFileToCache Tests
@Test
fun `streamFileToCache with valid stream should return Success with the cached file`() =
runTest {
val testData = "Test content".toByteArray()
val mockInputStream = createMockInputStream(testData)
every { uuidManager.generateUuid() } returns "mockUuid"
val result = fileManager.streamFileToCache(stream = mockInputStream)
val file = result.getOrThrow()
assertEquals(File(cacheDirectory, "mockUuid"), file)
assertArrayEquals(testData, file.readBytes())
verify(exactly = 1) { mockInputStream.close() }
}
@Test
fun `streamFileToCache with empty stream should return Success with an empty file`() = runTest {
val mockInputStream = createMockInputStream(testData = ByteArray(0))
every { uuidManager.generateUuid() } returns "mockUuid"
val result = fileManager.streamFileToCache(stream = mockInputStream)
val file = result.getOrThrow()
assertEquals(0, file.length())
}
@Test
fun `streamFileToCache with large stream should write the stream completely`() = runTest {
val testData = "L".repeat(5000).toByteArray()
val mockInputStream = createMockInputStream(testData)
every { uuidManager.generateUuid() } returns "mockUuid"
val result = fileManager.streamFileToCache(stream = mockInputStream)
assertArrayEquals(testData, result.getOrThrow().readBytes())
}
@Test
fun `streamFileToCache with read failure should return Failure`() = runTest {
val error = RuntimeException("Read failed")
val mockInputStream = mockk<InputStream> {
every { read(any<ByteArray>()) } throws error
every { close() } just runs
}
every { uuidManager.generateUuid() } returns "mockUuid"
val result = fileManager.streamFileToCache(stream = mockInputStream)
assertEquals(error, result.exceptionOrNull())
verify(exactly = 1) { mockInputStream.close() }
}
//endregion
//region Helper Methods
/**