Skip to content

feat: make:entity php 8 attributes support #926

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

Closed
wants to merge 10 commits into from
Closed

feat: make:entity php 8 attributes support #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d12c2f9
add attribute support
Jul 4, 2021
dd4cf98
add exception when expr node build failed
Jul 4, 2021
ede1cd2
add accidantily removed addDriver check
Jul 4, 2021
f8b3faa
add doc blocks
Jul 4, 2021
b9f701b
remove unused private method, remove unecessary doc blocks
Jul 4, 2021
32bec7f
cs fixer run
Jul 5, 2021
44cf99d
Added attribute mapping detection
Geekimo Jul 13, 2021
1f30ddc
CS fix
Geekimo Jul 13, 2021
8cd167c
Mapping type check condition operator fix
Geekimo Jul 14, 2021
f2aab49
Merge branch 'symfony:main' into feature/php8-make-entity-attribute-s…
Geekimo Jul 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/DependencyInjection/CompilerPass/SetDoctrineAnnotatedPrefixesPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ public function process(ContainerBuilder $container)
$methodCalls[$i] = $arguments;
}

$isAnnotated = false !== strpos($arguments[0], '_annotation_metadata_driver');
$annotatedPrefixes[$managerName][] = [
$arguments[1],
$isAnnotated ? new Reference($arguments[0]) : null,
new Reference($arguments[0]),
];
}

Expand Down
62 changes: 62 additions & 0 deletions src/Doctrine/DoctrineHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
use Doctrine\Common\Persistence\Mapping\MappingException as LegacyPersistenceMappingException;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\Driver\AttributeDriver;
use Doctrine\ORM\Mapping\MappingException as ORMMappingException;
use Doctrine\ORM\Mapping\NamingStrategy;
use Doctrine\ORM\Tools\DisconnectedClassMetadataFactory;
use Doctrine\Persistence\ManagerRegistry;
use Doctrine\Persistence\Mapping\AbstractClassMetadataFactory;
use Doctrine\Persistence\Mapping\ClassMetadata;
use Doctrine\Persistence\Mapping\Driver\AnnotationDriver;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use Doctrine\Persistence\Mapping\Driver\MappingDriverChain;
use Doctrine\Persistence\Mapping\MappingException as PersistenceMappingException;
use Symfony\Bundle\MakerBundle\Util\ClassNameDetails;
Expand Down Expand Up @@ -122,6 +124,43 @@ public function isClassAnnotated(string $className): bool
return false;
}

public function doesClassUsesAttributes(string $className): bool
{
/** @var EntityManagerInterface $em */
$em = $this->getRegistry()->getManagerForClass($className);

if (null === $em) {
throw new \InvalidArgumentException(sprintf('Cannot find the entity manager for class "%s"', $className));
}

if (null === $this->annotatedPrefixes) {
// doctrine-bundle <= 2.2
$metadataDriver = $em->getConfiguration()->getMetadataDriverImpl();

if (!$this->isInstanceOf($metadataDriver, MappingDriverChain::class)) {
return $metadataDriver instanceof AttributeDriver;
}

foreach ($metadataDriver->getDrivers() as $namespace => $driver) {
if (0 === strpos($className, $namespace)) {
return $driver instanceof AttributeDriver;
}
}

return $metadataDriver->getDefaultDriver() instanceof AttributeDriver;
}

$managerName = array_search($em, $this->getRegistry()->getManagers(), true);

foreach ($this->annotatedPrefixes[$managerName] as [$prefix, $attributeDriver]) {
if (0 === strpos($className, $prefix)) {
return null !== $attributeDriver;
}
}

return false;
}

public function getEntitiesForAutocomplete(): array
{
$entities = [];
Expand Down Expand Up @@ -268,4 +307,27 @@ public function isKeyword(string $name): bool

return $connection->getDatabasePlatform()->getReservedKeywordsList()->isKeyword($name);
}

