From 7615c7e780bb49030d235a54de6383ced178b935 Mon Sep 17 00:00:00 2001 From: Mo Sureerat Date: Thu, 3 Nov 2022 01:05:11 +0700 Subject: [PATCH 1/5] [ISSUE-17793] Insert record - fix null not selected if nullable UUID + insert error Signed-off-by: Mo Sureerat --- libraries/classes/InsertEdit.php | 9 +- test/classes/InsertEditTest.php | 192 +++++++++++++++++++++++++------ 2 files changed, 165 insertions(+), 36 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 3e6ab3e411..cea65e632c 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -1106,7 +1106,14 @@ class InsertEdit private function getSpecialCharsAndBackupFieldForInsertingMode( array $column ) { - if (! isset($column['Default'])) { + $isNullableUUID = ! empty($column['True_Type']) + && ! empty($column['Default']) + && ! empty($column['Null']) + && $column['True_Type'] === 'uuid' + && $column['Default'] === 'uuid()' + && $column['Null'] === 'YES'; + + if ($isNullableUUID || (! isset($column['Default']))) { $column['Default'] = ''; $realNullValue = true; $data = ''; diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 36186fd19e..06bf72becf 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -1590,38 +1590,21 @@ class InsertEditTest extends AbstractTestCase /** * Test for getSpecialCharsAndBackupFieldForInsertingMode + * + * @param array $column Column parameters + * @param array $expected Expected result + * @psalm-param array $column + * @psalm-param array $expected + * + * @dataProvider providerForTestGetSpecialCharsAndBackupFieldForInsertingMode */ - public function testGetSpecialCharsAndBackupFieldForInsertingMode(): void - { - $column = []; - $column['True_Type'] = 'bit'; - $column['Default'] = 'b\'101\''; - $column['is_binary'] = true; + public function testGetSpecialCharsAndBackupFieldForInsertingMode( + array $column, + array $expected + ): void { $GLOBALS['cfg']['ProtectBinary'] = false; $GLOBALS['cfg']['ShowFunctionFields'] = true; - $result = $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getSpecialCharsAndBackupFieldForInsertingMode', - [$column] - ); - - $this->assertEquals( - [ - false, - 'b\'101\'', - '101', - '', - '101', - ], - $result - ); - - // case 2 - unset($column['Default']); - $column['True_Type'] = 'char'; - $result = (array) $this->callFunction( $this->insertEdit, InsertEdit::class, @@ -1630,17 +1613,156 @@ class InsertEditTest extends AbstractTestCase ); $this->assertEquals( - [ - true, - '', - '', - '', - '', - ], + $expected, $result ); } + /** + * Data provider for test getSpecialCharsAndBackupFieldForInsertingMode() + * + * @return array + * @psalm-return array, array}> + */ + public function providerForTestGetSpecialCharsAndBackupFieldForInsertingMode(): array + { + return [ + 'bit' => [ + [ + 'True_Type' => 'bit', + 'Default' => 'b\'101\'', + 'is_binary' => true, + ], + [ + false, + 'b\'101\'', + '101', + '', + '101', + ], + ], + 'char' => [ + [ + 'True_Type' => 'char', + 'is_binary' => true, + ], + [ + true, + '', + '', + '', + '', + ], + ], + 'time with CURRENT_TIMESTAMP value' => [ + [ + 'True_Type' => 'time', + 'Default' => 'CURRENT_TIMESTAMP', + ], + [ + false, + 'CURRENT_TIMESTAMP', + 'CURRENT_TIMESTAMP', + '', + 'CURRENT_TIMESTAMP', + ], + ], + 'time with current_timestamp() value' => [ + [ + 'True_Type' => 'time', + 'Default' => 'current_timestamp()', + ], + [ + false, + 'current_timestamp()', + 'current_timestamp()', + '', + 'current_timestamp()', + ], + ], + 'time with no dot value' => [ + [ + 'True_Type' => 'time', + 'Default' => '10', + ], + [ + false, + '10', + '10.000000', + '', + '10.000000', + ], + ], + 'time with dot value' => [ + [ + 'True_Type' => 'time', + 'Default' => '10.08', + ], + [ + false, + '10.08', + '10.080000', + '', + '10.080000', + ], + ], + 'any text with escape text default' => [ + [ + 'True_Type' => 'text', + 'Default' => '"lorem\"ipsem"', + ], + [ + false, + '"lorem\"ipsem"', + 'lorem"ipsem', + '', + 'lorem"ipsem', + ], + ], + 'varchar with html special chars' => [ + [ + 'True_Type' => 'varchar', + 'Default' => 'hello world
lorem ipsem', + ], + [ + false, + 'hello world
lorem ipsem', + 'hello world<br><b>lorem</b> ipsem', + '', + 'hello world<br><b>lorem</b> ipsem', + ], + ], + 'uuid with nullable' => [ + [ + 'True_Type' => 'uuid', + 'Default' => 'uuid()', + 'Null' => 'YES', + ], + [ + true, + '', + '', + '', + '', + ], + ], + 'uuid with not nullable' => [ + [ + 'True_Type' => 'uuid', + 'Default' => 'uuid()', + 'Null' => 'NO', + ], + [ + false, + 'uuid()', + 'uuid()', + '', + 'uuid()', + ], + ], + ]; + } + /** * Test for getParamsForUpdateOrInsert */ From afd568e8890dbd7518b3f589efe9c99b807daf0c Mon Sep 17 00:00:00 2001 From: Mo Sureerat Date: Thu, 3 Nov 2022 05:11:19 +0700 Subject: [PATCH 2/5] [ISSUE-17793] Revert previous change + Fix default selection of uuid in alter table Signed-off-by: Mo Sureerat --- libraries/classes/InsertEdit.php | 9 +----- libraries/classes/Table/ColumnsDefinition.php | 7 +++++ test/classes/InsertEditTest.php | 28 ------------------- 3 files changed, 8 insertions(+), 36 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index cea65e632c..3e6ab3e411 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -1106,14 +1106,7 @@ class InsertEdit private function getSpecialCharsAndBackupFieldForInsertingMode( array $column ) { - $isNullableUUID = ! empty($column['True_Type']) - && ! empty($column['Default']) - && ! empty($column['Null']) - && $column['True_Type'] === 'uuid' - && $column['Default'] === 'uuid()' - && $column['Null'] === 'YES'; - - if ($isNullableUUID || (! isset($column['Default']))) { + if (! isset($column['Default'])) { $column['Default'] = ''; $realNullValue = true; $data = ''; diff --git a/libraries/classes/Table/ColumnsDefinition.php b/libraries/classes/Table/ColumnsDefinition.php index 37b1dba35f..d5e6d7ef9b 100644 --- a/libraries/classes/Table/ColumnsDefinition.php +++ b/libraries/classes/Table/ColumnsDefinition.php @@ -246,6 +246,8 @@ final class ColumnsDefinition case 'NULL': case 'CURRENT_TIMESTAMP': case 'current_timestamp()': + case 'UUID': + case 'uuid()': $columnMeta['Default'] = $columnMeta['DefaultType']; break; } @@ -303,6 +305,11 @@ final class ColumnsDefinition $columnMeta['DefaultType'] = 'CURRENT_TIMESTAMP'; $columnMeta['DefaultValue'] = ''; break; + case 'UUID': + case 'uuid()': + $columnMeta['DefaultType'] = 'UUID'; + $columnMeta['DefaultValue'] = ''; + break; default: $columnMeta['DefaultType'] = 'USER_DEFINED'; $columnMeta['DefaultValue'] = $columnMeta['Default']; diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 06bf72becf..6bbe885c12 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -1732,34 +1732,6 @@ class InsertEditTest extends AbstractTestCase 'hello world<br><b>lorem</b> ipsem', ], ], - 'uuid with nullable' => [ - [ - 'True_Type' => 'uuid', - 'Default' => 'uuid()', - 'Null' => 'YES', - ], - [ - true, - '', - '', - '', - '', - ], - ], - 'uuid with not nullable' => [ - [ - 'True_Type' => 'uuid', - 'Default' => 'uuid()', - 'Null' => 'NO', - ], - [ - false, - 'uuid()', - 'uuid()', - '', - 'uuid()', - ], - ], ]; } From 250be6adbc97636b8c5cf9758b5edf7d0a3da609 Mon Sep 17 00:00:00 2001 From: Mo Sureerat Date: Thu, 3 Nov 2022 22:56:29 +0700 Subject: [PATCH 3/5] [ISSUE-17793] Fix static analysis warning Signed-off-by: Mo Sureerat --- libraries/classes/Table/ColumnsDefinition.php | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/libraries/classes/Table/ColumnsDefinition.php b/libraries/classes/Table/ColumnsDefinition.php index d5e6d7ef9b..1a96a024ec 100644 --- a/libraries/classes/Table/ColumnsDefinition.php +++ b/libraries/classes/Table/ColumnsDefinition.php @@ -84,7 +84,7 @@ final class ColumnsDefinition ] ); if (isset($_POST['field_where'])) { - $form_params['after_field'] = $_POST['after_field']; + $form_params['after_field'] = (string) $_POST['after_field']; } } @@ -284,16 +284,15 @@ final class ColumnsDefinition $columnMeta['Expression'] = is_array($expressions) ? $expressions[$columnMeta['Field']] : null; } + $columnMeta['DefaultType'] = 'USER_DEFINED'; + $columnMeta['DefaultValue'] = ''; + switch ($columnMeta['Default']) { case null: if ($columnMeta['Default'] === null) { - if ($columnMeta['Null'] === 'YES') { - $columnMeta['DefaultType'] = 'NULL'; - $columnMeta['DefaultValue'] = ''; - } else { - $columnMeta['DefaultType'] = 'NONE'; - $columnMeta['DefaultValue'] = ''; - } + $columnMeta['DefaultType'] = $columnMeta['Null'] === 'YES' + ? 'NULL' + : 'NONE'; } else { // empty $columnMeta['DefaultType'] = 'USER_DEFINED'; $columnMeta['DefaultValue'] = $columnMeta['Default']; @@ -303,15 +302,14 @@ final class ColumnsDefinition case 'CURRENT_TIMESTAMP': case 'current_timestamp()': $columnMeta['DefaultType'] = 'CURRENT_TIMESTAMP'; - $columnMeta['DefaultValue'] = ''; + break; case 'UUID': case 'uuid()': $columnMeta['DefaultType'] = 'UUID'; - $columnMeta['DefaultValue'] = ''; + break; default: - $columnMeta['DefaultType'] = 'USER_DEFINED'; $columnMeta['DefaultValue'] = $columnMeta['Default']; if (substr($columnMeta['Type'], -4) === 'text') { From 8929288efd592047c81df3b9b8b8a8151edaac41 Mon Sep 17 00:00:00 2001 From: Mo Sureerat Date: Fri, 4 Nov 2022 00:33:41 +0700 Subject: [PATCH 4/5] [ISSUE-17793] Add more test for coverage Signed-off-by: Mo Sureerat --- libraries/classes/Table/ColumnsDefinition.php | 83 +++++++------ test/classes/Table/ColumnsDefinitionTest.php | 112 ++++++++++++++++++ 2 files changed, 160 insertions(+), 35 deletions(-) create mode 100644 test/classes/Table/ColumnsDefinitionTest.php diff --git a/libraries/classes/Table/ColumnsDefinition.php b/libraries/classes/Table/ColumnsDefinition.php index 1a96a024ec..b0a5d9342e 100644 --- a/libraries/classes/Table/ColumnsDefinition.php +++ b/libraries/classes/Table/ColumnsDefinition.php @@ -284,41 +284,8 @@ final class ColumnsDefinition $columnMeta['Expression'] = is_array($expressions) ? $expressions[$columnMeta['Field']] : null; } - $columnMeta['DefaultType'] = 'USER_DEFINED'; - $columnMeta['DefaultValue'] = ''; - - switch ($columnMeta['Default']) { - case null: - if ($columnMeta['Default'] === null) { - $columnMeta['DefaultType'] = $columnMeta['Null'] === 'YES' - ? 'NULL' - : 'NONE'; - } else { // empty - $columnMeta['DefaultType'] = 'USER_DEFINED'; - $columnMeta['DefaultValue'] = $columnMeta['Default']; - } - - break; - case 'CURRENT_TIMESTAMP': - case 'current_timestamp()': - $columnMeta['DefaultType'] = 'CURRENT_TIMESTAMP'; - - break; - case 'UUID': - case 'uuid()': - $columnMeta['DefaultType'] = 'UUID'; - - break; - default: - $columnMeta['DefaultValue'] = $columnMeta['Default']; - - if (substr($columnMeta['Type'], -4) === 'text') { - $textDefault = substr($columnMeta['Default'], 1, -1); - $columnMeta['Default'] = stripcslashes($textDefault); - } - - break; - } + $columnMetaDefault = self::decorateColumnMetaDefault($columnMeta); + $columnMeta = array_merge($columnMeta, $columnMetaDefault); } if (isset($columnMeta['Type'])) { @@ -527,4 +494,50 @@ final class ColumnsDefinition 'disable_is' => $cfg['Server']['DisableIS'], ]; } + + /** + * Set default type and default value according to the column metadata + * + * @param array $columnMeta Column Metadata + * @phpstan-param array $columnMeta + * + * @return array + */ + public static function decorateColumnMetaDefault(array $columnMeta): array + { + $metaDefault['DefaultType'] = 'USER_DEFINED'; + $metaDefault['DefaultValue'] = ''; + + switch ($columnMeta['Default']) { + case null: + if ($columnMeta['Null'] === 'YES') { + $metaDefault['DefaultType'] = 'NULL'; + } else { + $metaDefault['DefaultType'] = 'NONE'; + } + + break; + case 'CURRENT_TIMESTAMP': + case 'current_timestamp()': + $metaDefault['DefaultType'] = 'CURRENT_TIMESTAMP'; + + break; + case 'UUID': + case 'uuid()': + $metaDefault['DefaultType'] = 'UUID'; + + break; + default: + $metaDefault['DefaultValue'] = $columnMeta['Default']; + + if (substr($columnMeta['Type'], -4) === 'text') { + $textDefault = substr($columnMeta['Default'], 1, -1); + $metaDefault['Default'] = stripcslashes($textDefault); + } + + break; + } + + return $metaDefault; + } } diff --git a/test/classes/Table/ColumnsDefinitionTest.php b/test/classes/Table/ColumnsDefinitionTest.php new file mode 100644 index 0000000000..e972a22b8a --- /dev/null +++ b/test/classes/Table/ColumnsDefinitionTest.php @@ -0,0 +1,112 @@ + $columnMeta + * @phpstan-param array $expected + * + * @dataProvider providerColumnMetaDefault + */ + public function testDecorateColumnMetaDefault(array $columnMeta, array $expected): void + { + $result = ColumnsDefinition::decorateColumnMetaDefault($columnMeta); + + $this->assertEquals($expected, $result); + } + + /** + * Data provider for testDecorateColumnMetaDefault + * + * @return array + * @psalm-return array, array}> + */ + public function providerColumnMetaDefault(): array + { + return [ + 'when Default is null and Null is YES' => [ + [ + 'Default' => null, + 'Null' => 'YES', + ], + [ + 'DefaultType' => 'NULL', + 'DefaultValue' => '', + ], + ], + 'when Default is null and Null is NO' => [ + [ + 'Default' => null, + 'Null' => 'NO', + ], + [ + 'DefaultType' => 'NONE', + 'DefaultValue' => '', + ], + ], + 'when Default is CURRENT_TIMESTAMP' => [ + ['Default' => 'CURRENT_TIMESTAMP'], + [ + 'DefaultType' => 'CURRENT_TIMESTAMP', + 'DefaultValue' => '', + ], + ], + 'when Default is current_timestamp' => [ + ['Default' => 'current_timestamp()'], + [ + 'DefaultType' => 'CURRENT_TIMESTAMP', + 'DefaultValue' => '', + ], + ], + 'when Default is UUID' => [ + ['Default' => 'UUID'], + [ + 'DefaultType' => 'UUID', + 'DefaultValue' => '', + ], + ], + 'when Default is uuid()' => [ + ['Default' => 'uuid()'], + [ + 'DefaultType' => 'UUID', + 'DefaultValue' => '', + ], + ], + 'when Default is anything else and Type is text' => [ + [ + 'Default' => '"some\/thing"', + 'Type' => 'text', + ], + [ + 'Default' => 'some/thing', + 'DefaultType' => 'USER_DEFINED', + 'DefaultValue' => '"some\/thing"', + ], + ], + 'when Default is anything else and Type is not text' => [ + [ + 'Default' => '"some\/thing"', + 'Type' => 'something', + ], + [ + 'DefaultType' => 'USER_DEFINED', + 'DefaultValue' => '"some\/thing"', + ], + ], + ]; + } +} From a36ee101e7567cc6b7253d695fa1a614a501f58c Mon Sep 17 00:00:00 2001 From: Mo Sureerat Date: Fri, 4 Nov 2022 00:44:28 +0700 Subject: [PATCH 5/5] [ISSUE-17793] Remove unreachable condition Signed-off-by: Mo Sureerat --- libraries/classes/Table/ColumnsDefinition.php | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/libraries/classes/Table/ColumnsDefinition.php b/libraries/classes/Table/ColumnsDefinition.php index b0a5d9342e..1fb5117ed2 100644 --- a/libraries/classes/Table/ColumnsDefinition.php +++ b/libraries/classes/Table/ColumnsDefinition.php @@ -338,14 +338,10 @@ final class ColumnsDefinition } // old column type - if (isset($columnMeta['Type'])) { - // keep in uppercase because the new type will be in uppercase - $form_params['field_type_orig[' . $columnNumber . ']'] = mb_strtoupper($type); - if (isset($columnMeta['column_status']) && ! $columnMeta['column_status']['isEditable']) { - $form_params['field_type[' . $columnNumber . ']'] = mb_strtoupper($type); - } - } else { - $form_params['field_type_orig[' . $columnNumber . ']'] = ''; + // keep in uppercase because the new type will be in uppercase + $form_params['field_type_orig[' . $columnNumber . ']'] = mb_strtoupper($type); + if (isset($columnMeta['column_status']) && ! $columnMeta['column_status']['isEditable']) { + $form_params['field_type[' . $columnNumber . ']'] = mb_strtoupper($type); } // old column length @@ -411,9 +407,11 @@ final class ColumnsDefinition } if ($type_upper === 'BIT') { - $default_value = Util::convertBitDefaultValue($columnMeta['DefaultValue']); + $default_value = ! empty($columnMeta['DefaultValue']) + ? Util::convertBitDefaultValue($columnMeta['DefaultValue']) + : ''; } elseif ($type_upper === 'BINARY' || $type_upper === 'VARBINARY') { - $default_value = bin2hex($columnMeta['DefaultValue']); + $default_value = bin2hex((string) $columnMeta['DefaultValue']); } $content_cells[$columnNumber] = [ @@ -501,12 +499,14 @@ final class ColumnsDefinition * @param array $columnMeta Column Metadata * @phpstan-param array $columnMeta * - * @return array + * @return non-empty-array */ public static function decorateColumnMetaDefault(array $columnMeta): array { - $metaDefault['DefaultType'] = 'USER_DEFINED'; - $metaDefault['DefaultValue'] = ''; + $metaDefault = [ + 'DefaultType' => 'USER_DEFINED', + 'DefaultValue' => '', + ]; switch ($columnMeta['Default']) { case null: @@ -530,7 +530,7 @@ final class ColumnsDefinition default: $metaDefault['DefaultValue'] = $columnMeta['Default']; - if (substr($columnMeta['Type'], -4) === 'text') { + if (substr((string) $columnMeta['Type'], -4) === 'text') { $textDefault = substr($columnMeta['Default'], 1, -1); $metaDefault['Default'] = stripcslashes($textDefault); }