Skip to content

Commit dd4c9d5

Browse files
authored
fix: Refresh Scheduler - empty securityContext in driverFactory (#6316)
1 parent 3e216c8 commit dd4c9d5

File tree

8 files changed

+61
-66
lines changed

8 files changed

+61
-66
lines changed

packages/cubejs-api-gateway/src/gateway.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -565,12 +565,6 @@ class ApiGateway {
565565
public async metaExtended({ context, res }: { context: RequestContext, res: ResponseResultFn }) {
566566
const requestStarted = new Date();
567567

568-
// TODO: test and remove this function.
569-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
570-
function visibilityFilter(item) {
571-
return getEnv('devMode') || context.signedWithPlaygroundAuthSecret || item.isVisible;
572-
}
573-
574568
try {
575569
await this.assertPermission('meta', context.securityContext);
576570
const metaConfigExtended = await this.getCompilerApi(context).metaConfigExtended({

packages/cubejs-api-gateway/src/types/gateway.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type UserBackgroundContext = {
3838
/**
3939
* Function that should provides a logic of scheduled returning of
4040
* the user background context. Used as a part of a main
41-
* configuration object of the Gateway to provide extendabillity to
41+
* configuration object of the Gateway to provide extendability to
4242
* this logic.
4343
*/
4444
type ScheduledRefreshContextsFn =

packages/cubejs-query-orchestrator/src/orchestrator/QueryCache.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ export type QueryBody = {
4747
continueWait?: boolean;
4848
renewQuery?: boolean;
4949
requestId?: string;
50-
context?: any;
5150
external?: boolean;
5251
isJob?: boolean;
5352
forceNoCache?: boolean;

packages/cubejs-server-core/src/core/OptsHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ export class OptsHandler {
221221
);
222222
}
223223
// TODO (buntarb): wrapping this call with assertDriverFactoryResult
224-
// change assertions sequince and cause a fail of few tests. Review it.
224+
// change assertions sequence and cause a fail of few tests. Review it.
225225
return this.defaultDriverFactory(ctx);
226226
} else {
227227
return this.assertDriverFactoryResult(

packages/cubejs-server-core/src/core/OrchestratorApi.ts

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ import {
1010
QueryBody,
1111
} from '@cubejs-backend/query-orchestrator';
1212

13-
import { DbTypeAsyncFn, ExternalDbTypeFn, RequestContext } from './types';
13+
import { DatabaseType, RequestContext } from './types';
1414

1515
export interface OrchestratorApiOptions extends QueryOrchestratorOptions {
16-
contextToDbType: DbTypeAsyncFn;
17-
contextToExternalDbType: ExternalDbTypeFn;
16+
contextToDbType: (dataSource: string) => Promise<DatabaseType>;
17+
contextToExternalDbType: () => DatabaseType;
1818
redisPrefix?: string;
1919
}
2020

@@ -104,34 +104,19 @@ export class OrchestratorApi {
104104
requestId: query.requestId
105105
});
106106

107-
const extractDbType = async (response) => {
108-
const dbType = await this.options.contextToDbType({
109-
...query.context,
110-
dataSource: response.dataSource,
111-
});
112-
return dbType;
113-
};
114-
115-
const extractExternalDbType = (response) => (
116-
this.options.contextToExternalDbType({
117-
...query.context,
118-
dataSource: response.dataSource,
119-
})
120-
);
121-
122107
if (Array.isArray(data)) {
123108
const res = await Promise.all(
124109
data.map(async (item) => ({
125110
...item,
126-
dbType: await extractDbType(item),
127-
extDbType: extractExternalDbType(item),
111+
dbType: await this.options.contextToDbType(item.dataSource),
112+
extDbType: this.options.contextToExternalDbType(),
128113
}))
129114
);
130115
return res;
131116
}
132117

133-
data.dbType = await extractDbType(data);
134-
data.extDbType = extractExternalDbType(data);
118+
data.dbType = await this.options.contextToDbType(data.dataSource);
119+
data.extDbType = this.options.contextToExternalDbType();
135120

136121
return data;
137122
} catch (err) {

packages/cubejs-server-core/src/core/server.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -546,10 +546,11 @@ export class CubejsServerCore {
546546

547547
let externalPreAggregationsDriverPromise: Promise<BaseDriver> | null = null;
548548

549+
const contextToDbType: DbTypeAsyncFn = this.contextToDbType.bind(this);
549550
const externalDbType = this.contextToExternalDbType(context);
550551

551-
// orchestrator options can be empty, if user didnt define it.
552-
// so we are adding default and configuring queues concurrencies.
552+
// orchestrator options can be empty, if user didn't define it.
553+
// so we are adding default and configuring queues concurrency.
553554
const orchestratorOptions =
554555
this.optsHandler.getOrchestratorInitializedOptions(
555556
context,
@@ -638,8 +639,12 @@ export class CubejsServerCore {
638639
}
639640
})();
640641
}),
641-
contextToDbType: this.contextToDbType.bind(this),
642-
contextToExternalDbType: this.contextToExternalDbType.bind(this),
642+
contextToDbType: async (dataSource) => contextToDbType({
643+
...context,
644+
dataSource
645+
}),
646+
// speedup with cache
647+
contextToExternalDbType: () => externalDbType,
643648
redisPrefix: orchestratorId,
644649
skipExternalCacheAndQueue: externalDbType === 'cubestore',
645650
cacheAndQueueDriver: this.options.cacheAndQueueDriver,

packages/cubejs-server-core/test/unit/RefreshScheduler.test.ts

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import R from 'ramda';
22
import { BaseDriver } from '@cubejs-backend/query-orchestrator';
3-
import { SchemaFileRepository } from '@cubejs-backend/shared';
3+
import { pausePromise, SchemaFileRepository } from '@cubejs-backend/shared';
44
import { CubejsServerCore, DatabaseType } from '../../src';
55
import { RefreshScheduler } from '../../src/core/RefreshScheduler';
66
import { CompilerApi } from '../../src/core/CompilerApi';
@@ -333,34 +333,30 @@ class MockDriver extends BaseDriver {
333333

334334
let testCounter = 1;
335335

336-
const setupScheduler = ({ repository, useOriginalSqlPreAggregations }: { repository: SchemaFileRepository, useOriginalSqlPreAggregations?: boolean }) => {
336+
const setupScheduler = ({ repository, useOriginalSqlPreAggregations, skipAssertSecurityContext }: { repository: SchemaFileRepository, useOriginalSqlPreAggregations?: boolean, skipAssertSecurityContext?: true }) => {
337+
const mockDriver = new MockDriver();
338+
const externalDriver = new MockDriver();
339+
337340
const serverCore = new CubejsServerCore({
338-
dbType: 'postgres',
339341
apiSecret: 'foo',
340-
});
341-
const compilerApi = new CompilerApi(
342-
repository,
343-
async () => 'postgres',
344-
{
345-
compileContext: {
346-
useOriginalSqlPreAggregations,
347-
},
348-
logger: (msg, params) => {
349-
console.log(msg, params);
350-
},
351-
}
352-
);
342+
logger: (msg, params) => console.log(msg, params),
343+
driverFactory: async ({ securityContext }) => {
344+
expect(typeof securityContext).toEqual('object');
345+
if (!skipAssertSecurityContext) {
346+
expect(securityContext.hasOwnProperty('tenantId')).toEqual(true);
347+
}
353348

354-
const mockDriver = new MockDriver();
349+
return mockDriver;
350+
},
351+
externalDriverFactory: async ({ securityContext }) => {
352+
expect(typeof securityContext).toEqual('object');
353+
if (!skipAssertSecurityContext) {
354+
expect(securityContext.hasOwnProperty('tenantId')).toEqual(true);
355+
}
355356

356-
const orchestratorApi = new OrchestratorApi(
357-
() => mockDriver,
358-
(msg, params) => console.log(msg, params),
359-
{
360-
contextToDbType: async () => 'postgres',
361-
contextToExternalDbType(): DatabaseType {
362-
return 'cubestore';
363-
},
357+
return externalDriver;
358+
},
359+
orchestratorOptions: () => ({
364360
continueWaitTimeout: 0.1,
365361
queryCacheOptions: {
366362
queueOptions: () => ({
@@ -374,14 +370,26 @@ const setupScheduler = ({ repository, useOriginalSqlPreAggregations }: { reposit
374370
}),
375371
},
376372
redisPrefix: `TEST_${testCounter++}`,
373+
})
374+
});
375+
376+
const compilerApi = new CompilerApi(
377+
repository,
378+
async () => 'postgres',
379+
{
380+
compileContext: {
381+
useOriginalSqlPreAggregations,
382+
},
383+
logger: (msg, params) => {
384+
console.log(msg, params);
385+
},
377386
}
378387
);
379388

380389
jest.spyOn(serverCore, 'getCompilerApi').mockImplementation(() => compilerApi);
381-
jest.spyOn(serverCore, 'getOrchestratorApi').mockImplementation(() => <any>orchestratorApi);
382390

383391
const refreshScheduler = new RefreshScheduler(serverCore);
384-
return { refreshScheduler, orchestratorApi, mockDriver };
392+
return { refreshScheduler, compilerApi, mockDriver };
385393
};
386394

387395
describe('Refresh Scheduler', () => {
@@ -393,6 +401,11 @@ describe('Refresh Scheduler', () => {
393401
delete process.env.CUBEJS_DB_QUERY_TIMEOUT;
394402
});
395403

404+
afterAll(async () => {
405+
// align logs from STDOUT
406+
await pausePromise(100);
407+
});
408+
396409
test('Round robin pre-aggregation refresh by history priority', async () => {
397410
process.env.CUBEJS_EXTERNAL_DEFAULT = 'false';
398411
process.env.CUBEJS_SCHEDULED_REFRESH_DEFAULT = 'true';
@@ -876,6 +889,7 @@ describe('Refresh Scheduler', () => {
876889
process.env.CUBEJS_SCHEDULED_REFRESH_DEFAULT = 'true';
877890
const { refreshScheduler } = setupScheduler({
878891
repository: repositoryWithoutPreAggregations,
892+
skipAssertSecurityContext: true,
879893
});
880894

881895
for (let i = 0; i < 50; i++) {

packages/cubejs-server-core/test/unit/index.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,7 @@ describe('index.test', () => {
167167
test('driverFactory should return driver, failure', async () => {
168168
const options: CreateOptions = { dbType: () => <any>'mongo', driverFactory: () => <any>null, };
169169

170-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
171-
const [driverFactory, orchestratorOptions] = getCreateOrchestratorOptionsFromServer(options);
170+
const [driverFactory, _orchestratorOptions] = getCreateOrchestratorOptionsFromServer(options);
172171

173172
try {
174173
await driverFactory('default');
@@ -182,8 +181,7 @@ describe('index.test', () => {
182181
test('externalDriverFactory should return driver, failure', async () => {
183182
const options: CreateOptions = { dbType: () => <any>'mongo', externalDriverFactory: () => <any>null, };
184183

185-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
186-
const [driverFactory, orchestratorOptions] = getCreateOrchestratorOptionsFromServer(options);
184+
const [_driverFactory, orchestratorOptions] = getCreateOrchestratorOptionsFromServer(options);
187185

188186
try {
189187
await orchestratorOptions.externalDriverFactory();

0 commit comments

Comments
 (0)