diff --git a/lib/change-log.js b/lib/change-log.js index df95a31..ad07513 100644 --- a/lib/change-log.js +++ b/lib/change-log.js @@ -186,12 +186,12 @@ const _getChildChangeObjId = async function ( const _formatCompositionContext = async function (changes, reqData) { const childNodeChanges = [] - for (const change of changes) { + await Promise.all(changes.map(async (change) => { if (typeof change.valueChangedTo === "object") { if (!Array.isArray(change.valueChangedTo)) { change.valueChangedTo = [change.valueChangedTo] } - for (const childNodeChange of change.valueChangedTo) { + await Promise.all(change.valueChangedTo.map(async (childNodeChange) => { const curChange = Object.assign({}, change) const path = childNodeChange._path.split('/') const curNodePathVal = path.pop() @@ -203,10 +203,10 @@ const _formatCompositionContext = async function (changes, reqData) { reqData ) _formatCompositionValue(curChange, objId, childNodeChange, childNodeChanges) - } + })) change.valueChangedTo = undefined } - } + })) changes.push(...childNodeChanges) } @@ -267,7 +267,7 @@ const _getObjectIdByPath = async function ( const _formatObjectID = async function (changes, reqData) { const objectIdCache = new Map() - for (const change of changes) { + await Promise.all(changes.map(async (change) => { const path = change.serviceEntityPath.split('/') const curNodePathVal = path.pop() const parentNodePathVal = path.pop() @@ -295,7 +295,7 @@ const _formatObjectID = async function (changes, reqData) { change.entityID = curNodeObjId change.parentEntityID = parentNodeObjId change.parentKey = getUUIDFromPathVal(parentNodePathVal) - } + })) } const _isCompositionContextPath = function (aPath, hasComp) { @@ -309,6 +309,7 @@ const _isCompositionContextPath = function (aPath, hasComp) { } const _formatChangeLog = async function (changes, req) { + cds.context.dataloaders = {} await _formatObjectID(changes, req.data) await _formatAssociationContext(changes, req.data) await _formatCompositionContext(changes, req.data) diff --git a/lib/entity-helper.js b/lib/entity-helper.js index be564ce..67f930f 100644 --- a/lib/entity-helper.js +++ b/lib/entity-helper.js @@ -1,4 +1,5 @@ const cds = require("@sap/cds") +const DataLoader = require("dataloader"); const LOG = cds.log("change-log") @@ -29,11 +30,21 @@ const getObjIdElementNamesInArray = function (elements) { else return [] } -const getCurObjFromDbQuery = async function (entityName, queryVal, /**optional*/ queryKey='ID') { +const getCurObjFromDbQuery = async function (entityName, queryVal, /**optional*/ queryKey = 'ID') { + if (!(entityName in cds.context.dataloaders)) { + cds.context.dataloaders[entityName] = new DataLoader(async (keys) => { + // REVISIT: This always reads all elements -> should read required ones only! + const results = await SELECT.from(entityName).where(queryKey, 'in', keys) + const resultsByKey = results.reduce((acc, instance) => { + const key = instance[queryKey] + acc[key] = instance + return acc + }, {}) + return keys.map(key => resultsByKey[key] || {}) + }) + } if (!queryVal) return {} - // REVISIT: This always reads all elements -> should read required ones only! - const obj = await SELECT.one.from(entityName).where({[queryKey]: queryVal}) - return obj || {} + return cds.context.dataloaders[entityName].load(queryVal) } const getCurObjFromReqData = function (reqData, nodePathVal, pathVal) { diff --git a/package.json b/package.json index 255f393..1d707be 100644 --- a/package.json +++ b/package.json @@ -37,5 +37,8 @@ "model": "@cap-js/change-tracking" } } + }, + "dependencies": { + "dataloader": "^2.2.2" } } diff --git a/tests/integration/fiori-draft-enabled.test.js b/tests/integration/fiori-draft-enabled.test.js index c114449..a0c18a6 100644 --- a/tests/integration/fiori-draft-enabled.test.js +++ b/tests/integration/fiori-draft-enabled.test.js @@ -1,6 +1,7 @@ const cds = require("@sap/cds"); const bookshop = require("path").resolve(__dirname, "./../bookshop"); const { expect, data, POST, PATCH, DELETE } = cds.test(bookshop); +const { expect: jestExpect } = require('@jest/globals'); const { RequestSend } = require("../utils/api"); jest.setTimeout(5 * 60 * 1000); @@ -1569,4 +1570,50 @@ describe("change log integration test", () => { expect(changeLogs[0].entityKey).to.equal("5ab2a87b-3a56-4d97-a697-7af72334a384"); expect(changeLogs[0].serviceEntity).to.equal("AdminService.BookStores"); }); + + it("Issues-30 Should only optimize reads to the DB", async () => { + const action1 = POST.bind( + {}, + `/odata/v4/admin/BookStores(ID=64625905-c234-4d0d-9bc1-283ee8946770,IsActiveEntity=false)/books`, + { + ID: "9d703c23-54a8-4eff-81c1-cdce6b8376b2", + title: "test title 1", + descr: "test descr", + author_ID: "d4d4a1b3-5b83-4814-8a20-f039af6f0387", + stock: 1, + price: 1.0, + isUsed: true + } + ); + const action2 = POST.bind( + {}, + `/odata/v4/admin/BookStores(ID=64625905-c234-4d0d-9bc1-283ee8946770,IsActiveEntity=false)/books`, + { + ID: "3e53bf3b-53f7-4db8-87ce-8bb0818574d8", + title: "test title 2", + descr: "test descr", + author_ID: "d4d4a1b3-5b83-4814-8a20-f039af6f0387", + stock: 1, + price: 1.0, + isUsed: true + } + ); + const cdsRunSpy = jest.spyOn(cds.db, "run"); + await utils.apiAction("admin", "BookStores", "64625905-c234-4d0d-9bc1-283ee8946770", "AdminService", [action1, action2]); + jestExpect(cdsRunSpy).toHaveBeenCalledWith(jestExpect.objectContaining({ + SELECT: jestExpect.objectContaining({ + from: jestExpect.objectContaining({ ref: ["AdminService.Books"] }), + where: jestExpect.arrayContaining([ + jestExpect.objectContaining({ ref: ["ID"] }), + "in", + { + list: [ + { val: "9d703c23-54a8-4eff-81c1-cdce6b8376b2" }, + { val: "3e53bf3b-53f7-4db8-87ce-8bb0818574d8" } + ] + } + ]) + }) + })); + }); }); diff --git a/tests/utils/api.js b/tests/utils/api.js index d3b77ce..bd3cde1 100644 --- a/tests/utils/api.js +++ b/tests/utils/api.js @@ -8,7 +8,13 @@ class RequestSend { PreserveChanges: true, }); } - await action(); + if (Array.isArray(action)) { + for (const act of action) { + await act(); + } + } else { + await action(); + } await this.post(`/odata/v4/${serviceName}/${entityName}(ID=${id},IsActiveEntity=false)/${path}.draftPrepare`, { SideEffectsQualifier: "", });