Skip to content

feat(schema-compiler,api-gateway): Nested folders support #9659

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

Open
wants to merge 5 commits 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
4 changes: 3 additions & 1 deletion packages/cubejs-api-gateway/openspec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,9 @@ components:
members:
type: array
items:
type: "string"
oneOf:
- type: string
- $ref: "#/components/schemas/V1CubeMetaFolder"
V1CubeMetaHierarchy:
type: "object"
required:
Expand Down
65 changes: 41 additions & 24 deletions packages/cubejs-schema-compiler/src/compiler/CubeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,34 +259,51 @@ export class CubeEvaluator extends CubeSymbols {

private prepareFolders(cube: any, errorReporter: ErrorReporter) {
const folders = cube.rawFolders();
if (folders.length) {
cube.folders = folders.map(it => {
const includedMembers = this.allMembersOrList(cube, it.includes);
const includes = includedMembers.map(memberName => {
if (memberName.includes('.')) {
errorReporter.error(
`Paths aren't allowed in the 'folders' but '${memberName}' has been provided for ${cube.name}`
);
}
if (!folders.length) return;

const member = cube.includedMembers.find(m => m.name === memberName);
if (!member) {
errorReporter.error(
`Member '${memberName}' included in folder '${it.name}' not found`
);
return null;
}
const checkMember = (memberName: string, folderName: string) => {
if (memberName.includes('.')) {
errorReporter.error(
`Paths aren't allowed in the 'folders' but '${memberName}' has been provided for ${cube.name}`
);
}

const member = cube.includedMembers.find(m => m.name === memberName);
if (!member) {
errorReporter.error(
`Member '${memberName}' included in folder '${folderName}' not found`
);
return null;
}

return member;
})
.filter(Boolean);
return member;
};

const processFolder = (folder: any): any => {
let includedMembers: string[];
let includes: any[] = [];

if (folder.includes === '*') {
includedMembers = this.allMembersOrList(cube, folder.includes);
includes = includedMembers.map(m => checkMember(m, folder.name)).filter(Boolean);
} else if (Array.isArray(folder.includes)) {
includes = folder.includes.map(item => {
if (typeof item === 'object' && item !== null) {
return processFolder(item);
}

return ({
...it,
includes
return checkMember(item, folder.name);
});
});
}
}

return {
...folder,
type: 'folder',
includes: includes.filter(Boolean)
};
};

cube.folders = folders.map(processFolder);
}

private prepareHierarchies(cube: any, errorReporter: ErrorReporter): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ export class CubeToMetaTransformer {

const isCubeVisible = this.isVisible(cube, true);

const processFolderMember = (member) => {
if (member.type === 'folder') {
return {
name: member.name,
members: member.includes.map(processFolderMember),
};
}

return `${cube.name}.${member.name}`;
};

return {
config: {
name: cube.name,
Expand Down Expand Up @@ -115,7 +126,7 @@ export class CubeToMetaTransformer {
})),
folders: (cube.folders || []).map((it) => ({
name: it.name,
members: it.includes.map(member => `${cube.name}.${member.name}`),
members: it.includes.map(processFolderMember),
})),
},
};
Expand Down
21 changes: 14 additions & 7 deletions packages/cubejs-schema-compiler/src/compiler/CubeValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,19 @@ const cubeSchema = inherit(baseSchema, {
'object.xor': 'You must use either sql or sqlTable within a model, but not both'
});

const folderSchema = Joi.object().keys({
name: Joi.string().required(),
includes: Joi.alternatives([
Joi.string().valid('*'),
Joi.array().items(
Joi.alternatives([
Joi.string().required(),
Joi.link('#folderSchema'), // Can contain nested folders
]),
),
]).required(),
}).id('folderSchema');

const viewSchema = inherit(baseSchema, {
isView: Joi.boolean().strict(),
cubes: Joi.array().items(
Expand Down Expand Up @@ -817,13 +830,7 @@ const viewSchema = inherit(baseSchema, {
'object.oxor': 'Using split together with prefix is not supported'
})
),
folders: Joi.array().items(Joi.object().keys({
name: Joi.string().required(),
includes: Joi.alternatives([
Joi.string().valid('*'),
Joi.array().items(Joi.string().required())
]).required(),
})),
folders: Joi.array().items(folderSchema),
});

function formatErrorMessageFromDetails(explain, d) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1816,6 +1816,7 @@ Array [
},
],
"name": "folder1",
"type": "folder",
},
Object {
"includes": Array [
Expand All @@ -1831,6 +1832,7 @@ Array [
},
],
"name": "folder2",
"type": "folder",
},
]
`;
11 changes: 11 additions & 0 deletions packages/cubejs-schema-compiler/test/unit/fixtures/folders.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,17 @@ views:
includes:
- users_city
- users_renamed_in_view3_gender
- name: test_view4
extends: test_view3
folders:
- name: folder3
includes:
- users_city
- name: inner folder 4
includes:
- renamed_orders_status
- name: inner folder 5
includes: "*"

# - name: empty_view
# cubes:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ views:
- age
- renamed_gender
- users.age
- name: inner folder
includes:
- users.renamed_gender
- name: folder2
includes: '*'
- name: test_view2
Expand Down
45 changes: 45 additions & 0 deletions packages/cubejs-schema-compiler/test/unit/folders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,50 @@ describe('Cube Folders', () => {
);
});

it('a nested folders with some * and named members', async () => {
const testView = metaTransformer.cubes.find(
(it) => it.config.name === 'test_view4'
);

expect(testView.config.folders.length).toBe(3);

const folder1 = testView.config.folders.find(
(it) => it.name === 'folder1'
);
expect(folder1.members).toEqual([
'test_view4.users_age',
'test_view4.users_state',
'test_view4.renamed_orders_status',
]);

const folder2 = testView.config.folders.find(
(it) => it.name === 'folder2'
);
expect(folder2.members).toEqual(
expect.arrayContaining(['test_view4.users_city', 'test_view4.users_renamed_in_view3_gender'])
);

const folder3 = testView.config.folders.find(
(it) => it.name === 'folder3'
);
expect(folder3.members.length).toBe(3);
expect(folder3.members[1]).toEqual(
{ name: 'inner folder 4', members: ['test_view4.renamed_orders_status'] }
);
expect(folder3.members[2].name).toEqual('inner folder 5');
expect(folder3.members[2].members).toEqual([
'test_view4.renamed_orders_count',
'test_view4.renamed_orders_id',
'test_view4.renamed_orders_number',
'test_view4.renamed_orders_status',
'test_view4.users_age',
'test_view4.users_state',
'test_view4.users_gender',
'test_view4.users_city',
'test_view4.users_renamed_in_view3_gender',
]);
});

it('folders from view extending other view', async () => {
const view2 = metaTransformer.cubes.find(
(it) => it.config.name === 'test_view2'
Expand Down Expand Up @@ -93,6 +137,7 @@ describe('Cube Folders', () => {
throw new Error('should throw earlier');
} catch (e: any) {
expect(e.toString()).toMatch(/Paths aren't allowed in the 'folders' but 'users.age' has been provided for test_view/);
expect(e.toString()).toMatch(/Paths aren't allowed in the 'folders' but 'users.renamed_gender' has been provided for test_view/);
expect(e.toString()).toMatch(/Member 'users.age' included in folder 'folder1' not found/);
}
});
Expand Down
Loading