Merge branch '5' into 6.0

silverstripe · Jan 22, 2025 · cedd284 · cedd284
2 parents 039f919 + 0bd0410
commit cedd284
Show file tree

Hide file tree

Showing 8 changed files with 380 additions and 23 deletions.
diff --git a/src/Core/Manifest/ClassManifest.php b/src/Core/Manifest/ClassManifest.php
@@ -53,6 +53,16 @@ class ClassManifest
      */
     protected $cacheKey;
 
+    /**
+     * In memory cache array for individually parsed files
+     */
+    protected ?array $filesCache = null;
+
+    /**
+     * Key to use for files cache
+     */
+    protected string $filesCacheKey;
+
     /**
      * Array of properties to cache
      *
@@ -203,6 +213,7 @@ public function __construct($base, ?CacheFactory $cacheFactory = null)
         $this->base = $base;
         $this->cacheFactory = $cacheFactory;
         $this->cacheKey = 'manifest';
+        $this->filesCacheKey = 'manifestFiles';
     }
 
     private function buildCache($includeTests = false)
@@ -563,6 +574,7 @@ public function regenerate($includeTests)
 
         if ($this->cache) {
             $data = $this->getState();
+            $this->cache->set($this->filesCacheKey, $this->filesCache);
             $this->cache->set($this->cacheKey, $data);
             $this->cache->set('generated_at', time());
             $this->cache->delete('regenerate');
@@ -587,11 +599,15 @@ public function handleFile($basename, $pathname, $includeTests)
         // since just using the datetime lead to problems with upgrading.
         $key = preg_replace('/[^a-zA-Z0-9_]/', '_', $basename ?? '') . '_' . md5_file($pathname ?? '');
 
+        if ($this->cache && $this->filesCache === null) {
+            $this->filesCache = $this->cache->get($this->filesCacheKey);
+        }
+
         // Attempt to load from cache
         // Note: $classes, $interfaces and $traits arrays have correct-case keys, not lowercase
         $changed = false;
         if ($this->cache
-            && ($data = $this->cache->get($key))
+            && ($data = ($this->filesCache[$key] ?? null))
             && $this->validateItemCache($data)
         ) {
             $classes = $data['classes'];
@@ -696,8 +712,10 @@ public function handleFile($basename, $pathname, $includeTests)
                 'classes' => $classes,
                 'interfaces' => $interfaces,
                 'traits' => $traits,
+                'enums' => $enums,
             ];
-            $this->cache->set($key, $cache);
+
+            $this->filesCache[$key] = $cache;
         }
     }
 

diff --git a/src/ORM/Connect/DBConnector.php b/src/ORM/Connect/DBConnector.php
@@ -52,7 +52,7 @@ protected function databaseError($msg, $errorLevel = E_USER_ERROR, $sql = null,
 
         // Format query if given
         if (!empty($sql)) {
-            $formatter = new SQLFormatter();
+            $formatter = SQLFormatter::create();
             $formattedSQL = $formatter->formatPlain($sql);
             $msg = "Couldn't run query:\n\n{$formattedSQL}\n\n{$msg}";
         }
@@ -66,6 +66,23 @@ protected function databaseError($msg, $errorLevel = E_USER_ERROR, $sql = null,
         }
     }
 
+    protected function duplicateEntryError(
+        string $msg,
+        ?string $keyName,
+        ?string $duplicatedValue,
+        ?string $sql = null,
+        array $parameters = []
+    ): void {
+        // Format query if given
+        if (!empty($sql)) {
+            $formatter = SQLFormatter::create();
+            $formattedSQL = $formatter->formatPlain($sql);
+            $msg = "Couldn't run query:\n\n{$formattedSQL}\n\n{$msg}";
+        }
+
+        throw new DuplicateEntryException($msg, $keyName, $duplicatedValue, $sql, $parameters);
+    }
+
     /**
      * Determine if this SQL statement is a destructive operation (write or ddl)
      *

diff --git a/src/ORM/Connect/DuplicateEntryException.php b/src/ORM/Connect/DuplicateEntryException.php
@@ -0,0 +1,50 @@
+<?php
+
+namespace SilverStripe\ORM\Connect;
+
+/**
+ * Exception for errors related to duplicate entries (e.g. violating a unique index)
+ */
+class DuplicateEntryException extends DatabaseException
+{
+    private ?string $keyName = null;
+
+    private ?string $duplicatedValue = null;
+
+    /**
+     * Constructs the database exception
+     *
+     * @param string $message The Exception message to throw.
+     * @param string|null $keyName The name of the key which the error is for (e.g. index name)
+     * @param string|null $duplicatedValue The value which was duplicated (e.g. combined value of multiple columns in index)
+     * @param string|null $sql The SQL executed for this query
+     * @param array $parameters The parameters given for this query, if any
+     */
+    public function __construct(
+        string $message = '',
+        ?string $keyName = null,
+        ?string $duplicatedValue = null,
+        ?string $sql = null,
+        array $parameters = []
+    ) {
+        parent::__construct($message, sql: $sql, parameters: $parameters);
+        $this->keyName = $keyName;
+        $this->duplicatedValue = $duplicatedValue;
+    }
+
+    /**
+     * Get the name of the key which the error is for (e.g. index name)
+     */
+    public function getKeyName(): ?string
+    {
+        return $this->keyName;
+    }
+
+    /**
+     * Get the value which was duplicated (e.g. combined value of multiple columns in index)
+     */
+    public function getDuplicatedValue(): ?string
+    {
+        return $this->duplicatedValue;
+    }
+}
diff --git a/src/ORM/Connect/MySQLiConnector.php b/src/ORM/Connect/MySQLiConnector.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\ORM\Connect;
 
