-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Uid] Add MockUuidFactory for deterministic UUID generation in tests
#61807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shouldn't extend the current API of factories. Let's inherit from UuidFactory instead, and let's remove all the added methods, they're breaking the design of the factory API: v3+5 don't make sense mocking, v1/6/7 are all time-based and the abstraction can be kept, v4 is already what the randomBased method returns so we don't need new methods, etc.
39e3304 to
56eae5d
Compare
56eae5d to
1074c35
Compare
d09b379 to
c307d8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a review, I thought it would be quicker to write the factory, so here is how I see this could be. If you could give it a try and add tests that'd be awesome!
Details
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Uid\Factory;
use Symfony\Component\Uid\TimeBasedUidInterface;
use Symfony\Component\Uid\Uuid;
use Symfony\Component\Uid\UuidV3;
use Symfony\Component\Uid\UuidV4;
use Symfony\Component\Uid\UuidV5;
use Symfony\Component\Uid\UuidV7;
class MockUuidFactory extends UuidFactory
{
private \Iterator $sequence;
/**
* @param iterable<string|Uuid> $uuids
*/
public function __construct(
iterable $uuids,
private Uuid|string|null $timeBasedNode = null,
private Uuid|string|null $nameBasedNamespace = null,
) {
$this->sequence = match (true) {
\is_array($uuids) => new \ArrayIterator($uuids),
$uuids instanceof \Iterator => $uuids,
$uuids instanceof \Traversable => new \IteratorIterator($uuids),
};
}
public function create(): Uuid
{
if (!$this->sequence->valid()) {
throw new \RuntimeException('No more UUIDs in sequence.');
}
$uuid = $this->sequence->current();
$this->sequence->next();
return $uuid instanceof Uuid ? $uuid : Uuid::fromString($uuid);
}
public function randomBased(): RandomBasedUuidFactory
{
if (!($uuid = $this->create()) instanceof UuidV4) {
throw new \RuntimeException(\sprintf('Next UUID in sequence is not a UuidV4: "%s".', get_debug_type($uuid)));
}
return new class($uuid) extends RandomBasedUuidFactory {
public function __construct(
private UuidV4 $uuid,
) {
}
public function create(): UuidV4
{
return $this->uuid;
}
};
}
public function timeBased(Uuid|string|null $node = null): TimeBasedUuidFactory
{
if (!($uuid = $this->create()) instanceof TimeBasedUidInterface) {
throw new \RuntimeException(\sprintf('Next UUID in sequence is not a TimeBasedUidInterface: "%s".', get_debug_type($uuid)));
}
if (\is_string($node ??= $this->timeBasedNode)) {
$node = Uuid::fromString($node);
}
return new class($uuid, $node) extends TimeBasedUuidFactory {
public function __construct(
private TimeBasedUidInterface $uuid,
private ?Uuid $node = null,
) {
if ($uuid instanceof UuidV7) {
$this->node = null;
}
}
public function create(?\DateTimeInterface $time = null): Uuid&TimeBasedUidInterface
{
if (null !== $time && $this->uuid->getDateTime() != $time) {
throw new \RuntimeException(\sprintf('Next UUID in sequence does not match the expected time: "%s" != "%s".', $this->uuid->getDateTime()->format('@U.uT'), $time->format('@U.uT')));
}
if (null !== $this->node && $this->uuid->getNode() !== substr($this->node->toRfc4122(), -12)) {
throw new \RuntimeException(\sprintf('Next UUID in sequence does not match the expected node: "%s" != "%s".', $this->uuid->getNode(), substr($this->node->toRfc4122(), -12)));
}
return $this->uuid;
}
};
}
public function nameBased(Uuid|string|null $namespace = null): NameBasedUuidFactory
{
if (!($uuid = $this->create()) instanceof UuidV5 && !$uuid instanceof UuidV3) {
throw new \RuntimeException(\sprintf('Next UUID in sequence is not a UuidV5 or UuidV3: "%s".', get_debug_type($uuid)));
}
$factory = parent::nameBased($namespace);
return new class ($uuid, $factory) extends NameBasedUuidFactory {
public function __construct(
private UuidV5|UuidV3 $uuid,
private NameBasedUuidFactory $factory,
) {
}
public function create(string $name): UuidV5|UuidV3
{
if ($this->uuid->toRfc4122() !== $this->factory->create($name)->toRfc4122()) {
throw new \RuntimeException(\sprintf('Next UUID in sequence does not match the expected named UUID: "%s" != "%s".', $this->uuid->toRfc4122(), $this->factory->create($name)->toRfc4122()));
}
return $this->uuid;
}
};
}
}|
Ah, I can already tell you I made a mistake: the sequence should be consumed in child factories, not in the main one ! |
|
@nicolas-grekas looks good I'll try you suggestion. And about the sequence I'll take care of it. |
e2df988 to
1fa277b
Compare
2056709 to
fce9b9d
Compare
nicolas-grekas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the new tests!
here are some more tweaks for the code and then we should be good to go!
3640d31 to
8464f5e
Compare
MockUuidFactory for deterministic UUID generation in tests
OskarStark
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks 🚀
8464f5e to
adb84af
Compare
|
Thank you @momito69. |
|
In order to enjoy fully the MockUuidFactory, should we expose something similar to |
|
i don't know if it would be needed, but looking at the UuidFactory classes, i would suspect them to use Interfaces instead Like should MockUuidFactory extend from UuidFactory if none of the functions and properties are used? Same with |
|
We'd rather figure out a way to have Doctrine use a factory instead. |
|
if you use a custom id generator in the ORM, it can use any dependency it wants. And If you want to generate the id yourselves in the constructor of your entities, you cannot use dependency injection at all (as the instantiation is not managed by the DI component). But I don't think that's a reason to introduce a global state for accessing the configured factory. |
Indeed, I was doing in my entity |
Similar to the Clock component's ClockInterface and MockClock, I've made a MockUuidFactory and implemented an UuidFactoryInterface to have a way to mock UUID generation for testing purposes.
cc @OskarStark , @alexandre-daubois