From a85bb182c7ef0e1578228d536ec4af6b1d047810 Mon Sep 17 00:00:00 2001 From: Aneurin Barker Snook Date: Fri, 4 Sep 2026 21:45:02 +0100 Subject: [PATCH] De-duplicate controller boilerplate: owned-project lookup, reorder validation requireOwnedProjectId() was copy-pasted identically in CardController and CardStatusController; ProjectController's own requireOwnedProject() was the same lookup, just returning the full row instead of the id. New abstract ProjectScopedController (extends Controller) holds one copy of both, and all three controllers now extend it instead of Controller directly, forwarding their ProjectRepository to its constructor. Separately, CardController::reorder() (card_ids) and CardStatusController::reorder() (status_ids) each had the same inline 'must be an array of ids, no duplicates' check. Both now call a new Validator::intIdArray(), which does the same shape check once. Co-Authored-By: Claude Sonnet 5 --- src/Http/Controllers/CardController.php | 31 +++--------- src/Http/Controllers/CardStatusController.php | 30 +++--------- src/Http/Controllers/ProjectController.php | 23 ++------- .../Controllers/ProjectScopedController.php | 47 +++++++++++++++++++ src/Support/Validator.php | 32 +++++++++++++ 5 files changed, 95 insertions(+), 68 deletions(-) create mode 100644 src/Http/Controllers/ProjectScopedController.php diff --git a/src/Http/Controllers/CardController.php b/src/Http/Controllers/CardController.php index 66931e5..b1c032c 100644 --- a/src/Http/Controllers/CardController.php +++ b/src/Http/Controllers/CardController.php @@ -18,15 +18,16 @@ use Psr\Http\Message\ServerRequestInterface as Request; * ordering routes are addressed globally (by card id, or by an explicit * project_id/status_id column) since a card need not have a project. */ -final class CardController extends Controller +final class CardController extends ProjectScopedController { private const TEXT_MAX = 1000; public function __construct( - private readonly ProjectRepository $projects, + ProjectRepository $projects, private readonly CardRepository $cards, private readonly CardStatusRepository $statuses, ) { + parent::__construct($projects); } /** @@ -168,14 +169,10 @@ final class CardController extends Controller [$projectId, $statusId] = $this->targetColumn($ownerId, $body); - $order = $body['card_ids'] ?? null; - if (!is_array($order) || array_filter($order, static fn ($id) => !is_int($id)) !== []) { - throw new ApiException('card_ids must be an array of card IDs.', 422); - } - /** @var int[] $order */ - if (count($order) !== count(array_unique($order))) { - throw new ApiException('card_ids must not contain duplicates.', 422); - } + $validator = new Validator($body); + $order = $validator->intIdArray('card_ids'); + $validator->assert(); + if (array_diff($order, $this->cards->idsOwnedBy($ownerId)) !== []) { throw new ApiException('Every card_id must be a card you own.', 422); } @@ -234,20 +231,6 @@ final class CardController extends Controller return $this->statuses->allForProject($projectId)[0]['id']; } - /** - * @param array $args - */ - private function requireOwnedProjectId(Request $request, array $args): int - { - $project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']); - - if ($project === null) { - throw new ApiException('Project not found.', 404); - } - - return $project['id']; - } - /** * @param array $args * @return array{id: int, owner_id: int, project_id: int|null, text: string, complete: bool, position: int, status_id: int|null, status: array{id: int, name: string}|null, created_at: string, updated_at: string} diff --git a/src/Http/Controllers/CardStatusController.php b/src/Http/Controllers/CardStatusController.php index dd96d18..57a25d7 100644 --- a/src/Http/Controllers/CardStatusController.php +++ b/src/Http/Controllers/CardStatusController.php @@ -18,15 +18,16 @@ use Psr\Http\Message\ServerRequestInterface as Request; * one status, since a project card is required to have one (see the CHECK * constraint on `cards`); deleting the last one is rejected. */ -final class CardStatusController extends Controller +final class CardStatusController extends ProjectScopedController { private const NAME_MAX = 100; public function __construct( - private readonly ProjectRepository $projects, + ProjectRepository $projects, private readonly CardStatusRepository $statuses, private readonly CardRepository $cards, ) { + parent::__construct($projects); } /** @@ -68,14 +69,9 @@ final class CardStatusController extends Controller $projectId = $this->requireOwnedProjectId($request, $args); $body = $this->body($request); - $order = $body['status_ids'] ?? null; - if (!is_array($order) || array_filter($order, static fn ($id) => !is_int($id)) !== []) { - throw new ApiException('status_ids must be an array of status IDs.', 422); - } - /** @var int[] $order */ - if (count($order) !== count(array_unique($order))) { - throw new ApiException('status_ids must not contain duplicates.', 422); - } + $validator = new Validator($body); + $order = $validator->intIdArray('status_ids'); + $validator->assert(); $existingIds = array_column($this->statuses->allForProject($projectId), 'id'); if (array_diff($order, $existingIds) !== [] || array_diff($existingIds, $order) !== []) { @@ -147,20 +143,6 @@ final class CardStatusController extends Controller return $response->withStatus(204); } - /** - * @param array $args - */ - private function requireOwnedProjectId(Request $request, array $args): int - { - $project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']); - - if ($project === null) { - throw new ApiException('Project not found.', 404); - } - - return $project['id']; - } - /** * @param array{id: int, project_id: int, name: string, position: int, created_at: string, updated_at: string} $status * @return array diff --git a/src/Http/Controllers/ProjectController.php b/src/Http/Controllers/ProjectController.php index e308504..7049f4f 100644 --- a/src/Http/Controllers/ProjectController.php +++ b/src/Http/Controllers/ProjectController.php @@ -15,16 +15,17 @@ use Psr\Http\Message\ServerRequestInterface as Request; * CRUD for the authenticated user's projects. A project is only ever visible to * its owner; anything else responds 404. */ -final class ProjectController extends Controller +final class ProjectController extends ProjectScopedController { private const TITLE_MAX = 255; private const DESCRIPTION_MAX = 2000; private const MAX_PROJECTS_PER_OWNER = 100; public function __construct( - private readonly ProjectRepository $projects, + ProjectRepository $projects, private readonly CardStatusRepository $statuses, ) { + parent::__construct($projects); } /** @@ -105,24 +106,6 @@ final class ProjectController extends Controller return $response->withStatus(204); } - /** - * Load the project named in the route, or 404 if it is missing or not owned - * by the authenticated user. - * - * @param array $args - * @return array{id: int, owner_id: int, title: string, description: string, card_count: int, completed_count: int, created_at: string, updated_at: string} - */ - private function requireOwnedProject(Request $request, array $args): array - { - $project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']); - - if ($project === null) { - throw new ApiException('Project not found.', 404); - } - - return $project; - } - /** * @param array{id: int, owner_id: int, title: string, description: string, card_count: int, completed_count: int, created_at: string, updated_at: string} $project * @return array diff --git a/src/Http/Controllers/ProjectScopedController.php b/src/Http/Controllers/ProjectScopedController.php new file mode 100644 index 0000000..9ba0aac --- /dev/null +++ b/src/Http/Controllers/ProjectScopedController.php @@ -0,0 +1,47 @@ + $args + * @return array{id: int, owner_id: int, title: string, description: string, card_count: int, completed_count: int, created_at: string, updated_at: string} + */ + protected function requireOwnedProject(Request $request, array $args): array + { + $project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']); + + if ($project === null) { + throw new ApiException('Project not found.', 404); + } + + return $project; + } + + /** + * @param array $args + */ + protected function requireOwnedProjectId(Request $request, array $args): int + { + return $this->requireOwnedProject($request, $args)['id']; + } +} diff --git a/src/Support/Validator.php b/src/Support/Validator.php index 0dee993..518946a 100644 --- a/src/Support/Validator.php +++ b/src/Support/Validator.php @@ -90,6 +90,38 @@ final class Validator return $value; } + /** + * A required array of unique integer ids -- an ordered list of card or + * status ids to reorder, say. Shape only; whether the ids actually exist + * or are owned by the caller is the caller's job. + * + * @return int[] + */ + public function intIdArray(string $field): array + { + if (!$this->has($field)) { + $this->errors[$field][] = ucfirst($field) . ' is required.'; + + return []; + } + + $value = $this->data[$field]; + if (!is_array($value) || array_filter($value, static fn ($id): bool => !is_int($id)) !== []) { + $this->errors[$field][] = ucfirst($field) . ' must be an array of IDs.'; + + return []; + } + + /** @var int[] $value */ + if (count($value) !== count(array_unique($value))) { + $this->errors[$field][] = ucfirst($field) . ' must not contain duplicates.'; + + return []; + } + + return $value; + } + public function add(string $field, string $message): void { $this->errors[$field][] = $message;