summaryrefslogtreecommitdiff
path: root/java
diff options
context:
space:
mode:
author Joshua Trask <joshtrask@google.com> 2022-10-20 09:18:06 -0400
committer Joshua Trask <joshtrask@google.com> 2022-10-24 13:28:25 +0000
commitdeb9ddfb590c7533e1be565c40f9a522071c19a9 (patch)
treee73c8b845c66d66212731e7d64c09abcd85f0e30 /java
parent5551f9d425395ae427c98785425b81a32b1103dd (diff)
Rename & pull `getTargets` up to base `TargetInfo`
This was originally only offered on `MultiDisplayResolveInfo` but that just resulted in some type-checked conditional logic where the caller (`ChooserActivity`) took responsibility for implementing the other cases. Thus this CL is mainly based on the "Replace Conditional with Polymorphism" item in Fowler's _Refactoring_. (It also separates one piece of unrelated conditional behavior from the call site in `ChooserActivity` but leaves any further cleanup of that separate behavior out of scope.) The method rename is an attempt to call attention to the use of the legacy `DisplayResolveInfo` type (as distinguished from other types of "targets" i.e. `ChooserTargetInfo`), in advance of any cleanup of that design. This both simplifies the call site (in `ChooserActivity`) and consolidates `TargetInfo` requirements that were previously diffuse. It also progresses several of our tactical goals in the ongoing `TargetInfo` cleanup (go/chooser-targetinfo-cleanup): * In order to unify the types, all methods offered by any `TargetInfo` implementation have to be pulled up to the base interface; this CL prioritizes one method of particular benefit to callers in its new location, but it was going to have to move eventually regardless. * This encapsulates one of the two call sites of `TargetInfo.getDisplayResolveInfo()`. That API assumes callers have an understanding of the semantics of the legacy `DisplayResolveInfo` implementation, along with the semantics of the (legacy) concrete type of any other `TargetInfo` they might hold (in order to know whether they can use it as-is vs. descending through its `getDisplayResolveInfo()`). The new API focuses (slightly) more on the application requirements that client is trying to fulfill. * More broadly, this reduces call sites where clients depend on the (new-but-deprecated) `TargetInfo` methods that query the legacy type information. These usages _all_ need to be replaced with new APIs that are more expressive in the application domain. Test: atest IntentResolverUnitTests Bug: 202167050 Change-Id: Ibc6cbfb01f323db4b820053babfeb5f7c8024f26
Diffstat (limited to 'java')
-rw-r--r--java/src/com/android/intentresolver/ChooserActivity.java35
-rw-r--r--java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java2
-rw-r--r--java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java13
-rw-r--r--java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java6
-rw-r--r--java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java5
-rw-r--r--java/src/com/android/intentresolver/chooser/TargetInfo.java23
-rw-r--r--java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt3
7 files changed, 62 insertions, 25 deletions
diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java
index 26c7fbc9..91f1ce74 100644
--- a/java/src/com/android/intentresolver/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/ChooserActivity.java
@@ -1712,18 +1712,24 @@ public class ChooserActivity extends ResolverActivity implements
private void showTargetDetails(TargetInfo targetInfo) {
if (targetInfo == null) return;
- ArrayList<DisplayResolveInfo> targetList;
+ List<DisplayResolveInfo> targetList = targetInfo.getAllDisplayTargets();
+ if (targetList.isEmpty()) {
+ Log.e(TAG, "No displayable data to show target details");
+ return;
+ }
+
ChooserTargetActionsDialogFragment fragment = new ChooserTargetActionsDialogFragment();
Bundle bundle = new Bundle();
+ bundle.putParcelable(ChooserTargetActionsDialogFragment.USER_HANDLE_KEY,
+ mChooserMultiProfilePagerAdapter.getCurrentUserHandle());
+ bundle.putParcelableArrayList(ChooserTargetActionsDialogFragment.TARGET_INFOS_KEY,
+ new ArrayList<>(targetList));
+
if (targetInfo.isSelectableTargetInfo()) {
- if (targetInfo.getDisplayResolveInfo() == null
- || targetInfo.getChooserTarget() == null) {
- Log.e(TAG, "displayResolveInfo or chooserTarget in selectableTargetInfo are null");
- return;
- }
- targetList = new ArrayList<>();
- targetList.add(targetInfo.getDisplayResolveInfo());
+ // TODO: migrate this condition to polymorphic calls on TargetInfo (maybe in some cases
+ // we can safely drop the `isSelectableTargetInfo()` condition and populate the bundle
+ // with any non-null values we find, regardless of the target type?)
bundle.putString(ChooserTargetActionsDialogFragment.SHORTCUT_ID_KEY,
targetInfo.getChooserTarget().getIntentExtras().getString(
Intent.EXTRA_SHORTCUT_ID));
@@ -1735,20 +1741,9 @@ public class ChooserActivity extends ResolverActivity implements
bundle.putString(ChooserTargetActionsDialogFragment.SHORTCUT_TITLE_KEY,
targetInfo.getDisplayLabel().toString());
}
- } else if (targetInfo.isMultiDisplayResolveInfo()) {
- // For multiple targets, include info on all targets
- MultiDisplayResolveInfo mti = (MultiDisplayResolveInfo) targetInfo;
- targetList = mti.getTargets();
- } else {
- targetList = new ArrayList<DisplayResolveInfo>();
- targetList.add((DisplayResolveInfo) targetInfo);
}
- bundle.putParcelable(ChooserTargetActionsDialogFragment.USER_HANDLE_KEY,
- mChooserMultiProfilePagerAdapter.getCurrentUserHandle());
- bundle.putParcelableArrayList(ChooserTargetActionsDialogFragment.TARGET_INFOS_KEY,
- targetList);
- fragment.setArguments(bundle);
+ fragment.setArguments(bundle);
fragment.show(getSupportFragmentManager(), TARGET_DETAILS_FRAGMENT_TAG);
}
diff --git a/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java b/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java
index ae08ace2..b4e427a1 100644
--- a/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java
+++ b/java/src/com/android/intentresolver/ChooserStackedAppDialogFragment.java
@@ -43,7 +43,7 @@ public class ChooserStackedAppDialogFragment extends ChooserTargetActionsDialogF
void setStateFromBundle(Bundle b) {
mMultiDisplayResolveInfo = (MultiDisplayResolveInfo) b.get(MULTI_DRI_KEY);
- mTargetInfos = mMultiDisplayResolveInfo.getTargets();
+ mTargetInfos = mMultiDisplayResolveInfo.getAllDisplayTargets();
mUserHandle = (UserHandle) b.get(USER_HANDLE_KEY);
mParentWhich = b.getInt(WHICH_KEY);
}
diff --git a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
index 38a2759e..2de901cd 100644
--- a/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/ChooserTargetInfo.java
@@ -19,6 +19,9 @@ package com.android.intentresolver.chooser;
import android.service.chooser.ChooserTarget;
import android.text.TextUtils;
+import java.util.ArrayList;
+import java.util.Arrays;
+
/**
* A TargetInfo for Direct Share. Includes a {@link ChooserTarget} representing the
* Direct Share deep link into an application.
@@ -31,6 +34,16 @@ public abstract class ChooserTargetInfo implements TargetInfo {
}
@Override
+ public ArrayList<DisplayResolveInfo> getAllDisplayTargets() {
+ // TODO: consider making this the default behavior for all `TargetInfo` implementations
+ // (if it's reasonable for `DisplayResolveInfo.getDisplayResolveInfo()` to return `this`).
+ if (getDisplayResolveInfo() == null) {
+ return new ArrayList<>();
+ }
+ return new ArrayList<>(Arrays.asList(getDisplayResolveInfo()));
+ }
+
+ @Override
public boolean isSimilar(TargetInfo other) {
if (other == null) return false;
diff --git a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
index 5e1b8cab..cd6828c7 100644
--- a/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
+++ b/java/src/com/android/intentresolver/chooser/DisplayResolveInfo.java
@@ -35,6 +35,7 @@ import com.android.intentresolver.ResolverActivity;
import com.android.intentresolver.ResolverListAdapter.ResolveInfoPresentationGetter;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
/**
@@ -173,6 +174,11 @@ public class DisplayResolveInfo implements TargetInfo, Parcelable {
return mSourceIntents;
}
+ @Override
+ public ArrayList<DisplayResolveInfo> getAllDisplayTargets() {
+ return new ArrayList<>(Arrays.asList(this));
+ }
+
public void addAlternateSourceIntent(Intent alt) {
mSourceIntents.add(alt);
}
diff --git a/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java b/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java
index bad36077..29f00a35 100644
--- a/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java
+++ b/java/src/com/android/intentresolver/chooser/MultiDisplayResolveInfo.java
@@ -30,8 +30,6 @@ import java.util.List;
*/
public class MultiDisplayResolveInfo extends DisplayResolveInfo {
- /* TODO: hold as a generic `List<DisplayResolveInfo>` once we're unconstrained by the TODO
- * regarding the return type of `#getTargets()`. */
ArrayList<DisplayResolveInfo> mTargetInfos = new ArrayList<>();
// Index of selected target
@@ -71,7 +69,8 @@ public class MultiDisplayResolveInfo extends DisplayResolveInfo {
* TODO: provide as a generic {@code List<DisplayResolveInfo>} once {@link ChooserActivity}
* stops requiring the signature to match that of the other "lists" it builds up.
*/
- public ArrayList<DisplayResolveInfo> getTargets() {
+ @Override
+ public ArrayList<DisplayResolveInfo> getAllDisplayTargets() {
return mTargetInfos;
}
diff --git a/java/src/com/android/intentresolver/chooser/TargetInfo.java b/java/src/com/android/intentresolver/chooser/TargetInfo.java
index 4842cd80..bde2fcf0 100644
--- a/java/src/com/android/intentresolver/chooser/TargetInfo.java
+++ b/java/src/com/android/intentresolver/chooser/TargetInfo.java
@@ -30,6 +30,7 @@ import android.service.chooser.ChooserTarget;
import com.android.intentresolver.ResolverActivity;
+import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
@@ -129,6 +130,28 @@ public interface TargetInfo {
List<Intent> getAllSourceIntents();
/**
+ * @return the one or more {@link DisplayResolveInfo}s that this target represents in the UI.
+ *
+ * TODO: clarify the semantics of the {@link DisplayResolveInfo} branch of {@link TargetInfo}'s
+ * class hierarchy. Why is it that {@link MultiDisplayResolveInfo} can stand in for some
+ * "virtual" {@link DisplayResolveInfo} targets that aren't individually represented in the UI,
+ * but OTOH a {@link ChooserTargetInfo} (which doesn't inherit from {@link DisplayResolveInfo})
+ * can't provide its own UI treatment, and instead needs us to reach into its composed-in
+ * info via {@link #getDisplayResolveInfo()}? It seems like {@link DisplayResolveInfo} may be
+ * required to populate views in our UI, while {@link ChooserTargetInfo} may carry some other
+ * metadata. For non-{@link ChooserTargetInfo} targets (e.g. in {@link ResolverActivity}) the
+ * "naked" {@link DisplayResolveInfo} might also be taken to provide some of this metadata, but
+ * this presents a denormalization hazard since the "UI info" ({@link DisplayResolveInfo}) that
+ * represents a {@link ChooserTargetInfo} might provide different values than its enclosing
+ * {@link ChooserTargetInfo} (as they both implement {@link TargetInfo}). We could try to
+ * address this by splitting {@link DisplayResolveInfo} into two types; one (which implements
+ * the same {@link TargetInfo} interface as {@link ChooserTargetInfo}) provides the previously-
+ * implicit "metadata", and the other provides only the UI treatment for a target of any type
+ * (taking over the respective methods that previously belonged to {@link TargetInfo}).
+ */
+ ArrayList<DisplayResolveInfo> getAllDisplayTargets();
+
+ /**
* @return true if this target cannot be selected by the user
*/
boolean isSuspended();
diff --git a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
index 8494e296..4a5e1baf 100644
--- a/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
+++ b/java/tests/src/com/android/intentresolver/chooser/TargetInfoTest.kt
@@ -122,7 +122,8 @@ class TargetInfoTest {
assertThat(multiTargetInfo.getExtendedInfo()).isNull()
- assertThat(multiTargetInfo.getTargets()).containsExactly(firstTargetInfo, secondTargetInfo)
+ assertThat(multiTargetInfo.getAllDisplayTargets())
+ .containsExactly(firstTargetInfo, secondTargetInfo)
assertThat(multiTargetInfo.hasSelected()).isFalse()
assertThat(multiTargetInfo.getSelectedTarget()).isNull()