From ee9a22ae232083e53bd86208e6a8bb0f459d646d Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 27 Jul 2023 12:12:51 -0400 Subject: [PATCH 01/17] remove unused method --- src/Entries/Entry.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index f6a9fd5b8ea..78efce4e999 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -669,15 +669,6 @@ public function existsIn($locale) return $this->in($locale) !== null; } - public function addLocalization($entry) - { - $entry->origin($this); - - $this->localizations[$entry->locale()] = $entry; - - return $this; - } - public function makeLocalization($site) { $localization = Facades\Entry::make() From f90671573c137e6ecd5cd196f918973cd0bcd1c0 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 27 Jul 2023 12:13:02 -0400 Subject: [PATCH 02/17] dont cache to property --- src/Entries/Entry.php | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index 78efce4e999..a8b18a690cd 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -66,7 +66,6 @@ class Entry implements Contract, Augmentable, Responsable, Localization, Protect protected $blueprint; protected $date; protected $locale; - protected $localizations; protected $afterSaveCallbacks = []; protected $withEvents = true; protected $template; @@ -200,8 +199,6 @@ public function deleteDescendants() $entry->delete(); }); - $this->localizations = null; - return true; } @@ -648,14 +645,10 @@ public function in($locale) public function descendants() { - if (! $this->localizations) { - $this->localizations = Facades\Entry::query() - ->where('collection', $this->collectionHandle()) - ->where('origin', $this->id())->get() - ->keyBy->locale(); - } - - $localizations = collect($this->localizations); + $localizations = Facades\Entry::query() + ->where('collection', $this->collectionHandle()) + ->where('origin', $this->id())->get() + ->keyBy->locale(); foreach ($localizations as $loc) { $localizations = $localizations->merge($loc->descendants()); From 3f9f30a374455cb9e0bfce726dbaf51c17ad4f7e Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 27 Jul 2023 12:20:43 -0400 Subject: [PATCH 03/17] actually, just deprecate it otherwise its a breaking change --- src/Entries/Entry.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index a8b18a690cd..723aef19c21 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -662,6 +662,14 @@ public function existsIn($locale) return $this->in($locale) !== null; } + /** @deprecated */ + public function addLocalization($entry) + { + $entry->origin($this); + + return $this; + } + public function makeLocalization($site) { $localization = Facades\Entry::make() From 741ba4238d5423344dd750c1e3ec4abfd4bd7020 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 27 Jul 2023 16:48:32 -0400 Subject: [PATCH 04/17] blink descendants --- src/Entries/Entry.php | 28 +++++++++++++++++--- tests/Data/Entries/EntryTest.php | 44 +++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index 723aef19c21..45f2ec15353 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -199,6 +199,8 @@ public function deleteDescendants() $entry->delete(); }); + Blink::forget('entry-descendants-'.$this->id()); + return true; } @@ -326,6 +328,8 @@ public function save() Blink::store('structure-entries')->forget($this->id()); } + $this->ancestors()->each(fn ($entry) => Blink::forget('entry-descendants-'.$entry->id())); + $this->taxonomize(); optional(Collection::findByMount($this))->updateEntryUris(); @@ -643,12 +647,28 @@ public function in($locale) return $this->descendants()->get($locale); } + public function ancestors() + { + $ancestors = collect(); + + $origin = $this->origin(); + + while ($origin) { + $ancestors->push($origin); + $origin = $origin->origin(); + } + + return $ancestors; + } + public function descendants() { - $localizations = Facades\Entry::query() - ->where('collection', $this->collectionHandle()) - ->where('origin', $this->id())->get() - ->keyBy->locale(); + $localizations = Blink::once('entry-descendants-'.$this->id(), function () { + return Facades\Entry::query() + ->where('collection', $this->collectionHandle()) + ->where('origin', $this->id())->get() + ->keyBy->locale(); + }); foreach ($localizations as $loc) { $localizations = $localizations->merge($loc->descendants()); diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index 5d9fa2a331e..3a8f0bb3ed7 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -2097,5 +2097,47 @@ public function it_gets_preview_targets() ], $entryDe->previewTargets()->all()); } - // todo: add tests for localization things. in(), descendants(), addLocalization(), etc + /** @test */ + public function it_gets_descendants() + { + Facades\Site::setConfig(['default' => 'en', 'sites' => [ + 'en' => ['locale' => 'en_US', 'url' => '/'], + 'fr' => ['locale' => 'fr_FR', 'url' => '/fr/'], + 'fr_CA' => ['locale' => 'fr_CA', 'url' => '/fr-ca/'], + 'de' => ['locale' => 'de_DE', 'url' => '/de/'], + ]]); + + $one = EntryFactory::collection('test')->id('1')->locale('en')->create(); + $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); + $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('fr_CA')->create(); + $four = EntryFactory::collection('test')->id('4')->origin('2')->locale('de')->create(); + + $this->assertEquals(['fr' => $two, 'fr_CA' => $three, 'de' => $four], $one->descendants()->all()); + $this->assertEquals(['fr_CA' => $three, 'de' => $four], $two->descendants()->all()); + $this->assertEquals([], $three->descendants()->all()); + $this->assertEquals([], $four->descendants()->all()); + } + + /** @test */ + public function it_gets_ancestors() + { + Facades\Site::setConfig(['default' => 'en', 'sites' => [ + 'en' => ['locale' => 'en_US', 'url' => '/'], + 'fr' => ['locale' => 'fr_FR', 'url' => '/fr/'], + 'fr_CA' => ['locale' => 'fr_CA', 'url' => '/fr-ca/'], + 'de' => ['locale' => 'de_DE', 'url' => '/de/'], + ]]); + + $one = EntryFactory::collection('test')->id('1')->locale('en')->create(); + $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); + $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('fr_CA')->create(); + $four = EntryFactory::collection('test')->id('4')->origin('2')->locale('de')->create(); + + $this->assertEquals([], $one->ancestors()->all()); + $this->assertEquals([$one], $two->ancestors()->all()); + $this->assertEquals([$two, $one], $three->ancestors()->all()); + $this->assertEquals([$two, $one], $four->ancestors()->all()); + } + + // todo: add tests for localization things. in(), addLocalization(), etc } From 4526173b0d9e83d425d6ce79ee5de63211761991 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 27 Jul 2023 16:48:57 -0400 Subject: [PATCH 05/17] adjust test to avoid using deprecated method --- tests/Routing/UrlBuilderTest.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/Routing/UrlBuilderTest.php b/tests/Routing/UrlBuilderTest.php index 74c73550d61..76eb96a0f09 100644 --- a/tests/Routing/UrlBuilderTest.php +++ b/tests/Routing/UrlBuilderTest.php @@ -30,7 +30,7 @@ public function setUp(): void 'fr' => ['url' => '/fr/', 'locale' => 'fr_FR'], ]]); - $entry = \Statamic\Facades\Entry::make() + $entry = tap(\Statamic\Facades\Entry::make() ->id('post') ->locale('en') ->collection( @@ -38,11 +38,10 @@ public function setUp(): void ) ->slug('post') ->date('2015-01-02') - ->data(['foo' => 'bar', 'slashed' => 'foo/bar']); + ->data(['foo' => 'bar', 'slashed' => 'foo/bar']) + )->save(); - $entry->addLocalization( - $entry->makeLocalization('fr')->slug('le-post') - ); + $entry->makeLocalization('fr')->slug('le-post')->save(); $this->builder = app(UrlBuilder::class)->content($entry); $this->entry = $entry; From cac82c7b1a32ec075a998bea0ee4ed40b2251326 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 10:18:56 -0400 Subject: [PATCH 06/17] update tests to use factories since upcoming changes will require that the entries need to actually exist --- tests/Data/Entries/EntryTest.php | 42 +++++++++++++++++--------------- tests/Factories/EntryFactory.php | 12 ++++++++- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index 3a8f0bb3ed7..dd66367d1a2 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -24,6 +24,7 @@ use Statamic\Events\EntrySaved; use Statamic\Events\EntrySaving; use Statamic\Facades; +use Statamic\Facades\Blink; use Statamic\Fields\Blueprint; use Statamic\Fields\Fieldtype; use Statamic\Fields\Value; @@ -293,8 +294,8 @@ public function it_merges_in_additional_data() public function values_fall_back_to_the_origin_then_the_collection() { $collection = tap(Collection::make('test'))->save(); - $origin = (new Entry)->collection('test'); - $entry = (new Entry)->origin($origin)->collection('test'); + $origin = EntryFactory::collection('test')->create(); + $entry = EntryFactory::origin($origin)->collection('test')->create(); $this->assertNull($entry->value('test')); @@ -314,14 +315,14 @@ public function it_gets_values_from_origin_and_collection() 'three' => 'three in collection', ]))->save(); - $origin = (new Entry)->collection('test')->data([ + $origin = EntryFactory::collection('test')->data([ 'two' => 'two in origin', 'three' => 'three in origin', - ]); + ])->create(); - $entry = (new Entry)->origin($origin)->collection('test')->data([ + $entry = EntryFactory::origin($origin)->collection('test')->data([ 'three' => 'three in entry', - ]); + ])->create(); $this->assertEquals([ 'one' => 'one in collection', @@ -344,16 +345,16 @@ public function if_the_value_is_explicitly_set_to_null_then_it_should_not_fall_b 'four' => 'four in collection', ]))->save(); - $origin = (new Entry)->collection('test')->data([ + $origin = EntryFactory::collection('test')->data([ 'two' => null, 'three' => 'three in origin', 'four' => 'four in origin', - ]); + ])->create(); - $entry = (new Entry)->origin($origin)->collection('test')->data([ + $entry = EntryFactory::origin($origin)->collection('test')->data([ 'three' => null, 'four' => 'four in entry', - ]); + ])->create(); $this->assertEquals([ 'one' => 'one in collection', // falls all the way back @@ -1172,8 +1173,8 @@ public function it_gets_the_blueprint_when_defined_in_an_origin_value() 'second' => $second = (new Blueprint)->setHandle('second'), ])); Collection::make('blog')->save(); - $origin = (new Entry)->collection('blog')->set('blueprint', 'second'); - $entry = (new Entry)->collection('blog')->origin($origin); + $origin = EntryFactory::collection('blog')->data(['blueprint' => 'second'])->create(); + $entry = EntryFactory::collection('blog')->origin($origin)->create(); $this->assertSame($second, $entry->blueprint()); $this->assertNotSame($first, $second); @@ -1187,8 +1188,8 @@ public function it_gets_the_blueprint_when_defined_in_an_origin_property() 'second' => $second = (new Blueprint)->setHandle('second'), ])); Collection::make('blog')->save(); - $origin = (new Entry)->collection('blog')->blueprint('second'); - $entry = (new Entry)->collection('blog')->origin($origin); + $origin = EntryFactory::collection('blog')->blueprint('second')->create(); + $entry = EntryFactory::collection('blog')->origin($origin)->create(); $this->assertSame($second, $entry->blueprint()); $this->assertNotSame($first, $second); @@ -1258,6 +1259,7 @@ public function it_saves_through_the_api() Facades\Entry::shouldReceive('save')->with($entry); Facades\Entry::shouldReceive('taxonomize')->with($entry); Facades\Entry::shouldReceive('find')->with('a')->once()->andReturnNull(); + Blink::put('entry-descendants-a', collect()); // Prevents the query needing to be mocked. $return = $entry->save(); @@ -1283,6 +1285,7 @@ public function it_dispatches_entry_created_only_once() Facades\Entry::shouldReceive('save')->with($entry); Facades\Entry::shouldReceive('taxonomize')->with($entry); Facades\Entry::shouldReceive('find')->with('1')->times(3)->andReturn(null, $entry, $entry); + Blink::put('entry-descendants-1', collect()); // Prevents the query needing to be mocked. $entry->save(); $entry->save(); @@ -1302,6 +1305,7 @@ public function it_saves_quietly() Facades\Entry::shouldReceive('save')->with($entry); Facades\Entry::shouldReceive('taxonomize')->with($entry); Facades\Entry::shouldReceive('find')->with('a')->once()->andReturnNull(); + Blink::put('entry-descendants-a', collect()); // Prevents the query needing to be mocked. $return = $entry->saveQuietly(); @@ -1352,7 +1356,7 @@ public function it_performs_callbacks_after_saving_but_before_the_saved_event_an Event::fake(); $collection = (new Collection)->handle('pages')->save(); - $entry = (new Entry)->id('a')->collection($collection); + $entry = EntryFactory::id('a')->collection($collection)->create(); Facades\Entry::shouldReceive('save')->with($entry); Facades\Entry::shouldReceive('taxonomize')->with($entry); Facades\Entry::shouldReceive('find')->with('a')->times(2)->andReturn(null, $entry); @@ -1725,8 +1729,8 @@ public function the_blueprint_is_added_to_the_localized_file_contents_if_explici public function it_gets_and_sets_the_template() { $collection = tap(Collection::make('test'))->save(); - $origin = (new Entry)->collection($collection); - $entry = (new Entry)->collection($collection)->origin($origin); + $origin = EntryFactory::collection($collection)->create(); + $entry = EntryFactory::collection($collection)->origin($origin)->create(); // defaults to default $this->assertEquals('default', $entry->template()); @@ -1769,8 +1773,8 @@ public function it_gets_and_sets_an_inferred_template_from_blueprint() public function it_gets_and_sets_the_layout() { $collection = tap(Collection::make('test'))->save(); - $origin = (new Entry)->collection($collection); - $entry = (new Entry)->collection($collection)->origin($origin); + $origin = EntryFactory::collection($collection)->create(); + $entry = EntryFactory::collection($collection)->origin($origin)->create(); // defaults to layout $this->assertEquals('layout', $entry->layout()); diff --git a/tests/Factories/EntryFactory.php b/tests/Factories/EntryFactory.php index 306a6ebee13..d1efd30a52f 100644 --- a/tests/Factories/EntryFactory.php +++ b/tests/Factories/EntryFactory.php @@ -17,6 +17,7 @@ class EntryFactory protected $locale; protected $origin; protected $collection; + protected $blueprint; public function __construct() { @@ -79,6 +80,13 @@ public function origin($origin) return $this; } + public function blueprint($blueprint) + { + $this->blueprint = $blueprint; + + return $this; + } + public function make() { $entry = Entry::make() @@ -87,7 +95,8 @@ public function make() ->slug($this->slug) ->data($this->data) ->origin($this->origin) - ->published($this->published); + ->published($this->published) + ->blueprint($this->blueprint); if ($collection->dated()) { $entry->date($this->date); @@ -130,5 +139,6 @@ private function reset() $this->locale = 'en'; $this->origin = null; $this->collection = null; + $this->blueprint = null; } } From 13245f7a4c711f7f6cb9f4fcdead8f88a0b430d0 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 10:19:20 -0400 Subject: [PATCH 07/17] add test --- tests/Data/Entries/EntryTest.php | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index dd66367d1a2..1b87caea456 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -2144,4 +2144,42 @@ public function it_gets_ancestors() } // todo: add tests for localization things. in(), addLocalization(), etc + + /** @test */ + public function it_updates_the_origin_of_descendants_when_saving_an_entry_with_localizations() + { + // The issue this test is covering doesn't happen when using the + // array cache driver, since the objects are stored in memory. + config(['cache.default' => 'file']); + Cache::clear(); + + Facades\Site::setConfig([ + 'default' => 'en', + 'sites' => [ + 'en' => ['name' => 'English', 'locale' => 'en_US', 'url' => '/'], + 'fr' => ['name' => 'French', 'locale' => 'fr_FR', 'url' => '/fr/'], + 'de' => ['name' => 'German', 'locale' => 'de_DE', 'url' => '/de/'], + ], + ]); + + $one = EntryFactory::collection('test')->id('1')->locale('en')->data(['foo' => 'root'])->create(); + $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); + $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('de')->create(); + + $this->assertEquals('root', $one->foo); + $this->assertEquals('root', $two->foo); + $this->assertEquals('root', $three->foo); + + $one->data(['foo' => 'root updated'])->save(); + + $this->assertEquals('root updated', $one->foo); + $this->assertEquals('root updated', $two->foo); + $this->assertEquals('root updated', $three->foo); + + $two->data(['foo' => 'two updated'])->save(); + + $this->assertEquals('root updated', $one->foo); + $this->assertEquals('two updated', $two->foo); + $this->assertEquals('two updated', $three->foo); + } } From ce59513677d79f1cf5410e6b104e09149c0f47a8 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 10:20:15 -0400 Subject: [PATCH 08/17] only store the origin id, but blink it for performance --- src/Data/HasOrigin.php | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Data/HasOrigin.php b/src/Data/HasOrigin.php index dde3e1fb736..3148987a0cd 100644 --- a/src/Data/HasOrigin.php +++ b/src/Data/HasOrigin.php @@ -2,8 +2,13 @@ namespace Statamic\Data; +use Statamic\Facades\Blink; + trait HasOrigin { + /** + * @var string + */ protected $origin; public function keys() @@ -53,13 +58,18 @@ public function value($key) public function origin($origin = null) { + $key = 'origin-'.class_basename($this).'-'.$this->id(); + return $this->fluentlyGetOrSet('origin') - ->getter(function ($origin) { - if (is_string($origin)) { - $this->origin = $origin = $this->getOriginByString($origin); - } + ->getter(function ($origin) use ($key) { + return $origin + ? Blink::once($key, fn () => $this->getOriginByString($origin)) + : null; + }) + ->setter(function ($origin) use ($key) { + Blink::forget($key); - return $origin; + return is_string($origin) || is_null($origin) ? $origin : $origin->id(); }) ->args(func_get_args()); } From 63a83e3a7ca09061a880d316362c6bde311b71e5 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 10:20:22 -0400 Subject: [PATCH 09/17] save descendants on save --- src/Entries/Entry.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index 45f2ec15353..827c116b87e 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -330,6 +330,8 @@ public function save() $this->ancestors()->each(fn ($entry) => Blink::forget('entry-descendants-'.$entry->id())); + $this->descendants()->each->save(); + $this->taxonomize(); optional(Collection::findByMount($this))->updateEntryUris(); From ecb10f87786d9a30f66a39ae79ae50a6322ecc61 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 14:28:00 -0400 Subject: [PATCH 10/17] Clear the blink key ... It worked fine in the browser because the blink would be empty on the next request. In the test its all one process, and the blink would stick around. This would be the same problem in a worker too. --- src/Entries/Entry.php | 1 + tests/Data/Entries/EntryTest.php | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index 827c116b87e..72c481318ed 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -326,6 +326,7 @@ public function save() if ($this->id()) { Blink::store('structure-uris')->forget($this->id()); Blink::store('structure-entries')->forget($this->id()); + Blink::forget('origin-'.class_basename($this).'-'.$this->id()); } $this->ancestors()->each(fn ($entry) => Blink::forget('entry-descendants-'.$entry->id())); diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index 1b87caea456..d3a50a1190b 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -2181,5 +2181,9 @@ public function it_updates_the_origin_of_descendants_when_saving_an_entry_with_l $this->assertEquals('root updated', $one->foo); $this->assertEquals('two updated', $two->foo); $this->assertEquals('two updated', $three->foo); + + // Todo: explicitly test that the origin blink key is forgotten for all descendants + // At the moment it coincidentally when the Stache CollectionEntriesStore re-makes + // the file and calls ->origin($origin). } } From a6122d9f839473099ba885ce29d5566e4571fedb Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 15:27:59 -0400 Subject: [PATCH 11/17] directDescendants - like descendants without recursion --- src/Entries/Entry.php | 9 +++++++-- tests/Data/Entries/EntryTest.php | 33 ++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index 72c481318ed..af995c81d73 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -664,14 +664,19 @@ public function ancestors() return $ancestors; } - public function descendants() + public function directDescendants() { - $localizations = Blink::once('entry-descendants-'.$this->id(), function () { + return Blink::once('entry-descendants-'.$this->id(), function () { return Facades\Entry::query() ->where('collection', $this->collectionHandle()) ->where('origin', $this->id())->get() ->keyBy->locale(); }); + } + + public function descendants() + { + $localizations = $this->directDescendants(); foreach ($localizations as $loc) { $localizations = $localizations->merge($loc->descendants()); diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index d3a50a1190b..2d7bf0daa60 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -2102,26 +2102,51 @@ public function it_gets_preview_targets() } /** @test */ - public function it_gets_descendants() + public function it_gets_all_descendants() { Facades\Site::setConfig(['default' => 'en', 'sites' => [ 'en' => ['locale' => 'en_US', 'url' => '/'], 'fr' => ['locale' => 'fr_FR', 'url' => '/fr/'], 'fr_CA' => ['locale' => 'fr_CA', 'url' => '/fr-ca/'], 'de' => ['locale' => 'de_DE', 'url' => '/de/'], + 'it' => ['local' => 'it_IT', 'url' => '/it/'], ]]); $one = EntryFactory::collection('test')->id('1')->locale('en')->create(); $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('fr_CA')->create(); $four = EntryFactory::collection('test')->id('4')->origin('2')->locale('de')->create(); + $five = EntryFactory::collection('test')->id('5')->origin('3')->locale('it')->create(); - $this->assertEquals(['fr' => $two, 'fr_CA' => $three, 'de' => $four], $one->descendants()->all()); - $this->assertEquals(['fr_CA' => $three, 'de' => $four], $two->descendants()->all()); - $this->assertEquals([], $three->descendants()->all()); + $this->assertEquals(['fr' => $two, 'fr_CA' => $three, 'de' => $four, 'it' => $five], $one->descendants()->all()); + $this->assertEquals(['fr_CA' => $three, 'de' => $four, 'it' => $five], $two->descendants()->all()); + $this->assertEquals(['it' => $five], $three->descendants()->all()); $this->assertEquals([], $four->descendants()->all()); } + /** @test */ + public function it_gets_direct_descendants() + { + Facades\Site::setConfig(['default' => 'en', 'sites' => [ + 'en' => ['locale' => 'en_US', 'url' => '/'], + 'fr' => ['locale' => 'fr_FR', 'url' => '/fr/'], + 'fr_CA' => ['locale' => 'fr_CA', 'url' => '/fr-ca/'], + 'de' => ['locale' => 'de_DE', 'url' => '/de/'], + 'it' => ['local' => 'it_IT', 'url' => '/it/'], + ]]); + + $one = EntryFactory::collection('test')->id('1')->locale('en')->create(); + $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); + $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('fr_CA')->create(); + $four = EntryFactory::collection('test')->id('4')->origin('2')->locale('de')->create(); + $five = EntryFactory::collection('test')->id('5')->origin('3')->locale('it')->create(); + + $this->assertEquals(['fr' => $two], $one->directDescendants()->all()); + $this->assertEquals(['fr_CA' => $three, 'de' => $four], $two->directDescendants()->all()); + $this->assertEquals(['it' => $five], $three->directDescendants()->all()); + $this->assertEquals([], $four->directDescendants()->all()); + } + /** @test */ public function it_gets_ancestors() { From bc4ce3632766b470cdf49e09298309f29657dd00 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 15:28:43 -0400 Subject: [PATCH 12/17] make sure the descendants only get saved one time. track the blinks to make sure they get wiped. --- src/Entries/Entry.php | 2 +- tests/Data/Entries/EntryTest.php | 34 ++++++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index af995c81d73..8b4f27eb502 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -331,7 +331,7 @@ public function save() $this->ancestors()->each(fn ($entry) => Blink::forget('entry-descendants-'.$entry->id())); - $this->descendants()->each->save(); + $this->directDescendants()->each->save(); $this->taxonomize(); diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index 2d7bf0daa60..2c36b6d8772 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -33,6 +33,7 @@ use Statamic\Structures\CollectionTree; use Statamic\Structures\Page; use Statamic\Support\Arr; +use Statamic\Support\Str; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -2191,6 +2192,29 @@ public function it_updates_the_origin_of_descendants_when_saving_an_entry_with_l $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('de')->create(); + // We want to check that the origin blink key was explicitly cleared, + // so we'll keep track of it happening from within the Entry@save method. + // It would also get cleared coincidentally within the Stache. + Blink::swap($fakeBlink = new class extends \Statamic\Support\Blink + { + public $calls = []; + + public function __call($method, $args) + { + // Ugly. Sorry. ¯\_(ツ)_/¯ + $trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3); + if ( + 'Statamic\Entries\Entry@save' === $trace[2]['class'].'@'.$trace[2]['function'] + && $method === 'forget' + && Str::startsWith($args[0], 'origin-Entry-') + ) { + $this->calls[$args[0]][] = true; + } + + return parent::__call($method, $args); + } + }); + $this->assertEquals('root', $one->foo); $this->assertEquals('root', $two->foo); $this->assertEquals('root', $three->foo); @@ -2200,15 +2224,17 @@ public function it_updates_the_origin_of_descendants_when_saving_an_entry_with_l $this->assertEquals('root updated', $one->foo); $this->assertEquals('root updated', $two->foo); $this->assertEquals('root updated', $three->foo); + $this->assertCount(1, $fakeBlink->calls['origin-Entry-1']); + $this->assertCount(1, $fakeBlink->calls['origin-Entry-2']); + $this->assertCount(1, $fakeBlink->calls['origin-Entry-3']); $two->data(['foo' => 'two updated'])->save(); $this->assertEquals('root updated', $one->foo); $this->assertEquals('two updated', $two->foo); $this->assertEquals('two updated', $three->foo); - - // Todo: explicitly test that the origin blink key is forgotten for all descendants - // At the moment it coincidentally when the Stache CollectionEntriesStore re-makes - // the file and calls ->origin($origin). + $this->assertCount(1, $fakeBlink->calls['origin-Entry-1']); + $this->assertCount(2, $fakeBlink->calls['origin-Entry-2']); + $this->assertCount(2, $fakeBlink->calls['origin-Entry-3']); } } From 54c57624a2f414e727b502ee41417073d45aa263 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 28 Jul 2023 16:44:31 -0400 Subject: [PATCH 13/17] use merge which is immutable. using push affected unrelated areas. --- src/StaticCaching/DefaultInvalidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/StaticCaching/DefaultInvalidator.php b/src/StaticCaching/DefaultInvalidator.php index edf3c53e468..00ffdce6511 100644 --- a/src/StaticCaching/DefaultInvalidator.php +++ b/src/StaticCaching/DefaultInvalidator.php @@ -61,7 +61,7 @@ protected function invalidateAssetUrls($asset) protected function invalidateEntryUrls($entry) { - $entry->descendants()->push($entry)->each(function ($entry) { + $entry->descendants()->merge([$entry])->each(function ($entry) { if (! $entry->isRedirect() && $url = $entry->absoluteUrl()) { $this->cacher->invalidateUrl(...$this->splitUrlAndDomain($url)); } From 8ee3a192fcb93d8e89f9eb1e09b728e3f8da4b26 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 31 Jul 2023 14:12:47 -0400 Subject: [PATCH 14/17] avoid using property cache in globals ... variables test now needs make sure that localizations get added onto the global itself. the way the test was written wasn't a realistic setup. you'd need to add the localizations in reality anyway. --- src/Data/HasOrigin.php | 22 +++++++++----- src/Entries/Entry.php | 2 +- src/Globals/GlobalSet.php | 10 +++---- src/Globals/Variables.php | 10 +++++++ tests/Data/Globals/GlobalSetTest.php | 43 ++++++++++++++++++++++++++++ tests/Data/Globals/VariablesTest.php | 30 ++++++++++++++++++- 6 files changed, 103 insertions(+), 14 deletions(-) diff --git a/src/Data/HasOrigin.php b/src/Data/HasOrigin.php index 3148987a0cd..f3b6373dbb8 100644 --- a/src/Data/HasOrigin.php +++ b/src/Data/HasOrigin.php @@ -58,24 +58,32 @@ public function value($key) public function origin($origin = null) { - $key = 'origin-'.class_basename($this).'-'.$this->id(); - return $this->fluentlyGetOrSet('origin') - ->getter(function ($origin) use ($key) { + ->getter(function ($origin) { return $origin - ? Blink::once($key, fn () => $this->getOriginByString($origin)) + ? Blink::once($this->getOriginBlinkKey(), fn () => $this->getOriginByString($origin)) : null; }) - ->setter(function ($origin) use ($key) { - Blink::forget($key); + ->setter(function ($origin) { + Blink::forget($this->getOriginBlinkKey()); - return is_string($origin) || is_null($origin) ? $origin : $origin->id(); + return is_string($origin) || is_null($origin) ? $origin : $this->getOriginIdFromObject($origin); }) ->args(func_get_args()); } abstract public function getOriginByString($origin); + protected function getOriginBlinkKey() + { + return 'origin-'.class_basename($this).'-'.$this->id(); + } + + public function getOriginIdFromObject($origin) + { + return $origin->id(); + } + public function hasOrigin() { return $this->origin() !== null; diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index 8b4f27eb502..e72fe47f5e7 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -326,7 +326,7 @@ public function save() if ($this->id()) { Blink::store('structure-uris')->forget($this->id()); Blink::store('structure-entries')->forget($this->id()); - Blink::forget('origin-'.class_basename($this).'-'.$this->id()); + Blink::forget($this->getOriginBlinkKey()); } $this->ancestors()->each(fn ($entry) => Blink::forget('entry-descendants-'.$entry->id())); diff --git a/src/Globals/GlobalSet.php b/src/Globals/GlobalSet.php index eb2f5ec6e35..7b69424639b 100644 --- a/src/Globals/GlobalSet.php +++ b/src/Globals/GlobalSet.php @@ -9,6 +9,7 @@ use Statamic\Events\GlobalSetSaved; use Statamic\Events\GlobalSetSaving; use Statamic\Facades; +use Statamic\Facades\Blink; use Statamic\Facades\Blueprint; use Statamic\Facades\Site; use Statamic\Facades\Stache; @@ -21,7 +22,6 @@ class GlobalSet implements Contract protected $title; protected $handle; - protected $localizations; protected $afterSaveCallbacks = []; protected $withEvents = true; @@ -141,21 +141,21 @@ public function addLocalization($localization) { $localization->globalSet($this); - $this->localizations[$localization->locale()] = $localization; + $this->localizations()[$localization->locale()] = $localization; return $this; } public function removeLocalization($localization) { - unset($this->localizations[$localization->locale()]); + unset($this->localizations()[$localization->locale()]); return $this; } public function in($locale) { - return $this->localizations[$locale] ?? null; + return $this->localizations()[$locale] ?? null; } public function inSelectedSite() @@ -180,7 +180,7 @@ public function existsIn($locale) public function localizations() { - return collect($this->localizations); + return Blink::once('global-set-localizations-'.$this->id(), fn () => collect()); } public function editUrl() diff --git a/src/Globals/Variables.php b/src/Globals/Variables.php index 7f9a4447410..edec7375ea6 100644 --- a/src/Globals/Variables.php +++ b/src/Globals/Variables.php @@ -172,6 +172,16 @@ protected function getOriginByString($origin) return $this->globalSet()->in($origin); } + public function getOriginIdFromObject($origin) + { + return $origin->locale(); + } + + protected function getOriginBlinkKey() + { + return 'origin-globals-'.$this->id().'-'.$this->locale(); + } + public function newAugmentedInstance(): Augmented { return new AugmentedVariables($this); diff --git a/tests/Data/Globals/GlobalSetTest.php b/tests/Data/Globals/GlobalSetTest.php index 0416ce1c7dd..c9953499bd6 100644 --- a/tests/Data/Globals/GlobalSetTest.php +++ b/tests/Data/Globals/GlobalSetTest.php @@ -2,6 +2,7 @@ namespace Tests\Data\Globals; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; use Statamic\Events\GlobalSetCreated; use Statamic\Events\GlobalSetSaved; @@ -215,4 +216,46 @@ public function if_saving_event_returns_false_the_global_set_doesnt_save() Event::assertNotDispatched(GlobalSetSaved::class); } + + /** @test */ + public function it_updates_the_origin_of_descendants_when_saving_an_entry_with_localizations() + { + // The issue this test is covering doesn't happen when using the + // array cache driver, since the objects are stored in memory. + config(['cache.default' => 'file']); + Cache::clear(); + + Site::setConfig([ + 'default' => 'en', + 'sites' => [ + 'en' => ['name' => 'English', 'locale' => 'en_US', 'url' => '/'], + 'fr' => ['name' => 'French', 'locale' => 'fr_FR', 'url' => '/fr/'], + 'de' => ['name' => 'German', 'locale' => 'de_DE', 'url' => '/de/'], + ], + ]); + + $global = tap(GlobalSet::make('test'), function ($global) { + $global->addLocalization($global->makeLocalization('en')->data(['foo' => 'root'])); + $global->addLocalization($global->makeLocalization('fr')->origin('en')); + $global->addLocalization($global->makeLocalization('de')->origin('fr')); + })->save(); + + $this->assertEquals('root', $global->in('en')->foo); + $this->assertEquals('root', $global->in('fr')->foo); + $this->assertEquals('root', $global->in('de')->foo); + + $global = GlobalSet::find('test'); + $global->in('en')->data(['foo' => 'root updated'])->save(); + + $this->assertEquals('root updated', $global->in('en')->foo); + $this->assertEquals('root updated', $global->in('fr')->foo); + $this->assertEquals('root updated', $global->in('de')->foo); + + $global = GlobalSet::find('test'); + $global->in('fr')->data(['foo' => 'fr updated'])->save(); + + $this->assertEquals('root updated', $global->in('en')->foo); + $this->assertEquals('fr updated', $global->in('fr')->foo); + $this->assertEquals('fr updated', $global->in('de')->foo); + } } diff --git a/tests/Data/Globals/VariablesTest.php b/tests/Data/Globals/VariablesTest.php index 259e49ba8ba..22a82d8511d 100644 --- a/tests/Data/Globals/VariablesTest.php +++ b/tests/Data/Globals/VariablesTest.php @@ -9,18 +9,36 @@ use Statamic\Facades; use Statamic\Facades\Blueprint; use Statamic\Facades\GlobalSet; +use Statamic\Facades\Site; use Statamic\Fields\Fieldtype; use Statamic\Fields\Value; use Statamic\Globals\Variables; use Statamic\Support\Arr; +use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; class VariablesTest extends TestCase { + use PreventSavingStacheItemsToDisk; + + public function setUp(): void + { + parent::setUp(); + + Site::setConfig(['sites' => [ + 'a' => ['url' => '/', 'locale' => 'en'], + 'b' => ['url' => '/b/', 'locale' => 'fr'], + 'c' => ['url' => '/b/', 'locale' => 'fr'], + 'd' => ['url' => '/d/', 'locale' => 'fr'], + ]]); + } + /** @test */ public function it_gets_file_contents_for_saving() { - $entry = (new Variables)->data([ + $global = GlobalSet::make('test'); + + $entry = $global->makeLocalization('a')->data([ 'array' => ['first one', 'second one'], 'string' => 'The string', 'null' => null, // this... @@ -63,6 +81,10 @@ public function it_gets_file_contents_for_saving_a_localized_set() 'empty' => [], // and this should get stripped out because there's no origin to fall back to. ]); + $global->addLocalization($a); + $global->addLocalization($b); + $global->addLocalization($c); + $expected = <<<'EOT' array: - 'first one' @@ -130,6 +152,12 @@ public function if_the_value_is_explicitly_set_to_null_then_it_should_not_fall_b 'two' => null, ]); + $global->addLocalization($a); + $global->addLocalization($b); + $global->addLocalization($c); + $global->addLocalization($d); + $global->addLocalization($e); + $this->assertEquals([ 'one' => 'alfa', 'two' => 'bravo', From 4e854beff4f7295f8eac46072e1f49a225dc021a Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 31 Jul 2023 14:27:31 -0400 Subject: [PATCH 15/17] fake the origin method in the pluck modifier so passing an object still works --- tests/Modifiers/PluckTest.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/Modifiers/PluckTest.php b/tests/Modifiers/PluckTest.php index 2a9f429d5fc..1b5ab248325 100644 --- a/tests/Modifiers/PluckTest.php +++ b/tests/Modifiers/PluckTest.php @@ -188,12 +188,18 @@ class ItemWithOrigin public function __construct($data, $origin = null) { $this->data($data); - $this->origin($origin); + $this->origin = $origin; + } + + public function origin($origin = null) + { + // Bypass the logic to load the origin. Just use what was passed in. + return $this->origin; } public function getOriginByString($origin) { - // + // Required by trait } } From 5f233f36e64ba5c02e29d9757cf9b19abd605925 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 31 Jul 2023 15:39:17 -0400 Subject: [PATCH 16/17] flip logic to allow integer based ids to work --- src/Data/HasOrigin.php | 2 +- tests/Data/Entries/EntryTest.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Data/HasOrigin.php b/src/Data/HasOrigin.php index f3b6373dbb8..6490069e165 100644 --- a/src/Data/HasOrigin.php +++ b/src/Data/HasOrigin.php @@ -67,7 +67,7 @@ public function origin($origin = null) ->setter(function ($origin) { Blink::forget($this->getOriginBlinkKey()); - return is_string($origin) || is_null($origin) ? $origin : $this->getOriginIdFromObject($origin); + return is_object($origin) ? $this->getOriginIdFromObject($origin) : $origin; }) ->args(func_get_args()); } diff --git a/tests/Data/Entries/EntryTest.php b/tests/Data/Entries/EntryTest.php index 2c36b6d8772..a1c92be9da8 100644 --- a/tests/Data/Entries/EntryTest.php +++ b/tests/Data/Entries/EntryTest.php @@ -2136,11 +2136,11 @@ public function it_gets_direct_descendants() 'it' => ['local' => 'it_IT', 'url' => '/it/'], ]]); - $one = EntryFactory::collection('test')->id('1')->locale('en')->create(); - $two = EntryFactory::collection('test')->id('2')->origin('1')->locale('fr')->create(); - $three = EntryFactory::collection('test')->id('3')->origin('2')->locale('fr_CA')->create(); - $four = EntryFactory::collection('test')->id('4')->origin('2')->locale('de')->create(); - $five = EntryFactory::collection('test')->id('5')->origin('3')->locale('it')->create(); + $one = EntryFactory::collection('test')->id(1)->locale('en')->create(); + $two = EntryFactory::collection('test')->id(2)->origin(1)->locale('fr')->create(); + $three = EntryFactory::collection('test')->id(3)->origin(2)->locale('fr_CA')->create(); + $four = EntryFactory::collection('test')->id(4)->origin(2)->locale('de')->create(); + $five = EntryFactory::collection('test')->id(5)->origin(3)->locale('it')->create(); $this->assertEquals(['fr' => $two], $one->directDescendants()->all()); $this->assertEquals(['fr_CA' => $three, 'de' => $four], $two->directDescendants()->all()); From 4b479986c340b1e8f8890966445bd24326c1184f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 31 Jul 2023 15:42:54 -0400 Subject: [PATCH 17/17] protected is fine --- src/Data/HasOrigin.php | 2 +- src/Globals/Variables.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Data/HasOrigin.php b/src/Data/HasOrigin.php index 6490069e165..186299cfad9 100644 --- a/src/Data/HasOrigin.php +++ b/src/Data/HasOrigin.php @@ -79,7 +79,7 @@ protected function getOriginBlinkKey() return 'origin-'.class_basename($this).'-'.$this->id(); } - public function getOriginIdFromObject($origin) + protected function getOriginIdFromObject($origin) { return $origin->id(); } diff --git a/src/Globals/Variables.php b/src/Globals/Variables.php index edec7375ea6..8d2eac8c959 100644 --- a/src/Globals/Variables.php +++ b/src/Globals/Variables.php @@ -172,7 +172,7 @@ protected function getOriginByString($origin) return $this->globalSet()->in($origin); } - public function getOriginIdFromObject($origin) + protected function getOriginIdFromObject($origin) { return $origin->locale(); }