+use Exception;
 use mysqli;
 use mysqli_sql_exception;
 use mysqli_stmt;
@@ -65,12 +66,18 @@ public function prepareStatement($sql, &$success)
         // Record last statement for error reporting
         $statement = $this->dbConn->stmt_init();
         $this->setLastStatement($statement);
+
         try {
             $success = $statement->prepare($sql);
         } catch (mysqli_sql_exception $e) {
             $success = false;
-            $this->databaseError($e->getMessage(), E_USER_ERROR, $sql);
+            $this->throwRelevantError($e->getMessage(), $e->getCode(), E_USER_ERROR, $sql, []);
         }
+
+        if (!$success || $statement->error) {
+            $this->throwRelevantError($this->getLastError(), $this->getLastErrorCode(), E_USER_ERROR, $sql, []);
+        }
+
         return $statement;
     }
 
@@ -190,17 +197,19 @@ public function query($sql, $errorLevel = E_USER_ERROR)
     {
         $this->beforeQuery($sql);
 
-        $error = null;
+        $exception = null;
         $handle = null;
 
         try {
             // Benchmark query
             $handle = $this->dbConn->query($sql ?? '', MYSQLI_STORE_RESULT);
         } catch (mysqli_sql_exception $e) {
-            $error = $e->getMessage();
+            $exception = $e;
         } finally {
             if (!$handle || $this->dbConn->error) {
-                $this->databaseError($error ?? $this->getLastError(), $errorLevel, $sql);
+                $errorMsg = $exception ? $exception->getMessage() : $this->getLastError();
+                $errorCode = $exception ? $exception->getCode() : $this->getLastErrorCode();
+                $this->throwRelevantError($errorMsg, $errorCode, $errorLevel, $sql, []);
                 return null;
             }
         }
@@ -319,13 +328,13 @@ public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR)
                 $statement->execute();
             } catch (mysqli_sql_exception $e) {
                 $success = false;
-                $this->databaseError($e->getMessage(), E_USER_ERROR, $sql, $parameters);
+                $this->throwRelevantError($e->getMessage(), $e->getCode(), $errorLevel, $sql, $parameters);
             }
         }
 
         if (!$success || $statement->error) {
             $values = $this->parameterValues($parameters);
-            $this->databaseError($this->getLastError(), $errorLevel, $sql, $values);
+            $this->throwRelevantError($this->getLastError(), $this->getLastErrorCode(), $errorLevel, $sql, $values);
             return null;
         }
 
