Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix for DBAL-232 #129

Closed
wants to merge 5 commits into from

2 participants

@gedrox

Fixed the issue http://www.doctrine-project.org/jira/browse/DBAL-232.

The problem was that "unknown type" exception was raised after custom type removal from the code if the type has still D2Type column comments inside the database.

The tests are
Build Status.

@gedrox

Test fails on travis with postgresql. Locally everything is OK. I guess the postgresql version could be the reason for this.

@gedrox

Fixed with this issue not quite related test TypeConversionTest. It failed on Travis CI for this PR branch. Fail was because the system date changed from 2012-04-10 to 2012-04-11 while the test was running.

@deeky666
Collaborator

We decided not to do it this way. Please see the discussion in PR #406.

@deeky666 deeky666 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 5, 2012
  1. @gedrox

    Fix for DBAL-232

    gedrox authored
Commits on Apr 10, 2012
  1. @gedrox

    [DBAL-232] testcase added

    gedrox authored
  2. @gedrox
Commits on Apr 11, 2012
  1. @gedrox

    [#DBAL-232] Removed on some drivers failing assertion, because this c…

    gedrox authored
    …lass doesn't test the doctrine type comment feature after all.
  2. @gedrox
This page is out of date. Refresh to see the latest.
View
4 lib/Doctrine/DBAL/Schema/AbstractSchemaManager.php
@@ -884,7 +884,9 @@ public function getSchemaSearchPaths()
public function extractDoctrineTypeFromComment($comment, $currentType)
{
if (preg_match("(\(DC2Type:([a-zA-Z0-9_]+)\))", $comment, $match)) {
- $currentType = $match[1];
+ if (Types\Type::hasType($match[1])) {
+ $currentType = $match[1];
+ }
}
return $currentType;
}
View
95 tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL232Test.php
@@ -0,0 +1,95 @@
+<?php
+
+namespace Doctrine\Tests\DBAL\Functional\Ticket;
+
+use Doctrine\DBAL\Types\Type;
+use Doctrine\DBAL\Schema\Table;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+
+require_once __DIR__ . '/../../../TestInit.php';
+
+/**
+ * Test for [DBAL-232]
+ * @group DBAL-232
+ */
+class DBAL232Test extends \Doctrine\Tests\DbalFunctionalTestCase
+{
+ const TYPE_NAME = 'dbal232';
+
+ const TABLE_NAME = 'dbal232';
+
+ private function checkPlatform()
+ {
+ // Removed because this class doesn't test the doctrine type comment feature after all
+// // Skip sqlite with no type comment feature available
+// if ($this->_conn->getDatabasePlatform()->getName() == 'sqlite') {
+// self::markTestSkipped("Not working on sqlite");
+// }
+ }
+
+ public function setUp()
+ {
+ parent::setUp();
+
+ $this->checkPlatform();
+
+ if ( ! Type::hasType(self::TYPE_NAME)) {
+ Type::addType(self::TYPE_NAME, __NAMESPACE__ . '\DBAL232TestType');
+ $this->_conn->getDatabasePlatform()->markDoctrineTypeCommented(Type::getType(self::TYPE_NAME));
+ }
+
+ $table = new Table(self::TABLE_NAME);
+ $table->addColumn('id', 'integer');
+ $table->addColumn('value', self::TYPE_NAME);
+ $table->setPrimaryKey(array('id'));
+
+ $sm = $this->_conn->getSchemaManager();
+ $sm->createTable($table);
+ }
+
+ protected function tearDown()
+ {
+ $this->checkPlatform();
+
+ if ( ! $this->_conn instanceof \Doctrine\DBAL\Connection) {
+ return;
+ }
+
+ $sm = $this->_conn->getSchemaManager();
+ $sm->dropTable(self::TABLE_NAME);
+ }
+
+ public function testTypeRemoval()
+ {
+ $this->checkPlatform();
+
+ $sm = $this->_conn->getSchemaManager();
+
+ // Removed because this class doesn't test the doctrine type comment feature after all
+// $columns = $sm->listTableColumns(self::TABLE_NAME);
+// $type = $columns['value']->getType();
+// self::assertInstanceOf(__NAMESPACE__ . '\DBAL232TestType', $type);
+
+ // This simulates the type removal
+ Type::overrideType(self::TYPE_NAME, null);
+
+ // Will throw an "unknown type" exception without the fix, string type
+ // with it.
+ $columns = $sm->listTableColumns(self::TABLE_NAME);
+ $type = $columns['value']->getType();
+ self::assertEquals(Type::getType(Type::STRING), $type);
+ }
+}
+
+class DBAL232TestType extends Type
+{
+ public function getName()
+ {
+ return DBAL232Test::TYPE_NAME;
+ }
+
+ public function getSQLDeclaration(array $fieldDeclaration, AbstractPlatform $platform)
+ {
+ return $platform->getVarcharTypeDeclarationSQL($fieldDeclaration);
+ }
+}
View
10 tests/Doctrine/Tests/DBAL/Functional/TypeConversionTest.php
@@ -91,7 +91,15 @@ public function testIdempotentDataConversion($type, $originalValue, $expectedPhp
}
if ($type !== "datetimetz") {
- $this->assertEquals($originalValue, $actualDbValue, "Conversion between values should produce the same out as in value, but doesnt!");
+
+ // Only time part must be validated for "time" type
+ if ($type === "time") {
+ /* @var $actualDbValue \DateTime */
+ /* @var $originalValue \DateTime */
+ $this->assertEquals($originalValue->format('H:i:s'), $actualDbValue->format('H:i:s'), "Conversion between values should produce the same out as in value, but doesnt!");
+ } else {
+ $this->assertEquals($originalValue, $actualDbValue, "Conversion between values should produce the same out as in value, but doesnt!");
+ }
if ($originalValue instanceof \DateTime) {
$this->assertEquals($originalValue->getTimezone()->getName(), $actualDbValue->getTimezone()->getName(), "Timezones should be the same.");
Something went wrong with that request. Please try again.