From 941e8f6ea3f5a46a62550fb3753c22f1b7761fc1 Mon Sep 17 00:00:00 2001 From: phoebe-lew Date: Fri, 15 Dec 2023 18:17:15 +1100 Subject: [PATCH] http-server fixes + some tests (#83) --- .changeset/proud-icons-knock.md | 5 ++ packages/http-server/src/fakes.ts | 31 +++++++- .../src/request-handlers/get-exchanges.ts | 1 - .../src/request-handlers/submit-close.ts | 17 ++++- .../src/request-handlers/submit-order.ts | 24 +++++- .../http-server/tests/submit-close.spec.ts | 75 ++++++++++++++++++- .../http-server/tests/submit-order.spec.ts | 32 +++++++- 7 files changed, 166 insertions(+), 19 deletions(-) create mode 100644 .changeset/proud-icons-knock.md diff --git a/.changeset/proud-icons-knock.md b/.changeset/proud-icons-knock.md new file mode 100644 index 00000000..5e5ce9be --- /dev/null +++ b/.changeset/proud-icons-knock.md @@ -0,0 +1,5 @@ +--- +"@tbdex/http-server": patch +--- + +Improve http-server error handling and test coverage diff --git a/packages/http-server/src/fakes.ts b/packages/http-server/src/fakes.ts index 001b0aae..cd0decd6 100644 --- a/packages/http-server/src/fakes.ts +++ b/packages/http-server/src/fakes.ts @@ -83,26 +83,51 @@ export const fakeOfferingsApi: OfferingsApi = { async getOfferings() { return [offering] } } -export const fakeExchangesApi: ExchangesApi = { +export interface FakeExchangesApi extends ExchangesApi { + exchangeMessagesMap: Map, + addMessage(message: MessageKindClass): void + clearMessages(): void +} + +export const fakeExchangesApi: FakeExchangesApi = { + exchangeMessagesMap: new Map(), + getExchanges: function (): Promise { throw new Error('Function not implemented.') }, - getExchange: function (): Promise { - throw new Error('Function not implemented.') + + getExchange: function (opts: { id: string} ): Promise { + const messages = this.exchangeMessagesMap.get(opts.id) || undefined + return Promise.resolve(messages) }, + getRfq: function (): Promise { throw new Error('Function not implemented.') }, + getQuote: function (): Promise { throw new Error('Function not implemented.') }, + getOrder: function (): Promise { throw new Error('Function not implemented.') }, + getOrderStatuses: function (): Promise { throw new Error('Function not implemented.') }, + getClose: function (): Promise { throw new Error('Function not implemented.') + }, + + addMessage: function (message: MessageKindClass): void { + const messages = this.exchangeMessagesMap.get(message.exchangeId) || [] + messages.push(message) + this.exchangeMessagesMap.set(message.exchangeId, messages) + }, + + clearMessages: function (): void { + this.exchangeMessagesMap = new Map() } } \ No newline at end of file diff --git a/packages/http-server/src/request-handlers/get-exchanges.ts b/packages/http-server/src/request-handlers/get-exchanges.ts index 3cb1f29a..71f37f54 100644 --- a/packages/http-server/src/request-handlers/get-exchanges.ts +++ b/packages/http-server/src/request-handlers/get-exchanges.ts @@ -10,7 +10,6 @@ type GetExchangesOpts = { export function getExchanges(opts: GetExchangesOpts): RequestHandler { const { callback, exchangesApi } = opts return async function (request, response) { - // TODO: verify authz token (#issue 9) const authzHeader = request.headers['authorization'] if (!authzHeader) { return response.status(401).json({ errors: [{ detail: 'Authorization header required' }] }) diff --git a/packages/http-server/src/request-handlers/submit-close.ts b/packages/http-server/src/request-handlers/submit-close.ts index 63605d43..2afc3516 100644 --- a/packages/http-server/src/request-handlers/submit-close.ts +++ b/packages/http-server/src/request-handlers/submit-close.ts @@ -11,7 +11,7 @@ type SubmitCloseOpts = { } export function submitClose(opts: SubmitCloseOpts): RequestHandler { - const { callback } = opts + const { callback, exchangesApi } = opts return async function (req, res) { let message: Message @@ -28,10 +28,19 @@ export function submitClose(opts: SubmitCloseOpts): RequestHandler { return res.status(400).json({ errors: [errorResponse] }) } - // TODO: get most recent message added to exchange. use that to see if close is allowed (issue #1) - // return 409 if close is not allowed given the current state of the exchange. + const exchange = await exchangesApi.getExchange({id: message.exchangeId}) + if(exchange == undefined) { + const errorResponse: ErrorDetail = { detail: `No exchange found for ${message.exchangeId}` } - // TODO: return 404 if exchange not found (issue #2) + return res.status(404).json({ errors: [errorResponse] }) + } + + const last = exchange[exchange.length-1] + if(!last.validNext.has(message.kind)) { + const errorResponse: ErrorDetail = { detail: `cannot submit Close for an exchange where the last message is kind: ${last.kind}` } + + return res.status(409).json({ errors: [errorResponse] }) + } if (!callback) { return res.sendStatus(202) diff --git a/packages/http-server/src/request-handlers/submit-order.ts b/packages/http-server/src/request-handlers/submit-order.ts index 77fa44ce..e913dcc1 100644 --- a/packages/http-server/src/request-handlers/submit-order.ts +++ b/packages/http-server/src/request-handlers/submit-order.ts @@ -1,6 +1,6 @@ import type { SubmitCallback, RequestHandler, ExchangesApi } from '../types.js' import type { ErrorDetail } from '@tbdex/http-client' -import type { MessageKind } from '@tbdex/protocol' +import type { MessageKind, Quote } from '@tbdex/protocol' import { Message } from '@tbdex/protocol' import { CallbackError } from '../callback-error.js' @@ -28,14 +28,32 @@ export function submitOrder(opts: SubmitOrderOpts): RequestHandler { return res.status(400).json({ errors: [errorResponse] }) } - // TODO: return 409 if order is not allowed given the current state of the exchange. (#issue 4) + const exchange = await exchangesApi.getExchange({id: message.exchangeId}) + if(exchange == undefined) { + const errorResponse: ErrorDetail = { detail: `No exchange found for ${message.exchangeId}` } - const quote = await exchangesApi.getQuote({ exchangeId: message.exchangeId }) + return res.status(404).json({ errors: [errorResponse] }) + } + + const last = exchange[exchange.length-1] + if(!last.validNext.has('order')) { + const errorResponse: ErrorDetail = { detail: `cannot submit Order for an exchange where the last message is kind: ${last.kind}` } + + return res.status(409).json({ errors: [errorResponse] }) + } + + const quote = exchange.find((message) => message.isQuote) as Quote if(quote == undefined) { const errorResponse: ErrorDetail = { detail: 'quote is undefined' } return res.status(404).json({errors: [errorResponse]}) } + if(new Date(quote.expiresAt) < new Date(message.createdAt)){ + const errorResponse: ErrorDetail = { detail: `quote is expired` } + + return res.status(400).json({ errors: [errorResponse] }) + } + if (!callback) { return res.sendStatus(202) } diff --git a/packages/http-server/tests/submit-close.spec.ts b/packages/http-server/tests/submit-close.spec.ts index 88b0e22c..4595c0d3 100644 --- a/packages/http-server/tests/submit-close.spec.ts +++ b/packages/http-server/tests/submit-close.spec.ts @@ -1,17 +1,23 @@ import type { ErrorResponse } from '@tbdex/http-client' import type { Server } from 'http' -import { TbdexHttpServer } from '../src/main.js' +import { Close, DevTools, TbdexHttpServer } from '../src/main.js' import { expect } from 'chai' +import { FakeExchangesApi } from '../src/fakes.js' let api = new TbdexHttpServer() let server: Server +const did = await DevTools.createDid() describe('POST /exchanges/:exchangeId/close', () => { before(() => { server = api.listen(8000) }) + afterEach(() => { + (api.exchangesApi as FakeExchangesApi).clearMessages() + }) + after(() => { server.close() server.closeAllConnections() @@ -48,9 +54,70 @@ describe('POST /exchanges/:exchangeId/close', () => { expect(error.detail).to.include('not valid JSON') }) + it(`returns a 404 if the exchange doesn't exist`, async () => { + const close = Close.create({ + metadata: { + from : did.did, + to : did.did, + exchangeId : '123' + }, + data: {} + }) + await close.sign(did) + const resp = await fetch('http://localhost:8000/exchanges/123/close', { + method : 'POST', + body : JSON.stringify(close) + }) + + expect(resp.status).to.equal(404) + + const responseBody = await resp.json() as ErrorResponse + expect(responseBody.errors.length).to.equal(1) + + const [ error ] = responseBody.errors + expect(error.detail).to.exist + expect(error.detail).to.include('No exchange found for') + }) + + it(`returns a 409 if close is not allowed based on the exchange's current state`, async () => { + const close = Close.create({ + metadata: { + from : did.did, + to : did.did, + exchangeId : '123' + }, + data: {} + }) + await close.sign(did) + + const exchangesApi = api.exchangesApi as FakeExchangesApi + exchangesApi.addMessage(close) + + const close2 = Close.create({ + metadata: { + from : did.did, + to : did.did, + exchangeId : '123' + }, + data: {} + }) + await close2.sign(did) + const resp = await fetch('http://localhost:8000/exchanges/123/close', { + method : 'POST', + body : JSON.stringify(close2) + }) + + expect(resp.status).to.equal(409) + + const responseBody = await resp.json() as ErrorResponse + expect(responseBody.errors.length).to.equal(1) + + const [ error ] = responseBody.errors + expect(error.detail).to.exist + expect(error.detail).to.include('cannot submit Close for an exchange where the last message is kind: close') + }) + xit('returns a 400 if request body is not a valid Close') xit('returns a 400 if request body if integrity check fails') - xit('returns a 404 exchange doesnt exist') - xit(`returns a 409 if close is not allowed based on the exchange's current state`) - xit(`returns a 202 if order is accepted`) + xit(`returns a 202 if close is accepted`) }) \ No newline at end of file diff --git a/packages/http-server/tests/submit-order.spec.ts b/packages/http-server/tests/submit-order.spec.ts index 5ff44e5a..96108836 100644 --- a/packages/http-server/tests/submit-order.spec.ts +++ b/packages/http-server/tests/submit-order.spec.ts @@ -1,11 +1,12 @@ import type { ErrorResponse } from '@tbdex/http-client' import type { Server } from 'http' -import { TbdexHttpServer } from '../src/main.js' +import { DevTools, Order, TbdexHttpServer } from '../src/main.js' import { expect } from 'chai' let api = new TbdexHttpServer() let server: Server +const did = await DevTools.createDid() describe('POST /exchanges/:exchangeId/order', () => { before(() => { @@ -48,10 +49,33 @@ describe('POST /exchanges/:exchangeId/order', () => { expect(error.detail).to.include('not valid JSON') }) + it(`returns a 404 if the exchange doesn't exist`, async () => { + const order = Order.create({ + metadata: { + from : did.did, + to : did.did, + exchangeId : '123' + } + }) + await order.sign(did) + const resp = await fetch('http://localhost:8000/exchanges/123/order', { + method : 'POST', + body : JSON.stringify(order) + }) + + expect(resp.status).to.equal(404) + + const responseBody = await resp.json() as ErrorResponse + expect(responseBody.errors.length).to.equal(1) + + const [ error ] = responseBody.errors + expect(error.detail).to.exist + expect(error.detail).to.include('No exchange found for') + }) + + xit(`returns a 409 if order is not allowed based on the exchange's current state`) + xit(`returns a 400 if quote has expired`) xit('returns a 400 if request body is not a valid Order') xit('returns a 400 if request body if integrity check fails') - xit('returns a 404 exchange doesnt exist') - xit(`returns a 409 if order is not allowed based on the exchange's current state`) - xit(`returns a 400 if order is quote has expired`) xit(`returns a 202 if order is accepted`) }) \ No newline at end of file