VladPolskiy/nextcloud-tables
1
0
Fork 0
mirror of https://github.com/nextcloud/tables.git synced 2025-07-21 18:35:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: simplify Columns::findAllByTable method

Browse Source 
- simply this method, kill a lot of `null`s in the majority of the calls
- add a destinct method to cover the edge case
- improves clarity

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon
2025-06-17 13:31:31 +02:00
parent 82c99999b2
commit 37ca2f75ac
9 changed files with 49 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
lib/Analytics/AnalyticsDatasource.php
View File

@ -208,7 +208,7 @@ class AnalyticsDatasource implements IDatasource {
$rows = $this->rowService->findAllByView($nodeId, $this->userId, $limit, $offset);
} else {
// if no nodeType is provided, the old table selection is used to not break anything
$columns = $this->columnService->findAllByTable($nodeId, null, $this->userId);
$columns = $this->columnService->findAllByTable($nodeId, $this->userId);
$rows = $this->rowService->findAllByTable($nodeId, $this->userId, $limit, $offset);
}

2
lib/Api/V1Api.php
View File

@ -47,7 +47,7 @@ class V1Api {
$rows = $this->rowService->findAllByView($nodeId, $this->userId, $limit, $offset);
} else {
// if no nodeType is provided, the old table selection is used to not break anything
$columns = $this->columnService->findAllByTable($nodeId, null, $this->userId);
$columns = $this->columnService->findAllByTable($nodeId, $this->userId);
$rows = $this->rowService->findAllByTable($nodeId, $this->userId, $limit, $offset);
}

2
lib/Command/Clean.php
View File

@ -97,7 +97,7 @@ class Clean extends Command {
return;
}
$tableId = $this->rowMapper->getTableIdForRow($nextRowId);
$columns = $this->columnService->findAllByTable($tableId, null, '');
$columns = $this->columnService->findAllByTable($tableId, '');
$this->row = $this->rowMapper->find($nextRowId, $columns);
$this->offset = $this->row->getId();
} catch (Exception $e) {

4
lib/Command/TransferLegacyRows.php
View File

@ -134,7 +134,7 @@ class TransferLegacyRows extends Command {
* @throws Exception
*/
private function transferTable(Table $table, OutputInterface $output): void {
$columns = $this->columnService->findAllByTable($table->getId(), null, '');
$columns = $this->columnService->findAllByTable($table->getId(), '');
$output->writeln('---- Found ' . count($columns) . ' columns');
$legacyRows = $this->legacyRowMapper->findAllByTable($table->getId());
@ -155,7 +155,7 @@ class TransferLegacyRows extends Command {
$output->writeln('Start deleting data for tables that should be transferred.');
foreach ($tables as $table) {
try {
$columns = $this->columnService->findAllByTable($table->getId(), null, '');
$columns = $this->columnService->findAllByTable($table->getId(), '');
} catch (InternalError|PermissionError $e) {
$output->writeln('Could not delete data for table ' . $table->getId());
break;

20
lib/Controller/Api1Controller.php
View File

@ -708,7 +708,21 @@ class Api1Controller extends ApiController {
#[CORS]
public function indexTableColumns(int $tableId, ?int $viewId): DataResponse {
try {
return new DataResponse($this->columnService->formatColumns($this->columnService->findAllByTable($tableId, $viewId)));
try {
// permission check in service class
// they might be missing legally when only the view was shared
$columns = $this->columnService->findAllByTable($tableId);
} catch (PermissionError $e) {
if (!$viewId) {
throw $e;
}
$view = $this->viewService->find($viewId, false, $this->userId);
if ($tableId !== $view->getTableId()) {
throw new PermissionError('Given table is not a parent of the given view.');
}
$columns = $this->columnService->findAllByManagedView($view, $this->userId);
}
return new DataResponse($this->columnService->formatColumns($columns));
} catch (PermissionError $e) {
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
@ -717,6 +731,10 @@ class Api1Controller extends ApiController {
$this->logger->error('An internal error or exception occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_INTERNAL_SERVER_ERROR);
} catch (NotFoundError $e) {
$this->logger->warning('The view could not be found: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_NOT_FOUND);
}
}

2
lib/Migration/NewDbStructureRepairStep.php
View File

@ -92,7 +92,7 @@ class NewDbStructureRepairStep implements IRepairStep {
* @throws Exception
*/
private function transferTable(Table $table, IOutput $output) {
$columns = $this->columnService->findAllByTable($table->getId(), null, '');
$columns = $this->columnService->findAllByTable($table->getId(), '');
$output->info('---- Found ' . count($columns) . ' columns');
$legacyRows = $this->legacyRowMapper->findAllByTable($table->getId());

25
lib/Service/ColumnService.php
View File

@ -12,6 +12,7 @@ use Exception;
use OCA\Tables\Db\Column;
use OCA\Tables\Db\ColumnMapper;
use OCA\Tables\Db\TableMapper;
use OCA\Tables\Db\View;
use OCA\Tables\Dto\Column as ColumnDto;
use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
@ -64,8 +65,8 @@ class ColumnService extends SuperService {
* @throws InternalError
* @throws PermissionError
*/
public function findAllByTable(int $tableId, ?int $viewId = null, ?string $userId = null): array {
if ($this->permissionsService->canReadColumnsByTableId($tableId, $userId) || ($viewId != null && $this->permissionsService->canReadColumnsByViewId($viewId, $userId))) {
public function findAllByTable(int $tableId, ?string $userId = null): array {
if ($this->permissionsService->canReadColumnsByTableId($tableId, $userId)) {
try {
return $this->enhanceColumns($this->mapper->findAllByTable($tableId));
} catch (\OCP\DB\Exception $e) {
@ -77,6 +78,24 @@ class ColumnService extends SuperService {
}
}
/**
* @throws InternalError
* @throws PermissionError
* @return Column[]
*/
public function findAllByManagedView(View $view, string $userId): array {
if ($this->permissionsService->canManageView($view, $userId)) {
try {
return $this->enhanceColumns($this->mapper->findAllByTable($view->getTableId()));
} catch (\OCP\DB\Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
}
} else {
throw new PermissionError('no manage access to view id = ' . $view->getId());
}
}
/**
* @param int $viewId
* @param string|null $userId
@ -428,7 +447,7 @@ class ColumnService extends SuperService {
if ($viewId) {
$allColumns = $this->findAllByView($viewId, $userId);
} elseif ($tableId) {
$allColumns = $this->findAllByTable($tableId, null, $userId);
$allColumns = $this->findAllByTable($tableId, $userId);
} else {
$e = new Exception('Either tableId nor viewId is given.');
$this->logger->error($e->getMessage(), ['exception' => $e]);

2
lib/Service/TableService.php
View File

@ -404,7 +404,7 @@ class TableService extends SuperService {
// delete all columns for that table
try {
$columns = $this->columnService->findAllByTable($id, null, $userId);
$columns = $this->columnService->findAllByTable($id, $userId);
} catch (InternalError|PermissionError $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());

2
lib/Service/ViewService.php
View File

@ -287,7 +287,7 @@ class ViewService extends SuperService {
$columnService = \OCP\Server::get(ColumnService::class);
$columnIds = array_column(\json_decode($value, true), 'columnId');
$availableColumns = $columnService->findAllByTable($view->getTableId(), $view->getId(), $this->userId);
$availableColumns = $columnService->findAllByManagedView($view, $userId);
$availableColumns = array_map(static fn (Column $column) => $column->getId(), $availableColumns);
foreach ($columnIds as $columnId) {
if (!Column::isValidMetaTypeId($columnId) && !in_array($columnId, $availableColumns, true)) {