From 1b762abef7b05d2968623381655336e24655e943 Mon Sep 17 00:00:00 2001 From: Vadim Nikolaenko Date: Tue, 5 Jul 2022 11:56:12 +0300 Subject: [PATCH] fix(Pagination): Handle case when cursor filter overlaps entry filter (LLC-1909) (#399) --- package-lock.json | 113 +++++++++++++++--- package.json | 1 + src/mongoModelsRepo/utils/pagination.ts | 28 +++-- .../getIdentifiers/getIdentifiers.test.ts | 101 +++++++++++++--- 4 files changed, 199 insertions(+), 44 deletions(-) diff --git a/package-lock.json b/package-lock.json index 044e83584..ed859c2cd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2307,13 +2307,13 @@ "dev": true }, "@npmcli/arborist": { - "version": "5.0.6", + "version": "5.2.0", "bundled": true, "dev": true, "requires": { "@isaacs/string-locale-compare": "^1.1.0", "@npmcli/installed-package-contents": "^1.0.7", - "@npmcli/map-workspaces": "^2.0.0", + "@npmcli/map-workspaces": "^2.0.3", "@npmcli/metavuln-calculator": "^3.0.1", "@npmcli/move-file": "^2.0.0", "@npmcli/name-from-folder": "^1.0.1", @@ -2321,7 +2321,7 @@ "@npmcli/package-json": "^2.0.0", "@npmcli/run-script": "^3.0.0", "bin-links": "^3.0.0", - "cacache": "^16.0.0", + "cacache": "^16.0.6", "common-ancestor-path": "^1.0.1", "json-parse-even-better-errors": "^2.3.1", "json-stringify-nice": "^1.1.4", @@ -2332,7 +2332,7 @@ "npm-package-arg": "^9.0.0", "npm-pick-manifest": "^7.0.0", "npm-registry-fetch": "^13.0.0", - "npmlog": "^6.0.1", + "npmlog": "^6.0.2", "pacote": "^13.0.5", "parse-conflict-json": "^2.0.1", "proc-log": "^2.0.0", @@ -2341,7 +2341,7 @@ "read-package-json-fast": "^2.0.2", "readdir-scoped-modules": "^1.1.0", "rimraf": "^3.0.2", - "semver": "^7.3.5", + "semver": "^7.3.7", "ssri": "^9.0.0", "treeverse": "^2.0.0", "walk-up-path": "^1.0.0" @@ -2531,7 +2531,7 @@ } }, "builtins": { - "version": "5.0.0", + "version": "5.0.1", "bundled": true, "dev": true, "requires": { @@ -2738,7 +2738,7 @@ } }, "is-core-module": { - "version": "2.8.1", + "version": "2.9.0", "bundled": true, "dev": true, "requires": { @@ -2766,7 +2766,7 @@ "dev": true }, "just-diff": { - "version": "5.0.1", + "version": "5.0.2", "bundled": true, "dev": true }, @@ -2802,7 +2802,7 @@ } }, "libnpmexec": { - "version": "4.0.3", + "version": "4.0.5", "bundled": true, "dev": true, "requires": { @@ -2812,7 +2812,7 @@ "chalk": "^4.1.0", "mkdirp-infer-owner": "^2.0.0", "npm-package-arg": "^9.0.1", - "npmlog": "^6.0.1", + "npmlog": "^6.0.2", "pacote": "^13.0.5", "proc-log": "^2.0.0", "read": "^1.0.7", @@ -2847,24 +2847,24 @@ } }, "libnpmpack": { - "version": "4.0.3", + "version": "4.1.0", "bundled": true, "dev": true, "requires": { "@npmcli/run-script": "^3.0.0", "npm-package-arg": "^9.0.1", - "pacote": "^13.0.5" + "pacote": "^13.5.0" } }, "libnpmpublish": { - "version": "6.0.3", + "version": "6.0.4", "bundled": true, "dev": true, "requires": { "normalize-package-data": "^4.0.0", "npm-package-arg": "^9.0.1", "npm-registry-fetch": "^13.0.0", - "semver": "^7.1.3", + "semver": "^7.3.7", "ssri": "^9.0.0" } }, @@ -2886,7 +2886,7 @@ } }, "libnpmversion": { - "version": "3.0.3", + "version": "3.0.4", "bundled": true, "dev": true, "requires": { @@ -2894,11 +2894,11 @@ "@npmcli/run-script": "^3.0.0", "json-parse-even-better-errors": "^2.3.1", "proc-log": "^2.0.0", - "semver": "^7.3.5" + "semver": "^7.3.7" } }, "lru-cache": { - "version": "7.7.3", + "version": "7.9.0", "bundled": true, "dev": true }, @@ -4942,6 +4942,13 @@ "@types/node": "*" } }, + "@types/json5": { + "version": "0.0.29", + "resolved": "https://registry.npmjs.org/@types/json5/-/json5-0.0.29.tgz", + "integrity": "sha512-dRLjCWHYg4oaA77cxO64oO+7JwCwnIzkZPdrrC71jQmQtlhM556pwKo5bUzqvZndkVbeFLIIi+9TC40JNF5hNQ==", + "dev": true, + "optional": true + }, "@types/lodash": { "version": "4.14.157", "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.157.tgz", @@ -5295,6 +5302,12 @@ "resolved": "https://registry.npmjs.org/array-unique/-/array-unique-0.3.2.tgz", "integrity": "sha512-SleRWjh9JUud2wH1hPs9rZBZ33H6T9HOiL0uwGnGx9FpE6wKGyfWugmbkEOIs6qWrZhg0LWeLziLrEwQJhs5mQ==" }, + "arrify": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/arrify/-/arrify-1.0.1.tgz", + "integrity": "sha512-3CYzex9M9FGQjCGMGyi6/31c8GJbgb0qGyrx5HWxPd0aCwh4cB2YjMb2Xf9UuoogrMrlO9cTqnB5rI5GHZTcUA==", + "dev": true + }, "asap": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/asap/-/asap-2.0.6.tgz", @@ -8912,6 +8925,16 @@ "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz", "integrity": "sha1-Epai1Y/UXxmg9s4B1lcB4sc1tus=" }, + "json5": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/json5/-/json5-1.0.1.tgz", + "integrity": "sha512-aKS4WQjPenRxiQsC93MNfjx+nbF4PAdYzmd/1JIj8HYzqfbu86beTuNgXDzPknWk0n0uARlyewZo4s++ES36Ow==", + "dev": true, + "optional": true, + "requires": { + "minimist": "^1.2.0" + } + }, "jsonfile": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", @@ -9141,8 +9164,7 @@ "version": "1.3.6", "resolved": "https://registry.npmjs.org/make-error/-/make-error-1.3.6.tgz", "integrity": "sha512-s8UhlNe7vPKomQhC1qFelMokr/Sc3AgNbso3n74mVPA5LTZwkB9NlXf4XPamLxJE8h0gh73rM94xvwRT2CVInw==", - "dev": true, - "optional": true + "dev": true }, "make-fetch-happen": { "version": "10.1.8", @@ -14620,6 +14642,46 @@ "integrity": "sha512-c1PTsA3tYrIsLGkJkzHF+w9F2EyxfXGo4UyJc4pFL++FMjnq0HJS69T3M7d//gKrFKwy429bouPescbjecU+Zw==", "dev": true }, + "ts-mocha": { + "version": "10.0.0", + "resolved": "https://registry.npmjs.org/ts-mocha/-/ts-mocha-10.0.0.tgz", + "integrity": "sha512-VRfgDO+iiuJFlNB18tzOfypJ21xn2xbuZyDvJvqpTbWgkAgD17ONGr8t+Tl8rcBtOBdjXp5e/Rk+d39f7XBHRw==", + "dev": true, + "requires": { + "ts-node": "7.0.1", + "tsconfig-paths": "^3.5.0" + }, + "dependencies": { + "buffer-from": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/buffer-from/-/buffer-from-1.1.2.tgz", + "integrity": "sha512-E+XQCRwSbaaiChtv6k6Dwgc+bx+Bs6vuKJHHl5kox/BaKbhiXzqQOwK4cO22yElGp2OCmjwVhT3HmxgyPGnJfQ==", + "dev": true + }, + "ts-node": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-7.0.1.tgz", + "integrity": "sha512-BVwVbPJRspzNh2yfslyT1PSbl5uIk03EZlb493RKHN4qej/D06n1cEhjlOJG69oFsE7OT8XjpTUcYf6pKTLMhw==", + "dev": true, + "requires": { + "arrify": "^1.0.0", + "buffer-from": "^1.1.0", + "diff": "^3.1.0", + "make-error": "^1.1.1", + "minimist": "^1.2.0", + "mkdirp": "^0.5.1", + "source-map-support": "^0.5.6", + "yn": "^2.0.0" + } + }, + "yn": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/yn/-/yn-2.0.0.tgz", + "integrity": "sha512-uTv8J/wiWTgUTg+9vLTi//leUl5vDQS6uii/emeTb2ssY7vl6QWf2fFbIIGjnhjvbdKlU0ed7QPgY1htTC86jQ==", + "dev": true + } + } + }, "ts-node": { "version": "9.1.1", "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-9.1.1.tgz", @@ -14662,6 +14724,19 @@ } } }, + "tsconfig-paths": { + "version": "3.14.1", + "resolved": "https://registry.npmjs.org/tsconfig-paths/-/tsconfig-paths-3.14.1.tgz", + "integrity": "sha512-fxDhWnFSLt3VuTwtvJt5fpwxBHg5AdKWMsgcPOOIilyjymcYVZoCQF8fvFRezCNfblEXmi+PcM1eYHeOAgXCOQ==", + "dev": true, + "optional": true, + "requires": { + "@types/json5": "^0.0.29", + "json5": "^1.0.1", + "minimist": "^1.2.6", + "strip-bom": "^3.0.0" + } + }, "tslib": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.9.1.tgz", diff --git a/package.json b/package.json index dfaee5b23..73af0ae82 100644 --- a/package.json +++ b/package.json @@ -79,6 +79,7 @@ "rimraf": "2.7.1", "supertest": "3.4.2", "travis-cov": "0.2.5", + "ts-mocha": "^10.0.0", "tslint": "5.20.1", "tslint-consistent-codestyle": "1.16.0", "tslint-immutable": "4.9.1", diff --git a/src/mongoModelsRepo/utils/pagination.ts b/src/mongoModelsRepo/utils/pagination.ts index 9a8fb4581..1bf40ce1d 100644 --- a/src/mongoModelsRepo/utils/pagination.ts +++ b/src/mongoModelsRepo/utils/pagination.ts @@ -1,4 +1,4 @@ -import { first, last } from 'lodash'; +import { first, isEmpty, isUndefined, last } from 'lodash'; import { ObjectID } from 'mongodb'; import NoCursorBackwardsDirection from '../../errors/NoCursorBackwardsDirection'; import BaseModel from '../../models/BaseModel'; @@ -26,18 +26,28 @@ export default (config: Config, collectionName: string) => { const collection = (await config.db).collection(collectionName); - const theFilter = { - ...filter, - ...cursorToFilter({ - cursor, - direction, - sort, - }), + const cursorFilter = cursorToFilter({ + cursor, + direction, + sort, + }); + + const filters = [ + filter, + cursorFilter, + ].filter((item) => !isUndefined(item) && !isEmpty(item)); + + const combinedFilters = filters.length <= 1 + ? filters[0] + : { $and: filters }; + + const scopedFilter = { + ...combinedFilters, organisation: new ObjectID(organisation), }; const mongoCursor = collection.find() - .filter(theFilter) + .filter(scopedFilter) .sort(sort) .project(project) .limit(limit + 1) diff --git a/src/tests/getIdentifiers/getIdentifiers.test.ts b/src/tests/getIdentifiers/getIdentifiers.test.ts index 4b6da4905..f68f952d0 100644 --- a/src/tests/getIdentifiers/getIdentifiers.test.ts +++ b/src/tests/getIdentifiers/getIdentifiers.test.ts @@ -1,10 +1,11 @@ import * as assert from 'assert'; import assertError from 'jscommons/dist/tests/utils/assertError'; -import { assign, map, times } from 'lodash'; +import { assign } from 'lodash'; +import { ObjectId } from 'mongodb'; import NoCursorBackwardsDirection from '../../errors/NoCursorBackwardsDirection'; import Identifier from '../../models/Identifier'; +import Persona from '../../models/Persona'; import { modelToCursor } from '../../repoFactory/utils/cursor'; -import CreateIdentifierResult from '../../serviceFactory/results/CreateIdentifierResult'; import GetOptions, { CursorDirection } from '../../serviceFactory/utils/GetOptions'; import createTestPersona from '../utils/createTestPersona'; import setup from '../utils/setup'; @@ -40,26 +41,33 @@ describe('getIdentifiers', () => { }, }); - const addTestIdentifiers = async () => { - const persona = await createTestPersona(); + const getMboxFromPersona = (persona: Persona, index: number) => `${persona.name?.replace(/\s/g, '_')}_${index}@test.com`; - const NUM_IDENTIFERS = 12; - const resultsPromise: Promise[] = times(NUM_IDENTIFERS, - (i) => { - return service.createIdentifier({ + const addTestIdentifiers = async ( + customPersona?: Persona, + amount = 12, + ): Promise => { + const persona = customPersona === undefined + ? await createTestPersona() + : customPersona; + const results = []; + + for (let i = 0; i < amount; i++) { + results.push( + await service.createIdentifier({ ifi: { key: 'mbox', - value: `${i}_${TEST_IFI.value}`, + value: customPersona === undefined + ? `${i}_${TEST_IFI.value}` + : getMboxFromPersona(persona, i), }, organisation: TEST_ORGANISATION, persona: persona.id, - }); - }, - ); + }), + ); + } - const results: ArrayLike = await Promise.all(resultsPromise); - - return map(results, ({identifier}) => identifier ); + return results.map((result) => result.identifier); }; it('Should return the first 10 items', async () => { @@ -180,7 +188,7 @@ describe('getIdentifiers', () => { assert.equal(identifiersResult.pageInfo.hasPreviousPage, false); }); - it('should return undefiend cursor, if no identifiers', async () => { + it('should return undefined cursor, if no identifiers', async () => { const identifiersResult = await service.getIdentifiers( assign({}, getIdentifiersOptions, { limit: 1, @@ -227,4 +235,65 @@ describe('getIdentifiers', () => { assert.equal(identifiersResult.edges.length, 1); assert.equal(identifiersResult.edges[0].node.ifi.value, '9_test@test.com'); }); + + it('should properly use combination of filter with $or and cursor', async () => { + const firstPersona = await createTestPersona('Test Persona 1'); + const secondPersona = await createTestPersona('Test Persona 2'); + const thirdPersona = await createTestPersona('Test Persona 3'); + + const pageSize = 3; + + await addTestIdentifiers(firstPersona, 2); + await addTestIdentifiers(secondPersona, 2); + await addTestIdentifiers(thirdPersona, 1); + + const filter = { + $or: [ + { + persona: new ObjectId(firstPersona.id), + }, + { + persona: new ObjectId(secondPersona.id), + }, + ], + }; + + const queryOptions: GetOptions = { + ...getIdentifiersOptions, + filter, + limit: pageSize, + sort: { _id: 1 }, + }; + + const firstPageResult = await service.getIdentifiers(queryOptions); + + assert.equal(firstPageResult.edges.length, pageSize); + assert.equal(firstPageResult.pageInfo.hasNextPage, true); + assert.equal(firstPageResult.pageInfo.hasPreviousPage, false); + + assert.equal( + firstPageResult.edges.map((edge) => edge.node.ifi.value).join(','), + [ + getMboxFromPersona(firstPersona, 0), + getMboxFromPersona(firstPersona, 1), + getMboxFromPersona(secondPersona, 0), + ].join(','), + ); + + const secondPageResult = await service.getIdentifiers({ + ...queryOptions, + cursor: firstPageResult.pageInfo.endCursor, + }); + + assert.equal(secondPageResult.edges.length, 1); + assert.equal(secondPageResult.pageInfo.hasNextPage, false); + assert.equal(secondPageResult.pageInfo.hasPreviousPage, true); + + assert.equal( + secondPageResult.edges.map((edge) => edge.node.ifi.value).join(','), + [ + getMboxFromPersona(secondPersona, 1), + ].join(','), + ); + }); }); // tslint:disable-line: max-file-line-count