From 230ae585d290d08a507827de4bf714cdea2a27e9 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Wed, 6 Aug 2025 07:35:59 +0200 Subject: [PATCH 01/14] 4992: Changed how exceptions are handled in InstantBook --- src/Controller/InteractiveController.php | 20 ++++++++-- src/InteractiveSlide/InstantBook.php | 51 +++++++++++++----------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/Controller/InteractiveController.php b/src/Controller/InteractiveController.php index 16ebab476..a872ee0ac 100644 --- a/src/Controller/InteractiveController.php +++ b/src/Controller/InteractiveController.php @@ -7,12 +7,14 @@ use App\Entity\ScreenUser; use App\Entity\Tenant\Slide; use App\Entity\User; -use App\Exceptions\NotFoundException; +use App\Exceptions\InteractiveSlideException; use App\Service\InteractiveSlideService; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Attribute\AsController; +use Symfony\Component\HttpKernel\Exception\HttpException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; #[AsController] final readonly class InteractiveController @@ -26,14 +28,24 @@ public function __invoke(Request $request, Slide $slide): JsonResponse { $requestBody = $request->toArray(); - $interactionRequest = $this->interactiveSlideService->parseRequestBody($requestBody); + try { + $interactionRequest = $this->interactiveSlideService->parseRequestBody($requestBody); + } catch (InteractiveSlideException $e) { + throw new HttpException($e->getCode(), $e->getMessage()); + } $user = $this->security->getUser(); if (!($user instanceof User || $user instanceof ScreenUser)) { - throw new NotFoundException('User not found'); + throw new NotFoundHttpException('User not found'); + } + + try { + $actionResult = $this->interactiveSlideService->performAction($user, $slide, $interactionRequest); + } catch (InteractiveSlideException $e) { + throw new HttpException($e->getCode(), $e->getMessage()); } - return new JsonResponse($this->interactiveSlideService->performAction($user, $slide, $interactionRequest)); + return new JsonResponse($actionResult); } } diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index 62f2f2855..37c8ff035 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -9,14 +9,12 @@ use App\Entity\Tenant\InteractiveSlide; use App\Entity\Tenant\Slide; use App\Entity\User; +use App\Exceptions\InteractiveSlideException; use App\Service\InteractiveSlideService; use App\Service\KeyVaultService; use Psr\Cache\CacheItemInterface; use Psr\Cache\InvalidArgumentException; use Symfony\Bundle\SecurityBundle\Security; -use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -use Symfony\Component\HttpKernel\Exception\ConflictHttpException; -use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Contracts\Cache\CacheInterface; use Symfony\Contracts\HttpClient\HttpClientInterface; @@ -90,7 +88,7 @@ public function performAction(UserInterface $user, Slide $slide, InteractionSlid return match ($interactionRequest->action) { self::ACTION_GET_QUICK_BOOK_OPTIONS => $this->getQuickBookOptions($slide, $interactionRequest), self::ACTION_QUICK_BOOK => $this->quickBook($slide, $interactionRequest), - default => throw new BadRequestHttpException('Action not allowed'), + default => throw new InteractiveSlideException('Action not allowed'), }; } @@ -105,7 +103,7 @@ private function authenticate(array $configuration): array $password = $this->keyValueService->getValue($configuration['password']); if (4 !== count(array_filter([$tenantId, $clientId, $username, $password]))) { - throw new BadRequestHttpException('tenantId, clientId, username, password must all be set.'); + throw new InteractiveSlideException('tenantId, clientId, username, password must all be set.'); } $url = self::LOGIN_ENDPOINT.$tenantId.self::OAUTH_PATH; @@ -124,14 +122,14 @@ private function authenticate(array $configuration): array } /** - * @throws InvalidArgumentException + * @throws InteractiveSlideException|InvalidArgumentException */ private function getToken(Tenant $tenant, InteractiveSlide $interactive): string { $configuration = $interactive->getConfiguration(); if (null === $configuration) { - throw new BadRequestHttpException('InteractiveSlide has no configuration'); + throw new InteractiveSlideException('InteractiveSlide has no configuration'); } return $this->interactiveSlideCache->get( @@ -147,14 +145,15 @@ function (CacheItemInterface $item) use ($configuration): mixed { } /** - * @throws \Throwable + * @throws InteractiveSlideException + * @throws InvalidArgumentException */ private function getQuickBookOptions(Slide $slide, InteractionSlideRequest $interactionRequest): array { $resource = $interactionRequest->data['resource'] ?? null; if (null === $resource) { - throw new \Exception('Resource not set.'); + throw new InteractiveSlideException('Resource not set.', 400); } return $this->interactiveSlideCache->get(self::CACHE_KEY_OPTIONS_PREFIX.$resource, @@ -168,7 +167,7 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest) $interactive = $this->interactiveService->getInteractiveSlide($tenant, $interactionRequest->implementationClass); if (null === $interactive) { - throw new \Exception('InteractiveSlide not found'); + throw new InteractiveSlideException('InteractiveSlide not found', 400); } // Optional limiting of available resources. @@ -177,11 +176,11 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest) $feed = $slide->getFeed(); if (null === $feed) { - throw new \Exception('Slide feed not set.'); + throw new InteractiveSlideException('Slide feed not set.', 400); } if (!in_array($resource, $feed->getConfiguration()['resources'] ?? [])) { - throw new \Exception('Resource not in feed resources'); + throw new InteractiveSlideException('Resource not in feed resources', 400); } $token = $this->getToken($tenant, $interactive); @@ -273,7 +272,7 @@ function (CacheItemInterface $item) use ($now): \DateTime { ); if ($lastRequestDateTime->add(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_SPAM_PROTECT)) > $now) { - throw new ServiceUnavailableHttpException(60); + throw new InteractiveSlideException('Service unavailable', 503); } /** @var User|ScreenUser $activeUser */ @@ -283,7 +282,7 @@ function (CacheItemInterface $item) use ($now): \DateTime { $interactive = $this->interactiveService->getInteractiveSlide($tenant, $interactionRequest->implementationClass); if (null === $interactive) { - throw new BadRequestHttpException('Interactive not found'); + throw new InteractiveSlideException('Interactive not found', 400); } // Optional limiting of available resources. @@ -292,11 +291,11 @@ function (CacheItemInterface $item) use ($now): \DateTime { $feed = $slide->getFeed(); if (null === $feed) { - throw new BadRequestHttpException('Slide feed not set.'); + throw new InteractiveSlideException('Slide feed not set.', 400); } if (!in_array($resource, $feed->getConfiguration()['resources'] ?? [])) { - throw new BadRequestHttpException('Resource not in feed resources'); + throw new InteractiveSlideException('Resource not in feed resources', 400); } $token = $this->getToken($tenant, $interactive); @@ -304,7 +303,7 @@ function (CacheItemInterface $item) use ($now): \DateTime { $configuration = $interactive->getConfiguration(); if (null === $configuration) { - throw new BadRequestHttpException('Interactive no configuration'); + throw new InteractiveSlideException('Interactive has no configuration', 400); } $username = $this->keyValueService->getValue($configuration['username']); @@ -315,7 +314,7 @@ function (CacheItemInterface $item) use ($now): \DateTime { // Make sure interval is free. $busyIntervals = $this->getBusyIntervals($token, [$resource], $start, $startPlusDuration); if (count($busyIntervals[$resource]) > 0) { - throw new ConflictHttpException('Interval booked already'); + throw new InteractiveSlideException('Interval booked already', 409); } $requestBody = [ @@ -419,18 +418,21 @@ public function intervalFree(array $schedule, \DateTime $from, \DateTime $to): b return true; } + /** + * @throws InteractiveSlideException + */ private function getValueFromInterval(string $key, InteractionSlideRequest $interactionRequest): string|int { $interval = $interactionRequest->data['interval'] ?? null; if (null === $interval) { - throw new BadRequestHttpException('interval not set.'); + throw new InteractiveSlideException('interval not set.', 400); } $value = $interval[$key] ?? null; if (null === $value) { - throw new BadRequestHttpException("interval.'.$key.' not set."); + throw new InteractiveSlideException("interval.'.$key.' not set.", 400); } return $value; @@ -445,6 +447,9 @@ private function getHeaders(string $token): array ]; } + /** + * @throws InteractiveSlideException + */ private function checkPermission(InteractiveSlide $interactive, string $resource): void { $configuration = $interactive->getConfiguration(); @@ -453,7 +458,7 @@ private function checkPermission(InteractiveSlide $interactive, string $resource $allowedResources = $this->getAllowedResources($interactive); if (!in_array($resource, $allowedResources)) { - throw new \Exception('Not allowed'); + throw new InteractiveSlideException('Not allowed', 403); } } } @@ -468,13 +473,13 @@ private function getAllowedResources(InteractiveSlide $interactive): array $key = $configuration['resourceEndpoint'] ?? null; if (null === $key) { - throw new \Exception('resourceEndpoint not set'); + throw new InteractiveSlideException('resourceEndpoint not set', 400); } $resourceEndpoint = $this->keyValueService->getValue($key); if (null === $resourceEndpoint) { - throw new \Exception('resourceEndpoint value not set'); + throw new InteractiveSlideException('resourceEndpoint value not set', 400); } $response = $this->client->request('GET', $resourceEndpoint); From 15ecfdfd9fb2591731c6b22bf725df88485dc69c Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Wed, 6 Aug 2025 07:42:16 +0200 Subject: [PATCH 02/14] 4992: Added error code --- src/InteractiveSlide/InstantBook.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index 37c8ff035..e8b0c1ef1 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -88,7 +88,7 @@ public function performAction(UserInterface $user, Slide $slide, InteractionSlid return match ($interactionRequest->action) { self::ACTION_GET_QUICK_BOOK_OPTIONS => $this->getQuickBookOptions($slide, $interactionRequest), self::ACTION_QUICK_BOOK => $this->quickBook($slide, $interactionRequest), - default => throw new InteractiveSlideException('Action not allowed'), + default => throw new InteractiveSlideException('Action not allowed', 400), }; } @@ -103,7 +103,7 @@ private function authenticate(array $configuration): array $password = $this->keyValueService->getValue($configuration['password']); if (4 !== count(array_filter([$tenantId, $clientId, $username, $password]))) { - throw new InteractiveSlideException('tenantId, clientId, username, password must all be set.'); + throw new InteractiveSlideException('tenantId, clientId, username, password must all be set.', 400); } $url = self::LOGIN_ENDPOINT.$tenantId.self::OAUTH_PATH; @@ -129,7 +129,7 @@ private function getToken(Tenant $tenant, InteractiveSlide $interactive): string $configuration = $interactive->getConfiguration(); if (null === $configuration) { - throw new InteractiveSlideException('InteractiveSlide has no configuration'); + throw new InteractiveSlideException('InteractiveSlide has no configuration', 400); } return $this->interactiveSlideCache->get( From dab0fd08768d90137f111599524f154a90db3300 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Wed, 6 Aug 2025 07:45:16 +0200 Subject: [PATCH 03/14] 4992: Changed wording --- src/InteractiveSlide/InstantBook.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index e8b0c1ef1..ce71669d0 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -88,7 +88,7 @@ public function performAction(UserInterface $user, Slide $slide, InteractionSlid return match ($interactionRequest->action) { self::ACTION_GET_QUICK_BOOK_OPTIONS => $this->getQuickBookOptions($slide, $interactionRequest), self::ACTION_QUICK_BOOK => $this->quickBook($slide, $interactionRequest), - default => throw new InteractiveSlideException('Action not allowed', 400), + default => throw new InteractiveSlideException('Action not supported', 400), }; } From 16922e116eab689997715bda33f18c6e220e98ae Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Thu, 7 Aug 2025 07:22:27 +0200 Subject: [PATCH 04/14] 4992: Updated changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 784e7a696..6a25a4add 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- [#260](https://github.com/os2display/display-api-service/pull/260) + - Changed how exceptions are handled in InstantBook. + ## [2.5.1] - 2025-06-23 - [#245](https://github.com/os2display/display-api-service/pull/245) From 194111ee81a1aab8768011cee015d2e1ba8b8af1 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Tue, 19 Aug 2025 15:52:31 +0200 Subject: [PATCH 05/14] 4992: Fixed how none existing resources are handled --- src/InteractiveSlide/InstantBook.php | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index ce71669d0..54b8d6941 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -195,10 +195,7 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest) // Add resource to watchedResources, if not in list. if (!in_array($resource, $watchedResources)) { - $this->interactiveSlideCache->delete(self::CACHE_KEY_RESOURCES); - $watchedResources[] = $resource; - $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); } $schedules = $this->getBusyIntervals($token, $watchedResources, $start, $startPlus1Hour); @@ -206,7 +203,11 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest) $result = []; // Refresh entries for all watched resources. - foreach ($watchedResources as $watchResource) { + foreach ($watchedResources as $key => $watchResource) { + if (!isset($schedules[$watchResource])) { + unset($watchedResources[$key]); + } + $entry = $this->createEntry($watchResource, $schedules[$watchResource], $startFormatted, $start); if ($watchResource == $resource) { @@ -224,6 +225,9 @@ function (CacheItemInterface $item) use ($entry) { } } + $this->interactiveSlideCache->delete(self::CACHE_KEY_RESOURCES); + $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); + return $result; } ); @@ -388,9 +392,16 @@ public function getBusyIntervals(string $token, array $resources, \DateTime $sta $result = []; foreach ($scheduleData as $schedule) { - $scheduleId = $schedule['scheduleId']; + $scheduleId = $schedule['scheduleId'] ?? null; + $scheduleItems = $schedule['scheduleItems'] ?? null; + + if ($scheduleId === null ||$scheduleItems === null) { + continue; + } + $result[$scheduleId] = []; - foreach ($schedule['scheduleItems'] as $scheduleItem) { + + foreach ($scheduleItems as $scheduleItem) { $eventStartArray = $scheduleItem['start']; $eventEndArray = $scheduleItem['end']; From 3ac5c6aa13ff1d2cde71980250f860dc32a2ed8f Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Tue, 19 Aug 2025 16:50:36 +0200 Subject: [PATCH 06/14] 4992: Avoid reporting a lot of errors. Reporting no booking options instead --- public/fixture/resources.json | 6 ++ src/InteractiveSlide/InstantBook.php | 116 +++++++++++++++------------ 2 files changed, 71 insertions(+), 51 deletions(-) create mode 100644 public/fixture/resources.json diff --git a/public/fixture/resources.json b/public/fixture/resources.json new file mode 100644 index 000000000..48f467875 --- /dev/null +++ b/public/fixture/resources.json @@ -0,0 +1,6 @@ +[ + { + "ResourceMail": "DOKK1-Lokale-Test1@aarhus.dk", + "allowInstantBooking": "True" + } +] diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index 54b8d6941..5ca6f0ead 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -156,84 +156,94 @@ private function getQuickBookOptions(Slide $slide, InteractionSlideRequest $inte throw new InteractiveSlideException('Resource not set.', 400); } + $start = (new \DateTime())->setTimezone(new \DateTimeZone('UTC')); + $startFormatted = $start->format('c'); + return $this->interactiveSlideCache->get(self::CACHE_KEY_OPTIONS_PREFIX.$resource, - function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest) { + function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest, $start, $startFormatted) { $item->expiresAfter(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_OPTIONS)); - /** @var User|ScreenUser $activeUser */ - $activeUser = $this->security->getUser(); - $tenant = $activeUser->getActiveTenant(); + try { + /** @var User|ScreenUser $activeUser */ + $activeUser = $this->security->getUser(); + $tenant = $activeUser->getActiveTenant(); - $interactive = $this->interactiveService->getInteractiveSlide($tenant, $interactionRequest->implementationClass); + $interactive = $this->interactiveService->getInteractiveSlide($tenant, $interactionRequest->implementationClass); - if (null === $interactive) { - throw new InteractiveSlideException('InteractiveSlide not found', 400); - } + if (null === $interactive) { + throw new InteractiveSlideException('InteractiveSlide not found', 400); + } - // Optional limiting of available resources. - $this->checkPermission($interactive, $resource); + // Optional limiting of available resources. + $this->checkPermission($interactive, $resource); - $feed = $slide->getFeed(); + $feed = $slide->getFeed(); - if (null === $feed) { - throw new InteractiveSlideException('Slide feed not set.', 400); - } + if (null === $feed) { + throw new InteractiveSlideException('Slide feed not set.', 400); + } - if (!in_array($resource, $feed->getConfiguration()['resources'] ?? [])) { - throw new InteractiveSlideException('Resource not in feed resources', 400); - } + if (!in_array($resource, $feed->getConfiguration()['resources'] ?? [])) { + throw new InteractiveSlideException('Resource not in feed resources', 400); + } - $token = $this->getToken($tenant, $interactive); + $token = $this->getToken($tenant, $interactive); - $start = (new \DateTime())->setTimezone(new \DateTimeZone('UTC')); - $startFormatted = $start->format('c'); + $startPlus1Hour = (clone $start)->add(new \DateInterval('PT1H'))->setTimezone(new \DateTimeZone('UTC')); - $startPlus1Hour = (clone $start)->add(new \DateInterval('PT1H'))->setTimezone(new \DateTimeZone('UTC')); + // Get resources that are watched for availability. + $watchedResources = $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => []); - // Get resources that are watched for availability. - $watchedResources = $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => []); + // Add resource to watchedResources, if not in list. + if (!in_array($resource, $watchedResources)) { + $watchedResources[] = $resource; + } - // Add resource to watchedResources, if not in list. - if (!in_array($resource, $watchedResources)) { - $watchedResources[] = $resource; - } + $schedules = $this->getBusyIntervals($token, $watchedResources, $start, $startPlus1Hour); - $schedules = $this->getBusyIntervals($token, $watchedResources, $start, $startPlus1Hour); + $result = []; - $result = []; + // Refresh entries for all watched resources. + foreach ($watchedResources as $key => $watchResource) { + $schedule = $schedules[$watchResource] ?? null; - // Refresh entries for all watched resources. - foreach ($watchedResources as $key => $watchResource) { - if (!isset($schedules[$watchResource])) { - unset($watchedResources[$key]); - } + if ($schedule == null) { + unset($watchedResources[$key]); + } - $entry = $this->createEntry($watchResource, $schedules[$watchResource], $startFormatted, $start); + $entry = $this->createEntry($watchResource, $startFormatted, $start, $schedule); - if ($watchResource == $resource) { - $result = $entry; - } else { - // Refresh cache entry for resources in watch list that are not handled in current request. - $this->interactiveSlideCache->delete(self::CACHE_KEY_OPTIONS_PREFIX.$watchResource); - $this->interactiveSlideCache->get(self::CACHE_KEY_OPTIONS_PREFIX.$watchResource, - function (CacheItemInterface $item) use ($entry) { - $item->expiresAfter(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_OPTIONS)); + if ($watchResource == $resource) { + $result = $entry; + } else { + // Refresh cache entry for resources in watch list that are not handled in current request. + $this->interactiveSlideCache->delete(self::CACHE_KEY_OPTIONS_PREFIX.$watchResource); + $this->interactiveSlideCache->get(self::CACHE_KEY_OPTIONS_PREFIX.$watchResource, + function (CacheItemInterface $item) use ($entry) { + $item->expiresAfter(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_OPTIONS)); - return $entry; - } - ); + return $entry; + } + ); + } } - } - $this->interactiveSlideCache->delete(self::CACHE_KEY_RESOURCES); - $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); + $this->interactiveSlideCache->delete(self::CACHE_KEY_RESOURCES); + $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); - return $result; + return $result; + } catch (InteractiveSlideException $e) { + return [ + 'resource' => $resource, + 'from' => $startFormatted, + 'options' => [], + ]; + } } ); } - private function createEntry(string $resource, array $schedules, string $startFormatted, \DateTime $start): array + private function createEntry(string $resource, string $startFormatted, \DateTime $start, array $schedules = null): array { $entry = [ 'resource' => $resource, @@ -241,6 +251,10 @@ private function createEntry(string $resource, array $schedules, string $startFo 'options' => [], ]; + if ($schedules === null) { + return $entry; + } + foreach (self::DURATIONS as $durationMinutes) { $startPlus = (clone $start)->add(new \DateInterval('PT'.$durationMinutes.'M'))->setTimezone(new \DateTimeZone('UTC')); From 9c56681db73aaa9a4ceda7b98cad0fb61611ab24 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Tue, 19 Aug 2025 17:54:07 +0200 Subject: [PATCH 07/14] 4992: Applied coding standards --- src/InteractiveSlide/InstantBook.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index 5ca6f0ead..bc81a2d74 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -207,7 +207,7 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest, foreach ($watchedResources as $key => $watchResource) { $schedule = $schedules[$watchResource] ?? null; - if ($schedule == null) { + if (null == $schedule) { unset($watchedResources[$key]); } @@ -232,7 +232,7 @@ function (CacheItemInterface $item) use ($entry) { $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); return $result; - } catch (InteractiveSlideException $e) { + } catch (InteractiveSlideException) { return [ 'resource' => $resource, 'from' => $startFormatted, @@ -243,7 +243,7 @@ function (CacheItemInterface $item) use ($entry) { ); } - private function createEntry(string $resource, string $startFormatted, \DateTime $start, array $schedules = null): array + private function createEntry(string $resource, string $startFormatted, \DateTime $start, ?array $schedules = null): array { $entry = [ 'resource' => $resource, @@ -251,7 +251,7 @@ private function createEntry(string $resource, string $startFormatted, \DateTime 'options' => [], ]; - if ($schedules === null) { + if (null === $schedules) { return $entry; } @@ -409,7 +409,7 @@ public function getBusyIntervals(string $token, array $resources, \DateTime $sta $scheduleId = $schedule['scheduleId'] ?? null; $scheduleItems = $schedule['scheduleItems'] ?? null; - if ($scheduleId === null ||$scheduleItems === null) { + if (null === $scheduleId || null === $scheduleItems) { continue; } From bab679aef4eb98ace43785ce6af4de0bbfe321da Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Wed, 20 Aug 2025 09:06:26 +0200 Subject: [PATCH 08/14] 4992: Cleanup --- public/fixture/resources.json | 6 ------ src/InteractiveSlide/InstantBook.php | 13 ++++++++----- 2 files changed, 8 insertions(+), 11 deletions(-) delete mode 100644 public/fixture/resources.json diff --git a/public/fixture/resources.json b/public/fixture/resources.json deleted file mode 100644 index 48f467875..000000000 --- a/public/fixture/resources.json +++ /dev/null @@ -1,6 +0,0 @@ -[ - { - "ResourceMail": "DOKK1-Lokale-Test1@aarhus.dk", - "allowInstantBooking": "True" - } -] diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index bc81a2d74..b52c1474c 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -232,7 +232,7 @@ function (CacheItemInterface $item) use ($entry) { $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); return $result; - } catch (InteractiveSlideException) { + } catch (\Exception) { return [ 'resource' => $resource, 'from' => $startFormatted, @@ -368,10 +368,13 @@ function (CacheItemInterface $item) use ($now): \DateTime { $status = $response->getStatusCode(); - return ['status' => $status, 'interval' => [ - 'from' => $start->format('c'), - 'to' => $startPlusDuration->format('c'), - ]]; + return [ + 'status' => $status, + 'interval' => [ + 'from' => $start->format('c'), + 'to' => $startPlusDuration->format('c'), + ], + ]; } /** From 782eab290f1b3a2631c8e3a5fa1fc9f4efb284d3 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Thu, 21 Aug 2025 11:15:14 +0200 Subject: [PATCH 09/14] 4992: Cleaned up how exceptions are mapped to http exceptions --- config/packages/api_platform.yaml | 19 ++ public/fixture/resources.json | 6 + src/Controller/InteractiveController.php | 37 ++-- ...veSlide.php => InteractiveSlideConfig.php} | 3 +- src/Exceptions/ForbiddenException.php | 8 + src/Exceptions/InteractiveSlideException.php | 9 - src/Exceptions/NotAcceptableException.php | 8 + src/Exceptions/TooManyRequestsException.php | 8 + src/InteractiveSlide/InstantBook.php | 171 ++++++++++-------- .../InteractiveSlideInterface.php | 14 +- src/Repository/InteractiveSlideRepository.php | 14 +- src/Service/InteractiveSlideService.php | 41 ++--- 12 files changed, 199 insertions(+), 139 deletions(-) create mode 100644 public/fixture/resources.json rename src/Entity/Tenant/{InteractiveSlide.php => InteractiveSlideConfig.php} (90%) create mode 100644 src/Exceptions/ForbiddenException.php delete mode 100644 src/Exceptions/InteractiveSlideException.php create mode 100644 src/Exceptions/NotAcceptableException.php create mode 100644 src/Exceptions/TooManyRequestsException.php diff --git a/config/packages/api_platform.yaml b/config/packages/api_platform.yaml index 3455a12d1..89ad82cb3 100644 --- a/config/packages/api_platform.yaml +++ b/config/packages/api_platform.yaml @@ -50,3 +50,22 @@ api_platform: email: itkdev@mkb.aarhus.dk license: name: MIT + + # @see https://api-platform.com/docs/core/errors/#exception-to-status-configuration-using-symfony + exception_to_status: + # The 4 following handlers are registered by default, keep those lines to prevent unexpected side effects + Symfony\Component\Serializer\Exception\ExceptionInterface: 400 # Use a raw status code (recommended) + ApiPlatform\Exception\InvalidArgumentException: !php/const Symfony\Component\HttpFoundation\Response::HTTP_BAD_REQUEST + ApiPlatform\ParameterValidator\Exception\ValidationExceptionInterface: 400 + Doctrine\ORM\OptimisticLockException: 409 + + # Validation exception + ApiPlatform\Validator\Exception\ValidationException: !php/const Symfony\Component\HttpFoundation\Response::HTTP_UNPROCESSABLE_ENTITY + + # App exception mappings + App\Exceptions\BadRequestException: 400 + App\Exceptions\ForbiddenException: 403 + App\Exceptions\NotFoundException: 404 + App\Exceptions\NotAcceptableException: 406 + App\Exceptions\ConflictException: 409 + App\Exceptions\TooManyRequestsException: 429 diff --git a/public/fixture/resources.json b/public/fixture/resources.json new file mode 100644 index 000000000..48f467875 --- /dev/null +++ b/public/fixture/resources.json @@ -0,0 +1,6 @@ +[ + { + "ResourceMail": "DOKK1-Lokale-Test1@aarhus.dk", + "allowInstantBooking": "True" + } +] diff --git a/src/Controller/InteractiveController.php b/src/Controller/InteractiveController.php index a872ee0ac..d5e5b82f3 100644 --- a/src/Controller/InteractiveController.php +++ b/src/Controller/InteractiveController.php @@ -6,15 +6,16 @@ use App\Entity\ScreenUser; use App\Entity\Tenant\Slide; -use App\Entity\User; -use App\Exceptions\InteractiveSlideException; +use App\Exceptions\BadRequestException; +use App\Exceptions\ConflictException; +use App\Exceptions\NotAcceptableException; +use App\Exceptions\TooManyRequestsException; use App\Service\InteractiveSlideService; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Attribute\AsController; -use Symfony\Component\HttpKernel\Exception\HttpException; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; #[AsController] final readonly class InteractiveController @@ -24,27 +25,27 @@ public function __construct( private Security $security, ) {} + /** + * @throws ConflictException + * @throws BadRequestException + * @throws NotAcceptableException + * @throws TooManyRequestsException + */ public function __invoke(Request $request, Slide $slide): JsonResponse { - $requestBody = $request->toArray(); + $user = $this->security->getUser(); - try { - $interactionRequest = $this->interactiveSlideService->parseRequestBody($requestBody); - } catch (InteractiveSlideException $e) { - throw new HttpException($e->getCode(), $e->getMessage()); + if (!($user instanceof ScreenUser)) { + throw new AccessDeniedHttpException('Only screen user can perform action.'); } - $user = $this->security->getUser(); + $tenant = $user->getActiveTenant(); - if (!($user instanceof User || $user instanceof ScreenUser)) { - throw new NotFoundHttpException('User not found'); - } + $requestBody = $request->toArray(); - try { - $actionResult = $this->interactiveSlideService->performAction($user, $slide, $interactionRequest); - } catch (InteractiveSlideException $e) { - throw new HttpException($e->getCode(), $e->getMessage()); - } + $interactionRequest = $this->interactiveSlideService->parseRequestBody($requestBody); + + $actionResult = $this->interactiveSlideService->performAction($tenant, $slide, $interactionRequest); return new JsonResponse($actionResult); } diff --git a/src/Entity/Tenant/InteractiveSlide.php b/src/Entity/Tenant/InteractiveSlideConfig.php similarity index 90% rename from src/Entity/Tenant/InteractiveSlide.php rename to src/Entity/Tenant/InteractiveSlideConfig.php index faf36d679..457d2d2f1 100644 --- a/src/Entity/Tenant/InteractiveSlide.php +++ b/src/Entity/Tenant/InteractiveSlideConfig.php @@ -9,7 +9,8 @@ use Symfony\Component\Serializer\Annotation\Ignore; #[ORM\Entity(repositoryClass: InteractiveSlideRepository::class)] -class InteractiveSlide extends AbstractTenantScopedEntity +#[ORM\Table(name: "interactive_slide")] +class InteractiveSlideConfig extends AbstractTenantScopedEntity { #[Ignore] #[ORM\Column(nullable: true)] diff --git a/src/Exceptions/ForbiddenException.php b/src/Exceptions/ForbiddenException.php new file mode 100644 index 000000000..c8f6c2f75 --- /dev/null +++ b/src/Exceptions/ForbiddenException.php @@ -0,0 +1,8 @@ +action) { - self::ACTION_GET_QUICK_BOOK_OPTIONS => $this->getQuickBookOptions($slide, $interactionRequest), - self::ACTION_QUICK_BOOK => $this->quickBook($slide, $interactionRequest), - default => throw new InteractiveSlideException('Action not supported', 400), + self::ACTION_GET_QUICK_BOOK_OPTIONS => $this->getQuickBookOptions($tenant, $slide, $interactionRequest), + self::ACTION_QUICK_BOOK => $this->quickBook($tenant, $slide, $interactionRequest), + default => throw new NotAcceptableException('Action not supported'), }; } /** * @throws \Throwable + * @throws NotAcceptableException */ private function authenticate(array $configuration): array { @@ -103,7 +111,7 @@ private function authenticate(array $configuration): array $password = $this->keyValueService->getValue($configuration['password']); if (4 !== count(array_filter([$tenantId, $clientId, $username, $password]))) { - throw new InteractiveSlideException('tenantId, clientId, username, password must all be set.', 400); + throw new NotAcceptableException('tenantId, clientId, username, password must all be set.', 400); } $url = self::LOGIN_ENDPOINT.$tenantId.self::OAUTH_PATH; @@ -122,14 +130,15 @@ private function authenticate(array $configuration): array } /** - * @throws InteractiveSlideException|InvalidArgumentException + * @throws NotAcceptableException + * @throws InvalidArgumentException */ - private function getToken(Tenant $tenant, InteractiveSlide $interactive): string + private function getToken(Tenant $tenant, InteractiveSlideConfig $interactive): string { $configuration = $interactive->getConfiguration(); if (null === $configuration) { - throw new InteractiveSlideException('InteractiveSlide has no configuration', 400); + throw new NotAcceptableException('InteractiveSlide has no configuration'); } return $this->interactiveSlideCache->get( @@ -145,49 +154,45 @@ function (CacheItemInterface $item) use ($configuration): mixed { } /** - * @throws InteractiveSlideException + * @throws BadRequestException * @throws InvalidArgumentException */ - private function getQuickBookOptions(Slide $slide, InteractionSlideRequest $interactionRequest): array + private function getQuickBookOptions(Tenant $tenant, Slide $slide, InteractionSlideRequest $interactionRequest): array { $resource = $interactionRequest->data['resource'] ?? null; if (null === $resource) { - throw new InteractiveSlideException('Resource not set.', 400); + throw new BadRequestException('Resource not set.'); } $start = (new \DateTime())->setTimezone(new \DateTimeZone('UTC')); - $startFormatted = $start->format('c'); return $this->interactiveSlideCache->get(self::CACHE_KEY_OPTIONS_PREFIX.$resource, - function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest, $start, $startFormatted) { + function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest, $start, $tenant) { $item->expiresAfter(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_OPTIONS)); + // If any exceptions are thrown we return an empty options entry. try { - /** @var User|ScreenUser $activeUser */ - $activeUser = $this->security->getUser(); - $tenant = $activeUser->getActiveTenant(); - - $interactive = $this->interactiveService->getInteractiveSlide($tenant, $interactionRequest->implementationClass); + $interactiveSlideConfig = $this->interactiveService->getInteractiveSlideConfig($tenant, $interactionRequest->implementationClass); - if (null === $interactive) { - throw new InteractiveSlideException('InteractiveSlide not found', 400); + if (null === $interactiveSlideConfig) { + throw new NotAcceptableException('InteractiveSlideConfig not found'); } // Optional limiting of available resources. - $this->checkPermission($interactive, $resource); + $this->checkPermission($interactiveSlideConfig, $resource); $feed = $slide->getFeed(); if (null === $feed) { - throw new InteractiveSlideException('Slide feed not set.', 400); + throw new NotAcceptableException('Slide feed not set.'); } if (!in_array($resource, $feed->getConfiguration()['resources'] ?? [])) { - throw new InteractiveSlideException('Resource not in feed resources', 400); + throw new NotAcceptableException('Resource not in feed resources'); } - $token = $this->getToken($tenant, $interactive); + $token = $this->getToken($tenant, $interactiveSlideConfig); $startPlus1Hour = (clone $start)->add(new \DateInterval('PT1H'))->setTimezone(new \DateTimeZone('UTC')); @@ -207,11 +212,11 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest, foreach ($watchedResources as $key => $watchResource) { $schedule = $schedules[$watchResource] ?? null; - if (null == $schedule) { + if (!isset($schedules[$watchResource])) { unset($watchedResources[$key]); } - $entry = $this->createEntry($watchResource, $startFormatted, $start, $schedule); + $entry = $this->createEntry($watchResource, $start, $schedule); if ($watchResource == $resource) { $result = $entry; @@ -232,19 +237,18 @@ function (CacheItemInterface $item) use ($entry) { $this->interactiveSlideCache->get(self::CACHE_KEY_RESOURCES, fn () => $watchedResources); return $result; - } catch (\Exception) { - return [ - 'resource' => $resource, - 'from' => $startFormatted, - 'options' => [], - ]; + } catch (\Throwable) { + // All errors should result in empty options. + return $this->createEntry($resource, $start); } } ); } - private function createEntry(string $resource, string $startFormatted, \DateTime $start, ?array $schedules = null): array + private function createEntry(string $resource, \DateTime $start, ?array $schedules = null): array { + $startFormatted = $start->format('c'); + $entry = [ 'resource' => $resource, 'from' => $startFormatted, @@ -256,7 +260,11 @@ private function createEntry(string $resource, string $startFormatted, \DateTime } foreach (self::DURATIONS as $durationMinutes) { - $startPlus = (clone $start)->add(new \DateInterval('PT'.$durationMinutes.'M'))->setTimezone(new \DateTimeZone('UTC')); + try { + $startPlus = (clone $start)->add(new \DateInterval('PT'.$durationMinutes.'M'))->setTimezone(new \DateTimeZone('UTC')); + } catch (\Exception) { + continue; + } if ($this->intervalFree($schedules, $start, $startPlus)) { $entry['options'][] = [ @@ -270,9 +278,15 @@ private function createEntry(string $resource, string $startFormatted, \DateTime } /** + * @throws TooManyRequestsException + * @throws ConflictException + * @throws BadRequestException + * @throws InvalidArgumentException + * @throws NotAcceptableException + * @throws ForbiddenException * @throws \Throwable */ - private function quickBook(Slide $slide, InteractionSlideRequest $interactionRequest): array + private function quickBook(Tenant $tenant, Slide $slide, InteractionSlideRequest $interactionRequest): array { $resource = (string) $this->getValueFromInterval('resource', $interactionRequest); $durationMinutes = $this->getValueFromInterval('durationMinutes', $interactionRequest); @@ -290,38 +304,34 @@ function (CacheItemInterface $item) use ($now): \DateTime { ); if ($lastRequestDateTime->add(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_SPAM_PROTECT)) > $now) { - throw new InteractiveSlideException('Service unavailable', 503); + throw new TooManyRequestsException('Service unavailable', 503); } - /** @var User|ScreenUser $activeUser */ - $activeUser = $this->security->getUser(); - $tenant = $activeUser->getActiveTenant(); - - $interactive = $this->interactiveService->getInteractiveSlide($tenant, $interactionRequest->implementationClass); + $interactiveSlideConfig = $this->interactiveService->getInteractiveSlideConfig($tenant, $interactionRequest->implementationClass); - if (null === $interactive) { - throw new InteractiveSlideException('Interactive not found', 400); + if (null === $interactiveSlideConfig) { + throw new NotAcceptableException('InteractiveSlideConfig not found', 400); } // Optional limiting of available resources. - $this->checkPermission($interactive, $resource); + $this->checkPermission($interactiveSlideConfig, $resource); $feed = $slide->getFeed(); if (null === $feed) { - throw new InteractiveSlideException('Slide feed not set.', 400); + throw new NotAcceptableException('Slide feed not set.'); } if (!in_array($resource, $feed->getConfiguration()['resources'] ?? [])) { - throw new InteractiveSlideException('Resource not in feed resources', 400); + throw new NotAcceptableException('Resource not in feed resources'); } - $token = $this->getToken($tenant, $interactive); + $token = $this->getToken($tenant, $interactiveSlideConfig); - $configuration = $interactive->getConfiguration(); + $configuration = $interactiveSlideConfig->getConfiguration(); if (null === $configuration) { - throw new InteractiveSlideException('Interactive has no configuration', 400); + throw new NotAcceptableException('InteractiveSlideConfig has no configuration'); } $username = $this->keyValueService->getValue($configuration['username']); @@ -332,7 +342,7 @@ function (CacheItemInterface $item) use ($now): \DateTime { // Make sure interval is free. $busyIntervals = $this->getBusyIntervals($token, [$resource], $start, $startPlusDuration); if (count($busyIntervals[$resource]) > 0) { - throw new InteractiveSlideException('Interval booked already', 409); + throw new ConflictException('Interval booked already'); } $requestBody = [ @@ -379,10 +389,9 @@ function (CacheItemInterface $item) use ($now): \DateTime { /** * @see https://docs.microsoft.com/en-us/graph/api/calendar-getschedule?view=graph-rest-1.0&tabs=http - * * @throws \Throwable */ - public function getBusyIntervals(string $token, array $resources, \DateTime $startTime, \DateTime $endTime): array + private function getBusyIntervals(string $token, array $resources, \DateTime $startTime, \DateTime $endTime): array { $body = [ 'schedules' => $resources, @@ -435,7 +444,7 @@ public function getBusyIntervals(string $token, array $resources, \DateTime $sta return $result; } - public function intervalFree(array $schedule, \DateTime $from, \DateTime $to): bool + private function intervalFree(array $schedule, \DateTime $from, \DateTime $to): bool { foreach ($schedule as $scheduleEntry) { if (!($scheduleEntry['startTime'] > $to || $scheduleEntry['endTime'] < $from)) { @@ -447,20 +456,20 @@ public function intervalFree(array $schedule, \DateTime $from, \DateTime $to): b } /** - * @throws InteractiveSlideException + * @throws BadRequestException */ private function getValueFromInterval(string $key, InteractionSlideRequest $interactionRequest): string|int { $interval = $interactionRequest->data['interval'] ?? null; if (null === $interval) { - throw new InteractiveSlideException('interval not set.', 400); + throw new BadRequestException('interval not set.'); } $value = $interval[$key] ?? null; if (null === $value) { - throw new InteractiveSlideException("interval.'.$key.' not set.", 400); + throw new BadRequestException("interval.'.$key.' not set.", 400); } return $value; @@ -476,9 +485,11 @@ private function getHeaders(string $token): array } /** - * @throws InteractiveSlideException + * @throws NotAcceptableException + * @throws ForbiddenException + * @throws InvalidArgumentException */ - private function checkPermission(InteractiveSlide $interactive, string $resource): void + private function checkPermission(InteractiveSlideConfig $interactive, string $resource): void { $configuration = $interactive->getConfiguration(); // Optional limiting of available resources. @@ -486,29 +497,33 @@ private function checkPermission(InteractiveSlide $interactive, string $resource $allowedResources = $this->getAllowedResources($interactive); if (!in_array($resource, $allowedResources)) { - throw new InteractiveSlideException('Not allowed', 403); + throw new ForbiddenException('Not allowed'); } } } - private function getAllowedResources(InteractiveSlide $interactive): array + /** + * @throws NotAcceptableException + * @throws InvalidArgumentException + */ + private function getAllowedResources(InteractiveSlideConfig $interactive): array { - return $this->interactiveSlideCache->get(self::CACHE_ALLOWED_RESOURCES_PREFIX.$interactive->getId(), function (CacheItemInterface $item) use ($interactive) { - $item->expiresAfter(60 * 60); + $configuration = $interactive->getConfiguration(); - $configuration = $interactive->getConfiguration(); + $key = $configuration['resourceEndpoint'] ?? null; - $key = $configuration['resourceEndpoint'] ?? null; + if (null === $key) { + throw new NotAcceptableException('resourceEndpoint not set', 400); + } - if (null === $key) { - throw new InteractiveSlideException('resourceEndpoint not set', 400); - } + $resourceEndpoint = $this->keyValueService->getValue($key); - $resourceEndpoint = $this->keyValueService->getValue($key); + if (null === $resourceEndpoint) { + throw new NotAcceptableException('resourceEndpoint value not set', 400); + } - if (null === $resourceEndpoint) { - throw new InteractiveSlideException('resourceEndpoint value not set', 400); - } + return $this->interactiveSlideCache->get(self::CACHE_ALLOWED_RESOURCES_PREFIX.$interactive->getId(), function (CacheItemInterface $item) use ($resourceEndpoint) { + $item->expiresAfter(60 * 60); $response = $this->client->request('GET', $resourceEndpoint); $content = $response->toArray(); diff --git a/src/InteractiveSlide/InteractiveSlideInterface.php b/src/InteractiveSlide/InteractiveSlideInterface.php index 5ba3343d7..df957842e 100644 --- a/src/InteractiveSlide/InteractiveSlideInterface.php +++ b/src/InteractiveSlide/InteractiveSlideInterface.php @@ -4,9 +4,12 @@ namespace App\InteractiveSlide; +use App\Entity\Tenant; use App\Entity\Tenant\Slide; -use App\Exceptions\InteractiveSlideException; -use Symfony\Component\Security\Core\User\UserInterface; +use App\Exceptions\BadRequestException; +use App\Exceptions\ConflictException; +use App\Exceptions\NotAcceptableException; +use App\Exceptions\TooManyRequestsException; interface InteractiveSlideInterface { @@ -15,7 +18,10 @@ public function getConfigOptions(): array; /** * Perform the given InteractionRequest with the given Slide. * - * @throws InteractiveSlideException + * @throws ConflictException + * @throws BadRequestException + * @throws NotAcceptableException + * @throws TooManyRequestsException */ - public function performAction(UserInterface $user, Slide $slide, InteractionSlideRequest $interactionRequest): array; + public function performAction(Tenant $tenant, Slide $slide, InteractionSlideRequest $interactionRequest): array; } diff --git a/src/Repository/InteractiveSlideRepository.php b/src/Repository/InteractiveSlideRepository.php index a9d52ab90..5e263f4bf 100644 --- a/src/Repository/InteractiveSlideRepository.php +++ b/src/Repository/InteractiveSlideRepository.php @@ -4,22 +4,22 @@ namespace App\Repository; -use App\Entity\Tenant\InteractiveSlide; +use App\Entity\Tenant\InteractiveSlideConfig; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\Persistence\ManagerRegistry; /** - * @extends ServiceEntityRepository + * @extends ServiceEntityRepository * - * @method InteractiveSlide|null find($id, $lockMode = null, $lockVersion = null) - * @method InteractiveSlide|null findOneBy(array $criteria, array $orderBy = null) - * @method InteractiveSlide[] findAll() - * @method InteractiveSlide[] findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null) + * @method InteractiveSlideConfig|null find($id, $lockMode = null, $lockVersion = null) + * @method InteractiveSlideConfig|null findOneBy(array $criteria, array $orderBy = null) + * @method InteractiveSlideConfig[] findAll() + * @method InteractiveSlideConfig[] findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null) */ class InteractiveSlideRepository extends ServiceEntityRepository { public function __construct(ManagerRegistry $registry) { - parent::__construct($registry, InteractiveSlide::class); + parent::__construct($registry, InteractiveSlideConfig::class); } } diff --git a/src/Service/InteractiveSlideService.php b/src/Service/InteractiveSlideService.php index e4c8a6804..21460e002 100644 --- a/src/Service/InteractiveSlideService.php +++ b/src/Service/InteractiveSlideService.php @@ -4,17 +4,17 @@ namespace App\Service; -use App\Entity\ScreenUser; use App\Entity\Tenant; -use App\Entity\Tenant\InteractiveSlide; +use App\Entity\Tenant\InteractiveSlideConfig; use App\Entity\Tenant\Slide; -use App\Entity\User; -use App\Exceptions\InteractiveSlideException; +use App\Exceptions\BadRequestException; +use App\Exceptions\ConflictException; +use App\Exceptions\NotAcceptableException; +use App\Exceptions\TooManyRequestsException; use App\InteractiveSlide\InteractionSlideRequest; use App\InteractiveSlide\InteractiveSlideInterface; use App\Repository\InteractiveSlideRepository; use Doctrine\ORM\EntityManagerInterface; -use Symfony\Component\Security\Core\User\UserInterface; /** * Service for handling Slide interactions. @@ -33,7 +33,7 @@ public function __construct( * * @param array $requestBody the request body from the http request * - * @throws InteractiveSlideException + * @throws BadRequestException */ public function parseRequestBody(array $requestBody): InteractionSlideRequest { @@ -42,7 +42,7 @@ public function parseRequestBody(array $requestBody): InteractionSlideRequest $data = $requestBody['data'] ?? null; if (null === $implementationClass || null === $action || null === $data) { - throw new InteractiveSlideException('implementationClass, action and/or data not set.'); + throw new BadRequestException('implementationClass, action and/or data not set.'); } return new InteractionSlideRequest($implementationClass, $action, $data); @@ -51,27 +51,24 @@ public function parseRequestBody(array $requestBody): InteractionSlideRequest /** * Perform an action for an interactive slide. * - * @throws InteractiveSlideException + * @throws ConflictException + * @throws BadRequestException + * @throws NotAcceptableException + * @throws TooManyRequestsException */ - public function performAction(UserInterface $user, Slide $slide, InteractionSlideRequest $interactionRequest): array + public function performAction(Tenant $tenant, Slide $slide, InteractionSlideRequest $interactionRequest): array { - if (!$user instanceof ScreenUser && !$user instanceof User) { - throw new InteractiveSlideException('User is not supported'); - } - - $tenant = $user->getActiveTenant(); - $implementationClass = $interactionRequest->implementationClass; - $interactive = $this->getInteractiveSlide($tenant, $implementationClass); + $interactive = $this->getInteractiveSlideConfig($tenant, $implementationClass); if (null === $interactive) { - throw new InteractiveSlideException('Interactive slide not found'); + throw new NotAcceptableException('Interactive slide config not found'); } $interactiveImplementation = $this->getImplementation($interactive->getImplementationClass()); - return $interactiveImplementation->performAction($user, $slide, $interactionRequest); + return $interactiveImplementation->performAction($tenant, $slide, $interactionRequest); } /** @@ -91,7 +88,7 @@ public function getConfigurables(): array /** * Find the implementation class. * - * @throws InteractiveSlideException + * @throws BadRequestException */ public function getImplementation(?string $implementationClass): InteractiveSlideInterface { @@ -99,7 +96,7 @@ public function getImplementation(?string $implementationClass): InteractiveSlid $interactiveImplementations = array_filter($asArray, fn ($implementation) => $implementation::class === $implementationClass); if (0 === count($interactiveImplementations)) { - throw new InteractiveSlideException('Interactive implementation class not found'); + throw new BadRequestException('Interactive implementation class not found'); } return $interactiveImplementations[0]; @@ -108,7 +105,7 @@ public function getImplementation(?string $implementationClass): InteractiveSlid /** * Get the interactive slide. */ - public function getInteractiveSlide(Tenant $tenant, string $implementationClass): ?InteractiveSlide + public function getInteractiveSlideConfig(Tenant $tenant, string $implementationClass): ?InteractiveSlideConfig { return $this->interactiveSlideRepository->findOneBy([ 'implementationClass' => $implementationClass, @@ -127,7 +124,7 @@ public function saveConfiguration(Tenant $tenant, string $implementationClass, a ]); if (null === $entry) { - $entry = new InteractiveSlide(); + $entry = new InteractiveSlideConfig(); $entry->setTenant($tenant); $entry->setImplementationClass($implementationClass); From 64ad721fcae6ecae181997096091ee2aa85a45ea Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Thu, 21 Aug 2025 11:23:59 +0200 Subject: [PATCH 10/14] 4992: Applied coding standards --- public/fixture/resources.json | 6 ------ src/Entity/Tenant/InteractiveSlideConfig.php | 2 +- src/Exceptions/ForbiddenException.php | 3 ++- src/Exceptions/NotAcceptableException.php | 3 ++- src/Exceptions/TooManyRequestsException.php | 3 ++- src/InteractiveSlide/InstantBook.php | 1 + 6 files changed, 8 insertions(+), 10 deletions(-) delete mode 100644 public/fixture/resources.json diff --git a/public/fixture/resources.json b/public/fixture/resources.json deleted file mode 100644 index 48f467875..000000000 --- a/public/fixture/resources.json +++ /dev/null @@ -1,6 +0,0 @@ -[ - { - "ResourceMail": "DOKK1-Lokale-Test1@aarhus.dk", - "allowInstantBooking": "True" - } -] diff --git a/src/Entity/Tenant/InteractiveSlideConfig.php b/src/Entity/Tenant/InteractiveSlideConfig.php index 457d2d2f1..ea7fa46ab 100644 --- a/src/Entity/Tenant/InteractiveSlideConfig.php +++ b/src/Entity/Tenant/InteractiveSlideConfig.php @@ -9,7 +9,7 @@ use Symfony\Component\Serializer\Annotation\Ignore; #[ORM\Entity(repositoryClass: InteractiveSlideRepository::class)] -#[ORM\Table(name: "interactive_slide")] +#[ORM\Table(name: 'interactive_slide')] class InteractiveSlideConfig extends AbstractTenantScopedEntity { #[Ignore] diff --git a/src/Exceptions/ForbiddenException.php b/src/Exceptions/ForbiddenException.php index c8f6c2f75..74f7e0caa 100644 --- a/src/Exceptions/ForbiddenException.php +++ b/src/Exceptions/ForbiddenException.php @@ -1,8 +1,9 @@ Date: Thu, 21 Aug 2025 11:28:56 +0200 Subject: [PATCH 11/14] 4992: Fixed comment --- src/InteractiveSlide/InstantBook.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index a9fde005a..92aa57d5e 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -179,7 +179,6 @@ function (CacheItemInterface $item) use ($slide, $resource, $interactionRequest, throw new NotAcceptableException('InteractiveSlideConfig not found'); } - // Optional limiting of available resources. $this->checkPermission($interactiveSlideConfig, $resource); $feed = $slide->getFeed(); @@ -493,7 +492,8 @@ private function getHeaders(string $token): array private function checkPermission(InteractiveSlideConfig $interactive, string $resource): void { $configuration = $interactive->getConfiguration(); - // Optional limiting of available resources. + + // Will only limit access to resources if list is set up. if (null !== $configuration && !empty($configuration['resourceEndpoint'])) { $allowedResources = $this->getAllowedResources($interactive); From 735823085e4799d392f6b533d132b0240f934616 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Thu, 21 Aug 2025 18:49:37 +0200 Subject: [PATCH 12/14] 4992: Fixed test issue --- src/InteractiveSlide/InstantBook.php | 2 +- tests/Service/InteractiveServiceTest.php | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index 92aa57d5e..1a3a91ce7 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -444,7 +444,7 @@ private function getBusyIntervals(string $token, array $resources, \DateTime $st return $result; } - private function intervalFree(array $schedule, \DateTime $from, \DateTime $to): bool + public function intervalFree(array $schedule, \DateTime $from, \DateTime $to): bool { foreach ($schedule as $scheduleEntry) { if (!($scheduleEntry['startTime'] > $to || $scheduleEntry['endTime'] < $from)) { diff --git a/tests/Service/InteractiveServiceTest.php b/tests/Service/InteractiveServiceTest.php index 5f5fee8b4..9ce070292 100644 --- a/tests/Service/InteractiveServiceTest.php +++ b/tests/Service/InteractiveServiceTest.php @@ -5,7 +5,8 @@ namespace App\Tests\Service; use App\Entity\Tenant\Slide; -use App\Exceptions\InteractiveSlideException; +use App\Exceptions\BadRequestException; +use App\Exceptions\NotAcceptableException; use App\InteractiveSlide\InstantBook; use App\InteractiveSlide\InteractionSlideRequest; use App\Repository\UserRepository; @@ -41,7 +42,7 @@ public function testParseRequestBody(): void { $interactiveService = $this->container->get(InteractiveSlideService::class); - $this->expectException(InteractiveSlideException::class); + $this->expectException(BadRequestException::class); $interactiveService->parseRequestBody([ 'test' => 'test', @@ -73,19 +74,19 @@ public function testPerformAction(): void 'data' => [], ]); - $this->expectException(InteractiveSlideException::class); - $this->expectExceptionMessage('Interactive slide not found'); + $this->expectException(NotAcceptableException::class); + $this->expectExceptionMessage('Interactive slide config not found'); $tenant = $user->getActiveTenant(); - $interactiveService->performAction($user, $slide, $interactionRequest); + $interactiveService->performAction($tenant, $slide, $interactionRequest); $interactiveService->saveConfiguration($tenant, InstantBook::class, []); - $this->expectException(InteractiveSlideException::class); + $this->expectException(NotAcceptableException::class); $this->expectExceptionMessage('Action not allowed'); - $interactiveService->performAction($user, $slide, $interactionRequest); + $interactiveService->performAction($tenant, $slide, $interactionRequest); } public function testGetConfigurables(): void From acc47c43dbd42b226253328fac71b5a12a9690e3 Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Thu, 28 Aug 2025 10:45:25 +0200 Subject: [PATCH 13/14] 4992: Removed status codes --- src/InteractiveSlide/InstantBook.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/InteractiveSlide/InstantBook.php b/src/InteractiveSlide/InstantBook.php index 1a3a91ce7..489575561 100644 --- a/src/InteractiveSlide/InstantBook.php +++ b/src/InteractiveSlide/InstantBook.php @@ -111,7 +111,7 @@ private function authenticate(array $configuration): array $password = $this->keyValueService->getValue($configuration['password']); if (4 !== count(array_filter([$tenantId, $clientId, $username, $password]))) { - throw new NotAcceptableException('tenantId, clientId, username, password must all be set.', 400); + throw new NotAcceptableException('tenantId, clientId, username, password must all be set.'); } $url = self::LOGIN_ENDPOINT.$tenantId.self::OAUTH_PATH; @@ -303,13 +303,13 @@ function (CacheItemInterface $item) use ($now): \DateTime { ); if ($lastRequestDateTime->add(new \DateInterval(self::CACHE_LIFETIME_QUICK_BOOK_SPAM_PROTECT)) > $now) { - throw new TooManyRequestsException('Service unavailable', 503); + throw new TooManyRequestsException('Service unavailable'); } $interactiveSlideConfig = $this->interactiveService->getInteractiveSlideConfig($tenant, $interactionRequest->implementationClass); if (null === $interactiveSlideConfig) { - throw new NotAcceptableException('InteractiveSlideConfig not found', 400); + throw new NotAcceptableException('InteractiveSlideConfig not found'); } // Optional limiting of available resources. @@ -469,7 +469,7 @@ private function getValueFromInterval(string $key, InteractionSlideRequest $inte $value = $interval[$key] ?? null; if (null === $value) { - throw new BadRequestException("interval.'.$key.' not set.", 400); + throw new BadRequestException("interval.'.$key.' not set."); } return $value; @@ -514,13 +514,13 @@ private function getAllowedResources(InteractiveSlideConfig $interactive): array $key = $configuration['resourceEndpoint'] ?? null; if (null === $key) { - throw new NotAcceptableException('resourceEndpoint not set', 400); + throw new NotAcceptableException('resourceEndpoint not set'); } $resourceEndpoint = $this->keyValueService->getValue($key); if (null === $resourceEndpoint) { - throw new NotAcceptableException('resourceEndpoint value not set', 400); + throw new NotAcceptableException('resourceEndpoint value not set'); } return $this->interactiveSlideCache->get(self::CACHE_ALLOWED_RESOURCES_PREFIX.$interactive->getId(), function (CacheItemInterface $item) use ($resourceEndpoint) { From d07526c316907e6f4bd6b5ce93f36471e1595ffc Mon Sep 17 00:00:00 2001 From: Troels Ugilt Jensen <6103205+tuj@users.noreply.github.com> Date: Thu, 28 Aug 2025 11:33:06 +0200 Subject: [PATCH 14/14] 4992: Fixed table name. Renamed index --- migrations/Version20250828084617.php | 28 ++++++++++++++++++++ src/Entity/Tenant/InteractiveSlideConfig.php | 1 - 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 migrations/Version20250828084617.php diff --git a/migrations/Version20250828084617.php b/migrations/Version20250828084617.php new file mode 100644 index 000000000..6c526f84a --- /dev/null +++ b/migrations/Version20250828084617.php @@ -0,0 +1,28 @@ +addSql('RENAME TABLE interactive_slide TO interactive_slide_config;'); + $this->addSql('ALTER TABLE interactive_slide_config RENAME INDEX idx_138e544d9033212a TO IDX_D30060259033212A'); + } + + public function down(Schema $schema): void + { + $this->addSql('ALTER TABLE interactive_slide_config RENAME INDEX idx_d30060259033212a TO IDX_138E544D9033212A'); + $this->addSql('RENAME TABLE interactive_slide_config TO interactive_slide;'); + } +} diff --git a/src/Entity/Tenant/InteractiveSlideConfig.php b/src/Entity/Tenant/InteractiveSlideConfig.php index ea7fa46ab..987455b69 100644 --- a/src/Entity/Tenant/InteractiveSlideConfig.php +++ b/src/Entity/Tenant/InteractiveSlideConfig.php @@ -9,7 +9,6 @@ use Symfony\Component\Serializer\Annotation\Ignore; #[ORM\Entity(repositoryClass: InteractiveSlideRepository::class)] -#[ORM\Table(name: 'interactive_slide')] class InteractiveSlideConfig extends AbstractTenantScopedEntity { #[Ignore]