Add more logging to debug easier any issues

.changelog/current/2330-debug-logging-in-db-service-worker.md

@@ -0,0 +1,3 @@
+# Fixed
+
+- Added more verbose error logs in NC logs in case of problems during parsing

lib/Controller/Implementation/RecipeImplementation.php

@@ -2,6 +2,8 @@
 
 namespace OCA\Cookbook\Controller\Implementation;
 
+use Exception;
+use OCA\Cookbook\Exception\InvalidJSONFileException;
 use OCA\Cookbook\Exception\NoRecipeNameGivenException;
 use OCA\Cookbook\Exception\RecipeExistsException;
 use OCA\Cookbook\Helper\AcceptHeaderParsingHelper;
@@ -12,11 +14,13 @@
 use OCA\Cookbook\Service\RecipeService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataDisplayResponse;
+use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\FileDisplayResponse;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IURLGenerator;
+use Psr\Log\LoggerInterface;
 
 class RecipeImplementation {
 	/** @var RecipeService */
@@ -37,6 +41,8 @@ class RecipeImplementation {
 	private $request;
 	/** @var IL10N */
 	private $l;
+	/** @var LoggerInterface */
+	private $logger;
 
 
 	public function __construct(
@@ -48,7 +54,8 @@ public function __construct(
 		RecipeJSONOutputFilter $recipeJSONOutputFilter,
 		RecipeStubFilter $stubFilter,
 		AcceptHeaderParsingHelper $acceptHeaderParsingHelper,
-		IL10N $iL10N
+		IL10N $iL10N,
+		LoggerInterface $logger
 	) {
 		$this->request = $request;
 		$this->service = $recipeService;
@@ -59,6 +66,7 @@ public function __construct(
 		$this->stubFilter = $stubFilter;
 		$this->acceptHeaderParser = $acceptHeaderParsingHelper;
 		$this->l = $iL10N;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -132,7 +140,24 @@ public function update($id) {
 			];
 			return new JSONResponse($json, Http::STATUS_UNPROCESSABLE_ENTITY);
 		}
-		$this->dbCacheService->addRecipe($file);
+
+		try {
+			$this->dbCacheService->addRecipe($file);
+		} catch(InvalidJSONFileException $ex) {
+
+			try {
+				$this->service->deleteRecipe($file->getParent()->getId());
+			} catch (Exception $ex) {
+				$this->logger->warning('Cannot remove file after failed parsing: {ex}', ['ex' => $ex]);
+			}
+
+			$json = [
+				'msg' => $ex->getMessage(),
+				'file' => $ex->getFile(),
+				'line' => $ex->getLine(),
+			];
+			return new JSONResponse($json, Http::STATUS_UNPROCESSABLE_ENTITY);
+		}
 
 		return new JSONResponse($file->getParent()->getId(), Http::STATUS_OK);
 	}
@@ -148,9 +173,6 @@ public function create() {
 		$recipeData = $this->restParser->getParameters();
 		try {
 			$file = $this->service->addRecipe($recipeData);
-			$this->dbCacheService->addRecipe($file);
-
-			return new JSONResponse($file->getParent()->getId(), Http::STATUS_OK);
 		} catch (RecipeExistsException $ex) {
 			$json = [
 				'msg' => $ex->getMessage(),
@@ -166,6 +188,26 @@ public function create() {
 			];
 			return new JSONResponse($json, Http::STATUS_UNPROCESSABLE_ENTITY);
 		}
+
+		try {
+			$this->dbCacheService->addRecipe($file);
+		} catch(InvalidJSONFileException $ex) {
+
+			try {
+				$this->service->deleteRecipe($file->getParent()->getId());
+			} catch (Exception $ex) {
+				$this->logger->warning('Cannot remove file after failed parsing: {ex}', ['ex' => $ex]);
+			}
+
+			$json = [
+				'msg' => $ex->getMessage(),
+				'file' => $ex->getFile(),
+				'line' => $ex->getLine(),
+			];
+			return new JSONResponse($json, Http::STATUS_UNPROCESSABLE_ENTITY);
+		}
+
+		return new JSONResponse($file->getParent()->getId(), Http::STATUS_OK);
 	}
 
 	/**
@@ -236,9 +278,7 @@ public function import() {
 		try {
 			$recipe_file = $this->service->downloadRecipe($data['url']);
 			$recipe_json = $this->service->parseRecipeFile($recipe_file);
-			$this->dbCacheService->addRecipe($recipe_file);
 
-			return new JSONResponse($recipe_json, Http::STATUS_OK);
 		} catch (RecipeExistsException $ex) {
 			$json = [
 				'msg' => $ex->getMessage(),
@@ -249,6 +289,21 @@ public function import() {
 		} catch (\Exception $e) {
 			return new JSONResponse($e->getMessage(), 400);
 		}
+
+		try {
+			$this->dbCacheService->addRecipe($recipe_file);
+		} catch(InvalidJSONFileException $ex) {
+
+			try {
+				$this->service->deleteRecipe($recipe_file->getParent()->getId());
+			} catch (Exception $ex) {
+				$this->logger->warning('Cannot remove file after failed parsing: {ex}', ['ex' => $ex]);
+			}
+
+			return new DataResponse($ex->getMessage(), 400);
+		}
+
+		return new JSONResponse($recipe_json, Http::STATUS_OK);
 	}
 
 	/**

lib/Service/DbCacheService.php

@@ -2,13 +2,15 @@
 
 namespace OCA\Cookbook\Service;
 
+use Exception;
 use OCA\Cookbook\Db\RecipeDb;
 use OCA\Cookbook\Exception\InvalidJSONFileException;
 use OCA\Cookbook\Helper\Filter\DB\NormalizeRecipeFileFilter;
 use OCA\Cookbook\Helper\UserConfigHelper;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\Files\File;
 use OCP\IL10N;
+use Psr\Log\LoggerInterface;
 
 class DbCacheService {
 	private $userId;
@@ -33,6 +35,8 @@ class DbCacheService {
 	 * @var IL10N
 	 */
 	private $l;
+	/** @var LoggerInterface */
+	private $logger;
 
 	/** @var NormalizeRecipeFileFilter */
 	private $normalizeFileFilter;
@@ -55,14 +59,16 @@ public function __construct(
 		RecipeService $recipeService,
 		UserConfigHelper $userConfigHelper,
 		NormalizeRecipeFileFilter $normalizeRecipeFileFilter,
-		IL10N $l
+		IL10N $l,
+		LoggerInterface $logger
 	) {
 		$this->userId = $UserId;
 		$this->db = $db;
 		$this->recipeService = $recipeService;
 		$this->userConfigHelper = $userConfigHelper;
 		$this->normalizeFileFilter = $normalizeRecipeFileFilter;
 		$this->l = $l;
+		$this->logger = $logger;
 	}
 
 	public function updateCache() {
@@ -79,8 +85,8 @@ public function addRecipe(File $recipeFile) {
 		try {
 			$json = $this->parseJSONFile($recipeFile);
 		} catch (InvalidJSONFileException $e) {
-			// XXX Put a log message and infor the user of problem.
-			return;
+			$this->logger->error('Cannot parse JSON file {file}: {ex}', ['file' => $recipeFile->getPath(), 'ex' => $e]);
+			throw $e;
 		}
 
 		$id = $json['id'];
@@ -123,6 +129,7 @@ private function parseJSONFiles() {
 			try {
 				$json = $this->parseJSONFile($jsonFile);
 			} catch (InvalidJSONFileException $e) {
+				$this->logger->error('Cannot parse file {path}. Skipping it. Please fix manually.', ['path' => $jsonFile->getPath()]);
 				continue;
 			}
 			$id = $json['id'];
@@ -139,8 +146,13 @@ private function parseJSONFiles() {
 	 * @return array
 	 */
 	private function parseJSONFile(File $jsonFile): array {
+		try {
+			$content = $jsonFile->getContent();
+		} catch (Exception $ex) {
+			throw new InvalidJSONFileException($this->l->t('Cannot read content of JSON file %s', [$jsonFile->getPath()]), 0, $ex);
+		}
 		// XXX Export of file reading into library/service?
-		$json = json_decode($jsonFile->getContent(), true);
+		$json = json_decode($content, true);
 
 		if (!$json || !isset($json['name']) || $json['name'] === 'No name') {
 			$id = $jsonFile->getParent()->getId();

tests/Unit/Controller/Implementation/RecipeImplementationTest.php

@@ -22,6 +22,7 @@
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\MockObject\Stub;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
 /**
  * @covers \OCA\Cookbook\Controller\Implementation\RecipeImplementation
@@ -68,6 +69,8 @@ public function setUp(): void {
 		$l = $this->createStub(IL10N::class);
 		$l->method('t')->willReturnArgument(0);
 
+		$logger = $this->createStub(LoggerInterface::class);
+
 		$this->sut = new RecipeImplementation(
 			$this->request,
 			$this->recipeService,
@@ -77,7 +80,8 @@ public function setUp(): void {
 			$this->recipeFilter,
 			$this->stubFilter,
 			$this->acceptHeaderParser,
-			$l
+			$l,
+			$logger
 		);
 	}