Skip to content

Commit cbee961

Browse files
wachterjohanneswachterjohannes
committed
Improve elicitation schema validation and reduce code duplication
- Add content validation when action is accept - Add default value validation for NumberSchemaDefinition - Mark ElicitResult as final and add missing @author tags - Improve error handling in elicitation examples - Extract AbstractSchemaDefinition to eliminate ~150 lines of duplication
1 parent fa8a4b9 commit cbee961

File tree

11 files changed

+187
-67
lines changed

11 files changed

+187
-67
lines changed

‎examples/server/elicitation/ElicitationHandlers.php‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,25 @@ enumNames: ['None', 'Vegetarian', 'Vegan', 'Gluten-Free', 'Halal', 'Kosher'],
111111
];
112112
}
113113

114-
$content = $result->content ?? [];
115-
$partySize = (int) ($content['party_size'] ?? 2);
116-
$date = (string) ($content['date'] ?? '');
114+
$content = $result->content;
115+
if (null === $content) {
116+
throw new \RuntimeException('Expected content for accepted elicitation.');
117+
}
118+
119+
if (!isset($content['party_size']) || !isset($content['date'])) {
120+
throw new \RuntimeException('Missing required fields: party_size and date.');
121+
}
122+
123+
$partySize = (int) $content['party_size'];
124+
$date = (string) $content['date'];
117125
$dietary = (string) ($content['dietary'] ?? 'none');
118126

