Skip to content

Commit

Permalink
Merge pull request #35 from jembi/bug-openhim-response-fix
Browse files Browse the repository at this point in the history
Fix content type and body handling for OpenHIM responses
  • Loading branch information
rcrichton authored Mar 4, 2024
2 parents c15f93c + 40b39b2 commit df0aa93
Show file tree
Hide file tree
Showing 18 changed files with 54 additions and 52 deletions.
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "mpi-mediator",
"version": "v2.0.1",
"version": "v2.0.2",
"description": "An OpenHIM mediator to handle all interactions with an MPI component",
"main": "index.ts",
"scripts": {
Expand Down
2 changes: 1 addition & 1 deletion src/middlewares/mpi-mdm-everything.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,6 @@ export const mpiMdmEverythingMiddleware: RequestHandler = async (req, res, next)

const { status, body } = await fetchAllLinkedPatientResources(req.params.patientId);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(status).send(body);
};
2 changes: 1 addition & 1 deletion src/middlewares/mpi-mdm-summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ export const mpiMdmSummaryMiddleware: RequestHandler = async (req, res, next) =>

const { status, body } = await fetchAllLinkedPatientSummary(req.params.patientId);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(status).send(body);
};
2 changes: 2 additions & 0 deletions src/middlewares/openhim-proxy-interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export const openhimProxyResponseInterceptor = responseInterceptor(

const responseBody = buildOpenhimResponseObject(transactionStatus, statusCode, body);

_res.setHeader('Content-Type', 'application/json+openhim');

return JSON.stringify(responseBody);
}
);
Expand Down
13 changes: 9 additions & 4 deletions src/middlewares/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ import logger from '../logger';
import { ResponseObject } from '../types/response';
import { buildOpenhimResponseObject, isHttpStatusOk, postData } from '../utils/utils';

const { fhirDatastoreProtocol, fhirDatastoreHost, fhirDatastorePort, contentType, disableValidation } =
getConfig();
const {
fhirDatastoreProtocol,
fhirDatastoreHost,
fhirDatastorePort,
contentType,
disableValidation,
} = getConfig();

