From 07c2422ad0cae07006c7db8dae64ac44f993e164 Mon Sep 17 00:00:00 2001 From: Terry Wang Date: Fri, 11 Dec 2020 22:17:09 +0000 Subject: [PATCH] Revert "Split getInstance and createInstance in AppSearchImpl" This reverts commit 5caaa238eb78373b44830ed46a56eeda62e597c0. Reason for revert: Change-Id: Icc000c874ff1db45eba1a513e8aacf52b4ede147 --- .../app/appsearch/AppSearchSession.java | 19 ++++++------- .../appsearch/AppSearchManagerService.java | 27 +++++++++---------- .../server/appsearch/ImplInstanceManager.java | 26 ++---------------- 3 files changed, 25 insertions(+), 47 deletions(-) diff --git a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java index 1d86595a40639..b7cd4f5f8bce2 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java +++ b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSession.java @@ -18,6 +18,7 @@ package android.app.appsearch; import android.annotation.CallbackExecutor; import android.annotation.NonNull; +import android.app.appsearch.exceptions.AppSearchException; import android.os.Bundle; import android.os.ParcelableException; import android.os.RemoteException; @@ -166,9 +167,9 @@ public final class AppSearchSession { * of the returned {@link AppSearchBatchResult} are the URIs of the input * documents. The values are {@code null} if they were successfully indexed, * or a failed {@link AppSearchResult} otherwise. - * Or {@link BatchResultCallback#onSystemError} will be invoked with a - * {@link Throwable} if an unexpected internal error occurred in AppSearch - * service. + * Or {@link BatchResultCallback#onSystemError} will be invoked with an + * {@link AppSearchException} if an error occurred in AppSearch initialization + * or a cause {@link Throwable} if other error occurred in AppSearch service. */ public void putDocuments( @NonNull PutDocumentsRequest request, @@ -209,9 +210,9 @@ public final class AppSearchSession { * {@link AppSearchResult} otherwise. URIs that are not found will return a * failed {@link AppSearchResult} with a result code of * {@link AppSearchResult#RESULT_NOT_FOUND}. - * Or {@link BatchResultCallback#onSystemError} will be invoked with a - * {@link Throwable} if an unexpected internal error occurred in AppSearch - * service. + * Or {@link BatchResultCallback#onSystemError} will be invoked with an + * {@link AppSearchException} if an error occurred in AppSearch initialization + * or a cause {@link Throwable} if other error occurred in AppSearch service. */ public void getByUri( @NonNull GetByUriRequest request, @@ -337,9 +338,9 @@ public final class AppSearchSession { * are {@code null} on success, or a failed {@link AppSearchResult} otherwise. * URIs that are not found will return a failed {@link AppSearchResult} with a * result code of {@link AppSearchResult#RESULT_NOT_FOUND}. - * Or {@link BatchResultCallback#onSystemError} will be invoked with a - * {@link Throwable} if an unexpected internal error occurred in AppSearch - * service. + * Or {@link BatchResultCallback#onSystemError} will be invoked with an + * {@link AppSearchException} if an error occurred in AppSearch initialization + * or a cause {@link Throwable} if other error occurred in AppSearch service. */ public void removeByUri( @NonNull RemoveByUriRequest request, diff --git a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java index 53d097e844c8f..551347c5c202e 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java @@ -77,7 +77,7 @@ public class AppSearchManagerService extends SystemService { for (int i = 0; i < schemaBundles.size(); i++) { schemas.add(new AppSearchSchema(schemaBundles.get(i))); } - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); databaseName = rewriteDatabaseNameWithUid(databaseName, callingUid); impl.setSchema(databaseName, schemas, schemasNotPlatformSurfaceable, forceOverride); invokeCallbackOnResult(callback, @@ -103,7 +103,7 @@ public class AppSearchManagerService extends SystemService { try { AppSearchBatchResult.Builder resultBuilder = new AppSearchBatchResult.Builder<>(); - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); databaseName = rewriteDatabaseNameWithUid(databaseName, callingUid); for (int i = 0; i < documentBundles.size(); i++) { GenericDocument document = new GenericDocument(documentBundles.get(i)); @@ -138,7 +138,7 @@ public class AppSearchManagerService extends SystemService { try { AppSearchBatchResult.Builder resultBuilder = new AppSearchBatchResult.Builder<>(); - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); databaseName = rewriteDatabaseNameWithUid(databaseName, callingUid); for (int i = 0; i < uris.size(); i++) { String uri = uris.get(i); @@ -172,7 +172,7 @@ public class AppSearchManagerService extends SystemService { int callingUserId = UserHandle.getUserId(callingUid); final long callingIdentity = Binder.clearCallingIdentity(); try { - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); databaseName = rewriteDatabaseNameWithUid(databaseName, callingUid); SearchResultPage searchResultPage = impl.query( databaseName, @@ -198,7 +198,7 @@ public class AppSearchManagerService extends SystemService { int callingUserId = UserHandle.getUserId(callingUid); final long callingIdentity = Binder.clearCallingIdentity(); try { - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); SearchResultPage searchResultPage = impl.globalQuery( queryExpression, new SearchSpec(searchSpecBundle)); @@ -221,7 +221,7 @@ public class AppSearchManagerService extends SystemService { // TODO(b/162450968) check nextPageToken is being advanced by the same uid as originally // opened it try { - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); SearchResultPage searchResultPage = impl.getNextPage(nextPageToken); invokeCallbackOnResult(callback, AppSearchResult.newSuccessfulResult(searchResultPage.getBundle())); @@ -238,7 +238,7 @@ public class AppSearchManagerService extends SystemService { int callingUserId = UserHandle.getUserId(callingUid); final long callingIdentity = Binder.clearCallingIdentity(); try { - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); impl.invalidateNextPageToken(nextPageToken); } catch (Throwable t) { Log.d(TAG, "Unable to invalidate the query page token", t); @@ -257,10 +257,10 @@ public class AppSearchManagerService extends SystemService { int callingUid = Binder.getCallingUidOrThrow(); int callingUserId = UserHandle.getUserId(callingUid); final long callingIdentity = Binder.clearCallingIdentity(); + AppSearchBatchResult.Builder resultBuilder = + new AppSearchBatchResult.Builder<>(); try { - AppSearchBatchResult.Builder resultBuilder = - new AppSearchBatchResult.Builder<>(); - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); databaseName = rewriteDatabaseNameWithUid(databaseName, callingUid); for (int i = 0; i < uris.size(); i++) { String uri = uris.get(i); @@ -293,7 +293,7 @@ public class AppSearchManagerService extends SystemService { int callingUserId = UserHandle.getUserId(callingUid); final long callingIdentity = Binder.clearCallingIdentity(); try { - AppSearchImpl impl = ImplInstanceManager.getInstance(callingUserId); + AppSearchImpl impl = ImplInstanceManager.getInstance(getContext(), callingUserId); databaseName = rewriteDatabaseNameWithUid(databaseName, callingUid); impl.removeByQuery(databaseName, queryExpression, new SearchSpec(searchSpecBundle)); @@ -312,7 +312,7 @@ public class AppSearchManagerService extends SystemService { int callingUserId = UserHandle.getUserId(callingUid); final long callingIdentity = Binder.clearCallingIdentity(); try { - ImplInstanceManager.getOrCreateInstance(getContext(), callingUserId); + ImplInstanceManager.getInstance(getContext(), callingUserId); invokeCallbackOnResult(callback, AppSearchResult.newSuccessfulResult(null)); } catch (Throwable t) { invokeCallbackOnError(callback, t); @@ -374,14 +374,13 @@ public class AppSearchManagerService extends SystemService { } /** - * Invokes the {@link IAppSearchBatchResultCallback} with an unexpected internal throwable. + * Invokes the {@link IAppSearchBatchResultCallback} with an throwable. * *

The throwable is converted to {@link ParcelableException}. */ private void invokeCallbackOnError(IAppSearchBatchResultCallback callback, Throwable throwable) { try { - //TODO(b/175067650) verify ParcelableException could propagate throwable correctly. callback.onSystemError(new ParcelableException(throwable)); } catch (RemoteException e) { Log.d(TAG, "Unable to send error to the callback", e); diff --git a/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java b/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java index d26269132c5ab..2871eb622f114 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java @@ -41,7 +41,7 @@ public final class ImplInstanceManager { private ImplInstanceManager() {} /** - * Gets an instance of AppSearchImpl for the given user, or creates one if none exists. + * Gets an instance of AppSearchImpl for the given user. * *

If no AppSearchImpl instance exists for this user, Icing will be initialized and one will * be created. @@ -51,7 +51,7 @@ public final class ImplInstanceManager { * @return An initialized {@link AppSearchImpl} for this user */ @NonNull - public static AppSearchImpl getOrCreateInstance(@NonNull Context context, @UserIdInt int userId) + public static AppSearchImpl getInstance(@NonNull Context context, @UserIdInt int userId) throws AppSearchException { AppSearchImpl instance = sInstances.get(userId); if (instance == null) { @@ -66,28 +66,6 @@ public final class ImplInstanceManager { return instance; } - /** - * Gets an instance of AppSearchImpl for the given user. - * - *

This method should only be called by an initialized SearchSession, which has been already - * created the AppSearchImpl instance for the given user. - * - * @param userId The multi-user userId of the device user calling AppSearch - * @return An initialized {@link AppSearchImpl} for this user - */ - @NonNull - public static AppSearchImpl getInstance(@UserIdInt int userId) { - AppSearchImpl instance = sInstances.get(userId); - if (instance == null) { - // Impossible scenario, user cannot call an uninitialized SearchSession, - // getInstance should always find the instance for the given user and never try to - // create an instance for this user again. - throw new IllegalStateException( - "AppSearchImpl has never been created for this user: " + userId); - } - return instance; - } - private static AppSearchImpl createImpl(@NonNull Context context, @UserIdInt int userId) throws AppSearchException { File appSearchDir = getAppSearchDir(context, userId);