127+
if ($partySize < 1 || $partySize > 20) {
128+
throw new \RuntimeException(
129+
\sprintf('Invalid party size: %d. Must be between 1 and 20.', $partySize)
130+
);
131+
}
132+
119133
$this->logger->info(\sprintf(
120134
'Booking confirmed: %d guests on %s with %s dietary requirements',
121135
$partySize,
@@ -181,7 +195,16 @@ public function confirmAction(RequestContext $context, string $actionDescription
181195
];
182196
}
183197

184-
$confirmed = (bool) ($result->content['confirm'] ?? false);
198+
$content = $result->content;
199+
if (null === $content) {
200+
throw new \RuntimeException('Expected content for accepted elicitation.');
201+
}
202+
203+
if (!isset($content['confirm'])) {
204+
throw new \RuntimeException('Missing required field: confirm.');
205+
}
206+
207+
$confirmed = (bool) $content['confirm'];
185208

186209
if (!$confirmed) {
187210
return [
@@ -246,8 +269,16 @@ enumNames: ['1 - Poor', '2 - Fair', '3 - Good', '4 - Very Good', '5 - Excellent'
246269
];
247270
}
248271

249-
$content = $result->content ?? [];
250-
$rating = (string) ($content['rating'] ?? '3');
272+
$content = $result->content;
273+
if (null === $content) {
274+
throw new \RuntimeException('Expected content for accepted elicitation.');
275+
}
276+
277+
if (!isset($content['rating'])) {
278+
throw new \RuntimeException('Missing required field: rating.');
279+
}
280+
281+
$rating = (string) $content['rating'];
251282
$comments = (string) ($content['comments'] ?? '');
252283

253284
$this->logger->info(\sprintf('Feedback received: rating=%s, comments=%s', $rating, $comments));

‎src/Schema/Elicitation/AbstractSchemaDefinition.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of the official PHP MCP SDK.
7+
*
8+
* A collaboration between Symfony and the PHP Foundation.
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace Mcp\Schema\Elicitation;
15+
16+
use Mcp\Exception\InvalidArgumentException;
17+
18+
/**
19+
* Base class for schema definitions in elicitation requests.
20+
*
21+
* @author Johannes Wachter <johannes@sulu.io>
22+
*/
23+
abstract class AbstractSchemaDefinition implements \JsonSerializable
24+
{
25+
public function __construct(
26+
public readonly string $title,
27+
public readonly ?string $description = null,
28+
) {
29+
}
30+
31+
/**
32+
* Validate that title exists and is a string in the data array.
33+
*
34+
* @param array<string, mixed> $data
35+
*
36+
* @throws InvalidArgumentException
37+
*/
38+
protected static function validateTitle(array $data, string $schemaType): void
39+
{
40+
if (!isset($data['title']) || !\is_string($data['title'])) {
41+
throw new InvalidArgumentException(\sprintf('Missing or invalid "title" for %s schema definition.', $schemaType));
42+
}
43+
}
44+
45+
/**
46+
* Build the base JSON structure with type, title, and optional description.
47+
*
48+
* @return array<string, mixed>
49+
*/
50+
protected function buildBaseJson(string $type): array
51+
{
52+
$data = [
53+
'type' => $type,
54+
'title' => $this->title,
55+
];
56+
57+
if (null !== $this->description) {
58+
$data['description'] = $this->description;
59+
}
60+
61+
return $data;
62+
}
63+
64+
/**
65+
* @return array<string, mixed>
66+
*/
67+
abstract public function jsonSerialize(): array;
68+
}

‎src/Schema/Elicitation/BooleanSchemaDefinition.php‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,19 @@
2020
*
2121
* @author Johannes Wachter <johannes@sulu.io>
2222
*/
23-
final class BooleanSchemaDefinition implements \JsonSerializable
23+
final class BooleanSchemaDefinition extends AbstractSchemaDefinition
2424
{
2525
/**
2626
* @param string $title Human-readable title for the field
2727
* @param string|null $description Optional description/help text
2828
* @param bool|null $default Optional default value
2929
*/
3030
public function __construct(
31-
public readonly string $title,
32-
public readonly ?string $description = null,
31+
string $title,
32+
?string $description = null,
3333
public readonly ?bool $default = null,
3434
) {
35+
parent::__construct($title, $description);
3536
}
3637

3738
/**
@@ -43,9 +44,7 @@ public function __construct(
4344
*/
4445
public static function fromArray(array $data): self
4546
{
46-
if (!isset($data['title']) || !\is_string($data['title'])) {
47-
throw new InvalidArgumentException('Missing or invalid "title" for boolean schema definition.');
48-
}
47+
self::validateTitle($data, 'boolean');
4948

5049
return new self(
5150
title: $data['title'],
@@ -64,14 +63,7 @@ public static function fromArray(array $data): self
6463
*/
6564
public function jsonSerialize(): array
6665
{
67-
$data = [
68-
'type' => 'boolean',
69-
'title' => $this->title,
70-
];
71-
72-
if (null !== $this->description) {
73-
$data['description'] = $this->description;
74-
}
66+
$data = $this->buildBaseJson('boolean');
7567

7668
if (null !== $this->default) {
7769
$data['default'] = $this->default;

‎src/Schema/Elicitation/EnumSchemaDefinition.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
* @author Johannes Wachter <johannes@sulu.io>
2424
*/
25-
final class EnumSchemaDefinition implements \JsonSerializable
25+
final class EnumSchemaDefinition extends AbstractSchemaDefinition
2626
{
2727
/**
2828
* @param string $title Human-readable title for the field
@@ -32,12 +32,14 @@ final class EnumSchemaDefinition implements \JsonSerializable
3232
* @param string[]|null $enumNames Optional human-readable labels for each enum value
3333
*/
3434
public function __construct(
35-
public readonly string $title,
35+
string $title,
3636
public readonly array $enum,
37-
public readonly ?string $description = null,
37+
?string $description = null,
3838
public readonly ?string $default = null,
3939
public readonly ?array $enumNames = null,
4040
) {
41+
parent::__construct($title, $description);
42+
4143
if ([] === $enum) {
4244
throw new InvalidArgumentException('enum array must not be empty.');
4345
}
@@ -68,9 +70,7 @@ public function __construct(
6870
*/
6971
public static function fromArray(array $data): self
7072
{
71-
if (!isset($data['title']) || !\is_string($data['title'])) {
72-
throw new InvalidArgumentException('Missing or invalid "title" for enum schema definition.');
73-
}
73+
self::validateTitle($data, 'enum');
7474

7575
if (!isset($data['enum']) || !\is_array($data['enum'])) {
7676
throw new InvalidArgumentException('Missing or invalid "enum" for enum schema definition.');

‎src/Schema/Elicitation/NumberSchemaDefinition.php‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
* @author Johannes Wachter <johannes@sulu.io>
2424
*/
25-
final class NumberSchemaDefinition implements \JsonSerializable
25+
final class NumberSchemaDefinition extends AbstractSchemaDefinition
2626
{
2727
/**
2828
* @param string $title Human-readable title for the field
@@ -33,16 +33,32 @@ final class NumberSchemaDefinition implements \JsonSerializable
3333
* @param int|float|null $maximum Optional maximum value (inclusive)
3434
*/
3535
public function __construct(
36-
public readonly string $title,
36+
string $title,
3737
public readonly bool $integerOnly = false,
38-
public readonly ?string $description = null,
38+
?string $description = null,
3939
public readonly int|float|null $default = null,
4040
public readonly int|float|null $minimum = null,
4141
public readonly int|float|null $maximum = null,
4242
) {
43+
parent::__construct($title, $description);
44+
4345
if (null !== $minimum && null !== $maximum && $minimum > $maximum) {
4446
throw new InvalidArgumentException('minimum cannot be greater than maximum.');
4547
}
48+
49+
if (null !== $default && null !== $minimum && $default < $minimum) {
50+
throw new InvalidArgumentException('default value cannot be less than minimum.');
51+
}
52+
53+
if (null !== $default && null !== $maximum && $default > $maximum) {
54+
throw new InvalidArgumentException('default value cannot be greater than maximum.');
55+
}
56+
57+
if ($integerOnly && null !== $default && $default !== (int) $default) {
58+
throw new InvalidArgumentException(
59+
'default value must be an integer when integerOnly is true.'
60+
);
61+
}
4662
}
4763

4864
/**
@@ -57,9 +73,7 @@ public function __construct(
5773
*/
5874
public static function fromArray(array $data): self
5975
{
60-
if (!isset($data['title']) || !\is_string($data['title'])) {
61-
throw new InvalidArgumentException('Missing or invalid "title" for number schema definition.');
62-
}
76+
self::validateTitle($data, 'number');
6377

6478
$type = $data['type'] ?? 'number';
6579
$integerOnly = 'integer' === $type;
@@ -86,14 +100,7 @@ public static function fromArray(array $data): self
86100
*/
87101
public function jsonSerialize(): array
88102
{
89-
$data = [
90-
'type' => $this->integerOnly ? 'integer' : 'number',
91-
'title' => $this->title,
92-
];
93-
94-
if (null !== $this->description) {
95-
$data['description'] = $this->description;
96-
}
103+
$data = $this->buildBaseJson($this->integerOnly ? 'integer' : 'number');
97104

98105
if (null !== $this->default) {
99106
$data['default'] = $this->default;

‎src/Schema/Elicitation/StringSchemaDefinition.php‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
* @author Johannes Wachter <johannes@sulu.io>
2424
*/
25-
final class StringSchemaDefinition implements \JsonSerializable
25+
final class StringSchemaDefinition extends AbstractSchemaDefinition
2626
{
2727
private const VALID_FORMATS = ['date', 'date-time', 'email', 'uri'];
2828

@@ -35,13 +35,15 @@ final class StringSchemaDefinition implements \JsonSerializable
3535
* @param int|null $maxLength Optional maximum string length
3636
*/
3737
public function __construct(
38-
public readonly string $title,
39-
public readonly ?string $description = null,
38+
string $title,
39+
?string $description = null,
4040
public readonly ?string $default = null,
4141
public readonly ?string $format = null,
4242
public readonly ?int $minLength = null,
4343
public readonly ?int $maxLength = null,
4444
) {
45+
parent::__construct($title, $description);
46+
4547
if (null !== $format && !\in_array($format, self::VALID_FORMATS, true)) {
4648
throw new InvalidArgumentException(\sprintf('Invalid format "%s". Valid formats are: %s.', $format, implode(', ', self::VALID_FORMATS)));
4749
}
@@ -71,9 +73,7 @@ public function __construct(
7173
*/
7274
public static function fromArray(array $data): self
7375
{
74-
if (!isset($data['title']) || !\is_string($data['title'])) {
75-
throw new InvalidArgumentException('Missing or invalid "title" for string schema definition.');
76-
}
76+
self::validateTitle($data, 'string');
7777

7878
return new self(
7979
title: $data['title'],
@@ -98,14 +98,7 @@ public static function fromArray(array $data): self
9898
*/
9999
public function jsonSerialize(): array
100100
{
101-
$data = [
102-
'type' => 'string',
103-
'title' => $this->title,
104-
];
105-
106-
if (null !== $this->description) {
107-
$data['description'] = $this->description;
108-
}
101+
$data = $this->buildBaseJson('string');
109102

110103
if (null !== $this->default) {
111104
$data['default'] = $this->default;

‎src/Schema/Enum/ElicitAction.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
/**
1717
* Action taken by the user in response to an elicitation request.
1818
*
19-
* @author
19+
* @author Johannes Wachter <johannes@sulu.io>
2020
*/
2121
enum ElicitAction: string
2222
{

‎src/Schema/Request/ElicitRequest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
* The client will present the message and requested schema to the user, allowing them
2424
* to provide the requested information, decline, or cancel the operation.
2525
*
26-
* @author
26+
* @author Johannes Wachter <johannes@sulu.io>
2727
*/
2828
final class ElicitRequest extends Request
2929
{

0 commit comments

Comments
 (0)