Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

test: add tests for tenant isolation of subscriptions #577

Merged
merged 5 commits into from
Feb 24, 2022
Merged

test: add tests for tenant isolation of subscriptions #577

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
92b6222
test: add tests for tenant isolation of subscriptions
ssvegaraju Feb 24, 2022
c700773
only test if multi tenancy is enabled
ssvegaraju Feb 24, 2022
9002207
fix linting issues
ssvegaraju Feb 24, 2022
0b40507
replace header with env key
ssvegaraju Feb 24, 2022
f9095d6
refactor subscription creation
ssvegaraju Feb 24, 2022
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
79 changes: 56 additions & 23 deletions integration-tests/subscriptions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,15 @@

import * as AWS from 'aws-sdk';
import { AxiosInstance } from 'axios';
import { clone } from 'lodash';
import waitForExpect from 'wait-for-expect';
import { SubscriptionsHelper } from './SubscriptionsHelper';
import { getFhirClient } from './utils';

jest.setTimeout(700_000);

const {
SUBSCRIPTIONS_ENABLED,
SUBSCRIPTIONS_NOTIFICATIONS_TABLE,
// eslint-disable-next-line @typescript-eslint/no-unused-vars
SUBSCRIPTIONS_ENDPOINT,
API_AWS_REGION,
} = process.env;
const { SUBSCRIPTIONS_ENABLED, SUBSCRIPTIONS_NOTIFICATIONS_TABLE, SUBSCRIPTIONS_ENDPOINT, API_AWS_REGION } =
process.env;

if (API_AWS_REGION === undefined) {
throw new Error('API_AWS_REGION environment variable is not defined');
Expand All @@ -31,44 +27,81 @@ test('empty test placeholder', () => {
});

if (SUBSCRIPTIONS_ENABLED === 'true') {
if (!SUBSCRIPTIONS_ENDPOINT) {
throw new Error('SUBSCRIPTIONS_ENDPOINT environment variable is not defined');
}
let client: AxiosInstance;

const subscriptionResource = {
resourceType: 'Subscription',
status: 'requested',
// get a time 10 seconds (10000 ms) in the future
end: new Date(new Date().getTime() + 10000).toISOString(),
reason: 'Monitor Patients with name Smith',
criteria: 'Patient?name=Smith',
channel: {
type: 'rest-hook',
endpoint: SUBSCRIPTIONS_ENDPOINT!,
ssvegaraju marked this conversation as resolved.
Show resolved Hide resolved
payload: 'application/fhir+json',
header: [`x-api-key: ${process.env.SUBSCRIPTIONS_API_KEY}`],
ssvegaraju marked this conversation as resolved.
Show resolved Hide resolved
},
};

describe('FHIR Subscriptions', () => {
let subscriptionsHelper: SubscriptionsHelper;

beforeAll(() => {
beforeAll(async () => {
if (SUBSCRIPTIONS_NOTIFICATIONS_TABLE === undefined) {
throw new Error('SUBSCRIPTIONS_NOTIFICATIONS_TABLE environment variable is not defined');
}
subscriptionsHelper = new SubscriptionsHelper(SUBSCRIPTIONS_NOTIFICATIONS_TABLE);
client = await getFhirClient();
});

test('test', async () => {
const x = await subscriptionsHelper.getNotifications('/lala');
console.log(x);
});

if (process.env.MULTI_TENANCY_ENABLED === 'true') {
test('tenant isolation', async () => {
const clientAnotherTenant = await getFhirClient({ tenant: 'tenant2' });
// tenant 1 creates a subscription
const subResource = clone(subscriptionResource);
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to create a function that generates Subscriptions instead of cloning the same one over and over. An example function to generate test Patients is:

fhir-works-on-aws-deployment/integration-tests/utils.ts

Line 137 in 401e97d

export const randomPatient = () => {

I know some tests do the clone stuff, but it's not a good practice and we try not to d that anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, I will avoid clone in the future. For now I've created a rather basic method to create subscriptions that takes in a uuid in utils.ts

// make sure the end date isn't caught by the reaper before the test completes
subResource.end = new Date(new Date().getTime() + 100000).toISOString();
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest increasing time to at least 10 minutes (600_000). There's no need for it to be such a tight time window

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made this the default time window for subscriptions created in utils.ts, and just changed the end time for the one test that needed things to expire quickly.

const postResult = await client.post('Subscription', subscriptionResource);
expect(postResult.status).toEqual(201);
const resourceThatMatchesSubscription = {
resourceType: 'Patient',
name: [
{
given: ['Smith'],
family: 'Smith',
},
],
};
// post matching resource on another tenant
const postPatientResult = await clientAnotherTenant.post('Patient', resourceThatMatchesSubscription);
expect(postPatientResult.status).toEqual(201);
// give SLA of 20 seconds for notification to be placed in ddb table
await new Promise((r) => setTimeout(r, 20000));
// make sure no notification was receieved for first tenant
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to wait more time. Subscriptions may take up to a minute to actually start sending notifications(due to the cache in the SubscriptionMatcher), also, we need leniency on the SLA of 20 seconds (a system with steady traffic sends notifications well below 20 seconds, but with very little traffic (like integ tests envs) subscriptions face cold starts + batching windows in stream/sqs processing that make them slower)

I'd say update it to 2 minutes waiting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to 2 min!

const notifications = await subscriptionsHelper.getNotifications(
`/Patient/${postPatientResult.data.id}`,
Copy link
Contributor

@carvantes carvantes Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This can get mixed up with Subscriptions created by other tests.

I suggest you add a UUID to the endpoint in all test Subscriptions, so that you can lookup in DDB for the path with the exact same UUID.

endpoint: `${SUBSCRIPTIONS_ENDPOINT}/<uuid>`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used the uuid package to generate uuids for each test that generates subscriptions so no test interferes with another.

);
expect(notifications).toEqual([]);
});
}
});

let client: AxiosInstance;
describe('test subscription creation and deletion', () => {
beforeAll(async () => {
client = await getFhirClient();
});

test('creation of almost expiring subscription should be deleted by reaper', async () => {
// OPERATE
const subscriptionResource = {
resourceType: 'Subscription',
status: 'requested',
// get a time 10 seconds (10000 ms) in the future
end: new Date(new Date().getTime() + 10000).toISOString(),
reason: 'Monitor Patients with name Smith',
criteria: 'Patient?name=Smith',
channel: {
type: 'rest-hook',
endpoint: 'https://customer-endpoint.com',
payload: 'application/fhir+json',
header: ['Authorization: Bearer secret-token-abc-123'],
},
};
const postSubResult = await client.post('Subscription', subscriptionResource);
expect(postSubResult.status).toEqual(201); // ensure that the sub resource is created
const subResourceId = postSubResult.data.id;
Expand Down