From 54b3c0548d952f2966e8197986002e01b3b32f96 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Perrin?= <mperrin@greenflex.com>
Date: Tue, 17 Dec 2013 18:56:13 +0100
Subject: [PATCH] Fix persistence exception on a table with a schema on a
platform without schema support
---
.../ORM/Mapping/ClassMetadataFactory.php | 14 +-
.../ORM/Mapping/ClassMetadataInfo.php | 58 +++++++
.../ORM/Mapping/DefaultQuoteStrategy.php | 18 +-
.../ORM/Mapping/Driver/AnnotationDriver.php | 13 +-
lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 24 ++-
.../ORM/Mapping/Driver/YamlDriver.php | 23 ++-
.../ORM/Functional/Ticket/DDC2825Test.php | 157 ++++++++++++++++++
7 files changed, 284 insertions(+), 23 deletions(-)
create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2825Test.php
diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
index dd65c3932..976847f33 100644
--- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
+++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
@@ -576,9 +576,11 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory
// Platforms that do not have native IDENTITY support need a sequence to emulate this behaviour.
if ($this->targetPlatform->usesSequenceEmulatedIdentityColumns()) {
- $columnName = $class->getSingleIdentifierColumnName();
- $quoted = isset($class->fieldMappings[$fieldName]['quoted']) || isset($class->table['quoted']);
- $sequenceName = $this->targetPlatform->getIdentitySequenceName($class->getTableName(), $columnName);
+ $columnName = $class->getSingleIdentifierColumnName();
+ $quoted = isset($class->fieldMappings[$fieldName]['quoted']) || isset($class->table['quoted']);
+ $sequencePrefix = $class->getSequencePrefix($this->targetPlatform);
+
+ $sequenceName = $this->targetPlatform->getIdentitySequenceName($sequencePrefix, $columnName);
$definition = array(
'sequenceName' => $this->targetPlatform->fixSchemaElementName($sequenceName)
);
@@ -608,10 +610,10 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory
if ( ! $definition) {
$fieldName = $class->getSingleIdentifierFieldName();
- $columnName = $class->getSingleIdentifierColumnName();
+ $sequenceName = $class->getSequenceName($this->targetPlatform);
$quoted = isset($class->fieldMappings[$fieldName]['quoted']) || isset($class->table['quoted']);
- $sequenceName = $class->getTableName() . '_' . $columnName . '_seq';
- $definition = array(
+
+ $definition = array(
'sequenceName' => $this->targetPlatform->fixSchemaElementName($sequenceName),
'allocationSize' => 1,
'initialValue' => 1,
diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
index 76dd8f1e6..dfaa40111 100644
--- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
+++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
@@ -24,6 +24,7 @@ use Doctrine\Instantiator\Instantiator;
use InvalidArgumentException;
use RuntimeException;
use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
use ReflectionClass;
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
use Doctrine\Common\ClassLoader;
@@ -2014,6 +2015,16 @@ class ClassMetadataInfo implements ClassMetadata
return $this->table['name'];
}
+ /**
+ * Gets primary table's schema name.
+ *
+ * @return string|null
+ */
+ public function getSchemaName()
+ {
+ return isset($this->table['schema']) ? $this->table['schema'] : null;
+ }
+
/**
* Gets the table name to use for temporary identifier tables of this class.
*
@@ -2243,6 +2254,10 @@ class ClassMetadataInfo implements ClassMetadata
$this->table['name'] = $table['name'];
}
+ if (isset($table['schema'])) {
+ $this->table['schema'] = $table['schema'];
+ }
+
if (isset($table['indexes'])) {
$this->table['indexes'] = $table['indexes'];
}
@@ -3240,4 +3255,47 @@ class ClassMetadataInfo implements ClassMetadata
throw MappingException::duplicateFieldMapping($this->name, $fieldName);
}
}
+
+ /**
+ * Gets the sequence name based on class metadata.
+ *
+ * @param AbstractPlatform $platform
+ * @return string
+ *
+ * @todo Sequence names should be computed in DBAL depending on the platform
+ */
+ public function getSequenceName(AbstractPlatform $platform)
+ {
+ $sequencePrefix = $this->getSequencePrefix($platform);
+
+ $columnName = $this->getSingleIdentifierColumnName();
+ $sequenceName = $sequencePrefix . '_' . $columnName . '_seq';
+
+ return $sequenceName;
+ }
+
+ /**
+ * Gets the sequence name prefix based on class metadata.
+ *
+ * @param AbstractPlatform $platform
+ * @return string
+ *
+ * @todo Sequence names should be computed in DBAL depending on the platform
+ */
+ public function getSequencePrefix(AbstractPlatform $platform)
+ {
+ $tableName = $this->getTableName();
+ $sequencePrefix = $tableName;
+
+ // Prepend the schema name to the table name if there is one
+ if ($schemaName = $this->getSchemaName()) {
+ $sequencePrefix = $schemaName . '.' . $tableName;
+
+ if ( ! $platform->supportsSchemas() && $platform->canEmulateSchemas()) {
+ $sequencePrefix = $schemaName . '__' . $tableName;
+ }
+ }
+
+ return $sequencePrefix;
+ }
}
diff --git a/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php b/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php
index eb2d22332..eb5b85065 100644
--- a/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php
+++ b/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php
@@ -41,12 +41,24 @@ class DefaultQuoteStrategy implements QuoteStrategy
/**
* {@inheritdoc}
+ *
+ * @todo Table names should be computed in DBAL depending on the platform
*/
public function getTableName(ClassMetadata $class, AbstractPlatform $platform)
{
- return isset($class->table['quoted'])
- ? $platform->quoteIdentifier($class->table['name'])
- : $class->table['name'];
+ $tableName = $class->table['name'];
+
+ if ( ! empty($class->table['schema'])) {
+ $tableName = $class->table['schema'] . '.' . $class->table['name'];
+
+ if ( ! $platform->supportsSchemas() && $platform->canEmulateSchemas()) {
+ $tableName = $class->table['schema'] . '__' . $class->table['name'];
+ }
+ }
+
+ return isset($class->table['quoted'])
+ ? $platform->quoteIdentifier($tableName)
+ : $tableName;
}
/**
diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
index 85f40b2f5..774e45651 100644
--- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
+++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
@@ -94,9 +94,18 @@ class AnnotationDriver extends AbstractAnnotationDriver
// Evaluate Table annotation
if (isset($classAnnotations['Doctrine\ORM\Mapping\Table'])) {
$tableAnnot = $classAnnotations['Doctrine\ORM\Mapping\Table'];
+
+ $tableName = $tableAnnot->name;
+ $schemaName = $tableAnnot->schema;
+
+ // Split schema and table name from a table name like "myschema.mytable"
+ if (strpos($tableName, '.') !== false) {
+ list($schemaName, $tableName) = explode('.', $tableName);
+ }
+
$primaryTable = array(
- 'name' => $tableAnnot->name,
- 'schema' => $tableAnnot->schema
+ 'name' => $tableName,
+ 'schema' => $schemaName
);
if ($tableAnnot->indexes !== null) {
diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
index 2797fb126..658d9e53e 100644
--- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
+++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
@@ -77,8 +77,25 @@ class XmlDriver extends FileDriver
// Evaluate <entity...> attributes
$table = array();
+
+ $tableName = null;
+ $schemaName = null;
+
if (isset($xmlRoot['table'])) {
- $table['name'] = (string)$xmlRoot['table'];
+ $tableName = (string)$xmlRoot['table'];
+
+ // Split schema and table name from a table name like "myschema.mytable"
+ if (strpos($tableName, '.') !== false) {
+ list($schemaName, $tableName) = explode('.', $tableName);
+ }
+ }
+
+ if (isset($xmlRoot['schema'])) {
+ $schemaName = (string)$xmlRoot['schema'];
+ }
+
+ if (null !== $tableName) {
+ $table['name'] = $tableName;
}
$metadata->setPrimaryTable($table);
@@ -150,11 +167,6 @@ class XmlDriver extends FileDriver
}
}
- /* not implemented specially anyway. use table = schema.table
- if (isset($xmlRoot['schema'])) {
- $metadata->table['schema'] = (string)$xmlRoot['schema'];
- }*/
-
if (isset($xmlRoot['inheritance-type'])) {
$inheritanceType = (string)$xmlRoot['inheritance-type'];
$metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . $inheritanceType));
diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
index cf1dc5bc0..be1cd2ed0 100644
--- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
+++ b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
@@ -75,8 +75,24 @@ class YamlDriver extends FileDriver
// Evaluate root level properties
$table = array();
+ $tableName = null;
+ $schemaName = null;
+
if (isset($element['table'])) {
- $table['name'] = $element['table'];
+ $tableName = $element['table'];
+
+ // Split schema and table name from a table name like "myschema.mytable"
+ if (strpos($tableName, '.') !== false) {
+ list($schemaName, $tableName) = explode('.', $tableName);
+ }
+ }
+
+ if (isset($element['schema'])) {
+ $schemaName = $element['schema'];
+ }
+
+ if (null !== $tableName) {
+ $table['name'] = $tableName;
}
// Evaluate second level cache
@@ -163,11 +179,6 @@ class YamlDriver extends FileDriver
}
}
- /* not implemented specially anyway. use table = schema.table
- if (isset($element['schema'])) {
- $metadata->table['schema'] = $element['schema'];
- }*/
-
if (isset($element['inheritanceType'])) {
$metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . strtoupper($element['inheritanceType'])));
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2825Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2825Test.php
new file mode 100644
index 000000000..93628a5a4
--- /dev/null
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2825Test.php
@@ -0,0 +1,157 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\ORM\Mapping\ClassMetadataFactory;
+
+/**
+ * This class makes tests on the correct use of a database schema when entities are stored
+ *
+ * @group DDC-2825
+ */
+class DDC2825Test extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+ /**
+ * {@inheritDoc}
+ */
+ protected function setup()
+ {
+ parent::setup();
+
+ $platform = $this->_em->getConnection()->getDatabasePlatform();
+
+ if ( ! $platform->supportsSchemas() && ! $platform->canEmulateSchemas()) {
+ $this->markTestSkipped("This test is only useful for databases that support schemas or can emulate them.");
+ }
+
+ $this->_schemaTool->createSchema(array(
+ $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2825MySchemaMyTable'),
+ $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2825MySchemaMyTable2'),
+ $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2825MySchemaOrder'),
+ ));
+ }
+
+ public function testIssue()
+ {
+ // Test with a table with a schema
+ $myEntity = new DDC2825MySchemaMyTable();
+
+ $this->_em->persist($myEntity);
+ $this->_em->flush();
+ $this->_em->clear();
+
+ $entities = $this->_em->createQuery('SELECT mt FROM ' . __NAMESPACE__ . '\\DDC2825MySchemaMyTable mt')->execute();
+ $this->assertEquals(count($entities), 1);
+
+ $classMetadata = $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2825MySchemaMyTable');
+ $this->checkClassMetadata($classMetadata, 'myschema', 'mytable');
+
+ // Test with schema defined directly as a table annotation property
+ $myEntity2 = new DDC2825MySchemaMyTable2();
+
+ $this->_em->persist($myEntity2);
+ $this->_em->flush();
+ $this->_em->clear();
+
+ $entities = $this->_em->createQuery('SELECT mt2 FROM ' . __NAMESPACE__ . '\\DDC2825MySchemaMyTable2 mt2')->execute();
+ $this->assertEquals(count($entities), 1);
+
+ $classMetadata = $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2825MySchemaMyTable2');
+ $this->checkClassMetadata($classMetadata, 'myschema', 'mytable2');
+
+ // Test with a table named "order" (which is a reserved keyword) to make sure the table name is not
+ // incorrectly escaped when a schema is used and that the platform doesn't support schemas
+ $order = new DDC2825MySchemaOrder();
+
+ $this->_em->persist($order);
+ $this->_em->flush();
+ $this->_em->clear();
+
+ $classMetadata = $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC2825MySchemaOrder');
+ $this->checkClassMetadata($classMetadata, 'myschema', 'order');
+
+ $entities = $this->_em->createQuery('SELECT mso FROM ' . __NAMESPACE__ . '\\DDC2825MySchemaOrder mso')->execute();
+ $this->assertEquals(count($entities), 1);
+ }
+
+ /**
+ * Checks that class metadata is correctly stored when a database schema is used and
+ * checks that the table name is correctly converted whether the platform supports database
+ * schemas or not
+ *
+ * @param ClassMetadata $classMetadata Class metadata
+ * @param string $expectedSchemaName Expected schema name
+ * @param string $expectedTableName Expected table name
+ */
+ protected function checkClassMetadata($classMetadata, $expectedSchemaName, $expectedTableName)
+ {
+ $quoteStrategy = $this->_em->getConfiguration()->getQuoteStrategy();
+ $platform = $this->_em->getConnection()->getDatabasePlatform();
+ $quotedTableName = $quoteStrategy->getTableName($classMetadata, $platform);
+
+ // Check if table name and schema properties are defined in the class metadata
+ $this->assertEquals($classMetadata->table['name'], $expectedTableName);
+ $this->assertEquals($classMetadata->table['schema'], $expectedSchemaName);
+
+ if ($this->_em->getConnection()->getDatabasePlatform()->supportsSchemas()) {
+ $fullTableName = sprintf('%s.%s', $expectedSchemaName, $expectedTableName);
+ } else {
+ $fullTableName = sprintf('%s__%s', $expectedSchemaName, $expectedTableName);
+ }
+
+ $this->assertEquals($quotedTableName, $fullTableName);
+
+ // Checks sequence name validity
+ $expectedSchemaName = $fullTableName . '_' . $classMetadata->getSingleIdentifierColumnName() . '_seq';
+ $this->assertEquals($expectedSchemaName, $classMetadata->getSequenceName($platform));
+ }
+}
+
+/**
+ * @Entity
+ * @Table(name="myschema.mytable")
+ */
+class DDC2825MySchemaMyTable
+{
+ /**
+ * Test with a quoted column name to check that sequence names are
+ * correctly handled
+ *
+ * @Id @GeneratedValue
+ * @Column(name="`number`", type="integer")
+ *
+ * @var integer
+ */
+ public $id;
+}
+
+/**
+ * @Entity
+ * @Table(name="mytable2",schema="myschema")
+ */
+class DDC2825MySchemaMyTable2
+{
+ /**
+ * @Id @GeneratedValue
+ * @Column(type="integer")
+ *
+ * @var integer
+ */
+ public $id;
+}
+
+
+/**
+ * @Entity
+ * @Table(name="myschema.order")
+ */
+class DDC2825MySchemaOrder
+{
+ /**
+ * @Id @GeneratedValue
+ * @Column(type="integer")
+ *
+ * @var integer
+ */
+ public $id;
+}