Skip to content

Commit e60ad99

Browse files
authored
fix(schema-compiler): invalidate Jinja render cache when imported macro file changes (cube-js#10818)
* fix(schema-compiler): CUB-2357 invalidate Jinja render cache when imported macro file changes * fix(schema-compiler): use \0 separator in Jinja cache key for consistency
1 parent 4e26c26 commit e60ad99

2 files changed

Lines changed: 136 additions & 3 deletions

File tree

packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const NATIVE_IS_SUPPORTED = isNativeSupported();
2828
const moduleFileCache = {};
2929

3030
const JINJA_SYNTAX = /{%|%}|{{|}}/ig;
31+
const JINJA_MACRO_DEFINITION = /{%[-+]?\s*macro\s/;
3132

3233
const getThreadsCount = () => {
3334
const envThreads = getEnv('transpilationWorkerThreadsCount');
@@ -100,6 +101,7 @@ export type TranspileOptions = {
100101
compilerId?: string;
101102
stage?: 0 | 1 | 2 | 3;
102103
jinjaUsed?: boolean;
104+
jinjaMacrosFingerprint?: string;
103105
};
104106

105107
export type CompileStage = 0 | 1 | 2 | 3;
@@ -280,6 +282,8 @@ export class DataSchemaCompiler {
280282
this.loadJinjaTemplates(jinjaTemplatedFiles);
281283
}
282284

285+
const jinjaMacrosFingerprint = DataSchemaCompiler.computeJinjaMacrosFingerprint(jinjaTemplatedFiles);
286+
283287
const errorsReport = new ErrorReporter(null, [], this.errorReportOptions);
284288
this.errorsReporter = errorsReport;
285289

@@ -328,11 +332,11 @@ export class DataSchemaCompiler {
328332
}
329333

330334
const jinjaFilesTasks = jinjaTemplatedFiles
331-
.map(f => this.transpileJinjaFile(f, errorsReport, { cubeNames, cubeSymbols, transpilerNames }));
335+
.map(f => this.transpileJinjaFile(f, errorsReport, { cubeNames, cubeSymbols, transpilerNames, jinjaMacrosFingerprint }));
332336

333337
results = (await Promise.all([...jsFilesTasks, ...yamlFilesTasks, ...jinjaFilesTasks])).flat();
334338
} else {
335-
results = await Promise.all(toCompile.map(f => this.transpileFile(f, errorsReport, { cubeNames, cubeSymbols, transpilerNames })));
339+
results = await Promise.all(toCompile.map(f => this.transpileFile(f, errorsReport, { cubeNames, cubeSymbols, transpilerNames, jinjaMacrosFingerprint })));
336340
}
337341

338342
return results.filter(f => !!f) as FileContent[];
@@ -576,6 +580,33 @@ export class DataSchemaCompiler {
576580
});
577581
}
578582

