Correctly handle change notifications during schema migrations.
Our documentation promises that only SchemaChangeInfo will be dispatched
during schema migrations, but the implementation was that individual
DocumentChangeInfos were being dispatched for every migrated document,
contrary to our documentation.
Bug: 193494000
Test: GlobalSearchSessionCtsTest#testRegisterObserver_schemaMigration
Change-Id: I73e60876c12e6240f9b7782f9c58fc9888aaef3a
diff --git a/service/java/com/android/server/appsearch/AppSearchManagerService.java b/service/java/com/android/server/appsearch/AppSearchManagerService.java
index 658b8fc..e18d8e1 100644
--- a/service/java/com/android/server/appsearch/AppSearchManagerService.java
+++ b/service/java/com/android/server/appsearch/AppSearchManagerService.java
@@ -479,6 +479,7 @@
callerAttributionSource.getPackageName(),
databaseName,
document,
+ /*sendChangeNotifications=*/ true,
instance.getLogger());
resultBuilder.setSuccess(document.getId(), /*value=*/ null);
++operationSuccessCount;
@@ -921,10 +922,13 @@
break;
}
try {
+ // Per this method's documentation, individual document change
+ // notifications are not dispatched.
instance.getAppSearchImpl().putDocument(
callerAttributionSource.getPackageName(),
databaseName,
document,
+ /*sendChangeNotifications=*/ false,
/*logger=*/ null);
} catch (Throwable t) {
migrationFailureBundles.add(new SetSchemaResponse.MigrationFailure(
diff --git a/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java b/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
index 0341b34..6ed6c55 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java
@@ -557,6 +557,18 @@
version,
setSchemaStatsBuilder);
+ // This check is needed wherever setSchema is called to detect soft errors which do not
+ // throw an exception but also prevent the schema from actually being applied.
+ // TODO(b/229874420): Improve the usability of doSetSchemaNoChangeNotificationLocked() by
+ // adding documentation that the return value must be checked for these conditions, and by
+ // making it easier to check for these conditions via one of the ways documented in the
+ // bug.
+ if (!forceOverride
+ && (!setSchemaResponse.getDeletedTypes().isEmpty()
+ || !setSchemaResponse.getIncompatibleTypes().isEmpty())) {
+ return setSchemaResponse;
+ }
+
// Cache some lookup tables to help us work with the new schema
Map<String, AppSearchSchema> newSchemaNameToType = new ArrayMap<>(schemas.size());
// Maps unprefixed schema name to the set of listening packages that have visibility into
@@ -903,12 +915,15 @@
* @param packageName The package name that owns this document.
* @param databaseName The databaseName this document resides in.
* @param document The document to index.
+ * @param sendChangeNotifications Whether to dispatch {@link
+ * android.app.appsearch.observer.DocumentChangeInfo} messages to observers for this change.
* @throws AppSearchException on IcingSearchEngine error.
*/
public void putDocument(
@NonNull String packageName,
@NonNull String databaseName,
@NonNull GenericDocument document,
+ boolean sendChangeNotifications,
@Nullable AppSearchLogger logger)
throws AppSearchException {
PutDocumentStats.Builder pStatsBuilder = null;
@@ -967,14 +982,16 @@
checkSuccess(putResultProto.getStatus());
// Prepare notifications
- mObserverManager.onDocumentChange(
- packageName,
- databaseName,
- document.getNamespace(),
- document.getSchemaType(),
- document.getId(),
- mVisibilityStoreLocked,
- mVisibilityCheckerLocked);
+ if (sendChangeNotifications) {
+ mObserverManager.onDocumentChange(
+ packageName,
+ databaseName,
+ document.getNamespace(),
+ document.getSchemaType(),
+ document.getId(),
+ mVisibilityStoreLocked,
+ mVisibilityCheckerLocked);
+ }
} finally {
mReadWriteLock.writeLock().unlock();
diff --git a/service/java/com/android/server/appsearch/external/localstorage/visibilitystore/VisibilityStore.java b/service/java/com/android/server/appsearch/external/localstorage/visibilitystore/VisibilityStore.java
index c7f4eaa..b8cd0ad 100644
--- a/service/java/com/android/server/appsearch/external/localstorage/visibilitystore/VisibilityStore.java
+++ b/service/java/com/android/server/appsearch/external/localstorage/visibilitystore/VisibilityStore.java
@@ -159,6 +159,7 @@
VISIBILITY_PACKAGE_NAME,
VISIBILITY_DATABASE_NAME,
prefixedVisibilityDocument,
+ /*sendChangeNotifications=*/ false,
/*logger=*/ null);
mVisibilityDocumentMap.put(
prefixedVisibilityDocument.getId(), prefixedVisibilityDocument);
@@ -265,6 +266,7 @@
VISIBILITY_PACKAGE_NAME,
VISIBILITY_DATABASE_NAME,
migratedDocument,
+ /*sendChangeNotifications=*/ false,
/*logger=*/ null);
}
}