refactor: turn columnSettings data from loose array to value object

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

lib/BackgroundJob/ConvertViewColumnsFormat.php

@@ -7,6 +7,7 @@ declare(strict_types=1);
  */
 namespace OCA\Tables\BackgroundJob;
 
+use OCA\Tables\Service\ValueObject\ViewColumnInformation;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\BackgroundJob\TimedJob;
@@ -77,10 +78,7 @@ class ConvertViewColumnsFormat extends TimedJob {
 		// Create new columns structure
 		$newColumns = [];
 		foreach ($columns as $order => $columnId) {
-			$newColumns[] = [
-				'columnId' => (int)$columnId,
-				'order' => $order,
-			];
+			$newColumns[] = new ViewColumnInformation((int)$columnId, order: $order);
 		}
 
 		// Execute update query

lib/Db/View.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -10,6 +12,7 @@ namespace OCA\Tables\Db;
 use JsonSerializable;
 use OCA\Tables\Model\Permissions;
 use OCA\Tables\ResponseDefinitions;
+use OCA\Tables\Service\ValueObject\ViewColumnInformation;
 
 /**
  * @psalm-suppress PropertyNotSetInConstructor
@@ -93,14 +96,14 @@ class View extends EntitySuper implements JsonSerializable {
 	public function getColumnsArray(): array {
 
 		$columnSettings = $this->getColumnsSettingsArray();
-		usort($columnSettings, function ($a, $b) {
-			return $a['order'] - $b['order'];
+		usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) {
+			return $a->getOrder() - $b->getOrder();
 		});
-		return array_column($columnSettings, 'columnId');
+		return array_column($columnSettings, ViewColumnInformation::KEY_ID);
 	}
 
 	/**
-	 * @return array<array{columnId: int, order: int}>
+	 * @return array<ViewColumnInformation>
 	 */
 	public function getColumnsSettingsArray(): array {
 		$columns = $this->getArray($this->getColumns());
@@ -109,15 +112,12 @@ class View extends EntitySuper implements JsonSerializable {
 		}
 
 		if (is_array(reset($columns))) {
-			return $columns;
+			return array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns);
 		}
 
 		$result = [];
 		foreach ($columns as $index => $columnId) {
-			$result[] = [
-				'columnId' => $columnId,
-				'order' => (int)$index + 1
-			];
+			$result[] = new ViewColumnInformation($columnId, order: (int)$index + 1);
 		}
 		return $result;
 	}
@@ -160,10 +160,6 @@ class View extends EntitySuper implements JsonSerializable {
 		$this->setColumns(\json_encode($array));
 	}
 
-	public function setColumnsSettingsArray(array $array): void {
-		$this->setColumnSettings(\json_encode($array));
-	}
-
 	public function setSortArray(array $array):void {
 		$this->setSort(\json_encode($array));
 	}
@@ -211,6 +207,7 @@ class View extends EntitySuper implements JsonSerializable {
 	 * @return int[]
 	 */
 	public function getColumnIds(): array {
-		return array_column($this->getColumnsSettingsArray(), 'columnId');
+		$columns = $this->getColumnsSettingsArray();
+		return array_map(static fn (ViewColumnInformation $column): int => $column[ViewColumnInformation::KEY_ID], $columns);
 	}
 }

