Skip to content

Commit f55bb82

Browse files
soyukasoyuka
committed
fix(mcp): drop required @id in single-item output schema via gen_id
1 parent 73a57b3 commit f55bb82

4 files changed

Lines changed: 92 additions & 19 deletions

File tree

‎src/Hydra/JsonSchema/SchemaFactory.php‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@
2222
use ApiPlatform\JsonSchema\SchemaFactoryAwareInterface;
2323
use ApiPlatform\JsonSchema\SchemaFactoryInterface;
2424
use ApiPlatform\JsonSchema\SchemaUriPrefixTrait;
25-
use ApiPlatform\Metadata\CollectionOperationInterface;
26-
use ApiPlatform\Metadata\McpResource;
27-
use ApiPlatform\Metadata\McpTool;
2825
use ApiPlatform\Metadata\Operation;
2926
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
3027

@@ -136,7 +133,7 @@ public function buildSchema(string $className, string $format = 'jsonld', string
136133

137134
if (!$collectionKey) {
138135
$definitionName = $schema->getRootDefinitionKey() ?? $this->definitionNameFactory->create($className, $format, $inputOrOutputClass, $operation, $serializerContext);
139-
$this->decorateItemDefinition($definitionName, $definitions, $prefix, $type, $serializerContext, $operation);
136+
$this->decorateItemDefinition($definitionName, $definitions, $prefix, $type, $serializerContext);
140137

141138
if (isset($definitions[$definitionName])) {
142139
$currentDefinitions = $schema->getDefinitions();
@@ -257,7 +254,7 @@ public function buildSchema(string $className, string $format = 'jsonld', string
257254
unset($schema['items']);
258255

259256
if (isset($definitions[$collectionKey])) {
260-
$this->decorateItemDefinition($collectionKey, $definitions, $prefix, $type, $serializerContext, $operation);
257+
$this->decorateItemDefinition($collectionKey, $definitions, $prefix, $type, $serializerContext);
261258
}
262259

263260
return $schema;
@@ -270,16 +267,13 @@ public function setSchemaFactory(SchemaFactoryInterface $schemaFactory): void
270267
}
271268
}
272269

273-
private function decorateItemDefinition(string $definitionName, \ArrayObject $definitions, string $prefix, string $type, ?array $serializerContext, ?Operation $operation = null): void
270+
private function decorateItemDefinition(string $definitionName, \ArrayObject $definitions, string $prefix, string $type, ?array $serializerContext): void
274271
{
275272
if (!isset($definitions[$definitionName])) {
276273
return;
277274
}
278275

279-
// A single-item MCP operation has no routed item URI: the serializer omits `@id`, so don't require it.
280-
$isMcpItem = ($operation instanceof McpTool || $operation instanceof McpResource) && !$operation instanceof CollectionOperationInterface;
281-
282-
$hasNoId = Schema::TYPE_OUTPUT === $type && (false === ($serializerContext['gen_id'] ?? true) || $isMcpItem);
276+
$hasNoId = Schema::TYPE_OUTPUT === $type && false === ($serializerContext['gen_id'] ?? true);
283277
$baseName = self::ITEM_BASE_SCHEMA_NAME;
284278
if ($hasNoId) {
285279
$baseName = self::ITEM_WITHOUT_ID_BASE_SCHEMA_NAME;

‎src/Hydra/Tests/JsonSchema/SchemaFactoryTest.php‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
use ApiPlatform\Metadata\ApiResource;
2424
use ApiPlatform\Metadata\Get;
2525
use ApiPlatform\Metadata\GetCollection;
26-
use ApiPlatform\Metadata\McpTool;
27-
use ApiPlatform\Metadata\McpToolCollection;
2826
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
2927
use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
3028
use ApiPlatform\Metadata\Property\PropertyNameCollection;
@@ -163,10 +161,10 @@ public function testSchemaTypeBuildSchema(): void
163161
$this->assertEquals($resultSchema['allOf'][0]['$ref'], $forcedCollection['allOf'][0]['$ref']);
164162
}
165163

166-
// Single-item McpTool: serializer omits `@id`, so the output schema must not require it.
167-
public function testSingleItemMcpToolOutputSchemaDoesNotRequireId(): void
164+
// gen_id=false output schema must not require `@id` (e.g. an operation whose serializer omits the IRI).
165+
public function testGenIdFalseOutputSchemaDoesNotRequireId(): void
168166
{
169-
$resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new McpTool(class: Dummy::class));
167+
$resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new Get(), null, ['gen_id' => false]);
170168

171169
$definitions = $resultSchema->getDefinitions();
172170
$rootDefinitionKey = $resultSchema->getRootDefinitionKey();
@@ -177,14 +175,15 @@ public function testSingleItemMcpToolOutputSchemaDoesNotRequireId(): void
177175
$this->assertArrayNotHasKey('HydraItemBaseSchema', $definitions);
178176
}
179177

180-
// McpToolCollection: members get `@id` (member normalization sets item_uri_template), so it stays required.
181-
public function testMcpToolCollectionOutputSchemaStillRequiresIdOnMembers(): void
178+
// Default (gen_id left to its true default): the output schema keeps `@id` required.
179+
public function testOutputSchemaRequiresIdByDefault(): void
182180
{
183-
$resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new McpToolCollection(class: Dummy::class));
181+
$resultSchema = $this->schemaFactory->buildSchema(Dummy::class, 'jsonld', Schema::TYPE_OUTPUT, new Get());
184182

185183
$definitions = $resultSchema->getDefinitions();
184+
$rootDefinitionKey = $resultSchema->getRootDefinitionKey();
186185

187-
$this->assertSame(['$ref' => '#/definitions/HydraItemBaseSchema'], $definitions['Dummy.jsonld']['allOf'][0]);
186+
$this->assertSame(['$ref' => '#/definitions/HydraItemBaseSchema'], $definitions[$rootDefinitionKey]['allOf'][0]);
188187
$this->assertSame(['@id', '@type'], $definitions['HydraItemBaseSchema']['required']);
189188
$this->assertArrayNotHasKey('HydraItemBaseSchemaWithoutId', $definitions);
190189
}

