Always run full Package.latest* calculation when a version changes.

Author: isoos

CHANGELOG.md

@@ -2,6 +2,7 @@ Important changes to data models, configuration, and migrations between each
 AppEngine version, listed here to ease deployment and troubleshooting.
 
 ## Next Release (replace with git tag when deployed)
+ * Note: `Package.latest*` fields are recalculated with a full list of `PackageVersion` entities.
 
 ## `20240821t093600-all`
  * Bumped runtimeVersion to `2024.08.20`.

app/lib/admin/actions/moderate_package_versions.dart

@@ -88,15 +88,15 @@ Set the moderated flag on a package version (updating the flag and the timestamp
 
         // Update references to latest versions.
         final pkg = await tx.lookupValue<Package>(p.key);
-        if (pkg.mayAffectLatestVersions(v.semanticVersion)) {
-          final versions =
-              await tx.query<PackageVersion>(pkg.key).run().toList();
-          pkg.updateLatestVersionReferences(
-            versions,
-            dartSdkVersion: currentDartSdk.semanticVersion,
-            flutterSdkVersion: currentFlutterSdk.semanticVersion,
-            replaced: v,
-          );
+        final versions = await tx.query<PackageVersion>(pkg.key).run().toList();
+        pkg.updateVersions(
+          versions,
+          dartSdkVersion: currentDartSdk.semanticVersion,
+          flutterSdkVersion: currentFlutterSdk.semanticVersion,
+          replaced: v,
+        );
+        if (pkg.latestVersionKey == null) {
+          throw InvalidInputException('xx');
         }
         pkg.updated = clock.now().toUtc();
         tx.insert(pkg);

app/lib/admin/backend.dart

@@ -14,7 +14,6 @@ import 'package:convert/convert.dart';
 import 'package:gcloud/service_scope.dart' as ss;
 import 'package:logging/logging.dart';
 import 'package:pool/pool.dart';
-import 'package:pub_semver/pub_semver.dart';
 
 import '../account/backend.dart';
 import '../account/consent_backend.dart';
@@ -444,7 +443,6 @@ class AdminBackend {
       final versionNames = versions.map((v) => v.version).toList();
       if (versionNames.contains(version)) {
         tx.delete(packageKey.append(PackageVersion, id: version));
-        package.versionCount--;
         package.updated = clock.now().toUtc();
       } else {
         print('Package $packageName does not have a version $version.');
@@ -455,14 +453,11 @@ class AdminBackend {
             'Last version detected. Use full package removal without the version qualifier.');
       }
 
-      if (package.mayAffectLatestVersions(Version.parse(version))) {
-        package.updateLatestVersionReferences(
-          versions.where((v) => v.version != version).toList(),
-          dartSdkVersion: currentDartSdk.semanticVersion,
-          flutterSdkVersion: currentFlutterSdk.semanticVersion,
-        );
-      }
-
+      package.updateVersions(
+        versions.where((v) => v.version != version).toList(),
+        dartSdkVersion: currentDartSdk.semanticVersion,
+        flutterSdkVersion: currentFlutterSdk.semanticVersion,
+      );
       package.deletedVersions ??= <String>[];
       if (!package.deletedVersions!.contains(version)) {
         package.deletedVersions!.add(version);

app/lib/package/backend.dart

@@ -360,7 +360,7 @@ class PackageBackend {
         throw NotFoundException.resource('package "$package"');
       }
 
-      final changed = p.updateLatestVersionReferences(
+      final changed = p.updateVersions(
         versions,
         dartSdkVersion: dartSdkVersion!,
         flutterSdkVersion: flutterSdkVersion!,
@@ -633,21 +633,18 @@ class PackageBackend {
     pv.isRetracted = isRetracted;
     pv.retracted = isRetracted ? clock.now() : null;
 
-    // Update references to latest versions if the retracted version was
-    // the latest version or the restored version is newer than the latest.
-    if (p.mayAffectLatestVersions(pv.semanticVersion)) {
-      final versions = await tx.query<PackageVersion>(p.key).run().toList();
-      final currentDartSdk = await getCachedDartSdkVersion(
-          lastKnownStable: toolStableDartSdkVersion);
-      final currentFlutterSdk = await getCachedFlutterSdkVersion(
-          lastKnownStable: toolStableFlutterSdkVersion);
-      p.updateLatestVersionReferences(
-        versions,
-        dartSdkVersion: currentDartSdk.semanticVersion,
-        flutterSdkVersion: currentFlutterSdk.semanticVersion,
-        replaced: pv,
-      );
-    }
+    // Update references to latest versions.
+    final versions = await tx.query<PackageVersion>(p.key).run().toList();
+    final currentDartSdk = await getCachedDartSdkVersion(
+        lastKnownStable: toolStableDartSdkVersion);
+    final currentFlutterSdk = await getCachedFlutterSdkVersion(
+        lastKnownStable: toolStableFlutterSdkVersion);
+    p.updateVersions(
+      versions,
+      dartSdkVersion: currentDartSdk.semanticVersion,
+      flutterSdkVersion: currentFlutterSdk.semanticVersion,
+      replaced: pv,
+    );
 
     _logger.info(
         'Updating ${p.name} ${pv.version} options: isRetracted: $isRetracted');
@@ -1064,12 +1061,6 @@ class PackageBackend {
         lastKnownStable: toolStableFlutterSdkVersion);
     final existingPackage = await lookupPackage(newVersion.package);
     final isNew = existingPackage == null;
-    final existingLatestVersion = isNew
-        ? null
-        : await lookupPackageVersion(
-            existingPackage.name!, existingPackage.latestVersion!);
-    final existingLatestIsRetracted =
-        existingLatestVersion?.isRetracted ?? false;
 
     // check authorizations before the transaction
     await _requireUploadAuthorization(
@@ -1111,6 +1102,10 @@ class PackageBackend {
     final outgoingEmail = emailBackend.prepareEntity(email);
 
     Package? package;
+    final existingVersions = await db
+        .query<PackageVersion>(ancestorKey: newVersion.packageKey!)
+        .run()
+        .toList();
 
     // Add the new package to the repository by storing the tarball and
     // inserting metadata to datastore (which happens atomically).
@@ -1159,14 +1154,12 @@ class PackageBackend {
       newVersion.publisherId = package!.publisherId;
 
       // Keep the latest version in the package object up-to-date.
-      package!.updateVersion(
-        newVersion,
+      package!.updateVersions(
+        [...existingVersions, newVersion],
         dartSdkVersion: currentDartSdk.semanticVersion,
         flutterSdkVersion: currentFlutterSdk.semanticVersion,
-        existingLatestIsRetracted: existingLatestIsRetracted,
       );
       package!.updated = clock.now().toUtc();
-      package!.versionCount++;
 
       // update automated publisher identifiers if this is the first time they have been used
       _updatePackageAutomatedPublishingLock(package!, agent);

app/lib/package/models.dart

@@ -276,122 +276,106 @@ class Package extends db.ExpandoModel<String> {
   }
 
   /// Updates the latest* version fields using all the available versions
-  /// and the current Dart SDK.
+  /// and the current Dart and Flutter SDK version.
   ///
   /// If the update was triggered because of a single version changing, the
   /// [replaced] parameter can be used to replace the corresponding entry
-  /// from the [allVersions] parameter, which may have been loaded before the
+  /// from the [versions] parameter, which may have been loaded before the
   /// transaction started.
-  bool updateLatestVersionReferences(
-    Iterable<PackageVersion> allVersions, {
+  ///
+  /// Returns whether the internal state has changed.
+  bool updateVersions(
+    List<PackageVersion> versions, {
     required Version dartSdkVersion,
     required Version flutterSdkVersion,
     PackageVersion? replaced,
   }) {
-    final versions = allVersions
+    final oldStableVersion = latestVersionKey;
+    final oldPrereleaseVersion = latestPrereleaseVersionKey;
+    final oldPreviewVersion = latestPreviewVersionKey;
+    final oldLastVersionPublished = lastVersionPublished;
+    final oldVersionCount = versionCount;
+
+    versions = versions
         .map((v) => v.version == replaced?.version ? replaced! : v)
-        .where((v) => !v.isModerated)
         .toList();
-    if (versions.isEmpty) {
+
+    final isAllRetracted = versions.every((v) => v.isRetracted);
+    final isAllModerated = versions.every((v) => v.isModerated);
+    if (isAllModerated) {
       throw NotAcceptableException('No visible versions left.');
     }
 
-    final oldStableVersion = latestSemanticVersion;
-    final oldPrereleaseVersion = latestPrereleaseSemanticVersion;
-    final oldPreviewVersion = latestPreviewSemanticVersion;
-
     // reset field values
     latestVersionKey = null;
-    latestPrereleaseVersionKey = null;
+    latestPublished = null;
     latestPreviewVersionKey = null;
+    latestPreviewPublished = null;
+    latestPrereleaseVersionKey = null;
+    latestPrereleasePublished = null;
+    lastVersionPublished = null;
+    versionCount = 0;
+
+    for (final pv in versions) {
+      // Skip all moderated versions.
+      if (pv.isModerated) {
+        continue;
+      }
 
-    for (final v in versions.where((v) => !v.isRetracted)) {
-      updateVersion(
-        v,
+      versionCount++;
+
+      // `lastVersionPublished` is updated regardless of its retracted status.
+      if (lastVersionPublished == null ||
+          lastVersionPublished!.isBefore(pv.created!)) {
+        lastVersionPublished = pv.created;
+      }
+
+      // Skip retracted versions if there is a non-retracted version,
+      // otherwise process all of the retracted ones.
+      if (pv.isRetracted && !isAllRetracted) {
+        continue;
+      }
+
+      final newVersion = pv.semanticVersion;
+      final isOnPreviewSdk = pv.pubspec!.isPreviewForCurrentSdk(
         dartSdkVersion: dartSdkVersion,
         flutterSdkVersion: flutterSdkVersion,
       );
-    }
+      final isOnStableSdk = !isOnPreviewSdk;
+
+      if (latestVersionKey == null ||
+          (isNewer(latestSemanticVersion, newVersion, pubSorted: true) &&
+              (latestSemanticVersion.isPreRelease || isOnStableSdk))) {
+        latestVersionKey = pv.key;
+        latestPublished = pv.created;
+      }
+
+      if (latestPreviewVersionKey == null ||
+          isNewer(latestPreviewSemanticVersion!, newVersion, pubSorted: true)) {
+        latestPreviewVersionKey = pv.key;
+        latestPreviewPublished = pv.created;
+      }
 
-    if (latestVersionKey == null) {
-      // All versions are retracted, we use the latest regardless of retracted status.
-      for (final v in versions) {
-        updateVersion(
-          v,
-          dartSdkVersion: dartSdkVersion,
-          flutterSdkVersion: flutterSdkVersion,
-        );
+      if (latestPrereleaseVersionKey == null ||
+          isNewer(latestPrereleaseSemanticVersion!, newVersion,
+              pubSorted: false)) {
+        latestPrereleaseVersionKey = pv.key;
+        latestPrereleasePublished = pv.created;
       }
     }
 
-    final unchanged = oldStableVersion == latestSemanticVersion &&
-        oldPrereleaseVersion == latestPrereleaseSemanticVersion &&
-        oldPreviewVersion == latestPreviewSemanticVersion;
+    final unchanged = oldStableVersion == latestVersionKey &&
+        oldPrereleaseVersion == latestPrereleaseVersionKey &&
+        oldPreviewVersion == latestPreviewVersionKey &&
+        oldLastVersionPublished == lastVersionPublished &&
+        oldVersionCount == versionCount;
     if (unchanged) {
       return false;
     }
     updated = clock.now().toUtc();
     return true;
   }
 
-  /// Updates latest stable, prerelease and preview versions and published
-  /// timestamp with the new version.
-  void updateVersion(
-    PackageVersion pv, {
-    required Version dartSdkVersion,
-    required Version flutterSdkVersion,
-    bool existingLatestIsRetracted = false,
-  }) {
-    final newVersion = pv.semanticVersion;
-    final isOnStableSdk = !pv.pubspec!.isPreviewForCurrentSdk(
-      dartSdkVersion: dartSdkVersion,
-      flutterSdkVersion: flutterSdkVersion,
-    );
-
-    if (existingLatestIsRetracted ||
-        latestVersionKey == null ||
-        (isNewer(latestSemanticVersion, newVersion, pubSorted: true) &&
-            (latestSemanticVersion.isPreRelease || isOnStableSdk))) {
-      latestVersionKey = pv.key;
-      latestPublished = pv.created;
-    }
-
-    if (existingLatestIsRetracted ||
-        latestPreviewVersionKey == null ||
-        isNewer(latestPreviewSemanticVersion!, newVersion, pubSorted: true)) {
-      latestPreviewVersionKey = pv.key;
-      latestPreviewPublished = pv.created;
-    }
-
-    if (existingLatestIsRetracted ||
-        latestPrereleaseVersionKey == null ||
-        isNewer(latestPrereleaseSemanticVersion!, newVersion,
-            pubSorted: false)) {
-      latestPrereleaseVersionKey = pv.key;
-      latestPrereleasePublished = pv.created;
-    }
-
-    if (lastVersionPublished == null ||
-        lastVersionPublished!.isBefore(pv.created!)) {
-      lastVersionPublished = pv.created;
-    }
-  }
-
-  /// Checks if a change in a version's status may affect
-  /// the latest versions: is it one of them or is it newer
-  /// than one of the latests.
-  bool mayAffectLatestVersions(Version version) {
-    return latestVersion == version.toString() ||
-        latestPrereleaseVersion == version.toString() ||
-        latestPreviewVersion == version.toString() ||
-        isNewer(latestSemanticVersion, version) ||
-        (latestPrereleaseSemanticVersion != null &&
-            isNewer(latestPrereleaseSemanticVersion!, version,
-                pubSorted: false)) ||
-        (latestPreviewSemanticVersion != null &&
-            isNewer(latestPreviewSemanticVersion!, version, pubSorted: false));
-  }
-
   bool isNewPackage() => created!.difference(clock.now()).abs().inDays <= 30;
 
   /// List of tags from the flags on the current [Package] entity.

app/lib/shared/integrity.dart

@@ -365,7 +365,10 @@ class IntegrityChecker {
       // to prevent false alarms that could happing if a new version is being published
       // while the integrity check is running.
       if (!pv.created!.isAfter(p.lastVersionPublished!)) {
-        versionCountUntilLastPublished++;
+        // Moderated versions are not counted.
+        if (!pv.isModerated) {
+          versionCountUntilLastPublished++;
+        }
       }
     }
     if (p.versionCount != versionCountUntilLastPublished) {

app/test/package/models_test.dart

@@ -327,6 +327,8 @@ class _PublishSequence {
     ..name = 'pkg'
     ..created = DateTime(2021, 01, 29);
 
+  final _versions = <PackageVersion>[];
+
   void publish(String version, {int sdk = 0}) {
     final minSdk = sdk > 0 ? _futureSdk : (sdk < 0 ? _pastSdk : _currentSdk);
     final pv = PackageVersion.init()
@@ -341,7 +343,8 @@ class _PublishSequence {
           'sdk': '>=$minSdk <3.0.0',
         },
       });
-    _p.updateVersion(pv,
+    _versions.add(pv);
+    _p.updateVersions(_versions,
         dartSdkVersion: _currentSdk,
         flutterSdkVersion: Version.parse('3.20.0'));
   }

app/test/package/retracted_test.dart

@@ -206,6 +206,9 @@ void main() {
       final origLastVersionPublished = pkg.lastVersionPublished;
       final origLatestPublished = pkg.latestPublished;
       final origLatestPrereleasePublished = pkg.latestPrereleasePublished;
+      final pv200devPublished =
+          (await packageBackend.lookupPackageVersion('oxygen', '2.0.0-dev'))!
+              .created;
 
       final client =
           await createFakeAuthPubApiClient(email: adminAtPubDevEmail);
@@ -249,6 +252,7 @@ void main() {
       expect(pkg3.latestPrereleaseVersion, '2.0.0-dev');
       expect(
           pkg3.latestPrereleasePublished, isNot(origLatestPrereleasePublished));
+      expect(pkg3.latestPrereleasePublished, pv200devPublished);
       expect(pkg3.lastVersionPublished, origLastVersionPublished);
 
       // Retract all dev