feat(schema-compiler): Add duplicate folder name and time shift validation (cube-js#10423)

Author: ovr

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

@@ -318,6 +318,20 @@ export class CubeEvaluator extends CubeSymbols {
     const folders = cube.rawFolders();
     if (!folders.length) return;
 
+    const seen = new Set<string>();
+
+    for (const folder of folders) {
+      if (folder.name && seen.has(folder.name)) {
+        errorReporter.error(
+          `Folder names must be unique within a view. Found duplicate folder '${folder.name}' in view '${cube.name}'.`
+        );
+      }
+
+      if (folder.name) {
+        seen.add(folder.name);
+      }
+    }
+
     const checkMember = (memberName: string, folderName: string): ViewIncludedMember | null => {
       if (memberName.includes('.')) {
         errorReporter.error(

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

@@ -96,14 +96,14 @@ export class YamlCompiler {
 
     for (const key of Object.keys(yamlObj)) {
       if (key === 'cubes') {
-        this.checkDuplicateNames(yamlObj.cubes || [], 'cube', errorsReport);
+        this.checkDuplicateNames(yamlObj.cubes || [], errorsReport, (name) => `Found duplicate cube name '${name}'.`);
 
         (yamlObj.cubes || []).forEach(({ name, ...cube }) => {
           const transpiledCube = this.transpileAndPrepareJsFile('cube', { name, ...cube }, errorsReport);
           transpiledFilesContent.push(transpiledCube);
         });
       } else if (key === 'views') {
-        this.checkDuplicateNames(yamlObj.views || [], 'view', errorsReport);
+        this.checkDuplicateNames(yamlObj.views || [], errorsReport, (name) => `Found duplicate view name '${name}'.`);
 
         (yamlObj.views || []).forEach(({ name, ...cube }) => {
           const transpiledView = this.transpileAndPrepareJsFile('view', { name, ...cube }, errorsReport);
@@ -140,6 +140,7 @@ export class YamlCompiler {
     cubeObj.hierarchies = this.yamlArrayToObj(cubeObj.hierarchies || [], 'hierarchies', errorsReport, ctx);
 
     cubeObj.joins = cubeObj.joins || []; // For edge cases where joins are not defined/null
+
     if (!Array.isArray(cubeObj.joins)) {
       errorsReport.error('joins must be defined as array');
       cubeObj.joins = [];
@@ -320,19 +321,17 @@ export class YamlCompiler {
     return body?.expression;
   }
 
-  private checkDuplicateNames(items: any[], type: 'cube' | 'view', errorsReport: ErrorReporter) {
-    const seen = new Set<string>();
+  private checkDuplicateNames(items: any[], errorsReport: ErrorReporter, message: (name: string) => string) {
+    const names = items
+      .map(item => item?.name)
+      .filter((name): name is string => name != null);
 
-    for (const item of items) {
-      if (item?.name) {
-        if (seen.has(item.name)) {
-          errorsReport.error(
-            `Found duplicate ${type} name '${item.name}'.`
-          );
-        } else {
-          seen.add(item.name);
-        }
+    const seen = new Set<string>();
+    for (const name of names) {
+      if (seen.has(name)) {
+        errorsReport.error(message(name));
       }
+      seen.add(name);
     }
   }
 
@@ -348,29 +347,15 @@ export class YamlCompiler {
     }
 
     // Check for duplicate names
-    const names = yamlArray
-      .map(item => item?.name)
-      .filter(name => name != null);
-
-    const seen = new Set<string>();
-
-    for (const name of names) {
-      if (seen.has(name)) {
-        if (ctx.parent) {
-          errorsReport.error(
-            `Found duplicate ${memberType} '${name}' in ${ctx.parent.type} '${ctx.parent.name}' in cube '${ctx.cubeName}'.`
-          );
-        } else {
-          errorsReport.error(
-            `Member names must be unique within a cube. Found duplicate ${memberType} '${name}' in cube '${ctx.cubeName}'.`
-          );
-        }
+    this.checkDuplicateNames(yamlArray, errorsReport, (name) => {
+      if (ctx.parent) {
+        return `Found duplicate ${memberType} '${name}' in ${ctx.parent.type} '${ctx.parent.name}' in cube '${ctx.cubeName}'.`;
       }
 
-      seen.add(name);
-    }
+      return `Member names must be unique within a cube. Found duplicate ${memberType} '${name}' in cube '${ctx.cubeName}'.`;
+    });
 
-    const remapped = yamlArray.map(({ name, indexes, granularities, ...rest }) => {
+    const remapped = yamlArray.map(({ name, indexes, granularities, timeShift, ...rest }) => {
       if (!name) {
         errorsReport.error(`name isn't defined for ${memberType}: ${JSON.stringify(rest)}`);
         return {};
@@ -393,7 +378,18 @@ export class YamlCompiler {
         res[name] = { granularities, ...res[name] };
       }
 
+      if (timeShift) {
+        this.checkDuplicateNames(
+          timeShift,
+          errorsReport,
+          (shiftName) => `Time shift names must be unique within a ${memberType}. Found duplicate time shift '${shiftName}' in ${memberType} '${name}' in cube '${ctx.cubeName}'.`
+        );
+
+        res[name] = { timeShift, ...res[name] };
+      }
+
       res[name] = { ...res[name], ...rest };
+
       return res;
     });
 

packages/cubejs-schema-compiler/test/unit/yaml-schema.test.ts

@@ -241,6 +241,78 @@ cubes:
         expect(e.message).toContain("Found duplicate dimension.granularity 'fiscal_year' in time dimension 'created_at' in cube 'orders'");
       }
     });
+
+    it('detects duplicate folder names in views', async () => {
+      const { compiler } = prepareYamlCompiler(`
+cubes:
+  - name: orders
+    sql_table: orders
+    dimensions:
+      - name: id
+        sql: id
+        type: number
+        primary_key: true
+      - name: status
+        sql: status
+        type: string
+      - name: category
+        sql: category
+        type: string
+views:
+  - name: orders_view
+    cubes:
+      - join_path: orders
+        includes: "*"
+    folders:
+      - name: Details
+        includes:
+          - id
+          - status
+      - name: Details
+        includes:
+          - category
+      `);
+
+      try {
+        await compiler.compile();
+        throw new Error('compile must return an error');
+      } catch (e: any) {
+        expect(e.message).toContain(
+          "Folder names must be unique within a view. Found duplicate folder 'Details' in view 'orders_view'."
+        );
+      }
+    });
+
+    it('detects duplicate dimension time shifts', async () => {
+      const { compiler } = prepareYamlCompiler(`
+cubes:
+  - name: fiscal_calendar
+    sql: "SELECT 1"
+    dimensions:
+      - name: date_key
+        sql: date_key
+        type: time
+        primary_key: true
+      - name: date
+        sql: calendar_date
+        type: time
+        time_shift:
+          - name: prior_year
+            type: prior
+            interval: 1 year
+          - name: prior_year
+            sql: "{CUBE}.prior_year_date"
+      `);
+
+      try {
+        await compiler.compile();
+        throw new Error('compile must return an error');
+      } catch (e: any) {
+        expect(e.message).toContain(
+          "Time shift names must be unique within a dimension. Found duplicate time shift 'prior_year' in dimension 'date' in cube 'fiscal_calendar'."
+        );
+      }
+    });
   });
 
   it('members must be defined as arrays', async () => {