diff --git a/modules/bc_api_docs/assets/dist/.DS_Store b/modules/bc_api_docs/assets/dist/.DS_Store deleted file mode 100644 index cbbf28b..0000000 Binary files a/modules/bc_api_docs/assets/dist/.DS_Store and /dev/null differ diff --git a/modules/bc_api_docs/assets/dist/js/.DS_Store b/modules/bc_api_docs/assets/dist/js/.DS_Store deleted file mode 100644 index ac4d3e5..0000000 Binary files a/modules/bc_api_docs/assets/dist/js/.DS_Store and /dev/null differ diff --git a/modules/bc_api_example/bc_api_example.info.yml b/modules/bc_api_example/bc_api_example.info.yml index 41064b5..26f3cb9 100644 --- a/modules/bc_api_example/bc_api_example.info.yml +++ b/modules/bc_api_example/bc_api_example.info.yml @@ -6,6 +6,7 @@ core_version_requirement: ^10 || ^11 package: Bluecadet dependencies: + - drupal:taxonomy - bc_api_base:bc_api_base configure: null diff --git a/modules/bc_api_example/bc_api_example.routing.yml b/modules/bc_api_example/bc_api_example.routing.yml index bf7b0e5..f9efc64 100644 --- a/modules/bc_api_example/bc_api_example.routing.yml +++ b/modules/bc_api_example/bc_api_example.routing.yml @@ -20,3 +20,26 @@ bc_api_example.pirates_detail: parameters: nid: type: entity:node + +bc_api_example.cacheable.pirates: + path: '/api/cacheable/pirates' + methods: [GET, HEAD] + defaults: + _controller: '\Drupal\bc_api_example\Controller\ApiControllerCacheableResponsePirateExample:getResourceList' + requirements: + _permission: 'use api' + options: + _auth: [ 'key_auth' ] + +bc_api_example.cacheable.pirates_detail: + path: '/api/cacheable/pirates/{nid}' + methods: [GET, HEAD] + defaults: + _controller: '\Drupal\bc_api_example\Controller\ApiControllerCacheableResponsePirateExample:getResource' + requirements: + _permission: 'use api' + options: + _auth: [ 'key_auth' ] + parameters: + nid: + type: entity:node diff --git a/modules/bc_api_example/config/.DS_Store b/modules/bc_api_example/config/.DS_Store deleted file mode 100644 index 7d6a845..0000000 Binary files a/modules/bc_api_example/config/.DS_Store and /dev/null differ diff --git a/modules/bc_api_example/src/Controller/ApiControllerCacheableResponsePirateExample.php b/modules/bc_api_example/src/Controller/ApiControllerCacheableResponsePirateExample.php new file mode 100644 index 0000000..1d8a1ea --- /dev/null +++ b/modules/bc_api_example/src/Controller/ApiControllerCacheableResponsePirateExample.php @@ -0,0 +1,170 @@ +params)) { + $cid .= ":" . implode(":", $this->params); + } + + $cid .= ":page-" . $this->page; + $cid .= ":limit-" . $this->limit; + + return $cid; + } + + /** + * {@inheritdoc} + */ + public function initCacheTags() { + $this->cacheTags = [ + 'myAwesomeCoolCacheTag', + ]; + } + + /** + * {@inheritdoc} + */ + public function setParams() { + // It would be good to always run this. + parent::setParams(); + + $this->privateParams['status'] = 1; + + // Here we also want to handle any bad param errors... + } + + /** + * {@inheritdoc} + */ + public function getDefaultPlatform() { + return "cinder"; + } + + /** + * {@inheritdoc} + */ + public function getResourceQueryResult() { + // Just having this here as an example. + // Most times no need to override this method. + parent::getResourceQueryResult(); + } + + /** + * {@inheritdoc} + */ + public function getResourceListQueryResult() { + // This method should be overridden for any endpoint. + $query = $this->entityTypeManager->getStorage('node')->getQuery(); + $query->accessCheck(TRUE); + $query->condition('status', $this->privateParams['status']); + $query->condition('type', 'pirate'); + + $count_query = clone $query; + + $query->range(($this->page * $this->limit), $this->limit); + $entity_ids = $query->execute(); + + // Must set total result count so we can properly page. + $this->resultTotal = (int) $count_query->count()->execute(); + + // Process Items. + $nodes = $this->entityTypeManager->getStorage('node')->loadMultiple($entity_ids); + + $this->rawData = $nodes; + } + + /** + * {@inheritdoc} + */ + public function buildAllResourceData() { + $data = []; + + foreach ($this->rawData as $node) { + $this->cacheTags = array_merge($this->cacheTags, $node->getCacheTags()); + $this->addCacheableDependency($node); + $created_changed = $this->transformer->createdChangedFieldVals($node); + + $item = [ + 'nid' => (int) $node->id(), + 'cms_title' => $node->label(), + 'created' => $created_changed[0], + 'updated' => $created_changed[1], + ]; + $data[] = $item; + } + + $this->data = $data; + } + + /** + * {@inheritdoc} + */ + public function buildLinks() { + // This method should be overridden for any endpoint. + $base_url = $this->request->getSchemeAndHttpHost() . $this->request->getPathInfo(); + $tmp_query_params = $this->params; + $tmp_query_params['platform'] = $this->platform; + $tmp_query_params['limit'] = $this->limit; + + if ($this->page == 0) { + $this->prev = ""; + } + else { + $tmp_query_params['page'] = $this->page - 1; + $this->prev = $base_url . "?" . http_build_query($tmp_query_params); + } + + if ($this->resultTotal > (($this->page + 1) * $this->limit)) { + $tmp_query_params['page'] = $this->page + 1; + $this->next = $base_url . "?" . http_build_query($tmp_query_params); + } + else { + + $this->next = ""; + } + + } + +} diff --git a/modules/bc_api_example/src/Controller/ApiControllerPirateExample.php b/modules/bc_api_example/src/Controller/ApiControllerPirateExample.php index 81ced00..aaedb20 100644 --- a/modules/bc_api_example/src/Controller/ApiControllerPirateExample.php +++ b/modules/bc_api_example/src/Controller/ApiControllerPirateExample.php @@ -38,7 +38,8 @@ public function getApiCacheTime($id = NULL) { * {@inheritdoc} */ public function getCacheId() { - $cid = "SOMETHING_UNIQUE"; + // This should be a unique string, characters only. + $cid = "pirates"; if (!empty($this->params)) { $cid .= ":" . implode(":", $this->params); @@ -93,6 +94,7 @@ public function getResourceQueryResult() { public function getResourceListQueryResult() { // This method should be overridden for any endpoint. $query = $this->entityTypeManager->getStorage('node')->getQuery(); + $query->accessCheck(TRUE); $query->condition('status', $this->privateParams['status']); $query->condition('type', 'pirate'); diff --git a/modules/bc_api_logger/config/.DS_Store b/modules/bc_api_logger/config/.DS_Store deleted file mode 100644 index 85dc110..0000000 Binary files a/modules/bc_api_logger/config/.DS_Store and /dev/null differ diff --git a/src/CacheableJsonResponseTrait.php b/src/CacheableJsonResponseTrait.php new file mode 100644 index 0000000..f4d8f42 --- /dev/null +++ b/src/CacheableJsonResponseTrait.php @@ -0,0 +1,92 @@ +cacheMetadata)) { + $this->cacheMetadata = new CacheableMetadata(); + } + return $this->cacheMetadata; + } + + /** + * Adds a dependency on an object: merges its cacheability metadata. + * + * For instance, when a response depends on some configuration, an entity, or + * an access result, we must make sure their cacheability metadata is present + * on the response. This method makes doing that simple. + * + * @param \Drupal\Core\Cache\CacheableDependencyInterface|mixed $dependency + * The dependency. If the object implements CacheableDependencyInterface, + * then its cacheability metadata will be used. Otherwise, the passed in + * object must be assumed to be uncacheable, so max-age 0 is set. + * + * @return $this + */ + public function addCacheableDependency($dependency) { + + $this->cacheMetadata = $this->getCacheableMetadata()->merge(CacheableMetadata::createFromObject($dependency)); + + return $this; + } + + /** + * {@inheritdoc} + */ + public function getResourceQueryResult() { + parent::getResourceQueryResult(); + + // If the resource is set, add it as a cacheable dependency. + if (isset($this->resource) && !empty($this->resource)) { + $this->addCacheableDependency($this->resource); + } + } + + /** + * {@inheritdoc} + * + * Override the base classes response, to create a cacheable response. + */ + public function createResponse(): Response { + $response = new CacheableJsonResponse($this->return_data); + + // Attach cache metadata if available. + if ($cache_metadata = $this->getCacheableMetadata()) { + $response->addCacheableDependency($cache_metadata); + } + + // Alter it. + $this->responseAlter($response); + + return $response; + } + +} diff --git a/src/Controller/ApiControllerBase.php b/src/Controller/ApiControllerBase.php index e86e370..41f44ff 100644 --- a/src/Controller/ApiControllerBase.php +++ b/src/Controller/ApiControllerBase.php @@ -464,9 +464,8 @@ final public function getResource(Request $request) { return []; } - $response = new JsonResponse($this->return_data); - // Alter it. - $this->responseAlter($response); + // $response = new JsonResponse($this->return_data); + $response = $this->createResponse(); return $response; } @@ -533,9 +532,7 @@ final public function getResourceList(Request $request) { return []; } - $response = new JsonResponse($this->return_data); - // Alter it. - $this->responseAlter($response); + $response = $this->createResponse(); return $response; } @@ -589,6 +586,19 @@ public function buildLinks() { $this->next = ""; } + /** + * {@inheritdoc} + */ + public function createResponse(): Response { + $response = new JsonResponse($this->return_data); + $response->setStatusCode($this->return_data['status']); + + // Allow subclasses to alter it. + $this->responseAlter($response); + + return $response; + } + /** * {@inheritdoc} */ diff --git a/src/Controller/ApiControllerInterface.php b/src/Controller/ApiControllerInterface.php index 211cc9c..476cf5f 100644 --- a/src/Controller/ApiControllerInterface.php +++ b/src/Controller/ApiControllerInterface.php @@ -74,8 +74,23 @@ public function getResource(Request $request); */ public function getResourceList(Request $request); + /** + * Create the response object. + * + * We allow subclasses to override this method if they need to create + * different types of responses, however, they can also alter the response + * later, if they only need minor changes. + * + * @return Symfony\Component\HttpFoundation\Response + * The response object. + */ + public function createResponse(): Response; + /** * Alter the response object before executing. + * + * @param \Symfony\Component\HttpFoundation\Response $response + * The response object to alter. */ public function responseAlter(Response $response); diff --git a/src/EventSubscriber/ApiSubscriber.php b/src/EventSubscriber/ApiSubscriber.php index 361b558..e377b1a 100644 --- a/src/EventSubscriber/ApiSubscriber.php +++ b/src/EventSubscriber/ApiSubscriber.php @@ -138,9 +138,10 @@ public function onException($event) { public function on400(RequestEvent $event) { $request = $event->getRequest(); + $path = $request->getPathInfo(); $exception = $event->getThrowable(); - if (strpos($request->getRequestUri(), "/api/") === 0 || $request->getRequestUri() == "/api") { + if (strpos($request->getRequestUri(), "/api/") === 0 || $path == "/api") { $data = [ 'status' => (int) $exception->getStatusCode(), 'error_msg' => 'Bad Request', @@ -163,9 +164,10 @@ public function on400(RequestEvent $event) { public function on403(RequestEvent $event) { $request = $event->getRequest(); + $path = $request->getPathInfo(); $exception = $event->getThrowable(); - if (strpos($request->getRequestUri(), "/api/") === 0 || $request->getRequestUri() == "/api") { + if (strpos($request->getRequestUri(), "/api/") === 0 || $path == "/api") { $data = [ 'status' => (int) $exception->getStatusCode(), @@ -189,9 +191,10 @@ public function on403(RequestEvent $event) { public function on404(RequestEvent $event) { $request = $event->getRequest(); + $path = $request->getPathInfo(); $exception = $event->getThrowable(); - if (strpos($request->getRequestUri(), "/api/") === 0 || $request->getRequestUri() == "/api") { + if (strpos($request->getRequestUri(), "/api/") === 0 || $path == "/api") { $data = [ 'status' => (int) $exception->getStatusCode(), 'error_msg' => 'Not Found', @@ -201,7 +204,7 @@ public function on404(RequestEvent $event) { $event->setResponse($response); // Log this call. - $this->loggerFactory->get('bc_api')->error("403: Bad Api call. " . $exception->getMessage(), ["request" => $request, "exception" => $exception]); + $this->loggerFactory->get('bc_api')->error("404: Bad Api call. " . $exception->getMessage(), ["request" => $request, "exception" => $exception]); } } diff --git a/tests/src/.DS_Store b/tests/src/.DS_Store deleted file mode 100644 index ba4994a..0000000 Binary files a/tests/src/.DS_Store and /dev/null differ diff --git a/tests/src/Functional/ResponseTests.php b/tests/src/Functional/ResponseTests.php index 404f5b2..04a70d6 100644 --- a/tests/src/Functional/ResponseTests.php +++ b/tests/src/Functional/ResponseTests.php @@ -30,7 +30,7 @@ class ResponseTests extends BrowserTestBase { */ protected $keyAuthConfig; - protected $defaultTheme = 'classy'; + protected $defaultTheme = 'stable9'; protected $dumpHeaders = TRUE; @@ -55,7 +55,7 @@ class ResponseTests extends BrowserTestBase { /** * {@inheritdoc} */ - protected function setUp() { + protected function setUp(): void { parent::setUp(); $this->keyAuth = $this->container->get('key_auth'); @@ -88,7 +88,7 @@ public function testHttpResponses() { $user = $this->drupalCreateUser(['use api', 'use key authentication']); // Log in. - $this->drupalLogin($user); + // $this->drupalLogin($user); // Call with Query param. $this->drupalGet('api/pirates', [ @@ -112,7 +112,7 @@ public function testResponseData() { $user = $this->drupalCreateUser(['use key authentication', 'use api']); // Log in. - $this->drupalLogin($user); + // $this->drupalLogin($user); // Call with Query param. $data = $this->drupalGet('api/pirates', [ @@ -128,7 +128,7 @@ public function testResponseData() { $this->drupalGet('api/pirates', [], ['api-key' => $user->api_key->value]); $this->assertSession()->statusCodeEquals(200); - Drupal::moduleHandler()->loadInclude('bc_api_example', 'inc', 'bc_api_example.data'); + \Drupal::moduleHandler()->loadInclude('bc_api_example', 'inc', 'bc_api_example.data'); // Check we have a proper result count. $pirates = bc_api_example_get_pirates_data(); @@ -139,4 +139,42 @@ public function testResponseData() { } + /** + * Test the cacheable pirates API endpoint. + */ + public function testCacheablePiratesApiResponse() { + // Create API user. + $user = $this->drupalCreateUser(['use api', 'use key authentication']); + $this->drupalLogin($user); + + // Call the endpoint with the API key as a query param. + $this->drupalGet('api/cacheable/pirates', [ + 'query' => [ + 'api-key' => $user->api_key->value, + ], + ]); + $this->assertSession()->statusCodeEquals(200); + + // Check for JSON content type header. + $headers = $this->getSession()->getResponseHeaders(); + + $this->assertArrayHasKey('Content-Type', $headers); + $this->assertStringContainsString('application/json', $headers['Content-Type'][0]); + + // Check for cache headers. + $this->assertArrayHasKey('X-Drupal-Cache-Tags', $headers, 'Cache tags header is present'); + $this->assertArrayHasKey('Cache-Control', $headers, 'Cache-Control header is present'); + + // Check the response body. + $data = json_decode($this->getSession()->getPage()->getContent(), TRUE); + $this->assertIsArray($data['data']); + $this->assertArrayHasKey('resultTotal', $data); + $this->assertArrayHasKey('data', $data); + + // Check that at least one pirate has a cms_title. + if (!empty($data['data'])) { + $this->assertArrayHasKey('cms_title', $data['data'][0]); + } + } + } diff --git a/tests/src/Unit/CacheableJsonResponseTraitTest.php b/tests/src/Unit/CacheableJsonResponseTraitTest.php new file mode 100644 index 0000000..f304096 --- /dev/null +++ b/tests/src/Unit/CacheableJsonResponseTraitTest.php @@ -0,0 +1,87 @@ + 'bar']) { + return new class($return_data) extends ApiControllerBase { + use CacheableJsonResponseTrait; + + public function __construct($return_data) { + $this->return_data = $return_data; + } + + public function setCacheTags(array $tags) { + $this->getCacheableMetadata()->setCacheTags($tags); + } + + /** + * Public wrapper for testing the protected method. + */ + public function getCacheableMetadataPublic() { + return $this->getCacheableMetadata(); + } + + /** + * Public wrapper for the protected cacheableJsonResponse method. + */ + public function cacheableJsonResponsePublic(Response $response) { + return $this->cacheableJsonResponse($response); + } + + }; + } + + /** + * Tests that getCacheableMetadata returns an instance of CacheableMetadata. + */ + public function testGetCacheableMetadataReturnsObject() { + $controller = $this->getTestController(); + $metadata = $controller->getCacheableMetadataPublic(); + $this->assertInstanceOf(CacheableMetadata::class, $metadata); + } + + /** + * Tests that addCacheableDependency merges cacheable metadata. + */ + public function testSetAndGetCacheTags() { + $controller = $this->getTestController(); + $tags = ['foo:1', 'bar:2']; + $controller->setCacheTags($tags); + $metadata = $controller->getCacheableMetadataPublic(); + $this->assertEquals($tags, $metadata->getCacheTags()); + } + + /** + * Tests that cacheableJsonResponse returns a CacheableJsonResponse. + */ + public function testCreateResponseReturnsCacheableJsonResponse() { + $controller = $this->getTestController(['baz' => 'qux']); + $controller->setCacheTags(['baz:1']); + $response = $controller->createResponse(); + $this->assertInstanceOf(CacheableJsonResponse::class, $response); + $this->assertEquals(json_encode(['baz' => 'qux']), $response->getContent()); + $this->assertEquals(200, $response->getStatusCode()); + // Check cache tags are present. + $metadata = $response->getCacheableMetadata(); + $this->assertContains('baz:1', $metadata->getCacheTags()); + } + +} diff --git a/tests/src/Unit/QueryValidationTests.php b/tests/src/Unit/QueryValidationTests.php index 4cd35cf..a85e058 100644 --- a/tests/src/Unit/QueryValidationTests.php +++ b/tests/src/Unit/QueryValidationTests.php @@ -20,7 +20,7 @@ class QueryValidationTests extends UnitTestCase { /** * {@inheritdoc} */ - public function setUp() { + protected function setUp(): void { // Nothing to do here. parent::setUp(); }