Skip to content

Commit

Permalink
[BUGFIX] DefaultTcaSchema must not create SQL for tables not requested
Browse files Browse the repository at this point in the history 
The DefaultTcaSchema::enrich() function dynamically adds TCA-defined
ctrl-fields to the SQL schema definition for a provided set of tables.

The function used to add these definition for any table that is defined
in TCA, independent whether the table actually exists in any
ext_tables.sql file.
While this behaviour is no problem under the assumption that the
function is always called with the content of all ext_tables.sql files,
this is a problem if the function is called with only a subset of those
ext_tables.sql files. One example is the "extension manager tables"
upgrade in the Install Tool.

Despite the fact that the function was documented that it needs to be
called with the full set of tables, it actually is not strictly
necessary to do so.

This patch changes the requirement for the enrich() function and
changes the behaviour to not dynamically create tables if those are
found in TCA.

Technical detail to the reported bug:
The table definitions created by this class are usually overruled by
whatever might be defined in an ext_tables.sql file. In case of the
upgrade wizard (where only extension manager tables are requested) the
table definitions for all TCA tables were created based on the best
practice suggestions from the core. If the present DB schema (created by
the real ext_tables.sql files) mismatched those suggested definitions a
DB schema change was proposed, hence the wizard kept popping up.

Resolves: #89535
Releases: master, 9.5
Change-Id: I78594f1ee0878e8d3f5606901d0abb7fe0023059
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62848
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
  • Loading branch information
