feat(#9193): add functionality of getting people as pages or async iterables in cht-datasource

.eslintrc

@@ -16,6 +16,7 @@
     "build/**",
     "jsdocs/**",
     "shared-libs/cht-datasource/dist/**",
+    "shared-libs/cht-datasource/docs/**",
     "tests/scalability/report*/**",
     "tests/scalability/jmeter/**",
     "webapp/src/ts/providers/xpath-element-path.provider.ts"

api/src/controllers/person.js

@@ -32,7 +32,7 @@ module.exports = {
       await checkUserPermissions(req);
 
       const personType  = Qualifier.byContactType(req.query.personType);
-      const limit = Number(req.query.limit) || 100;
+      const limit = req.query.limit ? Number(req.query.limit) : req.query.limit;
 
       const docs = await getPageByType()( personType, req.query.cursor, limit );
 

api/src/server-utils.js

@@ -58,17 +58,17 @@ module.exports = {
     if (typeof err === 'string') {
       return module.exports.serverError(err, req, res);
     }
+
     // https://github.com/nodejs/node/issues/9027
     let code = err.code || err.statusCode || err.status || 500;
+    if (err instanceof InvalidArgumentError) {
+      code = 400;
+    }
     if (!Number.isInteger(code)) {
       logger.warn(`Non-numeric error code: ${code}`);
       code = 500;
     }
 
-    if (err instanceof InvalidArgumentError) {
-      code = 400;
-    }
-
     if (code === 401) {
       return module.exports.notLoggedIn(req, res, showPrompt);
     }

api/tests/mocha/controllers/person.spec.js

@@ -155,23 +155,23 @@ describe('Person Controller', () => {
       });
     });
 
-    describe('getPageByType', () => {
+    describe('getAll', () => {
       let personGetPageByType;
       let qualifierByContactType;
       const personType = 'person';
       const invalidPersonType = 'invalidPerson';
       const personTypeQualifier = { contactType: personType };
       const person = { name: 'John Doe' };
       const limit = 100;
-      const skip = 0;
+      const cursor = null;
       const people = Array.from({ length: 3 }, () => ({ ...person }));
 
       beforeEach(() => {
         req = {
           query: {
             personType,
+            cursor,
             limit,
-            skip
           }
         };
         personGetPageByType = sinon.stub();
@@ -195,7 +195,7 @@ describe('Person Controller', () => {
         expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true;
         expect(qualifierByContactType.calledOnceWithExactly(req.query.personType)).to.be.true;
         expect(dataContextBind.calledOnceWithExactly(Person.v1.getPage)).to.be.true;
-        expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true;
+        expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, cursor, limit)).to.be.true;
         expect(res.json.calledOnceWithExactly(people)).to.be.true;
         expect(serverUtilsError.notCalled).to.be.true;
       });
@@ -230,20 +230,19 @@ describe('Person Controller', () => {
       });
 
       it('returns 400 error when argument is invalid', async () => {
-        const errorMessage = `Invalid person type: [${invalidPersonType}]`;
-        const errorPayload = { status: 400, message: errorMessage };
+        const err = new InvalidArgumentError(`Invalid contact type: [${invalidPersonType}]`);
         isOnlineOnly.returns(true);
         hasAllPermissions.returns(true);
-        personGetPageByType.throws(new InvalidArgumentError(errorMessage));
+        personGetPageByType.throws(err);
 
         await controller.v1.getAll(req, res);
 
         expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true;
         expect(qualifierByContactType.calledOnceWithExactly(req.query.personType)).to.be.true;
         expect(dataContextBind.calledOnceWithExactly(Person.v1.getPage)).to.be.true;
-        expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true;
+        expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, cursor, limit)).to.be.true;
         expect(res.json.notCalled).to.be.true;
-        expect(serverUtilsError.calledOnceWithExactly(errorPayload, req, res)).to.be.true;
+        expect(serverUtilsError.calledOnceWithExactly(err, req, res)).to.be.true;
       });
 
       it('rethrows error in case of other errors', async () => {
@@ -257,7 +256,7 @@ describe('Person Controller', () => {
         expect(hasAllPermissions.calledOnceWithExactly(userCtx, 'can_view_contacts')).to.be.true;
         expect(qualifierByContactType.calledOnceWithExactly(req.query.personType)).to.be.true;
         expect(dataContextBind.calledOnceWithExactly(Person.v1.getPage)).to.be.true;
-        expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, limit, skip)).to.be.true;
+        expect(personGetPageByType.calledOnceWithExactly(personTypeQualifier, cursor, limit)).to.be.true;
         expect(res.json.notCalled).to.be.true;
         expect(serverUtilsError.calledOnceWithExactly(err, req, res)).to.be.true;
       });

api/tests/mocha/server-utils.spec.js

@@ -3,6 +3,7 @@ const chai = require('chai');
 const environment = require('@medic/environment');
 const serverUtils = require('../../src/server-utils');
 const cookie = require('../../src/services/cookie');
+const {InvalidArgumentError} = require('@medic/cht-datasource');
 
 let req;
 let res;
@@ -60,6 +61,18 @@ describe('Server utils', () => {
       chai.expect(res.end.callCount).to.equal(0);
     });
 
+    it('function handles InvalidArgument error as bad request(400) error', () => {
+      const err = new InvalidArgumentError('Bad Request');
+
+      serverUtils.error(err, req, res);
+
+      chai.expect(res.writeHead.callCount).to.eq(1);
+      chai.expect(res.writeHead.args[0][0]).to.eq(400);
+      chai.expect(res.writeHead.args[0][1]['Content-Type']).to.equal('text/plain');
+      chai.expect(res.end.callCount).to.equal(1);
+      chai.expect(res.end.args[0][0]).to.equal('Bad Request');
+    });
+
     it('function handles 503 errors - #3821', () => {
       // an example error thrown by the `request` library
       const error = {
@@ -124,7 +137,6 @@ describe('Server utils', () => {
       chai.expect(res.end.callCount).to.equal(1);
       chai.expect(res.end.args[0][0]).to.equal('foo');
     });
-
   });
 
   describe('notLoggedIn', () => {

shared-libs/cht-datasource/.eslintrc.js

@@ -1,3 +1,13 @@
+const JS_DOC_REQUIRED_CONTEXTS = [
+  'FunctionDeclaration',
+  'FunctionExpression',
+  'VariableDeclaration',
+  'TSInterfaceDeclaration',
+  'TSTypeAliasDeclaration',
+  'TSEnumDeclaration',
+  'TSMethodSignature'
+];
+
 module.exports = {
   overrides: [
     {
@@ -19,11 +29,6 @@ module.exports = {
       settings: {
         jsdoc: {
           contexts: [
-            'VariableDeclaration',
-            'TSInterfaceDeclaration',
-            'TSTypeAliasDeclaration',
-            'TSEnumDeclaration',
-            'TSMethodSignature'
           ]
         }
       },
@@ -42,9 +47,22 @@ module.exports = {
             FunctionExpression: true,
             MethodDefinition: true,
           },
-          publicOnly: true,
+          contexts: JS_DOC_REQUIRED_CONTEXTS,
+          publicOnly: true
         }],
-        ['jsdoc/check-tag-names']: ['error', { definedTags: ['typeParam'] }],
+        ['jsdoc/require-param']: ['error', {
+          contexts: JS_DOC_REQUIRED_CONTEXTS,
+          exemptedBy: ['inheritdoc', 'private', 'internal']
+        }],
+        ['jsdoc/require-returns']: ['error', {
+          contexts: JS_DOC_REQUIRED_CONTEXTS,
+          exemptedBy: ['inheritdoc', 'private', 'internal']
+        }],
+        ['jsdoc/require-yields']: ['error', {
+          contexts: JS_DOC_REQUIRED_CONTEXTS,
+          exemptedBy: ['inheritdoc', 'private', 'internal']
+        }],
+        ['jsdoc/check-tag-names']: ['error', { definedTags: ['typeParam']}],
       }
     }
   ]

shared-libs/cht-datasource/src/index.ts

@@ -91,13 +91,13 @@ export const getDatasource = (ctx: DataContext) => {
         /**
          * Returns an array of people for the provided page specifications.
          * @param personType the type of people to return
-         * @param cursor the token identifying which page to retrieve. A falsy value indicates the first page should be
+         * @param cursor the token identifying which page to retrieve. A `null` value indicates the first page should be
          * returned. Subsequent pages can be retrieved by providing the cursor returned with the previous page.
          * @param limit the maximum number of people to return. Default is 100.
          * @returns a page of people for the provided 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 cursor is not a valid page token
+         * @throws Error if the provided cursor is not a valid page token or `null`
          * @see {@link getByType} which provides the same data, but without having to manually account for paging
          */
         getPageByType: (

shared-libs/cht-datasource/src/local/person.ts

@@ -77,7 +77,10 @@ export namespace v1 {
       }
 
       // Adding a number skip variable here so as not to confuse ourselves
-      const skip = Number(cursor) || 0;
+      const skip = Number(cursor);
+      if (isNaN(skip) || skip < 0 || !Number.isInteger(skip)) {
+        throw new InvalidArgumentError(`Invalid cursor token: [${String(cursor)}]`);
+      }
 
       const fetchAndFilter = async (
         currentLimit: number,

shared-libs/cht-datasource/src/person.ts

@@ -48,8 +48,8 @@ export namespace v1 {
     }
   };
 
-  const assertCursor: (cursor: unknown) => asserts cursor is string | null = (cursor: unknown) => {
-    if (cursor && (typeof cursor !== 'string' || Number(cursor) < 0)) {
+  const assertCursor: (cursor: unknown) => asserts cursor is Nullable<string> = (cursor: unknown) => {
+    if (cursor !== null && (typeof cursor !== 'string' || !cursor.length)) {
       throw new InvalidArgumentError(`Invalid cursor token: [${String(cursor)}]`);
     }
   };
@@ -98,13 +98,13 @@ export namespace v1 {
     /**
      * Returns an array of people for the provided page specifications.
      * @param personType the type of people to return
-     * @param cursor the token identifying which page to retrieve. A falsy value indicates the first page should be
+     * @param cursor the token identifying which page to retrieve. A `null` value indicates the first page should be
      * returned. Subsequent pages can be retrieved by providing the cursor returned with the previous page.
      * @param limit the maximum number of people to return. Default is 100.
      * @returns a page of people for the provided specification
      * @throws Error if no type is provided or if the type is not for a person
      * @throws Error if the provided `limit` value is `<=0`
-     * @throws Error if the provided cursor is not a valid page token
+     * @throws Error if the provided cursor is not a valid page token or `null`
      */
     const curriedFn = async (
       personType: ContactTypeQualifier,

shared-libs/cht-datasource/test/index.spec.ts

@@ -144,19 +144,19 @@ describe('CHT Script API - getDatasource', () => {
         expect(byContactType.calledOnceWithExactly(personType)).to.be.true;
       });
 
-      it('getByType', async () => {
-        // eslint-disable-next-line @typescript-eslint/require-await
+      it('getByType', () => {
         const mockAsyncGenerator = async function* () {
+          await Promise.resolve();
           yield [];
         };
-        const personGetAll = sinon.stub().resolves(mockAsyncGenerator);
+
+        const personGetAll = sinon.stub().returns(mockAsyncGenerator);
         dataContextBind.returns(personGetAll);
         const personType = 'person';
         const personTypeQualifier = { contactType: personType };
         const byContactType = sinon.stub(Qualifier, 'byContactType').returns(personTypeQualifier);
 
-        // eslint-disable-next-line @typescript-eslint/await-thenable
-        const res =  await person.getByType(personType);
+        const res =  person.getByType(personType);
 
         expect(res).to.deep.equal(mockAsyncGenerator);
         expect(dataContextBind.calledOnceWithExactly(Person.v1.getAll)).to.be.true;

shared-libs/cht-datasource/test/libs/core.spec.ts

@@ -205,7 +205,7 @@ describe('core lib', () => {
     });
   });
 
-  describe('getDocumentStream', () => {
+  describe('getPagedGenerator', () => {
     let fetchFunctionStub: SinonStub;
     const limit = 100;
     const cursor = null;

shared-libs/cht-datasource/test/local/person.spec.ts

@@ -228,6 +228,7 @@ describe('local person', () => {
     describe('getPage', () => {
       const limit = 3;
       const cursor = null;
+      const notNullCursor = '5';
       const personIdentifier = 'person';
       const personTypeQualifier = {contactType: personIdentifier} as const;
       const invalidPersonTypeQualifier = { contactType: 'invalid' } as const;
@@ -263,9 +264,29 @@ describe('local person', () => {
         ).to.be.true;
         expect(queryDocsByKeyInner.calledOnceWithExactly([personIdentifier], limit, Number(cursor))).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]);
+        isPerson.args.forEach((arg) => expect(arg).to.deep.equal([settings, doc]));
+      });
+
+      it('returns a page of people when cursor is not null', async () => {
+        const doc = { type: 'person'};
+        const docs = [doc, doc, doc];
+        queryDocsByKeyInner.resolves(docs);
+        const expectedResult = {
+          cursor: '8',
+          data: docs
+        };
+
+        const res = await Person.v1.getPage(localContext)(personTypeQualifier, notNullCursor, limit);
+
+        expect(res).to.deep.equal(expectedResult);
+        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, Number(notNullCursor))).to.be.true;
+        expect(isPerson.callCount).to.equal(3);
+        isPerson.args.forEach((arg) => expect(arg).to.deep.equal([settings, doc]));
       });
 
       it('throws an error if person identifier is invalid/does not exist', async () => {
@@ -324,9 +345,7 @@ describe('local person', () => {
         expect(queryDocsByKeyInner.firstCall.args).to.deep.equal([[personIdentifier], limit, Number(cursor)]);
         expect(queryDocsByKeyInner.secondCall.args).to.deep.equal([[personIdentifier], 4, 3]);
         expect(isPerson.callCount).to.equal(6);
-        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]);
+        isPerson.args.forEach((arg) => expect(arg).to.deep.equal([settings, doc]));
       });
     });
   });