Sync exported APIs on moderation + delete a moderated version's exported files. #8282
Conversation
isoos
commented
Nov 13, 2024
isoos
commented
- Added export sync on moderation + tests (archive files and version listing checks).
- Implemented explicit deletion of exported archive files when a moderation happens on a package version. Only the archive file is deleted for now.
| final pv = await packageBackend.lookupPackageVersion(package, version); | ||
| if (pv != null && pv.isModerated) { | ||
| // We only delete the package if it is explicitly moderated. | ||
| // If we can't find it, then it's safer to assume that it's a bug. |
There was a problem hiding this comment.
Does that mean we should log something in that case?
There was a problem hiding this comment.
We just leave it there. If it is a stray file, it will get GC-d soon enough.
| // Check versions that are not exposed in the public API and if they are moderated, delete them. | ||
| for (final cv in versions.entries) { | ||
| final version = cv.key; | ||
| if (versionListing.versions.any((v) => v.version == version)) { | ||
| continue; | ||
| } | ||
| final pv = await packageBackend.lookupPackageVersion(package, version); | ||
| if (pv != null && pv.isModerated) { |
There was a problem hiding this comment.
We don't need this.
versions is the list of archives objects that exist in tarballStorage.
Later we do:
// Remove versions that are not exposed in the public API.
versions.removeWhere(
(version, _) => !versionListing.versions.any((v) => v.version == version),
);
which ensures that versions only contains entries that are explicitly listed versionListing.
And synchronizeTarballs will delete files not present in versions.
So we don't need to delete files, the only case where synchronizeTarballs doesn't delete an archive not in versions, is if the object was recently modified.
This is to avoid race conditions, but recently modified is 3 hours -- we could set that lower if we're at all concerned here.
