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 7da9e2f12..a1d224187 100644 --- a/WebServices/ResourcesWebService.php +++ b/WebServices/ResourcesWebService.php @@ -16,61 +16,45 @@ 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 */ 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) { + $scheduleIds = $this->parsePositiveIntegerIds(paramName: WebServiceQueryStringKeys::SCHEDULE_ID); + if ($scheduleIds === false) { + return; + } + + $groupIds = $this->parsePositiveIntegerIds(paramName: WebServiceQueryStringKeys::GROUP_ID); + if ($groupIds === false) { + return; + } + + if ($scheduleIds !== null) { + foreach ($scheduleIds as $scheduleId) { + if ($this->scheduleRepository->LoadById($scheduleId) === null) { $this->server->WriteResponse( - RestResponse::BadRequest("Invalid scheduleId '$id': must be a positive integer"), - RestResponse::BAD_REQUEST_CODE + restResponse: RestResponse::NotFound(), + statusCode: RestResponse::NOT_FOUND_CODE ); return; } } - $scheduleIds = array_map('intval', $rawIds); } $resources = $this->resourceRepository->GetUserResourceList(); @@ -85,6 +69,29 @@ public function GetAll() $resources = $filteredResources; } + 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 = []; + foreach ($groupIds as $groupId) { + if (!array_key_exists($groupId, $groupList)) { + $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); + $filteredResources = []; + foreach ($resources as $resource) { + if (in_array($resource->GetId(), $groupResourceIds)) { + $filteredResources[] = $resource; + } + } + $resources = $filteredResources; + } + $resourceIds = []; foreach ($resources as $resource) { $resourceIds[] = $resource->GetId(); @@ -224,6 +231,34 @@ public function GetGroups() $this->server->WriteResponse(new ResourceGroupsResponse($groups)); } + /** + * 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) + */ + 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_values(array_unique(array_map('intval', $rawIds))); + } + /** * @param BookableResource $resource * @param ReservationItemView[][] $reservations diff --git a/docs/source/API.rst b/docs/source/API.rst index 94a56a6a6..56aeb7164 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 ~~~~~~~~~~~~~ @@ -2165,7 +2171,15 @@ 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 +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/`` 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..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'); @@ -230,6 +316,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(); 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; }