Repository: badges/shields
Pull request #3184 (2019-03-08 19:40:43)

Rewrite [website]; also affects [nodeping]

shields-ci: 
<!--
  0 failure: 
  0 warning: 
  1 messages
  
  DangerID: danger-id-default;
-->



<table>
  <thead>
    <tr>
      <th width="50"></th>
      <th width="100%" data-danger-table="true">Messages</th>
    </tr>
  </thead>
  <tbody><tr>
      <td>:book:</td>
      <td>:sparkles: Thanks for your contribution to Shields, @paulmelnikow!</td>
    </tr>
  </tbody>
</table>


<p align="right">
  Generated by :no_entry_sign: <a href="https://danger.systems/js">dangerJS</a> against 1142c9a1bef7c9cd193f901fa88777957bce2435
</p>


Patch:
diff --git a/services/nodeping/nodeping-status.service.js b/services/nodeping/nodeping-status.service.js
index 6c55ee3c7a..7c67f4ff69 100644
--- a/services/nodeping/nodeping-status.service.js
+++ b/services/nodeping/nodeping-status.service.js
@@ -2,18 +2,16 @@
 
 const { BaseJsonService } = require('..')
 const Joi = require('joi')
+const {
+  queryParamSchema,
+  exampleQueryParams,
+  renderWebsiteStatus,
+} = require('../website-status')
 
 const schema = Joi.array()
   .items(Joi.object().keys({ su: Joi.boolean() }))
   .min(1)
 
-const queryParamSchema = Joi.object({
-  up_message: Joi.string(),
-  down_message: Joi.string(),
-  up_color: Joi.alternatives(Joi.string(), Joi.number()),
-  down_color: Joi.alternatives(Joi.string(), Joi.number()),
-}).required()
-
 /*
  * this is the checkUuid for the NodePing.com (as used on the [example page](https://nodeping.com/reporting.html#results))
  */
@@ -45,24 +43,8 @@ module.exports = class NodePingStatus extends BaseJsonService {
         namedParams: {
           checkUuid: exampleCheckUuid,
         },
-        staticPreview: this.render({ status: true }),
-      },
-      {
-        title: 'NodePing status (customized)',
-        namedParams: {
-          checkUuid: exampleCheckUuid,
-        },
-        queryParams: {
-          up_message: 'online',
-          up_color: 'blue',
-          down_message: 'offline',
-          down_color: 'lightgrey',
-        },
-        staticPreview: this.render({
-          status: true,
-          upMessage: 'online',
-          upColor: 'blue',
-        }),
+        queryParams: exampleQueryParams,
+        staticPreview: renderWebsiteStatus({ isUp: true }),
       },
     ]
   }
@@ -78,7 +60,7 @@ module.exports = class NodePingStatus extends BaseJsonService {
         },
       },
     })
