Reordering validation

lib/Controller/ApiTablesController.php

@@ -14,6 +14,8 @@
 use OCA\Tables\Errors\NotFoundError;
 use OCA\Tables\Errors\PermissionError;
 use OCA\Tables\Middleware\Attribute\RequirePermission;
+use OCA\Tables\Model\ColumnSettings;
+use OCA\Tables\Model\SortRuleSet;
 use OCA\Tables\Model\ViewUpdateInput;
 use OCA\Tables\ResponseDefinitions;
 use OCA\Tables\Service\ColumnService;
@@ -132,12 +134,19 @@ public function showScheme(int $id): DataResponse {
 	 * @param list<TablesView> $views views
 	 * @param list<array{columnId: int, order: int, readonly: bool}> $columnOrder Default column order settings
 	 * @param list<array{columnId: int, mode: 'ASC'|'DESC'}> $sort Default sort rules
-	 * @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
 	 *
 	 * 200: Tables returned
+	 * 400: Invalid request data
 	 */
 	#[NoAdminRequired]
 	public function createFromScheme(string $title, string $emoji, string $description, array $columns, array $views, array $columnOrder = [], array $sort = []): DataResponse {
+		try {
+			ColumnSettings::createFromInputArray($columnOrder);
+			SortRuleSet::createFromInputArray($sort);
+		} catch (\InvalidArgumentException $e) {
+			return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
+		}
 		try {
 			$this->db->beginTransaction();
 			$table = $this->service->create($title, 'custom', $emoji, $description);
@@ -178,18 +187,24 @@ public function createFromScheme(string $title, string $emoji, string $descripti
 				$colMap[$column['id']] = $col->getId();
 			}
 			if (!empty($columnOrder) || !empty($sort)) {
-				$remappedColumnOrder = !empty($columnOrder) ? array_map(static function (array $entry) use ($colMap): array {
-					if (isset($entry['columnId']) && $entry['columnId'] > 0) {
-						$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
-					}
-					return $entry;
-				}, $columnOrder) : null;
-				$remappedSort = !empty($sort) ? array_map(static function (array $entry) use ($colMap): array {
-					if (isset($entry['columnId']) && $entry['columnId'] > 0) {
-						$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
-					}
-					return $entry;
-				}, $sort) : null;
+				$remappedColumnOrder = null;
+				if (!empty($columnOrder)) {
+					$remappedColumnOrder = ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array {
+						if ($entry['columnId'] > 0) {
+							$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
+						}
+						return $entry;
+					}, $columnOrder));
+				}
+				$remappedSort = null;
+				if (!empty($sort)) {
+					$remappedSort = SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array {
+						if ($entry['columnId'] > 0) {
+							$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
+						}
+						return $entry;
+					}, $sort));
+				}
 				$table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort);
 			}
 			foreach ($views as $view) {
@@ -240,6 +255,14 @@ public function createFromScheme(string $title, string $emoji, string $descripti
 			}
 			$this->db->commit();
 			return new DataResponse($table->jsonSerialize());
+		} catch (\InvalidArgumentException $e) {
+			try {
+				$this->db->rollBack();
+			} catch (\OCP\DB\Exception $re) {
+				return $this->handleError($re);
+			}
+			$this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]);
+			return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
 		} catch (InternalError|Exception $e) {
 			try {
 				$this->db->rollBack();
@@ -281,9 +304,10 @@ public function create(string $title, ?string $emoji, ?string $description, stri
 	 * @param string $description the tables description
 	 * @param list<array{columnId: int, order: int, readonly: bool}>|string|null $columnSettings Default column order settings (array or JSON string)
 	 * @param list<array{columnId: int, mode: 'ASC'|'DESC'}>|string|null $sort Default sort rules (array or JSON string)
-	 * @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
 	 *
 	 * 200: Tables returned
+	 * 400: Invalid request data
 	 * 403: No permissions
 	 * 404: Not found
 	 */
@@ -297,9 +321,24 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?s
 			$sort = json_decode($sort, true) ?? null;
 		}
 		try {
-			return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize());
+			if ($columnSettings !== null && !is_array($columnSettings)) {
+				throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
+			}
+			if ($sort !== null && !is_array($sort)) {
+				throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
+			}
+			$columnSettingsObj = $columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null;
+			$sortObj = $sort !== null ? SortRuleSet::createFromInputArray($sort) : null;
+		} catch (\InvalidArgumentException $e) {
+			return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
+		}
+		try {
+			return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettingsObj, $sortObj)->jsonSerialize());
 		} catch (PermissionError $e) {
 			return $this->handlePermissionError($e);
+		} catch (\InvalidArgumentException $e) {
+			$this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]);
+			return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
 		} catch (InternalError $e) {
 			return $this->handleError($e);
 		} catch (NotFoundError $e) {

lib/Controller/TableController.php

@@ -9,6 +9,8 @@
 
 use OCA\Tables\AppInfo\Application;
 use OCA\Tables\Middleware\Attribute\RequirePermission;
+use OCA\Tables\Model\ColumnSettings;
+use OCA\Tables\Model\SortRuleSet;
 use OCA\Tables\Service\TableService;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -78,7 +80,17 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?b
 			$sort = json_decode($sort, true) ?? null;
 		}
 		return $this->handleError(function () use ($id, $title, $emoji, $archived, $columnSettings, $sort) {
-			return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort);
+			if ($columnSettings !== null && !is_array($columnSettings)) {
+				throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
+			}
+			if ($sort !== null && !is_array($sort)) {
+				throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
+			}
+			return $this->service->update(
+				$id, $title, $emoji, null, $archived, $this->userId,
+				$columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null,
+				$sort !== null ? SortRuleSet::createFromInputArray($sort) : null,
+			);
 		});
 	}
 }

