From 3fedd1b7173e88b156b5e417b9bd59b6659fc145 Mon Sep 17 00:00:00 2001 From: clarakosi Date: Thu, 25 Oct 2018 21:00:33 -0400 Subject: [PATCH 1/2] Added title normalization to wiktionary and tests Modified title in lib/normalize_title_filter.js to check for term in wiktionary before defaulting to title Added tests to check case sensitivty and namespaces in wiktionary Added wiktionary url to test/utils/server.js Modified v1/definition.yaml to register the filter --- lib/normalize_title_filter.js | 1 + test/features/pagecontent/redirects.js | 51 ++++++++++++++++++++++++++ test/utils/server.js | 4 +- v1/definition.yaml | 1 + 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/lib/normalize_title_filter.js b/lib/normalize_title_filter.js index 1a2a4c058..bb7796655 100644 --- a/lib/normalize_title_filter.js +++ b/lib/normalize_title_filter.js @@ -29,6 +29,7 @@ module.exports = (hyper, req, next, options, specInfo) => { } }); } + rp.title = rp[options.title] ? rp[options.title] : rp.title; if (!rp.title) { return next(hyper, req); } diff --git a/test/features/pagecontent/redirects.js b/test/features/pagecontent/redirects.js index 3ddb58e13..2aa4c9010 100644 --- a/test/features/pagecontent/redirects.js +++ b/test/features/pagecontent/redirects.js @@ -9,6 +9,57 @@ describe('redirects', () => { before(() => { return server.start(); }); describe('', () => { + it('should direct to a lowercase version of a title in wiktionary', () => { + return preq.get({ + uri: `${server.config.wiktionaryURI}/page/definition/cat`, + followRedirect: false + }) + .then((res) => { + assert.deepEqual(res.status, 200); + assert.deepEqual(res.headers['content-location'], + 'https://en.wiktionary.org/api/rest_v1/page/definition/cat'); + assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); + }); + }); + + it('should direct to an uppercase version of a title in wiktionary', () => { + return preq.get({ + uri: `${server.config.wiktionaryURI}/page/definition/Cat`, + followRedirect: false + }) + .then((res) => { + assert.deepEqual(res.status, 200); + assert.deepEqual(res.headers['content-location'], + 'https://en.wiktionary.org/api/rest_v1/page/definition/Cat'); + assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); + }); + }); + + it('should redirect to a normalized version of a title in wiktionary', () => { + return preq.get({ + uri: `${server.config.wiktionaryURI}/page/definition/Appendix%20Glossary`, + followRedirect: false + }) + .then((res) => { + assert.deepEqual(res.status, 301); + assert.deepEqual(res.headers.location, + '../../../en.wiktionary.org/v1/page/definition/Appendix%20Glossary'); + assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); + }); + }); + it('should redirect to a normalized version of a title in wiktionary, #2', () => { + return preq.get({ + uri: `${server.config.wiktionaryURI}/page/definition/Wiktionary%20ELE`, + followRedirect: false + }) + .then((res) => { + assert.deepEqual(res.status, 301); + assert.deepEqual(res.headers.location, + '../../../en.wiktionary.org/v1/page/definition/Wiktionary%20ELE'); + assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); + }); + }); + it('should redirect to a normalized version of a title', () => { return preq.get({ uri: `${server.config.bucketURL}/html/Main%20Page?test=mwAQ`, diff --git a/test/utils/server.js b/test/utils/server.js index c65767bcc..124514040 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -18,6 +18,7 @@ const labsBucketURL = `${labsURL}/page`; const variantsWikiURL = `${hostPort}/sr.wikipedia.beta.wmflabs.org/v1`; const variantsWikiBucketURL = `${variantsWikiURL}/page`; const parsoidURI = 'https://parsoid-beta.wmflabs.org'; +const wiktionaryURI = `${hostPort}/en.wiktionary.org/v1`; function loadConfig(path, forceSqlite) { let confString = fs.readFileSync(path).toString(); @@ -54,7 +55,8 @@ const config = { labsApiURL: 'https://en.wikipedia.beta.wmflabs.org/w/api.php', logStream: logStream(), conf: loadConfig(process.env.RB_TEST_CONFIG ? process.env.RB_TEST_CONFIG : `${__dirname}/../../config.test.yaml`), - parsoidURI + parsoidURI, + wiktionaryURI }; config.conf.num_workers = 0; diff --git a/v1/definition.yaml b/v1/definition.yaml index 7727ca835..65c1b985b 100644 --- a/v1/definition.yaml +++ b/v1/definition.yaml @@ -15,6 +15,7 @@ paths: /definition/{term}: x-route-filters: - path: ./lib/access_check_filter.js + - path: ./lib/normalize_title_filter.js options: title: term redirect_cache_control: '{{options.response_cache_control}}' From 259dd87beeec4fcfbddc8d17e7139ba047327afb Mon Sep 17 00:00:00 2001 From: clarakosi Date: Fri, 26 Oct 2018 17:43:28 -0400 Subject: [PATCH 2/2] updated normalize filter and added a test Updated lib/normalize_title_filter.js to better follow format of lib/mwUtil.js Modified lib/mwUtil.js to use {term} if present Removed previous wiktionary test and replaced with one test that checks filter --- lib/mwUtil.js | 3 +- lib/normalize_title_filter.js | 6 ++-- test/features/pagecontent/redirects.js | 44 ++------------------------ 3 files changed, 9 insertions(+), 44 deletions(-) diff --git a/lib/mwUtil.js b/lib/mwUtil.js index 3b5eee2ee..67043a8d9 100644 --- a/lib/mwUtil.js +++ b/lib/mwUtil.js @@ -341,7 +341,8 @@ mwUtil.createRelativeTitleRedirect = (path, req, options) => { const backString = Array.apply(null, { length: pathSuffixCount }).map(() => '../').join(''); let location; if (!options.dropPathAfterTitle) { - const pathPatternAfterTitle = path.substring(path.indexOf('{title}') - 1); + let pathPatternAfterTitle = path.substring(path.indexOf(`{${titleParamName}`) - 1); + pathPatternAfterTitle = pathPatternAfterTitle.replace(/term/, 'title'); location = backString + new URI(pathPatternAfterTitle, newReqParams, true).toString().substr(1) + mwUtil.getQueryString(req); diff --git a/lib/normalize_title_filter.js b/lib/normalize_title_filter.js index bb7796655..22b5b234b 100644 --- a/lib/normalize_title_filter.js +++ b/lib/normalize_title_filter.js @@ -29,7 +29,7 @@ module.exports = (hyper, req, next, options, specInfo) => { } }); } - rp.title = rp[options.title] ? rp[options.title] : rp.title; + rp.title = (options.title && rp[options.title]) || rp.title; if (!rp.title) { return next(hyper, req); } @@ -59,7 +59,9 @@ module.exports = (hyper, req, next, options, specInfo) => { return P.resolve({ status: 301, headers: { - location: mwUtil.createRelativeTitleRedirect(specInfo.path, req), + location: mwUtil.createRelativeTitleRedirect(specInfo.path, req, { + titleParamName: options.title + }), 'cache-control': options.redirect_cache_control || 'no-cache' } }); diff --git a/test/features/pagecontent/redirects.js b/test/features/pagecontent/redirects.js index 2aa4c9010..d266738f3 100644 --- a/test/features/pagecontent/redirects.js +++ b/test/features/pagecontent/redirects.js @@ -9,53 +9,14 @@ describe('redirects', () => { before(() => { return server.start(); }); describe('', () => { - it('should direct to a lowercase version of a title in wiktionary', () => { - return preq.get({ - uri: `${server.config.wiktionaryURI}/page/definition/cat`, - followRedirect: false - }) - .then((res) => { - assert.deepEqual(res.status, 200); - assert.deepEqual(res.headers['content-location'], - 'https://en.wiktionary.org/api/rest_v1/page/definition/cat'); - assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); - }); - }); - - it('should direct to an uppercase version of a title in wiktionary', () => { - return preq.get({ - uri: `${server.config.wiktionaryURI}/page/definition/Cat`, - followRedirect: false - }) - .then((res) => { - assert.deepEqual(res.status, 200); - assert.deepEqual(res.headers['content-location'], - 'https://en.wiktionary.org/api/rest_v1/page/definition/Cat'); - assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); - }); - }); - it('should redirect to a normalized version of a title in wiktionary', () => { return preq.get({ - uri: `${server.config.wiktionaryURI}/page/definition/Appendix%20Glossary`, - followRedirect: false - }) - .then((res) => { - assert.deepEqual(res.status, 301); - assert.deepEqual(res.headers.location, - '../../../en.wiktionary.org/v1/page/definition/Appendix%20Glossary'); - assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); - }); - }); - it('should redirect to a normalized version of a title in wiktionary, #2', () => { - return preq.get({ - uri: `${server.config.wiktionaryURI}/page/definition/Wiktionary%20ELE`, + uri: `${server.config.wiktionaryURI}/page/definition/weekend%20warrior`, followRedirect: false }) .then((res) => { assert.deepEqual(res.status, 301); - assert.deepEqual(res.headers.location, - '../../../en.wiktionary.org/v1/page/definition/Wiktionary%20ELE'); + assert.deepEqual(res.headers.location, 'weekend_warrior'); assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control'); }); }); @@ -66,6 +27,7 @@ describe('redirects', () => { followRedirect: false }) .then((res) => { + console.log(res); assert.deepEqual(res.status, 301); assert.deepEqual(res.headers.location, 'Main_Page?test=mwAQ'); assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control');