Merge pull request #3834 from nextcloud/backport/3831/stable29

[stable29] fix(client): Use shared default options for HTTP client requests
nextcloud · Jul 25, 2024 · d13c3e7 · d13c3e7
2 parents d6e7371 + ff4f22b
commit d13c3e7
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 28 deletions.
diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php
@@ -74,6 +74,7 @@
     'OCA\\Richdocuments\\Service\\FileTargetService' => $baseDir . '/../lib/Service/FileTargetService.php',
     'OCA\\Richdocuments\\Service\\FontService' => $baseDir . '/../lib/Service/FontService.php',
     'OCA\\Richdocuments\\Service\\InitialStateService' => $baseDir . '/../lib/Service/InitialStateService.php',
+    'OCA\\Richdocuments\\Service\\RemoteOptionsService' => $baseDir . '/../lib/Service/RemoteOptionsService.php',
     'OCA\\Richdocuments\\Service\\RemoteService' => $baseDir . '/../lib/Service/RemoteService.php',
     'OCA\\Richdocuments\\Service\\UserScopeService' => $baseDir . '/../lib/Service/UserScopeService.php',
     'OCA\\Richdocuments\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',

diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php
@@ -89,6 +89,7 @@ class ComposerStaticInitRichdocuments
         'OCA\\Richdocuments\\Service\\FileTargetService' => __DIR__ . '/..' . '/../lib/Service/FileTargetService.php',
         'OCA\\Richdocuments\\Service\\FontService' => __DIR__ . '/..' . '/../lib/Service/FontService.php',
         'OCA\\Richdocuments\\Service\\InitialStateService' => __DIR__ . '/..' . '/../lib/Service/InitialStateService.php',
+        'OCA\\Richdocuments\\Service\\RemoteOptionsService' => __DIR__ . '/..' . '/../lib/Service/RemoteOptionsService.php',
         'OCA\\Richdocuments\\Service\\RemoteService' => __DIR__ . '/..' . '/../lib/Service/RemoteService.php',
         'OCA\\Richdocuments\\Service\\UserScopeService' => __DIR__ . '/..' . '/../lib/Service/UserScopeService.php',
         'OCA\\Richdocuments\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',

diff --git a/lib/Preview/Office.php b/lib/Preview/Office.php
@@ -23,23 +23,30 @@
 
 use OCA\Richdocuments\AppConfig;
 use OCA\Richdocuments\Capabilities;
+use OCA\Richdocuments\Service\RemoteOptionsService;
 use OCP\Files\File;
+use OCP\Files\FileInfo;
 use OCP\Http\Client\IClientService;
 use OCP\IImage;
 use OCP\Image;
 use OCP\Preview\IProviderV2;
 use Psr\Log\LoggerInterface;
 
 abstract class Office implements IProviderV2 {
-	private array $capabilities = [];
-
-	public function __construct(private IClientService $clientService, private AppConfig $config, Capabilities $capabilities, private LoggerInterface $logger) {
-		$this->capabilitites = $capabilities->getCapabilities()['richdocuments'] ?? [];
+	private array $capabilities;
+
+	public function __construct(
+		private IClientService $clientService,
+		private AppConfig $config,
+		Capabilities $capabilities,
+		private LoggerInterface $logger,
+	) {
+		$this->capabilities = $capabilities->getCapabilities()['richdocuments'] ?? [];
 	}
 
-	public function isAvailable(\OCP\Files\FileInfo $file): bool {
-		if (isset($this->capabilitites['collabora']['convert-to']['available'])) {
-			return (bool)$this->capabilitites['collabora']['convert-to']['available'];
+	public function isAvailable(FileInfo $file): bool {
+		if (isset($this->capabilities['collabora']['convert-to']['available'])) {
+			return (bool)$this->capabilities['collabora']['convert-to']['available'];
 		}
 		return false;
 	}
@@ -61,11 +68,9 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		}
 
 		$client = $this->clientService->newClient();
-		$options = [
-			'timeout' => 25,
-			// FIXME: Can be removed once https://github.com/CollaboraOnline/online/issues/6983 is fixed upstream
-			'expect' => false,
-		];
+		$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;

diff --git a/lib/Service/CachedRequestService.php b/lib/Service/CachedRequestService.php
@@ -19,7 +19,6 @@
 use Psr\Log\LoggerInterface;
 
 abstract class CachedRequestService {
-
 	public function __construct(
 		private IClientService $clientService,
 		private ICacheFactory $cacheFactory,
@@ -91,12 +90,7 @@ public function resetCache(): void {
 	}
 
 	protected function getDefaultRequestOptions(): array {
-		$options = [
-			'timeout' => 5,
-			'nextcloud' => [
-				'allow_local_address' => true
-			]
-		];
+		$options = RemoteOptionsService::getDefaultOptions();
 
 		if ($this->appConfig->getValueString('richdocuments', 'disable_certificate_verification') === 'yes') {
 			$options['verify'] = false;

diff --git a/lib/Service/RemoteOptionsService.php b/lib/Service/RemoteOptionsService.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+
+namespace OCA\Richdocuments\Service;
+
+class RemoteOptionsService {
+	public const REMOTE_TIMEOUT_DEFAULT = 5;
+
+	public static function getDefaultOptions(int $timeout = self::REMOTE_TIMEOUT_DEFAULT): array {
+		return [
+			'timeout' => $timeout,
+			'nextcloud' => [
+				'allow_local_address' => true,
+			]
+		];
+	}
+
+}
diff --git a/lib/Service/RemoteService.php b/lib/Service/RemoteService.php
@@ -10,9 +10,6 @@
 use Psr\Log\LoggerInterface;
 
 class RemoteService {
-
-	public const REMOTE_TIMEOUT_DEFAULT = 25;
-
 	public function __construct(
 		private AppConfig $appConfig,
 		private IClientService $clientService,
@@ -66,12 +63,10 @@ private function getRequestOptionsForFile(File $file, ?string $target = null): a
 			$stream = $file->fopen('rb');
 		}
 
-		$options = [
-			'timeout' => self::REMOTE_TIMEOUT_DEFAULT,
-			'multipart' => [
-				['name' => $file->getName(), 'contents' => $stream],
-				['name' => 'target', 'contents' => $target]
-			]
+		$options = RemoteOptionsService::getDefaultOptions(25);
+		$options['multipart'] = [
+			['name' => $file->getName(), 'contents' => $stream],
+			['name' => 'target', 'contents' => $target]
 		];
 
 		if ($this->appConfig->getDisableCertificateValidation()) {