From 2409d1850feeae10b5f0ef6a3f67bc9739881f44 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 20 Oct 2021 00:58:56 +0100 Subject: [PATCH] Added TestCase for attachments API methods --- .../Api/AttachmentApiController.php | 10 +- app/Uploads/Attachment.php | 9 +- app/Uploads/AttachmentService.php | 6 +- tests/Api/AttachmentsApiTest.php | 330 ++++++++++++++++++ tests/Api/TestsApi.php | 10 + tests/Uploads/AttachmentTest.php | 4 +- 6 files changed, 359 insertions(+), 10 deletions(-) create mode 100644 tests/Api/AttachmentsApiTest.php diff --git a/app/Http/Controllers/Api/AttachmentApiController.php b/app/Http/Controllers/Api/AttachmentApiController.php index 2ee1c98a6..7aa4ee493 100644 --- a/app/Http/Controllers/Api/AttachmentApiController.php +++ b/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', - 'link' => 'file|min:1|max:255|safe_url' + 'file' => 'file', + '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); diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php index dfd7d980a..8ae53199e 100644 --- a/app/Uploads/Attachment.php +++ b/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( diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php index d530d8fbe..2ad1663ff 100644 --- a/app/Uploads/AttachmentService.php +++ b/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; + return $attachment->refresh(); } /** diff --git a/tests/Api/AttachmentsApiTest.php b/tests/Api/AttachmentsApiTest.php new file mode 100644 index 000000000..88b5b9ddd --- /dev/null +++ b/tests/Api/AttachmentsApiTest.php @@ -0,0 +1,330 @@ +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" => "id}\">my attachment", + "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" => "id}\">My file attachment", + "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); + } +} diff --git a/tests/Api/TestsApi.php b/tests/Api/TestsApi.php index 683ca0c74..97ca82ea7 100644 --- a/tests/Api/TestsApi.php +++ b/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. */ diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php index 2248bc2c5..60fd370b6 100644 --- a/tests/Uploads/AttachmentTest.php +++ b/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();