Skip to content

Commit ac5a16e

Browse files
committed
Make sure delete_before sees all versions
1 parent dbbdc5f commit ac5a16e

2 files changed

Lines changed: 57 additions & 4 deletions

File tree

src/Package/Updater.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,17 @@ public function update(IOInterface $io, Config $config, Package $package, VcsRep
281281
// hard-deleted. DELETE_BEFORE wipes the dev-branch trackers and lets the normal update
282282
// path reconcile stable rows via the immutability gate (matching refs are skipped,
283283
// diverging refs are blocked + audited).
284-
foreach ($package->getVersions() as $version) {
285-
if ($version->isDevelopment()) {
286-
$versionRepository->remove($version);
287-
}
284+
// Source the rows from the DB rather than $package->getVersions() — the in-memory
285+
// collection state depends on how the caller loaded the package, and we must not
286+
// silently skip removals just because the collection isn't lazy-loaded yet.
287+
/** @var list<Version> $devVersions */
288+
$devVersions = $versionRepository->createQueryBuilder('v')
289+
->where('v.package = :package')
290+
->andWhere('v.development = true')
291+
->setParameter('package', $package)
292+
->getQuery()->getResult();
293+
foreach ($devVersions as $version) {
294+
$versionRepository->remove($version);
288295
}
289296

290297
$em->flush();

tests/Package/UpdaterTest.php

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,52 @@ public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithN
491491
self::assertSame('newref9999999999', $reloaded->getSource()['reference'] ?? null);
492492
}
493493

494+
public function testDeleteBeforeWipesDevRowsButPreservesStableAndSoftDeletedStable(): void
495+
{
496+
// Seed three rows: an active stable, a maintainer-soft-deleted stable, and a dev branch.
497+
// DELETE_BEFORE must wipe only the dev row, leave both stable rows untouched, and the
498+
// post-loop prune must not crash on the survivors after $em->refresh($package).
499+
$this->seedStableVersion($this->package, '1.0.0', '1.0.0.0', 'abcdef1234567890');
500+
$softDeleted = $this->seedStableVersion($this->package, '1.1.0', '1.1.0.0', '1234567890abcdef');
501+
$softDeleted->setSoftDeletedAt(new \DateTimeImmutable('-2 hours'));
502+
$softDeleted->setDeletionReason(VersionDeletionReason::DeletedByMaintainer);
503+
$this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'devref1234567890');
504+
self::getEM()->persist($softDeleted);
505+
self::getEM()->flush();
506+
507+
// Upstream only returns the active stable version — dev-main has disappeared from upstream
508+
// and 1.1.0 is also missing (consistent with the maintainer pull).
509+
$upstream = $this->buildCompletePackage('test/pkg', '1.0.0', '1.0.0.0', 'abcdef1234567890');
510+
$this->repositoryMock = $this->createStub(VcsRepository::class);
511+
$this->repositoryMock->method('getPackages')->willReturn([$upstream]);
512+
$this->repositoryMock->method('getDriver')->willReturn($this->stableDriver());
513+
514+
$packageManagerMock = $this->createMock(PackageManager::class);
515+
$packageManagerMock->expects($this->never())->method('notifyVersionReferenceChangeBlocked');
516+
$this->rebuildUpdater($packageManagerMock);
517+
518+
$this->updater->update($this->ioMock, $this->config, $this->package, $this->repositoryMock, Updater::DELETE_BEFORE);
519+
520+
$em = self::getEM();
521+
$em->clear();
522+
$versionRepo = $em->getRepository(Version::class);
523+
524+
$active = $versionRepo->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => '1.0.0.0']);
525+
self::assertNotNull($active, 'active stable row must survive DELETE_BEFORE');
526+
self::assertNull($active->getSoftDeletedAt());
527+
self::assertSame('abcdef1234567890', $active->getSource()['reference'] ?? null);
528+
529+
$reloadedSoftDeleted = $versionRepo->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => '1.1.0.0']);
530+
self::assertNotNull($reloadedSoftDeleted, 'maintainer-soft-deleted stable row must survive DELETE_BEFORE');
531+
self::assertNotNull($reloadedSoftDeleted->getSoftDeletedAt(), 'soft-delete marker must stay');
532+
self::assertSame(VersionDeletionReason::DeletedByMaintainer, $reloadedSoftDeleted->getDeletionReason());
533+
534+
$dev = $versionRepo->findOneBy(['name' => 'test/pkg', 'normalizedVersion' => 'dev-main']);
535+
self::assertNull($dev, 'dev row must be hard-deleted by DELETE_BEFORE');
536+
537+
self::assertSame(0, $this->countAudits(AuditRecordType::VersionReferenceChangeBlocked));
538+
}
539+
494540
public function testAutoSoftDeletedDevVersionIsRecoveredWhenBranchReappearsWithUnchangedRef(): void
495541
{
496542
$existing = $this->seedDevVersion($this->package, 'dev-main', 'dev-main', 'sameref1234567890');

0 commit comments

Comments
 (0)