[app_dart] Add BuildPushMessage.fromPushMessage (#2463)
* This is a refactor of existing logic in the pubsubs to the common class
diff --git a/app_dart/lib/src/model/luci/push_message.dart b/app_dart/lib/src/model/luci/push_message.dart
index cc58def..c6c4b95 100644
--- a/app_dart/lib/src/model/luci/push_message.dart
+++ b/app_dart/lib/src/model/luci/push_message.dart
@@ -2,9 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'dart:convert';
+import 'dart:typed_data';
+
import 'package:json_annotation/json_annotation.dart';
import '../../request_handling/body.dart';
+import '../../service/logging.dart';
import '../common/json_converters.dart';
part 'push_message.g.dart';
@@ -82,13 +86,30 @@
}
/// The LUCI build data from a PubSub push message payload.
-@JsonSerializable(includeIfNull: false)
+@JsonSerializable(includeIfNull: false, fieldRename: FieldRename.snake)
class BuildPushMessage extends JsonBody {
const BuildPushMessage({
this.build,
this.hostname,
- this.userData,
- });
+ String? userData,
+ }) : rawUserData = userData;
+
+ /// Create [BuildPushMessage] from [PushMessage].
+ factory BuildPushMessage.fromPushMessage(PushMessage message) {
+ final data = message.data;
+ if (data == null) {
+ throw const FormatException('Cannot create BuildPushMessage from null data');
+ }
+
+ try {
+ final String decodedData = String.fromCharCodes(base64.decode(data));
+ log.info('Result message from base64: $decodedData');
+ return BuildPushMessage.fromJson(json.decode(decodedData) as Map<String, dynamic>);
+ } on FormatException {
+ log.info('Result message: $data');
+ return BuildPushMessage.fromJson(json.decode(data) as Map<String, dynamic>);
+ }
+ }
static BuildPushMessage fromJson(Map<String, dynamic> json) => _$BuildPushMessageFromJson(json);
@@ -98,9 +119,32 @@
/// The hostname for the build, e.g. `cr-buildbucket.appspot.com`.
final String? hostname;
- /// User data that was included in the LUCI build request.
+ /// Do not use this value for anything.
+ ///
+ /// This value cannot be marked private due to json_serializable not
+ /// generating on private fields.
+ ///
+ /// This value is used to generate [userData].
@JsonKey(name: 'user_data')
- final String? userData;
+ final String? rawUserData;
+
+ /// User data that was included in the LUCI build request.
+ Map<String, dynamic> get userData {
+ if (rawUserData == null) {
+ return <String, dynamic>{};
+ }
+
+ try {
+ return json.decode(rawUserData!) as Map<String, dynamic>;
+ } on FormatException {
+ final Uint8List bytes = base64.decode(rawUserData!);
+ final String rawJson = String.fromCharCodes(bytes);
+ if (rawJson.isEmpty) {
+ return <String, dynamic>{};
+ }
+ return json.decode(rawJson) as Map<String, dynamic>;
+ }
+ }
@override
Map<String, dynamic> toJson() => _$BuildPushMessageToJson(this);
diff --git a/app_dart/lib/src/model/luci/push_message.g.dart b/app_dart/lib/src/model/luci/push_message.g.dart
index e833e38..2926fea 100644
--- a/app_dart/lib/src/model/luci/push_message.g.dart
+++ b/app_dart/lib/src/model/luci/push_message.g.dart
@@ -81,7 +81,7 @@
writeNotNull('build', instance.build);
writeNotNull('hostname', instance.hostname);
- writeNotNull('user_data', instance.userData);
+ val['user_data'] = instance.userData;
return val;
}
diff --git a/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart
index 1d509b3..68f9a76 100644
--- a/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart
+++ b/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart
@@ -2,8 +2,6 @@
// 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:cocoon_service/ci_yaml.dart';
import 'package:gcloud/db.dart';
import 'package:github/github.dart';
@@ -46,42 +44,22 @@
Future<Body> post() async {
final DatastoreService datastore = datastoreProvider(config.db);
- final String data = message.data!;
- BuildPushMessage buildPushMessage;
- try {
- final String decodedData = String.fromCharCodes(base64.decode(data));
- log.info('Result message from base64: $decodedData');
- buildPushMessage = BuildPushMessage.fromJson(json.decode(decodedData) as Map<String, dynamic>);
- } on FormatException {
- log.info('Result message: $data');
- buildPushMessage = BuildPushMessage.fromJson(json.decode(data) as Map<String, dynamic>);
- }
- log.fine(buildPushMessage.userData);
+ final BuildPushMessage buildPushMessage = BuildPushMessage.fromPushMessage(message);
+ log.fine('userData=${buildPushMessage.userData}');
log.fine('Updating buildId=${buildPushMessage.build?.id} for result=${buildPushMessage.build?.result}');
- // Example user data:
- // {
- // "task_key": "key123",
- // }
- if (buildPushMessage.userData == null) {
+ if (buildPushMessage.userData.isEmpty) {
log.fine('User data is empty');
return Body.empty;
}
- Map<String, dynamic> userData;
- try {
- userData = jsonDecode(buildPushMessage.userData!) as Map<String, dynamic>;
- } on FormatException {
- userData = jsonDecode(String.fromCharCodes(base64.decode(buildPushMessage.userData!))) as Map<String, dynamic>;
- }
-
- if (userData.containsKey('repo_owner') && userData.containsKey('repo_name')) {
+ if (buildPushMessage.userData.containsKey('repo_owner') && buildPushMessage.userData.containsKey('repo_name')) {
// Message is coming from a github checks api (postsubmit) enabled repo. We need to
// create the slug from the data in the message and send the check status
// update.
final RepositorySlug slug = RepositorySlug(
- userData['repo_owner'] as String,
- userData['repo_name'] as String,
+ buildPushMessage.userData['repo_owner'] as String,
+ buildPushMessage.userData['repo_name'] as String,
);
await githubChecksService.updateCheckStatus(
buildPushMessage,
@@ -90,8 +68,8 @@
);
}
- final String? rawTaskKey = userData['task_key'] as String?;
- final String? rawCommitKey = userData['commit_key'] as String?;
+ final String? rawTaskKey = buildPushMessage.userData['task_key'] as String?;
+ final String? rawCommitKey = buildPushMessage.userData['commit_key'] as String?;
if (rawCommitKey == null) {
throw const BadRequestException('userData does not contain commit_key');
}
@@ -106,7 +84,7 @@
log.fine('Pulling builder name from parameters_json...');
log.fine(build.buildParameters);
final String? taskName = build.buildParameters?['builder_name'] as String?;
- if (taskName == null) {
+ if (taskName == null || taskName.isEmpty) {
throw const BadRequestException('task_key is null and parameters_json does not contain the builder name');
}
final List<Task> tasks = await datastore.queryRecentTasksByName(name: taskName).toList();
diff --git a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
index 8b35a98..f72c357 100644
--- a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
+++ b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
@@ -2,8 +2,6 @@
// 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:github/github.dart';
import 'package:meta/meta.dart';
@@ -47,14 +45,7 @@
@override
Future<Body> post() async {
RepositorySlug slug;
- final String data = message.data!;
- BuildPushMessage buildPushMessage;
- try {
- buildPushMessage =
- BuildPushMessage.fromJson(json.decode(String.fromCharCodes(base64.decode(data))) as Map<String, dynamic>);
- } on FormatException {
- buildPushMessage = BuildPushMessage.fromJson(json.decode(data) as Map<String, dynamic>);
- }
+ final BuildPushMessage buildPushMessage = BuildPushMessage.fromPushMessage(message);
final Build build = buildPushMessage.build!;
final String builderName = build.tagsByName('builder').single;
log.fine('Available tags: ${build.tags.toString()}');
@@ -64,21 +55,13 @@
return Body.empty;
}
log.fine('Setting status: ${buildPushMessage.toJson()} for $builderName');
-
- Map<String, dynamic>? userData;
- try {
- userData = jsonDecode(buildPushMessage.userData!) as Map<String, dynamic>;
- } on FormatException {
- userData = jsonDecode(String.fromCharCodes(base64.decode(buildPushMessage.userData!))) as Map<String, dynamic>;
- }
-
- if (userData.containsKey('repo_owner') && userData.containsKey('repo_name')) {
+ if (buildPushMessage.userData.containsKey('repo_owner') && buildPushMessage.userData.containsKey('repo_name')) {
// Message is coming from a github checks api enabled repo. We need to
// create the slug from the data in the message and send the check status
// update.
slug = RepositorySlug(
- userData['repo_owner'] as String,
- userData['repo_name'] as String,
+ buildPushMessage.userData['repo_owner'] as String,
+ buildPushMessage.userData['repo_name'] as String,
);
await githubChecksService.updateCheckStatus(
buildPushMessage,
diff --git a/app_dart/lib/src/service/github_checks_service.dart b/app_dart/lib/src/service/github_checks_service.dart
index a8bf579..1d5ec3a 100644
--- a/app_dart/lib/src/service/github_checks_service.dart
+++ b/app_dart/lib/src/service/github_checks_service.dart
@@ -2,8 +2,6 @@
// 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:github/github.dart' as github;
import 'package:github/hooks.dart';
@@ -73,29 +71,23 @@
github.RepositorySlug slug,
) async {
final push_message.Build? build = buildPushMessage.build;
- if (buildPushMessage.userData!.isEmpty) {
+ if (buildPushMessage.userData.isEmpty) {
return false;
}
- Map<String, dynamic> userData;
- try {
- userData = jsonDecode(buildPushMessage.userData!) as Map<String, dynamic>;
- } on FormatException {
- userData = jsonDecode(String.fromCharCodes(base64Decode(buildPushMessage.userData!))) as Map<String, dynamic>;
- }
- if (!userData.containsKey('check_run_id') ||
- !userData.containsKey('repo_owner') ||
- !userData.containsKey('repo_name')) {
+ if (!buildPushMessage.userData.containsKey('check_run_id') ||
+ !buildPushMessage.userData.containsKey('repo_owner') ||
+ !buildPushMessage.userData.containsKey('repo_name')) {
log.severe(
'UserData did not contain check_run_id,'
- 'repo_owner, or repo_name: $userData',
+ 'repo_owner, or repo_name: ${buildPushMessage.userData}',
);
return false;
}
final github.CheckRun checkRun = await githubChecksUtil.getCheckRun(
config,
slug,
- userData['check_run_id'] as int?,
+ buildPushMessage.userData['check_run_id'] as int?,
);
final github.CheckRunStatus status = statusForResult(build!.status);
final github.CheckRunConclusion? conclusion =
diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart
index 73b99f8..9179d1f 100644
--- a/app_dart/lib/src/service/luci_build_service.dart
+++ b/app_dart/lib/src/service/luci_build_service.dart
@@ -294,7 +294,6 @@
// V1 bucket name is "luci.flutter.prod" while the api
// is expecting just the last part after "."(prod).
final String bucketName = buildPushMessage.build!.bucket!.split('.').last;
- final Map<String, dynamic>? userData = jsonDecode(buildPushMessage.userData!) as Map<String, dynamic>?;
return await buildBucketClient.scheduleBuild(
ScheduleBuildRequest(
builderId: BuilderId(
@@ -311,7 +310,7 @@
(buildPushMessage.build!.buildParameters!['properties'] as Map<String, dynamic>).cast<String, String>(),
notify: NotificationConfig(
pubsubTopic: 'projects/flutter-dashboard/topics/luci-builds',
- userData: json.encode(userData),
+ userData: json.encode(buildPushMessage.userData),
),
),
);
diff --git a/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart b/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart
index 4022e1b..36a7f3c 100644
--- a/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart
+++ b/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart
@@ -63,7 +63,8 @@
tester.message = createBuildbucketPushMessage(
'COMPLETED',
result: 'SUCCESS',
- userData: '{}',
+ builderName: '',
+ userData: '{\\"commit_key\\":\\"flutter/main/abc123\\"}',
);
expect(() => tester.post(handler), throwsA(isA<BadRequestException>()));
diff --git a/app_dart/test/src/utilities/mocks.mocks.dart b/app_dart/test/src/utilities/mocks.mocks.dart
index 7f6ba5d..c5d48d5 100644
--- a/app_dart/test/src/utilities/mocks.mocks.dart
+++ b/app_dart/test/src/utilities/mocks.mocks.dart
@@ -2233,7 +2233,7 @@
) as _i17.Future<List<_i10.CheckSuite>>);
@override
_i17.Future<void> updateCheckRun(
- _i3.Config? cocoonConfig,
+ _i3.Config? config,
_i10.RepositorySlug? slug,
_i10.CheckRun? checkRun, {
_i10.CheckRunStatus? status = _i10.CheckRunStatus.queued,
@@ -2245,7 +2245,7 @@
Invocation.method(
#updateCheckRun,
[
- cocoonConfig,
+ config,
slug,
checkRun,
],
@@ -2261,7 +2261,7 @@
) as _i17.Future<void>);
@override
_i17.Future<_i10.CheckRun> getCheckRun(
- _i3.Config? cocoonConfig,
+ _i3.Config? config,
_i10.RepositorySlug? slug,
int? id,
) =>
@@ -2269,7 +2269,7 @@
Invocation.method(
#getCheckRun,
[
- cocoonConfig,
+ config,
slug,
id,
],
@@ -2279,7 +2279,7 @@
Invocation.method(
#getCheckRun,
[
- cocoonConfig,
+ config,
slug,
id,
],
@@ -2288,7 +2288,7 @@
) as _i17.Future<_i10.CheckRun>);
@override
_i17.Future<_i10.CheckRun> createCheckRun(
- _i3.Config? cocoonConfig,
+ _i3.Config? config,
_i10.RepositorySlug? slug,
String? sha,
String? name, {
@@ -2298,7 +2298,7 @@
Invocation.method(
#createCheckRun,
[
- cocoonConfig,
+ config,
slug,
sha,
name,
@@ -2310,7 +2310,7 @@
Invocation.method(
#createCheckRun,
[
- cocoonConfig,
+ config,
slug,
sha,
name,
