From e5ce6391e9e7807d038614d28a884f75dd4f427b Mon Sep 17 00:00:00 2001 From: John Koster Date: Fri, 8 Mar 2024 10:18:00 -0600 Subject: [PATCH 1/4] Implements `pluck` on query builder This implementation will resolve values from the Stache indexes --- src/Stache/Query/Builder.php | 34 +++++++++++-- src/Stache/Stores/AggregateStore.php | 9 ++++ src/Stache/Stores/Store.php | 51 ++++++++++++++++++++ tests/Data/Entries/EntryQueryBuilderTest.php | 31 ++++++++++++ 4 files changed, 122 insertions(+), 3 deletions(-) diff --git a/src/Stache/Query/Builder.php b/src/Stache/Query/Builder.php index 139d5553d7f..5381cd09cf0 100644 --- a/src/Stache/Query/Builder.php +++ b/src/Stache/Query/Builder.php @@ -2,6 +2,7 @@ namespace Statamic\Stache\Query; +use Illuminate\Support\Str; use Statamic\Data\DataCollection; use Statamic\Query\Builder as BaseBuilder; use Statamic\Stache\Stores\Store; @@ -21,15 +22,42 @@ public function count() return $this->getFilteredAndLimitedKeys()->count(); } - public function get($columns = ['*']) + protected function resolveKeys() { $keys = $this->getFilteredKeys(); $keys = $this->orderKeys($keys); - $keys = $this->limitKeys($keys); + return $this->limitKeys($keys); + } + + public function pluck($column, $key = null) + { + $keys = $this->resolveKeys(); + + return $this->store->getFromIndex( + $this->getKeysForIndexQuery($keys), + $column, + $key + ); + } + + protected function getKeysForIndexQuery($keys) + { + return $keys->map(function ($key) { + $queryKey = Str::after($key, '::'); + + if (! Str::contains($queryKey, '-') && is_numeric($queryKey)) { + return intval($queryKey); + } + + return $queryKey; + }); + } - $items = $this->getItems($keys); + public function get($columns = ['*']) + { + $items = $this->getItems($this->resolveKeys()); $items->each(fn ($item) => $item ->selectedQueryColumns($this->columns ?? $columns) diff --git a/src/Stache/Stores/AggregateStore.php b/src/Stache/Stores/AggregateStore.php index a531aaeca6b..8c410d660a7 100644 --- a/src/Stache/Stores/AggregateStore.php +++ b/src/Stache/Stores/AggregateStore.php @@ -14,6 +14,15 @@ public function __construct() $this->stores = collect(); } + protected function resolveFromIndex($keys, $column) + { + return $this->stores()->mapWithKeys(function ($store) use ($column) { + return $store->resolveIndex($column)->load()->items(); + })->where(function ($value, $key) use (&$keys) { + return $keys->has($key); + }); + } + public function store($key) { if (! $this->stores->has($key)) { diff --git a/src/Stache/Stores/Store.php b/src/Stache/Stores/Store.php index 41dc66bc9f5..37346e91706 100644 --- a/src/Stache/Stores/Store.php +++ b/src/Stache/Stores/Store.php @@ -27,6 +27,57 @@ abstract class Store protected $shouldCacheFileItems = false; protected $modified; protected $keys; + protected $identifiedBy = 'id'; + + protected function resolveFromIndex($keys, $column) + { + return $this->resolveIndex($column) + ->load() + ->items() + ->where(function ($value, $key) use (&$keys) { + return $keys->has($key); + }); + } + + private function isValidKey($value) + { + if (is_string($value) || is_int($value)) { + return true; + } + + return false; + } + + public function getFromIndex($keys, $column, $key = null) + { + if ($column === $this->identifiedBy && $key === null) { + return $keys; + } + + $keys = $keys->flip(); + $values = $this->resolveFromIndex($keys, $column); + + if ($key === null) { + return $values->values(); + } + + $keyValues = $this->resolveFromIndex($keys, $key); + $newValues = []; + + foreach ($keys->keys() as $keyValue) { + $newKeyValue = $keyValues[$keyValue] ?? null; + + if (! $this->isValidKey($newKeyValue)) { + continue; + } + + $newValue = $values[$keyValue] ?? null; + + $newValues[$newKeyValue] = $newValue; + } + + return collect($newValues); + } public function directory($directory = null) { diff --git a/tests/Data/Entries/EntryQueryBuilderTest.php b/tests/Data/Entries/EntryQueryBuilderTest.php index ac60eedbc20..43753b0ee27 100644 --- a/tests/Data/Entries/EntryQueryBuilderTest.php +++ b/tests/Data/Entries/EntryQueryBuilderTest.php @@ -4,6 +4,7 @@ use Facades\Tests\Factories\EntryFactory; use Illuminate\Support\Carbon; +use Illuminate\Support\Str; use Statamic\Facades\Blueprint; use Statamic\Facades\Collection; use Statamic\Facades\Entry; @@ -769,4 +770,34 @@ public function entries_are_found_using_lazy() $this->assertInstanceOf(\Illuminate\Support\LazyCollection::class, $entries); $this->assertCount(3, $entries); } + + /** @test */ + public function pluck_can_be_used_to_retrieve_values_from_index() + { + $this->createDummyCollectionAndEntries(); + + $this->assertEquals(collect([1, 2, 3]), Entry::query()->pluck('id')); + + $paths = Entry::query()->pluck('path')->map(fn ($path) => Str::afterLast($path, '/')); + + $this->assertEquals(collect([ + 'post-1.md', + 'post-2.md', + 'post-3.md', + ]), $paths); + + $this->assertEquals(collect([ + 1 => 'post-1', + 2 => 'post-2', + 3 => 'post-3', + ]), Entry::query()->pluck('slug', 'id')); + + $this->assertEquals(collect([ + 3 => 'post-3', + ]), Entry::query()->where('id', 3)->pluck('slug', 'id')); + + $this->assertEquals(collect([ + 'post-3' => 3, + ]), Entry::query()->where('id', 3)->pluck('id', 'slug')); + } } From 394b73af90801e6ee3a8559affcedc57b12b9359 Mon Sep 17 00:00:00 2001 From: John Koster Date: Fri, 8 Mar 2024 10:18:34 -0600 Subject: [PATCH 2/4] Refactor collection structure to use pluck when validating trees --- src/Structures/CollectionStructure.php | 3 +- .../Structures/CollectionStructureTest.php | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/Structures/CollectionStructure.php b/src/Structures/CollectionStructure.php index e2716b03358..3edb98f43a6 100644 --- a/src/Structures/CollectionStructure.php +++ b/src/Structures/CollectionStructure.php @@ -74,8 +74,7 @@ public function validateTree(array $tree, string $locale): array $thisCollectionsEntries = $this->collection()->queryEntries() ->where('site', $locale) - ->get(['id', 'site']) - ->map->id(); + ->pluck('id'); $otherCollectionEntries = $entryIds->diff($thisCollectionsEntries); diff --git a/tests/Data/Structures/CollectionStructureTest.php b/tests/Data/Structures/CollectionStructureTest.php index 4cd3f322ec5..198115dd30b 100644 --- a/tests/Data/Structures/CollectionStructureTest.php +++ b/tests/Data/Structures/CollectionStructureTest.php @@ -19,6 +19,7 @@ class CollectionStructureTest extends StructureTestCase private $collection; private $entryQueryBuilder; private $queryBuilderGetReturnValue; + private $queryBuilderPluckReturnValue; public function setUp(): void { @@ -30,6 +31,9 @@ public function setUp(): void $this->entryQueryBuilder->shouldReceive('get')->andReturnUsing(function () { return $this->queryBuilderGetReturnValue(); }); + $this->entryQueryBuilder->shouldReceive('pluck')->andReturnUsing(function () { + return $this->queryBuilderPluckReturnValue(); + }); $this->collection = $this->mock(Collection::class); $this->collection->shouldReceive('queryEntries')->andReturn($this->entryQueryBuilder); @@ -47,6 +51,11 @@ public function queryBuilderGetReturnValue() return $this->queryBuilderGetReturnValue ?? collect(); } + public function queryBuilderPluckReturnValue() + { + return $this->queryBuilderPluckReturnValue ?? collect(); + } + /** @test */ public function it_gets_and_sets_the_handle() { @@ -84,6 +93,10 @@ public function it_makes_a_tree() Entry::make()->id('1'), ]); + $this->queryBuilderPluckReturnValue = collect([ + 1, + ]); + $tree = $structure->makeTree('fr', [ ['entry' => 1], ]); @@ -261,6 +274,11 @@ public function the_tree_root_can_have_children_when_not_expecting_root() Entry::make()->id('456'), ]); + $this->queryBuilderPluckReturnValue = collect([ + 123, + 456, + ]); + parent::the_tree_root_can_have_children_when_not_expecting_root(); } @@ -274,6 +292,11 @@ public function only_entries_belonging_to_the_associated_collection_may_be_in_th Entry::make()->id('2'), ]); + $this->queryBuilderPluckReturnValue = collect([ + 1, + 2, + ]); + $validated = $this->structure('test')->validateTree([ [ 'entry' => '1', @@ -308,6 +331,14 @@ public function entries_not_explicitly_in_the_tree_should_be_appended_to_the_end Entry::make()->id('5'), ]); + $this->queryBuilderPluckReturnValue = collect([ + 1, + 2, + 3, + 4, + 5, + ]); + $actual = $this->structure('test')->validateTree([ [ 'entry' => '1', From d7740ec6e84dcbe3f881df45208b85519b92d96f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 29 Mar 2024 15:31:22 -0400 Subject: [PATCH 3/4] remove leftovers from merge --- src/Stache/Stores/AggregateStore.php | 9 ----- src/Stache/Stores/Store.php | 51 ---------------------------- 2 files changed, 60 deletions(-) diff --git a/src/Stache/Stores/AggregateStore.php b/src/Stache/Stores/AggregateStore.php index 40330377f92..334c0b63835 100644 --- a/src/Stache/Stores/AggregateStore.php +++ b/src/Stache/Stores/AggregateStore.php @@ -14,15 +14,6 @@ public function __construct() $this->stores = collect(); } - protected function resolveFromIndex($keys, $column) - { - return $this->stores()->mapWithKeys(function ($store) use ($column) { - return $store->resolveIndex($column)->load()->items(); - })->where(function ($value, $key) use (&$keys) { - return $keys->has($key); - }); - } - public function store($key) { if (! $this->stores->has($key)) { diff --git a/src/Stache/Stores/Store.php b/src/Stache/Stores/Store.php index ff1f422c46b..65785353faf 100644 --- a/src/Stache/Stores/Store.php +++ b/src/Stache/Stores/Store.php @@ -27,57 +27,6 @@ abstract class Store protected $shouldCacheFileItems = false; protected $modified; protected $keys; - protected $identifiedBy = 'id'; - - protected function resolveFromIndex($keys, $column) - { - return $this->resolveIndex($column) - ->load() - ->items() - ->where(function ($value, $key) use (&$keys) { - return $keys->has($key); - }); - } - - private function isValidKey($value) - { - if (is_string($value) || is_int($value)) { - return true; - } - - return false; - } - - public function getFromIndex($keys, $column, $key = null) - { - if ($column === $this->identifiedBy && $key === null) { - return $keys; - } - - $keys = $keys->flip(); - $values = $this->resolveFromIndex($keys, $column); - - if ($key === null) { - return $values->values(); - } - - $keyValues = $this->resolveFromIndex($keys, $key); - $newValues = []; - - foreach ($keys->keys() as $keyValue) { - $newKeyValue = $keyValues[$keyValue] ?? null; - - if (! $this->isValidKey($newKeyValue)) { - continue; - } - - $newValue = $values[$keyValue] ?? null; - - $newValues[$newKeyValue] = $newValue; - } - - return collect($newValues); - } public function directory($directory = null) { From c4d2622f2afc6ad49f6ee2207ba4188217ec6b4b Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 29 Mar 2024 15:37:01 -0400 Subject: [PATCH 4/4] now that we're plucking we dont need to get. assert that. --- .../Structures/CollectionStructureTest.php | 33 +------------------ 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/tests/Data/Structures/CollectionStructureTest.php b/tests/Data/Structures/CollectionStructureTest.php index 198115dd30b..13479040892 100644 --- a/tests/Data/Structures/CollectionStructureTest.php +++ b/tests/Data/Structures/CollectionStructureTest.php @@ -7,7 +7,6 @@ use Statamic\Contracts\Entries\Entry as EntryContract; use Statamic\Facades; use Statamic\Facades\Blink; -use Statamic\Facades\Entry; use Statamic\Stache\Query\EntryQueryBuilder; use Statamic\Structures\CollectionStructure; use Statamic\Structures\CollectionTree; @@ -18,7 +17,6 @@ class CollectionStructureTest extends StructureTestCase { private $collection; private $entryQueryBuilder; - private $queryBuilderGetReturnValue; private $queryBuilderPluckReturnValue; public function setUp(): void @@ -28,9 +26,7 @@ public function setUp(): void $this->entryQueryBuilder = $this->mock(EntryQueryBuilder::class); $this->entryQueryBuilder->shouldReceive('where')->with('site', 'en')->andReturnSelf(); $this->entryQueryBuilder->shouldReceive('where')->with('site', 'fr')->andReturnSelf(); - $this->entryQueryBuilder->shouldReceive('get')->andReturnUsing(function () { - return $this->queryBuilderGetReturnValue(); - }); + $this->entryQueryBuilder->shouldReceive('get')->never(); $this->entryQueryBuilder->shouldReceive('pluck')->andReturnUsing(function () { return $this->queryBuilderPluckReturnValue(); }); @@ -46,11 +42,6 @@ public function structure($handle = null) return (new CollectionStructure)->handle($handle); } - public function queryBuilderGetReturnValue() - { - return $this->queryBuilderGetReturnValue ?? collect(); - } - public function queryBuilderPluckReturnValue() { return $this->queryBuilderPluckReturnValue ?? collect(); @@ -89,10 +80,6 @@ public function it_makes_a_tree() $this->collection->shouldReceive('structure')->andReturn($structure); $this->collection->shouldReceive('handle')->andReturn('test'); - $this->queryBuilderGetReturnValue = collect([ - Entry::make()->id('1'), - ]); - $this->queryBuilderPluckReturnValue = collect([ 1, ]); @@ -269,11 +256,6 @@ public function the_tree_root_can_have_children_when_not_expecting_root() { Facades\Collection::shouldReceive('findByHandle')->with('test')->andReturn($this->collection); - $this->queryBuilderGetReturnValue = collect([ - Entry::make()->id('123'), - Entry::make()->id('456'), - ]); - $this->queryBuilderPluckReturnValue = collect([ 123, 456, @@ -287,11 +269,6 @@ public function only_entries_belonging_to_the_associated_collection_may_be_in_th { Facades\Collection::shouldReceive('findByHandle')->with('test')->andReturn($this->collection); - $this->queryBuilderGetReturnValue = collect([ - Entry::make()->id('1'), - Entry::make()->id('2'), - ]); - $this->queryBuilderPluckReturnValue = collect([ 1, 2, @@ -323,14 +300,6 @@ public function entries_not_explicitly_in_the_tree_should_be_appended_to_the_end { Facades\Collection::shouldReceive('findByHandle')->with('test')->andReturn($this->collection); - $this->queryBuilderGetReturnValue = collect([ - Entry::make()->id('1'), - Entry::make()->id('2'), - Entry::make()->id('3'), - Entry::make()->id('4'), - Entry::make()->id('5'), - ]); - $this->queryBuilderPluckReturnValue = collect([ 1, 2,