Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support other subclasses of EntityManagerInterface too #180

Closed
wants to merge 1 commit into from
Closed

Support other subclasses of EntityManagerInterface too #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@
}
],
"require": {
"php": "^5.4 || ^7 || ^8",
"php": "^7.4 || ^8",
"doctrine/dbal": "^2.5 || ^3.0",
"ramsey/uuid": "^3.5 || ^4"
},
Expand Down
24 changes: 21 additions & 3 deletions src/UuidGenerator.php
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@

namespace Ramsey\Uuid\Doctrine;

use Doctrine\ORM\EntityManager;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Id\AbstractIdGenerator;
use Doctrine\ORM\Mapping\Entity;
use Exception;
Expand All @@ -27,14 +27,32 @@ class UuidGenerator extends AbstractIdGenerator
/**
* Generate an identifier
*
* @param EntityManager $em
* @param EntityManagerInterface $em
* @param Entity $entity
*
* @return UuidInterface
*
* @throws Exception
*
* @deprecated as per parent. Accepts any EntityManagerInterface for maximum compatibility on PHP 7.4+.
* @see UuidGenerator::generateId()
*/
public function generate(EntityManagerInterface $em, $entity): UuidInterface
{
return Uuid::uuid4();
}

/**
* Generate an identifier
*
* @param EntityManagerInterface $em
* @param Entity $entity
*
* @return UuidInterface
*
* @throws Exception
*/
public function generate(EntityManager $em, $entity)
public function generateId(EntityManagerInterface $em, $entity): UuidInterface
Copy link
Owner

Choose a reason for hiding this comment

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

This introduces a BC break, right? If anyone extends UuidGenerator and overrides this method, making this change will result in an error like:

Fatal error: Declaration of Foo::generate(EntityManager $em, $entity) must be compatible with Ramsey\Uuid\Doctrine\UuidGenerator::generate(EntityManagerInterface $em, $entity): UuidInterface

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramsey Yes, sorry I wasn't clear. This is what I was trying to get at with:

I suggest this targets a new major release.

I would also expect the bump in minimum PHP version to necessitate a new major version as that's also a BC break, hence my thinking that the two changes could go hand in hand.

Copy link

Choose a reason for hiding this comment

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

A bump in minimum PHP version shouldn't be considered a BC break IMHO. The reasoning is explained under "Why dropping PHP support in a minor version is not a BC break" at https://www.doctrine-project.org/2017/07/25/php-7.1-requirement-and-composer.html

{
return Uuid::uuid4();
}
Expand Down
24 changes: 21 additions & 3 deletions src/UuidOrderedTimeGenerator.php
View file
Open in desktop
Expand Up @@ -12,7 +12,7 @@

namespace Ramsey\Uuid\Doctrine;

use Doctrine\ORM\EntityManager;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Id\AbstractIdGenerator;
use Doctrine\ORM\Mapping\Entity;
use Exception;
Expand Down Expand Up @@ -42,14 +42,32 @@ public function __construct()
/**
* Generates an identifier for an entity.
*
* @param EntityManager $em
* @param EntityManagerInterface $em
* @param Entity $entity
*
* @return UuidInterface
*
* @throws Exception
*
* @deprecated as per parent. Accepts any EntityManagerInterface for maximum compatibility on PHP 7.4+.
* @see UuidOrderedTimeGenerator::generateId()
*/
public function generate(EntityManagerInterface $em, $entity): UuidInterface
{
return $this->factory->uuid1();
}

/**
* Generates an identifier for an entity.
*
* @param EntityManagerInterface $em
* @param Entity $entity
*
* @return UuidInterface
*
* @throws Exception
*/
public function generate(EntityManager $em, $entity)
public function generateId(EntityManagerInterface $em, $entity): UuidInterface
{
return $this->factory->uuid1();
}
Expand Down
19 changes: 17 additions & 2 deletions tests/UuidGeneratorTest.php
View file
Open in desktop
Expand Up @@ -4,20 +4,35 @@

use Doctrine\ORM\Mapping\Entity;
use PHPUnit\Framework\TestCase;
use Ramsey\Uuid\UuidInterface;

class UuidGeneratorTest extends TestCase
{
/**
* @covers \Ramsey\Uuid\Doctrine\UuidGenerator::generate
*/
public function testUuidGeneratorGenerates()
public function testUuidGeneratorGeneratesInLegacyMode(): void
{
$em = new TestEntityManager();
$entity = new Entity();
$generator = new UuidGenerator();

$uuid = $generator->generate($em, $entity);

$this->assertInstanceOf('Ramsey\Uuid\UuidInterface', $uuid);
$this->assertInstanceOf(UuidInterface::class, $uuid);
}

/**
* @covers \Ramsey\Uuid\Doctrine\UuidGenerator::generateId
*/
public function testUuidGeneratorGenerates(): void
{
$em = new TestEntityManager();
$entity = new Entity();
$generator = new UuidGenerator();

$uuid = $generator->generateId($em, $entity);

$this->assertInstanceOf(UuidInterface::class, $uuid);
}
}
21 changes: 19 additions & 2 deletions tests/UuidOrderedTimeGeneratorTest.php
View file
Open in desktop
Expand Up @@ -4,22 +4,39 @@

use Doctrine\ORM\Mapping\Entity;
use PHPUnit\Framework\TestCase;
use Ramsey\Uuid\UuidInterface;

class UuidOrderedTimeGeneratorTest extends TestCase
{
/**
* @covers \Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator::generate
* @covers \Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator::__construct
*/
public function testUuidGeneratorGenerates()
public function testUuidGeneratorGeneratesInLegacyMode(): void
{
$em = new TestEntityManager();
$entity = new Entity();
$generator = new UuidOrderedTimeGenerator();

$uuid = $generator->generate($em, $entity);

$this->assertInstanceOf('Ramsey\Uuid\UuidInterface', $uuid);
$this->assertInstanceOf(UuidInterface::class, $uuid);
$this->assertSame(1, $uuid->getVersion());
}

/**
* @covers \Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator::generateId
* @covers \Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator::__construct
*/
public function testUuidGeneratorGenerates(): void
{
$em = new TestEntityManager();
$entity = new Entity();
$generator = new UuidOrderedTimeGenerator();

$uuid = $generator->generateId($em, $entity);

$this->assertInstanceOf(UuidInterface::class, $uuid);
$this->assertSame(1, $uuid->getVersion());
}
}