diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchBatchResult.java b/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchBatchResult.java index f0b04fccd4063..272e12db01249 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchBatchResult.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchBatchResult.java @@ -19,6 +19,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.util.ArrayMap; +import java.util.Collections; import java.util.Map; import java.util.Objects; @@ -48,9 +49,9 @@ public final class AppSearchBatchResult { @NonNull Map successes, @NonNull Map> failures, @NonNull Map> all) { - mSuccesses = successes; - mFailures = failures; - mAll = all; + mSuccesses = Objects.requireNonNull(successes); + mFailures = Objects.requireNonNull(failures); + mAll = Objects.requireNonNull(all); } /** Returns {@code true} if this {@link AppSearchBatchResult} has no failures. */ @@ -70,7 +71,7 @@ public final class AppSearchBatchResult { */ @NonNull public Map getSuccesses() { - return mSuccesses; + return Collections.unmodifiableMap(mSuccesses); } /** @@ -81,7 +82,7 @@ public final class AppSearchBatchResult { */ @NonNull public Map> getFailures() { - return mFailures; + return Collections.unmodifiableMap(mFailures); } /** @@ -92,7 +93,7 @@ public final class AppSearchBatchResult { */ @NonNull public Map> getAll() { - return mAll; + return Collections.unmodifiableMap(mAll); } /** @@ -128,20 +129,37 @@ public final class AppSearchBatchResult { * Associates the {@code key} with the provided successful return value. * *

Any previous mapping for a key, whether success or failure, is deleted. + * + *

This is a convenience function which is equivalent to {@code setResult(key, + * AppSearchResult.newSuccessfulResult(value))}. + * + * @param key The key to associate the result with; usually corresponds to some identifier + * from the input like an ID or name. + * @param value An optional value to associate with the successful result of the operation + * being performed. */ @SuppressWarnings("MissingGetterMatchingBuilder") // See getSuccesses @NonNull public Builder setSuccess( - @NonNull KeyType key, @Nullable ValueType result) { + @NonNull KeyType key, @Nullable ValueType value) { Objects.requireNonNull(key); resetIfBuilt(); - return setResult(key, AppSearchResult.newSuccessfulResult(result)); + return setResult(key, AppSearchResult.newSuccessfulResult(value)); } /** * Associates the {@code key} with the provided failure code and error message. * *

Any previous mapping for a key, whether success or failure, is deleted. + * + *

This is a convenience function which is equivalent to {@code setResult(key, + * AppSearchResult.newFailedResult(resultCode, errorMessage))}. + * + * @param key The key to associate the result with; usually corresponds to some identifier + * from the input like an ID or name. + * @param resultCode One of the constants documented in {@link + * AppSearchResult#getResultCode}. + * @param errorMessage An optional string describing the reason or nature of the failure. */ @SuppressWarnings("MissingGetterMatchingBuilder") // See getFailures @NonNull @@ -158,6 +176,10 @@ public final class AppSearchBatchResult { * Associates the {@code key} with the provided {@code result}. * *

Any previous mapping for a key, whether success or failure, is deleted. + * + * @param key The key to associate the result with; usually corresponds to some identifier + * from the input like an ID or name. + * @param result The result to associate with the key. */ @SuppressWarnings("MissingGetterMatchingBuilder") // See getAll @NonNull @@ -183,8 +205,7 @@ public final class AppSearchBatchResult { @NonNull public AppSearchBatchResult build() { mBuilt = true; - return new AppSearchBatchResult<>( - new ArrayMap<>(mSuccesses), new ArrayMap<>(mFailures), new ArrayMap<>(mAll)); + return new AppSearchBatchResult<>(mSuccesses, mFailures, mAll); } private void resetIfBuilt() { diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchResult.java b/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchResult.java index 30c98b0c88662..c57cf2e68993f 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchResult.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchResult.java @@ -175,14 +175,24 @@ public final class AppSearchResult { return "[FAILURE(" + mResultCode + ")]: " + mErrorMessage; } - /** Creates a new successful {@link AppSearchResult}. */ + /** + * Creates a new successful {@link AppSearchResult}. + * + * @param value An optional value to associate with the successful result of the operation being + * performed. + */ @NonNull public static AppSearchResult newSuccessfulResult( @Nullable ValueType value) { return new AppSearchResult<>(RESULT_OK, value, /*errorMessage=*/ null); } - /** Creates a new failed {@link AppSearchResult}. */ + /** + * Creates a new failed {@link AppSearchResult}. + * + * @param resultCode One of the constants documented in {@link AppSearchResult#getResultCode}. + * @param errorMessage An optional string describing the reason or nature of the failure. + */ @NonNull public static AppSearchResult newFailedResult( @ResultCode int resultCode, @Nullable String errorMessage) { diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/GetByDocumentIdRequest.java b/apex/appsearch/framework/java/external/android/app/appsearch/GetByDocumentIdRequest.java index e324b9f81d9c2..558899e493ae2 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/GetByDocumentIdRequest.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/GetByDocumentIdRequest.java @@ -164,8 +164,7 @@ public final class GetByDocumentIdRequest { @NonNull public GetByDocumentIdRequest build() { mBuilt = true; - return new GetByDocumentIdRequest( - mNamespace, new ArraySet<>(mIds), new ArrayMap<>(mProjectionTypePropertyPaths)); + return new GetByDocumentIdRequest(mNamespace, mIds, mProjectionTypePropertyPaths); } private void resetIfBuilt() { diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/ReportSystemUsageRequest.java b/apex/appsearch/framework/java/external/android/app/appsearch/ReportSystemUsageRequest.java index b6575c2030ff0..26bdf031a5b0e 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/ReportSystemUsageRequest.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/ReportSystemUsageRequest.java @@ -93,14 +93,25 @@ public final class ReportSystemUsageRequest { private final String mDocumentId; private Long mUsageTimestampMillis; - /** Creates a {@link ReportSystemUsageRequest.Builder} instance. */ + /** + * Creates a {@link ReportSystemUsageRequest.Builder} instance. + * + * @param packageName The package name of the app which owns the document that was used + * (e.g. from {@link SearchResult#getPackageName}). + * @param databaseName The database in which the document that was used resides (e.g. from + * {@link SearchResult#getDatabaseName}). + * @param namespace The namespace of the document that was used (e.g. from {@link + * GenericDocument#getNamespace}. + * @param documentId The ID of document that was used (e.g. from {@link + * GenericDocument#getId}. + */ public Builder( @NonNull String packageName, - @NonNull String database, + @NonNull String databaseName, @NonNull String namespace, @NonNull String documentId) { mPackageName = Objects.requireNonNull(packageName); - mDatabase = Objects.requireNonNull(database); + mDatabase = Objects.requireNonNull(databaseName); mNamespace = Objects.requireNonNull(namespace); mDocumentId = Objects.requireNonNull(documentId); } diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/ReportUsageRequest.java b/apex/appsearch/framework/java/external/android/app/appsearch/ReportUsageRequest.java index 59fa6e0743f65..c388bdebb00d1 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/ReportUsageRequest.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/ReportUsageRequest.java @@ -71,7 +71,14 @@ public final class ReportUsageRequest { private String mDocumentId; private Long mUsageTimestampMillis; - /** Creates a {@link ReportUsageRequest.Builder} instance. */ + /** + * Creates a new {@link ReportUsageRequest.Builder} instance. + * + * @param namespace The namespace of the document that was used (e.g. from {@link + * GenericDocument#getNamespace}. + * @param documentId The ID of document that was used (e.g. from {@link + * GenericDocument#getId}. + */ public Builder(@NonNull String namespace, @NonNull String documentId) { mNamespace = Objects.requireNonNull(namespace); mDocumentId = Objects.requireNonNull(documentId); diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaResponse.java b/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaResponse.java index 2c75b71f36065..3e5a2ca246e0b 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaResponse.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaResponse.java @@ -375,5 +375,19 @@ public class SetSchemaResponse { mBundle.getInt(RESULT_CODE_FIELD), mBundle.getString(ERROR_MESSAGE_FIELD, /*defaultValue=*/ "")); } + + @NonNull + @Override + public String toString() { + return "MigrationFailure { schemaType: " + + getSchemaType() + + ", namespace: " + + getNamespace() + + ", documentId: " + + getDocumentId() + + ", appSearchResult: " + + getAppSearchResult().toString() + + "}"; + } } } diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/util/LogUtil.java b/apex/appsearch/framework/java/external/android/app/appsearch/util/LogUtil.java new file mode 100644 index 0000000000000..f2cc3b969a60d --- /dev/null +++ b/apex/appsearch/framework/java/external/android/app/appsearch/util/LogUtil.java @@ -0,0 +1,104 @@ +/* + * 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 android.app.appsearch.util; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.util.Log; + +import java.util.Objects; + +/** + * Utilities for logging to logcat. + * + * @hide + */ +public final class LogUtil { + /** + * The {@link #piiTrace} logs are intended for sensitive data that can't be enabled in + * production, so they are build-gated by this constant. + * + *

+ * + *

    + *
  • 0: no tracing. + *
  • 1: fast tracing (statuses/counts only) + *
  • 2: full tracing (complete messages) + *
+ */ + private static final int PII_TRACE_LEVEL = 0; + + private final String mTag; + + public LogUtil(@NonNull String tag) { + mTag = Objects.requireNonNull(tag); + } + + /** Returns whether piiTrace() is enabled (PII_TRACE_LEVEL > 0). */ + public boolean isPiiTraceEnabled() { + return PII_TRACE_LEVEL > 0; + } + + /** + * If icing lib interaction tracing is enabled via {@link #PII_TRACE_LEVEL}, logs the provided + * message to logcat. + * + *

If {@link #PII_TRACE_LEVEL} is 0, nothing is logged and this method returns immediately. + */ + public void piiTrace(@NonNull String message) { + piiTrace(message, /*fastTraceObj=*/ null, /*fullTraceObj=*/ null); + } + + /** + * If icing lib interaction tracing is enabled via {@link #PII_TRACE_LEVEL}, logs the provided + * message and object to logcat. + * + *

If {@link #PII_TRACE_LEVEL} is 0, nothing is logged and this method returns immediately. + * + *

Otherwise, {@code traceObj} is logged if it is non-null. + */ + public void piiTrace(@NonNull String message, @Nullable Object traceObj) { + piiTrace(message, /*fastTraceObj=*/ traceObj, /*fullTraceObj=*/ null); + } + + /** + * If icing lib interaction tracing is enabled via {@link #PII_TRACE_LEVEL}, logs the provided + * message and objects to logcat. + * + *

If {@link #PII_TRACE_LEVEL} is 0, nothing is logged and this method returns immediately. + * + *

If {@link #PII_TRACE_LEVEL} is 1, {@code fastTraceObj} is logged if it is non-null. + * + *

If {@link #PII_TRACE_LEVEL} is 2, {@code fullTraceObj} is logged if it is non-null, else + * {@code fastTraceObj} is logged if it is non-null.. + */ + public void piiTrace( + @NonNull String message, @Nullable Object fastTraceObj, @Nullable Object fullTraceObj) { + if (PII_TRACE_LEVEL == 0) { + return; + } + StringBuilder builder = new StringBuilder("(trace) ").append(message); + if (PII_TRACE_LEVEL == 1 && fastTraceObj != null) { + builder.append(": ").append(fastTraceObj); + } else if (PII_TRACE_LEVEL == 2 && fullTraceObj != null) { + builder.append(": ").append(fullTraceObj); + } else if (PII_TRACE_LEVEL == 2 && fastTraceObj != null) { + builder.append(": ").append(fastTraceObj); + } + Log.i(mTag, builder.toString()); + } +} diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java index e5e20e733264a..1cb0cc51de346 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java @@ -28,6 +28,7 @@ import static com.android.server.appsearch.external.localstorage.util.PrefixUtil import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.WorkerThread; +import android.app.appsearch.AppSearchResult; import android.app.appsearch.AppSearchSchema; import android.app.appsearch.GenericDocument; import android.app.appsearch.GetByDocumentIdRequest; @@ -38,6 +39,7 @@ import android.app.appsearch.SearchSpec; import android.app.appsearch.SetSchemaResponse; import android.app.appsearch.StorageInfo; import android.app.appsearch.exceptions.AppSearchException; +import android.app.appsearch.util.LogUtil; import android.content.Context; import android.os.Bundle; import android.os.SystemClock; @@ -147,8 +149,11 @@ public final class AppSearchImpl implements Closeable { private final ReadWriteLock mReadWriteLock = new ReentrantReadWriteLock(); + private final LogUtil mLogUtil = new LogUtil(TAG); + @GuardedBy("mReadWriteLock") - private final IcingSearchEngine mIcingSearchEngineLocked; + @VisibleForTesting + final IcingSearchEngine mIcingSearchEngineLocked; @GuardedBy("mReadWriteLock") private final VisibilityStore mVisibilityStoreLocked; @@ -216,7 +221,7 @@ public final class AppSearchImpl implements Closeable { appSearchImpl.initializeVisibilityStore(); long prepareVisibilityStoreLatencyEndMillis = SystemClock.elapsedRealtime(); - if (logger != null && initStatsBuilder != null) { + if (logger != null) { initStatsBuilder .setTotalLatencyMillis( (int) (SystemClock.elapsedRealtime() - totalLatencyStartMillis)) @@ -247,66 +252,91 @@ public final class AppSearchImpl implements Closeable { IcingSearchEngineOptions.newBuilder() .setBaseDir(icingDir.getAbsolutePath()) .build(); + mLogUtil.piiTrace("Constructing IcingSearchEngine, request", options); mIcingSearchEngineLocked = new IcingSearchEngine(options); + mLogUtil.piiTrace( + "Constructing IcingSearchEngine, response", + Objects.hashCode(mIcingSearchEngineLocked)); + mVisibilityStoreLocked = new VisibilityStore(this, context, userId, globalQuerierPackage); - InitializeResultProto initializeResultProto = mIcingSearchEngineLocked.initialize(); - if (initStatsBuilder != null) { - initStatsBuilder - .setStatusCode( - statusProtoToAppSearchException(initializeResultProto.getStatus()) - .getResultCode()) - // TODO(b/173532925) how to get DeSyncs value - .setHasDeSync(false); - AppSearchLoggerHelper.copyNativeStats( - initializeResultProto.getInitializeStats(), initStatsBuilder); - } - - long prepareSchemaAndNamespacesLatencyStartMillis = SystemClock.elapsedRealtime(); - SchemaProto schemaProto; - GetAllNamespacesResultProto getAllNamespacesResultProto = null; + // The core initialization procedure. If any part of this fails, we bail into + // resetLocked(), deleting all data (but hopefully allowing AppSearchImpl to come up). try { - checkSuccess(initializeResultProto.getStatus()); - schemaProto = getSchemaProtoLocked(); - getAllNamespacesResultProto = mIcingSearchEngineLocked.getAllNamespaces(); - checkSuccess(getAllNamespacesResultProto.getStatus()); - } catch (AppSearchException e) { - Log.w(TAG, "Error initializing, resetting IcingSearchEngine.", e); - if (initStatsBuilder != null && getAllNamespacesResultProto != null) { + mLogUtil.piiTrace("icingSearchEngine.initialize, request"); + InitializeResultProto initializeResultProto = mIcingSearchEngineLocked.initialize(); + mLogUtil.piiTrace( + "icingSearchEngine.initialize, response", + initializeResultProto.getStatus(), + initializeResultProto); + + if (initStatsBuilder != null) { initStatsBuilder .setStatusCode( - statusProtoToAppSearchException( - getAllNamespacesResultProto.getStatus()) - .getResultCode()) + statusProtoToResultCode(initializeResultProto.getStatus())) + // TODO(b/173532925) how to get DeSyncs value + .setHasDeSync(false); + AppSearchLoggerHelper.copyNativeStats( + initializeResultProto.getInitializeStats(), initStatsBuilder); + } + + checkSuccess(initializeResultProto.getStatus()); + + long prepareSchemaAndNamespacesLatencyStartMillis = SystemClock.elapsedRealtime(); + SchemaProto schemaProto = getSchemaProtoLocked(); + + mLogUtil.piiTrace("init:getAllNamespaces, request"); + GetAllNamespacesResultProto getAllNamespacesResultProto = + mIcingSearchEngineLocked.getAllNamespaces(); + mLogUtil.piiTrace( + "init:getAllNamespaces, response", + getAllNamespacesResultProto.getNamespacesCount(), + getAllNamespacesResultProto); + + if (initStatsBuilder != null) { + initStatsBuilder + .setStatusCode( + statusProtoToResultCode( + getAllNamespacesResultProto.getStatus())) .setPrepareSchemaAndNamespacesLatencyMillis( (int) (SystemClock.elapsedRealtime() - prepareSchemaAndNamespacesLatencyStartMillis)); } + + checkSuccess(getAllNamespacesResultProto.getStatus()); + + // Populate schema map + for (SchemaTypeConfigProto schema : schemaProto.getTypesList()) { + String prefixedSchemaType = schema.getSchemaType(); + addToMap(mSchemaMapLocked, getPrefix(prefixedSchemaType), schema); + } + + // Populate namespace map + for (String prefixedNamespace : getAllNamespacesResultProto.getNamespacesList()) { + addToMap(mNamespaceMapLocked, getPrefix(prefixedNamespace), prefixedNamespace); + } + + // logging prepare_schema_and_namespaces latency + if (initStatsBuilder != null) { + initStatsBuilder.setPrepareSchemaAndNamespacesLatencyMillis( + (int) + (SystemClock.elapsedRealtime() + - prepareSchemaAndNamespacesLatencyStartMillis)); + } + + mLogUtil.piiTrace("Init completed successfully"); + + } catch (AppSearchException e) { // Some error. Reset and see if it fixes it. - resetLocked(); - return; + Log.e(TAG, "Error initializing, resetting IcingSearchEngine.", e); + if (initStatsBuilder != null) { + initStatsBuilder.setStatusCode(e.getResultCode()); + } + resetLocked(initStatsBuilder); } - // Populate schema map - for (SchemaTypeConfigProto schema : schemaProto.getTypesList()) { - String prefixedSchemaType = schema.getSchemaType(); - addToMap(mSchemaMapLocked, getPrefix(prefixedSchemaType), schema); - } - - // Populate namespace map - for (String prefixedNamespace : getAllNamespacesResultProto.getNamespacesList()) { - addToMap(mNamespaceMapLocked, getPrefix(prefixedNamespace), prefixedNamespace); - } - - // logging prepare_schema_and_namespaces latency - if (initStatsBuilder != null) { - initStatsBuilder.setPrepareSchemaAndNamespacesLatencyMillis( - (int) - (SystemClock.elapsedRealtime() - - prepareSchemaAndNamespacesLatencyStartMillis)); - } } finally { mReadWriteLock.writeLock().unlock(); } @@ -321,8 +351,9 @@ public final class AppSearchImpl implements Closeable { mReadWriteLock.writeLock().lock(); try { throwIfClosedLocked(); - + mLogUtil.piiTrace("Initializing VisibilityStore, request"); mVisibilityStoreLocked.initialize(); + mLogUtil.piiTrace("Initializing VisibilityStore, response"); } finally { mReadWriteLock.writeLock().unlock(); } @@ -348,9 +379,10 @@ public final class AppSearchImpl implements Closeable { if (mClosedLocked) { return; } - persistToDisk(PersistType.Code.FULL); + mLogUtil.piiTrace("icingSearchEngine.close, request"); mIcingSearchEngineLocked.close(); + mLogUtil.piiTrace("icingSearchEngine.close, response"); mClosedLocked = true; } catch (AppSearchException e) { Log.w(TAG, "Error when closing AppSearchImpl.", e); @@ -410,9 +442,12 @@ public final class AppSearchImpl implements Closeable { rewriteSchema(prefix, existingSchemaBuilder, newSchemaBuilder.build()); // Apply schema + SchemaProto finalSchema = existingSchemaBuilder.build(); + mLogUtil.piiTrace("setSchema, request", finalSchema.getTypesCount(), finalSchema); SetSchemaResultProto setSchemaResultProto = - mIcingSearchEngineLocked.setSchema( - existingSchemaBuilder.build(), forceOverride); + mIcingSearchEngineLocked.setSchema(finalSchema, forceOverride); + mLogUtil.piiTrace( + "setSchema, response", setSchemaResultProto.getStatus(), setSchemaResultProto); // Determine whether it succeeded. try { @@ -544,11 +579,16 @@ public final class AppSearchImpl implements Closeable { mReadWriteLock.readLock().lock(); try { throwIfClosedLocked(); + mLogUtil.piiTrace("getAllNamespaces, request"); // We can't just use mNamespaceMap here because we have no way to prune namespaces from // mNamespaceMap when they have no more documents (e.g. after setting schema to empty or // using deleteByQuery). GetAllNamespacesResultProto getAllNamespacesResultProto = mIcingSearchEngineLocked.getAllNamespaces(); + mLogUtil.piiTrace( + "getAllNamespaces, response", + getAllNamespacesResultProto.getNamespacesCount(), + getAllNamespacesResultProto); checkSuccess(getAllNamespacesResultProto.getStatus()); String prefix = createPrefix(packageName, databaseName); List results = new ArrayList<>(); @@ -601,17 +641,18 @@ public final class AppSearchImpl implements Closeable { String prefix = createPrefix(packageName, databaseName); addPrefixToDocument(documentBuilder, prefix); long rewriteDocumentTypeEndTimeMillis = SystemClock.elapsedRealtime(); + DocumentProto finalDocument = documentBuilder.build(); + mLogUtil.piiTrace("putDocument, request", finalDocument.getUri(), finalDocument); PutResultProto putResultProto = mIcingSearchEngineLocked.put(documentBuilder.build()); + mLogUtil.piiTrace("putDocument, response", putResultProto.getStatus(), putResultProto); addToMap(mNamespaceMapLocked, prefix, documentBuilder.getNamespace()); // Logging stats - if (logger != null && pStatsBuilder != null) { + if (pStatsBuilder != null) { pStatsBuilder .getGeneralStatsBuilder() - .setStatusCode( - statusProtoToAppSearchException(putResultProto.getStatus()) - .getResultCode()); + .setStatusCode(statusProtoToResultCode(putResultProto.getStatus())); pStatsBuilder .setGenerateDocumentProtoLatencyMillis( (int) @@ -629,7 +670,7 @@ public final class AppSearchImpl implements Closeable { } finally { mReadWriteLock.writeLock().unlock(); - if (logger != null && pStatsBuilder != null) { + if (logger != null) { long totalEndTimeMillis = SystemClock.elapsedRealtime(); pStatsBuilder .getGeneralStatsBuilder() @@ -685,8 +726,14 @@ public final class AppSearchImpl implements Closeable { .addAllTypePropertyMasks(prefixedPropertyMasks) .build(); + String finalNamespace = createPrefix(packageName, databaseName) + namespace; + if (mLogUtil.isPiiTraceEnabled()) { + mLogUtil.piiTrace( + "getDocument, request", finalNamespace + ", " + id + "," + getResultSpec); + } GetResultProto getResultProto = - mIcingSearchEngineLocked.get(prefix + namespace, id, getResultSpec); + mIcingSearchEngineLocked.get(finalNamespace, id, getResultSpec); + mLogUtil.piiTrace("getDocument, response", getResultProto.getStatus(), getResultProto); checkSuccess(getResultProto.getStatus()); // The schema type map cannot be null at this point. It could only be null if no @@ -754,7 +801,7 @@ public final class AppSearchImpl implements Closeable { sStatsBuilder); } finally { mReadWriteLock.readLock().unlock(); - if (logger != null && sStatsBuilder != null) { + if (logger != null) { sStatsBuilder.setTotalLatencyMillis( (int) (SystemClock.elapsedRealtime() - totalLatencyStartMillis)); logger.logStats(sStatsBuilder.build()); @@ -832,9 +879,10 @@ public final class AppSearchImpl implements Closeable { } } else { // Client didn't specify certain schemas to search over, check all schemas - Set prefixedSchemas = mSchemaMapLocked.get(prefix).keySet(); + Map prefixedSchemas = + mSchemaMapLocked.get(prefix); if (prefixedSchemas != null) { - for (String prefixedSchema : prefixedSchemas) { + for (String prefixedSchema : prefixedSchemas.keySet()) { if (packageName.equals(callerPackageName) || mVisibilityStoreLocked.isSchemaSearchableByCaller( prefix, prefixedSchema, callerUid)) { @@ -854,7 +902,7 @@ public final class AppSearchImpl implements Closeable { } finally { mReadWriteLock.readLock().unlock(); - if (logger != null && sStatsBuilder != null) { + if (logger != null) { sStatsBuilder.setTotalLatencyMillis( (int) (SystemClock.elapsedRealtime() - totalLatencyStartMillis)); logger.logStats(sStatsBuilder.build()); @@ -938,18 +986,25 @@ public final class AppSearchImpl implements Closeable { rewriteResultSpecForPrefixesLocked(resultSpecBuilder, prefixes, allowedPrefixedSchemas); ScoringSpecProto scoringSpec = SearchSpecToProtoConverter.toScoringSpecProto(searchSpec); + SearchSpecProto finalSearchSpec = searchSpecBuilder.build(); + ResultSpecProto finalResultSpec = resultSpecBuilder.build(); long rewriteSearchSpecLatencyEndMillis = SystemClock.elapsedRealtime(); + if (mLogUtil.isPiiTraceEnabled()) { + mLogUtil.piiTrace( + "search, request", + finalSearchSpec.getQuery(), + finalSearchSpec + ", " + scoringSpec + ", " + finalResultSpec); + } SearchResultProto searchResultProto = - mIcingSearchEngineLocked.search( - searchSpecBuilder.build(), scoringSpec, resultSpecBuilder.build()); + mIcingSearchEngineLocked.search(finalSearchSpec, scoringSpec, finalResultSpec); + mLogUtil.piiTrace( + "search, response", searchResultProto.getResultsCount(), searchResultProto); if (sStatsBuilder != null) { sStatsBuilder - .setStatusCode( - statusProtoToAppSearchException(searchResultProto.getStatus()) - .getResultCode()) + .setStatusCode(statusProtoToResultCode(searchResultProto.getStatus())) .setRewriteSearchSpecLatencyMillis( (int) (rewriteSearchSpecLatencyEndMillis @@ -985,8 +1040,13 @@ public final class AppSearchImpl implements Closeable { try { throwIfClosedLocked(); + mLogUtil.piiTrace("getNextPage, request", nextPageToken); SearchResultProto searchResultProto = mIcingSearchEngineLocked.getNextPage(nextPageToken); + mLogUtil.piiTrace( + "getNextPage, response", + searchResultProto.getResultsCount(), + searchResultProto); checkSuccess(searchResultProto.getStatus()); return rewriteSearchResultProto(searchResultProto, mSchemaMapLocked); } finally { @@ -1007,6 +1067,7 @@ public final class AppSearchImpl implements Closeable { try { throwIfClosedLocked(); + mLogUtil.piiTrace("invalidateNextPageToken, request", nextPageToken); mIcingSearchEngineLocked.invalidateNextPageToken(nextPageToken); } finally { mReadWriteLock.readLock().unlock(); @@ -1039,7 +1100,9 @@ public final class AppSearchImpl implements Closeable { .setUsageType(usageType) .build(); + mLogUtil.piiTrace("reportUsage, request", report.getDocumentUri(), report); ReportUsageResultProto result = mIcingSearchEngineLocked.reportUsage(report); + mLogUtil.piiTrace("reportUsage, response", result.getStatus(), result); checkSuccess(result.getStatus()); } finally { mReadWriteLock.writeLock().unlock(); @@ -1068,9 +1131,13 @@ public final class AppSearchImpl implements Closeable { throwIfClosedLocked(); String prefixedNamespace = createPrefix(packageName, databaseName) + namespace; + if (mLogUtil.isPiiTraceEnabled()) { + mLogUtil.piiTrace("removeById, request", prefixedNamespace + ", " + id); + } DeleteResultProto deleteResultProto = mIcingSearchEngineLocked.delete(prefixedNamespace, id); - + mLogUtil.piiTrace( + "removeById, response", deleteResultProto.getStatus(), deleteResultProto); checkSuccess(deleteResultProto.getStatus()); } finally { mReadWriteLock.writeLock().unlock(); @@ -1121,8 +1188,12 @@ public final class AppSearchImpl implements Closeable { searchSpecBuilder, Collections.singleton(prefix), allowedPrefixedSchemas)) { return; } + SearchSpecProto finalSearchSpec = searchSpecBuilder.build(); + mLogUtil.piiTrace("removeByQuery, request", finalSearchSpec); DeleteByQueryResultProto deleteResultProto = - mIcingSearchEngineLocked.deleteByQuery(searchSpecBuilder.build()); + mIcingSearchEngineLocked.deleteByQuery(finalSearchSpec); + mLogUtil.piiTrace( + "removeByQuery, response", deleteResultProto.getStatus(), deleteResultProto); // It seems that the caller wants to get success if the data matching the query is // not in the DB because it was not there or was successfully deleted. @@ -1202,7 +1273,10 @@ public final class AppSearchImpl implements Closeable { @NonNull private StorageInfo getStorageInfoForNamespacesLocked(@NonNull Set prefixedNamespaces) throws AppSearchException { + mLogUtil.piiTrace("getStorageInfo, request"); StorageInfoResultProto storageInfoResult = mIcingSearchEngineLocked.getStorageInfo(); + mLogUtil.piiTrace( + "getStorageInfo, response", storageInfoResult.getStatus(), storageInfoResult); checkSuccess(storageInfoResult.getStatus()); if (!storageInfoResult.hasStorageInfo() || !storageInfoResult.getStorageInfo().hasDocumentStorageInfo()) { @@ -1277,8 +1351,13 @@ public final class AppSearchImpl implements Closeable { try { throwIfClosedLocked(); + mLogUtil.piiTrace("persistToDisk, request", persistType); PersistToDiskResultProto persistToDiskResultProto = mIcingSearchEngineLocked.persistToDisk(persistType); + mLogUtil.piiTrace( + "persistToDisk, response", + persistToDiskResultProto.getStatus(), + persistToDiskResultProto); checkSuccess(persistToDiskResultProto.getStatus()); } finally { mReadWriteLock.writeLock().unlock(); @@ -1305,12 +1384,21 @@ public final class AppSearchImpl implements Closeable { newSchemaBuilder.addTypes(existingSchema.getTypes(i)); } } + SchemaProto finalSchema = newSchemaBuilder.build(); // Apply schema, set force override to true to remove all schemas and documents under // that package. + mLogUtil.piiTrace( + "clearPackageData.setSchema, request", + finalSchema.getTypesCount(), + finalSchema); SetSchemaResultProto setSchemaResultProto = mIcingSearchEngineLocked.setSchema( - newSchemaBuilder.build(), /*ignoreErrorsAndDeleteDocuments=*/ true); + finalSchema, /*ignoreErrorsAndDeleteDocuments=*/ true); + mLogUtil.piiTrace( + "clearPackageData.setSchema, response", + setSchemaResultProto.getStatus(), + setSchemaResultProto); // Determine whether it succeeded. checkSuccess(setSchemaResultProto.getStatus()); @@ -1330,12 +1418,24 @@ public final class AppSearchImpl implements Closeable { * @throws AppSearchException on IcingSearchEngine error. */ @GuardedBy("mReadWriteLock") - private void resetLocked() throws AppSearchException { + private void resetLocked(@Nullable InitializeStats.Builder initStatsBuilder) + throws AppSearchException { + mLogUtil.piiTrace("icingSearchEngine.reset, request"); ResetResultProto resetResultProto = mIcingSearchEngineLocked.reset(); + mLogUtil.piiTrace( + "icingSearchEngine.reset, response", + resetResultProto.getStatus(), + resetResultProto); mOptimizeIntervalCountLocked = 0; mSchemaMapLocked.clear(); mNamespaceMapLocked.clear(); + if (initStatsBuilder != null) { + initStatsBuilder + .setHasReset(true) + .setResetStatusCode(statusProtoToResultCode(resetResultProto.getStatus())); + } + // Must be called after everything else since VisibilityStore may repopulate // IcingSearchEngine with an initial schema. mVisibilityStoreLocked.handleReset(); @@ -1472,15 +1572,17 @@ public final class AppSearchImpl implements Closeable { // Empty namespaces on the search spec means to query over all namespaces. Set existingNamespaces = mNamespaceMapLocked.get(prefix); - if (namespaceFilters.isEmpty()) { - // Include all namespaces - searchSpecBuilder.addAllNamespaceFilters(existingNamespaces); - } else { - // Prefix the given namespaces. - for (int i = 0; i < namespaceFilters.size(); i++) { - String prefixedNamespace = prefix + namespaceFilters.get(i); - if (existingNamespaces.contains(prefixedNamespace)) { - searchSpecBuilder.addNamespaceFilters(prefixedNamespace); + if (existingNamespaces != null) { + if (namespaceFilters.isEmpty()) { + // Include all namespaces + searchSpecBuilder.addAllNamespaceFilters(existingNamespaces); + } else { + // Prefix the given namespaces. + for (int i = 0; i < namespaceFilters.size(); i++) { + String prefixedNamespace = prefix + namespaceFilters.get(i); + if (existingNamespaces.contains(prefixedNamespace)) { + searchSpecBuilder.addNamespaceFilters(prefixedNamespace); + } } } } @@ -1581,6 +1683,9 @@ public final class AppSearchImpl implements Closeable { Map> packageAndNamespaceToNamespaces = new ArrayMap<>(); for (String prefix : existingPrefixes) { Set prefixedNamespaces = mNamespaceMapLocked.get(prefix); + if (prefixedNamespaces == null) { + continue; + } String packageName = getPackageName(prefix); // Create a new prefix without the database name. This will allow us to group namespaces // that have the same name and package but a different database name together. @@ -1636,6 +1741,9 @@ public final class AppSearchImpl implements Closeable { Map> packageToNamespacesMap = new ArrayMap<>(); for (String prefix : existingPrefixes) { Set prefixedNamespaces = mNamespaceMapLocked.get(prefix); + if (prefixedNamespaces == null) { + continue; + } String packageName = getPackageName(prefix); List packageNamespaceList = packageToNamespacesMap.get(packageName); if (packageNamespaceList == null) { @@ -1677,6 +1785,9 @@ public final class AppSearchImpl implements Closeable { Map> namespaceToPrefixedNamespaces = new ArrayMap<>(); for (String prefix : existingPrefixes) { Set prefixedNamespaces = mNamespaceMapLocked.get(prefix); + if (prefixedNamespaces == null) { + continue; + } for (String prefixedNamespace : prefixedNamespaces) { String namespace; try { @@ -1707,7 +1818,9 @@ public final class AppSearchImpl implements Closeable { @VisibleForTesting @GuardedBy("mReadWriteLock") SchemaProto getSchemaProtoLocked() throws AppSearchException { + mLogUtil.piiTrace("getSchema, request"); GetSchemaResultProto schemaProto = mIcingSearchEngineLocked.getSchema(); + mLogUtil.piiTrace("getSchema, response", schemaProto.getStatus(), schemaProto); // TODO(b/161935693) check GetSchemaResultProto is success or not. Call reset() if it's not. // TODO(b/161935693) only allow GetSchemaResultProto NOT_FOUND on first run checkCodeOneOf(schemaProto.getStatus(), StatusProto.Code.OK, StatusProto.Code.NOT_FOUND); @@ -1781,7 +1894,9 @@ public final class AppSearchImpl implements Closeable { return; } - throw statusProtoToAppSearchException(statusProto); + throw new AppSearchException( + ResultCodeToProtoConverter.toResultCode(statusProto.getCode()), + statusProto.getMessage()); } /** @@ -1849,7 +1964,10 @@ public final class AppSearchImpl implements Closeable { public void optimize() throws AppSearchException { mReadWriteLock.writeLock().lock(); try { + mLogUtil.piiTrace("optimize, request"); OptimizeResultProto optimizeResultProto = mIcingSearchEngineLocked.optimize(); + mLogUtil.piiTrace( + "optimize, response", optimizeResultProto.getStatus(), optimizeResultProto); checkSuccess(optimizeResultProto.getStatus()); } finally { mReadWriteLock.writeLock().unlock(); @@ -1887,7 +2005,10 @@ public final class AppSearchImpl implements Closeable { @GuardedBy("mReadWriteLock") @VisibleForTesting GetOptimizeInfoResultProto getOptimizeInfoResultLocked() { - return mIcingSearchEngineLocked.getOptimizeInfo(); + mLogUtil.piiTrace("getOptimizeInfo, request"); + GetOptimizeInfoResultProto result = mIcingSearchEngineLocked.getOptimizeInfo(); + mLogUtil.piiTrace("getOptimizeInfo, response", result.getStatus(), result); + return result; } @GuardedBy("mReadWriteLock") @@ -1898,16 +2019,16 @@ public final class AppSearchImpl implements Closeable { } /** - * Converts an erroneous status code to an AppSearchException. Callers should ensure that the - * status code is not OK or WARNING_DATA_LOSS. + * Converts an erroneous status code from the Icing status enums to the AppSearchResult enums. * - * @param statusProto StatusProto with error code and message to translate into - * AppSearchException. - * @return AppSearchException with the parallel error code. + *

Callers should ensure that the status code is not OK or WARNING_DATA_LOSS. + * + * @param statusProto StatusProto with error code to translate into an {@link AppSearchResult} + * code. + * @return {@link AppSearchResult} error code */ - private static AppSearchException statusProtoToAppSearchException(StatusProto statusProto) { - return new AppSearchException( - ResultCodeToProtoConverter.toResultCode(statusProto.getCode()), - statusProto.getMessage()); + private static @AppSearchResult.ResultCode int statusProtoToResultCode( + @NonNull StatusProto statusProto) { + return ResultCodeToProtoConverter.toResultCode(statusProto.getCode()); } } diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/InitializeStats.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/InitializeStats.java index 5364a0caa1aaf..72befa795d9fb 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/InitializeStats.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/InitializeStats.java @@ -106,6 +106,10 @@ public final class InitializeStats { private final int mNativeNumDocuments; /** Returns number of schema types currently in the schema store. */ private final int mNativeNumSchemaTypes; + /** Whether we had to reset the index, losing all data, during initialization. */ + private final boolean mHasReset; + /** If we had to reset, contains the status code of the reset operation. */ + @AppSearchResult.ResultCode private final int mResetStatusCode; /** Returns the status of the initialization. */ @AppSearchResult.ResultCode @@ -214,6 +218,21 @@ public final class InitializeStats { return mNativeNumSchemaTypes; } + /** Returns whether we had to reset the index, losing all data, as part of initialization. */ + public boolean hasReset() { + return mHasReset; + } + + /** + * Returns the status of the reset, if one was performed according to {@link #hasReset}. + * + *

If no value has been set, the default value is {@link AppSearchResult#RESULT_OK}. + */ + @AppSearchResult.ResultCode + public int getResetStatusCode() { + return mResetStatusCode; + } + InitializeStats(@NonNull Builder builder) { Objects.requireNonNull(builder); mStatusCode = builder.mStatusCode; @@ -232,11 +251,14 @@ public final class InitializeStats { mNativeDocumentStoreDataStatus = builder.mNativeDocumentStoreDataStatus; mNativeNumDocuments = builder.mNativeNumDocuments; mNativeNumSchemaTypes = builder.mNativeNumSchemaTypes; + mHasReset = builder.mHasReset; + mResetStatusCode = builder.mResetStatusCode; } /** Builder for {@link InitializeStats}. */ public static class Builder { @AppSearchResult.ResultCode int mStatusCode; + int mTotalLatencyMillis; boolean mHasDeSync; int mPrepareSchemaAndNamespacesLatencyMillis; @@ -251,6 +273,8 @@ public final class InitializeStats { @DocumentStoreDataStatus int mNativeDocumentStoreDataStatus; int mNativeNumDocuments; int mNativeNumSchemaTypes; + boolean mHasReset; + @AppSearchResult.ResultCode int mResetStatusCode; /** Sets the status of the initialization. */ @NonNull @@ -392,6 +416,20 @@ public final class InitializeStats { return this; } + /** Sets whether we had to reset the index, losing all data, as part of initialization. */ + @NonNull + public Builder setHasReset(boolean hasReset) { + mHasReset = hasReset; + return this; + } + + /** Sets the status of the reset, if one was performed according to {@link #setHasReset}. */ + @NonNull + public Builder setResetStatusCode(@AppSearchResult.ResultCode int resetStatusCode) { + mResetStatusCode = resetStatusCode; + return this; + } + /** * Constructs a new {@link InitializeStats} from the contents of this {@link * InitializeStats.Builder} diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/util/PrefixUtil.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/util/PrefixUtil.java index 9ae9f18528492..80cfe89ffc1c6 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/util/PrefixUtil.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/util/PrefixUtil.java @@ -112,8 +112,11 @@ public class PrefixUtil { return prefixedString.substring(delimiterIndex + 1); } throw new AppSearchException( - AppSearchResult.RESULT_UNKNOWN_ERROR, - "The prefixed value doesn't contains a valid database name."); + AppSearchResult.RESULT_INTERNAL_ERROR, + "The prefixed value \"" + + prefixedString + + "\" doesn't contain a valid " + + "database name"); } /** @@ -128,8 +131,11 @@ public class PrefixUtil { int databaseDelimiterIndex = prefixedString.indexOf(DATABASE_DELIMITER); if (databaseDelimiterIndex == -1) { throw new AppSearchException( - AppSearchResult.RESULT_UNKNOWN_ERROR, - "The databaseName prefixed value doesn't contain a valid database name."); + AppSearchResult.RESULT_INTERNAL_ERROR, + "The prefixed value \"" + + prefixedString + + "\" doesn't contain a valid " + + "database name"); } // Add 1 to include the char size of the DATABASE_DELIMITER diff --git a/apex/appsearch/synced_jetpack_changeid.txt b/apex/appsearch/synced_jetpack_changeid.txt index 85d85aa5aa314..9723de6c5fcea 100644 --- a/apex/appsearch/synced_jetpack_changeid.txt +++ b/apex/appsearch/synced_jetpack_changeid.txt @@ -1 +1 @@ -I0216abecc41d020f16ed8947a9f37b710afd331e +a83c33a5a394141fea1d065ce0fab513a62d4bcf diff --git a/apex/appsearch/testing/java/com/android/server/appsearch/testing/external/AppSearchSessionShim.java b/apex/appsearch/testing/java/com/android/server/appsearch/testing/external/AppSearchSessionShim.java index a70d9b5facc32..d6ce3ebc267f5 100644 --- a/apex/appsearch/testing/java/com/android/server/appsearch/testing/external/AppSearchSessionShim.java +++ b/apex/appsearch/testing/java/com/android/server/appsearch/testing/external/AppSearchSessionShim.java @@ -30,6 +30,9 @@ import java.util.Set; *

An {@link AppSearchSessionShim} instance provides access to database operations such as * setting a schema, adding documents, and searching. * + *

Instances of this interface are usually obtained from a storage implementation, e.g. {@code + * AppSearchManager.createSearchSession()} or {@code PlatformStorage.createSearchSession()}. + * *

All implementations of this interface must be thread safe. * * @see GlobalSearchSessionShim diff --git a/core/tests/coretests/src/android/app/appsearch/external/app/SetSchemaRequestTest.java b/core/tests/coretests/src/android/app/appsearch/external/app/SetSchemaRequestTest.java deleted file mode 100644 index 677f4bd9e46d0..0000000000000 --- a/core/tests/coretests/src/android/app/appsearch/external/app/SetSchemaRequestTest.java +++ /dev/null @@ -1,186 +0,0 @@ -/* - * 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. - * 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 android.app.appsearch; - - -import static com.google.common.truth.Truth.assertThat; - -import static org.testng.Assert.expectThrows; - -import android.util.ArrayMap; - -import org.junit.Test; - -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - -public class SetSchemaRequestTest { - - private static Collection getSchemaTypesFromSetSchemaRequest(SetSchemaRequest request) { - HashSet schemaTypes = new HashSet<>(); - for (AppSearchSchema schema : request.getSchemas()) { - schemaTypes.add(schema.getSchemaType()); - } - return schemaTypes; - } - - @Test - public void testInvalidSchemaReferences_fromDisplayedBySystem() { - IllegalArgumentException expected = - expectThrows( - IllegalArgumentException.class, - () -> - new SetSchemaRequest.Builder() - .setSchemaTypeDisplayedBySystem("InvalidSchema", false) - .build()); - assertThat(expected).hasMessageThat().contains("referenced, but were not added"); - } - - @Test - public void testInvalidSchemaReferences_fromPackageVisibility() { - IllegalArgumentException expected = - expectThrows( - IllegalArgumentException.class, - () -> - new SetSchemaRequest.Builder() - .setSchemaTypeVisibilityForPackage( - "InvalidSchema", - /*visible=*/ true, - new PackageIdentifier( - "com.foo.package", - /*sha256Certificate=*/ new byte[] {})) - .build()); - assertThat(expected).hasMessageThat().contains("referenced, but were not added"); - } - - @Test - public void testSetSchemaTypeDisplayedBySystem_displayed() { - AppSearchSchema schema = new AppSearchSchema.Builder("Schema").build(); - - // By default, the schema is displayed. - SetSchemaRequest request = new SetSchemaRequest.Builder().addSchemas(schema).build(); - assertThat(request.getSchemasNotDisplayedBySystem()).isEmpty(); - - request = - new SetSchemaRequest.Builder() - .addSchemas(schema) - .setSchemaTypeDisplayedBySystem("Schema", true) - .build(); - assertThat(request.getSchemasNotDisplayedBySystem()).isEmpty(); - } - - @Test - public void testSetSchemaTypeDisplayedBySystem_notDisplayed() { - AppSearchSchema schema = new AppSearchSchema.Builder("Schema").build(); - SetSchemaRequest request = - new SetSchemaRequest.Builder() - .addSchemas(schema) - .setSchemaTypeDisplayedBySystem("Schema", false) - .build(); - assertThat(request.getSchemasNotDisplayedBySystem()).containsExactly("Schema"); - } - - @Test - public void testSchemaTypeVisibilityForPackage_visible() { - AppSearchSchema schema = new AppSearchSchema.Builder("Schema").build(); - - // By default, the schema is not visible. - SetSchemaRequest request = new SetSchemaRequest.Builder().addSchemas(schema).build(); - assertThat(request.getSchemasVisibleToPackages()).isEmpty(); - - PackageIdentifier packageIdentifier = - new PackageIdentifier("com.package.foo", new byte[] {100}); - Map> expectedVisibleToPackagesMap = new ArrayMap<>(); - expectedVisibleToPackagesMap.put("Schema", Collections.singleton(packageIdentifier)); - - request = - new SetSchemaRequest.Builder() - .addSchemas(schema) - .setSchemaTypeVisibilityForPackage( - "Schema", /*visible=*/ true, packageIdentifier) - .build(); - assertThat(request.getSchemasVisibleToPackages()) - .containsExactlyEntriesIn(expectedVisibleToPackagesMap); - } - - @Test - public void testSchemaTypeVisibilityForPackage_notVisible() { - AppSearchSchema schema = new AppSearchSchema.Builder("Schema").build(); - - SetSchemaRequest request = - new SetSchemaRequest.Builder() - .addSchemas(schema) - .setSchemaTypeVisibilityForPackage( - "Schema", - /*visible=*/ false, - new PackageIdentifier( - "com.package.foo", /*sha256Certificate=*/ new byte[] {})) - .build(); - assertThat(request.getSchemasVisibleToPackages()).isEmpty(); - } - - @Test - public void testSchemaTypeVisibilityForPackage_deduped() throws Exception { - AppSearchSchema schema = new AppSearchSchema.Builder("Schema").build(); - - PackageIdentifier packageIdentifier = - new PackageIdentifier("com.package.foo", new byte[] {100}); - Map> expectedVisibleToPackagesMap = new ArrayMap<>(); - expectedVisibleToPackagesMap.put("Schema", Collections.singleton(packageIdentifier)); - - SetSchemaRequest request = - new SetSchemaRequest.Builder() - .addSchemas(schema) - // Set it visible for "Schema" - .setSchemaTypeVisibilityForPackage( - "Schema", /*visible=*/ true, packageIdentifier) - // Set it visible for "Schema" again, which should be a no-op - .setSchemaTypeVisibilityForPackage( - "Schema", /*visible=*/ true, packageIdentifier) - .build(); - assertThat(request.getSchemasVisibleToPackages()) - .containsExactlyEntriesIn(expectedVisibleToPackagesMap); - } - - @Test - public void testSchemaTypeVisibilityForPackage_removed() throws Exception { - AppSearchSchema schema = new AppSearchSchema.Builder("Schema").build(); - - SetSchemaRequest request = - new SetSchemaRequest.Builder() - .addSchemas(schema) - // First set it as visible - .setSchemaTypeVisibilityForPackage( - "Schema", - /*visible=*/ true, - new PackageIdentifier( - "com.package.foo", /*sha256Certificate=*/ new byte[] {100})) - // Then make it not visible - .setSchemaTypeVisibilityForPackage( - "Schema", - /*visible=*/ false, - new PackageIdentifier( - "com.package.foo", /*sha256Certificate=*/ new byte[] {100})) - .build(); - - // Nothing should be visible. - assertThat(request.getSchemasVisibleToPackages()).isEmpty(); - } -} diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java index b36720303a55b..0dcc069e20ffe 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java @@ -24,6 +24,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.testng.Assert.expectThrows; +import android.app.appsearch.AppSearchResult; import android.app.appsearch.AppSearchSchema; import android.app.appsearch.GenericDocument; import android.app.appsearch.SearchResult; @@ -39,16 +40,19 @@ import android.util.ArraySet; import androidx.test.core.app.ApplicationProvider; import com.android.server.appsearch.external.localstorage.converter.GenericDocumentToProtoConverter; +import com.android.server.appsearch.external.localstorage.stats.InitializeStats; import com.android.server.appsearch.external.localstorage.util.PrefixUtil; import com.android.server.appsearch.proto.DocumentProto; import com.android.server.appsearch.proto.GetOptimizeInfoResultProto; import com.android.server.appsearch.proto.PersistType; import com.android.server.appsearch.proto.PropertyConfigProto; import com.android.server.appsearch.proto.PropertyProto; +import com.android.server.appsearch.proto.PutResultProto; import com.android.server.appsearch.proto.SchemaProto; import com.android.server.appsearch.proto.SchemaTypeConfigProto; import com.android.server.appsearch.proto.SearchResultProto; import com.android.server.appsearch.proto.SearchSpecProto; +import com.android.server.appsearch.proto.StatusProto; import com.android.server.appsearch.proto.StringIndexingConfig; import com.android.server.appsearch.proto.TermMatchType; @@ -86,8 +90,6 @@ public class AppSearchImplTest { /*logger=*/ null); } - // TODO(b/175430168) add test to verify reset is working properly. - /** * Ensure that we can rewrite an incoming schema type by adding the database as a prefix. While * also keeping any other existing schema types that may already be part of Icing's persisted @@ -485,6 +487,142 @@ public class AppSearchImplTest { .isLessThan(AppSearchImpl.CHECK_OPTIMIZE_INTERVAL); } + @Test + public void testReset() throws Exception { + // Setup the index + Context context = ApplicationProvider.getApplicationContext(); + File appsearchDir = mTemporaryFolder.newFolder(); + AppSearchImpl appSearchImpl = + AppSearchImpl.create( + appsearchDir, + context, + VisibilityStore.NO_OP_USER_ID, + /*globalQuerierPackage=*/ "", + /*logger=*/ null); + + // Insert schema + List schemas = + ImmutableList.of( + new AppSearchSchema.Builder("Type1").build(), + new AppSearchSchema.Builder("Type2").build()); + appSearchImpl.setSchema( + context.getPackageName(), + "database1", + schemas, + /*schemasNotPlatformSurfaceable=*/ Collections.emptyList(), + /*schemasPackageAccessible=*/ Collections.emptyMap(), + /*forceOverride=*/ false, + /*version=*/ 0); + + // Insert a valid doc + GenericDocument validDoc = + new GenericDocument.Builder<>("namespace1", "id1", "Type1").build(); + appSearchImpl.putDocument( + context.getPackageName(), "database1", validDoc, /*logger=*/ null); + + // Query it via global query. We use the same code again later so this is to make sure we + // have our global query configured right. + SearchResultPage results = + appSearchImpl.globalQuery( + /*queryExpression=*/ "", + new SearchSpec.Builder().addFilterSchemas("Type1").build(), + context.getPackageName(), + VisibilityStore.NO_OP_USER_ID, + /*logger=*/ null); + assertThat(results.getResults()).hasSize(1); + assertThat(results.getResults().get(0).getGenericDocument()).isEqualTo(validDoc); + + // Create a doc with a malformed namespace + DocumentProto invalidDoc = + DocumentProto.newBuilder() + .setNamespace("invalidNamespace") + .setUri("id2") + .setSchema(context.getPackageName() + "$database1/Type1") + .build(); + AppSearchException e = + expectThrows( + AppSearchException.class, + () -> PrefixUtil.getPrefix(invalidDoc.getNamespace())); + assertThat(e) + .hasMessageThat() + .isEqualTo( + "The prefixed value \"invalidNamespace\" doesn't contain a valid database" + + " name"); + + // Insert the invalid doc with an invalid namespace right into icing + PutResultProto putResultProto = appSearchImpl.mIcingSearchEngineLocked.put(invalidDoc); + assertThat(putResultProto.getStatus().getCode()).isEqualTo(StatusProto.Code.OK); + + // Create a logger for capturing initialization to make sure we are logging the recovery + // process correctly. + AppSearchLoggerTest.TestLogger testLogger = new AppSearchLoggerTest.TestLogger(); + + // Initialize AppSearchImpl. This should cause a reset. + appSearchImpl.close(); + appSearchImpl = + AppSearchImpl.create( + appsearchDir, + context, + VisibilityStore.NO_OP_USER_ID, + /*globalQuerierPackage=*/ context.getPackageName(), + testLogger); + + // Check recovery state + InitializeStats initStats = testLogger.mInitializeStats; + assertThat(initStats).isNotNull(); + assertThat(initStats.getStatusCode()).isEqualTo(AppSearchResult.RESULT_INTERNAL_ERROR); + assertThat(initStats.hasDeSync()).isFalse(); + assertThat(initStats.getDocumentStoreRecoveryCause()) + .isEqualTo(InitializeStats.RECOVERY_CAUSE_NONE); + // TODO(b/187879464): There should not be a recovery here, but icing lib reports one if the + // doc had no tokens. Once the mentioned bug is fixed, uncomment this. + // assertThat(initStats.getIndexRestorationCause()) + // .isEqualTo(InitializeStats.RECOVERY_CAUSE_NONE); + assertThat(initStats.getSchemaStoreRecoveryCause()) + .isEqualTo(InitializeStats.RECOVERY_CAUSE_NONE); + assertThat(initStats.getDocumentStoreDataStatus()) + .isEqualTo(InitializeStats.DOCUMENT_STORE_DATA_STATUS_NO_DATA_LOSS); + assertThat(initStats.hasReset()).isTrue(); + assertThat(initStats.getResetStatusCode()).isEqualTo(AppSearchResult.RESULT_OK); + + // Make sure all our data is gone + assertThat(appSearchImpl.getSchema(context.getPackageName(), "database1").getSchemas()) + .isEmpty(); + results = + appSearchImpl.globalQuery( + /*queryExpression=*/ "", + new SearchSpec.Builder().addFilterSchemas("Type1").build(), + context.getPackageName(), + VisibilityStore.NO_OP_USER_ID, + /*logger=*/ null); + assertThat(results.getResults()).isEmpty(); + + // Make sure the index can now be used successfully + appSearchImpl.setSchema( + context.getPackageName(), + "database1", + Collections.singletonList(new AppSearchSchema.Builder("Type1").build()), + /*schemasNotPlatformSurfaceable=*/ Collections.emptyList(), + /*schemasPackageAccessible=*/ Collections.emptyMap(), + /*forceOverride=*/ false, + /*version=*/ 0); + + // Insert a valid doc + appSearchImpl.putDocument( + context.getPackageName(), "database1", validDoc, /*logger=*/ null); + + // Query it via global query. + results = + appSearchImpl.globalQuery( + /*queryExpression=*/ "", + new SearchSpec.Builder().addFilterSchemas("Type1").build(), + context.getPackageName(), + VisibilityStore.NO_OP_USER_ID, + /*logger=*/ null); + assertThat(results.getResults()).hasSize(1); + assertThat(results.getResults().get(0).getGenericDocument()).isEqualTo(validDoc); + } + @Test public void testRewriteSearchSpec_oneInstance() throws Exception { SearchSpecProto.Builder searchSpecProto = SearchSpecProto.newBuilder().setQuery(""); diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java index 2aad275ba62e6..0e5c77c5182cd 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java @@ -68,7 +68,7 @@ public class AppSearchLoggerTest { } // Test only not thread safe. - public class TestLogger implements AppSearchLogger { + public static class TestLogger implements AppSearchLogger { @Nullable CallStats mCallStats; @Nullable PutDocumentStats mPutDocumentStats; @Nullable InitializeStats mInitializeStats; diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java index 2d039810446a6..edb683bf92f04 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java @@ -168,7 +168,9 @@ public class AppSearchStatsTest { .setSchemaStoreRecoveryLatencyMillis(nativeSchemaStoreRecoveryLatencyMillis) .setDocumentStoreDataStatus(nativeDocumentStoreDataStatus) .setDocumentCount(nativeNumDocuments) - .setSchemaTypeCount(nativeNumSchemaTypes); + .setSchemaTypeCount(nativeNumSchemaTypes) + .setHasReset(true) + .setResetStatusCode(AppSearchResult.RESULT_INVALID_SCHEMA); final InitializeStats iStats = iStatsBuilder.build(); assertThat(iStats.getStatusCode()).isEqualTo(TEST_STATUS_CODE); @@ -192,6 +194,8 @@ public class AppSearchStatsTest { assertThat(iStats.getDocumentStoreDataStatus()).isEqualTo(nativeDocumentStoreDataStatus); assertThat(iStats.getDocumentCount()).isEqualTo(nativeNumDocuments); assertThat(iStats.getSchemaTypeCount()).isEqualTo(nativeNumSchemaTypes); + assertThat(iStats.hasReset()).isTrue(); + assertThat(iStats.getResetStatusCode()).isEqualTo(AppSearchResult.RESULT_INVALID_SCHEMA); } @Test