export const validationMiddleware: RequestHandler = async (req, res, next) => {
logger.info('Validating Fhir Resources');
Expand All @@ -21,7 +26,7 @@ export const validationMiddleware: RequestHandler = async (req, res, next) => {
status: 400,
};
} else if (disableValidation) {
return next();
return next();
} else {
response = await postData(
fhirDatastoreProtocol,
Expand Down Expand Up @@ -56,6 +61,6 @@ export const validationMiddleware: RequestHandler = async (req, res, next) => {
response.body
);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(response.status).send(responseBody);
};
10 changes: 5 additions & 5 deletions src/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ routes.post(
asyncHandler(async (req, res) => {
const result = await matchSyncHandler(req.body);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(result.status).send(result.body);
})
);
Expand All @@ -40,7 +40,7 @@ routes.post(

const responseBody = buildOpenhimResponseObject(transactionStatus, status, body);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(status).send(responseBody);
})
);
Expand All @@ -67,7 +67,7 @@ routes.get(
asyncHandler(async (req, res) => {
const { status, body } = await fetchEverythingByRef(`Patient/${req.params.patientId}`);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(status).send(body);
})
);
Expand All @@ -78,7 +78,7 @@ routes.get(
asyncHandler(async (req, res) => {
const { status, body } = await fetchPatientSummaryByRef(`Patient/${req.params.patientId}`);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(status).send(body);
})
);
Expand All @@ -90,7 +90,7 @@ routes.post(
asyncHandler(async (req, res) => {
const result = await matchAsyncHandler(req.body);

res.set('Content-Type', 'application/openhim+json');
res.set('Content-Type', 'application/json+openhim');
res.status(result.status).send(result.body);
})
);
Expand Down
2 changes: 1 addition & 1 deletion src/types/response.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export interface Response {
status: number;
body: object;
body: string;
timestamp: string;
headers: HeadersInit;
}
Expand Down
4 changes: 2 additions & 2 deletions src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ export const buildOpenhimResponseObject = (
openhimTransactionStatus: string,
httpResponseStatusCode: number,
responseBody: object,
contentType = 'application/json'
contentType = 'application/fhir+json'

This comment has been minimized.

Copy link
@BMartinos

BMartinos Mar 6, 2024

Collaborator

@rcrichton I think i missed this initially, but wonder if this shouldnt be application/json+openhim instead? as the mediator is building an openhim response

): OpenHimResponseObject => {
const response: Response = {
status: httpResponseStatusCode,
headers: { 'Content-Type': contentType },
body: responseBody,
body: JSON.stringify(responseBody),
timestamp: format(new Date(), "yyyy-MM-dd'T'HH:mm:ss.SSSXXX"),
};

Expand Down
6 changes: 3 additions & 3 deletions tests/cucumber/step-definitions/fhirAccessProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Then(
'a successful response containing a bundle of related patient resources is sent back',
(): void => {
expect(responseBody.status).to.equal('Success');
expect(responseBody.response.body.resourceType).to.equal('Bundle');
expect(JSON.parse(responseBody.response.body).resourceType).to.equal('Bundle');
server.close();
}
);
Expand All @@ -55,7 +55,7 @@ When(
.set('content-type', 'application/fhir+json')
.expect(200);

const { response } = bundleSubmission.body.response.body.entry.find((entry) =>
const { response } = JSON.parse(bundleSubmission.body.response.body).entry.find((entry) =>
entry.response.location.startsWith('Patient')
);

Expand All @@ -71,7 +71,7 @@ When(

Then('a successful response containing a bundle is sent back', (): void => {
expect(responseBody.status).to.equal('Success');
expect(responseBody.response.body.resourceType).to.equal('Bundle');
expect(JSON.parse(responseBody.response.body).resourceType).to.equal('Bundle');
server.close();
});

Expand Down
9 changes: 5 additions & 4 deletions tests/cucumber/step-definitions/mpiAuthentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ When('a post request without body was sent to get patients', async (): Promise<v
});

Then('we should get an error response', (): void => {
expect(responseBody.response.body.error,
expect(
responseBody.response.body.error,
`Invalid Content! Type should be "${config.contentType}" and Length should be greater than 0"`
);
server.close();
Expand All @@ -51,8 +52,8 @@ When('a post request with body was sent to get patients', async (): Promise<void
resource: {
resourceType: 'Patient',
text: {
div: '<div xmlns=\"http://www.w3.org/1999/xhtml\">Patient</div>',
status: 'generated'
div: '<div xmlns="http://www.w3.org/1999/xhtml">Patient</div>',
status: 'generated',
},
name: [
{
Expand All @@ -79,6 +80,6 @@ When('a post request with body was sent to get patients', async (): Promise<void
});

Then('a response should be sent back', (): void => {
expect(responseBody.response.body.id).not.empty;
expect(JSON.parse(responseBody.response.body).id).not.empty;
server.close();
});
2 changes: 1 addition & 1 deletion tests/cucumber/step-definitions/patientSyncMatching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ When(
Then('a patient should be created on the client registry', async (): Promise<void> => {
const auth = await getMpiAuthToken();

const { response } = responseBody.response.body.entry.find((entry) =>
const { response } = JSON.parse(responseBody.response.body).entry.find((entry) =>
entry.response.location.startsWith('Patient')
);

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/fetchPatientResources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe('FetchPatientResources handler', (): void => {
.reply(200, {});

const result: MpiMediatorResponseObject = await fetchEverythingByRef(patientRef);
expect(result.body.response.body).to.deep.equal(emptyBundle);
expect(JSON.parse(result.body.response.body)).to.deep.equal(emptyBundle);
});

it('should succesfully return bundle of various resources', async (): Promise<void> => {
Expand All @@ -149,7 +149,7 @@ describe('FetchPatientResources handler', (): void => {

const result: MpiMediatorResponseObject = await fetchEverythingByRef(patientRef);

expect(result.body.response.body).to.deep.equal(bundle);
expect(JSON.parse(result.body.response.body)).to.deep.equal(bundle);
});
});
});
4 changes: 2 additions & 2 deletions tests/unit/fetchPatientSummaries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,13 @@ describe('FetchPatientSummaries handler', (): void => {
nock(fhirDatastoreUrl).get(`/fhir/${emptyPatientRef1}/$summary`).reply(200, {});

const result = await fetchPatientSummaryByRef(emptyPatientRef1);
expect(result.body.response.body).to.deep.equal(emptyBundle);
expect(JSON.parse(result.body.response.body)).to.deep.equal(emptyBundle);
});
it('should return a bundle with 2 entries for 1 given patient', async () => {
nock(fhirDatastoreUrl).get(`/fhir/${patientRef3}/$summary`).reply(200, patientSummary3);

const result = await fetchPatientSummaryByRef(patientRef3);
expect(result.body.response.body).to.deep.equal(combinedBundle2);
expect(JSON.parse(result.body.response.body)).to.deep.equal(combinedBundle2);
});
});
});
6 changes: 3 additions & 3 deletions tests/unit/kafkaAsyncPatientHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Kafka Async Patient Handler', (): void => {
'x-mediator-urn': '123',
response: {
status: 200,
body: {},
body: '',
timestamp: '12-12-2012',
headers: {
'content-type': 'application/json',
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('Kafka Async Patient Handler', (): void => {
'x-mediator-urn': '123',
response: {
status: 200,
body: {},
body: '',
timestamp: '12-12-2012',
headers: {
'content-type': 'application/json',
Expand Down Expand Up @@ -201,7 +201,7 @@ describe('Kafka Async Patient Handler', (): void => {
'x-mediator-urn': '123',
response: {
status: 200,
body: {},
body: '',
timestamp: '12-12-2012',
headers: {
'content-type': 'application/json',
Expand Down
10 changes: 6 additions & 4 deletions tests/unit/matchPatientSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('Match Patient Synchronously', (): void => {
response: {
status: 200,
headers: { 'content-type': 'application/json' },
body: { mesage: 'Success' },
body: '',
timestamp: '12/02/1991',
},
},
Expand Down Expand Up @@ -179,7 +179,9 @@ describe('Match Patient Synchronously', (): void => {

expect(handlerResponse.status).to.be.equal(500);
expect(handlerResponse.body.status).to.be.equal('Failed');
expect(handlerResponse.body.response.body).to.deep.equal({ errors: [error] });
expect(JSON.parse(handlerResponse.body.response.body)).to.deep.equal({
errors: [error],
});
stub.restore();
});

Expand Down Expand Up @@ -254,7 +256,7 @@ describe('Match Patient Synchronously', (): void => {
response: {
status: 200,
headers: { 'content-type': 'application/json' },
body: { mesage: 'Success' },
body: '',
timestamp: '12/02/1991',
},
},
Expand Down Expand Up @@ -420,7 +422,7 @@ describe('Match Patient Synchronously', (): void => {
response: {
status: 200,
headers: { 'Content-Type': 'application/json' },
body: { mesage: 'Success' },
body: '',
timestamp: '12/02/1991',
},
},
Expand Down
20 changes: 6 additions & 14 deletions tests/unit/middlewares.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,18 +327,10 @@ describe('Middlewares', (): void => {
nock(mpiUrl).get('/fhir/Patient/1').reply(200, patientFhirResource1);
nock(mpiUrl).get('/fhir/Patient/2').reply(200, patientFhirResource2);
nock(fhirDatastoreUrl)
.get(
`/fhir/Encounter?subject=${encodeURIComponent(
'Patient/1,Patient/2'
)}`
)
.get(`/fhir/Encounter?subject=${encodeURIComponent('Patient/1,Patient/2')}`)
.reply(200, Encounters);
nock(fhirDatastoreUrl)
.get(
`/fhir/Observation?subject=${encodeURIComponent(
'Patient/1,Patient/2'
)}`
)
.get(`/fhir/Observation?subject=${encodeURIComponent('Patient/1,Patient/2')}`)
.reply(200, Observations);

const request = {
Expand All @@ -362,8 +354,8 @@ describe('Middlewares', (): void => {
await mpiMdmEverythingMiddleware(request, response, () => {});
expect(statusCode).to.equal(200);
expect(result.status).to.equal('Success');
expect(result.response.body.total).to.equal(4);
expect(result.response.body.entry.length).to.equal(4);
expect(JSON.parse(result.response.body).total).to.equal(4);
expect(JSON.parse(result.response.body).entry.length).to.equal(4);
nock.cleanAll();
});
});
Expand Down Expand Up @@ -424,8 +416,8 @@ describe('Middlewares', (): void => {
await mpiMdmSummaryMiddleware(request, response, () => {});
expect(statusCode).to.equal(200);
expect(result.status).to.equal('Success');
expect(result.response.body.total).to.equal(2);
expect(result.response.body.entry.length).to.equal(2);
expect(JSON.parse(result.response.body).total).to.equal(2);
expect(JSON.parse(result.response.body).entry.length).to.equal(2);
nock.cleanAll();
});
});
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('Utils', (): void => {
expect(returnedObject.response.headers).to.deep.equal({
'Content-Type': contentType,
});
expect(returnedObject.response.body).to.deep.equal(body);
expect(JSON.parse(returnedObject.response.body)).to.deep.equal(body);
});
});

Expand Down Expand Up @@ -468,7 +468,7 @@ describe('Utils', (): void => {
);

expect(handlerResponse.status).to.equal(200);
expect(handlerResponse.body.response.body).to.deep.equal({
expect(JSON.parse(handlerResponse.body.response.body)).to.deep.equal({
message: 'Success',
});
});
Expand Down

0 comments on commit df0aa93

Please sign in to comment.