[6.x] Build sibling URIs once when validating a collection tree

src/Http/Controllers/CP/Collections/CollectionTreeController.php

@@ -76,21 +76,22 @@ private function validateUniqueUris($tree)
             return;
         }
 
+        $siblingUris = [];
+
         foreach ($tree->diff()->moved() as $id) {
             $page = $tree->find($id);
             $parent = $page->parent();
+            $isTopLevel = ! $parent || $parent->isRoot();
+            $key = $isTopLevel ? '' : $parent->id();
 
-            $siblings = (! $parent || $parent->isRoot())
-                ? $tree->pages()->all()->slice(1)
-                : $page->parent()->pages()->all();
-
-            $siblings = $siblings->reject(function ($sibling) use ($id) {
-                return $sibling->reference() === $id;
-            });
+            // Moved pages under the same parent share siblings, so build that list once.
+            $siblingUris[$key] ??= ($isTopLevel ? $tree->pages()->all()->slice(1) : $parent->pages()->all())
+                ->mapWithKeys(fn ($sibling) => [$sibling->reference() => $sibling->uri()]);
 
-            $uris = $siblings->map->uri();
+            // The root is sliced off the top-level siblings, so it won't be in the list.
+            $uri = $siblingUris[$key][$id] ?? $page->uri();
 
-            if ($uris->contains($uri = $page->uri())) {
+            if ($siblingUris[$key]->except($id)->contains($uri)) {
                 throw ValidationException::withMessages(['uri' => trans('statamic::validation.duplicate_uri', ['value' => $uri])]);
             }
         }

src/Structures/Page.php

@@ -49,6 +49,7 @@ class Page implements Arrayable, ArrayAccess, Augmentable, BulkAugmentable, Cont
     private $routeData;
     private $status;
     private $structure;
+    private $uri;
 
     public function __construct()
     {
@@ -120,6 +121,7 @@ public function isRedirect()
     public function setDepth($depth)
     {
         $this->depth = $depth;
+        $this->uri = null;
 
         return $this;
     }
@@ -170,6 +172,7 @@ public function setEntry($reference): self
 
         $this->reference = $reference;
         $this->routeData = null;
+        $this->uri = null;
 
         return $this;
     }
@@ -205,13 +208,15 @@ public function parent(): ?self
     public function setParent(?self $parent): self
     {
         $this->parent = $parent;
+        $this->uri = null;
 
         return $this;
     }
 
     public function setRoute(?string $route): self
     {
         $this->route = $route;
+        $this->uri = null;
 
         return $this;
     }
@@ -247,14 +252,20 @@ public function uri()
             return $cached;
         }
 
+        // With the shared cache off, a page still keeps its own URI,
+        // so siblings don't each rebuild their parent's.
+        if (! $this->tree->uriCacheEnabled() && ! is_null($this->uri)) {
+            return $this->uri;
+        }
+
         if (! $this->structure() instanceof CollectionStructure) {
-            return $uris[$this->reference] = $this->entry()->uri();
+            return $this->uri = $uris[$this->reference] = $this->entry()->uri();
         }
 
         $parentUri = $this->parent && ! $this->parent->isRoot() ? $this->parent->uri() : '';
         $parentUri = Str::replace(['.html', '.htm'], '', $parentUri);
 
