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.

Sign up for GitHub

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

Uppercasing UUIDs #89

Merged
merged 3 commits into from
Apr 12, 2024
Merged

Uppercasing UUIDs #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1a1650
Uppercasing UUIDs
chrisbloe Apr 12, 2024
aa7de3b
Uppercasing v4s
chrisbloe Apr 12, 2024
fea6888
[PRMT-4737] added lower case UUID checks for URLs, fixed create-dynam…
AndyFlintAnswerDigital Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion scripts/create-dynamodb-table.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/bin/bash

set -e
echo Creating a table for test in dynamodb-local...
cd "$(dirname "$0")"
Expand Down
83 changes: 43 additions & 40 deletions src/__tests__/app.integration.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,17 @@ describe('app', () => {
});

describe('GET /messages/:conversationId/:messageId', () => {
const conversationId = v4();
const messageId = v4();
const conversationId = v4().toUpperCase();
const messageId = v4().toUpperCase();

it('should return presigned url', async () => {
const response = await request(app)
.get(`/messages/${conversationId}/${messageId}`)
.set('Authorization', authorizationKeys);
expect(response.status).toBe(200);
expect(response.text).toContain(
`${config.localstackUrl}/${config.awsS3BucketName}/${conversationId}/${messageId}`
// UUIDs are going to return as lower case as they're part of a URL
`${config.localstackUrl}/${config.awsS3BucketName}/${conversationId.toLowerCase()}/${messageId.toLowerCase()}`
);
expectStructuredLogToContain(transportSpy, {
messageId,
Expand All @@ -80,9 +81,9 @@ describe('app', () => {
});

describe('GET /fragments/:conversationId/:messageId', () => {
const conversationId = v4();
const coreMessageId = v4();
const fragmentMessageId = v4();
const conversationId = v4().toUpperCase();
const coreMessageId = v4().toUpperCase();
const fragmentMessageId = v4().toUpperCase();
const nhsNumber = '2345678901';

it('should return presigned url when the fragment record exists', async () => {
Expand Down Expand Up @@ -131,7 +132,8 @@ describe('app', () => {
// then
expect(response.status).toBe(200);
expect(response.text).toContain(
`${config.localstackUrl}/${config.awsS3BucketName}/${conversationId}/${fragmentMessageId}`
// UUIDs are going to return as lower case as they're part of a URL
`${config.localstackUrl}/${config.awsS3BucketName}/${conversationId.toLowerCase()}/${fragmentMessageId.toLowerCase()}`
);
expectStructuredLogToContain(transportSpy, {
messageId: fragmentMessageId,
Expand All @@ -142,7 +144,7 @@ describe('app', () => {

it('should return 404 when the fragment record is not found', async () => {
// given
const nonExistentMessageId = uuid();
const nonExistentMessageId = uuid().toUpperCase();

// when
const response = await request(app)
Expand All @@ -158,8 +160,8 @@ describe('app', () => {
describe('GET /patients/:nhsNumber/health-records/:conversationId', () => {
it('should return 200', async () => {
// given
const conversationId = uuid();
const messageId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();
const nhsNumber = '1234567890';

const messageResponse = await request(app)
Expand Down Expand Up @@ -192,9 +194,9 @@ describe('app', () => {

it('should return 404 when health record is not complete', async () => {
// given
const conversationId = uuid();
const messageId = uuid();
const fragmentId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();
const fragmentId = uuid().toUpperCase();
const nhsNumber = '1234567890';

const messageResponse = await request(app)
Expand Down Expand Up @@ -227,7 +229,7 @@ describe('app', () => {

it('should return 404 when health record is not found', async () => {
// given
const conversationId = uuid();
const conversationId = uuid().toUpperCase();
const nhsNumber = '1234567890';

// when
Expand All @@ -252,10 +254,10 @@ describe('app', () => {
it('should return 200 and patient health record link', async () => {
// =============== given ====================
const nhsNumber = '1234567890';
const inboundConversationId = uuid();
const coreMessageId = uuid();
const fragmentId = uuid();
const nestedFragmentId = uuid();
const inboundConversationId = uuid().toUpperCase();
const coreMessageId = uuid().toUpperCase();
const fragmentId = uuid().toUpperCase();
const nestedFragmentId = uuid().toUpperCase();

// mimic the record at Conversation Layer, which should in actual case should be already created by ehr-transfer-service
await createConversationForTest(inboundConversationId, nhsNumber, {
Expand Down Expand Up @@ -312,7 +314,7 @@ describe('app', () => {
.set('Authorization', authorizationKeys);

expect(nestedFragmentResponse.status).toEqual(201);
const outboundConversationId = uuid();
const outboundConversationId = uuid().toUpperCase();

// =============== when ====================
const patientRes = await request(app)
Expand All @@ -323,7 +325,8 @@ describe('app', () => {
// =============== then ====================
expect(patientRes.status).toEqual(200);
expect(patientRes.body.coreMessageUrl).toContain(
`${config.localstackUrl}/${config.awsS3BucketName}/${inboundConversationId}/${coreMessageId}`
// UUIDs are going to return as lower case as they're part of a URL
`${config.localstackUrl}/${config.awsS3BucketName}/${inboundConversationId.toLowerCase()}/${coreMessageId.toLowerCase()}`
);
expect(patientRes.body.fragmentMessageIds).toHaveLength(2);
expect(patientRes.body.fragmentMessageIds).toEqual(
Expand All @@ -338,7 +341,7 @@ describe('app', () => {

it('should have conversation Id in the logging context', async () => {
// given
const outboundConversationId = uuid();
const outboundConversationId = uuid().toUpperCase();
const nhsNumber = '1234567890';

// when
Expand All @@ -356,10 +359,10 @@ describe('app', () => {

it('should return a 404 if no complete health record is found', async () => {
// given
const inboundConversationId = uuid();
const outboundConversationId = uuid();
const coreMessageId = uuid();
const fragmentId = uuid();
const inboundConversationId = uuid().toUpperCase();
const outboundConversationId = uuid().toUpperCase();
const coreMessageId = uuid().toUpperCase();
const fragmentId = uuid().toUpperCase();
const nhsNumber = '1234567891';

// mimic the record at Conversation Layer, which should in actual case should be already created by ehr-transfer-service
Expand Down Expand Up @@ -402,8 +405,8 @@ describe('app', () => {
let conversationId;
let messageId;
beforeEach(() => {
messageId = uuid();
conversationId = uuid();
messageId = uuid().toUpperCase();
conversationId = uuid().toUpperCase();
createConversationForTest(conversationId, nhsNumber, {
TransferStatus: ConversationStatus.STARTED
});
Expand Down Expand Up @@ -441,7 +444,7 @@ describe('app', () => {

it('should save health record with fragments in the database and return 201', async () => {
// given
const fragmentMessageId = uuid();
const fragmentMessageId = uuid().toUpperCase();

// when
const response = await request(app)
Expand All @@ -467,8 +470,8 @@ describe('app', () => {

it('should create large nested fragment messages in the database and return 201', async () => {
// given
const fragmentMessageId = uuid();
const nestedFragmentMessageId = uuid();
const fragmentMessageId = uuid().toUpperCase();
const nestedFragmentMessageId = uuid().toUpperCase();

// when
await request(app)
Expand Down Expand Up @@ -499,7 +502,7 @@ describe('app', () => {

it('should update database for fragments and return 201 when all fragments have been received', async () => {
// given
const fragmentMessageId = uuid();
const fragmentMessageId = uuid().toUpperCase();

// when
await request(app)
Expand Down Expand Up @@ -528,8 +531,8 @@ describe('app', () => {

it('should update database when a nested fragment arrives before its parent fragment', async () => {
// given
const fragmentMessageId = uuid();
const nestedFragmentId = uuid();
const fragmentMessageId = uuid().toUpperCase();
const nestedFragmentId = uuid().toUpperCase();

// when
await request(app)
Expand Down Expand Up @@ -607,9 +610,9 @@ describe('app', () => {

it('should mark the whole health record for the given NHS number as deleted', async () => {
// ============================= given ==================================
const inboundConversationId = uuid();
const coreMessageId = uuid();
const fragmentMessageIds = [uuid(), uuid(), uuid()];
const inboundConversationId = uuid().toUpperCase();
const coreMessageId = uuid().toUpperCase();
const fragmentMessageIds = [uuid().toUpperCase(), uuid().toUpperCase(), uuid().toUpperCase()];
await createCompleteRecord(
nhsNumber,
inboundConversationId,
Expand Down Expand Up @@ -661,10 +664,10 @@ describe('app', () => {

it('should delete all record if patient has more than one set of record in our storage', async () => {
// given
const inboundConversationId1 = uuid();
const coreMessageId1 = uuid();
const inboundConversationId2 = uuid();
const coreMessageId2 = uuid();
const inboundConversationId1 = uuid().toUpperCase();
const coreMessageId1 = uuid().toUpperCase();
const inboundConversationId2 = uuid().toUpperCase();
const coreMessageId2 = uuid().toUpperCase();
await createCompleteRecord(nhsNumber, inboundConversationId1, coreMessageId1);

await new Promise((resolve) => setTimeout(resolve, 1500)); // time buffer to ensure record 2 get a newer timestamp
Expand Down
12 changes: 6 additions & 6 deletions src/api/fragments/__tests__/get-fragment-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ describe('getFragmentController', () => {
describe('success', () => {
it('should return a 200 with presigned url in body', async () => {
// given
const conversationId = uuid();
const messageId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();
const presignedUrl = 'presigned-url';

// when
Expand All @@ -45,8 +45,8 @@ describe('getFragmentController', () => {

describe('error', () => {
// given
const conversationId = uuid();
const messageId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();

it('should return a 404 error when the record is not found', async () => {
// when
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('getFragmentController', () => {
it('should return 422 and an error message when conversationId is not a UUID', async () => {
// given
const conversationId = 'not-a-uuid';
const messageId = uuid();
const messageId = uuid().toUpperCase();
const expectedErrorMessage = [{ conversationId: "'conversationId' provided is not a UUID" }];

// when
Expand All @@ -102,7 +102,7 @@ describe('getFragmentController', () => {

it('should return 422 and an error message when messageId is not a UUID', async () => {
// given
const conversationId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = 'not-a-uuid';
const errorMessage = [{ messageId: "'messageId' provided is not a UUID" }];

Expand Down
16 changes: 8 additions & 8 deletions src/api/messages/__tests__/message-location-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ describe('messageLocationController', () => {
const authorizationKeys = 'correct-key';

describe('success', () => {
const conversationId = uuid();
const messageId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();

it('should return a 200 with presigned url in body', async () => {
getSignedUrl.mockResolvedValue('presigned-url');
Expand All @@ -39,8 +39,8 @@ describe('messageLocationController', () => {
});

describe('conflict', () => {
const conversationId = uuid();
const messageId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();

it('should return a 409 when ehr already exists', async () => {
fragmentAlreadyReceived.mockResolvedValueOnce(true);
Expand All @@ -56,8 +56,8 @@ describe('messageLocationController', () => {
});

describe('error', () => {
const conversationId = uuid();
const messageId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = uuid().toUpperCase();

it('should return a 503 when getSignedUrl promise is rejected', async () => {
const error = new Error('error');
Expand All @@ -77,7 +77,7 @@ describe('messageLocationController', () => {
describe('validation', () => {
it('should return 422 and an error message when conversationId is not a UUID', async () => {
const conversationId = 'not-a-uuid';
const messageId = uuid();
const messageId = uuid().toUpperCase();
const errorMessage = [{ conversationId: "'conversationId' provided is not a UUID" }];

const res = await request(app)
Expand All @@ -91,7 +91,7 @@ describe('messageLocationController', () => {
});

it('should return 422 and an error message when messageId is not a UUID', async () => {
const conversationId = uuid();
const conversationId = uuid().toUpperCase();
const messageId = 'not-a-uuid';
const errorMessage = [{ messageId: "'messageId' provided is not a UUID" }];

Expand Down
20 changes: 10 additions & 10 deletions src/api/messages/__tests__/store-message-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,9 @@ describe('storeMessageController', () => {
const requestBody = {
data: {
type: 'messages',
id: uuid(),
id: uuid().toUpperCase(),
attributes: {
conversationId: uuid(),
conversationId: uuid().toUpperCase(),
nhsNumber: 'not-an-nhs-number',
messageType: 'ehrExtract'
}
Expand Down Expand Up @@ -273,9 +273,9 @@ describe('storeMessageController', () => {
const requestBody = {
data: {
type: 'messages',
id: uuid(),
id: uuid().toUpperCase(),
attributes: {
conversationId: uuid(),
conversationId: uuid().toUpperCase(),
nhsNumber: '1234567890',
messageType: 'ehrExtract',
fragmentMessageIds: []
Expand Down Expand Up @@ -317,9 +317,9 @@ describe('storeMessageController', () => {
const requestBody = {
data: {
type: 'messages',
id: uuid(),
id: uuid().toUpperCase(),
attributes: {
conversationId: uuid(),
conversationId: uuid().toUpperCase(),
nhsNumber: '1234567890',
messageType: 'ehrExtract',
fragmentMessageIds: []
Expand Down Expand Up @@ -361,9 +361,9 @@ describe('storeMessageController', () => {
const requestBody = {
data: {
type: 'messages',
id: uuid(),
id: uuid().toUpperCase(),
attributes: {
conversationId: uuid(),
conversationId: uuid().toUpperCase(),
messageType: 'fragment',
fragmentMessageIds: []
}
Expand Down Expand Up @@ -427,9 +427,9 @@ describe('storeMessageController', () => {
const requestBody = {
data: {
type: 'messages',
id: uuid(),
id: uuid().toUpperCase(),
attributes: {
conversationId: uuid(),
conversationId: uuid().toUpperCase(),
nhsNumber: '1234567890',
messageType: 'ehrExtract'
}
Expand Down
2 changes: 1 addition & 1 deletion src/api/patients/__tests__/delete-ehr-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('deleteEhrController', () => {

describe('success', () => {
it('should return 200 when controller invoked correctly', async () => {
const conversationIds = [uuid(), uuid(), uuid()];
const conversationIds = [uuid().toUpperCase(), uuid().toUpperCase(), uuid().toUpperCase()];
markRecordAsSoftDeleteForPatient.mockResolvedValue(conversationIds);

const res = await request(app)
Expand Down
Loading
Loading