/**
* this method try to find the correct MappingDriver for the given namespace/class
* To determine which MappingDriver belongs to the class we check the prefixes configured in Doctrine and use the
* prefix that has the closest match to the given $namespace.
*/
public function getMappingDriverForNamespace(string $namespace): ?MappingDriver
{
$lowestCharacterDiff = null;
$foundDriver = null;

foreach ($this->annotatedPrefixes as $key => $mappings) {
foreach ($mappings as [$prefix, $driver]) {
$diff = substr_compare($namespace, $prefix, 0);
Copy link

Choose a reason for hiding this comment

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

This works incorrect. I have installed package gesdinet/jwt-refresh-token-bundle wich has annotated entity. And I have next:
substr_compare('App\Entity\Test', 'App\Entity', 0);//5
substr_compare('App\Entity\Test', 'Gesdinet\JWTRefreshTokenBundle\Entity', 0);//-6
And AnnotationDriver returns instead of AttributeDriver

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi !
This behavior sounds logical, as the purpose of this part of the code is to return the driver used for a mappping.
So for Gesdinet\JWTRefreshTokenBundle\Entity, which is annotated, the fact that you have a AnnotationDriver is correct.
Am I missing something ?

Copy link

@php-programmist php-programmist Jul 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I have 2 prefixes (name spaces):

  1. App\Entity - Attribute Driver
  2. Gesdinet\JWTRefreshTokenBundle\Entity - Annotation Driver

I executed next command:
php bin/console make:entity Test
I expected that would be created App\Entity\Test entity with attributes, but I got - annotations!
I figured out that method getMappingDriverForNamespace returns incorrect Driver. Now this method returns the last driver of $this->annotatedPrefixes array, not the closest match.
I see 2 problems in mentioned block:

  1. $lowestCharacterDiff always null. And if statement (row 325) always true
  2. For entity App\Entity\Test closest match must be App\Entity.

But
substr_compare('App\Entity\Test', 'App\Entity', 0); results as 5
substr_compare('App\Entity\Test', 'Gesdinet\JWTRefreshTokenBundle\Entity', 0); results as -6
-6 is less than 5, and this algorithm choosing incorrect driver
In my fork I using next variant of if block:

if (null === $lowestCharacterDiff || ($diff < $lowestCharacterDiff && $diff > 0)) {
      $lowestCharacterDiff = $diff;
      $foundDriver = $driver;
}

I am not absolutely sure that it is correct, but it work for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, your first message wasn't that explicit.
I'm gonna check differently this part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This point has been fixed by @adlpz in #920


if (null === $lowestCharacterDiff || $diff < $lowestCharacterDiff) {
$foundDriver = $driver;
}
}
}

return $foundDriver;
}
}
3 changes: 2 additions & 1 deletion src/Doctrine/EntityClassGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function __construct(Generator $generator, DoctrineHelper $doctrineHelper
$this->doctrineHelper = $doctrineHelper;
}

public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $apiResource, bool $withPasswordUpgrade = false, bool $generateRepositoryClass = true, bool $broadcast = false): string
public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $apiResource, bool $withPasswordUpgrade = false, bool $generateRepositoryClass = true, bool $broadcast = false, bool $doctrineAttributes = false): string
{
$repoClassDetails = $this->generator->createClassNameDetails(
$entityClassDetails->getRelativeName(),
Expand All @@ -53,6 +53,7 @@ public function generateEntityClass(ClassNameDetails $entityClassDetails, bool $
'broadcast' => $broadcast,
'should_escape_table_name' => $this->doctrineHelper->isKeyword($tableName),
'table_name' => $tableName,
'doctrine_use_attributes' => $doctrineAttributes,
]
);

Expand Down
48 changes: 40 additions & 8 deletions src/Maker/MakeEntity.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\DBAL\Types\Type;
use Doctrine\ORM\Mapping\Driver\AttributeDriver;
use Doctrine\Persistence\Mapping\Driver\AnnotationDriver;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use Symfony\Bundle\MakerBundle\ConsoleStyle;
use Symfony\Bundle\MakerBundle\DependencyBuilder;
use Symfony\Bundle\MakerBundle\Doctrine\DoctrineHelper;
Expand Down Expand Up @@ -160,6 +163,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
'Entity\\'
);

$mappingDriver = $this->doctrineHelper->getMappingDriverForNamespace($entityClassDetails->getFullName());
$classExists = class_exists($entityClassDetails->getFullName());
if (!$classExists) {
$broadcast = $input->getOption('broadcast');
Expand All @@ -168,7 +172,8 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
$input->getOption('api-resource'),
false,
true,
$broadcast
$broadcast,
$mappingDriver instanceof AttributeDriver
);

if ($broadcast) {
Expand All @@ -186,8 +191,11 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
$generator->writeChanges();
}

