Merge pull request #6818 from HavokInspiration/schema-reflection-mcfk

Composite foreign key constraints and Schema reflections

src/Database/Schema/BaseSchema.php

@@ -90,6 +90,25 @@ abstract class BaseSchema
     }
 
     /**
+     * Convert foreign key constraints references to a valid
+     * stringified list
+     *
+     * @param string|array $references The referenced columns of a foreign key constraint statement
+     * @return string
+     */
+    protected function _convertConstraintColumns($references)
+    {
+        if (is_string($references)) {
+            return $this->_driver->quoteIdentifier($references);
+        }
+
+        return implode(', ', array_map(
+            [$this->_driver, 'quoteIdentifier'],
+            $references
+        ));
+    }
+
+    /**
      * Generate the SQL to drop a table.
      *
      * @param \Cake\Database\Schema\Table $table Table instance

src/Database/Schema/MysqlSchema.php

@@ -422,7 +422,7 @@ class MysqlSchema extends BaseSchema
                 ' FOREIGN KEY (%s) REFERENCES %s (%s) ON UPDATE %s ON DELETE %s',
                 implode(', ', $columns),
                 $this->_driver->quoteIdentifier($data['references'][0]),
-                $this->_driver->quoteIdentifier($data['references'][1]),
+                $this->_convertConstraintColumns($data['references'][1]),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/PostgresSchema.php

@@ -274,35 +274,38 @@ class PostgresSchema extends BaseSchema
      */
     public function describeForeignKeySql($tableName, $config)
     {
-        $sql = "SELECT tc.constraint_name AS name,
+        $sql = "SELECT
+            rc.constraint_name AS name,
             tc.constraint_type AS type,
             kcu.column_name,
             rc.match_option AS match_type,
-
             rc.update_rule AS on_update,
             rc.delete_rule AS on_delete,
-            ccu.table_name AS references_table,
-            ccu.column_name AS references_field
-            FROM information_schema.table_constraints tc
 
-            LEFT JOIN information_schema.key_column_usage kcu
-            ON tc.constraint_catalog = kcu.constraint_catalog
-            AND tc.constraint_schema = kcu.constraint_schema
-            AND tc.constraint_name = kcu.constraint_name
+            kc.table_name AS references_table,
+            kc.column_name AS references_field
+
+            FROM information_schema.referential_constraints rc
+
+            JOIN information_schema.table_constraints tc
+                ON tc.constraint_name = rc.constraint_name
+                AND tc.constraint_schema = rc.constraint_schema
+                AND tc.constraint_name = rc.constraint_name
 
-            LEFT JOIN information_schema.referential_constraints rc
-            ON tc.constraint_catalog = rc.constraint_catalog
-            AND tc.constraint_schema = rc.constraint_schema
-            AND tc.constraint_name = rc.constraint_name
+            JOIN information_schema.key_column_usage kcu
+                ON kcu.constraint_name = rc.constraint_name
+                AND kcu.constraint_schema = rc.constraint_schema
+                AND kcu.constraint_name = rc.constraint_name
 
-            LEFT JOIN information_schema.constraint_column_usage ccu
-            ON rc.unique_constraint_catalog = ccu.constraint_catalog
-            AND rc.unique_constraint_schema = ccu.constraint_schema
-            AND rc.unique_constraint_name = ccu.constraint_name
+            JOIN information_schema.key_column_usage kc
+                ON kc.ordinal_position = kcu.position_in_unique_constraint
+                AND kc.constraint_name = rc.unique_constraint_name
 
-            WHERE tc.table_name = ?
-            AND tc.table_schema = ?
-            AND tc.constraint_type = 'FOREIGN KEY'";
+            WHERE kcu.table_name = ?
+              AND kc.table_schema = ?
+              AND tc.constraint_type = 'FOREIGN KEY'
+
+            ORDER BY rc.constraint_name, kcu.ordinal_position";
 
         $schema = empty($config['schema']) ? 'public' : $config['schema'];
         return [$sql, [$tableName, $schema]];
@@ -313,17 +316,13 @@ class PostgresSchema extends BaseSchema
      */
     public function convertForeignKeyDescription(Table $table, $row)
     {
-        $data = $table->constraint($row['name']);
-        if (empty($data)) {
-            $data = [
-                'type' => Table::CONSTRAINT_FOREIGN,
-                'columns' => [],
-                'references' => [$row['references_table'], $row['references_field']],
-                'update' => $this->_convertOnClause($row['on_update']),
-                'delete' => $this->_convertOnClause($row['on_delete']),
-            ];
-        }
-        $data['columns'][] = $row['column_name'];
+        $data = [
+            'type' => Table::CONSTRAINT_FOREIGN,
+            'columns' => $row['column_name'],
+            'references' => [$row['references_table'], $row['references_field']],
+            'update' => $this->_convertOnClause($row['on_update']),
+            'delete' => $this->_convertOnClause($row['on_delete']),
+        ];
         $table->addConstraint($row['name'], $data);
     }
 
@@ -468,7 +467,7 @@ class PostgresSchema extends BaseSchema
                 ' FOREIGN KEY (%s) REFERENCES %s (%s) ON UPDATE %s ON DELETE %s DEFERRABLE INITIALLY IMMEDIATE',
                 implode(', ', $columns),
                 $this->_driver->quoteIdentifier($data['references'][0]),
-                $this->_driver->quoteIdentifier($data['references'][1]),
+                $this->_convertConstraintColumns($data['references'][1]),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/SqliteSchema.php

@@ -24,6 +24,14 @@ class SqliteSchema extends BaseSchema
 {
 
     /**
+     * Array containing the foreign keys constraints names
+     * Necessary for composite foreign keys to be handled
+     *
+     * @var array
+     */
+    protected $_constraintsIdMap = [];
+
+    /**
      * Convert a column definition to the abstract types.
      *
      * The returned type will be a type that
@@ -207,6 +215,8 @@ class SqliteSchema extends BaseSchema
      */
     public function convertForeignKeyDescription(Table $table, $row)
     {
+        $name = $row['from'] . '_fk';
+
         $update = isset($row['on_update']) ? $row['on_update'] : '';
         $delete = isset($row['on_delete']) ? $row['on_delete'] : '';
         $data = [
@@ -216,7 +226,13 @@ class SqliteSchema extends BaseSchema
             'update' => $this->_convertOnClause($update),
             'delete' => $this->_convertOnClause($delete),
         ];
-        $name = $row['from'] . '_fk';
+
+        if (isset($this->_constraintsIdMap[$table->name()][$row['id']])) {
+            $name = $this->_constraintsIdMap[$table->name()][$row['id']];
+        } else {
+            $this->_constraintsIdMap[$table->name()][$row['id']] = $name;
+        }
+
         $table->addConstraint($name, $data);
     }
 
@@ -315,10 +331,11 @@ class SqliteSchema extends BaseSchema
         }
         if ($data['type'] === Table::CONSTRAINT_FOREIGN) {
             $type = 'FOREIGN KEY';
+
             $clause = sprintf(
                 ' REFERENCES %s (%s) ON UPDATE %s ON DELETE %s',
                 $this->_driver->quoteIdentifier($data['references'][0]),
-                $this->_driver->quoteIdentifier($data['references'][1]),
+                $this->_convertConstraintColumns($data['references'][1]),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/SqlserverSchema.php

@@ -419,7 +419,7 @@ class SqlserverSchema extends BaseSchema
                 ' FOREIGN KEY (%s) REFERENCES %s (%s) ON UPDATE %s ON DELETE %s',
                 implode(', ', $columns),
                 $this->_driver->quoteIdentifier($data['references'][0]),
-                $this->_driver->quoteIdentifier($data['references'][1]),
+                $this->_convertConstraintColumns($data['references'][1]),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/Table.php

@@ -511,9 +511,23 @@ class Table
 
         if ($attrs['type'] === static::CONSTRAINT_FOREIGN) {
             $attrs = $this->_checkForeignKey($attrs);
+
+            if (isset($this->_constraints[$name])) {
+                $this->_constraints[$name]['columns'] = array_merge(
+                    $this->_constraints[$name]['columns'],
+                    $attrs['columns']
+                );
+
+                $this->_constraints[$name]['references'][1] = array_merge(
+                    (array)$this->_constraints[$name]['references'][1],
+                    [$attrs['references'][1]]
+                );
+                return $this;
+            }
         } else {
             unset($attrs['references'], $attrs['update'], $attrs['delete']);
         }
+
         $this->_constraints[$name] = $attrs;
         return $this;
     }

tests/Fixture/ArticlesTagsFixture.php

@@ -33,7 +33,7 @@ class ArticlesTagsFixture extends TestFixture
         'tag_id' => ['type' => 'integer', 'null' => false],
         '_constraints' => [
             'unique_tag' => ['type' => 'primary', 'columns' => ['article_id', 'tag_id']],
-            'tag_idx' => [
+            'tag_id_fk' => [
                 'type' => 'foreign',
                 'columns' => ['tag_id'],
                 'references' => ['tags', 'id'],

tests/Fixture/OrdersFixture.php

@@ -0,0 +1,68 @@
+<?php
+/**
+ * CakePHP(tm) Tests <http://book.cakephp.org/2.0/en/development/testing.html>
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://book.cakephp.org/2.0/en/development/testing.html CakePHP(tm) Tests
+ * @since         3.0.7
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Test\Fixture;
+
+use Cake\TestSuite\Fixture\TestFixture;
+
+/**
+ * Class OrdersFixture
+ *
+ */
+class OrdersFixture extends TestFixture
+{
+
+    /**
+     * {@inheritDoc}
+     */
+    public $table = 'orders';
+
+    /**
+     * fields property
+     *
+     * @var array
+     */
+    public $fields = [
+        'id' => ['type' => 'integer'],
+        'product_category' => ['type' => 'integer', 'null' => false],
+        'product_id' => ['type' => 'integer', 'null' => false],
+        '_indexes' => [
+            'product_category' => [
+                'type' => 'index',
+                'columns' => ['product_category', 'product_id']
+            ]
+        ],
+        '_constraints' => [
+            'primary' => [
+                'type' => 'primary', 'columns' => ['id']
+            ],
+            'product_id_fk' => [
+                'type' => 'foreign',
+                'columns' => ['product_id', 'product_category'],
+                'references' => ['products', ['id', 'category']],
+                'update' => 'cascade',
+                'delete' => 'cascade',
+            ]
+        ]
+    ];
+
+    /**
+     * records property
+     *
+     * @var array
+     */
+    public $records = [
+        ['product_category' => 1, 'product_id' => 1]
+    ];
+}

tests/Fixture/ProductsFixture.php

@@ -0,0 +1,53 @@
+<?php
+/**
+ * CakePHP(tm) Tests <http://book.cakephp.org/2.0/en/development/testing.html>
+ * Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
+ * @link          http://book.cakephp.org/2.0/en/development/testing.html CakePHP(tm) Tests
+ * @since         3.0.7
+ * @license       http://www.opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Test\Fixture;
+
+use Cake\TestSuite\Fixture\TestFixture;
+
+/**
+ * Class ProductsFixture
+ *
+ */
+class ProductsFixture extends TestFixture
+{
+    /**
+     * {@inheritDoc}
+     */
+    public $table = 'products';
+
+    /**
+     * fields property
+     *
+     * @var array
+     */
+    public $fields = [
+        'id' => ['type' => 'integer'],
+        'category' => ['type' => 'integer', 'null' => false],
+        'name' => ['type' => 'string', 'null' => false],
+        'price' => ['type' => 'integer'],
+        '_constraints' => ['primary' => ['type' => 'primary', 'columns' => ['id', 'category']]]
+    ];
+
+    /**
+     * records property
+     *
+     * @var array
+     */
+    public $records = [
+        ['id' => 1, 'category' => 1, 'name' => 'First product', 'price' => 10],
+        ['id' => 2, 'category' => 2, 'name' => 'Second product', 'price' => 20],
+        ['id' => 3, 'category' => 3, 'name' => 'Third product', 'price' => 30]
+    ];
+}

tests/TestCase/Database/Schema/TableTest.php

@@ -15,6 +15,8 @@
 namespace Cake\Test\TestCase\Database\Schema;
 
 use Cake\Database\Schema\Table;
+use Cake\Datasource\ConnectionManager;
+use Cake\ORM\TableRegistry;
 use Cake\TestSuite\TestCase;
 
 /**
@@ -23,6 +25,14 @@ use Cake\TestSuite\TestCase;
 class TableTest extends TestCase
 {
 
+    public $fixtures = ['core.articles_tags', 'core.products', 'core.orders', 'core.tags'];
+
+    public function tearDown()
+    {
+        TableRegistry::clear();
+        parent::tearDown();
+    }
+
     /**
      * Test construction with columns
      *
@@ -406,6 +416,62 @@ class TableTest extends TestCase
     }
 
     /**
+     * Test single column foreign keys constraint support
+     *
+     * @return void
+     */
+    public function testConstraintForeignKey()
+    {
+        $table = TableRegistry::get('ArticlesTags');
+        $compositeConstraint = $table->schema()->constraint('tag_id_fk');
+        $expected = [
+            'type' => 'foreign',
+            'columns' => ['tag_id'],
+            'references' => ['tags', 'id'],
+            'update' => 'cascade',
+            'delete' => 'cascade',
+            'length' => []
+        ];
+
+        $this->assertEquals($expected, $compositeConstraint);
+
+        $expectedSubstring = 'CONSTRAINT <tag_id_fk> FOREIGN KEY \(<tag_id>\) REFERENCES <tags> \(<id>\)';
+        $this->assertQuotedQuery($expectedSubstring, $table->schema()->createSql(ConnectionManager::get('test'))[0]);
+    }
+
+    /**
+     * Test composite foreign keys support
+     *
+     * @return void
+     */
+    public function testConstraintForeignKeyTwoColumns()
+    {
+        $table = TableRegistry::get('Orders');
+        $compositeConstraint = $table->schema()->constraint('product_id_fk');
+        $expected = [
+            'type' => 'foreign',
+            'columns' => [
+                'product_id',
+                'product_category'
+            ],
+            'references' => [
+                'products',
+                ['id', 'category']
+            ],
+            'update' => 'cascade',
+            'delete' => 'cascade',
+            'length' => []
+        ];
+
+        $this->assertEquals($expected, $compositeConstraint);
+
+        $expectedSubstring = 'CONSTRAINT <product_id_fk> FOREIGN KEY \(<product_id>, <product_category>\)' .
+            ' REFERENCES <products> \(<id>, <category>\)';
+
+        $this->assertQuotedQuery($expectedSubstring, $table->schema()->createSql(ConnectionManager::get('test'))[0]);
+    }
+
+    /**
      * Provider for exceptionally bad foreign key data.
      *
      * @return array
@@ -462,4 +528,25 @@ class TableTest extends TestCase
         $table->temporary(false);
         $this->assertFalse($table->temporary());
     }
+
+    /**
+     * Assertion for comparing a regex pattern against a query having its identifiers
+     * quoted. It accepts queries quoted with the characters `<` and `>`. If the third
+     * parameter is set to true, it will alter the pattern to both accept quoted and
+     * unquoted queries
+     *
+     * @param string $pattern
+     * @param string $query the result to compare against
+     * @param bool $optional
+     * @return void
+     */
+    public function assertQuotedQuery($pattern, $query, $optional = false)
+    {
+        if ($optional) {
+            $optional = '?';
+        }
+        $pattern = str_replace('<', '[`"\[]' . $optional, $pattern);
+        $pattern = str_replace('>', '[`"\]]' . $optional, $pattern);
+        $this->assertRegExp('#' . $pattern . '#', $query);
+    }
 }