From 7bab04e1242cab89195db951fc2e526c4eec29f7 Mon Sep 17 00:00:00 2001 From: Steven Rombauts Date: Fri, 7 Nov 2025 12:42:22 +0100 Subject: [PATCH 1/5] Add command to transfer ownership in bulk --- src/Command/TransferOwnershipCommand.php | 167 ++++++++++++++++++ src/Entity/UserRepository.php | 17 ++ .../Command/TransferOwnershipCommandTest.php | 130 ++++++++++++++ tests/Entity/UserRepositoryTest.php | 46 +++++ 4 files changed, 360 insertions(+) create mode 100644 src/Command/TransferOwnershipCommand.php create mode 100644 tests/Command/TransferOwnershipCommandTest.php create mode 100644 tests/Entity/UserRepositoryTest.php diff --git a/src/Command/TransferOwnershipCommand.php b/src/Command/TransferOwnershipCommand.php new file mode 100644 index 000000000..b5e2cd10b --- /dev/null +++ b/src/Command/TransferOwnershipCommand.php @@ -0,0 +1,167 @@ + + * Nils Adermann + * + * 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\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('vendor', InputArgument::REQUIRED,'Vendor prefix'), + 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'); + } + + $vendor = $input->getArgument('vendor'); + $maintainers = $this->queryAndValidateMaintainers($input, $output); + + if (!count($maintainers)) { + return Command::FAILURE; + } + + $packages = $this->queryVendorPackages($vendor); + + if (!count($packages)) { + $output->writeln(sprintf('No packages found for vendor %s', $vendor)); + 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('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('%d maintainers could not be found: %s', count($notFound), implode(', ', $notFound))); + + return []; + } + + /** + * @return Package[] + */ + private function queryVendorPackages(string $vendor): array + { + return $this->getEM() + ->getRepository(Package::class) + ->getFilteredQueryBuilder(['vendor' => $vendor], true) + ->getQuery() + ->getResult(); + } + + /** + * @param Package[] $packages + * @param User[] $maintainers + */ + private function outputPackageTable(OutputInterface $output, array $packages, array $maintainers): void + { + $rows = []; + + $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) { + $package->getMaintainers()->clear(); + foreach ($maintainers as $maintainer) { + $package->addMaintainer($maintainer); + } + } + + $this->doctrine->getManager()->flush(); + } +} diff --git a/src/Entity/UserRepository.php b/src/Entity/UserRepository.php index 6b16df6ca..028d8a525 100644 --- a/src/Entity/UserRepository.php +++ b/src/Entity/UserRepository.php @@ -41,6 +41,23 @@ public function findOneByUsernameOrEmail(string $usernameOrEmail): ?User return $this->findOneBy(['usernameCanonical' => $usernameOrEmail]); } + /** + * @param string[] $usernames + * @param ?array $orderBy + * @return array + */ + 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 */ diff --git a/tests/Command/TransferOwnershipCommandTest.php b/tests/Command/TransferOwnershipCommandTest.php new file mode 100644 index 000000000..b27a1a04b --- /dev/null +++ b/tests/Command/TransferOwnershipCommandTest.php @@ -0,0 +1,130 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests\Command; + +use App\Command\TransferOwnershipCommand; +use App\Entity\Package; +use App\Entity\PackageRepository; +use App\Entity\User; +use App\Entity\UserRepository; +use App\Tests\IntegrationTestCase; +use Doctrine\ORM\EntityManager; +use Doctrine\Persistence\ManagerRegistry; +use PHPUnit\Framework\MockObject\MockObject; +use Symfony\Bundle\FrameworkBundle\Console\Application; +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Tester\CommandTester; + +class TransferOwnershipCommandTest extends IntegrationTestCase +{ + private CommandTester $commandTester; + + private Package $package1; + private Package $package2; + private Package $package3; + + protected function setUp(): void + { + parent::setUp(); + + $alice = self::createUser('alice', 'alice@example.org'); + $bob = self::createUser('bob', 'bob@example.org'); + $john = self::createUser('john', 'john@example.org'); + $this->store($alice, $bob, $john); + + $this->package1 = self::createPackage('vendor1/package1', 'https://github.com/vendor1/package1', maintainers: [$john, $alice]); + $this->package2 = self::createPackage('vendor1/package2', 'https://github.com/vendor1/package2',maintainers: [$john, $bob]); + $this->package3 = self::createPackage('vendor2/package1', 'https://github.com/vendor2/package1',maintainers: [$john]); + $this->store($this->package1, $this->package2, $this->package3); + + $command = new TransferOwnershipCommand(self::getContainer()->get(ManagerRegistry::class)); + $this->commandTester = new CommandTester($command); + } + + public function testExecuteSuccessWithAllMaintainersFound(): void + { + $this->commandTester->execute([ + 'vendor' => 'vendor1', + 'maintainers' => ['bob', 'alice'], + ]); + + $this->commandTester->assertCommandIsSuccessful(); + + $em = self::getEM(); + $em->clear(); + + $package1 = $em->find(Package::class, $this->package1->getId()); + $package2 = $em->find(Package::class, $this->package2->getId()); + $package3 = $em->find(Package::class, $this->package3->getId()); + + $this->assertNotNull($package1); + $this->assertNotNull($package2); + $this->assertNotNull($package3); + + $callable = fn (User $user) => $user->getUsernameCanonical(); + $this->assertEqualsCanonicalizing(['alice', 'bob'], array_map($callable, $package1->getMaintainers()->toArray())); + $this->assertEqualsCanonicalizing(['alice', 'bob'], array_map($callable, $package2->getMaintainers()->toArray())); + $this->assertEqualsCanonicalizing(['john'], array_map($callable, $package3->getMaintainers()->toArray()), 'vendor1 package maintainers should not be changed'); + } + + public function testExecuteWithDryRunDoesNothing(): void + { + $this->commandTester->execute([ + 'vendor' => 'vendor1', + 'maintainers' => ['alice'], + '--dry-run' => true, + ]); + + $this->commandTester->assertCommandIsSuccessful(); + + $em = self::getEM(); + $em->clear(); + + $package1 = $em->find(Package::class, $this->package1->getId()); + $package2 = $em->find(Package::class, $this->package2->getId()); + + $this->assertNotNull($package1); + $this->assertNotNull($package2); + + $callable = fn (User $user) => $user->getUsernameCanonical(); + $this->assertEqualsCanonicalizing(['john', 'alice'], array_map($callable, $package1->getMaintainers()->toArray())); + $this->assertEqualsCanonicalizing(['john', 'bob'], array_map($callable, $package2->getMaintainers()->toArray())); + } + + public function testExecuteFailsWithUnknownMaintainers(): void + { + $this->commandTester->execute([ + 'vendor' => 'vendor1', + 'maintainers' => ['unknown1', 'alice', 'unknown2'], + ]); + + $this->assertSame(Command::FAILURE, $this->commandTester->getStatusCode()); + + $output = $this->commandTester->getDisplay(); + $this->assertStringContainsString('2 maintainers could not be found', $output); + } + + public function testExecuteFailsIfNoVendorPackagesFound(): void + { + $this->commandTester->execute([ + 'vendor' => 'foobar', + 'maintainers' => ['bob', 'alice'], + ]); + + $this->assertSame(Command::FAILURE, $this->commandTester->getStatusCode()); + + $output = $this->commandTester->getDisplay(); + $this->assertStringContainsString('No packages found for vendor', $output); + } +} diff --git a/tests/Entity/UserRepositoryTest.php b/tests/Entity/UserRepositoryTest.php new file mode 100644 index 000000000..2e32eee7d --- /dev/null +++ b/tests/Entity/UserRepositoryTest.php @@ -0,0 +1,46 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests\Entity; + +use App\Entity\UserRepository; +use App\Tests\IntegrationTestCase; + +class UserRepositoryTest extends IntegrationTestCase +{ + private UserRepository $userRepository; + + protected function setUp(): void + { + parent::setUp(); + + $this->userRepository = self::getEM()->getRepository(\App\Entity\User::class); + } + + public function testFindUsersByUsernameWithMultipleValidUsernames(): void + { + $alice = self::createUser('Alice', 'alice@example.org'); + $bob = self::createUser('Bob', 'bob@example.org'); + $charlie = self::createUser('Charlie', 'charlie@example.org'); + $this->store($alice, $bob, $charlie); + + $result = $this->userRepository->findUsersByUsername(['alice', 'bob']); + + $this->assertCount(2, $result); + + $this->assertArrayHasKey('alice', $result); + $this->assertArrayHasKey('bob', $result); + + $this->assertSame($alice->getId(), $result['alice']->getId()); + $this->assertSame($bob->getId(), $result['bob']->getId()); + } +} From 7200a78ad6c397bf26f441bec9601e358a85fdf6 Mon Sep 17 00:00:00 2001 From: Steven Rombauts Date: Fri, 7 Nov 2025 14:52:08 +0100 Subject: [PATCH 2/5] Track package transferred event for each individual package --- src/Audit/AuditRecordType.php | 2 +- src/Command/TransferOwnershipCommand.php | 5 +++ src/Entity/AuditRecord.php | 13 ++++++++ .../Command/TransferOwnershipCommandTest.php | 33 +++++++++++++++---- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/Audit/AuditRecordType.php b/src/Audit/AuditRecordType.php index c2dcfcf78..dddb3253b 100644 --- a/src/Audit/AuditRecordType.php +++ b/src/Audit/AuditRecordType.php @@ -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'; diff --git a/src/Command/TransferOwnershipCommand.php b/src/Command/TransferOwnershipCommand.php index b5e2cd10b..b40573d6e 100644 --- a/src/Command/TransferOwnershipCommand.php +++ b/src/Command/TransferOwnershipCommand.php @@ -12,6 +12,7 @@ namespace App\Command; +use App\Entity\AuditRecord; use App\Entity\Package; use App\Entity\User; use App\Util\DoctrineTrait; @@ -156,10 +157,14 @@ private function outputPackageTable(OutputInterface $output, array $packages, ar 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))); } $this->doctrine->getManager()->flush(); diff --git a/src/Entity/AuditRecord.php b/src/Entity/AuditRecord.php index 4f0bd5f01..aee77c348 100644 --- a/src/Entity/AuditRecord.php +++ b/src/Entity/AuditRecord.php @@ -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), 'previous_maintainers' => $previous, 'current_maintainers' => $current], $actor?->getId(), $package->getVendor(), $package->getId()); + } + public static function versionDeleted(Version $version, ?User $actor): self { $package = $version->getPackage(); diff --git a/tests/Command/TransferOwnershipCommandTest.php b/tests/Command/TransferOwnershipCommandTest.php index b27a1a04b..e49642b15 100644 --- a/tests/Command/TransferOwnershipCommandTest.php +++ b/tests/Command/TransferOwnershipCommandTest.php @@ -12,17 +12,13 @@ namespace App\Tests\Command; +use App\Audit\AuditRecordType; use App\Command\TransferOwnershipCommand; +use App\Entity\AuditRecord; use App\Entity\Package; -use App\Entity\PackageRepository; use App\Entity\User; -use App\Entity\UserRepository; use App\Tests\IntegrationTestCase; -use Doctrine\ORM\EntityManager; use Doctrine\Persistence\ManagerRegistry; -use PHPUnit\Framework\MockObject\MockObject; -use Symfony\Bundle\FrameworkBundle\Console\Application; -use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; @@ -76,6 +72,10 @@ public function testExecuteSuccessWithAllMaintainersFound(): void $this->assertEqualsCanonicalizing(['alice', 'bob'], array_map($callable, $package1->getMaintainers()->toArray())); $this->assertEqualsCanonicalizing(['alice', 'bob'], array_map($callable, $package2->getMaintainers()->toArray())); $this->assertEqualsCanonicalizing(['john'], array_map($callable, $package3->getMaintainers()->toArray()), 'vendor1 package maintainers should not be changed'); + + $this->assertAuditLogWasCreated($package1, ['john', 'alice'], ['alice', 'bob']); + $this->assertAuditLogWasCreated($package2, ['john', 'bob'], ['alice', 'bob']); + } public function testExecuteWithDryRunDoesNothing(): void @@ -127,4 +127,25 @@ public function testExecuteFailsIfNoVendorPackagesFound(): void $output = $this->commandTester->getDisplay(); $this->assertStringContainsString('No packages found for vendor', $output); } + + /** + * @param string[] $oldMaintainers + * @param string[] $newMaintainers + */ + private function assertAuditLogWasCreated(Package $package, array $oldMaintainers, array $newMaintainers): void + { + $record = $this->getEM()->getRepository(AuditRecord::class)->findOneBy([ + 'type' => AuditRecordType::PackageTransferred->value, + 'packageId' => $package->getId(), + 'actorId' => null, + ]); + + $this->assertNotNull($record); + $this->assertSame('unknown', $record->attributes['actor']); + $this->assertSame($package->getId(), $record->packageId); + + $callable = fn (array $user) => $user['username']; + $this->assertEqualsCanonicalizing($oldMaintainers, array_map($callable, $record->attributes['previous_maintainers'])); + $this->assertEqualsCanonicalizing($newMaintainers, array_map($callable, $record->attributes['current_maintainers'])); + } } From 466eed6e607a874f76abb0bd6d10433bcd1bc7af Mon Sep 17 00:00:00 2001 From: Steven Rombauts Date: Fri, 7 Nov 2025 15:38:38 +0100 Subject: [PATCH 3/5] Use `admin` as fallback string for actor when transferring packages in bulk --- src/Entity/AuditRecord.php | 2 +- tests/Command/TransferOwnershipCommandTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Entity/AuditRecord.php b/src/Entity/AuditRecord.php index aee77c348..f9454b54f 100644 --- a/src/Entity/AuditRecord.php +++ b/src/Entity/AuditRecord.php @@ -75,7 +75,7 @@ public static function packageTransferred(Package $package, ?User $actor, array $previous = array_map($callback, $previousMaintainers); $current = array_map($callback, $currentMaintainers); - return new self(AuditRecordType::PackageTransferred, ['name' => $package->getName(), 'actor' => self::getUserData($actor), 'previous_maintainers' => $previous, 'current_maintainers' => $current], $actor?->getId(), $package->getVendor(), $package->getId()); + 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 diff --git a/tests/Command/TransferOwnershipCommandTest.php b/tests/Command/TransferOwnershipCommandTest.php index e49642b15..068a12d60 100644 --- a/tests/Command/TransferOwnershipCommandTest.php +++ b/tests/Command/TransferOwnershipCommandTest.php @@ -141,7 +141,7 @@ private function assertAuditLogWasCreated(Package $package, array $oldMaintainer ]); $this->assertNotNull($record); - $this->assertSame('unknown', $record->attributes['actor']); + $this->assertSame('admin', $record->attributes['actor']); $this->assertSame($package->getId(), $record->packageId); $callable = fn (array $user) => $user['username']; From 33169ca5e1ad4b928f6d34419bf7ffd99206b61a Mon Sep 17 00:00:00 2001 From: Steven Rombauts Date: Fri, 7 Nov 2025 15:59:43 +0100 Subject: [PATCH 4/5] Allow transfer command to accept a single package name too --- src/Command/TransferOwnershipCommand.php | 29 +++++++++----- .../Command/TransferOwnershipCommandTest.php | 39 +++++++++++++++---- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/Command/TransferOwnershipCommand.php b/src/Command/TransferOwnershipCommand.php index b40573d6e..119bd26fd 100644 --- a/src/Command/TransferOwnershipCommand.php +++ b/src/Command/TransferOwnershipCommand.php @@ -41,7 +41,7 @@ protected function configure(): void ->setName('packagist:transfer-ownership') ->setDescription('Transfer all packages of a vendor') ->setDefinition([ - new InputArgument('vendor', InputArgument::REQUIRED,'Vendor prefix'), + 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'), ]) @@ -56,17 +56,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int $output->writeln('ℹ️ DRY RUN'); } - $vendor = $input->getArgument('vendor'); + $vendorOrPackage = $input->getArgument('vendorOrPackage'); $maintainers = $this->queryAndValidateMaintainers($input, $output); if (!count($maintainers)) { return Command::FAILURE; } - $packages = $this->queryVendorPackages($vendor); + $packages = $this->queryPackages($vendorOrPackage); if (!count($packages)) { - $output->writeln(sprintf('No packages found for vendor %s', $vendor)); + $output->writeln(sprintf('No packages found for %s', $vendorOrPackage)); return Command::FAILURE; } @@ -111,13 +111,20 @@ private function queryAndValidateMaintainers(InputInterface $input, OutputInterf /** * @return Package[] */ - private function queryVendorPackages(string $vendor): array + private function queryPackages(string $vendorOrPackage): array { - return $this->getEM() - ->getRepository(Package::class) - ->getFilteredQueryBuilder(['vendor' => $vendor], true) - ->getQuery() - ->getResult(); + $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 + ]); } /** @@ -128,6 +135,8 @@ private function outputPackageTable(OutputInterface $output, array $packages, ar { $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) { diff --git a/tests/Command/TransferOwnershipCommandTest.php b/tests/Command/TransferOwnershipCommandTest.php index 068a12d60..6555c7618 100644 --- a/tests/Command/TransferOwnershipCommandTest.php +++ b/tests/Command/TransferOwnershipCommandTest.php @@ -19,6 +19,7 @@ use App\Entity\User; use App\Tests\IntegrationTestCase; use Doctrine\Persistence\ManagerRegistry; +use PHPUnit\Framework\Attributes\TestWith; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; @@ -48,10 +49,10 @@ protected function setUp(): void $this->commandTester = new CommandTester($command); } - public function testExecuteSuccessWithAllMaintainersFound(): void + public function testExecuteSuccessForVendor(): void { $this->commandTester->execute([ - 'vendor' => 'vendor1', + 'vendorOrPackage' => 'vendor1', 'maintainers' => ['bob', 'alice'], ]); @@ -71,17 +72,41 @@ public function testExecuteSuccessWithAllMaintainersFound(): void $callable = fn (User $user) => $user->getUsernameCanonical(); $this->assertEqualsCanonicalizing(['alice', 'bob'], array_map($callable, $package1->getMaintainers()->toArray())); $this->assertEqualsCanonicalizing(['alice', 'bob'], array_map($callable, $package2->getMaintainers()->toArray())); - $this->assertEqualsCanonicalizing(['john'], array_map($callable, $package3->getMaintainers()->toArray()), 'vendor1 package maintainers should not be changed'); + $this->assertEqualsCanonicalizing(['john'], array_map($callable, $package3->getMaintainers()->toArray()), 'vendor2 packages should not be changed'); $this->assertAuditLogWasCreated($package1, ['john', 'alice'], ['alice', 'bob']); $this->assertAuditLogWasCreated($package2, ['john', 'bob'], ['alice', 'bob']); + } + + public function testExecuteSuccessForPackage(): void + { + $this->commandTester->execute([ + 'vendorOrPackage' => 'vendor2/package1', + 'maintainers' => ['john', 'alice'], + ]); + + $this->commandTester->assertCommandIsSuccessful(); + + $em = self::getEM(); + $em->clear(); + + $package2 = $em->find(Package::class, $this->package2->getId()); + $package3 = $em->find(Package::class, $this->package3->getId()); + + $this->assertNotNull($package2); + $this->assertNotNull($package3); + + $callable = fn (User $user) => $user->getUsernameCanonical(); + $this->assertEqualsCanonicalizing(['bob', 'john'], array_map($callable, $package2->getMaintainers()->toArray()), 'vendor1 packages should not be changed'); + $this->assertEqualsCanonicalizing(['alice', 'john'], array_map($callable, $package3->getMaintainers()->toArray())); + $this->assertAuditLogWasCreated($package3, ['john'], ['alice', 'john']); } public function testExecuteWithDryRunDoesNothing(): void { $this->commandTester->execute([ - 'vendor' => 'vendor1', + 'vendorOrPackage' => 'vendor1', 'maintainers' => ['alice'], '--dry-run' => true, ]); @@ -105,7 +130,7 @@ public function testExecuteWithDryRunDoesNothing(): void public function testExecuteFailsWithUnknownMaintainers(): void { $this->commandTester->execute([ - 'vendor' => 'vendor1', + 'vendorOrPackage' => 'vendor1', 'maintainers' => ['unknown1', 'alice', 'unknown2'], ]); @@ -118,14 +143,14 @@ public function testExecuteFailsWithUnknownMaintainers(): void public function testExecuteFailsIfNoVendorPackagesFound(): void { $this->commandTester->execute([ - 'vendor' => 'foobar', + 'vendorOrPackage' => 'foobar', 'maintainers' => ['bob', 'alice'], ]); $this->assertSame(Command::FAILURE, $this->commandTester->getStatusCode()); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('No packages found for vendor', $output); + $this->assertStringContainsString('No packages found for foobar', $output); } /** From 102f95525a9d3e03c365fb73f16b92d7d1c3a745 Mon Sep 17 00:00:00 2001 From: Steven Rombauts Date: Fri, 7 Nov 2025 16:03:55 +0100 Subject: [PATCH 5/5] Use `mb_strtolower` to conform with the canonicalization in `User::setUsernameCanonical()` --- src/Command/TransferOwnershipCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Command/TransferOwnershipCommand.php b/src/Command/TransferOwnershipCommand.php index 119bd26fd..7868797f2 100644 --- a/src/Command/TransferOwnershipCommand.php +++ b/src/Command/TransferOwnershipCommand.php @@ -84,7 +84,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int */ private function queryAndValidateMaintainers(InputInterface $input, OutputInterface $output): array { - $usernames = array_map('strtolower', $input->getArgument('maintainers')); + $usernames = array_map('mb_strtolower', $input->getArgument('maintainers')); sort($usernames); $maintainers = $this->getEM()->getRepository(User::class)->findUsersByUsername($usernames, ['usernameCanonical' => 'ASC']);