Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ AppEngine version, listed here to ease deployment and troubleshooting.
## Next Release (replace with git tag when deployed)
* Bump runtimeVersion to `2025.05.21`.
* Upgraded stable Flutter analysis SDK to `3.32.0`.
* Note: started to backfill `Package[Version]`'s `isAdminDeleted` field.

## `20250519t093200-all`

Expand Down
2 changes: 1 addition & 1 deletion app/lib/admin/actions/package_version_retraction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ value of `set-retracted`, which should either be `true` or `false`.
if (pv == null) {
throw NotFoundException.resource(version);
}
if (pv.isModerated) {
if (pv.isNotVisible) {
throw ModeratedException.packageVersion(packageName, version);
}

Expand Down
4 changes: 2 additions & 2 deletions app/lib/frontend/handlers/atom_feed.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Future<shelf.Response> packageAtomFeedhandler(
/// Builds the content of the /feed.atom endpoint.
Future<String> buildAllPackagesAtomFeedContent() async {
final versions = await packageBackend.latestPackageVersions(limit: 100);
versions.removeWhere((pv) => pv.isModerated || pv.isRetracted);
versions.removeWhere((pv) => pv.isNotVisible || pv.isRetracted);
final feed = _allPackagesFeed(versions);
return feed.toXmlDocument();
}
Expand All @@ -69,7 +69,7 @@ Future<String> buildPackageAtomFeedContent(String package) async {
limit: 10,
)
.toList();
versions.removeWhere((pv) => pv.isModerated || pv.isRetracted);
versions.removeWhere((pv) => pv.isNotVisible || pv.isRetracted);
final feed = _packageFeed(package, versions);
return feed.toXmlDocument();
}
Expand Down
2 changes: 1 addition & 1 deletion app/lib/frontend/handlers/package.dart
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ Future<shelf.Response> _handlePackagePage({
} on NotFoundException {
return formattedNotFoundHandler(request);
}
if (data.version.isModerated) {
if (data.version.isNotVisible) {
final content = renderModeratedPackagePage(packageName);
return htmlResponse(content, status: 404);
}
Expand Down
4 changes: 2 additions & 2 deletions app/lib/package/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ class PackageBackend {
if (pv == null) {
throw NotFoundException.resource(version);
}
if (pv.isModerated) {
if (pv.isNotVisible) {
throw ModeratedException.packageVersion(package, version);
}

Expand Down Expand Up @@ -797,7 +797,7 @@ class PackageBackend {
throw NotFoundException.resource('package "$package"');
}
final packageVersions = (await packageBackend.versionsOfPackage(package))
.where((v) => !v.isModerated)
.where((v) => v.isVisible)
.toList();
if (packageVersions.isEmpty) {
throw NotFoundException.resource('package "$package"');
Expand Down
30 changes: 26 additions & 4 deletions app/lib/package/models.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,15 @@ class Package extends db.ExpandoModel<String> {
@db.DateTimeProperty()
DateTime? moderatedAt;

/// `true` if package was deleted by admins (pending final deletion).
/// TODO: mark `required: true` after backfill is done and all runtimes use the field
@db.BoolProperty(required: false)
bool? isAdminDeleted;

/// The timestamp when the package was deleted by admins.
@db.DateTimeProperty()
DateTime? adminDeletedAt;

/// Tags that are assigned to this package.
///
/// The permissions required to assign a tag typically depends on the tag.
Expand Down Expand Up @@ -179,19 +188,19 @@ class Package extends db.ExpandoModel<String> {
..isDiscontinued = false
..isUnlisted = false
..isModerated = false
..isAdminDeleted = false
..assignedTags = []
..deletedVersions = [];
}

// Convenience Fields:

bool get isVisible => !isModerated;
bool get isVisible => !isModerated && !(isAdminDeleted ?? false);
bool get isNotVisible => !isVisible;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this negated getter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I find it slightly more readable to use p.isNotVisible vs !p.isVisible, but don't really have strong preference. E.g. a similar API from the SDK is List.isEmpty and .isNotEmpty - I like to use both.


bool get isIncludedInRobots {
final now = clock.now();
return isVisible &&
!isModerated &&
!isDiscontinued &&
!isUnlisted &&
now.difference(created!) > robotsVisibilityMinAge &&
Expand Down Expand Up @@ -284,7 +293,7 @@ class Package extends db.ExpandoModel<String> {
.toList();

final isAllRetracted = versions.every((v) => v.isRetracted);
final isAllModerated = versions.every((v) => v.isModerated);
final isAllModerated = versions.every((v) => v.isNotVisible);
if (isAllModerated) {
throw NotAcceptableException('No visible versions left.');
}
Expand All @@ -301,7 +310,7 @@ class Package extends db.ExpandoModel<String> {

for (final pv in versions) {
// Skip all moderated versions.
if (pv.isModerated) {
if (pv.isNotVisible) {
continue;
}

Expand Down Expand Up @@ -586,13 +595,26 @@ class PackageVersion extends db.ExpandoModel<String> {
@db.DateTimeProperty()
DateTime? moderatedAt;

/// `true` if package version was deleted by admins (pending final deletion).
/// TODO: mark `required: true` after backfill is done and all runtimes use the field
@db.BoolProperty(required: false)
bool? isAdminDeleted;

/// The timestamp when the package version was deleted by admins.
@db.DateTimeProperty()
DateTime? adminDeletedAt;

PackageVersion();

PackageVersion.init() {
isModerated = false;
isAdminDeleted = false;
isRetracted = false;
}

late final isVisible = !isModerated && !(isAdminDeleted ?? false);
late final isNotVisible = !isVisible;

// Convenience Fields:

late final semanticVersion = Version.parse(version!);
Expand Down
2 changes: 1 addition & 1 deletion app/lib/package/tarball_storage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class TarballStorage {
if (lastPackage?.name != pv.package) {
lastPackage = await packageBackend.lookupPackage(pv.package);
}
final isModerated = lastPackage!.isModerated || pv.isModerated;
final isModerated = lastPackage!.isNotVisible || pv.isNotVisible;

final objectName = tarballObjectName(pv.package, pv.version!);
final publicInfo = await _publicBucket.tryInfo(objectName);
Expand Down
2 changes: 1 addition & 1 deletion app/lib/scorecard/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ class ScoreCardBackend {
throw NotFoundException(
'Package version "$packageName $packageVersion" does not exist.');
}
if (version.isModerated) {
if (version.isNotVisible) {
throw ModeratedException.packageVersion(packageName, packageVersion);
}
final status = PackageStatus.fromModels(package, version);
Expand Down
31 changes: 29 additions & 2 deletions app/lib/shared/integrity.dart
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ class IntegrityChecker {
// while the integrity check is running.
if (!pv.created!.isAfter(p.lastVersionPublished!)) {
// Moderated versions are not counted.
if (!pv.isModerated) {
if (pv.isVisible) {
versionCountUntilLastPublished++;
}
}
Expand Down Expand Up @@ -442,6 +442,12 @@ class IntegrityChecker {
isModerated: p.isModerated,
moderatedAt: p.moderatedAt,
);
yield* _checkAdminDeletedFlags(
kind: 'Package',
id: p.name!,
isAdminDeleted: p.isAdminDeleted ?? false,
adminDeletedAt: p.adminDeletedAt,
);
if (p.isModerated) {
_packagesWithIsModeratedFlag.add(p.name!);
}
Expand Down Expand Up @@ -575,7 +581,7 @@ class IntegrityChecker {
yield 'PackageVersion "${pv.qualifiedVersionKey}" is retracted, but `retracted` property is null.';
}
final shouldBeInPublicBucket =
!_packagesWithIsModeratedFlag.contains(pv.package) && !pv.isModerated;
!_packagesWithIsModeratedFlag.contains(pv.package) && pv.isVisible;
final tarballItems = await retry(
() async {
return await _checkTarballInBuckets(pv, archiveDownloadUri,
Expand All @@ -592,6 +598,12 @@ class IntegrityChecker {
isModerated: pv.isModerated,
moderatedAt: pv.moderatedAt,
);
yield* _checkAdminDeletedFlags(
kind: 'PackageVersion',
id: pv.qualifiedVersionKey.toString(),
isAdminDeleted: pv.isAdminDeleted ?? false,
adminDeletedAt: pv.adminDeletedAt,
);

// Sanity checks for the `created` property
if (pv.created == null) {
Expand Down Expand Up @@ -1012,3 +1024,18 @@ Stream<String> _checkModeratedFlags({
yield '$kind "$id" has `isModerated = false` but `moderatedAt` is not null.';
}
}

/// Check that `isAdminDeleted` and `adminDeletedAt` are consistent.
Stream<String> _checkAdminDeletedFlags({
required String kind,
required String id,
required bool isAdminDeleted,
required DateTime? adminDeletedAt,
}) async* {
if (isAdminDeleted && adminDeletedAt == null) {
yield '$kind "$id" has `isAdminDeleted = true` but `adminDeletedAt` is null.';
}
if (!isAdminDeleted && adminDeletedAt != null) {
yield '$kind "$id" has `isAdminDeleted = false` but `adminDeletedAt` is not null.';
}
}
2 changes: 1 addition & 1 deletion app/lib/task/backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ List<Version> _versionsToTrack(
// Ignore retracted versions
.where((pv) => !pv.isRetracted)
// Ignore moderated versions
.where((pv) => !pv.isModerated)
.where((pv) => pv.isVisible)
.map((pv) => pv.semanticVersion)
.toSet();
final visibleStableVersions = visibleVersions
Expand Down
29 changes: 28 additions & 1 deletion app/lib/tool/backfill/backfill_new_fields.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:logging/logging.dart';
import 'package:pub_dev/package/models.dart';
import 'package:pub_dev/shared/datastore.dart';

final _logger = Logger('backfill_new_fields');

Expand All @@ -12,5 +14,30 @@ final _logger = Logger('backfill_new_fields');
/// CHANGELOG.md must be updated with the new fields, and the next
/// release could remove the backfill from here.
Future<void> backfillNewFields() async {
_logger.info('No new fields.');
_logger.info('Backfill admin deleted fields...');
await for (final e in dbService.query<Package>().run()) {
if (e.isAdminDeleted == null) {
await withRetryTransaction(dbService, (tx) async {
final p = await tx.lookupOrNull<Package>(e.key);
if (p == null) {
return;
}
p.isAdminDeleted ??= false;
tx.insert(p);
});
}
}

await for (final e in dbService.query<PackageVersion>().run()) {
if (e.isAdminDeleted == null) {
await withRetryTransaction(dbService, (tx) async {
final p = await tx.lookupOrNull<PackageVersion>(e.key);
if (p == null) {
return;
}
p.isAdminDeleted ??= false;
tx.insert(p);
});
}
}
}