lib/Service/TableTemplateService.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 /**
  * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -13,6 +15,7 @@ use OCA\Tables\Dto\Column as ColumnDto;
 use OCA\Tables\Errors\InternalError;
 use OCA\Tables\Errors\NotFoundError;
 use OCA\Tables\Errors\PermissionError;
+use OCA\Tables\Service\ValueObject\ViewColumnInformation;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\DB\Exception;
@@ -466,13 +469,7 @@ class TableTemplateService {
 			[
 				'title' => $this->l->t('Create Vacation Request'),
 				'emoji' => '️➕',
-				'columnSettings' => json_encode(array_map(function ($columnId, $index) {
-					return [
-						'columnId' => $columnId,
-						'order' => $index
-					];
-				}, [$columns['employee']->getId(), $columns['from']->getId(), $columns['to']->getId(), $columns['workingDays']->getId(), $columns['dateRequest']->getId()], array_keys([$columns['employee']->getId(), $columns['from']->getId(), $columns['to']->getId(), $columns['workingDays']->getId(), $columns['dateRequest']->getId()]))),
-				'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_AT, 'mode' => 'ASC']]),
+				'columnSettings' => $this->columnsToColumnSettingsJsonString($columns),
 				'filter' => json_encode([[['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => 'is-equal', 'value' => '@my-name'], ['columnId' => $columns['approved']->getId(), 'operator' => 'is-empty', 'value' => '']]]),
 			]
 		);
@@ -480,12 +477,7 @@ class TableTemplateService {
 			[
 				'title' => $this->l->t('Open Request'),
 				'emoji' => '️📝',
-				'columnSettings' => json_encode(array_map(function ($column, $index) {
-					return [
-						'columnId' => $column->getId(),
-						'order' => $index
-					];
-				}, array_values($columns), array_keys(array_values($columns)))),
+				'columnSettings' => $this->columnsToColumnSettingsJsonString($columns),
 				'sort' => json_encode([['columnId' => $columns['from']->getId(), 'mode' => 'ASC']]),
 				'filter' => json_encode([[['columnId' => $columns['approved']->getId(), 'operator' => 'is-empty', 'value' => '']]]),
 			]
@@ -494,12 +486,7 @@ class TableTemplateService {
 			[
 				'title' => $this->l->t('Request Status'),
 				'emoji' => '️❓',
-				'columnSettings' => json_encode(array_map(function ($column, $index) {
-					return [
-						'columnId' => $column->getId(),
-						'order' => $index
-					];
-				}, array_values($columns), array_keys(array_values($columns)))),
+				'columnSettings' => $this->columnsToColumnSettingsJsonString($columns),
 				'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_BY, 'mode' => 'ASC']]),
 				'filter' => json_encode([[['columnId' => Column::TYPE_META_CREATED_BY, 'operator' => 'is-equal', 'value' => '@my-name']]]),
 			]
@@ -508,18 +495,30 @@ class TableTemplateService {
 			[
 				'title' => $this->l->t('Closed requests'),
 				'emoji' => '️✅',
-				'columnSettings' => json_encode(array_map(function ($column, $index) {
-					return [
-						'columnId' => $column->getId(),
-						'order' => $index
-					];
-				}, array_values($columns), array_keys(array_values($columns)))),
+				'columnSettings' => $this->columnsToColumnSettingsJsonString($columns),
 				'sort' => json_encode([['columnId' => Column::TYPE_META_UPDATED_BY, 'mode' => 'ASC']]),
 				'filter' => json_encode([[['columnId' => $columns['approved']->getId(), 'operator' => 'is-equal', 'value' => '@checked']], [['columnId' => $columns['approved']->getId(), 'operator' => 'is-equal', 'value' => '@unchecked']]]),
 			]
 		);
 	}
 
+	/**
+	 * @param array<?Column> $columns
+	 */
+	private function columnsToColumnSettingsJsonString(array $columns): string {
+		$columns = array_filter($columns, static function ($item) {
+			return $item instanceof Column;
+		});
+		return json_encode(
+			array_map(
+				static function (Column $column, int $index): ViewColumnInformation {
+					return new ViewColumnInformation($column->getId(), order: $index);
+				},
+				array_values($columns), array_keys(array_values($columns))
+			)
+		);
+	}
+
 	/**
 	 * @psalm-suppress PossiblyNullReference
 	 * @param Table $table
@@ -797,7 +796,14 @@ class TableTemplateService {
 		$this->createView($table, [
 			'title' => $this->l->t('Check yourself!'),
 			'emoji' => '🏁',
-			'columnSettings' => json_encode([['columnId' => $columns['what']->getId(), 'order' => 0], ['columnId' => $columns['how']->getId(), 'order' => 1], ['columnId' => $columns['ease']->getId(), 'order' => 2], ['columnId' => $columns['done']->getId(), 'order' => 3]]),
+			'columnSettings' => json_encode(
+				[
+					new ViewColumnInformation($columns['what']->getId(), order: 0),
+					new ViewColumnInformation($columns['how']->getId(), order: 1),
+					new ViewColumnInformation($columns['ease']->getId(), order: 2),
+					new ViewColumnInformation($columns['done']->getId(), order: 3),
+				]
+			),
 			'filter' => json_encode([[['columnId' => $columns['done']->getId(), 'operator' => 'is-equal', 'value' => '@unchecked']]]),
 		]);
 	}

lib/Service/ValueObject/ViewColumnInformation.php

@@ -0,0 +1,73 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Tables\Service\ValueObject;
+
+use ArrayAccess;
+use JsonSerializable;
+
+/**
+ * @template-implements ArrayAccess<string, mixed>
+ */
+class ViewColumnInformation implements ArrayAccess, JsonSerializable {
+	public const KEY_ID = 'columnId';
+	public const KEY_ORDER = 'order';
+
+	/** @var array{columndId?: int, order?: int} */
+	protected array $data = [];
+	protected const KEYS = [
+		self::KEY_ID,
+		self::KEY_ORDER,
+	];
+
+	public function __construct(
+		int $columnId,
+		int $order,
+	) {
+		$this->offsetSet(self::KEY_ID, $columnId);
+		$this->offsetSet(self::KEY_ORDER, $order);
+	}
+
+	public function getOrder(): int {
+		return $this->offsetGet(self::KEY_ORDER);
+	}
+
+	public static function fromArray(array $data): static {
+		$vci = new static($data[self::KEY_ID], $data[self::KEY_ORDER]);
+		foreach ($data as $key => $value) {
+			$vci[$key] = $value;
+		}
+		return $vci;
+	}
+
+	public function offsetExists(mixed $offset): bool {
+		return in_array((string)$offset, self::KEYS);
+	}
+
+	public function offsetGet(mixed $offset): mixed {
+		return $this->data[$offset] ?? null;
+	}
+
+	public function offsetSet(mixed $offset, mixed $value): void {
+		if (!$this->offsetExists($offset)) {
+			return;
+		}
+		$this->data[(string)$offset] = $value;
+	}
+
+	public function offsetUnset(mixed $offset): void {
+		if (!$this->offsetExists($offset)) {
+			return;
+		}
+		unset($this->data[(string)$offset]);
+	}
+
+	public function jsonSerialize(): array {
+		return $this->data;
+	}
+}

