[Capability] Apply a parameter's complete schema definition (#435)
* [Capability] Apply a parameter's complete schema definition Schema(definition:) is documented as taking precedence over the individual keywords, and generate() unwraps it that way at method level. At parameter level it was merged as an ordinary key instead, in both the plain and the variadic path, so the property reached clients carrying a literal "definition" member - not JSON Schema, read by nothing, and everything inside it lost. Both paths now replace the schema with the definition. The variadic one still forces type: array afterwards, since a variadic arrives as an array however the definition describes it, and the signature default survives a definition that states none - it is what the handler receives when the argument is omitted, and default is an annotation rather than a constraint. * Fix docblock description overwriting definition on variadic params * Apply suggestion from @chr-hertel
This commit is contained in:
parent
08d332c7ac
commit
cfe5e8f570
3 changed files with 120 additions and 6 deletions
|
|
@ -237,7 +237,17 @@ final class SchemaGenerator implements SchemaGeneratorInterface
|
|||
// Parameter-level takes highest precedence
|
||||
$parameterLevelSchema = $paramInfo['parameter_schema'];
|
||||
if (!empty($parameterLevelSchema)) {
|
||||
$mergedSchema = array_merge($mergedSchema, $parameterLevelSchema);
|
||||
// A complete definition replaces the schema, as it does at method level.
|
||||
if (isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition'])) {
|
||||
$mergedSchema = $parameterLevelSchema['definition'];
|
||||
|
||||
// The default comes from the signature, not from the schema.
|
||||
if (!\array_key_exists('default', $mergedSchema) && \array_key_exists('default', $inferredSchema)) {
|
||||
$mergedSchema['default'] = $inferredSchema['default'];
|
||||
}
|
||||
} else {
|
||||
$mergedSchema = array_merge($mergedSchema, $parameterLevelSchema);
|
||||
}
|
||||
}
|
||||
|
||||
// Run after all merges so that when a Schema attribute reshapes the parameter
|
||||
|
|
@ -324,17 +334,24 @@ final class SchemaGenerator implements SchemaGeneratorInterface
|
|||
{
|
||||
$paramSchema = ['type' => 'array'];
|
||||
|
||||
// Apply parameter-level Schema attributes first
|
||||
if (!empty($paramInfo['parameter_schema'])) {
|
||||
$paramSchema = array_merge($paramSchema, $paramInfo['parameter_schema']);
|
||||
// Ensure type is always array for variadic
|
||||
$paramSchema['type'] = 'array';
|
||||
}
|
||||
|
||||
// Add description from docblock before parameter-level schema is merged in,
|
||||
// so a definition's own description takes precedence (matches buildParameterSchema).
|
||||
if ($paramInfo['description']) {
|
||||
$paramSchema['description'] = $paramInfo['description'];
|
||||
}
|
||||
|
||||
// Apply parameter-level Schema attributes first
|
||||
if (!empty($paramInfo['parameter_schema'])) {
|
||||
$parameterLevelSchema = $paramInfo['parameter_schema'];
|
||||
|
||||
$paramSchema = isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition'])
|
||||
? $parameterLevelSchema['definition']
|
||||
: array_merge($paramSchema, $parameterLevelSchema);
|
||||
|
||||
// Ensure type is always array for variadic
|
||||
$paramSchema['type'] = 'array';
|
||||
}
|
||||
|
||||
// If no items specified by Schema attribute, infer from type
|
||||
if (!isset($paramSchema['items'])) {
|
||||
$itemJsonTypes = $this->mapPhpTypeToJsonSchemaType($paramInfo['type_string']);
|
||||
|
|
|
|||
|
|
@ -130,6 +130,30 @@ class SchemaGeneratorFixture
|
|||
|
||||
// ===== PARAMETER-LEVEL SCHEMA SCENARIOS =====
|
||||
|
||||
/**
|
||||
* Parameter-level Schema with complete definition.
|
||||
*/
|
||||
public function parameterLevelCompleteDefinition(
|
||||
#[Schema(definition: [
|
||||
'type' => 'string',
|
||||
'description' => 'The region to query.',
|
||||
'x-mcp-header' => 'Region',
|
||||
])]
|
||||
string $region = 'eu',
|
||||
#[Schema(definition: ['type' => 'integer', 'minimum' => 1, 'maximum' => 10])]
|
||||
int $limit = 3,
|
||||
): void {
|
||||
}
|
||||
|
||||
/**
|
||||
* Parameter-level definition that reshapes the parameter's type.
|
||||
*/
|
||||
public function definitionReshapingTheParameter(
|
||||
#[Schema(definition: ['type' => 'object', 'properties' => ['id' => ['type' => 'integer']]])]
|
||||
string $payload = '{}',
|
||||
): void {
|
||||
}
|
||||
|
||||
/**
|
||||
* Parameter-level Schema attributes only.
|
||||
*/
|
||||
|
|
@ -326,6 +350,27 @@ class SchemaGeneratorFixture
|
|||
|
||||
// ===== VARIADIC SCENARIOS =====
|
||||
|
||||
/**
|
||||
* Variadic parameter with a complete definition.
|
||||
*/
|
||||
public function variadicCompleteDefinition(
|
||||
#[Schema(definition: ['type' => 'array', 'description' => 'Tags to apply.', 'items' => ['type' => 'string']])]
|
||||
string ...$tags,
|
||||
): void {
|
||||
}
|
||||
|
||||
/**
|
||||
* Variadic parameter with both a docblock description and a complete definition;
|
||||
* the definition's own description must win.
|
||||
*
|
||||
* @param string ...$tags Docblock description that must lose to the definition's own
|
||||
*/
|
||||
public function variadicCompleteDefinitionWithDocblockDescription(
|
||||
#[Schema(definition: ['type' => 'array', 'description' => 'Tags to apply.', 'items' => ['type' => 'string']])]
|
||||
string ...$tags,
|
||||
): void {
|
||||
}
|
||||
|
||||
/**
|
||||
* Variadic parameter scenarios.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -92,6 +92,58 @@ final class SchemaGeneratorTest extends TestCase
|
|||
], $schema);
|
||||
}
|
||||
|
||||
public function testUsesCompleteSchemaDefinitionFromParameterLevelSchemaAttribute(): void
|
||||
{
|
||||
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'parameterLevelCompleteDefinition');
|
||||
$schema = $this->schemaGenerator->generate($method);
|
||||
|
||||
$this->assertEquals([
|
||||
'type' => 'string',
|
||||
'description' => 'The region to query.',
|
||||
'x-mcp-header' => 'Region',
|
||||
'default' => 'eu',
|
||||
], $schema['properties']['region']);
|
||||
|
||||
$this->assertEquals([
|
||||
'type' => 'integer',
|
||||
'minimum' => 1,
|
||||
'maximum' => 10,
|
||||
'default' => 3,
|
||||
], $schema['properties']['limit']);
|
||||
|
||||
$this->assertArrayNotHasKey('required', $schema);
|
||||
}
|
||||
|
||||
public function testUsesCompleteSchemaDefinitionFromVariadicParameter(): void
|
||||
{
|
||||
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'variadicCompleteDefinition');
|
||||
$schema = $this->schemaGenerator->generate($method);
|
||||
|
||||
// The array type is forced: a variadic always arrives as one.
|
||||
$this->assertEquals([
|
||||
'type' => 'array',
|
||||
'description' => 'Tags to apply.',
|
||||
'items' => ['type' => 'string'],
|
||||
], $schema['properties']['tags']);
|
||||
}
|
||||
|
||||
public function testDefinitionDescriptionWinsOverDocblockOnVariadicParameter(): void
|
||||
{
|
||||
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'variadicCompleteDefinitionWithDocblockDescription');
|
||||
$schema = $this->schemaGenerator->generate($method);
|
||||
|
||||
$this->assertSame('Tags to apply.', $schema['properties']['tags']['description']);
|
||||
}
|
||||
|
||||
public function testKeepsTheSignatureDefaultWhenADefinitionReshapesTheParameter(): void
|
||||
{
|
||||
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'definitionReshapingTheParameter');
|
||||
$schema = $this->schemaGenerator->generate($method);
|
||||
|
||||
$this->assertSame('{}', $schema['properties']['payload']['default']);
|
||||
$this->assertSame('object', $schema['properties']['payload']['type']);
|
||||
}
|
||||
|
||||
public function testGeneratesSchemaFromMethodLevelSchemaAttributeWithProperties(): void
|
||||
{
|
||||
$method = new \ReflectionMethod(SchemaGeneratorFixture::class, 'methodLevelWithProperties');
|
||||
|
|
|
|||
Loading…
Reference in a new issue