From ef6d7c760ac55e333c0dd7ef39c997173ace265a Mon Sep 17 00:00:00 2001 From: Oskar Stark Date: Mon, 6 Oct 2025 21:31:51 +0200 Subject: [PATCH] [Platform][Tool] Make ExecutionReference properties private with getters This change improves encapsulation by making the class and method properties private and providing public getter methods. --- .../Exception/ToolNotFoundException.php | 2 +- .../src/Toolbox/ToolCallArgumentResolver.php | 2 +- src/agent/src/Toolbox/Toolbox.php | 5 +- .../MetadataFactory/ChainFactoryTest.php | 2 +- .../MetadataFactory/MemoryFactoryTest.php | 6 +- .../MetadataFactory/ReflectionFactoryTest.php | 4 +- .../IsGrantedToolAttributeListener.php | 2 +- src/platform/src/Tool/ExecutionReference.php | 14 ++++- .../tests/Tool/ExecutionReferenceTest.php | 55 +++++++++++++++++++ 9 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 src/platform/tests/Tool/ExecutionReferenceTest.php diff --git a/src/agent/src/Toolbox/Exception/ToolNotFoundException.php b/src/agent/src/Toolbox/Exception/ToolNotFoundException.php index b5231d0e2..1cd9dea3c 100644 --- a/src/agent/src/Toolbox/Exception/ToolNotFoundException.php +++ b/src/agent/src/Toolbox/Exception/ToolNotFoundException.php @@ -31,6 +31,6 @@ public static function notFoundForToolCall(ToolCall $toolCall): self public static function notFoundForReference(ExecutionReference $reference): self { - return new self(\sprintf('Tool not found for reference: %s::%s.', $reference->class, $reference->method)); + return new self(\sprintf('Tool not found for reference: %s::%s.', $reference->getClass(), $reference->getMethod())); } } diff --git a/src/agent/src/Toolbox/ToolCallArgumentResolver.php b/src/agent/src/Toolbox/ToolCallArgumentResolver.php index 1e4dd975b..37e029767 100644 --- a/src/agent/src/Toolbox/ToolCallArgumentResolver.php +++ b/src/agent/src/Toolbox/ToolCallArgumentResolver.php @@ -43,7 +43,7 @@ public function __construct( */ public function resolveArguments(Tool $metadata, ToolCall $toolCall): array { - $method = new \ReflectionMethod($metadata->reference->class, $metadata->reference->method); + $method = new \ReflectionMethod($metadata->reference->getClass(), $metadata->reference->getMethod()); /** @var array $parameters */ $parameters = array_column($method->getParameters(), null, 'name'); diff --git a/src/agent/src/Toolbox/Toolbox.php b/src/agent/src/Toolbox/Toolbox.php index 342f777d2..53362ab23 100644 --- a/src/agent/src/Toolbox/Toolbox.php +++ b/src/agent/src/Toolbox/Toolbox.php @@ -82,7 +82,7 @@ public function execute(ToolCall $toolCall): mixed $arguments = $this->argumentResolver->resolveArguments($metadata, $toolCall); $this->eventDispatcher?->dispatch(new ToolCallArgumentsResolved($tool, $metadata, $arguments)); - $result = $tool->{$metadata->reference->method}(...$arguments); + $result = $tool->{$metadata->reference->getMethod()}(...$arguments); $this->eventDispatcher?->dispatch(new ToolCallSucceeded($tool, $metadata, $arguments, $result)); } catch (ToolExecutionExceptionInterface $e) { $this->eventDispatcher?->dispatch(new ToolCallFailed($tool, $metadata, $arguments ?? [], $e)); @@ -109,8 +109,9 @@ private function getMetadata(ToolCall $toolCall): Tool private function getExecutable(Tool $metadata): object { + $className = $metadata->reference->getClass(); foreach ($this->tools as $tool) { - if ($tool instanceof $metadata->reference->class) { + if ($tool instanceof $className) { return $tool; } } diff --git a/src/agent/tests/Toolbox/MetadataFactory/ChainFactoryTest.php b/src/agent/tests/Toolbox/MetadataFactory/ChainFactoryTest.php index bfc9e08c3..bc9eb5db1 100644 --- a/src/agent/tests/Toolbox/MetadataFactory/ChainFactoryTest.php +++ b/src/agent/tests/Toolbox/MetadataFactory/ChainFactoryTest.php @@ -67,7 +67,7 @@ public function testTestGetMetadataOverwrite() $this->assertCount(1, $metadata); $this->assertSame('optional_param', $metadata[0]->name); $this->assertSame('Tool with optional param', $metadata[0]->description); - $this->assertSame('bar', $metadata[0]->reference->method); + $this->assertSame('bar', $metadata[0]->reference->getMethod()); } public function testTestGetMetadataWithAttributeDoubleHit() diff --git a/src/agent/tests/Toolbox/MetadataFactory/MemoryFactoryTest.php b/src/agent/tests/Toolbox/MetadataFactory/MemoryFactoryTest.php index 88dadb8a9..e3e742f88 100644 --- a/src/agent/tests/Toolbox/MetadataFactory/MemoryFactoryTest.php +++ b/src/agent/tests/Toolbox/MetadataFactory/MemoryFactoryTest.php @@ -41,7 +41,7 @@ public function testGetMetadataWithDistinctToolPerClass() $this->assertInstanceOf(Tool::class, $metadata[0]); $this->assertSame('happy_birthday', $metadata[0]->name); $this->assertSame('Generates birthday message', $metadata[0]->description); - $this->assertSame('__invoke', $metadata[0]->reference->method); + $this->assertSame('__invoke', $metadata[0]->reference->getMethod()); $expectedParams = [ 'type' => 'object', @@ -68,7 +68,7 @@ public function testGetMetadataWithMultipleToolsInClass() $this->assertInstanceOf(Tool::class, $metadata[0]); $this->assertSame('checkout', $metadata[0]->name); $this->assertSame('Buys a number of items per product', $metadata[0]->description); - $this->assertSame('buy', $metadata[0]->reference->method); + $this->assertSame('buy', $metadata[0]->reference->getMethod()); $expectedParams = [ 'type' => 'object', @@ -84,7 +84,7 @@ public function testGetMetadataWithMultipleToolsInClass() $this->assertInstanceOf(Tool::class, $metadata[1]); $this->assertSame('cancel', $metadata[1]->name); $this->assertSame('Cancels an order', $metadata[1]->description); - $this->assertSame('cancel', $metadata[1]->reference->method); + $this->assertSame('cancel', $metadata[1]->reference->getMethod()); $expectedParams = [ 'type' => 'object', diff --git a/src/agent/tests/Toolbox/MetadataFactory/ReflectionFactoryTest.php b/src/agent/tests/Toolbox/MetadataFactory/ReflectionFactoryTest.php index 876ee7905..a3c13b43c 100644 --- a/src/agent/tests/Toolbox/MetadataFactory/ReflectionFactoryTest.php +++ b/src/agent/tests/Toolbox/MetadataFactory/ReflectionFactoryTest.php @@ -127,8 +127,8 @@ className: ToolMultiple::class, private function assertToolConfiguration(Tool $metadata, string $className, string $name, string $description, string $method, array $parameters): void { - $this->assertSame($className, $metadata->reference->class); - $this->assertSame($method, $metadata->reference->method); + $this->assertSame($className, $metadata->reference->getClass()); + $this->assertSame($method, $metadata->reference->getMethod()); $this->assertSame($name, $metadata->name); $this->assertSame($description, $metadata->description); $this->assertSame($parameters, $metadata->parameters); diff --git a/src/ai-bundle/src/Security/EventListener/IsGrantedToolAttributeListener.php b/src/ai-bundle/src/Security/EventListener/IsGrantedToolAttributeListener.php index 4c4b1de06..0a16ae3ba 100644 --- a/src/ai-bundle/src/Security/EventListener/IsGrantedToolAttributeListener.php +++ b/src/ai-bundle/src/Security/EventListener/IsGrantedToolAttributeListener.php @@ -37,7 +37,7 @@ public function __invoke(ToolCallArgumentsResolved $event): void { $tool = $event->tool; $class = new \ReflectionClass($tool); - $method = $class->getMethod($event->metadata->reference->method); + $method = $class->getMethod($event->metadata->reference->getMethod()); $classAttributes = $class->getAttributes(IsGrantedTool::class); $methodAttributes = $method->getAttributes(IsGrantedTool::class); diff --git a/src/platform/src/Tool/ExecutionReference.php b/src/platform/src/Tool/ExecutionReference.php index 0a68c005a..2f094c10b 100644 --- a/src/platform/src/Tool/ExecutionReference.php +++ b/src/platform/src/Tool/ExecutionReference.php @@ -17,8 +17,18 @@ final class ExecutionReference { public function __construct( - public string $class, - public string $method = '__invoke', + private string $class, + private string $method = '__invoke', ) { } + + public function getClass(): string + { + return $this->class; + } + + public function getMethod(): string + { + return $this->method; + } } diff --git a/src/platform/tests/Tool/ExecutionReferenceTest.php b/src/platform/tests/Tool/ExecutionReferenceTest.php new file mode 100644 index 000000000..9689ba1f6 --- /dev/null +++ b/src/platform/tests/Tool/ExecutionReferenceTest.php @@ -0,0 +1,55 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\AI\Platform\Tests\Tool; + +use PHPUnit\Framework\TestCase; +use Symfony\AI\Platform\Tool\ExecutionReference; + +final class ExecutionReferenceTest extends TestCase +{ + public function testGetClass() + { + $reference = new ExecutionReference('MyClass'); + + $this->assertSame('MyClass', $reference->getClass()); + } + + public function testGetMethod() + { + $reference = new ExecutionReference('MyClass', 'myMethod'); + + $this->assertSame('myMethod', $reference->getMethod()); + } + + public function testGetMethodReturnsDefaultInvokeMethod() + { + $reference = new ExecutionReference('MyClass'); + + $this->assertSame('__invoke', $reference->getMethod()); + } + + public function testConstructorWithClassAndMethod() + { + $reference = new ExecutionReference('MyClass', 'execute'); + + $this->assertSame('MyClass', $reference->getClass()); + $this->assertSame('execute', $reference->getMethod()); + } + + public function testConstructorWithOnlyClass() + { + $reference = new ExecutionReference('AnotherClass'); + + $this->assertSame('AnotherClass', $reference->getClass()); + $this->assertSame('__invoke', $reference->getMethod()); + } +}