lib/Service/ViewService.php

@@ -24,6 +24,7 @@ use OCA\Tables\Event\ViewDeletedEvent;
 use OCA\Tables\Helper\UserHelper;
 use OCA\Tables\Model\Permissions;
 use OCA\Tables\ResponseDefinitions;
+use OCA\Tables\Service\ValueObject\ViewColumnInformation;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -272,11 +273,8 @@ class ViewService extends SuperService {
 					$this->logger->info('The old columns format is deprecated. Please use the new format with columnId and order properties.');
 					$decodedValue = \json_decode($value, true);
 					$value = [];
-					foreach ($decodedValue as $order => $id) {
-						$value[] = [
-							'columnId' => $id,
-							'order' => $order
-						];
+					foreach ($decodedValue as $order => $columnId) {
+						$value[] = new ViewColumnInformation($columnId, order: $order);
 					}
 
 					$value = \json_encode($value);
@@ -285,7 +283,8 @@ class ViewService extends SuperService {
 				if ($key === 'columnSettings' || $key === 'columns') {
 					// we have to fetch the service here as ColumnService already depends on the ViewService, i.e. no DI
 					$columnService = \OCP\Server::get(ColumnService::class);
-					$columnIds = array_column(\json_decode($value, true), 'columnId');
+					$rawColumnsArray = \json_decode($value, true);
+					$columnIds = array_column($rawColumnsArray, ViewColumnInformation::KEY_ID);
 
 					$availableColumns = $columnService->findAllByManagedView($view, $userId);
 					$availableColumns = array_map(static fn (Column $column) => $column->getId(), $availableColumns);
@@ -295,6 +294,14 @@ class ViewService extends SuperService {
 						}
 					}
 
+					// ensure we have the correct format and expected values
+					try {
+						$columnsArray = array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $rawColumnsArray);
+						$value = \json_encode($columnsArray);
+					} catch (\Throwable $t) {
+						throw new \InvalidArgumentException('Invalid column data provided', 400, $t);
+					}
+
 					$key = 'columns';
 				}
 
@@ -543,8 +550,8 @@ class ViewService extends SuperService {
 			);
 
 			$columnSettings = $view->getColumnsSettingsArray();
-			$columnSettings = array_filter($columnSettings, function (array $setting) use ($columnId): bool {
-				return $setting['columnId'] !== $columnId;
+			$columnSettings = array_filter($columnSettings, static function (ViewColumnInformation $setting) use ($columnId): bool {
+				return $setting[ViewColumnInformation::KEY_ID] !== $columnId;
 			});
 			$columnSettings = array_values($columnSettings);
 
@@ -587,16 +594,12 @@ class ViewService extends SuperService {
 	 * @param string|null $userId The user ID performing the action
 	 * @return void
 	 * @throws InternalError
-	 * @throws PermissionError
 	 */
 	public function addColumnToView(View $view, Column $column, ?string $userId = null): void {
 		try {
 			$columnsSettings = $view->getColumnsSettingsArray();
-			$nextOrder = empty($columnsSettings) ? 0 : max(array_column($columnsSettings, 'order')) + 1;
-			$columnsSettings[] = [
-				'columnId' => $column->getId(),
-				'order' => $nextOrder
-			];
+			$nextOrder = empty($columnsSettings) ? 0 : max(array_column($columnsSettings, ViewColumnInformation::KEY_ORDER)) + 1;
+			$columnsSettings[] = new ViewColumnInformation($column->getId(), $nextOrder);
 			$this->update($view->getId(), ['columnSettings' => json_encode($columnsSettings)], $userId, true);
 		} catch (Exception $e) {
 			$this->logger->error($e->getMessage(), ['exception' => $e]);

tests/integration/features/bootstrap/FeatureContext.php

@@ -900,7 +900,10 @@ class FeatureContext implements Context {
 		$columns = explode(',', $columnList);
 		$columnSettings = array_map(function (string $columnAlias, int $index) {
 			if (is_numeric($columnAlias)) {
-				return (int)$columnAlias;
+				return [
+					'columnId' => (int)$columnAlias,
+					'order' => $index
+				];
 			}
 
 			$col = $this->collectionManager->getByAlias('column', $columnAlias);