Skip to content

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
...
Checking mergeability… Don’t worry, you can still create the pull request.
  • 4 commits
  • 14 files changed
  • 0 commit comments
  • 1 contributor
Commits on Jan 18, 2012
@apinstein apinstein [gh-149] Add a "normalize" command to normalize the structure of the …
…schema.xml file.

This optional feature helps minimize the changes to the schema XML file
(and thus the generated models) due to non-deterministic xml created via
external tools like "reverse" or the ERD->XML tools.
0e3caf9
Commits on Mar 03, 2012
@apinstein apinstein [gh-152] Revert change that was made to postgres schema parsing that
causes regressions and has no tests. Add tests for my case. I do not
understand the case that the person who introduced the bug was trying to
solve.
365ba2f
@apinstein apinstein [gh-118] Update full query logging support to log actual SQL in case of
error in DebugPDOStatement::execute, PropelPDO::exec, and
PropelPDO::query.

- Add PropelPDOException to allow propel to customize PDOExceptions
  without causing BC problems for code that expects PDOException.
- Include full SQL in PropelPDOException so you can view the full SQL
  which caused the error without having to look in the log file.
573b124
@apinstein apinstein [gh-267] Fix bug caused by failure to cast generated primary key data…
… under php 5.3.

- This primarily affects postgres.
- Specifically this bug was exposed for native autoincrement where
  sequences are used.
