From a8f5299ba3cdb162962261d44a04d17e3d545946 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Mon, 20 Jan 2025 17:31:25 +0530 Subject: [PATCH 01/24] Issue #3500052: Provide API to list available pages. --- experience_builder.routing.yml | 6 + src/Controller/XbApiPageListController.php | 41 ++++++ .../XbRouteOptionsEventSubscriber.php | 8 ++ .../XBApiPageListControllerTest.php | 131 ++++++++++++++++++ 4 files changed, 186 insertions(+) create mode 100644 src/Controller/XbApiPageListController.php create mode 100644 tests/src/Kernel/Controller/XBApiPageListControllerTest.php diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index d508fe8722..8973a21dea 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -70,6 +70,12 @@ entity.component.delete_form: requirements: _entity_access: 'component.delete' +experience_builder.xb_page_list: + path: '/api/page-list' + defaults: + _controller: 'Drupal\experience_builder\Controller\XbApiPageListController' +# Dynamic requirement is added in the alter route event subscriber. + experience_builder.api.component_inputs_form: path: '/xb-field-form/{entity_type}/{entity}' defaults: diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php new file mode 100644 index 0000000000..180aef2988 --- /dev/null +++ b/src/Controller/XbApiPageListController.php @@ -0,0 +1,41 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\experience_builder\Controller; + +use Drupal\Core\Cache\CacheableJsonResponse; +use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\Controller\ControllerBase; + +final class XbApiPageListController extends ControllerBase { + + /** + * Returns a list of pages that user has access to. + */ + public function __invoke(): CacheableJsonResponse { + $entity_query = $this->entityTypeManager()->getStorage('xb_page')->getQuery(); + $page_ids = $entity_query->accessCheck()->execute(); + /** @var \Drupal\experience_builder\Entity\Page[] $pages */ + $pages = $this->entityTypeManager()->getStorage('xb_page')->loadMultiple($page_ids); + $page_list = []; + $list_cache_tags = []; + foreach ($pages as $page) { + $list_cache_tags = $page->getEntityType()->getListCacheTags(); + if (!$page->access('update')) { + continue; + } + $page_list[] = [ + 'id' => $page->id(), + 'title' => $page->label(), + 'status' => (bool) $page->get('status')->value, + ]; + } + $cache = new CacheableMetadata(); + $cache->addCacheTags($list_cache_tags); + $json_response = new CacheableJsonResponse(['data' => $page_list]); + $json_response->addCacheableDependency($cache); + return $json_response; + } + +} diff --git a/src/EventSubscriber/XbRouteOptionsEventSubscriber.php b/src/EventSubscriber/XbRouteOptionsEventSubscriber.php index 987dde68e3..bd4b3566eb 100644 --- a/src/EventSubscriber/XbRouteOptionsEventSubscriber.php +++ b/src/EventSubscriber/XbRouteOptionsEventSubscriber.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Drupal\experience_builder\EventSubscriber; +use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\EventSubscriber\MainContentViewSubscriber; use Drupal\Core\Routing\RouteBuildEvent; use Drupal\Core\Routing\RouteMatchInterface; @@ -21,6 +22,7 @@ final class XbRouteOptionsEventSubscriber implements EventSubscriberInterface { public function __construct( private readonly RouteMatchInterface $routeMatch, + private readonly EntityTypeManagerInterface $entityTypeManager, ) {} public function transformWrapperFormatRouteOption(RequestEvent $event): void { @@ -50,6 +52,12 @@ final class XbRouteOptionsEventSubscriber implements EventSubscriberInterface { $route->setRequirement('_csrf_request_header_token', 'TRUE'); } } + // Add collection permission of xb_page entity type + // to page list route. + if ($name === 'experience_builder.xb_page_list') { + $collection_permission = $this->entityTypeManager->getDefinition('xb_page')?->getCollectionPermission(); + $route->setRequirement('_permission', $collection_permission); + } } } diff --git a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php new file mode 100644 index 0000000000..0a2646cb68 --- /dev/null +++ b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php @@ -0,0 +1,131 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\experience_builder\Kernel\Controller; + +use Drupal\experience_builder\Entity\Page; +use Drupal\KernelTests\KernelTestBase; +use Drupal\Tests\experience_builder\Kernel\Traits\PageTrait; +use Drupal\Tests\user\Traits\UserCreationTrait; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\HttpKernel\TerminableInterface; + +/** + * Tests tha api page list controller. + * + * @coversDefaultClass \Drupal\experience_builder\Controller\XBApiPageListController + * + * @group experience_builder + */ +class XBApiPageListControllerTest extends KernelTestBase { + + use PageTrait; + use UserCreationTrait; + + /** + * {@inheritdoc} + */ + protected static $modules = [ + 'experience_builder', + 'system', + 'image', + ...self::PAGE_TEST_MODULES, + ]; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + $this->installEntitySchema('user'); + // Create 3 xb_page entities. + $this->installPageEntitySchema(); + Page::create([ + 'title' => 'Page 1', + 'path' => ['alias' => '/page-1'], + 'status' => TRUE, + ])->save(); + Page::create([ + 'title' => 'Page 2', + 'path' => ['alias' => '/page-2'], + 'status' => FALSE, + ])->save(); + Page::create([ + 'title' => 'Page 3', + 'path' => ['alias' => '/page-3'], + 'status' => TRUE, + ])->save(); + // Create super user so that it doesn't interfere with the test. + $this->createUser(); + } + + /** + * Tests the API response. + */ + public function testApiResponse(): void { + /** @var \Drupal\user\UserInterface $allowed_user */ + $allowed_user = $this->createUser(['administer xb_page']); + $this->setCurrentUser($allowed_user); + $response = $this->doRequest(Request::create('/api/page-list')); + /** @var string $content */ + $content = $response->getContent(); + $data = \json_decode($content, TRUE); + self::assertCount(3, $data['data']); + self::assertEquals('Page 1', $data['data'][0]['title']); + self::assertEquals(1, $data['data'][0]['id']); + self::assertTrue($data['data'][0]['status']); + self::assertEquals('Page 2', $data['data'][1]['title']); + self::assertEquals(2, $data['data'][1]['id']); + self::assertFalse($data['data'][1]['status']); + self::assertEquals('Page 3', $data['data'][2]['title']); + self::assertEquals(3, $data['data'][2]['id']); + self::assertTrue($data['data'][2]['status']); + } + + public function testApiResponseWithoutPermission(): void { + /** @var \Drupal\user\UserInterface $allowed_user */ + $allowed_user = $this->createUser(['access content'], 'access_content_user'); + $this->setCurrentUser($allowed_user); + $response = $this->doRequest(Request::create('/api/page-list')); + self::assertEquals(Response::HTTP_FORBIDDEN, $response->getStatusCode()); + } + + /** + * Passes a request to the HTTP kernel and returns a response. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request. + * @param bool $terminate + * To handle request termination. + * + * @return \Symfony\Component\HttpFoundation\Response + * The response. + * + * @throws \Exception + */ + protected function doRequest(Request $request, bool $terminate = TRUE): Response { + $http_kernel = $this->container->get('http_kernel'); + self::assertInstanceOf( + HttpKernelInterface::class, + $http_kernel + ); + $response = $http_kernel->handle($request); + $content = $response->getContent(); + self::assertNotFalse($content); + $this->setRawContent($content); + + if ($terminate) { + self::assertInstanceOf( + TerminableInterface::class, + $http_kernel + ); + $http_kernel->terminate($request, $response); + } + + return $response; + } + +} -- GitLab From 2551d6dbd8237237cc36badf5147d8e0a499452b Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Mon, 20 Jan 2025 18:44:56 +0530 Subject: [PATCH 02/24] Add path to the api response. --- src/Controller/XbApiPageListController.php | 3 ++- tests/src/Kernel/Controller/XBApiPageListControllerTest.php | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php index 180aef2988..9e16bbf68e 100644 --- a/src/Controller/XbApiPageListController.php +++ b/src/Controller/XbApiPageListController.php @@ -26,9 +26,10 @@ final class XbApiPageListController extends ControllerBase { continue; } $page_list[] = [ - 'id' => $page->id(), + 'id' => (int) $page->id(), 'title' => $page->label(), 'status' => (bool) $page->get('status')->value, + 'path' => $page->toUrl()->toString() ?? '', ]; } $cache = new CacheableMetadata(); diff --git a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php index 0a2646cb68..d9bb01b994 100644 --- a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php +++ b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php @@ -76,13 +76,16 @@ class XBApiPageListControllerTest extends KernelTestBase { self::assertCount(3, $data['data']); self::assertEquals('Page 1', $data['data'][0]['title']); self::assertEquals(1, $data['data'][0]['id']); + self::assertEquals('/page-1', $data['data'][0]['path']); self::assertTrue($data['data'][0]['status']); self::assertEquals('Page 2', $data['data'][1]['title']); self::assertEquals(2, $data['data'][1]['id']); self::assertFalse($data['data'][1]['status']); + self::assertEquals('/page-2', $data['data'][1]['path']); self::assertEquals('Page 3', $data['data'][2]['title']); self::assertEquals(3, $data['data'][2]['id']); self::assertTrue($data['data'][2]['status']); + self::assertEquals('/page-3', $data['data'][2]['path']); } public function testApiResponseWithoutPermission(): void { -- GitLab From 9de39bc9d221716d84ab4bf64f85f28ac6e76cf3 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Tue, 21 Jan 2025 19:19:04 +0530 Subject: [PATCH 03/24] PR comments. --- experience_builder.routing.yml | 4 +- experience_builder.services.yml | 3 +- src/Controller/XbApiPageListController.php | 42 +++---- ...scriber.php => XbRouteEventSubscriber.php} | 15 +-- .../XBApiPageListControllerTest.php | 104 +++++++----------- 5 files changed, 74 insertions(+), 94 deletions(-) rename src/EventSubscriber/{XbRouteOptionsEventSubscriber.php => XbRouteEventSubscriber.php} (83%) diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index 8973a21dea..945b677f49 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -70,8 +70,8 @@ entity.component.delete_form: requirements: _entity_access: 'component.delete' -experience_builder.xb_page_list: - path: '/api/page-list' +experience_builder.api.xb_page: + path: '/xb/api/page' defaults: _controller: 'Drupal\experience_builder\Controller\XbApiPageListController' # Dynamic requirement is added in the alter route event subscriber. diff --git a/experience_builder.services.yml b/experience_builder.services.yml index 6cd42453ad..b1406d1626 100644 --- a/experience_builder.services.yml +++ b/experience_builder.services.yml @@ -66,7 +66,7 @@ services: tags: - { name: event_subscriber } experience_builder.route_options.subscriber: - class: Drupal\experience_builder\EventSubscriber\XbRouteOptionsEventSubscriber + class: Drupal\experience_builder\EventSubscriber\XbRouteEventSubscriber tags: - { name: 'event_subscriber' } Drupal\experience_builder\EventSubscriber\PreviewEnvelopeViewSubscriber: {} @@ -102,3 +102,4 @@ services: Drupal\experience_builder\Controller\ExperienceBuilderController: {} Drupal\experience_builder\Controller\ApiPublishAllController: {} Drupal\experience_builder\Controller\ApiPendingChangesController: {} + Drupal\experience_builder\Controller\XbApiPageListController: {} diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php index 9e16bbf68e..aa227dd19e 100644 --- a/src/Controller/XbApiPageListController.php +++ b/src/Controller/XbApiPageListController.php @@ -6,35 +6,35 @@ namespace Drupal\experience_builder\Controller; use Drupal\Core\Cache\CacheableJsonResponse; use Drupal\Core\Cache\CacheableMetadata; -use Drupal\Core\Controller\ControllerBase; +use Drupal\Core\Entity\EntityTypeManagerInterface; -final class XbApiPageListController extends ControllerBase { +final readonly class XbApiPageListController { + + public function __construct(private EntityTypeManagerInterface $entityTypeManager) { + } - /** - * Returns a list of pages that user has access to. - */ public function __invoke(): CacheableJsonResponse { - $entity_query = $this->entityTypeManager()->getStorage('xb_page')->getQuery(); - $page_ids = $entity_query->accessCheck()->execute(); - /** @var \Drupal\experience_builder\Entity\Page[] $pages */ - $pages = $this->entityTypeManager()->getStorage('xb_page')->loadMultiple($page_ids); - $page_list = []; - $list_cache_tags = []; - foreach ($pages as $page) { - $list_cache_tags = $page->getEntityType()->getListCacheTags(); - if (!$page->access('update')) { + $storage = $this->entityTypeManager->getStorage('xb_page'); + $xb_page_ids = $storage->getQuery()->accessCheck()->execute(); + /** @var \Drupal\experience_builder\Entity\Page[] $xb_pages */ + $xb_pages = $this->entityTypeManager->getStorage('xb_page')->loadMultiple($xb_page_ids); + $xb_page_list = []; + foreach ($xb_pages as $xb_page) { + if (!$xb_page->access('update')) { continue; } - $page_list[] = [ - 'id' => (int) $page->id(), - 'title' => $page->label(), - 'status' => (bool) $page->get('status')->value, - 'path' => $page->toUrl()->toString() ?? '', + $xb_page_list[] = [ + 'id' => (int) $xb_page->id(), + 'title' => $xb_page->label(), + 'status' => $xb_page->isPublished(), + 'path' => $xb_page->toUrl()->toString(), ]; } $cache = new CacheableMetadata(); - $cache->addCacheTags($list_cache_tags); - $json_response = new CacheableJsonResponse(['data' => $page_list]); + $cache->addCacheTags($storage->getEntityType()->getListCacheTags()); + $json_response = new CacheableJsonResponse([ + 'data' => $xb_page_list, + ]); $json_response->addCacheableDependency($cache); return $json_response; } diff --git a/src/EventSubscriber/XbRouteOptionsEventSubscriber.php b/src/EventSubscriber/XbRouteEventSubscriber.php similarity index 83% rename from src/EventSubscriber/XbRouteOptionsEventSubscriber.php rename to src/EventSubscriber/XbRouteEventSubscriber.php index bd4b3566eb..4302e863a5 100644 --- a/src/EventSubscriber/XbRouteOptionsEventSubscriber.php +++ b/src/EventSubscriber/XbRouteEventSubscriber.php @@ -18,7 +18,7 @@ use Symfony\Component\HttpKernel\KernelEvents; * * @internal */ -final class XbRouteOptionsEventSubscriber implements EventSubscriberInterface { +final class XbRouteEventSubscriber implements EventSubscriberInterface { public function __construct( private readonly RouteMatchInterface $routeMatch, @@ -52,21 +52,22 @@ final class XbRouteOptionsEventSubscriber implements EventSubscriberInterface { $route->setRequirement('_csrf_request_header_token', 'TRUE'); } } - // Add collection permission of xb_page entity type - // to page list route. - if ($name === 'experience_builder.xb_page_list') { - $collection_permission = $this->entityTypeManager->getDefinition('xb_page')?->getCollectionPermission(); - $route->setRequirement('_permission', $collection_permission); - } } } + public function addPermissionToXbPageApi(RouteBuildEvent $event): void { + $route = $event->getRouteCollection()->get('experience_builder.api.xb_page'); + $collection_permission = $this->entityTypeManager->getDefinition('xb_page')?->getCollectionPermission(); + $route?->setRequirement('_permission', $collection_permission); + } + /** * {@inheritdoc} */ public static function getSubscribedEvents(): array { $events[KernelEvents::REQUEST][] = ['transformWrapperFormatRouteOption']; $events[RoutingEvents::ALTER][] = ['addCsrfToken']; + $events[RoutingEvents::ALTER][] = ['addPermissionToXbPageApi']; return $events; } diff --git a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php index d9bb01b994..db4b87b9d5 100644 --- a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php +++ b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php @@ -4,14 +4,13 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Kernel\Controller; +use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException; use Drupal\experience_builder\Entity\Page; use Drupal\KernelTests\KernelTestBase; use Drupal\Tests\experience_builder\Kernel\Traits\PageTrait; +use Drupal\Tests\experience_builder\Kernel\Traits\RequestTrait; use Drupal\Tests\user\Traits\UserCreationTrait; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\HttpKernelInterface; -use Symfony\Component\HttpKernel\TerminableInterface; /** * Tests tha api page list controller. @@ -23,6 +22,7 @@ use Symfony\Component\HttpKernel\TerminableInterface; class XBApiPageListControllerTest extends KernelTestBase { use PageTrait; + use RequestTrait; use UserCreationTrait; /** @@ -35,6 +35,11 @@ class XBApiPageListControllerTest extends KernelTestBase { ...self::PAGE_TEST_MODULES, ]; + /** + * {@inheritdoc} + */ + protected bool $usesSuperUserAccessPolicy = FALSE; + /** * {@inheritdoc} */ @@ -58,8 +63,6 @@ class XBApiPageListControllerTest extends KernelTestBase { 'path' => ['alias' => '/page-3'], 'status' => TRUE, ])->save(); - // Create super user so that it doesn't interfere with the test. - $this->createUser(); } /** @@ -67,68 +70,43 @@ class XBApiPageListControllerTest extends KernelTestBase { */ public function testApiResponse(): void { /** @var \Drupal\user\UserInterface $allowed_user */ - $allowed_user = $this->createUser(['administer xb_page']); + $allowed_user = $this->createUser(['access content'], 'access_content_user'); $this->setCurrentUser($allowed_user); - $response = $this->doRequest(Request::create('/api/page-list')); - /** @var string $content */ - $content = $response->getContent(); - $data = \json_decode($content, TRUE); - self::assertCount(3, $data['data']); - self::assertEquals('Page 1', $data['data'][0]['title']); - self::assertEquals(1, $data['data'][0]['id']); - self::assertEquals('/page-1', $data['data'][0]['path']); - self::assertTrue($data['data'][0]['status']); - self::assertEquals('Page 2', $data['data'][1]['title']); - self::assertEquals(2, $data['data'][1]['id']); - self::assertFalse($data['data'][1]['status']); - self::assertEquals('/page-2', $data['data'][1]['path']); - self::assertEquals('Page 3', $data['data'][2]['title']); - self::assertEquals(3, $data['data'][2]['id']); - self::assertTrue($data['data'][2]['status']); - self::assertEquals('/page-3', $data['data'][2]['path']); - } - - public function testApiResponseWithoutPermission(): void { + try { + $this->request(Request::create('/xb/api/page')); + } + catch (\Exception $e) { + self::assertInstanceOf(CacheableAccessDeniedHttpException::class, $e); + self::assertEquals("The 'administer xb_page' permission is required.", $e->getMessage()); + } /** @var \Drupal\user\UserInterface $allowed_user */ - $allowed_user = $this->createUser(['access content'], 'access_content_user'); + $allowed_user = $this->createUser(['administer xb_page'], 'administer_xb_page_user'); $this->setCurrentUser($allowed_user); - $response = $this->doRequest(Request::create('/api/page-list')); - self::assertEquals(Response::HTTP_FORBIDDEN, $response->getStatusCode()); - } - - /** - * Passes a request to the HTTP kernel and returns a response. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The request. - * @param bool $terminate - * To handle request termination. - * - * @return \Symfony\Component\HttpFoundation\Response - * The response. - * - * @throws \Exception - */ - protected function doRequest(Request $request, bool $terminate = TRUE): Response { - $http_kernel = $this->container->get('http_kernel'); - self::assertInstanceOf( - HttpKernelInterface::class, - $http_kernel - ); - $response = $http_kernel->handle($request); + $response = $this->request(Request::create('/xb/api/page')); + /** @var string $content */ $content = $response->getContent(); - self::assertNotFalse($content); - $this->setRawContent($content); - - if ($terminate) { - self::assertInstanceOf( - TerminableInterface::class, - $http_kernel - ); - $http_kernel->terminate($request, $response); - } - - return $response; + $data = \json_decode($content, TRUE); + // Remove above assertions and change it with assertSame for the data. + self::assertSame([ + [ + 'id' => 1, + 'title' => 'Page 1', + 'status' => TRUE, + 'path' => '/page-1', + ], + [ + 'id' => 2, + 'title' => 'Page 2', + 'status' => FALSE, + 'path' => '/page-2', + ], + [ + 'id' => 3, + 'title' => 'Page 3', + 'status' => TRUE, + 'path' => '/page-3', + ], + ], $data['data']); } } -- GitLab From 916f0e042190f6294772bae2772b8e3f9725183a Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Tue, 21 Jan 2025 21:10:29 +0530 Subject: [PATCH 04/24] Add pagination. --- src/Controller/XbApiPageListController.php | 18 +++-- .../XBApiPageListControllerTest.php | 69 +++++++++---------- 2 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php index aa227dd19e..5f91449821 100644 --- a/src/Controller/XbApiPageListController.php +++ b/src/Controller/XbApiPageListController.php @@ -7,22 +7,29 @@ namespace Drupal\experience_builder\Controller; use Drupal\Core\Cache\CacheableJsonResponse; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityTypeManagerInterface; +use Symfony\Component\HttpFoundation\Request; final readonly class XbApiPageListController { public function __construct(private EntityTypeManagerInterface $entityTypeManager) { } - public function __invoke(): CacheableJsonResponse { + public function __invoke(Request $request): CacheableJsonResponse { + $page = (int) $request->query->get('page', 1) - 1; $storage = $this->entityTypeManager->getStorage('xb_page'); - $xb_page_ids = $storage->getQuery()->accessCheck()->execute(); + // We don't need to worry about the status of the page, + // as we need both published and unpublished pages on the frontend. + try { + $entity_query = $storage->getQuery()->range($page * 10, 10)->accessCheck(FALSE); + $xb_page_ids = $entity_query->execute(); + } + catch (\Exception) { + $xb_page_ids = []; + } /** @var \Drupal\experience_builder\Entity\Page[] $xb_pages */ $xb_pages = $this->entityTypeManager->getStorage('xb_page')->loadMultiple($xb_page_ids); $xb_page_list = []; foreach ($xb_pages as $xb_page) { - if (!$xb_page->access('update')) { - continue; - } $xb_page_list[] = [ 'id' => (int) $xb_page->id(), 'title' => $xb_page->label(), @@ -32,6 +39,7 @@ final readonly class XbApiPageListController { } $cache = new CacheableMetadata(); $cache->addCacheTags($storage->getEntityType()->getListCacheTags()); + $cache->addCacheContexts(['url.query_args:page']); $json_response = new CacheableJsonResponse([ 'data' => $xb_page_list, ]); diff --git a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php index db4b87b9d5..9e7e36aaf6 100644 --- a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php +++ b/tests/src/Kernel/Controller/XBApiPageListControllerTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Kernel\Controller; +use Drupal\Core\Cache\CacheableJsonResponse; use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException; use Drupal\experience_builder\Entity\Page; use Drupal\KernelTests\KernelTestBase; @@ -46,23 +47,15 @@ class XBApiPageListControllerTest extends KernelTestBase { protected function setUp(): void { parent::setUp(); $this->installEntitySchema('user'); - // Create 3 xb_page entities. + // Create xb_page entities. $this->installPageEntitySchema(); - Page::create([ - 'title' => 'Page 1', - 'path' => ['alias' => '/page-1'], - 'status' => TRUE, - ])->save(); - Page::create([ - 'title' => 'Page 2', - 'path' => ['alias' => '/page-2'], - 'status' => FALSE, - ])->save(); - Page::create([ - 'title' => 'Page 3', - 'path' => ['alias' => '/page-3'], - 'status' => TRUE, - ])->save(); + for ($i = 1; $i <= 30; $i++) { + Page::create([ + 'title' => "Page $i", + 'status' => !($i % 2 === 0), + 'path' => ['alias' => "/page-$i"], + ])->save(); + } } /** @@ -82,31 +75,31 @@ class XBApiPageListControllerTest extends KernelTestBase { /** @var \Drupal\user\UserInterface $allowed_user */ $allowed_user = $this->createUser(['administer xb_page'], 'administer_xb_page_user'); $this->setCurrentUser($allowed_user); - $response = $this->request(Request::create('/xb/api/page')); + $this->assertPaginatedData(); + $this->assertPaginatedData(2); + $this->assertPaginatedData(3); + } + + protected function assertPaginatedData(int $page_number = 1): void { + $response = $this->request(Request::create('/xb/api/page?page=' . $page_number)); + self::assertInstanceOf(CacheableJsonResponse::class, $response); /** @var string $content */ $content = $response->getContent(); $data = \json_decode($content, TRUE); - // Remove above assertions and change it with assertSame for the data. - self::assertSame([ - [ - 'id' => 1, - 'title' => 'Page 1', - 'status' => TRUE, - 'path' => '/page-1', - ], - [ - 'id' => 2, - 'title' => 'Page 2', - 'status' => FALSE, - 'path' => '/page-2', - ], - [ - 'id' => 3, - 'title' => 'Page 3', - 'status' => TRUE, - 'path' => '/page-3', - ], - ], $data['data']); + // Assert paginated data. + self::assertCount(10, $data['data']); + $page_data = []; + $limit = $page_number * 10; + $offset = $limit - 10 + 1; + for ($i = $offset; $i <= $limit; $i++) { + $page_data[] = [ + 'id' => $i, + 'title' => "Page $i", + 'status' => !($i % 2 === 0), + 'path' => "/page-$i", + ]; + } + self::assertEquals($page_data, $data['data']); } } -- GitLab From 5529deded882203458461a4dc05e3f539e019648 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Wed, 22 Jan 2025 17:12:58 +0530 Subject: [PATCH 05/24] Execute query in render context to bubble cache metadata. --- src/Controller/XbApiPageListController.php | 38 +++++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php index 5f91449821..bdb0462cd9 100644 --- a/src/Controller/XbApiPageListController.php +++ b/src/Controller/XbApiPageListController.php @@ -7,25 +7,24 @@ namespace Drupal\experience_builder\Controller; use Drupal\Core\Cache\CacheableJsonResponse; use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\Core\Entity\Query\QueryInterface; +use Drupal\Core\Render\RenderContext; +use Drupal\Core\Render\RendererInterface; use Symfony\Component\HttpFoundation\Request; final readonly class XbApiPageListController { - public function __construct(private EntityTypeManagerInterface $entityTypeManager) { + public function __construct(private EntityTypeManagerInterface $entityTypeManager, private RendererInterface $renderer) { } public function __invoke(Request $request): CacheableJsonResponse { $page = (int) $request->query->get('page', 1) - 1; $storage = $this->entityTypeManager->getStorage('xb_page'); + $cache = new CacheableMetadata(); // We don't need to worry about the status of the page, // as we need both published and unpublished pages on the frontend. - try { - $entity_query = $storage->getQuery()->range($page * 10, 10)->accessCheck(FALSE); - $xb_page_ids = $entity_query->execute(); - } - catch (\Exception) { - $xb_page_ids = []; - } + $entity_query = $storage->getQuery()->range($page * 10, 10)->accessCheck(FALSE); + $xb_page_ids = $this->executeQueryInRenderContext($entity_query, $cache); /** @var \Drupal\experience_builder\Entity\Page[] $xb_pages */ $xb_pages = $this->entityTypeManager->getStorage('xb_page')->loadMultiple($xb_page_ids); $xb_page_list = []; @@ -37,7 +36,6 @@ final readonly class XbApiPageListController { 'path' => $xb_page->toUrl()->toString(), ]; } - $cache = new CacheableMetadata(); $cache->addCacheTags($storage->getEntityType()->getListCacheTags()); $cache->addCacheContexts(['url.query_args:page']); $json_response = new CacheableJsonResponse([ @@ -47,4 +45,26 @@ final readonly class XbApiPageListController { return $json_response; } + /** + * Executes the query in a render context, to catch bubbled cacheability. + * + * @param \Drupal\Core\Entity\Query\QueryInterface $query + * The query to execute to get the return results. + * @param \Drupal\Core\Cache\CacheableMetadata $query_cacheability + * The value object to carry the query cacheability. + * + * @return array + * Returns ids of entities. + */ + private function executeQueryInRenderContext(QueryInterface $query, CacheableMetadata $query_cacheability) : array { + $context = new RenderContext(); + $results = $this->renderer->executeInRenderContext($context, function () use ($query) { + return $query->execute(); + }); + if (!$context->isEmpty()) { + $query_cacheability->addCacheableDependency($context->pop()); + } + return $results; + } + } -- GitLab From f1e94cfa2fd186c96e06473d9b2f0b121a91c79c Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Wed, 22 Jan 2025 18:15:44 +0530 Subject: [PATCH 06/24] Add openapi spec. --- openapi.yml | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/openapi.yml b/openapi.yml index e10bbf713b..9f187a3aa5 100644 --- a/openapi.yml +++ b/openapi.yml @@ -30,6 +30,55 @@ info: # @see https://spec.openapis.org/oas/v3.1.0.html#paths-object # @see https://spec.openapis.org/oas/v3.1.0.html#path-item-object paths: + '/xb/api/page': + get: + description: Provides api for page listing. + parameters: + - in: query + name: page + required: false + schema: + type: integer + example: 1 + description: The page number. + responses: + 200: + description: Page list generated successfully. + content: + application/json: + schema: + type: object + properties: + data: + type: array + items: + type: object + properties: + id: + type: string + description: The page id. + title: + type: string + description: The page title. + status: + type: boolean + description: The page status. + path: + type: string + description: The page path. + examples: + pageList: + value: + data: + - id: 1 + title: Home + status: true + path: /home + - id: 2 + title: About + status: false + path: /about + '/api/layout/{entityTypeId}/{entityId}': description: TODO get: -- GitLab From 1075e9be380658ed1661ea6d3c9de759489f0f63 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Wed, 22 Jan 2025 18:39:43 +0530 Subject: [PATCH 07/24] Fix openapi spec for tests. --- openapi.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi.yml b/openapi.yml index 9f187a3aa5..ad1d90dd02 100644 --- a/openapi.yml +++ b/openapi.yml @@ -55,7 +55,7 @@ paths: type: object properties: id: - type: string + type: integer description: The page id. title: type: string -- GitLab From 6aa2026ecbd3326381e936f2306aaac3cffdb2d4 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Fri, 24 Jan 2025 20:00:26 +0530 Subject: [PATCH 08/24] Add @see and comment about dynamic permission. --- experience_builder.routing.yml | 1 + src/EventSubscriber/XbRouteEventSubscriber.php | 3 +++ 2 files changed, 4 insertions(+) diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index 945b677f49..f9f3c6c8b6 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -75,6 +75,7 @@ experience_builder.api.xb_page: defaults: _controller: 'Drupal\experience_builder\Controller\XbApiPageListController' # Dynamic requirement is added in the alter route event subscriber. +# @see \Drupal\experience_builder\EventSubscriber\XbRouteEventSubscriber::addPermissionToXbPageApi() experience_builder.api.component_inputs_form: path: '/xb-field-form/{entity_type}/{entity}' diff --git a/src/EventSubscriber/XbRouteEventSubscriber.php b/src/EventSubscriber/XbRouteEventSubscriber.php index 4302e863a5..7f7ec68c91 100644 --- a/src/EventSubscriber/XbRouteEventSubscriber.php +++ b/src/EventSubscriber/XbRouteEventSubscriber.php @@ -57,6 +57,9 @@ final class XbRouteEventSubscriber implements EventSubscriberInterface { public function addPermissionToXbPageApi(RouteBuildEvent $event): void { $route = $event->getRouteCollection()->get('experience_builder.api.xb_page'); + // As of now we are setting collection permission for this route. + // Eventually, there will be a separate permission for edit and adding xb_page entity. + // @todo Update this event subscriber to set permission for edit xb_page entity once it lands. $collection_permission = $this->entityTypeManager->getDefinition('xb_page')?->getCollectionPermission(); $route?->setRequirement('_permission', $collection_permission); } -- GitLab From 9d2b466166fec5d3c2ddf0882f8a80d5f54bc4d6 Mon Sep 17 00:00:00 2001 From: Matt Glaman <matt.glaman@acquia.com> Date: Fri, 24 Jan 2025 14:12:41 -0600 Subject: [PATCH 09/24] hardcode permission in route, add todo for new permission. set accessCheck because it's user facing --- experience_builder.routing.yml | 5 +++-- src/Controller/XbApiPageListController.php | 4 +++- src/EventSubscriber/XbRouteEventSubscriber.php | 10 ---------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index f9f3c6c8b6..4af67d23d7 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -74,8 +74,9 @@ experience_builder.api.xb_page: path: '/xb/api/page' defaults: _controller: 'Drupal\experience_builder\Controller\XbApiPageListController' -# Dynamic requirement is added in the alter route event subscriber. -# @see \Drupal\experience_builder\EventSubscriber\XbRouteEventSubscriber::addPermissionToXbPageApi() + requirements: + # @todo update in https://www.drupal.org/i/3502048 + _permission: 'administer xb_page' experience_builder.api.component_inputs_form: path: '/xb-field-form/{entity_type}/{entity}' diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php index bdb0462cd9..d6872b29f2 100644 --- a/src/Controller/XbApiPageListController.php +++ b/src/Controller/XbApiPageListController.php @@ -23,7 +23,9 @@ final readonly class XbApiPageListController { $cache = new CacheableMetadata(); // We don't need to worry about the status of the page, // as we need both published and unpublished pages on the frontend. - $entity_query = $storage->getQuery()->range($page * 10, 10)->accessCheck(FALSE); + $entity_query = $storage->getQuery() + ->accessCheck() + ->range($page * 10, 10); $xb_page_ids = $this->executeQueryInRenderContext($entity_query, $cache); /** @var \Drupal\experience_builder\Entity\Page[] $xb_pages */ $xb_pages = $this->entityTypeManager->getStorage('xb_page')->loadMultiple($xb_page_ids); diff --git a/src/EventSubscriber/XbRouteEventSubscriber.php b/src/EventSubscriber/XbRouteEventSubscriber.php index 7f7ec68c91..6ea19e67ec 100644 --- a/src/EventSubscriber/XbRouteEventSubscriber.php +++ b/src/EventSubscriber/XbRouteEventSubscriber.php @@ -55,22 +55,12 @@ final class XbRouteEventSubscriber implements EventSubscriberInterface { } } - public function addPermissionToXbPageApi(RouteBuildEvent $event): void { - $route = $event->getRouteCollection()->get('experience_builder.api.xb_page'); - // As of now we are setting collection permission for this route. - // Eventually, there will be a separate permission for edit and adding xb_page entity. - // @todo Update this event subscriber to set permission for edit xb_page entity once it lands. - $collection_permission = $this->entityTypeManager->getDefinition('xb_page')?->getCollectionPermission(); - $route?->setRequirement('_permission', $collection_permission); - } - /** * {@inheritdoc} */ public static function getSubscribedEvents(): array { $events[KernelEvents::REQUEST][] = ['transformWrapperFormatRouteOption']; $events[RoutingEvents::ALTER][] = ['addCsrfToken']; - $events[RoutingEvents::ALTER][] = ['addPermissionToXbPageApi']; return $events; } -- GitLab From 9e3ddaa6f9d95696c8fecf591c9ea9b6feea63df Mon Sep 17 00:00:00 2001 From: Matt Glaman <matt.glaman@acquia.com> Date: Fri, 24 Jan 2025 14:21:09 -0600 Subject: [PATCH 10/24] undo rename XbRouteOptionsEventSubscriber --- experience_builder.services.yml | 2 +- ...eEventSubscriber.php => XbRouteOptionsEventSubscriber.php} | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) rename src/EventSubscriber/{XbRouteEventSubscriber.php => XbRouteOptionsEventSubscriber.php} (92%) diff --git a/experience_builder.services.yml b/experience_builder.services.yml index b1406d1626..e007c5a377 100644 --- a/experience_builder.services.yml +++ b/experience_builder.services.yml @@ -66,7 +66,7 @@ services: tags: - { name: event_subscriber } experience_builder.route_options.subscriber: - class: Drupal\experience_builder\EventSubscriber\XbRouteEventSubscriber + class: Drupal\experience_builder\EventSubscriber\XbRouteOptionsEventSubscriber tags: - { name: 'event_subscriber' } Drupal\experience_builder\EventSubscriber\PreviewEnvelopeViewSubscriber: {} diff --git a/src/EventSubscriber/XbRouteEventSubscriber.php b/src/EventSubscriber/XbRouteOptionsEventSubscriber.php similarity index 92% rename from src/EventSubscriber/XbRouteEventSubscriber.php rename to src/EventSubscriber/XbRouteOptionsEventSubscriber.php index 6ea19e67ec..987dde68e3 100644 --- a/src/EventSubscriber/XbRouteEventSubscriber.php +++ b/src/EventSubscriber/XbRouteOptionsEventSubscriber.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Drupal\experience_builder\EventSubscriber; -use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\EventSubscriber\MainContentViewSubscriber; use Drupal\Core\Routing\RouteBuildEvent; use Drupal\Core\Routing\RouteMatchInterface; @@ -18,11 +17,10 @@ use Symfony\Component\HttpKernel\KernelEvents; * * @internal */ -final class XbRouteEventSubscriber implements EventSubscriberInterface { +final class XbRouteOptionsEventSubscriber implements EventSubscriberInterface { public function __construct( private readonly RouteMatchInterface $routeMatch, - private readonly EntityTypeManagerInterface $entityTypeManager, ) {} public function transformWrapperFormatRouteOption(RequestEvent $event): void { -- GitLab From 6424c5e7b31cce6856887a720d4397c858fef844 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Tue, 28 Jan 2025 13:30:28 +0530 Subject: [PATCH 11/24] Remove pagination, move page listing and page creation apis to single controller, introduce todos and other nits. --- experience_builder.routing.yml | 9 +- experience_builder.services.yml | 2 +- src/Controller/ApiContentControllers.php | 102 ++++++++++++++++++ src/Controller/ApiContentCreateXbPage.php | 46 -------- src/Controller/XbApiPageListController.php | 72 ------------- ...Test.php => ApiContentControllersTest.php} | 71 ++++++------ 6 files changed, 148 insertions(+), 154 deletions(-) create mode 100644 src/Controller/ApiContentControllers.php delete mode 100644 src/Controller/ApiContentCreateXbPage.php delete mode 100644 src/Controller/XbApiPageListController.php rename tests/src/Kernel/Controller/{XBApiPageListControllerTest.php => ApiContentControllersTest.php} (63%) diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index 4af67d23d7..dc9487116c 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -38,7 +38,7 @@ experience_builder.api.content.update: experience_builder.api.content.create: path: '/xb/api/content-create/xb_page' defaults: - _controller: 'Drupal\experience_builder\Controller\ApiContentCreateXbPage' + _controller: 'Drupal\experience_builder\Controller\ApiContentControllers::create' requirements: _entity_create_access: 'xb_page:xb_page' methods: ['POST'] @@ -70,10 +70,11 @@ entity.component.delete_form: requirements: _entity_access: 'component.delete' -experience_builder.api.xb_page: - path: '/xb/api/page' +# @todo https://www.drupal.org/i/3498525 should generalize this to all eligible content entity types by using a route callback +experience_builder.api.content.list: + path: '/xb/api/content/xb_page' defaults: - _controller: 'Drupal\experience_builder\Controller\XbApiPageListController' + _controller: 'Drupal\experience_builder\Controller\ApiContentControllers::list' requirements: # @todo update in https://www.drupal.org/i/3502048 _permission: 'administer xb_page' diff --git a/experience_builder.services.yml b/experience_builder.services.yml index e007c5a377..63862d1a5f 100644 --- a/experience_builder.services.yml +++ b/experience_builder.services.yml @@ -97,7 +97,7 @@ services: Drupal\experience_builder\Controller\ApiLogController: {} Drupal\experience_builder\Controller\ApiPreviewController: {} Drupal\experience_builder\Controller\ApiContentUpdateForDemoController: {} - Drupal\experience_builder\Controller\ApiContentCreateXbPage: {} + Drupal\experience_builder\Controller\ApiContentControllers: {} Drupal\experience_builder\Controller\ComponentStatusController: {} Drupal\experience_builder\Controller\ExperienceBuilderController: {} Drupal\experience_builder\Controller\ApiPublishAllController: {} diff --git a/src/Controller/ApiContentControllers.php b/src/Controller/ApiContentControllers.php new file mode 100644 index 0000000000..9627768495 --- /dev/null +++ b/src/Controller/ApiContentControllers.php @@ -0,0 +1,102 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\experience_builder\Controller; + +use Drupal\Core\Cache\CacheableJsonResponse; +use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\Core\Entity\Query\QueryInterface; +use Drupal\Core\Render\RenderContext; +use Drupal\Core\Render\RendererInterface; +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\HttpFoundation\Response; + +/** + * Creates new xb_page entity. + * + * New entity must be unpublished and have a hardcoded initial title. + * + * @internal This HTTP API is intended only for the XB UI. These controllers + * and associated routes may change at any time. + * + * @todo https://www.drupal.org/i/3498525 should generalize this to all eligible content entity types + */ +final class ApiContentControllers { + + use StringTranslationTrait; + + public function __construct( + private readonly EntityTypeManagerInterface $entityTypeManager, + private readonly RendererInterface $renderer, + ) {} + + public function create(): JsonResponse { + // Note: this intentionally does not catch content entity type storage + // handler exceptions: the generic XB API exception subscriber handles them. + // @see \Drupal\experience_builder\EventSubscriber\ApiExceptionSubscriber + $page = $this->entityTypeManager->getStorage('xb_page')->create([ + 'title' => $this->t('Untitled page'), + 'status' => FALSE, + ]); + $page->save(); + + return new JsonResponse([ + 'entity_type' => $page->getEntityTypeId(), + 'entity_id' => $page->id(), + ], RESPONSE::HTTP_CREATED); + } + + public function list(): CacheableJsonResponse { + // @todo introduce pagination in https://www.drupal.org/i/3502691 + $storage = $this->entityTypeManager->getStorage('xb_page'); + $query_cacheability = new CacheableMetadata(); + // We don't need to worry about the status of the page, + // as we need both published and unpublished pages on the frontend. + $entity_query = $storage->getQuery()->accessCheck(TRUE); + $ids = $this->executeQueryInRenderContext($entity_query, $query_cacheability); + /** @var \Drupal\Core\Entity\EntityPublishedInterface[] $content_entities */ + $content_entities = $storage->loadMultiple($ids); + $content_list = []; + foreach ($content_entities as $content_entity) { + $content_list[] = [ + 'id' => (int) $content_entity->id(), + 'title' => $content_entity->label(), + 'status' => $content_entity->isPublished(), + 'path' => $content_entity->toUrl()->toString(), + ]; + } + $json_response = new CacheableJsonResponse([ + 'data' => $content_list, + ]); + // @todo add cache contexts for query params when introducing pagination in https://www.drupal.org/i/3502691. + $json_response->getCacheableMetadata()->addCacheTags($storage->getEntityType()->getListCacheTags()); + $json_response->addCacheableDependency($query_cacheability); + return $json_response; + } + + /** + * Executes the query in a render context, to catch bubbled cacheability. + * + * @param \Drupal\Core\Entity\Query\QueryInterface $query + * The query to execute to get the return results. + * @param \Drupal\Core\Cache\CacheableMetadata $query_cacheability + * The value object to carry the query cacheability. + * + * @return array + * Returns ids of entities. + */ + private function executeQueryInRenderContext(QueryInterface $query, CacheableMetadata $query_cacheability) : array { + $context = new RenderContext(); + $results = $this->renderer->executeInRenderContext($context, function () use ($query) { + return $query->execute(); + }); + if (!$context->isEmpty()) { + $query_cacheability->addCacheableDependency($context->pop()); + } + return $results; + } + +} diff --git a/src/Controller/ApiContentCreateXbPage.php b/src/Controller/ApiContentCreateXbPage.php deleted file mode 100644 index fd4a54fd82..0000000000 --- a/src/Controller/ApiContentCreateXbPage.php +++ /dev/null @@ -1,46 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\experience_builder\Controller; - -use Drupal\Core\Entity\EntityTypeManagerInterface; -use Drupal\Core\StringTranslation\StringTranslationTrait; -use Symfony\Component\HttpFoundation\JsonResponse; -use Symfony\Component\HttpFoundation\Response; - -/** - * Creates new xb_page entity. - * - * New entity must be unpublished and have a hardcoded initial title. - * - * @internal This HTTP API is intended only for the XB UI. These controllers - * and associated routes may change at any time. - * - * @todo https://www.drupal.org/i/3498525 should generalize this to all eligible content entity types - */ -final class ApiContentCreateXbPage { - - use StringTranslationTrait; - - public function __construct( - private readonly EntityTypeManagerInterface $entityTypeManager, - ) {} - - public function __invoke(): JsonResponse { - // Note: this intentionally does not catch content entity type storage - // handler exceptions: the generic XB API exception subscriber handles them. - // @see \Drupal\experience_builder\EventSubscriber\ApiExceptionSubscriber - $page = $this->entityTypeManager->getStorage('xb_page')->create([ - 'title' => $this->t('Untitled page'), - 'status' => FALSE, - ]); - $page->save(); - - return new JsonResponse([ - 'entity_type' => $page->getEntityTypeId(), - 'entity_id' => $page->id(), - ], RESPONSE::HTTP_CREATED); - } - -} diff --git a/src/Controller/XbApiPageListController.php b/src/Controller/XbApiPageListController.php deleted file mode 100644 index d6872b29f2..0000000000 --- a/src/Controller/XbApiPageListController.php +++ /dev/null @@ -1,72 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\experience_builder\Controller; - -use Drupal\Core\Cache\CacheableJsonResponse; -use Drupal\Core\Cache\CacheableMetadata; -use Drupal\Core\Entity\EntityTypeManagerInterface; -use Drupal\Core\Entity\Query\QueryInterface; -use Drupal\Core\Render\RenderContext; -use Drupal\Core\Render\RendererInterface; -use Symfony\Component\HttpFoundation\Request; - -final readonly class XbApiPageListController { - - public function __construct(private EntityTypeManagerInterface $entityTypeManager, private RendererInterface $renderer) { - } - - public function __invoke(Request $request): CacheableJsonResponse { - $page = (int) $request->query->get('page', 1) - 1; - $storage = $this->entityTypeManager->getStorage('xb_page'); - $cache = new CacheableMetadata(); - // We don't need to worry about the status of the page, - // as we need both published and unpublished pages on the frontend. - $entity_query = $storage->getQuery() - ->accessCheck() - ->range($page * 10, 10); - $xb_page_ids = $this->executeQueryInRenderContext($entity_query, $cache); - /** @var \Drupal\experience_builder\Entity\Page[] $xb_pages */ - $xb_pages = $this->entityTypeManager->getStorage('xb_page')->loadMultiple($xb_page_ids); - $xb_page_list = []; - foreach ($xb_pages as $xb_page) { - $xb_page_list[] = [ - 'id' => (int) $xb_page->id(), - 'title' => $xb_page->label(), - 'status' => $xb_page->isPublished(), - 'path' => $xb_page->toUrl()->toString(), - ]; - } - $cache->addCacheTags($storage->getEntityType()->getListCacheTags()); - $cache->addCacheContexts(['url.query_args:page']); - $json_response = new CacheableJsonResponse([ - 'data' => $xb_page_list, - ]); - $json_response->addCacheableDependency($cache); - return $json_response; - } - - /** - * Executes the query in a render context, to catch bubbled cacheability. - * - * @param \Drupal\Core\Entity\Query\QueryInterface $query - * The query to execute to get the return results. - * @param \Drupal\Core\Cache\CacheableMetadata $query_cacheability - * The value object to carry the query cacheability. - * - * @return array - * Returns ids of entities. - */ - private function executeQueryInRenderContext(QueryInterface $query, CacheableMetadata $query_cacheability) : array { - $context = new RenderContext(); - $results = $this->renderer->executeInRenderContext($context, function () use ($query) { - return $query->execute(); - }); - if (!$context->isEmpty()) { - $query_cacheability->addCacheableDependency($context->pop()); - } - return $results; - } - -} diff --git a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php b/tests/src/Kernel/Controller/ApiContentControllersTest.php similarity index 63% rename from tests/src/Kernel/Controller/XBApiPageListControllerTest.php rename to tests/src/Kernel/Controller/ApiContentControllersTest.php index 9e7e36aaf6..269d91bcca 100644 --- a/tests/src/Kernel/Controller/XBApiPageListControllerTest.php +++ b/tests/src/Kernel/Controller/ApiContentControllersTest.php @@ -16,11 +16,11 @@ use Symfony\Component\HttpFoundation\Request; /** * Tests tha api page list controller. * - * @coversDefaultClass \Drupal\experience_builder\Controller\XBApiPageListController + * @coversDefaultClass \Drupal\experience_builder\Controller\ApiContentControllers * * @group experience_builder */ -class XBApiPageListControllerTest extends KernelTestBase { +class ApiContentControllersTest extends KernelTestBase { use PageTrait; use RequestTrait; @@ -49,13 +49,21 @@ class XBApiPageListControllerTest extends KernelTestBase { $this->installEntitySchema('user'); // Create xb_page entities. $this->installPageEntitySchema(); - for ($i = 1; $i <= 30; $i++) { - Page::create([ - 'title' => "Page $i", - 'status' => !($i % 2 === 0), - 'path' => ['alias' => "/page-$i"], - ])->save(); - } + Page::create([ + 'title' => "Page 1", + 'status' => TRUE, + 'path' => ['alias' => "/page-1"], + ])->save(); + Page::create([ + 'title' => "Page 2", + 'status' => FALSE, + 'path' => ['alias' => "/page-2"], + ])->save(); + Page::create([ + 'title' => "Page 3", + 'status' => TRUE, + 'path' => ['alias' => "/page-3"], + ])->save(); } /** @@ -66,7 +74,7 @@ class XBApiPageListControllerTest extends KernelTestBase { $allowed_user = $this->createUser(['access content'], 'access_content_user'); $this->setCurrentUser($allowed_user); try { - $this->request(Request::create('/xb/api/page')); + $this->request(Request::create('/xb/api/content/xb_page')); } catch (\Exception $e) { self::assertInstanceOf(CacheableAccessDeniedHttpException::class, $e); @@ -75,31 +83,32 @@ class XBApiPageListControllerTest extends KernelTestBase { /** @var \Drupal\user\UserInterface $allowed_user */ $allowed_user = $this->createUser(['administer xb_page'], 'administer_xb_page_user'); $this->setCurrentUser($allowed_user); - $this->assertPaginatedData(); - $this->assertPaginatedData(2); - $this->assertPaginatedData(3); - } - - protected function assertPaginatedData(int $page_number = 1): void { - $response = $this->request(Request::create('/xb/api/page?page=' . $page_number)); + $response = $this->request(Request::create('/xb/api/content/xb_page')); self::assertInstanceOf(CacheableJsonResponse::class, $response); /** @var string $content */ $content = $response->getContent(); $data = \json_decode($content, TRUE); - // Assert paginated data. - self::assertCount(10, $data['data']); - $page_data = []; - $limit = $page_number * 10; - $offset = $limit - 10 + 1; - for ($i = $offset; $i <= $limit; $i++) { - $page_data[] = [ - 'id' => $i, - 'title' => "Page $i", - 'status' => !($i % 2 === 0), - 'path' => "/page-$i", - ]; - } - self::assertEquals($page_data, $data['data']); + self::assertCount(3, $data['data']); + self::assertSame([ + [ + 'id' => 1, + 'title' => 'Page 1', + 'status' => TRUE, + 'path' => '/page-1', + ], + [ + 'id' => 2, + 'title' => 'Page 2', + 'status' => FALSE, + 'path' => '/page-2', + ], + [ + 'id' => 3, + 'title' => 'Page 3', + 'status' => TRUE, + 'path' => '/page-3', + ], + ], $data['data']); } } -- GitLab From fb856a58c43d00ae211fc421f6188baf5fa41ba3 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Tue, 28 Jan 2025 13:52:00 +0530 Subject: [PATCH 12/24] Update openapi.yml with latest API changes. --- openapi.yml | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/openapi.yml b/openapi.yml index ad1d90dd02..dd176522e3 100644 --- a/openapi.yml +++ b/openapi.yml @@ -30,17 +30,9 @@ info: # @see https://spec.openapis.org/oas/v3.1.0.html#paths-object # @see https://spec.openapis.org/oas/v3.1.0.html#path-item-object paths: - '/xb/api/page': + '/xb/api/content/xb_page': get: description: Provides api for page listing. - parameters: - - in: query - name: page - required: false - schema: - type: integer - example: 1 - description: The page number. responses: 200: description: Page list generated successfully. @@ -48,6 +40,8 @@ paths: application/json: schema: type: object + required: + - data properties: data: type: array -- GitLab From 1eb184fc4d8abfaf18c2ee796a22f1e0cf29beb7 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <52467-amangrover90@users.noreply.drupalcode.org> Date: Tue, 28 Jan 2025 14:17:49 +0000 Subject: [PATCH 13/24] Remove redundant controller from service.yml. --- experience_builder.services.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/experience_builder.services.yml b/experience_builder.services.yml index 63862d1a5f..9e00b4b634 100644 --- a/experience_builder.services.yml +++ b/experience_builder.services.yml @@ -102,4 +102,3 @@ services: Drupal\experience_builder\Controller\ExperienceBuilderController: {} Drupal\experience_builder\Controller\ApiPublishAllController: {} Drupal\experience_builder\Controller\ApiPendingChangesController: {} - Drupal\experience_builder\Controller\XbApiPageListController: {} -- GitLab From ff036ba6dec49ec136cdbe7e1ac14c8cae93de4c Mon Sep 17 00:00:00 2001 From: Wim Leers <44946-wimleers@users.noreply.drupalcode.org> Date: Wed, 29 Jan 2025 18:28:02 +0000 Subject: [PATCH 14/24] Trivial fixes & nits. --- openapi.yml | 8 +++++++- src/Controller/ApiContentControllers.php | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/openapi.yml b/openapi.yml index dd176522e3..b1b3bc9d25 100644 --- a/openapi.yml +++ b/openapi.yml @@ -47,10 +47,16 @@ paths: type: array items: type: object + required: + - id + - title + - status + - path + additionalProperties: false properties: id: type: integer - description: The page id. + description: The page ID. title: type: string description: The page title. diff --git a/src/Controller/ApiContentControllers.php b/src/Controller/ApiContentControllers.php index 9627768495..640237be0a 100644 --- a/src/Controller/ApiContentControllers.php +++ b/src/Controller/ApiContentControllers.php @@ -87,6 +87,8 @@ final class ApiContentControllers { * * @return array * Returns ids of entities. + * + * @see \Drupal\jsonapi\Controller\EntityResource::executeQueryInRenderContext() */ private function executeQueryInRenderContext(QueryInterface $query, CacheableMetadata $query_cacheability) : array { $context = new RenderContext(); -- GitLab From f652bb8a465436b946e4c808bb42cbd62be631e7 Mon Sep 17 00:00:00 2001 From: Wim Leers <44946-wimleers@users.noreply.drupalcode.org> Date: Wed, 29 Jan 2025 18:33:30 +0000 Subject: [PATCH 15/24] Fix inaccurate docs + consistency nits. --- experience_builder.routing.yml | 2 +- src/Controller/ApiContentControllers.php | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index dc9487116c..f0db9d42c4 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -38,7 +38,7 @@ experience_builder.api.content.update: experience_builder.api.content.create: path: '/xb/api/content-create/xb_page' defaults: - _controller: 'Drupal\experience_builder\Controller\ApiContentControllers::create' + _controller: 'Drupal\experience_builder\Controller\ApiContentControllers::post' requirements: _entity_create_access: 'xb_page:xb_page' methods: ['POST'] diff --git a/src/Controller/ApiContentControllers.php b/src/Controller/ApiContentControllers.php index 640237be0a..c41e4fe428 100644 --- a/src/Controller/ApiContentControllers.php +++ b/src/Controller/ApiContentControllers.php @@ -15,9 +15,7 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Response; /** - * Creates new xb_page entity. - * - * New entity must be unpublished and have a hardcoded initial title. + * HTTP API for interacting with XB-eligible Content entity types. * * @internal This HTTP API is intended only for the XB UI. These controllers * and associated routes may change at any time. @@ -33,7 +31,7 @@ final class ApiContentControllers { private readonly RendererInterface $renderer, ) {} - public function create(): JsonResponse { + public function post(): JsonResponse { // Note: this intentionally does not catch content entity type storage // handler exceptions: the generic XB API exception subscriber handles them. // @see \Drupal\experience_builder\EventSubscriber\ApiExceptionSubscriber @@ -53,8 +51,8 @@ final class ApiContentControllers { // @todo introduce pagination in https://www.drupal.org/i/3502691 $storage = $this->entityTypeManager->getStorage('xb_page'); $query_cacheability = new CacheableMetadata(); - // We don't need to worry about the status of the page, - // as we need both published and unpublished pages on the frontend. + // We don't need to worry about the status of the page, as we need both + // published and unpublished pages on the frontend. $entity_query = $storage->getQuery()->accessCheck(TRUE); $ids = $this->executeQueryInRenderContext($entity_query, $query_cacheability); /** @var \Drupal\Core\Entity\EntityPublishedInterface[] $content_entities */ @@ -86,7 +84,7 @@ final class ApiContentControllers { * The value object to carry the query cacheability. * * @return array - * Returns ids of entities. + * Returns IDs of entities. * * @see \Drupal\jsonapi\Controller\EntityResource::executeQueryInRenderContext() */ -- GitLab From 61b5f68161936772f30f4a0b7833dc8a042fd688 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Thu, 30 Jan 2025 15:18:13 +0530 Subject: [PATCH 16/24] PR comments; simplified API response; updated openapi.yml; wrote functional test; removed kernel test. --- experience_builder.routing.yml | 1 + openapi.yml | 65 +++++----- src/Controller/ApiContentControllers.php | 10 +- tests/src/Functional/HttpApiTestBase.php | 62 ++++++++++ .../Functional/XbConfigEntityHttpApiTest.php | 51 +------- .../Functional/XbContentEntityHttpApiTest.php | 81 +++++++++++-- .../Controller/ApiContentControllersTest.php | 114 ------------------ 7 files changed, 173 insertions(+), 211 deletions(-) create mode 100644 tests/src/Functional/HttpApiTestBase.php delete mode 100644 tests/src/Kernel/Controller/ApiContentControllersTest.php diff --git a/experience_builder.routing.yml b/experience_builder.routing.yml index f0db9d42c4..18c67b1b92 100644 --- a/experience_builder.routing.yml +++ b/experience_builder.routing.yml @@ -78,6 +78,7 @@ experience_builder.api.content.list: requirements: # @todo update in https://www.drupal.org/i/3502048 _permission: 'administer xb_page' + methods: [GET] experience_builder.api.component_inputs_form: path: '/xb-field-form/{entity_type}/{entity}' diff --git a/openapi.yml b/openapi.yml index b1b3bc9d25..4a0b6c93bc 100644 --- a/openapi.yml +++ b/openapi.yml @@ -40,44 +40,39 @@ paths: application/json: schema: type: object - required: - - data - properties: - data: - type: array - items: - type: object - required: - - id - - title - - status - - path - additionalProperties: false - properties: - id: - type: integer - description: The page ID. - title: - type: string - description: The page title. - status: - type: boolean - description: The page status. - path: - type: string - description: The page path. + minProperties: 0 + additionalProperties: false + patternProperties: + '^[0-9]$': + type: object + required: + - id + - title + - status + - path + properties: + id: + type: integer + description: The page ID. + title: + type: string + description: The page title. + status: + type: boolean + description: The page status. examples: pageList: value: - data: - - id: 1 - title: Home - status: true - path: /home - - id: 2 - title: About - status: false - path: /about + '1': + id: 1 + title: Home + status: true + path: /home + '2': + id: 2 + title: About + status: false + path: /about '/api/layout/{entityTypeId}/{entityId}': description: TODO diff --git a/src/Controller/ApiContentControllers.php b/src/Controller/ApiContentControllers.php index c41e4fe428..db6493128f 100644 --- a/src/Controller/ApiContentControllers.php +++ b/src/Controller/ApiContentControllers.php @@ -59,18 +59,18 @@ final class ApiContentControllers { $content_entities = $storage->loadMultiple($ids); $content_list = []; foreach ($content_entities as $content_entity) { - $content_list[] = [ - 'id' => (int) $content_entity->id(), + $id = (int) $content_entity->id(); + $content_list[$id] = [ + 'id' => $id, 'title' => $content_entity->label(), 'status' => $content_entity->isPublished(), 'path' => $content_entity->toUrl()->toString(), ]; } - $json_response = new CacheableJsonResponse([ - 'data' => $content_list, - ]); + $json_response = new CacheableJsonResponse($content_list); // @todo add cache contexts for query params when introducing pagination in https://www.drupal.org/i/3502691. $json_response->getCacheableMetadata()->addCacheTags($storage->getEntityType()->getListCacheTags()); + $json_response->getCacheableMetadata()->addCacheContexts($storage->getEntityType()->getListCacheContexts()); $json_response->addCacheableDependency($query_cacheability); return $json_response; } diff --git a/tests/src/Functional/HttpApiTestBase.php b/tests/src/Functional/HttpApiTestBase.php new file mode 100644 index 0000000000..dd6691a248 --- /dev/null +++ b/tests/src/Functional/HttpApiTestBase.php @@ -0,0 +1,62 @@ +<?php + +declare(strict_types=1); + +namespace Drupal\Tests\experience_builder\Functional; + +use Drupal\Core\Url; +use Drupal\FunctionalTests\BrowserTestBaseTest; +use Drupal\Tests\ApiRequestTrait; + +abstract class HttpApiTestBase extends BrowserTestBaseTest { + + use ApiRequestTrait; + + /** + * @return ?array + * The decoded JSON response, or NULL if there is no body. + * + * @throws \JsonException + */ + protected function assertExpectedResponse(string $method, Url $url, array $request_options, int $expected_status, ?array $expected_cache_contexts, ?array $expected_cache_tags, ?string $expected_page_cache, ?string $expected_dynamic_page_cache, array $additional_expected_response_headers = []): ?array { + $request_options['headers']['X-CSRF-Token'] = $this->drupalGet('session/token'); + $response = $this->makeApiRequest($method, $url, $request_options); + $body = (string) $response->getBody(); + $this->assertSame($expected_status, $response->getStatusCode(), $body); + + // Cacheability headers. + $this->assertSame($expected_page_cache !== NULL, $response->hasHeader('X-Drupal-Cache')); + if ($expected_page_cache !== NULL) { + $this->assertSame($expected_page_cache, $response->getHeader('X-Drupal-Cache')[0], 'Page Cache response header'); + } + $this->assertSame($expected_dynamic_page_cache !== NULL, $response->hasHeader('X-Drupal-Dynamic-Cache')); + if ($expected_dynamic_page_cache !== NULL) { + $this->assertSame($expected_dynamic_page_cache, $response->getHeader('X-Drupal-Dynamic-Cache')[0], 'Dynamic Page Cache response header'); + } + $this->assertSame($expected_cache_tags !== NULL, $response->hasHeader('X-Drupal-Cache-Tags')); + if ($expected_cache_tags !== NULL) { + $this->assertEqualsCanonicalizing($expected_cache_tags, explode(' ', $response->getHeader('X-Drupal-Cache-Tags')[0])); + } + $this->assertSame($expected_cache_contexts !== NULL, $response->hasHeader('X-Drupal-Cache-Contexts')); + if ($expected_cache_contexts !== NULL) { + $this->assertEqualsCanonicalizing($expected_cache_contexts, explode(' ', $response->getHeader('X-Drupal-Cache-Contexts')[0])); + } + + // Optionally, additional expected response headers can be validated. + if ($additional_expected_response_headers) { + foreach ($additional_expected_response_headers as $header_name => $expected_value) { + $this->assertSame($response->getHeader($header_name), $expected_value); + } + } + + // Response must at least be decodable JSON, let this throw an exception + // otherwise. (Assertions of the contents happen outside this method.) + if ($body === '') { + return NULL; + } + $json = json_decode($body, associative: TRUE, flags: JSON_THROW_ON_ERROR); + + return $json; + } + +} diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index 3390f9096f..8c57ead694 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -5,7 +5,7 @@ declare(strict_types=1); use Drupal\Core\Url; use Drupal\system\Entity\Menu; use Drupal\Tests\ApiRequestTrait; -use Drupal\Tests\BrowserTestBase; +use Drupal\Tests\experience_builder\Functional\HttpApiTestBase; use Drupal\Tests\experience_builder\Traits\ContribStrictConfigSchemaTestTrait; use Drupal\Tests\experience_builder\Traits\TestDataUtilitiesTrait; use Drupal\user\UserInterface; @@ -16,7 +16,7 @@ use GuzzleHttp\RequestOptions; * @group experience_builder * @internal */ -class XbConfigEntityHttpApiTest extends BrowserTestBase { +class XbConfigEntityHttpApiTest extends HttpApiTestBase { use ApiRequestTrait; use TestDataUtilitiesTrait; @@ -361,51 +361,4 @@ class XbConfigEntityHttpApiTest extends BrowserTestBase { // This was now tested full circle! ✅ } - /** - * @return ?array - * The decoded JSON response, or NULL if there is no body. - * - * @throws \JsonException - */ - private function assertExpectedResponse(string $method, Url $url, array $request_options, int $expected_status, ?array $expected_cache_contexts, ?array $expected_cache_tags, ?string $expected_page_cache, ?string $expected_dynamic_page_cache, array $additional_expected_response_headers = []): ?array { - $request_options['headers']['X-CSRF-Token'] = $this->drupalGet('session/token'); - $response = $this->makeApiRequest($method, $url, $request_options); - $body = (string) $response->getBody(); - $this->assertSame($expected_status, $response->getStatusCode(), $body); - - // Cacheability headers. - $this->assertSame($expected_page_cache !== NULL, $response->hasHeader('X-Drupal-Cache')); - if ($expected_page_cache !== NULL) { - $this->assertSame($expected_page_cache, $response->getHeader('X-Drupal-Cache')[0], 'Page Cache response header'); - } - $this->assertSame($expected_dynamic_page_cache !== NULL, $response->hasHeader('X-Drupal-Dynamic-Cache')); - if ($expected_dynamic_page_cache !== NULL) { - $this->assertSame($expected_dynamic_page_cache, $response->getHeader('X-Drupal-Dynamic-Cache')[0], 'Dynamic Page Cache response header'); - } - $this->assertSame($expected_cache_tags !== NULL, $response->hasHeader('X-Drupal-Cache-Tags')); - if ($expected_cache_tags !== NULL) { - $this->assertEqualsCanonicalizing($expected_cache_tags, explode(' ', $response->getHeader('X-Drupal-Cache-Tags')[0])); - } - $this->assertSame($expected_cache_contexts !== NULL, $response->hasHeader('X-Drupal-Cache-Contexts')); - if ($expected_cache_contexts !== NULL) { - $this->assertEqualsCanonicalizing($expected_cache_contexts, explode(' ', $response->getHeader('X-Drupal-Cache-Contexts')[0])); - } - - // Optionally, additional expected response headers can be validated. - if ($additional_expected_response_headers) { - foreach ($additional_expected_response_headers as $header_name => $expected_value) { - $this->assertSame($response->getHeader($header_name), $expected_value); - } - } - - // Response must at least be decodable JSON, let this throw an exception - // otherwise. (Assertions of the contents happen outside this method.) - if ($body === '') { - return NULL; - } - $json = json_decode($body, associative: TRUE, flags: JSON_THROW_ON_ERROR); - - return $json; - } - } diff --git a/tests/src/Functional/XbContentEntityHttpApiTest.php b/tests/src/Functional/XbContentEntityHttpApiTest.php index 197a5b335f..c475f679a0 100644 --- a/tests/src/Functional/XbContentEntityHttpApiTest.php +++ b/tests/src/Functional/XbContentEntityHttpApiTest.php @@ -5,20 +5,17 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Functional; use Drupal\Core\Url; -use Drupal\Tests\ApiRequestTrait; -use Drupal\Tests\BrowserTestBase; +use Drupal\experience_builder\Entity\Page; use Drupal\user\Entity\Role; use Drupal\user\UserInterface; use GuzzleHttp\RequestOptions; /** - * @covers \Drupal\experience_builder\Controller\ApiContentCreateXbPage + * @covers \Drupal\experience_builder\Controller\ApiContentControllers * @group experience_builder * @internal */ -final class XbContentEntityHttpApiTest extends BrowserTestBase { - - use ApiRequestTrait; +final class XbContentEntityHttpApiTest extends HttpApiTestBase { /** * {@inheritdoc} @@ -32,7 +29,29 @@ final class XbContentEntityHttpApiTest extends BrowserTestBase { */ protected $defaultTheme = 'stark'; - public function test(): void { + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + Page::create([ + 'title' => "Page 1", + 'status' => TRUE, + 'path' => ['alias' => "/page-1"], + ])->save(); + Page::create([ + 'title' => "Page 2", + 'status' => FALSE, + 'path' => ['alias' => "/page-2"], + ])->save(); + Page::create([ + 'title' => "Page 3", + 'status' => TRUE, + 'path' => ['alias' => "/page-3"], + ])->save(); + } + + public function testPost(): void { $url = Url::fromUri('base:/xb/api/content-create/xb_page'); $request_options = [ RequestOptions::HEADERS => [ @@ -66,9 +85,55 @@ final class XbContentEntityHttpApiTest extends BrowserTestBase { $response = $this->makeApiRequest('POST', $url, $request_options); $this->assertSame(201, $response->getStatusCode()); $this->assertSame( - '{"entity_type":"xb_page","entity_id":"1"}', + '{"entity_type":"xb_page","entity_id":"4"}', (string) $response->getBody() ); } + public function testList(): void { + $url = Url::fromUri('base:/xb/api/content/xb_page'); + // Anonymously: 403. + $body = $this->assertExpectedResponse('GET', $url, [], 403, ['user.permissions'], ['4xx-response', 'config:user.role.anonymous', 'http_response'], 'MISS', NULL); + $this->assertSame([ + 'message' => "The 'administer xb_page' permission is required.", + ], $body); + // User without permission. + $user = $this->createUser(['access content'], 'access_content_user'); + assert($user instanceof UserInterface); + $this->drupalLogin($user); + $body = $this->assertExpectedResponse('GET', $url, [], 403, ['user.permissions'], ['4xx-response', 'http_response'], 'UNCACHEABLE (request policy)', NULL); + $this->assertSame([ + 'message' => "The 'administer xb_page' permission is required.", + ], $body); + // Authenticated, authorized: 200. + $user = $this->createUser(['administer xb_page'], 'administer_xb_page_user'); + assert($user instanceof UserInterface); + $this->drupalLogin($user); + $body = $this->assertExpectedResponse('GET', $url, [], 200, ['user.permissions'], ['http_response', 'xb_page_list'], 'UNCACHEABLE (request policy)', 'MISS'); + $this->assertSame( + [ + '1' => [ + 'id' => 1, + 'title' => 'Page 1', + 'status' => TRUE, + 'path' => '/page-1', + ], + '2' => [ + 'id' => 2, + 'title' => 'Page 2', + 'status' => FALSE, + 'path' => '/page-2', + ], + '3' => [ + 'id' => 3, + 'title' => 'Page 3', + 'status' => TRUE, + 'path' => '/page-3', + ], + ], + $body + ); + $this->assertExpectedResponse('GET', $url, [], 200, ['user.permissions'], ['http_response', 'xb_page_list'], 'UNCACHEABLE (request policy)', 'HIT'); + } + } diff --git a/tests/src/Kernel/Controller/ApiContentControllersTest.php b/tests/src/Kernel/Controller/ApiContentControllersTest.php deleted file mode 100644 index 269d91bcca..0000000000 --- a/tests/src/Kernel/Controller/ApiContentControllersTest.php +++ /dev/null @@ -1,114 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Drupal\Tests\experience_builder\Kernel\Controller; - -use Drupal\Core\Cache\CacheableJsonResponse; -use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException; -use Drupal\experience_builder\Entity\Page; -use Drupal\KernelTests\KernelTestBase; -use Drupal\Tests\experience_builder\Kernel\Traits\PageTrait; -use Drupal\Tests\experience_builder\Kernel\Traits\RequestTrait; -use Drupal\Tests\user\Traits\UserCreationTrait; -use Symfony\Component\HttpFoundation\Request; - -/** - * Tests tha api page list controller. - * - * @coversDefaultClass \Drupal\experience_builder\Controller\ApiContentControllers - * - * @group experience_builder - */ -class ApiContentControllersTest extends KernelTestBase { - - use PageTrait; - use RequestTrait; - use UserCreationTrait; - - /** - * {@inheritdoc} - */ - protected static $modules = [ - 'experience_builder', - 'system', - 'image', - ...self::PAGE_TEST_MODULES, - ]; - - /** - * {@inheritdoc} - */ - protected bool $usesSuperUserAccessPolicy = FALSE; - - /** - * {@inheritdoc} - */ - protected function setUp(): void { - parent::setUp(); - $this->installEntitySchema('user'); - // Create xb_page entities. - $this->installPageEntitySchema(); - Page::create([ - 'title' => "Page 1", - 'status' => TRUE, - 'path' => ['alias' => "/page-1"], - ])->save(); - Page::create([ - 'title' => "Page 2", - 'status' => FALSE, - 'path' => ['alias' => "/page-2"], - ])->save(); - Page::create([ - 'title' => "Page 3", - 'status' => TRUE, - 'path' => ['alias' => "/page-3"], - ])->save(); - } - - /** - * Tests the API response. - */ - public function testApiResponse(): void { - /** @var \Drupal\user\UserInterface $allowed_user */ - $allowed_user = $this->createUser(['access content'], 'access_content_user'); - $this->setCurrentUser($allowed_user); - try { - $this->request(Request::create('/xb/api/content/xb_page')); - } - catch (\Exception $e) { - self::assertInstanceOf(CacheableAccessDeniedHttpException::class, $e); - self::assertEquals("The 'administer xb_page' permission is required.", $e->getMessage()); - } - /** @var \Drupal\user\UserInterface $allowed_user */ - $allowed_user = $this->createUser(['administer xb_page'], 'administer_xb_page_user'); - $this->setCurrentUser($allowed_user); - $response = $this->request(Request::create('/xb/api/content/xb_page')); - self::assertInstanceOf(CacheableJsonResponse::class, $response); - /** @var string $content */ - $content = $response->getContent(); - $data = \json_decode($content, TRUE); - self::assertCount(3, $data['data']); - self::assertSame([ - [ - 'id' => 1, - 'title' => 'Page 1', - 'status' => TRUE, - 'path' => '/page-1', - ], - [ - 'id' => 2, - 'title' => 'Page 2', - 'status' => FALSE, - 'path' => '/page-2', - ], - [ - 'id' => 3, - 'title' => 'Page 3', - 'status' => TRUE, - 'path' => '/page-3', - ], - ], $data['data']); - } - -} -- GitLab From 2932646162290f830a1aa9fba642dd603e85f1b3 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Thu, 30 Jan 2025 15:28:53 +0530 Subject: [PATCH 17/24] Add namespace. --- tests/src/Functional/XbConfigEntityHttpApiTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php index 8c57ead694..ea0a55d5b6 100644 --- a/tests/src/Functional/XbConfigEntityHttpApiTest.php +++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php @@ -2,10 +2,10 @@ declare(strict_types=1); +namespace Drupal\Tests\experience_builder\Functional; + use Drupal\Core\Url; use Drupal\system\Entity\Menu; -use Drupal\Tests\ApiRequestTrait; -use Drupal\Tests\experience_builder\Functional\HttpApiTestBase; use Drupal\Tests\experience_builder\Traits\ContribStrictConfigSchemaTestTrait; use Drupal\Tests\experience_builder\Traits\TestDataUtilitiesTrait; use Drupal\user\UserInterface; @@ -18,7 +18,6 @@ use GuzzleHttp\RequestOptions; */ class XbConfigEntityHttpApiTest extends HttpApiTestBase { - use ApiRequestTrait; use TestDataUtilitiesTrait; use ContribStrictConfigSchemaTestTrait; -- GitLab From 7525a012abb0ab7ec34f6bfee9eca6646ccb02ba Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Thu, 30 Jan 2025 15:50:48 +0530 Subject: [PATCH 18/24] Change base class. --- tests/src/Functional/HttpApiTestBase.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/Functional/HttpApiTestBase.php b/tests/src/Functional/HttpApiTestBase.php index dd6691a248..6cfaba4a54 100644 --- a/tests/src/Functional/HttpApiTestBase.php +++ b/tests/src/Functional/HttpApiTestBase.php @@ -5,10 +5,10 @@ declare(strict_types=1); namespace Drupal\Tests\experience_builder\Functional; use Drupal\Core\Url; -use Drupal\FunctionalTests\BrowserTestBaseTest; use Drupal\Tests\ApiRequestTrait; +use Drupal\Tests\BrowserTestBase; -abstract class HttpApiTestBase extends BrowserTestBaseTest { +abstract class HttpApiTestBase extends BrowserTestBase { use ApiRequestTrait; -- GitLab From 42c5aa9a4d62a685f44a6ab2e21c71f1ab6fb37f Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Thu, 30 Jan 2025 16:33:35 +0530 Subject: [PATCH 19/24] Omit /web from assertions. --- tests/src/Functional/XbContentEntityHttpApiTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/src/Functional/XbContentEntityHttpApiTest.php b/tests/src/Functional/XbContentEntityHttpApiTest.php index c475f679a0..eb335adca4 100644 --- a/tests/src/Functional/XbContentEntityHttpApiTest.php +++ b/tests/src/Functional/XbContentEntityHttpApiTest.php @@ -110,7 +110,11 @@ final class XbContentEntityHttpApiTest extends HttpApiTestBase { assert($user instanceof UserInterface); $this->drupalLogin($user); $body = $this->assertExpectedResponse('GET', $url, [], 200, ['user.permissions'], ['http_response', 'xb_page_list'], 'UNCACHEABLE (request policy)', 'MISS'); - $this->assertSame( + // In CI, the base URL is /web/, but in local development it is /. + array_walk($body, function (&$item) { + $item['path'] = str_replace('/web', '', $item['path']); + }); + $this->assertEquals( [ '1' => [ 'id' => 1, -- GitLab From 1911ca2c17f2209f0207d379904357eab61eab34 Mon Sep 17 00:00:00 2001 From: Amandeep Singh <arshgrover32@gmail.com> Date: Thu, 30 Jan 2025 17:04:39 +0530 Subject: [PATCH 20/24] Fix phpstan. --- tests/src/Functional/XbContentEntityHttpApiTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/Functional/XbContentEntityHttpApiTest.php b/tests/src/Functional/XbContentEntityHttpApiTest.php index eb335adca4..5c6fa26199 100644 --- a/tests/src/Functional/XbContentEntityHttpApiTest.php +++ b/tests/src/Functional/XbContentEntityHttpApiTest.php @@ -111,6 +111,7 @@ final class XbContentEntityHttpApiTest extends HttpApiTestBase { $this->drupalLogin($user); $body = $this->assertExpectedResponse('GET', $url, [], 200, ['user.permissions'], ['http_response', 'xb_page_list'], 'UNCACHEABLE (request policy)', 'MISS'); // In CI, the base URL is /web/, but in local development it is /. + assert(is_array($body)); array_walk($body, function (&$item) { $item['path'] = str_replace('/web', '', $item['path']); }); -- GitLab From d5488f7cf2cbf1fce58d037a2f6c0075af5ca018 Mon Sep 17 00:00:00 2001 From: Wim Leers <44946-wimleers@users.noreply.drupalcode.org> Date: Thu, 30 Jan 2025 12:20:32 +0000 Subject: [PATCH 21/24] Readability nits. --- tests/src/Functional/XbContentEntityHttpApiTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/src/Functional/XbContentEntityHttpApiTest.php b/tests/src/Functional/XbContentEntityHttpApiTest.php index 5c6fa26199..6bf9a441e7 100644 --- a/tests/src/Functional/XbContentEntityHttpApiTest.php +++ b/tests/src/Functional/XbContentEntityHttpApiTest.php @@ -92,11 +92,13 @@ final class XbContentEntityHttpApiTest extends HttpApiTestBase { public function testList(): void { $url = Url::fromUri('base:/xb/api/content/xb_page'); + // Anonymously: 403. $body = $this->assertExpectedResponse('GET', $url, [], 403, ['user.permissions'], ['4xx-response', 'config:user.role.anonymous', 'http_response'], 'MISS', NULL); $this->assertSame([ 'message' => "The 'administer xb_page' permission is required.", ], $body); + // User without permission. $user = $this->createUser(['access content'], 'access_content_user'); assert($user instanceof UserInterface); @@ -105,6 +107,7 @@ final class XbContentEntityHttpApiTest extends HttpApiTestBase { $this->assertSame([ 'message' => "The 'administer xb_page' permission is required.", ], $body); + // Authenticated, authorized: 200. $user = $this->createUser(['administer xb_page'], 'administer_xb_page_user'); assert($user instanceof UserInterface); -- GitLab From 5efc427504e90469fb64576d07c94aa562a2084f Mon Sep 17 00:00:00 2001 From: Wim Leers <wim.leers@acquia.com> Date: Thu, 30 Jan 2025 13:32:05 +0100 Subject: [PATCH 22/24] Update the test to *correctly* test when Drupal/XB is served from a subdirectory of a domain. --- tests/src/Functional/XbContentEntityHttpApiTest.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/src/Functional/XbContentEntityHttpApiTest.php b/tests/src/Functional/XbContentEntityHttpApiTest.php index 6bf9a441e7..2e71d19ed3 100644 --- a/tests/src/Functional/XbContentEntityHttpApiTest.php +++ b/tests/src/Functional/XbContentEntityHttpApiTest.php @@ -113,30 +113,25 @@ final class XbContentEntityHttpApiTest extends HttpApiTestBase { assert($user instanceof UserInterface); $this->drupalLogin($user); $body = $this->assertExpectedResponse('GET', $url, [], 200, ['user.permissions'], ['http_response', 'xb_page_list'], 'UNCACHEABLE (request policy)', 'MISS'); - // In CI, the base URL is /web/, but in local development it is /. - assert(is_array($body)); - array_walk($body, function (&$item) { - $item['path'] = str_replace('/web', '', $item['path']); - }); $this->assertEquals( [ '1' => [ 'id' => 1, 'title' => 'Page 1', 'status' => TRUE, - 'path' => '/page-1', + 'path' => base_path() . 'page-1', ], '2' => [ 'id' => 2, 'title' => 'Page 2', 'status' => FALSE, - 'path' => '/page-2', + 'path' => base_path() . 'page-2', ], '3' => [ 'id' => 3, 'title' => 'Page 3', 'status' => TRUE, - 'path' => '/page-3', + 'path' => base_path() . 'page-3', ], ], $body -- GitLab From daaf3cf9c31add51a2b42ff5d15616d029bac94b Mon Sep 17 00:00:00 2001 From: Wim Leers <wim.leers@acquia.com> Date: Thu, 30 Jan 2025 13:40:19 +0100 Subject: [PATCH 23/24] Respect cacheability of generated URLs; avoid more complex sites from being yelled at by `EarlyRenderingControllerWrapperSubscriber`. --- src/Controller/ApiContentControllers.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Controller/ApiContentControllers.php b/src/Controller/ApiContentControllers.php index db6493128f..e4e72daf35 100644 --- a/src/Controller/ApiContentControllers.php +++ b/src/Controller/ApiContentControllers.php @@ -50,7 +50,10 @@ final class ApiContentControllers { public function list(): CacheableJsonResponse { // @todo introduce pagination in https://www.drupal.org/i/3502691 $storage = $this->entityTypeManager->getStorage('xb_page'); - $query_cacheability = new CacheableMetadata(); + $query_cacheability = (new CacheableMetadata()) + ->addCacheContexts($storage->getEntityType()->getListCacheContexts()) + ->addCacheTags($storage->getEntityType()->getListCacheTags()); + $url_cacheability = new CacheableMetadata(); // We don't need to worry about the status of the page, as we need both // published and unpublished pages on the frontend. $entity_query = $storage->getQuery()->accessCheck(TRUE); @@ -60,18 +63,19 @@ final class ApiContentControllers { $content_list = []; foreach ($content_entities as $content_entity) { $id = (int) $content_entity->id(); + $generated_url = $content_entity->toUrl()->toString(TRUE); $content_list[$id] = [ 'id' => $id, 'title' => $content_entity->label(), 'status' => $content_entity->isPublished(), - 'path' => $content_entity->toUrl()->toString(), + 'path' => $generated_url->getGeneratedUrl(), ]; + $url_cacheability->addCacheableDependency($generated_url); } $json_response = new CacheableJsonResponse($content_list); // @todo add cache contexts for query params when introducing pagination in https://www.drupal.org/i/3502691. - $json_response->getCacheableMetadata()->addCacheTags($storage->getEntityType()->getListCacheTags()); - $json_response->getCacheableMetadata()->addCacheContexts($storage->getEntityType()->getListCacheContexts()); - $json_response->addCacheableDependency($query_cacheability); + $json_response->addCacheableDependency($query_cacheability) + ->addCacheableDependency($url_cacheability); return $json_response; } -- GitLab From 969ce1c8cb43d4e756b34642b7bb0de33cc6e5a0 Mon Sep 17 00:00:00 2001 From: Wim Leers <wim.leers@acquia.com> Date: Thu, 30 Jan 2025 13:44:31 +0100 Subject: [PATCH 24/24] Make the concerns raised by @larowlan VERY explicit to avoid them being forgotten! --- src/Controller/ApiContentControllers.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Controller/ApiContentControllers.php b/src/Controller/ApiContentControllers.php index e4e72daf35..735c01d92c 100644 --- a/src/Controller/ApiContentControllers.php +++ b/src/Controller/ApiContentControllers.php @@ -47,6 +47,16 @@ final class ApiContentControllers { ], RESPONSE::HTTP_CREATED); } + /** + * Returns a list of XB Page content entities, with only high-level metadata. + * + * TRICKY: there are reasons XB has its own internal HTTP API rather than + * using Drupal core's JSON:API. As soon as this method is updated to return + * all fields instead of just high-level metadata, those reasons may start to + * outweigh the downsides of adding a dependency on JSON:API. + * + * @see https://www.drupal.org/project/experience_builder/issues/3500052#comment-15966496 + */ public function list(): CacheableJsonResponse { // @todo introduce pagination in https://www.drupal.org/i/3502691 $storage = $this->entityTypeManager->getStorage('xb_page'); -- GitLab