Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jan 13, 2019
4 changes: 2 additions & 2 deletions services/base-svg-scraping.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,15 @@ 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: {
method: 'POST',
qs: { queryParam: 123 },
},
})
return { message: value }
return { message }
}
}

Expand Down
4 changes: 2 additions & 2 deletions services/base-xml.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand Down
4 changes: 2 additions & 2 deletions services/base-yaml.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}

Expand Down
40 changes: 32 additions & 8 deletions services/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -330,15 +345,31 @@ class BaseService {
label: serviceLabel,
message: serviceMessage,
color: serviceColor,
labelColor: serviceLabelColor,
link: serviceLink,
} = serviceData

const {
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
Expand All @@ -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
Expand Down
38 changes: 37 additions & 1 deletion services/base.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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' }
Expand Down Expand Up @@ -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() {
Expand All @@ -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
})
})
})

Expand Down