From 5f6531f54fa9276ab1888675a1acf1479dbefe8c Mon Sep 17 00:00:00 2001 From: varungbt <4632477+varungbt@users.noreply.github.com> Date: Thu, 12 Aug 2021 01:26:13 -0500 Subject: [PATCH 1/5] dataset list pagination changes + openapi yaml changes --- docs/api/openapi.osdu.yaml | 23 ++++++++++++++++++++++- docs/api/openapi.yaml | 23 +++++++++++++++++++++++ src/services/dataset/dao.ts | 25 +++++++++++++++++++------ src/services/dataset/handler.ts | 13 ++++++++++--- src/services/dataset/model.ts | 19 +++++-------------- src/services/dataset/parser.ts | 22 ++++++++++++++++++++-- 6 files changed, 99 insertions(+), 26 deletions(-) diff --git a/docs/api/openapi.osdu.yaml b/docs/api/openapi.osdu.yaml index c63642d3..4fa9e0a4 100644 --- a/docs/api/openapi.osdu.yaml +++ b/docs/api/openapi.osdu.yaml @@ -662,7 +662,14 @@ paths: items: type: string collectionFormat: multi - + - description: 'Limit the number of datasets in the response' + in: query + name: limit + type: string + - description: 'Cursor for pagination on the datasets list' + in: query + name: cursor + type: string responses: 200: description: "The list of all datasets in the subproject if no gtags are in the request parameters. If gtags exist in the request parameters, then list all datasets that have the same list of gtags." @@ -670,6 +677,10 @@ paths: type: array items: $ref: "#/definitions/Dataset" + 201: + description: "Paginated dataset list with nextPageCursor. For documentation purposes, if limit or cursor is given, status code here is 200." + schema: + $ref: "#/definitions/PaginatedDatasets" 400: description: "Bad request." 401: @@ -2196,6 +2207,16 @@ definitions: type: string description: Next cursor for pagination. example: { datasets: ["folderA/", "folderB/", "dataset01"], nextPageCursor: "abc1234" } + + # PaginatedDatasets + PaginatedDatasets: + properties: + datasets: + type: array + items: + $ref: '#/definitions/Dataset' + nextPageCursor: + type: string # =========================================================================== # Endpoints Security Section diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 3421c16b..ae4a606e 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -672,6 +672,14 @@ paths: items: type: string collectionFormat: multi + - description: 'Limit the number of datasets in the response' + in: query + name: limit + type: string + - description: 'Cursor for pagination on the datasets list' + in: query + name: cursor + type: string responses: 200: @@ -680,6 +688,11 @@ paths: type: array items: $ref: "#/definitions/Dataset" + + 201: + description: "Paginated dataset list with nextPageCursor. For documentation purposes, if limit or cursor is given, status code here is 200." + schema: + $ref: "#/definitions/PaginatedDatasets" 400: description: "Bad request" 401: @@ -2205,6 +2218,16 @@ definitions: type: string description: Next cursor for pagination example: { datasets: ["folderA/", "folderB/", "dataset01"], nextPageCursor: "abc1234" } + + # PaginatedDatasets + PaginatedDatasets: + properties: + datasets: + type: array + items: + $ref: '#/definitions/Dataset' + nextPageCursor: + type: string # =========================================================================== # Endpoints Security Section diff --git a/src/services/dataset/dao.ts b/src/services/dataset/dao.ts index d045ef5d..f63ca56c 100644 --- a/src/services/dataset/dao.ts +++ b/src/services/dataset/dao.ts @@ -15,10 +15,10 @@ // ============================================================================ import { DatasetModel, PaginationModel } from '.'; -import { IJournal, IJournalTransaction } from '../../cloud'; -import { Config } from '../../cloud'; +import { Config, IJournal, IJournalTransaction } from '../../cloud'; import { Utils } from '../../shared'; import { Locker } from './locker'; +import { PaginatedDatasetList } from './model'; export class DatasetDAO { @@ -69,7 +69,9 @@ export class DatasetDAO { public static async list( journalClient: IJournal | IJournalTransaction, - dataset: DatasetModel): Promise { + dataset: DatasetModel, pagination: PaginationModel): + Promise { + let query: any; if (dataset.gtags === undefined || dataset.gtags.length === 0) { query = journalClient.createQuery( @@ -83,16 +85,27 @@ export class DatasetDAO { } } - let [entities] = await journalClient.runQuery(query); + if (pagination && pagination.cursor) { query = query.start(pagination.cursor); } + + if (pagination && pagination.limit) { query = query.limit(pagination.limit); } - entities = (entities) as DatasetModel[]; + const [entities, info] = await journalClient.runQuery(query); // Fix model for old entity for (let entity of entities) { entity = await this.fixOldModel(entity, dataset.tenant, dataset.subproject); } - return entities; + const output: PaginatedDatasetList = { + datasets: entities, + nextPageCursor: null + }; + + if (pagination) { + output.nextPageCursor = info.endCursor; + } + + return output; } diff --git a/src/services/dataset/handler.ts b/src/services/dataset/handler.ts index a3f3e998..bc2e21fe 100644 --- a/src/services/dataset/handler.ts +++ b/src/services/dataset/handler.ts @@ -318,7 +318,10 @@ export class DatasetHandler { private static async list(req: expRequest, tenant: TenantModel, subproject: SubProjectModel) { // Retrieve the dataset path information - const dataset = DatasetParser.list(req); + const userInput = DatasetParser.list(req); + + const dataset = userInput.dataset; + const pagination = userInput.pagination; // init journalClient client const journalClient = JournalFactoryTenantClient.get(tenant); @@ -340,9 +343,13 @@ export class DatasetHandler { item.ctag = item.ctag + tenant.gcpid + ';' + DESUtils.getDataPartitionID(tenant.esd); } + // Retrieve the list of datasets metadata - return datasets; + if (output.nextPageCursor) { + return output; + } + return output.datasets; } // delete a dataset @@ -834,7 +841,7 @@ export class DatasetHandler { // check if the dataset does not exist const lockKey = datasetIN.tenant + '/' + datasetIN.subproject + datasetIN.path + datasetIN.name; if (!dataset) { - if(await Locker.getLock(lockKey)) { + if (await Locker.getLock(lockKey)) { // if a previous call fails, the dataset is not created but the lock is acquired and not released await Locker.unlock(lockKey); return; diff --git a/src/services/dataset/model.ts b/src/services/dataset/model.ts index 2f1f0623..80825e91 100644 --- a/src/services/dataset/model.ts +++ b/src/services/dataset/model.ts @@ -1,18 +1,4 @@ // ============================================================================ -// Copyright 2017-2019, Schlumberger -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// ============================================================================ export interface IDatasetModel { name: string; @@ -46,4 +32,9 @@ export interface IPaginationModel { export interface IDatasetAcl { admins: string[], viewers: string[]; +} + +export interface PaginatedDatasetList { + datasets: IDatasetModel[]; + nextPageCursor: string; } \ No newline at end of file diff --git a/src/services/dataset/parser.ts b/src/services/dataset/parser.ts index af29d986..4514a90f 100644 --- a/src/services/dataset/parser.ts +++ b/src/services/dataset/parser.ts @@ -109,10 +109,28 @@ export class DatasetParser { req.query.seismicmeta === 'true']; } - public static list(req: expRequest): DatasetModel { + public static list(req: expRequest): any { const dataset = this.createDatasetModelFromRequest(req); dataset.gtags = req.query.gtag; - return dataset; + + const limit = parseInt(req.query.limit, 10); + if (limit < 0) { + throw (Error.make(Error.Status.BAD_REQUEST, + 'The \'limit\' query parameter can not be less than zero.')); + } + const cursor = req.query.cursor as string; + if (cursor === '') { + throw (Error.make(Error.Status.BAD_REQUEST, + 'The \'cursor\' query parameter can not be empty if supplied')); + } + let pagination = null; + if (limit || cursor) { + pagination = { limit, cursor }; + } + + // Retrieve the list of datasets metadata + return { dataset, pagination }; + } public static delete(req: expRequest): DatasetModel { -- GitLab From 07cdb044f996958d0ab8778a4bb19afaff38c8a4 Mon Sep 17 00:00:00 2001 From: varungbt <4632477+varungbt@users.noreply.github.com> Date: Thu, 12 Aug 2021 01:28:39 -0500 Subject: [PATCH 2/5] Update model.ts --- src/services/dataset/model.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/services/dataset/model.ts b/src/services/dataset/model.ts index 80825e91..4d24f316 100644 --- a/src/services/dataset/model.ts +++ b/src/services/dataset/model.ts @@ -1,4 +1,19 @@ // ============================================================================ +// Copyright 2017-2019, Schlumberger +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ============================================================================ + export interface IDatasetModel { name: string; -- GitLab From 41e21a6eb07c9cc0a523870d74a3cf53a7fb3005 Mon Sep 17 00:00:00 2001 From: varungbt <4632477+varungbt@users.noreply.github.com> Date: Thu, 12 Aug 2021 01:49:19 -0500 Subject: [PATCH 3/5] Update handler.ts --- src/services/dataset/handler.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/services/dataset/handler.ts b/src/services/dataset/handler.ts index bc2e21fe..28ed85a2 100644 --- a/src/services/dataset/handler.ts +++ b/src/services/dataset/handler.ts @@ -334,12 +334,11 @@ export class DatasetHandler { req.headers['impersonation-token-context'] as string); } - // Retrieve the list of datasets metadata - const datasets = await DatasetDAO.list(journalClient, dataset); + const output = await DatasetDAO.list(journalClient, dataset, pagination) as any; // attach the gcpid for fast check - for (const item of datasets) { + for (const item of output.datasets) { item.ctag = item.ctag + tenant.gcpid + ';' + DESUtils.getDataPartitionID(tenant.esd); } -- GitLab From 5a3b8edfb8bbcb23e3e17cfcdf26569c58151e83 Mon Sep 17 00:00:00 2001 From: varungbt <4632477+varungbt@users.noreply.github.com> Date: Thu, 12 Aug 2021 17:04:03 -0500 Subject: [PATCH 4/5] Update unit tests --- src/services/dataset/dao.ts | 2 +- tests/utest/dao/dataset.ts | 22 +++++++++++----------- tests/utest/services/dataset.ts | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/services/dataset/dao.ts b/src/services/dataset/dao.ts index f63ca56c..a08d17d9 100644 --- a/src/services/dataset/dao.ts +++ b/src/services/dataset/dao.ts @@ -70,7 +70,7 @@ export class DatasetDAO { public static async list( journalClient: IJournal | IJournalTransaction, dataset: DatasetModel, pagination: PaginationModel): - Promise { + Promise { let query: any; if (dataset.gtags === undefined || dataset.gtags.length === 0) { diff --git a/tests/utest/dao/dataset.ts b/tests/utest/dao/dataset.ts index 3589606b..aac88986 100644 --- a/tests/utest/dao/dataset.ts +++ b/tests/utest/dao/dataset.ts @@ -14,13 +14,12 @@ // limitations under the License. // ============================================================================ -import sinon from 'sinon'; - import { Datastore } from '@google-cloud/datastore'; import { Entity } from '@google-cloud/datastore/build/src/entity'; import { RunQueryResponse } from '@google-cloud/datastore/build/src/query'; -import { google } from '../../../src/cloud/providers'; +import sinon from 'sinon'; import { Config } from '../../../src/cloud'; +import { google } from '../../../src/cloud/providers'; import { RecordLatency } from '../../../src/metrics'; import { DatasetModel } from '../../../src/services/dataset'; import { DatasetDAO } from '../../../src/services/dataset/dao'; @@ -28,6 +27,7 @@ import { Locker } from '../../../src/services/dataset/locker'; import { IPaginationModel } from '../../../src/services/dataset/model'; import { Tx } from '../utils'; + export class TestDataset { private static dataset: DatasetModel; @@ -82,13 +82,13 @@ export class TestDataset { Tx.test(async (done: any) => { this.journal.save.resolves({} as never); - await DatasetDAO.register(this.journal, { key: {'key': 'dataset_key'}, data: TestDataset.dataset }); + await DatasetDAO.register(this.journal, { key: { 'key': 'dataset_key' }, data: TestDataset.dataset }); done(); }); Tx.test(async (done: any) => { this.journal.save.resolves(); - await DatasetDAO.register(this.journal, { key: {'key': 'dataset_key'}, data: TestDataset.dataset }); + await DatasetDAO.register(this.journal, { key: { 'key': 'dataset_key' }, data: TestDataset.dataset }); done(); }); } @@ -186,7 +186,7 @@ export class TestDataset { this.journal.runQuery.resolves([expectedResult, undefined]); this.sandbox.stub(DatasetDAO, 'fixOldModel').resolves(expectedResult[0]); - const result = await DatasetDAO.list(this.journal, this.dataset); + const result = await DatasetDAO.list(this.journal, this.dataset, null); Tx.checkTrue( this.journal.runQuery.calledWith(query) && result[0] === expectedResult[0], @@ -231,7 +231,7 @@ export class TestDataset { this.journal.runQuery.resolves([expectedResult, undefined]); this.sandbox.stub(DatasetDAO, 'fixOldModel').resolves(expectedResult[0]); - const result = await DatasetDAO.list(this.journal, this.dataset); + const result = await DatasetDAO.list(this.journal, this.dataset, null); Tx.checkTrue( this.journal.runQuery.calledWith(query) && result[0] === expectedResult[0], @@ -274,7 +274,7 @@ export class TestDataset { this.journal.getQueryFilterSymbolContains.returns('='); this.sandbox.stub(DatasetDAO, 'fixOldModel').resolves(expectedResult[0]); - const result = await DatasetDAO.list(this.journal, this.dataset); + const result = await DatasetDAO.list(this.journal, this.dataset, null); Tx.checkTrue( this.journal.runQuery.calledWith(query) && result[0] === expectedResult[0], @@ -320,7 +320,7 @@ export class TestDataset { this.journal.getQueryFilterSymbolContains.returns('='); this.sandbox.stub(DatasetDAO, 'fixOldModel').resolves(expectedResult[0]); - const result = await DatasetDAO.list(this.journal, this.dataset); + const result = await DatasetDAO.list(this.journal, this.dataset, null); Tx.checkTrue( this.journal.runQuery.calledWith(query) && result[0] === expectedResult[0], @@ -329,7 +329,7 @@ export class TestDataset { }); Tx.test(async (done: any) => { - Config.CLOUDPROVIDER='azure' + Config.CLOUDPROVIDER = 'azure'; this.dataset.gtags = ['tagA', 'tagB']; const expectedResult = [ { @@ -367,7 +367,7 @@ export class TestDataset { this.journal.getQueryFilterSymbolContains.returns('CONTAINS'); this.sandbox.stub(DatasetDAO, 'fixOldModel').resolves(expectedResult[0]); - const result = await DatasetDAO.list(this.journal, this.dataset); + const result = await DatasetDAO.list(this.journal, this.dataset, null); Tx.checkTrue( this.journal.runQuery.calledWith(query) && result[0] === expectedResult[0], diff --git a/tests/utest/services/dataset.ts b/tests/utest/services/dataset.ts index 16348bb7..afc7d140 100644 --- a/tests/utest/services/dataset.ts +++ b/tests/utest/services/dataset.ts @@ -444,7 +444,7 @@ export class TestDatasetSVC { Tx.testExp(async (done: any, expReq: expRequest, expRes: expResponse) => { this.sandbox.stub(TenantDAO, 'get').resolves({} as any); this.sandbox.stub(Auth, 'isReadAuthorized').resolves(undefined); - this.sandbox.stub(DatasetDAO, 'list').resolves([{}] as DatasetModel[]); + this.sandbox.stub(DatasetDAO, 'list').resolves({ datasets: [{} as DatasetModel], nextPageCursor: null }); this.sandbox.stub(Auth, 'isLegalTagValid').resolves(true); this.sandbox.stub(SubProjectDAO, 'get').resolves(this.testSubProject); this.sandbox.stub(DESUtils, 'getDataPartitionID').returns('datapartition'); @@ -1039,7 +1039,7 @@ export class TestDatasetSVC { this.journal.runQuery.resolves([[{}], {}] as never); this.sandbox.stub(DatasetDAO, 'fixOldModel').resolves(); this.sandbox.stub(SubProjectDAO, 'get').resolves(this.testSubProject); - await DatasetDAO.list(this.journal, this.dataset); + await DatasetDAO.list(this.journal, this.dataset, null); done(); }); } -- GitLab From 8f09684a70d2a4caf203cf0207716d8bc2030115 Mon Sep 17 00:00:00 2001 From: varungbt <4632477+varungbt@users.noreply.github.com> Date: Fri, 13 Aug 2021 09:58:12 -0500 Subject: [PATCH 5/5] Update NOTICE --- NOTICE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NOTICE b/NOTICE index d3a29b82..3db75a0b 100644 --- a/NOTICE +++ b/NOTICE @@ -468,7 +468,7 @@ The following software have components provided under the terms of this license: - cross-spawn (from https://www.npmjs.com/package/cross-spawn) - crypto-random-string (from https://www.npmjs.com/package/crypto-random-string) - d64 (from https://www.npmjs.com/package/d64) -- dashdash (from https://github.com/trentm/node-dashdash) +- dashdash (from https://www.npmjs.com/package/dashdash) - date-and-time (from https://www.npmjs.com/package/date-and-time) - date-format (from https://www.npmjs.com/package/date-format) - date-format (from https://www.npmjs.com/package/date-format) -- GitLab