Skip to content

Commit 80adf8a

Browse files
author
Sergii Kovalenko
committed
MAGETWO-81030: Schema Validation
1 parent 74a70a0 commit 80adf8a

File tree

14 files changed

+522
-14
lines changed

14 files changed

+522
-14
lines changed

‎app/etc/di.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,9 @@
14681468
<argument name="rules" xsi:type="array">
14691469
<item name="check_references" xsi:type="object">Magento\Setup\Model\Declaration\Schema\Declaration\ValidationRules\CheckReferenceColumnHasIndex</item>
14701470
<item name="real_types" xsi:type="object">Magento\Setup\Model\Declaration\Schema\Declaration\ValidationRules\RealTypes</item>
1471+
<item name="check_primary_key" xsi:type="object">Magento\Setup\Model\Declaration\Schema\Declaration\ValidationRules\PrimaryKeyCanBeCreated</item>
1472+
<item name="inconsistence_references" xsi:type="object">Magento\Setup\Model\Declaration\Schema\Declaration\ValidationRules\IncosistentReferenceDefinition</item>
1473+
<item name="auto_increment_validation" xsi:type="object">Magento\Setup\Model\Declaration\Schema\Declaration\ValidationRules\AutoIncrementColumnValidation</item>
14711474
</argument>
14721475
</arguments>
14731476
</type>

‎dev/tests/setup-integration/_files/Magento/TestSetupDeclarationModule8/etc/module.xml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
9+
<module name="Magento_TestSetupDeclarationModule8" setup_version="1.0.0" />
10+
</config>

‎dev/tests/setup-integration/_files/Magento/TestSetupDeclarationModule8/registration.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
use Magento\Framework\Component\ComponentRegistrar;
8+
9+
$registrar = new ComponentRegistrar();
10+
if ($registrar->getPath(ComponentRegistrar::MODULE, 'Magento_TestSetupDeclarationModule8') === null) {
11+
ComponentRegistrar::register(ComponentRegistrar::MODULE, 'Magento_TestSetupDeclarationModule8', __DIR__);
12+
}

‎dev/tests/setup-integration/_files/Magento/TestSetupDeclarationModule8/revisions/incosistence_reference_definition/db_schema.xml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<schema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
9+
xsi:noNamespaceSchemaLocation="urn:magento:setup:Model/Declaration/Schema/etc/schema.xsd">
10+
<table name="test_table" resource="default" comment="Test Table">
11+
<column xsi:type="int" name="page_id" nullable="false" unsigned="false" identity="true"/>
12+
<constraint xsi:type="primary" name="PRIMARY">
13+
<column name="page_id"/>
14+
</constraint>
15+
</table>
16+
<table name="dependent" resource="default" comment="Lol">
17+
<column xsi:type="int" name="page_id_on" nullable="true" unsigned="true"/>
18+
<constraint xsi:type="foreign" name="FOREIGN" table="dependent" column="page_id_on" referenceColumn="page_id"
19+
referenceTable="test_table"/>
20+
</table>
21+
</schema>

‎dev/tests/setup-integration/_files/Magento/TestSetupDeclarationModule8/revisions/invalid_auto_increment/db_schema.xml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<schema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
9+
xsi:noNamespaceSchemaLocation="urn:magento:setup:Model/Declaration/Schema/etc/schema.xsd">
10+
<table name="test_table" resource="default" comment="Test Table">
11+
<column xsi:type="int" name="page_id" nullable="false" identity="true" />
12+
</table>
13+
</schema>

‎dev/tests/setup-integration/_files/Magento/TestSetupDeclarationModule8/revisions/invalid_primary_key/db_schema.xml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<schema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
9+
xsi:noNamespaceSchemaLocation="urn:magento:setup:Model/Declaration/Schema/etc/schema.xsd">
10+
<table name="test_table" resource="default" comment="Test Table">
11+
<column xsi:type="int" name="page_id" nullable="false" identity="true" />
12+
<column xsi:type="varchar" name="email" nullable="true" />
13+
<column xsi:type="varchar" name="title" />
14+
<constraint xsi:type="primary" name="PRIMARY">
15+
<column name="page_id" />
16+
<column name="email" />
17+
</constraint>
18+
</table>
19+
</schema>

‎dev/tests/setup-integration/framework/Magento/TestFramework/Bootstrap/SetupDocBlock.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
namespace Magento\TestFramework\Bootstrap;
77

