Repository navigation
feat: enhance import logging #3510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e3dc968
3e5da93
30880c0
ab1434e
873bfb7
9389b3a
b6befe4
a17895b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| namespace OCA\Cookbook\Controller\Implementation; | ||
|
|
||
| use Exception; | ||
| use OCA\Cookbook\Exception\ImportException; | ||
| use OCA\Cookbook\Exception\InvalidDownloadURLException; | ||
| use OCA\Cookbook\Exception\InvalidJSONFileException; | ||
| use OCA\Cookbook\Exception\NoRecipeNameGivenException; | ||
|
|
@@ -15,6 +16,7 @@ | |
| use OCA\Cookbook\Helper\Filter\Output\RecipeJSONOutputFilter; | ||
| use OCA\Cookbook\Helper\Filter\Output\RecipeStubFilter; | ||
| use OCA\Cookbook\Helper\RestParameterParser; | ||
| use OCA\Cookbook\Helper\UrlRedactionHelper; | ||
| use OCA\Cookbook\Service\DbCacheService; | ||
| use OCA\Cookbook\Service\RecipeService; | ||
| use OCP\AppFramework\Http; | ||
|
|
@@ -124,7 +126,7 @@ | |
| * | ||
| * @param $id The id of the recipe in question | ||
| * @return JSONResponse | ||
| * @todo Parameter id is never used. Fix that | ||
|
Check warning on line 129 in lib/Controller/Implementation/RecipeImplementation.php
|
||
| */ | ||
| public function update($id) { | ||
| $this->dbCacheService->triggerCheck(); | ||
|
|
@@ -274,7 +276,7 @@ | |
| } | ||
|
|
||
| /** | ||
| * @todo Move this code into a dedicated class and service | ||
|
Check warning on line 279 in lib/Controller/Implementation/RecipeImplementation.php
|
||
| */ | ||
| private function isDownloadUrlValid(string $url): bool { | ||
| $urlValid = filter_var($url, FILTER_VALIDATE_URL); | ||
|
|
@@ -352,7 +354,15 @@ | |
| ]; | ||
|
|
||
| return new JSONResponse($json, Http::STATUS_CONFLICT); | ||
| } catch (ImportException $e) { | ||
| $this->logger->warning('Import failed at step "{step}": {reason}', [ | ||
| 'url' => UrlRedactionHelper::redact($data['url']), 'step' => $e->getStep(), 'reason' => $e->getMessage(), 'exception' => $e, | ||
| ]); | ||
| return new JSONResponse($e->getMessage(), 400); | ||
| } catch (\Exception $e) { | ||
| $this->logger->error('Unexpected error during import: {reason}', [ | ||
| 'url' => UrlRedactionHelper::redact($data['url']), 'reason' => $e->getMessage(), 'exception' => $e, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Url and exception seem not used, isn't it? |
||
| ]); | ||
| return new JSONResponse($e->getMessage(), 400); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,10 @@ | |
| namespace OCA\Cookbook\Helper\HTMLParser; | ||
|
|
||
| use OCA\Cookbook\Exception\HtmlParsingException; | ||
| use OCA\Cookbook\Helper\UrlRedactionHelper; | ||
| use OCA\Cookbook\Service\JsonService; | ||
| use OCP\IL10N; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| /** | ||
| * This class is an AbstractHtmlParser which tries to extract a JSON+LD script from the HTML page. | ||
|
|
@@ -20,8 +22,8 @@ | |
| */ | ||
| private $jsonService; | ||
|
|
||
| public function __construct(IL10N $l10n, JsonService $jsonService) { | ||
| parent::__construct($l10n); | ||
| public function __construct(IL10N $l10n, LoggerInterface $logger, JsonService $jsonService) { | ||
| parent::__construct($l10n, $logger); | ||
|
|
||
| $this->jsonService = $jsonService; | ||
| } | ||
|
|
@@ -32,6 +34,10 @@ | |
|
|
||
| $json_ld_elements = $xpath->query("//*[@type='application/ld+json']"); | ||
|
|
||
| $this->logger->debug('JSON-LD parser found {count} ld+json elements for {url}', [ | ||
| 'count' => $json_ld_elements->length, 'url' => UrlRedactionHelper::redact($url), | ||
| ]); | ||
|
|
||
| foreach ($json_ld_elements as $json_ld_element) { | ||
| if (!$json_ld_element || !$json_ld_element->nodeValue) { | ||
| continue; | ||
|
|
@@ -40,7 +46,9 @@ | |
| try { | ||
| return $this->parseJsonLdElement($json_ld_element); | ||
| } catch (HtmlParsingException $ex) { | ||
| // Parsing failed for this element. Let's see if there are more... | ||
| $this->logger->debug('JSON-LD element did not contain a valid recipe: {reason}', [ | ||
| 'reason' => $ex->getMessage(), | ||
| ]); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -62,6 +70,7 @@ | |
| $json = json_decode($string, true); | ||
|
|
||
| if ($json === null) { | ||
| $this->logger->debug('JSON-LD decode failed: {error}', ['error' => json_last_error_msg()]); | ||
| throw new HtmlParsingException($this->l->t('JSON cannot be decoded.')); | ||
| } | ||
|
|
||
|
|
@@ -120,7 +129,7 @@ | |
| * Some sites use the @graph property to define elements. | ||
| * This is a quick workaround to extract the corresponding recipe. | ||
| * | ||
| * @todo This only extracts the very first recipe in the graph and only that. | ||
|
Check warning on line 132 in lib/Helper/HTMLParser/HttpJsonLdParser.php
|
||
| * It might be favorable to look further into the json objects. | ||
| * This might especially be true when the recipe uses links to external JSON objects | ||
| * (as specified by the standard). | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,14 +12,16 @@ | |
| use DOMNodeList; | ||
| use DOMXPath; | ||
| use OCA\Cookbook\Exception\HtmlParsingException; | ||
| use OCA\Cookbook\Helper\UrlRedactionHelper; | ||
| use OCP\IL10N; | ||
| use Psr\Log\LoggerInterface; | ||
|
|
||
| /** | ||
| * This class is an AbstractHtmlParser which tries to extract micro data from the HTML page. | ||
| * @author Christian Wolf | ||
| * @todo Nutrition data is missing | ||
|
Check warning on line 22 in lib/Helper/HTMLParser/HttpMicrodataParser.php
|
||
| * @todo Category needs checking | ||
|
Check warning on line 23 in lib/Helper/HTMLParser/HttpMicrodataParser.php
|
||
| * @todo Tools need to be imported | ||
|
Check warning on line 24 in lib/Helper/HTMLParser/HttpMicrodataParser.php
|
||
| */ | ||
| class HttpMicrodataParser extends AbstractHtmlParser { | ||
| /** | ||
|
|
@@ -32,8 +34,8 @@ | |
| */ | ||
| private $recipe; | ||
|
|
||
| public function __construct(IL10N $l10n) { | ||
| parent::__construct($l10n); | ||
| public function __construct(IL10N $l10n, LoggerInterface $logger) { | ||
| parent::__construct($l10n, $logger); | ||
| } | ||
|
|
||
| #[\Override] | ||
|
|
@@ -49,9 +51,14 @@ | |
| $recipes = $this->xpath->query($xpathSelector); | ||
|
|
||
| if (count($recipes) === 0) { | ||
| $this->logger->info('Microdata parser found no Recipe itemscope elements for {url}', ['url' => UrlRedactionHelper::redact($url)]); | ||
| throw new HtmlParsingException($this->l->t('No recipe was found.')); | ||
| } | ||
|
|
||
| $this->logger->debug('Microdata parser found {count} Recipe itemscope elements for {url}', [ | ||
| 'count' => count($recipes), 'url' => UrlRedactionHelper::redact($url), | ||
| ]); | ||
|
|
||
| $this->recipe = [ | ||
| '@type' => 'Recipe', | ||
| '@context' => 'http://schema.org' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <?php | ||
|
|
||
| // SPDX-FileCopyrightText: 2026 Nextcloud cookbook contributors | ||
| // | ||
| // SPDX-License-Identifier: AGPL-3.0-only OR AGPL-3.0-or-later | ||
|
|
||
| namespace OCA\Cookbook\Helper; | ||
|
|
||
| /** | ||
| * Strips credentials, query and fragment from URLs before they are logged. | ||
| */ | ||
| class UrlRedactionHelper { | ||
| public static function redact(?string $url): string { | ||
| $parts = parse_url($url ?? ''); | ||
|
|
||
| if ($parts === false || !isset($parts['host'])) { | ||
| return '[invalid URL]'; | ||
| } | ||
|
|
||
| $scheme = isset($parts['scheme']) ? $parts['scheme'] . '://' : ''; | ||
| $port = isset($parts['port']) ? ':' . $parts['port'] : ''; | ||
|
|
||
| return $scheme . $parts['host'] . $port . ($parts['path'] ?? ''); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you put more entries into the array of snippet (second argument)?