From 459e3c5eddf57f1bc0d7106fffd28782cfa7e408 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Fri, 13 May 2022 18:09:39 +0100 Subject: [PATCH 1/3] Addresses #5850 - when importing/migrating views and building new views it checks if calculations are used and if they are then it does an empty check to decide whether or not the fields should be included in the calculation, required for real CouchDB nodes. --- .../server/src/api/controllers/view/utils.js | 6 ++-- .../src/api/controllers/view/viewBuilder.js | 33 ++++++++++++++----- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/controllers/view/utils.js b/packages/server/src/api/controllers/view/utils.js index 59d169ef7f..5bddbf345c 100644 --- a/packages/server/src/api/controllers/view/utils.js +++ b/packages/server/src/api/controllers/view/utils.js @@ -7,6 +7,7 @@ const { } = require("../../../db/utils") const env = require("../../../environment") const { getAppDB } = require("@budibase/backend-core/context") +const viewBuilder = require("./viewBuilder") exports.getView = async viewName => { const db = getAppDB() @@ -114,7 +115,8 @@ exports.deleteView = async viewName => { exports.migrateToInMemoryView = async (db, viewName) => { // delete the view initially const designDoc = await db.get("_design/database") - const view = designDoc.views[viewName] + // run the view back through the view builder to update it + const view = viewBuilder(designDoc.views[viewName].meta) delete designDoc.views[viewName] await db.put(designDoc) await exports.saveView(db, null, viewName, view) @@ -123,7 +125,7 @@ exports.migrateToInMemoryView = async (db, viewName) => { exports.migrateToDesignView = async (db, viewName) => { let view = await db.get(generateMemoryViewID(viewName)) const designDoc = await db.get("_design/database") - designDoc.views[viewName] = view.view + designDoc.views[viewName] = viewBuilder(view.view.meta) await db.put(designDoc) await db.remove(view._id, view._rev) } diff --git a/packages/server/src/api/controllers/view/viewBuilder.js b/packages/server/src/api/controllers/view/viewBuilder.js index 6e2e5c8527..125964a50e 100644 --- a/packages/server/src/api/controllers/view/viewBuilder.js +++ b/packages/server/src/api/controllers/view/viewBuilder.js @@ -10,6 +10,12 @@ const TOKEN_MAP = { OR: "||", } +const CONDITIONS = { + EMPTY: "EMPTY", + NOT_EMPTY: "NOT_EMPTY", + CONTAINS: "CONTAINS", +} + const isEmptyExpression = key => { return `( doc["${key}"] === undefined || @@ -77,13 +83,13 @@ function parseFilterExpression(filters) { expression.push(TOKEN_MAP[filter.conjunction]) } - if (filter.condition === "CONTAINS") { + if (filter.condition === CONDITIONS.CONTAINS) { expression.push( `doc["${filter.key}"].${TOKEN_MAP[filter.condition]}("${filter.value}")` ) - } else if (filter.condition === "EMPTY") { + } else if (filter.condition === CONDITIONS.EMPTY) { expression.push(isEmptyExpression(filter.key)) - } else if (filter.condition === "NOT_EMPTY") { + } else if (filter.condition === CONDITIONS.NOT_EMPTY) { expression.push(`!${isEmptyExpression(filter.key)}`) } else { const value = @@ -125,12 +131,6 @@ function viewTemplate({ field, tableId, groupBy, filters = [], calculation }) { if (filters && filters.length > 0 && filters[0].conjunction) { delete filters[0].conjunction } - const parsedFilters = parseFilterExpression(filters) - const filterExpression = parsedFilters ? `&& (${parsedFilters})` : "" - - const emitExpression = parseEmitExpression(field, groupBy) - - const reduction = field && calculation ? { reduce: `_${calculation}` } : {} let schema = null @@ -139,8 +139,23 @@ function viewTemplate({ field, tableId, groupBy, filters = [], calculation }) { ...(groupBy ? GROUP_PROPERTY : FIELD_PROPERTY), ...SCHEMA_MAP[calculation], } + if ( + !filters.find( + filter => + filter.key === field && filter.condition === CONDITIONS.NOT_EMPTY + ) + ) { + filters.push({ key: field, condition: CONDITIONS.NOT_EMPTY }) + } } + const parsedFilters = parseFilterExpression(filters) + const filterExpression = parsedFilters ? `&& (${parsedFilters})` : "" + + const emitExpression = parseEmitExpression(field, groupBy) + + const reduction = field && calculation ? { reduce: `_${calculation}` } : {} + return { meta: { field, From 08f53591e61a44a6019aa35b74587afe68b356c3 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 16 May 2022 12:31:07 +0100 Subject: [PATCH 2/3] Fixing test cases. --- .../src/middleware/passport/tests/oidc.spec.js | 14 ++++++++------ .../__snapshots__/viewBuilder.spec.js.snap | 14 ++++++++++++-- .../server/src/api/routes/tests/view.spec.js | 7 ++++++- packages/server/yarn.lock | 18 +++++++++--------- packages/worker/yarn.lock | 18 +++++++++--------- 5 files changed, 44 insertions(+), 27 deletions(-) diff --git a/packages/backend-core/src/middleware/passport/tests/oidc.spec.js b/packages/backend-core/src/middleware/passport/tests/oidc.spec.js index bfe9f97dc0..c5e9fe0034 100644 --- a/packages/backend-core/src/middleware/passport/tests/oidc.spec.js +++ b/packages/backend-core/src/middleware/passport/tests/oidc.spec.js @@ -71,7 +71,7 @@ describe("oidc", () => { describe("authenticate", () => { afterEach(() => { - jest.clearAllMocks(); + jest.clearAllMocks() }); // mock third party common authentication @@ -80,10 +80,10 @@ describe("oidc", () => { // mock the passport callback const mockDone = jest.fn() + const mockSaveUserFn = jest.fn() async function doAuthenticate() { const oidc = require("../oidc") - const mockSaveUserFn = jest.fn() const authenticate = await oidc.buildVerifyFn(mockSaveUserFn) await authenticate( @@ -105,11 +105,13 @@ describe("oidc", () => { expect(authenticateThirdParty).toHaveBeenCalledWith( user, false, - mockDone) + mockDone, + mockSaveUserFn, + ) } it("delegates authentication to third party common", async () => { - doTest() + await doTest() }) it("uses JWT email to get email", async () => { @@ -118,7 +120,7 @@ describe("oidc", () => { email : "mock@budibase.com" } - doTest() + await doTest() }) it("uses JWT username to get email", async () => { @@ -127,7 +129,7 @@ describe("oidc", () => { preferred_username : "mock@budibase.com" } - doTest() + await doTest() }) it("uses JWT invalid username to get email", async () => { diff --git a/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap b/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap index 27c8615a46..99f67593fd 100644 --- a/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap +++ b/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap @@ -3,14 +3,24 @@ exports[`viewBuilder Calculate creates a view with the calculation statistics schema 1`] = ` Object { "map": "function (doc) { - if (doc.tableId === \\"14f1c4e94d6a47b682ce89d35d4c78b0\\" ) { + if (doc.tableId === \\"14f1c4e94d6a47b682ce89d35d4c78b0\\" && (!( + doc[\\"myField\\"] === undefined || + doc[\\"myField\\"] === null || + doc[\\"myField\\"] === \\"\\" || + (Array.isArray(doc[\\"myField\\"]) && doc[\\"myField\\"].length === 0) + ))) { emit(doc[\\"_id\\"], doc[\\"myField\\"]); } }", "meta": Object { "calculation": "stats", "field": "myField", - "filters": Array [], + "filters": Array [ + Object { + "condition": "NOT_EMPTY", + "key": "myField", + }, + ], "groupBy": undefined, "schema": Object { "avg": Object { diff --git a/packages/server/src/api/routes/tests/view.spec.js b/packages/server/src/api/routes/tests/view.spec.js index b1c5f655c6..48d7f8c552 100644 --- a/packages/server/src/api/routes/tests/view.spec.js +++ b/packages/server/src/api/routes/tests/view.spec.js @@ -72,7 +72,12 @@ describe("/views", () => { field: "Price", calculation: "stats", tableId: table._id, - filters: [], + filters: [ + { + condition: "NOT_EMPTY", + key: "Price", + } + ], schema: { sum: { type: "number", diff --git a/packages/server/yarn.lock b/packages/server/yarn.lock index d7212c2f48..f5e51de813 100644 --- a/packages/server/yarn.lock +++ b/packages/server/yarn.lock @@ -1014,10 +1014,10 @@ resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== -"@budibase/backend-core@1.0.154": - version "1.0.154" - resolved "https://registry.yarnpkg.com/@budibase/backend-core/-/backend-core-1.0.154.tgz#c310834892e7621778b07579464955487c5c9830" - integrity sha512-mcZxt8XhGgOB4XRHKkWTvBEI4HGp2bo8qyzOJRCvDqlg56S9zqGJDl75Z0N/Wc8N3I53QRcxISerj48odX172A== +"@budibase/backend-core@1.0.160": + version "1.0.160" + resolved "https://registry.yarnpkg.com/@budibase/backend-core/-/backend-core-1.0.160.tgz#000e3b5a3ed91e73a542b4caa202a6f147d91294" + integrity sha512-XfAFU6sRPrCSEKlm58WeuPw8lUoJK+KwO0tcbT+bB2Nb7XCHplskryEgk/PM9ujRU6SMPDx11zKeqRebHlycbA== dependencies: "@techpass/passport-openidconnect" "^0.3.0" aws-sdk "^2.901.0" @@ -1091,12 +1091,12 @@ svelte-flatpickr "^3.2.3" svelte-portal "^1.0.0" -"@budibase/pro@1.0.154": - version "1.0.154" - resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-1.0.154.tgz#f4e31e30376b206159b711224038141d73a1118e" - integrity sha512-+O6bemrcgyWG4a+D5dIOoZ+LGjW4aN7tRdFeZqoaIPCc1pA6zNtLUkM1nb+Laafuwq2Aht37vEuaRy7jfzVprA== +"@budibase/pro@1.0.160": + version "1.0.160" + resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-1.0.160.tgz#921c4e3f65b866d84292644dfd8793c4d0b667c7" + integrity sha512-p+Jhnk1n98CWCJXydSQSO7a+HDpqGAHekGQbOR7aayuwuoYzyOXxTcHNLdBp+3lkXhLSZq9oIwfEGpgdrrhXPA== dependencies: - "@budibase/backend-core" "1.0.154" + "@budibase/backend-core" "1.0.160" node-fetch "^2.6.1" "@budibase/standard-components@^0.9.139": diff --git a/packages/worker/yarn.lock b/packages/worker/yarn.lock index a5e4c2d9e7..1b31c8ef50 100644 --- a/packages/worker/yarn.lock +++ b/packages/worker/yarn.lock @@ -293,10 +293,10 @@ resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39" integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw== -"@budibase/backend-core@1.0.154": - version "1.0.154" - resolved "https://registry.yarnpkg.com/@budibase/backend-core/-/backend-core-1.0.154.tgz#c310834892e7621778b07579464955487c5c9830" - integrity sha512-mcZxt8XhGgOB4XRHKkWTvBEI4HGp2bo8qyzOJRCvDqlg56S9zqGJDl75Z0N/Wc8N3I53QRcxISerj48odX172A== +"@budibase/backend-core@1.0.160": + version "1.0.160" + resolved "https://registry.yarnpkg.com/@budibase/backend-core/-/backend-core-1.0.160.tgz#000e3b5a3ed91e73a542b4caa202a6f147d91294" + integrity sha512-XfAFU6sRPrCSEKlm58WeuPw8lUoJK+KwO0tcbT+bB2Nb7XCHplskryEgk/PM9ujRU6SMPDx11zKeqRebHlycbA== dependencies: "@techpass/passport-openidconnect" "^0.3.0" aws-sdk "^2.901.0" @@ -321,12 +321,12 @@ uuid "^8.3.2" zlib "^1.0.5" -"@budibase/pro@1.0.154": - version "1.0.154" - resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-1.0.154.tgz#f4e31e30376b206159b711224038141d73a1118e" - integrity sha512-+O6bemrcgyWG4a+D5dIOoZ+LGjW4aN7tRdFeZqoaIPCc1pA6zNtLUkM1nb+Laafuwq2Aht37vEuaRy7jfzVprA== +"@budibase/pro@1.0.160": + version "1.0.160" + resolved "https://registry.yarnpkg.com/@budibase/pro/-/pro-1.0.160.tgz#921c4e3f65b866d84292644dfd8793c4d0b667c7" + integrity sha512-p+Jhnk1n98CWCJXydSQSO7a+HDpqGAHekGQbOR7aayuwuoYzyOXxTcHNLdBp+3lkXhLSZq9oIwfEGpgdrrhXPA== dependencies: - "@budibase/backend-core" "1.0.154" + "@budibase/backend-core" "1.0.160" node-fetch "^2.6.1" "@cspotcode/source-map-consumer@0.8.0": From 7b07cff0c6e1f5a34555078fa2300cc0147ad0e8 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 16 May 2022 13:37:00 +0100 Subject: [PATCH 3/3] Updating view builder to handle if stats has a filter as well, don't need a conjuction this way. --- .../__snapshots__/viewBuilder.spec.js.snap | 57 +++++++++++++++++-- .../view/tests/viewBuilder.spec.js | 18 ++++++ .../src/api/controllers/view/viewBuilder.js | 14 +++-- .../server/src/api/routes/tests/view.spec.js | 7 +-- 4 files changed, 81 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap b/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap index 99f67593fd..4572a8a24f 100644 --- a/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap +++ b/packages/server/src/api/controllers/view/tests/__snapshots__/viewBuilder.spec.js.snap @@ -1,14 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`viewBuilder Calculate creates a view with the calculation statistics schema 1`] = ` +exports[`viewBuilder Calculate and filter creates a view with the calculation statistics and filter schema 1`] = ` Object { "map": "function (doc) { - if (doc.tableId === \\"14f1c4e94d6a47b682ce89d35d4c78b0\\" && (!( + if ((doc.tableId === \\"14f1c4e94d6a47b682ce89d35d4c78b0\\" && !( doc[\\"myField\\"] === undefined || doc[\\"myField\\"] === null || doc[\\"myField\\"] === \\"\\" || (Array.isArray(doc[\\"myField\\"]) && doc[\\"myField\\"].length === 0) - ))) { + )) && (doc[\\"age\\"] > 17)) { emit(doc[\\"_id\\"], doc[\\"myField\\"]); } }", @@ -17,8 +17,9 @@ Object { "field": "myField", "filters": Array [ Object { - "condition": "NOT_EMPTY", - "key": "myField", + "condition": "MT", + "key": "age", + "value": 17, }, ], "groupBy": undefined, @@ -51,6 +52,52 @@ Object { } `; +exports[`viewBuilder Calculate creates a view with the calculation statistics schema 1`] = ` +Object { + "map": "function (doc) { + if ((doc.tableId === \\"14f1c4e94d6a47b682ce89d35d4c78b0\\" && !( + doc[\\"myField\\"] === undefined || + doc[\\"myField\\"] === null || + doc[\\"myField\\"] === \\"\\" || + (Array.isArray(doc[\\"myField\\"]) && doc[\\"myField\\"].length === 0) + )) ) { + emit(doc[\\"_id\\"], doc[\\"myField\\"]); + } + }", + "meta": Object { + "calculation": "stats", + "field": "myField", + "filters": Array [], + "groupBy": undefined, + "schema": Object { + "avg": Object { + "type": "number", + }, + "count": Object { + "type": "number", + }, + "field": Object { + "type": "string", + }, + "max": Object { + "type": "number", + }, + "min": Object { + "type": "number", + }, + "sum": Object { + "type": "number", + }, + "sumsqr": Object { + "type": "number", + }, + }, + "tableId": "14f1c4e94d6a47b682ce89d35d4c78b0", + }, + "reduce": "_stats", +} +`; + exports[`viewBuilder Filter creates a view with multiple filters and conjunctions 1`] = ` Object { "map": "function (doc) { diff --git a/packages/server/src/api/controllers/view/tests/viewBuilder.spec.js b/packages/server/src/api/controllers/view/tests/viewBuilder.spec.js index d1674bca08..58fb68cfa7 100644 --- a/packages/server/src/api/controllers/view/tests/viewBuilder.spec.js +++ b/packages/server/src/api/controllers/view/tests/viewBuilder.spec.js @@ -44,4 +44,22 @@ describe("viewBuilder", () => { })).toMatchSnapshot() }) }) + + describe("Calculate and filter", () => { + it("creates a view with the calculation statistics and filter schema", () => { + expect(viewTemplate({ + "name": "Calculate View", + "field": "myField", + "calculation": "stats", + "tableId": "14f1c4e94d6a47b682ce89d35d4c78b0", + "filters": [ + { + "value": 17, + "condition": "MT", + "key": "age", + } + ] + })).toMatchSnapshot() + }) + }) }); \ No newline at end of file diff --git a/packages/server/src/api/controllers/view/viewBuilder.js b/packages/server/src/api/controllers/view/viewBuilder.js index 125964a50e..6596e0d9e7 100644 --- a/packages/server/src/api/controllers/view/viewBuilder.js +++ b/packages/server/src/api/controllers/view/viewBuilder.js @@ -132,7 +132,8 @@ function viewTemplate({ field, tableId, groupBy, filters = [], calculation }) { delete filters[0].conjunction } - let schema = null + let schema = null, + statFilter = null if (calculation) { schema = { @@ -145,7 +146,9 @@ function viewTemplate({ field, tableId, groupBy, filters = [], calculation }) { filter.key === field && filter.condition === CONDITIONS.NOT_EMPTY ) ) { - filters.push({ key: field, condition: CONDITIONS.NOT_EMPTY }) + statFilter = parseFilterExpression([ + { key: field, condition: CONDITIONS.NOT_EMPTY }, + ]) } } @@ -153,7 +156,10 @@ function viewTemplate({ field, tableId, groupBy, filters = [], calculation }) { const filterExpression = parsedFilters ? `&& (${parsedFilters})` : "" const emitExpression = parseEmitExpression(field, groupBy) - + const tableExpression = `doc.tableId === "${tableId}"` + const coreExpression = statFilter + ? `(${tableExpression} && ${statFilter})` + : tableExpression const reduction = field && calculation ? { reduce: `_${calculation}` } : {} return { @@ -166,7 +172,7 @@ function viewTemplate({ field, tableId, groupBy, filters = [], calculation }) { calculation, }, map: `function (doc) { - if (doc.tableId === "${tableId}" ${filterExpression}) { + if (${coreExpression} ${filterExpression}) { ${emitExpression} } }`, diff --git a/packages/server/src/api/routes/tests/view.spec.js b/packages/server/src/api/routes/tests/view.spec.js index 48d7f8c552..b1c5f655c6 100644 --- a/packages/server/src/api/routes/tests/view.spec.js +++ b/packages/server/src/api/routes/tests/view.spec.js @@ -72,12 +72,7 @@ describe("/views", () => { field: "Price", calculation: "stats", tableId: table._id, - filters: [ - { - condition: "NOT_EMPTY", - key: "Price", - } - ], + filters: [], schema: { sum: { type: "number",