Skip to content

Commit

Permalink
Deprecate old static badge
Browse files Browse the repository at this point in the history
Now that the static badge has been moved in #2284, next in line for cleaning out `server.js` is this “static badge, old format.” I imagine this route is _very, very old_. (Actually I wouldn’t be surprised if it’s not used at all. I’d be cuious to see some stats on that endpoint.)

In the case of URLs which have permanently changed, an approach I’d like to try is issuing a 301 Redirect.

The benefit is that if a user pastes the URL into the address bar while they are previewing or editing it, the browser will replace the address with the corrected URL when it loads. I figure this will cause some people to update their URLs with no effort, simply because they previewed the badge in their browser, and others to change over, if they notice it.

We incur a slight cost, which is a second request. However many browsers cache the 301’s indefinitely, and we can set an effectively infinite cache duration so the CDN and most other downstream caches will keep them a long time. And handling the redirect is extremely cheap.

This is a nice way to preserve backward compatibility of old routes without having to complicate the new route, such as in the case of vso -> azure-devops. For maintenance purposes, the route that redirects can effectively be treated separately.

It’s also a nice, gentle, and confidence-inspiring way to signal that users should update their URLs.
  • Loading branch information
paulmelnikow committed Nov 17, 2018
1 parent 065dd57 commit 0dc77c4
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 30 deletions.
43 changes: 19 additions & 24 deletions server.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const GithubConstellation = require('./services/github/github-constellation')
const PrometheusMetrics = require('./lib/sys/prometheus-metrics')
const sysMonitor = require('./lib/sys/monitor')
const log = require('./lib/log')
const { staticBadgeUrl } = require('./lib/make-badge-url')
const makeBadge = require('./gh-badges/lib/make-badge')
const suggest = require('./lib/suggest')
const {
Expand Down Expand Up @@ -243,32 +244,26 @@ camp.route(/^\/flip\.svg$/, (data, match, end, ask) => {
makeSend('svg', ask.res, end)(svg)
})

// Any badge, old version.
camp.route(/^\/([^/]+)\/(.+).png$/, (data, match, end, ask) => {
const subject = match[1]
const status = match[2]
const color = data.color
// Any badge, old version. This route must be registered last.
camp.route(/^\/([^/]+)\/(.+).png$/, (queryParams, match, end, ask) => {
const [, label, message] = match
const { color } = queryParams

// Cache management - the badge is constant.
const cacheDuration = (3600 * 24 * 1) | 0 // 1 day.
const redirectUrl = staticBadgeUrl({
label,
message,
color,
format: 'png',
})

ask.res.statusCode = 301
ask.res.setHeader('Location', redirectUrl)

// The redirect is permanent.
const cacheDuration = (365 * 24 * 3600) | 0 // 1 year
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 = { text: [subject, status] }
badgeData.colorscheme = color
const svg = makeBadge(badgeData)
makeSend('png', ask.res, end)(svg)
} catch (e) {
const svg = makeBadge({ text: ['error', 'bad badge'], colorscheme: 'red' })
makeSend('png', ask.res, end)(svg)
}

ask.res.end()
})

if (config.redirectUri) {
Expand Down
13 changes: 7 additions & 6 deletions services/service-tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,14 @@ class ServiceTester {
})
// eslint-disable-next-line mocha/prefer-arrow-callback
.finally(function() {
let responseBody
try {
responseBody = JSON.parse(this._response.body)
} catch (e) {
responseBody = this._response.body
}
// `this` is the IcedFrisby instance.
trace.logTrace(
'outbound',
emojic.shield,
'Response',
JSON.parse(this._response.body)
)
trace.logTrace('outbound', emojic.shield, 'Response', responseBody)
})

this.specs.push(spec)
Expand Down
5 changes: 5 additions & 0 deletions services/static-badge/static-badge.tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,8 @@ t.create('Override colorB')
t.create('Override label')
.get('/badge/label-message-blue.json?style=_shields_test&label=mylabel')
.expectJSON({ name: 'mylabel', value: 'message', colorB: '#007ec6' })

t.create('Old static badge')
.get('/foo/bar.png?color=blue', { followRedirect: false })
.expectStatus(301)
.expectHeader('Location', '/badge/foo-bar-blue.png')

0 comments on commit 0dc77c4

Please sign in to comment.