mwoffliner
mwoffliner copied to clipboard
Remove MWCapabilities
MWCapabilities
is a dedicated class instanciated within Downloader
. This is not the right place. This should be available in the Mediawiki
class.
I want to have these 4 method, implemented as lazy in Mediawiki
:
-
hasVeApi()
-
hasCoordinatesApi()
-
hasDesktopRestApi()
-
hasMobileRestApi()
Probably the a lazy method should be as well implemented to get the MainPage:
-
getMainPage()
Remark: This is a necessary step to re-architecture the Mediawiki-Downloader-Renderer
This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.
@uriesk @pavel-karatsiuba IMO one of girst things to do in 1.13.0 before implementing support of any new backend. Do you agree? Have other comments?
I agree.
And currently we getArticleUrl
by appending the articleId to a baseUrl (which got choosen based on the Capabilities).
But not all APIs will always allow us to do that.
I would like a concept similar to this:
const apis = {
'ACTION': {
getArticleUrl: (articleId) => {
return `${baseUrl.href}action=visualeditor&mobileformat=html&format=json&paction=parse&page=${encodeURIComponent(articleId)}`;
},
},
'RESTV1': {
getArticleUrl: (articleId) => {
return `${baseUrl.href}/api/rest_v1/${encodeURIComponent(articleId)}/html`;
},
},
}
const choosenEndpoint = apis['ACTION'];
const url = choosenEndpoint.getArticleUrl('Article_Title');
@uriesk I think we are on the same wave length! Before building any new API endpoint support, we need first to reorganise the current modules in a way content endpoints calls an response treatment are modularised, ie. share AFAP a common interface/API for the rest of MWoffliner.
That way it will be easier to understand an test.
I want to open a dedicated ticket for that for a long time but actually I'm not sure how the fix should exactly look like... so I have open only this small one which was more obvious to me.
@pavel-karatsiuba Here we will need a rearchitecturing proposal from you.
@kelson42
Your proposed changes are a good and easy step to start changing architecture.
If we start from these changes then makes sense to decompose whall Downloader class and leave here only the methods needed to download content. And other methods need to be moved to other places.
Method getCompressedBody
which uses imagemin
should be separated. Methods to serialize and deserialize Url can be separated into another class.
@pavel-karatsiuba OK, but what I ask is a full proposal so I can figure out how it will look like at the end.
@kelson42 I have made a picture with propositions to change Downloader.ts
and redesign the file structure.
All changes are directed to the logical grouping the similar functions by modules. In the future, each group should be covered by tests. And tests also should be grouped.
https://drive.google.com/file/d/1Fncde4_gzHPol-jAWVlQALHx1XLmZ1m6/view?usp=sharing
@pavel-karatsiuba Where are the differents modules corresponding to the differrent mediawiki endpoints mwoffliner can deal with to retrieve the articles content?
Right now all Wikimedia uses the same endpoint and uses the same API. Before designing the classes structure for this situation, I need to know which parts will be different. So the next part of designing needs to have a description.
Right now all Wikimedia uses the same endpoint and uses the same API.
I don't know what a "Wikimedia" is... and actually we are using AFAIK many different API to retrieves the HTML (see function checkCapabilities()
. We want to:
- deprecate one and introduce a new one with https://github.com/openzim/mwoffliner/issues/1664
- introduce another one https://github.com/openzim/mwoffliner/issues/1601
I think you have enough information to design a common interface for all of them, so the rest of MWoffliner can just decide which module to deal with but wihtout having to change the rest of the code deal with the module.
We are using mwCapabilities
to define how will be downloaded and rendered article.
We need to create the Article
module. In this module, we need to have the function to set the apiURL
, because apiURL
depends on mwCapabilities
. Also, this module should have the function setArticleRenderer()
which should contain logic to select the article renderer function depending on mwCapabilities
.
Finally, the Article module should have getArticle()
and renderArticle()
functions which should use apiURL
and then articleRenderer
.
Example what should be in the Article module:
let articleRenderer: ArticleRenderer; // function to render the Article
let apiUrl; // URL to Wikimedia API which depends on mwCapabilities
export const setApiUrl = () => {
// set apiUrl depends on mwCapabilities
// use logic similar to Downloader.checkCapabilities() + Downloader.setBaseUrls()
}
export const setArticleRenderer= () => {
// set articleRenderer depends on mwCapabilities
}
export const getArticle= () => {
// use apiUrl to download Article Content
// execute renderArticle()
}
const renderArticle= () => {
// use articleRenderer to render downloaded Article
}
Different articleRenderer functions should be placed in the ArticleRenderer module.
@pavel-karatsiuba Your last comment goes against the task description and Mediawiki properties should not be part of an Article
class (but a Mediawiki
class).