diff --git a/lib/Command/Clean.php b/lib/Command/Clean.php index e145ac90..80d5e294 100644 --- a/lib/Command/Clean.php +++ b/lib/Command/Clean.php @@ -178,7 +178,7 @@ class Clean extends Command { $this->row->setData(array_values($data)); try { - $this->rowMapper->update($this->row, $this->columnService->findAllByTable($this->row->getTableId())); + $this->rowMapper->update($this->row); $this->print('Row successfully updated', self::PRINT_LEVEL_SUCCESS); } catch (InternalError|PermissionError $e) { $this->print('Error while updating row to db.', self::PRINT_LEVEL_ERROR); diff --git a/lib/Db/ColumnMapper.php b/lib/Db/ColumnMapper.php index a72968cc..ab38369b 100644 --- a/lib/Db/ColumnMapper.php +++ b/lib/Db/ColumnMapper.php @@ -10,6 +10,7 @@ namespace OCA\Tables\Db; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\QBMapper; +use OCP\Cache\CappedMemoryCache; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -19,10 +20,12 @@ use Psr\Log\LoggerInterface; class ColumnMapper extends QBMapper { protected string $table = 'tables_columns'; private LoggerInterface $logger; + private CappedMemoryCache $cacheColumn; public function __construct(IDBConnection $db, LoggerInterface $logger) { parent::__construct($db, $this->table, Column::class); $this->logger = $logger; + $this->cacheColumn = new CappedMemoryCache(); } /** @@ -34,11 +37,18 @@ class ColumnMapper extends QBMapper { * @throws MultipleObjectsReturnedException */ public function find(int $id): Column { + $key = $this->getCacheKey($id); + if ($this->cacheColumn->hasKey($key)) { + return $this->cacheColumn->get($key); + } + $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from($this->table) ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))); - return $this->findEntity($qb); + $result = $this->findEntity($qb); + $this->cacheColumn->set($key, $result); + return $result; } /** @@ -48,25 +58,35 @@ class ColumnMapper extends QBMapper { * @throws Exception */ public function findAll(array $id): array { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->table) - ->where($qb->expr()->in('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT_ARRAY))); - return $this->findEntities($qb); - } + $result = []; + $missingIds = []; - /** - * @param int[] $ids - * - * @return Column[] - * @throws Exception - */ - public function findMultiple(array $ids): array { - $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->table) - ->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))); - return $this->findEntities($qb); + // Check cache first + foreach ($id as $columnId) { + $key = $this->getCacheKey($columnId); + if ($this->cacheColumn->hasKey($key)) { + $result[] = $this->cacheColumn->get($key); + } else { + $missingIds[] = $columnId; + } + } + + // If we have missing IDs, fetch them from database + if (!empty($missingIds)) { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->table) + ->where($qb->expr()->in('id', $qb->createNamedParameter($missingIds, IQueryBuilder::PARAM_INT_ARRAY))); + $dbResults = $this->findEntities($qb); + + // Cache the new results + foreach ($dbResults as $column) { + $this->cacheColumn->set($this->getCacheKey($column->getId()), $column); + $result[] = $column; + } + } + + return $result; } /** @@ -79,7 +99,19 @@ class ColumnMapper extends QBMapper { $qb->select('*') ->from($this->table) ->where($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId))); - return $this->findEntities($qb); + + $dbResults = $this->findEntities($qb); + + $result = []; + // Cache the new results + foreach ($dbResults as $column) { + $key = $this->getCacheKey($column->getId()); + if (!$this->cacheColumn->hasKey($key)) { + $this->cacheColumn->set($key, $column); + } + $result[] = $column; + } + return $result; } /** @@ -153,4 +185,27 @@ class ColumnMapper extends QBMapper { return 0; } } + + /** + * Preloads columns data in bulk to optimize caching and reduce database queries. + * This method efficiently loads column data for a given set of columns, filters, and sorts + * by fetching all required data in a single database operation. + */ + public function preloadColumns(array $columns, ?array $filters = null, ?array $sorts = null): void { + $columnIds = $columns; + if (!is_null($sorts) && count($sorts) > 0) { + $columnIds = [...$columns, ...array_column($sorts, 'columnId')]; + } + if (!is_null($filters) && count($filters) > 0) { + foreach ($filters as $filterGroup) { + array_push($columnIds, ...array_column($filterGroup, 'columnId')); + } + } + + $this->findAll(array_unique($columnIds)); + } + + private function getCacheKey(int $id): string { + return 'column_' . $id; + } } diff --git a/lib/Db/LegacyRowMapper.php b/lib/Db/LegacyRowMapper.php index 28c9041d..73e8ce01 100644 --- a/lib/Db/LegacyRowMapper.php +++ b/lib/Db/LegacyRowMapper.php @@ -425,7 +425,7 @@ class LegacyRowMapper extends QBMapper { * @throws InternalError */ public function transferLegacyRow(LegacyRow $legacyRow, array $columns) { - $this->rowMapper->insert($this->migrateLegacyRow($legacyRow, $columns), $columns); + $this->rowMapper->insert($this->migrateLegacyRow($legacyRow, $columns)); } /** diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 07ebe772..24bb433c 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -35,11 +35,6 @@ class Row2Mapper { protected UserHelper $userHelper; protected ColumnMapper $columnMapper; - /* @var Column[] $columns */ - private array $columns = []; - /* @var Column[] $columns */ - private array $allColumns = []; - private ColumnsHelper $columnsHelper; public function __construct(?string $userId, IDBConnection $db, LoggerInterface $logger, UserHelper $userHelper, RowSleeveMapper $rowSleeveMapper, ColumnsHelper $columnsHelper, ColumnMapper $columnMapper) { @@ -82,7 +77,6 @@ class Row2Mapper { * @throws NotFoundError */ public function find(int $id, array $columns): Row2 { - $this->setColumns($columns); $columnIdsArray = array_map(fn (Column $column) => $column->getId(), $columns); $rows = $this->getRows([$id], $columnIdsArray); if (count($rows) === 1) { @@ -159,7 +153,7 @@ class Row2Mapper { } /** - * @param Column[] $columns + * @param int[] $showColumnIds * @param int $tableId * @param int|null $limit * @param int|null $offset @@ -169,13 +163,12 @@ class Row2Mapper { * @return Row2[] * @throws InternalError */ - public function findAll(array $tableColumns, array $columns, int $tableId, ?int $limit = null, ?int $offset = null, ?array $filter = null, ?array $sort = null, ?string $userId = null): array { - $this->setColumns($columns, $tableColumns); - $columnIdsArray = array_map(fn (Column $column) => $column->getId(), $columns); + public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, ?int $offset = null, ?array $filter = null, ?array $sort = null, ?string $userId = null): array { + $this->columnMapper->preloadColumns($showColumnIds, $filter, $sort); $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $limit, $offset); - return $this->getRows($wantedRowIdsArray, $columnIdsArray, $sort ?? []); + return $this->getRows($wantedRowIdsArray, $showColumnIds, $sort ?? []); } /** @@ -272,13 +265,13 @@ class Row2Mapper { private function addSortQueryForMultipleSleeveFinder(IQueryBuilder $qb, string $sleevesAlias, array $sort): void { $i = 1; foreach ($sort as $sortData) { - if (!isset($this->columns[$sortData['columnId']]) && !isset($this->allColumns[$sortData['columnId']]) && $sortData['columnId'] > 0) { + $column = $sortData['columnId'] > 0 ? $this->columnMapper->find($sortData['columnId']) : null; + if ($column === null && $sortData['columnId'] > 0) { throw new InternalError('No column found to build filter with for id ' . $sortData['columnId']); } // if is normal column if ($sortData['columnId'] >= 0) { - $column = $this->columns[$sortData['columnId']] ?? $this->allColumns[$sortData['columnId']]; $valueTable = 'tables_row_cells_' . $column->getType(); $alias = 'sort' . $i; $qb->leftJoin($sleevesAlias, $valueTable, $alias, @@ -344,15 +337,14 @@ class Row2Mapper { $filterExpressions = []; foreach ($filterGroup as $filter) { $columnId = $filter['columnId']; + $column = $columnId > 0 ? $this->columnMapper->find($columnId) : null; // Fail if the filter is for a column that is not in the list and no meta column - if (!isset($this->columns[$columnId]) && !isset($this->allColumns[$columnId]) && $columnId > 0) { + if ($column === null && $columnId > 0) { throw new InternalError('No column found to build filter with for id ' . $columnId); } // if is normal column if ($columnId >= 0) { - $column = $this->columns[$columnId] ?? $this->allColumns[$columnId]; - $sql = $qb->expr()->in( 'id', $qb->createFunction($this->getFilterExpression($qb, $column, $filter['operator'], $filter['value'])->getSQL()) @@ -557,13 +549,14 @@ class Row2Mapper { break; } - $columnType = $this->columns[$rowData['column_id']]->getType(); + $column = $this->columnMapper->find($rowData['column_id']); + $columnType = $column->getType(); $cellClassName = 'OCA\Tables\Db\RowCell' . ucfirst($columnType); $entity = call_user_func($cellClassName . '::fromRowData', $rowData); // >5.2.3 if (!isset($cellMapperCache[$columnType])) { $cellMapperCache[$columnType] = $this->getCellMapperFromType($columnType); } - $value = $cellMapperCache[$columnType]->formatEntity($this->columns[$rowData['column_id']], $entity); + $value = $cellMapperCache[$columnType]->formatEntity($column, $entity); $compositeKey = (string)$rowData['row_id'] . ',' . (string)$rowData['column_id']; if ($cellMapperCache[$columnType]->hasMultipleValues()) { if (array_key_exists($compositeKey, $rowValues)) { @@ -594,14 +587,11 @@ class Row2Mapper { /** * @param Row2 $row - * @param Column[] $columns * @return Row2 * @throws InternalError * @throws Exception */ - public function insert(Row2 $row, array $columns): Row2 { - $this->setColumns($columns); - + public function insert(Row2 $row): Row2 { if ($row->getId()) { // if row has an id from migration or import etc. $rowSleeve = $this->createRowSleeveFromExistingData($row->getId(), $row->getTableId(), $row->getCreatedAt(), $row->getCreatedBy(), $row->getLastEditBy(), $row->getLastEditAt()); @@ -611,12 +601,9 @@ class Row2Mapper { $row->setId($rowSleeve->getId()); } - // if the table/view has columns - if (count($columns) > 0) { - // write all cells to its db-table - foreach ($row->getData() as $cell) { - $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy()); - } + // write all cells to its db-table + foreach ($row->getData() as $cell) { + $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy()); } return $row; @@ -625,14 +612,10 @@ class Row2Mapper { /** * @throws InternalError */ - public function update(Row2 $row, array $columns): Row2 { - if (!$columns) { - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': columns are missing'); - } - $this->setColumns($columns); - + public function update(Row2 $row): Row2 { + $changedCells = $row->getChangedCells(); // if nothing has changed - if (count($row->getChangedCells()) === 0) { + if (count($changedCells) === 0) { return $row; } @@ -646,8 +629,10 @@ class Row2Mapper { throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } + $this->columnMapper->preloadColumns(array_column($changedCells, 'columnId')); + // write all changed cells to its db-table - foreach ($row->getChangedCells() as $cell) { + foreach ($changedCells as $cell) { $this->insertOrUpdateCell($sleeve->getId(), $cell['columnId'], $cell['value']); } @@ -704,21 +689,19 @@ class Row2Mapper { * @throws InternalError */ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEditAt = null, ?string $lastEditBy = null): void { - if (!isset($this->columns[$columnId])) { + try { + $column = $this->columnMapper->find($columnId); + } catch (DoesNotExistException $e) { $e = new Exception('Can not insert cell, because the given column-id is not known'); $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } - // insert new cell - $column = $this->columns[$columnId]; $cellMapper = $this->getCellMapper($column); - $column = $this->columns[$columnId]; - try { - $cellClassName = 'OCA\Tables\Db\RowCell' . ucfirst($this->columns[$columnId]->getType()); + $cellClassName = 'OCA\Tables\Db\RowCell' . ucfirst($column->getType()); if ($cellMapper->hasMultipleValues()) { foreach ($value as $val) { /** @var RowCellSuper $cell */ @@ -761,7 +744,8 @@ class Row2Mapper { * @throws InternalError */ private function insertOrUpdateCell(int $rowId, int $columnId, $value): void { - $cellMapper = $this->getCellMapper($this->columns[$columnId]); + $column = $this->columnMapper->find($columnId); + $cellMapper = $this->getCellMapper($column); try { if ($cellMapper->hasMultipleValues()) { $this->atomic(function () use ($cellMapper, $rowId, $columnId, $value) { @@ -772,7 +756,7 @@ class Row2Mapper { }, $this->db); } else { $cell = $cellMapper->findByRowAndColumn($rowId, $columnId); - $this->updateCell($cell, $cellMapper, $value, $this->columns[$columnId]); + $this->updateCell($cell, $cellMapper, $value, $column); } } catch (DoesNotExistException) { $this->insertCell($rowId, $columnId, $value); @@ -782,21 +766,6 @@ class Row2Mapper { } } - /** - * @param Column[] $columns - */ - private function setColumns(array $columns, array $tableColumns = []): void { - foreach ($columns as $column) { - $this->columns[$column->getId()] = $column; - } - - // We hold a list of all table columns to be used in filter expression building for those not visible in the view - foreach ($tableColumns as $column) { - $this->allColumns[$column->getId()] = $column; - } - - } - private function getCellMapper(Column $column): RowCellMapperSuper { return $this->getCellMapperFromType($column->getType()); } @@ -858,14 +827,13 @@ class Row2Mapper { /** * @param View $view * @param string $userId - * @param Column[] $columns * @return int */ - public function countRowsForView(View $view, string $userId, array $columns): int { - $this->setColumns($columns); - + public function countRowsForView(View $view, string $userId): int { $filter = $view->getFilterArray(); try { + $this->columnMapper->preloadColumns($view->getColumnsArray(), $filter, $view->getSortArray()); + $rowIds = $this->getWantedRowIds($userId, $view->getTableId(), $filter); } catch (InternalError $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index e4d90599..22f393a0 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -75,7 +75,9 @@ class RowService extends SuperService { try { if ($this->permissionsService->canReadRowsByElementId($tableId, 'table', $userId)) { $tableColumns = $this->columnMapper->findAllByTable($tableId); - return $this->row2Mapper->findAll($tableColumns, $tableColumns, $tableId, $limit, $offset, null, null, $userId); + $showColumnIds = array_map(fn (Column $column) => $column->getId(), $tableColumns); + + return $this->row2Mapper->findAll($showColumnIds, $tableId, $limit, $offset, null, null, $userId); } else { throw new PermissionError('no read access to table id = ' . $tableId); } @@ -100,10 +102,8 @@ class RowService extends SuperService { try { if ($this->permissionsService->canReadRowsByElementId($viewId, 'view', $userId)) { $view = $this->viewMapper->find($viewId); - $filterColumns = $this->columnMapper->findAll($view->getColumnIds()); - $tableColumns = $this->columnMapper->findAllByTable($view->getTableId()); - return $this->row2Mapper->findAll($tableColumns, $filterColumns, $view->getTableId(), $limit, $offset, $view->getFilterArray(), $view->getSortArray(), $userId); + return $this->row2Mapper->findAll($view->getColumnIds(), $view->getTableId(), $limit, $offset, $view->getFilterArray(), $view->getSortArray(), $userId); } else { throw new PermissionError('no read access to view id = ' . $viewId); } @@ -178,7 +178,7 @@ class RowService extends SuperService { throw new PermissionError('create row at the view id = ' . $viewId . ' is not allowed.'); } - $columns = $this->columnMapper->findMultiple($view->getColumnIds()); + $columns = $this->columnMapper->findAll($view->getColumnIds()); } if ($tableId) { try { @@ -212,7 +212,7 @@ class RowService extends SuperService { $row2->setTableId($tableId); $row2->setData($data); try { - $insertedRow = $this->row2Mapper->insert($row2, $this->columnMapper->findAllByTable($tableId)); + $insertedRow = $this->row2Mapper->insert($row2); $this->eventDispatcher->dispatchTyped(new RowAddedEvent($insertedRow)); @@ -426,7 +426,7 @@ class RowService extends SuperService { // fetch all needed columns try { - $columns = $this->columnMapper->findMultiple($view->getColumnIds()); + $columns = $this->columnMapper->findAll($view->getColumnIds()); } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); @@ -468,7 +468,7 @@ class RowService extends SuperService { } } - $updatedRow = $this->row2Mapper->update($item, $columns); + $updatedRow = $this->row2Mapper->update($item); $this->eventDispatcher->dispatchTyped(new RowUpdatedEvent($updatedRow, $previousData)); @@ -604,7 +604,7 @@ class RowService extends SuperService { public function getViewRowsCount(View $view, string $userId): int { if ($this->permissionsService->canReadRowsByElementId($view->getId(), 'view', $userId)) { try { - return $this->row2Mapper->countRowsForView($view, $userId, $this->columnMapper->findAllByTable($view->getTableId())); + return $this->row2Mapper->countRowsForView($view, $userId); } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); return 0;