Skip to content

Commit a67bc9d

Browse files
committed
Use Symfony locks to prevent a distribution from being resolved multiple times at once
1 parent f43ee9e commit a67bc9d

2 files changed

Lines changed: 99 additions & 11 deletions

File tree

src/Package/PackageDistributionResolver.php

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use CodedMonkey\Dirigent\Message\ResolveDistribution;
1212
use Symfony\Component\DependencyInjection\Attribute\Autowire;
1313
use Symfony\Component\Filesystem\Filesystem;
14+
use Symfony\Component\Lock\LockFactory;
1415
use Symfony\Component\Messenger\MessageBusInterface;
1516
use Symfony\Component\Messenger\Stamp\TransportNamesStamp;
1617

@@ -23,6 +24,7 @@ public function __construct(
2324
private MessageBusInterface $messenger,
2425
private ComposerClient $composer,
2526
private DistributionRepository $distributionRepository,
27+
private LockFactory $lockFactory,
2628
#[Autowire(param: 'dirigent.distributions.dev_versions')]
2729
private bool $includeDevVersions,
2830
#[Autowire(param: 'dirigent.storage.path')]
@@ -74,23 +76,34 @@ public function resolve(Metadata $metadata, string $reference, string $type, boo
7476
return false;
7577
}
7678

77-
if (null === $distribution = $this->distributionRepository->findOneByReferenceAndType($metadata, $reference, $type)) {
78-
$distribution = new Distribution($metadata, $reference, $type);
79-
}
80-
8179
$distributionUrl = $metadata->getDistributionUrl();
8280
$path = $this->path($metadata, $reference, $type);
8381

84-
$this->filesystem->mkdir(dirname($path));
82+
$lock = $this->lockFactory->createLock('distribution.' . hash('sha256', $path), ttl: null);
83+
$lock->acquire(blocking: true);
84+
85+
try {
86+
if ($this->filesystem->exists($path)) {
87+
return true;
88+
}
89+
90+
if (null === $distribution = $this->distributionRepository->findOneByReferenceAndType($metadata, $reference, $type)) {
91+
$distribution = new Distribution($metadata, $reference, $type);
92+
}
8593

86-
$httpDownloader = $this->composer->createHttpDownloader();
87-
$httpDownloader->copy($distributionUrl, $path);
94+
$this->filesystem->mkdir(dirname($path));
8895

89-
$distribution->setSource($distributionUrl);
90-
$distribution->setResolvedAt();
96+
$httpDownloader = $this->composer->createHttpDownloader();
97+
$httpDownloader->copy($distributionUrl, $path);
9198

92-
$this->distributionRepository->save($distribution, true);
99+
$distribution->setSource($distributionUrl);
100+
$distribution->setResolvedAt();
93101

94-
return true;
102+
$this->distributionRepository->save($distribution, true);
103+
104+
return true;
105+
} finally {
106+
$lock->release();
107+
}
95108
}
96109
}
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace CodedMonkey\Dirigent\Tests\UnitTests\Package;
6+
7+
use CodedMonkey\Dirigent\Composer\ComposerClient;
8+
use CodedMonkey\Dirigent\Doctrine\Repository\DistributionRepository;
9+
use CodedMonkey\Dirigent\Package\PackageDistributionResolver;
10+
use CodedMonkey\Dirigent\Tests\Helper\MockEntityFactoryTrait;
11+
use PHPUnit\Framework\Attributes\CoversClass;
12+
use PHPUnit\Framework\TestCase;
13+
use Symfony\Component\Filesystem\Filesystem;
14+
use Symfony\Component\Lock\LockFactory;
15+
use Symfony\Component\Lock\SharedLockInterface;
16+
use Symfony\Component\Messenger\MessageBusInterface;
17+
18+
#[CoversClass(PackageDistributionResolver::class)]
19+
class PackageDistributionResolverTest extends TestCase
20+
{
21+
use MockEntityFactoryTrait;
22+
23+
private string $storagePath;
24+
25+
#[\Override]
26+
protected function setUp(): void
27+
{
28+
$this->storagePath = sys_get_temp_dir() . '/dirigent-distribution-resolver-' . uniqid();
29+
}
30+
31+
#[\Override]
32+
protected function tearDown(): void
33+
{
34+
new Filesystem()->remove($this->storagePath);
35+
}
36+
37+
public function testResolveRechecksForDistributionAfterAcquiringLock(): void
38+
{
39+
[, , $metadata] = $this->createMockPackageWithMetadata();
40+
$metadata->setDistributionReference('reference');
41+
$metadata->setDistributionType('zip');
42+
$metadata->setDistributionUrl('https://example.com/distribution.zip');
43+
44+
$repository = $this->createMock(DistributionRepository::class);
45+
$repository->expects(self::never())->method('findOneByReferenceAndType');
46+
47+
$lock = $this->createMock(SharedLockInterface::class);
48+
$lockFactory = $this->createMock(LockFactory::class);
49+
$resolver = new PackageDistributionResolver(
50+
$this->createStub(MessageBusInterface::class),
51+
$this->createStub(ComposerClient::class),
52+
$repository,
53+
$lockFactory,
54+
true,
55+
$this->storagePath,
56+
);
57+
$path = $resolver->path($metadata, 'reference', 'zip');
58+
59+
$lockFactory->expects(self::once())
60+
->method('createLock')
61+
->with('distribution.' . hash('sha256', $path), null)
62+
->willReturn($lock);
63+
$lock->expects(self::once())
64+
->method('acquire')
65+
->with(true)
66+
->willReturnCallback(static function () use ($path): bool {
67+
new Filesystem()->dumpFile($path, 'distribution');
68+
69+
return true;
70+
});
71+
$lock->expects(self::once())->method('release');
72+
73+
self::assertTrue($resolver->resolve($metadata, 'reference', 'zip', async: false));
74+
}
75+
}

0 commit comments

Comments
 (0)