Search API: Updated handling of parent detail, added testing

Review of #5280.

- Removed additional non-needed loads which could ignore permissions.
- Updated new formatter method name to be more specific on use.
- Added test case to cover changes.
- Updated API examples to align parent id/info in info to be
  representative.

diff --git a/app/Api/ApiEntityListFormatter.php b/app/Api/ApiEntityListFormatter.php
index 7c2d09d4f3aa3dfe46ae02ba1614d1e69f1d4e16..3c94d96ee600789ce212296b1ba31bb49059f8cb 100644 (file)
--- a/app/Api/ApiEntityListFormatter.php
+++ b/app/Api/ApiEntityListFormatter.php
@@ -2,6 +2,7 @@
 
 namespace BookStack\Api;
 
+use BookStack\Entities\Models\BookChild;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\Page;
 
@@ -72,20 +73,20 @@ class ApiEntityListFormatter
     }
 
     /**
-     * Enable the inclusion of related book and chapter titles in the response.
+     * Include parent book/chapter info in the formatted data.
      */
-    public function withRelatedData(): self
+    public function withParents(): self
     {
         $this->withField('book', function (Entity $entity) {
-            if (method_exists($entity, 'book')) {
-                return $entity->book()->select(['id', 'name', 'slug'])->first();
+            if ($entity instanceof BookChild && $entity->book) {
+                return $entity->book->only(['id', 'name', 'slug']);
             }
             return null;
         });
 
         $this->withField('chapter', function (Entity $entity) {
-            if ($entity instanceof Page && $entity->chapter_id) {
-                return $entity->chapter()->select(['id', 'name', 'slug'])->first();
+            if ($entity instanceof Page && $entity->chapter) {
+                return $entity->chapter->only(['id', 'name', 'slug']);
             }
             return null;
         });
@@ -99,8 +100,6 @@ class ApiEntityListFormatter
      */
     public function format(): array
     {
-        $this->loadRelatedData();
-
         $results = [];
 
         foreach ($this->list as $item) {
@@ -110,23 +109,6 @@ class ApiEntityListFormatter
         return $results;
     }
 
-    /**
-     * Eager load the related book and chapter data when needed.
-     */
-    protected function loadRelatedData(): void
-    {
-        $pages = collect($this->list)->filter(fn($item) => $item instanceof Page);
-
-        foreach ($this->list as $entity) {
-            if (method_exists($entity, 'book')) {
-                $entity->load('book');
-            }
-            if ($entity instanceof Page && $entity->chapter_id) {
-                $entity->load('chapter');
-            }
-        }
-    }
-
     /**
      * Format a single entity item to a plain array.
      */

diff --git a/app/Search/SearchApiController.php b/app/Search/SearchApiController.php
index 28a3b53e6c3d512a211b00624b8ab9cf82a1d157..79cd8cfabd0e0166d082eb4c8eb8cc5e669f70d5 100644 (file)
--- a/app/Search/SearchApiController.php
+++ b/app/Search/SearchApiController.php
@@ -9,21 +9,18 @@ use Illuminate\Http\Request;
 
 class SearchApiController extends ApiController
 {
-    protected SearchRunner $searchRunner;
-    protected SearchResultsFormatter $resultsFormatter;
-
     protected $rules = [
         'all' => [
             'query' => ['required'],
-            'page' => ['integer', 'min:1'],
+            'page'  => ['integer', 'min:1'],
             'count' => ['integer', 'min:1', 'max:100'],
         ],
     ];
 
-    public function __construct(SearchRunner $searchRunner, SearchResultsFormatter $resultsFormatter)
-    {
-        $this->searchRunner = $searchRunner;
-        $this->resultsFormatter = $resultsFormatter;
+    public function __construct(
+        protected SearchRunner $searchRunner,
+        protected SearchResultsFormatter $resultsFormatter
+    ) {
     }
 
     /**
@@ -50,7 +47,7 @@ class SearchApiController extends ApiController
         $this->resultsFormatter->format($results['results']->all(), $options);
 
         $data = (new ApiEntityListFormatter($results['results']->all()))
-            ->withType()->withTags()->withRelatedData()
+            ->withType()->withTags()->withParents()
             ->withField('preview_html', function (Entity $entity) {
                 return [
                     'name' => (string) $entity->getAttribute('preview_name'),

diff --git a/dev/api/responses/search-all.json b/dev/api/responses/search-all.json
index f60a12f750d68d39e484bbc60512153e5e160f37..2ad89641693a1b71e4ba61ed2ba8aafb16b474ae 100644 (file)
--- a/dev/api/responses/search-all.json
+++ b/dev/api/responses/search-all.json
@@ -1,92 +1,92 @@
 {
-    "data": [
+  "data": [
+    {
+      "id": 84,
+      "book_id": 1,
+      "slug": "a-chapter-for-cats",
+      "name": "A chapter for cats",
+      "created_at": "2021-11-14T15:57:35.000000Z",
+      "updated_at": "2021-11-14T15:57:35.000000Z",
+      "type": "chapter",
+      "url": "https://example.com/books/cats/chapter/a-chapter-for-cats",
+      "book": {
+        "id": 1,
+        "name": "Cats",
+        "slug": "cats"
+      },
+      "preview_html": {
+        "name": "A chapter for <strong>cats</strong>",
+        "content": "...once a bunch of <strong>cats</strong> named tony...behaviour of <strong>cats</strong> is unsuitable"
+      },
+      "tags": []
+    },
+    {
+      "name": "The hows and whys of cats",
+      "id": 396,
+      "slug": "the-hows-and-whys-of-cats",
+      "book_id": 1,
+      "chapter_id": 75,
+      "draft": false,
+      "template": false,
+      "created_at": "2021-05-15T16:28:10.000000Z",
+      "updated_at": "2021-11-14T15:56:49.000000Z",
+      "type": "page",
+      "url": "https://example.com/books/cats/page/the-hows-and-whys-of-cats",
+      "book": {
+        "id": 1,
+        "name": "Cats",
+        "slug": "cats"
+      },
+      "chapter": {
+        "id": 75,
+        "name": "A chapter for cats",
+        "slug": "a-chapter-for-cats"
+      },
+      "preview_html": {
+        "name": "The hows and whys of <strong>cats</strong>",
+        "content": "...people ask why <strong>cats</strong>? but there are...the reason that <strong>cats</strong> are fast are due to..."
+      },
+      "tags": [
         {
-            "id": 84,
-            "book_id": 1,
-            "slug": "a-chapter-for-cats",
-            "name": "A chapter for cats",
-            "created_at": "2021-11-14T15:57:35.000000Z",
-            "updated_at": "2021-11-14T15:57:35.000000Z",
-            "type": "chapter",
-            "url": "https://example.com/books/my-book/chapter/a-chapter-for-cats",
-            "book": {
-                "id": 1,
-                "name": "Cats",
-                "slug": "cats"
-            },
-            "preview_html": {
-                "name": "A chapter for <strong>cats</strong>",
-                "content": "...once a bunch of <strong>cats</strong> named tony...behaviour of <strong>cats</strong> is unsuitable"
-            },
-            "tags": []
+          "name": "Animal",
+          "value": "Cat",
+          "order": 0
         },
         {
-            "name": "The hows and whys of cats",
-            "id": 396,
-            "slug": "the-hows-and-whys-of-cats",
-            "book_id": 1,
-            "chapter_id": 75,
-            "draft": false,
-            "template": false,
-            "created_at": "2021-05-15T16:28:10.000000Z",
-            "updated_at": "2021-11-14T15:56:49.000000Z",
-            "type": "page",
-            "url": "https://example.com/books/my-book/page/the-hows-and-whys-of-cats",
-            "book": {
-                "id": 1,
-                "name": "Cats",
-                "slug": "cats"
-            },
-            "chapter": {
-                "id": 84,
-                "name": "A chapter for cats",
-                "slug": "a-chapter-for-cats"
-            },
-            "preview_html": {
-                "name": "The hows and whys of <strong>cats</strong>",
-                "content": "...people ask why <strong>cats</strong>? but there are...the reason that <strong>cats</strong> are fast are due to..."
-            },
-            "tags": [
-                {
-                    "name": "Animal",
-                    "value": "Cat",
-                    "order": 0
-                },
-                {
-                    "name": "Category",
-                    "value": "Top Content",
-                    "order": 0
-                }
-            ]
-        },
-        {
-            "name": "How advanced are cats?",
-            "id": 362,
-            "slug": "how-advanced-are-cats",
-            "book_id": 13,
-            "chapter_id": 73,
-            "draft": false,
-            "template": false,
-            "created_at": "2020-11-29T21:55:07.000000Z",
-            "updated_at": "2021-11-14T16:02:39.000000Z",
-            "type": "page",
-            "url": "https://example.com/books/my-book/page/how-advanced-are-cats",
-            "book": {
-                "id": 1,
-                "name": "Cats",
-                "slug": "cats"
-            },
-            "chapter": {
-                "id": 84,
-                "name": "A chapter for cats",
-                "slug": "a-chapter-for-cats"
-            },
-            "preview_html": {
-                "name": "How advanced are <strong>cats</strong>?",
-                "content": "<strong>cats</strong> are some of the most advanced animals in the world."
-            },
-            "tags": []
+          "name": "Category",
+          "value": "Top Content",
+          "order": 0
         }
-    ],
-    "total": 3
+      ]
+    },
+    {
+      "name": "How advanced are cats?",
+      "id": 362,
+      "slug": "how-advanced-are-cats",
+      "book_id": 13,
+      "chapter_id": 73,
+      "draft": false,
+      "template": false,
+      "created_at": "2020-11-29T21:55:07.000000Z",
+      "updated_at": "2021-11-14T16:02:39.000000Z",
+      "type": "page",
+      "url": "https://example.com/books/big-cats/page/how-advanced-are-cats",
+      "book": {
+        "id": 13,
+        "name": "Big Cats",
+        "slug": "big-cats"
+      },
+      "chapter": {
+        "id": 73,
+        "name": "A chapter for bigger cats",
+        "slug": "a-chapter-for-bigger-cats"
+      },
+      "preview_html": {
+        "name": "How advanced are <strong>cats</strong>?",
+        "content": "<strong>cats</strong> are some of the most advanced animals in the world."
+      },
+      "tags": []
+    }
+  ],
+  "total": 3
 }

diff --git a/tests/Api/SearchApiTest.php b/tests/Api/SearchApiTest.php
index 3f2eb395c1101a2dd30b24cbddd6ede241eec612..9da7900ca9a433b776952f79a2d4c9e22c80bccc 100644 (file)
--- a/tests/Api/SearchApiTest.php
+++ b/tests/Api/SearchApiTest.php
@@ -13,7 +13,7 @@ class SearchApiTest extends TestCase
 {
     use TestsApi;
 
-    protected $baseEndpoint = '/api/search';
+    protected string $baseEndpoint = '/api/search';
 
     public function test_all_endpoint_returns_search_filtered_results_with_query()
     {
@@ -74,4 +74,46 @@ class SearchApiTest extends TestCase
         $resp = $this->actingAsApiEditor()->get($this->baseEndpoint . '?query=myqueryvalue');
         $resp->assertOk();
     }
+
+    public function test_all_endpoint_includes_parent_details_where_visible()
+    {
+        $page = $this->entities->pageWithinChapter();
+        $chapter = $page->chapter;
+        $book = $page->book;
+
+        $page->update(['name' => 'name with superextrauniquevalue within']);
+        $page->indexForSearch();
+
+        $editor = $this->users->editor();
+        $this->actingAsApiEditor();
+        $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertJsonFragment([
+            'id' => $page->id,
+            'type' => 'page',
+            'book' => [
+                'id' => $book->id,
+                'name' => $book->name,
+                'slug' => $book->slug,
+            ],
+            'chapter' => [
+                'id' => $chapter->id,
+                'name' => $chapter->name,
+                'slug' => $chapter->slug,
+            ],
+        ]);
+
+        $this->permissions->disableEntityInheritedPermissions($chapter);
+        $this->permissions->setEntityPermissions($page, ['view'], [$editor->roles()->first()]);
+
+        $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertJsonPath('data.0.id', $page->id);
+        $resp->assertJsonPath('data.0.book.name', $book->name);
+        $resp->assertJsonMissingPath('data.0.chapter');
+
+        $this->permissions->disableEntityInheritedPermissions($book);
+
+        $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertJsonPath('data.0.id', $page->id);
+        $resp->assertJsonMissingPath('data.0.book.name');
+    }
 }