Skip to content

Commit d3e0b9c

Browse files
oojacoboooojacoboo
committed
Fix MutableInterfaceType not frozen when used only in mutation return types (#308)
Two code paths in RecursiveTypeMapper allowed MutableInterfaceType to escape the freeze mechanism: 1. findInterfaces() called the underlying typeMapper directly, creating orphaned instances never registered in TypeRegistry or frozen. 2. mapNameToType() handled MutableObjectType and input types but not MutableInterfaceType in its freeze logic.
1 parent 2b84d38 commit d3e0b9c

5 files changed

Lines changed: 212 additions & 2 deletions

File tree

‎src/Mappers/RecursiveTypeMapper.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,14 @@ public function findInterfaces(string $className): array
243243
continue;
244244
}
245245

246+
// Map through RecursiveTypeMapper's pipeline to ensure the interface
247+
// is registered in the TypeRegistry, extended, and frozen.
248+
// Calling the underlying typeMapper directly would create an orphaned,
249+
// unfrozen instance that is never registered in the TypeRegistry.
250+
$this->mapClassToType($interface, null);
251+
252+
// Now retrieve the frozen MutableInterfaceType. TypeGenerator returns
253+
// the same instance from the TypeRegistry that mapClassToType registered.
246254
$interfaceType = $this->typeMapper->mapClassToType($interface, null);
247255

248256
assert($interfaceType instanceof MutableInterfaceType);
@@ -489,14 +497,14 @@ public function mapNameToType(string $typeName): Type&NamedType
489497
if ($cachedType !== $type) {
490498
throw new RuntimeException('Cached type in registry is not the type returned by type mapper.');
491499
}
492-
if ($cachedType instanceof MutableObjectType && $cachedType->getStatus() === MutableObjectType::STATUS_FROZEN) {
500+
if ($cachedType instanceof MutableInterface && $cachedType->getStatus() === MutableInterface::STATUS_FROZEN) {
493501
return $type;
494502
}
495503
}
496504

497505
$this->typeRegistry->registerType($type);
498506

