diff --git a/src/Entity/Parts/Category.php b/src/Entity/Parts/Category.php index cd2933fa..316e116f 100644 --- a/src/Entity/Parts/Category.php +++ b/src/Entity/Parts/Category.php @@ -87,7 +87,7 @@ use Symfony\Component\Validator\Constraints as Assert; mcp: [ 'list_categories' => new McpToolCollection( title: 'List/search categories', - description: 'List all part categories, optionally filtered by a keyword matched against the name and comment. Categories are used to group parts by their function.', + description: 'List all part categories, optionally filtered by a keyword matched against the name and comment. Categories are used to group parts by their function. Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Entity/Parts/Footprint.php b/src/Entity/Parts/Footprint.php index 6e38465e..54c85734 100644 --- a/src/Entity/Parts/Footprint.php +++ b/src/Entity/Parts/Footprint.php @@ -86,7 +86,7 @@ use Symfony\Component\Validator\Constraints as Assert; mcp: [ 'list_footprints' => new McpToolCollection( title: 'List/search footprints', - description: 'List all footprints, optionally filtered by a keyword matched against the name and comment. Footprints describe the physical package/shape of a part.', + description: 'List all footprints, optionally filtered by a keyword matched against the name and comment. Footprints describe the physical package/shape of a part. Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Entity/Parts/Manufacturer.php b/src/Entity/Parts/Manufacturer.php index ef491277..343ce685 100644 --- a/src/Entity/Parts/Manufacturer.php +++ b/src/Entity/Parts/Manufacturer.php @@ -85,7 +85,7 @@ use Symfony\Component\Validator\Constraints as Assert; mcp: [ 'list_manufacturers' => new McpToolCollection( title: 'List/search manufacturers', - description: 'List all manufacturers, optionally filtered by a keyword matched against the name and comment.', + description: 'List all manufacturers, optionally filtered by a keyword matched against the name and comment. Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Entity/Parts/MeasurementUnit.php b/src/Entity/Parts/MeasurementUnit.php index bd8ffd84..dbf8036b 100644 --- a/src/Entity/Parts/MeasurementUnit.php +++ b/src/Entity/Parts/MeasurementUnit.php @@ -90,7 +90,7 @@ use Symfony\Component\Validator\Constraints\Length; mcp: [ 'list_measurement_units' => new McpToolCollection( title: 'List/search measurement units', - description: 'List all measurement units, optionally filtered by a keyword matched against the name and comment. Measurement units describe how the amount of a part is measured (e.g. pieces, meters, grams).', + description: 'List all measurement units, optionally filtered by a keyword matched against the name and comment. Measurement units describe how the amount of a part is measured (e.g. pieces, meters, grams). Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Entity/Parts/PartCustomState.php b/src/Entity/Parts/PartCustomState.php index e9e1fa87..b5007e89 100644 --- a/src/Entity/Parts/PartCustomState.php +++ b/src/Entity/Parts/PartCustomState.php @@ -77,7 +77,7 @@ use Symfony\Component\Validator\Constraints as Assert; mcp: [ 'list_part_custom_states' => new McpToolCollection( title: 'List/search part custom states', - description: 'List all part custom states, optionally filtered by a keyword matched against the name and comment. Custom states are used to mark parts with a custom status (e.g. "obsolete", "needs review").', + description: 'List all part custom states, optionally filtered by a keyword matched against the name and comment. Custom states are used to mark parts with a custom status (e.g. "obsolete", "needs review"). Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Entity/Parts/StorageLocation.php b/src/Entity/Parts/StorageLocation.php index 3e8cc611..a7c7176e 100644 --- a/src/Entity/Parts/StorageLocation.php +++ b/src/Entity/Parts/StorageLocation.php @@ -86,7 +86,7 @@ use Symfony\Component\Validator\Constraints as Assert; mcp: [ 'list_storage_locations' => new McpToolCollection( title: 'List/search storage locations', - description: 'List all storage locations, optionally filtered by a keyword matched against the name and comment. Storage locations describe where parts are physically stored.', + description: 'List all storage locations, optionally filtered by a keyword matched against the name and comment. Storage locations describe where parts are physically stored. Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Entity/Parts/Supplier.php b/src/Entity/Parts/Supplier.php index 11dcf980..466f1789 100644 --- a/src/Entity/Parts/Supplier.php +++ b/src/Entity/Parts/Supplier.php @@ -90,7 +90,7 @@ use Symfony\Component\Validator\Constraints as Assert; mcp: [ 'list_suppliers' => new McpToolCollection( title: 'List/search suppliers', - description: 'List all suppliers, optionally filtered by a keyword matched against the name and comment.', + description: 'List all suppliers, optionally filtered by a keyword matched against the name and comment. Each entry includes its full hierarchical path, and results are sorted by that path so parents are immediately followed by their own children, making it easy to derive the tree structure from the flat list.', annotations: ['readOnlyHint' => true, 'destructiveHint' => false, 'idempotentHint' => true, 'openWorldHint' => false], output: StructuralElementOverview::class, normalizationContext: ['groups' => ['mcp_structural_overview:read']], diff --git a/src/Mcp/DTO/StructuralElementOverview.php b/src/Mcp/DTO/StructuralElementOverview.php index aa96de38..40591b44 100644 --- a/src/Mcp/DTO/StructuralElementOverview.php +++ b/src/Mcp/DTO/StructuralElementOverview.php @@ -42,6 +42,9 @@ readonly class StructuralElementOverview /** @var string The name of the element */ #[Groups(['mcp_structural_overview:read'])] public string $name, + /** @var string The full path of the element (including the names of all its parent elements, e.g. "Parent → Child") */ + #[Groups(['mcp_structural_overview:read'])] + public string $full_path, ) { } } diff --git a/src/Serializer/StructuralElementOverviewNormalizer.php b/src/Serializer/StructuralElementOverviewNormalizer.php index 4205593f..04a3ee9c 100644 --- a/src/Serializer/StructuralElementOverviewNormalizer.php +++ b/src/Serializer/StructuralElementOverviewNormalizer.php @@ -27,10 +27,10 @@ use App\Mcp\DTO\StructuralElementOverview; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; /** - * Normalizes StructuralElementOverview as a plain {id, name} object, without the JSON-LD "@id"/"@type" - * metadata that ApiPlatform\JsonLd\Serializer\ItemNormalizer would otherwise add (as a blank/genid node, - * since this DTO is intentionally not an API resource). That metadata is useless clutter for the list_X - * MCP tools, whose whole point is a lean id+name overview. + * Normalizes StructuralElementOverview as a plain {id, name, full_path} object, without the JSON-LD + * "@id"/"@type" metadata that ApiPlatform\JsonLd\Serializer\ItemNormalizer would otherwise add (as a + * blank/genid node, since this DTO is intentionally not an API resource). That metadata is useless + * clutter for the list_X MCP tools, whose whole point is a lean overview. * * @see \App\Tests\Serializer\StructuralElementOverviewNormalizerTest */ @@ -42,7 +42,7 @@ class StructuralElementOverviewNormalizer implements NormalizerInterface } /** - * @return array{id: int, name: string} + * @return array{id: int, name: string, full_path: string} */ public function normalize($object, ?string $format = null, array $context = []): array { @@ -53,6 +53,7 @@ class StructuralElementOverviewNormalizer implements NormalizerInterface return [ 'id' => $object->id, 'name' => $object->name, + 'full_path' => $object->full_path, ]; } diff --git a/src/State/Mcp/ListStructuralElementsProcessor.php b/src/State/Mcp/ListStructuralElementsProcessor.php index a3936b59..115312b4 100644 --- a/src/State/Mcp/ListStructuralElementsProcessor.php +++ b/src/State/Mcp/ListStructuralElementsProcessor.php @@ -27,18 +27,20 @@ use ApiPlatform\State\ProcessorInterface; use App\Entity\Base\AbstractStructuralDBElement; use App\Mcp\DTO\StructuralElementOverview; use App\Mcp\DTO\StructuralElementSearchInput; +use App\Services\Trees\NodesListBuilder; use Doctrine\ORM\EntityManagerInterface; /** * Generic list/search processor shared by all structural "master data" entities (categories, footprints, * manufacturers, storage locations, suppliers, measurement units, part custom states). The concrete entity * class is determined from the operation, so this single processor can be reused for all of them. - * Returns a lean id+name overview only; use the corresponding get_X_details tool for full details. + * Returns a lean id+name+full_path overview only; use the corresponding get_X_details tool for full details. */ -class ListStructuralElementsProcessor implements ProcessorInterface +readonly class ListStructuralElementsProcessor implements ProcessorInterface { public function __construct( - private readonly EntityManagerInterface $entityManager, + private EntityManagerInterface $entityManager, + private NodesListBuilder $nodesListBuilder, ) { } @@ -56,23 +58,41 @@ class ListStructuralElementsProcessor implements ProcessorInterface throw new \LogicException(sprintf('%s can only be used for resources extending %s', self::class, AbstractStructuralDBElement::class)); } - $qb = $this->entityManager->getRepository($class)->createQueryBuilder('element'); - $qb->select('element.id AS id', 'element.name AS name'); + if ($data->keyword === null || $data->keyword === '') { + //Without a filter, use the (cached) NodesListBuilder, which already returns the elements in + //correct hierarchical tree order (parents immediately followed by their own children), instead + //of fetching everything and re-sorting it ourselves. + /** @var AbstractStructuralDBElement[] $elements */ + $elements = $this->nodesListBuilder->typeToNodesList($class); - if ($data->keyword !== null && $data->keyword !== '') { - //Escape % and _ characters in the keyword, like PartSearchFilter does - $keyword = str_replace(['%', '_'], ['\%', '\_'], $data->keyword); - $qb->andWhere('ILIKE(element.name, :keyword) = TRUE OR ILIKE(element.comment, :keyword) = TRUE') - ->setParameter('keyword', '%'.$keyword.'%'); + return array_map($this->toOverview(...), $elements); } - $qb->orderBy('element.name', 'ASC'); + $qb = $this->entityManager->getRepository($class)->createQueryBuilder('element'); - $rows = $qb->getQuery()->getArrayResult(); + //Escape % and _ characters in the keyword, like PartSearchFilter does + $keyword = str_replace(['%', '_'], ['\%', '\_'], $data->keyword); + $qb->andWhere('ILIKE(element.name, :keyword) = TRUE OR ILIKE(element.comment, :keyword) = TRUE') + ->setParameter('keyword', '%'.$keyword.'%'); - return array_map( - static fn (array $row): StructuralElementOverview => new StructuralElementOverview(id: $row['id'], name: $row['name']), - $rows + /** @var AbstractStructuralDBElement[] $elements */ + $elements = $qb->getQuery()->getResult(); + + $overviews = array_map($this->toOverview(...), $elements); + + //Sort by full path (rather than the DB query), so that parents are always immediately followed by + //their own children, allowing clients to derive the hierarchical structure from a flat list. + usort($overviews, static fn (StructuralElementOverview $a, StructuralElementOverview $b): int => strnatcasecmp($a->full_path, $b->full_path)); + + return $overviews; + } + + private function toOverview(AbstractStructuralDBElement $element): StructuralElementOverview + { + return new StructuralElementOverview( + id: $element->getID(), + name: $element->getName(), + full_path: $element->getFullPath(), ); } } diff --git a/tests/Serializer/StructuralElementOverviewNormalizerTest.php b/tests/Serializer/StructuralElementOverviewNormalizerTest.php index d657114e..b0fbf467 100644 --- a/tests/Serializer/StructuralElementOverviewNormalizerTest.php +++ b/tests/Serializer/StructuralElementOverviewNormalizerTest.php @@ -38,15 +38,18 @@ final class StructuralElementOverviewNormalizerTest extends TestCase public function testNormalize(): void { - $overview = new StructuralElementOverview(id: 42, name: 'Test Node'); + $overview = new StructuralElementOverview(id: 42, name: 'Test Node', full_path: 'Parent → Test Node'); - //Must be a plain array with exactly id+name, no JSON-LD "@id"/"@type" clutter - $this->assertSame(['id' => 42, 'name' => 'Test Node'], $this->service->normalize($overview)); + //Must be a plain array with exactly id+name+full_path, no JSON-LD "@id"/"@type" clutter + $this->assertSame( + ['id' => 42, 'name' => 'Test Node', 'full_path' => 'Parent → Test Node'], + $this->service->normalize($overview) + ); } public function testSupportsNormalization(): void { $this->assertFalse($this->service->supportsNormalization(new \stdClass())); - $this->assertTrue($this->service->supportsNormalization(new StructuralElementOverview(id: 1, name: 'X'))); + $this->assertTrue($this->service->supportsNormalization(new StructuralElementOverview(id: 1, name: 'X', full_path: 'X'))); } } diff --git a/tests/State/Mcp/StructuralElementProcessorsTest.php b/tests/State/Mcp/StructuralElementProcessorsTest.php index d4fd8122..2f58ac2e 100644 --- a/tests/State/Mcp/StructuralElementProcessorsTest.php +++ b/tests/State/Mcp/StructuralElementProcessorsTest.php @@ -81,9 +81,10 @@ class StructuralElementProcessorsTest extends WebTestCase self::assertGreaterThanOrEqual(7, count($result)); foreach ($result as $overview) { self::assertInstanceOf(StructuralElementOverview::class, $overview); - //The list tool is a lean id+name overview only + //The list tool is a lean id+name+full_path overview only self::assertIsInt($overview->id); self::assertIsString($overview->name); + self::assertIsString($overview->full_path); } } @@ -96,6 +97,8 @@ class StructuralElementProcessorsTest extends WebTestCase self::assertCount(1, $result); self::assertSame('Node 3', $result[0]->name); + //Node 3 is a root node, so its full path is just its own name + self::assertSame('Node 3', $result[0]->full_path); $node3 = $this->em->getRepository(Category::class)->findOneBy(['name' => 'Node 3']); self::assertSame($node3->getID(), $result[0]->id); @@ -113,6 +116,47 @@ class StructuralElementProcessorsTest extends WebTestCase self::assertSame('Node 3', $result[0]->name); } + public function testListIncludesFullPathForNestedElements(): void + { + $result = $this->listProcessor->process( + new StructuralElementSearchInput(keyword: 'Node 1.1.1'), + $this->getOperationForClass(Category::class) + ); + + self::assertCount(1, $result); + self::assertSame('Node 1.1.1', $result[0]->name); + self::assertSame('Node 1 → Node 1.1 → Node 1.1.1', $result[0]->full_path); + } + + public function testListIsSortedByFullPathSoParentsPrecedeTheirChildren(): void + { + $result = $this->listProcessor->process(new StructuralElementSearchInput(), $this->getOperationForClass(Category::class)); + + $fullPaths = array_map(static fn (StructuralElementOverview $overview): string => $overview->full_path, $result); + + //The list must be sorted by full_path itself, not e.g. by name + $sorted = $fullPaths; + sort($sorted, SORT_NATURAL | SORT_FLAG_CASE); + self::assertSame($sorted, $fullPaths); + + //Since parent paths are always a prefix of their children's paths, sorting by full_path means + //"Node 1" is immediately followed by all of its descendants ("Node 1 → ...") before any sibling + //root node that sorts after "Node 1" (e.g. "Node 2") appears. + $node1Index = array_search('Node 1', $fullPaths, true); + $node11Index = array_search('Node 1 → Node 1.1', $fullPaths, true); + $node111Index = array_search('Node 1 → Node 1.1 → Node 1.1.1', $fullPaths, true); + $node2Index = array_search('Node 2', $fullPaths, true); + + self::assertNotFalse($node1Index); + self::assertNotFalse($node11Index); + self::assertNotFalse($node111Index); + self::assertNotFalse($node2Index); + + self::assertLessThan($node11Index, $node1Index); + self::assertLessThan($node111Index, $node11Index); + self::assertLessThan($node2Index, $node111Index); + } + public function testListRejectsNonStructuralResourceClass(): void { $this->expectException(\LogicException::class); @@ -183,7 +227,7 @@ class StructuralElementProcessorsTest extends WebTestCase self::assertNotNull($operation, sprintf('mcp tool "%s" not found on %s', $toolName, $entityClass)); $serializer = self::getContainer()->get(SerializerInterface::class); - $overview = new StructuralElementOverview(id: 123, name: 'Regression test node'); + $overview = new StructuralElementOverview(id: 123, name: 'Regression test node', full_path: 'Parent → Regression test node'); $result = $serializer->serialize([$overview], 'jsonld', [ 'groups' => $operation->getNormalizationContext()['groups'] ?? null, @@ -195,7 +239,8 @@ class StructuralElementProcessorsTest extends WebTestCase self::assertArrayHasKey('hydra:member', $decoded); self::assertSame(123, $decoded['hydra:member'][0]['id'] ?? null); self::assertSame('Regression test node', $decoded['hydra:member'][0]['name'] ?? null); - //The list_X tools are a lean id+name overview - no JSON-LD "@id"/"@type" clutter per item + self::assertSame('Parent → Regression test node', $decoded['hydra:member'][0]['full_path'] ?? null); + //The list_X tools are a lean overview - no JSON-LD "@id"/"@type" clutter per item self::assertArrayNotHasKey('@id', $decoded['hydra:member'][0]); self::assertArrayNotHasKey('@type', $decoded['hydra:member'][0]); }