Validate service data + support labelColor for the endpoint badge (#2740)

Ref #2473

services/base-svg-scraping.spec.js

@@ -78,7 +78,7 @@ describe('BaseSvgScrapingService', function() {
     it('forwards options to _sendAndCacheRequest', async function() {
       class WithCustomOptions extends DummySvgScrapingService {
         async handle() {
-          const { value } = await this._requestSvg({
+          const { message } = await this._requestSvg({
             schema,
             url: 'http://example.com/foo.svg',
             options: {
@@ -86,7 +86,7 @@ describe('BaseSvgScrapingService', function() {
               qs: { queryParam: 123 },
             },
           })
-          return { message: value }
+          return { message }
         }
       }
 

services/base-xml.spec.js

@@ -57,12 +57,12 @@ describe('BaseXmlService', function() {
     it('forwards options to _sendAndCacheRequest', async function() {
       class WithCustomOptions extends BaseXmlService {
         async handle() {
-          const { value } = await this._requestXml({
+          const { requiredString } = await this._requestXml({
             schema: dummySchema,
             url: 'http://example.com/foo.xml',
             options: { method: 'POST', qs: { queryParam: 123 } },
           })
-          return { message: value }
+          return { message: requiredString }
         }
       }
 

services/base-yaml.spec.js

@@ -77,12 +77,12 @@ describe('BaseYamlService', function() {
     it('forwards options to _sendAndCacheRequest', async function() {
       class WithOptions extends DummyYamlService {
         async handle() {
-          const { value } = await this._requestYaml({
+          const { requiredString } = await this._requestYaml({
             schema: dummySchema,
             url: 'http://example.com/foo.yaml',
             options: { method: 'POST', qs: { queryParam: 123 } },
           })
-          return { message: value }
+          return { message: requiredString }
         }
       }
 

services/base.js

@@ -28,9 +28,23 @@ const { assertValidServiceDefinition } = require('./service-definitions')
 const defaultBadgeDataSchema = Joi.object({
   label: Joi.string(),
   color: Joi.string(),
+  labelColor: Joi.string(),
   logo: Joi.string(),
 }).required()
 
+const serviceDataSchema = Joi.object({
+  isError: Joi.boolean(),
+  label: Joi.string().allow(''),
+  // While a number of badges pass a number here, in the long run we may want
+  // `render()` to always return a string.
+  message: Joi.alternatives(Joi.string().allow(''), Joi.number()).required(),
+  color: Joi.string(),
+  link: Joi.string().uri(),
+  // Generally services should not use these options, which are provided to
+  // support the Endpoint badge.
+  labelColor: Joi.string(),
+}).required()
+
 class BaseService {
   constructor({ sendAndCacheRequest }, { handleInternalErrors }) {
     this._requestFetcher = sendAndCacheRequest
@@ -304,6 +318,7 @@ class BaseService {
     let serviceData
     try {
       serviceData = await serviceInstance.handle(namedParams, queryParams)
+      Joi.assert(serviceData, serviceDataSchema)
     } catch (error) {
       serviceData = serviceInstance._handleError(error)
     }
@@ -330,6 +345,7 @@ class BaseService {
       label: serviceLabel,
       message: serviceMessage,
       color: serviceColor,
+      labelColor: serviceLabelColor,
       link: serviceLink,
     } = serviceData
 
@@ -337,8 +353,23 @@ class BaseService {
       color: defaultColor,
       logo: defaultLogo,
       label: defaultLabel,
+      labelColor: defaultLabelColor,
     } = this.defaultBadgeData
 
+    let color, labelColor
+    if (isError) {
+      // Disregard the override color.
+      color = coalesce(serviceColor, defaultColor, 'lightgrey')
+      labelColor = coalesce(serviceLabelColor, defaultLabelColor)
+    } else {
+      color = coalesce(overrideColor, serviceColor, defaultColor, 'lightgrey')
+      labelColor = coalesce(
+        overrideLabelColor,
+        serviceLabelColor,
+        defaultLabelColor
+      )
+    }
+
     const badgeData = {
       text: [
         // Use `coalesce()` to support empty labels and messages, as in the
@@ -353,16 +384,9 @@ class BaseService {
       }),
       logoWidth: +overrideLogoWidth,
       links: toArray(overrideLink || serviceLink),
-      colorA: makeColor(overrideLabelColor),
+      colorA: makeColor(labelColor),
     }
 
-    let color
-    if (isError) {
-      // Disregard the override color.
-      color = coalesce(serviceColor, defaultColor, 'lightgrey')
-    } else {
-      color = coalesce(overrideColor, serviceColor, defaultColor, 'lightgrey')
-    }
     setBadgeColor(badgeData, color)
 
     return badgeData

services/base.spec.js

@@ -5,6 +5,7 @@ const { expect } = require('chai')
 const { test, given, forCases } = require('sazerac')
 const sinon = require('sinon')
 const trace = require('./trace')
+const { colorScheme: colorsB } = require('./test-helpers')
 
 const {
   NotFound,
@@ -267,6 +268,31 @@ describe('BaseService', function() {
       })
     })
 
+    context('handle() returns invalid data', function() {
+      it('Throws a validation error', async function() {
+        class ThrowingService extends DummyService {
+          async handle() {
+            return {
+              some: 'nonsense',
+            }
+          }
+        }
+        try {
+          await ThrowingService.invoke(
+            {},
+            { handleInternalErrors: false },
+            { namedParamA: 'bar.bar.bar' }
+          )
+          expect.fail('Expected to throw')
+        } catch (e) {
+          expect(e.name).to.equal('ValidationError')
+          expect(e.details.map(({ message }) => message)).to.deep.equal([
+            '"message" is required',
+          ])
+        }
+      })
+    })
+
     describe('Handles known subtypes of ShieldsInternalError', function() {
       it('handles NotFound errors', async function() {
         class ThrowingService extends DummyService {
@@ -355,7 +381,7 @@ describe('BaseService', function() {
         expect(badgeData.text).to.deep.equal(['purr count', 'n/a'])
       })
 
-      it('overrides the colorA', function() {
+      it('overrides the label color', function() {
         const badgeData = DummyService._makeBadgeData(
           { colorA: '42f483' },
           { color: 'green' }
@@ -449,6 +475,11 @@ describe('BaseService', function() {
         const badgeData = DummyService._makeBadgeData({}, { color: 'red' })
         expect(badgeData.colorscheme).to.equal('red')
       })
+
+      it('applies the service label color', function() {
+        const badgeData = DummyService._makeBadgeData({}, { labelColor: 'red' })
+        expect(badgeData.colorA).to.equal(colorsB.red)
+      })
     })
 
     describe('Defaults', function() {
@@ -461,6 +492,11 @@ describe('BaseService', function() {
         const badgeData = DummyService._makeBadgeData({}, {})
         expect(badgeData.colorscheme).to.equal('lightgrey')
       })
+
+      it('provides no default colorA', function() {
+        const badgeData = DummyService._makeBadgeData({}, {})
+        expect(badgeData.colorA).to.be.undefined
+      })
     })
   })