-
Notifications
You must be signed in to change notification settings - Fork 101
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor the Autosubmit PullRequestCheck class so it can support mult…
…iple methods (#2921) The goal of this refactor is to made the coming work of separating the revert class much easier. This refactor will allow us to separate the checks for pull requests and revert pull requests to separate endpoints within autosubmit before ultimately separating them to individual services. Add the revert queue topic and subscription and make several of the static pubsub variables in the Config class non static as getters because these fields are not inheritable. *List which issues are fixed by this PR. You must list at least one issue.* Part of flutter/flutter#113867 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
- Loading branch information
1 parent
2859b12
commit f1216a8
Showing
6 changed files
with
182 additions
and
95 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| // Copyright 2023 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 'dart:convert'; | ||
|
|
||
| import 'package:auto_submit/server/authenticated_request_handler.dart'; | ||
| import 'package:auto_submit/service/approver_service.dart'; | ||
| import 'package:auto_submit/service/validation_service.dart'; | ||
| import 'package:github/github.dart'; | ||
| import 'package:googleapis/pubsub/v1.dart' as pub; | ||
| import 'package:shelf/shelf.dart'; | ||
|
|
||
| import '../request_handling/pubsub.dart'; | ||
|
|
||
| import '../service/log.dart'; | ||
|
|
||
| abstract class CheckRequest extends AuthenticatedRequestHandler { | ||
| const CheckRequest({ | ||
| required super.config, | ||
| required super.cronAuthProvider, | ||
| this.approverProvider = ApproverService.defaultProvider, | ||
| this.pubsub = const PubSub(), | ||
| }); | ||
|
|
||
| final PubSub pubsub; | ||
| final ApproverServiceProvider approverProvider; | ||
|
|
||
| @override | ||
| Future<Response> get(); | ||
|
|
||
| /// Process pull request messages from Pubsub. | ||
| Future<Response> process( | ||
| String pubSubSubscription, | ||
| int pubSubPulls, | ||
| int pubSubBatchSize, | ||
| ) async { | ||
| final Set<int> processingLog = <int>{}; | ||
| final List<pub.ReceivedMessage> messageList = await pullMessages( | ||
| pubSubSubscription, | ||
| pubSubPulls, | ||
| pubSubBatchSize, | ||
| ); | ||
| if (messageList.isEmpty) { | ||
| log.info('No messages are pulled.'); | ||
| return Response.ok('No messages are pulled.'); | ||
| } | ||
|
|
||
| log.info('Processing ${messageList.length} messages'); | ||
| // TODO (ricardoamador): This validation service will be passed in by the calling class. | ||
| final ValidationService validationService = ValidationService(config); | ||
| final List<Future<void>> futures = <Future<void>>[]; | ||
|
|
||
| for (pub.ReceivedMessage message in messageList) { | ||
| log.info(message.toJson()); | ||
| assert(message.message != null); | ||
| assert(message.message!.data != null); | ||
| final String messageData = message.message!.data!; | ||
|
|
||
| final Map<String, dynamic> rawBody = | ||
| json.decode(String.fromCharCodes(base64.decode(messageData))) as Map<String, dynamic>; | ||
| log.info('request raw body = $rawBody'); | ||
|
|
||
| final PullRequest pullRequest = PullRequest.fromJson(rawBody); | ||
|
|
||
| log.info('Processing message ackId: ${message.ackId}'); | ||
| log.info('Processing mesageId: ${message.message!.messageId}'); | ||
| log.info('Processing PR: $rawBody'); | ||
| if (processingLog.contains(pullRequest.number)) { | ||
| // Ack duplicate. | ||
| log.info('Ack the duplicated message : ${message.ackId!}.'); | ||
| await pubsub.acknowledge( | ||
| pubSubSubscription, | ||
| message.ackId!, | ||
| ); | ||
|
|
||
| continue; | ||
| } else { | ||
| final ApproverService approver = approverProvider(config); | ||
| log.info('Checking auto approval of pull request: $rawBody'); | ||
| await approver.autoApproval(pullRequest); | ||
| processingLog.add(pullRequest.number!); | ||
| } | ||
|
|
||
| futures.add( | ||
| validationService.processMessage( | ||
| // pullRequestMessage, | ||
| pullRequest, | ||
| message.ackId!, | ||
| pubsub, | ||
| ), | ||
| ); | ||
| } | ||
| await Future.wait(futures); | ||
| return Response.ok('Finished processing changes'); | ||
| } | ||
|
|
||
| /// Pulls queued Pub/Sub messages. | ||
| /// | ||
| /// Pub/Sub pull request API doesn't guarantee returning all messages each time. This | ||
| /// loops to pull `kPubsubPullNumber` times to try covering all queued messages. | ||
| Future<List<pub.ReceivedMessage>> pullMessages( | ||
| String subscription, | ||
| int pulls, | ||
| int batchSize, | ||
| ) async { | ||
| final Map<String, pub.ReceivedMessage> messageMap = <String, pub.ReceivedMessage>{}; | ||
| for (int i = 0; i < pulls; i++) { | ||
| final pub.PullResponse pullResponse = await pubsub.pull( | ||
| subscription, | ||
| batchSize, | ||
| ); | ||
| final List<pub.ReceivedMessage>? receivedMessages = pullResponse.receivedMessages; | ||
| if (receivedMessages == null) { | ||
| continue; | ||
| } | ||
| for (pub.ReceivedMessage message in receivedMessages) { | ||
| final String messageId = message.message!.messageId!; | ||
| messageMap[messageId] = message; | ||
| } | ||
| } | ||
| return messageMap.values.toList(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Copyright 2023 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:auto_submit/request_handling/pubsub.dart'; | ||
| import 'package:auto_submit/requests/check_request.dart'; | ||
| import 'package:auto_submit/service/approver_service.dart'; | ||
| import 'package:shelf/shelf.dart'; | ||
|
|
||
| // TODO (ricardoamador): provide implementation in https://github.com/flutter/flutter/issues/113867 | ||
|
|
||
| /// Handler for processing pull requests with 'revert' label. | ||
| /// | ||
| /// For pull requests where an 'revert' label was added in pubsub, | ||
| /// check if the revert request is mergable. | ||
| class CheckRevertRequest extends CheckRequest { | ||
| const CheckRevertRequest({ | ||
| required super.config, | ||
| required super.cronAuthProvider, | ||
| super.approverProvider = ApproverService.defaultProvider, | ||
| super.pubsub = const PubSub(), | ||
| }); | ||
|
|
||
| @override | ||
| Future<Response> get() async { | ||
| /// Currently this is unused and cannot be called. | ||
| return process( | ||
| config.pubsubRevertRequestSubscription, | ||
| config.kPubsubPullNumber, | ||
| config.kPullMesssageBatchSize, | ||
| ); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters