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

feat(#9237): add functionality of getting people with pagination in cht-datasource #9266

Merged
merged 20 commits into from
Aug 9, 2024
Merged

feat(#9237): add functionality of getting people with pagination in cht-datasource #9266

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
08e2b27
feat(#9237): Add functionality of getting people with pagination in c…
sugat009 Jul 15, 2024
ab92f0b
Update shared-libs/cht-datasource/src/local/libs/doc.ts
sugat009 Jul 17, 2024
de63197
feat(#9237): Address PR comments
sugat009 Jul 19, 2024
105d5db
Update shared-libs/cht-datasource/src/qualifier.ts
sugat009 Jul 23, 2024
2b92b4c
Update shared-libs/cht-datasource/test/local/person.spec.ts
sugat009 Jul 23, 2024
ba2e27a
Update shared-libs/cht-datasource/src/person.ts
sugat009 Jul 23, 2024
15068fa
Update shared-libs/cht-datasource/src/remote/person.ts
sugat009 Jul 23, 2024
a83a259
Update shared-libs/cht-datasource/src/person.ts
sugat009 Jul 23, 2024
29a3ad5
Update shared-libs/cht-datasource/src/index.ts
sugat009 Jul 23, 2024
8fbc245
feat(#9237): Address PR comments
sugat009 Jul 23, 2024
a99de72
feat(#9237): Add unit tests for getResources in remote mode
sugat009 Jul 23, 2024
d87c258
feat(#9237): Address PR comments
sugat009 Jul 24, 2024
eceeb75
(#feat): Minor fix
sugat009 Jul 24, 2024
1aab0e3
Update shared-libs/cht-datasource/src/person.ts
sugat009 Jul 26, 2024
54d04cb
Update shared-libs/cht-datasource/test/local/libs/lineage.spec.ts
sugat009 Jul 26, 2024
dcc3758
Update shared-libs/cht-datasource/test/local/libs/lineage.spec.ts
sugat009 Jul 26, 2024
e800b1a
Update shared-libs/cht-datasource/test/local/person.spec.ts
sugat009 Jul 26, 2024
34b466c
feat(#9237): Address PR comments
sugat009 Jul 26, 2024
4facbad
Implement cursor based pagination
sugat009 Aug 6, 2024
33d13fd
Fix unit tests according to implementation of cursor pagination
sugat009 Aug 6, 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
13 changes: 8 additions & 5 deletions shared-libs/cht-datasource/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,14 @@ export const getDatasource = (ctx: DataContext) => {
getByUuidWithLineage: (uuid: string) => ctx.bind(Person.v1.getWithLineage)(Qualifier.byUuid(uuid)),

/**
* Returns a list of people.
* @param personType the string that represents the person type
* @param limit the total number of records to retrieve
* @param skip the total number of records to skip
* @returns array of `Person`
* Returns an array of people for the provided page specifications.
* @param personType the type of people to return
* @param limit the maximum number of people to return. Default is 100.
* @param skip the number of people to skip. Default is 0.
* @returns an array of people for the provided page specifications
* @throws Error if no type is provided or if the type is not for a person
* @throws Error if the provided limit is `<= 0`
* @throws Error if the provided skip is `< 0`
*/
getPage: (personType: string, limit = 100, skip = 0) => ctx.bind(Person.v1.getPage)(
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
Qualifier.byContactType(personType), limit, skip
Expand Down
6 changes: 1 addition & 5 deletions shared-libs/cht-datasource/src/local/libs/lineage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ import logger from '@medic/logger';
* sorted such that the identified document is the first element and the parent documents are in order of lineage.
* @internal
*/
export const getLineageDocsById = (
medicDb: PouchDB.Database<Doc>
): (
id: string
) => Promise<Nullable<Doc>[]> => {
export const getLineageDocsById = (medicDb: PouchDB.Database<Doc>): (id: string) => Promise<Nullable<Doc>[]> => {
const fn = queryDocsByRange(medicDb, 'medic-client/docs_by_id_lineage');
return (id: string) => fn([id], [id, {}]);
};
Expand Down
18 changes: 9 additions & 9 deletions shared-libs/cht-datasource/src/local/person.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { getLineageDocsById, getPrimaryContactIds, hydrateLineage, hydratePrimar

/** @internal */
export namespace v1 {
const isPerson = (settings: SettingsService, uuid: string, doc: Nullable<Doc>): doc is Person.v1.Person => {
const isPerson = (settings: SettingsService, doc: Nullable<Doc>, uuid = ''): doc is Person.v1.Person => {
if (!doc) {
logger.warn(`No person found for identifier [${uuid}].`);
return false;
Expand All @@ -28,7 +28,7 @@ export namespace v1 {
const getMedicDocById = getDocById(medicDb);
return async (identifier: UuidQualifier): Promise<Nullable<Person.v1.Person>> => {
const doc = await getMedicDocById(identifier.uuid);
if (!isPerson(settings, identifier.uuid, doc)) {
if (!isPerson(settings, doc, identifier.uuid)) {
return null;
}
return doc;
Expand All @@ -41,7 +41,7 @@ export namespace v1 {
const getMedicDocsById = getDocsByIds(medicDb);
return async (identifier: UuidQualifier): Promise<Nullable<Person.v1.PersonWithLineage>> => {
const [person, ...lineagePlaces] = await getLineageDocs(identifier.uuid);
if (!isPerson(settings, identifier.uuid, person)) {
if (!isPerson(settings, person, identifier.uuid)) {
return null;
}
// Intentionally not further validating lineage. For passivity, lineage problems should not block retrieval.
Expand All @@ -61,19 +61,19 @@ export namespace v1 {

/** @internal */
export const getPage = ({ medicDb, settings }: LocalDataContext) => {
const personTypes = contactTypeUtils.getPersonTypes(settings.getAll());
const personTypesIds = personTypes.map(item => item.id);

const getDocsByPage = queryDocsByKey(medicDb, 'medic-client/contacts_by_type');

return async (personType: ContactTypeQualifier, limit: number, skip: number): Promise<Person.v1.Person[]> => {
const personTypes = contactTypeUtils.getPersonTypes(settings.getAll());
const personTypesIds = personTypes.map(item => item.id);

if (!personTypesIds.includes(personType.contactType)) {
throw new Error(`Invalid person type: ${personType.contactType}`);
}

const getDocsByPage = queryDocsByKey(medicDb, 'medic-client/contacts_by_type');

const docs = await getDocsByPage([personType.contactType], limit, skip);

return docs.filter((doc): doc is Person.v1.Person => isPerson(settings, doc?._id ?? '', doc));
return docs.filter((doc): doc is Person.v1.Person => isPerson(settings, doc, doc?._id));
Copy link
Contributor

Choose a reason for hiding this comment

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

@m5r @sugat009 I have realized a complication here while looking at the code over in #9281. Basically, we pass the limit through to the Couch view query and so we know docs.length === limit or we have reached the last page of results. However, before this function returns the persons, we are filtering the array. In theory it is possible that some of the results returned from queryDocsByKey could get filtered out (with our current validation logic I cannot come up with a feasible scenario where this could happen, but if/when we add more validations, this will become an issue).

The reason I think this filtering is a problem is because it could result in us giving back an array for the page that is < limit when there are still additional results to retrieve. Of course, this is bad for our getAll logic, but it will also probably be confusing to any other consumers of this code. (How would anyone know when to stop calling getPage?).

The only feasible option I can come up with here is to change the return type to Promise<Nullable<Person.v1.Person>[]> and just pass through null when a returned doc fails the isPerson check. If we try to do anything more fancy, like calling the queryDocsByKey again until we get the full limit of persons, it is going to mess up the skip value for any subsequent calls from the consumer... The only comfort I have here is that we do not really want folks directly calling this function anyway (and the getAll can still abstract away the Nullable). The REST api is going to be a bit janky, but I just don't see any other way around it.

Do either of you have other thoughts here? Ideas on better options?

Copy link
Member Author

@sugat009 sugat009 Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TBH, I'm surprised that we even need a isPerson validator since we explicitly pass a valid personType to the database. Anyways, @jkuester is right, if in some way an entry is filtered by the isPerson function then the sanctity of the pagination would be disturbed, so let's have a null value in the place of the non-person doc.

As an alternative approach, instead of setting the document entry to null, we could introduce an invalid property or similar indicator. This would signify that the person document contains incorrect or problematic data, without completely removing the entry and let the consumers decide what to do with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised that we even need a isPerson validator since we explicitly pass a valid personType to the database

This is true. I have gone back and forth on if we should keep the isPerson check since all it is doing currently is validating the contact type (which, in theory should always be correct since that is what we are querying one...). However, regardless of if we need to do any checks like this currently, I do think it is very important that we design these API patterns in such a way that we can filter data we get from Couch before returning it to the user. One of the main points of having cht-datasource is that not everything needs to be modeled directly in the db.... With that in mind, and seeing as the performance cost of isPerson is minimal, it makes sense to me to keep it for the sake of code consistency (and better support for future validations).

the sanctity of the pagination would be disturbed

Quote of the day! 🔥

we could introduce an invalid property or similar indicator

This is an interesting idea! 🤔 Similar to the Pouch API which returns the row objects with id value, etc, but may or may not actually have a doc. Ultimately, though, it seems like it might not actually be useful to consumers of this API (and anyone trying to do data validation will probably want to connect directly to the database).

Another approach we could take here would be to return some kind of page object like this:

Promise<{ entries: Person.v1.Person[], cursor: string }>

This is similar to the paging in GitHub's GraphQL api, where each request returns a different cursor that you can pass back in on subsequent requests to get the next page. In our case we could just return the proper skip value for the next Couch query, but the meaning/value of the cursor should be completely abstracted away from consumers of the cht-datasource api. Consumers can just treat it as a token string for getting the next page. That would give us flexibility in the future if we needed to change up some underlying details of how things work. In the context of the above discussion regarding isPerson, this approach would let us run followup queries to populate a full list of persons to return so that entries.length === limit while returning the proper skip value for the next page request as the cursor.

I don't know guys, the more I think about this approach, the more I like it! It decouples us more from the underlying Couch implementation, which I think is a win, giving us more flexibility for future innovation....

Copy link
Contributor

Choose a reason for hiding this comment

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

One variation on this new page object proposal would be to include a hasNextPage: boolean in the returned page object so that consumers know when more data is available. Then, we could clarify that the provided limit would be the max value possible for entries, (but some pages might be returned with less). This would free us from having to implement any kind of followup queries in the getPage logic (at least at this time). We could run the Couch query, do any necessary filtering, and return what is left.

Copy link
Member Author

@sugat009 sugat009 Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@jkuester Cursor-based pagination sounds great and would help if any record is filtered by the isPerson or any future filters. As for the implementation, should we do it via:

  1. overfetching method where we fetch more docs than the limit specifies such that if records are filtered out then, the over-fetched docs can fill in
  2. re-fetching method where we re-query the database with the amount of docs that were filtered out.

Usually, it is said to be better to minimize the no. of queries to the database which is even more true in our case. 😃 So, my vote is in the first method. What do you think?

Looping in @m5r .

Copy link
Member Author

Choose a reason for hiding this comment

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

I read through the linked sites by @m5r and tried a bit on our couchdb view(medic-client/contacts_by_type). The startkey as a cursor approach does not seem to work for the view as all the keys are person types. We probably need to create a new view designed specifically for pagination.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have not tried it yet, but at the end of that Couch guide, it notes that for views where the key is not unique, you will need to use a combination of startkey, startkey_docid, and limit. The startkey_docid is what will actual do the heavy lifting for our contacts_by_type query since we are not actually looking for a range of keys...

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point Josh, the fact that the key is the same is what enables couch to do the pagination here. I gave it a try locally and it should do exactly what we need:

Querying the view for persons:

$ curl "http://admin:password@localhost:5984/medic/_design/medic-client/_view/contacts_by_type?key=\[\"person\"\]"
{"total_rows":6,"offset":3,"rows":[
{"id":"3e93a66c-06c9-4766-b68b-eaec896ee935","key":["person"],"value":"false false 3 grootchw"},
{"id":"6892b0eb-7398-47a9-8e92-d40d3012f9f6","key":["person"],"value":"false false 3 grootvisor"},
{"id":"de324069-da24-4ffa-89f3-f7f67b859d70","key":["person"],"value":"false false 3 grootpatient"}
]}

Then using the person with id 6892b0eb-7398-47a9-8e92-d40d3012f9f6 as cursor:

$ curl "http://admin:password@localhost:5984/medic/_design/medic-client/_view/contacts_by_type?key=\[\"person\"\]&startkey_docid=6892b0eb-7398-47a9-8e92-d40d3012f9f6"
{"total_rows":6,"offset":4,"rows":[
{"id":"6892b0eb-7398-47a9-8e92-d40d3012f9f6","key":["person"],"value":"false false 3 grootvisor"},
{"id":"de324069-da24-4ffa-89f3-f7f67b859d70","key":["person"],"value":"false false 3 grootpatient"}
]}

Then limiting the number of rows that I get back:

$ curl "http://admin:password@localhost:5984/medic/_design/medic-client/_view/contacts_by_type?key=\[\"person\"\]&startkey_docid=6892b0eb-7398-47a9-8e92-d40d3012f9f6&limit=1"
{"total_rows":6,"offset":4,"rows":[
{"id":"6892b0eb-7398-47a9-8e92-d40d3012f9f6","key":["person"],"value":"false false 3 grootvisor"}
]}

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahhh! Gotcha. I was under the impression that the key also needed to be a uuid. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, so just recapping further discussion from the Slack Thread, it turns out that Pouch does not have any support for the startkey_docid parameter. 😓 Also from reading the Couch docs it seems like the performance for skip significantly improved after Couch 1.2 (I guess the guide linked above was old?). The Pouch Docs still note that skip has "poor performance on IndexedDB/LevelDB", but there does not seem to be any other way to actually page queries with Pouch, so... 🤷

So the decision is to stick with just using skip to page the queries for now. We should still do the refactor to return the skip value as a generic cursor to consumers instead of actually exposing skip. That will let us passively refactor our implementation once pouchdb/pouchdb#7326 is completed.

};
};
}
34 changes: 28 additions & 6 deletions shared-libs/cht-datasource/src/person.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,23 @@ export namespace v1 {
}
};

const assertTypeQualifier: (
const assertTypeQualifier: (qualifier: unknown) => asserts qualifier is ContactTypeQualifier = (
qualifier: unknown
) => asserts qualifier is ContactTypeQualifier = (qualifier: unknown) => {
) => {
if (!isContactTypeQualifier(qualifier)) {
throw new Error(`Invalid type [${JSON.stringify(qualifier)}].`);
}
};

const assertLimitAndSkip = (limit: unknown, skip: unknown) => {
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
if (typeof limit !== 'number' || limit <= 0) {
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('limit must be a positive number');
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
}
if (typeof skip !== 'number' || skip < 0) {
throw new Error('skip must be a non-negative number');
}
};

const getPerson = <T>(
localFn: (c: LocalDataContext) => (qualifier: UuidQualifier) => Promise<T>,
remoteFn: (c: RemoteDataContext) => (qualifier: UuidQualifier) => Promise<T>
Expand All @@ -59,10 +68,23 @@ export namespace v1 {
assertDataContext(context);
const fn = adapt(context, localFn, remoteFn);

return async (personType: ContactTypeQualifier, limit = 100, skip = 0): Promise<T> => {
/**
* Returns an array of people for the provided page specifications.
* @param personType the type of people to return
* @param limit the maximum number of people to return. Default is 100.
* @param skip the number of people to skip. Default is 0.
* @returns an array of people for the provided page specifications.
* @throws Error if `personType` qualifier is invalid
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
* @throws Error if the provided `limit` value is `<=0`
* @throws Error if the provided `skip` value is `<0`
*/
const curriedFn = async (personType: ContactTypeQualifier, limit = 100, skip = 0): Promise<T> => {
assertTypeQualifier(personType);
assertLimitAndSkip(limit, skip);

return fn(personType, limit, skip);
};
jkuester marked this conversation as resolved.
Show resolved Hide resolved
return curriedFn;
};

/**
Expand All @@ -82,10 +104,10 @@ export namespace v1 {
export const getWithLineage = getPerson(Local.Person.v1.getWithLineage, Remote.Person.v1.getWithLineage);

/**
* Returns an array of people.
* Returns a function for retrieving a paged array of people from the given data context.
* @param context the current data context
* @returns an array of people
* @throws Error if the provided context or personType qualifier is invalid
* @returns a function for retrieving a paged array of people
* @throws Error if a data context is not provided
*/
export const getPage = getPeople(Local.Person.v1.getPage, Remote.Person.v1.getPage);
}
2 changes: 1 addition & 1 deletion shared-libs/cht-datasource/src/qualifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const isUuidQualifier = (identifier: unknown): identifier is UuidQualifie
};

/**
* A qualifier that identifies an entity based on type
A qualifier that identifies contacts based on type.
*/
export type ContactTypeQualifier = Readonly<{ contactType: string }>;

Expand Down
2 changes: 1 addition & 1 deletion shared-libs/cht-datasource/src/remote/libs/data-context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const getResources = (context: RemoteDataContext, path: string) => async
): Promise<T> => {
const params = new URLSearchParams(queryParams).toString();
try {
const response = await fetch(`${context.url}/${path}}?${params}`);
const response = await fetch(`${context.url}/${path}?${params}`);
if (!response.ok) {
throw new Error(response.statusText);
}
Expand Down
2 changes: 1 addition & 1 deletion shared-libs/cht-datasource/src/remote/person.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export namespace v1 {
personType: ContactTypeQualifier,
limit: number,
skip: number
): Promise<null> => getPeople(remoteContext)(
): Promise<Person.v1.Person[]> => getPeople(remoteContext)(
{'limit': limit.toString(), 'skip': skip.toString(), 'contactType': personType.contactType}
);
}
2 changes: 1 addition & 1 deletion shared-libs/cht-datasource/test/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ describe('CHT Script API - getDatasource', () => {
});

it('getPage', async () => {
const expectedPeople: Index.Nullable<Doc>[] = [];
const expectedPeople: Person.v1.Person[] = [];
const personGetPage = sinon.stub().resolves(expectedPeople);
dataContextBind.returns(personGetPage);
const personType = 'person';
Expand Down
30 changes: 23 additions & 7 deletions shared-libs/cht-datasource/test/local/libs/doc.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ describe('local doc lib', () => {
});
isDoc.returns(true);

const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc0._id);
const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc1._id);

expect(result).to.deep.equal([doc0, doc1, doc2]);

expect(dbQuery.calledOnceWithExactly('medic-client/docs_by_id_lineage', {
include_docs: true,
startkey: doc0._id,
endkey: doc0._id
endkey: doc1._id
})).to.be.true;
expect(isDoc.args).to.deep.equal([[doc0], [doc1], [doc2]]);
});
Expand All @@ -187,12 +187,12 @@ describe('local doc lib', () => {
});
isDoc.returns(true);

const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc0._id);
const result = await queryDocsByRange(db, 'medic-client/docs_by_id_lineage')(doc0._id, doc2._id);

expect(result).to.deep.equal([doc0, null, doc2]);
expect(dbQuery.calledOnceWithExactly('medic-client/docs_by_id_lineage', {
startkey: doc0._id,
endkey: doc0._id,
endkey: doc2._id,
include_docs: true
})).to.be.true;
expect(isDoc.args).to.deep.equal([[doc0], [null], [doc2]]);
Expand Down Expand Up @@ -249,22 +249,38 @@ describe('local doc lib', () => {
});

it('returns empty array if docs are not found', async () => {
dbQuery.resolves({ rows: [] });
isDoc.returns(true);

const result = await queryDocsByKey(db, 'medic-client/contacts_by_type')(contactType, limit, skip);

expect(result).to.deep.equal([]);
expect(dbQuery.calledOnceWithExactly('medic-client/contacts_by_type', {
include_docs: true, key: contactType, limit, skip
})).to.be.true;
expect(isDoc.args).to.deep.equal([]);
});

it('returns null valued array if rows from database are not docs', async () => {
const doc0 = { _id: 'doc0' };

dbQuery.resolves({
rows: [
{ doc: doc0 },
]
});
isDoc.returns(true);
isDoc.returns(false);

const result = await queryDocsByKey(db, 'medic-client/contacts_by_type')(contactType, limit, skip);

expect(result).to.deep.equal([]);
expect(result).to.deep.equal([null]);
expect(dbQuery.calledOnceWithExactly('medic-client/contacts_by_type', {
include_docs: true,
key: contactType,
limit,
skip
})).to.be.true;
expect(isDoc.args).to.deep.equal([]);
expect(isDoc.args).to.deep.equal([[doc0]]);
});
});
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
});
4 changes: 2 additions & 2 deletions shared-libs/cht-datasource/test/local/libs/lineage.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ describe('local lineage lib', () => {
beforeEach(() => {
debug = sinon.stub(logger, 'debug');
});

//
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
afterEach(() => sinon.restore());

it('getLineageDocsById', async () => {
const uuid = '123';
const queryFn = sinon.stub().returns(Promise.resolve([]));
const queryFn = sinon.stub().returns([]);
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
const queryDocsByRange = sinon
.stub(LocalDoc, 'queryDocsByRange')
.returns(queryFn);
Expand Down
35 changes: 29 additions & 6 deletions shared-libs/cht-datasource/test/local/person.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,27 +252,28 @@ describe('local person', () => {
const res = await Person.v1.getPage(localContext)(personTypeQualifier, limit, skip);

expect(res).to.deep.equal(docs);
expect(settingsGetAll.callCount === 4).to.be.true;
expect(settingsGetAll.callCount).to.equal(4);
expect(getPersonTypes.calledOnceWithExactly(settings)).to.be.true;
expect(
queryDocsByKeyOuter.calledOnceWithExactly(localContext.medicDb, 'medic-client/contacts_by_type')
).to.be.true;
expect(queryDocsByKeyInner.calledOnceWithExactly([personIdentifier], limit, skip)).to.be.true;
expect(isPerson.callCount).to.equal(3);
expect(isPerson.getCall(0).args).to.deep.equal([settings, doc]);
expect(isPerson.getCall(1).args).to.deep.equal([settings, doc]);
expect(isPerson.getCall(2).args).to.deep.equal([settings, doc]);
});
sugat009 marked this conversation as resolved.
Show resolved Hide resolved

it('throws an error if person identifier is invalid/does not exist', async () => {
queryDocsByKeyInner.resolves([]);

await expect(Person.v1.getPage(localContext)(invalidPersonTypeQualifier, limit, skip)).to.be.rejectedWith(
`Invalid person type: ${invalidPersonTypeQualifier.contactType}`
);

expect(settingsGetAll.calledOnce).to.be.true;
expect(getPersonTypes.calledOnceWithExactly(settings)).to.be.true;
expect(queryDocsByKeyOuter.calledOnceWithExactly(
localContext.medicDb, 'medic-client/contacts_by_type'
)).to.be.true;
expect(queryDocsByKeyOuter.notCalled).to.be.true;
expect(queryDocsByKeyInner.notCalled).to.be.true;
expect(isPerson.notCalled).to.be.true;
});

it('returns empty array if people does not exist', async () => {
Expand All @@ -287,6 +288,28 @@ describe('local person', () => {
queryDocsByKeyOuter.calledOnceWithExactly(localContext.medicDb, 'medic-client/contacts_by_type')
).to.be.true;
expect(queryDocsByKeyInner.calledOnceWithExactly([personIdentifier], limit, skip)).to.be.true;
expect(isPerson.notCalled).to.be.true;
});

it('returns null valued array if rows returned from database are not docs', async () => {
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
const doc = { type: 'person'};
const docs = [doc, doc, doc];
queryDocsByKeyInner.resolves(docs);
isPerson.returns(false);

const res = await Person.v1.getPage(localContext)(personTypeQualifier, limit, skip);

expect(res).to.deep.equal([]);
expect(settingsGetAll.callCount).to.equal(4);
expect(getPersonTypes.calledOnceWithExactly(settings)).to.be.true;
expect(
queryDocsByKeyOuter.calledOnceWithExactly(localContext.medicDb, 'medic-client/contacts_by_type')
).to.be.true;
expect(queryDocsByKeyInner.calledOnceWithExactly([personIdentifier], limit, skip)).to.be.true;
expect(isPerson.callCount).to.equal(3);
expect(isPerson.getCall(0).args).to.deep.equal([settings, doc]);
expect(isPerson.getCall(1).args).to.deep.equal([settings, doc]);
expect(isPerson.getCall(2).args).to.deep.equal([settings, doc]);
});
});
sugat009 marked this conversation as resolved.
Show resolved Hide resolved
});
Expand Down
31 changes: 31 additions & 0 deletions shared-libs/cht-datasource/test/person.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ describe('person', () => {
const people = [{ _id: 'person1' }, { _id: 'person2' }, { _id: 'person3' }] as Person.v1.Person[];
const limit = 3;
const skip = 1;
const invalidLimit = -1;
const invalidSkip = -1;
const personTypeQualifier = {contactType: 'person'} as const;
const invalidQualifier = { contactType: 'invalid' } as const;
let getPage: SinonStub;
Expand All @@ -148,6 +150,8 @@ describe('person', () => {
expect(result).to.equal(people);
expect(assertDataContext.calledOnceWithExactly(dataContext)).to.be.true;
expect(adapt.calledOnceWithExactly(dataContext, Local.Person.v1.getPage, Remote.Person.v1.getPage)).to.be.true;
expect(getPage.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true;
expect(isContactTypeQualifier.calledOnceWithExactly((personTypeQualifier))).to.be.true;
});
sugat009 marked this conversation as resolved.
Show resolved Hide resolved

it('throws an error if the data context is invalid', () => {
Expand All @@ -159,6 +163,7 @@ describe('person', () => {
expect(assertDataContext.calledOnceWithExactly(dataContext)).to.be.true;
expect(adapt.notCalled).to.be.true;
expect(getPage.notCalled).to.be.true;
expect(isContactTypeQualifier.notCalled).to.be.true;
});

it('throws an error if the qualifier is invalid', async () => {
Expand All @@ -172,6 +177,32 @@ describe('person', () => {
expect(isContactTypeQualifier.calledOnceWithExactly(invalidQualifier)).to.be.true;
expect(getPage.notCalled).to.be.true;
});

it('throws an error if limit is invalid', async () => {
isContactTypeQualifier.returns(true);
getPage.resolves(people);

await expect(Person.v1.getPage(dataContext)(personTypeQualifier, invalidLimit, skip))
.to.be.rejectedWith(`limit must be a positive number`);

expect(assertDataContext.calledOnceWithExactly(dataContext)).to.be.true;
expect(adapt.calledOnceWithExactly(dataContext, Local.Person.v1.getPage, Remote.Person.v1.getPage)).to.be.true;
expect(isContactTypeQualifier.calledOnceWithExactly((personTypeQualifier))).to.be.true;
expect(getPage.notCalled).to.be.true;
});

it('throws an error if skip is invalid', async () => {
isContactTypeQualifier.returns(true);
getPage.resolves(people);

await expect(Person.v1.getPage(dataContext)(personTypeQualifier, limit, invalidSkip))
.to.be.rejectedWith(`skip must be a non-negative number`);

expect(assertDataContext.calledOnceWithExactly(dataContext)).to.be.true;
expect(adapt.calledOnceWithExactly(dataContext, Local.Person.v1.getPage, Remote.Person.v1.getPage)).to.be.true;
expect(isContactTypeQualifier.calledOnceWithExactly((personTypeQualifier))).to.be.true;
expect(getPage.notCalled).to.be.true;
});
});
});
});
Loading
Loading