diff --git a/services/base-svg-scraping.spec.js b/services/base-svg-scraping.spec.js index da484af58e..05b80babbf 100644 --- a/services/base-svg-scraping.spec.js +++ b/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 } } } diff --git a/services/base-xml.spec.js b/services/base-xml.spec.js index a02ac9709c..05ac79b59c 100644 --- a/services/base-xml.spec.js +++ b/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 } } } diff --git a/services/base-yaml.spec.js b/services/base-yaml.spec.js index b7a6be4d25..7d2e7a4bd7 100644 --- a/services/base-yaml.spec.js +++ b/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 } } } diff --git a/services/base.js b/services/base.js index ce8eb369a8..2e8b4996ad 100644 --- a/services/base.js +++ b/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 diff --git a/services/base.spec.js b/services/base.spec.js index 7de1269aff..165325376b 100644 --- a/services/base.spec.js +++ b/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 + }) }) })