Remove some duplicated URL generation code (#2240)

I went down a rabbit hole while trying to untangle the bug in the dockbit and bitrise examples #2234 (review).

The URL generation code is spaghetti-like, with functions, many of which I wrote, with opaque names, doing similar but not identical things, and making slightly incompatible assumptions about the way query strings are handled.

I got a bit lost and need to take a step back.

Meanwhile, this is a small piece of work I did that’s worth keeping. It doesn’t scratch the surface of the tangle, but it does remove a bit of duplication.

It also makes a minor stylistic ES6 change in the handling of default arguments.

Ref: #2027

frontend/lib/badge-url.js

@@ -1,7 +1,11 @@
 import resolveUrl from './resolve-url'
+import { staticBadgeUrl as makeStaticBadgeUrl } from '../../lib/make-badge-url'
 
-export default function resolveBadgeUrl(url, baseUrl, options) {
-  const { longCache, style, queryParams: inQueryParams } = options || {}
+export default function resolveBadgeUrl(
+  url,
+  baseUrl,
+  { longCache, style, queryParams: inQueryParams } = {}
+) {
   const outQueryParams = Object.assign({}, inQueryParams)
   if (longCache) {
     outQueryParams.maxAge = '2592000'
@@ -12,13 +16,9 @@ export default function resolveBadgeUrl(url, baseUrl, options) {
   return resolveUrl(url, baseUrl, outQueryParams)
 }
 
-export function encodeField(s) {
-  return encodeURIComponent(s.replace(/-/g, '--').replace(/_/g, '__'))
-}
-
-export function staticBadgeUrl(baseUrl, subject, status, color, options) {
-  const path = [subject, status, color].map(encodeField).join('-')
-  return resolveUrl(`/badge/${path}.svg`, baseUrl, options)
+export function staticBadgeUrl(baseUrl, label, message, color, options) {
+  const path = makeStaticBadgeUrl({ label, message, color })
+  return resolveUrl(path, baseUrl, options)
 }
 
 // Options can include: { prefix, suffix, color, longCache, style, queryParams }
@@ -28,10 +28,8 @@ export function dynamicBadgeUrl(
   label,
   dataUrl,
   query,
-  options = {}
+  { prefix, suffix, color, queryParams = {}, ...rest } = {}
 ) {
-  const { prefix, suffix, color, queryParams = {}, ...rest } = options
-
   Object.assign(queryParams, {
     label,
     url: dataUrl,

frontend/lib/badge-url.spec.js

@@ -1,9 +1,5 @@
 import { test, given } from 'sazerac'
-import resolveBadgeUrl, {
-  encodeField,
-  staticBadgeUrl,
-  dynamicBadgeUrl,
-} from './badge-url'
+import resolveBadgeUrl, { staticBadgeUrl, dynamicBadgeUrl } from './badge-url'
 
 const resolveBadgeUrlWithLongCache = (url, baseUrl) =>
   resolveBadgeUrl(url, baseUrl, { longCache: true })
@@ -27,14 +23,6 @@ describe('Badge URL functions', function() {
     )
   })
 
-  test(encodeField, () => {
-    given('foo').expect('foo')
-    given('').expect('')
-    given('happy go lucky').expect('happy%20go%20lucky')
-    given('do-right').expect('do--right')
-    given('it_is_a_snake').expect('it__is__a__snake')
-  })
-
   test(staticBadgeUrl, () => {
     given('http://img.example.com', 'foo', 'bar', 'blue', {
       style: 'plastic',

lib/make-badge-url.js

@@ -0,0 +1,31 @@
+'use strict'
+
+const queryString = require('query-string')
+
+function encodeField(s) {
+  return encodeURIComponent(s.replace(/-/g, '--').replace(/_/g, '__'))
+}
+
+function staticBadgeUrl({
+  baseUrl,
+  label,
+  message,
+  color = 'lightgray',
+  style,
+  format = 'svg',
+}) {
+  if (!label || !message) {
+    throw Error('label and message are required')
+  }
+  const path = [label, message, color].map(encodeField).join('-')
+  const outQueryString = queryString.stringify({
+    style,
+  })
+  const suffix = outQueryString ? `?${outQueryString}` : ''
+  return `/badge/${path}.${format}${suffix}`
+}
+
+module.exports = {
+  encodeField,
+  staticBadgeUrl,
+}

lib/make-badge-url.spec.js

@@ -0,0 +1,42 @@
+'use strict'
+
+const { test, given } = require('sazerac')
+const { encodeField, staticBadgeUrl } = require('./make-badge-url')
+
+describe('Badge URL generation functions', function() {
+  test(encodeField, () => {
+    given('foo').expect('foo')
+    given('').expect('')
+    given('happy go lucky').expect('happy%20go%20lucky')
+    given('do-right').expect('do--right')
+    given('it_is_a_snake').expect('it__is__a__snake')
+  })
+
+  test(staticBadgeUrl, () => {
+    given({
+      label: 'foo',
+      message: 'bar',
+      color: 'blue',
+      style: 'flat-square',
+    }).expect('/badge/foo-bar-blue.svg?style=flat-square')
+    given({
+      label: 'foo',
+      message: 'bar',
+      color: 'blue',
+      style: 'flat-square',
+      format: 'png',
+    }).expect('/badge/foo-bar-blue.png?style=flat-square')
+    given({
+      label: 'Hello World',
+      message: 'Привет Мир',
+      color: '#aabbcc',
+    }).expect(
+      '/badge/Hello%20World-%D0%9F%D1%80%D0%B8%D0%B2%D0%B5%D1%82%20%D0%9C%D0%B8%D1%80-%23aabbcc.svg'
+    )
+    given({
+      label: '123-123',
+      message: 'abc-abc',
+      color: 'blue',
+    }).expect('/badge/123--123-abc--abc-blue.svg')
+  })
+})

services/base.js

@@ -3,6 +3,7 @@
 // See available emoji at http://emoji.muan.co/
 const emojic = require('emojic')
 const Joi = require('joi')
+const queryString = require('query-string')
 const {
   NotFound,
   InvalidResponse,
@@ -11,13 +12,13 @@ const {
   Deprecated,
 } = require('./errors')
 const { checkErrorResponse } = require('../lib/error-helper')
-const queryString = require('query-string')
 const {
   makeLogo,
   toArray,
   makeColor,
   setBadgeColor,
 } = require('../lib/badge-data')
+const { staticBadgeUrl } = require('../lib/make-badge-url')
 const trace = require('./trace')
 
 class BaseService {
@@ -89,20 +90,11 @@ class BaseService {
 
   static _makeStaticExampleUrl(serviceData) {
     const badgeData = this._makeBadgeData({}, serviceData)
-    const color = badgeData.colorscheme || badgeData.colorB
-    return this._makeStaticExampleUrlFromTextAndColor(
-      badgeData.text[0],
-      badgeData.text[1],
-      color
-    )
-  }
-
-  static _makeStaticExampleUrlFromTextAndColor(text1, text2, color) {
-    return `/badge/${encodeURIComponent(
-      text1.replace('-', '--')
-    )}-${encodeURIComponent(text2).replace('-', '--')}-${encodeURIComponent(
-      color
-    )}`
+    return staticBadgeUrl({
+      label: badgeData.text[0],
+      message: `${badgeData.text[1]}`,
+      color: badgeData.colorscheme,
+    })
   }
 
   static _dotSvg(url) {
@@ -171,7 +163,7 @@ class BaseService {
             ? `${this._dotSvg(this._makeFullUrl(exampleUrl))}${suffix}`
             : undefined,
           previewUrl: staticExample
-            ? `${this._makeStaticExampleUrl(staticExample)}.svg`
+            ? this._makeStaticExampleUrl(staticExample)
             : `${this._dotSvg(this._makeFullUrl(previewUrl))}${suffix}`,
           urlPattern: urlPattern
             ? `${this._dotSvg(this._makeFullUrl(urlPattern))}${suffix}`

services/base.spec.js

@@ -398,35 +398,6 @@ describe('BaseService', function() {
     })
   })
 
-  describe('a generated static badge url', function() {
-    it('is concatenated text and color', function() {
-      const url = DummyService._makeStaticExampleUrlFromTextAndColor(
-        'name',
-        'value',
-        'green'
-      )
-      expect(url).to.equal('/badge/name-value-green')
-    })
-    it('uses url encoding', function() {
-      const url = DummyService._makeStaticExampleUrlFromTextAndColor(
-        'Hello World',
-        'Привет Мир',
-        '#aabbcc'
-      )
-      expect(url).to.equal(
-        '/badge/Hello%20World-%D0%9F%D1%80%D0%B8%D0%B2%D0%B5%D1%82%20%D0%9C%D0%B8%D1%80-%23aabbcc'
-      )
-    })
-    it('uses escapes minus signs', function() {
-      const url = DummyService._makeStaticExampleUrlFromTextAndColor(
-        '123-123',
-        'abc-abc',
-        'blue'
-      )
-      expect(url).to.equal('/badge/123--123-abc--abc-blue')
-    })
-  })
-
   describe('validate', function() {
     const dummySchema = Joi.object({
       requiredString: Joi.string().required(),