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.

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

Deprecate / remove TABLE id generator strategy #8961

Merged
merged 1 commit into from
Aug 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Upgrade to 2.10

## BC Break: Removed `TABLE` id generator strategy

The implementation was unfinished for 14 years.
It is now deprecated to rely on:
- `Doctrine\ORM\Id\TableGenerator`;
- `Doctrine\ORM\Mapping\ClassMetadata::GENERATOR_TYPE_TABLE`;
- `Doctrine\ORM\Mapping\ClassMetadata::$tableGeneratorDefinition`;
- or `Doctrine\ORM\Mapping\ClassMetadata::isIdGeneratorTable()`.

## BC Break: Removed possibility to extend the doctrine mapping xml schema with anything

If you want to extend it now you have to provide your own validation schema.
Expand Down
2 changes: 1 addition & 1 deletion docs/en/reference/annotations-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ Optional attributes:


- **strategy**: Set the name of the identifier generation strategy.
Valid values are ``AUTO``, ``SEQUENCE``, ``TABLE``, ``IDENTITY``, ``UUID`` (deprecated), ``CUSTOM`` and ``NONE``, explained
Valid values are ``AUTO``, ``SEQUENCE``, ``IDENTITY``, ``UUID`` (deprecated), ``CUSTOM`` and ``NONE``, explained
in the :ref:`Identifier Generation Strategies <identifier-generation-strategies>` section.
If not specified, default value is AUTO.

Expand Down
4 changes: 2 additions & 2 deletions docs/en/reference/attributes-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,8 @@ used as default.
Optional attributes:

- **strategy**: Set the name of the identifier generation strategy.
Valid values are ``AUTO``, ``SEQUENCE``, ``TABLE``, ``IDENTITY``,
``UUID`` (deprecated), ``CUSTOM`` and ``NONE``.
Valid values are ``AUTO``, ``SEQUENCE``, ``IDENTITY``, ``UUID``
(deprecated), ``CUSTOM`` and ``NONE``.
If not specified, the default value is ``AUTO``.

Example:
Expand Down
3 changes: 0 additions & 3 deletions docs/en/reference/basic-mapping.rst
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,6 @@ Here is the list of possible generation strategies:
(AUTO\_INCREMENT), MSSQL (IDENTITY) and PostgreSQL (SERIAL).
- ``UUID`` (deprecated): Tells Doctrine to use the built-in Universally
Unique Identifier generator. This strategy provides full portability.
- ``TABLE``: Tells Doctrine to use a separate table for ID
generation. This strategy provides full portability.
***This strategy is not yet implemented!***
- ``NONE``: Tells Doctrine that the identifiers are assigned (and
thus generated) by your code. The assignment must take place before
a new entity is passed to ``EntityManager#persist``. NONE is the
Expand Down
1 change: 0 additions & 1 deletion doctrine-mapping.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@
<xs:simpleType name="generator-strategy">
<xs:restriction base="xs:token">
<xs:enumeration value="NONE"/>
<xs:enumeration value="TABLE"/>
<xs:enumeration value="SEQUENCE"/>
<xs:enumeration value="IDENTITY"/>
<xs:enumeration value="AUTO"/>
Expand Down
2 changes: 2 additions & 0 deletions lib/Doctrine/ORM/Id/TableGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

/**
* Id generator that uses a single-row database table and a hi/lo algorithm.
*
* @deprecated no replacement planned
*/
class TableGenerator extends AbstractIdGenerator
{
Expand Down
10 changes: 2 additions & 8 deletions lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Doctrine\ORM\Id\IdentityGenerator;
use Doctrine\ORM\Id\SequenceGenerator;
use Doctrine\ORM\Id\UuidGenerator;
use Doctrine\ORM\Mapping\Exception\CannotGenerateIds;
use Doctrine\ORM\Mapping\Exception\InvalidCustomGenerator;
use Doctrine\ORM\Mapping\Exception\TableGeneratorNotImplementedYet;
use Doctrine\ORM\Mapping\Exception\UnknownGeneratorType;
Expand Down Expand Up @@ -626,11 +627,6 @@ private function completeIdGeneratorMapping(ClassMetadataInfo $class): void
$class->setIdGenerator(new UuidGenerator());
break;

case ClassMetadata::GENERATOR_TYPE_TABLE:
throw TableGeneratorNotImplementedYet::create();

break;

case ClassMetadata::GENERATOR_TYPE_CUSTOM:
$definition = $class->customGeneratorDefinition;
if ($definition === null) {
Expand Down Expand Up @@ -663,7 +659,7 @@ private function determineIdGeneratorStrategy(AbstractPlatform $platform): int
return ClassMetadata::GENERATOR_TYPE_SEQUENCE;
}

return ClassMetadata::GENERATOR_TYPE_TABLE;
throw CannotGenerateIds::withPlatform($platform);
}

