diff --git a/appinfo/info.xml b/appinfo/info.xml index 4f05ced258..82ef71244a 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -26,7 +26,7 @@ Share your tables and views with users and groups within your cloud. Have a good time and manage whatever you want. ]]> - 2.2.0 + 2.3.0 AGPL-3.0-or-later Nextcloud GmbH and Nextcloud contributors Tables @@ -58,6 +58,7 @@ Have a good time and manage whatever you want. OCA\Tables\Migration\NewDbStructureRepairStep OCA\Tables\Migration\DbRowSleeveSequence + OCA\Tables\Migration\CacheSleeveCells diff --git a/lib/Db/ColumnMapper.php b/lib/Db/ColumnMapper.php index 527b728d97..e0266c5ede 100644 --- a/lib/Db/ColumnMapper.php +++ b/lib/Db/ColumnMapper.php @@ -91,7 +91,7 @@ public function findAll(array $id): array { /** * @param integer $tableId - * @return array + * @return Column[] * @throws Exception */ public function findAllByTable(int $tableId): array { @@ -116,7 +116,7 @@ public function findAllByTable(int $tableId): array { /** * @param integer $tableID - * @return array + * @return int[] * @throws Exception */ public function findAllIdsByTable(int $tableID): array { diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 7ec48500f9..50623eff89 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -10,6 +10,8 @@ use DateTime; use DateTimeImmutable; use OCA\Tables\Constants\UsergroupType; +use OCA\Tables\Db\RowLoader\CachedRowLoader; +use OCA\Tables\Db\RowLoader\NormalizedRowLoader; use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; use OCA\Tables\Helper\ColumnsHelper; @@ -18,20 +20,14 @@ use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\AppFramework\Db\TTransactional; use OCP\DB\Exception; -use OCP\DB\IResult; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -use OCP\Server; -use Psr\Container\ContainerExceptionInterface; -use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; use Throwable; class Row2Mapper { use TTransactional; - private const DB_CHUNK_SIZE = 1_000; - private RowSleeveMapper $rowSleeveMapper; private ?string $userId; private IDBConnection $db; @@ -40,8 +36,15 @@ class Row2Mapper { protected ColumnMapper $columnMapper; private ColumnsHelper $columnsHelper; + /** + * @var array + */ + private array $rowLoaders; - public function __construct(?string $userId, IDBConnection $db, LoggerInterface $logger, UserHelper $userHelper, RowSleeveMapper $rowSleeveMapper, ColumnsHelper $columnsHelper, ColumnMapper $columnMapper) { + public function __construct(?string $userId, IDBConnection $db, LoggerInterface $logger, UserHelper $userHelper, RowSleeveMapper $rowSleeveMapper, ColumnsHelper $columnsHelper, ColumnMapper $columnMapper, + NormalizedRowLoader $normalizedRowLoader, + CachedRowLoader $cachedRowLoader, + ) { $this->rowSleeveMapper = $rowSleeveMapper; $this->userId = $userId; $this->db = $db; @@ -49,6 +52,10 @@ public function __construct(?string $userId, IDBConnection $db, LoggerInterface $this->userHelper = $userHelper; $this->columnsHelper = $columnsHelper; $this->columnMapper = $columnMapper; + $this->rowLoaders = [ + RowLoader\RowLoader::LOADER_NORMALIZED => $normalizedRowLoader, + RowLoader\RowLoader::LOADER_CACHED => $cachedRowLoader, + ]; } /** @@ -60,7 +67,7 @@ public function delete(Row2 $row): Row2 { $this->db->beginTransaction(); try { foreach ($this->columnsHelper->columns as $columnType) { - $this->getCellMapperFromType($columnType)->deleteAllForRow($row->getId()); + $this->columnsHelper->getCellMapperFromType($columnType)->deleteAllForRow($row->getId()); } $this->rowSleeveMapper->deleteById($row->getId()); $this->db->commit(); @@ -182,7 +189,7 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $sort, $limit, $offset); // Get rows without SQL sorting - $rows = $this->getRows($wantedRowIdsArray, $showColumnIds); + $rows = $this->getRows($wantedRowIdsArray, $showColumnIds, RowLoader\RowLoader::LOADER_CACHED); // Sort rows in PHP to preserve the order from getWantedRowIds return $this->sortRowsByIds($rows, $wantedRowIdsArray); @@ -195,79 +202,26 @@ public function findAll(array $showColumnIds, int $tableId, ?int $limit = null, /** * @param array $rowIds * @param array $columnIds + * @param RowLoader\RowLoader::LOADER_* $loader * @return Row2[] * @throws InternalError */ - private function getRows(array $rowIds, array $columnIds): array { + private function getRows(array $rowIds, array $columnIds, string $loader = RowLoader\RowLoader::LOADER_NORMALIZED): array { if (empty($rowIds) || empty($columnIds)) { return []; } - $allRows = []; - foreach (array_chunk($rowIds, self::DB_CHUNK_SIZE) as $rowIdChunk) { - $allRows[] = $this->getRowsChunk($rowIdChunk, $columnIds); + $columns = $mappers = []; + foreach ($columnIds as $columnId) { + $columns[$columnId] = $this->columnMapper->find($columnId); + $mappers[$columnId] = $this->getCellMapper($columns[$columnId]); } - return array_merge(...$allRows); - } - - /** - * @param array $rowIds - * @param array $columnIds - * @return Row2[] - * @throws InternalError - */ - private function getRowsChunk(array $rowIds, array $columnIds): array { - $qb = $this->db->getQueryBuilder(); - $qbSqlForColumnTypes = null; - foreach ($this->columnsHelper->columns as $columnType) { - $qbTmp = $this->db->getQueryBuilder(); - $qbTmp->select('row_id', 'column_id', 'last_edit_at', 'last_edit_by') - ->selectAlias($qb->expr()->castColumn('value', IQueryBuilder::PARAM_STR), 'value'); - - // This is not ideal but I cannot think of a good way to abstract this away into the mapper right now - // Ideally we dynamically construct this query depending on what additional selects the column type requires - // however the union requires us to match the exact number of selects for each column type - if ($columnType === Column::TYPE_USERGROUP) { - $qbTmp->selectAlias($qb->expr()->castColumn('value_type', IQueryBuilder::PARAM_STR), 'value_type'); - } else { - $qbTmp->selectAlias($qbTmp->createFunction('NULL'), 'value_type'); - } - - $qbTmp - ->from('tables_row_cells_' . $columnType) - ->where($qb->expr()->in('column_id', $qb->createNamedParameter($columnIds, IQueryBuilder::PARAM_INT_ARRAY, ':columnIds'))) - ->andWhere($qb->expr()->in('row_id', $qb->createNamedParameter($rowIds, IQueryBuilder::PARAM_INT_ARRAY, ':rowsIds'))); - - if ($qbSqlForColumnTypes) { - $qbSqlForColumnTypes .= ' UNION ALL ' . $qbTmp->getSQL() . ' '; - } else { - $qbSqlForColumnTypes = '(' . $qbTmp->getSQL(); - } - } - $qbSqlForColumnTypes .= ')'; - - $qb->select('row_id', 'column_id', 'created_by', 'created_at', 't1.last_edit_by', 't1.last_edit_at', 'value', 'table_id') - // Also should be more generic (see above) - ->addSelect('value_type') - ->from($qb->createFunction($qbSqlForColumnTypes), 't1') - ->innerJoin('t1', 'tables_row_sleeves', 'rs', 'rs.id = t1.row_id'); - - try { - $result = $qb->executeQuery(); - } catch (Exception $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), ); - } - - try { - $sleeves = $this->rowSleeveMapper->findMultiple($rowIds); - } catch (Exception $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + $rows = []; + foreach ($this->rowLoaders[$loader]->getRows($rowIds, $columns, $mappers) as $row) { + $rows[] = $row; } - - return $this->parseEntities($result, $sleeves); + return $rows; } /** @@ -656,61 +610,6 @@ private function getSqlOperator(string $operator, IQueryBuilder $qb, string $col } } - /** - * @param IResult $result - * @param RowSleeve[] $sleeves - * @return Row2[] - * @throws InternalError - */ - private function parseEntities(IResult $result, array $sleeves): array { - $rows = []; - foreach ($sleeves as $sleeve) { - $rows[$sleeve->getId()] = new Row2(); - $rows[$sleeve->getId()]->setId($sleeve->getId()); - $rows[$sleeve->getId()]->setCreatedBy($sleeve->getCreatedBy()); - $rows[$sleeve->getId()]->setCreatedAt($sleeve->getCreatedAt()); - $rows[$sleeve->getId()]->setLastEditBy($sleeve->getLastEditBy()); - $rows[$sleeve->getId()]->setLastEditAt($sleeve->getLastEditAt()); - $rows[$sleeve->getId()]->setTableId($sleeve->getTableId()); - } - - $rowValues = []; - $keyToColumnId = []; - $keyToRowId = []; - $cellMapperCache = []; - - while ($rowData = $result->fetch()) { - if (!isset($rowData['row_id'], $rows[$rowData['row_id']])) { - break; - } - - $column = $this->columnMapper->find($rowData['column_id']); - $columnType = $column->getType(); - if (!isset($cellMapperCache[$columnType])) { - $cellMapperCache[$columnType] = $this->getCellMapperFromType($columnType); - } - $value = $cellMapperCache[$columnType]->formatRowData($column, $rowData); - $compositeKey = (string)$rowData['row_id'] . ',' . (string)$rowData['column_id']; - if ($cellMapperCache[$columnType]->hasMultipleValues()) { - if (array_key_exists($compositeKey, $rowValues)) { - $rowValues[$compositeKey][] = $value; - } else { - $rowValues[$compositeKey] = [$value]; - } - } else { - $rowValues[$compositeKey] = $value; - } - $keyToColumnId[$compositeKey] = $rowData['column_id']; - $keyToRowId[$compositeKey] = $rowData['row_id']; - } - - foreach ($rowValues as $compositeKey => $value) { - $rows[$keyToRowId[$compositeKey]]->addCell($keyToColumnId[$compositeKey], $value); - } - - return array_values($rows); - } - /** * @throws InternalError */ @@ -739,9 +638,12 @@ public function insert(Row2 $row, ?string $userId = null): Row2 { } // write all cells to its db-table + $cachedCells = []; foreach ($row->getData() as $cell) { - $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy()); + $cachedCells[$cell['columnId']] = $this->insertCell($rowSleeve->getId(), $cell['columnId'], $cell['value'], $rowSleeve->getLastEditAt(), $rowSleeve->getLastEditBy()); } + $rowSleeve->setCachedCellsArray($cachedCells); + $this->rowSleeveMapper->update($rowSleeve); return $row; } @@ -766,7 +668,6 @@ public function update(Row2 $row, ?string $userId = null): Row2 { try { $sleeve = $this->rowSleeveMapper->find($row->getId()); $this->updateMetaData($sleeve); - $this->rowSleeveMapper->update($sleeve); } catch (DoesNotExistException|MultipleObjectsReturnedException|Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); @@ -775,9 +676,12 @@ public function update(Row2 $row, ?string $userId = null): Row2 { $this->columnMapper->preloadColumns(array_column($changedCells, 'columnId')); // write all changed cells to its db-table + $cachedCells = $sleeve->getCachedCellsArray(); foreach ($changedCells as $cell) { - $this->insertOrUpdateCell($sleeve->getId(), $cell['columnId'], $cell['value']); + $cachedCells[$cell['columnId']] = $this->insertOrUpdateCell($sleeve->getId(), $cell['columnId'], $cell['value']); } + $sleeve->setCachedCellsArray($cachedCells); + $this->rowSleeveMapper->update($sleeve); return $row; } @@ -829,9 +733,11 @@ private function updateMetaData($entity, bool $setCreate = false, ?string $lastE /** * Insert a cell to its specific db-table * + * @return array normalized cell data + * * @throws InternalError */ - private function insertCell(int $rowId, int $columnId, $value, ?string $lastEditAt = null, ?string $lastEditBy = null): void { + private function insertCell(int $rowId, int $columnId, $value, ?string $lastEditAt = null, ?string $lastEditBy = null): array { try { $column = $this->columnMapper->find($columnId); } catch (DoesNotExistException $e) { @@ -841,7 +747,7 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit // insert new cell $cellMapper = $this->getCellMapper($column); - + $cachedCell = []; try { $cellClassName = 'OCA\Tables\Db\RowCell' . ucfirst($column->getType()); if ($cellMapper->hasMultipleValues()) { @@ -853,6 +759,7 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit $this->updateMetaData($cell, false, $lastEditAt, $lastEditBy); $cellMapper->applyDataToEntity($column, $cell, $val); $cellMapper->insert($cell); + $cachedCell[] = $cellMapper->toArray($cell); } } else { /** @var RowCellSuper $cell */ @@ -862,11 +769,14 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit $this->updateMetaData($cell, false, $lastEditAt, $lastEditBy); $cellMapper->applyDataToEntity($column, $cell, $value); $cellMapper->insert($cell); + $cachedCell = $cellMapper->toArray($cell); } } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError('Failed to insert column: ' . $e->getMessage(), 0, $e); } + + return $cachedCell; } /** @@ -874,53 +784,51 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit * @param RowCellMapperSuper $mapper * @param mixed $value the value should be parsed to the correct format within the row service * @param Column $column + * + * @return array normalized cell data * @throws InternalError */ - private function updateCell(RowCellSuper $cell, RowCellMapperSuper $mapper, $value, Column $column): void { - $this->getCellMapper($column)->applyDataToEntity($column, $cell, $value); + private function updateCell(RowCellSuper $cell, RowCellMapperSuper $mapper, $value, Column $column): array { + $cellMapper = $this->getCellMapper($column); + $cellMapper->applyDataToEntity($column, $cell, $value); $this->updateMetaData($cell); $mapper->updateWrapper($cell); + + return $cellMapper->toArray($cell); } /** + * @return array normalized cell data * @throws InternalError */ - private function insertOrUpdateCell(int $rowId, int $columnId, $value): void { + private function insertOrUpdateCell(int $rowId, int $columnId, $value): array { $column = $this->columnMapper->find($columnId); $cellMapper = $this->getCellMapper($column); + $cachedCell = []; try { if ($cellMapper->hasMultipleValues()) { - $this->atomic(function () use ($cellMapper, $rowId, $columnId, $value) { + $this->atomic(function () use ($cellMapper, $rowId, $columnId, $value, &$cachedCell) { // For a usergroup field with mutiple values, each is inserted as a new cell // we need to delete all previous cells for this row and column, otherwise we get duplicates $cellMapper->deleteAllForColumnAndRow($columnId, $rowId); - $this->insertCell($rowId, $columnId, $value); + $cachedCell = $this->insertCell($rowId, $columnId, $value); }, $this->db); } else { $cell = $cellMapper->findByRowAndColumn($rowId, $columnId); - $this->updateCell($cell, $cellMapper, $value, $column); + $cachedCell = $this->updateCell($cell, $cellMapper, $value, $column); } } catch (DoesNotExistException) { - $this->insertCell($rowId, $columnId, $value); + $cachedCell = $this->insertCell($rowId, $columnId, $value); } catch (MultipleObjectsReturnedException|Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), $e->getCode(), $e); } - } - private function getCellMapper(Column $column): RowCellMapperSuper { - return $this->getCellMapperFromType($column->getType()); + return $cachedCell; } - private function getCellMapperFromType(string $columnType): RowCellMapperSuper { - $cellMapperClassName = 'OCA\Tables\Db\RowCell' . ucfirst($columnType) . 'Mapper'; - /** @var RowCellMapperSuper $cellMapper */ - try { - return Server::get($cellMapperClassName); - } catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) { - $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); - } + private function getCellMapper(Column $column): RowCellMapperSuper { + return $this->columnsHelper->getCellMapperFromType($column->getType()); } /** diff --git a/lib/Db/RowCellMapperSuper.php b/lib/Db/RowCellMapperSuper.php index be377c5d87..acfcbe25b3 100644 --- a/lib/Db/RowCellMapperSuper.php +++ b/lib/Db/RowCellMapperSuper.php @@ -49,6 +49,10 @@ public function applyDataToEntity(Column $column, RowCellSuper $cell, $data): vo $cell->setValue($data); } + public function toArray(RowCellSuper $cell): array { + return ['value' => $cell->getValue()]; + } + public function getDbParamType() { return IQueryBuilder::PARAM_STR; } @@ -113,6 +117,33 @@ public function findByRowAndColumn(int $rowId, int $columnId): RowCellSuper { return $this->findEntity($qb); } + /** + * @throws MultipleObjectsReturnedException + * @throws DoesNotExistException + * @throws Exception + * @return RowCellSuper[] + */ + public function findManyByRowAndColumn(int $rowId, int $columnId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->tableName) + ->where($qb->expr()->eq('row_id', $qb->createNamedParameter($rowId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('column_id', $qb->createNamedParameter($columnId, IQueryBuilder::PARAM_INT))); + return $this->findEntities($qb); + } + + /** + * @throws Exception + * @return RowCellSuper[] + */ + public function findAllForRow(int $rowId): array { + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->tableName) + ->where($qb->expr()->eq('row_id', $qb->createNamedParameter($rowId, IQueryBuilder::PARAM_INT))); + return $this->findEntities($qb); + } + /** * @throws MultipleObjectsReturnedException * @throws DoesNotExistException diff --git a/lib/Db/RowCellUsergroupMapper.php b/lib/Db/RowCellUsergroupMapper.php index af975a8c37..2ea15230d0 100644 --- a/lib/Db/RowCellUsergroupMapper.php +++ b/lib/Db/RowCellUsergroupMapper.php @@ -60,6 +60,13 @@ public function formatRowData(Column $column, array $row) { ]; } + public function toArray(RowCellSuper $cell): array { + return [ + 'value' => $cell->getValue(), + 'value_type' => $cell->getValueType(), + ]; + } + public function hasMultipleValues(): bool { return true; } diff --git a/lib/Db/RowLoader/CachedRowLoader.php b/lib/Db/RowLoader/CachedRowLoader.php new file mode 100644 index 0000000000..b5c4e17243 --- /dev/null +++ b/lib/Db/RowLoader/CachedRowLoader.php @@ -0,0 +1,92 @@ +getRowsChunk($chunkedRowIds, $columns, $mappers); + } + } + + /** + * @param array $rowIds + * @param array $columns Column per columnId + * @param array $mappers Mapper per columnId + * @return Row2[] + * @throws InternalError + */ + private function getRowsChunk(array $rowIds, array $columns, array $mappers): array { + try { + $sleeves = $this->rowSleeveMapper->findMultiple($rowIds); + } catch (Exception $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), $e->getCode(), $e); + } + + return $this->parseResult($sleeves, $columns, $mappers); + } + + /** + * @param list> $sleeves + * @param array $columns Column per columnId + * @param array $mappers Mapper per columnId + * @return Row2[] + */ + private function parseResult(array $sleeves, array $columns, $mappers): array { + $rows = []; + foreach ($sleeves as $sleeve) { + $id = (int)$sleeve['id']; + $row = new Row2(); + $row->setId($id); + $row->setCreatedBy($sleeve['created_by']); + $row->setCreatedAt($sleeve['created_at']); + $row->setLastEditBy($sleeve['last_edit_by']); + $row->setLastEditAt($sleeve['last_edit_at']); + $row->setTableId((int)$sleeve['table_id']); + + $cachedCells = json_decode($sleeve['cached_cells'] ?? '{}', true); + foreach ($columns as $columnId => $column) { + if (empty($cachedCells[$columnId])) { + continue; + } + + $cellMapper = $mappers[$columnId]; + if ($cellMapper->hasMultipleValues()) { + $value = array_map(static fn ($rowData) => $cellMapper->formatRowData($column, $rowData), + $cachedCells[$columnId]); + } else { + $value = $cellMapper->formatRowData($column, $cachedCells[$columnId]); + } + + $row->addCell($columnId, $value); + } + + $rows[] = $row; + } + + return $rows; + } +} diff --git a/lib/Db/RowLoader/NormalizedRowLoader.php b/lib/Db/RowLoader/NormalizedRowLoader.php new file mode 100644 index 0000000000..5a5e541109 --- /dev/null +++ b/lib/Db/RowLoader/NormalizedRowLoader.php @@ -0,0 +1,153 @@ +columnsHelper->columns)); + $chunkSize = max(1, $maxParametersPerType - count($columns)); + + foreach (array_chunk($rowIds, $chunkSize) as $chunkedRowIds) { + yield from $this->getRowsChunk($chunkedRowIds, $columns, $mappers); + } + } + + /** + * Builds and executes the UNION ALL query for a specific chunk of rows. + * @param array $rowIds + * @param array $columns Column per columnId + * @param array $mappers Mapper per columnId + * @return Row2[] + * @throws InternalError + */ + private function getRowsChunk(array $rowIds, array $columns, array $mappers): array { + $qb = $this->db->getQueryBuilder(); + + $subqueries = []; + foreach ($this->columnsHelper->columns as $columnType) { + $qbValues = $this->db->getQueryBuilder(); + $qbValues->select('row_id', 'column_id', 'last_edit_at', 'last_edit_by') + ->selectAlias($qb->expr()->castColumn('value', IQueryBuilder::PARAM_STR), 'value'); + + // This is not ideal but I cannot think of a good way to abstract this away into the mapper right now + // Ideally we dynamically construct this query depending on what additional selects the column type requires + // however the union requires us to match the exact number of selects for each column type + if ($columnType === Column::TYPE_USERGROUP) { + $qbValues->selectAlias($qb->expr()->castColumn('value_type', IQueryBuilder::PARAM_STR), 'value_type'); + } else { + $qbValues->selectAlias($qbValues->createFunction('NULL'), 'value_type'); + } + + $qbValues + ->from('tables_row_cells_' . $columnType) + ->where($qb->expr()->in('column_id', $qb->createNamedParameter(array_keys($columns), IQueryBuilder::PARAM_INT_ARRAY, ':columnIds'))) + ->andWhere($qb->expr()->in('row_id', $qb->createNamedParameter($rowIds, IQueryBuilder::PARAM_INT_ARRAY, ':rowsIds'))); + + $subqueries[] = $qbValues->getSQL(); + } + + $qb->select('row_id', 'column_id', 'created_by', 'created_at', 't1.last_edit_by', 't1.last_edit_at', 'value', 'table_id') + // Also should be more generic (see above) + ->addSelect('value_type') + ->from($qb->createFunction('(' . implode(' UNION ALL ', $subqueries) . ')'), 't1') + ->innerJoin('t1', 'tables_row_sleeves', 'rs', 'rs.id = t1.row_id'); + + try { + $result = $qb->executeQuery(); + } catch (Exception $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), $e->getCode(), $e); + } + + try { + $sleeves = $this->rowSleeveMapper->findMultiple($rowIds); + } catch (Exception $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), $e->getCode(), $e); + } + + return $this->parseResult($result, $sleeves, $columns, $mappers); + } + + /** + * @param IResult $result + * @param list> $sleeves + * @param array $columns Column per columnId + * @param array $mappers Mapper per columnId + * @return Row2[] + * @throws InternalError + */ + private function parseResult(IResult $result, array $sleeves, array $columns, array $mappers): array { + $rows = []; + foreach ($sleeves as $sleeve) { + $id = (int)$sleeve['id']; + $rows[$id] = new Row2(); + $rows[$id]->setId($id); + $rows[$id]->setCreatedBy($sleeve['created_by']); + $rows[$id]->setCreatedAt($sleeve['created_at']); + $rows[$id]->setLastEditBy($sleeve['last_edit_by']); + $rows[$id]->setLastEditAt($sleeve['last_edit_at']); + $rows[$id]->setTableId((int)$sleeve['table_id']); + } + + $rowValues = []; + $keyToColumnId = []; + $keyToRowId = []; + + while ($rowData = $result->fetch()) { + if (!isset($rowData['row_id'], $rows[$rowData['row_id']])) { + break; + } + + $column = $columns[$rowData['column_id']]; + $cellMapper = $mappers[$rowData['column_id']]; + $value = $cellMapper->formatRowData($column, $rowData); + $compositeKey = (string)$rowData['row_id'] . ',' . (string)$rowData['column_id']; + if ($cellMapper->hasMultipleValues()) { + if (array_key_exists($compositeKey, $rowValues)) { + $rowValues[$compositeKey][] = $value; + } else { + $rowValues[$compositeKey] = [$value]; + } + } else { + $rowValues[$compositeKey] = $value; + } + $keyToColumnId[$compositeKey] = $rowData['column_id']; + $keyToRowId[$compositeKey] = $rowData['row_id']; + } + + foreach ($rowValues as $compositeKey => $value) { + $rows[$keyToRowId[$compositeKey]]->addCell($keyToColumnId[$compositeKey], $value); + } + + return array_values($rows); + } +} diff --git a/lib/Db/RowLoader/RowLoader.php b/lib/Db/RowLoader/RowLoader.php new file mode 100644 index 0000000000..223a2f1eb4 --- /dev/null +++ b/lib/Db/RowLoader/RowLoader.php @@ -0,0 +1,29 @@ + $columns Column per columnId + * @param array $mappers Mapper per columnId + * @return iterable + * @throws InternalError + */ + public function getRows(array $rowIds, array $columns, array $mappers): iterable; +} diff --git a/lib/Db/RowSleeve.php b/lib/Db/RowSleeve.php index b909a3f44f..07ca20de94 100644 --- a/lib/Db/RowSleeve.php +++ b/lib/Db/RowSleeve.php @@ -14,6 +14,8 @@ * @psalm-suppress PropertyNotSetInConstructor * @method getTableId(): ?int * @method setTableId(int $columnId) + * @method getCachedCells(): string + * @method setCachedCells(string $cachedCells) * @method getCreatedBy(): string * @method setCreatedBy(string $createdBy) * @method getCreatedAt(): string @@ -25,6 +27,7 @@ */ class RowSleeve extends Entity implements JsonSerializable { protected ?int $tableId = null; + protected ?string $cachedCells = null; protected ?string $createdBy = null; protected ?string $createdAt = null; protected ?string $lastEditBy = null; @@ -33,12 +36,25 @@ class RowSleeve extends Entity implements JsonSerializable { public function __construct() { $this->addType('id', 'integer'); $this->addType('tableId', 'integer'); + $this->addType('cachedCells', 'string'); + } + + /** + * @return array Indexed by column ID + */ + public function getCachedCellsArray(): array { + return json_decode($this->cachedCells, true) ?: []; + } + + public function setCachedCellsArray(array $array):void { + $this->setCachedCells(json_encode($array)); } public function jsonSerialize(): array { return [ 'id' => $this->id, 'tableId' => $this->tableId, + 'cachedCells' => $this->cachedCells, 'createdBy' => $this->createdBy, 'createdAt' => $this->createdAt, 'lastEditBy' => $this->lastEditBy, diff --git a/lib/Db/RowSleeveMapper.php b/lib/Db/RowSleeveMapper.php index f5a21fe5c1..c70571a49f 100644 --- a/lib/Db/RowSleeveMapper.php +++ b/lib/Db/RowSleeveMapper.php @@ -40,7 +40,7 @@ public function find(int $id): RowSleeve { /** * @param int[] $ids - * @return RowSleeve[] + * @return list> Raw data from DB for the best performance * @throws Exception */ public function findMultiple(array $ids): array { @@ -49,6 +49,7 @@ public function findMultiple(array $ids): array { $qb->select( $sleeveAlias . '.id', $sleeveAlias . '.table_id', + $sleeveAlias . '.cached_cells', $sleeveAlias . '.created_by', $sleeveAlias . '.created_at', $sleeveAlias . '.last_edit_by', @@ -56,7 +57,8 @@ public function findMultiple(array $ids): array { ) ->from($this->table, $sleeveAlias) ->where($qb->expr()->in($sleeveAlias . '.id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))); - return $this->findEntities($qb); + + return $qb->executeQuery()->fetchAll(); } /** diff --git a/lib/Helper/ColumnsHelper.php b/lib/Helper/ColumnsHelper.php index f926d2ba3c..3a4b92d5af 100644 --- a/lib/Helper/ColumnsHelper.php +++ b/lib/Helper/ColumnsHelper.php @@ -9,11 +9,15 @@ use OCA\Tables\Constants\UsergroupType; use OCA\Tables\Db\Column; +use OCA\Tables\Db\RowCellMapperSuper; +use OCA\Tables\Errors\InternalError; use OCA\Tables\Service\ColumnTypes\IColumnTypeBusiness; use OCP\Server; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\NotFoundExceptionInterface; +use Psr\Log\LoggerInterface; class ColumnsHelper { - public array $columns = [ Column::TYPE_TEXT, Column::TYPE_NUMBER, @@ -28,9 +32,15 @@ class ColumnsHelper { */ private array $columnBusinesses = []; + /** + * @var array + */ + private array $cellMappers = []; + public function __construct( - private UserHelper $userHelper, - private CircleHelper $circleHelper, + private readonly UserHelper $userHelper, + private readonly CircleHelper $circleHelper, + private readonly LoggerInterface $logger, ) { } @@ -104,4 +114,21 @@ public function getColumnBusinessObject(Column $column): IColumnTypeBusiness { return $this->columnBusinesses[$cacheKey] = $columnBusiness; } + + public function getCellMapperFromType(string $columnType): RowCellMapperSuper { + if (isset($this->cellMappers[$columnType])) { + return $this->cellMappers[$columnType]; + } + + $cellMapperClassName = 'OCA\Tables\Db\RowCell' . ucfirst($columnType) . 'Mapper'; + /** @var RowCellMapperSuper $cellMapper */ + try { + $cellMapper = Server::get($cellMapperClassName); + } catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) { + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), previous: $e); + } + + return $this->cellMappers[$columnType] = $cellMapper; + } } diff --git a/lib/Migration/CacheSleeveCells.php b/lib/Migration/CacheSleeveCells.php new file mode 100644 index 0000000000..a9c18e1832 --- /dev/null +++ b/lib/Migration/CacheSleeveCells.php @@ -0,0 +1,104 @@ +config->getAppValue('tables', 'cachingSleeveCellsComplete', 'false') === 'true'; + if ($cachingSleeveCellsComplete) { + return; + } + + foreach ($this->getTableIds() as $tableId) { + $columns = $this->columnMapper->findAllByTable($tableId); + + while ($rowIds = $this->getPendingRowIds($tableId)) { + foreach ($rowIds as $rowId) { + $cachedCells = []; + foreach ($columns as $column) { + $cellMapper = $this->columnsHelper->getCellMapperFromType($column->getType()); + $cells = $cellMapper->findManyByRowAndColumn($rowId, $column->getId()); + foreach ($cells as $cell) { + if ($cellMapper->hasMultipleValues()) { + $cachedCells[$column->getId()][] = $cellMapper->toArray($cell); + } else { + $cachedCells[$column->getId()] = $cellMapper->toArray($cell); + } + } + } + + $sleeve = $this->rowSleeveMapper->find($rowId); + $sleeve->setCachedCellsArray($cachedCells); + $this->rowSleeveMapper->update($sleeve); + } + } + + $this->logger->info('Finished caching cells for table ' . $tableId); + } + + $this->config->setAppValue('tables', 'cachingSleeveCellsComplete', 'true'); + } + + /** + * @return int[] + */ + public function getTableIds(): array { + return $this->db->getQueryBuilder() + ->select('id') + ->from('tables_tables') + ->orderBy('id') + ->executeQuery() + ->fetchAll(\PDO::FETCH_COLUMN); + + } + + /** + * @return int[] + */ + private function getPendingRowIds(int $tableId): array { + $qb = $this->db->getQueryBuilder(); + + return $qb->select('id') + ->from('tables_row_sleeves') + ->where($qb->expr()->isNull('cached_cells')) + ->andWhere($qb->expr()->eq('table_id', $qb->createNamedParameter($tableId, \PDO::PARAM_INT))) + ->orderBy('id') + ->setMaxResults(1000) + ->executeQuery() + ->fetchAll(\PDO::FETCH_COLUMN); + } +} diff --git a/lib/Migration/Version002030Date20260620000000.php b/lib/Migration/Version002030Date20260620000000.php new file mode 100644 index 0000000000..01dcecb25b --- /dev/null +++ b/lib/Migration/Version002030Date20260620000000.php @@ -0,0 +1,41 @@ +getTable('tables_row_sleeves'); + if (!$table->hasColumn('cached_cells')) { + $table->addColumn('cached_cells', Types::JSON, [ + 'notnull' => false, + ]); + } + + return $schema; + } +} diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 2c37ca335a..afd9d54671 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -901,6 +901,7 @@ public function importRow(Table $table, array $row): int { $qb->insert('tables_row_sleeves') ->values([ 'table_id' => $qb->createNamedParameter($table->getId(), IQueryBuilder::PARAM_INT), + 'cached_cells' => $qb->createNamedParameter(json_encode([])), 'created_by' => $qb->createNamedParameter($table->getOwnership()), 'created_at' => $qb->createNamedParameter($row['createdAt']), 'last_edit_by' => $qb->createNamedParameter($table->getOwnership()), diff --git a/lib/UserMigration/TablesMigrator.php b/lib/UserMigration/TablesMigrator.php index 7a039ad38a..40373040ee 100644 --- a/lib/UserMigration/TablesMigrator.php +++ b/lib/UserMigration/TablesMigrator.php @@ -14,6 +14,7 @@ use OCA\Tables\Db\ContextNodeRelationMapper; use OCA\Tables\Db\RowCellDatetime; use OCA\Tables\Db\RowCellDatetimeMapper; +use OCA\Tables\Db\RowCellMapperSuper; use OCA\Tables\Db\RowCellNumber; use OCA\Tables\Db\RowCellNumberMapper; use OCA\Tables\Db\RowCellSelection; @@ -41,6 +42,7 @@ use OCP\UserMigration\IMigrator; use OCP\UserMigration\ISizeEstimationMigrator; use OCP\UserMigration\TMigratorBasicVersionHandling; +use Psr\Log\LoggerInterface; use Symfony\Component\Console\Output\OutputInterface; class TablesMigrator implements IMigrator, ISizeEstimationMigrator { @@ -83,6 +85,7 @@ public function __construct( protected RowService $rowService, private ContextService $contextService, private ShareService $shareService, + protected LoggerInterface $logger, ) { } @@ -279,6 +282,8 @@ public function import( $userId, ); + $this->rebuildCachedCells($rowIdMap); + $connection->commit(); } catch (\Throwable $e) { $connection->rollBack(); @@ -335,6 +340,39 @@ private function importRowCells(array $data, array $rowIdMap, array $columnIdMap } } + private function rebuildCachedCells(array $rowIdMap): void { + foreach ($rowIdMap as $newRowId) { + $sleeve = $this->rowSleeveMapper->find($newRowId); + $cachedCells = []; + + /** @var RowCellMapperSuper[] $cellMappers */ + $cellMappers = [ + $this->rowCellTextMapper, + $this->rowCellNumberMapper, + $this->rowCellDatetimeMapper, + $this->rowCellSelectionMapper, + $this->rowCellUsergroupMapper, + ]; + + foreach ($cellMappers as $mapper) { + try { + $cells = $mapper->findAllForRow($newRowId); + foreach ($cells as $cell) { + $cachedCells[$cell->getColumnId()] = $mapper->toArray($cell); + } + } catch (\Exception $e) { + $this->logger->error('Failed to load cells for row during cached_cells rebuild', [ + 'rowId' => $newRowId, + 'exception' => $e->getMessage(), + ]); + } + } + + $sleeve->setCachedCellsArray($cachedCells); + $this->rowSleeveMapper->update($sleeve); + } + } + /** * @param IImportSource $importSource * @param array $tableIdMap diff --git a/tests/unit/Database/DatabaseTestCase.php b/tests/unit/Database/DatabaseTestCase.php index a4412307a3..10507bf75d 100644 --- a/tests/unit/Database/DatabaseTestCase.php +++ b/tests/unit/Database/DatabaseTestCase.php @@ -292,6 +292,7 @@ protected function createTestRowWithData(int $tableId, array $rowData = [], arra if (!empty($cellsData)) { $this->addCellsToRow($rowId, $cellsData, $columnMapping); + $this->updateCachedCells($rowId, $cellsData, $columnMapping); } return $result; @@ -350,6 +351,31 @@ protected function insertCellIntoTypeTable(int $rowId, int $columnId, $value, st $qb->executeStatement(); } + /** + * Updates the cached_cells column for a row + */ + protected function updateCachedCells(int $rowId, array $cellsData, array $columnMapping = []): void { + $cachedCells = []; + foreach ($cellsData as $columnIdentifier => $value) { + // Convert test_ident to actual column ID if mapping is provided + if (is_string($columnIdentifier) && isset($columnMapping[$columnIdentifier])) { + $columnId = $columnMapping[$columnIdentifier]; + } else { + $columnId = $columnIdentifier; + } + + // Format the value as expected by CachedRowLoader (matches cell mapper toArray format) + $cachedCells[$columnId] = ['value' => $value]; + } + + $qb = $this->connection->getQueryBuilder(); + $qb->update('tables_row_sleeves') + ->set('cached_cells', $qb->createNamedParameter(json_encode($cachedCells))) + ->where($qb->expr()->eq('id', $qb->createNamedParameter($rowId))); + + $qb->executeStatement(); + } + /** * Creates a complete test table with columns and rows with data */ diff --git a/tests/unit/Db/Row2MapperTestDependencies.php b/tests/unit/Db/Row2MapperTestDependencies.php index 297eaf1674..3e2bf0cf79 100644 --- a/tests/unit/Db/Row2MapperTestDependencies.php +++ b/tests/unit/Db/Row2MapperTestDependencies.php @@ -12,11 +12,21 @@ use OCA\Tables\Db\Column; use OCA\Tables\Db\ColumnMapper; use OCA\Tables\Db\Row2Mapper; +use OCA\Tables\Db\RowCellDatetimeMapper; +use OCA\Tables\Db\RowCellNumberMapper; +use OCA\Tables\Db\RowCellSelectionMapper; +use OCA\Tables\Db\RowCellTextMapper; +use OCA\Tables\Db\RowCellUsergroupMapper; +use OCA\Tables\Db\RowLoader\CachedRowLoader; +use OCA\Tables\Db\RowLoader\NormalizedRowLoader; use OCA\Tables\Db\RowSleeveMapper; use OCA\Tables\Helper\CircleHelper; use OCA\Tables\Helper\ColumnsHelper; +use OCA\Tables\Helper\GroupHelper; use OCA\Tables\Helper\UserHelper; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IUserManager; +use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -35,6 +45,8 @@ trait Row2MapperTestDependencies { protected ColumnsHelper|MockObject $columnsHelper; protected LoggerInterface|MockObject $logger; protected CircleHelper|MockObject $circleHelper; + protected NormalizedRowLoader $normalizedRowLoader; + protected CachedRowLoader $cachedRowLoader; protected static bool $testDataInitialized = false; protected static int $testTableId; @@ -56,7 +68,22 @@ protected function setupDependencies(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->rowSleeveMapper = new RowSleeveMapper($this->connectionAdapter, $this->logger); - $this->columnsHelper = new ColumnsHelper($this->userHelper, $this->circleHelper); + $this->columnsHelper = $this->createMock(ColumnsHelper::class); + + // Mock getCellMapperFromType to return real cell mappers + $this->setupCellMappers(); + + // Use real loader instances since we have a real database + $this->normalizedRowLoader = new NormalizedRowLoader( + $this->connectionAdapter, + $this->rowSleeveMapper, + $this->columnsHelper, + $this->logger + ); + $this->cachedRowLoader = new CachedRowLoader( + $this->rowSleeveMapper, + $this->logger + ); $this->mapper = new Row2Mapper( 'test_user', @@ -65,7 +92,9 @@ protected function setupDependencies(): void { $this->userHelper, $this->rowSleeveMapper, $this->columnsHelper, - $this->columnMapper + $this->columnMapper, + $this->normalizedRowLoader, + $this->cachedRowLoader ); if (!self::$testDataInitialized) { @@ -74,6 +103,27 @@ protected function setupDependencies(): void { } } + /** + * Sets up real cell mappers by mocking getCellMapperFromType + * This is needed because ColumnsHelper uses Server::get() which doesn't work in unit tests + */ + private function setupCellMappers(): void { + $userManager = $this->createMock(IUserManager::class); + $userSession = $this->createMock(IUserSession::class); + + $this->columnsHelper->method('getCellMapperFromType') + ->willReturnCallback(function ($columnType) use ($userManager, $userSession) { + return match ($columnType) { + Column::TYPE_TEXT => new RowCellTextMapper($this->connectionAdapter), + Column::TYPE_NUMBER => new RowCellNumberMapper($this->connectionAdapter), + Column::TYPE_DATETIME => new RowCellDatetimeMapper($this->connectionAdapter), + Column::TYPE_SELECTION => new RowCellSelectionMapper($this->connectionAdapter), + Column::TYPE_USERGROUP => new RowCellUsergroupMapper($this->connectionAdapter, $userManager, $this->circleHelper, $this->createMock(GroupHelper::class), $userSession), + default => throw new \InvalidArgumentException("Unknown column type: $columnType"), + }; + }); + } + /** * Initializes comprehensive test data for sorting and querying tests * diff --git a/tests/unit/TablesMigratorTest.php b/tests/unit/TablesMigratorTest.php index 856d91adc3..fde2f9b8a6 100644 --- a/tests/unit/TablesMigratorTest.php +++ b/tests/unit/TablesMigratorTest.php @@ -36,6 +36,7 @@ use OCP\UserMigration\IExportDestination; use OCP\UserMigration\IImportSource; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; use Symfony\Component\Console\Output\NullOutput; class TablesMigratorTest extends TestCase { @@ -60,6 +61,7 @@ class TablesMigratorTest extends TestCase { private $rowService; private $contextService; private $shareService; + private $logger; protected function setUp(): void { $this->l10n = $this->createMock(IL10N::class); @@ -82,6 +84,7 @@ protected function setUp(): void { $this->rowService = $this->createMock(RowService::class); $this->contextService = $this->createMock(ContextService::class); $this->shareService = $this->createMock(ShareService::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->migrator = new TablesMigrator( $this->l10n, @@ -103,7 +106,8 @@ protected function setUp(): void { $this->columnService, $this->rowService, $this->contextService, - $this->shareService + $this->shareService, + $this->logger ); }