From 2c9449919369bbf3aa7ec7ce2ece5d56866a5178 Mon Sep 17 00:00:00 2001 From: Alexander Dorokhine Date: Fri, 20 Nov 2020 14:03:28 -0800 Subject: [PATCH] Update framework from androidx. Changes included: * 285c252: Updates and clarifications to GenericDocument comments. * 3c2a58e: Updates and clarifications to SearchSpec/SearchResult comments. * 764cec9: Cleanup locking strategy. Test: Presubmit Change-Id: I4e9954db7cc3f02cf7a30ee09cb5a4533641c585 --- .../app/appsearch/AppSearchSchema.java | 2 + .../app/appsearch/GenericDocument.java | 46 +++- .../app/appsearch/GetByUriRequest.java | 2 +- .../app/appsearch/PutDocumentsRequest.java | 4 +- .../app/appsearch/RemoveByUriRequest.java | 2 +- .../android/app/appsearch/SearchResult.java | 17 +- .../android/app/appsearch/SearchSpec.java | 35 +-- .../app/appsearch/SetSchemaRequest.java | 8 +- .../exceptions/AppSearchException.java | 3 +- .../external/localstorage/AppSearchImpl.java | 210 ++++++++++-------- .../localstorage/VisibilityStore.java | 4 +- .../converter/SearchSpecToProtoConverter.java | 2 +- apex/appsearch/synced_jetpack_changeid.txt | 2 +- .../external/app/SearchSpecTest.java | 2 +- .../external/app/cts/SearchSpecCtsTest.java | 4 +- .../localstorage/AppSearchImplTest.java | 53 +++-- .../localstorage/VisibilityStoreTest.java | 2 +- 17 files changed, 237 insertions(+), 161 deletions(-) diff --git a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSchema.java b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSchema.java index e2add9247d08b..7fa69368d45f5 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/AppSearchSchema.java +++ b/apex/appsearch/framework/java/android/app/appsearch/AppSearchSchema.java @@ -40,6 +40,8 @@ import java.util.Set; *

For example, an e-mail message or a music recording could be a schema type. * *

The schema consists of type information, properties, and config (like tokenization type). + * + * @see AppSearchSession#setSchema * @hide */ public final class AppSearchSchema { diff --git a/apex/appsearch/framework/java/android/app/appsearch/GenericDocument.java b/apex/appsearch/framework/java/android/app/appsearch/GenericDocument.java index cbbb2c62bb599..8d343460ddc04 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/GenericDocument.java +++ b/apex/appsearch/framework/java/android/app/appsearch/GenericDocument.java @@ -37,6 +37,10 @@ import java.util.Set; * Represents a document unit. * *

Documents are constructed via {@link GenericDocument.Builder}. + * + * @see AppSearchSession#putDocuments + * @see AppSearchSession#getByUri + * @see AppSearchSession#query * @hide */ public class GenericDocument { @@ -157,7 +161,11 @@ public class GenericDocument { return mSchemaType; } - /** Returns the creation timestamp of the {@link GenericDocument}, in milliseconds. */ + /** + * Returns the creation timestamp of the {@link GenericDocument}, in milliseconds. + * + *

