[Bug]: Fix unique constraint issue when updating advanced many-to-many (

pimcore#17344) * draft to run tests * Apply php-cs-fixer changes * readd migration * Apply php-cs-fixer changes * fix merge mistake --------- Co-authored-by: kingjia90 <[email protected]>
michalananapps · Oct 25, 2024 · eca112d · eca112d
1 parent fec1c09
commit eca112d
Show file tree

Hide file tree

Showing 4 changed files with 153 additions and 39 deletions.
diff --git a/bundles/CoreBundle/src/Migrations/Version20241025101923.php b/bundles/CoreBundle/src/Migrations/Version20241025101923.php
@@ -0,0 +1,142 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Pimcore
+ *
+ * This source file is available under two different licenses:
+ * - GNU General Public License version 3 (GPLv3)
+ * - Pimcore Commercial License (PCL)
+ * Full copyright and license information is available in
+ * LICENSE.md which is distributed with this source code.
+ *
+ *  @copyright  Copyright (c) Pimcore GmbH (http://www.pimcore.org)
+ *  @license    http://www.pimcore.org/license     GPLv3 and PCL
+ */
+
+namespace Pimcore\Bundle\CoreBundle\Migrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+use Pimcore\Model\Dao\AbstractDao;
+
+final class Version20241025101923 extends AbstractMigration
+{
+    private const ID_COLUMN = 'id';
+
+    private const O_PREFIX = 'o_';
+
+    private const PK_COLUMNS = '`' . self::ID_COLUMN .
+        '`,`dest_id`, `type`, `fieldname`, `column`, `ownertype`, `ownername`, `position`, `index`';
+
+    private const UNIQUE_KEY_NAME = 'metadata_un';
+
+    private const AUTO_ID = 'auto_id';
+
+    public function getDescription(): string
+    {
+        return 'Follow up migrating object_metadata schema to have a auto increment column';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('SET foreign_key_checks = 0');
+
+        $metaDataTables = $this->connection->fetchAllAssociative(
+            "SHOW FULL TABLES
+                   WHERE `Tables_in_{$this->connection->getDatabase()}`
+                    LIKE 'object_metadata_%' AND Table_type = 'BASE TABLE'"
+        );
+
+        foreach ($metaDataTables as $table) {
+            $tableName = current($table);
+            $metaDataTable = $schema->getTable($tableName);
+            $foreignKeyName = AbstractDao::getForeignKeyName($tableName, self::ID_COLUMN);
+            $foreignKeyNameWithOPrefix = AbstractDao::getForeignKeyName($tableName, self::O_PREFIX . self::ID_COLUMN);
+
+            if (!$metaDataTable->hasColumn(self::AUTO_ID)) {
+                if ($recreateForeignKey = $metaDataTable->hasForeignKey($foreignKeyName)) {
+                    $this->addSql('ALTER TABLE `' . $tableName . '` DROP FOREIGN KEY `' . $foreignKeyName . '`');
+                } elseif ($recreateForeignKey = $metaDataTable->hasForeignKey($foreignKeyNameWithOPrefix)) {
+                    $this->addSql('ALTER TABLE `' . $tableName . '` DROP FOREIGN KEY `' . $foreignKeyNameWithOPrefix . '`');
+                }
+
+                if ($metaDataTable->getPrimaryKey()) {
+                    $this->addSql('ALTER TABLE `' . $tableName . '` DROP PRIMARY KEY');
+                }
+
+                $this->addSql('ALTER TABLE ' . $tableName . ' ADD `' . self::AUTO_ID .
+                    '` int(11) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY FIRST');
+
+                if (!$metaDataTable->hasIndex(self::UNIQUE_KEY_NAME)) {
+                    $this->addSql(
+                        'ALTER TABLE `' . $tableName . '`
+                            ADD CONSTRAINT `' . self::UNIQUE_KEY_NAME . '`
+                            UNIQUE (' . self::PK_COLUMNS . ')'
+                    );
+                }
+
+                if ($recreateForeignKey) {
+                    $this->addSql(
+                        'ALTER TABLE `' . $tableName . '`
+                            ADD CONSTRAINT `'.$foreignKeyName.'`
+                            FOREIGN KEY (`' . self::ID_COLUMN . '`)
+                            REFERENCES `objects` (`' . self::ID_COLUMN . '`)
+                            ON UPDATE NO ACTION
+                            ON DELETE CASCADE;'
+                    );
+                }
+            }
+        }
+
+        $this->addSql('SET foreign_key_checks = 1');
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('SET foreign_key_checks = 0');
+
+        $metaDataTables = $this->connection->fetchAllAssociative(
+            "SHOW FULL TABLES
+                   WHERE `Tables_in_{$this->connection->getDatabase()}`
+                    LIKE 'object_metadata_%' AND Table_type = 'BASE TABLE'"
+        );
+
+        foreach ($metaDataTables as $table) {
+            $tableName = current($table);
+            $metaDataTable = $schema->getTable($tableName);
+            $foreignKeyName = AbstractDao::getForeignKeyName($tableName, self::ID_COLUMN);
+
+            if ($metaDataTable->hasColumn(self::AUTO_ID)) {
+                if ($recreateForeignKey = $metaDataTable->hasForeignKey($foreignKeyName)) {
+                    $this->addSql('ALTER TABLE `' . $tableName . '` DROP FOREIGN KEY `' . $foreignKeyName . '`');
+                }
+
+                $this->addSql('ALTER TABLE `' . $tableName . '` DROP COLUMN `' . self::AUTO_ID . '`');
+                $this->addSql(
+                    'ALTER TABLE `' . $tableName . '` ADD PRIMARY KEY (' . self::PK_COLUMNS  . ')'
+                );
+
+                if ($metaDataTable->hasIndex(self::UNIQUE_KEY_NAME)) {
+                    $this->addSql(
+                        'ALTER TABLE `' . $tableName . '` DROP INDEX `' . self::UNIQUE_KEY_NAME . '`'
+                    );
+                }
+
+                if ($recreateForeignKey) {
+                    $this->addSql(
+                        'ALTER TABLE `' . $tableName . '`
+                            ADD CONSTRAINT `'.$foreignKeyName.'`
+                            FOREIGN KEY (`' . self::ID_COLUMN . '`)
+                            REFERENCES `objects` (`' . self::ID_COLUMN . '`)
+                            ON UPDATE RESTRICT
+                            ON DELETE CASCADE;'
+                    );
+                }
+            }
+        }
+
+        $this->addSql('SET foreign_key_checks = 1');
+    }
+}
diff --git a/models/DataObject/Data/AbstractMetadata/Dao.php b/models/DataObject/Data/AbstractMetadata/Dao.php
@@ -27,6 +27,8 @@ class Dao extends Model\Dao\AbstractDao
 {
     use DataObject\ClassDefinition\Helper\Dao;
 
+    protected const UNIQUE_KEY_NAME = 'metadata_un';
+
     protected ?array $tableDefinitions = null;
 
     public function save(DataObject\Concrete $object, string $ownertype, string $ownername, string $position, int $index, string $type = 'object'): void
@@ -50,6 +52,7 @@ public function createOrUpdateTable(DataObject\ClassDefinition $class): void
         $table = 'object_metadata_' . $classId;
 
         $this->db->executeQuery('CREATE TABLE IF NOT EXISTS `' . $table . "` (
+              `auto_id` int(11) UNSIGNED NOT NULL AUTO_INCREMENT,
               `id` int(11) UNSIGNED NOT NULL default '0',
               `dest_id` int(11) NOT NULL default '0',
 	          `type` VARCHAR(50) NOT NULL DEFAULT '',
@@ -60,15 +63,19 @@ public function createOrUpdateTable(DataObject\ClassDefinition $class): void
               `ownername` VARCHAR(70) NOT NULL DEFAULT '',
               `position` VARCHAR(70) NOT NULL DEFAULT '0',
               `index` int(11) unsigned NOT NULL DEFAULT '0',
-              PRIMARY KEY (`id`, `dest_id`, `type`, `fieldname`, `column`, `ownertype`, `ownername`, `position`, `index`),
+              PRIMARY KEY (`auto_id`),
+              UNIQUE KEY `metadata_un` (
+                `id`, `dest_id`, `type`, `fieldname`, `column`, `ownertype`, `ownername`, `position`, `index`
+              ),
               INDEX `dest_id` (`dest_id`),
               INDEX `fieldname` (`fieldname`),
               INDEX `column` (`column`),
               INDEX `ownertype` (`ownertype`),
               INDEX `ownername` (`ownername`),
               INDEX `position` (`position`),
               INDEX `index` (`index`),
-              CONSTRAINT `".self::getForeignKeyName($table, 'id').'` FOREIGN KEY (`id`) REFERENCES objects (`id`) ON DELETE CASCADE
+              CONSTRAINT `".self::getForeignKeyName($table, 'id').'` FOREIGN KEY (`id`)
+              REFERENCES objects (`id`) ON DELETE CASCADE
 		) DEFAULT CHARSET=utf8mb4;');
 
         $this->handleEncryption($class, [$table]);

diff --git a/models/DataObject/Data/ElementMetadata/Dao.php b/models/DataObject/Data/ElementMetadata/Dao.php
@@ -43,7 +43,7 @@ public function save(DataObject\Concrete $object, string $ownertype, string $own
             $data = $dataTemplate;
             $data['column'] = $column;
             $data['data'] = $this->model->$getter();
-            Helper::upsert($this->db, $table, $data, $this->getPrimaryKey($table));
+            Helper::upsert($this->db, $table, $data, [parent::UNIQUE_KEY_NAME]);
         }
     }
 

diff --git a/models/DataObject/Data/ObjectMetadata/Dao.php b/models/DataObject/Data/ObjectMetadata/Dao.php
@@ -47,7 +47,7 @@ public function save(DataObject\Concrete $object, string $ownertype, string $own
             $data = $dataTemplate;
             $data['column'] = $column;
             $data['data'] = $this->model->$getter();
-            Helper::upsert($this->db, $table, $data, $this->getPrimaryKey($table));
+            Helper::upsert($this->db, $table, $data, [parent::UNIQUE_KEY_NAME]);
         }
     }
 
@@ -78,39 +78,4 @@ public function load(DataObject\Concrete $source, int $destinationId, string $fi
             return null;
         }
     }