‎src/Mcp/JsonSchema/SchemaFactory.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
use ApiPlatform\JsonSchema\Schema;
1717
use ApiPlatform\JsonSchema\SchemaFactoryInterface;
18+
use ApiPlatform\Metadata\CollectionOperationInterface;
19+
use ApiPlatform\Metadata\McpResource;
20+
use ApiPlatform\Metadata\McpTool;
1821
use ApiPlatform\Metadata\Operation;
1922

2023
/**
@@ -32,6 +35,16 @@ public function __construct(
3235

3336
public function buildSchema(string $className, string $format = 'json', string $type = Schema::TYPE_OUTPUT, ?Operation $operation = null, ?Schema $schema = null, ?array $serializerContext = null, bool $forceCollection = false): Schema
3437
{
38+
// A single-item MCP operation has no routed item URI (Mcp\Routing\IriConverter returns null),
39+
// so the serializer omits `@id`: mirror that here so the advertised output schema doesn't require it.
40+
if (Schema::TYPE_OUTPUT === $type
41+
&& ($operation instanceof McpTool || $operation instanceof McpResource)
42+
&& !$operation instanceof CollectionOperationInterface
43+
) {
44+
$serializerContext ??= [];
45+
$serializerContext['gen_id'] = false;
46+
}
47+
3548
$schema = $this->decorated->buildSchema($className, $format, $type, $operation, $schema, $serializerContext, $forceCollection);
3649

3750
$definitions = [];

‎src/Mcp/Tests/JsonSchema/SchemaFactoryTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
use ApiPlatform\JsonSchema\Schema;
1717
use ApiPlatform\JsonSchema\SchemaFactoryInterface;
1818
use ApiPlatform\Mcp\JsonSchema\SchemaFactory;
19+
use ApiPlatform\Metadata\McpResource;
20+
use ApiPlatform\Metadata\McpTool;
21+
use ApiPlatform\Metadata\McpToolCollection;
1922
use PHPUnit\Framework\TestCase;
2023

2124
class SchemaFactoryTest extends TestCase
@@ -422,4 +425,68 @@ public function testRefInsideAnyOfIsResolved(): void
422425
$this->assertSame(['type' => 'null'], $related['anyOf'][1]);
423426
$this->assertArrayNotHasKey('type', $related);
424427
}
428+
429+
/**
430+
* A single-item MCP operation has no routed item URI (Mcp\Routing\IriConverter returns null),
431+
* so the output schema must advertise `gen_id` as false to drop the required `@id`.
432+
*/
433+
public function testSingleItemMcpOperationForcesGenIdFalse(): void
434+
{
435+
foreach ([new McpTool(class: 'App\\Dummy'), new McpResource(uri: 'app://dummy', class: 'App\\Dummy')] as $operation) {
436+
$captured = null;
437+
$inner = $this->createMock(SchemaFactoryInterface::class);
438+
$inner->method('buildSchema')->willReturnCallback(function (...$args) use (&$captured) {
439+
$captured = $args[5] ?? null;
440+
441+
return $this->emptyObjectSchema();
442+
});
443+
444+
$factory = new SchemaFactory($inner);
445+
$factory->buildSchema('App\\Dummy', 'jsonld', Schema::TYPE_OUTPUT, $operation);
446+
447+
$this->assertFalse($captured['gen_id'] ?? null, $operation::class.' must force gen_id to false');
448+
}
449+
}
450+
451+
public function testMcpToolCollectionDoesNotForceGenIdFalse(): void
452+
{
453+
$captured = 'untouched';
454+
$inner = $this->createMock(SchemaFactoryInterface::class);
455+
$inner->method('buildSchema')->willReturnCallback(function (...$args) use (&$captured) {
456+
$captured = $args[5] ?? null;
457+
458+
return $this->emptyObjectSchema();
459+
});
460+
461+
$factory = new SchemaFactory($inner);
462+
$factory->buildSchema('App\\Dummy', 'jsonld', Schema::TYPE_OUTPUT, new McpToolCollection(class: 'App\\Dummy'));
463+
464+
$this->assertNull($captured);
465+
}
466+
467+
public function testInputSchemaDoesNotForceGenIdFalse(): void
468+
{
469+
$captured = 'untouched';
470+
$inner = $this->createMock(SchemaFactoryInterface::class);
471+
$inner->method('buildSchema')->willReturnCallback(function (...$args) use (&$captured) {
472+
$captured = $args[5] ?? null;
473+
474+
return $this->emptyObjectSchema();
475+
});
476+
477+
$factory = new SchemaFactory($inner);
478+
$factory->buildSchema('App\\Dummy', 'json', Schema::TYPE_INPUT, new McpTool(class: 'App\\Dummy'));
479+
480+
$this->assertNull($captured);
481+
}
482+
483+
private function emptyObjectSchema(): Schema
484+
{
485+
$schema = new Schema(Schema::VERSION_JSON_SCHEMA);
486+
unset($schema['$schema']);
487+
$schema->getDefinitions()['Dummy'] = new \ArrayObject(['type' => 'object']);
488+
$schema['$ref'] = '#/definitions/Dummy';
489+
490+
return $schema;
491+
}
425492
}

0 commit comments

Comments
 (0)