diff --git a/CHANGELOG.md b/CHANGELOG.md index d254f870b..6b12885c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ AppEngine version, listed here to ease deployment and troubleshooting. ## Next Release (replace with git tag when deployed) * Bump runtimeVersion to `2024.12.12`. * Upgraded runtime Dart SDK to `3.6.0`. + * Note: Started reporting unmapped fields + * Started and deleting `Package.isWithheld` and `Package.withheldReason`. ## `20241212t111200-all` * Bump runtimeVersion to `2024.12.11`. diff --git a/app/lib/shared/integrity.dart b/app/lib/shared/integrity.dart index bc90eea83..b300a56f8 100644 --- a/app/lib/shared/integrity.dart +++ b/app/lib/shared/integrity.dart @@ -35,11 +35,22 @@ import 'utils.dart' show canonicalizeVersion, ByteArrayEqualsExt; final _logger = Logger('integrity.check'); final _random = math.Random.secure(); +/// The unmapped/unused fields that we expect to be present on some entities. +/// The presence of such fields won't be reported as integrity issue, only +/// the absent ones will be reported. +const _allowedUnmappedFields = { + 'Package.isWithheld', + 'Package.withheldReason', +}; + /// Checks the integrity of the datastore. class IntegrityChecker { final DatastoreDB _db; final int _concurrency; + /// Maps an unmapped field in the form of `.` to an + /// object identifier (usually the `id` value of the entity). + final _unmappedFieldsToObject = {}; final _userToOauth = {}; final _oauthToUser = {}; final _deletedUsers = {}; @@ -93,7 +104,13 @@ class IntegrityChecker { yield* _checkAuditLogs(); yield* _checkModerationCases(); yield* _reportPubspecVersionIssues(); - // TODO: report unmapped properties + + if (_unmappedFieldsToObject.isNotEmpty) { + for (final entry in _unmappedFieldsToObject.entries) { + if (_allowedUnmappedFields.contains(entry.key)) continue; + yield 'Unmapped field found: "${entry.key}" on entity "${entry.value}".'; + } + } } finally { _httpClient.close(); } @@ -448,6 +465,7 @@ class IntegrityChecker { } await for (final pvi in pviQuery.run()) { + _updateUnmappedFields(pvi); final key = pvi.qualifiedVersionKey; pviKeys.add(key); yield* checkPackageVersionKey('PackageVersionInfo', key); @@ -475,6 +493,7 @@ class IntegrityChecker { ..filter('package =', p.name); final foundAssetIds = {}; await for (final pva in pvaQuery.run()) { + _updateUnmappedFields(pva); final key = pva.qualifiedVersionKey; if (pva.id != Uri(pathSegments: [pva.package!, pva.version!, pva.kind!]).path) { @@ -907,6 +926,16 @@ class IntegrityChecker { } } + void _updateUnmappedFields(Model m) { + if (m is ExpandoModel && m.additionalProperties.isNotEmpty) { + for (final key in m.additionalProperties.keys) { + final qualifiedField = [m.runtimeType.toString(), key].join('.'); + if (_unmappedFieldsToObject.containsKey(qualifiedField)) continue; + _unmappedFieldsToObject[qualifiedField] = m.id.toString(); + } + } + } + Stream _queryWithPool( Stream Function(R model) fn) async* { final query = _db.query(); @@ -914,6 +943,7 @@ class IntegrityChecker { final futures = >>[]; try { await for (final m in query.run()) { + _updateUnmappedFields(m); final f = pool.withResource(() => fn(m).toList()); futures.add(f); } diff --git a/app/lib/tool/backfill/backfill_new_fields.dart b/app/lib/tool/backfill/backfill_new_fields.dart index 2871f3009..7cf1a53df 100644 --- a/app/lib/tool/backfill/backfill_new_fields.dart +++ b/app/lib/tool/backfill/backfill_new_fields.dart @@ -4,6 +4,7 @@ import 'package:clock/clock.dart'; import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; import 'package:pub_dev/account/models.dart'; import 'package:pub_dev/package/api_export/api_exporter.dart'; import 'package:pub_dev/package/backend.dart'; @@ -20,9 +21,11 @@ final _logger = Logger('backfill_new_fields'); /// release could remove the backfill from here. Future backfillNewFields() async { await migrateIsBlocked(); + await _removeKnownUnmappedFields(); } /// Migrates entities from the `isBlocked` fields to the new `isModerated` instead. +@visibleForTesting Future migrateIsBlocked() async { _logger.info('Migrating isBlocked...'); final pkgQuery = dbService.query()..filter('isBlocked =', true); @@ -88,3 +91,18 @@ Future migrateIsBlocked() async { _logger.info('isBlocked migration completed.'); } + +Future _removeKnownUnmappedFields() async { + await for (final p in dbService.query().run()) { + if (p.additionalProperties.isEmpty) continue; + if (p.additionalProperties.containsKey('isWithheld') || + p.additionalProperties.containsKey('withheldReason')) { + await withRetryTransaction(dbService, (tx) async { + final pkg = await tx.lookupValue(p.key); + pkg.additionalProperties.remove('isWithheld'); + pkg.additionalProperties.remove('withheldReason'); + tx.insert(pkg); + }); + } + } +}