Skip to content

Commit

Permalink
fix: RecursiveTypeMapper returns exception which can't be handled by …
Browse files Browse the repository at this point in the history
…StandardServer (#646)

* fix: RecursiveTypeMapper returns exception which can't be handled by StandardServer

RecursiveTypeMapper throws CannotMapTypeException when fails to find class by name. This exception goes through the GraphQl executor instead of catching and wrapping into ExecutionResult. Non wrapped exception does not trigger HttpCodeDecider.
Schema test trust more to input than configuration which is not optimal, great possibility to improve configuration validation.

Fixes: #336

* feat: new special Exception class when request type unknown

Fixes: #336

* fix: move asserts

Fixes: #336

* polish changes

Fixes: #336
  • Loading branch information
fogrye authored Feb 11, 2024
1 parent 6f31b08 commit 8c97363
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/Mappers/RecursiveTypeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,6 @@ public function mapNameToType(string $typeName): Type&NamedType
return $this->mapClassToInterfaceOrType($className, null);
}

throw CannotMapTypeException::createForName($typeName);
throw TypeNotFoundException::createError($typeName);
}
}
23 changes: 23 additions & 0 deletions src/Mappers/TypeNotFoundException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace TheCodingMachine\GraphQLite\Mappers;

use GraphQL\Error\Error;
use GraphQL\Utils\Utils;

/**
* TypeNotFoundException thrown when RecursiveTypeMapper fails to find a type.
*
* While CannotMapTypeException is more about error with configuration this exception can occur when request
* contains a type which is unknown to the server.
* Should not be handled by the user, webonyx/graphql-php will transform this under 'errors' key in the response.
*/
class TypeNotFoundException extends Error
{
public static function createError(string $typeName): Error
{
return static::createLocatedError('Unknown type ' . Utils::printSafe($typeName));
}
}
6 changes: 4 additions & 2 deletions tests/Mappers/RecursiveTypeMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public function testMapNameToType2(): void
$this->assertSame('ClassA', $recursiveMapper->mapNameToType('ClassA')->name);
$this->assertSame('ClassAInterface', $recursiveMapper->mapNameToType('ClassAInterface')->name);

$this->expectException(CannotMapTypeException::class);
$this->expectException(TypeNotFoundException::class);
$recursiveMapper->mapNameToType('NotExists');
}

Expand Down Expand Up @@ -233,6 +233,8 @@ public function testDuplicateDetection(): void

/**
* Tests that the RecursiveTypeMapper behaves correctly if there are no types to map.
*
* @see \GraphQL\Server\Helper::promiseToExecuteOperation()
*/
public function testMapNoTypes(): void
{
Expand All @@ -244,7 +246,7 @@ public function testMapNoTypes(): void
$this->getAnnotationReader()
);

$this->expectException(CannotMapTypeException::class);
$this->expectException(TypeNotFoundException::class);
$recursiveTypeMapper->mapNameToType('Foo');
}

Expand Down
69 changes: 42 additions & 27 deletions tests/SchemaFactoryTest.php
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
<?php

declare(strict_types=1);

namespace TheCodingMachine\GraphQLite;

use Doctrine\Common\Annotations\AnnotationReader;
use GraphQL\Error\DebugFlag;
use GraphQL\Executor\ExecutionResult;
use GraphQL\GraphQL;
use GraphQL\Type\SchemaConfig;
use Mouf\Composer\ClassNameMapper;
Expand All @@ -20,22 +24,19 @@
use TheCodingMachine\GraphQLite\Fixtures\Integration\Types\ContactOtherType;
use TheCodingMachine\GraphQLite\Fixtures\Integration\Types\ContactType;
use TheCodingMachine\GraphQLite\Fixtures\Integration\Types\ExtendedContactType;
use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException;
use TheCodingMachine\GraphQLite\Fixtures\TestSelfType;
use TheCodingMachine\GraphQLite\Mappers\CompositeTypeMapper;
use TheCodingMachine\GraphQLite\Mappers\DuplicateMappingException;
use TheCodingMachine\GraphQLite\Mappers\Parameters\ParameterMiddlewarePipe;
use TheCodingMachine\GraphQLite\Mappers\Root\VoidRootTypeMapperFactory;
use TheCodingMachine\GraphQLite\Mappers\StaticClassListTypeMapperFactory;
use TheCodingMachine\GraphQLite\Middlewares\FieldMiddlewarePipe;
use TheCodingMachine\GraphQLite\Mappers\Parameters\ParameterMiddlewarePipe;
use TheCodingMachine\GraphQLite\Middlewares\InputFieldMiddlewarePipe;
use TheCodingMachine\GraphQLite\Security\VoidAuthenticationService;
use TheCodingMachine\GraphQLite\Security\VoidAuthorizationService;
use TheCodingMachine\GraphQLite\Fixtures\TestSelfType;


