From f69bfd86b8c4733d2aadc3ee9a32ad9ebbc077d4 Mon Sep 17 00:00:00 2001 From: mark_story Date: Fri, 21 Jun 2013 22:01:00 -0400 Subject: [PATCH] Add additional validation when adding constraints & indexes. Add more extensive checks when adding indexes & constraints. Allow columns key to be defined as a string. This is an easy to correct error user error. --- lib/Cake/Database/Schema/Table.php | 8 +++ .../TestCase/Database/Schema/TableTest.php | 65 ++++++++++++------- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/lib/Cake/Database/Schema/Table.php b/lib/Cake/Database/Schema/Table.php index 84d3a080d68..07105ee1119 100644 --- a/lib/Cake/Database/Schema/Table.php +++ b/lib/Cake/Database/Schema/Table.php @@ -249,6 +249,10 @@ public function addIndex($name, $attrs) { if (!in_array($attrs['type'], $this->_validIndexTypes, true)) { throw new Exception(__d('cake_dev', 'Invalid index type "%s"', $attrs['type'])); } + if (empty($attrs['columns'])) { + throw new Exception(__d('cake_dev', 'Indexes must define columns.')); + } + $attrs['columns'] = (array)$attrs['columns']; foreach ($attrs['columns'] as $field) { if (empty($this->_columns[$field])) { throw new Exception(__d('cake_dev', 'Columns used in indexes must already exist.')); @@ -325,6 +329,10 @@ public function addConstraint($name, $attrs) { if (!in_array($attrs['type'], $this->_validConstraintTypes, true)) { throw new Exception(__d('cake_dev', 'Invalid constraint type "%s"', $attrs['type'])); } + if (empty($attrs['columns'])) { + throw new Exception(__d('cake_dev', 'Constraints must define columns.')); + } + $attrs['columns'] = (array)$attrs['columns']; foreach ($attrs['columns'] as $field) { if (empty($this->_columns[$field])) { throw new Exception(__d('cake_dev', 'Columns used in constraints must already exist.')); diff --git a/lib/Cake/Test/TestCase/Database/Schema/TableTest.php b/lib/Cake/Test/TestCase/Database/Schema/TableTest.php index e8d86137483..df2a89f39f2 100644 --- a/lib/Cake/Test/TestCase/Database/Schema/TableTest.php +++ b/lib/Cake/Test/TestCase/Database/Schema/TableTest.php @@ -122,17 +122,35 @@ public function testAddConstraint() { } /** - * Test that an exception is raised when constraintes + * Dataprovider for invalid addConstraint calls. + * + * @return array + */ + public static function addConstaintErrorProvider() { + return [ + // No properties + [[]], + // Empty columns + [['columns' => '']], + [['columns' => []]], + // Missing column + [['columns' => ['derp']]], + // Invalid type + [['columns' => 'author_id', 'type' => 'derp']], + ]; + } +/** + * Test that an exception is raised when constraints * are added for fields that do not exist. * + * @dataProvider addConstaintErrorProvider * @expectedException Cake\Database\Exception * @return void */ - public function testAddConstraintErrorWhenFieldIsMissing() { + public function testAddConstraintError($props) { $table = new Table('articles'); - $table->addConstraint('author_idx', [ - 'columns' => ['author_id'] - ]); + $table->addColumn('author_id', 'integer'); + $table->addConstraint('author_idx', $props); } /** @@ -154,33 +172,36 @@ public function testAddIndex() { } /** - * Test that an exception is raised when indexes - * are added for fields that do not exist. + * Dataprovider for invalid addIndex calls * - * @expectedException Cake\Database\Exception - * @return void + * @return array */ - public function testAddIndexErrorWhenFieldIsMissing() { - $table = new Table('articles'); - $table->addIndex('author_idx', [ - 'columns' => ['author_id'] - ]); + public static function addIndexErrorProvider() { + return [ + // Empty + [[]], + // No columns + [['columns' => '']], + [['columns' => []]], + // Missing column + [['columns' => ['not_there']]], + // Invalid type + [['columns' => 'author_id', 'type' => 'derp']], + ]; } /** - * Test that exceptions are raised when indexes - * are added with invalid types + * Test that an exception is raised when indexes + * are added for fields that do not exist. * + * @dataProvider addIndexErrorProvider * @expectedException Cake\Database\Exception * @return void */ - public function testAddIndexErrorWrongType() { + public function testAddIndexError($props) { $table = new Table('articles'); - $table->addColumn('author_id', 'integer') - ->addIndex('author_idx', [ - 'type' => 'derp', - 'columns' => ['author_id'] - ]); + $table->addColumn('author_id', 'integer'); + $table->addIndex('author_idx', $props); } /**