From 065dd570ad1ee4fc5c8436049a1d8b90f987ff49 Mon Sep 17 00:00:00 2001
From: Paul Melnikow <github@paulmelnikow.com>
Date: Fri, 16 Nov 2018 19:21:48 -0500
Subject: [PATCH] Move [StaticBadge] to own service & add test; also affects
[gitter] (#2284)
This picks up @RedSparr0w's work in #1802.
1. The handler for the static badge is moved into its own service with a synchronous handler. Avoiding an async call may make the static badges slightly faster, though it may be worth profiling this if it turns out we want asynchronous static badges in the future. If it doesn't make a performance difference we could make this handler `async` like the others.
2. Most of the custom static-badge logic is in a BaseStaticBadge base class.
3. Rewrite the static Gitter badge to use BaseStaticBadge.
4. A bit of minor cleanup in related functions.
---
gh-badges/lib/make-badge.js | 1 -
lib/request-handler.js | 10 +-
server.js | 55 +--------
services/base-static.js | 58 ++++++++++
services/base.js | 108 ++++++++++--------
services/gitter/gitter.service.js | 35 +++---
services/static-badge/static-badge.service.js | 20 ++++
services/static-badge/static-badge.tester.js | 40 +++++++
8 files changed, 203 insertions(+), 124 deletions(-)
create mode 100644 services/base-static.js
create mode 100644 services/static-badge/static-badge.service.js
create mode 100644 services/static-badge/static-badge.tester.js
diff --git a/gh-badges/lib/make-badge.js b/gh-badges/lib/make-badge.js
index a86622a096..cbd8af8477 100644
--- a/gh-badges/lib/make-badge.js
+++ b/gh-badges/lib/make-badge.js
@@ -105,7 +105,6 @@ function assignColor(color = '', colorschemeType = 'colorB') {
const definedColorschemes = require(path.join(__dirname, 'colorscheme.json'))
-// Inject the measurer to avoid placing any persistent state in this module.
function makeBadge({
format,
template,
diff --git a/lib/request-handler.js b/lib/request-handler.js
index cfca8780e2..7fc717b278 100644
--- a/lib/request-handler.js
+++ b/lib/request-handler.js
@@ -62,13 +62,13 @@ function getBadgeMaxAge(handlerOptions, queryParams) {
? parseInt(process.env.BADGE_MAX_AGE_SECONDS)
: 120
if (handlerOptions.cacheLength) {
- // if we've set a more specific cache length for this badge (or category),
- // use that instead of env.BADGE_MAX_AGE_SECONDS
- maxAge = parseInt(handlerOptions.cacheLength)
+ // If we've set a more specific cache length for this badge (or category),
+ // use that instead of env.BADGE_MAX_AGE_SECONDS.
+ maxAge = handlerOptions.cacheLength
}
if (isInt(queryParams.maxAge) && parseInt(queryParams.maxAge) > maxAge) {
- // only allow queryParams.maxAge to override the default
- // if it is greater than the default
+ // Only allow queryParams.maxAge to override the default if it is greater
+ // than the default.
maxAge = parseInt(queryParams.maxAge)
}
return maxAge
diff --git a/server.js b/server.js
index d386545070..ea256dbed5 100644
--- a/server.js
+++ b/server.js
@@ -22,8 +22,6 @@ const log = require('./lib/log')
const makeBadge = require('./gh-badges/lib/make-badge')
const suggest = require('./lib/suggest')
const {
- makeColorB,
- makeLabel: getLabel,
makeBadgeData: getBadgeData,
setBadgeColor,
} = require('./lib/badge-data')
@@ -33,7 +31,6 @@ const {
} = require('./lib/request-handler')
const { clearRegularUpdateCache } = require('./lib/regular-update')
const { makeSend } = require('./lib/result-sender')
-const { escapeFormat } = require('./lib/path-helpers')
const serverStartTime = new Date(new Date().toGMTString())
@@ -107,7 +104,10 @@ camp.notfound(/.*/, (query, match, end, request) => {
loadServiceClasses().forEach(serviceClass =>
serviceClass.register(
{ camp, handleRequest: cache, githubApiProvider },
- { handleInternalErrors: config.handleInternalErrors }
+ {
+ handleInternalErrors: config.handleInternalErrors,
+ profiling: config.profiling,
+ }
)
)
@@ -227,53 +227,6 @@ camp.route(
})
)
-// Any badge.
-camp.route(
- /^\/(:|badge\/)(([^-]|--)*?)-?(([^-]|--)*)-(([^-]|--)+)\.(svg|png|gif|jpg)$/,
- (data, match, end, ask) => {
- const subject = escapeFormat(match[2])
- const status = escapeFormat(match[4])
- const color = escapeFormat(match[6])
- const format = match[8]
-
- analytics.noteRequest(data, match)
-
- // Cache management - the badge is constant.
- const cacheDuration = (3600 * 24 * 1) | 0 // 1 day.
- ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`)
- if (+new Date(ask.req.headers['if-modified-since']) >= +serverStartTime) {
- ask.res.statusCode = 304
- ask.res.end() // not modified.
- return
- }
- ask.res.setHeader('Last-Modified', serverStartTime.toGMTString())
-
- // Badge creation.
- try {
- const badgeData = getBadgeData(subject, data)
- badgeData.text[0] = getLabel(undefined, { label: subject })
- badgeData.text[1] = status
- badgeData.colorB = makeColorB(color, data)
- badgeData.template = data.style
- if (config.profiling.makeBadge) {
- console.time('makeBadge total')
- }
- const svg = makeBadge(badgeData)
- if (config.profiling.makeBadge) {
- console.timeEnd('makeBadge total')
- }
- makeSend(format, ask.res, end)(svg)
- } catch (e) {
- log.error(e.stack)
- const svg = makeBadge({
- text: ['error', 'bad badge'],
- colorscheme: 'red',
- })
- makeSend(format, ask.res, end)(svg)
- }
- }
-)
-
// Production cache debugging.
let bitFlip = false
camp.route(/^\/flip\.svg$/, (data, match, end, ask) => {
diff --git a/services/base-static.js b/services/base-static.js
new file mode 100644
index 0000000000..cbaac7ea03
--- /dev/null
+++ b/services/base-static.js
@@ -0,0 +1,58 @@
+'use strict'
+
+const makeBadge = require('../gh-badges/lib/make-badge')
+const { makeSend } = require('../lib/result-sender')
+const analytics = require('../lib/analytics')
+const BaseService = require('./base')
+
+const serverStartTime = new Date(new Date().toGMTString())
+
+module.exports = class BaseStaticService extends BaseService {
+ // Note: Since this is a static service, it is not `async`.
+ handle(namedParams, queryParams) {
+ throw new Error(`Handler not implemented for ${this.constructor.name}`)
+ }
+
+ static register({ camp }, serviceConfig) {
+ camp.route(this._regex, (queryParams, match, end, ask) => {
+ analytics.noteRequest(queryParams, match)
+
+ if (+new Date(ask.req.headers['if-modified-since']) >= +serverStartTime) {
+ // Send Not Modified.
+ ask.res.statusCode = 304
+ ask.res.end()
+ return
+ }
+
+ const serviceInstance = new this({}, serviceConfig)
+ const namedParams = this._namedParamsForMatch(match)
+ let serviceData
+ try {
+ // Note: no `await`.
+ serviceData = serviceInstance.handle(namedParams, queryParams)
+ } catch (error) {
+ serviceData = serviceInstance._handleError(error)
+ }
+
+ const badgeData = this._makeBadgeData(queryParams, serviceData)
+
+ // The final capture group is the extension.
+ const format = match.slice(-1)[0]
+ badgeData.format = format
+
+ if (serviceConfig.profiling.makeBadge) {
+ console.time('makeBadge total')
+ }
+ const svg = makeBadge(badgeData)
+ if (serviceConfig.profiling.makeBadge) {
+ console.timeEnd('makeBadge total')
+ }
+
+ const cacheDuration = 3600 * 24 * 1 // 1 day.
+ ask.res.setHeader('Cache-Control', `max-age=${cacheDuration}`)
+ ask.res.setHeader('Last-Modified', serverStartTime.toGMTString())
+
+ makeSend(format, ask.res, end)(svg)
+ })
+ }
+}
diff --git a/services/base.js b/services/base.js
index 87869ddefb..f3d7c543af 100644
--- a/services/base.js
+++ b/services/base.js
@@ -22,6 +22,10 @@ const {
const { staticBadgeUrl } = require('../lib/make-badge-url')
const trace = require('./trace')
+function coalesce(...candidates) {
+ return candidates.find(c => typeof c === 'string')
+}
+
class BaseService {
constructor({ sendAndCacheRequest }, { handleInternalErrors }) {
this._requestFetcher = sendAndCacheRequest
@@ -248,6 +252,52 @@ class BaseService {
return result
}
+ _handleError(error) {
+ if (error instanceof NotFound || error instanceof InvalidParameter) {
+ trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
+ return {
+ message: error.prettyMessage,
+ color: 'red',
+ }
+ } else if (
+ error instanceof InvalidResponse ||
+ error instanceof Inaccessible ||
+ error instanceof Deprecated
+ ) {
+ trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
+ return {
+ message: error.prettyMessage,
+ color: 'lightgray',
+ }
+ } else if (this._handleInternalErrors) {
+ if (
+ !trace.logTrace(
+ 'unhandledError',
+ emojic.boom,
+ 'Unhandled internal error',
+ error
+ )
+ ) {
+ // This is where we end up if an unhandled exception is thrown in
+ // production. Send the error to the logs.
+ console.log(error)
+ }
+ return {
+ label: 'shields',
+ message: 'internal error',
+ color: 'lightgray',
+ }
+ } else {
+ trace.logTrace(
+ 'unhandledError',
+ emojic.boom,
+ 'Unhandled internal error',
+ error
+ )
+ throw error
+ }
+ }
+
async invokeHandler(namedParams, queryParams) {
trace.logTrace(
'inbound',
@@ -260,49 +310,7 @@ class BaseService {
try {
return await this.handle(namedParams, queryParams)
} catch (error) {
- if (error instanceof NotFound || error instanceof InvalidParameter) {
- trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
- return {
- message: error.prettyMessage,
- color: 'red',
- }
- } else if (
- error instanceof InvalidResponse ||
- error instanceof Inaccessible ||
- error instanceof Deprecated
- ) {
- trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
- return {
- message: error.prettyMessage,
- color: 'lightgray',
- }
- } else if (this._handleInternalErrors) {
- if (
- !trace.logTrace(
- 'unhandledError',
- emojic.boom,
- 'Unhandled internal error',
- error
- )
- ) {
- // This is where we end up if an unhandled exception is thrown in
- // production. Send the error to the logs.
- console.log(error)
- }
- return {
- label: 'shields',
- message: 'internal error',
- color: 'lightgray',
- }
- } else {
- trace.logTrace(
- 'unhandledError',
- emojic.boom,
- 'Unhandled internal error',
- error
- )
- throw error
- }
+ return this._handleError(error)
}
}
@@ -333,8 +341,10 @@ class BaseService {
const badgeData = {
text: [
- overrideLabel || serviceLabel || defaultLabel || this.category,
- serviceMessage || 'n/a',
+ // Use `coalesce()` to support empty labels and messages, as in the
+ // static badge.
+ coalesce(overrideLabel, serviceLabel, defaultLabel, this.category),
+ coalesce(serviceMessage, 'n/a'),
],
template: style,
logo: makeLogo(style === 'social' ? defaultLogo : undefined, {
@@ -352,15 +362,12 @@ class BaseService {
}
static register({ camp, handleRequest, githubApiProvider }, serviceConfig) {
- const ServiceClass = this // In a static context, "this" is the class.
-
camp.route(
this._regex,
handleRequest({
queryParams: this.route.queryParams,
handler: async (queryParams, match, sendBadge, request) => {
- const namedParams = this._namedParamsForMatch(match)
- const serviceInstance = new ServiceClass(
+ const serviceInstance = new this(
{
sendAndCacheRequest: request.asPromise,
sendAndCacheRequestWithCallbacks: request,
@@ -368,6 +375,7 @@ class BaseService {
},
serviceConfig
)
+ const namedParams = this._namedParamsForMatch(match)
const serviceData = await serviceInstance.invokeHandler(
namedParams,
queryParams
@@ -375,7 +383,7 @@ class BaseService {
trace.logTrace('outbound', emojic.shield, 'Service data', serviceData)
const badgeData = this._makeBadgeData(queryParams, serviceData)
- // Assumes the final capture group is the extension
+ // The final capture group is the extension.
const format = match.slice(-1)[0]
sendBadge(format, badgeData)
},
diff --git a/services/gitter/gitter.service.js b/services/gitter/gitter.service.js
index e6913b3b73..1abb3ff4a4 100644
--- a/services/gitter/gitter.service.js
+++ b/services/gitter/gitter.service.js
@@ -1,38 +1,39 @@
'use strict'
-const LegacyService = require('../legacy-service')
-const { makeBadgeData: getBadgeData } = require('../../lib/badge-data')
+const BaseStaticService = require('../base-static')
-module.exports = class Gitter extends LegacyService {
+module.exports = class Gitter extends BaseStaticService {
static get category() {
return 'chat'
}
static get route() {
- return { base: 'gitter/room' }
+ return {
+ base: 'gitter/room',
+ pattern: ':user/:repo',
+ }
}
static get examples() {
return [
{
title: 'Gitter',
- previewUrl: 'nwjs/nw.js',
+ urlPattern: ':user/:repo',
+ staticExample: this.render(),
+ exampleUrl: 'nwjs/nw.js',
},
]
}
- static registerLegacyRouteHandler({ camp, cache }) {
- camp.route(
- /^\/gitter\/room\/([^/]+\/[^/]+)\.(svg|png|gif|jpg|json)$/,
- cache((data, match, sendBadge, request) => {
- // match[1] is the repo, which is not used.
- const format = match[2]
+ static get defaultBadgeData() {
+ return { label: 'chat' }
+ }
+
+ static render() {
+ return { message: 'on gitter', color: 'brightgreen' }
+ }
- const badgeData = getBadgeData('chat', data)
- badgeData.text[1] = 'on gitter'
- badgeData.colorscheme = 'brightgreen'
- sendBadge(format, badgeData)
- })
- )
+ handle() {
+ return this.constructor.render()
}
}
diff --git a/services/static-badge/static-badge.service.js b/services/static-badge/static-badge.service.js
new file mode 100644
index 0000000000..e621137839
--- /dev/null
+++ b/services/static-badge/static-badge.service.js
@@ -0,0 +1,20 @@
+'use strict'
+
+const BaseStaticService = require('../base-static')
+
+module.exports = class StaticBadge extends BaseStaticService {
+ static get category() {
+ return 'other'
+ }
+
+ static get route() {
+ return {
+ format: '(?::|badge/)((?:[^-]|--)*?)-?((?:[^-]|--)*)-((?:[^-]|--)+)',
+ capture: ['label', 'message', 'color'],
+ }
+ }
+
+ handle({ label, message, color }) {
+ return { label, message, color }
+ }
+}
diff --git a/services/static-badge/static-badge.tester.js b/services/static-badge/static-badge.tester.js
new file mode 100644
index 0000000000..13371ea3af
--- /dev/null
+++ b/services/static-badge/static-badge.tester.js
@@ -0,0 +1,40 @@
+'use strict'
+
+const t = require('../create-service-tester')()
+module.exports = t
+
+t.create('Shields colorscheme color')
+ .get('/badge/label-message-blue.json?style=_shields_test')
+ .expectJSON({ name: 'label', value: 'message', colorB: '#007ec6' })
+
+t.create('CSS named color')
+ .get('/badge/label-message-whitesmoke.json?style=_shields_test')
+ .expectJSON({ name: 'label', value: 'message', colorB: 'whitesmoke' })
+
+t.create('RGB color')
+ .get('/badge/label-message-rgb(123,123,123).json?style=_shields_test')
+ .expectJSON({ name: 'label', value: 'message', colorB: 'rgb(123,123,123)' })
+
+t.create('All one color')
+ .get('/badge/all%20one%20color-red.json?style=_shields_test')
+ .expectJSON({ name: '', value: 'all one color', colorB: '#e05d44' })
+
+t.create('Not a valid color')
+ .get('/badge/label-message-notacolor.json?style=_shields_test')
+ .expectJSON({ name: 'label', value: 'message', colorB: '#e05d44' })
+
+t.create('Missing message')
+ .get('/badge/label--blue.json?style=_shields_test')
+ .expectJSON({ name: 'label', value: '', colorB: '#007ec6' })
+
+t.create('Missing label')
+ .get('/badge/-message-blue.json?style=_shields_test')
+ .expectJSON({ name: '', value: 'message', colorB: '#007ec6' })
+
+t.create('Override colorB')
+ .get('/badge/label-message-blue.json?style=_shields_test&colorB=yellow')
+ .expectJSON({ name: 'label', value: 'message', colorB: '#dfb317' })
+
+t.create('Override label')
+ .get('/badge/label-message-blue.json?style=_shields_test&label=mylabel')
+ .expectJSON({ name: 'mylabel', value: 'message', colorB: '#007ec6' })
--
GitLab