Refactor attachment loading

appinfo/routes.php

@@ -29,6 +29,8 @@
 
 return [
 	'routes' => [
+		/** @see Controller\AttachmentController::getAttachmentList() */
+		['name' => 'Attachment#getAttachmentList', 'url' => '/attachments', 'verb' => 'POST'],
 		/** @see Controller\AttachmentController::insertAttachmentFile() */
 		['name' => 'Attachment#insertAttachmentFile', 'url' => '/attachment/filepath', 'verb' => 'POST'],
 		/** @see Controller\AttachmentController::uploadAttachment() */
@@ -39,8 +41,6 @@
 		['name' => 'Attachment#getMediaFile', 'url' => '/media', 'verb' => 'GET'],
 		/** @see Controller\AttachmentController::getMediaFilePreview() */
 		['name' => 'Attachment#getMediaFilePreview', 'url' => '/mediaPreview', 'verb' => 'GET'],
-		/** @see Controller\AttachmentController::getMediaFileMetadata() */
-		['name' => 'Attachment#getMediaFileMetadata', 'url' => '/mediaMetadata', 'verb' => 'GET'],
 
 		/** @see Controller\SessionController::create() */
 		['name' => 'Session#create', 'url' => '/session/{documentId}/create', 'verb' => 'PUT'],

composer/composer/autoload_classmap.php

@@ -45,6 +45,7 @@
     'OCA\\Text\\Listeners\\NodeCopiedListener' => $baseDir . '/../lib/Listeners/NodeCopiedListener.php',
     'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => $baseDir . '/../lib/Listeners/RegisterDirectEditorEventListener.php',
     'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSession.php',
+    'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php',
     'OCA\\Text\\Middleware\\SessionMiddleware' => $baseDir . '/../lib/Middleware/SessionMiddleware.php',
     'OCA\\Text\\Migration\\ResetSessionsBeforeYjs' => $baseDir . '/../lib/Migration/ResetSessionsBeforeYjs.php',
     'OCA\\Text\\Migration\\Version010000Date20190617184535' => $baseDir . '/../lib/Migration/Version010000Date20190617184535.php',

composer/composer/autoload_static.php

@@ -60,6 +60,7 @@ class ComposerStaticInitText
         'OCA\\Text\\Listeners\\NodeCopiedListener' => __DIR__ . '/..' . '/../lib/Listeners/NodeCopiedListener.php',
         'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => __DIR__ . '/..' . '/../lib/Listeners/RegisterDirectEditorEventListener.php',
         'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSession.php',
+        'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php',
         'OCA\\Text\\Middleware\\SessionMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/SessionMiddleware.php',
         'OCA\\Text\\Migration\\ResetSessionsBeforeYjs' => __DIR__ . '/..' . '/../lib/Migration/ResetSessionsBeforeYjs.php',
         'OCA\\Text\\Migration\\Version010000Date20190617184535' => __DIR__ . '/..' . '/../lib/Migration/Version010000Date20190617184535.php',

cypress/e2e/attachments.spec.js

@@ -111,7 +111,7 @@ const checkAttachment = (documentId, fileName, fileId, index, isImage = true) =>
 				.find('img')
 				.should('have.attr', 'src')
 				.should('contain', 'apps/text/' + srcPathEnd + '?documentId=' + documentId)
-				.should('contain', srcFileNameParam + '=' + encodeURIComponent(fileName))
+				.should('contain', srcFileNameParam + '=' + fixedEncodeURIComponent(fileName))
 
 			return isImage
 				? cy.wrap($el)

cypress/e2e/nodes/ImageView.spec.js

@@ -79,23 +79,6 @@ describe('Image View', () => {
 				.should('have.attr', 'src')
 				.should('contain', `/dav/files/${user.userId}/github.png`)
 		})
-
-		it('with preview', () => {
-			cy.getFile('github.png')
-				.should('have.attr', 'data-cy-files-list-row-fileid')
-				.then(imageId => {
-					const fileName = `${Cypress.currentTest.title}.md`
-
-					createMarkdown(fileName, `# from image id\n\n ![${imageId}](github.png?fileId=${imageId}&hasPreview=true)`)
-
-					cy.openFile(fileName, { force: true })
-
-					cy.getContent()
-						.find('[data-component="image-view"] img')
-						.should('have.attr', 'src')
-						.should('contains', `core/preview?fileId=${imageId}&file=${encodeURIComponent('/github.png')}`, { timeout: 5000 })
-				})
-		})
 	})
 
 	describe('fail to load', () => {
@@ -123,4 +106,46 @@ describe('Image View', () => {
 				.should('have.value', 'yaha')
 		})
 	})
