From 8e98e0749a568f114f40aff21face3f0d5b0bf47 Mon Sep 17 00:00:00 2001 From: Sebastian Krupinski Date: Fri, 4 Sep 2026 20:56:51 -0400 Subject: [PATCH] refactor: nodeList generator Signed-off-by: Sebastian Krupinski --- lib/Providers/Personal/PersonalService.php | 17 ++---- lib/Store/MetaStore.php | 12 +++-- tests/php/Unit/NodeListTest.php | 63 ++++++++++++++++++++++ 3 files changed, 75 insertions(+), 17 deletions(-) create mode 100644 tests/php/Unit/NodeListTest.php diff --git a/lib/Providers/Personal/PersonalService.php b/lib/Providers/Personal/PersonalService.php index 9182e2a..f1146be 100644 --- a/lib/Providers/Personal/PersonalService.php +++ b/lib/Providers/Personal/PersonalService.php @@ -334,7 +334,7 @@ class PersonalService implements ServiceBaseInterface, ServiceCollectionMutableI // If not forcing, ensure the collection is empty if (!$force) { $children = $this->metaStore->nodeList($this->serviceTenantId, $this->serviceUserId, $identifier, false); - if (!empty($children)) { + if ($children->valid()) { throw new InvalidParameterException("Collection is not empty: $identifier"); } } @@ -808,18 +808,11 @@ class PersonalService implements ServiceBaseInterface, ServiceCollectionMutableI // Node operations (unified collections + entities) - public function nodeList(string|int|null $location, ?IFilter $filter = null, ?ISort $sort = null, ?IRange $range = null): array { + public function nodeList(string|int|null $location, ?IFilter $filter = null, ?ISort $sort = null, ?IRange $range = null): Generator { $location = $this->normalizeLocation($location); - $entries = $this->metaStore->nodeList($this->serviceTenantId, $this->serviceUserId, $location, false, $filter, $sort, $range); - // cache nodes - foreach ($entries as $id => $node) { - if ($node instanceof CollectionResource) { - $this->serviceCollectionCache[$id] = $node; - } elseif ($node instanceof EntityResource) { - $this->serviceEntityCache[$id] = $node; - } - } - return $entries; + // Do not retain yielded nodes in the service caches: large consumers must + // be able to release each node as they advance through the cursor. + yield from $this->metaStore->nodeList($this->serviceTenantId, $this->serviceUserId, $location, false, $filter, $sort, $range); } public function nodeListFilter(): IFilter { diff --git a/lib/Store/MetaStore.php b/lib/Store/MetaStore.php index 9a69d80..db8d882 100644 --- a/lib/Store/MetaStore.php +++ b/lib/Store/MetaStore.php @@ -531,9 +531,11 @@ class MetaStore { // ========== Node Operations (Unified/Recursive) ========== /** - * List all nodes (collections and entities) + * Lazily yield nodes from the database cursor, keyed by identifier. + * + * @return \Generator */ - public function nodeList(string $tenantId, string $userId, string|int|null $location = null, bool $recursive = false, ?Filter $filter = null, ?Sort $sort = null, ?IRange $range = null): array { + public function nodeList(string $tenantId, string $userId, string|int|null $location = null, bool $recursive = false, ?Filter $filter = null, ?Sort $sort = null, ?IRange $range = null): \Generator { $query = [ 'tid' => $tenantId, 'uid' => $userId, @@ -573,17 +575,17 @@ class MetaStore { } $cursor = $this->_store->selectCollection($this->_NodeTable)->find($query, $options); - $list = []; foreach ($cursor as $entry) { $nodeType = $entry['type']; if ($nodeType === NodeType::Collection->value) { $node = (new CollectionResource())->fromStore($entry); } else if ($nodeType === NodeType::Entity->value) { $node = (new EntityResource())->fromStore($entry); + } else { + continue; } - $list[$node->identifier()] = $node; + yield $node->identifier() => $node; } - return $list; } /** diff --git a/tests/php/Unit/NodeListTest.php b/tests/php/Unit/NodeListTest.php new file mode 100644 index 0000000..6c06418 --- /dev/null +++ b/tests/php/Unit/NodeListTest.php @@ -0,0 +1,63 @@ + 'folder', 'cid' => 'parent', 'type' => NodeType::Collection->value]; + $consumed++; + yield ['nid' => 'file', 'cid' => 'parent', 'type' => NodeType::Entity->value]; + })(); + $collection = $this->createMock(Collection::class); + $collection->expects(self::once())->method('find')->with( + ['tid' => 'tenant', 'uid' => 'user', 'cid' => 'parent'], + self::callback(static fn(array $options): bool => + $options['skip'] === 64 && $options['limit'] === 64 && $options['sort']['type'] === 1), + )->willReturn(new Cursor($rows)); + $database = $this->createMock(DataStore::class); + $database->expects(self::once())->method('selectCollection')->willReturn($collection); + $range = new RangeTally(); + $range->setPosition(64); + $range->setTally(64); + $nodes = (new MetaStore($database))->nodeList('tenant', 'user', 'parent', false, null, null, $range); + + self::assertSame(0, $consumed); + self::assertTrue($nodes->valid()); + self::assertSame(1, $consumed); + self::assertSame('folder', $nodes->key()); + self::assertInstanceOf(CollectionResource::class, $nodes->current()); + $nodes->next(); + self::assertSame(2, $consumed); + self::assertSame('file', $nodes->key()); + self::assertInstanceOf(EntityResource::class, $nodes->current()); + $nodes->next(); + self::assertFalse($nodes->valid()); + } + + public function testEmptyCursorProducesAnEmptyGenerator(): void + { + $collection = $this->createStub(Collection::class); + $collection->method('find')->willReturn(new Cursor(new \ArrayIterator([]))); + $database = $this->createStub(DataStore::class); + $database->method('selectCollection')->willReturn($collection); + $nodes = (new MetaStore($database))->nodeList('tenant', 'user', 'parent'); + self::assertFalse($nodes->valid()); + } +}