-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add APQ plugin #1189
Merged
Merged
feat: add APQ plugin #1189
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3ea3cbb
feat: add APQ plugin
chrfritsch fcf32de
Merge branch '8.x-4.x' into apq_plugin
chrfritsch bfc90b7
test: add a test for the plugin
chrfritsch 01a43ce
fix: cs issues
chrfritsch c827324
fix: make it more robust
chrfritsch b37e144
fix: nobody should care about PHP 7.2
chrfritsch 4501685
Merge branch '8.x-4.x' into apq_plugin
chrfritsch 0d8ae36
Merge branch '8.x-4.x' into apq_plugin
chrfritsch b16a770
fix: use array_key_exists
chrfritsch d00ece8
fix: return type
chrfritsch 656d9aa
fix: cs
chrfritsch 1b98386
Merge branch '8.x-4.x' into apq_plugin
chrfritsch e13a696
Make ApqSubscriber query and query_hash checks type explicit
Kingdutch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
<?php | ||
|
||
namespace Drupal\graphql\EventSubscriber; | ||
|
||
use Drupal\Core\Cache\CacheBackendInterface; | ||
use Drupal\graphql\Event\OperationEvent; | ||
use Symfony\Component\EventDispatcher\EventSubscriberInterface; | ||
use GraphQL\Error\Error; | ||
|
||
/** | ||
* Save persisted queries to cache. | ||
*/ | ||
class ApqSubscriber implements EventSubscriberInterface { | ||
|
||
/** | ||
* The cache to store persisted queries. | ||
* | ||
* @var \Drupal\Core\Cache\CacheBackendInterface | ||
*/ | ||
protected $cache; | ||
|
||
/** | ||
* Constructs a ApqSubscriber object. | ||
* | ||
* @param \Drupal\Core\Cache\CacheBackendInterface $cache | ||
* The cache to store persisted queries. | ||
*/ | ||
public function __construct(CacheBackendInterface $cache) { | ||
$this->cache = $cache; | ||
} | ||
|
||
/** | ||
* Handle operation start events. | ||
* | ||
* @param \Drupal\graphql\Event\OperationEvent $event | ||
* The kernel event object. | ||
* | ||
* @throws \GraphQL\Error\Error | ||
*/ | ||
public function onBeforeOperation(OperationEvent $event): void { | ||
if (!array_key_exists('automatic_persisted_query', $event->getContext()->getServer()->getPersistedQueryInstances() ?? [])) { | ||
return; | ||
} | ||
$query = $event->getContext()->getOperation()->query; | ||
$queryHash = $event->getContext()->getOperation()->extensions['persistedQuery']['sha256Hash'] ?? ''; | ||
|
||
if ($query && $queryHash) { | ||
Kingdutch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$computedQueryHash = hash('sha256', $query); | ||
if ($queryHash !== $computedQueryHash) { | ||
throw new Error('Provided sha does not match query'); | ||
} | ||
$this->cache->set($queryHash, $query); | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public static function getSubscribedEvents() { | ||
return [ | ||
OperationEvent::GRAPHQL_OPERATION_BEFORE => 'onBeforeOperation', | ||
]; | ||
} | ||
|
||
} |
56 changes: 56 additions & 0 deletions
56
src/Plugin/GraphQL/PersistedQuery/AutomaticPersistedQuery.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
<?php | ||
|
||
namespace Drupal\graphql\Plugin\GraphQL\PersistedQuery; | ||
|
||
use Drupal\Core\Cache\CacheBackendInterface; | ||
use Drupal\Core\Plugin\ContainerFactoryPluginInterface; | ||
use Drupal\graphql\PersistedQuery\PersistedQueryPluginBase; | ||
use GraphQL\Server\OperationParams; | ||
use GraphQL\Server\RequestError; | ||
use Symfony\Component\DependencyInjection\ContainerInterface; | ||
|
||
/** | ||
* Load persisted queries from the cache. | ||
* | ||
* @PersistedQuery( | ||
* id = "automatic_persisted_query", | ||
* label = "Automatic Persisted Query", | ||
* description = "Load persisted queries from the cache." | ||
* ) | ||
*/ | ||
class AutomaticPersistedQuery extends PersistedQueryPluginBase implements ContainerFactoryPluginInterface { | ||
|
||
/** | ||
* The cache to store persisted queries. | ||
* | ||
* @var \Drupal\Core\Cache\CacheBackendInterface | ||
*/ | ||
protected $cache; | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function __construct(array $configuration, $plugin_id, $plugin_definition, CacheBackendInterface $cache) { | ||
parent::__construct($configuration, $plugin_id, $plugin_definition); | ||
|
||
$this->cache = $cache; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { | ||
return new static($configuration, $plugin_id, $plugin_definition, $container->get('cache.graphql.apq')); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function getQuery($id, OperationParams $operation) { | ||
if ($query = $this->cache->get($id)) { | ||
return $query->data; | ||
} | ||
throw new RequestError('PersistedQueryNotFound'); | ||
} | ||
|
||
} |
96 changes: 96 additions & 0 deletions
96
tests/src/Kernel/Framework/AutomaticPersistedQueriesTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
<?php | ||
|
||
namespace Drupal\Tests\graphql\Kernel\Framework; | ||
|
||
use Drupal\Tests\graphql\Kernel\GraphQLTestBase; | ||
use Symfony\Component\HttpFoundation\Request; | ||
|
||
/** | ||
* Tests the automatic persisted query plugin. | ||
* | ||
* @group graphql | ||
*/ | ||
class AutomaticPersistedQueriesTest extends GraphQLTestBase { | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
protected function setUp(): void { | ||
parent::setUp(); | ||
$schema = <<<GQL | ||
schema { | ||
query: Query | ||
} | ||
type Query { | ||
field_one: String | ||
} | ||
GQL; | ||
|
||
$this->setUpSchema($schema); | ||
$this->mockResolver('Query', 'field_one', 'this is the field one'); | ||
|
||
/** @var \Drupal\graphql\Plugin\DataProducerPluginManager $manager */ | ||
$manager = $this->container->get('plugin.manager.graphql.persisted_query'); | ||
|
||
$this->plugin_apq = $manager->createInstance('automatic_persisted_query'); | ||
} | ||
|
||
/** | ||
* Test the automatic persisted queries plugin. | ||
*/ | ||
public function testAutomaticPersistedQueries(): void { | ||
// Before adding the persisted query plugins to the server, we want to make | ||
// sure that there are no existing plugins already there. | ||
$this->server->removeAllPersistedQueryInstances(); | ||
$this->server->addPersistedQueryInstance($this->plugin_apq); | ||
$this->server->save(); | ||
|
||
$endpoint = $this->server->get('endpoint'); | ||
|
||
$query = 'query { field_one } '; | ||
$parameters['extensions']['persistedQuery']['sha256Hash'] = 'some random hash'; | ||
|
||
// Check we get PersistedQueryNotFound. | ||
$request = Request::create($endpoint, 'GET', $parameters); | ||
$result = $this->container->get('http_kernel')->handle($request); | ||
$this->assertSame(200, $result->getStatusCode()); | ||
$this->assertSame([ | ||
'errors' => [ | ||
[ | ||
'message' => 'PersistedQueryNotFound', | ||
'extensions' => ['category' => 'request'], | ||
], | ||
], | ||
], json_decode($result->getContent(), TRUE)); | ||
|
||
// Post query to endpoint with a not matching hash. | ||
$content = json_encode(['query' => $query] + $parameters); | ||
$request = Request::create($endpoint, 'POST', [], [], [], [], $content); | ||
$result = $this->container->get('http_kernel')->handle($request); | ||
$this->assertSame(200, $result->getStatusCode()); | ||
$this->assertSame([ | ||
'errors' => [ | ||
[ | ||
'message' => 'Provided sha does not match query', | ||
'extensions' => ['category' => 'graphql'], | ||
], | ||
], | ||
], json_decode($result->getContent(), TRUE)); | ||
|
||
// Post query to endpoint to get the result and cache it. | ||
$parameters['extensions']['persistedQuery']['sha256Hash'] = hash('sha256', $query); | ||
|
||
$content = json_encode(['query' => $query] + $parameters); | ||
$request = Request::create($endpoint, 'POST', [], [], [], [], $content); | ||
$result = $this->container->get('http_kernel')->handle($request); | ||
$this->assertSame(200, $result->getStatusCode()); | ||
$this->assertSame(['data' => ['field_one' => 'this is the field one']], json_decode($result->getContent(), TRUE)); | ||
|
||
// Execute first request again. | ||
$request = Request::create($endpoint, 'GET', $parameters); | ||
$result = $this->container->get('http_kernel')->handle($request); | ||
$this->assertSame(200, $result->getStatusCode()); | ||
$this->assertSame(['data' => ['field_one' => 'this is the field one']], json_decode($result->getContent(), TRUE)); | ||
} | ||
|
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would technically be a breaking change. Since the following code would break:
I suspect this is something suggested by PHPStan? I can see where it comes from in the Server implementation. That implementation uses PHP's "convert NULL to empty array on array access" side-effect.
I think the better (and more ergonomic for callers) fix would be to add
$this-> persisted_query_instances = [];
, right after theis_null
if statement's closing bracked at the top of the function. That has a few positive effects:getPersistedQueryInstances
is called (which happens now, because in the no-plugin case,$this->persisted_query_instances
is stillNULL
when the function is called after the first time).For clarity we should probably just split that change out into a separate PR since it's not necessarily related to APQ? Then this PR doesn't need to be blocked by that change either :)