Merge pull request #3951 from nextcloud/feat/fill-and-convert

feat: Add parameter to allow converting files after filling out fields

.github/workflows/integration.yml

@@ -55,7 +55,7 @@ jobs:
         php-versions: ['8.1']
         databases: ['sqlite']
         server-versions: ['master']
-        scenarios: ['wopi', 'direct', 'federation']
+        scenarios: ['wopi', 'direct', 'federation', 'api']
 
     name: integration-${{ matrix.code-image }}-${{ matrix.scenarios }}-${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }}
 
@@ -127,7 +127,7 @@ jobs:
         php-versions: ['8.1']
         databases: ['mysql']
         server-versions: ['master']
-        scenarios: ['wopi', 'direct', 'federation']
+        scenarios: ['wopi', 'direct', 'federation', 'api']
 
     name: integration-${{ matrix.scenarios }}-${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }}
 
@@ -206,7 +206,7 @@ jobs:
         php-versions: ['8.1']
         databases: ['pgsql']
         server-versions: ['master']
-        scenarios: ['wopi', 'direct', 'federation']
+        scenarios: ['wopi', 'direct', 'federation', 'api']
 
     name: integration-${{ matrix.scenarios }}-${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }}
 
@@ -287,7 +287,7 @@ jobs:
         php-versions: ['8.1']
         databases: ['oci']
         server-versions: ['master']
-        scenarios: ['wopi', 'direct', 'federation']
+        scenarios: ['wopi', 'direct', 'federation', 'api']
 
     name: integration-${{ matrix.scenarios }}-${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }}
 

REUSE.toml

@@ -18,7 +18,7 @@ SPDX-FileCopyrightText = "2016 Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"
 
 [[annotations]]
-path = ["js/**.js.map", "js/**.js", "js/**.mjs", "js/**.mjs.map", "js/templates/**.handlebars", "emptyTemplates/**", "cypress/fixtures/**", "tests/features/**.feature", "tests/psalm-baseline.xml"]
+path = ["js/**.js.map", "js/**.js", "js/**.mjs", "js/**.mjs.map", "js/templates/**.handlebars", "emptyTemplates/**", "cypress/fixtures/**", "tests/data/**", "tests/features/**.feature", "tests/psalm-baseline.xml"]
 precedence = "aggregate"
 SPDX-FileCopyrightText = "2016 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"

lib/Controller/TemplateFieldController.php

@@ -8,7 +8,6 @@
 namespace OCA\Richdocuments\Controller;
 
 use OCA\Richdocuments\Service\TemplateFieldService;
-use OCA\Richdocuments\TemplateManager;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\DataResponse;
@@ -16,27 +15,12 @@ use OCP\AppFramework\OCSController;
 use OCP\IRequest;
 
 class TemplateFieldController extends OCSController {
-	private TemplateFieldService $templateFieldService;
-	private TemplateManager $templateManager;
-
-	/**
-	 * Template fields controller
-	 *
-	 * @param string $appName,
-	 * @param IRequest $request,
-	 * @param TemplateFieldService $templateFieldService
-	 * @param TemplateManager $templateManager
-	 */
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		TemplateFieldService $templateFieldService,
-		TemplateManager $templateManager
+		private TemplateFieldService $templateFieldService,
 	) {
 		parent::__construct($appName, $request);
-
-		$this->templateFieldService = $templateFieldService;
-		$this->templateManager = $templateManager;
 	}
 
 	/**
@@ -60,9 +44,14 @@ class TemplateFieldController extends OCSController {
 	 * @return DataResponse
 	 */
 	#[NoAdminRequired]
