Skip to content

Commit

Permalink
Updates github gold status to Firestore (flutter#3513)
Browse files Browse the repository at this point in the history
Part of flutter/flutter#142951

This PR:
1) starts updating Github Gold Status to Firestore
2) updates status in a try block to avoid breaking prod workflow
3) creates a corresponding Firestore model
4) updates `documentsToWrites` to have default `exists=null` to support generic document writes.
  • Loading branch information
keyonghan authored Feb 23, 2024
1 parent 16b1937 commit 857e09a
Show file tree
Hide file tree
Showing 10 changed files with 266 additions and 7 deletions.
2 changes: 1 addition & 1 deletion app_dart/lib/src/model/firestore/commit.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'package:googleapis/firestore/v1.dart' hide Status;
import '../../service/firestore.dart';

class Commit extends Document {
/// Lookup [Task] from Firestore.
/// Lookup [Commit] from Firestore.
///
/// `documentName` follows `/projects/{project}/databases/{database}/documents/{document_path}`
static Future<Commit> fromFirestore({
Expand Down
73 changes: 73 additions & 0 deletions app_dart/lib/src/model/firestore/github_gold_status.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright 2019 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:cocoon_service/cocoon_service.dart';
import 'package:github/github.dart';
import 'package:googleapis/firestore/v1.dart' hide Status;

import '../../service/firestore.dart';

class GithubGoldStatus extends Document {
/// Lookup [GithubGoldStatus] from Firestore.
///
/// `documentName` follows `/projects/{project}/databases/{database}/documents/{document_path}`
static Future<GithubGoldStatus> fromFirestore({
required FirestoreService firestoreService,
required String documentName,
}) async {
final Document document = await firestoreService.getDocument(documentName);
return GithubGoldStatus.fromDocument(githubGoldStatus: document);
}

/// Create [Commit] from a Commit Document.
static GithubGoldStatus fromDocument({
required Document githubGoldStatus,
}) {
return GithubGoldStatus()
..fields = githubGoldStatus.fields!
..name = githubGoldStatus.name!;
}

// The flutter-gold status cannot report a `failure` status
// due to auto-rollers. This is why we hold a `pending` status
// when there are image changes. This provides the opportunity
// for images to be triaged, and the auto-roller to proceed.
// For more context, see: https://github.com/flutter/flutter/issues/48744

static const String statusCompleted = 'success';

static const String statusRunning = 'pending';

int? get prNumber => int.parse(fields![kGithubGoldStatusPrNumberField]!.integerValue!);

String? get head => fields![kGithubGoldStatusHeadField]!.stringValue!;

String? get status => fields![kGithubGoldStatusStatusField]!.stringValue!;

String? get description => fields![kGithubGoldStatusDescriptionField]!.stringValue!;

int? get updates => int.parse(fields![kGithubGoldStatusUpdatesField]!.integerValue!);

/// A serializable form of [slug].
///
/// This will be of the form `<org>/<repo>`. e.g. `flutter/flutter`.
String? get repository => fields![kGithubGoldStatusRepositoryField]!.stringValue!;

/// [RepositorySlug] of where this commit exists.
RepositorySlug get slug => RepositorySlug.full(repository!);

@override
String toString() {
final StringBuffer buf = StringBuffer()
..write('$runtimeType(')
..write(', $kGithubGoldStatusPrNumberField: $prNumber')
..write(', $kGithubGoldStatusHeadField: $head')
..write(', $kGithubGoldStatusStatusField: $status')
..write(', $kGithubGoldStatusDescriptionField $description')
..write(', $kGithubGoldStatusUpdatesField: $updates')
..write(', $kGithubGoldStatusRepositoryField: $repository')
..write(')');
return buf.toString();
}
}
24 changes: 22 additions & 2 deletions app_dart/lib/src/request_handlers/push_gold_status_to_github.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,18 @@ import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:cocoon_service/cocoon_service.dart';
import 'package:github/github.dart';
import 'package:googleapis/firestore/v1.dart';
import 'package:gql/language.dart' as lang;
import 'package:graphql/client.dart';
import 'package:http/http.dart' as http;
import 'package:meta/meta.dart';

import '../model/appengine/github_gold_status_update.dart';
import '../model/firestore/github_gold_status.dart';
import '../request_handling/api_request_handler.dart';
import '../request_handling/body.dart';
import '../request_handling/exceptions.dart';
import '../service/config.dart';
import '../service/datastore.dart';
import '../service/logging.dart';

Expand Down Expand Up @@ -205,6 +206,25 @@ class PushGoldStatusToGithub extends ApiRequestHandler<Body> {
}
await datastore.insert(statusUpdates);
log.fine('Committed all updates for $slug');

// TODO(keyonghan): remove try block after fully migrated to firestore
// https://github.com/flutter/flutter/issues/142951
try {
await updateGithubGoldStatusDocuments(statusUpdates);
} catch (error) {
log.warning('Failed to update github gold status in Firestore: $error');
}
}

Future<void> updateGithubGoldStatusDocuments(List<GithubGoldStatusUpdate> statusUpdates) async {
if (statusUpdates.isEmpty) {
return;
}
final List<GithubGoldStatus> githubGoldStatusDocuments =
statusUpdates.map((e) => githubGoldStatusToDocument(e)).toList();
final List<Write> writes = documentsToWrites(githubGoldStatusDocuments);
final FirestoreService firestoreService = await config.createFirestoreService();
await firestoreService.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase);
}

/// Returns a GitHub Status for the given state and description.
Expand Down
33 changes: 32 additions & 1 deletion app_dart/lib/src/service/firestore.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import 'package:googleapis/firestore/v1.dart';
import 'package:http/http.dart';

import '../model/appengine/commit.dart';
import '../model/appengine/github_gold_status_update.dart';
import '../model/appengine/task.dart';
import '../model/firestore/commit.dart' as firestore_comit;
import '../model/firestore/github_gold_status.dart';
import '../model/firestore/task.dart' as firestore;
import '../model/ci_yaml/target.dart';
import 'access_client_provider.dart';
Expand Down Expand Up @@ -42,6 +44,14 @@ const String kCommitMessageField = 'message';
const String kCommitRepositoryPathField = 'repositoryPath';
const String kCommitShaField = 'sha';

const String kGithubGoldStatusCollectionId = 'githubGoldStatuses';
const String kGithubGoldStatusPrNumberField = 'prNumber';
const String kGithubGoldStatusHeadField = 'head';
const String kGithubGoldStatusStatusField = 'status';
const String kGithubGoldStatusDescriptionField = 'description';
const String kGithubGoldStatusUpdatesField = 'updates';
const String kGithubGoldStatusRepositoryField = 'repository';

class FirestoreService {
const FirestoreService(this.accessClientProvider);

Expand Down Expand Up @@ -172,8 +182,29 @@ firestore.Task taskToTaskDocument(Task task) {
);
}

/// Generates GithubGoldStatus document based on datastore GithubGoldStatusUpdate data model.
GithubGoldStatus githubGoldStatusToDocument(GithubGoldStatusUpdate githubGoldStatus) {
return GithubGoldStatus.fromDocument(
githubGoldStatus: Document(
name: '$kDatabase/documents/$kGithubGoldStatusCollectionId/${githubGoldStatus.head}_${githubGoldStatus.pr}',
fields: <String, Value>{
kGithubGoldStatusDescriptionField: Value(stringValue: githubGoldStatus.description),
kGithubGoldStatusHeadField: Value(stringValue: githubGoldStatus.head),
kGithubGoldStatusPrNumberField: Value(integerValue: githubGoldStatus.pr.toString()),
kGithubGoldStatusRepositoryField: Value(stringValue: githubGoldStatus.repository),
kGithubGoldStatusStatusField: Value(stringValue: githubGoldStatus.status),
kGithubGoldStatusUpdatesField: Value(integerValue: githubGoldStatus.updates.toString()),
},
),
);
}

/// Creates a list of [Write] based on documents.
List<Write> documentsToWrites(List<Document> documents, {bool exists = false}) {
///
/// Null `exists` means either update when a document exists or insert when a document doesn't.
/// `exists = false` means inserting a new document, assuming a document doesn't exist.
/// `exists = true` means updating an existing document, assuming it exisits.
List<Write> documentsToWrites(List<Document> documents, {bool? exists}) {
return documents
.map(
(Document document) => Write(
Expand Down
2 changes: 1 addition & 1 deletion app_dart/lib/src/service/luci_build_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ class LuciBuildService {
final int newAttempt = int.parse(taskDocument.name!.split('_').last) + 1;
tags['current_attempt'] = <String>[newAttempt.toString()];
taskDocument.resetAsRetry(attempt: newAttempt);
final List<Write> writes = documentsToWrites([taskDocument]);
final List<Write> writes = documentsToWrites([taskDocument], exists: false);
await firestoreService!.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase);
} catch (error) {
log.warning('Failed to insert retried task in Firestore: $error');
Expand Down
2 changes: 1 addition & 1 deletion app_dart/lib/src/service/scheduler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class Scheduler {
await _uploadToBigQuery(commit);
final firestore_commmit.Commit commitDocument = commitToCommitDocument(commit);
final List<firestore.Task> taskDocuments = targetsToTaskDocuments(commit, initialTargets);
final List<Write> writes = documentsToWrites([...taskDocuments, commitDocument]);
final List<Write> writes = documentsToWrites([...taskDocuments, commitDocument], exists: false);
final FirestoreService firestoreService = await config.createFirestoreService();
// TODO(keyonghan): remove try catch logic after validated to work.
try {
Expand Down
39 changes: 39 additions & 0 deletions app_dart/test/model/firestore/github_gold_status_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2019 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:cocoon_service/src/model/firestore/github_gold_status.dart';
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

import '../../src/utilities/entity_generators.dart';
import '../../src/utilities/mocks.dart';

void main() {
group('GithubGoldStatus.fromFirestore', () {
late MockFirestoreService mockFirestoreService;

setUp(() {
mockFirestoreService = MockFirestoreService();
});

test('generates githubGoldStatus correctly', () async {
final GithubGoldStatus githubGoldStatus = generateFirestoreGithubGoldStatus(1);
when(
mockFirestoreService.getDocument(
captureAny,
),
).thenAnswer((Invocation invocation) {
return Future<GithubGoldStatus>.value(
githubGoldStatus,
);
});
final GithubGoldStatus resultedGithubGoldStatus = await GithubGoldStatus.fromFirestore(
firestoreService: mockFirestoreService,
documentName: 'test',
);
expect(resultedGithubGoldStatus.name, githubGoldStatus.name);
expect(resultedGithubGoldStatus.fields, githubGoldStatus.fields);
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import 'dart:async';
import 'dart:io';

import 'package:cocoon_service/src/model/appengine/github_gold_status_update.dart';
import 'package:cocoon_service/src/model/firestore/github_gold_status.dart';
import 'package:cocoon_service/src/request_handlers/push_gold_status_to_github.dart';
import 'package:cocoon_service/src/request_handling/body.dart';
import 'package:cocoon_service/src/service/datastore.dart';
import 'package:cocoon_service/src/service/logging.dart';
import 'package:gcloud/db.dart' as gcloud_db;
import 'package:gcloud/db.dart';
import 'package:github/github.dart';
import 'package:googleapis/firestore/v1.dart';
import 'package:graphql/client.dart';
import 'package:http/http.dart' as http;
import 'package:http/testing.dart';
Expand All @@ -36,6 +38,7 @@ void main() {
late FakeClientContext clientContext;
FakeAuthenticatedContext authContext;
late FakeAuthenticationProvider auth;
late MockFirestoreService mockFirestoreService;
late FakeDatastoreDB db;
late ApiRequestHandlerTester tester;
late PushGoldStatusToGithub handler;
Expand All @@ -51,12 +54,14 @@ void main() {

setUp(() {
clientContext = FakeClientContext();
mockFirestoreService = MockFirestoreService();
authContext = FakeAuthenticatedContext(clientContext: clientContext);
auth = FakeAuthenticationProvider(clientContext: clientContext);
githubGraphQLClient = FakeGraphQLClient();
db = FakeDatastoreDB();
config = FakeConfig(
dbValue: db,
firestoreService: mockFirestoreService,
);
tester = ApiRequestHandlerTester(context: authContext);
mockHttpClient = MockClient((_) async => http.Response('{}', HttpStatus.ok));
Expand Down Expand Up @@ -749,6 +754,16 @@ void main() {
});

group('updates GitHub and/or Datastore', () {
setUp(() {
when(
mockFirestoreService.batchWriteDocuments(
captureAny,
captureAny,
),
).thenAnswer((Invocation invocation) {
return Future<BatchWriteResponse>.value(BatchWriteResponse());
});
});
test('new commit, checks running', () async {
// New commit
final PullRequest flutterPr = newPullRequest(123, 'f-abc', 'master');
Expand Down Expand Up @@ -779,6 +794,22 @@ void main() {
expect(records.where((LogRecord record) => record.level == Level.WARNING), isEmpty);
expect(records.where((LogRecord record) => record.level == Level.SEVERE), isEmpty);

final List<dynamic> captured =
verify(mockFirestoreService.batchWriteDocuments(captureAny, captureAny)).captured;
expect(captured.length, 4);
// The first element corresponds to the `status`.
final BatchWriteRequest batchWriteRequest = captured[0] as BatchWriteRequest;
expect(batchWriteRequest.writes!.length, 1);
final GithubGoldStatus updatedDocument =
GithubGoldStatus.fromDocument(githubGoldStatus: batchWriteRequest.writes![0].update!);
expect(updatedDocument.updates, status.updates);
// The third element corresponds to the `engineStatus`.
final BatchWriteRequest batchWriteRequestEngine = captured[2] as BatchWriteRequest;
expect(batchWriteRequestEngine.writes!.length, 1);
final GithubGoldStatus updatedDocumentEngine =
GithubGoldStatus.fromDocument(githubGoldStatus: batchWriteRequest.writes![0].update!);
expect(updatedDocumentEngine.updates, engineStatus.updates);

// Should not apply labels or make comments
verifyNever(
issuesService.addLabelsToIssue(
Expand Down Expand Up @@ -1353,6 +1384,14 @@ void main() {
});

test('Completed pull request does not skip follow-up prs with early return', () async {
when(
mockFirestoreService.batchWriteDocuments(
captureAny,
captureAny,
),
).thenAnswer((Invocation invocation) {
return Future<BatchWriteResponse>.value(BatchWriteResponse());
});
final PullRequest completedPR = newPullRequest(123, 'abc', 'master');
final PullRequest followUpPR = newPullRequest(456, 'def', 'master');
prsFromGitHub = <PullRequest>[
Expand Down Expand Up @@ -1466,6 +1505,14 @@ void main() {
});

test('uses the correct Gold endpoint to get status', () async {
when(
mockFirestoreService.batchWriteDocuments(
captureAny,
captureAny,
),
).thenAnswer((Invocation invocation) {
return Future<BatchWriteResponse>.value(BatchWriteResponse());
});
// New commit
final PullRequest pr = newPullRequest(123, 'abc', 'master');
prsFromGitHub = <PullRequest>[pr];
Expand Down
Loading

0 comments on commit 857e09a

Please sign in to comment.