The value is in the {@link System#currentTimeMillis} time base. + */ public long getCreationTimestampMillis() { return mCreationTimestampMillis; } @@ -165,6 +173,10 @@ public class GenericDocument { /** * Returns the TTL (Time To Live) of the {@link GenericDocument}, in milliseconds. * + *

The TTL is measured against {@link #getCreationTimestampMillis}. At the timestamp of + * {@code creationTimestampMillis + ttlMillis}, measured in the {@link System#currentTimeMillis} + * time base, the document will be auto-deleted. + * *

The default value is 0, which means the document is permanent and won't be auto-deleted * until the app is uninstalled. */ @@ -175,10 +187,13 @@ public class GenericDocument { /** * Returns the score of the {@link GenericDocument}. * - *

The score is a query-independent measure of the document's quality, relative to other - * {@link GenericDocument}s of the same type. + *

The score is a query-independent measure of the document's quality, relative to + * other {@link GenericDocument}s of the same type. * - *

The default value is 0. + *

Results may be sorted by score using {@link SearchSpec.Builder#setRankingStrategy}. + * Documents with higher scores are considered better than documents with lower scores. + * + *

Any nonnegative integer can be used a score. */ public int getScore() { return mBundle.getInt(SCORE_FIELD, DEFAULT_SCORE); @@ -448,8 +463,8 @@ public class GenericDocument { } /** - * Deeply checks two bundles are equally or not. - *

Two bundles will be considered equally if they contain same content. + * Deeply checks whether two bundles are equal. + *

Two bundles will be considered equal if they contain the same content. */ @SuppressWarnings("unchecked") private static boolean bundleEquals(Bundle one, Bundle two) { @@ -704,6 +719,11 @@ public class GenericDocument { *

The score is a query-independent measure of the document's quality, relative to * other {@link GenericDocument}s of the same type. * + *

Results may be sorted by score using {@link SearchSpec.Builder#setRankingStrategy}. + * Documents with higher scores are considered better than documents with lower scores. + * + *

Any nonnegative integer can be used a score. + * * @throws IllegalArgumentException If the provided value is negative. */ @NonNull @@ -717,8 +737,10 @@ public class GenericDocument { } /** - * Sets the creation timestamp of the {@link GenericDocument}, in milliseconds. Should be - * set using a value obtained from the {@link System#currentTimeMillis()} time base. + * Sets the creation timestamp of the {@link GenericDocument}, in milliseconds. + * + *

Should be set using a value obtained from the {@link System#currentTimeMillis} time + * base. */ @NonNull public BuilderType setCreationTimestampMillis(long creationTimestampMillis) { @@ -731,8 +753,12 @@ public class GenericDocument { /** * Sets the TTL (Time To Live) of the {@link GenericDocument}, in milliseconds. * - *

After this many milliseconds since the {@link #setCreationTimestampMillis creation - * timestamp}, the document is deleted. + *

The TTL is measured against {@link #getCreationTimestampMillis}. At the timestamp of + * {@code creationTimestampMillis + ttlMillis}, measured in the + * {@link System#currentTimeMillis} time base, the document will be auto-deleted. + * + *

The default value is 0, which means the document is permanent and won't be + * auto-deleted until the app is uninstalled. * * @param ttlMillis A non-negative duration in milliseconds. * @throws IllegalArgumentException If the provided value is negative. diff --git a/apex/appsearch/framework/java/android/app/appsearch/GetByUriRequest.java b/apex/appsearch/framework/java/android/app/appsearch/GetByUriRequest.java index e1e0eda7558c6..e4be785997ec3 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/GetByUriRequest.java +++ b/apex/appsearch/framework/java/android/app/appsearch/GetByUriRequest.java @@ -28,7 +28,7 @@ import java.util.Set; /** * Encapsulates a request to retrieve documents by namespace and URI. * - * @see AppSearchManager#getByUri + * @see AppSearchSession#getByUri * @hide */ public final class GetByUriRequest { diff --git a/apex/appsearch/framework/java/android/app/appsearch/PutDocumentsRequest.java b/apex/appsearch/framework/java/android/app/appsearch/PutDocumentsRequest.java index 1f90bc184f6f8..3f24ea9eefd47 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/PutDocumentsRequest.java +++ b/apex/appsearch/framework/java/android/app/appsearch/PutDocumentsRequest.java @@ -29,9 +29,9 @@ import java.util.Collections; import java.util.List; /** - * Encapsulates a request to index a document into an {@link AppSearchManager} database. + * Encapsulates a request to index a document into an {@link AppSearchSession} database. * - * @see AppSearchManager#putDocuments + * @see AppSearchSession#putDocuments * @hide */ public final class PutDocumentsRequest { diff --git a/apex/appsearch/framework/java/android/app/appsearch/RemoveByUriRequest.java b/apex/appsearch/framework/java/android/app/appsearch/RemoveByUriRequest.java index 486857fba1dea..58aadcd082c0e 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/RemoveByUriRequest.java +++ b/apex/appsearch/framework/java/android/app/appsearch/RemoveByUriRequest.java @@ -28,7 +28,7 @@ import java.util.Set; /** * Encapsulates a request to remove documents by namespace and URI. * - * @see AppSearchManager#removeByUri + * @see AppSearchSession#removeByUri * @hide */ public final class RemoveByUriRequest { diff --git a/apex/appsearch/framework/java/android/app/appsearch/SearchResult.java b/apex/appsearch/framework/java/android/app/appsearch/SearchResult.java index 99cb2f16ca4d7..1a79b1085bfd2 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/SearchResult.java +++ b/apex/appsearch/framework/java/android/app/appsearch/SearchResult.java @@ -28,10 +28,19 @@ import java.util.ArrayList; import java.util.List; /** - * This class represents one of the results obtained from the query. + * This class represents one of the results obtained from an AppSearch query. * - *

It contains the document which matched, information about which section(s) in the document - * matched, and snippet information containing textual summaries of the document's match(es). + *

This allows clients to obtain: + *

+ * + *

"Snippet" refers to a substring of text from the content of document that is returned as a + * part of search result. + * + * @see SearchResults * @hide */ public final class SearchResult { @@ -116,8 +125,6 @@ public final class SearchResult { } /** - * Snippet: It refers to a substring of text from the content of document that is returned as a - * part of search result. * This class represents a match objects for any Snippets that might be present in * {@link SearchResults} from query. Using this class * user can get the full text, exact matches and Snippets of document content for a given match. diff --git a/apex/appsearch/framework/java/android/app/appsearch/SearchSpec.java b/apex/appsearch/framework/java/android/app/appsearch/SearchSpec.java index f9f719e744a61..261f5ac5e50ab 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/SearchSpec.java +++ b/apex/appsearch/framework/java/android/app/appsearch/SearchSpec.java @@ -20,6 +20,7 @@ import android.annotation.SuppressLint; import android.os.Bundle; import android.annotation.IntDef; +import android.annotation.IntRange; import android.annotation.NonNull; import android.app.appsearch.exceptions.AppSearchException; @@ -55,7 +56,8 @@ public final class SearchSpec { public static final int DEFAULT_NUM_PER_PAGE = 10; - // TODO(b/170371356): In framework, we may want these limits might be flag controlled. + // TODO(b/170371356): In framework, we may want these limits to be flag controlled. + // If that happens, the @IntRange() directives in this class may have to change. private static final int MAX_NUM_PER_PAGE = 10_000; private static final int MAX_SNIPPET_COUNT = 10_000; private static final int MAX_SNIPPET_PER_PROPERTY_COUNT = 10_000; @@ -176,8 +178,8 @@ public final class SearchSpec { return Collections.unmodifiableList(namespaces); } - /** Returns the number of results per page in the returned object. */ - public int getNumPerPage() { + /** Returns the number of results per page in the result set. */ + public int getResultCountPerPage() { return mBundle.getInt(NUM_PER_PAGE_FIELD, DEFAULT_NUM_PER_PAGE); } @@ -288,10 +290,12 @@ public final class SearchSpec { /** * Sets the number of results per page in the returned object. - *

The default number of results per page is 10. And should be set in range [0, 10k]. + * + *

The default number of results per page is 10. */ @NonNull - public SearchSpec.Builder setNumPerPage(int numPerPage) { + public SearchSpec.Builder setResultCountPerPage( + @IntRange(from = 0, to = MAX_NUM_PER_PAGE) int numPerPage) { Preconditions.checkState(!mBuilt, "Builder has already been used"); Preconditions.checkArgumentInRange(numPerPage, 0, MAX_NUM_PER_PAGE, "NumPerPage"); mBundle.putInt(NUM_PER_PAGE_FIELD, numPerPage); @@ -309,8 +313,9 @@ public final class SearchSpec { } /** - * Indicates the order of returned search results, the default is DESC, meaning that results - * with higher scores come first. + * Indicates the order of returned search results, the default is + * {@link #ORDER_DESCENDING}, meaning that results with higher scores come first. + * *

This order field will be ignored if RankingStrategy = {@code RANKING_STRATEGY_NONE}. */ @NonNull @@ -328,11 +333,10 @@ public final class SearchSpec { * *

If set to 0 (default), snippeting is disabled and {@link SearchResult#getMatches} will * return {@code null} for that result. - * - *

The value should be set in range[0, 10k]. */ @NonNull - public SearchSpec.Builder setSnippetCount(int snippetCount) { + public SearchSpec.Builder setSnippetCount( + @IntRange(from = 0, to = MAX_SNIPPET_COUNT) int snippetCount) { Preconditions.checkState(!mBuilt, "Builder has already been used"); Preconditions.checkArgumentInRange(snippetCount, 0, MAX_SNIPPET_COUNT, "snippetCount"); mBundle.putInt(SNIPPET_COUNT_FIELD, snippetCount); @@ -345,11 +349,11 @@ public final class SearchSpec { * *

If set to 0, snippeting is disabled and {@link SearchResult#getMatches} * will return {@code null} for that result. - * - *

The value should be set in range[0, 10k]. */ @NonNull - public SearchSpec.Builder setSnippetCountPerProperty(int snippetCountPerProperty) { + public SearchSpec.Builder setSnippetCountPerProperty( + @IntRange(from = 0, to = MAX_SNIPPET_PER_PROPERTY_COUNT) + int snippetCountPerProperty) { Preconditions.checkState(!mBuilt, "Builder has already been used"); Preconditions.checkArgumentInRange(snippetCountPerProperty, 0, MAX_SNIPPET_PER_PROPERTY_COUNT, "snippetCountPerProperty"); @@ -368,11 +372,10 @@ public final class SearchSpec { * *

Ex. {@code maxSnippetSize} = 16. "foo bar baz bat rat" with a query of "baz" will * return a window of "bar baz bat" which is only 11 bytes long. - * - *

The value should be in range[0, 10k]. */ @NonNull - public SearchSpec.Builder setMaxSnippetSize(int maxSnippetSize) { + public SearchSpec.Builder setMaxSnippetSize( + @IntRange(from = 0, to = MAX_SNIPPET_SIZE_LIMIT) int maxSnippetSize) { Preconditions.checkState(!mBuilt, "Builder has already been used"); Preconditions.checkArgumentInRange( maxSnippetSize, 0, MAX_SNIPPET_SIZE_LIMIT, "maxSnippetSize"); diff --git a/apex/appsearch/framework/java/android/app/appsearch/SetSchemaRequest.java b/apex/appsearch/framework/java/android/app/appsearch/SetSchemaRequest.java index f2c81564908c0..09c353bef3fdc 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/SetSchemaRequest.java +++ b/apex/appsearch/framework/java/android/app/appsearch/SetSchemaRequest.java @@ -30,9 +30,9 @@ import java.util.List; import java.util.Set; /** - * Encapsulates a request to update the schema of an {@link AppSearchManager} database. + * Encapsulates a request to update the schema of an {@link AppSearchSession} database. * - * @see AppSearchManager#setSchema + * @see AppSearchSession#setSchema * @hide */ public final class SetSchemaRequest { @@ -82,9 +82,9 @@ public final class SetSchemaRequest { * follow the new schema. * *

By default, this is {@code false} and schema incompatibility causes the - * {@link AppSearchManager#setSchema} call to fail. + * {@link AppSearchSession#setSchema} call to fail. * - * @see AppSearchManager#setSchema + * @see AppSearchSession#setSchema */ @NonNull public Builder setForceOverride(boolean forceOverride) { diff --git a/apex/appsearch/framework/java/android/app/appsearch/exceptions/AppSearchException.java b/apex/appsearch/framework/java/android/app/appsearch/exceptions/AppSearchException.java index 15d0992cd0816..416e90ba709e0 100644 --- a/apex/appsearch/framework/java/android/app/appsearch/exceptions/AppSearchException.java +++ b/apex/appsearch/framework/java/android/app/appsearch/exceptions/AppSearchException.java @@ -21,13 +21,12 @@ import android.annotation.Nullable; import android.app.appsearch.AppSearchResult; /** - * An exception thrown by {@code android.app.appsearch.AppSearchManager} or a subcomponent. + * An exception thrown by {@link android.app.appsearch.AppSearchSession} or a subcomponent. * *

These exceptions can be converted into a failed {@link AppSearchResult} * for propagating to the client. * @hide */ -//TODO(b/157082794): Linkify to AppSearchManager once that API is public public class AppSearchException extends Exception { private final @AppSearchResult.ResultCode int mResultCode; 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 684bd2bd8205e..0b3c809aa9183 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 @@ -118,27 +118,28 @@ public final class AppSearchImpl { private final ReadWriteLock mReadWriteLock = new ReentrantReadWriteLock(); @GuardedBy("mReadWriteLock") - private final IcingSearchEngine mIcingSearchEngine; + private final IcingSearchEngine mIcingSearchEngineLocked; @GuardedBy("mReadWriteLock") - private final VisibilityStore mVisibilityStore; + private final VisibilityStore mVisibilityStoreLocked; // The map contains schemaTypes and namespaces for all database. All values in the map have // the database name prefix. // TODO(b/172360376): Check if this can be replaced with an ArrayMap @GuardedBy("mReadWriteLock") - private final Map> mSchemaMap = new HashMap<>(); + private final Map> mSchemaMapLocked = new HashMap<>(); // TODO(b/172360376): Check if this can be replaced with an ArrayMap @GuardedBy("mReadWriteLock") - private final Map> mNamespaceMap = new HashMap<>(); + private final Map> mNamespaceMapLocked = new HashMap<>(); /** - * The counter to check when to call {@link #checkForOptimize(boolean)}. The interval is + * The counter to check when to call {@link #checkForOptimizeLocked(boolean)}. The + * interval is * {@link #CHECK_OPTIMIZE_INTERVAL}. */ @GuardedBy("mReadWriteLock") - private int mOptimizeIntervalCount = 0; + private int mOptimizeIntervalCountLocked = 0; /** * Creates and initializes an instance of {@link AppSearchImpl} which writes data to the given @@ -161,15 +162,15 @@ public final class AppSearchImpl { // than once. It's unnecessary and can be a costly operation. IcingSearchEngineOptions options = IcingSearchEngineOptions.newBuilder() .setBaseDir(icingDir.getAbsolutePath()).build(); - mIcingSearchEngine = new IcingSearchEngine(options); + mIcingSearchEngineLocked = new IcingSearchEngine(options); - InitializeResultProto initializeResultProto = mIcingSearchEngine.initialize(); + InitializeResultProto initializeResultProto = mIcingSearchEngineLocked.initialize(); SchemaProto schemaProto = null; GetAllNamespacesResultProto getAllNamespacesResultProto = null; try { checkSuccess(initializeResultProto.getStatus()); - schemaProto = getSchemaProto(); - getAllNamespacesResultProto = mIcingSearchEngine.getAllNamespaces(); + schemaProto = getSchemaProtoLocked(); + getAllNamespacesResultProto = mIcingSearchEngineLocked.getAllNamespaces(); checkSuccess(getAllNamespacesResultProto.getStatus()); } catch (AppSearchException e) { Log.w(TAG, "Error initializing, resetting IcingSearchEngine.", e); @@ -181,22 +182,23 @@ public final class AppSearchImpl { // Populate schema map for (SchemaTypeConfigProto schema : schemaProto.getTypesList()) { String qualifiedSchemaType = schema.getSchemaType(); - addToMap(mSchemaMap, getDatabaseName(qualifiedSchemaType), qualifiedSchemaType); + addToMap(mSchemaMapLocked, getDatabaseName(qualifiedSchemaType), + qualifiedSchemaType); } // Populate namespace map for (String qualifiedNamespace : getAllNamespacesResultProto.getNamespacesList()) { - addToMap(mNamespaceMap, getDatabaseName(qualifiedNamespace), + addToMap(mNamespaceMapLocked, getDatabaseName(qualifiedNamespace), qualifiedNamespace); } // TODO(b/155939114): It's possible to optimize after init, which would reduce the time // to when we're able to serve queries. Consider moving this optimize call out. if (!isReset) { - checkForOptimize(/* force= */ true); + checkForOptimizeLocked(/* force= */ true); } - mVisibilityStore = new VisibilityStore(this); + mVisibilityStoreLocked = new VisibilityStore(this); } finally { mReadWriteLock.writeLock().unlock(); } @@ -208,7 +210,12 @@ public final class AppSearchImpl { * @throws AppSearchException on IcingSearchEngine error. */ void initializeVisibilityStore() throws AppSearchException { - mVisibilityStore.initialize(); + mReadWriteLock.writeLock().lock(); + try { + mVisibilityStoreLocked.initialize(); + } finally { + mReadWriteLock.writeLock().unlock(); + } } /** @@ -226,7 +233,7 @@ public final class AppSearchImpl { boolean forceOverride) throws AppSearchException { mReadWriteLock.writeLock().lock(); try { - SchemaProto.Builder existingSchemaBuilder = getSchemaProto().toBuilder(); + SchemaProto.Builder existingSchemaBuilder = getSchemaProtoLocked().toBuilder(); SchemaProto.Builder newSchemaBuilder = SchemaProto.newBuilder(); for (AppSearchSchema schema : schemas) { @@ -242,7 +249,8 @@ public final class AppSearchImpl { // Apply schema SetSchemaResultProto setSchemaResultProto = - mIcingSearchEngine.setSchema(existingSchemaBuilder.build(), forceOverride); + mIcingSearchEngineLocked.setSchema(existingSchemaBuilder.build(), + forceOverride); // Determine whether it succeeded. try { @@ -263,8 +271,8 @@ public final class AppSearchImpl { } // Update derived data structures. - mSchemaMap.put(databaseName, rewrittenSchemaResults.mRewrittenQualifiedTypes); - mVisibilityStore.updateSchemas(databaseName, + mSchemaMapLocked.put(databaseName, rewrittenSchemaResults.mRewrittenQualifiedTypes); + mVisibilityStoreLocked.updateSchemas(databaseName, rewrittenSchemaResults.mDeletedQualifiedTypes); // Determine whether to schedule an immediate optimize. @@ -274,7 +282,7 @@ public final class AppSearchImpl { // Any existing schemas which is not in 'schemas' will be deleted, and all // documents of these types were also deleted. And so well if we force override // incompatible schemas. - checkForOptimize(/* force= */true); + checkForOptimizeLocked(/* force= */true); } } finally { mReadWriteLock.writeLock().unlock(); @@ -301,7 +309,7 @@ public final class AppSearchImpl { Set qualifiedSchemasHiddenFromPlatformSurface = new ArraySet<>(schemasHiddenFromPlatformSurfaces.size()); for (String schema : schemasHiddenFromPlatformSurfaces) { - Set existingSchemas = mSchemaMap.get(databaseName); + Set existingSchemas = mSchemaMapLocked.get(databaseName); if (existingSchemas == null || !existingSchemas.contains(databasePrefix + schema)) { throw new AppSearchException(AppSearchResult.RESULT_NOT_FOUND, "Unknown schema(s): " + schemasHiddenFromPlatformSurfaces @@ -309,7 +317,8 @@ public final class AppSearchImpl { } qualifiedSchemasHiddenFromPlatformSurface.add(databasePrefix + schema); } - mVisibilityStore.setVisibility(databaseName, qualifiedSchemasHiddenFromPlatformSurface); + mVisibilityStoreLocked.setVisibility(databaseName, + qualifiedSchemasHiddenFromPlatformSurface); } finally { mReadWriteLock.writeLock().lock(); } @@ -333,11 +342,11 @@ public final class AppSearchImpl { PutResultProto putResultProto; mReadWriteLock.writeLock().lock(); try { - putResultProto = mIcingSearchEngine.put(documentBuilder.build()); - addToMap(mNamespaceMap, databaseName, documentBuilder.getNamespace()); + putResultProto = mIcingSearchEngineLocked.put(documentBuilder.build()); + addToMap(mNamespaceMapLocked, databaseName, documentBuilder.getNamespace()); // The existing documents with same URI will be deleted, so there maybe some resources // could be released after optimize(). - checkForOptimize(/* force= */false); + checkForOptimizeLocked(/* force= */false); } finally { mReadWriteLock.writeLock().unlock(); } @@ -361,7 +370,7 @@ public final class AppSearchImpl { GetResultProto getResultProto; mReadWriteLock.readLock().lock(); try { - getResultProto = mIcingSearchEngine.get( + getResultProto = mIcingSearchEngineLocked.get( getDatabasePrefix(databaseName) + namespace, uri); } finally { mReadWriteLock.readLock().unlock(); @@ -390,7 +399,13 @@ public final class AppSearchImpl { @NonNull String databaseName, @NonNull String queryExpression, @NonNull SearchSpec searchSpec) throws AppSearchException { - return doQuery(Collections.singleton(databaseName), queryExpression, searchSpec); + mReadWriteLock.readLock().lock(); + try { + return doQueryLocked(Collections.singleton(databaseName), queryExpression, + searchSpec); + } finally { + mReadWriteLock.readLock().unlock(); + } } /** @@ -412,10 +427,18 @@ public final class AppSearchImpl { // TODO(b/169883602): Check if the platform is querying us at a higher level. At this // point, we should add all platform-surfaceable schemas assuming the querier has been // verified. - return doQuery(mNamespaceMap.keySet(), queryExpression, searchSpec); + mReadWriteLock.readLock().lock(); + try { + // We use the mNamespaceMap.keySet here because it's the smaller set of valid databases + // that could exist. + return doQueryLocked(mNamespaceMapLocked.keySet(), queryExpression, searchSpec); + } finally { + mReadWriteLock.readLock().unlock(); + } } - private SearchResultPage doQuery( + @GuardedBy("mReadWriteLock") + private SearchResultPage doQueryLocked( @NonNull Set databases, @NonNull String queryExpression, @NonNull SearchSpec searchSpec) throws AppSearchException { @@ -427,22 +450,19 @@ public final class AppSearchImpl { ResultSpecProto resultSpec = SearchSpecToProtoConverter.toResultSpecProto(searchSpec); ScoringSpecProto scoringSpec = SearchSpecToProtoConverter.toScoringSpecProto(searchSpec); SearchResultProto searchResultProto; - mReadWriteLock.readLock().lock(); - try { - // rewriteSearchSpecForDatabases will return false if none of the databases that the - // client is trying to search on exist, so we can return an empty SearchResult and skip - // sending request to Icing. - // We use the mNamespaceMap.keySet here because it's the smaller set of valid databases - // that could exist. - if (!rewriteSearchSpecForDatabases(searchSpecBuilder, databases)) { - return new SearchResultPage(Bundle.EMPTY); - } - searchResultProto = mIcingSearchEngine.search( - searchSpecBuilder.build(), scoringSpec, resultSpec); - } finally { - mReadWriteLock.readLock().unlock(); + + // rewriteSearchSpecForDatabases will return false if none of the databases that the + // client is trying to search on exist, so we can return an empty SearchResult and skip + // sending request to Icing. + // We use the mNamespaceMap.keySet here because it's the smaller set of valid databases + // that could exist. + if (!rewriteSearchSpecForDatabasesLocked(searchSpecBuilder, databases)) { + return new SearchResultPage(Bundle.EMPTY); } + searchResultProto = mIcingSearchEngineLocked.search( + searchSpecBuilder.build(), scoringSpec, resultSpec); checkSuccess(searchResultProto.getStatus()); + return rewriteSearchResultProto(searchResultProto); } @@ -459,9 +479,15 @@ public final class AppSearchImpl { @NonNull public SearchResultPage getNextPage(long nextPageToken) throws AppSearchException { - SearchResultProto searchResultProto = mIcingSearchEngine.getNextPage(nextPageToken); - checkSuccess(searchResultProto.getStatus()); - return rewriteSearchResultProto(searchResultProto); + mReadWriteLock.readLock().lock(); + try { + SearchResultProto searchResultProto = mIcingSearchEngineLocked.getNextPage( + nextPageToken); + checkSuccess(searchResultProto.getStatus()); + return rewriteSearchResultProto(searchResultProto); + } finally { + mReadWriteLock.readLock().unlock(); + } } /** @@ -473,7 +499,12 @@ public final class AppSearchImpl { * Invalidated. */ public void invalidateNextPageToken(long nextPageToken) { - mIcingSearchEngine.invalidateNextPageToken(nextPageToken); + mReadWriteLock.readLock().lock(); + try { + mIcingSearchEngineLocked.invalidateNextPageToken(nextPageToken); + } finally { + mReadWriteLock.readLock().unlock(); + } } /** @@ -492,8 +523,8 @@ public final class AppSearchImpl { DeleteResultProto deleteResultProto; mReadWriteLock.writeLock().lock(); try { - deleteResultProto = mIcingSearchEngine.delete(qualifiedNamespace, uri); - checkForOptimize(/* force= */false); + deleteResultProto = mIcingSearchEngineLocked.delete(qualifiedNamespace, uri); + checkForOptimizeLocked(/* force= */false); } finally { mReadWriteLock.writeLock().unlock(); } @@ -513,7 +544,6 @@ public final class AppSearchImpl { public void removeByQuery(@NonNull String databaseName, @NonNull String queryExpression, @NonNull SearchSpec searchSpec) throws AppSearchException { - SearchSpecProto searchSpecProto = SearchSpecToProtoConverter.toSearchSpecProto(searchSpec); SearchSpecProto.Builder searchSpecBuilder = searchSpecProto.toBuilder() @@ -524,13 +554,13 @@ public final class AppSearchImpl { // Only rewrite SearchSpec for non empty database. // rewriteSearchSpecForNonEmptyDatabase will return false for empty database, we // should skip sending request to Icing and return in here. - if (!rewriteSearchSpecForDatabases(searchSpecBuilder, + if (!rewriteSearchSpecForDatabasesLocked(searchSpecBuilder, Collections.singleton(databaseName))) { return; } - deleteResultProto = mIcingSearchEngine.deleteByQuery( + deleteResultProto = mIcingSearchEngineLocked.deleteByQuery( searchSpecBuilder.build()); - checkForOptimize(/* force= */true); + checkForOptimizeLocked(/* force= */true); } finally { mReadWriteLock.writeLock().unlock(); } @@ -551,14 +581,14 @@ public final class AppSearchImpl { ResetResultProto resetResultProto; mReadWriteLock.writeLock().lock(); try { - resetResultProto = mIcingSearchEngine.reset(); - mOptimizeIntervalCount = 0; - mSchemaMap.clear(); - mNamespaceMap.clear(); + resetResultProto = mIcingSearchEngineLocked.reset(); + mOptimizeIntervalCountLocked = 0; + mSchemaMapLocked.clear(); + mNamespaceMapLocked.clear(); // Must be called after everything else since VisibilityStore may repopulate // IcingSearchEngine with an initial schema. - mVisibilityStore.handleReset(); + mVisibilityStoreLocked.handleReset(); } finally { mReadWriteLock.writeLock().unlock(); } @@ -589,7 +619,7 @@ public final class AppSearchImpl { * database as well as a set of schema types that were deleted from the database. */ @VisibleForTesting - RewrittenSchemaResults rewriteSchema(@NonNull String databaseName, + static RewrittenSchemaResults rewriteSchema(@NonNull String databaseName, @NonNull SchemaProto.Builder existingSchema, @NonNull SchemaProto newSchema) throws AppSearchException { String prefix = getDatabasePrefix(databaseName); @@ -655,7 +685,7 @@ public final class AppSearchImpl { * @param prefix The prefix to add */ @VisibleForTesting - void addPrefixToDocument( + static void addPrefixToDocument( @NonNull DocumentProto.Builder documentBuilder, @NonNull String prefix) { // Rewrite the type name to include/remove the prefix. @@ -691,7 +721,7 @@ public final class AppSearchImpl { * @param documentBuilder The document to mutate */ @VisibleForTesting - void removeDatabasesFromDocument(@NonNull DocumentProto.Builder documentBuilder) + static void removeDatabasesFromDocument(@NonNull DocumentProto.Builder documentBuilder) throws AppSearchException { // Rewrite the type name and namespace to remove the prefix. documentBuilder.setSchema(removeDatabasePrefix(documentBuilder.getSchema())); @@ -728,11 +758,11 @@ public final class AppSearchImpl { */ @VisibleForTesting @GuardedBy("mReadWriteLock") - boolean rewriteSearchSpecForDatabases( + boolean rewriteSearchSpecForDatabasesLocked( @NonNull SearchSpecProto.Builder searchSpecBuilder, @NonNull Set databaseNames) { // Create a copy since retainAll() modifies the original set. - Set existingDatabases = new ArraySet<>(mNamespaceMap.keySet()); + Set existingDatabases = new ArraySet<>(mNamespaceMapLocked.keySet()); existingDatabases.retainAll(databaseNames); if (existingDatabases.isEmpty()) { @@ -749,7 +779,7 @@ public final class AppSearchImpl { // Rewrite filters to include a database prefix. for (String databaseName : existingDatabases) { - Set existingSchemaTypes = mSchemaMap.get(databaseName); + Set existingSchemaTypes = mSchemaMapLocked.get(databaseName); String databaseNamePrefix = getDatabasePrefix(databaseName); if (schemaTypeFilters.isEmpty()) { // Include all schema types @@ -764,7 +794,7 @@ public final class AppSearchImpl { } } - Set existingNamespaces = mNamespaceMap.get(databaseName); + Set existingNamespaces = mNamespaceMapLocked.get(databaseName); if (namespaceFilters.isEmpty()) { // Include all namespaces searchSpecBuilder.addAllNamespaceFilters(existingNamespaces); @@ -783,8 +813,9 @@ public final class AppSearchImpl { } @VisibleForTesting - SchemaProto getSchemaProto() throws AppSearchException { - GetSchemaResultProto schemaProto = mIcingSearchEngine.getSchema(); + @GuardedBy("mReadWriteLock") + SchemaProto getSchemaProtoLocked() throws AppSearchException { + GetSchemaResultProto schemaProto = mIcingSearchEngineLocked.getSchema(); // 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); @@ -793,11 +824,11 @@ public final class AppSearchImpl { /** Returns true if {@code databaseName} has a {@code schemaType} */ @GuardedBy("mReadWriteLock") - boolean hasSchemaType(@NonNull String databaseName, @NonNull String schemaType) { + boolean hasSchemaTypeLocked(@NonNull String databaseName, @NonNull String schemaType) { Preconditions.checkNotNull(databaseName); Preconditions.checkNotNull(schemaType); - Set schemaTypes = mSchemaMap.get(databaseName); + Set schemaTypes = mSchemaMapLocked.get(databaseName); if (schemaTypes == null) { return false; } @@ -806,9 +837,10 @@ public final class AppSearchImpl { } /** Returns a set of all databases AppSearchImpl knows about. */ + @GuardedBy("mReadWriteLock") @NonNull - Set getDatabases() { - return mSchemaMap.keySet(); + Set getDatabasesLocked() { + return mSchemaMapLocked.keySet(); } @NonNull @@ -830,7 +862,7 @@ public final class AppSearchImpl { } @NonNull - private String getDatabaseName(@NonNull String prefixedValue) throws AppSearchException { + private static String getDatabaseName(@NonNull String prefixedValue) throws AppSearchException { int delimiterIndex = prefixedValue.indexOf(DATABASE_DELIMITER); if (delimiterIndex == -1) { throw new AppSearchException(AppSearchResult.RESULT_UNKNOWN_ERROR, @@ -839,8 +871,8 @@ public final class AppSearchImpl { return prefixedValue.substring(0, delimiterIndex); } - @GuardedBy("mReadWriteLock") - private void addToMap(Map> map, String databaseName, String prefixedValue) { + private static void addToMap(Map> map, String databaseName, + String prefixedValue) { Set values = map.get(databaseName); if (values == null) { values = new ArraySet<>(); @@ -854,7 +886,7 @@ public final class AppSearchImpl { * * @throws AppSearchException on error codes. */ - private void checkSuccess(StatusProto statusProto) throws AppSearchException { + private static void checkSuccess(StatusProto statusProto) throws AppSearchException { checkCodeOneOf(statusProto, StatusProto.Code.OK); } @@ -862,7 +894,7 @@ public final class AppSearchImpl { * Checks the given status code is one of the provided codes, and throws an * {@link AppSearchException} if it is not. */ - private void checkCodeOneOf(StatusProto statusProto, StatusProto.Code... codes) + private static void checkCodeOneOf(StatusProto statusProto, StatusProto.Code... codes) throws AppSearchException { for (int i = 0; i < codes.length; i++) { if (codes[i] == statusProto.getCode()) { @@ -894,11 +926,11 @@ public final class AppSearchImpl { * @param force whether we should directly call {@link IcingSearchEngine#getOptimizeInfo()}. */ @GuardedBy("mReadWriteLock") - private void checkForOptimize(boolean force) throws AppSearchException { - ++mOptimizeIntervalCount; - if (force || mOptimizeIntervalCount >= CHECK_OPTIMIZE_INTERVAL) { - mOptimizeIntervalCount = 0; - GetOptimizeInfoResultProto optimizeInfo = getOptimizeInfoResult(); + private void checkForOptimizeLocked(boolean force) throws AppSearchException { + ++mOptimizeIntervalCountLocked; + if (force || mOptimizeIntervalCountLocked >= CHECK_OPTIMIZE_INTERVAL) { + mOptimizeIntervalCountLocked = 0; + GetOptimizeInfoResultProto optimizeInfo = getOptimizeInfoResultLocked(); checkSuccess(optimizeInfo.getStatus()); // Second threshold, decide when to call optimize(). if (optimizeInfo.getOptimizableDocs() >= OPTIMIZE_THRESHOLD_DOC_COUNT @@ -906,7 +938,7 @@ public final class AppSearchImpl { >= OPTIMIZE_THRESHOLD_BYTES) { // TODO(b/155939114): call optimize in the same thread will slow down api calls // significantly. Move this call to background. - OptimizeResultProto optimizeResultProto = mIcingSearchEngine.optimize(); + OptimizeResultProto optimizeResultProto = mIcingSearchEngineLocked.optimize(); checkSuccess(optimizeResultProto.getStatus()); } // TODO(b/147699081): Return OptimizeResultProto & log lost data detail once we add @@ -916,7 +948,7 @@ public final class AppSearchImpl { } /** Remove the rewritten schema types from any result documents. */ - private SearchResultPage rewriteSearchResultProto( + private static SearchResultPage rewriteSearchResultProto( @NonNull SearchResultProto searchResultProto) throws AppSearchException { SearchResultProto.Builder resultsBuilder = searchResultProto.toBuilder(); for (int i = 0; i < searchResultProto.getResultsCount(); i++) { @@ -932,14 +964,16 @@ public final class AppSearchImpl { return SearchResultToProtoConverter.convertToSearchResultPage(resultsBuilder); } + @GuardedBy("mReadWriteLock") @VisibleForTesting - GetOptimizeInfoResultProto getOptimizeInfoResult() { - return mIcingSearchEngine.getOptimizeInfo(); + GetOptimizeInfoResultProto getOptimizeInfoResultLocked() { + return mIcingSearchEngineLocked.getOptimizeInfo(); } + @GuardedBy("mReadWriteLock") @VisibleForTesting - VisibilityStore getVisibilityStore() { - return mVisibilityStore; + VisibilityStore getVisibilityStoreLocked() { + return mVisibilityStoreLocked; } /** @@ -950,7 +984,7 @@ public final class AppSearchImpl { * AppSearchException. * @return AppSearchException with the parallel error code. */ - private AppSearchException statusProtoToAppSearchException(StatusProto statusProto) { + private static AppSearchException statusProtoToAppSearchException(StatusProto statusProto) { switch (statusProto.getCode()) { case INVALID_ARGUMENT: return new AppSearchException(AppSearchResult.RESULT_INVALID_ARGUMENT, diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/VisibilityStore.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/VisibilityStore.java index 24238c56d1e5f..07c64da10bc80 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/VisibilityStore.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/VisibilityStore.java @@ -90,7 +90,7 @@ class VisibilityStore { * @throws AppSearchException AppSearchException on AppSearchImpl error. */ public void initialize() throws AppSearchException { - if (!mAppSearchImpl.hasSchemaType(DATABASE_NAME, SCHEMA_TYPE)) { + if (!mAppSearchImpl.hasSchemaTypeLocked(DATABASE_NAME, SCHEMA_TYPE)) { // Schema type doesn't exist yet. Add it. mAppSearchImpl.setSchema(DATABASE_NAME, Collections.singleton(new AppSearchSchema.Builder(SCHEMA_TYPE) @@ -105,7 +105,7 @@ class VisibilityStore { } // Populate visibility settings map - for (String database : mAppSearchImpl.getDatabases()) { + for (String database : mAppSearchImpl.getDatabasesLocked()) { if (database.equals(DATABASE_NAME)) { // Our own database. Skip continue; diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/converter/SearchSpecToProtoConverter.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/converter/SearchSpecToProtoConverter.java index c1b827fe25e08..37f64578151f6 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/converter/SearchSpecToProtoConverter.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/converter/SearchSpecToProtoConverter.java @@ -57,7 +57,7 @@ public final class SearchSpecToProtoConverter { public static ResultSpecProto toResultSpecProto(@NonNull SearchSpec spec) { Preconditions.checkNotNull(spec); return ResultSpecProto.newBuilder() - .setNumPerPage(spec.getNumPerPage()) + .setNumPerPage(spec.getResultCountPerPage()) .setSnippetSpec( ResultSpecProto.SnippetSpecProto.newBuilder() .setNumToSnippet(spec.getSnippetCount()) diff --git a/apex/appsearch/synced_jetpack_changeid.txt b/apex/appsearch/synced_jetpack_changeid.txt index a8e72ddee7bfc..1747503d952be 100644 --- a/apex/appsearch/synced_jetpack_changeid.txt +++ b/apex/appsearch/synced_jetpack_changeid.txt @@ -1 +1 @@ -I9ba99ecc4f9a7eb177e678d22d083750efce81b5 +Iac5514634d17fbe20e1dda04df5ee27a22e89304 diff --git a/core/tests/coretests/src/android/app/appsearch/external/app/SearchSpecTest.java b/core/tests/coretests/src/android/app/appsearch/external/app/SearchSpecTest.java index 4747fe4a9beef..4bfb4a1fb3af4 100644 --- a/core/tests/coretests/src/android/app/appsearch/external/app/SearchSpecTest.java +++ b/core/tests/coretests/src/android/app/appsearch/external/app/SearchSpecTest.java @@ -32,7 +32,7 @@ public class SearchSpecTest { .setSnippetCount(5) .setSnippetCountPerProperty(10) .setMaxSnippetSize(15) - .setNumPerPage(42) + .setResultCountPerPage(42) .setOrder(SearchSpec.ORDER_ASCENDING) .setRankingStrategy(SearchSpec.RANKING_STRATEGY_DOCUMENT_SCORE) .build(); diff --git a/core/tests/coretests/src/android/app/appsearch/external/app/cts/SearchSpecCtsTest.java b/core/tests/coretests/src/android/app/appsearch/external/app/cts/SearchSpecCtsTest.java index c5b800ab18e74..e50694585975e 100644 --- a/core/tests/coretests/src/android/app/appsearch/external/app/cts/SearchSpecCtsTest.java +++ b/core/tests/coretests/src/android/app/appsearch/external/app/cts/SearchSpecCtsTest.java @@ -42,7 +42,7 @@ public class SearchSpecCtsTest { .setSnippetCount(5) .setSnippetCountPerProperty(10) .setMaxSnippetSize(15) - .setNumPerPage(42) + .setResultCountPerPage(42) .setOrder(SearchSpec.ORDER_ASCENDING) .setRankingStrategy(SearchSpec.RANKING_STRATEGY_DOCUMENT_SCORE) .build(); @@ -55,7 +55,7 @@ public class SearchSpecCtsTest { assertThat(searchSpec.getSnippetCount()).isEqualTo(5); assertThat(searchSpec.getSnippetCountPerProperty()).isEqualTo(10); assertThat(searchSpec.getMaxSnippetSize()).isEqualTo(15); - assertThat(searchSpec.getNumPerPage()).isEqualTo(42); + assertThat(searchSpec.getResultCountPerPage()).isEqualTo(42); assertThat(searchSpec.getOrder()).isEqualTo(SearchSpec.ORDER_ASCENDING); assertThat(searchSpec.getRankingStrategy()) .isEqualTo(SearchSpec.RANKING_STRATEGY_DOCUMENT_SCORE); 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 40c795b562321..bc7df550f3163 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 @@ -292,7 +292,7 @@ public class AppSearchImplTest { } // Check optimize() will release 0 docs since there is no deletion. - GetOptimizeInfoResultProto optimizeInfo = mAppSearchImpl.getOptimizeInfoResult(); + GetOptimizeInfoResultProto optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); assertThat(optimizeInfo.getOptimizableDocs()).isEqualTo(0); // delete 999 documents , we will reach the threshold to trigger optimize() in next @@ -302,7 +302,7 @@ public class AppSearchImplTest { } // optimize() still not be triggered since we are in the interval to call getOptimizeInfo() - optimizeInfo = mAppSearchImpl.getOptimizeInfoResult(); + optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); assertThat(optimizeInfo.getOptimizableDocs()) .isEqualTo(AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - 1); @@ -314,7 +314,7 @@ public class AppSearchImplTest { } // Verify optimize() is triggered - optimizeInfo = mAppSearchImpl.getOptimizeInfoResult(); + optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); assertThat(optimizeInfo.getOptimizableDocs()) .isLessThan(AppSearchImpl.CHECK_OPTIMIZE_INTERVAL); } @@ -335,8 +335,9 @@ public class AppSearchImplTest { mAppSearchImpl.putDocument("database", document); // Rewrite SearchSpec - mAppSearchImpl.rewriteSearchSpecForDatabases(searchSpecProto, Collections.singleton( - "database")); + mAppSearchImpl.rewriteSearchSpecForDatabasesLocked(searchSpecProto, + Collections.singleton( + "database")); assertThat(searchSpecProto.getSchemaTypeFiltersList()).containsExactly("database/type"); assertThat(searchSpecProto.getNamespaceFiltersList()).containsExactly("database/namespace"); } @@ -363,7 +364,7 @@ public class AppSearchImplTest { mAppSearchImpl.putDocument("database2", document2); // Rewrite SearchSpec - mAppSearchImpl.rewriteSearchSpecForDatabases(searchSpecProto, + mAppSearchImpl.rewriteSearchSpecForDatabasesLocked(searchSpecProto, ImmutableSet.of("database1", "database2")); assertThat(searchSpecProto.getSchemaTypeFiltersList()).containsExactly( "database1/typeA", "database1/typeB", "database2/typeA", "database2/typeB"); @@ -424,7 +425,7 @@ public class AppSearchImplTest { List expectedTypes = new ArrayList<>(); expectedTypes.add(mVisibilitySchemaProto); expectedTypes.addAll(expectedProto.getTypesList()); - assertThat(mAppSearchImpl.getSchemaProto().getTypesList()) + assertThat(mAppSearchImpl.getSchemaProtoLocked().getTypesList()) .containsExactlyElementsIn(expectedTypes); } @@ -435,7 +436,7 @@ public class AppSearchImplTest { mAppSearchImpl.setVisibility("database", Set.of("schema1")); // "schema1" is platform hidden now - assertThat(mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas( + assertThat(mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( "database")).containsExactly("database/schema1"); // Add a new schema, and include the already-existing "schema1" @@ -445,7 +446,7 @@ public class AppSearchImplTest { // Check that "schema1" is still platform hidden, but "schema2" is the default platform // visible. - assertThat(mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas( + assertThat(mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( "database")).containsExactly("database/schema1"); } @@ -467,7 +468,7 @@ public class AppSearchImplTest { List expectedTypes = new ArrayList<>(); expectedTypes.add(mVisibilitySchemaProto); expectedTypes.addAll(expectedProto.getTypesList()); - assertThat(mAppSearchImpl.getSchemaProto().getTypesList()) + assertThat(mAppSearchImpl.getSchemaProtoLocked().getTypesList()) .containsExactlyElementsIn(expectedTypes); final Set finalSchemas = Collections.singleton(new AppSearchSchema.Builder( @@ -489,7 +490,7 @@ public class AppSearchImplTest { expectedTypes = new ArrayList<>(); expectedTypes.add(mVisibilitySchemaProto); expectedTypes.addAll(expectedProto.getTypesList()); - assertThat(mAppSearchImpl.getSchemaProto().getTypesList()) + assertThat(mAppSearchImpl.getSchemaProtoLocked().getTypesList()) .containsExactlyElementsIn(expectedTypes); } @@ -516,7 +517,7 @@ public class AppSearchImplTest { List expectedTypes = new ArrayList<>(); expectedTypes.add(mVisibilitySchemaProto); expectedTypes.addAll(expectedProto.getTypesList()); - assertThat(mAppSearchImpl.getSchemaProto().getTypesList()) + assertThat(mAppSearchImpl.getSchemaProtoLocked().getTypesList()) .containsExactlyElementsIn(expectedTypes); // Save only Email to database1 this time. @@ -535,7 +536,7 @@ public class AppSearchImplTest { expectedTypes = new ArrayList<>(); expectedTypes.add(mVisibilitySchemaProto); expectedTypes.addAll(expectedProto.getTypesList()); - assertThat(mAppSearchImpl.getSchemaProto().getTypesList()) + assertThat(mAppSearchImpl.getSchemaProtoLocked().getTypesList()) .containsExactlyElementsIn(expectedTypes); } @@ -547,7 +548,7 @@ public class AppSearchImplTest { mAppSearchImpl.setVisibility("database", Set.of("schema1")); // "schema1" is platform hidden now - assertThat(mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas( + assertThat(mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( "database")).containsExactly("database/schema1"); // Remove "schema1" by force overriding @@ -555,14 +556,16 @@ public class AppSearchImplTest { // Check that "schema1" is no longer considered platform hidden assertThat( - mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas("database")).isEmpty(); + mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( + "database")).isEmpty(); // Add "schema1" back, it gets default visibility settings which means it's not platform // hidden. mAppSearchImpl.setSchema("database", Collections.singleton(new AppSearchSchema.Builder( "schema1").build()), /*forceOverride=*/false); assertThat( - mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas("database")).isEmpty(); + mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( + "database")).isEmpty(); } @Test @@ -570,7 +573,8 @@ public class AppSearchImplTest { mAppSearchImpl.setSchema("database", Collections.singleton(new AppSearchSchema.Builder( "Schema").build()), /*forceOverride=*/false); assertThat( - mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas("database")).isEmpty(); + mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( + "database")).isEmpty(); } @Test @@ -578,7 +582,7 @@ public class AppSearchImplTest { mAppSearchImpl.setSchema("database", Collections.singleton(new AppSearchSchema.Builder( "Schema").build()), /*forceOverride=*/false); mAppSearchImpl.setVisibility("database", Set.of("Schema")); - assertThat(mAppSearchImpl.getVisibilityStore().getPlatformHiddenSchemas( + assertThat(mAppSearchImpl.getVisibilityStoreLocked().getPlatformHiddenSchemas( "database")).containsExactly("database/Schema"); } @@ -597,30 +601,31 @@ public class AppSearchImplTest { @Test public void testHasSchemaType() throws Exception { // Nothing exists yet - assertThat(mAppSearchImpl.hasSchemaType("database", "Schema")).isFalse(); + assertThat(mAppSearchImpl.hasSchemaTypeLocked("database", "Schema")).isFalse(); mAppSearchImpl.setSchema("database", Collections.singleton(new AppSearchSchema.Builder( "Schema").build()), /*forceOverride=*/false); - assertThat(mAppSearchImpl.hasSchemaType("database", "Schema")).isTrue(); + assertThat(mAppSearchImpl.hasSchemaTypeLocked("database", "Schema")).isTrue(); - assertThat(mAppSearchImpl.hasSchemaType("database", "UnknownSchema")).isFalse(); + assertThat(mAppSearchImpl.hasSchemaTypeLocked("database", "UnknownSchema")).isFalse(); } @Test public void testGetDatabases() throws Exception { // No client databases exist yet, but the VisibilityStore's does - assertThat(mAppSearchImpl.getDatabases()).containsExactly(VisibilityStore.DATABASE_NAME); + assertThat(mAppSearchImpl.getDatabasesLocked()).containsExactly( + VisibilityStore.DATABASE_NAME); // Has database1 mAppSearchImpl.setSchema("database1", Collections.singleton(new AppSearchSchema.Builder( "schema").build()), /*forceOverride=*/false); - assertThat(mAppSearchImpl.getDatabases()).containsExactly( + assertThat(mAppSearchImpl.getDatabasesLocked()).containsExactly( VisibilityStore.DATABASE_NAME, "database1"); // Has both databases mAppSearchImpl.setSchema("database2", Collections.singleton(new AppSearchSchema.Builder( "schema").build()), /*forceOverride=*/false); - assertThat(mAppSearchImpl.getDatabases()).containsExactly( + assertThat(mAppSearchImpl.getDatabasesLocked()).containsExactly( VisibilityStore.DATABASE_NAME, "database1", "database2"); } } diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/VisibilityStoreTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/VisibilityStoreTest.java index ddf0808f7c751..29619feb03c23 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/VisibilityStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/VisibilityStoreTest.java @@ -36,7 +36,7 @@ public class VisibilityStoreTest { @Before public void setUp() throws Exception { mAppSearchImpl = AppSearchImpl.create(mTemporaryFolder.newFolder()); - mVisibilityStore = mAppSearchImpl.getVisibilityStore(); + mVisibilityStore = mAppSearchImpl.getVisibilityStoreLocked(); } @Test