From dc04987a68a9656b82c5c664ae5e72b2e5ce6b41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 15 Feb 2018 12:31:04 -0200 Subject: [PATCH 1/3] Replace static methods with instance methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- .../Table/TableStructureController.php | 9 ++- libraries/classes/CreateAddField.php | 62 +++++++++---------- tbl_addfield.php | 15 +++-- tbl_create.php | 6 +- test/classes/CreateAddFieldTest.php | 15 +++-- 5 files changed, 62 insertions(+), 45 deletions(-) diff --git a/libraries/classes/Controllers/Table/TableStructureController.php b/libraries/classes/Controllers/Table/TableStructureController.php index 4d7b52b647..850a0ed92b 100644 --- a/libraries/classes/Controllers/Table/TableStructureController.php +++ b/libraries/classes/Controllers/Table/TableStructureController.php @@ -68,6 +68,11 @@ class TableStructureController extends TableController */ protected $_showtable; + /** + * @var CreateAddField + */ + private $createAddField; + /** * TableStructureController constructor * @@ -114,6 +119,8 @@ class TableStructureController extends TableController $this->_tbl_collation = $tbl_collation; $this->_showtable = $showtable; $this->table_obj = $this->dbi->getTable($this->db, $this->table); + + $this->createAddField = new CreateAddField(); } /** @@ -725,7 +732,7 @@ class TableStructureController extends TableController protected function updatePartitioning() { $sql_query = "ALTER TABLE " . Util::backquote($this->table) . " " - . CreateAddField::getPartitionsDefinition(); + . $this->createAddField->getPartitionsDefinition(); // Execute alter query $result = $this->dbi->tryQuery($sql_query); diff --git a/libraries/classes/CreateAddField.php b/libraries/classes/CreateAddField.php index 3939452cb1..c7b7fead5c 100644 --- a/libraries/classes/CreateAddField.php +++ b/libraries/classes/CreateAddField.php @@ -1,7 +1,7 @@ getStatementPrefix($is_create_tbl) . Table::generateFieldSpec( trim($_REQUEST['field_name'][$i]), $_REQUEST['field_type'][$i], @@ -89,7 +89,7 @@ class CreateAddField : '' ); - $definition .= self::setColumnCreationStatementSuffix($i, $prev_field, $is_create_tbl); + $definition .= $this->setColumnCreationStatementSuffix($i, $prev_field, $is_create_tbl); $prev_field = $i; $definitions[] = $definition; } // end for @@ -107,7 +107,7 @@ class CreateAddField * * @return string $sql_suffix suffix */ - private static function setColumnCreationStatementSuffix($current_field_num, $prev_field, + private function setColumnCreationStatementSuffix($current_field_num, $prev_field, $is_create_tbl = true ) { // no suffix is needed if request is a table creation @@ -149,7 +149,7 @@ class CreateAddField * * @return array an array of sql statements for indexes */ - private static function buildIndexStatements(array $index, $index_choice, + private function buildIndexStatements(array $index, $index_choice, $is_create_tbl = true ) { $statement = array(); @@ -157,7 +157,7 @@ class CreateAddField return $statement; } - $sql_query = self::getStatementPrefix($is_create_tbl) + $sql_query = $this->getStatementPrefix($is_create_tbl) . ' ' . $index_choice; if (! empty($index['Key_name']) && $index['Key_name'] != 'PRIMARY') { @@ -208,14 +208,14 @@ class CreateAddField } /** - * Statement prefix for the self::buildColumnCreationStatement() + * Statement prefix for the buildColumnCreationStatement() * * @param boolean $is_create_tbl true if requirement is to get the statement * for table creation * * @return string $sql_prefix prefix */ - private static function getStatementPrefix($is_create_tbl = true) + private function getStatementPrefix($is_create_tbl = true) { $sql_prefix = " "; if (! $is_create_tbl) { @@ -235,11 +235,11 @@ class CreateAddField * * @return array $index_definitions */ - private static function mergeIndexStatements( + private function mergeIndexStatements( array $definitions, $is_create_tbl, array $indexed_columns, $index_keyword ) { foreach ($indexed_columns as $index) { - $statements = self::buildIndexStatements( + $statements = $this->buildIndexStatements( $index, " " . $index_keyword . " ", $is_create_tbl ); $definitions = array_merge($definitions, $statements); @@ -256,18 +256,18 @@ class CreateAddField * * @return string sql statement */ - private static function getColumnCreationStatements($is_create_tbl = true) + private function getColumnCreationStatements($is_create_tbl = true) { $sql_statement = ""; list($field_cnt, $field_primary, $field_index, $field_unique, $field_fulltext, $field_spatial - ) = self::getIndexedColumns(); - $definitions = self::buildColumnCreationStatement( + ) = $this->getIndexedColumns(); + $definitions = $this->buildColumnCreationStatement( $field_cnt, $is_create_tbl ); // Builds the PRIMARY KEY statements - $primary_key_statements = self::buildIndexStatements( + $primary_key_statements = $this->buildIndexStatements( isset($field_primary[0]) ? $field_primary[0] : array(), " PRIMARY KEY ", $is_create_tbl @@ -275,22 +275,22 @@ class CreateAddField $definitions = array_merge($definitions, $primary_key_statements); // Builds the INDEX statements - $definitions = self::mergeIndexStatements( + $definitions = $this->mergeIndexStatements( $definitions, $is_create_tbl, $field_index, "INDEX" ); // Builds the UNIQUE statements - $definitions = self::mergeIndexStatements( + $definitions = $this->mergeIndexStatements( $definitions, $is_create_tbl, $field_unique, "UNIQUE" ); // Builds the FULLTEXT statements - $definitions = self::mergeIndexStatements( + $definitions = $this->mergeIndexStatements( $definitions, $is_create_tbl, $field_fulltext, "FULLTEXT" ); // Builds the SPATIAL statements - $definitions = self::mergeIndexStatements( + $definitions = $this->mergeIndexStatements( $definitions, $is_create_tbl, $field_spatial, "SPATIAL" ); @@ -308,7 +308,7 @@ class CreateAddField * * @return string partitioning clause */ - public static function getPartitionsDefinition() + public function getPartitionsDefinition() { $sql_query = ""; if (! empty($_REQUEST['partition_by']) @@ -335,7 +335,7 @@ class CreateAddField $i = 0; $partitions = array(); foreach ($_REQUEST['partitions'] as $partition) { - $partitions[] = self::getPartitionDefinition($partition); + $partitions[] = $this->getPartitionDefinition($partition); $i++; } $sql_query .= " (" . implode(", ", $partitions) . ")"; @@ -352,7 +352,7 @@ class CreateAddField * * @return string partition/subpartition definition */ - private static function getPartitionDefinition(array $partition, $isSubPartition = false) + private function getPartitionDefinition(array $partition, $isSubPartition = false) { $sql_query = " " . ($isSubPartition ? "SUB" : "") . "PARTITION "; $sql_query .= $partition['name']; @@ -394,7 +394,7 @@ class CreateAddField $j = 0; $subpartitions = array(); foreach ($partition['subpartitions'] as $subpartition) { - $subpartitions[] = self::getPartitionDefinition( + $subpartitions[] = $this->getPartitionDefinition( $subpartition, true ); @@ -414,10 +414,10 @@ class CreateAddField * * @return string */ - public static function getTableCreationQuery($db, $table) + public function getTableCreationQuery($db, $table) { // get column addition statements - $sql_statement = self::getColumnCreationStatements(true); + $sql_statement = $this->getColumnCreationStatements(true); // Builds the 'create table' statement $sql_query = 'CREATE TABLE ' . Util::backquote($db) . '.' @@ -443,7 +443,7 @@ class CreateAddField $sql_query .= ' COMMENT = \'' . $GLOBALS['dbi']->escapeString($_REQUEST['comment']) . '\''; } - $sql_query .= self::getPartitionsDefinition(); + $sql_query .= $this->getPartitionsDefinition(); $sql_query .= ';'; return $sql_query; @@ -454,7 +454,7 @@ class CreateAddField * * @return int */ - public static function getNumberOfFieldsFromRequest() + public function getNumberOfFieldsFromRequest() { if (isset($_REQUEST['submit_num_fields'])) { // adding new fields $num_fields = intval($_REQUEST['orig_num_fields']) + intval($_REQUEST['added_fields']); @@ -481,10 +481,10 @@ class CreateAddField * * @return array */ - public static function tryColumnCreationQuery($db, $table, $err_url) + public function tryColumnCreationQuery($db, $table, $err_url) { // get column addition statements - $sql_statement = self::getColumnCreationStatements(false); + $sql_statement = $this->getColumnCreationStatements(false); // To allow replication, we first select the db to use and then run queries // on this db. diff --git a/tbl_addfield.php b/tbl_addfield.php index 159090a0e9..c364595667 100644 --- a/tbl_addfield.php +++ b/tbl_addfield.php @@ -7,9 +7,11 @@ */ use PhpMyAdmin\CreateAddField; +use PhpMyAdmin\Message; use PhpMyAdmin\Response; use PhpMyAdmin\Transformations; use PhpMyAdmin\Url; +use PhpMyAdmin\Util; /** * Get some core libraries @@ -22,8 +24,7 @@ $scripts = $header->getScripts(); $scripts->addFile('tbl_structure.js'); // Check parameters -PhpMyAdmin\Util::checkParameters(array('db', 'table')); - +Util::checkParameters(array('db', 'table')); /** * Defines the url to return to in case of error in a sql statement @@ -63,7 +64,9 @@ if (isset($_REQUEST['do_save_data'])) { //tbl_structure.php below unset($_REQUEST['do_save_data']); - list($result, $sql_query) = CreateAddField::tryColumnCreationQuery($db, $table, $err_url); + $createAddField = new CreateAddField(); + + list($result, $sql_query) = $createAddField->tryColumnCreationQuery($db, $table, $err_url); if ($result === true) { // Update comment table for mime types [MIME] @@ -89,17 +92,17 @@ if (isset($_REQUEST['do_save_data'])) { } // Go back to the structure sub-page - $message = PhpMyAdmin\Message::success( + $message = Message::success( __('Table %1$s has been altered successfully.') ); $message->addParam($table); $response->addJSON( 'message', - PhpMyAdmin\Util::getMessage($message, $sql_query, 'success') + Util::getMessage($message, $sql_query, 'success') ); exit; } else { - $error_message_html = PhpMyAdmin\Util::mysqlDie( + $error_message_html = Util::mysqlDie( '', '', false, diff --git a/tbl_create.php b/tbl_create.php index d9160a6d12..0afe8d8d56 100644 --- a/tbl_create.php +++ b/tbl_create.php @@ -50,9 +50,11 @@ if ($GLOBALS['dbi']->getColumns($db, $table)) { ); } +$createAddField = new CreateAddField(); + // for libraries/tbl_columns_definition_form.inc.php // check number of fields to be created -$num_fields = CreateAddField::getNumberOfFieldsFromRequest(); +$num_fields = $createAddField->getNumberOfFieldsFromRequest(); $action = 'tbl_create.php'; @@ -60,7 +62,7 @@ $action = 'tbl_create.php'; * The form used to define the structure of the table has been submitted */ if (isset($_REQUEST['do_save_data'])) { - $sql_query = CreateAddField::getTableCreationQuery($db, $table); + $sql_query = $createAddField->getTableCreationQuery($db, $table); // If there is a request for SQL previewing. if (isset($_REQUEST['preview_sql'])) { diff --git a/test/classes/CreateAddFieldTest.php b/test/classes/CreateAddFieldTest.php index 49ba2f7eb9..bc4764cdf0 100644 --- a/test/classes/CreateAddFieldTest.php +++ b/test/classes/CreateAddFieldTest.php @@ -11,14 +11,19 @@ use PhpMyAdmin\CreateAddField; use PHPUnit\Framework\TestCase; /** - * PhpMyAdmin\CreateAddFieldTest class - * * This class is for testing PhpMyAdmin\CreateAddField methods * * @package PhpMyAdmin-test */ class CreateAddFieldTest extends TestCase { + private $createAddField; + + protected function setUp() + { + $this->createAddField = new CreateAddField(); + } + /** * Test for getPartitionsDefinition * @@ -32,7 +37,7 @@ class CreateAddFieldTest extends TestCase public function testGetPartitionsDefinition($expected, $request) { $_REQUEST = $request; - $actual = CreateAddField::getPartitionsDefinition(); + $actual = $this->createAddField->getPartitionsDefinition(); $this->assertEquals($expected, $actual); } @@ -68,7 +73,7 @@ class CreateAddFieldTest extends TestCase public function testGetTableCreationQuery($expected, $db, $table, $request) { $_REQUEST = $request; - $actual = CreateAddField::getTableCreationQuery($db, $table); + $actual = $this->createAddField->getTableCreationQuery($db, $table); $this->assertEquals($expected, $actual); } @@ -104,7 +109,7 @@ class CreateAddFieldTest extends TestCase public function testGetNumberOfFieldsFromRequest($expected, $request) { $_REQUEST = $request; - $actual = CreateAddField::getNumberOfFieldsFromRequest(); + $actual = $this->createAddField->getNumberOfFieldsFromRequest(); $this->assertEquals($expected, $actual); } From a1e088d5f84382efb3b8c01592126974302a3131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 15 Feb 2018 14:12:43 -0200 Subject: [PATCH 2/3] Fix coding style issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/CreateAddField.php | 306 +++++++++++++++------------ test/classes/CreateAddFieldTest.php | 5 + 2 files changed, 175 insertions(+), 136 deletions(-) diff --git a/libraries/classes/CreateAddField.php b/libraries/classes/CreateAddField.php index c7b7fead5c..e94ece5153 100644 --- a/libraries/classes/CreateAddField.php +++ b/libraries/classes/CreateAddField.php @@ -26,42 +26,47 @@ class CreateAddField */ private function getIndexedColumns() { - $field_cnt = count($_REQUEST['field_name']); - $field_primary = json_decode($_REQUEST['primary_indexes'], true); - $field_index = json_decode($_REQUEST['indexes'], true); - $field_unique = json_decode($_REQUEST['unique_indexes'], true); - $field_fulltext = json_decode($_REQUEST['fulltext_indexes'], true); - $field_spatial = json_decode($_REQUEST['spatial_indexes'], true); + $fieldCount = count($_REQUEST['field_name']); + $fieldPrimary = json_decode($_REQUEST['primary_indexes'], true); + $fieldIndex = json_decode($_REQUEST['indexes'], true); + $fieldUnique = json_decode($_REQUEST['unique_indexes'], true); + $fieldFullText = json_decode($_REQUEST['fulltext_indexes'], true); + $fieldSpatial = json_decode($_REQUEST['spatial_indexes'], true); - return array( - $field_cnt, $field_primary, $field_index, $field_unique, - $field_fulltext, $field_spatial - ); + return [ + $fieldCount, + $fieldPrimary, + $fieldIndex, + $fieldUnique, + $fieldFullText, + $fieldSpatial, + ]; } /** * Initiate the column creation statement according to the table creation or * add columns to a existing table * - * @param int $field_cnt number of columns - * @param boolean $is_create_tbl true if requirement is to get the statement + * @param int $fieldCount number of columns + * @param boolean $isCreateTable true if requirement is to get the statement * for table creation * * @return array $definitions An array of initial sql statements * according to the request */ private function buildColumnCreationStatement( - $field_cnt, $is_create_tbl = true + $fieldCount, + $isCreateTable = true ) { - $definitions = array(); - $prev_field = -1; - for ($i = 0; $i < $field_cnt; ++$i) { + $definitions = []; + $previousField = -1; + for ($i = 0; $i < $fieldCount; ++$i) { // '0' is also empty for php :-( if (strlen($_REQUEST['field_name'][$i]) === 0) { continue; } - $definition = $this->getStatementPrefix($is_create_tbl) . + $definition = $this->getStatementPrefix($isCreateTable) . Table::generateFieldSpec( trim($_REQUEST['field_name'][$i]), $_REQUEST['field_type'][$i], @@ -89,8 +94,8 @@ class CreateAddField : '' ); - $definition .= $this->setColumnCreationStatementSuffix($i, $prev_field, $is_create_tbl); - $prev_field = $i; + $definition .= $this->setColumnCreationStatementSuffix($i, $previousField, $isCreateTable); + $previousField = $i; $definitions[] = $definition; } // end for @@ -100,85 +105,89 @@ class CreateAddField /** * Set column creation suffix according to requested position of the new column * - * @param int $current_field_num current column number - * @param int $prev_field previous field for ALTER statement - * @param boolean $is_create_tbl true if requirement is to get the statement - * for table creation + * @param int $currentFieldNumber current column number + * @param int $previousField previous field for ALTER statement + * @param boolean $isCreateTable true if requirement is to get the statement + * for table creation * - * @return string $sql_suffix suffix + * @return string $sqlSuffix suffix */ - private function setColumnCreationStatementSuffix($current_field_num, $prev_field, - $is_create_tbl = true + private function setColumnCreationStatementSuffix( + $currentFieldNumber, + $previousField, + $isCreateTable = true ) { // no suffix is needed if request is a table creation - $sql_suffix = ' '; - if ($is_create_tbl) { - return $sql_suffix; + $sqlSuffix = ' '; + if ($isCreateTable) { + return $sqlSuffix; } if ((string) $_REQUEST['field_where'] === 'last') { - return $sql_suffix; + return $sqlSuffix; } // Only the first field can be added somewhere other than at the end - if ($prev_field == -1) { + if ($previousField == -1) { if ((string) $_REQUEST['field_where'] === 'first') { - $sql_suffix .= ' FIRST'; + $sqlSuffix .= ' FIRST'; } else { - $sql_suffix .= ' AFTER ' + $sqlSuffix .= ' AFTER ' . Util::backquote($_REQUEST['after_field']); } } else { - $sql_suffix .= ' AFTER ' + $sqlSuffix .= ' AFTER ' . Util::backquote( - $_REQUEST['field_name'][$prev_field] + $_REQUEST['field_name'][$previousField] ); } - return $sql_suffix; + return $sqlSuffix; } /** * Create relevant index statements * * @param array $index an array of index columns - * @param string $index_choice index choice that which represents + * @param string $indexChoice index choice that which represents * the index type of $indexed_fields - * @param boolean $is_create_tbl true if requirement is to get the statement + * @param boolean $isCreateTable true if requirement is to get the statement * for table creation * * @return array an array of sql statements for indexes */ - private function buildIndexStatements(array $index, $index_choice, - $is_create_tbl = true + private function buildIndexStatements( + array $index, + $indexChoice, + $isCreateTable = true ) { - $statement = array(); + $statement = []; if (!count($index)) { return $statement; } - $sql_query = $this->getStatementPrefix($is_create_tbl) - . ' ' . $index_choice; + $sqlQuery = $this->getStatementPrefix($isCreateTable) + . ' ' . $indexChoice; if (! empty($index['Key_name']) && $index['Key_name'] != 'PRIMARY') { - $sql_query .= ' ' . Util::backquote($index['Key_name']); + $sqlQuery .= ' ' . Util::backquote($index['Key_name']); } - $index_fields = array(); + $indexFields = []; foreach ($index['columns'] as $key => $column) { - $index_fields[$key] = Util::backquote( + $indexFields[$key] = Util::backquote( $_REQUEST['field_name'][$column['col_index']] ); if ($column['size']) { - $index_fields[$key] .= '(' . $column['size'] . ')'; + $indexFields[$key] .= '(' . $column['size'] . ')'; } - } // end while + } - $sql_query .= ' (' . implode(', ', $index_fields) . ')'; + $sqlQuery .= ' (' . implode(', ', $indexFields) . ')'; $keyBlockSizes = $index['Key_block_size']; if (! empty($keyBlockSizes)) { - $sql_query .= " KEY_BLOCK_SIZE = " + $sqlQuery .= " KEY_BLOCK_SIZE = " . $GLOBALS['dbi']->escapeString($keyBlockSizes); } @@ -188,21 +197,21 @@ class CreateAddField && $index['Index_choice'] != 'FULLTEXT' && in_array($type, Index::getIndexTypes()) ) { - $sql_query .= ' USING ' . $type; + $sqlQuery .= ' USING ' . $type; } $parser = $index['Parser']; if ($index['Index_choice'] == 'FULLTEXT' && ! empty($parser)) { - $sql_query .= " WITH PARSER " . $GLOBALS['dbi']->escapeString($parser); + $sqlQuery .= " WITH PARSER " . $GLOBALS['dbi']->escapeString($parser); } $comment = $index['Index_comment']; if (! empty($comment)) { - $sql_query .= " COMMENT '" . $GLOBALS['dbi']->escapeString($comment) + $sqlQuery .= " COMMENT '" . $GLOBALS['dbi']->escapeString($comment) . "'"; } - $statement[] = $sql_query; + $statement[] = $sqlQuery; return $statement; } @@ -210,37 +219,42 @@ class CreateAddField /** * Statement prefix for the buildColumnCreationStatement() * - * @param boolean $is_create_tbl true if requirement is to get the statement + * @param boolean $isCreateTable true if requirement is to get the statement * for table creation * - * @return string $sql_prefix prefix + * @return string $sqlPrefix prefix */ - private function getStatementPrefix($is_create_tbl = true) + private function getStatementPrefix($isCreateTable = true) { - $sql_prefix = " "; - if (! $is_create_tbl) { - $sql_prefix = ' ADD '; + $sqlPrefix = " "; + if (! $isCreateTable) { + $sqlPrefix = ' ADD '; } - return $sql_prefix; + return $sqlPrefix; } /** * Merge index definitions for one type of index * - * @param array $definitions the index definitions to merge to - * @param boolean $is_create_tbl true if requirement is to get the statement - * for table creation - * @param array $indexed_columns the columns for one type of index - * @param string $index_keyword the index keyword to use in the definition + * @param array $definitions the index definitions to merge to + * @param boolean $isCreateTable true if requirement is to get the statement + * for table creation + * @param array $indexedColumns the columns for one type of index + * @param string $indexKeyword the index keyword to use in the definition * * @return array $index_definitions */ private function mergeIndexStatements( - array $definitions, $is_create_tbl, array $indexed_columns, $index_keyword + array $definitions, + $isCreateTable, + array $indexedColumns, + $indexKeyword ) { - foreach ($indexed_columns as $index) { + foreach ($indexedColumns as $index) { $statements = $this->buildIndexStatements( - $index, " " . $index_keyword . " ", $is_create_tbl + $index, + " " . $indexKeyword . " ", + $isCreateTable ); $definitions = array_merge($definitions, $statements); } @@ -251,56 +265,73 @@ class CreateAddField * Returns sql statement according to the column and index specifications as * requested * - * @param boolean $is_create_tbl true if requirement is to get the statement + * @param boolean $isCreateTable true if requirement is to get the statement * for table creation * * @return string sql statement */ - private function getColumnCreationStatements($is_create_tbl = true) + private function getColumnCreationStatements($isCreateTable = true) { - $sql_statement = ""; - list($field_cnt, $field_primary, $field_index, - $field_unique, $field_fulltext, $field_spatial - ) = $this->getIndexedColumns(); + $sqlStatement = ""; + list( + $fieldCount, + $fieldPrimary, + $fieldIndex, + $fieldUnique, + $fieldFullText, + $fieldSpatial + ) = $this->getIndexedColumns(); $definitions = $this->buildColumnCreationStatement( - $field_cnt, $is_create_tbl + $fieldCount, + $isCreateTable ); // Builds the PRIMARY KEY statements - $primary_key_statements = $this->buildIndexStatements( - isset($field_primary[0]) ? $field_primary[0] : array(), + $primaryKeyStatements = $this->buildIndexStatements( + isset($fieldPrimary[0]) ? $fieldPrimary[0] : [], " PRIMARY KEY ", - $is_create_tbl + $isCreateTable ); - $definitions = array_merge($definitions, $primary_key_statements); + $definitions = array_merge($definitions, $primaryKeyStatements); // Builds the INDEX statements $definitions = $this->mergeIndexStatements( - $definitions, $is_create_tbl, $field_index, "INDEX" + $definitions, + $isCreateTable, + $fieldIndex, + "INDEX" ); // Builds the UNIQUE statements $definitions = $this->mergeIndexStatements( - $definitions, $is_create_tbl, $field_unique, "UNIQUE" + $definitions, + $isCreateTable, + $fieldUnique, + "UNIQUE" ); // Builds the FULLTEXT statements $definitions = $this->mergeIndexStatements( - $definitions, $is_create_tbl, $field_fulltext, "FULLTEXT" + $definitions, + $isCreateTable, + $fieldFullText, + "FULLTEXT" ); // Builds the SPATIAL statements $definitions = $this->mergeIndexStatements( - $definitions, $is_create_tbl, $field_spatial, "SPATIAL" + $definitions, + $isCreateTable, + $fieldSpatial, + "SPATIAL" ); if (count($definitions)) { - $sql_statement = implode(', ', $definitions); + $sqlStatement = implode(', ', $definitions); } - $sql_statement = preg_replace('@, $@', '', $sql_statement); - - return $sql_statement; + $sqlStatement = preg_replace('@, $@', '', $sqlStatement); + return $sqlStatement; } /** @@ -310,13 +341,13 @@ class CreateAddField */ public function getPartitionsDefinition() { - $sql_query = ""; + $sqlQuery = ""; if (! empty($_REQUEST['partition_by']) && ! empty($_REQUEST['partition_expr']) && ! empty($_REQUEST['partition_count']) && $_REQUEST['partition_count'] > 1 ) { - $sql_query .= " PARTITION BY " . $_REQUEST['partition_by'] + $sqlQuery .= " PARTITION BY " . $_REQUEST['partition_by'] . " (" . $_REQUEST['partition_expr'] . ")" . " PARTITIONS " . $_REQUEST['partition_count']; } @@ -326,22 +357,22 @@ class CreateAddField && ! empty($_REQUEST['subpartition_count']) && $_REQUEST['subpartition_count'] > 1 ) { - $sql_query .= " SUBPARTITION BY " . $_REQUEST['subpartition_by'] + $sqlQuery .= " SUBPARTITION BY " . $_REQUEST['subpartition_by'] . " (" . $_REQUEST['subpartition_expr'] . ")" . " SUBPARTITIONS " . $_REQUEST['subpartition_count']; } if (! empty($_REQUEST['partitions'])) { $i = 0; - $partitions = array(); + $partitions = []; foreach ($_REQUEST['partitions'] as $partition) { $partitions[] = $this->getPartitionDefinition($partition); $i++; } - $sql_query .= " (" . implode(", ", $partitions) . ")"; + $sqlQuery .= " (" . implode(", ", $partitions) . ")"; } - return $sql_query; + return $sqlQuery; } /** @@ -354,45 +385,45 @@ class CreateAddField */ private function getPartitionDefinition(array $partition, $isSubPartition = false) { - $sql_query = " " . ($isSubPartition ? "SUB" : "") . "PARTITION "; - $sql_query .= $partition['name']; + $sqlQuery = " " . ($isSubPartition ? "SUB" : "") . "PARTITION "; + $sqlQuery .= $partition['name']; if (! empty($partition['value_type'])) { - $sql_query .= " VALUES " . $partition['value_type']; + $sqlQuery .= " VALUES " . $partition['value_type']; if ($partition['value_type'] != 'LESS THAN MAXVALUE') { - $sql_query .= " (" . $partition['value'] . ")"; + $sqlQuery .= " (" . $partition['value'] . ")"; } } if (! empty($partition['engine'])) { - $sql_query .= " ENGINE = " . $partition['engine']; + $sqlQuery .= " ENGINE = " . $partition['engine']; } if (! empty($partition['comment'])) { - $sql_query .= " COMMENT = '" . $partition['comment'] . "'"; + $sqlQuery .= " COMMENT = '" . $partition['comment'] . "'"; } if (! empty($partition['data_directory'])) { - $sql_query .= " DATA DIRECTORY = '" . $partition['data_directory'] . "'"; + $sqlQuery .= " DATA DIRECTORY = '" . $partition['data_directory'] . "'"; } if (! empty($partition['index_directory'])) { - $sql_query .= " INDEX_DIRECTORY = '" . $partition['index_directory'] . "'"; + $sqlQuery .= " INDEX_DIRECTORY = '" . $partition['index_directory'] . "'"; } if (! empty($partition['max_rows'])) { - $sql_query .= " MAX_ROWS = " . $partition['max_rows']; + $sqlQuery .= " MAX_ROWS = " . $partition['max_rows']; } if (! empty($partition['min_rows'])) { - $sql_query .= " MIN_ROWS = " . $partition['min_rows']; + $sqlQuery .= " MIN_ROWS = " . $partition['min_rows']; } if (! empty($partition['tablespace'])) { - $sql_query .= " TABLESPACE = " . $partition['tablespace']; + $sqlQuery .= " TABLESPACE = " . $partition['tablespace']; } if (! empty($partition['node_group'])) { - $sql_query .= " NODEGROUP = " . $partition['node_group']; + $sqlQuery .= " NODEGROUP = " . $partition['node_group']; } if (! empty($partition['subpartitions'])) { $j = 0; - $subpartitions = array(); + $subpartitions = []; foreach ($partition['subpartitions'] as $subpartition) { $subpartitions[] = $this->getPartitionDefinition( $subpartition, @@ -400,10 +431,10 @@ class CreateAddField ); $j++; } - $sql_query .= " (" . implode(", ", $subpartitions) . ")"; + $sqlQuery .= " (" . implode(", ", $subpartitions) . ")"; } - return $sql_query; + return $sqlQuery; } /** @@ -417,36 +448,36 @@ class CreateAddField public function getTableCreationQuery($db, $table) { // get column addition statements - $sql_statement = $this->getColumnCreationStatements(true); + $sqlStatement = $this->getColumnCreationStatements(true); // Builds the 'create table' statement - $sql_query = 'CREATE TABLE ' . Util::backquote($db) . '.' - . Util::backquote(trim($table)) . ' (' . $sql_statement . ')'; + $sqlQuery = 'CREATE TABLE ' . Util::backquote($db) . '.' + . Util::backquote(trim($table)) . ' (' . $sqlStatement . ')'; // Adds table type, character set, comments and partition definition if (!empty($_REQUEST['tbl_storage_engine']) && ($_REQUEST['tbl_storage_engine'] != 'Default') ) { - $sql_query .= ' ENGINE = ' . $_REQUEST['tbl_storage_engine']; + $sqlQuery .= ' ENGINE = ' . $_REQUEST['tbl_storage_engine']; } if (!empty($_REQUEST['tbl_collation'])) { - $sql_query .= Util::getCharsetQueryPart($_REQUEST['tbl_collation']); + $sqlQuery .= Util::getCharsetQueryPart($_REQUEST['tbl_collation']); } if (! empty($_REQUEST['connection']) && ! empty($_REQUEST['tbl_storage_engine']) && $_REQUEST['tbl_storage_engine'] == 'FEDERATED' ) { - $sql_query .= " CONNECTION = '" + $sqlQuery .= " CONNECTION = '" . $GLOBALS['dbi']->escapeString($_REQUEST['connection']) . "'"; } if (!empty($_REQUEST['comment'])) { - $sql_query .= ' COMMENT = \'' + $sqlQuery .= ' COMMENT = \'' . $GLOBALS['dbi']->escapeString($_REQUEST['comment']) . '\''; } - $sql_query .= $this->getPartitionsDefinition(); - $sql_query .= ';'; + $sqlQuery .= $this->getPartitionsDefinition(); + $sqlQuery .= ';'; - return $sql_query; + return $sqlQuery; } /** @@ -456,51 +487,54 @@ class CreateAddField */ public function getNumberOfFieldsFromRequest() { + // Limit to 4096 fields (MySQL maximal value) + $mysqlLimit = 4096; + if (isset($_REQUEST['submit_num_fields'])) { // adding new fields - $num_fields = intval($_REQUEST['orig_num_fields']) + intval($_REQUEST['added_fields']); + $numberOfFields = intval($_REQUEST['orig_num_fields']) + intval($_REQUEST['added_fields']); } elseif (isset($_REQUEST['orig_num_fields'])) { // retaining existing fields - $num_fields = intval($_REQUEST['orig_num_fields']); + $numberOfFields = intval($_REQUEST['orig_num_fields']); } elseif (isset($_REQUEST['num_fields']) && intval($_REQUEST['num_fields']) > 0 ) { // new table with specified number of fields - $num_fields = intval($_REQUEST['num_fields']); + $numberOfFields = intval($_REQUEST['num_fields']); } else { // new table with unspecified number of fields - $num_fields = 4; + $numberOfFields = 4; } - // Limit to 4096 fields (MySQL maximal value) - return min($num_fields, 4096); + return min($numberOfFields, $mysqlLimit); } /** * Function to execute the column creation statement * - * @param string $db current database - * @param string $table current table - * @param string $err_url error page url + * @param string $db current database + * @param string $table current table + * @param string $errorUrl error page url * * @return array */ - public function tryColumnCreationQuery($db, $table, $err_url) + public function tryColumnCreationQuery($db, $table, $errorUrl) { // get column addition statements - $sql_statement = $this->getColumnCreationStatements(false); + $sqlStatement = $this->getColumnCreationStatements(false); // To allow replication, we first select the db to use and then run queries // on this db. if (!($GLOBALS['dbi']->selectDb($db))) { Util::mysqlDie( $GLOBALS['dbi']->getError(), - 'USE ' . Util::backquote($db), false, - $err_url + 'USE ' . Util::backquote($db), + false, + $errorUrl ); } - $sql_query = 'ALTER TABLE ' . - Util::backquote($table) . ' ' . $sql_statement . ';'; + $sqlQuery = 'ALTER TABLE ' . + Util::backquote($table) . ' ' . $sqlStatement . ';'; // If there is a request for SQL previewing. if (isset($_REQUEST['preview_sql'])) { - Core::previewSQL($sql_query); + Core::previewSQL($sqlQuery); } - return array($GLOBALS['dbi']->tryQuery($sql_query) , $sql_query); + return [$GLOBALS['dbi']->tryQuery($sqlQuery), $sqlQuery]; } } diff --git a/test/classes/CreateAddFieldTest.php b/test/classes/CreateAddFieldTest.php index bc4764cdf0..53bb0fa4d5 100644 --- a/test/classes/CreateAddFieldTest.php +++ b/test/classes/CreateAddFieldTest.php @@ -19,6 +19,11 @@ class CreateAddFieldTest extends TestCase { private $createAddField; + /** + * Set up for test cases + * + * @return void + */ protected function setUp() { $this->createAddField = new CreateAddField(); From 79ab4f4a506f81038d690a10fa15bfd44ea6f5a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Thu, 15 Feb 2018 14:24:55 -0200 Subject: [PATCH 3/3] Use DI for DatabaseInterface instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- .../Table/TableStructureController.php | 2 +- libraries/classes/CreateAddField.php | 32 ++++++++++++++----- tbl_addfield.php | 2 +- tbl_create.php | 2 +- test/classes/CreateAddFieldTest.php | 5 ++- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/libraries/classes/Controllers/Table/TableStructureController.php b/libraries/classes/Controllers/Table/TableStructureController.php index 850a0ed92b..3db699d808 100644 --- a/libraries/classes/Controllers/Table/TableStructureController.php +++ b/libraries/classes/Controllers/Table/TableStructureController.php @@ -120,7 +120,7 @@ class TableStructureController extends TableController $this->_showtable = $showtable; $this->table_obj = $this->dbi->getTable($this->db, $this->table); - $this->createAddField = new CreateAddField(); + $this->createAddField = new CreateAddField($dbi); } /** diff --git a/libraries/classes/CreateAddField.php b/libraries/classes/CreateAddField.php index e94ece5153..a7b46eee39 100644 --- a/libraries/classes/CreateAddField.php +++ b/libraries/classes/CreateAddField.php @@ -8,6 +8,7 @@ namespace PhpMyAdmin; use PhpMyAdmin\Core; +use PhpMyAdmin\DatabaseInterface; use PhpMyAdmin\Index; use PhpMyAdmin\Table; use PhpMyAdmin\Util; @@ -19,6 +20,21 @@ use PhpMyAdmin\Util; */ class CreateAddField { + /** + * @var DatabaseInterface + */ + private $dbi; + + /** + * Constructor + * + * @param DatabaseInterface $dbi DatabaseInterface interface + */ + public function __construct(DatabaseInterface $dbi) + { + $this->dbi = $dbi; + } + /** * Transforms the radio button field_key into 4 arrays * @@ -188,7 +204,7 @@ class CreateAddField $keyBlockSizes = $index['Key_block_size']; if (! empty($keyBlockSizes)) { $sqlQuery .= " KEY_BLOCK_SIZE = " - . $GLOBALS['dbi']->escapeString($keyBlockSizes); + . $this->dbi->escapeString($keyBlockSizes); } // specifying index type is allowed only for primary, unique and index only @@ -202,12 +218,12 @@ class CreateAddField $parser = $index['Parser']; if ($index['Index_choice'] == 'FULLTEXT' && ! empty($parser)) { - $sqlQuery .= " WITH PARSER " . $GLOBALS['dbi']->escapeString($parser); + $sqlQuery .= " WITH PARSER " . $this->dbi->escapeString($parser); } $comment = $index['Index_comment']; if (! empty($comment)) { - $sqlQuery .= " COMMENT '" . $GLOBALS['dbi']->escapeString($comment) + $sqlQuery .= " COMMENT '" . $this->dbi->escapeString($comment) . "'"; } @@ -468,11 +484,11 @@ class CreateAddField && $_REQUEST['tbl_storage_engine'] == 'FEDERATED' ) { $sqlQuery .= " CONNECTION = '" - . $GLOBALS['dbi']->escapeString($_REQUEST['connection']) . "'"; + . $this->dbi->escapeString($_REQUEST['connection']) . "'"; } if (!empty($_REQUEST['comment'])) { $sqlQuery .= ' COMMENT = \'' - . $GLOBALS['dbi']->escapeString($_REQUEST['comment']) . '\''; + . $this->dbi->escapeString($_REQUEST['comment']) . '\''; } $sqlQuery .= $this->getPartitionsDefinition(); $sqlQuery .= ';'; @@ -521,9 +537,9 @@ class CreateAddField // To allow replication, we first select the db to use and then run queries // on this db. - if (!($GLOBALS['dbi']->selectDb($db))) { + if (!($this->dbi->selectDb($db))) { Util::mysqlDie( - $GLOBALS['dbi']->getError(), + $this->dbi->getError(), 'USE ' . Util::backquote($db), false, $errorUrl @@ -535,6 +551,6 @@ class CreateAddField if (isset($_REQUEST['preview_sql'])) { Core::previewSQL($sqlQuery); } - return [$GLOBALS['dbi']->tryQuery($sqlQuery), $sqlQuery]; + return [$this->dbi->tryQuery($sqlQuery), $sqlQuery]; } } diff --git a/tbl_addfield.php b/tbl_addfield.php index c364595667..a0f9a46d58 100644 --- a/tbl_addfield.php +++ b/tbl_addfield.php @@ -64,7 +64,7 @@ if (isset($_REQUEST['do_save_data'])) { //tbl_structure.php below unset($_REQUEST['do_save_data']); - $createAddField = new CreateAddField(); + $createAddField = new CreateAddField($GLOBALS['dbi']); list($result, $sql_query) = $createAddField->tryColumnCreationQuery($db, $table, $err_url); diff --git a/tbl_create.php b/tbl_create.php index 0afe8d8d56..f17707ef55 100644 --- a/tbl_create.php +++ b/tbl_create.php @@ -50,7 +50,7 @@ if ($GLOBALS['dbi']->getColumns($db, $table)) { ); } -$createAddField = new CreateAddField(); +$createAddField = new CreateAddField($GLOBALS['dbi']); // for libraries/tbl_columns_definition_form.inc.php // check number of fields to be created diff --git a/test/classes/CreateAddFieldTest.php b/test/classes/CreateAddFieldTest.php index 53bb0fa4d5..a2916debd2 100644 --- a/test/classes/CreateAddFieldTest.php +++ b/test/classes/CreateAddFieldTest.php @@ -17,6 +17,9 @@ use PHPUnit\Framework\TestCase; */ class CreateAddFieldTest extends TestCase { + /** + * @var CreateAddField + */ private $createAddField; /** @@ -26,7 +29,7 @@ class CreateAddFieldTest extends TestCase */ protected function setUp() { - $this->createAddField = new CreateAddField(); + $this->createAddField = new CreateAddField($GLOBALS['dbi']); } /**