diff --git a/apex/appsearch/framework/api/current.txt b/apex/appsearch/framework/api/current.txt index 77cb93034e770..761405cc50e85 100644 --- a/apex/appsearch/framework/api/current.txt +++ b/apex/appsearch/framework/api/current.txt @@ -206,7 +206,7 @@ package android.app.appsearch { method @NonNull public android.app.appsearch.GetByDocumentIdRequest build(); } - public class GetSchemaResponse { + public final class GetSchemaResponse { method @NonNull public java.util.Set getSchemas(); method @IntRange(from=0) public int getVersion(); } diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchSchema.java b/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchSchema.java index fede698fdbf53..237e6242b22a4 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchSchema.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/AppSearchSchema.java @@ -389,12 +389,24 @@ public final class AppSearchSchema { public @interface TokenizerType {} /** - * It is only valid for tokenizer_type to be 'NONE' if {@link #getIndexingType} is {@link + * This value indicates that no tokens should be extracted from this property. + * + *

It is only valid for tokenizer_type to be 'NONE' if {@link #getIndexingType} is {@link * #INDEXING_TYPE_NONE}. */ public static final int TOKENIZER_TYPE_NONE = 0; - /** Tokenization for plain text. */ + /** + * Tokenization for plain text. This value indicates that tokens should be extracted from + * this property based on word breaks. Segments of whitespace and punctuation are not + * considered tokens. + * + *

Ex. A property with "foo bar. baz." will produce tokens for "foo", "bar" and "baz". + * The segments " " and "." will not be considered tokens. + * + *

It is only valid for tokenizer_type to be 'PLAIN' if {@link #getIndexingType} is + * {@link #INDEXING_TYPE_EXACT_TERMS} or {@link #INDEXING_TYPE_PREFIXES}. + */ public static final int TOKENIZER_TYPE_PLAIN = 1; StringPropertyConfig(@NonNull Bundle bundle) { @@ -474,6 +486,17 @@ public final class AppSearchSchema { /** Constructs a new {@link StringPropertyConfig} from the contents of this builder. */ @NonNull public StringPropertyConfig build() { + if (mTokenizerType == TOKENIZER_TYPE_NONE) { + Preconditions.checkState( + mIndexingType == INDEXING_TYPE_NONE, + "Cannot set " + + "TOKENIZER_TYPE_NONE with an indexing type other than " + + "INDEXING_TYPE_NONE."); + } else { + Preconditions.checkState( + mIndexingType != INDEXING_TYPE_NONE, + "Cannot set " + "TOKENIZER_TYPE_PLAIN with INDEXING_TYPE_NONE."); + } Bundle bundle = new Bundle(); bundle.putString(NAME_FIELD, mPropertyName); bundle.putInt(DATA_TYPE_FIELD, DATA_TYPE_STRING); diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/GenericDocument.java b/apex/appsearch/framework/java/external/android/app/appsearch/GenericDocument.java index cc48ccb463ee5..bcd341ec18a85 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/GenericDocument.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/GenericDocument.java @@ -27,8 +27,6 @@ import android.os.Bundle; import android.os.Parcelable; import android.util.Log; -import com.android.internal.util.Preconditions; - import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; @@ -1018,17 +1016,14 @@ public class GenericDocument { // GenericDocument. @SuppressLint("StaticFinalBuilder") public static class Builder { - - private final Bundle mBundle; - private final Bundle mProperties; + private Bundle mBundle; + private Bundle mProperties; private final BuilderType mBuilderTypeInstance; private boolean mBuilt = false; /** * Creates a new {@link GenericDocument.Builder}. * - *

Once {@link #build} is called, the instance can no longer be used. - * *

Document IDs are unique within a namespace. * *

The number of namespaces per app should be kept small for efficiency reasons. @@ -1053,9 +1048,6 @@ public class GenericDocument { mBundle.putString(GenericDocument.NAMESPACE_FIELD, namespace); mBundle.putString(GenericDocument.ID_FIELD, id); mBundle.putString(GenericDocument.SCHEMA_TYPE_FIELD, schemaType); - // Set current timestamp for creation timestamp by default. - mBundle.putLong( - GenericDocument.CREATION_TIMESTAMP_MILLIS_FIELD, System.currentTimeMillis()); mBundle.putLong(GenericDocument.TTL_MILLIS_FIELD, DEFAULT_TTL_MILLIS); mBundle.putInt(GenericDocument.SCORE_FIELD, DEFAULT_SCORE); @@ -1063,7 +1055,11 @@ public class GenericDocument { mBundle.putBundle(PROPERTIES_FIELD, mProperties); } - /** Creates a new {@link GenericDocument.Builder} from the given Bundle. */ + /** + * Creates a new {@link GenericDocument.Builder} from the given Bundle. + * + *

The bundle is NOT copied. + */ @SuppressWarnings("unchecked") Builder(@NonNull Bundle bundle) { mBundle = Objects.requireNonNull(bundle); @@ -1079,13 +1075,12 @@ public class GenericDocument { * *

The number of namespaces per app should be kept small for efficiency reasons. * - * @throws IllegalStateException if the builder has already been used. * @hide */ @NonNull public BuilderType setNamespace(@NonNull String namespace) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(namespace); + resetIfBuilt(); mBundle.putString(GenericDocument.NAMESPACE_FIELD, namespace); return mBuilderTypeInstance; } @@ -1096,13 +1091,12 @@ public class GenericDocument { * *

Document IDs are unique within a namespace. * - * @throws IllegalStateException if the builder has already been used. * @hide */ @NonNull public BuilderType setId(@NonNull String id) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(id); + resetIfBuilt(); mBundle.putString(GenericDocument.ID_FIELD, id); return mBuilderTypeInstance; } @@ -1113,13 +1107,12 @@ public class GenericDocument { *

To successfully index a document, the schema type must match the name of an {@link * AppSearchSchema} object previously provided to {@link AppSearchSession#setSchema}. * - * @throws IllegalStateException if the builder has already been used. * @hide */ @NonNull public BuilderType setSchemaType(@NonNull String schemaType) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(schemaType); + resetIfBuilt(); mBundle.putString(GenericDocument.SCHEMA_TYPE_FIELD, schemaType); return mBuilderTypeInstance; } @@ -1136,14 +1129,13 @@ public class GenericDocument { *

Any non-negative integer can be used a score. By default, scores are set to 0. * * @param score any non-negative {@code int} representing the document's score. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setScore(@IntRange(from = 0, to = Integer.MAX_VALUE) int score) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); if (score < 0) { throw new IllegalArgumentException("Document score cannot be negative."); } + resetIfBuilt(); mBundle.putInt(GenericDocument.SCORE_FIELD, score); return mBuilderTypeInstance; } @@ -1154,13 +1146,14 @@ public class GenericDocument { *

This should be set using a value obtained from the {@link System#currentTimeMillis} * time base. * + *

If this method is not called, this will be set to the time the object is built. + * * @param creationTimestampMillis a creation timestamp in milliseconds. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setCreationTimestampMillis( @CurrentTimeMillisLong long creationTimestampMillis) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); + resetIfBuilt(); mBundle.putLong( GenericDocument.CREATION_TIMESTAMP_MILLIS_FIELD, creationTimestampMillis); return mBuilderTypeInstance; @@ -1177,14 +1170,13 @@ public class GenericDocument { * auto-deleted until the app is uninstalled or {@link AppSearchSession#remove} is called. * * @param ttlMillis a non-negative duration in milliseconds. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setTtlMillis(long ttlMillis) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); if (ttlMillis < 0) { throw new IllegalArgumentException("Document ttlMillis cannot be negative."); } + resetIfBuilt(); mBundle.putLong(GenericDocument.TTL_MILLIS_FIELD, ttlMillis); return mBuilderTypeInstance; } @@ -1197,13 +1189,12 @@ public class GenericDocument { * @param values the {@code String} values of the property. * @throws IllegalArgumentException if no values are provided, if provided values exceed * maximum repeated property length, or if a passed in {@code String} is {@code null}. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setPropertyString(@NonNull String name, @NonNull String... values) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); Objects.requireNonNull(values); + resetIfBuilt(); putInPropertyBundle(name, values); return mBuilderTypeInstance; } @@ -1216,13 +1207,12 @@ public class GenericDocument { * property as given in {@link AppSearchSchema.PropertyConfig#getName}. * @param values the {@code boolean} values of the property. * @throws IllegalArgumentException if values exceed maximum repeated property length. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setPropertyBoolean(@NonNull String name, @NonNull boolean... values) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); Objects.requireNonNull(values); + resetIfBuilt(); putInPropertyBundle(name, values); return mBuilderTypeInstance; } @@ -1234,13 +1224,12 @@ public class GenericDocument { * property as given in {@link AppSearchSchema.PropertyConfig#getName}. * @param values the {@code long} values of the property. * @throws IllegalArgumentException if values exceed maximum repeated property length. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setPropertyLong(@NonNull String name, @NonNull long... values) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); Objects.requireNonNull(values); + resetIfBuilt(); putInPropertyBundle(name, values); return mBuilderTypeInstance; } @@ -1252,13 +1241,12 @@ public class GenericDocument { * property as given in {@link AppSearchSchema.PropertyConfig#getName}. * @param values the {@code double} values of the property. * @throws IllegalArgumentException if values exceed maximum repeated property length. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setPropertyDouble(@NonNull String name, @NonNull double... values) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); Objects.requireNonNull(values); + resetIfBuilt(); putInPropertyBundle(name, values); return mBuilderTypeInstance; } @@ -1271,13 +1259,12 @@ public class GenericDocument { * @param values the {@code byte[]} of the property. * @throws IllegalArgumentException if no values are provided, if provided values exceed * maximum repeated property length, or if a passed in {@code byte[]} is {@code null}. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setPropertyBytes(@NonNull String name, @NonNull byte[]... values) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); Objects.requireNonNull(values); + resetIfBuilt(); putInPropertyBundle(name, values); return mBuilderTypeInstance; } @@ -1292,14 +1279,13 @@ public class GenericDocument { * @throws IllegalArgumentException if no values are provided, if provided values exceed if * provided values exceed maximum repeated property length, or if a passed in {@link * GenericDocument} is {@code null}. - * @throws IllegalStateException if the builder has already been used. */ @NonNull public BuilderType setPropertyDocument( @NonNull String name, @NonNull GenericDocument... values) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); Objects.requireNonNull(values); + resetIfBuilt(); putInPropertyBundle(name, values); return mBuilderTypeInstance; } @@ -1314,8 +1300,8 @@ public class GenericDocument { */ @NonNull public BuilderType clearProperty(@NonNull String name) { - Preconditions.checkState(!mBuilt, "Builder has already been used"); Objects.requireNonNull(name); + resetIfBuilt(); mProperties.remove(name); return mBuilderTypeInstance; } @@ -1399,16 +1385,25 @@ public class GenericDocument { } } - /** - * Builds the {@link GenericDocument} object. - * - * @throws IllegalStateException if the builder has already been used. - */ + /** Builds the {@link GenericDocument} object. */ @NonNull public GenericDocument build() { - Preconditions.checkState(!mBuilt, "Builder has already been used"); mBuilt = true; + // Set current timestamp for creation timestamp by default. + if (mBundle.getLong(GenericDocument.CREATION_TIMESTAMP_MILLIS_FIELD, -1) == -1) { + mBundle.putLong( + GenericDocument.CREATION_TIMESTAMP_MILLIS_FIELD, + System.currentTimeMillis()); + } return new GenericDocument(mBundle); } + + private void resetIfBuilt() { + if (mBuilt) { + mBundle = BundleUtil.deepCopy(mBundle); + mProperties = mBundle.getBundle(PROPERTIES_FIELD); + mBuilt = false; + } + } } } diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/GetSchemaResponse.java b/apex/appsearch/framework/java/external/android/app/appsearch/GetSchemaResponse.java index 8816c787673b0..018f16de28cb0 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/GetSchemaResponse.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/GetSchemaResponse.java @@ -26,7 +26,7 @@ import java.util.Objects; import java.util.Set; /** The response class of {@link AppSearchSession#getSchema} */ -public class GetSchemaResponse { +public final class GetSchemaResponse { private static final String VERSION_FIELD = "version"; private static final String SCHEMAS_FIELD = "schemas"; diff --git a/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaRequest.java b/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaRequest.java index 96002c5291eed..0af3e7a8987ac 100644 --- a/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaRequest.java +++ b/apex/appsearch/framework/java/external/android/app/appsearch/SetSchemaRequest.java @@ -322,6 +322,8 @@ public final class SetSchemaRequest { /** * Sets a Map of {@link Migrator}s. * + *

The key of the map is the schema type that the {@link Migrator} value applies to. + * *

The {@link Migrator} migrates all {@link GenericDocument}s under given schema type * from the current version number stored in AppSearch to the final version set via {@link * #setVersion}. @@ -335,7 +337,8 @@ public final class SetSchemaRequest { * SetSchemaRequest}. * * @param migrators A {@link Map} of migrators that translate a document from it's current - * version to the final version set via {@link #setVersion}. + * version to the final version set via {@link #setVersion}. The key of the map is the + * schema type that the {@link Migrator} value applies to. * @see SetSchemaRequest.Builder#setVersion * @see SetSchemaRequest.Builder#addSchemas * @see AppSearchSession#setSchema diff --git a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java index 754cc41960fc0..0c07fa01f45d9 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java @@ -930,7 +930,12 @@ public class AppSearchManagerService extends SystemService { for (int i = 0; i < ids.size(); i++) { String id = ids.get(i); try { - impl.remove(packageName, databaseName, namespace, id); + impl.remove( + packageName, + databaseName, + namespace, + id, + /*removeStatsBuilder=*/ null); ++operationSuccessCount; resultBuilder.setSuccess(id, /*result= */ null); } catch (Throwable t) { @@ -1007,7 +1012,8 @@ public class AppSearchManagerService extends SystemService { packageName, databaseName, queryExpression, - new SearchSpec(searchSpecBundle)); + new SearchSpec(searchSpecBundle), + /*removeStatsBuilder=*/ null); // Now that the batch has been written. Persist the newly written data. impl.persistToDisk(PersistType.Code.LITE); ++operationSuccessCount; 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 1cb0cc51de346..d008f3b43a00f 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 @@ -58,6 +58,7 @@ import com.android.server.appsearch.external.localstorage.converter.SetSchemaRes import com.android.server.appsearch.external.localstorage.converter.TypePropertyPathToProtoConverter; import com.android.server.appsearch.external.localstorage.stats.InitializeStats; import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats; +import com.android.server.appsearch.external.localstorage.stats.RemoveStats; import com.android.server.appsearch.external.localstorage.stats.SearchStats; import com.google.android.icing.IcingSearchEngine; @@ -1118,14 +1119,17 @@ public final class AppSearchImpl implements Closeable { * @param databaseName The databaseName the document is in. * @param namespace Namespace of the document to remove. * @param id ID of the document to remove. + * @param removeStatsBuilder builder for {@link RemoveStats} to hold stats for remove * @throws AppSearchException on IcingSearchEngine error. */ public void remove( @NonNull String packageName, @NonNull String databaseName, @NonNull String namespace, - @NonNull String id) + @NonNull String id, + @Nullable RemoveStats.Builder removeStatsBuilder) throws AppSearchException { + long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); mReadWriteLock.writeLock().lock(); try { throwIfClosedLocked(); @@ -1138,9 +1142,20 @@ public final class AppSearchImpl implements Closeable { mIcingSearchEngineLocked.delete(prefixedNamespace, id); mLogUtil.piiTrace( "removeById, response", deleteResultProto.getStatus(), deleteResultProto); + + if (removeStatsBuilder != null) { + removeStatsBuilder.setStatusCode( + statusProtoToResultCode(deleteResultProto.getStatus())); + AppSearchLoggerHelper.copyNativeStats( + deleteResultProto.getDeleteStats(), removeStatsBuilder); + } checkSuccess(deleteResultProto.getStatus()); } finally { mReadWriteLock.writeLock().unlock(); + if (removeStatsBuilder != null) { + removeStatsBuilder.setTotalLatencyMillis( + (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis)); + } } } @@ -1153,14 +1168,17 @@ public final class AppSearchImpl implements Closeable { * @param databaseName The databaseName the document is in. * @param queryExpression Query String to search. * @param searchSpec Defines what and how to remove + * @param removeStatsBuilder builder for {@link RemoveStats} to hold stats for remove * @throws AppSearchException on IcingSearchEngine error. */ public void removeByQuery( @NonNull String packageName, @NonNull String databaseName, @NonNull String queryExpression, - @NonNull SearchSpec searchSpec) + @NonNull SearchSpec searchSpec, + @Nullable RemoveStats.Builder removeStatsBuilder) throws AppSearchException { + long totalLatencyStartTimeMillis = SystemClock.elapsedRealtime(); mReadWriteLock.writeLock().lock(); try { throwIfClosedLocked(); @@ -1195,12 +1213,24 @@ public final class AppSearchImpl implements Closeable { mLogUtil.piiTrace( "removeByQuery, response", deleteResultProto.getStatus(), deleteResultProto); + if (removeStatsBuilder != null) { + removeStatsBuilder.setStatusCode( + statusProtoToResultCode(deleteResultProto.getStatus())); + // TODO(b/187206766) also log query stats here once IcingLib returns it + AppSearchLoggerHelper.copyNativeStats( + deleteResultProto.getDeleteStats(), removeStatsBuilder); + } + // 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. checkCodeOneOf( deleteResultProto.getStatus(), StatusProto.Code.OK, StatusProto.Code.NOT_FOUND); } finally { mReadWriteLock.writeLock().unlock(); + if (removeStatsBuilder != null) { + removeStatsBuilder.setTotalLatencyMillis( + (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis)); + } } } diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLogger.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLogger.java index 97c886922c32d..d92f4f05a1a4f 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLogger.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLogger.java @@ -22,6 +22,7 @@ import android.app.appsearch.exceptions.AppSearchException; import com.android.server.appsearch.external.localstorage.stats.CallStats; import com.android.server.appsearch.external.localstorage.stats.InitializeStats; import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats; +import com.android.server.appsearch.external.localstorage.stats.RemoveStats; import com.android.server.appsearch.external.localstorage.stats.SearchStats; /** @@ -47,5 +48,8 @@ public interface AppSearchLogger { /** Logs {@link SearchStats} */ void logStats(@NonNull SearchStats stats) throws AppSearchException; + /** Logs {@link RemoveStats} */ + void logStats(@NonNull RemoveStats stats) throws AppSearchException; + // TODO(b/173532925) Add remaining logStats once we add all the stats. } diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java index 4b92ce6fc4656..aa9200abe3be7 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchLoggerHelper.java @@ -20,8 +20,10 @@ import android.annotation.NonNull; import com.android.server.appsearch.external.localstorage.stats.InitializeStats; import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats; +import com.android.server.appsearch.external.localstorage.stats.RemoveStats; import com.android.server.appsearch.external.localstorage.stats.SearchStats; +import com.google.android.icing.proto.DeleteStatsProto; import com.google.android.icing.proto.InitializeStatsProto; import com.google.android.icing.proto.PutDocumentStatsProto; import com.google.android.icing.proto.QueryStatsProto; @@ -119,4 +121,21 @@ public final class AppSearchLoggerHelper { .setDocumentRetrievingLatencyMillis( fromNativeStats.getDocumentRetrievalLatencyMs()); } + + /* + * Copy native Query stats to buiilder. + * + * @param fromNativeStats Stats copied from. + * @param toStatsBuilder Stats copied to. + */ + static void copyNativeStats( + @NonNull DeleteStatsProto fromNativeStats, + @NonNull RemoveStats.Builder toStatsBuilder) { + Objects.requireNonNull(fromNativeStats); + Objects.requireNonNull(toStatsBuilder); + toStatsBuilder + .setNativeLatencyMillis(fromNativeStats.getLatencyMs()) + .setDeleteType(fromNativeStats.getDeleteType().getNumber()) + .setDeletedDocumentCount(fromNativeStats.getNumDocumentsDeleted()); + } } diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/SearchStats.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/SearchStats.java index 7b22b2f23142f..d7904f3ca49fc 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/SearchStats.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/SearchStats.java @@ -52,6 +52,9 @@ public final class SearchStats { // Searches the global documents. Including platform surfaceable and 3p-access. public static final int VISIBILITY_SCOPE_GLOBAL = 2; + // TODO(b/173532925): Add a field searchType to indicate where the search is used(normal + // query vs in removeByQuery vs during migration) + @NonNull private final String mPackageName; @Nullable private final String mDatabase; /** diff --git a/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java b/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java index 3027002f22115..abcf1611ab049 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java @@ -34,6 +34,7 @@ import com.android.server.appsearch.external.localstorage.AppSearchLogger; import com.android.server.appsearch.external.localstorage.stats.CallStats; import com.android.server.appsearch.external.localstorage.stats.InitializeStats; import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats; +import com.android.server.appsearch.external.localstorage.stats.RemoveStats; import com.android.server.appsearch.external.localstorage.stats.SearchStats; import java.io.UnsupportedEncodingException; @@ -213,6 +214,11 @@ public final class PlatformLogger implements AppSearchLogger { } } + @Override + public void logStats(@androidx.annotation.NonNull RemoveStats stats) throws AppSearchException { + // TODO(b/173532925): Log stats + } + /** * Removes cached UID for package. * diff --git a/apex/appsearch/synced_jetpack_changeid.txt b/apex/appsearch/synced_jetpack_changeid.txt index 9bfe07111741d..fe9117b8fc597 100644 --- a/apex/appsearch/synced_jetpack_changeid.txt +++ b/apex/appsearch/synced_jetpack_changeid.txt @@ -1 +1 @@ -c6630eba424d98dd54ece674e769d9b0b883e410 +be6d5138cbd64d3fd401a83d30bf9ad22a6c2d17 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 0dcc069e20ffe..4cac5233f3460 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 @@ -455,7 +455,8 @@ public class AppSearchImplTest { // delete 999 documents, we will reach the threshold to trigger optimize() in next // deletion. for (int i = 0; i < AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - 1; i++) { - mAppSearchImpl.remove("package", "database", "namespace", "id" + i); + mAppSearchImpl.remove( + "package", "database", "namespace", "id" + i, /*removeStatsBuilder=*/ null); } // Updates the check for optimize counter, checkForOptimize() will be triggered since @@ -475,7 +476,8 @@ public class AppSearchImplTest { < AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT + AppSearchImpl.CHECK_OPTIMIZE_INTERVAL; i++) { - mAppSearchImpl.remove("package", "database", "namespace", "id" + i); + mAppSearchImpl.remove( + "package", "database", "namespace", "id" + i, /*removeStatsBuilder=*/ null); } // updates the check for optimize counter, will reach both CHECK_OPTIMIZE_INTERVAL and // OPTIMIZE_THRESHOLD_DOC_COUNT this time and trigger a optimize(). @@ -885,17 +887,20 @@ public class AppSearchImplTest { .addFilterSchemas("FakeType") .setTermMatch(TermMatchType.Code.PREFIX_VALUE) .build(); - mAppSearchImpl.removeByQuery("package", "EmptyDatabase", "", searchSpec); + mAppSearchImpl.removeByQuery( + "package", "EmptyDatabase", "", searchSpec, /*statsBuilder=*/ null); searchSpec = new SearchSpec.Builder() .addFilterNamespaces("FakeNamespace") .setTermMatch(TermMatchType.Code.PREFIX_VALUE) .build(); - mAppSearchImpl.removeByQuery("package", "EmptyDatabase", "", searchSpec); + mAppSearchImpl.removeByQuery( + "package", "EmptyDatabase", "", searchSpec, /*statsBuilder=*/ null); searchSpec = new SearchSpec.Builder().setTermMatch(TermMatchType.Code.PREFIX_VALUE).build(); - mAppSearchImpl.removeByQuery("package", "EmptyDatabase", "", searchSpec); + mAppSearchImpl.removeByQuery( + "package", "EmptyDatabase", "", searchSpec, /*statsBuilder=*/ null); } @Test @@ -1661,8 +1666,7 @@ public class AppSearchImplTest { context, VisibilityStore.NO_OP_USER_ID, /*globalQuerierPackage=*/ "", - /*logger - =*/ null); + /*logger=*/ null); // Initial check that we could do something at first. List schemas = @@ -1768,7 +1772,8 @@ public class AppSearchImplTest { expectThrows( IllegalStateException.class, () -> { - appSearchImpl.remove("package", "database", "namespace", "id"); + appSearchImpl.remove( + "package", "database", "namespace", "id", /*statsBuilder=*/ null); }); expectThrows( @@ -1780,7 +1785,8 @@ public class AppSearchImplTest { "query", new SearchSpec.Builder() .setTermMatch(TermMatchType.Code.PREFIX_VALUE) - .build()); + .build(), + /*statsBuilder=*/ null); }); expectThrows( @@ -1894,7 +1900,7 @@ public class AppSearchImplTest { assertThat(getResult).isEqualTo(document2); // Delete the first document - appSearchImpl.remove("package", "database", "namespace1", "id1"); + appSearchImpl.remove("package", "database", "namespace1", "id1", /*statsBuilder=*/ null); appSearchImpl.persistToDisk(PersistType.Code.LITE); expectThrows( AppSearchException.class, @@ -1983,7 +1989,8 @@ public class AppSearchImplTest { new SearchSpec.Builder() .addFilterNamespaces("namespace1") .setTermMatch(SearchSpec.TERM_MATCH_EXACT_ONLY) - .build()); + .build(), + /*statsBuilder=*/ null); appSearchImpl.persistToDisk(PersistType.Code.LITE); expectThrows( AppSearchException.class, 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 b6bb394638bb8..190e173b6dcc5 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 @@ -32,7 +32,9 @@ import androidx.test.core.app.ApplicationProvider; import com.android.server.appsearch.external.localstorage.stats.CallStats; import com.android.server.appsearch.external.localstorage.stats.InitializeStats; import com.android.server.appsearch.external.localstorage.stats.PutDocumentStats; +import com.android.server.appsearch.external.localstorage.stats.RemoveStats; import com.android.server.appsearch.external.localstorage.stats.SearchStats; +import com.android.server.appsearch.proto.DeleteStatsProto; import com.android.server.appsearch.proto.InitializeStatsProto; import com.android.server.appsearch.proto.PutDocumentStatsProto; import com.android.server.appsearch.proto.QueryStatsProto; @@ -73,6 +75,7 @@ public class AppSearchLoggerTest { @Nullable PutDocumentStats mPutDocumentStats; @Nullable InitializeStats mInitializeStats; @Nullable SearchStats mSearchStats; + @Nullable RemoveStats mRemoveStats; @Override public void logStats(@NonNull CallStats stats) { @@ -93,6 +96,11 @@ public class AppSearchLoggerTest { public void logStats(@NonNull SearchStats stats) { mSearchStats = stats; } + + @Override + public void logStats(@NonNull RemoveStats stats) { + mRemoveStats = stats; + } } @Test @@ -252,6 +260,27 @@ public class AppSearchLoggerTest { .isEqualTo(nativeDocumentRetrievingLatencyMillis); } + @Test + public void testAppSearchLoggerHelper_testCopyNativeStats_remove() { + final int nativeLatencyMillis = 1; + final int nativeDeleteType = 2; + final int nativeNumDocumentDeleted = 3; + DeleteStatsProto nativeDeleteStatsProto = + DeleteStatsProto.newBuilder() + .setLatencyMs(nativeLatencyMillis) + .setDeleteType(DeleteStatsProto.DeleteType.Code.forNumber(nativeDeleteType)) + .setNumDocumentsDeleted(nativeNumDocumentDeleted) + .build(); + RemoveStats.Builder rBuilder = new RemoveStats.Builder("packageName", "database"); + + AppSearchLoggerHelper.copyNativeStats(nativeDeleteStatsProto, rBuilder); + + RemoveStats rStats = rBuilder.build(); + assertThat(rStats.getNativeLatencyMillis()).isEqualTo(nativeLatencyMillis); + assertThat(rStats.getDeleteType()).isEqualTo(nativeDeleteType); + assertThat(rStats.getDeletedDocumentCount()).isEqualTo(nativeNumDocumentDeleted); + } + // // Testing actual logging // @@ -355,4 +384,74 @@ public class AppSearchLoggerTest { assertThat(sStats.isFirstPage()).isTrue(); assertThat(sStats.getScoredDocumentCount()).isEqualTo(1); } + + @Test + public void testLoggingStats_remove() throws Exception { + // Insert schema + final String testPackageName = "testPackage"; + final String testDatabase = "testDatabase"; + final String testNamespace = "testNameSpace"; + final String testId = "id"; + List schemas = + Collections.singletonList(new AppSearchSchema.Builder("type").build()); + mAppSearchImpl.setSchema( + testPackageName, + testDatabase, + schemas, + /*schemasNotPlatformSurfaceable=*/ Collections.emptyList(), + /*schemasPackageAccessible=*/ Collections.emptyMap(), + /*forceOverride=*/ false, + /*version=*/ 0); + GenericDocument document = + new GenericDocument.Builder<>(testNamespace, testId, "type").build(); + mAppSearchImpl.putDocument(testPackageName, testDatabase, document, /*logger=*/ null); + + RemoveStats.Builder rStatsBuilder = new RemoveStats.Builder(testPackageName, testDatabase); + mAppSearchImpl.remove(testPackageName, testDatabase, testNamespace, testId, rStatsBuilder); + RemoveStats rStats = rStatsBuilder.build(); + + assertThat(rStats.getPackageName()).isEqualTo(testPackageName); + assertThat(rStats.getDatabase()).isEqualTo(testDatabase); + // delete by namespace + id + assertThat(rStats.getDeleteType()).isEqualTo(DeleteStatsProto.DeleteType.Code.SINGLE_VALUE); + assertThat(rStats.getDeletedDocumentCount()).isEqualTo(1); + } + + @Test + public void testLoggingStats_removeByQuery() throws Exception { + // Insert schema + final String testPackageName = "testPackage"; + final String testDatabase = "testDatabase"; + final String testNamespace = "testNameSpace"; + List schemas = + Collections.singletonList(new AppSearchSchema.Builder("type").build()); + mAppSearchImpl.setSchema( + testPackageName, + testDatabase, + schemas, + /*schemasNotPlatformSurfaceable=*/ Collections.emptyList(), + /*schemasPackageAccessible=*/ Collections.emptyMap(), + /*forceOverride=*/ false, + /*version=*/ 0); + GenericDocument document1 = + new GenericDocument.Builder<>(testNamespace, "id1", "type").build(); + GenericDocument document2 = + new GenericDocument.Builder<>(testNamespace, "id2", "type").build(); + mAppSearchImpl.putDocument(testPackageName, testDatabase, document1, mLogger); + mAppSearchImpl.putDocument(testPackageName, testDatabase, document2, mLogger); + // No query filters specified. package2 should only get its own documents back. + SearchSpec searchSpec = + new SearchSpec.Builder().setTermMatch(TermMatchType.Code.PREFIX_VALUE).build(); + + RemoveStats.Builder rStatsBuilder = new RemoveStats.Builder(testPackageName, testDatabase); + mAppSearchImpl.removeByQuery( + testPackageName, testDatabase, /*queryExpression=*/ "", searchSpec, rStatsBuilder); + RemoveStats rStats = rStatsBuilder.build(); + + assertThat(rStats.getPackageName()).isEqualTo(testPackageName); + assertThat(rStats.getDatabase()).isEqualTo(testDatabase); + // delete by query + assertThat(rStats.getDeleteType()).isEqualTo(DeleteStatsProto.DeleteType.Code.QUERY_VALUE); + assertThat(rStats.getDeletedDocumentCount()).isEqualTo(2); + } }