@liayn @ervaude
liayn authored and ervaude committed Jan 12, 2020
1 parent a0f12a5 commit 31cc90f
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 93 deletions.
23 changes: 8 additions & 15 deletions typo3/sysext/core/Classes/Database/Schema/DefaultTcaSchema.php
View file
Expand Up @@ -42,8 +42,6 @@ class DefaultTcaSchema
* "soft delete" ['ctrl']['delete'] and adds the field if it has not been
* defined in ext_tables.sql, yet.
*
* Note the incoming $tables array must be created from all ext_tables.sql files
* of all loaded extensions, and TCA must be up-to-date.
*
* @param Table[] $tables
* @return Table[]
Expand All @@ -52,21 +50,16 @@ public function enrich(array $tables): array
{
foreach ($GLOBALS['TCA'] as $tableName => $tableDefinition) {
$isTableDefined = $this->isTableDefined($tables, $tableName);
$tablePosition = null;
if ($isTableDefined) {
// If the table is given in existing $tables list, add all fields to the first
// position of that table - in case it is in there multiple times which happens
// if extensions add single fields to tables that have been defined in
// other ext_tables.sql, too.
$tablePosition = $this->getTableFirstPosition($tables, $tableName);
} else {
// Else create this table and add it to table list
$table = GeneralUtility::makeInstance(Table::class, $tableName);
$tables[] = $table;
$tableKeys = array_keys($tables);
$tablePosition = end($tableKeys);
if (!$isTableDefined) {
continue;
}

// If the table is given in existing $tables list, add all fields to the first
// position of that table - in case it is in there multiple times which happens
// if extensions add single fields to tables that have been defined in
// other ext_tables.sql, too.
$tablePosition = $this->getTableFirstPosition($tables, $tableName);

// uid column and primary key if uid is not defined
if (!$this->isColumnDefinedForTable($tables, $tableName, 'uid')) {
$tables[$tablePosition]->addColumn(
Expand Down
82 changes: 44 additions & 38 deletions typo3/sysext/core/Tests/Unit/Database/Schema/DefaultTcaSchemaSqliteTest.php
View file
Expand Up @@ -34,11 +34,17 @@ class DefaultTcaSchemaSqliteTest extends UnitTestCase
*/
protected $subject;

public function setUp()
/**
* @var Table
*/
protected $defaultTable;

public function setUp(): void
{
parent::setUp();
$this->subject = $this->getAccessibleMock(DefaultTcaSchema::class, ['tableRunsOnSqlite']);
$this->subject->method('tableRunsOnSqlite')->willReturn(true);
$this->defaultTable = new Table('aTable');
}

/**
Expand Down Expand Up @@ -121,7 +127,7 @@ public function enrichAddsFieldToFirstTableDefinitionOfThatName()
public function enrichAddsUidAndNoPrimaryKey()
{
$GLOBALS['TCA']['aTable']['ctrl'] = [];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedUidColumn = new Column(
'uid',
Type::getType('integer'),
Expand All @@ -141,7 +147,7 @@ public function enrichAddsUidAndNoPrimaryKey()
public function enrichAddsPid()
{
$GLOBALS['TCA']['aTable']['ctrl'] = [];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedPidColumn = new Column(
'pid',
Type::getType('integer'),
Expand All @@ -162,7 +168,7 @@ public function enrichAddsSignedPidWithEnabledWorkspace()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedPidColumn = new Column(
'pid',
Type::getType('integer'),
Expand All @@ -183,7 +189,7 @@ public function enrichAddsTstamp()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'tstamp' => 'updatedon',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`updatedon`',
Type::getType('integer'),
Expand All @@ -204,7 +210,7 @@ public function enrichAddsCrdate()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'crdate' => 'createdon',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`createdon`',
Type::getType('integer'),
Expand All @@ -225,7 +231,7 @@ public function enrichAddsCruserid()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'cruser_id' => 'createdby',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`createdby`',
Type::getType('integer'),
Expand All @@ -246,7 +252,7 @@ public function enrichAddsDeleted()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'delete' => 'deleted',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`deleted`',
Type::getType('smallint'),
Expand All @@ -269,7 +275,7 @@ public function enrichAddsDisabled()
'disabled' => 'disabled',
]
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`disabled`',
Type::getType('smallint'),
Expand All @@ -292,7 +298,7 @@ public function enrichAddsStarttime()
'starttime' => 'starttime',
]
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`starttime`',
Type::getType('integer'),
Expand All @@ -315,7 +321,7 @@ public function enrichAddsEndtime()
'endtime' => 'endtime',
]
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`endtime`',
Type::getType('integer'),
Expand All @@ -338,7 +344,7 @@ public function enrichAddsFegroup()
'fe_group' => 'fe_group',
]
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`fe_group`',
Type::getType('string'),
Expand All @@ -359,7 +365,7 @@ public function enrichAddsSorting()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'sortby' => 'sorting',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`sorting`',
Type::getType('integer'),
Expand All @@ -378,7 +384,7 @@ public function enrichAddsSorting()
public function enrichAddsParentKey()
{
$GLOBALS['TCA']['aTable']['ctrl'] = [];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedIndex = new Index('parent', ['pid']);
$this->assertEquals($expectedIndex, $result[0]->getIndex('parent'));
}
Expand All @@ -391,7 +397,7 @@ public function enrichAddsParentKeyWithDelete()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'delete' => 'deleted',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedIndex = new Index('parent', ['pid', 'deleted']);
$this->assertEquals($expectedIndex, $result[0]->getIndex('parent'));
}
Expand All @@ -406,7 +412,7 @@ public function enrichAddsParentKeyWithDisabled()
'disabled' => 'disabled',
],
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedIndex = new Index('parent', ['pid', 'disabled']);
$this->assertEquals($expectedIndex, $result[0]->getIndex('parent'));
}
Expand All @@ -422,7 +428,7 @@ public function enrichAddsParentKeyInCorrectOrder()
'disabled' => 'disabled',
],
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedIndex = new Index('parent', ['pid', 'deleted', 'disabled']);
$this->assertEquals($expectedIndex, $result[0]->getIndex('parent'));
}
Expand All @@ -435,7 +441,7 @@ public function enrichAddsSysLanguageUid()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'languageField' => 'sys_language_uid',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`sys_language_uid`',
Type::getType('integer'),
Expand All @@ -457,7 +463,7 @@ public function enrichAddsL10nParent()
'languageField' => 'sys_language_uid',
'transOrigPointerField' => 'l10n_parent',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`l10n_parent`',
Type::getType('integer'),
Expand All @@ -478,7 +484,7 @@ public function enrichDoesNotAddL10nParentIfLanguageFieldIsNotDefined()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'transOrigPointerField' => 'l10n_parent',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$this->expectException(SchemaException::class);
$result[0]->getColumn('l10n_parent');
}
Expand All @@ -491,7 +497,7 @@ public function enrichAddsDescription()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'descriptionColumn' => 'rowDescription',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`rowDescription`',
Type::getType('text'),
Expand All @@ -511,7 +517,7 @@ public function enrichAddsEditlock()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'editlock' => 'editlock'
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`editlock`',
Type::getType('smallint'),
Expand All @@ -533,7 +539,7 @@ public function enrichAddsL10nSource()
'languageField' => 'sys_language_uid',
'translationSource' => 'l10n_source',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`l10n_source`',
Type::getType('integer'),
Expand All @@ -554,7 +560,7 @@ public function enrichDoesNotAddL10nSourceIfLanguageFieldIsNotDefined()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'translationSource' => 'l10n_source',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$this->expectException(SchemaException::class);
$result[0]->getColumn('l10n_source');
}
Expand All @@ -568,7 +574,7 @@ public function enrichAddsL10nState()
'languageField' => 'sys_language_uid',
'transOrigPointerField' => 'l10n_parent',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`l10n_state`',
Type::getType('text'),
Expand All @@ -588,7 +594,7 @@ public function enrichDoesNotAddL10nStateIfLanguageFieldIsNotDefined()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'transOrigPointerField' => 'l10n_parent',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$this->expectException(SchemaException::class);
$result[0]->getColumn('l10n_state');
}
Expand All @@ -601,7 +607,7 @@ public function enrichDoesNotAddL10nStateIfTransOrigPointerFieldIsNotDefined()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'languageField' => 'sys_language_uid',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$this->expectException(SchemaException::class);
$result[0]->getColumn('l10n_state');
}
Expand All @@ -614,7 +620,7 @@ public function enrichAddsT3origUid()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'origUid' => 't3_origuid',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3_origuid`',
Type::getType('integer'),
Expand All @@ -635,7 +641,7 @@ public function enrichAddsL10nDiffsource()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'transOrigDiffSourceField' => 'l18n_diffsource',
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`l18n_diffsource`',
Type::getType('blob'),
Expand All @@ -655,7 +661,7 @@ public function enrichAddsT3verOid()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_oid`',
Type::getType('integer'),
Expand All @@ -676,7 +682,7 @@ public function enrichAddsT3verId()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_id`',
Type::getType('integer'),
Expand All @@ -697,7 +703,7 @@ public function enrichAddsT3verWsid()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_wsid`',
Type::getType('integer'),
Expand All @@ -718,7 +724,7 @@ public function enrichAddsT3verState()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_state`',
Type::getType('smallint'),
Expand All @@ -739,7 +745,7 @@ public function enrichAddsT3verStage()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_stage`',
Type::getType('integer'),
Expand All @@ -760,7 +766,7 @@ public function enrichAddsT3verCount()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_count`',
Type::getType('integer'),
Expand All @@ -781,7 +787,7 @@ public function enrichAddsT3verTstamp()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_tstamp`',
Type::getType('integer'),
Expand All @@ -802,7 +808,7 @@ public function enrichAddsT3verMoveId()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedColumn = new Column(
'`t3ver_move_id`',
Type::getType('integer'),
Expand All @@ -823,7 +829,7 @@ public function enrichAddsT3verOidIndex()
$GLOBALS['TCA']['aTable']['ctrl'] = [
'versioningWS' => true,
];
$result = $this->subject->enrich([]);
$result = $this->subject->enrich([$this->defaultTable]);
$expectedIndex = new Index('t3ver_oid', ['t3ver_oid', 't3ver_wsid']);
$this->assertEquals($expectedIndex, $result[0]->getIndex('t3ver_oid'));
}
Expand Down

0 comments on commit 31cc90f

Please sign in to comment.