From c755847fb383687f0d2e2d22e995e230a523baa4 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Tue, 12 Dec 2023 13:55:06 +0100 Subject: [PATCH 1/2] Retry archive bucket integrity checks. --- app/lib/package/backend.dart | 2 +- app/lib/shared/integrity.dart | 72 ++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart index 3b77ff3c91..c3a564896d 100644 --- a/app/lib/package/backend.dart +++ b/app/lib/package/backend.dart @@ -1552,7 +1552,7 @@ class PackageBackend { } /// Gets the file info of a [package] in the given [version]. - Future packageTarballinfo(String package, String version) async { + Future packageTarballInfo(String package, String version) async { return await _publicBucket.tryInfo(tarballObjectName(package, version)); } diff --git a/app/lib/shared/integrity.dart b/app/lib/shared/integrity.dart index fa75e9d954..6adceac5aa 100644 --- a/app/lib/shared/integrity.dart +++ b/app/lib/shared/integrity.dart @@ -13,6 +13,8 @@ import 'package:http/http.dart' as http; import 'package:logging/logging.dart'; import 'package:pool/pool.dart'; import 'package:pub_dev/publisher/backend.dart'; +import 'package:pub_dev/shared/env_config.dart'; +import 'package:retry/retry.dart'; import '../account/agent.dart'; import '../account/models.dart'; @@ -26,7 +28,6 @@ import '../tool/utils/http.dart'; import 'configuration.dart'; import 'datastore.dart'; import 'email.dart' show looksLikeEmail; -import 'env_config.dart'; import 'storage.dart'; import 'urls.dart' as urls; import 'utils.dart' show canonicalizeVersion, ByteArrayEqualsExt; @@ -526,35 +527,13 @@ class IntegrityChecker { yield 'PackageVersion "${pv.qualifiedVersionKey}" is retracted, but `retracted` property is null.'; } if (!envConfig.isRunningLocally) { - final info = - await packageBackend.packageTarballinfo(pv.package, pv.version!); - if (info == null) { - yield 'PackageVersion "${pv.qualifiedVersionKey}" has no matching archive file.'; - } - final canonicalInfo = await storageService - .bucket(activeConfiguration.canonicalPackagesBucketName!) - // ignore: invalid_use_of_visible_for_testing_member - .tryInfo(tarballObjectName(pv.package, pv.version!)); - - if (canonicalInfo != null) { - if (!canonicalInfo.hasSameSignatureAs(info)) { - yield 'Canonical archive for PackageVersion "${pv.qualifiedVersionKey}" differs in old bucket.'; - } - - final publicInfo = await storageService - .bucket(activeConfiguration.publicPackagesBucketName!) - // ignore: invalid_use_of_visible_for_testing_member - .tryInfo(tarballObjectName(pv.package, pv.version!)); - if (!canonicalInfo.hasSameSignatureAs(publicInfo)) { - yield 'Canonical archive for PackageVersion "${pv.qualifiedVersionKey}" differs in the public bucket.'; - } - } - - // Also issue a HTTP request. - final rs = await _httpClient.head(archiveDownloadUri); - if (rs.statusCode != 200) { - yield 'PackageVersion "${pv.qualifiedVersionKey}" has no matching archive file (HTTP status ${rs.statusCode}).'; - } + final tartballItems = await retry( + () async { + return await _checkTarballInBuckets(pv, archiveDownloadUri).toList(); + }, + maxAttempts: 2, + ); + yield* Stream.fromIterable(tartballItems); } // TODO: remove null check after the backfill should have filled the property. @@ -595,6 +574,39 @@ class IntegrityChecker { } } + Stream _checkTarballInBuckets( + PackageVersion pv, Uri archiveDownloadUri) async* { + final info = + await packageBackend.packageTarballInfo(pv.package, pv.version!); + if (info == null) { + yield 'PackageVersion "${pv.qualifiedVersionKey}" has no matching archive file.'; + } + final canonicalInfo = await storageService + .bucket(activeConfiguration.canonicalPackagesBucketName!) + // ignore: invalid_use_of_visible_for_testing_member + .tryInfo(tarballObjectName(pv.package, pv.version!)); + + if (canonicalInfo != null) { + if (!canonicalInfo.hasSameSignatureAs(info)) { + yield 'Canonical archive for PackageVersion "${pv.qualifiedVersionKey}" differs in old bucket.'; + } + + final publicInfo = await storageService + .bucket(activeConfiguration.publicPackagesBucketName!) + // ignore: invalid_use_of_visible_for_testing_member + .tryInfo(tarballObjectName(pv.package, pv.version!)); + if (!canonicalInfo.hasSameSignatureAs(publicInfo)) { + yield 'Canonical archive for PackageVersion "${pv.qualifiedVersionKey}" differs in the public bucket.'; + } + } + + // Also issue a HTTP request. + final rs = await _httpClient.head(archiveDownloadUri); + if (rs.statusCode != 200) { + yield 'PackageVersion "${pv.qualifiedVersionKey}" has no matching archive file (HTTP status ${rs.statusCode}).'; + } + } + Stream _checkLikes() async* { _logger.info('Scanning Likes...'); From 70c3d25aca5f7a9af80507fba975456890fbee10 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Tue, 12 Dec 2023 17:35:33 +0100 Subject: [PATCH 2/2] Fix spelling --- app/lib/shared/integrity.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/lib/shared/integrity.dart b/app/lib/shared/integrity.dart index 6adceac5aa..d7994e7859 100644 --- a/app/lib/shared/integrity.dart +++ b/app/lib/shared/integrity.dart @@ -527,13 +527,13 @@ class IntegrityChecker { yield 'PackageVersion "${pv.qualifiedVersionKey}" is retracted, but `retracted` property is null.'; } if (!envConfig.isRunningLocally) { - final tartballItems = await retry( + final tarballItems = await retry( () async { return await _checkTarballInBuckets(pv, archiveDownloadUri).toList(); }, maxAttempts: 2, ); - yield* Stream.fromIterable(tartballItems); + yield* Stream.fromIterable(tarballItems); } // TODO: remove null check after the backfill should have filled the property.