-        return $uris[$this->reference] = app(UrlBuilder::class)
+        return $this->uri = $uris[$this->reference] = app(UrlBuilder::class)
             ->content($this)
             ->merge([
                 'parent_uri' => $parentUri,
@@ -299,13 +310,15 @@ public function isRoot()
     public function setTree($tree)
     {
         $this->tree = $tree;
+        $this->uri = null;
 
         return $this;
     }
 
     public function setRoot(bool $isRoot)
     {
         $this->isRoot = $isRoot;
+        $this->uri = null;
 
         return $this;
     }

tests/Data/Structures/PageTest.php

@@ -188,6 +188,44 @@ public function slug($slug = null)
         $this->assertFalse($page->hasCustomUrl());
     }
 
+    #[Test]
+    public function it_rebuilds_the_uri_when_its_route_changes_while_the_uri_cache_is_disabled()
+    {
+        $entry = new class extends Entry
+        {
+            public function id($id = null)
+            {
+                return 'a';
+            }
+
+            public function slug($slug = null)
+            {
+                return 'entry-slug';
+            }
+        };
+        $collection = tap(\Statamic\Facades\Collection::make('test'))->save();
+        $entry->collection('test');
+
+        $parent = Mockery::mock(Page::class);
+        $parent->shouldReceive('uri')->andReturn('/first/parent');
+        $parent->shouldReceive('isRoot')->andReturnFalse();
+
+        $tree = $this->newTree()->disableUriCache()->setStructure(
+            $this->mock(CollectionStructure::class)->shouldReceive('collection')->andReturn($collection)->getMock()
+        );
+
+        $page = (new Page)
+            ->setTree($tree)
+            ->setRoute('{parent_uri}/{slug}')
+            ->setParent($parent)
+            ->setEntry($entry);
+
+        $this->assertEquals('/first/parent/entry-slug', $page->uri());
+
+        $page->setRoute('/foo/{parent_uri}/{slug}');
+        $this->assertEquals('/foo/first/parent/entry-slug', $page->uri());
+    }
+
     #[Test]
     #[DataProvider('stripExtensionFromParentUriProvider')]
     public function it_builds_a_uri_and_strips_out_file_extensions_from_parent_uri($ext)

tests/Feature/Collections/UpdateCollectionTreeTest.php

@@ -5,9 +5,12 @@
 use Facades\Tests\Factories\EntryFactory;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
+use Statamic\Contracts\Routing\UrlBuilder as UrlBuilderContract;
 use Statamic\Facades\Collection;
 use Statamic\Facades\Entry;
 use Statamic\Facades\User;
+use Statamic\Routing\UrlBuilder;
+use Statamic\Structures\Page;
 use Tests\FakesRoles;
 use Tests\PreventSavingStacheItemsToDisk;
 use Tests\TestCase;
@@ -126,6 +129,56 @@ private function duplicateUriTest($expectsRoot)
         ], $collection->structure()->in('en')->tree());
     }
 
+    #[Test]
+    public function it_builds_each_siblings_uri_once_when_validating_moved_pages()
+    {
+        $user = tap(User::make()->makeSuper())->save();
+        $collection = tap(Collection::make('test')->routes('{parent_uri}/{slug}'))->save();
+        EntryFactory::id('e1')->collection($collection)->slug('alfa')->create();
+        EntryFactory::id('e2')->collection($collection)->slug('bravo')->create();
+        EntryFactory::id('e3')->collection($collection)->slug('charlie')->create();
+        $collection->structureContents(['root' => false])->save();
+        $collection->structure()->in('en')->tree([
+            ['entry' => 'e1', 'children' => [
+                ['entry' => 'e2'],
+                ['entry' => 'e3'],
+            ]],
+        ])->save();
+
+        // Saving leaves the tree's pages cached in memory, where a request would load it fresh.
+        $collection->structure()->in('en')->flushCache();
+
+        $built = collect();
+
+        $this->app->bind(UrlBuilderContract::class, fn () => new class($built) extends UrlBuilder
+        {
+            public function __construct(private $built)
+            {
+            }
+
+            public function build($route)
+            {
+                if ($this->content instanceof Page) {
+                    $this->built->push($this->content->reference());
+                }
+
+                return parent::build($route);
+            }
+        });
+
+        $this
+            ->actingAs($user)
+            ->update($collection, ['pages' => [
+                ['id' => 'e1', 'children' => [
+                    ['id' => 'e3', 'children' => []],
+                    ['id' => 'e2', 'children' => []],
+                ]],
+            ]])
+            ->assertOk();
+
+        $this->assertEquals(['e1' => 1, 'e2' => 1, 'e3' => 1], $built->countBy()->sortKeys()->all());
+    }
+
     #[Test]
     public function it_deletes_entries_scheduled_for_deletion()
     {