From 030b6aacb8d616b03bf5e4f1b1deb0b210df4fba Mon Sep 17 00:00:00 2001 From: Sergei Samokhvalov Date: Mon, 25 Nov 2024 14:50:53 +0300 Subject: [PATCH] Add new handlers to get workbooks and collections list by ids (#210) * Add new handler getWorkbooksListByIds * Add route getCollectionsListByIds * Add int tests * Change test * Refactor * Refactor getCollectionsListByIds service * Add additional tests * Change route name * Add features --- src/controllers/collections.ts | 16 +++ src/controllers/workbooks.ts | 16 +++ src/routes.ts | 10 ++ .../new/collection/delete-collections.ts | 24 +++- .../collection/get-collections-list-by-ids.ts | 81 ++++++++++---- .../new/workbook/get-workbooks-list-by-ids.ts | 7 +- .../get-collections-list-by-ids.test.ts | 104 ++++++++++++++++++ .../get-workbooks-list-by-ids.test.ts | 104 ++++++++++++++++++ src/tests/int/routes.ts | 2 + 9 files changed, 339 insertions(+), 25 deletions(-) create mode 100644 src/tests/int/env/platform/suites/collections/get-collections-list-by-ids.test.ts create mode 100644 src/tests/int/env/platform/suites/workbooks/get-workbooks-list-by-ids.test.ts diff --git a/src/controllers/collections.ts b/src/controllers/collections.ts index 18668cad..90a97dd0 100644 --- a/src/controllers/collections.ts +++ b/src/controllers/collections.ts @@ -14,6 +14,7 @@ import { OrderDirection, Mode, deleteCollections, + getCollectionsListByIds, } from '../services/new/collection'; import { formatCollectionModel, @@ -65,6 +66,21 @@ export default { res.status(code).send(response); }, + getCollectionsListByIds: async (req: Request, res: Response) => { + const {body} = req; + + const result = await getCollectionsListByIds( + {ctx: req.ctx}, + { + collectionIds: body.collectionIds, + }, + ); + + const formattedResponse = result.map((instance) => formatCollectionModel(instance.model)); + const {code, response} = await prepareResponseAsync({data: formattedResponse}); + res.status(code).send(response); + }, + /** * @deprecated for structureItemsController.getStructureItems, * @todo remove, after successful deploy with UI. diff --git a/src/controllers/workbooks.ts b/src/controllers/workbooks.ts index 81eab33b..b9d85c1f 100644 --- a/src/controllers/workbooks.ts +++ b/src/controllers/workbooks.ts @@ -29,6 +29,7 @@ import { formatRestoreWorkbook, formatGetWorkbookContent, } from '../services/new/workbook/formatters'; +import {getWorkbooksListByIds} from '../services/new/workbook/get-workbooks-list-by-ids'; export default { create: async (req: Request, res: Response) => { @@ -112,6 +113,21 @@ export default { res.status(code).send(response); }, + getWorkbooksListByIds: async (req: Request, res: Response) => { + const {body} = req; + + const result = await getWorkbooksListByIds( + {ctx: req.ctx}, + { + workbookIds: body.workbookIds, + }, + ); + + const formattedResponse = result.map((instance) => formatWorkbookModel(instance.model)); + const {code, response} = await prepareResponseAsync({data: formattedResponse}); + res.status(code).send(response); + }, + update: async (req: Request, res: Response) => { const {body, params} = req; diff --git a/src/routes.ts b/src/routes.ts index cb2c053c..5e72c85b 100644 --- a/src/routes.ts +++ b/src/routes.ts @@ -296,6 +296,11 @@ export function getRoutes(_nodekit: NodeKit, options: GetRoutesOptions) { handler: workbooksController.getList, features: [Feature.CollectionsEnabled], }), + getWorkbooksListByIds: makeRoute({ + route: 'POST /v2/workbooks-get-list-by-ids', + handler: workbooksController.getWorkbooksListByIds, + features: [Feature.CollectionsEnabled], + }), updateWorkbook: makeRoute({ route: 'POST /v2/workbooks/:workbookId/update', handler: workbooksController.update, @@ -404,6 +409,11 @@ export function getRoutes(_nodekit: NodeKit, options: GetRoutesOptions) { private: true, features: [Feature.CollectionsEnabled], }), + getCollectionsListByIds: makeRoute({ + route: 'POST /v1/collections-get-list-by-ids', + handler: collectionsController.getCollectionsListByIds, + features: [Feature.CollectionsEnabled], + }), getCollectionContent: makeRoute({ route: 'GET /v1/collection-content', handler: collectionsController.getContent, diff --git a/src/services/new/collection/delete-collections.ts b/src/services/new/collection/delete-collections.ts index 42524462..71944b93 100644 --- a/src/services/new/collection/delete-collections.ts +++ b/src/services/new/collection/delete-collections.ts @@ -11,7 +11,7 @@ import {CollectionPermission} from '../../../entities/collection'; import {AppError} from '@gravity-ui/nodekit'; import {getCollectionsListByIds} from './get-collections-list-by-ids'; import {markCollectionsAsDeleted} from './utils/mark-collections-as-deleted'; -import {makeCollectionsWithParentsMap} from './utils'; +import {makeCollectionsWithParentsMap, checkAndSetCollectionPermission} from './utils'; const validateArgs = makeSchemaValidator({ type: 'object', @@ -46,9 +46,25 @@ export const deleteCollections = async ( const targetTrx = getPrimary(trx); - await getCollectionsListByIds( - {ctx, trx: getReplica(trx), skipValidation, skipCheckPermissions}, - {collectionIds, permission: CollectionPermission.Delete}, + const collectionsInstances = await getCollectionsListByIds( + {ctx, trx: getReplica(trx), skipValidation, skipCheckPermissions: true}, + {collectionIds}, + ); + + await Promise.all( + collectionsInstances.map(async (collectionInstance) => { + const collection = await checkAndSetCollectionPermission( + {ctx, trx}, + { + collectionInstance, + skipCheckPermissions, + includePermissionsInfo: false, + permission: CollectionPermission.Delete, + }, + ); + + return collection; + }), ); const recursiveName = 'collectionChildren'; diff --git a/src/services/new/collection/get-collections-list-by-ids.ts b/src/services/new/collection/get-collections-list-by-ids.ts index f8502c0e..e99c1430 100644 --- a/src/services/new/collection/get-collections-list-by-ids.ts +++ b/src/services/new/collection/get-collections-list-by-ids.ts @@ -2,9 +2,11 @@ import {ServiceArgs} from '../types'; import {getReplica} from '../utils'; import {makeSchemaValidator} from '../../../components/validation-schema-compiler'; import {CollectionModel, CollectionModelColumn} from '../../../db/models/new/collection'; -import {CollectionPermission} from '../../../entities/collection'; import Utils from '../../../utils'; -import {checkAndSetCollectionPermission} from './utils'; +import {makeCollectionsWithParentsMap} from './utils'; +import {Feature, isEnabledFeature} from '../../../components/features'; +import {CollectionPermission} from '../../../entities/collection'; +import {CollectionInstance} from '../../../registry/common/entities/collection/types'; const validateArgs = makeSchemaValidator({ type: 'object', @@ -12,6 +14,8 @@ const validateArgs = makeSchemaValidator({ properties: { collectionIds: { type: 'array', + minItems: 1, + maxItems: 1000, items: {type: 'string'}, }, includePermissionsInfo: { @@ -26,14 +30,13 @@ const validateArgs = makeSchemaValidator({ export interface GetCollectionsListByIdsArgs { collectionIds: string[]; includePermissionsInfo?: boolean; - permission?: CollectionPermission; } export const getCollectionsListByIds = async ( {ctx, trx, skipValidation = false, skipCheckPermissions = false}: ServiceArgs, args: GetCollectionsListByIdsArgs, ) => { - const {collectionIds, includePermissionsInfo = false, permission} = args; + const {collectionIds, includePermissionsInfo = false} = args; ctx.log('GET_COLLECTIONS_LIST_BY_IDS_START', { collectionIds: await Utils.macrotasksMap(collectionIds, (id) => Utils.encodeId(id)), @@ -56,30 +59,70 @@ export const getCollectionsListByIds = async ( .whereIn(CollectionModelColumn.CollectionId, collectionIds) .timeout(CollectionModel.DEFAULT_QUERY_TIMEOUT); - const modelsWithPermissions = await Promise.all( - models.map(async (model) => { - const {Collection} = registry.common.classes.get(); + const {accessServiceEnabled} = ctx.config; + + const {Collection} = registry.common.classes.get(); - const collectionInstance = new Collection({ - ctx, - model, + if (!accessServiceEnabled || skipCheckPermissions) { + if (includePermissionsInfo) { + return models.map((model) => { + const collection = new Collection({ctx, model}); + collection.enableAllPermissions(); + return collection; }); + } + + return models.map((model) => new Collection({ctx, model})); + } + + const collectionsMap = await makeCollectionsWithParentsMap({ctx, trx}, {models}); + const acceptedCollectionsMap = new Map(); + + const checkPermissionPromises: Promise[] = []; + + collectionsMap.forEach((parentIds, collection) => { + const promise = collection + .checkPermission({ + parentIds, + permission: isEnabledFeature(ctx, Feature.UseLimitedView) + ? CollectionPermission.LimitedView + : CollectionPermission.View, + }) + .then(() => { + acceptedCollectionsMap.set(collection.model, parentIds); + + return collection; + }) + .catch(() => {}); - const collection = await checkAndSetCollectionPermission( - {ctx, trx}, - {collectionInstance, skipCheckPermissions, includePermissionsInfo, permission}, - ); + checkPermissionPromises.push(promise); + }); + + let collections = await Promise.all(checkPermissionPromises); + + if (includePermissionsInfo) { + const {bulkFetchCollectionsAllPermissions} = registry.common.functions.get(); + + const mappedCollections: {model: CollectionModel; parentIds: string[]}[] = []; + + acceptedCollectionsMap.forEach((parentIds, collectionModel) => { + mappedCollections.push({ + model: collectionModel, + parentIds, + }); + }); + + collections = await bulkFetchCollectionsAllPermissions(ctx, mappedCollections); + } - return collection; - }), - ); + const result = collections.filter((item) => Boolean(item)) as CollectionInstance[]; ctx.log('GET_COLLECTIONS_LIST_BY_IDS_FINISH', { collectionIds: await Utils.macrotasksMap( - models.map((item) => item.collectionId), + result.map((item) => item.model.collectionId), (id) => Utils.encodeId(id), ), }); - return modelsWithPermissions; + return result; }; diff --git a/src/services/new/workbook/get-workbooks-list-by-ids.ts b/src/services/new/workbook/get-workbooks-list-by-ids.ts index 93de57c1..2ffea732 100644 --- a/src/services/new/workbook/get-workbooks-list-by-ids.ts +++ b/src/services/new/workbook/get-workbooks-list-by-ids.ts @@ -16,8 +16,11 @@ const validateArgs = makeSchemaValidator({ type: 'object', required: ['workbookIds'], properties: { - workbookId: { - type: ['array', 'string'], + workbookIds: { + type: 'array', + minItems: 1, + maxItems: 1000, + items: {type: 'string'}, }, includePermissionsInfo: { type: 'boolean', diff --git a/src/tests/int/env/platform/suites/collections/get-collections-list-by-ids.test.ts b/src/tests/int/env/platform/suites/collections/get-collections-list-by-ids.test.ts new file mode 100644 index 00000000..30992c39 --- /dev/null +++ b/src/tests/int/env/platform/suites/collections/get-collections-list-by-ids.test.ts @@ -0,0 +1,104 @@ +import request from 'supertest'; +import {app, auth, getCollectionBinding, US_ERRORS} from '../../auth'; +import {createMockCollection} from '../../helpers'; +import {routes} from '../../../../routes'; +import {COLLECTIONS_DEFAULT_FIELDS} from '../../../../models'; + +const rootCollection = { + collectionId: '', + title: 'Empty root collection', +}; + +const rootCollection2 = { + collectionId: '', + title: 'Empty root collection 2', +}; + +describe('Setup', () => { + test('Create collections', async () => { + const collection = await createMockCollection({ + title: rootCollection.title, + parentId: null, + }); + rootCollection.collectionId = collection.collectionId; + + const collection2 = await createMockCollection({ + title: rootCollection2.title, + parentId: null, + }); + rootCollection2.collectionId = collection2.collectionId; + }); +}); + +describe('Get collections by ids', () => { + test('Auth error', async () => { + await request(app) + .post(routes.getCollectionsListByIds) + .send({ + collectionIds: [rootCollection.collectionId, rootCollection2.collectionId], + }) + .expect(401); + }); + + test('Get list without permissions, should return empty list', async () => { + const response = await auth(request(app).post(routes.getCollectionsListByIds)) + .send({ + collectionIds: [rootCollection.collectionId, rootCollection2.collectionId], + }) + .expect(200); + + expect(response.body).toStrictEqual([]); + }); + + test('Get list with permission only 1 collection, should return 1 collection', async () => { + const response = await auth(request(app).post(routes.getCollectionsListByIds), { + accessBindings: [getCollectionBinding(rootCollection.collectionId, 'limitedView')], + }) + .send({ + collectionIds: [rootCollection.collectionId, rootCollection.collectionId], + }) + .expect(200); + + expect(response.body).toStrictEqual([ + { + ...COLLECTIONS_DEFAULT_FIELDS, + collectionId: rootCollection.collectionId, + }, + ]); + }); + + test('Get list without ids, should be a validation error', async () => { + const response = await auth(request(app).post(routes.getCollectionsListByIds), { + accessBindings: [ + getCollectionBinding(rootCollection.collectionId, 'limitedView'), + getCollectionBinding(rootCollection2.collectionId, 'limitedView'), + ], + }).expect(400); + + expect(response.body.code).toBe(US_ERRORS.VALIDATION_ERROR); + }); + + test('Successfully get list by ids', async () => { + const response = await auth(request(app).post(routes.getCollectionsListByIds), { + accessBindings: [ + getCollectionBinding(rootCollection.collectionId, 'limitedView'), + getCollectionBinding(rootCollection2.collectionId, 'limitedView'), + ], + }) + .send({ + collectionIds: [rootCollection.collectionId, rootCollection2.collectionId], + }) + .expect(200); + + expect(response.body).toStrictEqual([ + { + ...COLLECTIONS_DEFAULT_FIELDS, + collectionId: rootCollection.collectionId, + }, + { + ...COLLECTIONS_DEFAULT_FIELDS, + collectionId: rootCollection2.collectionId, + }, + ]); + }); +}); diff --git a/src/tests/int/env/platform/suites/workbooks/get-workbooks-list-by-ids.test.ts b/src/tests/int/env/platform/suites/workbooks/get-workbooks-list-by-ids.test.ts new file mode 100644 index 00000000..32855fd0 --- /dev/null +++ b/src/tests/int/env/platform/suites/workbooks/get-workbooks-list-by-ids.test.ts @@ -0,0 +1,104 @@ +import request from 'supertest'; +import {app, auth, getWorkbookBinding, US_ERRORS} from '../../auth'; +import {createMockWorkbook} from '../../helpers'; +import {routes} from '../../../../routes'; +import {WORKBOOK_DEFAULT_FIELDS} from '../../../../models'; + +const rootWorkbook = { + workbookId: '', + title: 'Empty root workbook', +}; + +const rootWorkbook2 = { + workbookId: '', + title: 'Empty root workbook 2', +}; + +describe('Setup', () => { + test('Create workbooks', async () => { + const workbook = await createMockWorkbook({ + title: rootWorkbook.title, + collectionId: null, + }); + rootWorkbook.workbookId = workbook.workbookId; + + const workbook2 = await createMockWorkbook({ + title: rootWorkbook2.title, + collectionId: null, + }); + rootWorkbook2.workbookId = workbook2.workbookId; + }); +}); + +describe('Get workbooks by ids', () => { + test('Auth error', async () => { + await request(app) + .post(routes.getWorkbooksListByIds) + .send({ + workbookIds: [rootWorkbook.workbookId, rootWorkbook2.workbookId], + }) + .expect(401); + }); + + test('Get list without permissions, should return empty list', async () => { + const response = await auth(request(app).post(routes.getWorkbooksListByIds)) + .send({ + workbookIds: [rootWorkbook.workbookId, rootWorkbook2.workbookId], + }) + .expect(200); + + expect(response.body).toStrictEqual([]); + }); + + test('Get list with permission only 1 workbook, should return 1 workbook', async () => { + const response = await auth(request(app).post(routes.getWorkbooksListByIds), { + accessBindings: [getWorkbookBinding(rootWorkbook.workbookId, 'limitedView')], + }) + .send({ + workbookIds: [rootWorkbook.workbookId, rootWorkbook2.workbookId], + }) + .expect(200); + + expect(response.body).toStrictEqual([ + { + ...WORKBOOK_DEFAULT_FIELDS, + workbookId: rootWorkbook.workbookId, + }, + ]); + }); + + test('Get list without ids, should be a validation error', async () => { + const response = await auth(request(app).post(routes.getWorkbooksListByIds), { + accessBindings: [ + getWorkbookBinding(rootWorkbook.workbookId, 'limitedView'), + getWorkbookBinding(rootWorkbook2.workbookId, 'limitedView'), + ], + }).expect(400); + + expect(response.body.code).toBe(US_ERRORS.VALIDATION_ERROR); + }); + + test('Successfully get list by ids', async () => { + const response = await auth(request(app).post(routes.getWorkbooksListByIds), { + accessBindings: [ + getWorkbookBinding(rootWorkbook.workbookId, 'limitedView'), + getWorkbookBinding(rootWorkbook2.workbookId, 'limitedView'), + ], + }) + .send({ + workbookIds: [rootWorkbook.workbookId, rootWorkbook2.workbookId], + }) + .expect(200); + + expect(response.body).toStrictEqual([ + { + ...WORKBOOK_DEFAULT_FIELDS, + workbookId: rootWorkbook.workbookId, + }, + { + ...WORKBOOK_DEFAULT_FIELDS, + workbookId: rootWorkbook2.workbookId, + }, + ]); + }); +}); diff --git a/src/tests/int/routes.ts b/src/tests/int/routes.ts index 5107b013..64dea2e1 100644 --- a/src/tests/int/routes.ts +++ b/src/tests/int/routes.ts @@ -8,6 +8,8 @@ export const routes = { collections: '/v1/collections', deleteCollections: '/v1/delete-collections', deleteWorkbooks: '/v2/delete-workbooks', + getCollectionsListByIds: '/v1/collections-get-list-by-ids', + getWorkbooksListByIds: '/v2/workbooks-get-list-by-ids', moveCollections: '/v1/move-collections', moveWorkbooks: '/v2/move-workbooks', privateCollections: '/private/v1/collections',