Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge over AppSearchResult, AppSearchBatchResult and setSchema error improvements.

Browse Source 
Bug: 162450968
Test: AppSearchSchemaTest#testRemoveSchema
Change-Id: I724dc01e24acc45f10b416eb4b6728733a4b2b1b
This commit is contained in:
Alexander Dorokhine
2020-10-08 13:43:24 -07:00
parent 2c9639a347
commit 0dd233f384
6 changed files with 119 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
apex/appsearch/framework/java/android/app/appsearch/AppSearchBatchResult.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020 The Android Open Source Project
* Copyright 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -22,6 +22,8 @@ import android.os.Parcel;
import android.os.Parcelable;
import android.util.ArrayMap;
import com.android.internal.util.Preconditions;
import java.util.Collections;
import java.util.Map;
@@ -33,11 +35,11 @@ import java.util.Map;
* @param <ValueType> The type of result objects associated with the keys.
* @hide
*/
public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
public final class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
@NonNull private final Map<KeyType, ValueType> mSuccesses;
@NonNull private final Map<KeyType, AppSearchResult<ValueType>> mFailures;
private AppSearchBatchResult(
AppSearchBatchResult(
@NonNull Map<KeyType, ValueType> successes,
@NonNull Map<KeyType, AppSearchResult<ValueType>> failures) {
mSuccesses = successes;
@@ -61,8 +63,8 @@ public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
}
/**
* Returns a {@link Map} of all successful keys mapped to the successful {@link ValueType}
* values they produced.
* Returns a {@link Map} of all successful keys mapped to the successful
* {@link AppSearchResult}s they produced.
*
* <p>The values of the {@link Map} will not be {@code null}.
*/
@@ -82,6 +84,22 @@ public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
return mFailures;
}
/**
* Asserts that this {@link AppSearchBatchResult} has no failures.
* @hide
*/
public void checkSuccess() {
if (!isSuccess()) {
throw new IllegalStateException("AppSearchBatchResult has failures: " + this);
}
}
@Override
@NonNull
public String toString() {
return "{\n successes: " + mSuccesses + "\n failures: " + mFailures + "\n}";
}
@Override
public int describeContents() {
return 0;
@@ -112,16 +130,18 @@ public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
public static final class Builder<KeyType, ValueType> {
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}. */
public Builder() {}
private boolean mBuilt = false;
/**
* Associates the {@code key} with the given successful return value.
*
* <p>Any previous mapping for a key, whether success or failure, is deleted.
*/
public Builder setSuccess(@NonNull KeyType key, @Nullable ValueType result) {
@NonNull
public Builder<KeyType, ValueType> setSuccess(
@NonNull KeyType key, @Nullable ValueType result) {
Preconditions.checkState(!mBuilt, "Builder has already been used");
Preconditions.checkNotNull(key);
return setResult(key, AppSearchResult.newSuccessfulResult(result));
}
@@ -130,10 +150,13 @@ public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
*
* <p>Any previous mapping for a key, whether success or failure, is deleted.
*/
public Builder setFailure(
@NonNull
public Builder<KeyType, ValueType> setFailure(
@NonNull KeyType key,
@AppSearchResult.ResultCode int resultCode,
@Nullable String errorMessage) {
Preconditions.checkState(!mBuilt, "Builder has already been used");
Preconditions.checkNotNull(key);
return setResult(key, AppSearchResult.newFailedResult(resultCode, errorMessage));
}
@@ -143,7 +166,11 @@ public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
* <p>Any previous mapping for a key, whether success or failure, is deleted.
*/
@NonNull
public Builder setResult(@NonNull KeyType key, @NonNull AppSearchResult<ValueType> result) {
public Builder<KeyType, ValueType> setResult(
@NonNull KeyType key, @NonNull AppSearchResult<ValueType> result) {
Preconditions.checkState(!mBuilt, "Builder has already been used");
Preconditions.checkNotNull(key);
Preconditions.checkNotNull(result);
if (result.isSuccess()) {
mSuccesses.put(key, result.getResultValue());
mFailures.remove(key);
@@ -157,6 +184,8 @@ public class AppSearchBatchResult<KeyType, ValueType> implements Parcelable {
/** Builds an {@link AppSearchBatchResult} from the contents of this {@link Builder}. */
@NonNull
public AppSearchBatchResult<KeyType, ValueType> build() {
Preconditions.checkState(!mBuilt, "Builder has already been used");
mBuilt = true;
return new AppSearchBatchResult<>(mSuccesses, mFailures);
}
}

32
apex/appsearch/framework/java/android/app/appsearch/AppSearchResult.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright (C) 2020 The Android Open Source Project
* Copyright 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -32,9 +32,12 @@ import java.util.Objects;
* @param <ValueType> The type of result object for successful calls.
* @hide
*/
public class AppSearchResult<ValueType> implements Parcelable {
/** Result codes from {@link AppSearchManager} methods. */
@IntDef(prefix = {"RESULT_"}, value = {
public final class AppSearchResult<ValueType> implements Parcelable {
/**
* Result codes from {@link AppSearchManager} methods.
* @hide
*/
@IntDef(value = {
RESULT_OK,
RESULT_UNKNOWN_ERROR,
RESULT_INTERNAL_ERROR,
@@ -120,15 +123,18 @@ public class AppSearchResult<ValueType> implements Parcelable {
}
/**
* Returns the returned value associated with this result.
* Returns the result value associated with this result, if it was successful.
*
* <p>If {@link #isSuccess} is {@code false}, the result value is always {@code null}. The value
* may be {@code null} even if {@link #isSuccess} is {@code true}. See the documentation of the
* particular {@link AppSearchManager} call producing this {@link AppSearchResult} for what is
* returned by {@link #getResultValue}.
* <p>See the documentation of the particular {@link AppSearchManager} call producing this
* {@link AppSearchResult} for what is placed in the result value by that call.
*
* @throws IllegalStateException if this {@link AppSearchResult} is not successful.
*/
@Nullable
public ValueType getResultValue() {
if (!isSuccess()) {
throw new IllegalStateException("AppSearchResult is a failure: " + this);
}
return mResultValue;
}
@@ -146,14 +152,14 @@ public class AppSearchResult<ValueType> implements Parcelable {
}
@Override
public boolean equals(Object other) {
public boolean equals(@Nullable Object other) {
if (this == other) {
return true;
}
if (!(other instanceof AppSearchResult)) {
return false;
}
AppSearchResult<?> otherResult = (AppSearchResult) other;
AppSearchResult<?> otherResult = (AppSearchResult<?>) other;
return mResultCode == otherResult.mResultCode
&& Objects.equals(mResultValue, otherResult.mResultValue)
&& Objects.equals(mErrorMessage, otherResult.mErrorMessage);
@@ -168,9 +174,9 @@ public class AppSearchResult<ValueType> implements Parcelable {
@NonNull
public String toString() {
if (isSuccess()) {
return "AppSearchResult [SUCCESS]: " + mResultValue;
return "[SUCCESS]: " + mResultValue;
}
return "AppSearchResult [FAILURE(" + mResultCode + ")]: " + mErrorMessage;
return "[FAILURE(" + mResultCode + ")]: " + mErrorMessage;
}
@Override

4
apex/appsearch/framework/java/android/app/appsearch/exceptions/AppSearchException.java
View File

@@ -54,6 +54,10 @@ public class AppSearchException extends Exception {
mResultCode = resultCode;
}
public @AppSearchResult.ResultCode int getResultCode() {
return mResultCode;
}
/**
* Converts this {@link java.lang.Exception} into a failed {@link AppSearchResult}
*/

4
apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java
View File

@@ -69,9 +69,7 @@ public final class ImplInstanceManager {
private static AppSearchImpl createImpl(@NonNull Context context, @UserIdInt int userId)
throws AppSearchException {
File appSearchDir = getAppSearchDir(context, userId);
AppSearchImpl appSearchImpl = new AppSearchImpl(appSearchDir);
appSearchImpl.initialize();
return appSearchImpl;
return AppSearchImpl.create(appSearchDir);
}
private static File getAppSearchDir(@NonNull Context context, @UserIdInt int userId) {

135
apex/appsearch/service/java/com/android/server/appsearch/external/localbackend/AppSearchImpl.java vendored
View File

@@ -18,7 +18,6 @@ package com.android.server.appsearch.external.localbackend;
import android.util.Log;
import android.annotation.AnyThread;
import com.android.internal.annotations.GuardedBy;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -27,6 +26,7 @@ import com.android.internal.annotations.VisibleForTesting;
import android.annotation.WorkerThread;
import android.app.appsearch.AppSearchResult;
import android.app.appsearch.exceptions.AppSearchException;
import com.android.internal.util.Preconditions;
import com.google.android.icing.IcingSearchEngine;
import com.google.android.icing.proto.DeleteByNamespaceResultProto;
@@ -58,7 +58,6 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -66,8 +65,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
* Manages interaction with the native IcingSearchEngine and other components to implement AppSearch
* functionality.
*
* <p>Callers should call {@link #initialize} before using the AppSearchImpl instance. Never create
* two instances using the same folder.
* <p>Never create two instances using the same folder.
*
* <p>A single instance of {@link AppSearchImpl} can support all databases. Schemas and documents
* are physically saved together in {@link IcingSearchEngine}, but logically isolated:
@@ -106,9 +104,7 @@ public final class AppSearchImpl {
static final int CHECK_OPTIMIZE_INTERVAL = 100;
private final ReadWriteLock mReadWriteLock = new ReentrantReadWriteLock();
private final CountDownLatch mInitCompleteLatch = new CountDownLatch(1);
private final File mIcingDir;
private IcingSearchEngine mIcingSearchEngine;
private final IcingSearchEngine mIcingSearchEngine;
// The map contains schemaTypes and namespaces for all database. All values in the map have
// been already added database name prefix.
@@ -121,33 +117,24 @@ public final class AppSearchImpl {
*/
private int mOptimizeIntervalCount = 0;
/** Creates an instance of {@link AppSearchImpl} which writes data to the given folder. */
@AnyThread
public AppSearchImpl(@NonNull File icingDir) {
mIcingDir = icingDir;
/**
* Creates and initializes an instance of {@link AppSearchImpl} which writes data to the given
* folder.
*/
@NonNull
public static AppSearchImpl create(@NonNull File icingDir) throws AppSearchException {
Preconditions.checkNotNull(icingDir);
return new AppSearchImpl(icingDir);
}
/**
* Initializes the underlying IcingSearchEngine.
*
* <p>This method belongs to mutate group.
*
* @throws AppSearchException on IcingSearchEngine error.
*/
public void initialize() throws AppSearchException {
if (isInitialized()) {
return;
}
private AppSearchImpl(@NonNull File icingDir) throws AppSearchException {
boolean isReset = false;
mReadWriteLock.writeLock().lock();
try {
// We synchronize here because we don't want to call IcingSearchEngine.initialize() more
// than once. It's unnecessary and can be a costly operation.
if (isInitialized()) {
return;
}
// We synchronize here because we don't want to call IcingSearchEngine.initialize() more
// than once. It's unnecessary and can be a costly operation.
IcingSearchEngineOptions options = IcingSearchEngineOptions.newBuilder()
.setBaseDir(mIcingDir.getAbsolutePath()).build();
.setBaseDir(icingDir.getAbsolutePath()).build();
mIcingSearchEngine = new IcingSearchEngine(options);
InitializeResultProto initializeResultProto = mIcingSearchEngine.initialize();
@@ -170,7 +157,8 @@ public final class AppSearchImpl {
for (String qualifiedNamespace : getAllNamespacesResultProto.getNamespacesList()) {
addToMap(mNamespaceMap, getDatabaseName(qualifiedNamespace), qualifiedNamespace);
}
mInitCompleteLatch.countDown();
// TODO(b/155939114): It's possible to optimize after init, which would reduce the time
// to when we're able to serve queries. Consider moving this optimize call out.
if (!isReset) {
checkForOptimize(/* force= */ true);
}
@@ -179,12 +167,6 @@ public final class AppSearchImpl {
}
}
/** Checks if the internal state of {@link AppSearchImpl} has been initialized. */
@AnyThread
public boolean isInitialized() {
return mInitCompleteLatch.getCount() == 0;
}
/**
* Updates the AppSearch schema for this app.
*
@@ -195,12 +177,9 @@ public final class AppSearchImpl {
* @param forceOverride Whether to force-apply the schema even if it is incompatible. Documents
* which do not comply with the new schema will be deleted.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
public void setSchema(@NonNull String databaseName, @NonNull SchemaProto origSchema,
boolean forceOverride) throws AppSearchException, InterruptedException {
awaitInitialized();
boolean forceOverride) throws AppSearchException {
SchemaProto schemaProto = getSchemaProto();
SchemaProto.Builder existingSchemaBuilder = schemaProto.toBuilder();
@@ -212,10 +191,32 @@ public final class AppSearchImpl {
SetSchemaResultProto setSchemaResultProto;
mReadWriteLock.writeLock().lock();
try {
setSchemaResultProto = mIcingSearchEngine.setSchema(existingSchemaBuilder.build(),
forceOverride);
checkSuccess(setSchemaResultProto.getStatus());
// Apply schema
setSchemaResultProto =
mIcingSearchEngine.setSchema(existingSchemaBuilder.build(), forceOverride);
// Determine whether it succeeded.
try {
checkSuccess(setSchemaResultProto.getStatus());
} catch (AppSearchException e) {
// Improve the error message by merging in information about incompatible types.
if (setSchemaResultProto.getDeletedSchemaTypesCount() > 0
|| setSchemaResultProto.getIncompatibleSchemaTypesCount() > 0) {
String newMessage = e.getMessage()
+ "\n Deleted types: "
+ setSchemaResultProto.getDeletedSchemaTypesList()
+ "\n Incompatible types: "
+ setSchemaResultProto.getIncompatibleSchemaTypesList();
throw new AppSearchException(e.getResultCode(), newMessage, e.getCause());
} else {
throw e;
}
}
// Update derived data structures.
mSchemaMap.put(databaseName, newTypeNames);
// Determine whether to schedule an immediate optimize.
if (setSchemaResultProto.getDeletedSchemaTypesCount() > 0
|| (setSchemaResultProto.getIncompatibleSchemaTypesCount() > 0
&& forceOverride)) {
@@ -237,12 +238,9 @@ public final class AppSearchImpl {
* @param databaseName The databaseName this document resides in.
* @param document The document to index.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
public void putDocument(@NonNull String databaseName, @NonNull DocumentProto document)
throws AppSearchException, InterruptedException {
awaitInitialized();
throws AppSearchException {
DocumentProto.Builder documentBuilder = document.toBuilder();
rewriteDocumentTypes(getDatabasePrefix(databaseName), documentBuilder, /*add=*/ true);
@@ -270,12 +268,10 @@ public final class AppSearchImpl {
* @param uri The URI of the document to get.
* @return The Document contents, or {@code null} if no such URI exists in the system.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
@Nullable
public DocumentProto getDocument(@NonNull String databaseName, @NonNull String namespace,
@NonNull String uri) throws AppSearchException, InterruptedException {
awaitInitialized();
@NonNull String uri) throws AppSearchException {
GetResultProto getResultProto;
mReadWriteLock.readLock().lock();
try {
@@ -303,16 +299,13 @@ public final class AppSearchImpl {
* @return The results of performing this search The proto might have no {@code results} if no
* documents matched the query.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
@NonNull
public SearchResultProto query(
@NonNull String databaseName,
@NonNull SearchSpecProto searchSpec,
@NonNull ResultSpecProto resultSpec,
@NonNull ScoringSpecProto scoringSpec) throws AppSearchException, InterruptedException {
awaitInitialized();
@NonNull ScoringSpecProto scoringSpec) throws AppSearchException {
SearchSpecProto.Builder searchSpecBuilder = searchSpec.toBuilder();
SearchResultProto searchResultProto;
mReadWriteLock.readLock().lock();
@@ -347,13 +340,10 @@ public final class AppSearchImpl {
* @param nextPageToken The token of pre-loaded results of previously executed query.
* @return The next page of results of previously executed query.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
@NonNull
public SearchResultProto getNextPage(@NonNull String databaseName, long nextPageToken)
throws AppSearchException, InterruptedException {
awaitInitialized();
throws AppSearchException {
SearchResultProto searchResultProto = mIcingSearchEngine.getNextPage(nextPageToken);
checkSuccess(searchResultProto.getStatus());
if (searchResultProto.getResultsCount() == 0) {
@@ -367,8 +357,7 @@ public final class AppSearchImpl {
* @param nextPageToken The token of pre-loaded results of previously executed query to be
* Invalidated.
*/
public void invalidateNextPageToken(long nextPageToken) throws InterruptedException {
awaitInitialized();
public void invalidateNextPageToken(long nextPageToken) {
mIcingSearchEngine.invalidateNextPageToken(nextPageToken);
}
@@ -381,12 +370,9 @@ public final class AppSearchImpl {
* @param namespace Namespace of the document to remove.
* @param uri URI of the document to remove.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
public void remove(@NonNull String databaseName, @NonNull String namespace,
@NonNull String uri) throws AppSearchException, InterruptedException {
awaitInitialized();
@NonNull String uri) throws AppSearchException {
String qualifiedNamespace = getDatabasePrefix(databaseName) + namespace;
DeleteResultProto deleteResultProto;
mReadWriteLock.writeLock().lock();
@@ -407,12 +393,9 @@ public final class AppSearchImpl {
* @param databaseName The databaseName that contains documents of schemaType.
* @param schemaType The schemaType of documents to remove.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
public void removeByType(@NonNull String databaseName, @NonNull String schemaType)
throws AppSearchException, InterruptedException {
awaitInitialized();
throws AppSearchException {
String qualifiedType = getDatabasePrefix(databaseName) + schemaType;
DeleteBySchemaTypeResultProto deleteBySchemaTypeResultProto;
mReadWriteLock.writeLock().lock();
@@ -437,12 +420,9 @@ public final class AppSearchImpl {
* @param databaseName The databaseName that contains documents of namespace.
* @param namespace The namespace of documents to remove.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
public void removeByNamespace(@NonNull String databaseName, @NonNull String namespace)
throws AppSearchException, InterruptedException {
awaitInitialized();
throws AppSearchException {
String qualifiedNamespace = getDatabasePrefix(databaseName) + namespace;
DeleteByNamespaceResultProto deleteByNamespaceResultProto;
mReadWriteLock.writeLock().lock();
@@ -469,11 +449,9 @@ public final class AppSearchImpl {
*
* @param databaseName The databaseName to remove all documents from.
* @throws AppSearchException on IcingSearchEngine error.
* @throws InterruptedException if the current thread was interrupted during execution.
*/
public void removeAll(@NonNull String databaseName)
throws AppSearchException, InterruptedException {
awaitInitialized();
throws AppSearchException {
mReadWriteLock.writeLock().lock();
try {
Set<String> existingNamespaces = mNamespaceMap.get(databaseName);
@@ -732,15 +710,6 @@ public final class AppSearchImpl {
values.add(prefixedValue);
}
/**
* Waits for the instance to become initialized.
*
* @throws InterruptedException if the current thread was interrupted during waiting.
*/
private void awaitInitialized() throws InterruptedException {
mInitCompleteLatch.await();
}
/**
* Checks the given status code and throws an {@link AppSearchException} if code is an error.
*

6
services/tests/servicestests/src/com/android/server/appsearch/external/localbackend/AppSearchImplTest.java vendored
View File

@@ -50,8 +50,7 @@ public class AppSearchImplTest {
@Before
public void setUp() throws Exception {
mAppSearchImpl = new AppSearchImpl(mTemporaryFolder.newFolder());
mAppSearchImpl.initialize();
mAppSearchImpl = AppSearchImpl.create(mTemporaryFolder.newFolder());
}
/**
@@ -288,7 +287,8 @@ public class AppSearchImplTest {
SchemaProto finalSchemaProto = schemaProto;
AppSearchException e = expectThrows(AppSearchException.class, () ->
mAppSearchImpl.setSchema("database1", finalSchemaProto, /*forceOverride=*/false));
assertThat(e).hasMessageThat().isEqualTo("Schema is incompatible.");
assertThat(e).hasMessageThat().contains("Schema is incompatible");
assertThat(e).hasMessageThat().contains("Deleted types: [database1/Document]");
// ForceOverride to delete.
mAppSearchImpl.setSchema("database1", finalSchemaProto, /*forceOverride=*/true);