Skip to content

Commit e7b992e

Browse files
bsod90bsod90
authored
fix(schema-compiler): make sure view members are resolvable in DAP (#9059)
1 parent 7279ac0 commit e7b992e

File tree

10 files changed

+244
-7
lines changed

10 files changed

+244
-7
lines changed

‎packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ export class DataSchemaCompiler {
2323
this.cubeCompilers = options.cubeCompilers || [];
2424
this.contextCompilers = options.contextCompilers || [];
2525
this.transpilers = options.transpilers || [];
26+
this.viewCompilers = options.viewCompilers || [];
2627
this.preTranspileCubeCompilers = options.preTranspileCubeCompilers || [];
28+
this.viewCompilationGate = options.viewCompilationGate;
2729
this.cubeNameCompilers = options.cubeNameCompilers || [];
2830
this.extensions = options.extensions || {};
2931
this.cubeFactory = options.cubeFactory;
@@ -93,7 +95,10 @@ export class DataSchemaCompiler {
9395
const compilePhase = (compilers) => this.compileCubeFiles(compilers, transpile(), errorsReport);
9496

9597
return compilePhase({ cubeCompilers: this.cubeNameCompilers })
96-
.then(() => compilePhase({ cubeCompilers: this.preTranspileCubeCompilers }))
98+
.then(() => compilePhase({ cubeCompilers: this.preTranspileCubeCompilers.concat([this.viewCompilationGate]) }))
99+
.then(() => (this.viewCompilationGate.shouldCompileViews() ?
100+
compilePhase({ cubeCompilers: this.viewCompilers })
101+
: Promise.resolve()))
97102
.then(() => compilePhase({
98103
cubeCompilers: this.cubeCompilers,
99104
contextCompilers: this.contextCompilers,

‎packages/cubejs-schema-compiler/src/compiler/PrepareCompiler.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { JoinGraph } from './JoinGraph';
2020
import { CubeToMetaTransformer } from './CubeToMetaTransformer';
2121
import { CompilerCache } from './CompilerCache';
2222
import { YamlCompiler } from './YamlCompiler';
23+
import { ViewCompilationGate } from './ViewCompilationGate';
2324

2425
export type PrepareCompilerOptions = {
2526
nativeInstance?: NativeInstance,
@@ -37,19 +38,21 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp
3738
const nativeInstance = options.nativeInstance || new NativeInstance();
3839
const cubeDictionary = new CubeDictionary();
3940
const cubeSymbols = new CubeSymbols();
41+
const viewCompiler = new CubeSymbols(true);
42+
const viewCompilationGate = new ViewCompilationGate();
4043
const cubeValidator = new CubeValidator(cubeSymbols);
4144
const cubeEvaluator = new CubeEvaluator(cubeValidator);
4245
const contextEvaluator = new ContextEvaluator(cubeEvaluator);
4346
const joinGraph = new JoinGraph(cubeValidator, cubeEvaluator);
4447
const metaTransformer = new CubeToMetaTransformer(cubeValidator, cubeEvaluator, contextEvaluator, joinGraph);
4548
const { maxQueryCacheSize, maxQueryCacheAge } = options;
4649
const compilerCache = new CompilerCache({ maxQueryCacheSize, maxQueryCacheAge });
47-
const yamlCompiler = new YamlCompiler(cubeSymbols, cubeDictionary, nativeInstance);
50+
const yamlCompiler = new YamlCompiler(cubeSymbols, cubeDictionary, nativeInstance, viewCompiler);
4851

4952
const transpilers: TranspilerInterface[] = [
5053
new ValidationTranspiler(),
5154
new ImportExportTranspiler(),
52-
new CubePropContextTranspiler(cubeSymbols, cubeDictionary),
55+
new CubePropContextTranspiler(cubeSymbols, cubeDictionary, viewCompiler),
5356
];
5457

5558
if (!options.allowJsDuplicatePropsInSchema) {
@@ -60,6 +63,8 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp
6063
cubeNameCompilers: [cubeDictionary],
6164
preTranspileCubeCompilers: [cubeSymbols, cubeValidator],
6265
transpilers,
66+
viewCompilationGate,
67+
viewCompilers: [viewCompiler],
6368
cubeCompilers: [cubeEvaluator, joinGraph, metaTransformer],
6469
contextCompilers: [contextEvaluator],
6570
cubeFactory: cubeSymbols.createCube.bind(cubeSymbols),
@@ -72,7 +77,7 @@ export const prepareCompiler = (repo: SchemaFileRepository, options: PrepareComp
7277
compileContext: options.compileContext,
7378
standalone: options.standalone,
7479
nativeInstance,
75-
yamlCompiler
80+
yamlCompiler,
7681
}, options));
7782

7883
return {

‎packages/cubejs-schema-compiler/src/compiler/ViewCompilationGate.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
export class ViewCompilationGate {
2+
private shouldCompile: any;
3+
4+
public constructor() {
5+
this.shouldCompile = false;
6+
}
7+
8+
public compile(cubes: any[]) {
9+
// When developing Data Access Policies feature, we've came across a
10+
// limitation that Cube members can't be referenced in access policies defined on Views,
11+
// because views aren't (yet) compiled at the time of access policy evaluation.
12+
// To workaround this limitation and additional compilation pass is necessary,
13+
// however it comes with a significant performance penalty.
14+
// This gate check whether the data model contains views with access policies,
15+
// and only then allows the additional compilation pass.
16+
//
17+
// Check out the DataSchemaCompiler.ts to see how this gate is used.
18+
if (this.viewsHaveAccessPolicies(cubes)) {
19+
this.shouldCompile = true;
20+
}
21+
}
22+
23+
private viewsHaveAccessPolicies(cubes: any[]) {
24+
return cubes.some(c => c.isView && c.accessPolicy);
25+
}
26+
27+
public shouldCompileViews() {
28+
return this.shouldCompile;
29+
}
30+
}

‎packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export class YamlCompiler {
3333
private readonly cubeSymbols: CubeSymbols,
3434
private readonly cubeDictionary: CubeDictionary,
3535
private readonly nativeInstance: NativeInstance,
36+
private readonly viewCompiler: CubeSymbols,
3637
) {
3738
}
3839

@@ -288,7 +289,9 @@ export class YamlCompiler {
288289
},
289290
);
290291

291-
resolveSymbol = resolveSymbol || (n => this.cubeSymbols.resolveSymbol(cubeName, n) || this.cubeSymbols.isCurrentCube(n));
292+
resolveSymbol = resolveSymbol || (n => this.viewCompiler.resolveSymbol(cubeName, n) ||
293+
this.cubeSymbols.resolveSymbol(cubeName, n) ||
294+
this.cubeSymbols.isCurrentCube(n));
292295

293296
const traverseObj = {
294297
Program: (babelPath) => {

‎packages/cubejs-schema-compiler/src/compiler/transpilers/CubePropContextTranspiler.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export class CubePropContextTranspiler implements TranspilerInterface {
4141
public constructor(
4242
protected readonly cubeSymbols: CubeSymbols,
4343
protected readonly cubeDictionary: CubeDictionary,
44+
protected readonly viewCompiler: CubeSymbols,
4445
) {
4546
}
4647

@@ -88,7 +89,9 @@ export class CubePropContextTranspiler implements TranspilerInterface {
8889
}
8990

9091
protected sqlAndReferencesFieldVisitor(cubeName): TraverseObject {
91-
const resolveSymbol = n => this.cubeSymbols.resolveSymbol(cubeName, n) || this.cubeSymbols.isCurrentCube(n);
92+
const resolveSymbol = n => this.viewCompiler.resolveSymbol(cubeName, n) ||
93+
this.cubeSymbols.resolveSymbol(cubeName, n) ||
94+
this.cubeSymbols.isCurrentCube(n);
9295

9396
return {
9497
ObjectProperty: (path) => {

‎packages/cubejs-testing/birdbox-fixtures/rbac-python/model/views/users_view.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
views:
2+
- name: users_view
3+
cubes:
4+
- join_path: users
5+
includes: "*"
6+
7+
access_policy:
8+
- role: '*'
9+
row_level:
10+
filters:
11+
- member: id
12+
operator: gt
13+
values: [10]

‎packages/cubejs-testing/birdbox-fixtures/rbac/model/views/users.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
view('users_view', {
2+
cubes: [{
3+
join_path: users,
4+
includes: '*',
5+
}],
6+
accessPolicy: [{
7+
role: '*',
8+
rowLevel: {
9+
filters: [{
10+
member: 'id',
11+
operator: 'gt',
12+
values: [10],
13+
}],
14+
},
15+
}]
16+
});

‎packages/cubejs-testing/test/__snapshots__/smoke-rbac.test.ts.snap‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,81 @@ Array [
88
]
99
`;
1010

11+
exports[`Cube RBAC Engine [Python config] RBAC via SQL API [python config] SELECT * from users_view: users_view_python 1`] = `
12+
Array [
13+
Object {
14+
"__cubeJoinField": null,
15+
"__user": null,
16+
"city": "Austin",
17+
"count": "1",
18+
"id": 400,
19+
},
20+
Object {
21+
"__cubeJoinField": null,
22+
"__user": null,
23+
"city": "Austin",
24+
"count": "1",
25+
"id": 401,
26+
},
27+
Object {
28+
"__cubeJoinField": null,
29+
"__user": null,
30+
"city": "Austin",
31+
"count": "1",
32+
"id": 402,
33+
},
34+
Object {
35+
"__cubeJoinField": null,
36+
"__user": null,
37+
"city": "Austin",
38+
"count": "1",
39+
"id": 403,
40+
},
41+
Object {
42+
"__cubeJoinField": null,
43+
"__user": null,
44+
"city": "Austin",
45+
"count": "1",
46+
"id": 404,
47+
},
48+
Object {
49+
"__cubeJoinField": null,
50+
"__user": null,
51+
"city": "Austin",
52+
"count": "1",
53+
"id": 405,
54+
},
55+
Object {
56+
"__cubeJoinField": null,
57+
"__user": null,
58+
"city": "Austin",
59+
"count": "1",
60+
"id": 406,
61+
},
62+
Object {
63+
"__cubeJoinField": null,
64+
"__user": null,
65+
"city": "Austin",
66+
"count": "1",
67+
"id": 407,
68+
},
69+
Object {
70+
"__cubeJoinField": null,
71+
"__user": null,
72+
"city": "Austin",
73+
"count": "1",
74+
"id": 408,
75+
},
76+
Object {
77+
"__cubeJoinField": null,
78+
"__user": null,
79+
"city": "Austin",
80+
"count": "1",
81+
"id": 409,
82+
},
83+
]
84+
`;
85+
1186
exports[`Cube RBAC Engine [Python config][dev mode] products with no matching policy: products_no_policy_python 1`] = `
1287
Array [
1388
Object {
@@ -611,6 +686,81 @@ Array [
611686
]
612687
`;
613688

689+
exports[`Cube RBAC Engine RBAC via SQL API SELECT * from users_view: users_view_js 1`] = `
690+
Array [
691+
Object {
692+
"__cubeJoinField": null,
693+
"__user": null,
694+
"city": "Austin",
695+
"count": "1",
696+
"id": 400,
697+
},
698+
Object {
699+
"__cubeJoinField": null,
700+
"__user": null,
701+
"city": "Austin",
702+
"count": "1",
703+
"id": 401,
704+
},
705+
Object {
706+
"__cubeJoinField": null,
707+
"__user": null,
708+
"city": "Austin",
709+
"count": "1",
710+
"id": 402,
711+
},
712+
Object {
713+
"__cubeJoinField": null,
714+
"__user": null,
715+
"city": "Austin",
716+
"count": "1",
717+
"id": 403,
718+
},
719+
Object {
720+
"__cubeJoinField": null,
721+
"__user": null,
722+
"city": "Austin",
723+
"count": "1",
724+
"id": 404,
725+
},
726+
Object {
727+
"__cubeJoinField": null,
728+
"__user": null,
729+
"city": "Austin",
730+
"count": "1",
731+
"id": 405,
732+
},
733+
Object {
734+
"__cubeJoinField": null,
735+
"__user": null,
736+
"city": "Austin",
737+
"count": "1",
738+
"id": 406,
739+
},
740+
Object {
741+
"__cubeJoinField": null,
742+
"__user": null,
743+
"city": "Austin",
744+
"count": "1",
745+
"id": 407,
746+
},
747+
Object {
748+
"__cubeJoinField": null,
749+
"__user": null,
750+
"city": "Austin",
751+
"count": "1",
752+
"id": 408,
753+
},
754+
Object {
755+
"__cubeJoinField": null,
756+
"__user": null,
757+
"city": "Austin",
758+
"count": "1",
759+
"id": 409,
760+
},
761+
]
762+
`;
763+
614764
exports[`Cube RBAC Engine RBAC via SQL API default policy SELECT with member expressions: users_member_expression 1`] = `
615765
Array [
616766
Object {

‎packages/cubejs-testing/test/smoke-rbac.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,12 @@ describe('Cube RBAC Engine', () => {
152152
// Querying a cube with nested filters and mixed values should not cause any issues
153153
expect(res.rows).toMatchSnapshot('users');
154154
});
155+
156+
test('SELECT * from users_view', async () => {
157+
const res = await connection.query('SELECT * FROM users_view limit 10');
158+
// Make sure view policies are evaluated correctly in yaml schemas
159+
expect(res.rows).toMatchSnapshot('users_view_js');
160+
});
155161
});
156162

157163
describe('RBAC via SQL API manager', () => {
@@ -398,6 +404,12 @@ describe('Cube RBAC Engine [Python config]', () => {
398404
// It should also exclude the `created_at` dimension as per memberLevel policy
399405
expect(res.rows).toMatchSnapshot('users_python');
400406
});
407+
408+
test('SELECT * from users_view', async () => {
409+
const res = await connection.query('SELECT * FROM users_view limit 10');
410+
// Make sure view policies are evaluated correctly in yaml schemas
411+
expect(res.rows).toMatchSnapshot('users_view_python');
412+
});
401413
});
402414
});
403415

‎yarn.lock‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9890,7 +9890,7 @@
98909890
dependencies:
98919891
"@types/node" "*"
98929892

9893-
"@types/node@*", "@types/node@^12", "@types/node@^16", "@types/node@^18":
9893+
"@types/node@*", "@types/node@^12", "@types/node@^18":
98949894
version "18.19.46"
98959895
resolved "https://registry.yarnpkg.com/@types/node/-/node-18.19.46.tgz#51801396c01153e0626e36f43386e83bc768b072"
98969896
integrity sha512-vnRgMS7W6cKa1/0G3/DTtQYpVrZ8c0Xm6UkLaVFrb9jtcVC3okokW09Ki1Qdrj9ISokszD69nY4WDLRlvHlhAA==

0 commit comments

Comments
 (0)