8+
use Magento\TestFramework\Annotation\AppIsolation;
9+
810
/**
911
* Bootstrap of the custom DocBlock annotations.
1012
*
@@ -31,6 +33,7 @@ protected function _getSubscribers(\Magento\TestFramework\Application $applicati
3133
new \Magento\TestFramework\Annotation\ComponentRegistrarFixture($this->_fixturesBaseDir),
3234
new \Magento\TestFramework\Annotation\SchemaFixture($this->_fixturesBaseDir),
3335
new \Magento\TestFramework\Annotation\Cache(),
36+
new AppIsolation($application),
3437
new \Magento\TestFramework\Workaround\CacheClean(),
3538
new \Magento\TestFramework\Annotation\ReinstallInstance($application),
3639
new \Magento\TestFramework\Annotation\CopyModules(),

‎dev/tests/setup-integration/testsuite/Magento/Setup/DeclarativeSchemaBuilderTest.php

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Magento\TestFramework\TestCase\SetupTestCase;
1717

1818
/**
19+
* @magentoAppIsolation enabled
1920
* The purpose of this test is verifying initial InstallSchema, InstallData scripts.
2021
*/
2122
class DeclarativeSchemaBuilderTest extends SetupTestCase
@@ -38,7 +39,7 @@ class DeclarativeSchemaBuilderTest extends SetupTestCase
3839
public function setUp()
3940
{
4041
$objectManager = Bootstrap::getObjectManager();
41-
$this->schemaConfig = $objectManager->get(SchemaConfig::class);
42+
$this->schemaConfig = $objectManager->create(SchemaConfig::class);
4243
$this->moduleManager = $objectManager->get(TestModuleManager::class);
4344
$this->cliCommad = $objectManager->get(CliCommand::class);
4445
}
@@ -60,26 +61,84 @@ public function testSchemaBuilder()
6061
//Test primary key and renaming
6162
$referenceTable = $schemaTables['reference_table'];
6263
/**
63-
* @var Internal $primaryKey
64-
*/
64+
* @var Internal $primaryKey
65+
*/
6566
$primaryKey = $referenceTable->getPrimaryConstraint();
6667
$columns = $primaryKey->getColumns();
6768
self::assertEquals('tinyint_ref', reset($columns)->getName());
6869
//Test column
6970
$testTable = $schemaTables['test_table'];
7071
/**
71-
* @var Timestamp $timestampColumn
72-
*/
72+
* @var Timestamp $timestampColumn
73+
*/
7374
$timestampColumn = $testTable->getColumnByName('timestamp');
7475
self::assertEquals('CURRENT_TIMESTAMP', $timestampColumn->getOnUpdate());
7576
//Test disabled
7677
self::assertArrayNotHasKey('varbinary_rename', $testTable->getColumns());
7778
//Test foreign key
7879
/**
79-
* @var Reference $foreignKey
80-
*/
80+
* @var Reference $foreignKey
81+
*/
8182
$foreignKey = $testTable->getConstraintByName('some_foreign_key');
8283
self::assertEquals('NO ACTION', $foreignKey->getOnDelete());
8384
self::assertEquals('tinyint_ref', $foreignKey->getReferenceColumn()->getName());
8485
}
86+
87+
/**
88+
* @expectedException \Magento\Setup\Exception
89+
* @expectedExceptionMessageRegExp /Primary key can`t be applied on table "test_table". All columns should be not nullable/
90+
* @moduleName Magento_TestSetupDeclarationModule8
91+
*/
92+
public function testFailOnInvalidPrimaryKey()
93+
{
94+
$this->cliCommad->install(
95+
['Magento_TestSetupDeclarationModule8']
96+
);
97+
$this->moduleManager->updateRevision(
98+
'Magento_TestSetupDeclarationModule8',
99+
'invalid_primary_key',
100+
'db_schema.xml',
101+
'etc'
102+
);
103+
104+
$this->schemaConfig->getDeclarationConfig();
105+
}
106+
107+
/**
108+
* @expectedException \Magento\Setup\Exception
109+
* @expectedExceptionMessageRegExp /Column definition "page_id_on" and reference column definition "page_id" are different in tables "dependent" and "test_table"/
110+
* @moduleName Magento_TestSetupDeclarationModule8
111+
*/
112+
public function testFailOnIncosistentReferenceDefinition()
113+
{
114+
$this->cliCommad->install(
115+
['Magento_TestSetupDeclarationModule8']
116+
);
117+
$this->moduleManager->updateRevision(
118+
'Magento_TestSetupDeclarationModule8',
119+
'incosistence_reference_definition',
120+
'db_schema.xml',
121+
'etc'
122+
);
123+
$this->schemaConfig->getDeclarationConfig();
124+
}
125+
126+
/**
127+
* @expectedException \Magento\Setup\Exception
128+
* @expectedExceptionMessageRegExp /Auto Increment column do not have index. Column - "page_id"/
129+
* @moduleName Magento_TestSetupDeclarationModule8
130+
*/
131+
public function testFailOnInvalidAutoIncrementFiel()
132+
{
133+
$this->cliCommad->install(
134+
['Magento_TestSetupDeclarationModule8']
135+
);
136+
$this->moduleManager->updateRevision(
137+
'Magento_TestSetupDeclarationModule8',
138+
'invalid_auto_increment',
139+
'db_schema.xml',
140+
'etc'
141+
);
142+
$this->schemaConfig->getDeclarationConfig();
143+
}
85144
}

‎setup/src/Magento/Setup/Model/Declaration/Schema/Declaration/SchemaBuilder.php

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use Magento\Framework\Stdlib\BooleanUtils;
99
use Magento\Setup\Exception;
10+
use Magento\Setup\Model\Declaration\Schema\Dto\Column;
1011
use Magento\Setup\Model\Declaration\Schema\Dto\Constraint;
1112
use Magento\Setup\Model\Declaration\Schema\Dto\ElementFactory;
1213
use Magento\Setup\Model\Declaration\Schema\Dto\Index;
@@ -239,6 +240,24 @@ private function processTable(Schema $schema, array $tableData)
239240
return $schema->getTableByName($tableData['name']);
240241
}
241242

