Skip to content

Commit d54e6fe

Browse files
committed
rely on db constraint for technical name uniqueness
Signed-off-by: samin-z <samin.zavarkesh@gmail.com>
1 parent dfd8f32 commit d54e6fe

1 file changed

Lines changed: 17 additions & 22 deletions

File tree

lib/Service/ColumnService.php

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -308,21 +308,19 @@ public function create(
308308
$item = Column::fromDto($columnDto);
309309
$item->setTitle($newTitle);
310310
$item->setTableId($table->getId());
311-
if ($item->getTechnicalName() !== null) {
312-
$this->assertTechnicalNameUnique($table->getId(), $item->getTechnicalName());
313-
}
314311
$this->updateMetadata($item, $userId, true);
315312

316313
try {
317314
$entity = $this->mapper->insert($item);
318315
} catch (\OCP\DB\Exception $e) {
319-
$this->logger->error($e->getMessage(), ['exception' => $e]);
320-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
316+
$this->handleColumnPersistDbException($e, get_class($this) . ' - ' . __FUNCTION__);
321317
}
322318
if ($entity->getTechnicalName() === null || $entity->getTechnicalName() === '') {
323319
$entity->setTechnicalName($this->buildDefaultTechnicalName($entity->getId()));
324320
try {
325321
$entity = $this->mapper->update($entity);
322+
} catch (\OCP\DB\Exception $e) {
323+
$this->handleColumnPersistDbException($e, get_class($this) . ' - ' . __FUNCTION__);
326324
} catch (Exception $e) {
327325
$this->logger->error($e->getMessage(), ['exception' => $e]);
328326
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
@@ -380,7 +378,6 @@ public function update(
380378
$item->setTitle($title);
381379
}
382380
if ($columnDto->getTechnicalName() !== null) {
383-
$this->assertTechnicalNameUnique($item->getTableId(), $columnDto->getTechnicalName(), $item->getId());
384381
$item->setTechnicalName($columnDto->getTechnicalName());
385382
}
386383
if ($columnDto->getType() !== null) {
@@ -427,7 +424,11 @@ public function update(
427424
$item->setCustomSettings($columnDto->getCustomSettings());
428425

429426
$this->updateMetadata($item, $userId);
430-
return $this->enhanceColumn($this->mapper->update($item));
427+
try {
428+
return $this->enhanceColumn($this->mapper->update($item));
429+
} catch (\OCP\DB\Exception $e) {
430+
$this->handleColumnPersistDbException($e, get_class($this) . ' - ' . __FUNCTION__);
431+
}
431432
} catch (BadRequestError $e) {
432433
throw $e;
433434
} catch (Exception $e) {
@@ -726,6 +727,10 @@ public function importColumn(Table $table, array $column): int {
726727
$newColumn->setTechnicalName($this->buildDefaultTechnicalName($newColumn->getId()));
727728
$newColumn = $this->mapper->update($newColumn);
728729
}
730+
} catch (BadRequestError $e) {
731+
throw $e;
732+
} catch (\OCP\DB\Exception $e) {
733+
$this->handleColumnPersistDbException($e, 'importColumn insert error');
729734
} catch (\Exception $e) {
730735
$this->logger->error('importColumn insert error: ' . $e->getMessage());
731736
throw new InternalError('importColumn insert error: ' . $e->getMessage());
@@ -741,22 +746,12 @@ private function buildDefaultTechnicalName(int $columnId): string {
741746
* @throws BadRequestError
742747
* @throws InternalError
743748
*/
744-
private function assertTechnicalNameUnique(int $tableId, string $technicalName, ?int $excludeCurrentColumnId = null): void {
745-
try {
746-
$columns = $this->mapper->findAllByTable($tableId);
747-
} catch (Exception $e) {
748-
$this->logger->error($e->getMessage(), ['exception' => $e]);
749-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
749+
private function handleColumnPersistDbException(\OCP\DB\Exception $e, string $context): void {
750+
if ($e->getReason() === \OCP\DB\Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
751+
throw new BadRequestError('Technical name must be unique in the table.');
750752
}
751753

752-
foreach ($columns as $column) {
753-
if ($excludeCurrentColumnId !== null && $column->getId() === $excludeCurrentColumnId) {
754-
continue;
755-
}
756-
757-
if ($column->getTechnicalName() === $technicalName) {
758-
throw new BadRequestError('Technical name must be unique in the table.');
759-
}
760-
}
754+
$this->logger->error($e->getMessage(), ['exception' => $e]);
755+
throw new InternalError($context . ': ' . $e->getMessage());
761756
}
762757
}

0 commit comments

Comments
 (0)