-    return { status: rows[0].su }
+    return { isUp: rows[0].su }
   }
 
   async handle(
@@ -90,19 +72,13 @@ module.exports = class NodePingStatus extends BaseJsonService {
       down_color: downColor,
     }
   ) {
-    const { status } = await this.fetch({ checkUuid })
-    return this.constructor.render({
-      status,
+    const { isUp } = await this.fetch({ checkUuid })
+    return renderWebsiteStatus({
+      isUp,
       upMessage,
       downMessage,
       upColor,
       downColor,
     })
   }
-
-  static render({ status, upMessage, downMessage, upColor, downColor }) {
-    return status
-      ? { message: upMessage || 'up', color: upColor || 'brightgreen' }
-      : { message: downMessage || 'down', color: downColor || 'red' }
-  }
 }
diff --git a/services/website-status.js b/services/website-status.js
new file mode 100644
index 0000000000..c22cf690bd
--- /dev/null
+++ b/services/website-status.js
@@ -0,0 +1,37 @@
+'use strict'
+
+const Joi = require('joi')
+
+const queryParamSchema = Joi.object({
+  up_message: Joi.string(),
+  down_message: Joi.string(),
+  up_color: Joi.alternatives(Joi.string(), Joi.number()),
+  down_color: Joi.alternatives(Joi.string(), Joi.number()),
+}).required()
+
+const exampleQueryParams = {
+  up_message: 'online',
+  up_color: 'blue',
+  down_message: 'offline',
+  down_color: 'lightgrey',
+}
+
+function renderWebsiteStatus({
+  isUp,
+  upMessage = 'up',
+  downMessage = 'down',
+  upColor = 'brightgreen',
+  downColor = 'red',
+}) {
+  if (isUp) {
+    return { message: upMessage, color: upColor }
+  } else {
+    return { message: downMessage, color: downColor }
+  }
+}
+
+module.exports = {
+  queryParamSchema,
+  exampleQueryParams,
+  renderWebsiteStatus,
+}
diff --git a/services/website-status.spec.js b/services/website-status.spec.js
new file mode 100644
index 0000000000..b27771fcec
--- /dev/null
+++ b/services/website-status.spec.js
@@ -0,0 +1,26 @@
+'use strict'
+
+const { test, given } = require('sazerac')
+const { renderWebsiteStatus } = require('./website-status')
+
+describe('Website status helpers', function() {
+  const customOptions = {
+    upMessage: 'groovy',
+    upColor: 'papayawhip',
+    downMessage: 'no good',
+    downColor: 'gray',
+  }
+
+  test(renderWebsiteStatus, () => {
+    given({ isUp: true }).expect({ message: 'up', color: 'brightgreen' })
+    given({ isUp: false }).expect({ message: 'down', color: 'red' })
+    given({ isUp: true, ...customOptions }).expect({
+      message: 'groovy',
+      color: 'papayawhip',
+    })
+    given({ isUp: false, ...customOptions }).expect({
+      message: 'no good',
+      color: 'gray',
+    })
+  })
+})
diff --git a/services/website/website-redirect.service.js b/services/website/website-redirect.service.js
new file mode 100644
index 0000000000..adaf5a7bf2
--- /dev/null
+++ b/services/website/website-redirect.service.js
@@ -0,0 +1,85 @@
+'use strict'
+
+const { escapeFormat } = require('../../core/badge-urls/path-helpers')
+const { redirector } = require('..')
+
+function escapeFormatSlashes(t) {
+  return (
+    escapeFormat(t)
+      // Double slash
+      .replace(/\/\//g, '/')
+  )
+}
+
+/*
+Old documentation, for reference:
+
+<p>
+  <code>[OPTIONS]</code> can be:
+  <ul>
+    <li>
+      Nothing:&nbsp;
+      <code>…/website/…</code>
+    </li>
+    <li>
+      Online and offline text:&nbsp;
+      <code>…/website-up-down/…</code>
+    </li>
+    <li>
+      Online and offline text, then online and offline colors:&nbsp;
+      <code>…/website-up-down-green-orange/…</code></li>
+    </li>
+  </ul>
+  <table class="centered"><tbody>
+    <tr><td>   Dashes <code>--</code>
+      </td><td>  →
+      </td><td>  <code>-</code> Dash
+    </td></tr>
+    <tr><td>   Underscores <code>__</code>
+      </td><td>  →
+      </td><td>  <code>_</code> Underscore <br/>
+    </td></tr>
+    <tr><td>   Slashes <code>//</code>
+      </td><td>  →
+      </td><td>  <code>/</code> Slash <br/>
+    </td></tr>
+    <tr><td>   <code>_</code> or Space <code>&nbsp;</code>
+      </td><td>  →
+      </td><td>  <code>&nbsp;</code> Space
+    </td></tr>
+  </tbody></table>
+</p>
+*/
+
+module.exports = redirector({
+  category: 'monitoring',
+  route: {
+    base: '',
+    format:
+      'website-(([^-/]|--|//)+)-(([^-/]|--|//)+)(-(([^-/]|--|//)+)-(([^-/]|--|//)+))?/([^/]+)/(.+)',
+    capture: [
+      // Some of these could be made into non-capturing groups so these unused
+      // params would not need to be declared.
+      'upMessage',
+      'unused2',
+      'downMessage',
+      'unused4',
+      'unused5',
+      'upColor',
+      'unused7',
+      'downColor',
+      'unused8',
+      'protocol',
+      'hostAndPath',
+    ],
+  },
+  transformPath: ({ protocol, hostAndPath }) =>
+    `/website/${protocol}/${hostAndPath}`,
+  transformQueryParams: ({ upMessage, downMessage, upColor, downColor }) => ({
+    up_message: upMessage ? escapeFormatSlashes(upMessage) : undefined,
+    down_message: downMessage ? escapeFormatSlashes(downMessage) : undefined,
+    up_color: upColor,
+    down_color: downColor,
+  }),
+  dateAdded: new Date('2019-03-08'),
+})
diff --git a/services/website/website.service.js b/services/website/website.service.js
index d69e2e23b9..6993ca662b 100644
--- a/services/website/website.service.js
+++ b/services/website/website.service.js
@@ -1,24 +1,16 @@
 'use strict'
 
-const LegacyService = require('../legacy-service')
-const { makeBadgeData: getBadgeData } = require('../../lib/badge-data')
-const { escapeFormat } = require('../../core/badge-urls/path-helpers')
-
-function escapeFormatSlashes(t) {
-  return (
-    escapeFormat(t)
-      // Double slash
-      .replace(/\/\//g, '/')
-  )
-}
+const { BaseService } = require('..')
+const {
+  queryParamSchema,
+  exampleQueryParams,
+  renderWebsiteStatus,
+} = require('../website-status')
 
 const documentation = `
 <p>
   The badge is of the form
-  <code>https://img.shields.io/website[OPTIONS]/PROTOCOL/URLREST.svg</code>,
-  the simplest case being
-  <code>https://img.shields.io/website/http/example.com.svg</code>.
-  More options are described below.
+  <code>https://img.shields.io/website/PROTOCOL/URLREST.svg</code>.
 </p>
 <p>
   The whole URL is obtained by concatenating the <code>PROTOCOL</code>
@@ -31,64 +23,20 @@ const documentation = `
   <code>https://img.shields.io/website/http/www.website.com/path/to/page.html.svg</code>.
 </p>
 <p>
-  The URLREST should be URLEncoded:
-  <br>
-  <input type="text" id="websiteDocUrlField" placeholder="Paste your URL (without the protocol) here" /><br>
-  <button onclick="(function(el) { el.value = encodeURIComponent(el.value); })(document.getElementById('websiteDocUrlField'))">Encode</button>
-  <button onclick="(function(el) { el.value = decodeURIComponent(el.value); })(document.getElementById('websiteDocUrlField'))">Decode</button>
-</p>
-<p>
-  <code>[OPTIONS]</code> can be:
-  <ul>
-    <li>
-      Nothing:&nbsp;
-      <code>…/website/…</code>
-    </li>
-    <li>
-      Online and offline text:&nbsp;
-      <code>…/website-up-down/…</code>
-    </li>
-    <li>
-      Online and offline text, then online and offline colors:&nbsp;
-      <code>…/website-up-down-green-orange/…</code></li>
-    </li>
-  </ul>
-  <table class="centered"><tbody>
-    <tr><td>   Dashes <code>--</code>
-      </td><td>  →
-      </td><td>  <code>-</code> Dash
-    </td></tr>
-    <tr><td>   Underscores <code>__</code>
-      </td><td>  →
-      </td><td>  <code>_</code> Underscore <br/>
-    </td></tr>
-    <tr><td>   Slashes <code>//</code>
-      </td><td>  →
-      </td><td>  <code>/</code> Slash <br/>
-    </td></tr>
-    <tr><td>   <code>_</code> or Space <code>&nbsp;</code>
-      </td><td>  →
-      </td><td>  <code>&nbsp;</code> Space
-    </td></tr>
-  </tbody></table>
+  The messages and colors for the up and down states can also be customized.
 </p>
 `
 
-// This legacy service should be rewritten to use e.g. BaseJsonService.
-//
-// Tips for rewriting:
-// https://github.com/badges/shields/blob/master/doc/rewriting-services.md
-//
-// Do not base new services on this code.
-module.exports = class Website extends LegacyService {
+module.exports = class Website extends BaseService {
   static get category() {
     return 'monitoring'
   }
 
   static get route() {
     return {
-      base: '',
-      pattern: '',
+      base: 'website',
+      pattern: ':protocol(https|http)/:hostAndPath+',
+      queryParamSchema,
     }
   }
 
@@ -96,60 +44,55 @@ module.exports = class Website extends LegacyService {
     return [
       {
         title: 'Website',
-        pattern: 'website-up-down-green-red/:protocol(https|http)/:hostAndPath',
         namedParams: {
           protocol: 'https',
           hostAndPath: 'shields.io',
         },
-        staticPreview: {
-          label: 'website',
-          message: 'up',
-          color: 'green',
-        },
+        queryParams: exampleQueryParams,
+        staticPreview: renderWebsiteStatus({ isUp: true }),
         documentation,
       },
     ]
   }
 
-  static registerLegacyRouteHandler({ camp, cache }) {
-    camp.route(
-      /^\/website(-(([^-/]|--|\/\/)+)-(([^-/]|--|\/\/)+)(-(([^-/]|--|\/\/)+)-(([^-/]|--|\/\/)+))?)?\/([^/]+)\/(.+)\.(svg|png|gif|jpg|json)$/,
-      cache((data, match, sendBadge, request) => {
-        const onlineMessage = escapeFormatSlashes(
-          match[2] != null ? match[2] : 'online'
-        )
-        const offlineMessage = escapeFormatSlashes(
-          match[4] != null ? match[4] : 'offline'
-        )
-        const onlineColor = escapeFormatSlashes(
-          match[7] != null ? match[7] : 'brightgreen'
-        )
-        const offlineColor = escapeFormatSlashes(
-          match[9] != null ? match[9] : 'red'
-        )
-        const userProtocol = match[11]
-        const userURI = match[12]
-        const format = match[13]
-        const withProtocolURI = `${userProtocol}://${userURI}`
-        const options = {
+  static get defaultBadgeData() {
+    return {
+      label: 'website',
+    }
+  }
+
+  async handle(
+    { protocol, hostAndPath },
+    {
+      up_message: upMessage,
+      down_message: downMessage,
+      up_color: upColor,
+      down_color: downColor,
+    }
+  ) {
+    let isUp
+    try {
+      const {
+        res: { statusCode },
+      } = await this._request({
+        url: `${protocol}://${hostAndPath}`,
+        options: {
           method: 'HEAD',
-          uri: withProtocolURI,
-        }
-        const badgeData = getBadgeData('website', data)
-        badgeData.colorscheme = undefined
-        request(options, (err, res) => {
-          // We consider all HTTP status codes below 310 as success.
-          if (err != null || res.statusCode >= 310) {
-            badgeData.text[1] = offlineMessage
-            badgeData.colorB = offlineColor
-            sendBadge(format, badgeData)
-          } else {
-            badgeData.text[1] = onlineMessage
-            badgeData.colorB = onlineColor
-            sendBadge(format, badgeData)
-          }
-        })
+        },
       })
-    )
+      // We consider all HTTP status codes below 310 as success.
+      isUp = statusCode < 310
+    } catch (e) {
+      // Catch all errors thrown by the request.
+      isUp = false
+    }
+
+    return renderWebsiteStatus({
+      isUp,
+      upMessage,
+      downMessage,
+      upColor,
+      downColor,
+    })
   }
 }


Test patch:
diff --git a/services/website/website-redirect.tester.js b/services/website/website-redirect.tester.js
new file mode 100644
index 0000000000..36815e5edd
--- /dev/null
+++ b/services/website/website-redirect.tester.js
@@ -0,0 +1,29 @@
+'use strict'
+
+const { ServiceTester } = require('../tester')
+
+const t = (module.exports = new ServiceTester({
+  id: 'WebsiteRedirect',
+  title: 'WebsiteRedirect',
+  pathPrefix: '',
+}))
+
+t.create('Website with custom messages')
+  .get('/website-up-down/https/www.google.com.svg', {
+    followRedirect: false,
+  })
+  .expectStatus(301)
+  .expectHeader(
+    'Location',
+    '/website/https/www.google.com.svg?down_message=down&up_message=up'
+  )
+
+t.create('Website with custom messages and colors')
+  .get('/website-up-down-yellow-gray/https/www.google.com.svg', {
+    followRedirect: false,
+  })
+  .expectStatus(301)
+  .expectHeader(
+    'Location',
+    '/website/https/www.google.com.svg?down_color=gray&down_message=down&up_color=yellow&up_message=up'
+  )
diff --git a/services/website/website.tester.js b/services/website/website.tester.js
index edfa297d7d..2e1ed0e037 100644
--- a/services/website/website.tester.js
+++ b/services/website/website.tester.js
@@ -3,35 +3,37 @@
 const t = (module.exports = require('../tester').createServiceTester())
 
 t.create('status of http://shields.io')
-  .get('/website/http/shields.io.json?style=_shields_test')
-  .expectBadge({ label: 'website', message: 'online', color: 'brightgreen' })
+  .get('/http/shields.io.json?style=_shields_test')
+  .expectBadge({ label: 'website', message: 'up', color: 'brightgreen' })
 
 t.create('status of https://shields.io')
-  .get('/website/https/shields.io.json?style=_shields_test')
-  .expectBadge({ label: 'website', message: 'online', color: 'brightgreen' })
+  .get('/https/shields.io.json?style=_shields_test')
+  .expectBadge({ label: 'website', message: 'up', color: 'brightgreen' })
 
 t.create('status of nonexistent domain')
-  .get('/website/https/shields-io.io.json?style=_shields_test')
-  .expectBadge({ label: 'website', message: 'offline', color: 'red' })
+  .get('/https/shields-io.io.json?style=_shields_test')
+  .expectBadge({ label: 'website', message: 'down', color: 'red' })
 
 t.create('status when network is off')
-  .get('/website/http/shields.io.json?style=_shields_test')
+  .get('/http/shields.io.json?style=_shields_test')
   .networkOff()
-  .expectBadge({ label: 'website', message: 'offline', color: 'red' })
+  .expectBadge({ label: 'website', message: 'down', color: 'red' })
 
 t.create('custom online label, online message and online color')
   .get(
-    '/website-up-down-green-grey/http/online.com.json?style=_shields_test&label=homepage'
+    '/http/online.com.json?style=_shields_test&up_message=up&down_message=down&up_color=green&down_color=grey'
   )
   .intercept(nock =>
     nock('http://online.com')
       .head('/')
       .reply(200)
   )
-  .expectBadge({ label: 'homepage', message: 'up', color: 'green' })
+  .expectBadge({ label: 'website', message: 'up', color: 'green' })
 
 t.create('custom offline message and offline color')
-  .get('/website-up-down-green-grey/http/offline.com.json?style=_shields_test')
+  .get(
+    '/http/offline.com.json?style=_shields_test&up_message=up&down_message=down&up_color=green&down_color=grey'
+  )
   .intercept(nock =>
     nock('http://offline.com')
       .head('/')