address review comments

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>

2 files changed, 23 insertions, 37 deletions

diff --git a/lib/Controller/ImageController.php b/lib/Controller/ImageController.php
index 1903351ae..5e8e587b0 100644
--- a/lib/Controller/ImageController.php
+++ b/lib/Controller/ImageController.php
@@ -90,7 +90,7 @@ class ImageController extends Controller {
 				return new DataResponse($insertResult);
 			}
 		} catch (Exception $e) {
-			$this->logger->error('File insertion error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->error('File insertion error', ['exception' => $e]);
 			return new DataResponse(['error' => 'File insertion error'], Http::STATUS_BAD_REQUEST);
 		}
 	}
@@ -111,7 +111,7 @@ class ImageController extends Controller {
 				return new DataResponse($downloadResult);
 			}
 		} catch (Exception $e) {
-			$this->logger->error('Link insertion error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->error('Link insertion error', ['exception' => $e]);
 			return new DataResponse(['error' => 'Link insertion error'], Http::STATUS_BAD_REQUEST);
 		}
 	}
@@ -134,7 +134,7 @@ class ImageController extends Controller {
 				return new DataResponse($downloadResult);
 			}
 		} catch (Exception $e) {
-			$this->logger->error('Link insertion error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->error('Link insertion error', ['exception' => $e]);
 			return new DataResponse(['error' => 'Link insertion error'], Http::STATUS_BAD_REQUEST);
 		}
 	}
@@ -164,7 +164,7 @@ class ImageController extends Controller {
 				return new DataResponse(['error' => 'No uploaded file'], Http::STATUS_BAD_REQUEST);
 			}
 		} catch (Exception $e) {
-			$this->logger->error('Upload error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->error('Upload error', ['exception' => $e]);
 			return new DataResponse(['error' => 'Upload error'], Http::STATUS_BAD_REQUEST);
 		}
 	}
@@ -196,7 +196,7 @@ class ImageController extends Controller {
 				return new DataResponse(['error' => 'No uploaded file'], Http::STATUS_BAD_REQUEST);
 			}
 		} catch (Exception $e) {
-			$this->logger->error('Upload error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->error('Upload error', ['exception' => $e]);
 			return new DataResponse(['error' => 'Upload error'], Http::STATUS_BAD_REQUEST);
 		}
 	}
@@ -233,6 +233,11 @@ class ImageController extends Controller {
 	 * @param string $imageFileName
 	 * @param string $shareToken
 	 * @return DataDisplayResponse
+	 * @throws \OCP\Files\InvalidPathException
+	 * @throws \OCP\Files\NotFoundException
+	 * @throws \OCP\Files\NotPermittedException
+	 * @throws \OCP\Lock\LockedException
+	 * @throws \OC\User\NoUserException
 	 */
 	public function getImagePublic(int $textFileId, string $imageFileName, string $shareToken): DataDisplayResponse {
 		$imageFile = $this->imageService->getImagePublic($textFileId, $imageFileName, $shareToken);
diff --git a/lib/Service/ImageService.php b/lib/Service/ImageService.php
index 61aee840f..7e402c700 100644
--- a/lib/Service/ImageService.php
+++ b/lib/Service/ImageService.php
@@ -259,11 +259,10 @@ class ImageService {
 				'id' => $targetFile->getId(),
 				'textFileId' => $textFile->getId(),
 			];
-		} else {
-			return [
-				'error' => 'Unsupported file type',
-			];
 		}
+		return [
+			'error' => 'Unsupported file type',
+		];
 	}
 
 	/**
@@ -346,7 +345,7 @@ class ImageService {
 	private function downloadLink(Folder $saveDir, string $link, File $textFile): array {
 		$fileName = (string) time();
 		$savedFile = $saveDir->newFile($fileName);
-		$resource = $savedFile->fopen('w');
+		$resource = $savedFile->fopen('wb');
 		$res = $this->simpleDownload($link, $resource);
 		if (is_resource($resource)) {
 			fclose($resource);
@@ -520,14 +519,12 @@ class ImageService {
 
 	/**
 	 * Download a file and write it to a resource
-	 *
 	 * @param string $url
 	 * @param $resource
 	 * @param array $params
-	 * @param string $method
-	 * @return array|string[]
+	 * @return array
 	 */
-	private function simpleDownload(string $url, $resource, array $params = [], string $method = 'GET'): array {
+	private function simpleDownload(string $url, $resource, array $params = []): array {
 		$client = $this->clientService->newClient();
 		try {
 			$options = [
@@ -542,27 +539,11 @@ class ImageService {
 			];
 
 			if (count($params) > 0) {
-				if ($method === 'GET') {
-					$paramsContent = http_build_query($params);
-					$url .= '?' . $paramsContent;
-				} else {
-					$options['body'] = json_encode($params);
-				}
-			}
-
-			if ($method === 'GET') {
-				$response = $client->get($url, $options);
-			} elseif ($method === 'POST') {
-				$response = $client->post($url, $options);
-			} elseif ($method === 'PUT') {
-				$response = $client->put($url, $options);
-			} elseif ($method === 'DELETE') {
-				$response = $client->delete($url, $options);
-			} else {
-				return ['error' => 'Bad HTTP method'];
+				$paramsContent = http_build_query($params);
+				$url .= '?' . $paramsContent;
 			}
-			$respCode = $response->getStatusCode();
 
+			$response = $client->get($url, $options);
 			$body = $response->getBody();
 			while (!feof($body)) {
 				// write ~5 MB chunks
@@ -574,14 +555,14 @@ class ImageService {
 		} catch (ServerException | ClientException $e) {
 			//$response = $e->getResponse();
 			//if ($response->getStatusCode() === 401) {
-			$this->logger->warning('Impossible to download image: '.$e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->warning('Impossible to download image', ['exception' => $e]);
 			return ['error' => 'Impossible to download image'];
 		} catch (ConnectException $e) {
-			$this->logger->error('Connection error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
+			$this->logger->error('Connection error', ['exception' => $e]);
 			return ['error' => 'Connection error'];
 		} catch (Throwable | Exception $e) {
-			$this->logger->error('Unknown error: ' . $e->getMessage(), ['app' => Application::APP_NAME]);
-			return ['error' => 'Unknown error'];
+			$this->logger->error('Unknown download error', ['exception' => $e]);
+			return ['error' => 'Unknown download error'];
 		}
 	}
 }