Added TestCase for attachments API methods

app/Http/Controllers/Api/AttachmentApiController.php

@@ -25,8 +25,8 @@ class AttachmentApiController extends ApiController
         'update' => [
             'name' => 'min:1|max:255|string',
             'uploaded_to' => 'integer|exists:pages,id',
-            'file' => 'link|file',
+            'file' => 'file',
-            'link' => 'file|min:1|max:255|safe_url'
+            'link' => 'min:1|max:255|safe_url'
         ],
     ];
 
@@ -87,7 +87,9 @@ class AttachmentApiController extends ApiController
     public function read(string $id)
     {
         /** @var Attachment $attachment */
-        $attachment = Attachment::visible()->findOrFail($id);
+        $attachment = Attachment::visible()
+            ->with(['createdBy', 'updatedBy'])
+            ->findOrFail($id);
 
         $attachment->setAttribute('links', [
             'html'     => $attachment->htmlLink(),
@@ -129,7 +131,7 @@ class AttachmentApiController extends ApiController
 
         if ($request->hasFile('file')) {
             $uploadedFile = $request->file('file');
-            $attachment = $this->attachmentService->saveUpdatedUpload($uploadedFile, $page->id);
+            $attachment = $this->attachmentService->saveUpdatedUpload($uploadedFile, $attachment);
         }
 
         $this->attachmentService->updateFile($attachment, $requestData);

app/Uploads/Attachment.php

@@ -3,6 +3,7 @@
 namespace BookStack\Uploads;
 
 use BookStack\Auth\Permissions\PermissionService;
+use BookStack\Auth\User;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\Page;
 use BookStack\Model;
@@ -18,6 +19,8 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
  * @property ?Page $page
  * @property bool $external
  * @property int $uploaded_to
+ * @property User $updatedBy
+ * @property User $createdBy
  *
  * @method static Entity|Builder visible()
  */
@@ -26,6 +29,10 @@ class Attachment extends Model
     use HasCreatorAndUpdater;
 
     protected $fillable = ['name', 'order'];
+    protected $hidden = ['path'];
+    protected $casts = [
+        'external' => 'bool',
+    ];
 
     /**
      * Get the downloadable file name for this upload.
@@ -80,7 +87,7 @@ class Attachment extends Model
     /**
      * Scope the query to those attachments that are visible based upon related page permissions.
      */
-    public function scopeVisible(): string
+    public function scopeVisible(): Builder
     {
         $permissionService = app()->make(PermissionService::class);
         return $permissionService->filterRelatedEntity(

app/Uploads/AttachmentService.php

@@ -162,16 +162,16 @@ class AttachmentService
         $link = trim($requestData['link'] ?? '');
 
         if (!empty($link)) {
-            $attachment->path = $requestData['link'];
             if (!$attachment->external) {
                 $this->deleteFileInStorage($attachment);
                 $attachment->external = true;
+                $attachment->extension = '';
             }
+            $attachment->path = $requestData['link'];
         }
 
         $attachment->save();
-
+        return $attachment->refresh();
-        return $attachment;
     }
 
     /**

tests/Api/AttachmentsApiTest.php

@@ -0,0 +1,330 @@
+<?php
+
+namespace Tests\Api;
+
+use BookStack\Entities\Models\Page;
+use BookStack\Uploads\Attachment;
+use Illuminate\Http\UploadedFile;
+use Tests\TestCase;
+
+class AttachmentsApiTest extends TestCase
+{
+    use TestsApi;
+
+    protected $baseEndpoint = '/api/attachments';
+
+    public function test_index_endpoint_returns_expected_book()
+    {
+        $this->actingAsApiEditor();
+        $page = Page::query()->first();
+        $attachment = $this->createAttachmentForPage($page, [
+            'name' => 'My test attachment',
+            'external' => true,
+        ]);
+
+        $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+        $resp->assertJson(['data' => [
+            [
+                'id' => $attachment->id,
+                'name' => 'My test attachment',
+                'uploaded_to' => $page->id,
+                'external' => true,
+            ],
+        ]]);
+    }
+
+    public function test_attachments_listing_based_upon_page_visibility()
+    {
+        $this->actingAsApiEditor();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $attachment = $this->createAttachmentForPage($page, [
+            'name' => 'My test attachment',
+            'external' => true,
+        ]);
+
+        $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+        $resp->assertJson(['data' => [
+            [
+                'id' => $attachment->id,
+            ],
+        ]]);
+
+        $page->restricted = true;
+        $page->save();
+        $this->regenEntityPermissions($page);
+
+        $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+        $resp->assertJsonMissing(['data' => [
+            [
+                'id' => $attachment->id,
+            ],
+        ]]);
+    }
+
+    public function test_create_endpoint_for_link_attachment()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+
+        $details = [
+            'name' => 'My attachment',
+            'uploaded_to' => $page->id,
+            'link' => 'https://cats.example.com',
+        ];
+
+        $resp = $this->postJson($this->baseEndpoint, $details);
+        $resp->assertStatus(200);
+        /** @var Attachment $newItem */
+        $newItem = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+        $resp->assertJson(['id' => $newItem->id, 'external' => true, 'name' => $details['name'], 'uploaded_to' => $page->id]);
+    }
+
+    public function test_create_endpoint_for_upload_attachment()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $file = $this->getTestFile('textfile.txt');
+
+        $details = [
+            'name' => 'My attachment',
+            'uploaded_to' => $page->id,
+        ];
+
+        $resp = $this->call('POST', $this->baseEndpoint, $details, [], ['file' => $file]);
+        $resp->assertStatus(200);
+        /** @var Attachment $newItem */
+        $newItem = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+        $resp->assertJson(['id' => $newItem->id, 'external' => false, 'extension' => 'txt', 'name' => $details['name'], 'uploaded_to' => $page->id]);
+        $this->assertTrue(file_exists(storage_path($newItem->path)));
+        unlink(storage_path($newItem->path));
+    }
+
+    public function test_name_needed_to_create()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+
+        $details = [
+            'uploaded_to' => $page->id,
+            'link' => 'https://example.com',
+        ];
+
+        $resp = $this->postJson($this->baseEndpoint, $details);
+        $resp->assertStatus(422);
+        $resp->assertJson([
+            'error' => [
+                'message' => 'The given data was invalid.',
+                'validation' => [
+                    'name' => ['The name field is required.'],
+                ],
+                'code' => 422,
+            ],
+        ]);
+    }
+
+    public function test_link_or_file_needed_to_create()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+
+        $details = [
+            'name' => 'my attachment',
+            'uploaded_to' => $page->id,
+        ];
+
+        $resp = $this->postJson($this->baseEndpoint, $details);
+        $resp->assertStatus(422);
+        $resp->assertJson([
+            'error' => [
+                'message' => 'The given data was invalid.',
+                'validation' => [
+                    "file" => ["The file field is required when link is not present."],
+                    "link" => ["The link field is required when file is not present."],
+                ],
+                'code' => 422,
+            ],
+        ]);
+    }
+
+    public function test_read_endpoint_for_link_attachment()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+
+        $attachment = $this->createAttachmentForPage($page, [
+            'name' => 'my attachment',
+            'path' => 'https://example.com',
+            'order' => 1,
+        ]);
+
+        $resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}");
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id' => $attachment->id,
+            'content' => 'https://example.com',
+            'external' => true,
+            'uploaded_to' => $page->id,
+            'order' => 1,
+            'created_by' => [
+                'name' => $attachment->createdBy->name,
+            ],
+            'updated_by' => [
+                'name' => $attachment->createdBy->name,
+            ],
+            'links' => [
+                "html" => "<a target=\"_blank\" href=\"http://localhost/attachments/{$attachment->id}\">my attachment</a>",
+                "markdown" => "[my attachment](http://localhost/attachments/{$attachment->id})"
+            ],
+        ]);
+    }
+
+    public function test_read_endpoint_for_file_attachment()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $file = $this->getTestFile('textfile.txt');
+
+        $details = [
+            'name' => 'My file attachment',
+            'uploaded_to' => $page->id,
+        ];
+        $this->call('POST', $this->baseEndpoint, $details, [], ['file' => $file]);
+        /** @var Attachment $attachment */
+        $attachment = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->firstOrFail();
+
+        $resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}");
+
+        $resp->assertStatus(200);
+        $resp->assertJson([
+            'id' => $attachment->id,
+            'content' => base64_encode(file_get_contents(storage_path($attachment->path))),
+            'external' => false,
+            'uploaded_to' => $page->id,
+            'order' => 1,
+            'created_by' => [
+                'name' => $attachment->createdBy->name,
+            ],
+            'updated_by' => [
+                'name' => $attachment->updatedBy->name,
+            ],
+            'links' => [
+                "html" => "<a target=\"_blank\" href=\"http://localhost/attachments/{$attachment->id}\">My file attachment</a>",
+                "markdown" => "[My file attachment](http://localhost/attachments/{$attachment->id})"
+            ],
+        ]);
+
+        unlink(storage_path($attachment->path));
+    }
+
+    public function test_update_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $attachment = $this->createAttachmentForPage($page);
+
+        $details = [
+            'name' => 'My updated API attachment',
+        ];
+
+        $resp = $this->putJson("{$this->baseEndpoint}/{$attachment->id}", $details);
+        $attachment->refresh();
+
+        $resp->assertStatus(200);
+        $resp->assertJson(['id' => $attachment->id, 'name' => 'My updated API attachment']);
+    }
+
+    public function test_update_link_attachment_to_file()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $attachment = $this->createAttachmentForPage($page);
+        $file = $this->getTestFile('textfile.txt');
+
+
+        $resp = $this->call('PUT', "{$this->baseEndpoint}/{$attachment->id}", ['name' => 'My updated file'], [], ['file' => $file]);
+        $resp->assertStatus(200);
+
+        $attachment->refresh();
+        $this->assertFalse($attachment->external);
+        $this->assertEquals('txt', $attachment->extension);
+        $this->assertStringStartsWith('uploads/files/', $attachment->path);
+        $this->assertFileExists(storage_path($attachment->path));
+
+        unlink(storage_path($attachment->path));
+    }
+
+    public function test_update_file_attachment_to_link()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $file = $this->getTestFile('textfile.txt');
+        $this->call('POST', $this->baseEndpoint, ['name' => 'My file attachment', 'uploaded_to' => $page->id], [], ['file' => $file]);
+        /** @var Attachment $attachment */
+        $attachment = Attachment::query()->where('name', '=', 'My file attachment')->firstOrFail();
+
+        $filePath = storage_path($attachment->path);
+        $this->assertFileExists($filePath);
+
+        $details = [
+            'name' => 'My updated API attachment',
+            'link' => 'https://cats.example.com'
+        ];
+
+        $resp = $this->putJson("{$this->baseEndpoint}/{$attachment->id}", $details);
+        $resp->assertStatus(200);
+        $attachment->refresh();
+
+        $this->assertFileDoesNotExist($filePath);
+        $this->assertTrue($attachment->external);
+        $this->assertEquals('https://cats.example.com', $attachment->path);
+        $this->assertEquals('', $attachment->extension);
+    }
+
+    public function test_delete_endpoint()
+    {
+        $this->actingAsApiAdmin();
+        /** @var Page $page */
+        $page = Page::query()->first();
+        $attachment = $this->createAttachmentForPage($page);
+
+        $resp = $this->deleteJson("{$this->baseEndpoint}/{$attachment->id}");
+
+        $resp->assertStatus(204);
+        $this->assertDatabaseMissing('attachments', ['id' => $attachment->id]);
+    }
+
+    protected function createAttachmentForPage(Page $page, $attributes = []): Attachment
+    {
+        $admin = $this->getAdmin();
+        /** @var Attachment $attachment */
+        $attachment = $page->attachments()->forceCreate(array_merge([
+            'uploaded_to' => $page->id,
+            'name' => 'test attachment',
+            'external' => true,
+            'order' => 1,
+            'created_by' => $admin->id,
+            'updated_by' => $admin->id,
+            'path' => 'https://attachment.example.com'
+        ], $attributes));
+        return $attachment;
+    }
+
+    /**
+     * Get a test file that can be uploaded.
+     */
+    protected function getTestFile(string $fileName): UploadedFile
+    {
+        return new UploadedFile(base_path('tests/test-data/test-file.txt'), $fileName, 'text/plain', 55, null, true);
+    }
+}

tests/Api/TestsApi.php

@@ -17,6 +17,16 @@ trait TestsApi
         return $this;
     }
 
+    /**
+     * Set the API admin role as the current user via the API driver.
+     */
+    protected function actingAsApiAdmin()
+    {
+        $this->actingAs($this->getAdmin(), 'api');
+
+        return $this;
+    }
+
     /**
      * Format the given items into a standardised error format.
      */

tests/Uploads/AttachmentTest.php

@@ -76,9 +76,9 @@ class AttachmentTest extends TestCase
         $upload->assertStatus(200);
 
         $attachment = Attachment::query()->orderBy('id', 'desc')->first();
-        $expectedResp['path'] = $attachment->path;
-
         $upload->assertJson($expectedResp);
+
+        $expectedResp['path'] = $attachment->path;
         $this->assertDatabaseHas('attachments', $expectedResp);
 
         $this->deleteUploads();