-
-    public function createOrUpdateTable(DataObject\ClassDefinition $class): void
-    {
-        $classId = $class->getId();
-        $table = 'object_metadata_' . $classId;
-
-        $this->db->executeQuery('CREATE TABLE IF NOT EXISTS `' . $table . "` (
-              `auto_id` int(11) UNSIGNED NOT NULL AUTO_INCREMENT,
-              `id` int(11) UNSIGNED NOT NULL default '0',
-              `dest_id` int(11) NOT NULL default '0',
-	          `type` VARCHAR(50) NOT NULL DEFAULT '',
-              `fieldname` varchar(71) NOT NULL,
-              `column` varchar(190) NOT NULL,
-              `data` text,
-              `ownertype` ENUM('object','fieldcollection','localizedfield','objectbrick') NOT NULL DEFAULT 'object',
-              `ownername` VARCHAR(70) NOT NULL DEFAULT '',
-              `position` VARCHAR(70) NOT NULL DEFAULT '0',
-              `index` int(11) unsigned NOT NULL DEFAULT '0',
-              PRIMARY KEY (`auto_id`),
-              UNIQUE KEY `metadata_un` (
-                `id`, `dest_id`, `type`, `fieldname`, `column`, `ownertype`, `ownername`, `position`, `index`
-              ),
-              INDEX `dest_id` (`dest_id`),
-              INDEX `fieldname` (`fieldname`),
-              INDEX `column` (`column`),
-              INDEX `ownertype` (`ownertype`),
-              INDEX `ownername` (`ownername`),
-              INDEX `position` (`position`),
-              INDEX `index` (`index`),
-              CONSTRAINT `".self::getForeignKeyName($table, 'id').'` FOREIGN KEY (`id`)
-              REFERENCES objects (`id`) ON DELETE CASCADE
-		) DEFAULT CHARSET=utf8mb4;');
-
-        $this->handleEncryption($class, [$table]);
-    }
 }