Trimming profile names to fit byte budget and remove leading/trailing spaces.

master
Alan Evans 2020-01-24 10:42:58 -05:00 committed by Greyson Parrelli
parent 7d15c602a6
commit e7f568e162
6 changed files with 84 additions and 52 deletions

View File

@ -5,6 +5,7 @@ import android.os.Parcelable;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.annimon.stream.Stream;
@ -24,8 +25,8 @@ public final class ProfileName implements Parcelable {
private final String joinedName;
private ProfileName(@Nullable String givenName, @Nullable String familyName) {
this.givenName = sanitize(givenName);
this.familyName = sanitize(familyName);
this.givenName = givenName == null ? "" : givenName;
this.familyName = familyName == null ? "" : familyName;
this.joinedName = getJoinedName(this.givenName, this.familyName);
}
@ -43,7 +44,8 @@ public final class ProfileName implements Parcelable {
return familyName;
}
public boolean isProfileNameCJKV() {
@VisibleForTesting
boolean isProfileNameCJKV() {
return isCJKV(givenName, familyName);
}
@ -51,8 +53,12 @@ public final class ProfileName implements Parcelable {
return joinedName.isEmpty();
}
public boolean isGivenNameEmpty() {
return givenName.isEmpty();
}
public @NonNull String serialize() {
if (isEmpty()) {
if (isGivenNameEmpty()) {
return "";
}
@ -87,12 +93,19 @@ public final class ProfileName implements Parcelable {
* Creates a profile name, trimming chars until it fits the limits.
*/
public static @NonNull ProfileName fromParts(@Nullable String givenName, @Nullable String familyName) {
if (givenName == null || givenName.isEmpty()) return EMPTY;
givenName = givenName == null ? "" : givenName;
familyName = familyName == null ? "" : familyName;
givenName = trimToFit(givenName .trim());
familyName = trimToFit(familyName.trim());
return new ProfileName(givenName, familyName);
}
private static @NonNull String sanitize(@Nullable String name) {
/**
* Trims a name string to fit into the byte length requirement.
*/
public static @NonNull String trimToFit(@Nullable String name) {
if (name == null) return "";
// At least one byte per char, so shorten string to reduce loop

View File

@ -2,15 +2,13 @@ package org.thoughtcrime.securesms.profiles.edit;
import android.Manifest;
import android.animation.Animator;
import android.annotation.SuppressLint;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.net.Uri;
import android.os.AsyncTask;
import android.os.Build;
import android.os.Bundle;
import android.text.Selection;
import android.text.Editable;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewAnimationUtils;
@ -26,7 +24,6 @@ import androidx.annotation.Nullable;
import androidx.annotation.RequiresApi;
import androidx.annotation.StringRes;
import androidx.fragment.app.Fragment;
import androidx.lifecycle.Observer;
import androidx.lifecycle.ViewModelProviders;
import androidx.navigation.NavDirections;
import androidx.navigation.Navigation;
@ -45,6 +42,7 @@ import org.thoughtcrime.securesms.profiles.ProfileName;
import org.thoughtcrime.securesms.util.BitmapDecodingException;
import org.thoughtcrime.securesms.util.BitmapUtil;
import org.thoughtcrime.securesms.util.FeatureFlags;
import org.thoughtcrime.securesms.util.concurrent.SimpleTask;
import org.thoughtcrime.securesms.util.text.AfterTextChanged;
import org.whispersystems.libsignal.util.guava.Optional;
@ -70,7 +68,7 @@ public class EditProfileFragment extends Fragment {
private TextView username;
private Intent nextIntent;
private File captureFile;
private File captureFile;
private EditProfileViewModel viewModel;
@ -113,7 +111,7 @@ public class EditProfileFragment extends Fragment {
@Override
public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
initializeResources(view);
initializeViewModel(getArguments().getBoolean(EXCLUDE_SYSTEM, false));
initializeViewModel(requireArguments().getBoolean(EXCLUDE_SYSTEM, false));
initializeProfileName();
initializeProfileAvatar();
initializeUsername();
@ -122,7 +120,7 @@ public class EditProfileFragment extends Fragment {
}
@Override
public void onRequestPermissionsResult(int requestCode, @NonNull String permissions[], @NonNull int[] grantResults) {
public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
Permissions.onRequestPermissionsResult(this, requestCode, permissions, grantResults);
}
@ -151,9 +149,7 @@ public class EditProfileFragment extends Fragment {
break;
case AvatarSelection.REQUEST_CODE_CROP_IMAGE:
if (resultCode == Activity.RESULT_OK) {
new AsyncTask<Void, Void, byte[]>() {
@Override
protected byte[] doInBackground(Void... params) {
SimpleTask.run(() -> {
try {
BitmapUtil.ScaleResult result = BitmapUtil.createScaledBytes(requireActivity(), AvatarSelection.getResultUri(data), new ProfileMediaConstraints());
return result.getBitmap();
@ -161,14 +157,12 @@ public class EditProfileFragment extends Fragment {
Log.w(TAG, e);
return null;
}
}
@Override
protected void onPostExecute(byte[] result) {
if (result != null) {
viewModel.setAvatar(result);
},
(avatarBytes) -> {
if (avatarBytes != null) {
viewModel.setAvatar(avatarBytes);
GlideApp.with(EditProfileFragment.this)
.load(result)
.load(avatarBytes)
.skipMemoryCache(true)
.diskCacheStrategy(DiskCacheStrategy.NONE)
.circleCrop()
@ -177,7 +171,7 @@ public class EditProfileFragment extends Fragment {
Toast.makeText(requireActivity(), R.string.CreateProfileActivity_error_setting_profile_photo, Toast.LENGTH_LONG).show();
}
}
}.execute();
);
}
break;
}
@ -191,6 +185,7 @@ public class EditProfileFragment extends Fragment {
}
private void initializeResources(@NonNull View view) {
Bundle arguments = requireArguments();
this.avatar = view.findViewById(R.id.avatar);
this.givenName = view.findViewById(R.id.given_name);
@ -201,9 +196,9 @@ public class EditProfileFragment extends Fragment {
this.username = view.findViewById(R.id.profile_overview_username);
this.usernameEditButton = view.findViewById(R.id.profile_overview_username_edit_button);
this.usernameLabel = view.findViewById(R.id.profile_overview_username_label);
this.nextIntent = getArguments().getParcelable(NEXT_INTENT);
this.nextIntent = arguments.getParcelable(NEXT_INTENT);
if (FeatureFlags.usernames() && getArguments().getBoolean(DISPLAY_USERNAME, false)) {
if (FeatureFlags.usernames() && arguments.getBoolean(DISPLAY_USERNAME, false)) {
username.setVisibility(View.VISIBLE);
usernameEditButton.setVisibility(View.VISIBLE);
usernameLabel.setVisibility(View.VISIBLE);
@ -215,8 +210,14 @@ public class EditProfileFragment extends Fragment {
.onAnyResult(this::startAvatarSelection)
.execute());
this.givenName.addTextChangedListener(new AfterTextChanged(s -> viewModel.setGivenName(s.toString())));
this.familyName.addTextChangedListener(new AfterTextChanged(s -> viewModel.setFamilyName(s.toString())));
this.givenName .addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s);
viewModel.setGivenName(s.toString());
}));
this.familyName.addTextChangedListener(new AfterTextChanged(s -> {
trimInPlace(s);
viewModel.setFamilyName(s.toString());
}));
this.finishButton.setOnClickListener(v -> {
this.finishButton.setIndeterminateProgressMode(true);
@ -224,7 +225,7 @@ public class EditProfileFragment extends Fragment {
handleUpload();
});
this.finishButton.setText(getArguments().getInt(NEXT_BUTTON_TEXT, R.string.CreateProfileActivity_next));
this.finishButton.setText(arguments.getInt(NEXT_BUTTON_TEXT, R.string.CreateProfileActivity_next));
this.usernameEditButton.setOnClickListener(v -> {
NavDirections action = EditProfileFragmentDirections.actionEditUsername();
@ -238,8 +239,10 @@ public class EditProfileFragment extends Fragment {
updateFieldIfNeeded(givenName, profileName.getGivenName());
updateFieldIfNeeded(familyName, profileName.getFamilyName());
finishButton.setEnabled(!profileName.isEmpty());
finishButton.setAlpha(!profileName.isEmpty() ? 1f : 0.5f);
boolean validEntry = !profileName.isGivenNameEmpty();
finishButton.setEnabled(validEntry);
finishButton.setAlpha(validEntry ? 1f : 0.5f);
preview.setText(profileName.toString());
});
@ -260,9 +263,11 @@ public class EditProfileFragment extends Fragment {
viewModel.username().observe(this, this::onUsernameChanged);
}
private void updateFieldIfNeeded(@NonNull EditText field, @NonNull String value) {
if (!field.getText().toString().equals(value)) {
private static void updateFieldIfNeeded(@NonNull EditText field, @NonNull String value) {
String fieldTrimmed = field.getText().toString().trim();
String valueTrimmed = value.trim();
if (!fieldTrimmed.equals(valueTrimmed)) {
boolean setSelectionToEnd = field.getText().length() == 0;
field.setText(value);
@ -273,13 +278,8 @@ public class EditProfileFragment extends Fragment {
}
}
@SuppressLint("SetTextI18n")
private void onUsernameChanged(@NonNull Optional<String> username) {
if (username.isPresent()) {
this.username.setText("@" + username.get());
} else {
this.username.setText("");
}
this.username.setText(username.transform(s -> "@" + s).or(""));
}
private void startAvatarSelection() {
@ -287,10 +287,13 @@ public class EditProfileFragment extends Fragment {
}
private void handleUpload() {
viewModel.submitProfile(uploadResult -> {
if (uploadResult == EditProfileRepository.UploadResult.SUCCESS) {
if (captureFile != null) captureFile.delete();
if (captureFile != null) {
if (!captureFile.delete()) {
Log.w(TAG, "Failed to delete capture file " + captureFile);
}
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) handleFinishedLollipop();
else handleFinishedLegacy();
} else {
@ -345,6 +348,13 @@ public class EditProfileFragment extends Fragment {
animation.start();
}
private static void trimInPlace(Editable s) {
int trimmedLength = ProfileName.trimToFit(s.toString()).length();
if (s.length() > trimmedLength) {
s.delete(trimmedLength, s.length());
}
}
public interface Controller {
void onProfileNameUploadCompleted();
}

View File

@ -20,7 +20,7 @@ class EditProfileViewModel extends ViewModel {
pair -> ProfileName.fromParts(pair.first(), pair.second()));
private final MutableLiveData<byte[]> internalAvatar = new MutableLiveData<>();
private final MutableLiveData<Optional<String>> internalUsername = new MutableLiveData<>();
private final EditProfileRepository repository;
private final EditProfileRepository repository;
private EditProfileViewModel(@NonNull EditProfileRepository repository) {
this.repository = repository;
@ -70,10 +70,6 @@ class EditProfileViewModel extends ViewModel {
repository.uploadProfile(profileName, internalAvatar.getValue(), uploadResultConsumer);
}
private ProfileName currentProfileName() {
return internalProfileName.getValue();
}
static class Factory implements ViewModelProvider.Factory {
private final EditProfileRepository repository;

View File

@ -120,9 +120,9 @@
android:layout_marginTop="13dp"
android:layout_marginEnd="16dp"
android:layout_weight="1"
android:autofillHints="given-name"
android:hint="@string/CreateProfileActivity_first_name_required"
android:inputType="textPersonName"
android:maxLength="@integer/profile_name_part_max_length"
android:singleLine="true" />
<EditText
@ -135,9 +135,9 @@
android:layout_marginTop="13dp"
android:layout_marginEnd="16dp"
android:layout_weight="1"
android:autofillHints="family-name"
android:hint="@string/CreateProfileActivity_last_name_optional"
android:inputType="textPersonName"
android:maxLength="@integer/profile_name_part_max_length"
android:singleLine="true" />
</LinearLayout>

View File

@ -4,6 +4,4 @@
<integer name="reaction_scrubber_reveal_duration">400</integer>
<integer name="reaction_scrubber_reveal_emoji_duration">380</integer>
<integer name="reaction_scrubber_emoji_reveal_duration_start_delay_factor">10</integer>
<integer name="profile_name_part_max_length">26</integer>
</resources>

View File

@ -10,7 +10,7 @@ import static org.junit.Assert.assertTrue;
public final class ProfileNameTest {
@Test
@Test
public void givenEmpty_thenIExpectSaneDefaults() {
// GIVEN
ProfileName profileName = ProfileName.EMPTY;
@ -23,7 +23,7 @@ public final class ProfileNameTest {
}
@Test
public void givenNullProfileName_whenIFromDataString_thenIExpectSaneDefaults() {
public void givenNullProfileName_whenIFromSerialized_thenIExpectExactlyEmpty() {
// GIVEN
ProfileName profileName = ProfileName.fromSerialized(null);
@ -128,6 +128,7 @@ public final class ProfileNameTest {
// THEN
assertEquals("Blank String should be returned (For back compat)", "", data);
assertEquals("Family", name.getFamilyName());
}
@Test
@ -161,4 +162,18 @@ public final class ProfileNameTest {
assertEquals("GivenSomeVeryLongNameSomeV", name.getGivenName());
assertEquals("FamilySomeVeryLongNameSome", name.getFamilyName());
}
@Test
public void givenProfileNameWithGivenNameAndFamilyNameWithSpaces_whenIToDataString_thenIExpectTrimmedProfileName() {
// GIVEN
ProfileName name = ProfileName.fromParts(" Given ", " Family");
// WHEN
String data = name.serialize();
// THEN
assertEquals(data, "Given\0Family");
assertEquals(name.getGivenName(), "Given");
assertEquals(name.getFamilyName(), "Family");
}
}