Skip to content

Commit

Permalink
feat: strip metadata from profile pictures (WPB-11170) (#3590)
Browse files Browse the repository at this point in the history
  • Loading branch information
yamilmedina authored Nov 8, 2024
1 parent 9d69cf6 commit 1334a23
Show file tree
Hide file tree
Showing 10 changed files with 346 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

package com.wire.android.ui.userprofile.avatarpicker

import android.content.Context
import android.net.Uri
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
Expand All @@ -33,10 +31,8 @@ import androidx.compose.material3.HorizontalDivider
import androidx.compose.material3.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.dp
import androidx.core.net.toUri
Expand Down Expand Up @@ -65,13 +61,6 @@ import com.wire.android.ui.common.visbility.rememberVisibilityState
import com.wire.android.ui.home.conversations.PermissionPermanentlyDeniedDialogState
import com.wire.android.ui.theme.wireColorScheme
import com.wire.android.ui.userprofile.avatarpicker.AvatarPickerViewModel.PictureState
import com.wire.android.util.ImageUtil
import com.wire.android.util.resampleImageAndCopyToTempPath
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import okio.Path

@RootNavGraph
@WireDestination
Expand All @@ -84,18 +73,15 @@ fun AvatarPickerScreen(
val permissionPermanentlyDeniedDialogState =
rememberVisibilityState<PermissionPermanentlyDeniedDialogState>()

val context = LocalContext.current

val targetAvatarPath = viewModel.defaultAvatarPath
val targetAvatarUri = viewModel.temporaryAvatarUri

val scope = rememberCoroutineScope()
val state = rememberAvatarPickerState(
onImageSelected = { originalUri ->
onNewAvatarPicked(originalUri, targetAvatarPath, scope, context, viewModel)
viewModel.updatePickedAvatarUri(originalUri, targetAvatarPath.toFile().toUri())
},
onPictureTaken = {
onNewAvatarPicked(targetAvatarUri, targetAvatarPath, scope, context, viewModel)
viewModel.updatePickedAvatarUri(targetAvatarUri, targetAvatarPath.toFile().toUri())
},
targetPictureFileUri = targetAvatarUri,
onGalleryPermissionPermanentlyDenied = {
Expand Down Expand Up @@ -141,19 +127,6 @@ fun AvatarPickerScreen(
)
}

// TODO: Mateusz: I think we should refactor this, it takes some values from the ViewModel, part of the logic is executed inside
// the UI, part of the logic is exectued inside the ViewModel, I see no reasons to handle the logic inside the UI
// personally it was a confusing part for me to read when investing the bugs, unless there is a valid reason to move the logic to the UI
// that I am not aware of ?
fun onNewAvatarPicked(originalUri: Uri, targetAvatarPath: Path, scope: CoroutineScope, context: Context, viewModel: AvatarPickerViewModel) {
scope.launch {
sanitizeAvatarImage(originalUri, targetAvatarPath, context)
withContext(Dispatchers.Main) {
viewModel.updatePickedAvatarUri(targetAvatarPath.toFile().toUri())
}
}
}

@Composable
private fun AvatarPickerContent(
pictureState: PictureState,
Expand Down Expand Up @@ -287,7 +260,3 @@ private fun AvatarPickerTopBar(onCloseClick: () -> Unit) {
title = stringResource(R.string.profile_image_top_bar_label),
)
}

private suspend fun sanitizeAvatarImage(originalAvatarUri: Uri, avatarPath: Path, appContext: Context) {
originalAvatarUri.resampleImageAndCopyToTempPath(appContext, avatarPath, ImageUtil.ImageSizeClass.Small)
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ import com.wire.android.appLogger
import com.wire.android.datastore.UserDataStore
import com.wire.android.model.SnackBarMessage
import com.wire.android.util.AvatarImageManager
import com.wire.android.util.ImageUtil
import com.wire.android.util.dispatchers.DispatcherProvider
import com.wire.android.util.resampleImageAndCopyToTempPath
import com.wire.android.util.toByteArray
import com.wire.android.util.ui.UIText
import com.wire.kalium.logic.NetworkFailure
Expand Down Expand Up @@ -104,10 +106,24 @@ class AvatarPickerViewModel @Inject constructor(
}
}

fun updatePickedAvatarUri(updatedUri: Uri) = viewModelScope.launch(dispatchers.main()) {
fun updatePickedAvatarUri(originalUri: Uri, updatedUri: Uri) = viewModelScope.launch {
sanitizeAvatarImage(originalUri, defaultAvatarPath)
pictureState = PictureState.Picked(updatedUri)
}

/**
* Resamples the image and removes unnecessary metadata before uploading it.
* This to avoid uploading unnecessarily large images for profile pictures and sensitive metadata.
*/
private suspend fun sanitizeAvatarImage(originalAvatarUri: Uri, avatarPath: Path) {
originalAvatarUri.resampleImageAndCopyToTempPath(
context = appContext,
tempCachePath = avatarPath,
sizeClass = ImageUtil.ImageSizeClass.Small,
shouldRemoveMetadata = true
)
}

fun uploadNewPickedAvatar(onComplete: (avatarAssetId: String?) -> Unit) {
val imgUri = pictureState.avatarUri

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.wire.android.BuildConfig
import com.wire.android.appLogger
import com.wire.android.datastore.GlobalDataStore
import com.wire.android.datastore.UserDataStore
import com.wire.android.di.AuthServerConfigProvider
import com.wire.android.di.CurrentAccount
import com.wire.android.feature.AccountSwitchUseCase
import com.wire.android.feature.SwitchAccountActions
Expand All @@ -42,7 +40,6 @@ import com.wire.android.ui.legalhold.banner.LegalHoldUIState
import com.wire.android.ui.userprofile.self.dialog.StatusDialogData
import com.wire.android.util.dispatchers.DispatcherProvider
import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.configuration.server.ServerConfig
import com.wire.kalium.logic.data.call.Call
import com.wire.kalium.logic.data.id.QualifiedIdMapper
import com.wire.kalium.logic.data.id.toQualifiedID
Expand All @@ -61,7 +58,6 @@ import com.wire.kalium.logic.feature.team.GetUpdatedSelfTeamUseCase
import com.wire.kalium.logic.feature.user.GetSelfUserUseCase
import com.wire.kalium.logic.feature.user.IsReadOnlyAccountUseCase
import com.wire.kalium.logic.feature.user.ObserveValidAccountsUseCase
import com.wire.kalium.logic.feature.user.SelfServerConfigUseCase
import com.wire.kalium.logic.feature.user.UpdateSelfAvailabilityStatusUseCase
import com.wire.kalium.logic.functional.getOrNull
import dagger.hilt.android.lifecycle.HiltViewModel
Expand Down Expand Up @@ -94,8 +90,6 @@ class SelfUserProfileViewModel @Inject constructor(
private val observeLegalHoldStatusForSelfUser: ObserveLegalHoldStateForSelfUserUseCase,
private val dispatchers: DispatcherProvider,
private val wireSessionImageLoader: WireSessionImageLoader,
private val authServerConfigProvider: AuthServerConfigProvider,
private val selfServerLinks: SelfServerConfigUseCase,
private val otherAccountMapper: OtherAccountMapper,
private val observeEstablishedCalls: ObserveEstablishedCallsUseCase,
private val accountSwitch: AccountSwitchUseCase,
Expand Down Expand Up @@ -266,27 +260,6 @@ class SelfUserProfileViewModel @Inject constructor(
}
}

// todo. cleanup unused code
fun tryToInitAddingAccount(onSucceeded: () -> Unit) {
viewModelScope.launch {
// the total number of accounts is otherAccounts + 1 for the current account
val canAddNewAccounts: Boolean = (userProfileState.otherAccounts.size + 1) < BuildConfig.MAX_ACCOUNTS

if (!canAddNewAccounts) {
userProfileState = userProfileState.copy(maxAccountsReached = true)
return@launch
}

val selfServerLinks: ServerConfig.Links =
when (val result = selfServerLinks()) {
is SelfServerConfigUseCase.Result.Failure -> return@launch
is SelfServerConfigUseCase.Result.Success -> result.serverLinks.links
}
authServerConfigProvider.updateAuthServer(selfServerLinks)
onSucceeded()
}
}

fun dismissStatusDialog() {
userProfileState = userProfileState.copy(statusDialogData = null)
}
Expand Down Expand Up @@ -321,11 +294,6 @@ class SelfUserProfileViewModel @Inject constructor(
}
}

// todo. cleanup unused code
fun onMaxAccountReachedDialogDismissed() {
userProfileState = userProfileState.copy(maxAccountsReached = false)
}

private fun setNotShowStatusRationaleAgainIfNeeded(status: UserAvailabilityStatus) {
userProfileState.statusDialogData.let { dialogState ->
if (dialogState?.isCheckBoxChecked == true) {
Expand Down
Loading

0 comments on commit 1334a23

Please sign in to comment.