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

Implement colocated mapping driver #9587

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Implement colocated mapping driver #9587

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions UPGRADE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Upgrade to 2.12

## BC Break: `AttributeDriver` and `AnnotationDriver` no longer extends parent class from `doctrine/persistence`

Both these classes used to extend an abstract `AnnotationDriver` class defined
in `doctrine/persistence`, and no longer do.

## Deprecate `AttributeDriver::getReader()` and `AnnotationDriver::getReader()`

That method was inherited from the abstract `AnnotationDriver` class of
`doctrine/persistence`, and does not seem to serve any purpose.

## Un-deprecate `Doctrine\ORM\Proxy\Proxy`

Because no forward-compatible new proxy solution had been implemented yet, the
Expand Down
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"doctrine/inflector": "^1.4 || ^2.0",
"doctrine/instantiator": "^1.3",
"doctrine/lexer": "^1.2.3",
"doctrine/persistence": "^2.2",
"doctrine/persistence": "^2.4",
"psr/cache": "^1 || ^2 || ^3",
"symfony/console": "^3.0 || ^4.0 || ^5.0 || ^6.0",
"symfony/polyfill-php72": "^1.23",
Expand Down
65 changes: 63 additions & 2 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@
namespace Doctrine\ORM\Mapping\Driver;

use Doctrine\Common\Annotations\AnnotationReader;
use Doctrine\Common\Annotations\Reader;
use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\Events;
use Doctrine\ORM\Mapping;
use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\Persistence\Mapping\ClassMetadata;
use Doctrine\Persistence\Mapping\Driver\AnnotationDriver as AbstractAnnotationDriver;
use Doctrine\Persistence\Mapping\Driver\ColocatedMappingDriver;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use ReflectionClass;
use ReflectionMethod;
use ReflectionProperty;
Expand All @@ -29,8 +32,19 @@
/**
* The AnnotationDriver reads the mapping metadata from docblock annotations.
*/
class AnnotationDriver extends AbstractAnnotationDriver
class AnnotationDriver implements MappingDriver
Copy link
Member

Choose a reason for hiding this comment

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

This BC break needs to be reverted, and moved to the next major.

Ref: #9668

{
use ColocatedMappingDriver;

/**
* The annotation reader.
*
* @internal this property will be private in 3.0
*
* @var Reader
*/
protected $reader;
Copy link
Member

Choose a reason for hiding this comment

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

Shall we flag the property as @internal and make it private on 3.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't we deprecate AnnotationDriver and remove it entirely in 3.0?
Also, I'm wondering why AttributeDriver has a getReader() method, and is not final 🤔


/**
* @var int[]
* @psalm-var array<class-string, int>
Expand All @@ -40,6 +54,20 @@ class AnnotationDriver extends AbstractAnnotationDriver
Mapping\MappedSuperclass::class => 2,
];

/**
* Initializes a new AnnotationDriver that uses the given AnnotationReader for reading
* docblock annotations.
*
* @param Reader $reader The AnnotationReader to use
Copy link
Member

Choose a reason for hiding this comment

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

why not using an actual parameter type ?

Copy link
Member

Choose a reason for hiding this comment

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

That would be a BC break. The old constructor explicitly mentioned the possibility of duck-typed annotation readers.

* @param string|string[]|null $paths One or multiple paths where mapping classes can be found.
*/
public function __construct($reader, $paths = null)
{
$this->reader = $reader;

$this->addPaths((array) $paths);
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -786,6 +814,39 @@ private function columnToArray(string $fieldName, Mapping\Column $column): array
return $mapping;
}

/**
* Retrieve the current annotation reader
*
* @return Reader
*/
public function getReader()
{
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/pull/9587',
'%s is deprecated with no replacement',
__METHOD__
);

return $this->reader;
}

/**
* {@inheritDoc}
*/
public function isTransient($className)
{
$classAnnotations = $this->reader->getClassAnnotations(new ReflectionClass($className));

foreach ($classAnnotations as $annot) {
if (isset($this->entityAnnotationClasses[get_class($annot)])) {
return false;
}
}

return true;
}

