Skip to content

Commit

Permalink
fix: Pass dbType in DialectContext for dialectFactory (#1756)
Browse files Browse the repository at this point in the history
Fixes #1728
  • Loading branch information
ovr committed Jan 13, 2021
1 parent 14098ad commit 5cf88bf
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 31 deletions.
8 changes: 5 additions & 3 deletions packages/cubejs-server-core/src/core/CompilerApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ export class CompilerApi {
return this.dbType;
}

getDialectClass(dataSource) {
return this.dialectClass && this.dialectClass({ dataSource: dataSource || 'default' });
getDialectClass(dataSource, dbType) {
return this.dialectClass && this.dialectClass({ dataSource: dataSource || 'default', dbType });
}

async getSql(query, options) {
Expand Down Expand Up @@ -101,7 +101,9 @@ export class CompilerApi {
}

createQueryByDataSource(compilers, query, dataSource) {
return this.createQuery(compilers, this.getDbType(dataSource), this.getDialectClass(dataSource), query);
const dbType = this.getDbType(dataSource);

return this.createQuery(compilers, dbType, this.getDialectClass(dataSource, dbType), query);
}

createQuery(compilers, dbType, dialectClass, query) {
Expand Down
48 changes: 21 additions & 27 deletions packages/cubejs-server-core/src/core/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ import type { BaseDriver } from '@cubejs-backend/query-orchestrator';
import type {
ContextToAppIdFn,
CreateOptions,
DatabaseType, DbTypeFn,
DriverContext, ExternalDbTypeFn, OrchestratorOptionsFn, PreAggregationsSchemaFn,
DatabaseType,
DbTypeFn,
ExternalDbTypeFn,
OrchestratorOptionsFn,
PreAggregationsSchemaFn,
RequestContext,
SchemaFileRepository,
} from './types';
Expand Down Expand Up @@ -64,8 +67,10 @@ type RequireOne<T, K extends keyof T> = {

export type ServerCoreInitializedOptions = RequireOne<
CreateOptions,
'dbType' | 'apiSecret' | 'devServer' | 'telemetry' |
'driverFactory' | 'dialectFactory' | 'dashboardAppPath' | 'dashboardAppPort'
// This fields are required, because we add default values in constructor
'dbType' | 'apiSecret' | 'devServer' | 'telemetry' | 'dashboardAppPath' | 'dashboardAppPort' |
'driverFactory' | 'dialectFactory' |
'externalDriverFactory' | 'externalDialectFactory'
>;

function wrapToFnIfNeeded<T, R>(possibleFn: T|((a: R) => T)): (a: R) => T {
Expand All @@ -79,16 +84,8 @@ function wrapToFnIfNeeded<T, R>(possibleFn: T|((a: R) => T)): (a: R) => T {
export class CubejsServerCore {
public readonly repository: FileRepository;

protected readonly driverFactory: (context: DriverContext) => any;

protected readonly externalDriverFactory: (context: RequestContext) => any;

protected readonly externalDialectFactory: any;

protected devServer: DevServer|undefined;

protected dialectFactory: any;

protected readonly orchestratorStorage: OrchestratorStorage = new OrchestratorStorage();

protected readonly repositoryFactory: ((context: RequestContext) => SchemaFileRepository) | (() => FileRepository);
Expand Down Expand Up @@ -143,9 +140,8 @@ export class CubejsServerCore {
externalDbType,
devServer,
driverFactory: () => typeof dbType === 'string' && CubejsServerCore.createDriver(dbType),
dialectFactory: () => typeof dbType === 'string' &&
CubejsServerCore.lookupDriverClass(dbType).dialectClass &&
CubejsServerCore.lookupDriverClass(dbType).dialectClass(),
dialectFactory: (ctx) => CubejsServerCore.lookupDriverClass(ctx.dbType).dialectClass &&
CubejsServerCore.lookupDriverClass(ctx.dbType).dialectClass(),
externalDriverFactory: externalDbType && (
() => new (CubejsServerCore.lookupDriverClass(externalDbType))({
host: process.env.CUBEJS_EXT_DB_HOST,
Expand Down Expand Up @@ -183,10 +179,6 @@ export class CubejsServerCore {
}

this.options = options;
this.driverFactory = options.driverFactory;
this.externalDriverFactory = options.externalDriverFactory;
this.dialectFactory = options.dialectFactory;
this.externalDialectFactory = options.externalDialectFactory;

this.logger = options.logger || (
process.env.NODE_ENV !== 'production'
Expand Down Expand Up @@ -452,24 +444,26 @@ export class CubejsServerCore {
return this.apiGatewayInstance;
}

public getCompilerApi(context) {
public getCompilerApi(context: RequestContext) {
const appId = this.contextToAppId(context);
let compilerApi = this.compilerCache.get(appId);
const currentSchemaVersion = this.options.schemaVersion && (() => this.options.schemaVersion(context));

if (!compilerApi) {
compilerApi = this.createCompilerApi(
this.repositoryFactory(context), {
dbType: (dataSourceContext) => this.contextToDbType({ ...context, ...dataSourceContext }),
externalDbType: this.contextToExternalDbType(context),
dialectClass: (dataSourceContext) => this.dialectFactory &&
this.dialectFactory({ ...context, ...dataSourceContext }),
externalDialectClass: this.externalDialectFactory && this.externalDialectFactory(context),
dialectClass: (dialectContext) => this.options.dialectFactory &&
this.options.dialectFactory({ ...context, ...dialectContext }),
externalDialectClass: this.options.externalDialectFactory && this.options.externalDialectFactory(context),
schemaVersion: currentSchemaVersion,
preAggregationsSchema: this.preAggregationsSchema(context),
context,
allowJsDuplicatePropsInSchema: this.options.allowJsDuplicatePropsInSchema
}
);

this.compilerCache.set(appId, compilerApi);
}

Expand All @@ -491,7 +485,7 @@ export class CubejsServerCore {
getDriver: async (dataSource) => {
if (!driverPromise[dataSource || 'default']) {
orchestratorApi.addDataSeenSource(dataSource);
const driver = await this.driverFactory({ ...context, dataSource });
const driver = await this.options.driverFactory({ ...context, dataSource });
if (driver.setLogger) {
driver.setLogger(this.logger);
}
Expand All @@ -502,9 +496,9 @@ export class CubejsServerCore {
}
return driverPromise[dataSource || 'default'];
},
getExternalDriverFactory: this.externalDriverFactory && (async () => {
getExternalDriverFactory: this.options.externalDriverFactory && (async () => {
if (!externalPreAggregationsDriverPromise) {
const driver = await this.externalDriverFactory(context);
const driver = await this.options.externalDriverFactory(context);
if (driver.setLogger) {
driver.setLogger(this.logger);
}
Expand Down Expand Up @@ -557,7 +551,7 @@ export class CubejsServerCore {

public async getDriver() {
if (!this.driver) {
const driver = this.driverFactory(<any>{});
const driver = this.options.driverFactory(<any>{});
await driver.testConnection(); // TODO mutex
this.driver = driver;
}
Expand Down
6 changes: 5 additions & 1 deletion packages/cubejs-server-core/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export interface DriverContext extends RequestContext {
dataSource: string;
}

export interface DialectContext extends DriverContext {
dbType: string;
}

export interface FileContent {
fileName: string;
content: string;
Expand Down Expand Up @@ -88,7 +92,7 @@ export interface CreateOptions {
apiSecret?: string;
logger?: (msg: string, params: any) => void;
driverFactory?: (context: DriverContext) => any;
dialectFactory?: (context: DriverContext) => any;
dialectFactory?: (context: DialectContext) => any;
externalDriverFactory?: (context: RequestContext) => any;
externalDialectFactory?: (context: RequestContext) => any;
contextToAppId?: ContextToAppIdFn;
Expand Down

0 comments on commit 5cf88bf

Please sign in to comment.