+
+	describe('native attachments', () => {
+		before(() => {
+			cy.login(user)
+			cy.visit('/apps/files')
+			const fileName = 'native attachments.md'
+			createMarkdown(fileName, '# open image in modal\n\n ![git](.attachments.123/github.png)\n\n ![file.txt.gz](.attachments.123/file.txt.gz)')
+
+			cy.getFileId(fileName).then((fileId) => {
+				const attachmentsFolder = `.attachments.${fileId}`
+				cy.createFolder(attachmentsFolder)
+				cy.uploadFile('github.png', 'image/png', `${attachmentsFolder}/github.png`)
+				cy.uploadFile('file.txt.gz', 'application/gzip', `${attachmentsFolder}/file.txt.gz`)
+			})
+		})
+
+		it('open image in modal', () => {
+			const fileName = 'native attachments.md'
+			cy.openFile(fileName)
+
+			cy.getContent()
+				.find('[data-component="image-view"][data-src=".attachments.123/github.png"] img')
+				.click()
+
+			cy.get('.modal__content img')
+				.should('have.attr', 'src')
+				.should('contain', 'imageFileName=github.png')
+		})
+
+		it('download non-image gzip attachment', () => {
+			const fileName = 'native attachments.md'
+			cy.openFile(fileName)
+
+			cy.getContent()
+				.find('[data-component="image-view"][data-src=".attachments.123/file.txt.gz"] img')
+				.click()
+
+			const downloadsFolder = Cypress.config('downloadsFolder')
+			cy.log(`downloadsFolder: ${downloadsFolder}`)
+			cy.readFile(`${downloadsFolder}/file.txt.gz`)
+		})
+	})
 })

cypress/support/commands.js

@@ -323,6 +323,11 @@ Cypress.Commands.add('openFolder', (name) => {
 	cy.wait(`@open-${name}`)
 })
 
+Cypress.Commands.add('getFileId', (fileName, params = {}) => {
+	return cy.get(`[data-cy-files-list] tr[data-cy-files-list-row-name="${fileName}"]`)
+		.invoke('attr', 'data-cy-files-list-row-fileid')
+})
+
 Cypress.Commands.add('openFile', (fileName, params = {}) => {
 	cy.get(`[data-cy-files-list] tr[data-cy-files-list-row-name="${fileName}"] a[data-cy-files-list-row-name-link]`).click(params)
 })

lib/Controller/AttachmentController.php

@@ -26,8 +26,10 @@
 namespace OCA\Text\Controller;
 
 use Exception;
+use OCA\Text\Exception\InvalidSessionException;
 use OCA\Text\Exception\UploadException;
 use OCA\Text\Middleware\Attribute\RequireDocumentSession;
+use OCA\Text\Middleware\Attribute\RequireDocumentSessionOrUserOrShareToken;
 use OCA\Text\Service\AttachmentService;
 use OCP\AppFramework\ApiController;
 use OCP\AppFramework\Http;
@@ -81,6 +83,27 @@ public function __construct(
 		parent::__construct($appName, $request);
 	}
 
+	#[NoAdminRequired]
+	#[PublicPage]
+	#[RequireDocumentSessionOrUserOrShareToken]
+	public function getAttachmentList(?string $shareToken = null): DataResponse {
+		$documentId = $this->getDocument()->getId();
+		try {
+			$session = $this->getSession();
+		} catch (InvalidSessionException) {
+			$session = null;
+		}
+
+		if ($shareToken) {
+			$attachments = $this->attachmentService->getAttachmentList($documentId, null, $session, $shareToken);
+		} else {
+			$userId = $this->getUserId();
+			$attachments = $this->attachmentService->getAttachmentList($documentId, $userId, $session, null);
+		}
+
+		return new DataResponse($attachments);
+	}
+
 	#[NoAdminRequired]
 	#[PublicPage]
 	#[RequireDocumentSession]
