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

Delete BaseHTTPService and implement new BaseXmlService (affects [eclipse-marketplace f-droid]; also testing on [uptimerobot circleci]) #2037

Merged
merged 15 commits into from
Sep 3, 2018
Merged
25 changes: 0 additions & 25 deletions lib/all-badge-examples.js
Original file line number Diff line number Diff line change
Expand Up @@ -735,16 +735,6 @@ const allBadgeExamples = [
previewUrl: '/vscode-marketplace/d/ritwickdey.LiveServer.svg',
keywords: ['vscode-marketplace'],
},
{
title: 'Eclipse Marketplace',
previewUrl: '/eclipse-marketplace/dt/notepad4e.svg',
keywords: ['eclipse', 'marketplace'],
},
{
title: 'Eclipse Marketplace',
previewUrl: '/eclipse-marketplace/dm/notepad4e.svg',
keywords: ['eclipse', 'marketplace'],
},
{
title: 'JetBrains IntelliJ plugins',
previewUrl: '/jetbrains/plugin/d/1347-scala.svg',
Expand Down Expand Up @@ -1390,11 +1380,6 @@ const allBadgeExamples = [
previewUrl: '/vscode-marketplace/v/ritwickdey.LiveServer.svg',
keywords: ['vscode-marketplace'],
},
{
title: 'Eclipse Marketplace',
previewUrl: '/eclipse-marketplace/v/notepad4e.svg',
keywords: ['eclipse', 'marketplace'],
},
{
title: 'iTunes App Store',
previewUrl: '/itunes/v/803453959.svg',
Expand Down Expand Up @@ -1619,16 +1604,6 @@ const allBadgeExamples = [
previewUrl:
'/swagger/valid/2.0/https/raw.githubusercontent.com/OAI/OpenAPI-Specification/master/examples/v2.0/json/petstore-expanded.json.svg',
},
{
title: 'Eclipse Marketplace',
previewUrl: '/eclipse-marketplace/favorites/notepad4e.svg',
keywords: ['eclipse', 'marketplace'],
},
{
title: 'Eclipse Marketplace',
previewUrl: '/eclipse-marketplace/last-update/notepad4e.svg',
keywords: ['eclipse', 'marketplace'],
},
{
title: 'Vaadin Directory',
previewUrl: '/vaadin-directory/status/vaadinvaadin-grid.svg',
Expand Down
12 changes: 0 additions & 12 deletions lib/error-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,6 @@ checkErrorResponse.asPromise = function(errorMessages = {}) {
}
}

async function asJson({ buffer, res }) {
try {
return JSON.parse(buffer)
} catch (err) {
throw new InvalidResponse({
prettyMessage: 'unparseable json response',
underlyingError: err,
})
}
}

module.exports = {
checkErrorResponse,
asJson,
}
29 changes: 29 additions & 0 deletions lib/response-parsers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
'use strict'

const { InvalidResponse } = require('../services/errors')
const fastXmlParser = require('fast-xml-parser')

async function asJson({ buffer, res }) {
try {
return JSON.parse(buffer)
} catch (err) {
throw new InvalidResponse({
prettyMessage: 'unparseable json response',
underlyingError: err,
})
}
}

async function asXml({ buffer, res }) {
if (fastXmlParser.validate(buffer) === true) {
return fastXmlParser.parse(buffer)
}
throw new InvalidResponse({
prettyMessage: 'unparseable xml response',
})
}

module.exports = {
asJson,
asXml,
}
44 changes: 44 additions & 0 deletions lib/response-parsers.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict'

const { expect } = require('chai')
const { asJson, asXml } = require('./response-parsers')
const { InvalidResponse } = require('../services/errors')

describe('Json parser', function() {
it('parses json', async function() {
const json = await asJson({ buffer: '{"foo": "bar"}' })

expect(json).to.deep.equal({ foo: 'bar' })
})

it('thows invalid response if json parsing fails', async function() {
try {
await asJson({ buffer: 'not json' })
expect.fail('Expected to throw')
} catch (e) {
expect(e).to.be.an.instanceof(InvalidResponse)
expect(e.message).to.equal(
'Invalid Response: Unexpected token o in JSON at position 1'
)
expect(e.prettyMessage).to.equal('unparseable json response')
}
})
})

describe('Xml parser', function() {
it('parses xml', async function() {
const xml = await asXml({ buffer: '<foo>bar</foo>' })

expect(xml).to.deep.include({ foo: 'bar' })
})

it('thows invalid response if xml parsing fails', async function() {
try {
await asXml({ buffer: 'not xml' })
expect.fail('Expected to throw')
} catch (e) {
expect(e).to.be.an.instanceof(InvalidResponse)
expect(e.prettyMessage).to.equal('unparseable xml response')
}
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding these tests! Well worth keeping and adjusting if this functionality is moved inline, which would widen the bracket a bit.

We aren't using nock directly anywhere right now, though it's probably a good idea to start. I've written service tests for things that ought to be unit tests, and smaller-bracket unit tests for things, e.g. the one in #2036, which probably should mock an HTTP response.

})
Loading