Skip to content

Commit 22af467

Browse files
blizzzblizzz
authored
Merge pull request #2186 from nextcloud/fix/noid/row2mapper-cleanup
refactor: remove unnecessary parameter (plus cleanup)
2 parents 3eb13ea + 014f5d6 commit 22af467

1 file changed

Lines changed: 28 additions & 38 deletions

File tree

‎lib/Db/Row2Mapper.php‎

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\Tables\Db;
99

1010
use DateTime;
11+
use DateTimeImmutable;
1112
use OCA\Tables\Constants\UsergroupType;
1213
use OCA\Tables\Errors\InternalError;
1314
use OCA\Tables\Errors\NotFoundError;
@@ -30,7 +31,7 @@ class Row2Mapper {
3031
use TTransactional;
3132

3233
private RowSleeveMapper $rowSleeveMapper;
33-
private ?string $userId = null;
34+
private ?string $userId;
3435
private IDBConnection $db;
3536
private LoggerInterface $logger;
3637
protected UserHelper $userHelper;
@@ -80,17 +81,20 @@ public function delete(Row2 $row): Row2 {
8081
public function find(int $id, array $columns): Row2 {
8182
$columnIdsArray = array_map(fn (Column $column) => $column->getId(), $columns);
8283
$rows = $this->getRows([$id], $columnIdsArray);
84+
8385
if (count($rows) === 1) {
8486
return $rows[0];
85-
} elseif (count($rows) === 0) {
87+
}
88+
89+
if (count($rows) === 0) {
8690
$e = new Exception('Wanted row not found.');
8791
$this->logger->error($e->getMessage(), ['exception' => $e]);
8892
throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
89-
} else {
90-
$e = new Exception('Too many results for one wanted row.');
91-
$this->logger->error($e->getMessage(), ['exception' => $e]);
92-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
9393
}
94+
95+
$e = new Exception('Too many results for one wanted row.');
96+
$this->logger->error($e->getMessage(), ['exception' => $e]);
97+
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
9498
}
9599

96100
/**
@@ -102,7 +106,7 @@ public function findNextId(int $offsetId = -1): ?int {
102106
} catch (MultipleObjectsReturnedException|Exception $e) {
103107
$this->logger->error($e->getMessage(), ['exception' => $e]);
104108
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
105-
} catch (DoesNotExistException $e) {
109+
} catch (DoesNotExistException) {
106110
return null;
107111
}
108112
return $rowSleeve->getId();
@@ -119,12 +123,6 @@ public function getTableIdForRow(int $rowId): ?int {
119123
}
120124

121125
/**
122-
* @param string $userId
123-
* @param int $tableId
124-
* @param array|null $filter
125-
* @param array $sort
126-
* @param int|null $limit
127-
* @param int|null $offset
128126
* @return int[]
129127
* @throws InternalError
130128
*/
@@ -245,20 +243,13 @@ private function getRows(array $rowIds, array $columnIds): array {
245243
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
246244
}
247245

248-
try {
249-
$columnTypes = $this->columnMapper->getColumnTypes($columnIds);
250-
} catch (Exception $e) {
251-
$this->logger->error($e->getMessage(), ['exception' => $e]);
252-
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
253-
}
254-
255-
return $this->parseEntities($result, $sleeves, $columnTypes);
246+
return $this->parseEntities($result, $sleeves);
256247
}
257248

258249
/**
259250
* @throws InternalError
260251
*/
261-
private function addFilterToQuery(IQueryBuilder &$qb, array $filters, string $userId): void {
252+
private function addFilterToQuery(IQueryBuilder $qb, array $filters, string $userId): void {
262253
// TODO move this into service
263254
$this->replacePlaceholderValues($filters, $userId);
264255

@@ -284,9 +275,12 @@ private function addSortQueryForMultipleSleeveFinder(IQueryBuilder $qb, string $
284275

285276
$i = 1;
286277
foreach ($sort as $sortData) {
278+
if (!in_array($sortData['mode'], ['ASC', 'DESC'])) {
279+
continue;
280+
}
287281
try {
288282
$column = $sortData['columnId'] > 0 ? $this->columnMapper->find($sortData['columnId']) : null;
289-
} catch (DoesNotExistException $e) {
283+
} catch (DoesNotExistException) {
290284
$this->logger->debug('No column found to build filter with for id ' . $sortData['columnId']);
291285
continue;
292286
}
@@ -301,7 +295,7 @@ private function addSortQueryForMultipleSleeveFinder(IQueryBuilder $qb, string $
301295
$qb->expr()->eq($alias . '.column_id', $qb->createNamedParameter($sortData['columnId']))
302296
)
303297
);
304-
$qb->addOrderBy($qb->createFunction("MAX($alias.value)"), $sortData['mode']);
298+
$qb->addOrderBy($qb->func()->max($alias . '.value'), $sortData['mode']);
305299
} elseif (Column::isValidMetaTypeId($sortData['columnId'])) {
306300
$fieldName = match ($sortData['columnId']) {
307301
Column::TYPE_META_ID => 'id',
@@ -323,7 +317,7 @@ private function addSortQueryForMultipleSleeveFinder(IQueryBuilder $qb, string $
323317
continue;
324318
}
325319

326-
$qb->addOrderBy($qb->createFunction("MAX($sleevesAlias.$fieldName)"), $sortData['mode']);
320+
$qb->addOrderBy($qb->func()->max($sleevesAlias . '.' . $fieldName), $sortData['mode']);
327321
}
328322
$i++;
329323
}
@@ -332,7 +326,7 @@ private function addSortQueryForMultipleSleeveFinder(IQueryBuilder $qb, string $
332326
private function replacePlaceholderValues(array &$filters, string $userId): void {
333327
foreach ($filters as &$filterGroup) {
334328
foreach ($filterGroup as &$filter) {
335-
if (substr($filter['value'], 0, 1) === '@') {
329+
if (str_starts_with($filter['value'], '@')) {
336330
$columnId = (int)($filter['columnId'] ?? 0);
337331
$column = $columnId > 0 ? $this->columnMapper->find($columnId) : null;
338332
$filter['value'] = $this->columnsHelper->resolveSearchValue($filter['value'], $userId, $column);
@@ -344,7 +338,7 @@ private function replacePlaceholderValues(array &$filters, string $userId): void
344338
/**
345339
* @throws InternalError
346340
*/
347-
private function getFilterGroups(IQueryBuilder &$qb, array $filters): array {
341+
private function getFilterGroups(IQueryBuilder $qb, array $filters): array {
348342
$filterGroups = [];
349343
foreach ($filters as $filterGroup) {
350344
$filterGroups[] = $qb->expr()->andX(...$this->getFilter($qb, $filterGroup));
@@ -355,7 +349,7 @@ private function getFilterGroups(IQueryBuilder &$qb, array $filters): array {
355349
/**
356350
* @throws InternalError
357351
*/
358-
private function getFilter(IQueryBuilder &$qb, array $filterGroup): array {
352+
private function getFilter(IQueryBuilder $qb, array $filterGroup): array {
359353
$filterExpressions = [];
360354
foreach ($filterGroup as $filter) {
361355
$columnId = $filter['columnId'];
@@ -404,7 +398,6 @@ private function getFilterExpression(IQueryBuilder $qb, Column $column, string $
404398

405399
// We try to match the requested value against the default before building the query
406400
// so we know if we shall include rows that have no entry in the column_TYPE tables upfront
407-
$includeDefault = false;
408401
$defaultValue = $this->getFormattedDefaultValue($column);
409402

410403
$qb2 = $this->db->getQueryBuilder();
@@ -578,15 +571,13 @@ private function getMetaFilterExpression(IQueryBuilder $qb, int $columnId, strin
578571
$qb2->where($this->getSqlOperator($operator, $qb, 'created_by', $value, IQueryBuilder::PARAM_STR));
579572
break;
580573
case Column::TYPE_META_CREATED_AT:
581-
$value = new \DateTimeImmutable($value);
582-
$qb2->where($this->getSqlOperator($operator, $qb, 'created_at', $value, IQueryBuilder::PARAM_DATE));
574+
$qb2->where($this->getSqlOperator($operator, $qb, 'created_at', new DateTimeImmutable($value), IQueryBuilder::PARAM_DATE));
583575
break;
584576
case Column::TYPE_META_UPDATED_BY:
585577
$qb2->where($this->getSqlOperator($operator, $qb, 'last_edit_by', $value, IQueryBuilder::PARAM_STR));
586578
break;
587579
case Column::TYPE_META_UPDATED_AT:
588-
$value = new \DateTimeImmutable($value);
589-
$qb2->where($this->getSqlOperator($operator, $qb, 'last_edit_at', $value, IQueryBuilder::PARAM_DATE));
580+
$qb2->where($this->getSqlOperator($operator, $qb, 'last_edit_at', new DateTimeImmutable($value), IQueryBuilder::PARAM_DATE));
590581
break;
591582
}
592583
return $qb2;
@@ -634,7 +625,7 @@ private function getSqlOperator(string $operator, IQueryBuilder $qb, string $col
634625
* @return Row2[]
635626
* @throws InternalError
636627
*/
637-
private function parseEntities(IResult $result, array $sleeves, array $columnTypes): array {
628+
private function parseEntities(IResult $result, array $sleeves): array {
638629
$rows = [];
639630
foreach ($sleeves as $sleeve) {
640631
$rows[$sleeve->getId()] = new Row2();
@@ -652,7 +643,7 @@ private function parseEntities(IResult $result, array $sleeves, array $columnTyp
652643
$cellMapperCache = [];
653644

654645
while ($rowData = $result->fetch()) {
655-
if (!isset($rowData['row_id']) || !isset($rows[$rowData['row_id']])) {
646+
if (!isset($rowData['row_id'], $rows[$rowData['row_id']])) {
656647
break;
657648
}
658649

@@ -799,8 +790,7 @@ private function insertCell(int $rowId, int $columnId, $value, ?string $lastEdit
799790
try {
800791
$column = $this->columnMapper->find($columnId);
801792
} catch (DoesNotExistException $e) {
802-
$e = new Exception('Can not insert cell, because the given column-id is not known');
803-
$this->logger->error($e->getMessage(), ['exception' => $e]);
793+
$this->logger->error('Can not insert cell, because the given column-id is not known', ['exception' => $e]);
804794
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
805795
}
806796

@@ -891,7 +881,7 @@ private function getCellMapperFromType(string $columnType): RowCellMapperSuper {
891881
/**
892882
* @throws InternalError
893883
*/
894-
private function getColumnDbParamType(Column $column) {
884+
private function getColumnDbParamType(Column $column): int {
895885
return $this->getCellMapper($column)->getDbParamType();
896886
}
897887

0 commit comments

Comments
 (0)