From 18534284aa41f2d750e051f8767048d4c068f99a Mon Sep 17 00:00:00 2001 From: Michael Newton Date: Fri, 30 May 2025 13:10:29 -0600 Subject: [PATCH 1/3] PSR-12 compliance, modern arrays, consistent quotes no changes to logic --- .../BMO/Database/Migration.class.php | 400 +++++++++--------- 1 file changed, 204 insertions(+), 196 deletions(-) diff --git a/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php b/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php index eebb5f91ee..333a209078 100644 --- a/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php +++ b/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php @@ -1,5 +1,4 @@ conn = $conn; - $this->version = $version; - $this->driver = $driverName; - //http://wildlyinaccurate.com/doctrine-2-resolving-unknown-database-type-enum-requested/ - $this->conn->getDatabasePlatform()->registerDoctrineTypeMapping('enum', 'string'); - } + public function __construct($conn, $version, $driverName) + { + $this->conn = $conn; + $this->version = $version; + $this->driver = $driverName; + // http://wildlyinaccurate.com/doctrine-2-resolving-unknown-database-type-enum-requested/ + $this->conn->getDatabasePlatform()->registerDoctrineTypeMapping('enum', 'string'); + } - public function setTable($table) { - $this->table = trim($table); - } + public function setTable($table) + { + $this->table = trim($table); + } - /** - * Generate Update Array used to create or update tables - * @method generateUpdateArray - * @return array Array of table - */ - public function generateUpdateArray() { - if(empty($this->table)) { - throw new \Exception("Table not set!"); - } - $sm = $this->conn->getSchemaManager(); - $fromSchema = $sm->createSchema(); - $schema = new Schema(); + /** + * Generate Update Array used to create or update tables + * + * @return array Array of table + */ + public function generateUpdateArray() + { + if (empty($this->table)) { + throw new Exception('Table not set!'); + } + $sm = $this->conn->getSchemaManager(); + $fromSchema = $sm->createSchema(); + $schema = new Schema(); - $diff = Comparator::compareSchemas($schema,$fromSchema); - if(!isset($diff->newTables[$this->table])) { - throw new \Exception("Table does not exist"); - } - //$table = $sm->listTableDetails($this->table); - $columns = $sm->listTableColumns($this->table); - $foreignKeys = $sm->listTableForeignKeys($this->table); - $indexes = $sm->listTableIndexes($this->table); + $diff = Comparator::compareSchemas($schema,$fromSchema); + if (!isset($diff->newTables[$this->table])) { + throw new Exception('Table does not exist'); + } + //$table = $sm->listTableDetails($this->table); + $columns = $sm->listTableColumns($this->table); + $foreignKeys = $sm->listTableForeignKeys($this->table); + $indexes = $sm->listTableIndexes($this->table); - $export = array(); - $expindexes = array(); - foreach ($columns as $column) { - $type = $column->getType()->getName(); - $name = $column->getName(); - switch($type) { - case 'string': - $export[$name]['type'] = $type; - $export[$name]['length'] = $column->getLength(); - break; - case 'blob': - case 'integer': - case 'bigint': - case 'smallint': - case 'date': - case 'datetime': - case 'text': - case 'boolean': - $export[$name]['type'] = $type; - break; - case 'float': - case 'decimal': - $export[$name]['type'] = $type; - $export[$name]['precision'] = $column->getPrecision(); - $export[$name]['scale'] = $column->getScale(); - break; - default: - throw new \Exception("Unknown type: ".$type); - break; - } - if(!$column->getNotnull()) { - $export[$name]['notnull'] = $column->getNotnull(); - } - if($column->getAutoincrement()) { - $export[$name]['autoincrement'] = $column->getAutoincrement(); - } - if($column->getUnsigned()) { - $export[$name]['unsigned'] = $column->getUnsigned(); - } - $default = $column->getDefault(); - if(!in_array($type,array("datetime","datetime/timestamp")) && !is_null($default)){ - $export[$name]['default'] = $default; - } - } - foreach ($indexes as $index) { - $name = $index->getName(); - if($index->isPrimary()) { - foreach($index->getColumns() as $col) { - $export[$col]['primarykey'] = true; - } - continue; - } elseif($index->isUnique()) { - $expindexes[$name]['type'] = 'unique'; - } else { - $expindexes[$name]['type'] = 'index'; - } - $expindexes[$name]['cols'] = $index->getColumns(); - } + $export = []; + $expindexes = []; + foreach ($columns as $column) { + $type = $column->getType()->getName(); + $name = $column->getName(); + switch ($type) { + case 'string': + $export[$name]['type'] = $type; + $export[$name]['length'] = $column->getLength(); + break; + case 'blob': + case 'integer': + case 'bigint': + case 'smallint': + case 'date': + case 'datetime': + case 'text': + case 'boolean': + $export[$name]['type'] = $type; + break; + case 'float': + case 'decimal': + $export[$name]['type'] = $type; + $export[$name]['precision'] = $column->getPrecision(); + $export[$name]['scale'] = $column->getScale(); + break; + default: + throw new Exception('Unknown type: ' . $type); + } + if (!$column->getNotnull()) { + $export[$name]['notnull'] = $column->getNotnull(); + } + if ($column->getAutoincrement()) { + $export[$name]['autoincrement'] = $column->getAutoincrement(); + } + if ($column->getUnsigned()) { + $export[$name]['unsigned'] = $column->getUnsigned(); + } + $default = $column->getDefault(); + if (!in_array($type, ['datetime', 'datetime/timestamp']) && !is_null($default)) { + $export[$name]['default'] = $default; + } + } + foreach ($indexes as $index) { + $name = $index->getName(); + if($index->isPrimary()) { + foreach ($index->getColumns() as $col) { + $export[$col]['primarykey'] = true; + } + continue; + } elseif ($index->isUnique()) { + $expindexes[$name]['type'] = 'unique'; + } else { + $expindexes[$name]['type'] = 'index'; + } + $expindexes[$name]['cols'] = $index->getColumns(); + } - if(!empty($foreignKeys)) { - throw new \Exception("There are foreign keys here. Cant accurately generate tables"); - } - /* - foreach ($foreignKeys as $foreignKey) { - //echo $foreignKey->getName() . ': ' . $foreignKey->getLocalTableName() ."\n"; - } - */ + if (!empty($foreignKeys)) { + throw new Exception('There are foreign keys here. Cant accurately generate tables'); + } + /* + foreach ($foreignKeys as $foreignKey) { + //echo $foreignKey->getName() . ': ' . $foreignKey->getLocalTableName() .'\n'; + } + */ - return array("columns"=>$export, "indexes" => $expindexes); - } + return ['columns' => $export, 'indexes' => $expindexes]; + } - /** - * Modify Multiple Tables - * @method modify - * @param array $tables The tables to update - * @param bool $dryrun If set to true dont execute just return the sql modification string - * @return mixed - */ - public function modifyMultiple($tables=array(),$dryrun=false) { - $synchronizer = new \FreePBX\Database\DBAL\SingleDatabaseSynchronizer($this->conn); - $schemaConfig = new SchemaConfig(); - if($this->driver == "pdo_mysql" && version_compare($this->version, "5.5.3", "ge")) { - $schemaConfig->setDefaultTableOptions(array( - "collate"=>"utf8mb4_unicode_ci", - "charset"=>"utf8mb4" - )); - } - $schema = new Schema(array(),array(),$schemaConfig); - foreach($tables as $tname => $tdata) { - $table = $schema->createTable($tname); - $primaryKeys = array(); - foreach($tdata['columns'] as $name => $options) { - $type = $options['type']; - unset($options['type']); - $pk = isset($options['primaryKey']) ? $options['primaryKey'] : (isset($options['primarykey']) ? $options['primarykey'] : null); - if(!is_null($pk)) { - if($pk) { - $primaryKeys[] = $name; - if(isset($options['primaryKey'])) { - unset($options['primaryKey']); - }else if(isset($options['primarykey'])){ - unset($options['primarykey']); - } - } - } - $table->addColumn($name, $type, $options); - } - if(!empty($primaryKeys)) { - $table->setPrimaryKey($primaryKeys); - } - if(!empty($tdata['indexes']) && is_array($tdata['indexes'])) { - foreach($tdata['indexes'] as $name => $data) { - $type = $data['type']; - $columns = $data['cols']; - switch($type) { - case "unique": - $table->addUniqueIndex($columns,$name); - break; - case "index": - $table->addIndex($columns,$name); - break; - case "fulltext": - if($this->driver == "pdo_mysql" && version_compare($this->version, "5.6", "le")) { - $table->addOption('engine' , 'MyISAM'); - } - $table->addIndex($columns,$name,array("fulltext")); - break; - case "foreign": - $table->addForeignKeyConstraint($data['foreigntable'], $columns, $data['foreigncols'], $data['options'], $name); - break; - } - } - } - } - //with true to prevent drops - if($dryrun) { - return $synchronizer->getUpdateSchema($schema, true); - } else { - return $synchronizer->updateSchema($schema, true); - } - } + /** + * Modify Multiple Tables + * + * @param array $tables The tables to update + * @param bool $dryrun If set to true dont execute just return the sql modification string + * @return mixed + */ + public function modifyMultiple($tables = [], $dryrun = false) + { + $synchronizer = new SingleDatabaseSynchronizer($this->conn); + $schemaConfig = new SchemaConfig(); + if ($this->driver === 'pdo_mysql' && version_compare($this->version, '5.5.3', 'ge')) { + $schemaConfig->setDefaultTableOptions([ + 'collate' => 'utf8mb4_unicode_ci', + 'charset' => 'utf8mb4', + ]); + } + $schema = new Schema([], [], $schemaConfig); + foreach ($tables as $tname => $tdata) { + $table = $schema->createTable($tname); + $primaryKeys = []; + foreach ($tdata['columns'] as $name => $options) { + $type = $options['type']; + unset($options['type']); + $pk = $options['primaryKey'] ?? $options['primarykey'] ?? null; + if(!is_null($pk)) { + if ($pk) { + $primaryKeys[] = $name; + if (isset($options['primaryKey'])) { + unset($options['primaryKey']); + } elseif (isset($options['primarykey'])) { + unset($options['primarykey']); + } + } + } + $table->addColumn($name, $type, $options); + } + if (!empty($primaryKeys)) { + $table->setPrimaryKey($primaryKeys); + } + if (!empty($tdata['indexes']) && is_array($tdata['indexes'])) { + foreach ($tdata['indexes'] as $name => $data) { + $type = $data['type']; + $columns = $data['cols']; + switch ($type) { + case 'unique': + $table->addUniqueIndex($columns, $name); + break; + case 'index': + $table->addIndex($columns, $name); + break; + case 'fulltext': + if($this->driver == 'pdo_mysql' && version_compare($this->version, '5.6', 'le')) { + $table->addOption('engine', 'MyISAM'); + } + $table->addIndex($columns,$name,array('fulltext')); + break; + case 'foreign': + $table->addForeignKeyConstraint($data['foreigntable'], $columns, $data['foreigncols'], $data['options'], $name); + break; + } + } + } + } + //with true to prevent drops + if ($dryrun) { + return $synchronizer->getUpdateSchema($schema, true); + } else { + return $synchronizer->updateSchema($schema, true); + } + } - /** - * Modify Single Table - * @method modify - * @param array $columns Columns to update - * @param array $indexes Indexes to update - * @param bool $dryrun If set to true dont execute just return the sql modification string - * @return mixed - */ - public function modify($columns=array(),$indexes=array(),$dryrun=false) { - if(empty($this->table)) { - throw new \Exception("Table not set!"); - } - $table = $this->table; - return $this->modifyMultiple(array( - $table => array( - 'columns' => $columns, - 'indexes' => $indexes - ) - ),$dryrun); - } + /** + * Modify Single Table + * + * @param array $columns Columns to update + * @param array $indexes Indexes to update + * @param bool $dryrun If set to true dont execute just return the sql modification string + * @return mixed + */ + public function modify($columns = [], $indexes = [], $dryrun=false) + { + if (empty($this->table)) { + throw new Exception('Table not set!'); + } + $table = $this->table; + return $this->modifyMultiple([ + $table => [ + 'columns' => $columns, + 'indexes' => $indexes, + ] + ], $dryrun); + } } From 3ab14ac719d85f8714867a6bb02cb08cc636017a Mon Sep 17 00:00:00 2001 From: Michael Newton Date: Fri, 30 May 2025 13:45:22 -0600 Subject: [PATCH 2/3] properly create foreign keys, remove legacy version checks --- .../BMO/Database/Migration.class.php | 84 ++++++++----------- 1 file changed, 33 insertions(+), 51 deletions(-) diff --git a/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php b/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php index 333a209078..cb2ee9937c 100644 --- a/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php +++ b/amp_conf/htdocs/admin/libraries/BMO/Database/Migration.class.php @@ -9,9 +9,9 @@ namespace FreePBX\Database; +use Doctrine\DBAL\Schema\SchemaConfig; use Doctrine\DBAL\Schema\Comparator; use Doctrine\DBAL\Schema\Schema; -use Doctrine\DBAL\Schema\SchemaConfig; use Exception; use FreePBX\Database\DBAL\SingleDatabaseSynchronizer; @@ -19,14 +19,10 @@ class Migration { private $conn; private $table; - private $version; - private $driver; - public function __construct($conn, $version, $driverName) + public function __construct($conn) { $this->conn = $conn; - $this->version = $version; - $this->driver = $driverName; // http://wildlyinaccurate.com/doctrine-2-resolving-unknown-database-type-enum-requested/ $this->conn->getDatabasePlatform()->registerDoctrineTypeMapping('enum', 'string'); } @@ -54,7 +50,6 @@ public function generateUpdateArray() if (!isset($diff->newTables[$this->table])) { throw new Exception('Table does not exist'); } - //$table = $sm->listTableDetails($this->table); $columns = $sm->listTableColumns($this->table); $foreignKeys = $sm->listTableForeignKeys($this->table); $indexes = $sm->listTableIndexes($this->table); @@ -120,11 +115,6 @@ public function generateUpdateArray() if (!empty($foreignKeys)) { throw new Exception('There are foreign keys here. Cant accurately generate tables'); } - /* - foreach ($foreignKeys as $foreignKey) { - //echo $foreignKey->getName() . ': ' . $foreignKey->getLocalTableName() .'\n'; - } - */ return ['columns' => $export, 'indexes' => $expindexes]; } @@ -140,12 +130,10 @@ public function modifyMultiple($tables = [], $dryrun = false) { $synchronizer = new SingleDatabaseSynchronizer($this->conn); $schemaConfig = new SchemaConfig(); - if ($this->driver === 'pdo_mysql' && version_compare($this->version, '5.5.3', 'ge')) { - $schemaConfig->setDefaultTableOptions([ - 'collate' => 'utf8mb4_unicode_ci', - 'charset' => 'utf8mb4', - ]); - } + $schemaConfig->setDefaultTableOptions([ + 'collate' => 'utf8mb4_unicode_ci', + 'charset' => 'utf8mb4', + ]); $schema = new Schema([], [], $schemaConfig); foreach ($tables as $tname => $tdata) { $table = $schema->createTable($tname); @@ -153,47 +141,41 @@ public function modifyMultiple($tables = [], $dryrun = false) foreach ($tdata['columns'] as $name => $options) { $type = $options['type']; unset($options['type']); - $pk = $options['primaryKey'] ?? $options['primarykey'] ?? null; - if(!is_null($pk)) { - if ($pk) { - $primaryKeys[] = $name; - if (isset($options['primaryKey'])) { - unset($options['primaryKey']); - } elseif (isset($options['primarykey'])) { - unset($options['primarykey']); - } - } + if ($options['primaryKey'] ?? $options['primarykey'] ?? null) { + $primaryKeys[] = $name; + unset($options['primaryKey'], $options['primarykey']); } $table->addColumn($name, $type, $options); } if (!empty($primaryKeys)) { $table->setPrimaryKey($primaryKeys); } - if (!empty($tdata['indexes']) && is_array($tdata['indexes'])) { - foreach ($tdata['indexes'] as $name => $data) { - $type = $data['type']; - $columns = $data['cols']; - switch ($type) { - case 'unique': - $table->addUniqueIndex($columns, $name); - break; - case 'index': - $table->addIndex($columns, $name); - break; - case 'fulltext': - if($this->driver == 'pdo_mysql' && version_compare($this->version, '5.6', 'le')) { - $table->addOption('engine', 'MyISAM'); - } - $table->addIndex($columns,$name,array('fulltext')); - break; - case 'foreign': - $table->addForeignKeyConstraint($data['foreigntable'], $columns, $data['foreigncols'], $data['options'], $name); - break; - } + foreach ($tdata['indexes'] ?? [] as $name => $data) { + $type = $data['type']; + $columns = $data['cols']; + switch ($type) { + case 'unique': + $table->addUniqueIndex($columns, $name); + break; + case 'index': + $table->addIndex($columns, $name); + break; + case 'fulltext': + $table->addIndex($columns, $name, ['fulltext']); + break; + case 'foreign': + $opts = [ + 'onUpdate' => $data['onUpdate'] ?? $data['onupdate'] ?? null, + 'onDelete' => $data['onDelete'] ?? $data['ondelete'] ?? null, + ]; + $opts = array_filter($opts); + $foreigncols = array_map('trim', explode(',', $data['foreigncols'])); + $table->addForeignKeyConstraint($data['foreigntable'], $columns, $foreigncols, $opts, $name); + break; } } } - //with true to prevent drops + // with true to prevent drops if ($dryrun) { return $synchronizer->getUpdateSchema($schema, true); } else { @@ -209,7 +191,7 @@ public function modifyMultiple($tables = [], $dryrun = false) * @param bool $dryrun If set to true dont execute just return the sql modification string * @return mixed */ - public function modify($columns = [], $indexes = [], $dryrun=false) + public function modify($columns = [], $indexes = [], $dryrun = false) { if (empty($this->table)) { throw new Exception('Table not set!'); From 4e2956d1839ec41c4a29257a79ecaa9d97c59acc Mon Sep 17 00:00:00 2001 From: Michael Newton Date: Fri, 30 May 2025 13:53:44 -0600 Subject: [PATCH 3/3] disable foreign key checks before dropping a table --- amp_conf/htdocs/admin/libraries/modulefunctions.class.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/amp_conf/htdocs/admin/libraries/modulefunctions.class.php b/amp_conf/htdocs/admin/libraries/modulefunctions.class.php index daf8111038..53377a577a 100644 --- a/amp_conf/htdocs/admin/libraries/modulefunctions.class.php +++ b/amp_conf/htdocs/admin/libraries/modulefunctions.class.php @@ -2282,9 +2282,11 @@ function uninstall($modulename, $force = false) { foreach($xml->database->table as $table) { $tname = (string)$table->attributes()->name; outn(sprintf(_("Dropping table %s..."),$tname)); + FreePBX::Database()->query("SET FOREIGN_KEY_CHECKS=0"); $sth = FreePBX::Database()->prepare("DROP TABLE IF EXISTS ".$tname); - out(_("Done")); $sth->execute(); + FreePBX::Database()->query("SET FOREIGN_KEY_CHECKS=1"); + out(_("Done")); } } }