499-
if ($type instanceof MutableObjectType) {
507+
if ($type instanceof MutableObjectType || $type instanceof MutableInterfaceType) {
500508
if ($this->typeMapper->canExtendTypeForName($typeName, $type)) {
501509
$this->typeMapper->extendTypeForName($typeName, $type);
502510
}

‎tests/Fixtures/MutationInterfaceFreeze/Controllers/MutationOnlyController.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TheCodingMachine\GraphQLite\Fixtures\MutationInterfaceFreeze\Controllers;
6+
7+
use TheCodingMachine\GraphQLite\Annotations\Mutation;
8+
use TheCodingMachine\GraphQLite\Fixtures\MutationInterfaceFreeze\Types\ConcreteResult;
9+
use TheCodingMachine\GraphQLite\Fixtures\MutationInterfaceFreeze\Types\ResultInterface;
10+
11+
class MutationOnlyController
12+
{
13+
#[Mutation]
14+
public function mutateResult(): ResultInterface
15+
{
16+
return new ConcreteResult('success');
17+
}
18+
}

‎tests/Fixtures/MutationInterfaceFreeze/Types/ConcreteResult.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TheCodingMachine\GraphQLite\Fixtures\MutationInterfaceFreeze\Types;
6+
7+
use TheCodingMachine\GraphQLite\Annotations\Field;
8+
use TheCodingMachine\GraphQLite\Annotations\Type;
9+
10+
#[Type]
11+
class ConcreteResult implements ResultInterface
12+
{
13+
public function __construct(private readonly string $message)
14+
{
15+
}
16+
17+
#[Field]
18+
public function getMessage(): string
19+
{
20+
return $this->message;
21+
}
22+
23+
#[Field]
24+
public function getExtra(): string
25+
{
26+
return 'extra';
27+
}
28+
}

‎tests/Fixtures/MutationInterfaceFreeze/Types/ResultInterface.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TheCodingMachine\GraphQLite\Fixtures\MutationInterfaceFreeze\Types;
6+
7+
use TheCodingMachine\GraphQLite\Annotations\Field;
8+
use TheCodingMachine\GraphQLite\Annotations\Type;
9+
10+
#[Type]
11+
interface ResultInterface
12+
{
13+
#[Field]
14+
public function getMessage(): string;
15+
}

‎tests/Integration/MutationInterfaceFreezeTest.php‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TheCodingMachine\GraphQLite\Integration;
6+
7+
use GraphQL\Error\DebugFlag;
8+
use GraphQL\GraphQL;
9+
use PHPUnit\Framework\TestCase;
10+
use Symfony\Component\Cache\Adapter\ArrayAdapter;
11+
use Symfony\Component\Cache\Psr16Cache;
12+
use TheCodingMachine\GraphQLite\Containers\BasicAutoWiringContainer;
13+
use TheCodingMachine\GraphQLite\Containers\EmptyContainer;
14+
use TheCodingMachine\GraphQLite\Schema;
15+
use TheCodingMachine\GraphQLite\SchemaFactory;
16+
17+
/**
18+
* Regression test for issue #308: MutableInterfaceType not frozen when used
19+
* exclusively as a mutation return type.
20+
*
21+
* @see https://github.com/thecodingmachine/graphqlite/issues/308
22+
*/
23+
class MutationInterfaceFreezeTest extends TestCase
24+
{
25+
private Schema $schema;
26+
27+
public function setUp(): void
28+
{
29+
$container = new BasicAutoWiringContainer(new EmptyContainer());
30+
31+
$schemaFactory = new SchemaFactory(new Psr16Cache(new ArrayAdapter()), $container);
32+
$schemaFactory->addNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\MutationInterfaceFreeze');
33+
34+
$this->schema = $schemaFactory->createSchema();
35+
}
36+
37+
/**
38+
* Schema validation triggers getTypeMap() which traverses all types.
39+
* Before the fix, this would throw:
40+
* "You must freeze() a MutableObjectType before fetching its fields."
41+
*/
42+
public function testSchemaValidationDoesNotThrowForMutationOnlyInterface(): void
43+
{
44+
$this->schema->assertValid();
45+
$this->addToAssertionCount(1);
46+
}
47+
48+
/**
49+
* Executing a mutation that returns an interface type should work
50+
* without any query also returning that interface.
51+
*/
52+
public function testMutationReturningInterfaceCanBeExecuted(): void
53+
{
54+
$queryString = '
55+
mutation {
56+
mutateResult {
57+
message
58+
}
59+
}
60+
';
61+
62+
$result = GraphQL::executeQuery(
63+
$this->schema,
64+
$queryString,
65+
);
66+
67+
$this->assertSame(
68+
['mutateResult' => ['message' => 'success']],
69+
$result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS)['data'],
70+
);
71+
}
72+
73+
/**
74+
* Inline fragments on an interface type in a mutation should work.
75+
* Before the fix, the OverlappingFieldsCanBeMerged validator would
76+
* access the unfrozen MutableInterfaceType and crash.
77+
*/
78+
public function testMutationWithInlineFragmentOnInterface(): void
79+
{
80+
$queryString = '
81+
mutation {
82+
mutateResult {
83+
... on ResultInterface {
84+
message
85+
}
86+
... on ConcreteResult {
87+
message
88+
extra
89+
}
90+
}
91+
}
92+
';
93+
94+
$result = GraphQL::executeQuery(
95+
$this->schema,
96+
$queryString,
97+
);
98+
99+
$this->assertSame(
100+
[
101+
'mutateResult' => [
102+
'message' => 'success',
103+
'extra' => 'extra',
104+
],
105+
],
106+
$result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS)['data'],
107+
);
108+
}
109+
110+
/**
111+
* Introspection must include the mutation return interface type
112+
* and it must be resolvable without errors.
113+
*/
114+
public function testIntrospectionIncludesMutationInterfaceType(): void
115+
{
116+
$queryString = '
117+
{
118+
__type(name: "ResultInterface") {
119+
kind
120+
name
121+
fields {
122+
name
123+
}
124+
}
125+
}
126+
';
127+
128+
$result = GraphQL::executeQuery(
129+
$this->schema,
130+
$queryString,
131+
);
132+
133+
$data = $result->toArray(DebugFlag::RETHROW_INTERNAL_EXCEPTIONS)['data'];
134+
$this->assertSame('INTERFACE', $data['__type']['kind']);
135+
$this->assertSame('ResultInterface', $data['__type']['name']);
136+
$this->assertContains(
137+
['name' => 'message'],
138+
$data['__type']['fields'],
139+
);
140+
}
141+
}

0 commit comments

Comments
 (0)