Rearchitecture MWOffliner HTML/CSS/JS scraping (part #1) #1839

DonAlexandro · 2023-05-11T11:51:56Z

Github issue

Fixes partly #1830

Overview

16.05.2023

Moved article treatment methods from saveArticle.ts to article.treatment class
Refactored the code with the new class
Fixed unit tests for the new class
Moved media treatment methods from saveArticle.ts to media.treatment class
Refactored the code with the media.treatment class
Fixed unit tests for the media.treatment class

15.05.2023

Completely finished refactoring pieces of code dedicated to specific API endpoints
Fixed all existing unit tests
Fixed and added a couple of new ones for directors

12.05.2023

Finished unit test
Almost finished replacing old code with new directors

11.05.2023

Created directors for all URLs
Updated an initial implementation of URL builder
Studied how to run unit tests on the project and started writing some for builder and its directors

21.08.2023

Remove mw capabilities for desktop and visual rendering in favor of respective lazy methods in MediaWiki class (fixes Remove MWCapabilities #1357).
Convert MediaWiki class into the singleton.
Create renderer builder class to handle rendering between different kind of API (such as Wikimedia Desktop, VisualEditor and placeholder for WikimediaMobile).
Refactored unit tests to handle renderers and MediaWiki singleton.

codecov · 2023-05-11T12:04:11Z

Codecov Report

Patch coverage: 80.35% and project coverage change: +0.29% 🎉

Comparison is base (eb9fc5c) 71.24% compared to head (d949fd8) 71.53%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1839      +/-   ##
==========================================
+ Coverage   71.24%   71.53%   +0.29%     
==========================================
  Files          30       34       +4     
  Lines        2671     2737      +66     
  Branches      595      606      +11     
==========================================
+ Hits         1903     1958      +55     
- Misses        659      667       +8     
- Partials      109      112       +3

Files Changed	Coverage Δ
src/util/builders/url/basic.director.ts	`100.00% <ø> (ø)`
src/util/builders/url/url.builder.ts	`97.43% <ø> (ø)`
src/util/const.ts	`100.00% <ø> (ø)`
src/util/misc.ts	`73.22% <ø> (ø)`
src/util/renderers/renderer.builder.ts	`22.50% <22.50%> (ø)`
src/util/renderers/visual-editor.renderer.ts	`62.85% <62.85%> (ø)`
src/Downloader.ts	`55.71% <64.00%> (-6.91%)`	⬇️
src/util/saveArticles.ts	`78.31% <66.66%> (-3.81%)`	⬇️
src/util/rewriteUrls.ts	`83.10% <72.72%> (ø)`
src/util/renderers/wikimedia-desktop.renderer.ts	`83.33% <83.33%> (ø)`
... and 8 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kelson42

@DonAlexandro I had really little time to look at your work still. I'm pretty convinced by your approach around URL and I like many things around that part. Therefore I think we could merge your work arounds URLs pretty quickly. For the rest, this is more complex and I'll have a look tomorrow morning. Could you please isolate all the URL stuff in a dedicated PR, so we can polish and merge?

src/Downloader.ts

src/util/builders/url/api.director.ts

src/Downloader.ts

src/MediaWiki.ts

VadimKovalenkoSNF

Overall looks OK, I applied some fixes related to MCS decommission and unit test tweaks on top of PR #1854

src/util/renderers/article.renderer.ts

test/unit/builders/url/mobile.director.test.ts

src/util/saveArticles.ts

test/unit/downloader.test.ts

src/Downloader.ts

kelson42

At a first look, this PR does not tacle #1830:

I see almost no architectural changes beside the creation of the URL helper. Lot of code has been moved around, so I might miss something.
Point "No clear separation between the pieces of code dedicated to specific API end-points" is not fixed
Point "No common interface to use in the same way a module dealing with end-point number 1 or end-point number 2 " is not implemented
Subticket Remove MWCapabilities #1357 is not done at all

src/Downloader.ts

src/util/renderers/article.renderer.ts

src/util/treatments/article.treatment.ts

kelson42

My two cents

src/util/renderers/article.renderer.ts

src/util/renderers/parsoidHtmlRestApi.renderer.ts

kelson42 · 2023-08-11T13:34:37Z

src/util/renderers/abstract.renderer.ts

@@ -0,0 +1,3 @@
+export abstract class Renderer {
+  abstract render(renderOpts: any): Promise<any>


We should have a strict interface renderOpts: any this is really too error prone.

src/util/saveArticles.ts

src/util/renderers/renderer.builder.ts

src/util/saveArticles.ts

kelson42

Still many oddities which looks strange to me. We definitly need to take a moment together.

src/Downloader.ts

src/MediaWiki.ts

src/mwoffliner.lib.ts

src/util/renderers/renderer.builder.ts

src/util/renderers/visual-editor.renderer.ts

kelson42

Do we have open tickets that this PR fixes fully? If "yes" description should be updated with Fixes #xyz.

src/Downloader.ts

src/MediaWiki.ts

src/util/renderers/renderer.builder.ts

…ondition

….e2e.test.ts

…ct renderer

kelson42

Finally LGTM

DonAlexandro requested a review from kelson42 May 11, 2023 11:51

kelson42 marked this pull request as draft May 11, 2023 11:56

kelson42 force-pushed the feature/1830-rearchitecture branch from f22dea4 to c5e4ae7 Compare May 24, 2023 14:58

kelson42 reviewed May 30, 2023

View reviewed changes

src/Downloader.ts Show resolved Hide resolved

src/util/builders/url/api.director.ts Outdated Show resolved Hide resolved

src/Downloader.ts Outdated Show resolved Hide resolved

src/Downloader.ts Show resolved Hide resolved

src/MediaWiki.ts Outdated Show resolved Hide resolved

DonAlexandro force-pushed the feature/1830-rearchitecture branch from c5e4ae7 to a5ef1bf Compare June 19, 2023 12:39

DonAlexandro marked this pull request as ready for review June 19, 2023 12:40

DonAlexandro marked this pull request as draft June 19, 2023 12:40

kelson42 mentioned this pull request Jul 18, 2023

Use "parsoid" in query string to retrieve CSS/JS dependencies #1846

Closed

kelson42 requested a review from VadimKovalenkoSNF July 28, 2023 13:07

VadimKovalenkoSNF force-pushed the feature/1830-rearchitecture branch 2 times, most recently from a31595d to 4be0b61 Compare July 31, 2023 14:43

VadimKovalenkoSNF reviewed Aug 1, 2023

View reviewed changes

VadimKovalenkoSNF marked this pull request as ready for review August 1, 2023 04:40

VadimKovalenkoSNF force-pushed the feature/1830-rearchitecture branch from 08b8dff to 6a00b54 Compare August 1, 2023 12:05

VadimKovalenkoSNF requested a review from kelson42 August 1, 2023 14:06

kelson42 requested changes Aug 3, 2023

View reviewed changes

src/Downloader.ts Outdated Show resolved Hide resolved

src/util/renderers/article.renderer.ts Outdated Show resolved Hide resolved

src/util/treatments/article.treatment.ts Show resolved Hide resolved

kelson42 reviewed Aug 8, 2023

View reviewed changes

kelson42 changed the title ~~[Feature] Rearchitecture MWOffliner HTML/CSS/JS scraping part~~ Rearchitecture MWOffliner HTML/CSS/JS scraping (part #1) Aug 11, 2023

kelson42 reviewed Aug 11, 2023

View reviewed changes

src/util/saveArticles.ts Outdated Show resolved Hide resolved

kelson42 reviewed Aug 11, 2023

View reviewed changes

src/util/renderers/renderer.builder.ts Outdated Show resolved Hide resolved

kelson42 reviewed Aug 11, 2023

View reviewed changes

src/util/saveArticles.ts Outdated Show resolved Hide resolved

VadimKovalenkoSNF force-pushed the feature/1830-rearchitecture branch from 3c1020f to e368436 Compare August 14, 2023 11:35

This was referenced Aug 15, 2023

Remove MWCapabilities object (partial impl) #1878

Closed

Modularization of article treatment #1881

Closed

VadimKovalenkoSNF requested a review from kelson42 August 15, 2023 11:07

kelson42 requested changes Aug 17, 2023

View reviewed changes

VadimKovalenkoSNF requested a review from kelson42 August 18, 2023 15:05

kelson42 requested changes Aug 19, 2023

View reviewed changes

VadimKovalenkoSNF added 26 commits August 24, 2023 15:21

Increase test timeout for article treatment test

a6cf366

Abstract renderers for Parsoid HTML and Visual Editor (partial impl)

061df37

Minor fixes for abstract renderers and tests (partial impl)

1b76939

Wrap renderers using Builder pattern, refactor Render instantiation

1c63a52

Refactor renderer classes and invocation of getting article

cbe598b

Suppress tests for VE, fix tests related to parsoid html renderer

fd2c47b

Fix Visual Editor renderer

06369dd

Replace mwCapabilities to Mediawiki class (partial impl)

454c0b1

Fix mw capabilities in setting URLs method

a3fb4a5

Add method to check coordination param availability

cac1006

Refactor Mediawiki API capabilities

781dc4a

Temporary remove isMainPage condition while choosing renderers

1dd1930

Document Downloader class

1c264fa

Minor fixes related to naming

6b44200

Make placeholder for WikimediaMobile rendering, refactor isMainPage c…

eb7ec21

…ondition

Create renderer builder

3774efd

Document URLBuilder class

d07d3ae

Add unit tests for the renderer builder

a17a7f6

Fix typo

5e47491

Refactor Mediawiki class to act as a singleton

9fac280

Update error handling for the renderer builder, minor fixes

763328d

Update Downloader class notation

2fc046d

Add reset method to Mediawiki, improve Renderer builder

261a0ef

Add temporary workaround for article treatment to bypass test/e2e/cmd…

85a32d5

….e2e.test.ts

Refactor Renderer builder, replace types and interfaces to the Abstra…

28f26b1

…ct renderer

Move getArticleIds to mw-api, minor fixes

d949fd8

kelson42 force-pushed the feature/1830-rearchitecture branch from 478c1f7 to d949fd8 Compare August 24, 2023 08:21

kelson42 approved these changes Aug 24, 2023

View reviewed changes

kelson42 merged commit 77a064a into main Aug 24, 2023
5 of 6 checks passed

kelson42 deleted the feature/1830-rearchitecture branch August 24, 2023 08:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rearchitecture MWOffliner HTML/CSS/JS scraping (part #1) #1839

Rearchitecture MWOffliner HTML/CSS/JS scraping (part #1) #1839

DonAlexandro commented May 11, 2023 •

edited by VadimKovalenkoSNF

Loading

codecov bot commented May 11, 2023 •

edited

Loading

kelson42 left a comment

VadimKovalenkoSNF left a comment

kelson42 left a comment •

edited

Loading

kelson42 left a comment

kelson42 Aug 11, 2023

VadimKovalenkoSNF Aug 14, 2023

kelson42 left a comment

kelson42 left a comment

kelson42 left a comment

		@@ -0,0 +1,3 @@
		export abstract class Renderer {
		abstract render(renderOpts: any): Promise<any>

Rearchitecture MWOffliner HTML/CSS/JS scraping (part #1) #1839

Rearchitecture MWOffliner HTML/CSS/JS scraping (part #1) #1839

Conversation

DonAlexandro commented May 11, 2023 • edited by VadimKovalenkoSNF Loading

Github issue

Overview

16.05.2023

15.05.2023

12.05.2023

11.05.2023

21.08.2023

codecov bot commented May 11, 2023 • edited Loading

Codecov Report

kelson42 left a comment

Choose a reason for hiding this comment

VadimKovalenkoSNF left a comment

Choose a reason for hiding this comment

kelson42 left a comment • edited Loading

Choose a reason for hiding this comment

kelson42 left a comment

Choose a reason for hiding this comment

kelson42 Aug 11, 2023

Choose a reason for hiding this comment

VadimKovalenkoSNF Aug 14, 2023

Choose a reason for hiding this comment

kelson42 left a comment

Choose a reason for hiding this comment

kelson42 left a comment

Choose a reason for hiding this comment

kelson42 left a comment

Choose a reason for hiding this comment

DonAlexandro commented May 11, 2023 •

edited by VadimKovalenkoSNF

Loading

codecov bot commented May 11, 2023 •

edited

Loading

kelson42 left a comment •

edited

Loading