Skip to content

Commit

Permalink
Clean up request-handler, github-auth, and analytics; upgrade to Moch…
Browse files Browse the repository at this point in the history
…a 4 (#1142)

- Add tests to request-handler to prepare for some tweaks to caching for #820
- Clean up code in request-handler: renames, DRY, arrows, imports
- Allow for clean shutdown of `setInterval` code. This requires the ability to cancel autosaving.
- Upgrade to Mocha 4, and clean up so the process exits on its own (see mochajs/mocha#3044)
- Better encapsulate analytics
  • Loading branch information
paulmelnikow authored Oct 18, 2017
1 parent f371c8f commit dc44ba7
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 84 deletions.
56 changes: 41 additions & 15 deletions lib/analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,32 @@ if (process.env.REDISTOGO_URL) {
}

let analytics = {};
let autosaveIntervalId;

const analyticsAutoSaveFileName = process.env.SHIELDS_ANALYTICS_FILE || './analytics.json';
const analyticsAutoSavePeriod = 10000;
setInterval(function analyticsAutoSave() {
const analyticsPath = process.env.SHIELDS_ANALYTICS_FILE || './analytics.json';

function performAutosave() {
const contents = JSON.stringify(analytics);
if (useRedis) {
redis.set(analyticsAutoSaveFileName, JSON.stringify(analytics));
redis.set(analyticsPath, contents);
} else {
fs.writeFileSync(analyticsAutoSaveFileName, JSON.stringify(analytics));
fs.writeFileSync(analyticsPath, contents);
}
}, analyticsAutoSavePeriod);
}

function scheduleAutosaving() {
const analyticsAutoSavePeriod = 10000;
autosaveIntervalId = setInterval(performAutosave, analyticsAutoSavePeriod);
}

// For a clean shutdown.
function cancelAutosaving() {
if (autosaveIntervalId) {
clearInterval(autosaveIntervalId);
autosaveIntervalId = null;
}
performAutosave();
}

function defaultAnalytics() {
const analytics = Object.create(null);
Expand All @@ -43,11 +59,10 @@ function defaultAnalytics() {
return analytics;
}

// Auto-load analytics.
function analyticsAutoLoad() {
function load() {
const defaultAnalyticsObject = defaultAnalytics();
if (useRedis) {
redis.get(analyticsAutoSaveFileName, function(err, value) {
redis.get(analyticsPath, function(err, value) {
if (err == null && value != null) {
// if/try/return trick:
// if error, then the rest of the function is run.
Expand All @@ -70,7 +85,7 @@ function analyticsAutoLoad() {
} else {
// Not using Redis.
try {
analytics = JSON.parse(fs.readFileSync(analyticsAutoSaveFileName));
analytics = JSON.parse(fs.readFileSync(analyticsPath));
// Extend analytics with a new value.
for (const key in defaultAnalyticsObject) {
if (!(key in analytics)) {
Expand Down Expand Up @@ -106,12 +121,23 @@ function incrMonthlyAnalytics(monthlyAnalytics) {
} catch(e) { console.error(e.stack); }
}

function getAnalytics() {
return analytics;
function noteRequest(queryParams, match) {
incrMonthlyAnalytics(analytics.vendorMonthly);
if (queryParams.style === 'flat') {
incrMonthlyAnalytics(analytics.vendorFlatMonthly);
} else if (queryParams.style === 'flat-square') {
incrMonthlyAnalytics(analytics.vendorFlatSquareMonthly);
}
}

function setRoutes (server) {
server.ajax.on('analytics/v1', (json, end) => { end(analytics); });
}

module.exports = {
analyticsAutoLoad,
incrMonthlyAnalytics,
getAnalytics
load,
scheduleAutosaving,
cancelAutosaving,
noteRequest,
setRoutes
};
49 changes: 35 additions & 14 deletions lib/github-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,37 @@ try {
// is stored in this JSON data.
serverSecrets = require('../private/secret.json');
} catch(e) {}

// This is an initial value which makes the code work while the initial data
// is loaded. In the then() callback of scheduleAutosaving(), it's reassigned
// with a JsonSave object.
let githubUserTokens = {data: []};
const githubUserTokensFile = './private/github-user-tokens.json';
autosave(githubUserTokensFile, {data: []}).then(function(f) {
githubUserTokens = f;
for (let i = 0; i < githubUserTokens.data.length; i++) {
addGithubToken(githubUserTokens.data[i]);
}
// Personal tokens allow access to GitHub private repositories.
// You can manage your personal GitHub token at
// <https://github.com/settings/tokens>.
if (serverSecrets && serverSecrets.gh_token) {
addGithubToken(serverSecrets.gh_token);

function scheduleAutosaving() {
const githubUserTokensFile = './private/github-user-tokens.json';
autosave(githubUserTokensFile, {data: []}).then(save => {
githubUserTokens = save;
for (let i = 0; i < githubUserTokens.data.length; i++) {
addGithubToken(githubUserTokens.data[i]);
}
// Personal tokens allow access to GitHub private repositories.
// You can manage your personal GitHub token at
// <https://github.com/settings/tokens>.
if (serverSecrets && serverSecrets.gh_token) {
addGithubToken(serverSecrets.gh_token);
}
}).catch(e => {
console.error('Could not create ' + githubUserTokensFile);
});
}

function cancelAutosaving() {
if (githubUserTokens.stop) {
githubUserTokens.stop();
githubUserTokens.save();
githubUserTokens = {data: []};
}
}).catch(function(e) { console.error('Could not create ' + githubUserTokensFile); });
}

function setRoutes(server) {
server.route(/^\/github-auth$/, function(data, match, end, ask) {
Expand Down Expand Up @@ -261,5 +278,9 @@ function constEq(a, b) {
return (zero === 0);
}

exports.setRoutes = setRoutes;
exports.request = githubRequest;
module.exports = {
scheduleAutosaving,
cancelAutosaving,
request: githubRequest,
setRoutes,
};
2 changes: 1 addition & 1 deletion lib/in-process-server-test-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function reset (server) {
*/
function stop (server) {
if (server) {
server.camp.close();
server.stop();
}
}

Expand Down
51 changes: 21 additions & 30 deletions lib/request-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,11 @@
const domain = require('domain');
const request = require('request');
const badge = require('./badge');
const {
makeBadgeData: getBadgeData
} = require('./badge-data');
const { makeBadgeData: getBadgeData } = require('./badge-data');
const log = require('./log');
const LruCache = require('./lru-cache');
const {
incrMonthlyAnalytics,
getAnalytics
} = require('./analytics');
const {
makeSend
} = require('./result-sender');
const analytics = require('./analytics');
const { makeSend } = require('./result-sender');

// We avoid calling the vendor's server for computation of the information in a
// number of badges.
Expand All @@ -35,33 +28,30 @@ const requestCache = new LruCache(1000);

// Deep error handling for vendor hooks.
const vendorDomain = domain.create();
vendorDomain.on('error', function(err) {
vendorDomain.on('error', err => {
log.error('Vendor hook error:', err.stack);
});

function handleRequest (vendorRequestHandler) {
return function getRequest(data, match, end, ask) {
if (data.maxAge !== undefined && /^[0-9]+$/.test(data.maxAge)) {
ask.res.setHeader('Cache-Control', 'max-age=' + data.maxAge);
return (queryParams, match, end, ask) => {
if (queryParams.maxAge !== undefined && /^[0-9]+$/.test(queryParams.maxAge)) {
ask.res.setHeader('Cache-Control', 'max-age=' + queryParams.maxAge);
} else {
// Cache management - no cache, so it won't be cached by GitHub's CDN.
ask.res.setHeader('Cache-Control', 'no-cache, no-store, must-revalidate');
}

const reqTime = new Date();
const date = (reqTime).toGMTString();
const date = reqTime.toGMTString();
ask.res.setHeader('Expires', date); // Proxies, GitHub, see #221.
ask.res.setHeader('Date', date);
incrMonthlyAnalytics(getAnalytics().vendorMonthly);
if (data.style === 'flat') {
incrMonthlyAnalytics(getAnalytics().vendorFlatMonthly);
} else if (data.style === 'flat-square') {
incrMonthlyAnalytics(getAnalytics().vendorFlatSquareMonthly);
}

const cacheIndex = match[0] + '?label=' + data.label + '&style=' + data.style
+ '&logo=' + data.logo + '&logoWidth=' + data.logoWidth
+ '&link=' + JSON.stringify(data.link) + '&colorA=' + data.colorA
+ '&colorB=' + data.colorB;
analytics.noteRequest(queryParams, match);

const cacheIndex = match[0] + '?label=' + queryParams.label + '&style=' + queryParams.style
+ '&logo=' + queryParams.logo + '&logoWidth=' + queryParams.logoWidth
+ '&link=' + JSON.stringify(queryParams.link) + '&colorA=' + queryParams.colorA
+ '&colorB=' + queryParams.colorB;
// Should we return the data right away?
const cached = requestCache.get(cacheIndex);
let cachedVersionSent = false;
Expand All @@ -78,7 +68,7 @@ function handleRequest (vendorRequestHandler) {

// In case our vendor servers are unresponsive.
let serverUnresponsive = false;
const serverResponsive = setTimeout(function() {
const serverResponsive = setTimeout(() => {
serverUnresponsive = true;
if (cachedVersionSent) { return; }
if (requestCache.has(cacheIndex)) {
Expand All @@ -87,7 +77,7 @@ function handleRequest (vendorRequestHandler) {
return;
}
ask.res.setHeader('Cache-Control', 'no-cache, no-store, must-revalidate');
const badgeData = getBadgeData('vendor', data);
const badgeData = getBadgeData('vendor', queryParams);
badgeData.text[1] = 'unresponsive';
let extension;
try {
Expand All @@ -109,7 +99,8 @@ function handleRequest (vendorRequestHandler) {
}
options.headers = options.headers || {};
options.headers['User-Agent'] = options.headers['User-Agent'] || 'Shields.io';
return request(options, function(err, res, body) {

request(options, (err, res, body) => {
if (res != null && res.headers != null) {
const cacheControl = res.headers['cache-control'];
if (cacheControl != null) {
Expand All @@ -125,8 +116,8 @@ function handleRequest (vendorRequestHandler) {
});
}

vendorDomain.run(function() {
vendorRequestHandler(data, match, function sendBadge(format, badgeData) {
vendorDomain.run(() => {
vendorRequestHandler(queryParams, match, function sendBadge(format, badgeData) {
if (serverUnresponsive) { return; }
clearTimeout(serverResponsive);
// Check for a change in the data.
Expand Down
75 changes: 75 additions & 0 deletions lib/request-handler.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
'use strict';

const assert = require('assert');
const fetch = require('node-fetch');
const config = require('./test-config');
const Camp = require('camp');
const analytics = require('./analytics');
const { makeBadgeData: getBadgeData } = require('./badge-data');
const {
handleRequest,
clearRequestCache
} = require('./request-handler');

const baseUri = `http://127.0.0.1:${config.port}`;

function performTwoRequests (first, second) {
return fetch(`${baseUri}${first}`)
.then(res => {
assert.ok(res.ok);
return fetch(`${baseUri}${second}`)
.then(res => {
assert.ok(res.ok);
})
});
}

describe('The request handler', function() {
before(analytics.load);

let camp;
beforeEach(function (done) {
camp = Camp.start({ port: config.port, hostname: '::' });
camp.on('listening', () => done());
});
afterEach(function (done) {
clearRequestCache();
if (camp) {
camp.close(() => done());
camp = null;
}
});

let handlerCallCount;
beforeEach(function () {
handlerCallCount = 0;
camp.route(/^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/,
handleRequest((queryParams, match, sendBadge, request) => {
++handlerCallCount;
const [, someValue, format] = match;
const badgeData = getBadgeData('testing', queryParams);
badgeData.text[1] = someValue;
sendBadge(format, badgeData);
}));
});

it('should cache identical requests', function () {
return performTwoRequests('/testing/123.svg', '/testing/123.svg').then(() => {
assert.equal(handlerCallCount, 1);
});
});

it('should differentiate known query parameters', function () {
return performTwoRequests(
'/testing/123.svg?label=foo',
'/testing/123.svg?label=bar'
).then(() => { assert.equal(handlerCallCount, 2); });
});

it('should ignore unknown query parameters', function () {
return performTwoRequests(
'/testing/123.svg?foo=1',
'/testing/123.svg?foo=2'
).then(() => { assert.equal(handlerCallCount, 1); });
});
});
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"is-svg": "^2.1.0",
"lodash.difference": "^4.5.0",
"minimist": "^1.2.0",
"mocha": "^3.2.0",
"mocha": "^4.0.1",
"nock": "^9.0.13",
"node-fetch": "^1.6.3",
"nyc": "^11.2.1",
Expand Down
Loading

0 comments on commit dc44ba7

Please sign in to comment.