Extract parsing into its own service class

[christianlupus]

This PR should reduce the complexity of the RecipeService and pull out some of the functionality in smaller chunks that might also simplify debugging.

For now, the recipeParseHtml() method is only deprecated but should be removed soon.

[codecov]

Codecov Report

Merging #553 (dda8eb2) into master (1b97ad5) will increase coverage by 8.39%.
The diff coverage is 94.65%.

❗ Current head dda8eb2 differs from pull request most recent head 8651abd. Consider uploading reports for the commit 8651abd to get more accurate results

@@            Coverage Diff             @@
##           master     #553      +/-   ##
==========================================
+ Coverage   32.26%   40.65%   +8.39%     
==========================================
  Files          32       42      +10     
  Lines        1621     1852     +231     
==========================================
+ Hits          523      753     +230     
- Misses       1098     1099       +1     

Flag	Coverage Δ
integration	4.58% <0.00%> (-0.66%)	⬇️
unittests	36.06% <94.65%> (+9.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/Service/RecipeService.php	0.00% <0.00%> (ø)
lib/Helper/HtmlToDomParser.php	95.16% <95.16%> (ø)
lib/Exception/HtmlParsingException.php	100.00% <100.00%> (ø)
lib/Exception/ImportException.php	100.00% <100.00%> (ø)
lib/Helper/HTMLFilter/HtmlEntityDecodeFilter.php	100.00% <100.00%> (ø)
lib/Helper/HTMLParser/AbstractHtmlParser.php	100.00% <100.00%> (ø)
...b/Helper/HTMLParser/AttributeNotFoundException.php	100.00% <100.00%> (ø)
lib/Helper/HTMLParser/HttpJsonLdParser.php	100.00% <100.00%> (ø)
lib/Helper/HTMLParser/HttpMicrodataParser.php	100.00% <100.00%> (ø)
lib/Service/HtmlDownloadService.php	100.00% <100.00%> (ø)
... and 1 more

[christianlupus]

@seyfeb I tried here to start the codebase refractory and pull some functionality into its own service. I know this is quite a bigger pack of changes which makes it hard to check line by line. What do you think, how should we proceed here? First writing tests for all changes involved here or building the tests later? I tried to make it possible to test the code but did not write the tests yet.

If we want to do this with everything tested, I would have to solve #537 first, I assume.

Any further remarks?

[seyfeb]

I’m not sure either. We might get some regressions when doing this without tests, so this might be a good moment to start writing tests. However, I’m not sure if, with our limited time to work on this, it might just be okay to deal with them later and first get the better structure in the code and hope for the best (keeping in mind to add tests once we have #537).

[christianlupus]

OK, then we wait for #537, rebase to have these changes, and try to implement some tests afterward. I converted to draft to make this obvious.

[christianlupus]

Closes #740

[github-actions]

Unit Test Results

     24 files       24 suites   8m 44s ⏱️
   186 tests    186 ✔️ 0 💤 0 ❌
2 232 runs  2 232 ✔️ 0 💤 0 ❌

Results for commit 8651abd.

♻️ This comment has been updated with latest results.

[christianlupus]

OK, I added tests for all new classes except for the RecipeService. This special class is not yet refactored and is not in a testable shape.

Also, no integration tests were written so far. This is related to the fact that no clear idea of what sites should be taken as examples for these tests.

[seyfeb]

I tested this with a recipe and it worked as expected. I did not go through all the test cases though..

[christianlupus]

I am going to merge this now. This will allow breaking the work into smaller chunks that can be handled more easily. Especially adding more test and integration tests can be done separately. See also #1017.