Skip to content

Commit

Permalink
Support Firestore in build status service (flutter#3574)
Browse files Browse the repository at this point in the history 
This is to prepare query logics switch to Firestore. 

This PR is a no op for existing workflow, but just inject the firestore service to prepare for future logic updates.
  • Loading branch information
@keyonghan
keyonghan authored Mar 13, 2024
1 parent ea28b6a commit 64667f6
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 36 deletions.
6 changes: 3 additions & 3 deletions app_dart/lib/src/request_handlers/get_build_status.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@

import 'dart:async';

import 'package:cocoon_service/cocoon_service.dart';
import 'package:github/github.dart';
import 'package:meta/meta.dart';

import '../../protos.dart' show BuildStatusResponse, EnumBuildStatus;
import '../request_handling/body.dart';
import '../request_handling/request_handler.dart';
import '../service/build_status_provider.dart';
import '../service/datastore.dart';

Expand All @@ -34,7 +33,8 @@ class GetBuildStatus extends RequestHandler<Body> {

Future<BuildStatusResponse> createResponse() async {
final DatastoreService datastore = datastoreProvider(config.db);
final BuildStatusService buildStatusService = buildStatusProvider(datastore);
final FirestoreService firestoreService = await config.createFirestoreService();
final BuildStatusService buildStatusService = buildStatusProvider(datastore, firestoreService);
final String repoName = request!.uri.queryParameters[kRepoParam] ?? 'flutter';
final RepositorySlug slug = RepositorySlug('flutter', repoName);
final BuildStatus status = (await buildStatusService.calculateCumulativeStatus(slug))!;
Expand Down
4 changes: 3 additions & 1 deletion app_dart/lib/src/request_handlers/get_green_commits.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import '../request_handling/request_handler.dart';
import '../service/build_status_provider.dart';
import '../service/config.dart';
import '../service/datastore.dart';
import '../service/firestore.dart';

/// Returns [List<String>] of the commit shas that had all passing tests.
///
Expand Down Expand Up @@ -52,7 +53,8 @@ class GetGreenCommits extends RequestHandler<Body> {
final RepositorySlug slug = RepositorySlug('flutter', repoName);
final String branch = request!.uri.queryParameters[kBranchParam] ?? Config.defaultBranch(slug);
final DatastoreService datastore = datastoreProvider(config.db);
final BuildStatusService buildStatusService = buildStatusProvider(datastore);
final FirestoreService firestoreService = await config.createFirestoreService();
final BuildStatusService buildStatusService = buildStatusProvider(datastore, firestoreService);
final int commitNumber = config.commitNumber;

final List<String?> greenCommits = await buildStatusService
Expand Down
4 changes: 3 additions & 1 deletion app_dart/lib/src/request_handlers/get_status.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import '../request_handling/request_handler.dart';
import '../service/build_status_provider.dart';
import '../service/config.dart';
import '../service/datastore.dart';
import '../service/firestore.dart';

@immutable
class GetStatus extends RequestHandler<Body> {
Expand All @@ -39,7 +40,8 @@ class GetStatus extends RequestHandler<Body> {
final RepositorySlug slug = RepositorySlug('flutter', repoName);
final String branch = request!.uri.queryParameters[kBranchParam] ?? Config.defaultBranch(slug);
final DatastoreService datastore = datastoreProvider(config.db);
final BuildStatusService buildStatusService = buildStatusProvider(datastore);
final FirestoreService firestoreService = await config.createFirestoreService();
final BuildStatusService buildStatusService = buildStatusProvider(datastore, firestoreService);
final KeyHelper keyHelper = config.keyHelper;
final int commitNumber = config.commitNumber;
final int lastCommitTimestamp = await _obtainTimestamp(encodedLastCommitKey, keyHelper, datastore);
Expand Down
3 changes: 2 additions & 1 deletion app_dart/lib/src/request_handlers/push_build_status_to_github.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class PushBuildStatusToGithub extends ApiRequestHandler<Body> {
final String repository = request!.uri.queryParameters[fullNameRepoParam] ?? 'flutter/flutter';
final RepositorySlug slug = RepositorySlug.full(repository);
final DatastoreService datastore = datastoreProvider(config.db);
final BuildStatusService buildStatusService = buildStatusServiceProvider(datastore);
final FirestoreService firestoreService = await config.createFirestoreService();
final BuildStatusService buildStatusService = buildStatusServiceProvider(datastore, firestoreService);

final BuildStatus status = (await buildStatusService.calculateCumulativeStatus(slug))!;
await _insertBigquery(slug, status.githubStatus, Config.defaultBranch(slug), config);
Expand Down
16 changes: 12 additions & 4 deletions app_dart/lib/src/service/build_status_provider.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:async';

import 'package:cocoon_service/cocoon_service.dart';
import 'package:github/github.dart';
import 'package:meta/meta.dart';

Expand All @@ -14,20 +15,27 @@ import '../model/appengine/task.dart';
import 'datastore.dart';

/// Function signature for a [BuildStatusService] provider.
typedef BuildStatusServiceProvider = BuildStatusService Function(DatastoreService datastoreService);
typedef BuildStatusServiceProvider = BuildStatusService Function(
DatastoreService datastoreService,
FirestoreService firestoreService,
);

/// Branches that are used to calculate the tree status.
const Set<String> defaultBranches = <String>{'refs/heads/main', 'refs/heads/master'};

/// Class that calculates the current build status.
class BuildStatusService {
const BuildStatusService(this.datastoreService);
const BuildStatusService(
this.datastoreService,
this.firestoreService,
);

final DatastoreService datastoreService;
final FirestoreService firestoreService;

/// Creates and returns a [DatastoreService] using [db] and [maxEntityGroups].
static BuildStatusService defaultProvider(DatastoreService datastoreService) {
return BuildStatusService(datastoreService);
static BuildStatusService defaultProvider(DatastoreService datastoreService, FirestoreService firestoreService) {
return BuildStatusService(datastoreService, firestoreService);
}

@visibleForTesting
Expand Down
4 changes: 3 additions & 1 deletion app_dart/lib/src/service/scheduler.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class Scheduler {
final HttpClientProvider httpClientProvider;

late DatastoreService datastore;
late FirestoreService firestoreService;
LuciBuildService luciBuildService;

/// Name of the subcache to store scheduler related values in redis.
Expand Down Expand Up @@ -638,7 +639,8 @@ class Scheduler {
/// to ensure new targets without `bringup: true` label are not added into the build.
Future<Commit> generateTotCommit({required String branch, required RepositorySlug slug}) async {
datastore = datastoreProvider(config.db);
final BuildStatusService buildStatusService = buildStatusProvider(datastore);
firestoreService = await config.createFirestoreService();
final BuildStatusService buildStatusService = buildStatusProvider(datastore, firestoreService);
final Commit totCommit = (await buildStatusService
.retrieveCommitStatus(
limit: 1,
Expand Down
17 changes: 10 additions & 7 deletions app_dart/test/request_handlers/get_green_commits_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ import '../src/request_handling/fake_http.dart';
import '../src/request_handling/request_handler_tester.dart';
import '../src/service/fake_build_status_provider.dart';
import '../src/utilities/entity_generators.dart';
import '../src/utilities/mocks.dart';

void main() {
group('GetGreenCommits', () {
late FakeConfig config;
FakeClientContext clientContext;
FakeKeyHelper keyHelper;
FakeBuildStatusService buildStatusService;
late MockFirestoreService mockFirestoreService;
late RequestHandlerTester tester;
late GetGreenCommits handler;

Expand Down Expand Up @@ -74,14 +76,15 @@ void main() {

setUp(() {
clientContext = FakeClientContext();
mockFirestoreService = MockFirestoreService();
keyHelper = FakeKeyHelper(applicationContext: clientContext.applicationContext);
tester = RequestHandlerTester();
config = FakeConfig(keyHelperValue: keyHelper);
config = FakeConfig(keyHelperValue: keyHelper, firestoreService: mockFirestoreService);
buildStatusService = FakeBuildStatusService(commitStatuses: <CommitStatus>[]);
handler = GetGreenCommits(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);
});

Expand All @@ -100,7 +103,7 @@ void main() {
handler = GetGreenCommits(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

final List<String?> result = (await decodeHandlerBody())!;
Expand All @@ -122,7 +125,7 @@ void main() {
handler = GetGreenCommits(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

final List<String?> result = (await decodeHandlerBody())!;
Expand All @@ -141,7 +144,7 @@ void main() {
handler = GetGreenCommits(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

final List<String?> result = (await decodeHandlerBody())!;
Expand All @@ -160,7 +163,7 @@ void main() {
handler = GetGreenCommits(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

final List<String?> result = (await decodeHandlerBody())!;
Expand All @@ -186,7 +189,7 @@ void main() {
handler = GetGreenCommits(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

final List<String?> result = (await decodeHandlerBody())!;
Expand Down
13 changes: 8 additions & 5 deletions app_dart/test/request_handlers/get_status_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import '../src/request_handling/fake_authentication.dart';
import '../src/request_handling/fake_http.dart';
import '../src/request_handling/request_handler_tester.dart';
import '../src/service/fake_build_status_provider.dart';
import '../src/utilities/mocks.dart';

void main() {
group('GetStatus', () {
Expand All @@ -27,6 +28,7 @@ void main() {
FakeBuildStatusService buildStatusService;
late RequestHandlerTester tester;
late GetStatus handler;
late MockFirestoreService mockFirestoreService;

late Commit commit1;
late Commit commit2;
Expand All @@ -38,14 +40,15 @@ void main() {

setUp(() {
clientContext = FakeClientContext();
mockFirestoreService = MockFirestoreService();
keyHelper = FakeKeyHelper(applicationContext: clientContext.applicationContext);
tester = RequestHandlerTester();
config = FakeConfig(keyHelperValue: keyHelper);
config = FakeConfig(keyHelperValue: keyHelper, firestoreService: mockFirestoreService);
buildStatusService = FakeBuildStatusService(commitStatuses: <CommitStatus>[]);
handler = GetStatus(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);
commit1 = Commit(
key: config.db.emptyKey.append(Commit, id: 'flutter/flutter/ea28a9c34dc701de891eaf74503ca4717019f829'),
Expand Down Expand Up @@ -79,7 +82,7 @@ void main() {
handler = GetStatus(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

final Map<String, dynamic> result = (await decodeHandlerBody())!;
Expand Down Expand Up @@ -112,7 +115,7 @@ void main() {
handler = GetStatus(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

const String expectedLastCommitKeyEncoded =
Expand Down Expand Up @@ -154,7 +157,7 @@ void main() {
handler = GetStatus(
config: config,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
buildStatusProvider: (_) => buildStatusService,
buildStatusProvider: (_, __) => buildStatusService,
);

const String branch = 'flutter-1.1-candidate.1';
Expand Down
2 changes: 1 addition & 1 deletion app_dart/test/request_handlers/push_build_status_to_github_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void main() {
handler = PushBuildStatusToGithub(
config: config,
authenticationProvider: FakeAuthenticationProvider(clientContext: clientContext),
buildStatusServiceProvider: (_) => buildStatusService,
buildStatusServiceProvider: (_, __) => buildStatusService,
datastoreProvider: (DatastoreDB db) => DatastoreService(config.db, 5),
);

Expand Down
7 changes: 5 additions & 2 deletions app_dart/test/service/build_status_provider_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'package:test/test.dart';
import '../src/datastore/fake_config.dart';
import '../src/datastore/fake_datastore.dart';
import '../src/utilities/entity_generators.dart';
import '../src/utilities/mocks.dart';

List<Commit> oneCommit = <Commit>[
Commit(
Expand Down Expand Up @@ -95,14 +96,16 @@ void main() {
late BuildStatusService buildStatusService;
FakeConfig config;
DatastoreService datastoreService;
late MockFirestoreService mockFirestoreService;

final RepositorySlug slug = RepositorySlug('flutter', 'flutter');

setUp(() {
db = FakeDatastoreDB();
config = FakeConfig(dbValue: db);
mockFirestoreService = MockFirestoreService();
config = FakeConfig(dbValue: db, firestoreService: mockFirestoreService);
datastoreService = DatastoreService(config.db, 5);
buildStatusService = BuildStatusService.defaultProvider(datastoreService);
buildStatusService = BuildStatusService.defaultProvider(datastoreService, mockFirestoreService);
});

group('calculateStatus', () {
Expand Down
Loading

0 comments on commit 64667f6

Please sign in to comment.