d6bd6aa
View
29 generator/build-propel.xml
@@ -53,6 +53,9 @@
<taskdef
name="propel-schema-reverse"
classname="task.PropelSchemaReverseTask" classpathRef="propelclasses"/>
+ <taskdef
+ name="propel-schema-normalize"
+ classname="task.PropelSchemaNormalizeTask" classpathRef="propelclasses"/>
<taskdef
name="propel-sql"
classname="task.PropelSQLTask" classpathRef="propelclasses"/>
@@ -288,6 +291,29 @@
addVendorInfo="${propel.addVendorInfo}"
addValidators="${propel.addValidators}"
/>
+ <propel-schema-normalize
+ schemaFile="${propel.schema.dir}/${propel.default.schema.basename}.xml"
+ />
+
+ </target>
+
+ <!-- ================================================================ -->
+ <!-- N O R M A L I Z E S C H E M A -->
+ <!-- ================================================================ -->
+
+ <target
+ name="normalize"
+ description="==> normalize xml file to consistent sort ordering">
+
+ <echo message="+-----------------------------------------------+"/>
+ <echo message="| |"/>
+ <echo message="| Normalizing XML Schema ! |"/>
+ <echo message="| |"/>
+ <echo message="+-----------------------------------------------+"/>
+
+ <propel-schema-normalize
+ schemaFile="${propel.schema.dir}/${propel.default.schema.basename}.xml"
+ />
</target>
@@ -592,6 +618,9 @@
</fileset>
<mapper type="regexp" from="^(.*)\.xml$" to="\1.schema.xml"/>
</xslt>
+ <propel-schema-normalize
+ schemaFile="${propel.schema.dir}/${propel.propel.dbd2propel.xsl.file}"
+ />
</target>
<target
View
6 generator/build.properties-sample
@@ -170,12 +170,16 @@ propel.packageObjectModel = false
# You can cherry-pick allowed validators by using a comma-separated value, e.g
# maxvalue,type,required
#
+# normalizeXmlOrder
+# If true, the reverse task will sort all entities in alphabetical order
+# by name. This ensures the same XML output regardless of the order of
+# changes that were applied to the database over time.
# -------------------------------------------------------------------
# propel.samePhpName = false
# propel.addVendorInfo=true
# propel.addValidators=none
-
+# propel.normalizeXmlOrder=false
# -------------------------------------------------------------------
#
View
5 generator/build.xml
@@ -89,6 +89,7 @@
<echo message=" up"/>
<echo message=" down"/>
<echo message=" reverse"/>
+ <echo message=" normalize"/>
<echo message=" ... and a few others."/>
<echo message="But you should start by creating a schema."/>
<echo message="=========================================================="/>
@@ -164,6 +165,10 @@
<phing phingfile="build-propel.xml" target="reverse"/>
</target>
+<target name="normalize" depends="configure">
+ <phing phingfile="build-propel.xml" target="normalize"/>
+</target>
+
<target name="datadump" depends="configure">
<phing phingfile="build-propel.xml" target="datadump"/>
</target>
View
1 generator/default.properties
@@ -75,6 +75,7 @@ propel.database.password =
propel.samePhpName = false
propel.addVendorInfo = false
propel.addValidators = none
+propel.normalizeXmlOrder = false
# -------------------------------------------------------------------
#
View
4 generator/lib/builder/om/PHP5ObjectBuilder.php
@@ -4403,7 +4403,9 @@ protected function addDoInsertBodyRaw()
$script .= "
if (null === \$this->{$columnProperty}) {
try {";
- $script .= $platform->getIdentifierPhp('$this->'. $columnProperty, '$con', $primaryKeyMethodInfo, ' ');
+ $script .= $platform->getIdentifierPhp('$newId', '$con', $primaryKeyMethodInfo, ' ');
+ $script .= "
+ \$this->set{$column->getPhpName()}(\$newId);";
$script .= "
} catch (Exception \$e) {
throw new PropelException('Unable to get sequence id.', \$e);
View
4 generator/lib/reverse/pgsql/PgsqlSchemaParser.php
@@ -225,8 +225,8 @@ protected function addColumns(Table $table, $oid, $version)
// if column has a default
if (($boolHasDefault == 't') && (strlen (trim ($default)) > 0)) {
if (!preg_match('/^nextval\(/', $default)) {
- $strDefault= preg_replace('/::[\W\D]*/', '', $default);
- $default = preg_replace('/(\'?)\'/', '${1}', $strDefault);
+ $strDefault= preg_replace ('/::[\W\D]*/', '', $default);
+ $default = str_replace ("'", '', $strDefault);
} else {
$autoincrement = true;
$default = null;
View
161 generator/lib/task/PropelSchemaNormalizeTask.php
@@ -0,0 +1,161 @@
+<?php
+
+/**
+ * This file is part of the Propel package.
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @license MIT License
+ */
+
+require_once 'phing/tasks/ext/pdo/PDOTask.php';
+require_once 'config/GeneratorConfig.php';
+require_once 'model/PropelTypes.php';
+
+/**
+ * This class generates an XML schema of an existing database from
+ * the database metadata.
+ *
+ * @author Hans Lellelid <hans@xmpl.org>
+ * @version $Revision$
+ * @package propel.generator.task
+ */
+class PropelSchemaNormalizeTask extends PDOTask
+{
+ protected $xmlSchema;
+
+ private $sortPlan = array(
+ // entity => attrForSort
+ 'behavior' => 'name',
+ 'column' => 'name',
+ 'database' => 'name',
+ 'foreign-key' => 'name',
+ 'id-method-parameter' => 'name',
+ 'index' => 'name',
+ 'index-column' => 'name',
+ 'reference' => array('local', 'foreign'),
+ 'table' => 'name',
+ 'unique' => 'name',
+ 'unique-column' => 'name',
+ 'validator' => 'columnname',
+ 'vendor-info' => 'type',
+ );
+
+ /**
+ * Sets the output name for the XML file.
+ *
+ * @param PhingFile $v
+ */
+ public function setSchemaFile(PhingFile $v)
+ {
+ $this->xmlSchema = $v;
+ }
+
+ function xmlRecurse($node, $currentPath = '') {
+ if ($node->hasChildNodes()) {
+ foreach ($node->childNodes as $childNode) {
+ if ($childNode instanceof DomElement)
+ {
+ $this->xmlRecurse($childNode, "{$currentPath}/{$node->tagName}");
+ }
+ }
+
+ $this->sortXmlByEntityAndPrimaryAttribute($node);
+ }
+ }
+
+ public function sortXmlByEntityAndPrimaryAttribute($domNode)
+ {
+ $nodesToSort = array();
+ // prune all children while building a list of nodes to sort
+ $nodeIndex = 0;
+ while ($domNode->hasChildNodes()) {
+ $possiblySortableDomNode = $domNode->removeChild($domNode->childNodes->item(0));
+
+ if ($possiblySortableDomNode instanceof DomText && $possiblySortableDomNode->isElementContentWhitespace()) continue;
+
+ // figure out sortKey for element
+ if ($possiblySortableDomNode instanceof DomElement)
+ {
+ if (!isset($this->sortPlan[$possiblySortableDomNode->tagName]))
+ {
+ $this->log("WARNING: no sort plan on file for entity {$possiblySortableDomNode->tagName}");
+ }
+
+ $sortKeys = array();
+ $sortKeys[] = $possiblySortableDomNode->tagName;
+
+ if (isset($this->sortPlan[$possiblySortableDomNode->tagName]))
+ {
+ $moreSortKeys = is_array($this->sortPlan[$possiblySortableDomNode->tagName]) ?
+ $this->sortPlan[$possiblySortableDomNode->tagName]
+ :
+ array($this->sortPlan[$possiblySortableDomNode->tagName])
+ ;
+ foreach ($moreSortKeys as $additionalSortKey) {
+ $sortKeys[] = $possiblySortableDomNode->getAttribute($additionalSortKey);
+ }
+ }
+ else
+ {
+ $sortKeys[] = $nodeIndex;
+ }
+
+ $sortKey = join('/', $sortKeys);
+ }
+ else
+ {
+ $sortKey = "0_{$nodeIndex}";
+ }
+
+ if (isset($nodesToSort[$sortKey])) throw new BuildException("ERROR: key collision for {$domNode->tagName} / {$sortKey}");
+ $nodesToSort[$sortKey] = $possiblySortableDomNode;
+
+ $nodeIndex++;
+ }
+
+ // ksort the array
+ ksort($nodesToSort);
+
+ // add back all children to domNode
+ foreach ($nodesToSort as $k => $node) {
+ $domNode->appendChild($node);
+ }
+ }
+
+ /**
+ * @throws BuildException
+ */
+ public function main()
+ {
+ $props = $this->getProject()->getProperties();
+ if (!isset($props['propel.normalizeXmlOrder']) or $props['propel.normalizeXmlOrder'] === false)
+ {
+ $this->log("Schema Normalization disabled.",Project::MSG_VERBOSE);
+ return;
+ }
+
+ try {
+ // load
+ $xml = new DOMDocument();
+ $xml->load($this->xmlSchema);
+ $xml->normalizeDocument();
+ if ($xml === false) throw new BuildException("Failed to open schema XML: {$this->xmlSchema}");
+
+ $this->xmlRecurse($xml);
+
+ // save
+ $this->log("Writing XML to file: " . $this->xmlSchema->getPath());
+ $xml->formatOutput = true; // pretty printing
+ $out = new FileWriter($this->xmlSchema);
+ $xmlstr = $xml->saveXML();
+ $out->write($xmlstr);
+ $out->close();
+ } catch (Exception $e) {
+ $this->log("There was an error building XML from metadata: " . $e->getMessage(), Project::MSG_ERR);
+ return false;
+ }
+
+ $this->log("Schema normalization finished.");
+ }
+}
View
6 generator/pear/pear-build.xml
@@ -94,6 +94,10 @@
<phing phingfile="build-propel.xml" target="reverse"/>
</target>
+<target name="normalize" depends="configure">
+ <phing phingfile="build-propel.xml" target="normalize"/>
+</target>
+
<target name="datadtd" depends="configure">
<phing phingfile="build-propel.xml" target="datadtd"/>
</target>
@@ -164,4 +168,4 @@
<phing phingfile="build-propel.xml" target="graphviz"/>
</target>
-</project>
+</project>
View
1 runtime/lib/Propel.php
@@ -190,6 +190,7 @@ class Propel
'DebugPDOStatement' => 'connection/DebugPDOStatement.php',
'PropelException' => 'exception/PropelException.php',
+ 'PropelPDOException' => 'exception/PropelPDOException.php',
'ModelWith' => 'formatter/ModelWith.php',
'PropelArrayFormatter' => 'formatter/PropelArrayFormatter.php',
View
13 runtime/lib/connection/DebugPDOStatement.php
@@ -87,13 +87,24 @@ public function getExecutedQueryString()
public function execute($input_parameters = null)
{
$debug = $this->pdo->getDebugSnapshot();
- $return = parent::execute($input_parameters);
+
+ $exception = null;
+ try {
+ $return = parent::execute($input_parameters);
+ } catch (Exception $e) {
+ $exception = $e;
+ }
$sql = $this->getExecutedQueryString();
$this->pdo->log($sql, null, __METHOD__, $debug);
$this->pdo->setLastExecutedQuery($sql);
$this->pdo->incrementQueryCount();
+ if ($exception)
+ {
+ throw new PropelPDOException("Unable to execute SQL:\n{$sql}.", $exception);
+ }
+
return $return;
}
View
29 runtime/lib/connection/PropelPDO.php
@@ -400,13 +400,22 @@ public function exec($sql)
$debug = $this->getDebugSnapshot();
}
- $return = parent::exec($sql);
+ $exception = null;
+ try {
+ $return = parent::exec($sql);
+ } catch (PDOException $e) {
+ $exception = $e;
+ }
if ($this->useDebug) {
$this->log($sql, null, __METHOD__, $debug);
$this->setLastExecutedQuery($sql);
$this->incrementQueryCount();
}
+ if ($exception)
+ {
+ throw new PropelPDOException("Unable to execute SQL:\n{$sql}.", $exception);
+ }
return $return;
}
@@ -428,10 +437,16 @@ public function query()
}
$args = func_get_args();
- if (version_compare(PHP_VERSION, '5.3', '<')) {
- $return = call_user_func_array(array($this, 'parent::query'), $args);
- } else {
- $return = call_user_func_array('parent::query', $args);
+
+ $exception = null;
+ try {
+ if (version_compare(PHP_VERSION, '5.3', '<')) {
+ $return = call_user_func_array(array($this, 'parent::query'), $args);
+ } else {
+ $return = call_user_func_array('parent::query', $args);
+ }
+ } catch (PDOException $e) {
+ $exception = $e;
}
if ($this->useDebug) {
@@ -440,6 +455,10 @@ public function query()
$this->setLastExecutedQuery($sql);
$this->incrementQueryCount();
}
+ if ($exception)
+ {
+ throw new PropelPDOException("Unable to execute SQL:\n{$sql}.", $exception);
+ }
return $return;
}
View
68 runtime/lib/exception/PropelPDOException.php
@@ -0,0 +1,68 @@
+<?php
+
+/**
+ * This file is part of the Propel package.
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @license MIT License
+ */
+
+/**
+ * A Propel subclass of the PDOException so we can customize PDOExceptions without breaking exisitng code that looks for PDOException.
+ * @author Alan Pinstein <apinstein@mac.com>
+ * @version $Revision$
+ * @package propel.runtime.exception
+ */
+class PropelPDOException extends PDOException
+{
+ /**
+ * The nested "cause" exception.
+ *
+ * @var Exception
+ */
+ protected $cause;
+
+ /**
+ * Emulates wrapped exceptions for PHP < 5.3
+ *
+ * @param string $message
+ * @param Exception $previous
+ *
+ * @return PropelException
+ */
+ public function __construct($message = null, Exception $previous = null)
+ {
+ if ($previous === null && $message instanceof Exception) {
+ $previous = $message;
+ $message = "";
+ }
+
+ if ($previous !== null) {
+ $message .= " [wrapped: " . $previous->getMessage() ."]";
+ if (version_compare(PHP_VERSION, '5.3.0') >= 0) {
+ parent::__construct($message, 0, $previous);
+ } else {
+ parent::__construct($message);
+ $this->cause = $previous;
+ }
+ } else {
+ parent::__construct($message);
+ }
+ }
+
+ /**
+ * Get the previous Exception
+ * We can't override getPrevious() since it's final
+ *
+ * @return Exception The previous exception
+ */
+ public function getCause()
+ {
+ if (version_compare(PHP_VERSION, '5.3.0') >= 0) {
+ return $this->getPrevious();
+ } else {
+ return $this->cause;
+ }
+ }
+}
View
37 test/fixtures/reverse/pgsql/runtime-conf.xml
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="ISO-8859-1"?>
+<config>
+ <!--
+ #
+ # P R O P E L P R O P E R T I E S
+ #
+ # Note that you can configure multiple datasources; for example if your
+ # project uses several databases.
+ -->
+ <propel>
+ <datasources default="reverse-bookstore">
+
+ <datasource id="reverse-bookstore">
+ <adapter>pgsql</adapter>
+ <connection>
+ <classname>DebugPDO</classname>
+ <dsn>pgsql:dbname=reverse_bookstore;user=postgres</dsn>
+ <!--
+ For MySQL and Oracle you must specify username + password separate from DSN:
+ <user>bookstore</user>
+ <password></password>
+ -->
+ <options>
+ <option id="ATTR_PERSISTENT">false</option>
+ </options>
+ <attributes>
+ <option id="ATTR_EMULATE_PREPARES">true</option>
+ </attributes>
+ <settings>
+ <setting id="charset">utf8</setting>
+ </settings>
+ </connection>
+ </datasource>
+
+ </datasources>
+ </propel>
+</config>
View
98 test/testsuite/generator/reverse/pgsql/PgsqlSchemaParserTest.php
@@ -0,0 +1,98 @@
+<?php
+
+/**
+ * This file is part of the Propel package.
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @license MIT License
+ */
+
+require_once dirname(__FILE__) . '/../../../../../runtime/lib/Propel.php';
+
+require_once dirname(__FILE__) . '/../../../../../generator/lib/reverse/pgsql/PgsqlSchemaParser.php';
+require_once dirname(__FILE__) . '/../../../../../generator/lib/config/QuickGeneratorConfig.php';
+require_once dirname(__FILE__) . '/../../../../../generator/lib/model/PropelTypes.php';
+require_once dirname(__FILE__) . '/../../../../../generator/lib/model/Database.php';
+require_once dirname(__FILE__) . '/../../../../../generator/lib/platform/DefaultPlatform.php';
+
+set_include_path(get_include_path().PATH_SEPARATOR.dirname(__FILE__).'/../../../../../generator/lib');
+require_once dirname(__FILE__) . '/../../../../../generator/lib/task/PropelConvertConfTask.php';
+
+/**
+ * Tests for Pgsql database schema parser.
+ *
+ * @author Alan Pinstein
+ * @version $Revision$
+ * @package propel.generator.reverse.pgsql
+ */
+class PgsqlSchemaParserTest extends PHPUnit_Framework_TestCase
+{
+ protected function setUp()
+ {
+ parent::setUp();
+
+ $xmlDom = new DOMDocument();
+ $xmlDom->load(dirname(__FILE__) . '/../../../../fixtures/reverse/pgsql/runtime-conf.xml');
+ $xml = simplexml_load_string($xmlDom->saveXML());
+ $phpconf = OpenedPropelConvertConfTask::simpleXmlToArray($xml);
+
+ Propel::setConfiguration($phpconf);
+ Propel::initialize();
+
+ $this->con = Propel::getConnection('reverse-bookstore');
+ $this->con->beginTransaction();
+ }
+
+ protected function tearDown()
+ {
+ $this->con->rollback();
+
+ parent::tearDown();
+ Propel::init(dirname(__FILE__) . '/../../../../fixtures/bookstore/build/conf/bookstore-conf.php');
+ }
+
+ function parseDataProvider()
+ {
+ return array(
+ // columnDDL expectedColumnPhpName expectedColumnDefaultType expectedColumnDefaultValue
+ array("my_column varchar(20) default null", "MyColumn", ColumnDefaultValue::TYPE_VALUE, "NULL"),
+ array("my_column varchar(20) default ''", "MyColumn", ColumnDefaultValue::TYPE_VALUE, ""),
+ );
+ }
+
+ /**
+ * @dataProvider parseDataProvider
+ */
+ public function testParse($columnDDL, $expectedColumnPhpName, $expectedColumnDefaultType, $expectedColumnDefaultValue)
+ {
+ $this->con->query("create table foo ( {$columnDDL} );");
+ $parser = new PgsqlSchemaParser($this->con);
+ $parser->setGeneratorConfig(new QuickGeneratorConfig());
+
+ $database = new Database();
+ $database->setPlatform(new DefaultPlatform());
+
+ // make sure our DDL insert produced exactly the SQL we inserted
+ $this->assertEquals(1, $parser->parse($database), 'One table and one view defined should return one as we exclude views');
+ $tables = $database->getTables();
+ $this->assertEquals(1, count($tables));
+ $table = $tables[0];
+ $columns = $table->getColumns();
+ $this->assertEquals(1, count($columns));
+
+ // check out our rev-eng column info
+ $defaultValue = $columns[0]->getDefaultValue();
+ $this->assertEquals($expectedColumnPhpName, $columns[0]->getPhpName());
+ $this->assertEquals($expectedColumnDefaultType, $defaultValue->getType());
+ $this->assertEquals($expectedColumnDefaultValue, $defaultValue->getValue());
+ }
+}
+
+class OpenedPropelConvertConfTask extends PropelConvertConfTask
+{
+ public static function simpleXmlToArray($xml)
+ {
+ return parent::simpleXmlToArray($xml);
+ }
+}

No commit comments for this range

Something went wrong with that request. Please try again.