lib/Db/Table.php

@@ -11,7 +11,7 @@
 use OCA\Tables\Model\Permissions;
 use OCA\Tables\Model\SortRuleSet;
 use OCA\Tables\ResponseDefinitions;
-use OCA\Tables\Service\ValueObject\ViewColumnInformation;
+use OCA\Tables\Service\ValueObject\ColumnOrderInformation;
 
 /**
  * @psalm-suppress PropertyNotSetInConstructor
@@ -44,7 +44,7 @@
  * @method setRowsCount(int $rowsCount)
  * @method getColumnsCount(): int
  * @method setColumnsCount(int $columnsCount)
- * @method getViews(): ?array
+ * @method getViews(): ?array<TablesView>
  * @method setViews(array $views)
  * @method getColumns(): array
  * @method setColumns(array $columns)
@@ -109,14 +109,17 @@ public function jsonSerialize(): array {
 			'lastEditAt' => $this->lastEditAt ?: '',
 			'archived' => $this->archived,
 			'isShared' => (bool)$this->isShared,
-			'favorite' => $this->favorite,
+			'favorite' => (bool)$this->favorite,
 			'onSharePermissions' => $this->getSharePermissions()?->jsonSerialize(),
 			'hasShares' => (bool)$this->hasShares,
 			'rowsCount' => $this->rowsCount ?: 0,
 			'columnsCount' => $this->columnsCount ?: 0,
-			'views' => $this->getViewsArray(),
-			'description' => $this->description ?:'',
-			'columnOrder' => $this->getColumnOrderSettingsArray(),
+			'description' => $this->description ?: '',
+			'views' => array_values($this->getViewsArray()),
+			'columnOrder' => array_map(
+				static fn (ColumnOrderInformation $c) => ['columnId' => $c->getId(), 'order' => $c->getOrder()],
+				$this->getColumnOrderSettingsArray()
+			),
 			'sort' => $this->getSortArray(),
 		];
 	}
@@ -126,47 +129,45 @@ private function getSharePermissions(): ?Permissions {
 	}
 
 	/**
-	 * @psalm-suppress MismatchingDocblockReturnType
 	 * @return TablesView[]
 	 */
 	private function getViewsArray(): array {
 		return $this->getViews() ?: [];
 	}
 
 	/**
-	 * @psalm-suppress MismatchingDocblockReturnType
 	 * @return int[]
 	 */
 	public function getColumnOrderArray(): array {
 		$columnSettings = $this->getColumnOrderSettingsArray();
-		usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) {
+		usort($columnSettings, static function (ColumnOrderInformation $a, ColumnOrderInformation $b) {
 			return $a->getOrder() - $b->getOrder();
 		});
-		return array_map(static fn (ViewColumnInformation $vci): int => $vci->getId(), $columnSettings);
+		/** @var list<ColumnOrderInformation> $columnSettings */
+		return array_map(static fn (ColumnOrderInformation $vci): int => $vci->getId(), $columnSettings);
 	}
 
 	/**
-	 * @return array<ViewColumnInformation>
+	 * @return list<ColumnOrderInformation>
 	 */
 	public function getColumnOrderSettingsArray(): array {
 		$columns = $this->getArray($this->getColumnOrder());
 		if (empty($columns)) {
 			return [];
 		}
 
-		if (is_array(reset($columns))) {
-			return array_values(array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns));
+		if (is_array($columns[array_key_first($columns)] ?? null)) {
+			return array_values(array_map(static fn (array $a): ColumnOrderInformation => ColumnOrderInformation::fromArray($a), $columns));
 		}
 
 		$result = [];
 		foreach ($columns as $index => $columnId) {
-			$result[] = new ViewColumnInformation($columnId, order: (int)$index + 1);
+			$result[] = new ColumnOrderInformation((int)$columnId, order: (int)$index + 1);
 		}
 		return $result;
 	}
 
 	/**
-	 * @psalm-suppress MismatchingDocblockReturnType
 	 * @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
 	 */
 	public function getSortArray(): array {
@@ -176,9 +177,8 @@ public function getSortArray(): array {
 
 	private function getArray(?string $json): array {
 		if ($json !== '' && $json !== null && $json !== 'null') {
-			return \json_decode($json, true);
-		} else {
-			return [];
+			return \json_decode($json, true) ?? [];
 		}
+		return [];
 	}
 }

lib/Model/ColumnSettings.php

@@ -11,18 +11,19 @@
 
 use Generator;
 use JsonSerializable;
+use OCA\Tables\Service\ValueObject\ColumnOrderInformation;
 use OCA\Tables\Service\ValueObject\ViewColumnInformation;
 
-class ColumnSettings implements JSONSerializable {
+class ColumnSettings implements JsonSerializable {
 	/**
-	 * @param ViewColumnInformation[] $columnSettings
+	 * @param ColumnOrderInformation[] $columnSettings
 	 */
 	public function __construct(
 		protected array $columnSettings,
 	) {
 		foreach ($this->columnSettings as $columnSetting) {
-			if (!$columnSetting instanceof ViewColumnInformation) {
-				throw new \InvalidArgumentException('Provided column settings must be of type ViewColumnInformation');
+			if (!$columnSetting instanceof ColumnOrderInformation) {
+				throw new \InvalidArgumentException('Provided column settings must be of type ColumnOrderInformation');
 			}
 		}
 	}
@@ -33,15 +34,35 @@ public function columnInformation(): Generator {
 		}
 	}
 
+	/**
+	 * Creates column settings with only columnId and order (for table use).
+	 */
 	public static function createFromInputArray(array $inputColumnSettings): self {
 		$columnSettings = [];
 		foreach ($inputColumnSettings as $inputColumnSetting) {
+			if (!is_array($inputColumnSetting)) {
+				throw new \InvalidArgumentException('Each column settings entry must be an array');
+			}
+			$columnSettings[] = ColumnOrderInformation::fromArray($inputColumnSetting);
+		}
+		return new self($columnSettings);
+	}
+
+	/**
+	 * Creates column settings with view-specific fields (columnId, order, readonly, mandatory).
+	 */
+	public static function createViewSettingsFromInputArray(array $inputColumnSettings): self {
+		$columnSettings = [];
+		foreach ($inputColumnSettings as $inputColumnSetting) {
+			if (!is_array($inputColumnSetting)) {
+				throw new \InvalidArgumentException('Each column settings entry must be an array');
+			}
 			$columnSettings[] = ViewColumnInformation::fromArray($inputColumnSetting);
 		}
 		return new self($columnSettings);
 	}
 
-	public function jsonSerialize(): mixed {
-		return $this->columnSettings;
+	public function jsonSerialize(): array {
+		return array_map(static fn (ColumnOrderInformation $vci) => $vci->jsonSerialize(), $this->columnSettings);
 	}
 }

lib/Model/SortRuleSet.php

@@ -35,6 +35,9 @@ public function __construct(
 	public static function createFromInputArray(array $data): self {
 		$sortRules = [];
 		foreach ($data as $inputSortRule) {
+			if (!is_array($inputSortRule)) {
+				throw new InvalidArgumentException('Each sort rule entry must be an array');
+			}
 			if (!isset($inputSortRule['columnId'], $inputSortRule['mode'])) {
 				throw new InvalidArgumentException('Required sort parameters are missing');
 			}
@@ -47,6 +50,9 @@ public static function createFromInputArray(array $data): self {
 		return new self($sortRules);
 	}
 
+	/**
+	 * @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
+	 */
 	public function jsonSerialize(): array {
 		return array_map(static fn (SortRule $s) => $s->jsonSerialize(), $this->sortRules);
 	}

lib/Model/ViewUpdateInput.php

@@ -84,7 +84,7 @@ public static function fromInputArray(array $data): self {
 			title: ($data['title'] ?? null) ? new Title($data['title']) : null,
 			description: $data['description'] ?? null,
 			emoji: ($data['emoji'] ?? null) ? new Emoji($data['emoji']) : null,
-			columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createFromInputArray($data['columnSettings']) : null,
+			columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createViewSettingsFromInputArray($data['columnSettings']) : null,
 			filterSet: ($data['filter'] ?? null) ? FilterSet::createFromInputArray($data['filter']) : null,
 			sortRuleSet: ($data['sort'] ?? null) ? SortRuleSet::createFromInputArray($data['sort']) : null,
 		);

lib/ResponseDefinitions.php

@@ -64,9 +64,10 @@
  * 	},
  *  hasShares: bool,
  *  rowsCount: int,
+ *  description: string,
  *  views: list<TablesView>,
  *  columnsCount: int,
- *  columnOrder: list<array{columnId: int, order: int, readonly: bool}>,
+ *  columnOrder: list<array{columnId: int, order: int}>,
  *  sort: list<array{columnId: int, mode: 'ASC'|'DESC'}>,
  * }
  *

lib/Service/TableService.php

@@ -476,7 +476,7 @@ public function delete(int $id, ?string $userId = null): Table {
 	 * @throws NotFoundError
 	 * @throws PermissionError
 	 */
-	public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?array $columnSettings = null, ?array $sort = null): Table {
+	public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?ColumnSettings $columnSettings = null, ?SortRuleSet $sort = null): Table {
 		$userId = $this->permissionsService->preCheckUserId($userId);
 
 		try {
@@ -509,10 +509,10 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip
 			$table->setDescription($description);
 		}
 		if ($columnSettings !== null) {
-			$table->setColumnOrder(\json_encode(ColumnSettings::createFromInputArray($columnSettings)->jsonSerialize()));
+			$table->setColumnOrder(\json_encode($columnSettings->jsonSerialize()));
 		}
 		if ($sort !== null) {
-			$table->setSort(\json_encode(SortRuleSet::createFromInputArray($sort)->jsonSerialize()));
+			$table->setSort(\json_encode($sort->jsonSerialize()));
 		}
 		$table->setLastEditBy($userId);
 		$table->setLastEditAt($time->format('Y-m-d H:i:s'));