[builder] Fix handling of 'active_configurations' array in scripts
Change-Id: I1734c4880c96ba944e23dd0a81dc066be58e4113
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/219463
Commit-Queue: William Hesse <whesse@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/builder/lib/src/builder.dart b/builder/lib/src/builder.dart
index 0d61846..e10b61d 100644
--- a/builder/lib/src/builder.dart
+++ b/builder/lib/src/builder.dart
@@ -189,7 +189,8 @@
// 'approved: $approved');
}
// Removes the configuration from the list of active configurations.
- await firestore.updateActiveResult(activeResult, change['configuration']);
+ await firestore.removeActiveConfiguration(
+ activeResult, change['configuration']);
}
}
}
diff --git a/builder/lib/src/firestore.dart b/builder/lib/src/firestore.dart
index da9f1eb..4a3caf5 100644
--- a/builder/lib/src/firestore.dart
+++ b/builder/lib/src/firestore.dart
@@ -281,11 +281,12 @@
?.name;
}
- Future<void> storeResult(Map<String, dynamic> result) async {
+ Future<Document> storeResult(Map<String, dynamic> result) async {
final document = Document()..fields = taggedMap(result);
final createdDocument = await firestore.projects.databases.documents
.createDocument(document, documents, 'results');
log('created document ${createdDocument.name}');
+ return createdDocument;
}
Future<bool> updateResult(
@@ -465,16 +466,14 @@
/// Removes [configuration] from the active configurations and marks the
/// active result inactive when we remove the last active config.
- Future<void> updateActiveResult(
+ Future<void> removeActiveConfiguration(
Document activeResult, String configuration) async {
final data = DataWrapper(activeResult);
final configurations = data.getList('active_configurations');
assert(configurations.contains(configuration));
- if (configurations.length > 1) {
- await removeArrayEntry(
- activeResult, 'active_configurations', taggedValue(configuration));
- activeResult = await getDocument(activeResult.name);
- }
+ await removeArrayEntry(
+ activeResult, 'active_configurations', taggedValue(configuration));
+ activeResult = await getDocument(activeResult.name);
if (DataWrapper(activeResult).getList('active_configurations').isEmpty) {
activeResult.fields.remove('active_configurations');
activeResult.fields.remove('active');
@@ -491,6 +490,7 @@
await _executeWrite([
Write()
..transform = (DocumentTransform()
+ ..document = document.name
..fieldTransforms = [
FieldTransform()
..fieldPath = fieldName
diff --git a/builder/lib/src/firestore_helpers.dart b/builder/lib/src/firestore_helpers.dart
index 824b69b..c4c2f99 100644
--- a/builder/lib/src/firestore_helpers.dart
+++ b/builder/lib/src/firestore_helpers.dart
@@ -123,7 +123,9 @@
}
List<dynamic> getList(String name) {
- return fields[name]?.arrayValue?.values?.map(getValue)?.toList();
+ final arrayValue = fields[name]?.arrayValue;
+ if (arrayValue == null) return null;
+ return arrayValue.values?.map(getValue)?.toList() ?? [];
}
bool isNull(String name) {
diff --git a/builder/test/fakes.dart b/builder/test/fakes.dart
index ad3f25f..e0720e0 100644
--- a/builder/test/fakes.dart
+++ b/builder/test/fakes.dart
@@ -117,10 +117,11 @@
}
@override
- Future<void> storeResult(Map<String, dynamic> result) async {
+ Future<Document> storeResult(Map<String, dynamic> result) async {
final id = 'resultDocumentID$addedResultIdCounter';
addedResultIdCounter++;
results[id] = result;
+ return null;
}
@override
@@ -152,7 +153,7 @@
}
@override
- Future<void> updateActiveResult(
+ Future<void> removeActiveConfiguration(
Document activeResult, String configuration) async {
final result = Map<String, dynamic>.from(results[activeResult.name]);
result[fActiveConfigurations] = List.from(result[fActiveConfigurations])
diff --git a/builder/test/firestore_test.dart b/builder/test/firestore_test.dart
index f35a79c..beb50d8 100644
--- a/builder/test/firestore_test.dart
+++ b/builder/test/firestore_test.dart
@@ -29,8 +29,15 @@
'Error: firestore_test_nodejs.dart is being run on production'));
}
- const testReviewDocument =
- 'projects/dart-ci-staging/databases/(default)/documents/reviews/$testReview';
+ const testDatabaseName =
+ 'projects/dart-ci-staging/databases/(default)/documents';
+ const testReviewDocument = '$testDatabaseName/reviews/$testReview';
+ const removeActiveConfigurationTestName =
+ 'firestore_test/Remove_active_configuration';
+ final createdResult = {
+ ...activeFailureResult,
+ 'name': removeActiveConfigurationTestName
+ };
tearDownAll(() => baseClient.close());
@@ -57,9 +64,46 @@
await firestore.deleteDocument(doc.document.name);
}
}
+ snapshot = await firestore.query(
+ from: 'results',
+ where: fieldEquals('name', removeActiveConfigurationTestName));
+ for (final doc in snapshot) {
+ if (doc.document != null) {
+ await firestore.deleteDocument(doc.document.name);
+ }
+ }
await firestore.deleteDocument(testReviewDocument);
});
+ test('Remove active configuration', () async {
+ // Remove the two active configurations from createdResultDocument,
+ // checking that the document is updated correctly at each stage.
+ final createdResultDocument = await firestore.storeResult(createdResult);
+ final name = removeActiveConfigurationTestName;
+
+ var foundActiveResults =
+ await firestore.findActiveResults(name, testConfiguration);
+ var activeResult = foundActiveResults.single;
+ expect(createdResultDocument.name, activeResult.name);
+
+ await firestore.removeActiveConfiguration(
+ activeResult, testConfiguration);
+ foundActiveResults =
+ await firestore.findActiveResults(name, testConfiguration);
+ expect(foundActiveResults, isEmpty);
+ foundActiveResults =
+ await firestore.findActiveResults(name, 'configuration 2');
+ activeResult = foundActiveResults.single;
+
+ expect(activeResult.fields, contains('active'));
+ await firestore.removeActiveConfiguration(
+ activeResult, 'configuration 2');
+ final document = await firestore.getDocument(createdResultDocument.name);
+ expect(document.fields, isNot(contains('active')));
+ expect(document.fields, isNot(contains('active_configurations')));
+ await firestore.deleteDocument(createdResultDocument.name);
+ });
+
test('approved try result fetching', () async {
await firestore.storeReview(
testReview.toString(),
