Address issue with pagination limit builder class class (#83)
This commit is contained in:
parent
3ec4cf612a
commit
095ea2af50
5 changed files with 58 additions and 45 deletions
|
|
@ -78,30 +78,12 @@ parameters:
|
||||||
count: 1
|
count: 1
|
||||||
path: src/Server/ServerBuilder.php
|
path: src/Server/ServerBuilder.php
|
||||||
|
|
||||||
-
|
|
||||||
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$discoveryExcludeDirs type has no value type specified in iterable type array\.$#'
|
|
||||||
identifier: missingType.iterableValue
|
|
||||||
count: 1
|
|
||||||
path: src/Server/ServerBuilder.php
|
|
||||||
|
|
||||||
-
|
-
|
||||||
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$instructions is never read, only written\.$#'
|
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$instructions is never read, only written\.$#'
|
||||||
identifier: property.onlyWritten
|
identifier: property.onlyWritten
|
||||||
count: 1
|
count: 1
|
||||||
path: src/Server/ServerBuilder.php
|
path: src/Server/ServerBuilder.php
|
||||||
|
|
||||||
-
|
|
||||||
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$paginationLimit \(int\|null\) is never assigned null so it can be removed from the property type\.$#'
|
|
||||||
identifier: property.unusedType
|
|
||||||
count: 1
|
|
||||||
path: src/Server/ServerBuilder.php
|
|
||||||
|
|
||||||
-
|
|
||||||
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$paginationLimit is never read, only written\.$#'
|
|
||||||
identifier: property.onlyWritten
|
|
||||||
count: 1
|
|
||||||
path: src/Server/ServerBuilder.php
|
|
||||||
|
|
||||||
-
|
-
|
||||||
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$prompts type has no value type specified in iterable type array\.$#'
|
message: '#^Property Mcp\\Server\\ServerBuilder\:\:\$prompts type has no value type specified in iterable type array\.$#'
|
||||||
identifier: missingType.iterableValue
|
identifier: missingType.iterableValue
|
||||||
|
|
|
||||||
|
|
@ -73,6 +73,7 @@ class Handler
|
||||||
SessionStoreInterface $sessionStore,
|
SessionStoreInterface $sessionStore,
|
||||||
SessionFactoryInterface $sessionFactory,
|
SessionFactoryInterface $sessionFactory,
|
||||||
LoggerInterface $logger = new NullLogger(),
|
LoggerInterface $logger = new NullLogger(),
|
||||||
|
int $paginationLimit = 50,
|
||||||
): self {
|
): self {
|
||||||
return new self(
|
return new self(
|
||||||
messageFactory: MessageFactory::make(),
|
messageFactory: MessageFactory::make(),
|
||||||
|
|
@ -82,12 +83,12 @@ class Handler
|
||||||
new NotificationHandler\InitializedHandler(),
|
new NotificationHandler\InitializedHandler(),
|
||||||
new RequestHandler\InitializeHandler($registry->getCapabilities(), $implementation),
|
new RequestHandler\InitializeHandler($registry->getCapabilities(), $implementation),
|
||||||
new RequestHandler\PingHandler(),
|
new RequestHandler\PingHandler(),
|
||||||
new RequestHandler\ListPromptsHandler($referenceProvider),
|
new RequestHandler\ListPromptsHandler($referenceProvider, $paginationLimit),
|
||||||
new RequestHandler\GetPromptHandler($promptGetter),
|
new RequestHandler\GetPromptHandler($promptGetter),
|
||||||
new RequestHandler\ListResourcesHandler($referenceProvider),
|
new RequestHandler\ListResourcesHandler($referenceProvider, $paginationLimit),
|
||||||
new RequestHandler\ReadResourceHandler($resourceReader),
|
new RequestHandler\ReadResourceHandler($resourceReader),
|
||||||
new RequestHandler\CallToolHandler($toolCaller, $logger),
|
new RequestHandler\CallToolHandler($toolCaller, $logger),
|
||||||
new RequestHandler\ListToolsHandler($referenceProvider),
|
new RequestHandler\ListToolsHandler($referenceProvider, $paginationLimit),
|
||||||
],
|
],
|
||||||
logger: $logger,
|
logger: $logger,
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -12,9 +12,9 @@
|
||||||
namespace Mcp\Server\RequestHandler\Reference;
|
namespace Mcp\Server\RequestHandler\Reference;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @implements \ArrayAccess<int|string, mixed>
|
* @extends \ArrayObject<int|string, mixed>
|
||||||
*/
|
*/
|
||||||
final class Page implements \Countable, \ArrayAccess
|
final class Page extends \ArrayObject
|
||||||
{
|
{
|
||||||
/**
|
/**
|
||||||
* @param array<int|string, mixed> $references Items can be Tool, Prompt, ResourceTemplate, or Resource
|
* @param array<int|string, mixed> $references Items can be Tool, Prompt, ResourceTemplate, or Resource
|
||||||
|
|
@ -23,30 +23,11 @@ final class Page implements \Countable, \ArrayAccess
|
||||||
public readonly array $references,
|
public readonly array $references,
|
||||||
public readonly ?string $nextCursor,
|
public readonly ?string $nextCursor,
|
||||||
) {
|
) {
|
||||||
|
parent::__construct($references, \ArrayObject::ARRAY_AS_PROPS);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function count(): int
|
public function count(): int
|
||||||
{
|
{
|
||||||
return \count($this->references);
|
return \count($this->references);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function offsetExists(mixed $offset): bool
|
|
||||||
{
|
|
||||||
return isset($this->references[$offset]);
|
|
||||||
}
|
|
||||||
|
|
||||||
public function offsetGet(mixed $offset): mixed
|
|
||||||
{
|
|
||||||
return $this->references[$offset] ?? null;
|
|
||||||
}
|
|
||||||
|
|
||||||
public function offsetSet(mixed $offset, mixed $value): void
|
|
||||||
{
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
public function offsetUnset(mixed $offset): void
|
|
||||||
{
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -76,7 +76,7 @@ final class ServerBuilder
|
||||||
|
|
||||||
private int $sessionTtl = 3600;
|
private int $sessionTtl = 3600;
|
||||||
|
|
||||||
private ?int $paginationLimit = 50;
|
private int $paginationLimit = 50;
|
||||||
|
|
||||||
private ?string $instructions = null;
|
private ?string $instructions = null;
|
||||||
|
|
||||||
|
|
@ -119,6 +119,9 @@ final class ServerBuilder
|
||||||
* @var array|string[]
|
* @var array|string[]
|
||||||
*/
|
*/
|
||||||
private array $discoveryScanDirs = [];
|
private array $discoveryScanDirs = [];
|
||||||
|
/**
|
||||||
|
* @var array|string[]
|
||||||
|
*/
|
||||||
private array $discoveryExcludeDirs = [];
|
private array $discoveryExcludeDirs = [];
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -337,6 +340,7 @@ final class ServerBuilder
|
||||||
sessionStore: $sessionStore,
|
sessionStore: $sessionStore,
|
||||||
sessionFactory: $sessionFactory,
|
sessionFactory: $sessionFactory,
|
||||||
logger: $logger,
|
logger: $logger,
|
||||||
|
paginationLimit: $this->paginationLimit,
|
||||||
),
|
),
|
||||||
logger: $logger,
|
logger: $logger,
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -137,7 +137,7 @@ class ListToolsHandlerTest extends TestCase
|
||||||
public function testReturnsAllToolsWhenCountIsLessThanPageSize(): void
|
public function testReturnsAllToolsWhenCountIsLessThanPageSize(): void
|
||||||
{
|
{
|
||||||
// Arrange
|
// Arrange
|
||||||
$this->addToolsToRegistry(2); // Less than page size (3)
|
$this->addToolsToRegistry(2); // Less than page size 3
|
||||||
$request = $this->createListToolsRequest();
|
$request = $this->createListToolsRequest();
|
||||||
|
|
||||||
// Act
|
// Act
|
||||||
|
|
@ -239,6 +239,51 @@ class ListToolsHandlerTest extends TestCase
|
||||||
$this->assertEquals($result1->tools, $result2->tools);
|
$this->assertEquals($result1->tools, $result2->tools);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[TestDox('Uses custom page size when provided')]
|
||||||
|
public function testUsesCustomPageSizeWhenProvided(): void
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
$customPageSize = 5;
|
||||||
|
$customHandler = new ListToolsHandler($this->registry, pageSize: $customPageSize);
|
||||||
|
$this->addToolsToRegistry(10);
|
||||||
|
$request = $this->createListToolsRequest();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
$response = $customHandler->handle($request, $this->session);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
/** @var ListToolsResult $result */
|
||||||
|
$result = $response->result;
|
||||||
|
$this->assertInstanceOf(ListToolsResult::class, $result);
|
||||||
|
$this->assertCount($customPageSize, $result->tools);
|
||||||
|
$this->assertNotNull($result->nextCursor);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[TestDox('Different page sizes produce different pagination results')]
|
||||||
|
public function testDifferentPageSizesProduceDifferentPaginationResults(): void
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
$this->addToolsToRegistry(10);
|
||||||
|
$smallPageHandler = new ListToolsHandler($this->registry, pageSize: 2);
|
||||||
|
$largePageHandler = new ListToolsHandler($this->registry, pageSize: 7);
|
||||||
|
$request = $this->createListToolsRequest();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
$smallPageResponse = $smallPageHandler->handle($request, $this->session);
|
||||||
|
$largePageResponse = $largePageHandler->handle($request, $this->session);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
/** @var ListToolsResult $smallResult */
|
||||||
|
$smallResult = $smallPageResponse->result;
|
||||||
|
/** @var ListToolsResult $largeResult */
|
||||||
|
$largeResult = $largePageResponse->result;
|
||||||
|
|
||||||
|
$this->assertCount(2, $smallResult->tools);
|
||||||
|
$this->assertCount(7, $largeResult->tools);
|
||||||
|
$this->assertNotNull($smallResult->nextCursor);
|
||||||
|
$this->assertNotNull($largeResult->nextCursor);
|
||||||
|
}
|
||||||
|
|
||||||
private function addToolsToRegistry(int $count): void
|
private function addToolsToRegistry(int $count): void
|
||||||
{
|
{
|
||||||
for ($i = 0; $i < $count; ++$i) {
|
for ($i = 0; $i < $count; ++$i) {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue