Package[Version] new fields: isAdminDeleted, adminDeletedAt (#8778)

isoos · web-flow · commit 4dce8b35f70a · 2025-05-22T10:03:35.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ AppEngine version, listed here to ease deployment and troubleshooting.
  * Bump runtimeVersion to `2025.05.21`.
  * Upgraded runtime Dart SDK to `3.8.0`.
  * Upgraded stable Flutter analysis SDK to `3.32.0`.
+ * Note: started to backfill `Package[Version]`'s `isAdminDeleted` field.
 
 ## `20250519t093200-all`
 
diff --git a/app/lib/admin/actions/package_version_retraction.dart b/app/lib/admin/actions/package_version_retraction.dart
@@ -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);
         }
 
diff --git a/app/lib/frontend/handlers/atom_feed.dart b/app/lib/frontend/handlers/atom_feed.dart
@@ -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();
 }
@@ -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();
 }
diff --git a/app/lib/frontend/handlers/package.dart b/app/lib/frontend/handlers/package.dart
@@ -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);
     }
diff --git a/app/lib/package/backend.dart b/app/lib/package/backend.dart
@@ -501,7 +501,7 @@ class PackageBackend {
       if (pv == null) {
         throw NotFoundException.resource(version);
       }
-      if (pv.isModerated) {
+      if (pv.isNotVisible) {
         throw ModeratedException.packageVersion(package, version);
       }
 
@@ -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"');
diff --git a/app/lib/package/models.dart b/app/lib/package/models.dart
@@ -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.
@@ -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;
 
   bool get isIncludedInRobots {
     final now = clock.now();
     return isVisible &&
-        !isModerated &&
         !isDiscontinued &&
         !isUnlisted &&
         now.difference(created!) > robotsVisibilityMinAge &&
@@ -284,8 +293,8 @@ class Package extends db.ExpandoModel<String> {
         .toList();
 
     final isAllRetracted = versions.every((v) => v.isRetracted);
-    final isAllModerated = versions.every((v) => v.isModerated);
-    if (isAllModerated) {
+    final noVisibleVersions = versions.every((v) => v.isNotVisible);
+    if (noVisibleVersions) {
       throw NotAcceptableException('No visible versions left.');
     }
 
@@ -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;
       }
 
@@ -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!);
diff --git a/app/lib/package/tarball_storage.dart b/app/lib/package/tarball_storage.dart
@@ -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);
diff --git a/app/lib/scorecard/backend.dart b/app/lib/scorecard/backend.dart
@@ -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);
diff --git a/app/lib/shared/integrity.dart b/app/lib/shared/integrity.dart
@@ -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++;
         }
       }
@@ -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!);
     }
@@ -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,
@@ -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) {
@@ -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.';
+  }
+}
diff --git a/app/lib/task/backend.dart b/app/lib/task/backend.dart
@@ -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
diff --git a/app/lib/tool/backfill/backfill_new_fields.dart b/app/lib/tool/backfill/backfill_new_fields.dart
@@ -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');
 
@@ -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);
+      });
+    }
+  }
 }

Original file line number	Diff line number	Diff line change
@@ -69,7 +69,7 @@ value of `set-retracted`, which should either be `true` or `false`.
`69`	`69`	`if (pv == null) {`
`70`	`70`	`throw NotFoundException.resource(version);`
`71`	`71`	`}`
`72`		`- if (pv.isModerated) {`
	`72`	`+ if (pv.isNotVisible) {`
`73`	`73`	`throw ModeratedException.packageVersion(packageName, version);`
`74`	`74`	`}`
`75`	`75`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ Future<shelf.Response> packageAtomFeedhandler(`
`52`	`52`	`/// Builds the content of the /feed.atom endpoint.`
`53`	`53`	`Future<String> buildAllPackagesAtomFeedContent() async {`
`54`	`54`	`final versions = await packageBackend.latestPackageVersions(limit: 100);`
`55`		`- versions.removeWhere((pv) => pv.isModerated \|\| pv.isRetracted);`
	`55`	`+ versions.removeWhere((pv) => pv.isNotVisible \|\| pv.isRetracted);`
`56`	`56`	`final feed = _allPackagesFeed(versions);`
`57`	`57`	`return feed.toXmlDocument();`
`58`	`58`	`}`
`@@ -69,7 +69,7 @@ Future<String> buildPackageAtomFeedContent(String package) async {`
`69`	`69`	`limit: 10,`
`70`	`70`	`)`
`71`	`71`	`.toList();`
`72`		`- versions.removeWhere((pv) => pv.isModerated \|\| pv.isRetracted);`
	`72`	`+ versions.removeWhere((pv) => pv.isNotVisible \|\| pv.isRetracted);`
`73`	`73`	`final feed = _packageFeed(package, versions);`
`74`	`74`	`return feed.toXmlDocument();`
`75`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -317,7 +317,7 @@ Future<shelf.Response> _handlePackagePage({`
`317`	`317`	`} on NotFoundException {`
`318`	`318`	`return formattedNotFoundHandler(request);`
`319`	`319`	`}`
`320`		`- if (data.version.isModerated) {`
	`320`	`+ if (data.version.isNotVisible) {`
`321`	`321`	`final content = renderModeratedPackagePage(packageName);`
`322`	`322`	`return htmlResponse(content, status: 404);`
`323`	`323`	`}`
Original file line number	Diff line number	Diff line change
`@@ -501,7 +501,7 @@ class PackageBackend {`
`501`	`501`	`if (pv == null) {`
`502`	`502`	`throw NotFoundException.resource(version);`
`503`	`503`	`}`
`504`		`- if (pv.isModerated) {`
	`504`	`+ if (pv.isNotVisible) {`
`505`	`505`	`throw ModeratedException.packageVersion(package, version);`
`506`	`506`	`}`
`507`	`507`
`@@ -797,7 +797,7 @@ class PackageBackend {`
`797`	`797`	`throw NotFoundException.resource('package "$package"');`
`798`	`798`	`}`
`799`	`799`	`final packageVersions = (await packageBackend.versionsOfPackage(package))`
`800`		`- .where((v) => !v.isModerated)`
	`800`	`+ .where((v) => v.isVisible)`
`801`	`801`	`.toList();`
`802`	`802`	`if (packageVersions.isEmpty) {`
`803`	`803`	`throw NotFoundException.resource('package "$package"');`
Original file line number	Diff line number	Diff line change
`@@ -217,7 +217,7 @@ class TarballStorage {`
`217`	`217`	`if (lastPackage?.name != pv.package) {`
`218`	`218`	`lastPackage = await packageBackend.lookupPackage(pv.package);`
`219`	`219`	`}`
`220`		`- final isModerated = lastPackage!.isModerated \|\| pv.isModerated;`
	`220`	`+ final isModerated = lastPackage!.isNotVisible \|\| pv.isNotVisible;`
`221`	`221`
`222`	`222`	`final objectName = tarballObjectName(pv.package, pv.version!);`
`223`	`223`	`final publicInfo = await _publicBucket.tryInfo(objectName);`
Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ class ScoreCardBackend {`
`142`	`142`	`throw NotFoundException(`
`143`	`143`	`'Package version "$packageName $packageVersion" does not exist.');`
`144`	`144`	`}`
`145`		`- if (version.isModerated) {`
	`145`	`+ if (version.isNotVisible) {`
`146`	`146`	`throw ModeratedException.packageVersion(packageName, packageVersion);`
`147`	`147`	`}`
`148`	`148`	`final status = PackageStatus.fromModels(package, version);`