@@ -382,4 +391,38 @@ public function getLastError()
         }
         return $this->dbConn->error;
     }
+
+    public function getLastErrorCode(): int
+    {
+        // Check if a statement was used for the most recent query
+        if ($this->lastStatement && $this->lastStatement->errno) {
+            return $this->lastStatement->errno;
+        }
+        return $this->dbConn->errno;
+    }
+
+    /**
+     * Throw the correct DatabaseException for this error
+     *
+     * @throws DatabaseException
+     */
+    private function throwRelevantError(string $message, int $code, int $errorLevel, ?string $sql, array $parameters): void
+    {
+        if ($errorLevel === E_USER_ERROR && ($code === 1062 || $code === 1586)) {
+            // error 1062 is for a duplicate entry
+            // see https://dev.mysql.com/doc/mysql-errors/8.4/en/server-error-reference.html#error_er_dup_entry
+            // error 1586 is ALSO for a duplicate entry and uses the same error message
+            // see https://dev.mysql.com/doc/mysql-errors/8.4/en/server-error-reference.html#error_er_dup_entry_with_key_name
+            preg_match('/Duplicate entry \'(?P<val>[^\']+)\' for key \'?(?P<key>[^\']+)\'?/', $message, $matches);
+            // MySQL includes the table name in the key, but MariaDB doesn't.
+            $key = $matches['key'];
+            if (str_contains($key ?? '', '.')) {
+                $parts = explode('.', $key);
+                $key = array_pop($parts);
+            }
+            $this->duplicateEntryError($message, $key, $matches['val'], $sql, $parameters);
+        } else {
+            $this->databaseError($message, $errorLevel, $sql, $parameters);
+        }
+    }
 }
diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php
@@ -28,6 +28,7 @@
 use SilverStripe\i18n\i18nEntityProvider;
 use SilverStripe\Model\List\ArrayList;
 use SilverStripe\Model\List\SS_List;
+use SilverStripe\ORM\Connect\DuplicateEntryException;
 use SilverStripe\ORM\Connect\MySQLSchemaManager;
 use SilverStripe\ORM\FieldType\DBComposite;
 use SilverStripe\ORM\FieldType\DBDatetime;
@@ -1625,13 +1626,17 @@ public function write($showDebug = false, $forceInsert = false, $forceWrite = fa
             }
             $this->record['LastEdited'] = $now;
 
-            // New records have their insert into the base data table done first, so that they can pass the
-            // generated primary key on to the rest of the manipulation
-            $baseTable = $this->baseTable();
-            $this->writeBaseRecord($baseTable, $now);
-
-            // Write the DB manipulation for all changed fields
-            $this->writeManipulation($baseTable, $now, $isNewRecord);
+            // Try write the changes - but throw a validation exception if we violate a unique index
+            try {
+                // New records have their insert into the base data table done first, so that they can pass the
+                // generated primary key on to the rest of the manipulation
+                $baseTable = $this->baseTable();
+                $this->writeBaseRecord($baseTable, $now);
+                // Write the DB manipulation for all changed fields
+                $this->writeManipulation($baseTable, $now, $isNewRecord);
+            } catch (DuplicateEntryException $e) {
+                throw new ValidationException($this->buildValidationResultForDuplicateEntry($e));
+            }
 
             // If there's any relations that couldn't be saved before, save them now (we have an ID here)
             $this->writeRelations();
@@ -4579,4 +4584,38 @@ public function findAllRelatedData(array $excludedClasses = []): SS_List
         $service = Injector::inst()->get(RelatedDataService::class);
         return $service->findAll($this, $excludedClasses);
     }
