Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(schema-compiler): make sure view members are resolvable in DAP #9059

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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]
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
Loading