Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -8,6 +8,7 @@ AppEngine version, listed here to ease deployment and troubleshooting.
* Upgraded dartdoc to `8.3.0`.
* Upgraded pana to `0.22.16`.
* Upgraded dependencies.
* Note: started migrating `isBlocked` flags to `isModerated`.

## `20241121t150900-all`
* Bump runtimeVersion to `2024.11.21`.
Expand Down
76 changes: 75 additions & 1 deletion app/lib/tool/backfill/backfill_new_fields.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@
// 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 'package:clock/clock.dart';
import 'package:logging/logging.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';
import 'package:pub_dev/package/models.dart';
import 'package:pub_dev/publisher/models.dart';
import 'package:pub_dev/shared/datastore.dart';

final _logger = Logger('backfill_new_fields');

Expand All @@ -12,5 +19,72 @@ 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('Nothing to backfill');
await migrateIsBlocked();
}

/// Migrates entities from the `isBlocked` fields to the new `isModerated` instead.
Future<void> migrateIsBlocked() async {
_logger.info('Migrating isBlocked...');
final pkgQuery = dbService.query<Package>()..filter('isBlocked =', true);
await for (final entity in pkgQuery.run()) {
await withRetryTransaction(dbService, (tx) async {
final pkg = await tx.lookupValue<Package>(entity.key);
// sanity check
if (!pkg.isBlocked) {
return;
}
pkg
..isModerated = true
..moderatedAt = pkg.moderatedAt ?? pkg.blocked ?? clock.now()
..isBlocked = false
..blocked = null
..blockedReason = null;
tx.insert(pkg);
});

// sync exported API(s)
await apiExporter?.synchronizePackage(entity.name!, forceDelete: true);

// retract or re-populate public archive files
await packageBackend.tarballStorage.updatePublicArchiveBucket(
package: entity.name!,
ageCheckThreshold: Duration.zero,
deleteIfOlder: Duration.zero,
);
Comment on lines +45 to +53
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we want to do this? Or should we just let this happen naturally?

I don't really have any strong feelings on the subject 🤣

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are we sure we want to do this? Or should we just let this happen naturally?

The tests would fail with integrity checks, otherwise we could let it happen naturally. I don't mind that it happens right away.

}

final publisherQuery = dbService.query<Publisher>()
..filter('isBlocked =', true);
await for (final entity in publisherQuery.run()) {
await withRetryTransaction(dbService, (tx) async {
final publisher = await tx.lookupValue<Publisher>(entity.key);
// sanity check
if (!publisher.isBlocked) {
return;
}
publisher
..isModerated = true
..moderatedAt = publisher.moderatedAt ?? clock.now()
..isBlocked = false;
tx.insert(publisher);
});
}

final userQuery = dbService.query<User>()..filter('isBlocked =', true);
await for (final entity in userQuery.run()) {
await withRetryTransaction(dbService, (tx) async {
final user = await tx.lookupValue<User>(entity.key);
// sanity check
if (!user.isBlocked) {
return;
}
user
..isModerated = true
..moderatedAt = user.moderatedAt ?? clock.now()
..isBlocked = false;
tx.insert(user);
});
}

_logger.info('isBlocked migration completed.');
}
10 changes: 9 additions & 1 deletion app/test/shared/test_services.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import 'package:pub_dev/shared/redis_cache.dart';
import 'package:pub_dev/shared/versions.dart';
import 'package:pub_dev/task/cloudcompute/fakecloudcompute.dart';
import 'package:pub_dev/task/global_lock.dart';
import 'package:pub_dev/tool/backfill/backfill_new_fields.dart';
import 'package:pub_dev/tool/test_profile/import_source.dart';
import 'package:pub_dev/tool/test_profile/importer.dart';
import 'package:pub_dev/tool/test_profile/models.dart';
Expand Down Expand Up @@ -120,7 +121,6 @@ class FakeAppengineEnv {
await fork(() async {
await fn();
});
// post-test integrity check
final problems =
await IntegrityChecker(dbService).findProblems().toList();
if (problems.isNotEmpty &&
Expand All @@ -131,6 +131,14 @@ class FakeAppengineEnv {
} else if (problems.isEmpty && integrityProblem != null) {
throw Exception('Integrity problem expected but not present.');
}

// TODO: run all background tasks here
await backfillNewFields();

// re-run integrity checks on the updated state
final laterProblems =
await IntegrityChecker(dbService).findProblems().toList();
expect(laterProblems, problems);
},
);
}) as R;
Expand Down
52 changes: 52 additions & 0 deletions app/test/tool/maintenance/migrate_isblocked_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// 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 'package:pub_dev/account/backend.dart';
import 'package:pub_dev/package/backend.dart';
import 'package:pub_dev/publisher/backend.dart';
import 'package:pub_dev/shared/datastore.dart';
import 'package:pub_dev/tool/backfill/backfill_new_fields.dart';
import 'package:test/test.dart';

import '../../shared/test_services.dart';

void main() {
group('Migrate isBlocked', () {
testWithProfile('package', fn: () async {
final p1 = await packageBackend.lookupPackage('oxygen');
await dbService.commit(
inserts: [p1!..updateIsBlocked(isBlocked: true, reason: 'abc')]);
await migrateIsBlocked();

final p2 = await packageBackend.lookupPackage('oxygen');
expect(p2!.isModerated, true);
});

testWithProfile('publisher', fn: () async {
final p1 = await publisherBackend.getPublisher('example.com');
await dbService.commit(inserts: [p1!..markForBlocked()]);
final members =
await publisherBackend.listPublisherMembers('example.com');
for (final m in members) {
await accountBackend.updateBlockedFlag(m.userId, true);
}
final neon = await packageBackend.lookupPackage('neon');
await dbService.commit(inserts: [neon!..isDiscontinued = true]);

await migrateIsBlocked();

final p2 = await publisherBackend.getPublisher('example.com');
expect(p2!.isModerated, true);
});

testWithProfile('user', fn: () async {
final u1 = await accountBackend.lookupUserByEmail('user@pub.dev');
await dbService.commit(inserts: [u1..isBlocked = true]);
await migrateIsBlocked();

final u2 = await accountBackend.lookupUserByEmail('user@pub.dev');
expect(u2.isModerated, true);
});
});
}
3 changes: 3 additions & 0 deletions pkg/fake_gcloud/lib/mem_datastore.dart
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ class MemDatastore implements Datastore {
return -1;
}
}
if (a is bool && b is bool) {
return a == b ? 0 : (a ? 1 : -1);
}
if (a is Key && b is Key) {
if (a.elements.length != 1) {
throw UnimplementedError();
Expand Down
Loading