+
+    private function buildValidationResultForDuplicateEntry(DuplicateEntryException $exception): ValidationResult
+    {
+        $key = $exception->getKeyName();
+        $singleName = static::i18n_singular_name();
+        $indexes = DataObject::getSchema()->databaseIndexes(static::class);
+        $columns = $indexes[$key]['columns'] ?? [];
+        $validationResult = ValidationResult::create();
+        if (empty($columns)) {
+            $validationResult->addError(_t(
+                __CLASS__ . '.NO_DUPLICATE',
+                'Cannot create duplicate {type}',
+                ['type' => $singleName]
+            ));
+        } elseif (count($columns) === 1) {
+            $duplicateField = $columns[0];
+            $validationResult->addFieldError(
+                $duplicateField,
+                _t(
+                    __CLASS__ . '.NO_DUPLICATE_SINGLE_FIELD',
+                    'Cannot create duplicate {type} with "{field}" set to "{value}"',
+                    ['type' => $singleName, 'field' => $this->fieldLabel($duplicateField), 'value' => $exception->getDuplicatedValue()]
+                )
+            );
+        } else {
+            $duplicateFieldNames = array_map(fn ($column) => $this->fieldLabel($column), $columns);
+            $validationResult->addError(_t(
+                __CLASS__ . '.NO_DUPLICATE_MULTI_FIELD',
+                'Cannot create duplicate {type} - at least one of the following fields need to be changed: {fields}',
+                ['type' => $singleName, 'fields' => implode(', ', $duplicateFieldNames)]
+            ));
+        }
+        return $validationResult;
+    }
 }
diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php
@@ -69,6 +69,7 @@ class DataObjectTest extends SapphireTest
         DataObjectTest\OverriddenDataObject::class,
         DataObjectTest\InjectedDataObject::class,
         DataObjectTest\SettersAndGetters::class,
+        DataObjectTest\UniqueIndexObject::class,
     ];
 
     protected function setUp(): void
@@ -2784,4 +2785,61 @@ public function testGetDatabaseBackedField(string $fieldPath, $expected)
         $databaseBackedField = $method->invokeArgs($class, [$fieldPath]);
         $this->assertSame($expected, $databaseBackedField);
     }
+
+    public function provideExceptionForUniqueIndexViolation()
+    {
+        return [
+            'violate SingleFieldIndex only' => [
+                'fieldsRecordOne' => [
+                    'SingleField' => 'Same Value',
+                    'Name' => 'Value1',
+                    'Code' => 'Value1',
+                ],
+                'fieldsRecordTwo' => [
+                    'SingleField' => 'Same Value',
+                    'Name' => 'Value2',
+                    'Code' => 'Value2',
+                ],
+                'expectedMessage' => 'Cannot create duplicate Unique Index Object with "Single field" set to "Same Value"',
+            ],
+            'violate MultiFieldIndex only' => [
+                'fieldsRecordOne' => [
+                    'SingleField' => 'Value1',
+                    'Name' => 'Name Value',
+                    'Code' => 'Code Value',
+                ],
+                'fieldsRecordTwo' => [
+                    'SingleField' => 'Value2',
+                    'Name' => 'Name Value',
+                    'Code' => 'Code Value',
+                ],
+                'expectedMessage' => 'Cannot create duplicate Unique Index Object - at least one of the following fields need to be changed: Name, Code',
+            ],
+            'violate both indexes' => [
+                'fieldsRecordOne' => [
+                    'SingleField' => 'Same Value',
+                    'Name' => 'Name Value',
+                    'Code' => 'Code Value',
+                ],
+                'fieldsRecordTwo' => [
+                    'SingleField' => 'Same Value',
+                    'Name' => 'Name Value',
+                    'Code' => 'Code Value',
+                ],
+                'expectedMessage' => 'Cannot create duplicate Unique Index Object with "Single field" set to "Same Value"',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideExceptionForUniqueIndexViolation
+     */
+    public function testExceptionForUniqueIndexViolation(array $fieldsRecordOne, array $fieldsRecordTwo, string $expectedMessage): void
+    {
+        DataObjectTest\UniqueIndexObject::create($fieldsRecordOne)->write();
+        $record2 = DataObjectTest\UniqueIndexObject::create($fieldsRecordTwo);
+        $this->expectException(ValidationException::class);
+        $this->expectExceptionMessage($expectedMessage);
+        $record2->write();
+    }
 }