Skip to content

Commit

Permalink
Simplifies sitemaps implementation (#72)
Browse files Browse the repository at this point in the history 
* Expect sitemap entry loc property over url; fix sort on custom sitemap

* Fixes tests

* Refactors sitemaps to remove handlebars

* Adds video definition to standard prelude in case user has video elements
  • Loading branch information
@cperryk
cperryk authored Mar 13, 2018
1 parent 297993c commit 8753e72
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 191 deletions.
2 changes: 1 addition & 1 deletion lib/page-list/publish.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function onPublish({ uri, data, user }) {
return utils.getPage(uri).then(function (page) {
page.published = true;
page.publishTime = new Date();
page.firstPublishTime = page.firstPublishTime || new Date();
page.firstPublishTime = page.firstPublishTime || page.publishTime;
page.url = data.url;
page.history.push({ action: 'publish', timestamp: new Date(), users: [utils.userOrRobot(user)] });

Expand Down
4 changes: 2 additions & 2 deletions lib/routes/_sitemaps.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function textSitemap(req, res) {
}

return sitemaps.streamEntries(res.locals.site.slug, year)
.map(entry => `${entry.url}\n`)
.map(entry => `${entry.loc}\n`)
.pipe(res);
}

Expand All @@ -46,7 +46,7 @@ function xmlSitemap(req, res) {
function newsSitemap(req, res) {
res.write(sitemaps.preludes.news);
return sitemaps.streamNewsEntries(res.locals.site.slug)
.map(sitemaps.renderNewsEntry)
.map(sitemaps.renderEntry)
.append('</urlset>')
.pipe(res);
}
Expand Down
7 changes: 3 additions & 4 deletions lib/routes/_sitemaps.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ describe(_.startCase(filename), function () {
sandbox.stub(sitemaps, 'renderEntry');
sandbox.stub(sitemaps, 'newsSitemapExists');
sandbox.stub(sitemaps, 'streamNewsEntries');
sandbox.stub(sitemaps, 'renderNewsEntry');
});

afterEach(function () {
Expand All @@ -37,7 +36,7 @@ describe(_.startCase(filename), function () {
sitemaps.sitemapsEnabled.returns(true);
sitemaps.streamEntries
.withArgs('foo', 2014)
.returns(h([{url: 'a'}, {url: 'b'}]));
.returns(h([{loc: 'a'}, {loc: 'b'}]));
lib(app);

return request(app)
Expand All @@ -53,7 +52,7 @@ describe(_.startCase(filename), function () {
sitemaps.sitemapsEnabled.returns(true);
sitemaps.streamEntries
.withArgs('foo', new Date().getFullYear())
.returns(h([{url: 'a'}, {url: 'b'}]));
.returns(h([{loc: 'a'}, {loc: 'b'}]));
lib(app);

return request(app)
Expand Down Expand Up @@ -122,7 +121,7 @@ describe(_.startCase(filename), function () {
sitemaps.sitemapsEnabled.returns(true);
sitemaps.newsSitemapExists.returns(true);
sitemaps.streamNewsEntries.returns(h([1,2]));
sitemaps.renderNewsEntry
sitemaps.renderEntry
.onFirstCall().returns('a')
.onSecondCall().returns('b');
lib(app);
Expand Down
53 changes: 26 additions & 27 deletions lib/services/sitemaps/index.js → lib/services/sitemaps.js
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
'use strict';

const elastic = require('../elastic'),
helpers = require('../elastic-helpers'),
const elastic = require('./elastic'),
helpers = require('./elastic-helpers'),
h = require('highland'),
handlebars = require('handlebars'),
fs = require('fs'),
path = require('path'),
setup = require('../../setup'),
setup = require('../setup'),
xmljs = require('xml-js'),
_ = require('lodash'),
URLS_PER_PAGE = 50000,
MAX_NEWS_URLS = 1000;
MAX_NEWS_URLS = 1000,
STANDARD_PRELUDE = '<?xml version="1.0" encoding="UTF-8"?><urlset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.sitemaps.org/schemas/sitemap/0.9 http://www.sitemaps.org/schemas/sitemap/0.9/sitemap.xsd" xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:image="http://www.google.com/schemas/sitemap-image/1.1" xmlns:video="http://www.google.com/schemas/sitemap-video/1.1">',
NEWS_PRELUDE = '<?xml version="1.0" encoding="UTF-8"?><urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:news="http://www.google.com/schemas/sitemap-news/0.9">';

/**
* Stream sitemap entries derived from the pages index.
Expand Down Expand Up @@ -51,7 +52,7 @@ function streamEntriesDefault(site, year) {
.invoke('toString')
.map(JSON.parse)
.map(page => ({
url: page.url,
loc: page.url,
lastmod: new Date(page.publishTime).toISOString()
}));
}
Expand All @@ -77,7 +78,6 @@ function streamEntriesCustom(site, year) {
type: '_doc',
scroll: '10s',
size: '50',
sort: [{lastmod: 'desc'}],
body: {
query: {
bool: {
Expand All @@ -93,7 +93,8 @@ function streamEntriesCustom(site, year) {
}
}]
}
}
},
sort: [{lastmod: 'desc'}]
}
});

Expand Down Expand Up @@ -129,7 +130,7 @@ function streamNewsEntries(site) {
term: {site}
}, {
range: {
publication_date: {
'news:news.news:publication_date': {
gte : 'now-2d',
lt: 'now',
}
Expand All @@ -146,19 +147,6 @@ function streamNewsEntries(site) {
.map(JSON.parse);
}

/**
* Return a handlebars render fnc for the specified template.
* @param {string} name e.g. "foo"
* @return {function}
*/
function getRenderer(name) {
const filename = `${name}.handlebars`,
filepath = path.join(__dirname, filename),
s = fs.readFileSync(filepath, 'utf8');

return handlebars.compile(s);
}

/**
* Return true if a mapping and handler for a custom sitemap exists.
* @return {boolean}
Expand All @@ -185,11 +173,22 @@ function sitemapsEnabled() {
return setup.options.sitemaps === true;
}

/**
* Render a sites document
* @param {Object} doc
* @return {string}
*/
function renderEntry(doc) {
return xmljs.js2xml({url: _.omit(doc, ['site'])}, {compact: true});
}

module.exports.preludes = {
standard: STANDARD_PRELUDE,
news: NEWS_PRELUDE
};
module.exports.customSitemapExists = customSitemapExists;
module.exports.newsSitemapExists = newsSitemapExists;
module.exports.preludes = require('./preludes');
module.exports.renderEntry = getRenderer('entry');
module.exports.renderNewsEntry = getRenderer('news-entry');
module.exports.renderEntry = renderEntry;
module.exports.sitemapsEnabled = sitemapsEnabled;
module.exports.streamEntries = streamEntries;
module.exports.streamNewsEntries = streamNewsEntries;
99 changes: 25 additions & 74 deletions lib/services/sitemaps/index.test.js → lib/services/sitemaps.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ var _ = require('lodash'),
expect = require('chai').expect,
filename = __filename.split('/').pop().split('.').shift(),
lib = require('./' + filename),
elastic = require('../elastic'),
setup = require('../../setup'),
xml2js = require('xml2js').parseString,
TestEsStream = require('../../../test/mocks/es-stream');
elastic = require('./elastic'),
setup = require('../setup'),
xmljs = require('xml-js'),
TestEsStream = require('../../test/mocks/es-stream');

describe(_.startCase(filename), function () {
let sandbox;
Expand Down Expand Up @@ -104,10 +104,10 @@ describe(_.startCase(filename), function () {
.toPromise(Promise)
.then(results => {
expect(results).to.eql([{
url: 'http://foo.com/_pages/1',
loc: 'http://foo.com/_pages/1',
lastmod: '2018-01-01T00:00:00.000Z'
}, {
url: 'http://foo.com/_pages/2',
loc: 'http://foo.com/_pages/2',
lastmod: '2018-01-01T00:00:00.000Z'
}]);
expect(elastic.scrollStream.getCall(0).args[0].index).to.equal('pages');
Expand Down Expand Up @@ -258,7 +258,7 @@ describe(_.startCase(filename), function () {
);
expect(elasticOpts.body.query.bool.filter).to.include({
range: {
publication_date: {
'news:news.news:publication_date': {
gte : 'now-2d',
lt: 'now',
}
Expand All @@ -284,79 +284,30 @@ describe(_.startCase(filename), function () {
describe('renderEntry', function () {
const fn = lib[this.title];

it ('renders all expected props of a sitemap', function (done) {
const rendered = fn({
url: 'a',
it ('renders all props of specified doc, excluding "site" prop', function () {
const doc = {
loc: 'a',
lastmod: 'b',
changefreq: 'c',
priority: 'd',
images: [
{location: 'e', caption: 'f'}
]
}),
expectedXml = {
'image:image': [{'image:loc': 'e', 'image:caption': 'f'}],
site: 'foo'
},
expectedOut = {
url: {
loc: ['a'],
lastmod: ['b'],
changefreq: ['c'],
priority: ['d'],
'image:image': [{
'image:loc': ['e'],
'image:caption': ['f']
}]
}
};

xml2js(rendered, (err, result) => {
expect(err).to.be.null;
expect(result).to.eql(expectedXml);
done();
});
});
});

describe('renderNewsEntry', function () {
const fn = lib[this.title];

it ('renders all expected props of a news sitemap', function (done) {
const rendered = fn({
url: 'a',
lastmod: 'b',
publication: {
name: 'c',
language: 'd'
},
publication_date: 'e',
title: 'f',
language: 'g',
keywords: ['h','i','j'],
stock_tickers: ['k','l','m'],
genres: ['n','o','p']
}),
expectedXml = {
url: {
loc: ['a'],
lastmod: ['b'],
'news:news': [{
'news:publication': [{
'news:name': ['c'],
'news:language': ['d']
}],
'news:publication_date': ['e'],
'news:title': ['f'],
'news:language': ['g'],
'news:keywords': ['h, i, j'],
'news:stock_tickers': ['k, l, m'],
'news:genres': ['n, o, p']
}]
loc: {_text: 'a'},
lastmod: {_text: 'b'},
changefreq: {_text: 'c'},
priority: {_text: 'd'},
'image:image': {
'image:loc': {_text: 'e'},
'image:caption': {_text: 'f'}
}
}
};
},
rendered = fn(doc);

xml2js(rendered, (err, result) => {
expect(err).to.be.null;
expect(result).to.eql(expectedXml);
done();
});
expect(xmljs.xml2js(rendered, {compact: true})).to.eql(expectedOut);
});
});
});
12 changes: 0 additions & 12 deletions lib/services/sitemaps/entry.handlebars
View file

This file was deleted.

42 changes: 0 additions & 42 deletions lib/services/sitemaps/news-entry.handlebars
View file

This file was deleted.

3 changes: 0 additions & 3 deletions lib/services/sitemaps/preludes.js
View file

This file was deleted.

Loading

0 comments on commit 8753e72

Please sign in to comment.