Update Framework from Jetpack.
Changes included:
* 517c1ee: Schema Migration API updates to match API council recommendations.
* 407144d: Refactor some migration helper methods to Utils class.
* 2297b3a: Add a test for migrate to a nonexistent schema.
* 16a38d0: Log the PutDocumentStats in AppSearchImpl
Bug: 178060626
Bug: 179804862
Bug: 173532925
Test: Presubmit
Change-Id: I8b879bc9a5f30f9be3ce08d07dc60f4b177fb11b
diff --git a/service/java/com/android/server/appsearch/AppSearchManagerService.java b/service/java/com/android/server/appsearch/AppSearchManagerService.java
index 27c9ccb..77aa14c 100644
--- a/service/java/com/android/server/appsearch/AppSearchManagerService.java
+++ b/service/java/com/android/server/appsearch/AppSearchManagerService.java
@@ -193,7 +193,7 @@
try {
// TODO(b/173451571): reduce burden of binder thread by enqueue request onto
// a separate thread.
- impl.putDocument(packageName, databaseName, document);
+ impl.putDocument(packageName, databaseName, document, /*logger=*/ null);
resultBuilder.setSuccess(document.getUri(), /*result=*/ null);
} catch (Throwable t) {
resultBuilder.setResult(document.getUri(), throwableToFailedResult(t));
diff --git a/service/java/com/android/server/appsearch/VisibilityStore.java b/service/java/com/android/server/appsearch/VisibilityStore.java
index 7c92456..ad94a0a 100644
--- a/service/java/com/android/server/appsearch/VisibilityStore.java
+++ b/service/java/com/android/server/appsearch/VisibilityStore.java
@@ -368,7 +368,8 @@
packageAccessibleDocuments.toArray(new GenericDocument[0]));
}
- mAppSearchImpl.putDocument(PACKAGE_NAME, DATABASE_NAME, visibilityDocument.build());
+ mAppSearchImpl.putDocument(
+ PACKAGE_NAME, DATABASE_NAME, visibilityDocument.build(), /*logger=*/ null);
// Update derived data structures.
mNotPlatformSurfaceableMap.put(prefix, schemasNotPlatformSurfaceable);
diff --git a/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java b/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
index e2c211b..5e8760e 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
@@ -17,6 +17,7 @@
package com.android.server.appsearch.external.localstorage;
import android.annotation.NonNull;
+import android.annotation.Nullable;
import android.annotation.WorkerThread;
import android.app.appsearch.AppSearchResult;
import android.app.appsearch.AppSearchSchema;
@@ -29,6 +30,7 @@
import android.app.appsearch.exceptions.AppSearchException;
import android.content.Context;
import android.os.Bundle;
+import android.os.SystemClock;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
@@ -43,6 +45,7 @@
import com.android.server.appsearch.external.localstorage.converter.SearchSpecToProtoConverter;
import com.android.server.appsearch.external.localstorage.converter.SetSchemaResponseToProtoConverter;
import com.android.server.appsearch.external.localstorage.converter.TypePropertyPathToProtoConverter;
+import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats;
import com.google.android.icing.IcingSearchEngine;
import com.google.android.icing.proto.DeleteByQueryResultProto;
@@ -449,23 +452,65 @@
public void putDocument(
@NonNull String packageName,
@NonNull String databaseName,
- @NonNull GenericDocument document)
+ @NonNull GenericDocument document,
+ @Nullable AppSearchLogger logger)
throws AppSearchException {
+ PutDocumentStats.Builder pStatsBuilder = null;
+ if (logger != null) {
+ pStatsBuilder = new PutDocumentStats.Builder(packageName, databaseName);
+ }
+ long totalStartTimeMillis = SystemClock.elapsedRealtime();
+
mReadWriteLock.writeLock().lock();
try {
throwIfClosedLocked();
+ // Generate Document Proto
+ long generateDocumentProtoStartTimeMillis = SystemClock.elapsedRealtime();
DocumentProto.Builder documentBuilder =
GenericDocumentToProtoConverter.toDocumentProto(document).toBuilder();
+ long generateDocumentProtoEndTimeMillis = SystemClock.elapsedRealtime();
+
+ // Rewrite Document Type
+ long rewriteDocumentTypeStartTimeMillis = SystemClock.elapsedRealtime();
String prefix = createPrefix(packageName, databaseName);
addPrefixToDocument(documentBuilder, prefix);
+ long rewriteDocumentTypeEndTimeMillis = SystemClock.elapsedRealtime();
PutResultProto putResultProto = mIcingSearchEngineLocked.put(documentBuilder.build());
addToMap(mNamespaceMapLocked, prefix, documentBuilder.getNamespace());
+ // Logging stats
+ if (logger != null) {
+ pStatsBuilder
+ .getGeneralStatsBuilder()
+ .setStatusCode(
+ statusProtoToAppSearchException(putResultProto.getStatus())
+ .getResultCode());
+ pStatsBuilder
+ .setGenerateDocumentProtoLatencyMillis(
+ (int)
+ (generateDocumentProtoEndTimeMillis
+ - generateDocumentProtoStartTimeMillis))
+ .setRewriteDocumentTypesLatencyMillis(
+ (int)
+ (rewriteDocumentTypeEndTimeMillis
+ - rewriteDocumentTypeStartTimeMillis));
+ AppSearchLoggerHelper.copyNativeStats(
+ putResultProto.getPutDocumentStats(), pStatsBuilder);
+ }
+
checkSuccess(putResultProto.getStatus());
} finally {
mReadWriteLock.writeLock().unlock();
+
+ if (logger != null) {
+ long totalEndTimeMillis = SystemClock.elapsedRealtime();
+ pStatsBuilder
+ .getGeneralStatsBuilder()
+ .setTotalLatencyMillis((int) (totalEndTimeMillis - totalStartTimeMillis));
+ logger.logStats(pStatsBuilder.build());
+ }
}
}
diff --git a/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java b/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java
new file mode 100644
index 0000000..5680670
--- /dev/null
+++ b/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2021 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.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.appsearch.external.localstorage;
+
+import android.annotation.NonNull;
+
+import com.android.internal.util.Preconditions;
+import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats;
+
+import com.google.android.icing.proto.PutDocumentStatsProto;
+
+/**
+ * Class contains helper functions for logging.
+ *
+ * <p>E.g. we need to have helper functions to copy numbers from IcingLib to stats classes.
+ *
+ * @hide
+ */
+public final class AppSearchLoggerHelper {
+ private AppSearchLoggerHelper() {}
+
+ /**
+ * Copies native stats to builder.
+ *
+ * @param fromNativeStats stats copied from
+ * @param toStatsBuilder stats copied to
+ */
+ static void copyNativeStats(
+ @NonNull PutDocumentStatsProto fromNativeStats,
+ @NonNull PutDocumentStats.Builder toStatsBuilder) {
+ Preconditions.checkNotNull(fromNativeStats);
+ Preconditions.checkNotNull(toStatsBuilder);
+ toStatsBuilder
+ .setNativeLatencyMillis(fromNativeStats.getLatencyMs())
+ .setNativeDocumentStoreLatencyMillis(fromNativeStats.getDocumentStoreLatencyMs())
+ .setNativeIndexLatencyMillis(fromNativeStats.getIndexLatencyMs())
+ .setNativeIndexMergeLatencyMillis(fromNativeStats.getIndexMergeLatencyMs())
+ .setNativeDocumentSizeBytes(fromNativeStats.getDocumentSize())
+ .setNativeNumTokensIndexed(
+ fromNativeStats.getTokenizationStats().getNumTokensIndexed())
+ .setNativeExceededMaxNumTokens(
+ fromNativeStats.getTokenizationStats().getExceededMaxTokenNum());
+ }
+}
diff --git a/service/java/com/android/server/appsearch/external/localstorage/AppSearchMigrationHelperImpl.java b/service/java/com/android/server/appsearch/external/localstorage/AppSearchMigrationHelperImpl.java
deleted file mode 100644
index 4b8ce6d..0000000
--- a/service/java/com/android/server/appsearch/external/localstorage/AppSearchMigrationHelperImpl.java
+++ /dev/null
@@ -1,168 +0,0 @@
-/*
- * Copyright 2021 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.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.server.appsearch.external.localstorage;
-
-import static android.app.appsearch.AppSearchResult.throwableToFailedResult;
-
-import android.annotation.NonNull;
-import android.app.appsearch.AppSearchBatchResult;
-import android.app.appsearch.AppSearchMigrationHelper;
-import android.app.appsearch.GenericDocument;
-import android.app.appsearch.SearchResultPage;
-import android.app.appsearch.SearchSpec;
-import android.app.appsearch.SetSchemaResponse;
-import android.app.appsearch.exceptions.AppSearchException;
-import android.os.Bundle;
-import android.os.Parcel;
-
-import com.android.internal.util.Preconditions;
-
-import com.google.protobuf.CodedInputStream;
-import com.google.protobuf.CodedOutputStream;
-
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.Map;
-
-/**
- * An implementation of {@link AppSearchMigrationHelper} which query document and save post-migrated
- * documents to locally in the app's storage space.
- */
-class AppSearchMigrationHelperImpl implements AppSearchMigrationHelper {
- private final AppSearchImpl mAppSearchImpl;
- private final String mPackageName;
- private final String mDatabaseName;
- private final File mFile;
- private final Map<String, Integer> mCurrentVersionMap;
- private final Map<String, Integer> mFinalVersionMap;
-
- AppSearchMigrationHelperImpl(
- @NonNull AppSearchImpl appSearchImpl,
- @NonNull Map<String, Integer> currentVersionMap,
- @NonNull Map<String, Integer> finalVersionMap,
- @NonNull String packageName,
- @NonNull String databaseName)
- throws IOException {
- mAppSearchImpl = Preconditions.checkNotNull(appSearchImpl);
- mCurrentVersionMap = Preconditions.checkNotNull(currentVersionMap);
- mFinalVersionMap = Preconditions.checkNotNull(finalVersionMap);
- mPackageName = Preconditions.checkNotNull(packageName);
- mDatabaseName = Preconditions.checkNotNull(databaseName);
- mFile = File.createTempFile(/*prefix=*/ "appsearch", /*suffix=*/ null);
- }
-
- @Override
- public void queryAndTransform(
- @NonNull String schemaType, @NonNull AppSearchMigrationHelper.Transformer migrator)
- throws Exception {
- Preconditions.checkState(mFile.exists(), "Internal temp file does not exist.");
- int currentVersion = mCurrentVersionMap.get(schemaType);
- int finalVersion = mFinalVersionMap.get(schemaType);
- try (FileOutputStream outputStream = new FileOutputStream(mFile)) {
- // TODO(b/151178558) change the output stream so that we can use it in platform
- CodedOutputStream codedOutputStream = CodedOutputStream.newInstance(outputStream);
- SearchResultPage searchResultPage =
- mAppSearchImpl.query(
- mPackageName,
- mDatabaseName,
- /*queryExpression=*/ "",
- new SearchSpec.Builder()
- .addFilterSchemas(schemaType)
- .setTermMatch(SearchSpec.TERM_MATCH_PREFIX)
- .build());
- while (!searchResultPage.getResults().isEmpty()) {
- for (int i = 0; i < searchResultPage.getResults().size(); i++) {
- GenericDocument newDocument =
- migrator.transform(
- currentVersion,
- finalVersion,
- searchResultPage.getResults().get(i).getGenericDocument());
- Bundle bundle = newDocument.getBundle();
- Parcel parcel = Parcel.obtain();
- parcel.writeBundle(bundle);
- byte[] serializedMessage = parcel.marshall();
- parcel.recycle();
- codedOutputStream.writeByteArrayNoTag(serializedMessage);
- }
- codedOutputStream.flush();
- searchResultPage = mAppSearchImpl.getNextPage(searchResultPage.getNextPageToken());
- outputStream.flush();
- }
- }
- }
-
- /**
- * Reads {@link GenericDocument} from the temperate file and saves them to AppSearch.
- *
- * <p>This method should be only called once.
- *
- * @return the {@link AppSearchBatchResult} for migration documents.
- */
- @NonNull
- public SetSchemaResponse readAndPutDocuments(SetSchemaResponse.Builder responseBuilder)
- throws IOException, AppSearchException {
- Preconditions.checkState(mFile.exists(), "Internal temp file does not exist.");
- try (InputStream inputStream = new FileInputStream(mFile)) {
- CodedInputStream codedInputStream = CodedInputStream.newInstance(inputStream);
- while (!codedInputStream.isAtEnd()) {
- GenericDocument document = readDocumentFromInputStream(codedInputStream);
- try {
- mAppSearchImpl.putDocument(mPackageName, mDatabaseName, document);
- } catch (Throwable t) {
- responseBuilder.addMigrationFailure(
- new SetSchemaResponse.MigrationFailure.Builder()
- .setNamespace(document.getNamespace())
- .setSchemaType(document.getSchemaType())
- .setUri(document.getUri())
- .setAppSearchResult(throwableToFailedResult(t))
- .build());
- }
- }
- mAppSearchImpl.persistToDisk();
- return responseBuilder.build();
- } finally {
- mFile.delete();
- }
- }
-
- void deleteTempFile() {
- mFile.delete();
- }
-
- /**
- * Reads {@link GenericDocument} from given {@link CodedInputStream}.
- *
- * @param codedInputStream The codedInputStream to read from
- * @throws IOException on File operation error.
- */
- @NonNull
- private static GenericDocument readDocumentFromInputStream(
- @NonNull CodedInputStream codedInputStream) throws IOException {
- byte[] serializedMessage = codedInputStream.readByteArray();
-
- Parcel parcel = Parcel.obtain();
- parcel.unmarshall(serializedMessage, 0, serializedMessage.length);
- parcel.setDataPosition(0);
- Bundle bundle = parcel.readBundle();
- parcel.recycle();
-
- return new GenericDocument(bundle);
- }
-}
diff --git a/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java b/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java
index 81a5067..a724f95 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java
@@ -76,7 +76,7 @@
CallStats(@NonNull Builder builder) {
Preconditions.checkNotNull(builder);
- mGeneralStats = Preconditions.checkNotNull(builder.mGeneralStats);
+ mGeneralStats = Preconditions.checkNotNull(builder.mGeneralStatsBuilder).build();
mCallType = builder.mCallType;
mEstimatedBinderLatencyMillis = builder.mEstimatedBinderLatencyMillis;
mNumOperationsSucceeded = builder.mNumOperationsSucceeded;
@@ -132,15 +132,23 @@
/** Builder for {@link CallStats}. */
public static class Builder {
- @NonNull final GeneralStats mGeneralStats;
+ @NonNull final GeneralStats.Builder mGeneralStatsBuilder;
@CallType int mCallType;
int mEstimatedBinderLatencyMillis;
int mNumOperationsSucceeded;
int mNumOperationsFailed;
- /** Builder takes {@link GeneralStats} to hold general stats. */
- public Builder(@NonNull GeneralStats generalStats) {
- mGeneralStats = Preconditions.checkNotNull(generalStats);
+ /** Builder takes {@link GeneralStats.Builder}. */
+ public Builder(@NonNull String packageName, @NonNull String database) {
+ Preconditions.checkNotNull(packageName);
+ Preconditions.checkNotNull(database);
+ mGeneralStatsBuilder = new GeneralStats.Builder(packageName, database);
+ }
+
+ /** Returns {@link GeneralStats.Builder}. */
+ @NonNull
+ public GeneralStats.Builder getGeneralStatsBuilder() {
+ return mGeneralStatsBuilder;
}
/** Sets type of the call. */
diff --git a/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java b/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java
index d2a45d5..8ce8eda 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java
@@ -82,18 +82,18 @@
public static class Builder {
@NonNull final String mPackageName;
@NonNull final String mDatabase;
- @AppSearchResult.ResultCode int mStatusCode;
+ @AppSearchResult.ResultCode int mStatusCode = AppSearchResult.RESULT_UNKNOWN_ERROR;
int mTotalLatencyMillis;
/**
* Constructor
*
* @param packageName name of the package logging stats
- * @param dataBase name of the database logging stats
+ * @param database name of the database logging stats
*/
- public Builder(@NonNull String packageName, @NonNull String dataBase) {
+ public Builder(@NonNull String packageName, @NonNull String database) {
mPackageName = Preconditions.checkNotNull(packageName);
- mDatabase = Preconditions.checkNotNull(dataBase);
+ mDatabase = Preconditions.checkNotNull(database);
}
/** Sets status code returned from {@link AppSearchResult#getResultCode()} */
diff --git a/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java b/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java
index b1b643b..c1f6fb1 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java
@@ -54,12 +54,14 @@
/** Number of tokens added to the index. */
private final int mNativeNumTokensIndexed;
- /** Number of tokens clipped for exceeding the max number. */
- private final int mNativeNumTokensClipped;
+ /**
+ * Whether the number of tokens to be indexed exceeded the max number of tokens per document.
+ */
+ private final boolean mNativeExceededMaxNumTokens;
PutDocumentStats(@NonNull Builder builder) {
Preconditions.checkNotNull(builder);
- mGeneralStats = Preconditions.checkNotNull(builder.mGeneralStats);
+ mGeneralStats = Preconditions.checkNotNull(builder.mGeneralStatsBuilder).build();
mGenerateDocumentProtoLatencyMillis = builder.mGenerateDocumentProtoLatencyMillis;
mRewriteDocumentTypesLatencyMillis = builder.mRewriteDocumentTypesLatencyMillis;
mNativeLatencyMillis = builder.mNativeLatencyMillis;
@@ -68,7 +70,7 @@
mNativeIndexMergeLatencyMillis = builder.mNativeIndexMergeLatencyMillis;
mNativeDocumentSizeBytes = builder.mNativeDocumentSizeBytes;
mNativeNumTokensIndexed = builder.mNativeNumTokensIndexed;
- mNativeNumTokensClipped = builder.mNativeNumTokensClipped;
+ mNativeExceededMaxNumTokens = builder.mNativeExceededMaxNumTokens;
}
/** Returns the {@link GeneralStats} object attached to this instance. */
@@ -117,14 +119,17 @@
return mNativeNumTokensIndexed;
}
- /** Returns number of tokens clipped for exceeding the max number. */
- public int getNativeNumTokensClipped() {
- return mNativeNumTokensClipped;
+ /**
+ * Returns whether the number of tokens to be indexed exceeded the max number of tokens per
+ * document.
+ */
+ public boolean getNativeExceededMaxNumTokens() {
+ return mNativeExceededMaxNumTokens;
}
/** Builder for {@link PutDocumentStats}. */
public static class Builder {
- @NonNull final GeneralStats mGeneralStats;
+ @NonNull final GeneralStats.Builder mGeneralStatsBuilder;
int mGenerateDocumentProtoLatencyMillis;
int mRewriteDocumentTypesLatencyMillis;
int mNativeLatencyMillis;
@@ -133,11 +138,19 @@
int mNativeIndexMergeLatencyMillis;
int mNativeDocumentSizeBytes;
int mNativeNumTokensIndexed;
- int mNativeNumTokensClipped;
+ boolean mNativeExceededMaxNumTokens;
- /** Builder takes {@link GeneralStats} to hold general stats. */
- public Builder(@NonNull GeneralStats generalStats) {
- mGeneralStats = Preconditions.checkNotNull(generalStats);
+ /** Builder takes {@link GeneralStats.Builder}. */
+ public Builder(@NonNull String packageName, @NonNull String database) {
+ Preconditions.checkNotNull(packageName);
+ Preconditions.checkNotNull(database);
+ mGeneralStatsBuilder = new GeneralStats.Builder(packageName, database);
+ }
+
+ /** Returns {@link GeneralStats.Builder}. */
+ @NonNull
+ public GeneralStats.Builder getGeneralStatsBuilder() {
+ return mGeneralStatsBuilder;
}
/** Sets how much time we spend for generating document proto, in milliseconds. */
@@ -200,10 +213,13 @@
return this;
}
- /** Sets number of tokens clipped for exceeding the max number. */
+ /**
+ * Sets whether the number of tokens to be indexed exceeded the max number of tokens per
+ * document.
+ */
@NonNull
- public Builder setNativeNumTokensClipped(int nativeNumTokensClipped) {
- mNativeNumTokensClipped = nativeNumTokensClipped;
+ public Builder setNativeExceededMaxNumTokens(boolean nativeExceededMaxNumTokens) {
+ mNativeExceededMaxNumTokens = nativeExceededMaxNumTokens;
return this;
}