-	public function fillFields(int $fileId, array $fields, ?string $destination = null): DataResponse {
+	public function fillFields(int $fileId, array $fields = [], ?string $destination = null, ?string $convert = null): DataResponse {
 		try {
-			$this->templateFieldService->fillFields($fileId, $fields, $destination);
+			$content = $this->templateFieldService->fillFields($fileId, $fields, $destination, $convert);
+
+			if ($destination === null) {
+				echo $content;
+				die();
+			}
 
 			return new DataResponse([], Http::STATUS_OK);
 		} catch (\Exception $e) {

lib/Preview/Office.php

@@ -5,12 +5,10 @@
  */
 namespace OCA\Richdocuments\Preview;
 
-use OCA\Richdocuments\AppConfig;
 use OCA\Richdocuments\Capabilities;
-use OCA\Richdocuments\Service\RemoteOptionsService;
+use OCA\Richdocuments\Service\RemoteService;
 use OCP\Files\File;
 use OCP\Files\FileInfo;
-use OCP\Http\Client\IClientService;
 use OCP\IImage;
 use OCP\Image;
 use OCP\Preview\IProviderV2;
@@ -20,10 +18,9 @@ abstract class Office implements IProviderV2 {
 	private array $capabilities;
 
 	public function __construct(
-		private IClientService $clientService,
-		private AppConfig $config,
-		Capabilities $capabilities,
+		private RemoteService $remoteService,
 		private LoggerInterface $logger,
+		Capabilities $capabilities,
 	) {
 		$this->capabilities = $capabilities->getCapabilities()['richdocuments'] ?? [];
 	}
@@ -35,47 +32,25 @@ abstract class Office implements IProviderV2 {
 		return false;
 	}
 
-	/**
-	 * {@inheritDoc}
-	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		if ($file->getSize() === 0) {
 			return null;
 		}
 
-		$useTempFile = $file->isEncrypted() || !$file->getStorage()->isLocal();
-		if ($useTempFile) {
-			$fileName = $file->getStorage()->getLocalFile($file->getInternalPath());
-			$stream = fopen($fileName, 'r');
-		} else {
-			$stream = $file->fopen('r');
-		}
-
-		$client = $this->clientService->newClient();
-		$options = RemoteOptionsService::getDefaultOptions();
-		// FIXME: can be removed once https://github.com/CollaboraOnline/online/issues/6983 is fixed upstream
-		$options['expect'] = false;
-
-		if ($this->config->getAppValue('richdocuments', 'disable_certificate_verification') === 'yes') {
-			$options['verify'] = false;
-		}
-
-		$options['multipart'] = [['name' => $file->getName(), 'contents' => $stream]];
-
 		try {
-			$response = $client->post($this->config->getCollaboraUrlInternal() . '/cool/convert-to/png', $options);
+			$response = $this->remoteService->convertFileTo($file, 'png');
+			$image = new Image();
+			$image->loadFromData($response);
+
+			if ($image->valid()) {
+				$image->scaleDownToFit($maxX, $maxY);
+				return $image;
+			}
 		} catch (\Exception $e) {
 			$this->logger->info('Failed to convert preview: ' . $e->getMessage(), ['exception' => $e]);
 			return null;
 		}
 
-		$image = new Image();
-		$image->loadFromData($response->getBody());
-
-		if ($image->valid()) {
-			$image->scaleDownToFit($maxX, $maxY);
-			return $image;
-		}
 		return null;
 	}
 }

lib/Service/RemoteService.php

@@ -54,6 +54,49 @@ class RemoteService {
 		return null;
 	}
 
+	/**
+	 * @return resource|string
+	 */
+	public function convertFileTo(File $file, string $format) {
+		$useTempFile = $file->isEncrypted() || !$file->getStorage()->isLocal();
+		if ($useTempFile) {
+			$fileName = $file->getStorage()->getLocalFile($file->getInternalPath());
+			$stream = fopen($fileName, 'r');
+		} else {
+			$stream = $file->fopen('r');
+		}
+
+		if ($stream === false) {
+			throw new Exception('Failed to open stream');
+		}
+		return $this->convertTo($file->getName(), $stream, $format);
+	}
+
+	/**
+	 * @param resource $stream
+	 * @return resource|string
+	 */
+	public function convertTo(string $filename, $stream, string $format) {
+		$client = $this->clientService->newClient();
+		$options = RemoteOptionsService::getDefaultOptions();
+		// FIXME: can be removed once https://github.com/CollaboraOnline/online/issues/6983 is fixed upstream
+		$options['expect'] = false;
+
+		if ($this->appConfig->getDisableCertificateValidation()) {
+			$options['verify'] = false;
+		}
+
+		$options['multipart'] = [['name' => $filename, 'contents' => $stream]];
+
+		try {
+			$response = $client->post($this->appConfig->getCollaboraUrlInternal() . '/cool/convert-to/' . $format, $options);
+			return $response->getBody();
+		} catch (\Exception $e) {
+			$this->logger->error('Failed to convert preview: ' . $e->getMessage(), ['exception' => $e]);
+			throw $e;
+		}
+	}
+
 	private function getRequestOptionsForFile(File $file, ?string $target = null): array {
 		$useTempFile = $file->isEncrypted() || !$file->getStorage()->isLocal();
 		if ($useTempFile) {

lib/Service/TemplateFieldService.php

@@ -19,6 +19,7 @@ use OCP\Files\Template\FieldType;
 use OCP\Files\Template\InvalidFieldTypeException;
 use OCP\Http\Client\IClientService;
 use OCP\ICacheFactory;
+use OCP\ITempManager;
 use Psr\Log\LoggerInterface;
 
 class TemplateFieldService {
@@ -29,6 +30,8 @@ class TemplateFieldService {
 		private IRootFolder $rootFolder,
 		private LoggerInterface $logger,
 		private ICacheFactory $cacheFactory,
+		private RemoteService $remoteService,
+		private ITempManager $tempManager,
 		private PdfService $pdfService,
 		private ?string $userId,
 	) {
@@ -134,7 +137,7 @@ class TemplateFieldService {
 	 * @param array<string, array{content: string}> $fields
 	 * @return string|resource
 	 */
-	public function fillFields(Node|int $file, array $fields = [], ?string $destination = null) {
+	public function fillFields(Node|int $file, array $fields = [], ?string $destination = null, ?string $format = null) {
 		if (!$this->capabilitiesService->hasFormFilling()) {
 			throw new \RuntimeException('Form filling not supported by the Collabora server');
 		}
@@ -187,6 +190,14 @@ class TemplateFieldService {
 			);
 
 			$content = $response->getBody();
+
+			if ($format !== null) {
+				$tmp = $this->tempManager->getTemporaryFile();
+				file_put_contents($tmp, $content);
+				$fp = fopen($tmp, 'rb');
+				$content = $this->remoteService->convertTo($file->getName(), $fp, $format);
+			}
+
 			if ($destination !== null) {
 				$this->writeToDestination($destination, $content);
 			}

tests/config/behat.yml

@@ -21,6 +21,7 @@ default:
         - WopiContext
         - DirectContext
         - FederationContext
+        - ApiContext
 
 
   extensions:

tests/features/api.feature

@@ -0,0 +1,43 @@
+Feature: API
+
+  Background:
+    Given user "user1" exists
+
+  Scenario: Extract field values
+    Given as user "user1"
+    And User "user1" uploads file "./data/form.odt" to "/form.odt"
+    Then User "user1" requests the form field data of "/form.odt"
+    And the response contains the field "Name of the Organizer/Organization"
+    And the response contains the field "Road closures - Length of road closures (in meters)"
+
+  Scenario: Extract field values and fill in field
+    Given as user "user1"
+    And User "user1" uploads file "./data/form.odt" to "/form.odt"
+    Then User "user1" requests the form field data of "/form.odt"
+    And the response contains the field "Name of the Organizer/Organization"
+    And the response contains the field "Road closures - Length of road closures (in meters)"
+    Then User "user1" fills in fields of "/form.odt" with values as "odt" to "/filled.odt"
+      | ContentControls.ByIndex.19 | 100 |
+    Then User "user1" requests the form field data of "/filled.odt"
+    And the response contains the field "Road closures - Length of road closures (in meters)" with "100"
+    And the resulting file is a "application/vnd.oasis.opendocument.text"
+
+  Scenario: Extract field values and fill in field as pdf
+    Given as user "user1"
+    And User "user1" uploads file "./data/form.odt" to "/form.odt"
+    Then User "user1" requests the form field data of "/form.odt"
+    And the response contains the field "Name of the Organizer/Organization"
+    And the response contains the field "Road closures - Length of road closures (in meters)"
+    Then User "user1" fills in fields of "/form.odt" with values as "pdf" to "/filled.pdf"
+      | ContentControls.ByIndex.19 | 100 |
+    And the resulting file is a "application/pdf"
+
+  Scenario: Extract field values and fill in field as pdf
+    Given as user "user1"
+    And User "user1" uploads file "./data/form.odt" to "/form.odt"
+    Then User "user1" requests the form field data of "/form.odt"
+    And the response contains the field "Name of the Organizer/Organization"
+    And the response contains the field "Road closures - Length of road closures (in meters)"
+    Then User "user1" fills in fields of "/form.odt" with values as "pdf"
+      | ContentControls.ByIndex.19 | 100 |
+    And the resulting file is a "application/pdf"

tests/features/bootstrap/ApiContext.php

@@ -0,0 +1,142 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+use Behat\Behat\Context\Context;
+use Behat\Behat\Hook\Scope\BeforeScenarioScope;
+use Behat\Gherkin\Node\TableNode;
+use JuliusHaertl\NextcloudBehat\Context\FilesContext;
+use JuliusHaertl\NextcloudBehat\Context\ServerContext;
+use PHPUnit\Framework\Assert;
+
+class ApiContext implements Context {
+	private ServerContext $serverContext;
+	private FilesContext $filesContext;
+
+	private array $extractResult;
+
+	/** @BeforeScenario */
+	public function gatherContexts(BeforeScenarioScope $scope) {
+		$environment = $scope->getEnvironment();
+		$this->serverContext = $environment->getContext(ServerContext::class);
+		$this->filesContext = $environment->getContext(FilesContext::class);
+	}
+
+	/**
+	 * @Then /^User "([^"]*)" requests the form field data of "([^"]*)"$/
+	 */
+	public function userRequestsTheFormFieldData($user, $filePath) {
+		$davClient = $this->filesContext->getSabreClient($user);
+		$davPath = $this->filesContext->makeSabrePath($user, $filePath);
+		$result = $davClient->propFind($davPath, ['{http://owncloud.org/ns}fileid']);
+		$fileId = $result['{http://owncloud.org/ns}fileid'];
+		$this->serverContext->sendOCSRequest('GET', 'apps/richdocuments/api/v1/template/fields/extract/' . $fileId);
+
+		$body = (string)$this->serverContext->getResponse()->getBody();
+		;
+		$this->extractResult = json_decode($body, true);
+
+		$this->serverContext->assertHttpStatusCode(200);
+	}
+
+	/**
+	 * @Given /^the response contains the field "([^"]*)"$/
+	 */
+	public function theResponseContainsTheField($arg1) {
+		$found = false;
+		foreach ($this->extractResult['ocs']['data'] as $index => $field) {
+			if ($field['alias'] === $arg1) {
+				$found = true;
+			}
+		}
+		Assert::assertTrue($found, 'Field was not found');
+	}
+
+	/**
+	 * @Then /^User "([^"]*)" fills in fields of "([^"]*)" with values as "([^"]*)" to "([^"]*)"$/
+	 */
+	public function userFillsInFieldsOfWithValuesAsTo($user, $source, ?string $convert = null, ?string $target = null, ?TableNode $table = null) {
+		$davClient = $this->filesContext->getSabreClient($user);
+		$davPath = $this->filesContext->makeSabrePath($user, $source);
+		$result = $davClient->propFind($davPath, ['{http://owncloud.org/ns}fileid']);
+		$fileId = $result['{http://owncloud.org/ns}fileid'];
+
+		$formData = [];
+		foreach ($table->getRows() as $row) {
+			$formData[$row[0]] = [
+				'content' => $row[1],
+			];
+		}
+
+		$this->serverContext->sendOCSRequest('POST', 'apps/richdocuments/api/v1/template/fields/fill/' . $fileId  . '?'
+			. ($convert ? '&convert=' . $convert : '')
+			. ($target ? '&destination=' . $target : ''), ['fields' => $formData]);
+
+		$this->serverContext->assertHttpStatusCode(200);
+
+		if ($target === null) {
+			file_put_contents('data/output.pdf', $this->serverContext->getResponse()->getBody());
+		} else {
+			$this->serverContext->sendRawRequest('GET', '/remote.php/dav/files/' . $user . '/' . $target);
+			file_put_contents('data/output.pdf', $this->serverContext->getResponse()->getBody());
+		}
+	}
+
+	/**
+	 * @Given /^the response contains the field "([^"]*)" with "([^"]*)"$/
+	 */
+	public function theResponseContainsTheFieldWith($arg1, $arg2) {
+		$found = false;
+		foreach ($this->extractResult['ocs']['data'] as $index => $field) {
+			if ($field['alias'] === $arg1) {
+				Assert::assertEquals($arg2, $field['content']);
+				$found = true;
+			}
+		}
+		Assert::assertTrue($found, 'Field was not found');
+	}
+
+	/**
+	 * @Then /^User "([^"]*)" fills in fields of "([^"]*)" with values as "([^"]*)"$/
+	 */
+	public function userFillsInFieldsOfWithValuesAs($arg1, $arg2, $arg3, TableNode $table) {
+		$this->userFillsInFieldsOfWithValuesAsTo($arg1, $arg2, $arg3, null, $table);
+	}
+
+	/**
+	 * @Then /^User "([^"]*)" downloads the file and compares it with the baseline$/
+	 */
+	public function userDownloadsTheFile($arg1) {
+		$this->serverContext->setCurrentUser($arg1);
+		$this->serverContext->sendRawRequest('GET', '/remote.php/dav/files/' . $arg1 . '/filled.pdf');
+		file_put_contents('data/output.pdf', $this->serverContext->getResponse()->getBody());
+
+		Assert::assertEquals(
+			sha1_file('data/filled.pdf'),
+			sha1_file('data/output.pdf'),
+		);
+	}
+
+	/**
+	 * @Given /^the resulting file is a "([^"]*)"$/
+	 */
+	public function theResultingFileIsA($mimetype) {
+		$filetype = mime_content_type('data/output.pdf');
+		Assert::assertEquals($mimetype, $filetype);
+	}
+
+	/**
+	 * @Then /^compares the returned file with the baseline$/
+	 */
+	public function comparesTheReturnedFileWithTheBaseline() {
+		file_put_contents('data/output.pdf', $this->serverContext->getResponse()->getBody());
+
+		Assert::assertEquals(
+			sha1_file('data/filled.pdf'),
+			sha1_file('data/output.pdf'),
+		);
+	}
+}