Skip to content

chore: Fix deprecations and tests for symfony 6, graphqlite 6 and php8.1 #174

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

Merged
merged 6 commits into from
Jun 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 14 additions & 25 deletions Command/DumpSchemaCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

namespace TheCodingMachine\GraphQLite\Bundle\Command;

use GraphQL\Type\Definition\TypeWithFields;
use GraphQL\Type\Schema as TypeSchema;
use GraphQL\Utils\SchemaPrinter;
use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
Expand All @@ -16,10 +17,9 @@
/**
* Shamelessly stolen from Api Platform
*/
#[AsCommand('graphqlite:dump-schema')]
class DumpSchemaCommand extends Command
{
protected static $defaultName = 'graphqlite:dump-schema';

/**
* @var Schema
*/
Expand All @@ -43,10 +43,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
{
$io = new SymfonyStyle($input, $output);

// Trying to guarantee deterministic order
$this->sortSchema();

$schemaExport = SchemaPrinter::doPrint($this->schema);
$schemaExport = SchemaPrinterForGraphQLite::doPrint($this->schema, ['sortTypes' => true]);

$filename = $input->getOption('output');
if (\is_string($filename)) {
Expand All @@ -58,24 +55,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int

return 0;
}
}

private function sortSchema(): void
{
$config = $this->schema->getConfig();

$refl = new \ReflectionProperty(TypeWithFields::class, 'fields');
$refl->setAccessible(true);

if ($config->query) {
$fields = $config->query->getFields();
ksort($fields);
$refl->setValue($config->query, $fields);
}
class SchemaPrinterForGraphQLite extends SchemaPrinter {

if ($config->mutation) {
$fields = $config->mutation->getFields();
ksort($fields);
$refl->setValue($config->mutation, $fields);
}
protected static function hasDefaultRootOperationTypes(TypeSchema $schema): bool
{
return $schema->getQueryType() === $schema->getType('Query')
&& $schema->getMutationType() === $schema->getType('Mutation')
// Commenting this out because graphqlite cannot map Subscription type
// && $schema->getSubscriptionType() === $schema->getType('Subscription');
;
}
}
}
3 changes: 3 additions & 0 deletions DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@

class Configuration implements ConfigurationInterface
{
/**
* @return TreeBuilder
*/
public function getConfigTreeBuilder()
{
$treeBuilder = new TreeBuilder('graphqlite');
Expand Down
2 changes: 1 addition & 1 deletion DependencyInjection/GraphQLiteCompilerPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public function process(ContainerBuilder $container): void
// ServerConfig rules
$serverConfigDefinition = $container->findDefinition(ServerConfig::class);
$rulesDefinition = [];
if ($container->getParameter('graphqlite.security.introspection') === false) {
if ($container->getParameter('graphqlite.security.disableIntrospection')) {
$rulesDefinition[] = $container->findDefinition(DisableIntrospection::class);
}

Expand Down
2 changes: 1 addition & 1 deletion DependencyInjection/GraphQLiteExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function($namespace): string {
$container->setParameter('graphqlite.namespace.types', $namespaceType);
$container->setParameter('graphqlite.security.enable_login', $enableLogin);
$container->setParameter('graphqlite.security.enable_me', $enableMe);
$container->setParameter('graphqlite.security.introspection', $config['security']['introspection'] ?? true);
$container->setParameter('graphqlite.security.disableIntrospection', !($config['security']['introspection'] ?? true));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason for inverting the logic here is because i had to pass this argument to the service DisableIntrospection which now takes $enabled as argument which needs to be true for disabling introspection

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$container->setParameter('graphqlite.security.disableIntrospection', !($config['security']['introspection'] ?? true));
$container->setParameter('graphqlite.security.disable_introspection', !($config['security']['introspection'] ?? true));

$container->setParameter('graphqlite.security.maximum_query_complexity', $config['security']['maximum_query_complexity'] ?? null);
$container->setParameter('graphqlite.security.maximum_query_depth', $config['security']['maximum_query_depth'] ?? null);
$container->setParameter('graphqlite.security.firewall_name', $config['security']['firewall_name'] ?? 'main');
Expand Down
6 changes: 6 additions & 0 deletions GraphiQL/EndpointResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public function __construct(RequestStack $requestStack)
$this->requestStack = $requestStack;
}

/**
* @return string
*/
public function getBySchema($name)
{
if ('default' === $name) {
Expand All @@ -31,6 +34,9 @@ public function getBySchema($name)
throw GraphQLEndpointInvalidSchemaException::forSchemaAndResolver($name, self::class);
}

/**
* @return string
*/
public function getDefault()
{
return $this->getBySchema('default');
Expand Down
4 changes: 1 addition & 3 deletions Mappers/RequestParameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@ class RequestParameter implements ParameterInterface
/**
* @param array<string, mixed> $args
* @param mixed $context
*
* @return mixed
*/
public function resolve(?object $source, array $args, $context, ResolveInfo $info)
public function resolve(?object $source, array $args, $context, ResolveInfo $info): mixed
{
if (!$context instanceof SymfonyRequestContextInterface) {
throw new GraphQLException('Cannot type-hint on a Symfony Request object in your query/mutation/field. The request context must implement SymfonyRequestContextInterface.');
Expand Down
4 changes: 3 additions & 1 deletion Resources/config/container/graphqlite.xml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@
</call>
</service>

<service id="GraphQL\Validator\Rules\DisableIntrospection" />
<service id="GraphQL\Validator\Rules\DisableIntrospection">
<argument key="$enabled">%graphqlite.security.disableIntrospection%</argument>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<argument key="$enabled">%graphqlite.security.disableIntrospection%</argument>
<argument key="$enabled">%graphqlite.security.disable_introspection%</argument>

</service>

<service id="GraphQL\Validator\Rules\QueryComplexity" />

Expand Down
4 changes: 1 addition & 3 deletions Server/ServerConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ class ServerConfig extends \GraphQL\Server\ServerConfig
*
* @param ValidationRule[]|callable $validationRules
*
* @return \GraphQL\Server\ServerConfig
*
* @api
*/
public function setValidationRules($validationRules)
public function setValidationRules($validationRules): \GraphQL\Server\ServerConfig
{
parent::setValidationRules(
function (OperationParams $params, DocumentNode $doc, string $operationType) use ($validationRules): array {
Expand Down
21 changes: 5 additions & 16 deletions Tests/Fixtures/Controller/MyException.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,23 @@
namespace TheCodingMachine\GraphQLite\Bundle\Tests\Fixtures\Controller;


use GraphQL\Error\ClientAware;
use TheCodingMachine\GraphQLite\Exceptions\GraphQLExceptionInterface;

class MyException extends \Exception implements ClientAware
class MyException extends \Exception implements GraphQLExceptionInterface
{

/**
* Returns true when exception message is safe to be displayed to a client.
*
* @return bool
*
* @api
*/
public function isClientSafe()
public function isClientSafe(): bool
{
return true;
}

/**
* Returns string describing a category of the error.
*
* Value "graphql" is reserved for errors produced by query parsing or validation, do not use it.
*
* @return string
*
* @api
*/
public function getCategory()
public function getExtensions(): array
{
return 'foobar';
return ['category' => 'foobar'];
}
}
2 changes: 1 addition & 1 deletion Tests/Fixtures/Controller/TestGraphqlController.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function triggerException(int $code = 0): string
public function triggerAggregateException(): string
{
$exception1 = new GraphQLException('foo', 401);
$exception2 = new GraphQLException('bar', 404, null, 'MyCat', ['field' => 'baz']);
$exception2 = new GraphQLException('bar', 404, null, 'MyCat', ['field' => 'baz', 'category' => 'MyCat']);
throw new GraphQLAggregateException([$exception1, $exception2]);
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/FunctionalTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ public function testWithIntrospection(): void

public function testDisableIntrospection(): void
{
$kernel = new GraphQLiteTestingKernel(true, null, true, null, false, 2, 2);
$kernel = new GraphQLiteTestingKernel(true, null, true, null, false, 3, 2);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the webonyx pkg changed how they calculate complexity, it was now failing on 2 raised it to 3

$kernel->boot();

$parameters = ['query' => '
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"require-dev": {
"symfony/security-bundle": "^6",
"symfony/yaml": "^6",
"beberlei/porpaginas": "^1.2",
"beberlei/porpaginas": "^1.2 || ^2.0",
Copy link
Contributor Author

@aszenz aszenz Jun 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowed v2, v2 fixed some deprecation's on php 8.1, I couldn't find any breaking change that could affect this pkg.

We could remove support for v1 also

"php-coveralls/php-coveralls": "^2.1.0",
"symfony/phpunit-bridge": "^6",
"phpstan/phpstan": "^1.8",
Expand Down