private function truncateSequenceName(string $schemaElementName): string
Expand All @@ -689,8 +685,6 @@ private function inheritIdGeneratorMapping(ClassMetadataInfo $class, ClassMetada
{
if ($parent->isIdGeneratorSequence()) {
$class->setSequenceGeneratorDefinition($parent->sequenceGeneratorDefinition);
} elseif ($parent->isIdGeneratorTable()) {
$class->tableGeneratorDefinition = $parent->tableGeneratorDefinition;
}

if ($parent->generatorType) {
Expand Down
14 changes: 10 additions & 4 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ class ClassMetadataInfo implements ClassMetadata

/**
* TABLE means a separate table is used for id generation.
* Offers full portability.
* Offers full portability (in that it results in an exception being thrown
* no matter the platform).
*
* @deprecated no replacement planned
*/
public const GENERATOR_TYPE_TABLE = 3;

Expand Down Expand Up @@ -628,8 +631,9 @@ class ClassMetadataInfo implements ClassMetadata
* READ-ONLY: The definition of the table generator of this class. Only used for the
* TABLE generation strategy.
*
* @deprecated
*
* @var array<string, mixed>
* @todo Merge with tableGeneratorDefinition into generic generatorDefinition
*/
public $tableGeneratorDefinition;

Expand Down Expand Up @@ -2230,11 +2234,13 @@ public function isIdGeneratorSequence()
/**
* Checks whether the class uses a table for id generation.
*
* @return bool TRUE if the class uses the TABLE generator, FALSE otherwise.
* @deprecated
*
* @return false
*/
public function isIdGeneratorTable()
{
return $this->generatorType === self::GENERATOR_TYPE_TABLE;
return false;
}

/**
Expand Down
2 changes: 0 additions & 2 deletions lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,6 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
'initialValue' => $seqGeneratorAnnot->initialValue,
]
);
} elseif ($this->reader->getPropertyAnnotation($property, TableGenerator::class)) {
throw MappingException::tableIdGeneratorNotImplemented($className);
} else {
$customGeneratorAnnot = $this->reader->getPropertyAnnotation($property, Mapping\CustomIdGenerator::class);
if ($customGeneratorAnnot) {
Expand Down
2 changes: 0 additions & 2 deletions lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,6 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
'class' => (string) $customGenerator['class'],
]
);
} elseif (isset($idElement->{'table-generator'})) {
throw MappingException::tableIdGeneratorNotImplemented($className);
}
}

Expand Down
4 changes: 1 addition & 3 deletions lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
. strtoupper($idElement['generator']['strategy'])));
}

// Check for SequenceGenerator/TableGenerator definition
// Check for SequenceGenerator definition
if (isset($idElement['sequenceGenerator'])) {
$metadata->setSequenceGeneratorDefinition($idElement['sequenceGenerator']);
} elseif (isset($idElement['customIdGenerator'])) {
Expand All @@ -360,8 +360,6 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
'class' => (string) $customGenerator['class'],
]
);
} elseif (isset($idElement['tableGenerator'])) {
throw MappingException::tableIdGeneratorNotImplemented($className);
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions lib/Doctrine/ORM/Mapping/Exception/CannotGenerateIds.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Doctrine\ORM\Mapping\Exception;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\ORM\Exception\ORMException;
use LogicException;

use function get_class;
use function sprintf;

final class CannotGenerateIds extends ORMException
{
public static function withPlatform(AbstractPlatform $platform): self
{
return new self(sprintf(
'Platform %s does not support generating identifiers',
get_class($platform)
));
}
}

This file was deleted.

10 changes: 0 additions & 10 deletions lib/Doctrine/ORM/Mapping/MappingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -383,16 +383,6 @@ public static function propertyTypeIsRequired($className, $propertyName)
));
}

/**
* @param string $className
*
* @return MappingException
*/
public static function tableIdGeneratorNotImplemented($className)
{
return new self(sprintf('TableIdGenerator is not yet implemented for use with class %s', $className));
}

/**
* @param string $entity The entity's name.
* @param string $fieldName The name of the field that was already declared.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ private function displayEntity(
$this->formatField('Composite identifier?', $metadata->isIdentifierComposite),
$this->formatField('Foreign identifier?', $metadata->containsForeignIdentifier),
$this->formatField('Sequence generator definition', $metadata->sequenceGeneratorDefinition),
$this->formatField('Table generator definition', $metadata->tableGeneratorDefinition),
$this->formatField('Change tracking policy', $metadata->changeTrackingPolicy),
$this->formatField('Versioned?', $metadata->isVersioned),
$this->formatField('Version field', $metadata->versionField),
Expand Down
1 change: 0 additions & 1 deletion lib/Doctrine/ORM/Tools/EntityGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ class EntityGenerator
protected static $generatorStrategyMap = [
ClassMetadataInfo::GENERATOR_TYPE_AUTO => 'AUTO',
ClassMetadataInfo::GENERATOR_TYPE_SEQUENCE => 'SEQUENCE',
ClassMetadataInfo::GENERATOR_TYPE_TABLE => 'TABLE',
ClassMetadataInfo::GENERATOR_TYPE_IDENTITY => 'IDENTITY',
ClassMetadataInfo::GENERATOR_TYPE_NONE => 'NONE',
ClassMetadataInfo::GENERATOR_TYPE_UUID => 'UUID',
Expand Down
3 changes: 0 additions & 3 deletions lib/Doctrine/ORM/Tools/Export/Driver/AbstractExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,6 @@ protected function _getIdGeneratorTypeString($type)
case ClassMetadataInfo::GENERATOR_TYPE_SEQUENCE:
return 'SEQUENCE';

case ClassMetadataInfo::GENERATOR_TYPE_TABLE:
return 'TABLE';

case ClassMetadataInfo::GENERATOR_TYPE_IDENTITY:
return 'IDENTITY';

Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -520,11 +520,6 @@ parameters:
count: 1
path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

-
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

-
message: "#^Array \\(array\\('name' \\=\\> string, 'schema' \\=\\> string, 'indexes' \\=\\> array, 'uniqueConstraints' \\=\\> array\\)\\) does not accept key 'options'\\.$#"
count: 1
Expand Down
20 changes: 20 additions & 0 deletions tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
use Doctrine\ORM\Mapping\DiscriminatorColumn;
use Doctrine\ORM\Mapping\DiscriminatorMap;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\Exception\CannotGenerateIds;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
use Doctrine\ORM\Mapping\InheritanceType;
use Doctrine\ORM\Mapping\MappingException;
use Doctrine\Persistence\Mapping\Driver\MappingDriver;
use Doctrine\Persistence\Mapping\RuntimeReflectionService;
use Doctrine\Tests\Mocks\ConnectionMock;
use Doctrine\Tests\Mocks\DatabasePlatformMock;
use Doctrine\Tests\Mocks\DriverMock;
use Doctrine\Tests\Mocks\EntityManagerMock;
use Doctrine\Tests\Mocks\MetadataDriverMock;
Expand Down Expand Up @@ -85,6 +87,24 @@ public function testGetMetadataForSingleClass(): void
self::assertTrue($cmMap1->hasField('name'));
}

public function testItThrowsWhenUsingAutoWithIncompatiblePlatform(): void
{
$cm1 = $this->createValidClassMetadata();
$cm1->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_AUTO);
$entityManager = $this->createEntityManager(new MetadataDriverMock());
$connection = $entityManager->getConnection();
assert($connection instanceof ConnectionMock);
$platform = $connection->getDatabasePlatform();
assert($platform instanceof DatabasePlatformMock);
$platform->setSupportsIdentityColumns(false);
$cmf = new ClassMetadataFactoryTestSubject();
$cmf->setEntityManager($entityManager);
$cmf->setMetadataForClass($cm1->name, $cm1);
$this->expectException(CannotGenerateIds::class);

$actual = $cmf->getMetadataFor($cm1->name);
}

public function testGetMetadataForReturnsLoadedCustomIdGenerator(): void
{
$cm1 = $this->createValidClassMetadata();
Expand Down
4 changes: 4 additions & 0 deletions tests/Doctrine/Tests/ORM/Tools/EntityGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,10 @@ public function testGetIdGeneratorTypeString(): void
continue;
}

if ($name === 'GENERATOR_TYPE_TABLE') {
continue;
}

$expected = preg_replace($pattern, '', $name);
$actual = $method->invoke($this->generator, $value);

Expand Down