Follow-up to Datastore model and fields: removing Secret entity and requiring isAdminDeleted fields. (#8827)

Author: isoos

app/lib/admin/backend.dart

@@ -887,7 +887,7 @@ class AdminBackend {
       ..order('adminDeletedAt');
     await for (final package in pQuery.run()) {
       // sanity check
-      if (!(package.isAdminDeleted ?? false)) {
+      if (!package.isAdminDeleted) {
         continue;
       }
 
@@ -902,7 +902,7 @@ class AdminBackend {
       ..order('adminDeletedAt');
     await for (final version in pvQuery.run()) {
       // sanity check
-      if (!(version.isAdminDeleted ?? false)) {
+      if (!version.isAdminDeleted) {
         continue;
       }
 

app/lib/admin/tools/delete_all_staging.dart

@@ -9,7 +9,6 @@ import '../../account/models.dart';
 import '../../audit/models.dart';
 import '../../package/models.dart';
 import '../../publisher/models.dart';
-import '../../service/secret/models.dart';
 import '../../shared/datastore.dart';
 import '../../shared/env_config.dart';
 import '../../task/models.dart';
@@ -54,7 +53,6 @@ Future<String> executeDeleteAllStaging(List<String> args) async {
     dbService.query<Publisher>(): 500,
     dbService.query<ModeratedPackage>(): 500,
     dbService.query<NeatTaskStatus>(): 500,
-    dbService.query<Secret>(): 500,
     dbService.query<PackageState>(): 100,
   };
 

app/lib/package/models.dart

@@ -130,9 +130,8 @@ class Package extends db.ExpandoModel<String> {
   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;
+  @db.BoolProperty(required: true)
+  bool isAdminDeleted = false;
 
   /// The timestamp when the package was deleted by admins.
   @db.DateTimeProperty()
@@ -195,7 +194,7 @@ class Package extends db.ExpandoModel<String> {
 
   // Convenience Fields:
 
-  bool get isVisible => !isModerated && !(isAdminDeleted ?? false);
+  bool get isVisible => !isModerated && !isAdminDeleted;
   bool get isNotVisible => !isVisible;
 
   bool get isIncludedInRobots {
@@ -604,9 +603,8 @@ class PackageVersion extends db.ExpandoModel<String> {
   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;
+  @db.BoolProperty(required: true)
+  bool isAdminDeleted = false;
 
   /// The timestamp when the package version was deleted by admins.
   @db.DateTimeProperty()
@@ -620,7 +618,7 @@ class PackageVersion extends db.ExpandoModel<String> {
     isRetracted = false;
   }
 
-  late final isVisible = !isModerated && !(isAdminDeleted ?? false);
+  late final isVisible = !isModerated && !isAdminDeleted;
   late final isNotVisible = !isVisible;
 
   // Convenience Fields:

app/lib/service/secret/models.dart

@@ -2,17 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import '../../shared/datastore.dart' as db;
-
-/// A secret value stored in Datastore, typically an access credential used by
-/// the application.
-@db.Kind(name: 'Secret', idType: db.IdType.String)
-class Secret extends db.Model {
-  @db.StringProperty(required: true)
-  String? value;
-}
-
-/// Identifiers of the [Secret] keys.
+/// Identifiers and secret keys.
 abstract class SecretKey {
   static const String redisConnectionString = 'redis-connection-string';
 

app/lib/shared/integrity.dart

@@ -445,7 +445,7 @@ class IntegrityChecker {
     yield* _checkAdminDeletedFlags(
       kind: 'Package',
       id: p.name!,
-      isAdminDeleted: p.isAdminDeleted ?? false,
+      isAdminDeleted: p.isAdminDeleted,
       adminDeletedAt: p.adminDeletedAt,
     );
     if (p.isModerated) {
@@ -601,7 +601,7 @@ class IntegrityChecker {
     yield* _checkAdminDeletedFlags(
       kind: 'PackageVersion',
       id: pv.qualifiedVersionKey.toString(),
-      isAdminDeleted: pv.isAdminDeleted ?? false,
+      isAdminDeleted: pv.isAdminDeleted,
       adminDeletedAt: pv.adminDeletedAt,
     );
 

app/lib/tool/backfill/backfill_new_fields.dart

@@ -3,9 +3,6 @@
 // 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/service/secret/models.dart';
-import 'package:pub_dev/shared/datastore.dart';
 
 final _logger = Logger('backfill_new_fields');
 
@@ -15,33 +12,5 @@ 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('Deleting Secret entries');
-  await dbService.deleteWithQuery(dbService.query<Secret>());
-
-  _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);
-      });
-    }
-  }
+  _logger.info('Nothing to backfill.');
 }