/**
* Factory method for the Annotation Driver.
*
Expand Down
29 changes: 27 additions & 2 deletions lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@

namespace Doctrine\ORM\Mapping\Driver;

use Doctrine\Deprecations\Deprecation;
use Doctrine\ORM\Events;
use Doctrine\ORM\Mapping;
use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\Persistence\Mapping\ClassMetadata;
use Doctrine\Persistence\Mapping\Driver\AnnotationDriver;
use Doctrine\Persistence\Mapping\Driver\ColocatedMappingDriver;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use LogicException;
use ReflectionClass;
use ReflectionMethod;
Expand All @@ -25,8 +27,10 @@

use const PHP_VERSION_ID;

class AttributeDriver extends AnnotationDriver
class AttributeDriver implements MappingDriver
Copy link
Member Author

@greg0ire greg0ire Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I should implement getReader as well, removing that method is a BC-break I guess 😅 … but since it's not breaking any tests, I think I should instantly deprecate it.

{
use ColocatedMappingDriver;

/** @var array<string,int> */
// @phpcs:ignore
protected $entityAnnotationClasses = [
Expand All @@ -37,6 +41,8 @@ class AttributeDriver extends AnnotationDriver
/**
* The annotation reader.
*
* @internal this property will be private in 3.0
*
* @var AttributeReader
*/
protected $reader;
Expand All @@ -57,6 +63,25 @@ public function __construct(array $paths)
$this->addPaths($paths);
}

/**
* Retrieve the current annotation reader
*
* @deprecated no replacement planned.
*
* @return AttributeReader
*/
public function getReader()
{
Deprecation::trigger(
'doctrine/orm',
'https://github.com/doctrine/orm/pull/9587',
'%s is deprecated with no replacement',
__METHOD__
);

return $this->reader;
}

/**
* {@inheritDoc}
*/
Expand Down
10 changes: 0 additions & 10 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,16 +260,6 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php

-
message: "#^PHPDoc type Doctrine\\\\ORM\\\\Mapping\\\\Driver\\\\AttributeReader of property Doctrine\\\\ORM\\\\Mapping\\\\Driver\\\\AttributeDriver\\:\\:\\$reader is not covariant with PHPDoc type Doctrine\\\\Common\\\\Annotations\\\\Reader of overridden property Doctrine\\\\Persistence\\\\Mapping\\\\Driver\\\\AnnotationDriver\\:\\:\\$reader\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php

-
message: "#^PHPDoc type array\\<string, int\\> of property Doctrine\\\\ORM\\\\Mapping\\\\Driver\\\\AttributeDriver\\:\\:\\$entityAnnotationClasses is not covariant with PHPDoc type array\\<class\\-string, bool\\|int\\> of overridden property Doctrine\\\\Persistence\\\\Mapping\\\\Driver\\\\AnnotationDriver\\:\\:\\$entityAnnotationClasses\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php

-
message: "#^Parameter \\#1 \\$metadata of static method Doctrine\\\\ORM\\\\Mapping\\\\Builder\\\\EntityListenerBuilder\\:\\:bindEntityListener\\(\\) expects Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadata, Doctrine\\\\ORM\\\\Mapping\\\\ClassMetadataInfo&Doctrine\\\\Persistence\\\\Mapping\\\\ClassMetadata\\<T of object\\> given\\.$#"
count: 1
Expand Down
7 changes: 0 additions & 7 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,9 +747,6 @@
<code>$mapping</code>
</LessSpecificReturnStatement>
<MoreSpecificReturnType occurrences="1"/>
<NonInvariantDocblockPropertyType occurrences="1">
<code>$entityAnnotationClasses</code>
</NonInvariantDocblockPropertyType>
<PossiblyNullArgument occurrences="1">
<code>$listenerClassName</code>
</PossiblyNullArgument>
Expand Down Expand Up @@ -782,10 +779,6 @@
<code>$mapping</code>
</LessSpecificReturnStatement>
<MoreSpecificReturnType occurrences="1"/>
<NonInvariantDocblockPropertyType occurrences="2">
<code>$entityAnnotationClasses</code>
<code>$reader</code>
</NonInvariantDocblockPropertyType>
<PossiblyNullArgument occurrences="1">
<code>$listenerClassName</code>
</PossiblyNullArgument>
Expand Down