Skip to content
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

Refactor mwApiPath (to mwActionApiPath), mwRestApiPath, mwModulePath and mwWikiPath parameters + fix regression of test render template #1939

Merged
merged 14 commits into from
Nov 13, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename mwApiPath to mwActionApiPath, add e2e tests for mw api path pa…
…rams
  • Loading branch information
VadimKovalenkoSNF committed Nov 10, 2023
commit e6078f1c967610ada4a67b7e15af780709ed1640
10 changes: 5 additions & 5 deletions src/Downloader.ts
Original file line number Diff line number Diff line change
@@ -108,7 +108,7 @@ class Downloader {
this.optimisationCacheUrl = optimisationCacheUrl
this.webp = webp
this.s3 = s3
this.apiUrlDirector = new ApiURLDirector(MediaWiki.apiUrl.href)
this.apiUrlDirector = new ApiURLDirector(MediaWiki.actionApiUrl.href)

this.backoffOptions = {
strategy: new backoff.ExponentialStrategy(),
@@ -176,13 +176,13 @@ class Downloader {
this.baseUrl = basicURLDirector.buildDownloaderBaseUrl([
{ condition: await MediaWiki.hasWikimediaMobileApi(), value: MediaWiki.WikimediaMobileApiUrl.href },
{ condition: await MediaWiki.hasWikimediaDesktopApi(), value: MediaWiki.WikimediaDesktopApiUrl.href },
// { condition: await MediaWiki.hasVisualEditorApi(), value: MediaWiki.visualEditorApiUrl.href },
{ condition: await MediaWiki.hasVisualEditorApi(), value: MediaWiki.visualEditorApiUrl.href },
])

//* Objects order in array matters!
this.baseUrlForMainPage = basicURLDirector.buildDownloaderBaseUrl([
{ condition: await MediaWiki.hasWikimediaDesktopApi(), value: MediaWiki.WikimediaDesktopApiUrl.href },
// { condition: await MediaWiki.hasVisualEditorApi(), value: MediaWiki.visualEditorApiUrl.href },
{ condition: await MediaWiki.hasVisualEditorApi(), value: MediaWiki.visualEditorApiUrl.href },
{ condition: await MediaWiki.hasWikimediaMobileApi(), value: MediaWiki.WikimediaMobileApiUrl.href },
])
} else {
@@ -621,7 +621,7 @@ class Downloader {
}

private async getSubCategories(articleId: string, continueStr = ''): Promise<Array<{ pageid: number; ns: number; title: string }>> {
const apiUrlDirector = new ApiURLDirector(MediaWiki.apiUrl.href)
const apiUrlDirector = new ApiURLDirector(MediaWiki.actionApiUrl.href)

const { query, continue: cont } = await this.getJSON<any>(apiUrlDirector.buildSubCategoriesURL(articleId, continueStr))
const items = query.categorymembers.filter((a: any) => a && a.title)
@@ -652,7 +652,7 @@ class Downloader {
let jsDependenciesList: string[] = []
let styleDependenciesList: string[] = []

const apiUrlDirector = new ApiURLDirector(MediaWiki.apiUrl.href)
const apiUrlDirector = new ApiURLDirector(MediaWiki.actionApiUrl.href)

const articleApiUrl = apiUrlDirector.buildArticleApiURL(title)

26 changes: 14 additions & 12 deletions src/MediaWiki.ts
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ class MediaWiki {
public queryOpts: QueryOpts

#wikiPath: string
#apiPath: string
#actionApiPath: string
#restApiPath: string
#modulePathOpt: string
#username: string
@@ -57,7 +57,7 @@ class MediaWiki {
private visualEditorURLDirector: VisualEditorURLDirector

public visualEditorApiUrl: URL
public apiUrl: URL
public actionApiUrl: URL
public modulePath: string // only for reading
public mobileModulePath: string
public webUrl: URL
@@ -77,9 +77,9 @@ class MediaWiki {
this.#password = value
}

set apiPath(value: string) {
set actionApiPath(value: string) {
if (value) {
this.#apiPath = value
this.#actionApiPath = value
this.initApiURLDirector()
}
}
@@ -96,7 +96,8 @@ class MediaWiki {
}

set wikiPath(value: string) {
if (value) {
// Empty string is a valid parameter for the wikiPath
if (value !== null || value !== undefined) {
this.#wikiPath = value
this.initApiURLDirector()
}
@@ -132,7 +133,7 @@ class MediaWiki {
this.#password = ''
this.getCategories = false

this.#apiPath = 'w/api.php'
this.#actionApiPath = 'w/api.php'
this.#restApiPath = 'api/rest_v1'
this.#wikiPath = 'wiki/'
this.#modulePathOpt = 'w/load.php'
@@ -215,15 +216,15 @@ class MediaWiki {
private initApiURLDirector() {
// TODO: this.webUrl probably shouldn't accept hardcoded 'wiki/' param, check test/e2e/extra.e2e.test.ts
this.webUrl = this.baseUrlDirector.buildURL('wiki/')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, here the mwWikiPath value should be taken

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored this. Also, 'wiki/' value is default again.

this.apiUrl = this.baseUrlDirector.buildURL(this.#apiPath, this.#wikiPath)
this.apiUrlDirector = new ApiURLDirector(this.apiUrl.href)
this.actionApiUrl = this.baseUrlDirector.buildURL(this.#actionApiPath, this.#wikiPath)
this.apiUrlDirector = new ApiURLDirector(this.actionApiUrl.href)
this.visualEditorApiUrl = this.apiUrlDirector.buildVisualEditorURL()
this.visualEditorURLDirector = new VisualEditorURLDirector(this.visualEditorApiUrl.href)
}

public async login(downloader: Downloader) {
if (this.#username && this.#password) {
let url = this.apiUrl.href + '?'
let url = this.actionApiUrl.href + '?'

// Add domain if configured
if (this.#domain) {
@@ -234,7 +235,7 @@ class MediaWiki {
const { content, responseHeaders } = await downloader.downloadContent(url + 'action=query&meta=tokens&type=login&format=json&formatversion=2')

// Logging in
await axios(this.apiUrl.href, {
await axios(this.actionApiUrl.href, {
data: qs.stringify({
action: 'login',
format: 'json',
@@ -442,13 +443,14 @@ class MediaWiki {

const mwMetaData: MWMetaData = {
webUrl: this.webUrl.href,
apiUrl: this.apiUrl.href,
actionApiUrl: this.actionApiUrl.href,
modulePathOpt: this.#modulePathOpt,
modulePath: this.modulePath,
mobileModulePath: this.mobileModulePath,
webUrlPath: this.webUrl.pathname,
wikiPath: this.#wikiPath,
baseUrl: this.baseUrl.href,
apiPath: this.#apiPath,
actionApiPath: this.#actionApiPath,
restApiPath: this.#restApiPath,
domain: this.#domain,

8 changes: 4 additions & 4 deletions src/mwoffliner.lib.ts
Original file line number Diff line number Diff line change
@@ -75,7 +75,7 @@ async function execute(argv: any) {
keepEmptyParagraphs,
mwUrl,
mwWikiPath,
mwApiPath,
mwActionApiPath,
mwRestApiPath,
mwModulePath,
mwDomain,
@@ -158,13 +158,13 @@ async function execute(argv: any) {
/* Wikipedia/... URL; Normalize by adding trailing / as necessary */
MediaWiki.base = mwUrl
MediaWiki.getCategories = !!argv.getCategories
MediaWiki.apiPath = mwApiPath
MediaWiki.wikiPath = mwWikiPath
MediaWiki.actionApiPath = mwActionApiPath
MediaWiki.restApiPath = mwRestApiPath
MediaWiki.modulePathOpt = mwModulePath
MediaWiki.domain = mwDomain
MediaWiki.password = mwPassword
MediaWiki.username = mwUsername
MediaWiki.wikiPath = mwWikiPath

/* Download helpers; TODO: Merge with something else / expand this. */
const downloader = new Downloader({
@@ -513,7 +513,7 @@ async function execute(argv: any) {
}
}

const apiUrlDirector = new ApiURLDirector(MediaWiki.apiUrl.href)
const apiUrlDirector = new ApiURLDirector(MediaWiki.actionApiUrl.href)

const body = await downloader.getJSON<any>(apiUrlDirector.buildSiteInfoURL())

2 changes: 1 addition & 1 deletion src/parameterList.ts
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@ export const parameterDescriptions = {
'Specify a flavour for the scraping. If missing, scrape all article contents. Each --format argument will cause a new local file to be created but options can be combined. Supported options are:\n * novid: no video & audio content\n * nopic: no pictures (implies "novid")\n * nopdf: no PDF files\n * nodet: only the first/head paragraph (implies "novid")\nFormat names can also be aliased using a ":"\nExample: "... --format=nopic:mini --format=novid,nopdf"',
keepEmptyParagraphs: 'Keep all paragraphs, even empty ones.',
mwWikiPath: 'Mediawiki wiki base path (per default "/wiki/")',
mwApiPath: 'Mediawiki API path (per default "/w/api.php")',
mwActionApiPath: 'Mediawiki API path (per default "/w/api.php")',
mwRestApiPath: 'Mediawiki Rest API path (per default "/api/rest_v1")',
mwModulePath: 'Mediawiki module load path (per default "/w/load.php")',
mwDomain: 'Mediawiki user domain (thought for private wikis)',
38 changes: 38 additions & 0 deletions src/sanitize-argument.ts
Original file line number Diff line number Diff line change
@@ -31,6 +31,10 @@ export async function sanitize_all(argv: any) {
customZimLongDescription,
customZimDescription,
forceRender,
mwWikiPath,
mwActionApiPath,
mwRestApiPath,
mwModulePath,
} = argv

sanitizeDoubleUsedParameters(argv)
@@ -78,6 +82,18 @@ export async function sanitize_all(argv: any) {
throw err
})

// sanitizing mwWikiPath
sanitizeWikiPath(mwWikiPath)

// sanitizing mwRestApiPath
sanitizeApiPathParam(mwRestApiPath)

// sanitizing mwActionApiPath
sanitizeApiPathParam(mwActionApiPath)

// sanitizing mwModulePath
sanitizeApiPathParam(mwModulePath)

// sanitize Custom Main Page
if (argv.customMainPage) {
argv.customMainPage = argv.customMainPage.replace(/ /g, '_')
@@ -103,6 +119,28 @@ export async function sanitize_all(argv: any) {
}
}

export function sanitizeWikiPath(mwWikiPath = '') {
mwWikiPath = sanitizeApiPathParam(mwWikiPath)

if (mwWikiPath?.endsWith('/')) {
mwWikiPath += '/'
}

return mwWikiPath
}

export function sanitizeApiPathParam(apiPathParam: string) {
if (!apiPathParam) {
return
}

if (apiPathParam.startsWith('/')) {
apiPathParam = apiPathParam.slice(1)
}

return apiPathParam
}

export function sanitizeStringMaxLength(text: string, key: string, length: number) {
if (text && text.length > length) {
throw new Error(`${key} should be less than ${length} characters.`)
7 changes: 4 additions & 3 deletions src/types.d.ts
Original file line number Diff line number Diff line change
@@ -159,13 +159,14 @@ interface MWMetaData {

baseUrl: string
wikiPath: string
apiPath: string
actionApiPath: string
restApiPath: string
domain: string
webUrl: string
apiUrl: string
actionApiUrl: string
webUrlPath: string
modulePath: string
modulePathOpt: string
mobileModulePath: string
}

@@ -180,7 +181,7 @@ interface MWNamespaces {
interface MWConfig {
base: string
wikiPath?: string
apiPath?: string
actionApiPath?: string
domain?: string
username?: string
password?: string
9 changes: 0 additions & 9 deletions src/util/builders/url/base.director.ts
Original file line number Diff line number Diff line change
@@ -11,11 +11,6 @@ export default class BaseURLDirector {
}

buildURL(path: string, wikiPath = '') {
path = this.stripLeadingSlash(path)
wikiPath = this.stripLeadingSlash(wikiPath)
if (wikiPath && !wikiPath.endsWith('/')) {
wikiPath += '/'
}
path = `${wikiPath}${path}`
return urlBuilder.setDomain(this.baseDomain).setPath(path).build(true)
}
@@ -54,8 +49,4 @@ export default class BaseURLDirector {
.setPath(path ?? 'api/rest_v1/page/mobile-html-offline-resources')
.build(false, '/')
}

private stripLeadingSlash(s) {
return s?.startsWith('/') ? s.slice(1) : s
}
}
4 changes: 1 addition & 3 deletions src/util/mw-api.ts
Original file line number Diff line number Diff line change
@@ -260,9 +260,7 @@ export function mwRetToArticleDetail(obj: QueryMwRet): KVS<ArticleDetail> {
export async function checkApiAvailability(url: string, loginCookie = ''): Promise<boolean> {
try {
const resp = await axios.get(decodeURI(url), { maxRedirects: 0, headers: { cookie: loginCookie } })
// TODO: Enable back once regression Phabricator:T350117 fixed
// return resp.status === 200 && !resp.headers['mediawiki-api-error']
return resp.status === 200
return resp.status === 200 && !resp.headers['mediawiki-api-error']
} catch (err) {
return false
}
51 changes: 51 additions & 0 deletions test/e2e/apiPathParamsSanitizing.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { testAllRenders } from '../testAllRenders.js'
import { zimcheck } from '../util.js'
import 'dotenv/config.js'
import { jest } from '@jest/globals'
import rimraf from 'rimraf'
import { sanitizeApiPathParam, sanitizeWikiPath } from '../../src/sanitize-argument.js'

jest.setTimeout(60000)

const parameters = {
mwUrl: 'https://en.wikipedia.org',
articleList: 'BMW',
adminEmail: 'test@kiwix.org',
mwActionApiPath: sanitizeApiPathParam('/w/api.php'),
mwRestApiPath: sanitizeApiPathParam('/api/rest_v1'),
mwModulePath: sanitizeApiPathParam('/w/load.php'),
mwWikiPath: sanitizeWikiPath('/'),
}

await testAllRenders(parameters, async (outFiles) => {
describe(`e2e test for api url params for en.wikipedia.org for ${outFiles[0]?.renderer} renderer`, () => {
test('Mediawiki actionApiPath ', () => {
expect(outFiles[0].mwMetaData.actionApiPath).toBe('w/api.php')
})

test('Mediawiki restApiPath option', () => {
expect(outFiles[0].mwMetaData.restApiPath).toBe('api/rest_v1')
})

test('Mediawiki wikiPath option', () => {
expect(outFiles[0].mwMetaData.wikiPath).toBe('')
})

test('Mediawiki modulePathOpt option', () => {
expect(outFiles[0].mwMetaData.modulePathOpt).toBe('w/load.php')
})

test('Mediawiki modulePath and actionApiUrl options', () => {
expect(outFiles[0].mwMetaData.modulePath).toBe('https://en.wikipedia.org/w/load.php?')
expect(outFiles[0].mwMetaData.actionApiUrl).toBe('https://en.wikipedia.org/w/api.php')
})

test(`test zim integrity for ${outFiles[0]?.renderer} renderer`, async () => {
await expect(zimcheck(outFiles[0].outFile)).resolves.not.toThrowError()
})

afterAll(() => {
rimraf.sync(`./${outFiles[0].testId}`)
})
})
})
4 changes: 1 addition & 3 deletions test/e2e/articleLists.test.ts
Original file line number Diff line number Diff line change
@@ -16,14 +16,12 @@ const parameters = {
articleListToIgnore,
redis: process.env.REDIS,
format: ['nopic'],
mwApiPath: 'w/api.php',
mwActionApiPath: 'w/api.php',
mwWikiPath: '/',
}

await testAllRenders(parameters, async (outFiles) => {
describe('articleList', () => {
const now = new Date()
const testId = `mwo-test-${+now}`
const listMinusIgnore = 2

test(`articleList and articleListIgnore check using ${outFiles[0].renderer} renderer`, async () => {
2 changes: 1 addition & 1 deletion test/e2e/bm.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ const parameters = {
adminEmail: 'test@kiwix.org',
redis: process.env.REDIS,
format: ['nopic'],
mwApiPath: 'w/api.php',
mwActionApiPath: 'w/api.php',
mwWikiPath: '/',
}

2 changes: 1 addition & 1 deletion test/e2e/downloadImage.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ const parameters = {
articleList: 'Paris',
format: ['nodet'],
optimisationCacheUrl: process.env.S3_URL,
mwApiPath: 'w/api.php',
mwActionApiPath: 'w/api.php',
mwWikiPath: '/',
}

2 changes: 1 addition & 1 deletion test/e2e/en.e2e.test.ts
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ const parameters = {
mwUrl: 'https://en.wikipedia.org',
articleList: 'BMW',
adminEmail: 'test@kiwix.org',
mwApiPath: 'w/api.php',
mwActionApiPath: 'w/api.php',
mwWikiPath: '/',
}

Loading