NEW Provide clear UX validation errors when violating unique index

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)
      *

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;
+    }
+}

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,19 @@ 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 null;
         }
+
         return $statement;
     }
 
@@ -190,17 +198,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 +329,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 +392,32 @@ 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);
+            $this->duplicateEntryError($message, $matches['key'], $matches['val'], $sql, $parameters);
+        } else {
+            $this->databaseError($message, $errorLevel, $sql, $parameters);
+        }
+    }
 }

src/ORM/DataObject.php

@@ -24,6 +24,7 @@
 use SilverStripe\Forms\SearchableDropdownField;
 use SilverStripe\i18n\i18n;
 use SilverStripe\i18n\i18nEntityProvider;
+use SilverStripe\ORM\Connect\DuplicateEntryException;
 use SilverStripe\ORM\Connect\MySQLSchemaManager;
 use SilverStripe\ORM\FieldType\DBComposite;
 use SilverStripe\ORM\FieldType\DBDatetime;
@@ -1642,13 +1643,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();
@@ -4638,4 +4643,31 @@ 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();
+        $columns = DataObject::getSchema()->getIndexFields(static::class, $key);
+        $validationResult = ValidationResult::create();
+        if (count($columns) === 1) {
+            $duplicateField = $columns[0];
+            $validationResult->addFieldError(
+                $duplicateField,
+                _t(
+                    __CLASS__ . '.',
+                    '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__ . '.',
+                'Cannot create duplicate {type} - at least one of the following fields need to be changed: {fields}',
+                ['type' => $singleName, 'fields' => implode(', ', $duplicateFieldNames)]
+            ));
+        }
+        return $validationResult;
+    }
 }

src/ORM/DataObjectSchema.php

@@ -404,6 +404,20 @@ public function databaseIndexes($class, $aggregated = true)
         return array_merge($indexes, $this->databaseIndexes(get_parent_class($class ?? '')));
     }
 
+    /**
+     * Get an array of field names which are included in a given index.
+     * If the index doesn't exist, the array is empty.
+     */
+    public function getIndexFields(string $class, string $indexName): array
+    {
+        $indexes = $this->databaseIndexes($class);
+        $index = $indexes[$indexName] ?? null;
+        if (!$index) {
+            return [];
+        }
+        return $index['columns'];
+    }
+
     /**
      * Check if the given class has a table
      *

tests/php/ORM/DataObjectTest.php

@@ -68,6 +68,7 @@ class DataObjectTest extends SapphireTest
         DataObjectTest\OverriddenDataObject::class,
         DataObjectTest\InjectedDataObject::class,
         DataObjectTest\SettersAndGetters::class,
+        DataObjectTest\UniqueIndexObject::class,
     ];
 
     protected function setUp(): void
@@ -2782,4 +2783,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();
+    }
 }

tests/php/ORM/DataObjectTest/UniqueIndexObject.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class UniqueIndexObject extends DataObject implements TestOnly
+{
+    private static string $table_name = 'DataObjectTest_UniqueIndexObject';
+
+    private static array $db = [
+        'SingleField' => 'Varchar',
+        'Name' => 'Varchar',
+        'Code' => 'Varchar',
+    ];
+
+    private static array $indexes = [
+        'SingleFieldIndex' => [
+            'type' => 'unique',
+            'columns' => [
+                'SingleField',
+            ],
+        ],
+        'MultiFieldIndex' => [
+            'type' => 'unique',
+            'columns' => [
+                'Name',
+                'Code',
+            ],
+        ],
+    ];
+}