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
2 changes: 1 addition & 1 deletion src/Audit/AuditRecordType.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ enum AuditRecordType: string
// package ownership
case MaintainerAdded = 'maintainer_added'; // TODO
case MaintainerRemoved = 'maintainer_removed'; // TODO
case PackageTransferred = 'package_transferred'; // TODO
case PackageTransferred = 'package_transferred';

// package management
case PackageCreated = 'package_created';
Expand Down
181 changes: 181 additions & 0 deletions src/Command/TransferOwnershipCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
<?php declare(strict_types=1);

/*
* This file is part of Packagist.
*
* (c) Jordi Boggiano <j.boggiano@seld.be>
* Nils Adermann <naderman@naderman.de>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace App\Command;

use App\Entity\AuditRecord;
use App\Entity\Package;
use App\Entity\User;
use App\Util\DoctrineTrait;
use Composer\Console\Input\InputOption;
use Doctrine\Persistence\ManagerRegistry;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Helper\Table;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class TransferOwnershipCommand extends Command
{
use DoctrineTrait;

public function __construct(
private readonly ManagerRegistry $doctrine,
)
{
parent::__construct();
}

protected function configure(): void
{
$this
->setName('packagist:transfer-ownership')
->setDescription('Transfer all packages of a vendor')
->setDefinition([
new InputArgument('vendorOrPackage', InputArgument::REQUIRED,'Vendor or package name'),
new InputArgument('maintainers', InputArgument::IS_ARRAY|InputArgument::REQUIRED, 'The usernames of the new maintainers'),
new InputOption('dry-run', null, InputOption::VALUE_NONE, 'Dry run'),
])
;
}

protected function execute(InputInterface $input, OutputInterface $output): int
{
$dryRun = $input->getOption('dry-run');

if ($dryRun) {
$output->writeln('ℹ️ DRY RUN');
}

$vendorOrPackage = $input->getArgument('vendorOrPackage');
$maintainers = $this->queryAndValidateMaintainers($input, $output);

if (!count($maintainers)) {
return Command::FAILURE;
}

$packages = $this->queryPackages($vendorOrPackage);

if (!count($packages)) {
$output->writeln(sprintf('<error>No packages found for %s</error>', $vendorOrPackage));
return Command::FAILURE;
}

$this->outputPackageTable($output, $packages, $maintainers);

if (!$dryRun) {
$this->transferOwnership($packages, $maintainers);
}

return Command::SUCCESS;
}

/**
* @return User[]
*/
private function queryAndValidateMaintainers(InputInterface $input, OutputInterface $output): array
{
$usernames = array_map('mb_strtolower', $input->getArgument('maintainers'));
sort($usernames);

$maintainers = $this->getEM()->getRepository(User::class)->findUsersByUsername($usernames, ['usernameCanonical' => 'ASC']);

if (array_keys($maintainers) === $usernames) {
return $maintainers;
}

$notFound = [];

foreach ($usernames as $username) {
if (!array_key_exists($username, $maintainers)) {
$notFound[] = $username;
}
}

sort($notFound);

$output->writeln(sprintf('<error>%d maintainers could not be found: %s</error>', count($notFound), implode(', ', $notFound)));

return [];
}

/**
* @return Package[]
*/
private function queryPackages(string $vendorOrPackage): array
{
$repository = $this->getEM()->getRepository(Package::class);
$isPackageName = str_contains($vendorOrPackage, '/');

if ($isPackageName) {
$package = $repository->findOneBy(['name' => $vendorOrPackage]);

return $package ? [$package] : [];
}

return $repository->findBy([
'vendor' => $vendorOrPackage
]);
}

/**
* @param Package[] $packages
* @param User[] $maintainers
*/
private function outputPackageTable(OutputInterface $output, array $packages, array $maintainers): void
{
$rows = [];

usort($packages, fn (Package $a, Package $b) => strcasecmp($a->getName(), $b->getName()));

$newMaintainers = array_map(fn (User $user) => $user->getUsername(), $maintainers);

foreach ($packages as $package) {
$currentMaintainers = $package->getMaintainers()->map(fn (User $user) => $user->getUsername())->toArray();
sort($currentMaintainers);

$rows[] = [
$package->getVendor(),
$package->getPackageName(),
implode(', ', $currentMaintainers),
implode(', ', $newMaintainers),
];
}

$table = new Table($output);
$table
->setHeaders(['Vendor', 'Package', 'Current Maintainers', 'New Maintainers'])
->setRows($rows)
;
$table->render();
}

/**
* @param Package[] $packages
* @param User[] $maintainers
*/
private function transferOwnership(array $packages, array $maintainers): void
{
foreach ($packages as $package) {
$oldMaintainers = $package->getMaintainers()->toArray();

$package->getMaintainers()->clear();
foreach ($maintainers as $maintainer) {
$package->addMaintainer($maintainer);
}

$this->doctrine->getManager()->persist(AuditRecord::packageTransferred($package, null, $oldMaintainers, array_values($maintainers)));
Copy link
Contributor

Choose a reason for hiding this comment

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

What about packages that were already own by the new maintainers ? Should they actually be marked as transferred to the same maintainers ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on my understanding, this transfer command is only necessary when people no longer have access, and it's highly unlikely that current maintainers remain in place. Besides that, storing all old and new maintainers also tells us exactly what arguments the command was used with?

Copy link
Contributor

Choose a reason for hiding this comment

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

When applying that to a whole vendor namespace, the existing packages might have different maintainers. Maybe one of them already has only the new maintainers.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think what we could do here is at least check that the old and new arent' an exact match and if they are skip transferring that package as that's a noop. I'll add that post merge.

}

$this->doctrine->getManager()->flush();
}
}
13 changes: 13 additions & 0 deletions src/Entity/AuditRecord.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ public static function canonicalUrlChange(Package $package, ?User $actor, string
return new self(AuditRecordType::CanonicalUrlChanged, ['name' => $package->getName(), 'repository_from' => $oldRepository, 'repository_to' => $package->getRepository(), 'actor' => self::getUserData($actor)], $actor?->getId(), $package->getVendor(), $package->getId());
}

/**
* @param User[] $previousMaintainers
* @param User[] $currentMaintainers
*/
public static function packageTransferred(Package $package, ?User $actor, array $previousMaintainers, array $currentMaintainers): self
{
$callback = fn (User $user) => self::getUserData($user);
$previous = array_map($callback, $previousMaintainers);
$current = array_map($callback, $currentMaintainers);

return new self(AuditRecordType::PackageTransferred, ['name' => $package->getName(), 'actor' => self::getUserData($actor, 'admin'), 'previous_maintainers' => $previous, 'current_maintainers' => $current], $actor?->getId(), $package->getVendor(), $package->getId());
}

public static function versionDeleted(Version $version, ?User $actor): self
{
$package = $version->getPackage();
Expand Down
17 changes: 17 additions & 0 deletions src/Entity/UserRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,23 @@ public function findOneByUsernameOrEmail(string $usernameOrEmail): ?User
return $this->findOneBy(['usernameCanonical' => $usernameOrEmail]);
}

/**
* @param string[] $usernames
* @param ?array<string, string> $orderBy
* @return array<string, User>
*/
public function findUsersByUsername(array $usernames, ?array $orderBy = null): array
{
$matches = $this->findBy(['usernameCanonical' => $usernames], $orderBy);

$users = [];
foreach ($matches as $match) {
$users[$match->getUsernameCanonical()] = $match;
}

return $users;
}

/**
* @return list<User>
*/
Expand Down
Loading