Port the getDocuments() API to be synchronous.
Bug: 146526096
Test: atest CtsAppSearchTestCases FrameworksCoreTests:android.app.appsearch FrameworksServicesTests:com.android.server.appsearch.impl
Change-Id: Ibd8bfecc900b5b563c5c1ecbc71506ee75753197
diff --git a/framework/java/android/app/appsearch/AppSearchBatchResult.java b/framework/java/android/app/appsearch/AppSearchBatchResult.java
index b282b35..dc75825 100644
--- a/framework/java/android/app/appsearch/AppSearchBatchResult.java
+++ b/framework/java/android/app/appsearch/AppSearchBatchResult.java
@@ -34,11 +34,11 @@
* @hide
*/
public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
- @NonNull private final Map<KeyType, AppSearchResult<ValueType>> mSuccesses;
+ @NonNull private final Map<KeyType, ValueType> mSuccesses;
@NonNull private final Map<KeyType, AppSearchResult<ValueType>> mFailures;
private AppSearchBatchResult(
- @NonNull Map<KeyType, AppSearchResult<ValueType>> successes,
+ @NonNull Map<KeyType, ValueType> successes,
@NonNull Map<KeyType, AppSearchResult<ValueType>> failures) {
mSuccesses = successes;
mFailures = failures;
@@ -61,13 +61,13 @@
}
/**
- * Returns a {@link Map} of all successful keys mapped to the successful
- * {@link AppSearchResult}s they produced.
+ * Returns a {@link Map} of all successful keys mapped to the successful {@link ValueType}
+ * values they produced.
*
* <p>The values of the {@link Map} will not be {@code null}.
*/
@NonNull
- public Map<KeyType, AppSearchResult<ValueType>> getSuccesses() {
+ public Map<KeyType, ValueType> getSuccesses() {
return mSuccesses;
}
@@ -110,7 +110,7 @@
* @hide
*/
public static final class Builder<KeyType, ValueType> {
- private final Map<KeyType, AppSearchResult<ValueType>> mSuccesses = new ArrayMap<>();
+ private final Map<KeyType, ValueType> mSuccesses = new ArrayMap<>();
private final Map<KeyType, AppSearchResult<ValueType>> mFailures = new ArrayMap<>();
/** Creates a new {@link Builder} for this {@link AppSearchBatchResult}. */
@@ -126,6 +126,18 @@
}
/**
+ * Associates the {@code key} with the given failure code and error message.
+ *
+ * <p>Any previous mapping for a key, whether success or failure, is deleted.
+ */
+ public Builder setFailure(
+ @NonNull KeyType key,
+ @AppSearchResult.ResultCode int resultCode,
+ @Nullable String errorMessage) {
+ return setResult(key, AppSearchResult.newFailedResult(resultCode, errorMessage));
+ }
+
+ /**
* Associates the {@code key} with the given {@code result}.
*
* <p>Any previous mapping for a key, whether success or failure, is deleted.
@@ -133,7 +145,7 @@
@NonNull
public Builder setResult(@NonNull KeyType key, @NonNull AppSearchResult<ValueType> result) {
if (result.isSuccess()) {
- mSuccesses.put(key, result);
+ mSuccesses.put(key, result.getResultValue());
mFailures.remove(key);
} else {
mFailures.put(key, result);
diff --git a/framework/java/android/app/appsearch/AppSearchManager.java b/framework/java/android/app/appsearch/AppSearchManager.java
index cecf2fe..e5a1639 100644
--- a/framework/java/android/app/appsearch/AppSearchManager.java
+++ b/framework/java/android/app/appsearch/AppSearchManager.java
@@ -33,6 +33,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.function.BiConsumer;
@@ -193,44 +194,62 @@
* {@code AppSearch#getDocuments()} API provided by JetPack.
*
* @param uris URIs of the documents to look up.
- * @param executor Executor on which to invoke the callback.
- * @param callback Callback to receive the documents or error.
+ * @return An {@link AppSearchBatchResult} mapping the document URIs to
+ * {@link AppSearchDocument} values if they were successfully retrieved, a {@code null}
+ * failure if they were not found, or a {@link Throwable} failure describing the problem if
+ * an error occurred.
*/
- public void getDocuments(
- @NonNull List<String> uris,
- @NonNull @CallbackExecutor Executor executor,
- @NonNull BiConsumer<List<AppSearchDocument>, ? super Throwable> callback) {
- AndroidFuture<List<byte[]>> future = new AndroidFuture<>();
- future.whenCompleteAsync((documentProtos, err) -> {
- if (err != null) {
- callback.accept(null, err);
- return;
- }
- if (documentProtos != null) {
- List<AppSearchDocument> results = new ArrayList<>(documentProtos.size());
- for (int i = 0; i < documentProtos.size(); i++) {
- DocumentProto documentProto;
- try {
- documentProto = DocumentProto.parseFrom(documentProtos.get(i));
- } catch (InvalidProtocolBufferException e) {
- callback.accept(null, e);
- return;
- }
- results.add(new AppSearchDocument(documentProto));
- }
- callback.accept(results, null);
- return;
- }
- // Nothing was supplied in the future at all
- callback.accept(null, new IllegalStateException(
- "Unknown failure occurred while retrieving documents"));
- }, executor);
- // TODO(b/146386470) stream uris?
+ public AppSearchBatchResult<String, AppSearchDocument> getDocuments(
+ @NonNull List<String> uris) {
+ // TODO(b/146386470): Transmit the result documents as a RemoteStream instead of sending
+ // them in one big list.
+ AndroidFuture<AppSearchBatchResult> future = new AndroidFuture<>();
try {
- mService.getDocuments(uris.toArray(new String[uris.size()]), future);
+ mService.getDocuments(uris, future);
} catch (RemoteException e) {
future.completeExceptionally(e);
}
+
+ // Deserialize the protos into Document objects
+ AppSearchBatchResult<String, byte[]> protoResults = getFutureOrThrow(future);
+ AppSearchBatchResult.Builder<String, AppSearchDocument> documentResultBuilder =
+ new AppSearchBatchResult.Builder<>();
+
+ // Translate successful results
+ for (Map.Entry<String, byte[]> protoResult : protoResults.getSuccesses().entrySet()) {
+ DocumentProto documentProto;
+ try {
+ documentProto = DocumentProto.parseFrom(protoResult.getValue());
+ } catch (InvalidProtocolBufferException e) {
+ documentResultBuilder.setFailure(
+ protoResult.getKey(), AppSearchResult.RESULT_IO_ERROR, e.getMessage());
+ continue;
+ }
+ AppSearchDocument document;
+ try {
+ document = new AppSearchDocument(documentProto);
+ } catch (Throwable t) {
+ // These documents went through validation, so how could this fail? We must have
+ // done something wrong.
+ documentResultBuilder.setFailure(
+ protoResult.getKey(),
+ AppSearchResult.RESULT_INTERNAL_ERROR,
+ t.getMessage());
+ continue;
+ }
+ documentResultBuilder.setSuccess(protoResult.getKey(), document);
+ }
+
+ // Translate failed results
+ for (Map.Entry<String, AppSearchResult<byte[]>> protoResult :
+ protoResults.getFailures().entrySet()) {
+ documentResultBuilder.setFailure(
+ protoResult.getKey(),
+ protoResult.getValue().getResultCode(),
+ protoResult.getValue().getErrorMessage());
+ }
+
+ return documentResultBuilder.build();
}
/**
diff --git a/framework/java/android/app/appsearch/AppSearchResult.java b/framework/java/android/app/appsearch/AppSearchResult.java
index 1d73559..7f38348 100644
--- a/framework/java/android/app/appsearch/AppSearchResult.java
+++ b/framework/java/android/app/appsearch/AppSearchResult.java
@@ -56,7 +56,6 @@
/**
* An internal error occurred within AppSearch, which the caller cannot address.
*
- *
* This error may be considered similar to {@link IllegalStateException}
*/
public static final int RESULT_INTERNAL_ERROR = 2;
diff --git a/framework/java/android/app/appsearch/IAppSearchManager.aidl b/framework/java/android/app/appsearch/IAppSearchManager.aidl
index 8c8daef..8e3f1e7 100644
--- a/framework/java/android/app/appsearch/IAppSearchManager.aidl
+++ b/framework/java/android/app/appsearch/IAppSearchManager.aidl
@@ -51,11 +51,14 @@
* Retrieves documents from the index.
*
* @param uris The URIs of the documents to retrieve
- * @param callback {@link AndroidFuture}<{@link List}<byte[]>>. Will be completed
- * with a {@link List} containing serialized DocumentProtos, or completed exceptionally if
- * get fails.
+ * @param callback
+ * {@link AndroidFuture}<{@link AppSearchBatchResult}<{@link String}, {@link byte[]}>>.
+ * If the call fails to start, {@code callback} will be completed exceptionally. Otherwise,
+ * {@code callback} will be completed with an
+ * {@link AppSearchBatchResult}<{@link String}, {@link byte[]}>
+ * where the keys are document URIs, and the values are serialized Document protos.
*/
- void getDocuments(in String[] uris, in AndroidFuture callback);
+ void getDocuments(in List<String> uris, in AndroidFuture<AppSearchBatchResult> callback);
/**
* Searches a document based on a given specifications.
diff --git a/service/java/com/android/server/appsearch/AppSearchManagerService.java b/service/java/com/android/server/appsearch/AppSearchManagerService.java
index bd99f2d..d26b4e3 100644
--- a/service/java/com/android/server/appsearch/AppSearchManagerService.java
+++ b/service/java/com/android/server/appsearch/AppSearchManagerService.java
@@ -37,7 +37,6 @@
import com.google.android.icing.protobuf.InvalidProtocolBufferException;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.List;
/**
@@ -112,7 +111,8 @@
}
@Override
- public void getDocuments(String[] uris, AndroidFuture callback) {
+ public void getDocuments(
+ @NonNull List<String> uris, @NonNull AndroidFuture<AppSearchBatchResult> callback) {
Preconditions.checkNotNull(uris);
Preconditions.checkNotNull(callback);
int callingUid = Binder.getCallingUidOrThrow();
@@ -120,13 +120,23 @@
long callingIdentity = Binder.clearCallingIdentity();
try {
AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId);
- // Contains serialized DocumentProto. byte[][] is not transmissible via Binder.
- List<byte[]> results = new ArrayList<>(uris.length);
- for (String uri : uris) {
- DocumentProto result = impl.getDocument(callingUid, uri);
- results.add(result.toByteArray());
+ AppSearchBatchResult.Builder<String, byte[]> resultBuilder =
+ new AppSearchBatchResult.Builder<>();
+ for (int i = 0; i < uris.size(); i++) {
+ String uri = uris.get(i);
+ try {
+ DocumentProto document = impl.getDocument(callingUid, uri);
+ if (document == null) {
+ resultBuilder.setFailure(
+ uri, AppSearchResult.RESULT_NOT_FOUND, /*errorMessage=*/ null);
+ } else {
+ resultBuilder.setSuccess(uri, document.toByteArray());
+ }
+ } catch (Throwable t) {
+ resultBuilder.setResult(uri, throwableToFailedResult(t));
+ }
}
- callback.complete(results);
+ callback.complete(resultBuilder.build());
} catch (Throwable t) {
callback.completeExceptionally(t);
} finally {