if (!$this->doesEntityUseAnnotationMapping($entityClassDetails->getFullName())) {
throw new RuntimeCommandException(sprintf('Only annotation mapping is supported by make:entity, but the <info>%s</info> class uses a different format. If you would like this command to generate the properties & getter/setter methods, add your mapping configuration, and then re-run this command with the <info>--regenerate</info> flag.', $entityClassDetails->getFullName()));
if (
!$this->doesEntityUseAnnotationMapping($entityClassDetails->getFullName())
&& !$this->doesEntityUseAttributeMapping($entityClassDetails->getFullName())
) {
throw new RuntimeCommandException(sprintf('Only annotation or attribute mapping is supported by make:entity, but the <info>%s</info> class uses a different format. If you would like this command to generate the properties & getter/setter methods, add your mapping configuration, and then re-run this command with the <info>--regenerate</info> flag.', $entityClassDetails->getFullName()));
}

if ($classExists) {
Expand All @@ -204,7 +212,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
}

$currentFields = $this->getPropertyNames($entityClassDetails->getFullName());
$manipulator = $this->createClassManipulator($entityPath, $io, $overwrite);
$manipulator = $this->createClassManipulator($entityPath, $io, $overwrite, $mappingDriver);

$isFirstField = true;
while (true) {
Expand Down Expand Up @@ -232,7 +240,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
$otherManipulator = $manipulator;
} else {
$otherManipulatorFilename = $this->getPathOfClass($newField->getInverseClass());
$otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite);
$otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite, $mappingDriver);
}
switch ($newField->getType()) {
case EntityRelation::MANY_TO_ONE:
Expand All @@ -247,7 +255,7 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
// the new field being added to THIS entity is the inverse
$newFieldName = $newField->getInverseProperty();
$otherManipulatorFilename = $this->getPathOfClass($newField->getOwningClass());
$otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite);
$otherManipulator = $this->createClassManipulator($otherManipulatorFilename, $io, $overwrite, $mappingDriver);

// The *other* class will receive the ManyToOne
$otherManipulator->addManyToOneRelation($newField->getOwningRelation());
Expand Down Expand Up @@ -791,9 +799,13 @@ private function askRelationType(ConsoleStyle $io, string $entityClass, string $
return $io->askQuestion($question);
}

private function createClassManipulator(string $path, ConsoleStyle $io, bool $overwrite): ClassSourceManipulator
private function createClassManipulator(string $path, ConsoleStyle $io, bool $overwrite, ?MappingDriver $mappingDriver = null): ClassSourceManipulator
{
$manipulator = new ClassSourceManipulator($this->fileManager->getFileContents($path), $overwrite);
$useAnnotations = null === $mappingDriver || $mappingDriver instanceof AnnotationDriver && !$mappingDriver instanceof AttributeDriver;
$useAttributes = $mappingDriver instanceof AttributeDriver;

$manipulator = new ClassSourceManipulator($this->fileManager->getFileContents($path), $overwrite, $useAnnotations, true, $useAttributes);

$manipulator->setIo($io);

return $manipulator;
Expand Down Expand Up @@ -848,6 +860,26 @@ private function doesEntityUseAnnotationMapping(string $className): bool
return $this->doctrineHelper->isClassAnnotated($className);
}

private function doesEntityUseAttributeMapping(string $className): bool
{
if (\PHP_VERSION < 80000) {
return false;
}

if (!class_exists($className)) {
$otherClassMetadatas = $this->doctrineHelper->getMetadata(Str::getNamespace($className).'\\', true);

// if we have no metadata, we should assume this is the first class being mapped
if (empty($otherClassMetadatas)) {
return false;
}

$className = reset($otherClassMetadatas)->getName();
}

return $this->doctrineHelper->doesClassUsesAttributes($className);
}

private function getEntityNamespace(): string
{
return $this->doctrineHelper->getEntityNamespace();
Expand Down
16 changes: 13 additions & 3 deletions src/Resources/skeleton/doctrine/Entity.tpl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,39 @@
<?php if ($broadcast): ?>use Symfony\UX\Turbo\Attribute\Broadcast;
<?php endif ?>

<?php if(!$use_attributes || !$doctrine_use_attributes): ?>
/**
<?php if ($api_resource && !$use_attributes): ?> * @ApiResource()
<?php endif ?>
<?php if ($broadcast && !$use_attributes): ?> * @Broadcast()
<?php endif ?>
* @ORM\Entity(repositoryClass=<?= $repository_class_name ?>::class)
<?php if(!$doctrine_use_attributes): ?> * @ORM\Entity(repositoryClass=<?= $repository_class_name ?>::class)
<?php endif ?>
<?php if ($should_escape_table_name): ?> * @ORM\Table(name="`<?= $table_name ?>`")
<?php endif ?>
*/
<?php endif ?>
<?php if ($api_resource && $use_attributes): ?>
#[ApiResource]
<?php endif ?>
<?php if ($broadcast && $use_attributes): ?>
#[Broadcast]
<?php endif ?>
<?php if($doctrine_use_attributes): ?>
#[ORM\Entity(repositoryClass: <?= $repository_class_name ?>::class)]
<?php if($should_escape_table_name): ?> #[ORM\Table(name: '`<?= $table_name ?>`')]<?php endif ?>
<?php endif?>
class <?= $class_name."\n" ?>
{
/**
<?php if(!$doctrine_use_attributes): ?>/**
* @ORM\Id
* @ORM\GeneratedValue
* @ORM\Column(type="integer")
*/
private $id;
<?php else: ?>#[ORM\Id]
#[ORM\GeneratedValue]
#[ORM\Column(type: 'integer')]
<?php endif ?>private $id;

public function getId(): ?int
{
Expand Down
Loading