Skip to content

Commit

Permalink
fix(schema-compiler): make sure view members are resolvable in DAP
Browse files Browse the repository at this point in the history
  • Loading branch information
bsod90 committed Jan 13, 2025
1 parent 646bd9d commit 84d1489
Show file tree
Hide file tree
Showing 9 changed files with 225 additions and 6 deletions.
18 changes: 17 additions & 1 deletion packages/cubejs-schema-compiler/src/compiler/CubeSymbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@ const CONTEXT_SYMBOLS = {
const CURRENT_CUBE_CONSTANTS = ['CUBE', 'TABLE'];

export class CubeSymbols {
constructor(evaluateViews) {
constructor(evaluateViews, skipCompilationIfNoAccessPolicies) {
this.symbols = {};
this.builtCubes = {};
this.cubeDefinitions = {};
this.funcArgumentsValues = {};
this.cubeList = [];
this.evaluateViews = evaluateViews || false;
// This flag controls whether the additional compilation pass for views will happen or not.
// Background:
// When implemmenting Data Access Policies we realized, that in order to properly resolve
// Cube references in views, we need to have an additional pass of .compile with the evaluateViews
// flag set to true. This pass comes with a significant performance penalty and we'd like to avoid
// it if no access policies are defined for views in the data model.
this.skipCompilationIfNoAccessPolicies = skipCompilationIfNoAccessPolicies || false;
}

compile(cubes, errorReporter) {
Expand All @@ -37,6 +44,11 @@ export class CubeSymbols {
R.fromPairs
)(cubes);
this.cubeList = cubes.map(c => (c.name ? this.getCubeDefinition(c.name) : this.createCube(c)));

if (this.skipCompilationIfNoAccessPolicies && !this.viewsHaveAccessPolicies(cubes)) {
return;
}

// TODO support actual dependency sorting to allow using views inside views
const sortedByDependency = R.pipe(
R.sortBy(c => !!c.isView),
Expand All @@ -52,6 +64,10 @@ export class CubeSymbols {
}
}

viewsHaveAccessPolicies(cubes) {
return cubes.some(c => c.isView && c.accessPolicy);
}

getCubeDefinition(cubeName) {
if (!this.builtCubes[cubeName]) {
const cubeDefinition = this.cubeDefinitions[cubeName];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export class DataSchemaCompiler {
this.cubeCompilers = options.cubeCompilers || [];
this.contextCompilers = options.contextCompilers || [];
this.transpilers = options.transpilers || [];
this.viewCompilers = options.viewCompilers || [];
this.preTranspileCubeCompilers = options.preTranspileCubeCompilers || [];
this.cubeNameCompilers = options.cubeNameCompilers || [];
this.extensions = options.extensions || {};
Expand Down Expand Up @@ -94,6 +95,7 @@ export class DataSchemaCompiler {

return compilePhase({ cubeCompilers: this.cubeNameCompilers })
.then(() => compilePhase({ cubeCompilers: this.preTranspileCubeCompilers }))
.then(() => compilePhase({ cubeCompilers: this.viewCompilers }))
.then(() => compilePhase({
cubeCompilers: this.cubeCompilers,
contextCompilers: this.contextCompilers,
Expand Down
10 changes: 7 additions & 3 deletions packages/cubejs-schema-compiler/src/compiler/PrepareCompiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,22 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp
const nativeInstance = options.nativeInstance || new NativeInstance();
const cubeDictionary = new CubeDictionary();
const cubeSymbols = new CubeSymbols();
// check the comment in CubeSymbols.js for the second agument
// skipCompilationIfNoAccessPolicies
const viewCompiler = new CubeSymbols(true, true);
const cubeValidator = new CubeValidator(cubeSymbols);
const cubeEvaluator = new CubeEvaluator(cubeValidator);
const contextEvaluator = new ContextEvaluator(cubeEvaluator);
const joinGraph = new JoinGraph(cubeValidator, cubeEvaluator);
const metaTransformer = new CubeToMetaTransformer(cubeValidator, cubeEvaluator, contextEvaluator, joinGraph);
const { maxQueryCacheSize, maxQueryCacheAge } = options;
const compilerCache = new CompilerCache({ maxQueryCacheSize, maxQueryCacheAge });
const yamlCompiler = new YamlCompiler(cubeSymbols, cubeDictionary, nativeInstance);
const yamlCompiler = new YamlCompiler(cubeSymbols, cubeDictionary, nativeInstance, viewCompiler);

const transpilers: TranspilerInterface[] = [
new ValidationTranspiler(),
new ImportExportTranspiler(),
new CubePropContextTranspiler(cubeSymbols, cubeDictionary),
new CubePropContextTranspiler(cubeSymbols, cubeDictionary, viewCompiler),
];

if (!options.allowJsDuplicatePropsInSchema) {
Expand All @@ -60,6 +63,7 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp
cubeNameCompilers: [cubeDictionary],
preTranspileCubeCompilers: [cubeSymbols, cubeValidator],
transpilers,
viewCompilers: [viewCompiler],
cubeCompilers: [cubeEvaluator, joinGraph, metaTransformer],
contextCompilers: [contextEvaluator],
cubeFactory: cubeSymbols.createCube.bind(cubeSymbols),
Expand All @@ -72,7 +76,7 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp
compileContext: options.compileContext,
standalone: options.standalone,
nativeInstance,
yamlCompiler
yamlCompiler,
}, options));

return {
Expand Down
5 changes: 4 additions & 1 deletion packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export class YamlCompiler {
private readonly cubeSymbols: CubeSymbols,
private readonly cubeDictionary: CubeDictionary,
private readonly nativeInstance: NativeInstance,
private readonly viewCompiler: CubeSymbols,
) {
}

Expand Down Expand Up @@ -288,7 +289,9 @@ export class YamlCompiler {
},
);

resolveSymbol = resolveSymbol || (n => this.cubeSymbols.resolveSymbol(cubeName, n) || this.cubeSymbols.isCurrentCube(n));
resolveSymbol = resolveSymbol || (n => this.viewCompiler.resolveSymbol(cubeName, n) ||
this.cubeSymbols.resolveSymbol(cubeName, n) ||
this.cubeSymbols.isCurrentCube(n));

const traverseObj = {
Program: (babelPath) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class CubePropContextTranspiler implements TranspilerInterface {
public constructor(
protected readonly cubeSymbols: CubeSymbols,
protected readonly cubeDictionary: CubeDictionary,
protected readonly viewCompiler: CubeSymbols,
) {
}

Expand Down Expand Up @@ -88,7 +89,9 @@ export class CubePropContextTranspiler implements TranspilerInterface {
}

protected sqlAndReferencesFieldVisitor(cubeName): TraverseObject {
const resolveSymbol = n => this.cubeSymbols.resolveSymbol(cubeName, n) || this.cubeSymbols.isCurrentCube(n);
const resolveSymbol = n => this.viewCompiler.resolveSymbol(cubeName, n) ||
this.cubeSymbols.resolveSymbol(cubeName, n) ||
this.cubeSymbols.isCurrentCube(n);

return {
ObjectProperty: (path) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
views:
- name: users_view
cubes:
- join_path: users
includes: "*"

access_policy:
- role: '*'
row_level:
filters:
- member: id
operator: gt
values: [10]
16 changes: 16 additions & 0 deletions packages/cubejs-testing/birdbox-fixtures/rbac/model/views/users.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
view('users_view', {
cubes: [{
join_path: users,
includes: '*',
}],
accessPolicy: [{
role: '*',
rowLevel: {
filters: [{
member: 'id',
operator: 'gt',
values: [10],
}],
},
}]
});
150 changes: 150 additions & 0 deletions packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,81 @@ Array [
]
`;

exports[`Cube RBAC Engine [Python config] RBAC via SQL API [python config] SELECT * from users_view: users_view_python 1`] = `
Array [
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 400,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 401,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 402,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 403,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 404,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 405,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 406,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 407,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 408,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 409,
},
]
`;

exports[`Cube RBAC Engine [Python config][dev mode] products with no matching policy: products_no_policy_python 1`] = `
Array [
Object {
Expand Down Expand Up @@ -611,6 +686,81 @@ Array [
]
`;

exports[`Cube RBAC Engine RBAC via SQL API SELECT * from users_view: users_view_js 1`] = `
Array [
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 400,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 401,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 402,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 403,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 404,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 405,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 406,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 407,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 408,
},
Object {
"__cubeJoinField": null,
"__user": null,
"city": "Austin",
"count": "1",
"id": 409,
},
]
`;

exports[`Cube RBAC Engine RBAC via SQL API default policy SELECT with member expressions: users_member_expression 1`] = `
Array [
Object {
Expand Down
12 changes: 12 additions & 0 deletions packages/cubejs-testing/test/smoke-rbac.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ describe('Cube RBAC Engine', () => {
// Querying a cube with nested filters and mixed values should not cause any issues
expect(res.rows).toMatchSnapshot('users');
});

test('SELECT * from users_view', async () => {
const res = await connection.query('SELECT * FROM users_view limit 10');
// Make sure view policies are evaluated correctly in yaml schemas
expect(res.rows).toMatchSnapshot('users_view_js');
});
});

describe('RBAC via SQL API manager', () => {
Expand Down Expand Up @@ -398,6 +404,12 @@ describe('Cube RBAC Engine [Python config]', () => {
// It should also exclude the `created_at` dimension as per memberLevel policy
expect(res.rows).toMatchSnapshot('users_python');
});

test('SELECT * from users_view', async () => {
const res = await connection.query('SELECT * FROM users_view limit 10');
// Make sure view policies are evaluated correctly in yaml schemas
expect(res.rows).toMatchSnapshot('users_view_python');
});
});
});

Expand Down

0 comments on commit 84d1489

Please sign in to comment.