From 04b3de3ca8dcbf3de20907e241be12b4a446c36e Mon Sep 17 00:00:00 2001 From: D050513 Date: Thu, 31 Oct 2024 11:15:41 +0100 Subject: [PATCH 1/5] stash --- lib/utils.js | 65 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 207dfd2..e8edbe0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -9,7 +9,7 @@ const $visitedUp = Symbol('@cap-js/audit-logging:visitedUp') const $visitedDown = Symbol('@cap-js/audit-logging:visitedDown') const hasPersonalData = entity => { - if (!entity.own($hasPersonalData)) { + if (entity.own($hasPersonalData) == null) { if (!entity['@PersonalData.EntitySemantics']) entity.set($hasPersonalData, false) else { // default role to entity name @@ -20,6 +20,7 @@ const hasPersonalData = entity => { element['@PersonalData.IsPotentiallyPersonal'] || element['@PersonalData.IsPotentiallySensitive'] || (element['@PersonalData.FieldSemantics'] && element['@PersonalData.FieldSemantics'] === 'DataSubjectID')) + // if (hasPersonalData && entity.name.startsWith('CollaborationsService.')) debugger entity.set($hasPersonalData, hasPersonalData) } } @@ -48,6 +49,7 @@ const _isDataSubject = (element, target) => { const getPick = event => { return (element, target) => { + // if (target.name === 'CollaborationsService.Collaborations') debugger if (!hasPersonalData(target)) return const categories = [] if (!element.isAssociation && element.key) categories.push('ObjectID') @@ -121,7 +123,18 @@ const _buildSubSelect = (model, { entity, relative, element, next }, row, previo const targetAlias = _alias(element._target) const relativeAlias = _alias(relative) - childCqn.where(relative._relations[element.name].join(targetAlias, relativeAlias)) + let w = relative._relations[element.name].join(targetAlias, relativeAlias) + + // REVISIT: rewrite to path expression, if alias for relative is already used in subselect + if (previousCqn?._aliases.has(relativeAlias)) { + let t + for (const a in entity.associations) if (entity.associations[a].target === relative.name) t = entity.associations[a] + if (t && w[0]?.xpr) for (const ele of w[0].xpr) if (ele.ref?.[0] === relativeAlias) ele.ref.splice(0, 1, as, t.name) + } + + childCqn.where(w) + + childCqn._aliases = new Set(previousCqn ? [...previousCqn._aliases.values(), as] : [as]) if (previousCqn) childCqn.where('exists', previousCqn) else childCqn.where(_addKeysToWhere(keys, row, as)) @@ -147,22 +160,26 @@ const _getDataSubjectIdQuery = ({ dataSubjectEntity, subs }, row, model) => { } const _getUps = (entity, model) => { - if (entity.own($parents)) return entity[$parents] - const ups = [] - for (const def of Object.values(model.definitions)) { - if (def.kind !== 'entity' || !def.associations) continue - for (const element of Object.values(def.associations)) { - if (element.target !== entity.name || element._isBacklink || element.name === 'SiblingEntity') continue - ups.push(element) + if (entity.own($parents) == null) { + const ups = [] + for (const def of Object.values(model.definitions)) { + if (def.kind !== 'entity' || !def.associations) continue + for (const element of Object.values(def.associations)) { + if (element.target !== entity.name || element._isBacklink || element.name === 'SiblingEntity') continue + ups.push(element) + } } + entity.set($parents, ups) } - return entity.set($parents, ups) + return entity[$parents] } const _getDataSubjectUp = (root, model, entity, prev, next, result) => { - for (const element of _getUps(entity, model)) { + const _ups = _getUps(entity, model) + const __dss = _ups.filter(e => e.parent['@PersonalData.EntitySemantics'] === 'DataSubject') + for (const element of _ups) { // cycle detection - if (!element.own($visitedUp)) element.set($visitedUp, new Set()) + if (element.own($visitedUp) == null) element.set($visitedUp, new Set()) if (element.own($visitedUp).has(root)) continue element.own($visitedUp).add(root) @@ -173,6 +190,7 @@ const _getDataSubjectUp = (root, model, entity, prev, next, result) => { result.subs.push(next || me) return result } else { + // REVISIT: with dfs alone, we don't find the shortest path // dfs is a must here result = _getDataSubjectUp(root, model, element.parent, me, next || me, result) } @@ -192,7 +210,7 @@ const _getDataSubjectDown = (root, entity, prev, next) => { } for (const element of associations) { // cycle detection - if (!element.own($visitedDown)) element.set($visitedDown, new Set()) + if (element.own($visitedDown) == null) element.set($visitedDown, new Set()) if (element.own($visitedDown).has(root)) continue element.own($visitedDown).add(root) @@ -204,12 +222,15 @@ const _getDataSubjectDown = (root, entity, prev, next) => { } const getDataSubject = (entity, model) => { - if (entity.own($dataSubject)) return entity[$dataSubject] - // entities with EntitySemantics 'DataSubjectDetails' or 'Other' must not necessarily - // be always below or always above 'DataSubject' entity in CSN tree - let dataSubjectInfo = _getDataSubjectUp(entity.name, model, entity) - if (!dataSubjectInfo) dataSubjectInfo = _getDataSubjectDown(entity.name, entity) - return entity.set($dataSubject, dataSubjectInfo) + if (entity.own($dataSubject) == null) { + // if (true) { + // entities with EntitySemantics 'DataSubjectDetails' or 'Other' must not necessarily + // be always below or always above 'DataSubject' entity in CSN tree + let dataSubjectInfo = _getDataSubjectUp(entity.name, model, entity) + if (!dataSubjectInfo) dataSubjectInfo = _getDataSubjectDown(entity.name, entity) + entity.set($dataSubject, dataSubjectInfo) + } + return entity[$dataSubject] } const _getDataSubjectsMap = req => { @@ -243,7 +264,11 @@ const resolveDataSubjects = async (logs, req) => { if (map.has(q)) { each.data_subject.id = map.get(q) } else { - const res = await q + let res = await q + // if (res === undefined) { + // res = await q + // debugger + // } map.set(q, res) each.data_subject.id = res } From 2736f63f16b33699441caf90f40c819bda8015ba Mon Sep 17 00:00:00 2001 From: D050513 Date: Thu, 31 Oct 2024 14:12:54 +0100 Subject: [PATCH 2/5] use WeakMap --- lib/modification.js | 4 ++-- lib/utils.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/modification.js b/lib/modification.js index 7148c87..8cf78f9 100644 --- a/lib/modification.js +++ b/lib/modification.js @@ -73,7 +73,7 @@ const _getDataWithAppliedTransitions = (data, req) => { const addDiffToCtx = async function (req) { // store diff in audit data structure at context const _audit = (req.context._audit ??= {}) - if (!_audit.diffs) _audit.diffs = new Map() + if (!_audit.diffs) _audit.diffs = new WeakMap() // get diff let diff = (await req.diff()) || {} @@ -184,7 +184,7 @@ const _calcModificationLogsHandler = async function (req, beforeWrite, that) { const modificationLogs = _getDataModificationLogs(req, that, _audit.diffs.get(mapKey), beforeWrite) // store modificationLogs in audit data structure at context - if (!_audit.modificationLogs) _audit.modificationLogs = new Map() + if (!_audit.modificationLogs) _audit.modificationLogs = new WeakMap() const existingLogs = _audit.modificationLogs.get(mapKey) || {} _audit.modificationLogs.set(mapKey, Object.assign(existingLogs, modificationLogs)) diff --git a/lib/utils.js b/lib/utils.js index e8edbe0..da7b0ea 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -236,7 +236,7 @@ const getDataSubject = (entity, model) => { const _getDataSubjectsMap = req => { const mapKey = getMapKeyForCurrentRequest(req) const _audit = (req.context._audit ??= {}) - if (!_audit.dataSubjects) _audit.dataSubjects = new Map() + if (!_audit.dataSubjects) _audit.dataSubjects = new WeakMap() if (!_audit.dataSubjects.has(mapKey)) _audit.dataSubjects.set(mapKey, new Map()) return _audit.dataSubjects.get(mapKey) } From 043eadd893bcb59f7e0b627a69a65dd7e5cd781a Mon Sep 17 00:00:00 2001 From: D050513 Date: Mon, 4 Nov 2024 10:32:45 +0100 Subject: [PATCH 3/5] AL_WITH_PLACEHOLDERS, AL_OLD_SUBJECT_SEARCH --- lib/utils.js | 127 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 25 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index da7b0ea..4834b3a 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,9 +1,12 @@ +const { AL_WITH_PLACEHOLDERS, AL_OLD_SUBJECT_SEARCH } = process.env + const cds = require('@sap/cds') const WRITE = { CREATE: 1, UPDATE: 1, DELETE: 1 } const $hasPersonalData = Symbol('@cap-js/audit-logging:hasPersonalData') const $dataSubject = Symbol('@cap-js/audit-logging:dataSubject') +const $dataSubjectQuery = Symbol('@cap-js/audit-logging:dataSubjectQuery') const $parents = Symbol('@cap-js/audit-logging:parents') const $visitedUp = Symbol('@cap-js/audit-logging:visitedUp') const $visitedDown = Symbol('@cap-js/audit-logging:visitedDown') @@ -92,7 +95,7 @@ const addDataSubject = (log, row, key, entity) => { } } -const _addKeysToWhere = (keys, row, alias) => { +const _addKeysToWhereWithValues = (keys, row, alias) => { return keys .filter(key => !key.isAssociation && key.name !== 'IsActiveEntity') .reduce((keys, key) => { @@ -102,6 +105,18 @@ const _addKeysToWhere = (keys, row, alias) => { }, []) } +const _addKeysToWhereWithPlaceholders = (keys, row, alias) => { + return keys + .filter(key => !key.isAssociation && key.name !== 'IsActiveEntity') + .reduce((keys, key) => { + if (keys.length) keys.push('and') + keys.push({ ref: [alias, key.name] }, '=', { val: `$$$_${key.name}_$$$` }) + return keys + }, []) +} + +const _addKeysToWhere = AL_WITH_PLACEHOLDERS ? _addKeysToWhereWithPlaceholders : _addKeysToWhereWithValues + const _keyColumns = (keys, alias) => { return keys .filter(key => !key.isAssociation && key.name !== 'IsActiveEntity') @@ -125,20 +140,22 @@ const _buildSubSelect = (model, { entity, relative, element, next }, row, previo let w = relative._relations[element.name].join(targetAlias, relativeAlias) - // REVISIT: rewrite to path expression, if alias for relative is already used in subselect + // REVISIT: rewrite to path expression, if alias for relative is already used in subselect to avoid sql error if (previousCqn?._aliases.has(relativeAlias)) { let t for (const a in entity.associations) if (entity.associations[a].target === relative.name) t = entity.associations[a] if (t && w[0]?.xpr) for (const ele of w[0].xpr) if (ele.ref?.[0] === relativeAlias) ele.ref.splice(0, 1, as, t.name) } + childCqn._aliases = new Set(previousCqn ? [...previousCqn._aliases.values(), as] : [as]) childCqn.where(w) - childCqn._aliases = new Set(previousCqn ? [...previousCqn._aliases.values(), as] : [as]) - if (previousCqn) childCqn.where('exists', previousCqn) else childCqn.where(_addKeysToWhere(keys, row, as)) + // measure distance between root and data subject entity + childCqn._depth = previousCqn ? previousCqn._depth + 1 : 1 + if (next) return _buildSubSelect(model, next, {}, childCqn) return childCqn @@ -153,6 +170,9 @@ const _getDataSubjectIdQuery = ({ dataSubjectEntity, subs }, row, model) => { .columns(_keyColumns(keys, as)) .where(['exists', _buildSubSelect(model, subs[0], row)]) + // measure distance between root and data subject entity + cqn._depth = cqn.SELECT.where[1]._depth + 1 + // entity reused in different branches => must check all for (let i = 1; i < subs.length; i++) cqn.or(['exists', _buildSubSelect(model, subs[i], row)]) @@ -176,7 +196,9 @@ const _getUps = (entity, model) => { const _getDataSubjectUp = (root, model, entity, prev, next, result) => { const _ups = _getUps(entity, model) - const __dss = _ups.filter(e => e.parent['@PersonalData.EntitySemantics'] === 'DataSubject') + + if (!AL_OLD_SUBJECT_SEARCH && _ups.every(e => e.own($visitedUp)?.has(root))) return 'exhausted' + for (const element of _ups) { // cycle detection if (element.own($visitedUp) == null) element.set($visitedUp, new Set()) @@ -186,20 +208,24 @@ const _getDataSubjectUp = (root, model, entity, prev, next, result) => { const me = { entity, relative: element.parent, element } if (prev) prev.next = me if (element.parent['@PersonalData.EntitySemantics'] === 'DataSubject') { - if (!result) result = { dataSubjectEntity: element.parent, subs: [] } + if (!result || typeof result !== 'object') result = { dataSubjectEntity: element.parent, subs: [] } result.subs.push(next || me) return result } else { - // REVISIT: with dfs alone, we don't find the shortest path + // REVISIT: why MUST it be dfs? with dfs alone, we don't find the shortest path // dfs is a must here result = _getDataSubjectUp(root, model, element.parent, me, next || me, result) } } + return result } const _getDataSubjectDown = (root, entity, prev, next) => { const associations = Object.values(entity.associations || {}).filter(e => !e._isBacklink) + + if (!AL_OLD_SUBJECT_SEARCH && associations.every(e => e.own($visitedDown)?.has(root))) return 'exhausted' + // bfs makes more sense here -> check all own assocs first before going deeper for (const element of associations) { const me = { entity, relative: entity, element } @@ -208,6 +234,7 @@ const _getDataSubjectDown = (root, entity, prev, next) => { return { dataSubjectEntity: element._target, subs: [next || me] } } } + for (const element of associations) { // cycle detection if (element.own($visitedDown) == null) element.set($visitedDown, new Set()) @@ -221,18 +248,55 @@ const _getDataSubjectDown = (root, entity, prev, next) => { } } -const getDataSubject = (entity, model) => { +const _getDataSubjectGreedy = (entity, model) => { if (entity.own($dataSubject) == null) { - // if (true) { // entities with EntitySemantics 'DataSubjectDetails' or 'Other' must not necessarily // be always below or always above 'DataSubject' entity in CSN tree let dataSubjectInfo = _getDataSubjectUp(entity.name, model, entity) if (!dataSubjectInfo) dataSubjectInfo = _getDataSubjectDown(entity.name, entity) entity.set($dataSubject, dataSubjectInfo) } - return entity[$dataSubject] + return { dataSubjectInfo: entity[$dataSubject] } } +const _getDataSubjectExhaustive = (entity, model) => { + if (entity.own($dataSubject) == null) { + let dataSubjectInfo + let dataSubjectQuery = { _depth: Infinity } + + while (dataSubjectQuery._depth > 2) { + let up = _getDataSubjectUp(entity.name, model, entity) + if (up === 'exhausted') break + if (up) { + const q = _getDataSubjectIdQuery(up, {}, model) + if (q._depth < dataSubjectQuery._depth) { + dataSubjectInfo = up + dataSubjectQuery = q + } + } + } + + while (dataSubjectQuery._depth > 2) { + let down = _getDataSubjectDown(entity.name, entity) + if (down === 'exhausted') break + if (down) { + const q = _getDataSubjectIdQuery(down, {}, model) + if (q._depth < dataSubjectQuery._depth) { + dataSubjectInfo = down + dataSubjectQuery = q + } + } + } + + entity.set($dataSubject, dataSubjectInfo) + entity.set($dataSubjectQuery, dataSubjectQuery) + } + + return { dataSubjectInfo: entity[$dataSubject], dataSubjectQuery: entity[$dataSubjectQuery] } +} + +const _getDataSubject = AL_OLD_SUBJECT_SEARCH ? _getDataSubjectGreedy : _getDataSubjectExhaustive + const _getDataSubjectsMap = req => { const mapKey = getMapKeyForCurrentRequest(req) const _audit = (req.context._audit ??= {}) @@ -242,38 +306,51 @@ const _getDataSubjectsMap = req => { } const addDataSubjectForDetailsEntity = (row, log, req, entity, model) => { - const dataSubjectInfo = getDataSubject(entity, model) + const { dataSubjectInfo, dataSubjectQuery } = _getDataSubject(entity, model) const role = dataSubjectInfo.dataSubjectEntity['@PersonalData.DataSubjectRole'] log.data_subject.role ??= role log.data_subject.type = dataSubjectInfo.dataSubjectEntity.name + /* * for each req (cf. $batch with atomicity) and data subject role (e.g., customer vs supplier), * store (in audit data structure at context) and reuse a single promise to look up the respective data subject */ const map = _getDataSubjectsMap(req) - if (map.has(role)) log.data_subject.id = map.get(role) - // REVISIT by downward lookups row might already contain ID - some potential to optimize - else map.set(role, _getDataSubjectIdQuery(dataSubjectInfo, row, model)) + + if (AL_WITH_PLACEHOLDERS) { + if (!map.has(role)) { + let y = JSON.stringify(dataSubjectQuery) + for (const each of y.match(/\$\$\$_\w+_\$\$\$/g)) { + const keyName = each.match(/\$\$\$_(\w+)_\$\$\$/)[1] + y = y.replace(each, row[keyName] || row._old?.[keyName]) + } + const z = new SELECT() + z.SELECT = JSON.parse(y).SELECT + map.set(role, z) + } + } else { + if (!map.has(role)) map.set(role, _getDataSubjectIdQuery(dataSubjectInfo, row, model)) + } + + log.data_subject.id = map.get(role) } -const resolveDataSubjects = async (logs, req) => { +const resolveDataSubjects = (logs, req) => { + let ps = [] + const map = _getDataSubjectsMap(req) for (const each of Object.values(logs)) { if (each.data_subject.id instanceof cds.ql.Query) { const q = each.data_subject.id - if (map.has(q)) { - each.data_subject.id = map.get(q) - } else { - let res = await q - // if (res === undefined) { - // res = await q - // debugger - // } - map.set(q, res) - each.data_subject.id = res + if (!map.has(q)) { + const p = cds.run(q).then(res => each.data_subject.id = res) + map.set(q, p) + ps.push(p) } } } + + return Promise.all(ps) } module.exports = { From 15554ce9cec9db60c625d6c47df3ce0b486954c9 Mon Sep 17 00:00:00 2001 From: sjvans <30337871+sjvans@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:13:50 +0100 Subject: [PATCH 4/5] Apply suggestions from code review --- lib/utils.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 4834b3a..f81759a 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -23,7 +23,6 @@ const hasPersonalData = entity => { element['@PersonalData.IsPotentiallyPersonal'] || element['@PersonalData.IsPotentiallySensitive'] || (element['@PersonalData.FieldSemantics'] && element['@PersonalData.FieldSemantics'] === 'DataSubjectID')) - // if (hasPersonalData && entity.name.startsWith('CollaborationsService.')) debugger entity.set($hasPersonalData, hasPersonalData) } } @@ -52,7 +51,6 @@ const _isDataSubject = (element, target) => { const getPick = event => { return (element, target) => { - // if (target.name === 'CollaborationsService.Collaborations') debugger if (!hasPersonalData(target)) return const categories = [] if (!element.isAssociation && element.key) categories.push('ObjectID') From 5dc37ead91ea481d809e0f90f6b44192ab96f00c Mon Sep 17 00:00:00 2001 From: sjvans <30337871+sjvans@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:22:29 +0100 Subject: [PATCH 5/5] WeakMap is not necessary --- lib/modification.js | 4 ++-- lib/utils.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/modification.js b/lib/modification.js index 8cf78f9..7148c87 100644 --- a/lib/modification.js +++ b/lib/modification.js @@ -73,7 +73,7 @@ const _getDataWithAppliedTransitions = (data, req) => { const addDiffToCtx = async function (req) { // store diff in audit data structure at context const _audit = (req.context._audit ??= {}) - if (!_audit.diffs) _audit.diffs = new WeakMap() + if (!_audit.diffs) _audit.diffs = new Map() // get diff let diff = (await req.diff()) || {} @@ -184,7 +184,7 @@ const _calcModificationLogsHandler = async function (req, beforeWrite, that) { const modificationLogs = _getDataModificationLogs(req, that, _audit.diffs.get(mapKey), beforeWrite) // store modificationLogs in audit data structure at context - if (!_audit.modificationLogs) _audit.modificationLogs = new WeakMap() + if (!_audit.modificationLogs) _audit.modificationLogs = new Map() const existingLogs = _audit.modificationLogs.get(mapKey) || {} _audit.modificationLogs.set(mapKey, Object.assign(existingLogs, modificationLogs)) diff --git a/lib/utils.js b/lib/utils.js index f81759a..8421043 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -298,7 +298,7 @@ const _getDataSubject = AL_OLD_SUBJECT_SEARCH ? _getDataSubjectGreedy : _getData const _getDataSubjectsMap = req => { const mapKey = getMapKeyForCurrentRequest(req) const _audit = (req.context._audit ??= {}) - if (!_audit.dataSubjects) _audit.dataSubjects = new WeakMap() + if (!_audit.dataSubjects) _audit.dataSubjects = new Map() if (!_audit.dataSubjects.has(mapKey)) _audit.dataSubjects.set(mapKey, new Map()) return _audit.dataSubjects.get(mapKey) }