Skip to content

Commit

Permalink
Merge d86b3ad into 988c18b
Browse files Browse the repository at this point in the history
  • Loading branch information
@Pchelolo
Pchelolo committed Jun 20, 2018
2 parents 988c18b + d86b3ad commit 39014c2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 25 deletions.
27 changes: 11 additions & 16 deletions lib/language_variants_filter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ module.exports = (hyper, req, next, options, specInfo) => {
if (!acceptLanguage || mwUtil.isNoCacheRequest(req)) {
delete req.headers['accept-language'];
return next(hyper, req)
// TODO: Store the Vary header so that if the page is possible
// to be converted we return a proper Vary even when accept-language
// is not specified.
// For now we hacky check by domain.
// TODO: For now we hacky check by domain as the vary header is not stored for everything.
.then((res) => {
const langCode = req.params.domain.substring(0, req.params.domain.indexOf('.'));
if (mwUtil.canConvertLangVariant(hyper, req, langCode)) {
res.headers.vary = 'accept-language';
if (!res.headers.vary) {
const langCode = req.params.domain.substring(0, req.params.domain.indexOf('.'));
if (mwUtil.canConvertLangVariant(hyper, req, langCode)) {
res.headers.vary = 'accept-language';
}
}
return res;
});
Expand All @@ -35,9 +34,11 @@ module.exports = (hyper, req, next, options, specInfo) => {
if (!shouldConvert) {
delete req.headers['accept-language'];
return next(hyper, req)
// TODO: Eventually we want to store the Vary header we get from Parsoid!
// TODO: For now we hacky check by domain as the vary header
// is not stored for everything.
.then((res) => {
if (mwUtil.canConvertLangVariant(hyper, req, revision.page_language)) {
if (!res.headers.vary
&& mwUtil.canConvertLangVariant(hyper, req, revision.page_language)) {
res.headers.vary = 'accept-language';
}
return res;
Expand Down Expand Up @@ -67,13 +68,7 @@ module.exports = (hyper, req, next, options, specInfo) => {
}
}
}
}))
// TODO: Now we fix up the Vary header cause Parsoid
// sets content-type in there which is wrong.
.then((res) => {
res.headers.vary = 'accept-language';
return res;
});
}));
} else {
// TODO: It's something else, so just forward to MCS
return next(hyper, req);
Expand Down
11 changes: 7 additions & 4 deletions sys/multi_content_bucket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ function returnRevision(req) {
return (dbResult) => {
if (dbResult.body && dbResult.body.items && dbResult.body.items.length) {
const row = dbResult.body.items[0];
const headers = {
const headers = Object.assign(row.headers || {}, {
etag: mwUtil.makeETag(row.rev, row.tid),
'content-type': row['content-type']
};
'content-type': row['content-type'],
});
return {
status: 200,
headers,
Expand Down Expand Up @@ -87,6 +87,7 @@ class MultiContentBucket {
rev,
tid,
'content-type': req.body[cTypeSpec.name].headers['content-type'],
headers: req.body[cTypeSpec.name].headers,
value: req.body[cTypeSpec.name].body
}
}
Expand All @@ -101,6 +102,7 @@ class MultiContentBucket {
rev,
tid,
'content-type': req.body[mainCTypeName].headers['content-type'],
headers: req.body[mainCTypeName].headers,
value: req.body[mainCTypeName].body
}
}
Expand Down Expand Up @@ -218,7 +220,7 @@ class MultiContentBucket {
}

makeSchema(opts) {
const schemaVersionMajor = 3;
const schemaVersionMajor = 4;

return {
table: opts.table,
Expand All @@ -245,6 +247,7 @@ class MultiContentBucket {
// Redirect
'content-location': 'string',
'content-type': 'string',
headers: 'json',
tags: 'set<string>'
},
index: [
Expand Down
10 changes: 5 additions & 5 deletions test/features/pagecontent/language_variants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('Language variants', function() {
return preq.get({ uri: `${server.config.variantsWikiBucketURL}/html/${variantsPageTitle}`})
.then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
assert.checkString(res.headers.vary.toLowerCase(), /accept-language/);
assert.deepEqual(/1\. Ово је тестна страница/.test(res.body), true);
assert.deepEqual(/2\. Ovo je testna stranica/.test(res.body), true);
});
Expand All @@ -34,7 +34,7 @@ describe('Language variants', function() {
})
.then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
assert.checkString(res.headers.vary.toLowerCase(), /accept-language/);
assert.deepEqual(/1\. Ово је тестна страница/.test(res.body), true);
assert.deepEqual(/2\. Ovo je testna stranica/.test(res.body), true);
});
Expand All @@ -49,7 +49,7 @@ describe('Language variants', function() {
})
.then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
assert.checkString(res.headers.vary.toLowerCase(), /accept-language/);
assert.deepEqual(/1\. Ово је тестна страница/.test(res.body), true);
assert.deepEqual(/2\. Ovo je testna stranica/.test(res.body), true);
});
Expand All @@ -64,7 +64,7 @@ describe('Language variants', function() {
})
.then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
assert.checkString(res.headers.vary.toLowerCase(), /accept-language/);
assert.deepEqual(/1\. Ово је тестна страница/.test(res.body), true);
assert.deepEqual(/2\. Ово је тестна страница/.test(res.body), true);
});
Expand All @@ -79,7 +79,7 @@ describe('Language variants', function() {
})
.then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
assert.checkString(res.headers.vary.toLowerCase(), /accept-language/);
assert.deepEqual(/1\. Ovo je testna stranica/.test(res.body), true);
assert.deepEqual(/2\. Ovo je testna stranica/.test(res.body), true);
});
Expand Down

0 comments on commit 39014c2

Please sign in to comment.