From 37273ad3bc3965e96beb68da0d182ddbb079cc84 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Thu, 9 Apr 2026 12:40:00 -0700 Subject: [PATCH 1/3] feat(api): Add groupId filter to GET Resources endpoint Add optional groupId query string parameter to GET /Resources/. Accepts one or more comma-separated positive integer IDs (e.g. ?groupId=1,2,3). Returns only resources belonging to those resource groups, including resources in sub-groups. Returns 400 Bad Request if any value is non-integer or zero. Returns 404 Not Found if a non-existing groupId is used. Updates API documentation in both the inline @description and docs/source/API.rst. Closes: #1326 --- WebServices/ResourcesWebService.php | 38 +++ docs/source/API.rst | 13 + lib/WebService/WebServiceQueryStringKeys.php | 1 + tests/WebServices/ResourcesWebServiceTest.php | 249 ++++++++++++++++++ 4 files changed, 301 insertions(+) diff --git a/WebServices/ResourcesWebService.php b/WebServices/ResourcesWebService.php index 7da9e2f12..0caaa439e 100644 --- a/WebServices/ResourcesWebService.php +++ b/WebServices/ResourcesWebService.php @@ -52,6 +52,7 @@ public function __construct( * @name GetAllResources * @description Loads all resources * Optional query string parameter: scheduleId. One or more schedule IDs, comma-separated (e.g. scheduleId=1,2,3). If provided, only resources belonging to those schedules will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a 400 Bad Request is returned. + * Optional query string parameter: groupId. One or more resource group IDs, comma-separated (e.g. groupId=1,2,3). If provided, only resources belonging to those groups (including sub-groups) will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a 400 Bad Request is returned. If any group ID does not exist, a 404 Not Found is returned. * @response ResourcesResponse * @return void */ @@ -73,6 +74,22 @@ public function GetAll() $scheduleIds = array_map('intval', $rawIds); } + $groupIds = null; + $groupIdParam = $this->server->GetQueryString(WebServiceQueryStringKeys::GROUP_ID); + if ($groupIdParam !== null && $groupIdParam !== '') { + $rawIds = explode(',', $groupIdParam); + foreach ($rawIds as $id) { + if (!ctype_digit($id) || (int)$id === 0) { + $this->server->WriteResponse( + RestResponse::BadRequest("Invalid groupId '$id': must be a positive integer"), + RestResponse::BAD_REQUEST_CODE + ); + return; + } + } + $groupIds = array_map('intval', $rawIds); + } + $resources = $this->resourceRepository->GetUserResourceList(); if ($scheduleIds !== null) { @@ -85,6 +102,27 @@ public function GetAll() $resources = $filteredResources; } + if ($groupIds !== null) { + $groupTree = $this->resourceRepository->GetResourceGroups(); + $groupList = $groupTree->GetGroupList(); + $groupResourceIds = []; + foreach ($groupIds as $groupId) { + if (!array_key_exists($groupId, $groupList)) { + $this->server->WriteResponse(RestResponse::NotFound(), RestResponse::NOT_FOUND_CODE); + return; + } + $groupResourceIds = array_merge($groupResourceIds, $groupTree->GetResourceIds($groupId)); + } + $groupResourceIds = array_unique($groupResourceIds); + $filteredResources = []; + foreach ($resources as $resource) { + if (in_array($resource->GetId(), $groupResourceIds)) { + $filteredResources[] = $resource; + } + } + $resources = $filteredResources; + } + $resourceIds = []; foreach ($resources as $resource) { $resourceIds[] = $resource->GetId(); diff --git a/docs/source/API.rst b/docs/source/API.rst index 94a56a6a6..2c34e6cf3 100644 --- a/docs/source/API.rst +++ b/docs/source/API.rst @@ -62,6 +62,12 @@ To combine multiple parameters:: /Web/Services/index.php/Reservations/?scheduleId=1&userId=5 +To filter resources by group:: + + /Web/Services/index.php/Resources/?groupId=1 + + /Web/Services/index.php/Resources/?groupId=1,2,3 + POST Requests ~~~~~~~~~~~~~ @@ -2167,6 +2173,13 @@ belonging to those schedules will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a ``400 Bad Request`` is returned. +Optional query string parameter: ``groupId``. One or more resource group IDs, +comma-separated (e.g. ``groupId=1,2,3``). If provided, only resources belonging +to those groups (including resources in sub-groups) will be returned. Each value +must be a positive integer (greater than zero); if any value is non-integer or +zero, a ``400 Bad Request`` is returned. If any group ID does not exist, a +``404 Not Found`` is returned. + **Route:** ``/Web/Services/index.php/Resources/`` *This service is secure and requires authentication* diff --git a/lib/WebService/WebServiceQueryStringKeys.php b/lib/WebService/WebServiceQueryStringKeys.php index 7d0760e6a..f604494db 100644 --- a/lib/WebService/WebServiceQueryStringKeys.php +++ b/lib/WebService/WebServiceQueryStringKeys.php @@ -6,6 +6,7 @@ class WebServiceQueryStringKeys public const DATE_TIME = 'dateTime'; public const START_DATE_TIME = 'startDateTime'; public const END_DATE_TIME = 'endDateTime'; + public const GROUP_ID = 'groupId'; public const RESOURCE_ID = 'resourceId'; public const SCHEDULE_ID = 'scheduleId'; public const UPDATE_SCOPE = 'updateScope'; diff --git a/tests/WebServices/ResourcesWebServiceTest.php b/tests/WebServices/ResourcesWebServiceTest.php index d37b7e133..a210166ed 100644 --- a/tests/WebServices/ResourcesWebServiceTest.php +++ b/tests/WebServices/ResourcesWebServiceTest.php @@ -230,6 +230,255 @@ public function testReturns400ForZeroScheduleIdAlone() ); } + private function buildGroupTree(int $groupId, array $resourceIds): ResourceGroupTree + { + $tree = new ResourceGroupTree(); + $tree->AddGroup(new ResourceGroup($groupId, "Group $groupId")); + foreach ($resourceIds as $resourceId) { + $tree->AddAssignment(new ResourceGroupAssignment( + group_id: $groupId, + resource_name: "Resource $resourceId", + resource_id: $resourceId, + resourceAdminGroupId: null, + scheduleId: 1, + statusId: ResourceStatus::AVAILABLE, + scheduleAdminGroupId: null, + requiresApproval: false, + isCheckInEnabled: false, + isAutoReleased: false, + autoReleaseMinutes: null, + minLength: null, + resourceTypeId: null, + color: null, + maxConcurrentReservations: null + )); + } + return $tree; + } + + public function testFiltersResourceListByGroupId() + { + $groupId = 3; + $matchingResourceId = 123; + $otherResourceId = 456; + + $matchingResource = new FakeBookableResource($matchingResourceId); + $otherResource = new FakeBookableResource($otherResourceId); + + $this->server->SetQueryString(WebServiceQueryStringKeys::GROUP_ID, $groupId); + + $this->repository->expects($this->once()) + ->method('GetUserResourceList') + ->willReturn([$matchingResource, $otherResource]); + + $groupTree = $this->buildGroupTree($groupId, [$matchingResourceId]); + $this->repository->expects($this->once()) + ->method('GetResourceGroups') + ->willReturn($groupTree); + + $attributes = new AttributeList(); + $this->attributeService->expects($this->once()) + ->method('GetAttributes') + ->with( + $this->equalTo(CustomAttributeCategory::RESOURCE), + $this->equalTo([$matchingResourceId]) + ) + ->willReturn($attributes); + + $this->service->GetAll(); + + $this->assertEquals( + new ResourcesResponse($this->server, [$matchingResource], $attributes), + $this->server->_LastResponse + ); + } + + public function testFiltersResourceListByMultipleGroupIds() + { + $groupId1 = 3; + $groupId2 = 7; + $resourceId1 = 123; + $resourceId2 = 456; + $otherResourceId = 789; + + $resource1 = new FakeBookableResource($resourceId1); + $resource2 = new FakeBookableResource($resourceId2); + $otherResource = new FakeBookableResource($otherResourceId); + + $this->server->SetQueryString(WebServiceQueryStringKeys::GROUP_ID, "$groupId1,$groupId2"); + + $this->repository->expects($this->once()) + ->method('GetUserResourceList') + ->willReturn([$resource1, $resource2, $otherResource]); + + $tree = new ResourceGroupTree(); + $tree->AddGroup(new ResourceGroup($groupId1, "Group $groupId1")); + $tree->AddGroup(new ResourceGroup($groupId2, "Group $groupId2")); + $tree->AddAssignment(new ResourceGroupAssignment( + group_id: $groupId1, + resource_name: "Resource $resourceId1", + resource_id: $resourceId1, + resourceAdminGroupId: null, + scheduleId: 1, + statusId: ResourceStatus::AVAILABLE, + scheduleAdminGroupId: null, + requiresApproval: false, + isCheckInEnabled: false, + isAutoReleased: false, + autoReleaseMinutes: null, + minLength: null, + resourceTypeId: null, + color: null, + maxConcurrentReservations: null + )); + $tree->AddAssignment(new ResourceGroupAssignment( + group_id: $groupId2, + resource_name: "Resource $resourceId2", + resource_id: $resourceId2, + resourceAdminGroupId: null, + scheduleId: 1, + statusId: ResourceStatus::AVAILABLE, + scheduleAdminGroupId: null, + requiresApproval: false, + isCheckInEnabled: false, + isAutoReleased: false, + autoReleaseMinutes: null, + minLength: null, + resourceTypeId: null, + color: null, + maxConcurrentReservations: null + )); + + $this->repository->expects($this->once()) + ->method('GetResourceGroups') + ->willReturn($tree); + + $attributes = new AttributeList(); + $this->attributeService->expects($this->once()) + ->method('GetAttributes') + ->with( + $this->equalTo(CustomAttributeCategory::RESOURCE), + $this->equalTo([$resourceId1, $resourceId2]) + ) + ->willReturn($attributes); + + $this->service->GetAll(); + + $this->assertEquals( + new ResourcesResponse($this->server, [$resource1, $resource2], $attributes), + $this->server->_LastResponse + ); + } + + public function testFiltersResourceListByGroupIdIncludesSubGroups() + { + $parentGroupId = 3; + $childGroupId = 5; + $matchingResourceId = 123; + $otherResourceId = 456; + + $matchingResource = new FakeBookableResource($matchingResourceId); + $otherResource = new FakeBookableResource($otherResourceId); + + $this->server->SetQueryString(WebServiceQueryStringKeys::GROUP_ID, $parentGroupId); + + $this->repository->expects($this->once()) + ->method('GetUserResourceList') + ->willReturn([$matchingResource, $otherResource]); + + $tree = new ResourceGroupTree(); + $tree->AddGroup(new ResourceGroup($parentGroupId, 'Parent')); + $tree->AddGroup(new ResourceGroup($childGroupId, 'Child', $parentGroupId)); + $tree->AddAssignment(new ResourceGroupAssignment( + group_id: $childGroupId, + resource_name: "Resource $matchingResourceId", + resource_id: $matchingResourceId, + resourceAdminGroupId: null, + scheduleId: 1, + statusId: ResourceStatus::AVAILABLE, + scheduleAdminGroupId: null, + requiresApproval: false, + isCheckInEnabled: false, + isAutoReleased: false, + autoReleaseMinutes: null, + minLength: null, + resourceTypeId: null, + color: null, + maxConcurrentReservations: null + )); + + $this->repository->expects($this->once()) + ->method('GetResourceGroups') + ->willReturn($tree); + + $attributes = new AttributeList(); + $this->attributeService->expects($this->once()) + ->method('GetAttributes') + ->with( + $this->equalTo(CustomAttributeCategory::RESOURCE), + $this->equalTo([$matchingResourceId]) + ) + ->willReturn($attributes); + + $this->service->GetAll(); + + $this->assertEquals( + new ResourcesResponse($this->server, [$matchingResource], $attributes), + $this->server->_LastResponse + ); + } + + public function testReturns400ForNonIntegerGroupId() + { + $this->server->SetQueryString(WebServiceQueryStringKeys::GROUP_ID, '1,abc,2'); + + $this->repository->expects($this->never()) + ->method('GetUserResourceList'); + + $this->service->GetAll(); + + $this->assertEquals(RestResponse::BAD_REQUEST_CODE, $this->server->_LastResponseCode); + $this->assertEquals( + RestResponse::BadRequest("Invalid groupId 'abc': must be a positive integer"), + $this->server->_LastResponse + ); + } + + public function testReturns400ForZeroGroupId() + { + $this->server->SetQueryString(WebServiceQueryStringKeys::GROUP_ID, '0'); + + $this->repository->expects($this->never()) + ->method('GetUserResourceList'); + + $this->service->GetAll(); + + $this->assertEquals(RestResponse::BAD_REQUEST_CODE, $this->server->_LastResponseCode); + $this->assertEquals( + RestResponse::BadRequest("Invalid groupId '0': must be a positive integer"), + $this->server->_LastResponse + ); + } + + public function testReturns404ForNonExistentGroupId() + { + $this->server->SetQueryString(WebServiceQueryStringKeys::GROUP_ID, '999'); + + $this->repository->expects($this->once()) + ->method('GetUserResourceList') + ->willReturn([]); + + $groupTree = new ResourceGroupTree(); + $this->repository->expects($this->once()) + ->method('GetResourceGroups') + ->willReturn($groupTree); + + $this->service->GetAll(); + + $this->assertEquals(RestResponse::NOT_FOUND_CODE, $this->server->_LastResponseCode); + $this->assertEquals(RestResponse::NotFound(), $this->server->_LastResponse); + } + public function testGetsStatuses() { $this->service->GetStatuses(); From 1e58e1d4c3c6189212da1db18729c01476b6cadb Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Thu, 9 Apr 2026 13:00:09 -0700 Subject: [PATCH 2/3] refactor(api): Extract helper to parse comma-separated positive integer IDs Replace duplicated query string parsing logic in GetAll() with a shared private method parsePositiveIntegerIds(). Both the scheduleId and groupId filters now delegate validation and parsing to this helper. The helper returns null (param absent), false (invalid value, 400 already written), or int[] (valid IDs), allowing callers to early-return cleanly. --- WebServices/ResourcesWebService.php | 61 ++++++++++++++++------------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/WebServices/ResourcesWebService.php b/WebServices/ResourcesWebService.php index 0caaa439e..db5c1c7f7 100644 --- a/WebServices/ResourcesWebService.php +++ b/WebServices/ResourcesWebService.php @@ -58,36 +58,14 @@ public function __construct( */ public function GetAll() { - $scheduleIds = null; - $scheduleIdParam = $this->server->GetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID); - if ($scheduleIdParam !== null && $scheduleIdParam !== '') { - $rawIds = explode(',', $scheduleIdParam); - foreach ($rawIds as $id) { - if (!ctype_digit($id) || (int)$id === 0) { - $this->server->WriteResponse( - RestResponse::BadRequest("Invalid scheduleId '$id': must be a positive integer"), - RestResponse::BAD_REQUEST_CODE - ); - return; - } - } - $scheduleIds = array_map('intval', $rawIds); + $scheduleIds = $this->parsePositiveIntegerIds(paramName: WebServiceQueryStringKeys::SCHEDULE_ID); + if ($scheduleIds === false) { + return; } - $groupIds = null; - $groupIdParam = $this->server->GetQueryString(WebServiceQueryStringKeys::GROUP_ID); - if ($groupIdParam !== null && $groupIdParam !== '') { - $rawIds = explode(',', $groupIdParam); - foreach ($rawIds as $id) { - if (!ctype_digit($id) || (int)$id === 0) { - $this->server->WriteResponse( - RestResponse::BadRequest("Invalid groupId '$id': must be a positive integer"), - RestResponse::BAD_REQUEST_CODE - ); - return; - } - } - $groupIds = array_map('intval', $rawIds); + $groupIds = $this->parsePositiveIntegerIds(paramName: WebServiceQueryStringKeys::GROUP_ID); + if ($groupIds === false) { + return; } $resources = $this->resourceRepository->GetUserResourceList(); @@ -262,6 +240,33 @@ public function GetGroups() $this->server->WriteResponse(new ResourceGroupsResponse($groups)); } + /** + * Parses a comma-separated list of positive integers from a query string parameter. + * + * @return int[]|false|null int[] on success, null if param absent/empty, false if invalid + * (a 400 response has already been written) + */ + private function parsePositiveIntegerIds(string $paramName): array|false|null + { + $param = $this->server->GetQueryString($paramName); + if ($param === null || $param === '') { + return null; + } + + $rawIds = explode(',', $param); + foreach ($rawIds as $id) { + if (!ctype_digit($id) || (int)$id === 0) { + $this->server->WriteResponse( + RestResponse::BadRequest("Invalid $paramName '$id': must be a positive integer"), + RestResponse::BAD_REQUEST_CODE + ); + return false; + } + } + + return array_map('intval', $rawIds); + } + /** * @param BookableResource $resource * @param ReservationItemView[][] $reservations From ccb5d3db2befe97c8676de12d8b74419ccbcb46e Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Thu, 9 Apr 2026 17:12:57 -0700 Subject: [PATCH 3/3] feat(api): Return 404 for non-existent scheduleId in GET Resources endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an existence check for each scheduleId filter value against the schedule repository before fetching resources. If any requested schedule ID does not exist, return 404 Not Found — consistent with the existing behaviour of the groupId filter. Inject IScheduleRepository into ResourcesWebService and convert the constructor to use constructor property promotion to remove manual assignment boilerplate. Update the @description docblock, API.rst, and existing schedule filter tests to stub LoadById(). Add testReturns404ForNonExistentScheduleId. --- Web/Services/index.php | 8 +- WebServices/ResourcesWebService.php | 52 +++++------ docs/source/API.rst | 3 +- tests/WebServices/ResourcesWebServiceTest.php | 90 ++++++++++++++++++- tests/fakes/FakeResource.php | 4 +- 5 files changed, 121 insertions(+), 36 deletions(-) diff --git a/Web/Services/index.php b/Web/Services/index.php index 4460489aa..04298ab7f 100644 --- a/Web/Services/index.php +++ b/Web/Services/index.php @@ -152,7 +152,13 @@ function RegisterResources(SlimServer $server, SlimWebServiceRegistry $registry) { $resourceRepository = new ResourceRepository(); $attributeService = new AttributeService(new AttributeRepository()); - $webService = new ResourcesWebService($server, $resourceRepository, $attributeService, new ReservationViewRepository()); + $webService = new ResourcesWebService( + server: $server, + resourceRepository: $resourceRepository, + attributeService: $attributeService, + reservationRepository: new ReservationViewRepository(), + scheduleRepository: new ScheduleRepository() + ); $writeWebService = new ResourcesWriteWebService($server, new ResourceSaveController($resourceRepository, new ResourceRequestValidator($attributeService))); $roGroupId = GetConfigGroup(ConfigKeys::API_RESOURCES_RO_GROUP); diff --git a/WebServices/ResourcesWebService.php b/WebServices/ResourcesWebService.php index db5c1c7f7..a1d224187 100644 --- a/WebServices/ResourcesWebService.php +++ b/WebServices/ResourcesWebService.php @@ -16,42 +16,19 @@ class ResourcesWebService { - /** - * @var IRestServer - */ - private $server; - - /** - * @var IResourceRepository - */ - private $resourceRepository; - - /** - * @var IAttributeService - */ - private $attributeService; - - /** - * @var IReservationViewRepository - */ - private $reservationRepository; - public function __construct( - IRestServer $server, - IResourceRepository $resourceRepository, - IAttributeService $attributeService, - IReservationViewRepository $reservationRepository + private IRestServer $server, + private IResourceRepository $resourceRepository, + private IAttributeService $attributeService, + private IReservationViewRepository $reservationRepository, + private IScheduleRepository $scheduleRepository ) { - $this->server = $server; - $this->resourceRepository = $resourceRepository; - $this->attributeService = $attributeService; - $this->reservationRepository = $reservationRepository; } /** * @name GetAllResources * @description Loads all resources - * Optional query string parameter: scheduleId. One or more schedule IDs, comma-separated (e.g. scheduleId=1,2,3). If provided, only resources belonging to those schedules will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a 400 Bad Request is returned. + * Optional query string parameter: scheduleId. One or more schedule IDs, comma-separated (e.g. scheduleId=1,2,3). If provided, only resources belonging to those schedules will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a 400 Bad Request is returned. If any schedule ID does not exist, a 404 Not Found is returned. * Optional query string parameter: groupId. One or more resource group IDs, comma-separated (e.g. groupId=1,2,3). If provided, only resources belonging to those groups (including sub-groups) will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a 400 Bad Request is returned. If any group ID does not exist, a 404 Not Found is returned. * @response ResourcesResponse * @return void @@ -68,6 +45,18 @@ public function GetAll() return; } + if ($scheduleIds !== null) { + foreach ($scheduleIds as $scheduleId) { + if ($this->scheduleRepository->LoadById($scheduleId) === null) { + $this->server->WriteResponse( + restResponse: RestResponse::NotFound(), + statusCode: RestResponse::NOT_FOUND_CODE + ); + return; + } + } + } + $resources = $this->resourceRepository->GetUserResourceList(); if ($scheduleIds !== null) { @@ -81,6 +70,7 @@ public function GetAll() } if ($groupIds !== null) { + // GetResourceGroups() returns the full tree, used for both existence checks and sub-group traversal. $groupTree = $this->resourceRepository->GetResourceGroups(); $groupList = $groupTree->GetGroupList(); $groupResourceIds = []; @@ -89,6 +79,7 @@ public function GetAll() $this->server->WriteResponse(RestResponse::NotFound(), RestResponse::NOT_FOUND_CODE); return; } + // GetResourceIds() recurses into sub-groups. $groupResourceIds = array_merge($groupResourceIds, $groupTree->GetResourceIds($groupId)); } $groupResourceIds = array_unique($groupResourceIds); @@ -242,6 +233,7 @@ public function GetGroups() /** * Parses a comma-separated list of positive integers from a query string parameter. + * Duplicate values are removed; the returned array is unique and re-indexed. * * @return int[]|false|null int[] on success, null if param absent/empty, false if invalid * (a 400 response has already been written) @@ -264,7 +256,7 @@ private function parsePositiveIntegerIds(string $paramName): array|false|null } } - return array_map('intval', $rawIds); + return array_values(array_unique(array_map('intval', $rawIds))); } /** diff --git a/docs/source/API.rst b/docs/source/API.rst index 2c34e6cf3..56aeb7164 100644 --- a/docs/source/API.rst +++ b/docs/source/API.rst @@ -2171,7 +2171,8 @@ Optional query string parameter: ``scheduleId``. One or more schedule IDs, comma-separated (e.g. ``scheduleId=1,2,3``). If provided, only resources belonging to those schedules will be returned. Each value must be a positive integer (greater than zero); if any value is non-integer or zero, a ``400 Bad -Request`` is returned. +Request`` is returned. If any schedule ID does not exist, a ``404 Not Found`` +is returned. Optional query string parameter: ``groupId``. One or more resource group IDs, comma-separated (e.g. ``groupId=1,2,3``). If provided, only resources belonging diff --git a/tests/WebServices/ResourcesWebServiceTest.php b/tests/WebServices/ResourcesWebServiceTest.php index a210166ed..facfd4089 100644 --- a/tests/WebServices/ResourcesWebServiceTest.php +++ b/tests/WebServices/ResourcesWebServiceTest.php @@ -17,6 +17,8 @@ class ResourcesWebServiceTest extends TestBase private IAttributeService&\PHPUnit\Framework\MockObject\MockObject $attributeService; + private IScheduleRepository&\PHPUnit\Framework\MockObject\MockObject $scheduleRepository; + /** * @var ResourcesWebService */ @@ -30,8 +32,15 @@ public function setUp(): void $this->repository = $this->createMock('IResourceRepository'); $this->reservationRepository = $this->createMock('IReservationViewRepository'); $this->attributeService = $this->createMock('IAttributeService'); - - $this->service = new ResourcesWebService($this->server, $this->repository, $this->attributeService, $this->reservationRepository); + $this->scheduleRepository = $this->createMock('IScheduleRepository'); + + $this->service = new ResourcesWebService( + server: $this->server, + resourceRepository: $this->repository, + attributeService: $this->attributeService, + reservationRepository: $this->reservationRepository, + scheduleRepository: $this->scheduleRepository + ); } public function testGetsResourceById() @@ -121,6 +130,11 @@ public function testFiltersResourceListByScheduleId() $this->server->SetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID, $scheduleId); + $this->scheduleRepository->expects($this->once()) + ->method('LoadById') + ->with($scheduleId) + ->willReturn(new Schedule(id: $scheduleId, name: null, isDefault: false, weekdayStart: null, daysVisible: null)); + $this->repository->expects($this->once()) ->method('GetUserResourceList') ->willReturn([$matchingResource, $otherResource]); @@ -161,6 +175,13 @@ public function testFiltersResourceListByMultipleScheduleIds() $this->server->SetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID, "$scheduleId1,$scheduleId2"); + $this->scheduleRepository->expects($this->exactly(2)) + ->method('LoadById') + ->willReturnMap([ + [$scheduleId1, new Schedule(id: $scheduleId1, name: null, isDefault: false, weekdayStart: null, daysVisible: null)], + [$scheduleId2, new Schedule(id: $scheduleId2, name: null, isDefault: false, weekdayStart: null, daysVisible: null)], + ]); + $this->repository->expects($this->once()) ->method('GetUserResourceList') ->willReturn([$resource1, $resource2, $otherResource]); @@ -182,6 +203,71 @@ public function testFiltersResourceListByMultipleScheduleIds() ); } + public function testReturns404ForNonExistentScheduleId() + { + $this->server->SetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID, '999'); + + $this->scheduleRepository->expects($this->once()) + ->method('LoadById') + ->with(999) + ->willReturn(null); + + $this->repository->expects($this->never()) + ->method('GetUserResourceList'); + + $this->service->GetAll(); + + $this->assertEquals(RestResponse::NOT_FOUND_CODE, $this->server->_LastResponseCode); + $this->assertEquals(RestResponse::NotFound(), $this->server->_LastResponse); + } + + public function testReturns404WhenOneOfMultipleScheduleIdsDoesNotExist() + { + // ID 1 exists, ID 999 does not — the endpoint must return 404 regardless. + $this->server->SetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID, '1,999'); + + $this->scheduleRepository->expects($this->exactly(2)) + ->method('LoadById') + ->willReturnMap([ + [1, new Schedule(id: 1, name: null, isDefault: false, weekdayStart: null, daysVisible: null)], + [999, null], + ]); + + $this->repository->expects($this->never()) + ->method('GetUserResourceList'); + + $this->service->GetAll(); + + $this->assertEquals(RestResponse::NOT_FOUND_CODE, $this->server->_LastResponseCode); + $this->assertEquals(RestResponse::NotFound(), $this->server->_LastResponse); + } + + public function testDeduplicatesScheduleIds() + { + $scheduleId = 1; + $resource = new FakeBookableResource(resourceId: 1, name: 'resource-1'); + $resource->SetScheduleId($scheduleId); + + // Three duplicate IDs — parsePositiveIntegerIds should reduce these to one. + $this->server->SetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID, '1,1,1'); + + // The core assertion: LoadById must be called exactly once, not three times, + // proving de-duplication happened before the existence-check loop. + $this->scheduleRepository->expects($this->once()) + ->method('LoadById') + ->with($scheduleId) + ->willReturn(new Schedule(id: $scheduleId, name: null, isDefault: false, weekdayStart: null, daysVisible: null)); + + $this->repository->expects($this->once()) + ->method('GetUserResourceList') + ->willReturn([$resource]); + + // Stub with an empty AttributeList so the response can be built; content is not under test here. + $this->attributeService->method('GetAttributes')->willReturn(new AttributeList()); + + $this->service->GetAll(); + } + public function testReturns400ForNonIntegerScheduleId() { $this->server->SetQueryString(WebServiceQueryStringKeys::SCHEDULE_ID, '1,abc,2'); diff --git a/tests/fakes/FakeResource.php b/tests/fakes/FakeResource.php index 936380310..e2a9c31b0 100644 --- a/tests/fakes/FakeResource.php +++ b/tests/fakes/FakeResource.php @@ -4,9 +4,9 @@ class FakeBookableResource extends BookableResource { - public function __construct($id, $name = null) + public function __construct($resourceId, $name = null) { - $this->_resourceId = $id; + $this->_resourceId = $resourceId; $this->_name = $name; }