Skip to content

Allow special default values for certain columns #291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bennothommo merged 6 commits into from
Jul 17, 2019
Merged

Allow special default values for certain columns #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9413e5b
Allow special default values for certain columns
bennothommo Jul 16, 2019
19eed33
Fix misplaced bracket
bennothommo Jul 16, 2019
b179fb7
Remove repeated "not null" check on column
bennothommo Jul 16, 2019
e76f56e
Display `null` in editor if field is nulled
bennothommo Jul 17, 2019
a6ca84d
Move null conversion to schema creator
bennothommo Jul 17, 2019
5294c83
Skip validation for nullable columns if `null`
bennothommo Jul 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions classes/DatabaseTableModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static function listPluginTables($pluginCode)

$tables = self::getSchemaManager()->listTableNames();

return array_filter($tables, function($item) use($prefix) {
return array_filter($tables, function ($item) use ($prefix) {
return Str::startsWith($item, $prefix);
});
}
Expand Down Expand Up @@ -104,7 +104,7 @@ public function validate()
'name.max' => Lang::get('rainlab.builder::lang.database.error_table_name_too_long')
];

Validator::extend('tablePrefix', function($attribute, $value, $parameters) use ($prefix) {
Validator::extend('tablePrefix', function ($attribute, $value, $parameters) use ($prefix) {
$value = trim($value);

if (!Str::startsWith($value, $prefix)) {
Expand All @@ -114,7 +114,7 @@ public function validate()
return true;
});

Validator::extend('uniqueTableName', function($attribute, $value, $parameters) {
Validator::extend('uniqueTableName', function ($attribute, $value, $parameters) {
$value = trim($value);

$schema = $this->getSchema();
Expand Down Expand Up @@ -189,7 +189,8 @@ protected function validateColumnNameLengths()

if (Str::length($name) > 64) {
throw new ValidationException([
'columns' => Lang::get('rainlab.builder::lang.database.error_column_name_too_long',
'columns' => Lang::get(
'rainlab.builder::lang.database.error_column_name_too_long',
['column' => $name]
)
]);
Expand All @@ -199,11 +200,12 @@ protected function validateColumnNameLengths()

protected function validateDuplicateColumns()
{
foreach ($this->columns as $outerIndex=>$outerColumn) {
foreach ($this->columns as $innerIndex=>$innerColumn) {
foreach ($this->columns as $outerIndex => $outerColumn) {
foreach ($this->columns as $innerIndex => $innerColumn) {
if ($innerIndex != $outerIndex && $innerColumn['name'] == $outerColumn['name']) {
throw new ValidationException([
'columns' => Lang::get('rainlab.builder::lang.database.error_table_duplicate_column',
'columns' => Lang::get(
'rainlab.builder::lang.database.error_table_duplicate_column',
['column' => $outerColumn['name']]
)
]);
Expand Down Expand Up @@ -281,8 +283,7 @@ protected function validateColumnsLengthParameter()
foreach ($this->columns as $column) {
try {
MigrationColumnType::validateLength($column['type'], $column['length']);
}
catch (Exception $ex) {
} catch (Exception $ex) {
throw new ValidationException([
'columns' => $ex->getMessage()
]);
Expand All @@ -296,6 +297,10 @@ protected function validateDefaultValues()
if (!strlen($column['default'])) {
continue;
}
// Allow null value for all nullable columns
if (strtolower($column['default']) === 'null' && (bool) $column['allow_null'] === true) {
continue;
}

$default = trim($column['default']);

Expand Down Expand Up @@ -326,7 +331,7 @@ protected function validateDefaultValues()
}

if ($column['type'] == MigrationColumnType::TYPE_BOOLEAN) {
if (!preg_match('/^0|1$/', $default)) {
if (!preg_match('/^0|1|true|false$/i', $default)) {
throw new ValidationException([
'columns' => Lang::get('rainlab.builder::lang.database.error_boolean_default_value', ['column'=>$column['name']])
]);
Expand Down Expand Up @@ -383,6 +388,15 @@ protected function loadColumnsFromTableInfo()
'id' => $columnName,
];

// Format quoted "null" values with quotes
if ($column->getNotnull() === false) {
if ($item['default'] === null) {
$item['default'] = 'null';
} elseif (strtolower($item['default']) === 'null') {
$item['default'] = "'null'";
}
}

$this->columns[] = $item;
}
}
Expand Down
16 changes: 13 additions & 3 deletions classes/DatabaseTableSchemaCreator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,22 @@ protected function formatOptions($type, $options)

$default = trim($options['default']);

// Note - this code doesn't allow to set empty string as default.
// Note - this code doesn't allow to set empty string as default.
// But converting empty strings to NULLs is required for the further
// work with Doctrine types. As an option - empty strings could be specified
// as '' in the editor UI (table column editor).
$result['default'] = $default === '' ? null : $default;
if ($result['notnull'] === false) {
if (strtolower($default) === 'null') {
$result['default'] = null;
} elseif (preg_match('/^[\'"]null[\'"]$/i', $default)) {
$result['default'] = 'null';
} else {
$result['default'] = $default === '' ? null : $default;
}
} else {
$result['default'] = $default === '' ? null : $default;
}

return $result;
}
}
}
58 changes: 26 additions & 32 deletions classes/TableMigrationCodeGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ class TableMigrationCodeGenerator extends BaseModel
* Generates code for creating or updating a database table.
* @param \Doctrine\DBAL\Schema\Table $updatedTable Specifies the updated table schema.
* @param \Doctrine\DBAL\Schema\Table $existingTable Specifies the existing table schema, if applicable.
* @param string $newTableName An updated name of the theme.
* @return string|boolean Returns the migration up() and down() methods code.
* @param string $newTableName An updated name of the theme.
* @return string|boolean Returns the migration up() and down() methods code.
* Returns false if there the table was not changed.
*/
public function createOrUpdateTable($updatedTable, $existingTable, $newTableName)
Expand All @@ -48,8 +48,7 @@ public function createOrUpdateTable($updatedTable, $existingTable, $newTableName

$tableDiff->newName = $newTableName;
}
}
else {
} else {
/*
* The table doesn't exist
*/
Expand Down Expand Up @@ -99,7 +98,7 @@ public function wrapMigrationCode($scriptFilename, $code, $pluginCodeObj)
/**
* Generates code for dropping a database table.
* @param \Doctrine\DBAL\Schema\Table $existingTable Specifies the existing table schema.
* @return string Returns the migration up() and down() methods code.
* @return string Returns the migration up() and down() methods code.
*/
public function dropTable($existingTable)
{
Expand All @@ -113,7 +112,7 @@ protected function generateCreateOrUpdateCode($tableDiff, $isNewTable, $newOrUpd
{
/*
* Although it might seem that a reverse diff could be used
* for the down() method, that's not so. The up and down operations
* for the down() method, that's not so. The up and down operations
* are not fully symmetrical.
*/

Expand Down Expand Up @@ -171,16 +170,16 @@ protected function generateCreateOrUpdateUpCode($tableDiff, $isNewTable, $newOrU
$result .= $this->generateColumnCode($columnDiff, self::COLUMN_MODE_CHANGE);
}

foreach ($tableDiff->renamedColumns as $oldName=>$column) {
foreach ($tableDiff->renamedColumns as $oldName => $column) {
$result .= $this->generateColumnRenameCode($oldName, $column->getName());
}

foreach ($tableDiff->removedColumns as $name=>$column) {
foreach ($tableDiff->removedColumns as $name => $column) {
$result .= $this->generateColumnRemoveCode($name);
}

$primaryKey = $changedPrimaryKey ?
$this->findPrimaryKeyIndex($tableDiff->changedIndexes, $newOrUpdatedTable) :
$this->findPrimaryKeyIndex($tableDiff->changedIndexes, $newOrUpdatedTable) :
$this->findPrimaryKeyIndex($tableDiff->addedIndexes, $newOrUpdatedTable);

if ($primaryKey) {
Expand All @@ -198,8 +197,7 @@ protected function generateCreateOrUpdateDownCode($tableDiff, $isNewTable, $newO

if ($isNewTable) {
$result = $this->generateTableDropCode($tableDiff->name);
}
else {
} else {
$changedPrimaryKey = $this->getChangedOrRemovedPrimaryKey($tableDiff);
$addedPrimaryKey = $this->findPrimaryKeyIndex($tableDiff->addedIndexes, $newOrUpdatedTable);

Expand Down Expand Up @@ -232,11 +230,11 @@ protected function generateCreateOrUpdateDownCode($tableDiff, $isNewTable, $newO
$result .= $this->generateColumnCode($columnDiff, self::COLUMN_MODE_REVERT);
}

foreach ($tableDiff->renamedColumns as $oldName=>$column) {
foreach ($tableDiff->renamedColumns as $oldName => $column) {
$result .= $this->generateColumnRenameCode($column->getName(), $oldName);
}

foreach ($tableDiff->removedColumns as $name=>$column) {
foreach ($tableDiff->removedColumns as $name => $column) {
$result .= $this->generateColumnCode($column, self::COLUMN_MODE_CREATE);
}

Expand Down Expand Up @@ -361,22 +359,22 @@ protected function generateColumnCode($columnData, $mode)
$forceFlagsChange = false;

switch ($mode) {
case self::COLUMN_MODE_CREATE:
case self::COLUMN_MODE_CREATE:
$column = $columnData;
$changeMode = false;
break;
case self::COLUMN_MODE_CHANGE:
break;
case self::COLUMN_MODE_CHANGE:
$column = $columnData->column;
$changeMode = true;

$forceFlagsChange = in_array('type', $columnData->changedProperties);
break;
case self::COLUMN_MODE_REVERT:
break;
case self::COLUMN_MODE_REVERT:
$column = $columnData->fromColumn;
$changeMode = true;

$forceFlagsChange = in_array('type', $columnData->changedProperties);
break;
break;
}

$result = $this->generateColumnMethodCall($column);
Expand Down Expand Up @@ -433,8 +431,7 @@ protected function generateNullable($column, $changeMode, $columnData, $forceFla
if (!$column->getNotnull()) {
$result = $this->generateBooleanMethod('nullable', true);
}
}
elseif (in_array('notnull', $columnData->changedProperties) || $forceFlagsChange) {
} elseif (in_array('notnull', $columnData->changedProperties) || $forceFlagsChange) {
$result = $this->generateBooleanMethod('nullable', !$column->getNotnull());
}

Expand All @@ -449,8 +446,7 @@ protected function generateUnsigned($column, $changeMode, $columnData, $forceFla
if ($column->getUnsigned()) {
$result = $this->generateBooleanMethod('unsigned', true);
}
}
elseif (in_array('unsigned', $columnData->changedProperties) || $forceFlagsChange) {
} elseif (in_array('unsigned', $columnData->changedProperties) || $forceFlagsChange) {
$result = $this->generateBooleanMethod('unsigned', $column->getUnsigned());
}

Expand All @@ -470,12 +466,10 @@ protected function generateDefault($column, $changeMode, $columnData, $forceFlag
if (strlen($default)) {
$result = $this->generateDefaultMethodCall($default, $column);
}
}
elseif (in_array('default', $columnData->changedProperties) || $forceFlagsChange) {
} elseif (in_array('default', $columnData->changedProperties) || $forceFlagsChange) {
if (strlen($default)) {
$result = $this->generateDefaultMethodCall($default, $column);
}
elseif ($changeMode) {
} elseif ($changeMode) {
$result = sprintf('->default(null)');
}
}
Expand All @@ -490,7 +484,7 @@ protected function generateDefaultMethodCall($default, $column)

$type = MigrationColumnType::toMigrationMethodName($typeName, $columnName);

if (in_array($type, MigrationColumnType::getIntegerTypes()) ||
if (in_array($type, MigrationColumnType::getIntegerTypes()) ||
in_array($type, MigrationColumnType::getDecimalTypes()) ||
$type == MigrationColumnType::TYPE_BOOLEAN) {
return sprintf('->default(%s)', $default);
Expand Down Expand Up @@ -548,9 +542,9 @@ protected function implodeColumnList($columnNames)

protected function tableHasNameOrColumnChanges($tableDiff, $columnChangesOnly = false)
{
$result = $tableDiff->addedColumns
|| $tableDiff->changedColumns
|| $tableDiff->removedColumns
$result = $tableDiff->addedColumns
|| $tableDiff->changedColumns
|| $tableDiff->removedColumns
|| $tableDiff->renamedColumns;

if ($columnChangesOnly) {
Expand All @@ -562,7 +556,7 @@ protected function tableHasNameOrColumnChanges($tableDiff, $columnChangesOnly =

protected function tableHasPrimaryKeyChanges($tableDiff)
{
return $this->findPrimaryKeyIndex($tableDiff->addedIndexes, $tableDiff->fromTable) ||
return $this->findPrimaryKeyIndex($tableDiff->addedIndexes, $tableDiff->fromTable) ||
$this->findPrimaryKeyIndex($tableDiff->changedIndexes, $tableDiff->fromTable) ||
$this->findPrimaryKeyIndex($tableDiff->removedIndexes, $tableDiff->fromTable);
}
Expand Down
2 changes: 1 addition & 1 deletion lang/en/lang.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
'error_unsigned_type_not_int' => "Error in the ':column' column. The Unsigned flag can be applied only to integer type columns.",
'error_integer_default_value' => "Invalid default value for the integer column ':column'. The allowed formats are '10', '-10'.",
'error_decimal_default_value' => "Invalid default value for the decimal or double column ':column'. The allowed formats are '1.00', '-1.00'.",
'error_boolean_default_value' => "Invalid default value for the boolean column ':column'. The allowed values are '0' and '1'.",
'error_boolean_default_value' => "Invalid default value for the boolean column ':column'. The allowed values are '0' and '1', or 'true' and 'false'.",
'error_unsigned_negative_value' => "The default value for the unsigned column ':column' can't be negative.",
'error_table_already_exists' => "The table ':name' already exists in the database.",
'error_table_name_too_long' => "The table name should not be longer than 64 characters.",
Expand Down