Skip to content

Rename PackageStorage -> TarballStorage. #8207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Rename PackageStorage -> TarballStorage. #8207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/lib/admin/actions/moderate_package.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ Note: the action may take a longer time to complete as the public archive bucket
});

// retract or re-populate public archive files
await packageBackend.packageStorage.updatePublicArchiveBucket(
await packageBackend.tarballStorage.updatePublicArchiveBucket(
package: package,
ageCheckThreshold: Duration.zero,
deleteIfOlder: Duration.zero,
Expand Down
2 changes: 1 addition & 1 deletion app/lib/admin/actions/moderate_package_versions.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ Set the moderated flag on a package version (updating the flag and the timestamp
});

// retract or re-populate public archive files
await packageBackend.packageStorage.updatePublicArchiveBucket(
await packageBackend.tarballStorage.updatePublicArchiveBucket(
package: package,
ageCheckThreshold: Duration.zero,
deleteIfOlder: Duration.zero,
Expand Down
2 changes: 1 addition & 1 deletion app/lib/admin/backend.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,7 @@ class AdminBackend {
'Deleting moderated package version: ${version.qualifiedVersionKey}');

// deleting from canonical bucket
await packageBackend.packageStorage
await packageBackend.tarballStorage
.deleteArchiveFromCanonicalBucket(version.package, version.version!);

// deleting from datastore
Expand Down
24 changes: 12 additions & 12 deletions app/lib/package/backend.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
import 'package:pool/pool.dart';
import 'package:pub_dev/package/api_export/export_api_to_bucket.dart';
import 'package:pub_dev/package/package_storage.dart';
import 'package:pub_dev/package/tarball_storage.dart';
import 'package:pub_dev/service/async_queue/async_queue.dart';
import 'package:pub_dev/service/rate_limit/rate_limit.dart';
import 'package:pub_dev/shared/versions.dart';
Expand Down Expand Up @@ -81,8 +81,8 @@ class PackageBackend {
/// - `tmp/$guid` (incoming package archive that was uploaded, but not yet processed)
final Bucket _incomingBucket;

/// The storage handling for the archive files.
final PackageStorage packageStorage;
/// The storage handling for the package archive files.
final TarballStorage tarballStorage;

@visibleForTesting
int maxVersionsPerPackage = _defaultMaxVersionsPerPackage;
Expand All @@ -93,8 +93,8 @@ class PackageBackend {
this._incomingBucket,
Bucket canonicalBucket,
Bucket publicBucket,
) : packageStorage =
PackageStorage(db, storage, canonicalBucket, publicBucket);
) : tarballStorage =
TarballStorage(db, storage, canonicalBucket, publicBucket);

/// Whether the package exists and is not blocked or deleted.
Future<bool> isPackageVisible(String package) async {
Expand Down Expand Up @@ -329,7 +329,7 @@ class PackageBackend {
// NOTE: We should maybe check for existence first?
// return storage.bucket(bucket).info(object)
// .then((info) => info.downloadLink);
return packageStorage.getPublicDownloadUrl(package, cv!);
return tarballStorage.getPublicDownloadUrl(package, cv!);
}

/// Updates the stable, prerelease and preview versions of [package].
Expand Down Expand Up @@ -943,7 +943,7 @@ class PackageBackend {

// Check canonical archive.
final canonicalContentMatch =
await packageStorage.matchArchiveContentInCanonical(
await tarballStorage.matchArchiveContentInCanonical(
pubspec.name,
versionString,
fileBytes,
Expand Down Expand Up @@ -1184,14 +1184,14 @@ class PackageBackend {
);
if (!hasCanonicalArchiveObject) {
// Copy archive to canonical bucket.
await packageStorage.copyFromTempToCanonicalBucket(
await tarballStorage.copyFromTempToCanonicalBucket(
sourceAbsoluteObjectName:
_incomingBucket.absoluteObjectName(tmpObjectName(guid)),
package: newVersion.package,
version: newVersion.version!,
);
}
await packageStorage.copyArchiveFromCanonicalToPublicBucket(
await tarballStorage.copyArchiveFromCanonicalToPublicBucket(
newVersion.package, newVersion.version!);

final inserts = <Model>[
Expand Down Expand Up @@ -1256,7 +1256,7 @@ class PackageBackend {
apiExporter!
.updatePackageVersion(newVersion.package, newVersion.version!),
]);
await packageStorage.updateContentDispositionOnPublicBucket(
await tarballStorage.updateContentDispositionOnPublicBucket(
newVersion.package, newVersion.version!);
} catch (e, st) {
final v = newVersion.qualifiedVersionKey;
Expand Down Expand Up @@ -1634,12 +1634,12 @@ class PackageBackend {

/// Deletes the tarball of a [package] in the given [version] permanently.
Future<void> removePackageTarball(String package, String version) async {
await packageStorage.deleteArchiveFromAllBuckets(package, version);
await tarballStorage.deleteArchiveFromAllBuckets(package, version);
}

/// Gets the file info of a [package] in the given [version].
Future<ObjectInfo?> packageTarballInfo(String package, String version) async {
return await packageStorage.getPublicBucketArchiveInfo(package, version);
return await tarballStorage.getPublicBucketArchiveInfo(package, version);
}

void _updatePackageAutomatedPublishingLock(
Expand Down
6 changes: 4 additions & 2 deletions app/lib/package/package_storage.dart → app/lib/package/tarball_storage.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'dart:typed_data';
import 'package:crypto/crypto.dart';
import 'package:gcloud/storage.dart';
import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
import '../shared/datastore.dart';
import '../shared/storage.dart';
import '../shared/utils.dart';
Expand All @@ -16,13 +17,14 @@ import 'models.dart';
final _logger = Logger('package_storage');

/// The GCS object name of a tarball object - excluding leading '/'.
@visibleForTesting
String tarballObjectName(String package, String version) =>
'${_tarballObjectNamePackagePrefix(package)}$version.tar.gz';

/// The GCS object prefix of a tarball object for a given [package] - excluding leading '/'.
String _tarballObjectNamePackagePrefix(String package) => 'packages/$package-';

class PackageStorage {
class TarballStorage {
Copy link
Member

@jonasfj jonasfj Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is our thinking that this object will mainly be responsible for the canonicalBucket, right?

As eventually, the public bucket will go out of use, when ApiExporter starts updating ExportedApi and we've migrated the load balancer to use this.

Note. I don't mind that we let this object keep responsibility for the public bucket, until the public bucket is phased out.

Copy link
Collaborator Author

@isoos isoos Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, correct.

final DatastoreDB _dbService;
final Storage _storage;

Expand All @@ -36,7 +38,7 @@ class PackageStorage {
/// - `packages/$package-$version.tar.gz` (package archive)
final Bucket _publicBucket;

PackageStorage(
TarballStorage(
this._dbService,
this._storage,
this._canonicalBucket,
Expand Down
4 changes: 2 additions & 2 deletions app/lib/shared/integrity.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ class IntegrityChecker {
Uri archiveDownloadUri, {
required bool shouldBeInPublicBucket,
}) async* {
final canonicalInfo = await packageBackend.packageStorage
final canonicalInfo = await packageBackend.tarballStorage
.getCanonicalBucketArchiveInfo(pv.package, pv.version!);
if (canonicalInfo == null) {
yield 'PackageVersion "${pv.qualifiedVersionKey}" has no matching canonical archive file.';
Expand All @@ -628,7 +628,7 @@ class IntegrityChecker {
yield 'Canonical archive for PackageVersion "${pv.qualifiedVersionKey}" differs from public bucket.';
}

final publicInfo = await packageBackend.packageStorage
final publicInfo = await packageBackend.tarballStorage
.getPublicBucketArchiveInfo(pv.package, pv.version!);
if (!canonicalInfo.hasSameSignatureAs(publicInfo)) {
yield 'Canonical archive for PackageVersion "${pv.qualifiedVersionKey}" differs in the public bucket.';
Expand Down
2 changes: 1 addition & 1 deletion app/lib/tool/neat_task/pub_dev_tasks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ void _setupGenericPeriodicTasks() {
name: 'sync-public-bucket-from-canonical-bucket',
isRuntimeVersioned: false,
task: () async =>
await packageBackend.packageStorage.updatePublicArchiveBucket(),
await packageBackend.tarballStorage.updatePublicArchiveBucket(),
);

// Exports the package name completion data to a bucket.
Expand Down
8 changes: 4 additions & 4 deletions app/test/admin/moderate_package_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,13 @@ void main() {
await expectStatusCode(404);

// another check after background tasks are running
await packageBackend.packageStorage.updatePublicArchiveBucket();
await packageBackend.tarballStorage.updatePublicArchiveBucket();
await expectStatusCode(404);

await _moderate('oxygen', state: false, caseId: mc.caseId);
await expectStatusCode(200);
// another check after background tasks are running
await packageBackend.packageStorage.updatePublicArchiveBucket();
await packageBackend.tarballStorage.updatePublicArchiveBucket();
final restoredBytes = await expectStatusCode(200);
expect(restoredBytes, bytes);
});
Expand Down Expand Up @@ -381,7 +381,7 @@ void main() {

// canonical file is present
expect(
await packageBackend.packageStorage
await packageBackend.tarballStorage
.getCanonicalBucketArchiveInfo('oxygen', '1.2.0'),
isNotNull,
);
Expand All @@ -397,7 +397,7 @@ void main() {
isNull,
);
expect(
await packageBackend.packageStorage
await packageBackend.tarballStorage
.getCanonicalBucketArchiveInfo('oxygen', '1.2.0'),
isNull,
);
Expand Down
10 changes: 5 additions & 5 deletions app/test/admin/moderate_package_version_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,13 @@ void main() {
await expectStatusCode(200, version: '1.2.0');

// another check after background tasks are running
await packageBackend.packageStorage.updatePublicArchiveBucket();
await packageBackend.tarballStorage.updatePublicArchiveBucket();
await expectStatusCode(404);
await expectStatusCode(200, version: '1.2.0');

await _moderate('oxygen', '1.0.0', state: false);
await expectStatusCode(200);
await packageBackend.packageStorage.updatePublicArchiveBucket();
await packageBackend.tarballStorage.updatePublicArchiveBucket();
final restoredBytes = await expectStatusCode(200);
expect(restoredBytes, bytes);
});
Expand Down Expand Up @@ -429,7 +429,7 @@ void main() {
fn: () async {
// canonical file is present
expect(
await packageBackend.packageStorage
await packageBackend.tarballStorage
.getCanonicalBucketArchiveInfo('oxygen', '1.0.0'),
isNotNull,
);
Expand Down Expand Up @@ -459,7 +459,7 @@ void main() {

// canonical file is not present
expect(
await packageBackend.packageStorage
await packageBackend.tarballStorage
.getCanonicalBucketArchiveInfo('oxygen', '1.0.0'),
isNull,
);
Expand All @@ -470,7 +470,7 @@ void main() {
isNotNull,
);
expect(
await packageBackend.packageStorage
await packageBackend.tarballStorage
.getCanonicalBucketArchiveInfo('oxygen', '1.2.0'),
isNotNull,
);
Expand Down
2 changes: 1 addition & 1 deletion app/test/package/tarball_storage_namer_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import 'package:gcloud/storage.dart';
import 'package:pub_dev/package/backend.dart';
import 'package:pub_dev/package/package_storage.dart';
import 'package:pub_dev/package/tarball_storage.dart';
import 'package:pub_dev/shared/configuration.dart';
import 'package:pub_dev/shared/storage.dart';
import 'package:test/test.dart';
Expand Down
4 changes: 2 additions & 2 deletions app/test/package/update_public_bucket_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import 'package:gcloud/storage.dart';
import 'package:pub_dev/package/backend.dart';
import 'package:pub_dev/package/package_storage.dart';
import 'package:pub_dev/package/tarball_storage.dart';
import 'package:pub_dev/shared/configuration.dart';
import 'package:test/test.dart';

Expand All @@ -17,7 +17,7 @@ void main() {
Duration? ageCheckThreshold,
Duration? deleteIfOlder,
}) async {
return await packageBackend.packageStorage.updatePublicArchiveBucket(
return await packageBackend.tarballStorage.updatePublicArchiveBucket(
package: package,
ageCheckThreshold: ageCheckThreshold ?? Duration.zero,
deleteIfOlder: deleteIfOlder ?? const Duration(days: 7),
Expand Down
Loading