583+
/**
584+
* Macro files are hidden dependencies of any cube file that imports them —
585+
* minijinja resolves `{% import %}` lazily against its template store, so
586+
* the per-file Jinja render cache must be invalidated when *any* macro file
587+
* changes. Hashing all macro files together rather than tracking per-cube
588+
* imports keeps the implementation simple at the cost of over-invalidating
589+
* when macro edits happen (which is rare). CUB-2357.
590+
*/
591+
private static computeJinjaMacrosFingerprint(files: FileContent[]): string {
592+
const macroFiles = files
593+
.filter((f) => JINJA_MACRO_DEFINITION.test(f.content))
594+
.sort((a, b) => a.fileName.localeCompare(b.fileName));
595+
596+
if (macroFiles.length === 0) {
597+
return '';
598+
}
599+
600+
const hash = crypto.createHash('md5');
601+
for (const f of macroFiles) {
602+
hash.update(f.fileName);
603+
hash.update('\0');
604+
hash.update(f.content);
605+
hash.update('\0');
606+
}
607+
return hash.digest('hex');
608+
}
609+
579610
private prepareTranspileSymbols() {
580611
const cubeNames: string[] = this.cubeDictionary.cubeNames();
581612
// We need only cubes and all its member names for transpiling.
@@ -802,7 +833,11 @@ export class DataSchemaCompiler {
802833
errorsReport: ErrorReporter,
803834
options: TranspileOptions
804835
): Promise<(FileContent | undefined)> {
805-
const cacheKey = crypto.createHash('md5').update(file.content).digest('hex');
836+
const cacheKey = crypto.createHash('md5')
837+
.update(file.content)
838+
.update('\0')
839+
.update(options.jinjaMacrosFingerprint || '')
840+
.digest('hex');
806841

807842
let renderedFileContent: string;
808843

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { LRUCache } from 'lru-cache';
2+
import { FileContent, isNativeSupported } from '@cubejs-backend/shared';
3+
4+
import { prepareCompiler } from '../../../src/compiler/PrepareCompiler';
5+
6+
const suite = isNativeSupported() === true ? describe : xdescribe;
7+
8+
const cubeFile = (name: string, extraDimsBlock: string): string => `{% import 'macros.yml' as macros %}
9+
10+
cubes:
11+
- name: ${name}
12+
sql: >
13+
SELECT 1 AS id
14+
15+
dimensions:
16+
- name: id
17+
sql: id
18+
type: number
19+
primary_key: true
20+
${extraDimsBlock}
21+
`;
22+
23+
const macroFile = (dimensionName: string) => `{% macro dimensions() %}
24+
- name: ${dimensionName}
25+
sql: ${dimensionName}
26+
type: string
27+
{% endmacro %}
28+
`;
29+
30+
async function compileWith(files: FileContent[], compiledJinjaCache: LRUCache<string, string>) {
31+
const repo = {
32+
localPath: () => __dirname,
33+
dataSchemaFiles: () => Promise.resolve(files),
34+
};
35+
36+
const { compiler, metaTransformer } = prepareCompiler(repo, {
37+
adapter: 'postgres',
38+
compiledJinjaCache,
39+
} as any);
40+
41+
await compiler.compile();
42+
43+
return { metaTransformer };
44+
}
45+
46+
function dimensionNames(metaTransformer: any, cubeName: string): string[] {
47+
const cube = metaTransformer.cubes.find((c: any) => c.config.name === cubeName);
48+
return cube.config.dimensions.map((d: any) => d.name);
49+
}
50+
51+
suite('Jinja macro cache invalidation', () => {
52+
it('invalidates the cube file render cache when a macro file changes (CUB-2357)', async () => {
53+
const sharedCache = new LRUCache<string, string>({ max: 250 });
54+
55+
const filesV1: FileContent[] = [
56+
{ fileName: 'orders.yml', content: cubeFile('orders', '{{ macros.dimensions() }}') },
57+
{ fileName: 'macros.yml', content: macroFile('status') },
58+
];
59+
60+
const v1 = await compileWith(filesV1, sharedCache);
61+
expect(dimensionNames(v1.metaTransformer, 'orders')).toEqual(['orders.id', 'orders.status']);
62+
63+
const filesV2: FileContent[] = [
64+
filesV1[0],
65+
{ fileName: 'macros.yml', content: macroFile('priority') },
66+
];
67+
68+
const v2 = await compileWith(filesV2, sharedCache);
69+
expect(dimensionNames(v2.metaTransformer, 'orders')).toEqual(['orders.id', 'orders.priority']);
70+
});
71+
72+
it('reuses the render cache for unchanged cube files when a sibling cube file changes', async () => {
73+
const sharedCache = new LRUCache<string, string>({ max: 250 });
74+
75+
const filesV1: FileContent[] = [
76+
{ fileName: 'orders.yml', content: cubeFile('orders', '') },
77+
{ fileName: 'products.yml', content: cubeFile('products', '') },
78+
{ fileName: 'macros.yml', content: macroFile('unused') },
79+
];
80+
await compileWith(filesV1, sharedCache);
81+
82+
const cacheSizeAfterFirstCompile = sharedCache.size;
83+
84+
const filesV2: FileContent[] = [
85+
{
86+
fileName: 'orders.yml',
87+
content: cubeFile('orders', ' - name: status\n sql: status\n type: string\n'),
88+
},
89+
filesV1[1],
90+
filesV1[2],
91+
];
92+
await compileWith(filesV2, sharedCache);
93+
94+
// Only the changed orders.yml should miss the cache; products.yml and
95+
// macros.yml are byte-identical and the macros fingerprint is unchanged.
96+
expect(sharedCache.size).toBe(cacheSizeAfterFirstCompile + 1);
97+
});
98+
});

0 commit comments

Comments
 (0)