Add a failing test to show that Schema reflection does not correctly reflects

composite foreign keys and fix it for all the Schema classes

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);
     }
 
@@ -464,11 +463,20 @@ class PostgresSchema extends BaseSchema
             $data['columns']
         );
         if ($data['type'] === Table::CONSTRAINT_FOREIGN) {
+            if (!is_array($data['references'][1])) {
+                $data['references'][1] = [$data['references'][1]];
+            }
+
+            $columnsReference = array_map(
+                [$this->_driver, 'quoteIdentifier'],
+                $data['references'][1]
+            );
+
             return $prefix . sprintf(
                 ' 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]),
+                implode(', ', $columnsReference),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/SqliteSchema.php

@@ -24,6 +24,14 @@ class SqliteSchema extends BaseSchema
 {
 
     /**
+     * Array containing the FK names
+     * Necessary for composite foreign keys to be handled
+     *
+     * @var array
+     */
+    protected $_fkIdMap = [];
+
+    /**
      * 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->_fkIdMap[$table->name()][$row['id']])) {
+            $name = $this->_fkIdMap[$table->name()][$row['id']];
+        } else {
+            $this->_fkIdMap[$table->name()][$row['id']] = $name;
+        }
+
         $table->addConstraint($name, $data);
     }
 
@@ -315,10 +331,20 @@ class SqliteSchema extends BaseSchema
         }
         if ($data['type'] === Table::CONSTRAINT_FOREIGN) {
             $type = 'FOREIGN KEY';
+
+            if (!is_array($data['references'][1])) {
+                $data['references'][1] = [$data['references'][1]];
+            }
+
+            $columnsReference = array_map(
+                [$this->_driver, 'quoteIdentifier'],
+                $data['references'][1]
+            );
+
             $clause = sprintf(
                 ' REFERENCES %s (%s) ON UPDATE %s ON DELETE %s',
                 $this->_driver->quoteIdentifier($data['references'][0]),
-                $this->_driver->quoteIdentifier($data['references'][1]),
+                implode(', ', $columnsReference),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/SqlserverSchema.php

@@ -415,11 +415,20 @@ class SqlserverSchema extends BaseSchema
             $data['columns']
         );
         if ($data['type'] === Table::CONSTRAINT_FOREIGN) {
+            if (!is_array($data['references'][1])) {
+                $data['references'][1] = [$data['references'][1]];
+            }
+
+            $columnsReference = array_map(
+                [$this->_driver, 'quoteIdentifier'],
+                $data['references'][1]
+            );
+
             return $prefix . sprintf(
                 ' 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]),
+                implode(', ', $columnsReference),
                 $this->_foreignOnClause($data['update']),
                 $this->_foreignOnClause($data['delete'])
             );

src/Database/Schema/Table.php

@@ -511,9 +511,27 @@ 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']
+                );
+
+                if (is_string($this->_constraints[$name]['references'][1])) {
+                    $this->_constraints[$name]['references'][1] = [$this->_constraints[$name]['references'][1]];
+                }
+
+                $this->_constraints[$name]['references'][1] = array_merge(
+                    $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/CustomersFixture.php

@@ -24,6 +24,11 @@ class CustomersFixture extends TestFixture
 {
 
     /**
+     * {@inheritDoc}
+     */
+    public $table = 'customers';
+
+    /**
      * fields property
      *
      * @var array

tests/Fixture/ProductOrdersFixture.php

@@ -17,13 +17,18 @@ namespace Cake\Test\Fixture;
 use Cake\TestSuite\Fixture\TestFixture;
 
 /**
- * Class ProductOrdersFixture
+ * Class OrdersFixture
  *
  */
-class ProductOrdersFixture extends TestFixture
+class OrdersFixture extends TestFixture
 {
 
     /**
+     * {@inheritDoc}
+     */
+    public $table = 'orders';
+
+    /**
      * fields property
      *
      * @var array
@@ -47,17 +52,17 @@ class ProductOrdersFixture extends TestFixture
             'primary' => [
                 'type' => 'primary', 'columns' => ['id']
             ],
-            'product_order_ibfk_1' => [
+            'product_id_fk' => [
                 'type' => 'foreign',
-                'columns' => ['product_category', 'product_id'],
-                'references' => ['product', ['category', 'id']],
+                'columns' => ['product_id', 'product_category'],
+                'references' => ['products', ['id', 'category']],
                 'update' => 'cascade',
                 'delete' => 'cascade',
             ],
-            'product_order_ibfk_2' => [
+            'order_ibfk_2' => [
                 'type' => 'foreign',
                 'columns' => ['customer_id'],
-                'references' => ['customer', 'id'],
+                'references' => ['customers', 'id'],
                 'update' => 'cascade',
                 'delete' => 'cascade',
             ]

tests/Fixture/ProductsFixture.php

@@ -22,6 +22,10 @@ use Cake\TestSuite\Fixture\TestFixture;
  */
 class ProductsFixture extends TestFixture
 {
+    /**
+     * {@inheritDoc}
+     */
+    public $table = 'products';
 
     /**
      * fields property
@@ -42,8 +46,8 @@ class ProductsFixture extends TestFixture
      * @var array
      */
     public $records = [
-        ['category' => 1, 'name' => 'First product', 'price' => 10],
-        ['category' => 2, 'name' => 'Second product', 'price' => 20],
-        ['category' => 3, 'name' => 'Third product', 'price' => 30]
+        ['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,7 +15,10 @@
 namespace Cake\Test\TestCase\Database\Schema;
 
 use Cake\Database\Schema\Table;
+use Cake\Datasource\ConnectionManager;
+use Cake\ORM\TableRegistry;
 use Cake\TestSuite\TestCase;
+use Symfony\Component\Yaml\Exception\RuntimeException;
 
 /**
  * Test case for Table
@@ -23,6 +26,8 @@ use Cake\TestSuite\TestCase;
 class TableTest extends TestCase
 {
 
+    public $fixtures = ['core.customers', 'core.products', 'core.orders'];
+
     /**
      * Test construction with columns
      *
@@ -406,6 +411,39 @@ class TableTest extends TestCase
     }
 
     /**
+     * Test composite foreign keys support
+     *
+     * @return void
+     */
+    public function testAddConstraintForeignKeyTwoColumns()
+    {
+        $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>\)#';
+        $expectedSubstring = str_replace(['<', '>'], ['[`"\[]', '[`"\]]'], $expectedSubstring);
+
+        $this->assertRegExp($expectedSubstring, $table->schema()->createSql(ConnectionManager::get('default'))[0]);
+    }
+
+    /**
      * Provider for exceptionally bad foreign key data.
      *
      * @return array
@@ -462,4 +500,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);
+    }
 }