From 8d19fc50d22335e52ec8cea72a7b2c1b670ff8cf Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 13 Feb 2024 08:25:06 +0600 Subject: [PATCH 1/5] feat: add nc-common-glide-lib for image loading For SSO flow, sso-glide library is recommended to use instead of vanilla glide lib; becasue it uses SSO network stack. --- app/build.gradle | 7 +++++-- .../notes/accountswitcher/AccountSwitcherViewHolder.java | 2 +- .../notes/manageaccounts/ManageAccountViewHolder.java | 2 +- .../notes/shared/account/AccountChooserViewHolder.java | 2 +- .../owncloud/notes/shared/util/DisplayUtils.java | 6 ++++++ 5 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index e4b884d73..0affabc4d 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -67,10 +67,13 @@ dependencies { exclude group: 'org.jetbrains', module: 'annotations-java5' exclude group: 'com.github.nextcloud', module: 'Android-SingleSignOn' } + implementation ('com.github.stefan-niedermann.nextcloud-commons:sso-glide:2.1.0') { + exclude group: 'com.github.nextcloud', module: 'Android-SingleSignOn' + } // Glide - implementation 'com.github.bumptech.glide:glide:4.14.2' - annotationProcessor 'com.github.bumptech.glide:compiler:4.14.2' + implementation 'com.github.bumptech.glide:glide:4.16.0' + annotationProcessor 'com.github.bumptech.glide:compiler:4.16.0' // Android X implementation 'androidx.appcompat:appcompat:1.5.1' diff --git a/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java b/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java index 512c9a9c2..b358b8914 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java @@ -28,7 +28,7 @@ public class AccountSwitcherViewHolder extends RecyclerView.ViewHolder { binding.accountName.setText(localAccount.getDisplayName()); binding.accountHost.setText(Uri.parse(localAccount.getUrl()).getHost()); Glide.with(itemView.getContext()) - .load(DisplayUtils.getAvatarUrl(localAccount)) + .load(DisplayUtils.getSSOAvatarUrl(localAccount)) .placeholder(R.drawable.ic_account_circle_grey_24dp) .error(R.drawable.ic_account_circle_grey_24dp) .apply(RequestOptions.circleCropTransform()) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java b/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java index a8dfd718a..f44bf2db4 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java @@ -39,7 +39,7 @@ public class ManageAccountViewHolder extends RecyclerView.ViewHolder { binding.accountName.setText(localAccount.getUserName()); binding.accountHost.setText(Uri.parse(localAccount.getUrl()).getHost()); Glide.with(itemView.getContext()) - .load(DisplayUtils.getAvatarUrl(localAccount)) + .load(DisplayUtils.getSSOAvatarUrl(localAccount)) .error(R.drawable.ic_account_circle_grey_24dp) .apply(RequestOptions.circleCropTransform()) .into(binding.accountItemAvatar); diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java index 494f92917..8cb80e7b9 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java @@ -24,7 +24,7 @@ public class AccountChooserViewHolder extends RecyclerView.ViewHolder { public void bind(Account localAccount, Consumer targetAccountConsumer) { Glide .with(binding.accountItemAvatar.getContext()) - .load(DisplayUtils.getAvatarUrl(localAccount)) + .load(DisplayUtils.getSSOAvatarUrl(localAccount)) .placeholder(R.drawable.ic_account_circle_grey_24dp) .error(R.drawable.ic_account_circle_grey_24dp) .apply(RequestOptions.circleCropTransform()) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java index 0d1388d9c..0e99761c5 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.stream.Collectors; import java.util.stream.Stream; +import it.niedermann.nextcloud.sso.glide.SingleSignOnUrl; import it.niedermann.owncloud.notes.R; import it.niedermann.owncloud.notes.main.navigation.NavigationAdapter; import it.niedermann.owncloud.notes.main.navigation.NavigationItem; @@ -109,4 +110,9 @@ public class DisplayUtils { public static String getAvatarUrl(@NonNull Account account) { return account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64"; } + + @NonNull + public static SingleSignOnUrl getSSOAvatarUrl(@NonNull Account account) { + return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64"); + } } -- GitLab From 3e4f2ad0091aba302243d6bd62b841f6512849c5 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 16 Feb 2024 11:58:14 +0600 Subject: [PATCH 2/5] fix: kotlin.collection.emptyList conversion to java.collection NC-common-glide lib is written in kotlin & NC-SSO part is in java. glide lib is passing kotlin.collection.emptyList when no query is passed on the url, which is not deserializable to java.collection class. To resolve this, we have to pass dummy query value. issue: https://gitlab.e.foundation/e/backlog/-/issues/6839 --- .../notes/accountswitcher/AccountSwitcherViewHolder.java | 2 +- .../it/niedermann/owncloud/notes/main/MainActivity.java | 1 - .../notes/manageaccounts/ManageAccountViewHolder.java | 2 +- .../notes/shared/account/AccountChooserViewHolder.java | 2 +- .../owncloud/notes/shared/util/DisplayUtils.java | 9 ++------- 5 files changed, 5 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java b/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java index b358b8914..512c9a9c2 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/accountswitcher/AccountSwitcherViewHolder.java @@ -28,7 +28,7 @@ public class AccountSwitcherViewHolder extends RecyclerView.ViewHolder { binding.accountName.setText(localAccount.getDisplayName()); binding.accountHost.setText(Uri.parse(localAccount.getUrl()).getHost()); Glide.with(itemView.getContext()) - .load(DisplayUtils.getSSOAvatarUrl(localAccount)) + .load(DisplayUtils.getAvatarUrl(localAccount)) .placeholder(R.drawable.ic_account_circle_grey_24dp) .error(R.drawable.ic_account_circle_grey_24dp) .apply(RequestOptions.circleCropTransform()) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java index 6f7e7c392..36b377152 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java @@ -15,7 +15,6 @@ import android.accounts.NetworkErrorException; import android.animation.AnimatorInflater; import android.app.SearchManager; import android.content.Intent; -import android.net.Uri; import android.os.Bundle; import android.text.TextUtils; import android.view.View; diff --git a/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java b/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java index f44bf2db4..a8dfd718a 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/manageaccounts/ManageAccountViewHolder.java @@ -39,7 +39,7 @@ public class ManageAccountViewHolder extends RecyclerView.ViewHolder { binding.accountName.setText(localAccount.getUserName()); binding.accountHost.setText(Uri.parse(localAccount.getUrl()).getHost()); Glide.with(itemView.getContext()) - .load(DisplayUtils.getSSOAvatarUrl(localAccount)) + .load(DisplayUtils.getAvatarUrl(localAccount)) .error(R.drawable.ic_account_circle_grey_24dp) .apply(RequestOptions.circleCropTransform()) .into(binding.accountItemAvatar); diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java index 8cb80e7b9..494f92917 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/account/AccountChooserViewHolder.java @@ -24,7 +24,7 @@ public class AccountChooserViewHolder extends RecyclerView.ViewHolder { public void bind(Account localAccount, Consumer targetAccountConsumer) { Glide .with(binding.accountItemAvatar.getContext()) - .load(DisplayUtils.getSSOAvatarUrl(localAccount)) + .load(DisplayUtils.getAvatarUrl(localAccount)) .placeholder(R.drawable.ic_account_circle_grey_24dp) .error(R.drawable.ic_account_circle_grey_24dp) .apply(RequestOptions.circleCropTransform()) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java index 0e99761c5..6e4588015 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java @@ -107,12 +107,7 @@ public class DisplayUtils { } @NonNull - public static String getAvatarUrl(@NonNull Account account) { - return account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64"; - } - - @NonNull - public static SingleSignOnUrl getSSOAvatarUrl(@NonNull Account account) { - return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64"); + public static SingleSignOnUrl getAvatarUrl(@NonNull Account account) { + return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64?dummy=1"); //added dummy query `dummy`, to resolve https://gitlab.e.foundation/e/backlog/-/issues/6839. NC glide lib pass kotlin.collection.emptyList which is failed to convert to java.collection } } -- GitLab From aaa98bbbe8d63168237c182f53dbbb2947c2f487 Mon Sep 17 00:00:00 2001 From: Sayantan Roychowdhury Date: Fri, 16 Feb 2024 11:11:35 +0000 Subject: [PATCH 3/5] Apply 1 suggestion(s) to 1 file(s) --- .../niedermann/owncloud/notes/shared/util/DisplayUtils.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java index 6e4588015..e389ee2fe 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java @@ -108,6 +108,10 @@ public class DisplayUtils { @NonNull public static SingleSignOnUrl getAvatarUrl(@NonNull Account account) { - return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64?dummy=1"); //added dummy query `dummy`, to resolve https://gitlab.e.foundation/e/backlog/-/issues/6839. NC glide lib pass kotlin.collection.emptyList which is failed to convert to java.collection + // added dummy query `dummy`, to resolve https://gitlab.e.foundation/e/backlog/-/issues/6839. + // NC glide lib pass kotlin.collection.emptyList which is failed to convert to java.collection + return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + + "/index.php/avatar/" + + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64?dummy=1"); } } -- GitLab From e208e9d93a782b56098152b214b0a34cf17007df Mon Sep 17 00:00:00 2001 From: Jonathan Klee Date: Tue, 20 Feb 2024 08:43:18 +0000 Subject: [PATCH 4/5] chore: comment-typo --- .../it/niedermann/owncloud/notes/shared/util/DisplayUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java index e389ee2fe..b6a26dabb 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java @@ -109,7 +109,7 @@ public class DisplayUtils { @NonNull public static SingleSignOnUrl getAvatarUrl(@NonNull Account account) { // added dummy query `dummy`, to resolve https://gitlab.e.foundation/e/backlog/-/issues/6839. - // NC glide lib pass kotlin.collection.emptyList which is failed to convert to java.collection + // NC glide lib pass kotlin.collection.emptyList which fails to convert to java.collection return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + "/index.php/avatar/" + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64?dummy=1"); -- GitLab From 159c75b76f650cdadfaf93fdd9cf6646e4b6b658 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 20 Feb 2024 17:34:59 +0600 Subject: [PATCH 5/5] chore: refactor according to review --- .../owncloud/notes/shared/util/DisplayUtils.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java index b6a26dabb..6bf6f2ba4 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/shared/util/DisplayUtils.java @@ -33,6 +33,14 @@ import it.niedermann.owncloud.notes.persistence.entity.CategoryWithNotesCount; public class DisplayUtils { + private static final String AVATAR_URL_MID_PART = "/index.php/avatar/"; + + // added dummy query `dummy`, to resolve https://gitlab.e.foundation/e/backlog/-/issues/6839. + // NC glide lib pass kotlin.collection.emptyList which fails to convert to java.collection + // * + // 64 means that we want the 64px X 64px image. + private static final String AVATAR_URL_END_PART = "/64?dummy=1"; + private static final Map> SPECIAL_CATEGORY_REPLACEMENTS = Map.of( R.drawable.ic_library_music_grey600_24dp, singletonList(R.string.category_music), R.drawable.ic_local_movies_grey600_24dp, asList(R.string.category_movies, R.string.category_movie), @@ -108,10 +116,7 @@ public class DisplayUtils { @NonNull public static SingleSignOnUrl getAvatarUrl(@NonNull Account account) { - // added dummy query `dummy`, to resolve https://gitlab.e.foundation/e/backlog/-/issues/6839. - // NC glide lib pass kotlin.collection.emptyList which fails to convert to java.collection - return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + - "/index.php/avatar/" + - Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + "/64?dummy=1"); + return new SingleSignOnUrl(account.getAccountName(), account.getUrl() + AVATAR_URL_MID_PART + + Uri.encode(SSOUtil.sanitizeUserId(account.getUserName())) + AVATAR_URL_END_PART); } } -- GitLab