@@ -169,22 +192,22 @@ private function getUploadedFile(string $key): array {
 	#[NoAdminRequired]
 	#[PublicPage]
 	#[NoCSRFRequired]
-	#[RequireDocumentSession]
+	#[RequireDocumentSessionOrUserOrShareToken]
 	public function getImageFile(string $imageFileName, ?string $shareToken = null,
 		int $preferRawImage = 0): DataResponse|DataDownloadResponse {
-		$documentId = $this->getSession()->getDocumentId();
+		$documentId = $this->getDocument()->getId();
 
 		try {
 			if ($shareToken) {
 				$imageFile = $this->attachmentService->getImageFilePublic($documentId, $imageFileName, $shareToken, $preferRawImage === 1);
 			} else {
-				$userId = $this->getSession()->getUserId();
+				$userId = $this->getUserId();
 				$imageFile = $this->attachmentService->getImageFile($documentId, $imageFileName, $userId, $preferRawImage === 1);
 			}
 			return $imageFile !== null
 				? new DataDownloadResponse(
 					$imageFile->getContent(),
-					(string) Http::STATUS_OK,
+					$imageFile->getName(),
 					$this->getSecureMimeType($imageFile->getMimeType())
 				)
 				: new DataResponse('', Http::STATUS_NOT_FOUND);
@@ -204,21 +227,21 @@ public function getImageFile(string $imageFileName, ?string $shareToken = null,
 	#[NoAdminRequired]
 	#[PublicPage]
 	#[NoCSRFRequired]
-	#[RequireDocumentSession]
+	#[RequireDocumentSessionOrUserOrShareToken]
 	public function getMediaFile(string $mediaFileName, ?string $shareToken = null): DataResponse|DataDownloadResponse {
-		$documentId = $this->getSession()->getDocumentId();
+		$documentId = $this->getDocument()->getId();
 
 		try {
 			if ($shareToken) {
 				$mediaFile = $this->attachmentService->getMediaFilePublic($documentId, $mediaFileName, $shareToken);
 			} else {
-				$userId = $this->getSession()->getUserId();
+				$userId = $this->getUserId();
 				$mediaFile = $this->attachmentService->getMediaFile($documentId, $mediaFileName, $userId);
 			}
 			return $mediaFile !== null
 				? new DataDownloadResponse(
 					$mediaFile->getContent(),
-					(string) Http::STATUS_OK,
+					$mediaFile->getName(),
 					$this->getSecureMimeType($mediaFile->getMimeType())
 				)
 				: new DataResponse('', Http::STATUS_NOT_FOUND);
@@ -235,15 +258,15 @@ public function getMediaFile(string $mediaFileName, ?string $shareToken = null):
 	#[NoAdminRequired]
 	#[PublicPage]
 	#[NoCSRFRequired]
-	#[RequireDocumentSession]
+	#[RequireDocumentSessionOrUserOrShareToken]
 	public function getMediaFilePreview(string $mediaFileName, ?string $shareToken = null) {
-		$documentId = $this->getSession()->getDocumentId();
+		$documentId = $this->getDocument()->getId();
 
 		try {
 			if ($shareToken) {
 				$preview = $this->attachmentService->getMediaFilePreviewPublic($documentId, $mediaFileName, $shareToken);
 			} else {
-				$userId = $this->getSession()->getUserId();
+				$userId = $this->getUserId();
 				$preview = $this->attachmentService->getMediaFilePreview($documentId, $mediaFileName, $userId);
 			}
 			if ($preview === null) {
@@ -252,7 +275,7 @@ public function getMediaFilePreview(string $mediaFileName, ?string $shareToken =
 			if ($preview['type'] === 'file') {
 				return new DataDownloadResponse(
 					$preview['file']->getContent(),
-					(string) Http::STATUS_OK,
+					$mediaFileName,
 					$this->getSecureMimeType($preview['file']->getMimeType())
 				);
 			} elseif ($preview['type'] === 'icon') {
@@ -264,32 +287,6 @@ public function getMediaFilePreview(string $mediaFileName, ?string $shareToken =
 		return new DataResponse('', Http::STATUS_NOT_FOUND);
 	}
 
-	/**
-	 * Serve the media files metadata in the editor
-	 */
-	#[NoAdminRequired]
-	#[PublicPage]
-	#[NoCSRFRequired]
-	#[RequireDocumentSession]
-	public function getMediaFileMetadata(string $mediaFileName, ?string $shareToken = null): DataResponse {
-		$documentId = $this->getSession()->getDocumentId();
-		try {
-			if ($shareToken) {
-				$metadata = $this->attachmentService->getMediaFileMetadataPublic($documentId, $mediaFileName, $shareToken);
-			} else {
-				$userId = $this->getSession()->getUserId();
-				$metadata = $this->attachmentService->getMediaFileMetadataPrivate($documentId, $mediaFileName, $userId);
-			}
-			if ($metadata === null) {
-				return new DataResponse('', Http::STATUS_NOT_FOUND);
-			}
-			return new DataResponse($metadata);
-		} catch (Exception $e) {
-			$this->logger->error('getMediaFileMetadata error', ['exception' => $e]);
-			return new DataResponse('', Http::STATUS_NOT_FOUND);
-		}
-	}
-
 	/**
 	 * Allow all supported mimetypes
 	 * Use mimetype detector for the other ones

lib/Controller/ISessionAwareController.php

@@ -10,4 +10,6 @@ public function getSession(): Session;
 	public function setSession(Session $session): void;
 	public function getDocument(): Document;
 	public function setDocument(Document $document): void;
+	public function getUserId(): string;
+	public function setUserId(string $userId): void;
 }

lib/Controller/TSessionAwareController.php

@@ -11,6 +11,7 @@
 trait TSessionAwareController {
 	private ?Session $textSession = null;
 	private ?Document $document = null;
+	private ?string $userId = null;
 
 	public function setSession(?Session $session): void {
 		$this->textSession = $session;
@@ -20,6 +21,10 @@ public function setDocument(?Document $document): void {
 		$this->document = $document;
 	}
 
+	public function setUserId(?string $userId): void {
+		$this->userId = $userId;
+	}
+
 	public function getSession(): Session {
 		if ($this->textSession === null) {
 			throw new InvalidSessionException();
@@ -36,4 +41,11 @@ public function getDocument(): Document {
 		return $this->document;
 	}
 
+	public function getUserId(): string {
+		if ($this->userId === null) {
+			throw new InvalidSessionException();
+		}
+
+		return $this->userId;
+	}
 }

lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace OCA\Text\Middleware\Attribute;
+
+use Attribute;
+
+#[Attribute(Attribute::TARGET_METHOD)]
+class RequireDocumentSessionOrUserOrShareToken {
+}