class SchemaFactoryTest extends TestCase
{

public function testCreateSchema(): void
{
$container = new BasicAutoWiringContainer(new EmptyContainer());
Expand Down Expand Up @@ -65,7 +66,7 @@ public function testSetters(): void

$factory->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration\\Controllers');
$factory->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration');
$factory->setDoctrineAnnotationReader(new \Doctrine\Common\Annotations\AnnotationReader())
$factory->setDoctrineAnnotationReader(new AnnotationReader())
->setAuthenticationService(new VoidAuthenticationService())
->setAuthorizationService(new VoidAuthorizationService())
->setNamingStrategy(new NamingStrategy())
Expand All @@ -89,8 +90,8 @@ public function testClassNameMapperInjectionWithValidMapper(): void
$factory = new SchemaFactory(
new Psr16Cache(new ArrayAdapter()),
new BasicAutoWiringContainer(
new EmptyContainer()
)
new EmptyContainer(),
),
);
$factory->setAuthenticationService(new VoidAuthenticationService())
->setAuthorizationService(new VoidAuthorizationService())
Expand Down Expand Up @@ -132,17 +133,16 @@ public function testClassNameMapperInjectionWithInvalidMapper(): void
$factory = new SchemaFactory(
new Psr16Cache(new ArrayAdapter()),
new BasicAutoWiringContainer(
new EmptyContainer()
)
new EmptyContainer(),
),
);
$factory->setAuthenticationService(new VoidAuthenticationService())
->setAuthorizationService(new VoidAuthorizationService())
->setClassNameMapper(new ClassNameMapper())
->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration\\Controllers')
->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration');

$this->expectException(CannotMapTypeException::class);
$this->doTestSchema($factory->createSchema());
$this->doTestSchemaWithError($factory->createSchema());
}

public function testException(): void
Expand All @@ -168,9 +168,8 @@ public function testException2(): void
$factory->createSchema();
}

private function doTestSchema(Schema $schema): void
private function execTestQuery(Schema $schema): ExecutionResult
{

$schema->assertValid();

$queryString = '
Expand All @@ -185,34 +184,50 @@ private function doTestSchema(Schema $schema): void
}
';

$result = GraphQL::executeQuery(
return GraphQL::executeQuery(
$schema,
$queryString
$queryString,
);
}

private function doTestSchemaWithError(Schema $schema): void
{
$result = $this->execTestQuery($schema);
$resultArr = $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS);
$this->assertArrayHasKey('errors', $resultArr);
$this->assertArrayNotHasKey('data', $resultArr);
$this->assertCount(1, $resultArr);
$this->assertSame('Unknown type "User"', $resultArr['errors'][0]['message']);
}

private function doTestSchema(Schema $schema): void
{
$result = $this->execTestQuery($schema);
$resultArr = $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS);
$this->assertArrayHasKey('data', $resultArr);
$this->assertSame([
'contacts' => [
[
'name' => 'Joe',
'uppercaseName' => 'JOE'
'uppercaseName' => 'JOE',
],
[
'name' => 'Bill',
'uppercaseName' => 'BILL',
'email' => 'bill@example.com'
]
'email' => 'bill@example.com',
],

]
], $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS)['data']);
],
], $resultArr['data']);
}

public function testDuplicateQueryException(): void
{
$factory = new SchemaFactory(
new Psr16Cache(new ArrayAdapter()),
new BasicAutoWiringContainer(
new EmptyContainer()
)
new EmptyContainer(),
),
);
$factory->setAuthenticationService(new VoidAuthenticationService())
->setAuthorizationService(new VoidAuthorizationService())
Expand All @@ -229,7 +244,7 @@ public function testDuplicateQueryException(): void
';
GraphQL::executeQuery(
$schema,
$queryString
$queryString,
);
}

Expand All @@ -238,8 +253,8 @@ public function testDuplicateQueryInTwoControllersException(): void
$factory = new SchemaFactory(
new Psr16Cache(new ArrayAdapter()),
new BasicAutoWiringContainer(
new EmptyContainer()
)
new EmptyContainer(),
),
);
$factory->setAuthenticationService(new VoidAuthenticationService())
->setAuthorizationService(new VoidAuthorizationService())
Expand All @@ -256,7 +271,7 @@ public function testDuplicateQueryInTwoControllersException(): void
';
GraphQL::executeQuery(
$schema,
$queryString
$queryString,
);
}
}

0 comments on commit 8c97363

Please sign in to comment.