From 84b26dd0fe58018482d530d2b60e2961dd9ec2f8 Mon Sep 17 00:00:00 2001 From: "dogan.ay" <65234588+DayTF@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:00:14 +0100 Subject: [PATCH] feat(native-query): execute chart query requests (#1225) --- packages/_example/src/forest/agent.ts | 10 +- .../routes/access/native-query-datasource.ts | 161 ++++++++++ packages/agent/src/routes/index.ts | 15 + .../services/authorization/authorization.ts | 24 +- .../agent/src/utils/context-filter-factory.ts | 1 + packages/agent/src/utils/query-string.ts | 17 ++ .../agent/test/routes/access/chart.test.ts | 69 ++++- .../test/routes/access/count-related.test.ts | 4 +- .../agent/test/routes/access/count.test.ts | 11 +- .../test/routes/access/list-related.test.ts | 4 +- .../agent/test/routes/access/list.test.ts | 6 +- .../access/native-query-datasource.test.ts | 281 ++++++++++++++++++ packages/agent/test/routes/index.test.ts | 36 ++- .../routes/modification/action/action.test.ts | 21 +- .../modification/associate-related.test.ts | 3 +- .../test/routes/modification/delete.test.ts | 21 +- .../dissociate-delete-related.test.ts | 45 ++- .../authorization/authorization.test.ts | 90 ++++++ .../test/utils/context-filter-factory.test.ts | 2 + .../agent/test/utils/query-string.test.ts | 36 ++- .../src/decorators/segment/collection.ts | 60 +++- .../decorators/segment/collection.test.ts | 68 ++++- .../interfaces/query/filter/unpaginated.ts | 3 + .../forestadmin-client/src/charts/types.ts | 1 + .../src/permissions/hash-chart.ts | 1 + .../is-segment-query-allowed-on-connection.ts | 29 ++ .../src/permissions/permission-with-cache.ts | 1 + .../src/permissions/rendering-permission.ts | 9 + .../src/permissions/types.ts | 1 + packages/forestadmin-client/src/types.ts | 1 + ...egment-query-allowed-on-connection.test.ts | 103 +++++++ .../permissions/rendering-permission.test.ts | 70 ++++- .../test/flatten-column.test.ts | 20 +- .../test/flatten-json-column.test.ts | 20 +- 34 files changed, 1171 insertions(+), 73 deletions(-) create mode 100644 packages/agent/src/routes/access/native-query-datasource.ts create mode 100644 packages/agent/test/routes/access/native-query-datasource.test.ts create mode 100644 packages/forestadmin-client/src/permissions/is-segment-query-allowed-on-connection.ts create mode 100644 packages/forestadmin-client/test/permissions/is-segment-query-allowed-on-connection.test.ts diff --git a/packages/_example/src/forest/agent.ts b/packages/_example/src/forest/agent.ts index 3bfa639f28..facd1f911b 100644 --- a/packages/_example/src/forest/agent.ts +++ b/packages/_example/src/forest/agent.ts @@ -37,7 +37,9 @@ export default function makeAgent() { .addDataSource( // Using an URI - createSqlDataSource('mariadb://example:password@localhost:3808/example'), + createSqlDataSource('mariadb://example:password@localhost:3808/example', { + liveQueryConnections: 'Main database', + }), { include: ['customer'] }, ) .addDataSource( @@ -52,7 +54,11 @@ export default function makeAgent() { { include: ['card', 'active_cards'] }, // active_cards is a view ) .addDataSource(createTypicode()) - .addDataSource(createSequelizeDataSource(sequelizePostgres)) + .addDataSource( + createSequelizeDataSource(sequelizePostgres, { + liveQueryConnections: 'Business intel', + }), + ) .addDataSource(createSequelizeDataSource(sequelizeMySql)) .addDataSource(createSequelizeDataSource(sequelizeMsSql)) .addDataSource( diff --git a/packages/agent/src/routes/access/native-query-datasource.ts b/packages/agent/src/routes/access/native-query-datasource.ts new file mode 100644 index 0000000000..8b047bf7c3 --- /dev/null +++ b/packages/agent/src/routes/access/native-query-datasource.ts @@ -0,0 +1,161 @@ +import { Caller, DataSource, UnprocessableError } from '@forestadmin/datasource-toolkit'; +import { ChartType, QueryChart } from '@forestadmin/forestadmin-client'; +import Router from '@koa/router'; +import { Context } from 'koa'; +import { v1 as uuidv1 } from 'uuid'; + +import { ForestAdminHttpDriverServices } from '../../services'; +import { AgentOptionsWithDefaults, RouteType } from '../../types'; +import BaseRoute from '../base-route'; + +function isQueryChartRequest(body): body is QueryChart { + return ( + Boolean(body.query) && + Object.values(ChartType).includes(body.type) && + body.type !== ChartType.Smart + ); +} + +export default class DataSourceNativeQueryRoute extends BaseRoute { + readonly type = RouteType.PrivateRoute; + private dataSource: DataSource; + + constructor( + services: ForestAdminHttpDriverServices, + options: AgentOptionsWithDefaults, + dataSource: DataSource, + ) { + super(services, options); + this.dataSource = dataSource; + } + + setupRoutes(router: Router): void { + router.post(`/_internal/native_query`, this.handleNativeQuery.bind(this)); + } + + private async handleNativeQuery(context: Context) { + if (!isQueryChartRequest(context.request.body)) { + throw new UnprocessableError('Native query endpoint only supports Query Chart Requests'); + } + + const chartRequest = context.request.body; + + if (!chartRequest.connectionName) { + throw new UnprocessableError('Missing native query connection attribute'); + } + + if (!this.dataSource.nativeQueryConnections[chartRequest.connectionName]) { + throw new UnprocessableError( + `Native query connection '${chartRequest.connectionName}' is unknown`, + ); + } + + await this.services.authorization.assertCanExecuteChart(context); + + context.response.body = { + data: { + id: uuidv1(), + type: 'stats', + attributes: { value: await this.makeChart(context, chartRequest) }, + }, + }; + } + + private async makeChart(context: Context, chartRequest: QueryChart) { + const { renderingId, id: userId } = context.state.user; + + const { query, contextVariables } = await this.services.chartHandler.getQueryForChart({ + userId, + renderingId, + chartRequest, + }); + + let result; + + try { + result = (await this.dataSource.executeNativeQuery( + chartRequest.connectionName, + query, + contextVariables, + )) as Record[]; + } catch (error) { + throw new UnprocessableError( + `Error during chart native query execution: ${(error as Error).message}`, + ); + } + + let body; + + switch (chartRequest.type) { + case ChartType.Value: + if (result.length) { + const resultLine = result[0]; + + if (resultLine.value === undefined) { + this.getErrorQueryColumnsName(resultLine, ['value']); + } else { + body = { + countCurrent: resultLine.value, + countPrevious: resultLine.previous, + }; + } + } + + break; + case ChartType.Pie: + case ChartType.Leaderboard: + if (result.length) { + result.forEach(resultLine => { + if (resultLine.value === undefined || resultLine.key === undefined) { + this.getErrorQueryColumnsName(resultLine, ['key', 'value']); + } + }); + } + + break; + case ChartType.Line: + if (result.length) { + result.forEach(resultLine => { + if (resultLine.value === undefined || resultLine.key === undefined) { + this.getErrorQueryColumnsName(resultLine, ['key', 'value']); + } + }); + } + + body = result.map(resultLine => ({ + label: resultLine.key, + values: { + value: resultLine.value, + }, + })); + break; + case ChartType.Objective: + if (result.length) { + const resultLine = result[0]; + + if (resultLine.value === undefined || resultLine.objective === undefined) { + this.getErrorQueryColumnsName(resultLine, ['value', 'objective']); + } else { + body = { + objective: resultLine.objective, + value: resultLine.value, + }; + } + } + + break; + default: + throw new Error('Unknown Chart type'); + } + + return body || result; + } + + private getErrorQueryColumnsName(result: Record, keyNames: string[]) { + const message = `The result columns must be named ${keyNames.join( + ', ', + )} instead of '${Object.keys(result).join("', '")}'.`; + + throw new UnprocessableError(message); + } +} diff --git a/packages/agent/src/routes/index.ts b/packages/agent/src/routes/index.ts index fcd283ddca..b528f8ddea 100644 --- a/packages/agent/src/routes/index.ts +++ b/packages/agent/src/routes/index.ts @@ -10,6 +10,7 @@ import CsvRelated from './access/csv-related'; import Get from './access/get'; import List from './access/list'; import ListRelated from './access/list-related'; +import NativeQueryDatasource from './access/native-query-datasource'; import BaseRoute from './base-route'; import Capabilities from './capabilities'; import ActionRoute from './modification/action/action'; @@ -57,6 +58,7 @@ export const RELATED_ROUTES_CTOR = [ ]; export const RELATED_RELATION_ROUTES_CTOR = [UpdateRelation]; export const CAPABILITIES_ROUTES_CTOR = [Capabilities]; +export const NATIVE_QUERY_ROUTES_CTOR = [NativeQueryDatasource]; function getRootRoutes(options: Options, services: Services): BaseRoute[] { return ROOT_ROUTES_CTOR.map(Route => new Route(services, options)); @@ -106,6 +108,18 @@ function getCapabilitiesRoutes( return routes; } +function getNativeQueryRoutes( + dataSource: DataSource, + options: Options, + services: Services, +): BaseRoute[] { + const routes: BaseRoute[] = []; + + routes.push(...NATIVE_QUERY_ROUTES_CTOR.map(Route => new Route(services, options, dataSource))); + + return routes; +} + function getRelatedRoutes( dataSource: DataSource, options: Options, @@ -159,6 +173,7 @@ export default function makeRoutes( ...getRootRoutes(options, services), ...getCrudRoutes(dataSource, options, services), ...getCapabilitiesRoutes(dataSource, options, services), + ...getNativeQueryRoutes(dataSource, options, services), ...getApiChartRoutes(dataSource, options, services), ...getRelatedRoutes(dataSource, options, services), ...getActionRoutes(dataSource, options, services), diff --git a/packages/agent/src/services/authorization/authorization.ts b/packages/agent/src/services/authorization/authorization.ts index 52ad334586..79455fbf4d 100644 --- a/packages/agent/src/services/authorization/authorization.ts +++ b/packages/agent/src/services/authorization/authorization.ts @@ -43,7 +43,7 @@ export default class AuthorizationService { context: Context, collectionName: string, ) { - const { id: userId } = context.state.user; + const { id: userId, renderingId } = context.state.user; const canOnCollection = await this.forestAdminClient.permissionService.canOnCollection({ userId, @@ -51,6 +51,28 @@ export default class AuthorizationService { collectionName, }); + if ( + context.request?.body && + CollectionActionEvent.Browse === event && + (context.request.body as { segmentQuery?: string }).segmentQuery + ) { + const { segmentQuery, connectionName } = context.request.body as { + segmentQuery?: string; + connectionName?: string; + }; + + const canExecuteSegmentQuery = + await this.forestAdminClient.permissionService.canExecuteSegmentQuery({ + userId, + collectionName, + renderingId, + segmentQuery, + connectionName, + }); + + if (!canExecuteSegmentQuery) context.throw(HttpCode.Forbidden, 'Forbidden'); + } + if (!canOnCollection) { context.throw(HttpCode.Forbidden, 'Forbidden'); } diff --git a/packages/agent/src/utils/context-filter-factory.ts b/packages/agent/src/utils/context-filter-factory.ts index 34e9b02fb7..528fe054d2 100644 --- a/packages/agent/src/utils/context-filter-factory.ts +++ b/packages/agent/src/utils/context-filter-factory.ts @@ -33,6 +33,7 @@ export default class ContextFilterFactory { return new Filter({ search: QueryStringParser.parseSearch(collection, context), segment: QueryStringParser.parseSegment(collection, context), + liveQuerySegment: QueryStringParser.parseLiveQuerySegment(context), searchExtended: QueryStringParser.parseSearchExtended(context), conditionTree: ConditionTreeFactory.intersect( QueryStringParser.parseConditionTree(collection, context), diff --git a/packages/agent/src/utils/query-string.ts b/packages/agent/src/utils/query-string.ts index ce61e1abcb..3c6e3a9c8e 100644 --- a/packages/agent/src/utils/query-string.ts +++ b/packages/agent/src/utils/query-string.ts @@ -12,6 +12,7 @@ import { Sort, SortFactory, SortValidator, + UnprocessableError, ValidationError, } from '@forestadmin/datasource-toolkit'; import { Context } from 'koa'; @@ -117,6 +118,22 @@ export default class QueryStringParser { return segment; } + static parseLiveQuerySegment(context: Context) { + const { query } = context.request as any; + const segmentQuery = query.segmentQuery?.toString(); + const connectionName = query.connectionName?.toString(); + + if (!segmentQuery) { + return null; + } + + if (!connectionName) { + throw new UnprocessableError('Missing native query connection attribute'); + } + + return { query: segmentQuery, connectionName }; + } + static parseCaller(context: Context): Caller { const timezone = context.request.query.timezone?.toString(); const { ip } = context.request; diff --git a/packages/agent/test/routes/access/chart.test.ts b/packages/agent/test/routes/access/chart.test.ts index 95b9bca7cd..6252fc1dec 100644 --- a/packages/agent/test/routes/access/chart.test.ts +++ b/packages/agent/test/routes/access/chart.test.ts @@ -132,7 +132,13 @@ describe('ChartRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { field: undefined, groups: undefined, operation: 'Count' }, ); expect(getChartWithContextInjectedMock).toHaveBeenCalledWith({ @@ -274,7 +280,8 @@ describe('ChartRoute', () => { conditionTree: { field: 'name', operator: 'Present', value: null }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: undefined, groups: undefined, operation: 'Count' }, ); @@ -335,7 +342,8 @@ describe('ChartRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: undefined, groups: undefined, operation: 'Count' }, ); @@ -410,7 +418,13 @@ describe('ChartRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { field: undefined, groups: undefined, operation: 'Count' }, ); expect(getChartWithContextInjectedMock).toHaveBeenCalledWith({ @@ -460,7 +474,8 @@ describe('ChartRoute', () => { conditionTree: { field: 'title', operator: 'NotContains', value: '[test]' }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: undefined, groups: undefined, operation: 'Count' }, ); @@ -505,7 +520,13 @@ describe('ChartRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { field: undefined, groups: [{ field: 'author:firstName' }], operation: 'Count' }, ); expect(getChartWithContextInjectedMock).toHaveBeenCalledWith({ @@ -566,7 +587,8 @@ describe('ChartRoute', () => { conditionTree: { field: 'title', operator: 'NotContains', value: '[test]' }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: undefined, groups: [{ field: 'author:firstName' }], operation: 'Count' }, ); @@ -628,7 +650,8 @@ describe('ChartRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: undefined, @@ -709,7 +732,8 @@ describe('ChartRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: undefined, @@ -770,7 +794,13 @@ describe('ChartRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { field: 'id', groups: [{ field: 'author:id' }], operation: 'Sum' }, 2, ); @@ -821,7 +851,13 @@ describe('ChartRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { operation: 'Count', field: null, groups: [{ field: 'publisher:id' }] }, 2, ); @@ -887,7 +923,8 @@ describe('ChartRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { field: 'id', groups: [{ field: 'author:id' }], operation: 'Sum' }, 2, @@ -941,7 +978,13 @@ describe('ChartRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { groups: [{ field: 'author:id' }], operation: 'Count' }, 2, ); diff --git a/packages/agent/test/routes/access/count-related.test.ts b/packages/agent/test/routes/access/count-related.test.ts index 95bfb36c4c..f1db4f7e62 100644 --- a/packages/agent/test/routes/access/count-related.test.ts +++ b/packages/agent/test/routes/access/count-related.test.ts @@ -134,6 +134,7 @@ describe('CountRelatedRoute', () => { search: 'searched argument', searchExtended: false, segment: 'a-valid-segment', + liveQuerySegment: expect.toBeNil(), conditionTree: new ConditionTreeLeaf( 'id', 'Equal', @@ -198,7 +199,7 @@ describe('CountRelatedRoute', () => { ], }), }; - const segmentParams = { segment: 'a-valid-segment' }; + const segmentParams = { segment: 'a-valid-segment', liveQuerySegment: expect.toBeNil() }; const context = createMockContext({ customProperties: { @@ -238,6 +239,7 @@ describe('CountRelatedRoute', () => { search: 'searched argument', searchExtended: false, segment: 'a-valid-segment', + liveQuerySegment: expect.toBeNil(), conditionTree: ConditionTreeFactory.fromPlainObject({ aggregator: 'And', conditions: [ diff --git a/packages/agent/test/routes/access/count.test.ts b/packages/agent/test/routes/access/count.test.ts index b702e504d7..d68b5b8362 100644 --- a/packages/agent/test/routes/access/count.test.ts +++ b/packages/agent/test/routes/access/count.test.ts @@ -39,7 +39,13 @@ describe('CountRoute', () => { request: { ip: expect.any(String) }, timezone: 'Europe/Paris', }, - { conditionTree: null, search: null, searchExtended: false, segment: null }, + { + conditionTree: null, + search: null, + searchExtended: false, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + }, { operation: 'Count' }, ); expect(context.response.body).toEqual({ count: 2 }); @@ -94,7 +100,8 @@ describe('CountRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { operation: 'Count' }, ); diff --git a/packages/agent/test/routes/access/list-related.test.ts b/packages/agent/test/routes/access/list-related.test.ts index dd1deb8949..4199151be7 100644 --- a/packages/agent/test/routes/access/list-related.test.ts +++ b/packages/agent/test/routes/access/list-related.test.ts @@ -115,6 +115,7 @@ describe('ListRelatedRoute', () => { page: new Page(0, 15), sort: new Sort({ field: 'id', ascending: true }), segment: 'a-valid-segment', + liveQuerySegment: expect.toBeNil(), conditionTree: new ConditionTreeLeaf( 'id', 'Equal', @@ -184,7 +185,7 @@ describe('ListRelatedRoute', () => { ], }), }; - const segmentParams = { segment: 'a-valid-segment' }; + const segmentParams = { segment: 'a-valid-segment', liveQuerySegment: expect.toBeNil() }; const projectionParams = { 'fields[persons]': 'id,name' }; const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, @@ -225,6 +226,7 @@ describe('ListRelatedRoute', () => { page: new Page(0, 15), sort: new Sort({ field: 'id', ascending: true }), segment: 'a-valid-segment', + liveQuerySegment: expect.toBeNil(), conditionTree: ConditionTreeFactory.fromPlainObject({ aggregator: 'And', conditions: [ diff --git a/packages/agent/test/routes/access/list.test.ts b/packages/agent/test/routes/access/list.test.ts index 6c42c56375..5049c084c2 100644 --- a/packages/agent/test/routes/access/list.test.ts +++ b/packages/agent/test/routes/access/list.test.ts @@ -60,7 +60,8 @@ describe('ListRoute', () => { conditionTree: null, search: '2', searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), page: { limit: 15, skip: 0 }, sort: [{ ascending: true, field: 'id' }], }, @@ -132,7 +133,8 @@ describe('ListRoute', () => { }, search: '2', searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), page: { limit: 15, skip: 0 }, sort: [{ ascending: true, field: 'id' }], }, diff --git a/packages/agent/test/routes/access/native-query-datasource.test.ts b/packages/agent/test/routes/access/native-query-datasource.test.ts new file mode 100644 index 0000000000..127b8ac1bf --- /dev/null +++ b/packages/agent/test/routes/access/native-query-datasource.test.ts @@ -0,0 +1,281 @@ +import { UnprocessableError } from '@forestadmin/datasource-toolkit'; +import { ChartType } from '@forestadmin/forestadmin-client'; +import { Context } from 'koa'; + +import makeRoutes from '../../../src/routes'; +import DataSourceNativeQueryRoute from '../../../src/routes/access/native-query-datasource'; +import * as factories from '../../__factories__'; + +describe('DataSourceNativeQueryRoute', () => { + describe('setupRoutes', () => { + afterEach(() => jest.resetAllMocks()); + + const setupWithLiveQuery = () => { + return factories.dataSource.build({ + nativeQueryConnections: { main: {} }, + schema: { charts: ['myChart'] }, + collections: [ + factories.collection.build({ + name: 'books', + schema: factories.collectionSchema.build({ + charts: ['myChart'], + }), + }), + ], + }); + }; + + const router = factories.router.build(); + router.post = jest.fn(); + + const dataSource = setupWithLiveQuery(); + const routes = makeRoutes( + dataSource, + factories.forestAdminHttpDriverOptions.build(), + factories.forestAdminHttpDriverServices.build(), + ); + const liveQueryRoute = routes.find( + route => route instanceof DataSourceNativeQueryRoute, + ) as DataSourceNativeQueryRoute; + + test('calling the setup', () => { + liveQueryRoute.setupRoutes(router); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith('/_internal/native_query', expect.toBeFunction()); + }); + + describe('call handling', () => { + test.each([ + { type: ChartType.Leaderboard, result: { value: 111, key: 'machines' } }, + { type: ChartType.Pie, result: { value: 111, key: 'machines' } }, + { type: ChartType.Value, result: { value: 111 } }, + { type: ChartType.Line, result: { value: 111, key: 'machines' } }, + { type: ChartType.Objective, result: { value: 111, objective: 500 } }, + ])('it should execute the native query', async ({ type, result }) => { + liveQueryRoute.setupRoutes(router); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + liveQueryRoute.services.chartHandler.getQueryForChart = jest.fn().mockResolvedValue({ + query: 'Select * FROM toto', + contextVariables: {}, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + liveQueryRoute.dataSource.executeNativeQuery = jest.fn().mockResolvedValue([result]); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith('/_internal/native_query', expect.toBeFunction()); + const handleNativeQuery = (router.post as jest.Mock).mock.calls[0][1] as ( + context: Context, + ) => Promise; + + const context = { + state: { + user: { + renderingId: 101, + userId: 1, + }, + }, + response: {}, + request: { + body: { + query: 'Select * FROM toto', + connectionName: 'main', + type, + }, + }, + } as Context; + + await handleNativeQuery(context); + + expect(context.response.body).toEqual({ + data: { + id: expect.toBeString(), + type: 'stats', + attributes: { value: expect.toSatisfy(e => Array.isArray(e) || typeof e === 'object') }, + }, + }); + }); + + describe('error cases', () => { + describe('when the query is not a chart request', () => { + test('it should throw a specific error', async () => { + liveQueryRoute.setupRoutes(router); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith( + '/_internal/native_query', + expect.toBeFunction(), + ); + const handleNativeQuery = (router.post as jest.Mock).mock.calls[0][1] as ( + context: Context, + ) => Promise; + + await expect(() => + handleNativeQuery({ request: { body: {} } } as Context), + ).rejects.toThrow( + new UnprocessableError('Native query endpoint only supports Query Chart Requests'), + ); + }); + }); + + describe('when connectionName is unknown', () => { + test('it should throw an error', async () => { + liveQueryRoute.setupRoutes(router); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith( + '/_internal/native_query', + expect.toBeFunction(), + ); + const handleNativeQuery = (router.post as jest.Mock).mock.calls[0][1] as ( + context: Context, + ) => Promise; + + await expect(() => + handleNativeQuery({ + request: { + body: { + query: 'Select * FROM toto', + connectionName: 'Something', + type: ChartType.Pie, + }, + }, + } as Context), + ).rejects.toThrow( + new UnprocessableError(`Native query connection 'Something' is unknown`), + ); + }); + }); + + describe('when connectionName is missing', () => { + test('it should throw an error', async () => { + liveQueryRoute.setupRoutes(router); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith( + '/_internal/native_query', + expect.toBeFunction(), + ); + const handleNativeQuery = (router.post as jest.Mock).mock.calls[0][1] as ( + context: Context, + ) => Promise; + + await expect(() => + handleNativeQuery({ + request: { + body: { + query: 'Select * FROM toto', + type: ChartType.Pie, + }, + }, + } as Context), + ).rejects.toThrow(new UnprocessableError(`Missing native query connection attribute`)); + }); + }); + + describe('when sql query causes an error', () => { + test('it should throw an error', async () => { + liveQueryRoute.setupRoutes(router); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + liveQueryRoute.services.chartHandler.getQueryForChart = jest.fn().mockResolvedValue({ + query: 'Select * FROM toto', + contextVariables: {}, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + liveQueryRoute.dataSource.executeNativeQuery = jest + .fn() + .mockRejectedValue(new Error('Something bad happened')); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith( + '/_internal/native_query', + expect.toBeFunction(), + ); + const handleNativeQuery = (router.post as jest.Mock).mock.calls[0][1] as ( + context: Context, + ) => Promise; + + const context = { + state: { + user: { + renderingId: 101, + userId: 1, + }, + }, + response: {}, + request: { + body: { + query: 'Select * FROM toto', + connectionName: 'main', + type: ChartType.Pie, + }, + }, + } as Context; + + await expect(handleNativeQuery(context)).rejects.toThrow( + new UnprocessableError( + 'Error during chart native query execution: Something bad happened', + ), + ); + }); + }); + + describe('when query returns wrong keys', () => { + test.each([ + { type: ChartType.Leaderboard }, + { type: ChartType.Pie }, + { type: ChartType.Value }, + { type: ChartType.Line }, + { type: ChartType.Objective }, + ])('it should throw an error', async ({ type }) => { + liveQueryRoute.setupRoutes(router); + + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + liveQueryRoute.services.chartHandler.getQueryForChart = jest.fn().mockResolvedValue({ + query: 'Select * FROM toto', + contextVariables: {}, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + liveQueryRoute.dataSource.executeNativeQuery = jest.fn().mockResolvedValue([{ id: 1 }]); + + expect(router.post).toHaveBeenCalledOnce(); + expect(router.post).toHaveBeenCalledWith( + '/_internal/native_query', + expect.toBeFunction(), + ); + const handleNativeQuery = (router.post as jest.Mock).mock.calls[0][1] as ( + context: Context, + ) => Promise; + + const context = { + state: { + user: { + renderingId: 101, + userId: 1, + }, + }, + response: {}, + request: { + body: { + query: 'Select * FROM toto', + connectionName: 'main', + type, + }, + }, + } as Context; + + await expect(handleNativeQuery(context)).rejects.toThrow(UnprocessableError); + }); + }); + }); + }); + }); +}); diff --git a/packages/agent/test/routes/index.test.ts b/packages/agent/test/routes/index.test.ts index 4ece79871a..2afaaa7ceb 100644 --- a/packages/agent/test/routes/index.test.ts +++ b/packages/agent/test/routes/index.test.ts @@ -3,6 +3,7 @@ import { DataSource } from '@forestadmin/datasource-toolkit'; import makeRoutes, { CAPABILITIES_ROUTES_CTOR, COLLECTION_ROUTES_CTOR, + NATIVE_QUERY_ROUTES_CTOR, RELATED_RELATION_ROUTES_CTOR, RELATED_ROUTES_CTOR, ROOT_ROUTES_CTOR, @@ -15,6 +16,7 @@ import CsvRelated from '../../src/routes/access/csv-related'; import Get from '../../src/routes/access/get'; import List from '../../src/routes/access/list'; import ListRelated from '../../src/routes/access/list-related'; +import DataSourceNativeQueryRoute from '../../src/routes/access/native-query-datasource'; import Capabilities from '../../src/routes/capabilities'; import AssociateRelated from '../../src/routes/modification/associate-related'; import Create from '../../src/routes/modification/create'; @@ -62,9 +64,11 @@ describe('Route index', () => { ]); expect(CAPABILITIES_ROUTES_CTOR).toEqual([Capabilities]); expect(RELATED_RELATION_ROUTES_CTOR).toEqual([UpdateRelation]); + expect(NATIVE_QUERY_ROUTES_CTOR).toEqual([DataSourceNativeQueryRoute]); }); - const BASE_ROUTE_SIZE = ROOT_ROUTES_CTOR.length + CAPABILITIES_ROUTES_CTOR.length; + const BASE_ROUTE_SIZE = + ROOT_ROUTES_CTOR.length + CAPABILITIES_ROUTES_CTOR.length + NATIVE_QUERY_ROUTES_CTOR.length; describe('makeRoutes', () => { describe('when a data source without relations', () => { @@ -264,5 +268,35 @@ describe('Route index', () => { expect(routes.length).toEqual(BASE_ROUTE_SIZE + COLLECTION_ROUTES_CTOR.length + 2); }); }); + + describe('with a data source with live query connections', () => { + const setupWithChart = (): DataSource => { + return factories.dataSource.build({ + nativeQueryConnections: { name: 'Postgre' }, + schema: { charts: ['myChart'] }, + collections: [ + factories.collection.build({ + name: 'books', + schema: factories.collectionSchema.build({ + charts: ['myChart'], + }), + }), + ], + }); + }; + + test('should instantiate the live query routes', async () => { + const dataSource = setupWithChart(); + + const routes = makeRoutes( + dataSource, + factories.forestAdminHttpDriverOptions.build(), + factories.forestAdminHttpDriverServices.build(), + ); + const lqRoute = routes.find(route => route instanceof DataSourceNativeQueryRoute); + + expect(lqRoute).toBeTruthy(); + }); + }); }); }); diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index 6e45c998c7..94e17e3b8a 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -346,7 +346,8 @@ describe('ActionRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, ); }); @@ -567,7 +568,8 @@ describe('ActionRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { changedField: undefined, @@ -634,7 +636,8 @@ describe('ActionRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, { searchField: undefined, @@ -707,7 +710,8 @@ describe('ActionRoute', () => { }, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }, ); }); @@ -764,7 +768,8 @@ describe('ActionRoute', () => { conditionTree: null, search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); @@ -850,7 +855,8 @@ describe('ActionRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); @@ -940,7 +946,8 @@ describe('ActionRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); diff --git a/packages/agent/test/routes/modification/associate-related.test.ts b/packages/agent/test/routes/modification/associate-related.test.ts index b63239bb13..28a87c9f3e 100644 --- a/packages/agent/test/routes/modification/associate-related.test.ts +++ b/packages/agent/test/routes/modification/associate-related.test.ts @@ -107,7 +107,8 @@ describe('AssociateRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), { bookId: '123e4567-e89b-12d3-a456-111111111111' }, ); diff --git a/packages/agent/test/routes/modification/delete.test.ts b/packages/agent/test/routes/modification/delete.test.ts index 00948c7a9e..761b39d7be 100644 --- a/packages/agent/test/routes/modification/delete.test.ts +++ b/packages/agent/test/routes/modification/delete.test.ts @@ -73,7 +73,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); @@ -145,7 +146,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); @@ -196,7 +198,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -253,7 +256,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -304,7 +308,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -386,7 +391,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); @@ -450,7 +456,8 @@ describe('DeleteRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); }); diff --git a/packages/agent/test/routes/modification/dissociate-delete-related.test.ts b/packages/agent/test/routes/modification/dissociate-delete-related.test.ts index 208614e733..c4e49c8bc5 100644 --- a/packages/agent/test/routes/modification/dissociate-delete-related.test.ts +++ b/packages/agent/test/routes/modification/dissociate-delete-related.test.ts @@ -123,7 +123,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), { bookId: null }, ); @@ -193,7 +194,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), { bookId: null }, ); @@ -247,7 +249,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), { bookId: null }, ); @@ -387,7 +390,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -456,7 +460,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -514,7 +519,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -631,7 +637,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -698,7 +705,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -744,7 +752,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -880,7 +889,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); @@ -915,7 +925,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -993,7 +1004,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); @@ -1028,7 +1040,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); expect(context.response.status).toEqual(HttpCode.NoContent); @@ -1091,7 +1104,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); @@ -1110,7 +1124,8 @@ describe('DissociateDeleteRelatedRoute', () => { }), search: null, searchExtended: false, - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), }), ); diff --git a/packages/agent/test/services/authorization/authorization.test.ts b/packages/agent/test/services/authorization/authorization.test.ts index 87a7dc3dbf..67d21b51fc 100644 --- a/packages/agent/test/services/authorization/authorization.test.ts +++ b/packages/agent/test/services/authorization/authorization.test.ts @@ -81,6 +81,96 @@ describe('AuthorizationService', () => { }); }); + describe('assertCanBrowseOnCollection', () => { + describe('when a live query segment is present', () => { + it('should not do anything for authorized users', async () => { + const forestAdminClient = factories.forestAdminClient.build(); + const authorizationService = new AuthorizationService(forestAdminClient); + + const context = { + request: { + body: { + segmentQuery: `SELECT id FROM books where type = 'comics'`, + connectionName: 'library', + }, + }, + state: { + user: { + id: 35, + renderingId: 42, + }, + }, + throw: jest.fn(), + } as unknown as Context; + + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue(true); + (forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock).mockResolvedValue( + true, + ); + + await authorizationService.assertCanBrowse(context, 'books'); + + expect(context.throw).not.toHaveBeenCalled(); + + expect(forestAdminClient.permissionService.canOnCollection).toHaveBeenCalledWith({ + userId: 35, + event: CollectionActionEvent.Browse, + collectionName: 'books', + }); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: `SELECT id FROM books where type = 'comics'`, + connectionName: 'library', + }); + }); + + describe('when query is not authorized', () => { + it('should throw forbidden error', async () => { + const forestAdminClient = factories.forestAdminClient.build(); + const authorizationService = new AuthorizationService(forestAdminClient); + + const context = { + request: { + body: { + segmentQuery: `DROP TABLE books;`, + connectionName: 'library', + }, + }, + state: { + user: { + id: 35, + renderingId: 42, + }, + }, + throw: jest.fn(), + } as unknown as Context; + + (forestAdminClient.permissionService.canOnCollection as jest.Mock).mockResolvedValue( + true, + ); + ( + forestAdminClient.permissionService.canExecuteSegmentQuery as jest.Mock + ).mockResolvedValue(false); + + await authorizationService.assertCanBrowse(context, 'books'); + + expect(context.throw).toHaveBeenCalledWith(HttpCode.Forbidden, 'Forbidden'); + + expect(forestAdminClient.permissionService.canExecuteSegmentQuery).toHaveBeenCalledWith({ + userId: 35, + collectionName: 'books', + renderingId: 42, + segmentQuery: `DROP TABLE books;`, + connectionName: 'library', + }); + }); + }); + }); + }); + describe('getScope', () => { it('should return the scope for the given user', async () => { const forestAdminClient = factories.forestAdminClient.build(); diff --git a/packages/agent/test/utils/context-filter-factory.test.ts b/packages/agent/test/utils/context-filter-factory.test.ts index 6885ef524d..cf52814d4c 100644 --- a/packages/agent/test/utils/context-filter-factory.test.ts +++ b/packages/agent/test/utils/context-filter-factory.test.ts @@ -68,6 +68,7 @@ describe('FilterFactory', () => { page: new Page(20, 10), searchExtended: true, segment: 'a-valid-segment', + liveQuerySegment: expect.toBeNil(), }), ); }); @@ -87,6 +88,7 @@ describe('FilterFactory', () => { page: new Page(20, 10), searchExtended: true, segment: 'a-valid-segment', + liveQuerySegment: expect.toBeNil(), }), ); }); diff --git a/packages/agent/test/utils/query-string.test.ts b/packages/agent/test/utils/query-string.test.ts index 1dab597ca1..d3b7f5d699 100644 --- a/packages/agent/test/utils/query-string.test.ts +++ b/packages/agent/test/utils/query-string.test.ts @@ -1,5 +1,6 @@ -import { Projection, ValidationError } from '@forestadmin/datasource-toolkit'; +import { Projection, UnprocessableError, ValidationError } from '@forestadmin/datasource-toolkit'; import { createMockContext } from '@shopify/jest-koa-mocks'; +import { Context } from 'koa'; import QueryStringParser from '../../src/utils/query-string'; import * as factories from '../__factories__'; @@ -385,6 +386,39 @@ describe('QueryStringParser', () => { }); }); + describe('parseLiveQuerySegment', () => { + test('should return null when no segment query is provided', () => { + const context = { + request: { query: {} }, + } as unknown as Context; + + expect(QueryStringParser.parseLiveQuerySegment(context)).toEqual(null); + }); + + test('should parse live query segment properties', () => { + const context = { + request: { query: { segmentQuery: 'SELECT * FROM toto', connectionName: 'main' } }, + } as unknown as Context; + + expect(QueryStringParser.parseLiveQuerySegment(context)).toEqual({ + query: 'SELECT * FROM toto', + connectionName: 'main', + }); + }); + + describe('when no connectionName is provided', () => { + test('should throw an error', () => { + const context = { + request: { query: { segmentQuery: 'SELECT * FROM toto' } }, + } as unknown as Context; + + expect(() => QueryStringParser.parseLiveQuerySegment(context)).toThrow( + new UnprocessableError('Missing native query connection attribute'), + ); + }); + }); + }); + describe('parseCaller', () => { test('should return the timezone and the user', () => { const context = createMockContext({ diff --git a/packages/datasource-customizer/src/decorators/segment/collection.ts b/packages/datasource-customizer/src/decorators/segment/collection.ts index e52e5c077c..565cfbec7b 100644 --- a/packages/datasource-customizer/src/decorators/segment/collection.ts +++ b/packages/datasource-customizer/src/decorators/segment/collection.ts @@ -1,11 +1,14 @@ import { + BusinessError, Caller, CollectionDecorator, CollectionSchema, ConditionTree, ConditionTreeFactory, + ConditionTreeLeaf, ConditionTreeValidator, PaginatedFilter, + SchemaUtils, } from '@forestadmin/datasource-toolkit'; import { SegmentDefinition } from './types'; @@ -34,7 +37,18 @@ export default class SegmentCollectionDecorator extends CollectionDecorator { return null; } - let { conditionTree, segment } = filter; + const trees = await Promise.all([ + this.refineFilterSegment(caller, filter), + this.refineFilterLiveQuerySegment(filter), + ]); + + const conditionTree = ConditionTreeFactory.intersect(filter.conditionTree, ...trees); + + return filter.override({ conditionTree, segment: null, liveQuerySegment: null }); + } + + private async refineFilterSegment(caller: Caller, filter: PaginatedFilter) { + const { segment } = filter; if (segment && this.segments[segment]) { const definition = this.segments[segment]; @@ -43,15 +57,49 @@ export default class SegmentCollectionDecorator extends CollectionDecorator { ? await definition(new CollectionCustomizationContext(this, caller)) : await definition; - const conditionTreeSegment = + const conditionTree = result instanceof ConditionTree ? result : ConditionTreeFactory.fromPlainObject(result); - ConditionTreeValidator.validate(conditionTreeSegment, this); + ConditionTreeValidator.validate(conditionTree, this); + + return conditionTree; + } + + return null; + } + + private async refineFilterLiveQuerySegment(filter: PaginatedFilter) { + const { liveQuerySegment } = filter; + + if (liveQuerySegment) { + const { query, connectionName } = liveQuerySegment; + + try { + const result = (await this.dataSource.executeNativeQuery( + connectionName, + query, + {}, + )) as Record[]; + + const [primaryKey] = SchemaUtils.getPrimaryKeys(this.childCollection.schema); + + const conditionTree = new ConditionTreeLeaf( + primaryKey, + 'In', + result.map(row => row[primaryKey]), + ); + + ConditionTreeValidator.validate(conditionTree, this); - conditionTree = ConditionTreeFactory.intersect(conditionTree, conditionTreeSegment); - segment = null; + return conditionTree; + } catch (error) { + throw new BusinessError( + `An error occurred during the execution of the segment query - ${error.message}`, + error, + ); + } } - return filter.override({ conditionTree, segment }); + return null; } } diff --git a/packages/datasource-customizer/test/decorators/segment/collection.test.ts b/packages/datasource-customizer/test/decorators/segment/collection.test.ts index e855627f23..8128931203 100644 --- a/packages/datasource-customizer/test/decorators/segment/collection.test.ts +++ b/packages/datasource-customizer/test/decorators/segment/collection.test.ts @@ -1,4 +1,10 @@ -import { Collection, DataSource, MissingFieldError } from '@forestadmin/datasource-toolkit'; +import { + BusinessError, + Collection, + ConditionTreeLeaf, + DataSource, + MissingFieldError, +} from '@forestadmin/datasource-toolkit'; import * as factories from '@forestadmin/datasource-toolkit/dist/test/__factories__'; import SegmentCollectionDecorator from '../../../src/decorators/segment/collection'; @@ -14,6 +20,11 @@ describe('SegmentCollectionDecorator', () => { name: 'books', schema: factories.collectionSchema.build({ fields: { + id: factories.columnSchema.build({ + isPrimaryKey: true, + columnType: 'Number', + filterOperators: new Set(['In']), + }), name: factories.columnSchema.build({ filterOperators: new Set(['Equal', 'In']), }), @@ -39,14 +50,13 @@ describe('SegmentCollectionDecorator', () => { describe('when there is a filter', () => { describe('when the segment is not managed by this decorator', () => { - it('should return the given filter', async () => { + it('should not generate condition tree', async () => { const conditionTreeGenerator = jest.fn(); segmentDecorator.addSegment('segmentName', conditionTreeGenerator); - const aFilter = factories.filter.build({ segment: 'aSegment' }); - const filter = await segmentDecorator.refineFilter(factories.caller.build(), aFilter); + const aFilter = factories.filter.build({ segment: 'aSegment', conditionTree: null }); + await segmentDecorator.refineFilter(factories.caller.build(), aFilter); - expect(filter).toEqual(aFilter); expect(conditionTreeGenerator).not.toHaveBeenCalled(); }); }); @@ -76,7 +86,8 @@ describe('SegmentCollectionDecorator', () => { expect(conditionTreeGenerator).toHaveBeenCalled(); expect(filter).toEqual({ - segment: null, + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), conditionTree: { aggregator: 'And', conditions: [ @@ -107,5 +118,50 @@ describe('SegmentCollectionDecorator', () => { expect(conditionTreeGenerator).toHaveBeenCalled(); }); }); + + describe('when there is a live query segment', () => { + it('should return the filter with the merged conditionTree from the query', async () => { + jest + .mocked(dataSource.executeNativeQuery) + .mockResolvedValue([{ id: 1 }, { id: 2 }, { id: 3 }]); + + const filter = await segmentDecorator.refineFilter( + factories.caller.build(), + factories.filter.build({ + liveQuerySegment: { query: 'SELECT id from toto', connectionName: 'main' }, + }), + ); + + const conditionTree = new ConditionTreeLeaf('id', 'In', [1, 2, 3]); + + expect(filter).toEqual( + expect.objectContaining({ + segment: expect.toBeNil(), + liveQuerySegment: expect.toBeNil(), + conditionTree, + }), + ); + }); + + describe('when an error occurs during the execution', () => { + it('should throw a business error', async () => { + const error = new Error('Database error'); + jest.mocked(dataSource.executeNativeQuery).mockRejectedValue(error); + + await expect( + segmentDecorator.refineFilter( + factories.caller.build(), + factories.filter.build({ + liveQuerySegment: { query: 'SELECT id from toto', connectionName: 'main' }, + }), + ), + ).rejects.toThrow( + new BusinessError( + `An error occurred during the execution of the segment query - Database error`, + ), + ); + }); + }); + }); }); }); diff --git a/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts b/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts index 2315adb2e4..f511add2bc 100644 --- a/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts +++ b/packages/datasource-toolkit/src/interfaces/query/filter/unpaginated.ts @@ -5,6 +5,7 @@ export type FilterComponents = { search?: string | null; searchExtended?: boolean; segment?: string; + liveQuerySegment?: { query: string; connectionName: string }; }; export type PlainFilter = { @@ -19,6 +20,7 @@ export default class Filter { search?: string; searchExtended?: boolean; segment?: string; + liveQuerySegment?: { query: string; connectionName: string }; get isNestable(): boolean { return !this.search && !this.segment; @@ -29,6 +31,7 @@ export default class Filter { this.search = parts.search; this.searchExtended = parts.searchExtended; this.segment = parts.segment; + this.liveQuerySegment = parts.liveQuerySegment; } override(fields: FilterComponents): Filter { diff --git a/packages/forestadmin-client/src/charts/types.ts b/packages/forestadmin-client/src/charts/types.ts index e18b3c9240..99feeb25d3 100644 --- a/packages/forestadmin-client/src/charts/types.ts +++ b/packages/forestadmin-client/src/charts/types.ts @@ -19,6 +19,7 @@ export interface ApiRouteChart extends BaseChart { export interface QueryChart extends BaseChart { type: Exclude; query: string; + connectionName?: string; } export interface FilterableChart extends BaseChart { diff --git a/packages/forestadmin-client/src/permissions/hash-chart.ts b/packages/forestadmin-client/src/permissions/hash-chart.ts index a88d8f1efc..bf214a5f3b 100644 --- a/packages/forestadmin-client/src/permissions/hash-chart.ts +++ b/packages/forestadmin-client/src/permissions/hash-chart.ts @@ -8,6 +8,7 @@ function hashChart(chart: Chart): string { 'apiRoute', 'smartRoute', 'query', + 'connectionName', 'labelFieldName', 'filter', 'sourceCollectionName', diff --git a/packages/forestadmin-client/src/permissions/is-segment-query-allowed-on-connection.ts b/packages/forestadmin-client/src/permissions/is-segment-query-allowed-on-connection.ts new file mode 100644 index 0000000000..6cae565e58 --- /dev/null +++ b/packages/forestadmin-client/src/permissions/is-segment-query-allowed-on-connection.ts @@ -0,0 +1,29 @@ +import { CollectionRenderingPermissionV4 } from './types'; + +export default function isSegmentQueryAllowedOnConnection( + collectionPermissions: CollectionRenderingPermissionV4, + segmentQuery: string, + connectionName: string, +): boolean { + if (!collectionPermissions || !collectionPermissions.liveQuerySegments || !connectionName) { + return false; + } + + const queries = collectionPermissions.liveQuerySegments + .filter(liveQuerySegment => liveQuerySegment.connectionName === connectionName) + .map(({ query }) => query); + + // NOTICE: Handle UNION queries made by the FRONT to display available actions on details view + // NOTICE: This can only be used on related data (Has Many relationships) to detect available + // Smart Actions restricted to segment when a Smart Action is available on multiple SQL segments + const unionQueries = segmentQuery.split('/*MULTI-SEGMENTS-QUERIES-UNION*/ UNION '); + + if (unionQueries.length > 1) { + const authorizedQueries = new Set(queries.map(query => query.replace(/;\s*/i, '').trim())); + + return unionQueries.every((unionQuery: string) => authorizedQueries.has(unionQuery.trim())); + } + + // NOTICE: Queries made by the FRONT to browse to an SQL segment + return queries.some(query => query === segmentQuery); +} diff --git a/packages/forestadmin-client/src/permissions/permission-with-cache.ts b/packages/forestadmin-client/src/permissions/permission-with-cache.ts index ea4d90deaf..cf5f8a990e 100644 --- a/packages/forestadmin-client/src/permissions/permission-with-cache.ts +++ b/packages/forestadmin-client/src/permissions/permission-with-cache.ts @@ -36,6 +36,7 @@ export default class PermissionServiceWithCache implements PermissionService { collectionName: string; renderingId: number; segmentQuery: string; + connectionName?: string; }) { return this.renderingPermissionService.canExecuteSegmentQuery(params); } diff --git a/packages/forestadmin-client/src/permissions/rendering-permission.ts b/packages/forestadmin-client/src/permissions/rendering-permission.ts index 77c9adc8ef..2a9baa8b58 100644 --- a/packages/forestadmin-client/src/permissions/rendering-permission.ts +++ b/packages/forestadmin-client/src/permissions/rendering-permission.ts @@ -1,4 +1,5 @@ import { hashChartRequest, hashServerCharts } from './hash-chart'; +import isSegmentQueryAllowedOnConnection from './is-segment-query-allowed-on-connection'; import isSegmentQueryAllowed from './is-segment-query-authorized'; import { CollectionRenderingPermissionV4, @@ -96,11 +97,13 @@ export default class RenderingPermissionService { renderingId, collectionName, segmentQuery, + connectionName, userId, }: { renderingId: number | string; collectionName: string; segmentQuery: string; + connectionName?: string; userId: number; }): Promise { return ( @@ -108,6 +111,7 @@ export default class RenderingPermissionService { renderingId, collectionName, segmentQuery, + connectionName, userId, // Only allow retry when not using server events allowRetry: !this.options.instantCacheRefresh, @@ -119,12 +123,14 @@ export default class RenderingPermissionService { renderingId, collectionName, segmentQuery, + connectionName, allowRetry, userId, }: { renderingId: number | string; collectionName: string; segmentQuery: string; + connectionName?: string; allowRetry: boolean; userId: number; }): Promise { @@ -150,6 +156,8 @@ export default class RenderingPermissionService { if ( !collectionPermissions || + (connectionName && + !isSegmentQueryAllowedOnConnection(collectionPermissions, segmentQuery, connectionName)) || !isSegmentQueryAllowed(segmentQuery, collectionPermissions.segments) ) { if (allowRetry) { @@ -159,6 +167,7 @@ export default class RenderingPermissionService { renderingId, collectionName, segmentQuery, + connectionName, userId, allowRetry: false, }); diff --git a/packages/forestadmin-client/src/permissions/types.ts b/packages/forestadmin-client/src/permissions/types.ts index 5bebc23212..5c5d3ab7f4 100644 --- a/packages/forestadmin-client/src/permissions/types.ts +++ b/packages/forestadmin-client/src/permissions/types.ts @@ -153,6 +153,7 @@ export type DynamicScopesValues = { export type CollectionRenderingPermissionV4 = { scope: RawTree | null; + liveQuerySegments?: { connectionName: string; query: string }[]; segments: string[]; }; diff --git a/packages/forestadmin-client/src/types.ts b/packages/forestadmin-client/src/types.ts index bc30063b4b..1e5579656b 100644 --- a/packages/forestadmin-client/src/types.ts +++ b/packages/forestadmin-client/src/types.ts @@ -106,6 +106,7 @@ export interface PermissionService { collectionName: string; renderingId: number | string; segmentQuery: string; + connectionName?: string; }): Promise; getConditionalTriggerCondition(params: { diff --git a/packages/forestadmin-client/test/permissions/is-segment-query-allowed-on-connection.test.ts b/packages/forestadmin-client/test/permissions/is-segment-query-allowed-on-connection.test.ts new file mode 100644 index 0000000000..d05787c4fe --- /dev/null +++ b/packages/forestadmin-client/test/permissions/is-segment-query-allowed-on-connection.test.ts @@ -0,0 +1,103 @@ +import isSegmentQueryAllowedOnConnection from '../../src/permissions/is-segment-query-allowed-on-connection'; +import { CollectionRenderingPermissionV4 } from '../../src/permissions/types'; + +describe('isSegmentQueryAllowedOnConnection', () => { + it('should return false if there is no permissions', () => { + const permissions = {} as unknown as CollectionRenderingPermissionV4; + expect(isSegmentQueryAllowedOnConnection(permissions, '', '')).toBe(false); + }); + + describe('when there are multiple queries', () => { + it('should authorize the query if every subquery is allowed', () => { + const permissions = { + liveQuerySegments: [ + { + query: 'SELECT * from users;', + connectionName: 'main', + }, + { + query: 'SELECT * from admins;', + connectionName: 'main', + }, + ], + } as unknown as CollectionRenderingPermissionV4; + expect( + isSegmentQueryAllowedOnConnection( + permissions, + 'SELECT * from users /*MULTI-SEGMENTS-QUERIES-UNION*/ UNION SELECT * from admins', + 'main', + ), + ).toBe(true); + }); + + it('should reject the query if one subquery is not allowed', () => { + const permissions = { + liveQuerySegments: [ + { + query: 'SELECT * from users;', + connectionName: 'main', + }, + ], + } as unknown as CollectionRenderingPermissionV4; + expect( + isSegmentQueryAllowedOnConnection( + permissions, + 'SELECT * from users /*MULTI-SEGMENTS-QUERIES-UNION*/ UNION SELECT * from admins', + 'main', + ), + ).toBe(false); + }); + + it('should reject if the queries are not on the same connectionName', () => { + const permissions = { + liveQuerySegments: [ + { + query: 'SELECT * from users;', + connectionName: 'main', + }, + { + query: 'SELECT * from admins;', + connectionName: 'secondary', + }, + ], + } as unknown as CollectionRenderingPermissionV4; + expect( + isSegmentQueryAllowedOnConnection( + permissions, + 'SELECT * from users /*MULTI-SEGMENTS-QUERIES-UNION*/ UNION SELECT * from admins', + 'main', + ), + ).toBe(false); + }); + }); + + describe('when there is only one query', () => { + it('should return true if the query is allowed', () => { + const permissions = { + liveQuerySegments: [ + { + query: 'SELECT * from users;', + connectionName: 'main', + }, + ], + } as unknown as CollectionRenderingPermissionV4; + expect(isSegmentQueryAllowedOnConnection(permissions, 'SELECT * from users;', 'main')).toBe( + true, + ); + }); + + it('should return false if the query is not allowed', () => { + const permissions = { + liveQuerySegments: [ + { + query: 'SELECT * from admins;', + connectionName: 'main', + }, + ], + } as unknown as CollectionRenderingPermissionV4; + expect(isSegmentQueryAllowedOnConnection(permissions, 'SELECT * from users;', 'main')).toBe( + false, + ); + }); + }); +}); diff --git a/packages/forestadmin-client/test/permissions/rendering-permission.test.ts b/packages/forestadmin-client/test/permissions/rendering-permission.test.ts index d8dac4b7e7..7ae01e58d8 100644 --- a/packages/forestadmin-client/test/permissions/rendering-permission.test.ts +++ b/packages/forestadmin-client/test/permissions/rendering-permission.test.ts @@ -3,6 +3,7 @@ import ChainedSQLQueryError from '../../src/permissions/errors/chained-sql-query import EmptySQLQueryError from '../../src/permissions/errors/empty-sql-query-error'; import NonSelectSQLQueryError from '../../src/permissions/errors/non-select-sql-query-error'; import { hashChartRequest, hashServerCharts } from '../../src/permissions/hash-chart'; +import isSegmentQueryAllowedOnConnection from '../../src/permissions/is-segment-query-allowed-on-connection'; import isSegmentQueryAllowed from '../../src/permissions/is-segment-query-authorized'; import RenderingPermissionService from '../../src/permissions/rendering-permission'; import { PermissionLevel, RawTree } from '../../src/permissions/types'; @@ -22,6 +23,11 @@ jest.mock('../../src/permissions/is-segment-query-authorized', () => ({ default: jest.fn(), })); +jest.mock('../../src/permissions/is-segment-query-allowed-on-connection', () => ({ + __esModule: true, + default: jest.fn(), +})); + jest.mock('../../src/permissions/verify-sql-query', () => ({ __esModule: true, default: jest.fn(), @@ -56,6 +62,7 @@ describe('RenderingPermissionService', () => { const hashChartRequestMock = hashChartRequest as jest.Mock; const verifySQLQueryMock = verifySQLQuery as jest.Mock; const isSegmentQueryAllowedMock = isSegmentQueryAllowed as jest.Mock; + const isSegmentQueryAllowedOnConnectionMock = isSegmentQueryAllowedOnConnection as jest.Mock; return { userPermission, @@ -66,6 +73,7 @@ describe('RenderingPermissionService', () => { hashChartRequestMock, verifySQLQueryMock, isSegmentQueryAllowedMock, + isSegmentQueryAllowedOnConnectionMock, options, }; } @@ -853,6 +861,7 @@ describe('RenderingPermissionService', () => { serverInterface, verifySQLQueryMock, isSegmentQueryAllowedMock, + isSegmentQueryAllowedOnConnectionMock, options, } = setup(); @@ -872,6 +881,7 @@ describe('RenderingPermissionService', () => { serverInterface.getRenderingPermissions = jest.fn().mockResolvedValue(permissions); verifySQLQueryMock.mockReturnValue(true); isSegmentQueryAllowedMock.mockReturnValue(true); + isSegmentQueryAllowedOnConnectionMock.mockReturnValue(false); const result = await renderingPermission.canExecuteSegmentQuery({ renderingId: 42, @@ -930,7 +940,13 @@ describe('RenderingPermissionService', () => { ); it('should return false if the collection does not have an entry in permissions', async () => { - const { renderingPermission, serverInterface, isSegmentQueryAllowedMock, options } = setup(); + const { + renderingPermission, + serverInterface, + isSegmentQueryAllowedMock, + isSegmentQueryAllowedOnConnectionMock, + options, + } = setup(); const permissions = { team: { id: 33 }, @@ -950,6 +966,7 @@ describe('RenderingPermissionService', () => { expect(result).toBe(false); expect(isSegmentQueryAllowedMock).not.toHaveBeenCalled(); + expect(isSegmentQueryAllowedOnConnectionMock).not.toHaveBeenCalled(); expect(serverInterface.getRenderingPermissions).toHaveBeenCalledWith(42, options); }); @@ -959,6 +976,7 @@ describe('RenderingPermissionService', () => { serverInterface, verifySQLQueryMock, isSegmentQueryAllowedMock, + isSegmentQueryAllowedOnConnectionMock, options, } = setup(); @@ -978,6 +996,7 @@ describe('RenderingPermissionService', () => { serverInterface.getRenderingPermissions = jest.fn().mockResolvedValue(permissions); verifySQLQueryMock.mockReturnValue(true); isSegmentQueryAllowedMock.mockReturnValue(false); + isSegmentQueryAllowedOnConnectionMock.mockReturnValue(false); const result = await renderingPermission.canExecuteSegmentQuery({ renderingId: 42, @@ -1000,6 +1019,7 @@ describe('RenderingPermissionService', () => { renderingPermission, serverInterface, isSegmentQueryAllowedMock, + isSegmentQueryAllowedOnConnectionMock, verifySQLQueryMock, options, } = setup(); @@ -1035,6 +1055,7 @@ describe('RenderingPermissionService', () => { .mockResolvedValueOnce(permissions1) .mockResolvedValueOnce(permissions2); isSegmentQueryAllowedMock.mockReturnValueOnce(false).mockReturnValueOnce(true); + isSegmentQueryAllowedOnConnectionMock.mockReturnValue(false); verifySQLQueryMock.mockReturnValue(true); const result = await renderingPermission.canExecuteSegmentQuery({ @@ -1054,5 +1075,52 @@ describe('RenderingPermissionService', () => { expect(serverInterface.getRenderingPermissions).toHaveBeenCalledTimes(2); expect(serverInterface.getRenderingPermissions).toHaveBeenCalledWith(42, options); }); + + describe('when connectionName is present', () => { + it('should return true if the segment query is allowed', async () => { + const { + renderingPermission, + serverInterface, + verifySQLQueryMock, + isSegmentQueryAllowedMock, + isSegmentQueryAllowedOnConnectionMock, + options, + } = setup(); + + const actorsPermissions = { + liveQuerySegments: [{ query: 'foo', connectionName: 'main' }], + }; + + const permissions = { + team: { id: 33 }, + collections: { + actors: actorsPermissions, + }, + stats: {}, + }; + + serverInterface.getRenderingPermissions = jest.fn().mockResolvedValue(permissions); + isSegmentQueryAllowedOnConnectionMock.mockReturnValue(true); + isSegmentQueryAllowedMock.mockReturnValue(true); + verifySQLQueryMock.mockReturnValue(true); + + const result = await renderingPermission.canExecuteSegmentQuery({ + renderingId: 42, + collectionName: 'actors', + connectionName: 'main', + segmentQuery: 'SELECT * from actors', + userId: 21, + }); + + expect(result).toBe(true); + + expect(isSegmentQueryAllowedOnConnectionMock).toHaveBeenCalledWith( + actorsPermissions, + 'SELECT * from actors', + 'main', + ); + expect(serverInterface.getRenderingPermissions).toHaveBeenCalledWith(42, options); + }); + }); }); }); diff --git a/packages/plugin-flattener/test/flatten-column.test.ts b/packages/plugin-flattener/test/flatten-column.test.ts index 0a36c8bdb8..77f1c6b4dd 100644 --- a/packages/plugin-flattener/test/flatten-column.test.ts +++ b/packages/plugin-flattener/test/flatten-column.test.ts @@ -7,7 +7,11 @@ import flattenColumn from '../src/flatten-column'; const logger = () => {}; // Working around a bug from a decorator which rewrites search in filters -const filter = factories.filter.build({ search: null as unknown as undefined }); +const filter = factories.filter.build({ + search: null as unknown as undefined, + segment: null as unknown as undefined, + liveQuerySegment: null as unknown as undefined, +}); const caller = factories.caller.build(); describe('flattenColumn', () => { @@ -293,12 +297,22 @@ describe('flattenColumn', () => { expect(baseUpdate).toHaveBeenCalledWith( caller, // not sure why I need to specify search == null. It's related to a bug in a decorator. - { conditionTree: { field: 'id', operator: 'Equal', value: '1' }, search: null }, + { + conditionTree: { field: 'id', operator: 'Equal', value: '1' }, + search: null, + segment: null, + liveQuerySegment: null, + }, { author: { name: 'Tolkien', address: { city: 'New York' } } }, ); expect(baseUpdate).toHaveBeenCalledWith( caller, - { conditionTree: { field: 'id', operator: 'In', value: ['2', '3'] }, search: null }, + { + conditionTree: { field: 'id', operator: 'In', value: ['2', '3'] }, + search: null, + segment: null, + liveQuerySegment: null, + }, { author: { name: 'Tolkien' } }, ); }); diff --git a/packages/plugin-flattener/test/flatten-json-column.test.ts b/packages/plugin-flattener/test/flatten-json-column.test.ts index 6a2c1b646f..fc0bf2e853 100644 --- a/packages/plugin-flattener/test/flatten-json-column.test.ts +++ b/packages/plugin-flattener/test/flatten-json-column.test.ts @@ -7,7 +7,11 @@ import flattenJsonColumn from '../src/flatten-json-column'; const logger = () => {}; // Working around a bug from a decorator which rewrites search in filters -const filter = factories.filter.build({ search: null as unknown as undefined }); +const filter = factories.filter.build({ + search: null as unknown as undefined, + segment: null as unknown as undefined, + liveQuerySegment: null as unknown as undefined, +}); const caller = factories.caller.build(); describe('flattenJsonColumn', () => { @@ -239,12 +243,22 @@ describe('flattenJsonColumn', () => { expect(baseUpdate).toHaveBeenCalledWith( caller, // not sure why I need to specify search == null. It's related to a bug in a decorator. - { conditionTree: { field: 'id', operator: 'Equal', value: '1' }, search: null }, + { + conditionTree: { field: 'id', operator: 'Equal', value: '1' }, + search: null, + segment: null, + liveQuerySegment: null, + }, { author: { name: 'Tolkien', address: { city: 'New York' } } }, ); expect(baseUpdate).toHaveBeenCalledWith( caller, - { conditionTree: { field: 'id', operator: 'In', value: ['2', '3'] }, search: null }, + { + conditionTree: { field: 'id', operator: 'In', value: ['2', '3'] }, + search: null, + segment: null, + liveQuerySegment: null, + }, { author: { name: 'Tolkien' } }, ); });