243+
/**
244+
* @param string $columnName
245+
* @param Table $table
246+
* @return Column
247+
*/
248+
private function getColumnByName(string $columnName, Table $table)
249+
{
250+
$columnCandidate = $table->getColumnByName($columnName);
251+
252+
if (!$columnCandidate) {
253+
throw new \LogicException(
254+
sprintf('Table %s do not have column with name %s', $table->getName(), $columnName)
255+
);
256+
}
257+
258+
return $columnCandidate;
259+
}
260+
242261
/**
243262
* Convert column names to objects.
244263
*
@@ -251,7 +270,7 @@ private function convertColumnNamesToObjects(array $columnNames, Table $table)
251270
$columns = [];
252271

253272
foreach ($columnNames as $columnName) {
254-
$columns[] = $table->getColumnByName($columnName);
273+
$columns[] = $this->getColumnByName($columnName, $table);
255274
}
256275

257276
return $columns;
@@ -311,7 +330,7 @@ private function processConstraints(array $tableData, $resource, Schema $schema,
311330
$constraintData = $this->processGenericData($constraintData, $resource, $table);
312331
//As foreign constraint has different schema we need to process it in different way
313332
if ($constraintData['type'] === 'foreign') {
314-
$constraintData['column'] = $table->getColumnByName($constraintData['column']);
333+
$constraintData['column'] = $this->getColumnByName($constraintData['column'], $table);
315334
$referenceTableData = $this->tablesData[$constraintData['referenceTable']];
316335
//If we are referenced to the same table we need to specify it
317336
//Get table name from resource connection regarding prefix settings
@@ -327,11 +346,14 @@ private function processConstraints(array $tableData, $resource, Schema $schema,
327346
$constraintData['referenceTable'] = $referenceTable;
328347

329348
if (!$constraintData['referenceTable']) {
330-
throw new \LogicException("Cannot find reference table");
349+
throw new \LogicException(
350+
sprintf('Cannot find reference table with name %s', $constraints['referenceTable'])
351+
);
331352
}
332353

333-
$constraintData['referenceColumn'] = $constraintData['referenceTable']->getColumnByName(
334-
$constraintData['referenceColumn']
354+
$constraintData['referenceColumn'] = $this->getColumnByName(
355+
$constraintData['referenceColumn'],
356+
$constraintData['referenceTable']
335357
);
336358
$constraint = $this->elementFactory->create($constraintData['type'], $constraintData);
337359
$constraints[$constraint->getName()] = $constraint;

‎setup/src/Magento/Setup/Model/Declaration/Schema/Declaration/ValidationRules/AutoIncrementColumnValidation.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
namespace Magento\Setup\Model\Declaration\Schema\Declaration\ValidationRules;
7+
8+
use Magento\Setup\Model\Declaration\Schema\Declaration\ValidationInterface;
9+
use Magento\Setup\Model\Declaration\Schema\Dto\Columns\ColumnIdentityAwareInterface;
10+
use Magento\Setup\Model\Declaration\Schema\Dto\Constraints\Internal;
11+
use Magento\Setup\Model\Declaration\Schema\Dto\Schema;
12+
13+
/**
14+
* Check whether autoincrement column is valid
15+
*
16+
* @inheritdoc
17+
*/
18+
class AutoIncrementColumnValidation implements ValidationInterface
19+
{
20+
/**
21+
* Error code.
22+
*/
23+
const ERROR_TYPE = 'auto_increment_column_is_valid';
24+
25+
/**
26+
* Error message, that will be shown.
27+
*/
28+
const ERROR_MESSAGE = 'Auto Increment column do not have index. Column - "%s"';
29+
30+
/**
31+
* @inheritdoc
32+
*/
33+
public function validate(Schema $schema)
34+
{
35+
$errors = [];
36+
foreach ($schema->getTables() as $table) {
37+
foreach ($table->getColumns() as $column) {
38+
if ($column instanceof ColumnIdentityAwareInterface && $column->isIdentity()) {
39+
foreach ($table->getConstraints() as $constraint) {
40+
if ($constraint instanceof Internal &&
41+
in_array($column->getName(), $constraint->getColumnNames())
42+
) {
43+
//If we find that for auto increment column we have index or key
44+
continue 3;
45+
}
46+
}
47+
48+
foreach ($table->getIndexes() as $index) {
49+
if (in_array($column->getName(), $index->getColumnNames())) {
50+
//If we find that for auto increment column we have index or key
51+
continue 3;
52+
}
53+
}
54+
55+
$errors[] = [
56+
'column' => $column->getName(),
57+
'message' => sprintf(self::ERROR_MESSAGE, $column->getName())
58+
];
59+
}
60+
}
61+
}
62+
63+
return $errors;
64+
}
65+
}

0 commit comments

Comments
 (0)