From ff9b4dfc5ee53036ba03e057182f26b8eb5a7c98 Mon Sep 17 00:00:00 2001 From: Alexander Dorokhine Date: Wed, 24 Mar 2021 01:57:45 -0700 Subject: [PATCH] Add another executor to setSchema() for running framework migrations. The advice to add a second executor is based on API council guidance, who do not recommend reusing the callback executor. The executor is not yet used but will be used once schema migration is implemented in framework. Bug: 183177268 Test: Builds Change-Id: Ic6735e696bc868884adffddd1433a198a005cad6 --- apex/appsearch/framework/api/current.txt | 3 +- .../app/appsearch/AppSearchSession.java | 33 ++++++++++++++----- .../testing/AppSearchSessionShimImpl.java | 2 +- .../appsearch/AppSearchSessionUnitTest.java | 2 +- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/apex/appsearch/framework/api/current.txt b/apex/appsearch/framework/api/current.txt index a3e05dc7cd858..47cf17cb1d4ca 100644 --- a/apex/appsearch/framework/api/current.txt +++ b/apex/appsearch/framework/api/current.txt @@ -149,7 +149,8 @@ package android.app.appsearch { method public void remove(@NonNull String, @NonNull android.app.appsearch.SearchSpec, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer>); method public void reportUsage(@NonNull android.app.appsearch.ReportUsageRequest, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer>); method @NonNull public android.app.appsearch.SearchResults search(@NonNull String, @NonNull android.app.appsearch.SearchSpec); - method public void setSchema(@NonNull android.app.appsearch.SetSchemaRequest, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer>); + method @Deprecated public void setSchema(@NonNull android.app.appsearch.SetSchemaRequest, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer>); + method public void setSchema(@NonNull android.app.appsearch.SetSchemaRequest, @NonNull java.util.concurrent.Executor, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer>); } public interface BatchResultCallback { diff --git a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java index ce4aad1f0c1eb..bf733ed77442c 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java +++ b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java @@ -103,6 +103,18 @@ public final class AppSearchSession implements Closeable { mDatabaseName = databaseName; } + /** + * TODO(b/181887768): This method exists only for dogfooder transition and must be removed. + * @deprecated This method exists only for dogfooder transition and must be removed. + */ + @Deprecated + public void setSchema( + @NonNull SetSchemaRequest request, + @NonNull @CallbackExecutor Executor callbackExecutor, + @NonNull Consumer> callback) { + setSchema(request, callbackExecutor, callbackExecutor, callback); + } + /** * Sets the schema that represents the organizational structure of data within the AppSearch * database. @@ -113,7 +125,9 @@ public final class AppSearchSession implements Closeable { * no-op call. * * @param request the schema to set or update the AppSearch database to. - * @param executor Executor on which to invoke the callback. + * @param workExecutor Executor on which to schedule heavy client-side background work such as + * transforming documents. + * @param callbackExecutor Executor on which to invoke the callback. * @param callback Callback to receive errors resulting from setting the schema. If the * operation succeeds, the callback will be invoked with {@code null}. */ @@ -121,10 +135,12 @@ public final class AppSearchSession implements Closeable { // exposed. public void setSchema( @NonNull SetSchemaRequest request, - @NonNull @CallbackExecutor Executor executor, + @NonNull Executor workExecutor, + @NonNull @CallbackExecutor Executor callbackExecutor, @NonNull Consumer> callback) { Objects.requireNonNull(request); - Objects.requireNonNull(executor); + Objects.requireNonNull(workExecutor); + Objects.requireNonNull(callbackExecutor); Objects.requireNonNull(callback); Preconditions.checkState(!mIsClosed, "AppSearchSession has already been closed"); List schemaBundles = new ArrayList<>(request.getSchemas().size()); @@ -153,10 +169,12 @@ public final class AppSearchSession implements Closeable { request.getVersion(), new IAppSearchResultCallback.Stub() { public void onResult(AppSearchResult result) { - executor.execute(() -> { + callbackExecutor.execute(() -> { if (result.isSuccess()) { callback.accept( // TODO(b/177266929) implement Migration in platform. + // TODO(b/183177268): once migration is implemented, run + // it on workExecutor. AppSearchResult.newSuccessfulResult( new SetSchemaResponse.Builder().build())); } else { @@ -332,8 +350,7 @@ public final class AppSearchSession implements Closeable { // Translate successful results for (Map.Entry bundleEntry : - (Set>) - result.getSuccesses().entrySet()) { + ((Map) result.getSuccesses()).entrySet()) { GenericDocument document; try { document = new GenericDocument(bundleEntry.getValue()); @@ -352,8 +369,8 @@ public final class AppSearchSession implements Closeable { // Translate failed results for (Map.Entry> bundleEntry : - (Set>>) - result.getFailures().entrySet()) { + ((Map>) + result.getFailures()).entrySet()) { documentResultBuilder.setFailure( bundleEntry.getKey(), bundleEntry.getValue().getResultCode(), diff --git a/apex/appsearch/testing/java/com/android/server/appsearch/testing/AppSearchSessionShimImpl.java b/apex/appsearch/testing/java/com/android/server/appsearch/testing/AppSearchSessionShimImpl.java index bc3064155d344..f0de4962ad3ca 100644 --- a/apex/appsearch/testing/java/com/android/server/appsearch/testing/AppSearchSessionShimImpl.java +++ b/apex/appsearch/testing/java/com/android/server/appsearch/testing/AppSearchSessionShimImpl.java @@ -89,7 +89,7 @@ public class AppSearchSessionShimImpl implements AppSearchSessionShim { @NonNull public ListenableFuture setSchema(@NonNull SetSchemaRequest request) { SettableFuture> future = SettableFuture.create(); - mAppSearchSession.setSchema(request, mExecutor, future::set); + mAppSearchSession.setSchema(request, mExecutor, mExecutor, future::set); return Futures.transformAsync(future, this::transformResult, mExecutor); } diff --git a/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java b/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java index 7ef1d5e426cc4..6d9e2ea5acabc 100644 --- a/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java +++ b/core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java @@ -51,7 +51,7 @@ public class AppSearchSessionUnitTest { CompletableFuture> schemaFuture = new CompletableFuture<>(); mSearchSession.setSchema( - new SetSchemaRequest.Builder().setForceOverride(true).build(), mExecutor, + new SetSchemaRequest.Builder().setForceOverride(true).build(), mExecutor, mExecutor, schemaFuture::complete); schemaFuture.get().getResultValue();