fix: mod logs for deep with renamings (#71)

closes #70 \+ tests run with old and new db
cap-js · Dec 7, 2023 · 51bbf86 · 51bbf86
1 parent 7315fd5
commit 51bbf86
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 78 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
+## Version 0.5.2 - 2023-12-07
+
+### Fixed
+
+- Automatic personal data modification logging for deep data structures with renamings
+
 ## Version 0.5.1 - 2023-11-30
 
 ### Fixed

diff --git a/jest.config.js b/jest.config.js
@@ -1,6 +1,3 @@
-// FIXME: should not be necessary
-process.env.CDS_ENV = 'better-sqlite'
-
 const config = {
   testTimeout: 42222,
   testMatch: ['**/*.test.js'],

diff --git a/lib/modification.js b/lib/modification.js
@@ -17,27 +17,39 @@ const {
 
 let audit
 
-const _applyTransitionRecursively = (transition, data, d) => {
+const _applyTransitionRecursively = (transition, data, d, old = {}) => {
   for (const [k, v] of transition.mapping) {
     if (v.transition) {
-      if (Array.isArray(data[k])) {
+      const ok = v.ref?.[0] || k
+      if (!data[ok]) continue
+      if (Array.isArray(data[ok])) {
         d[k] = []
-        for (let i = 0; i < data[k].length; i++) {
-          d[k].push({ _op: data[k][i]._op })
-          if (data[k][i]._old) d[k][i]._old = {}
-          _applyTransitionRecursively(v.transition, data[k][i], d[k][i])
+        for (let i = 0; i < data[ok].length; i++) {
+          const _op = data[ok][i]._op || d._op
+          d[k].push(_op ? { _op } : {})
+          const _old = old[ok]?.[i] || data[ok][i]._old
+          if (_old) d[k][i]._old = _old
+          _applyTransitionRecursively(v.transition, data[ok][i], d[k][i], _old)
+        }
+        if (old[ok] && data[ok].length !== old[ok].length) {
+          for (const each of Object.values(old[ok]).filter(ele => !ele.__visited)) {
+            const i = d[k].push({ _op: 'delete' })
+            d[k][i - 1]._old = each
+            _applyTransitionRecursively(v.transition, each, d[k][i - 1], each)
+          }
         }
       } else {
-        d[k] = { _op: data[k]._op }
-        if (data[k]._old) d[k]._old = {}
-        _applyTransitionRecursively(v.transition, data[k], d[k])
+        d[k] = { _op: data[ok]._op || d._op }
+        const _old = old[ok] || data[ok]._old
+        if (_old) d[k]._old = _old
+        _applyTransitionRecursively(v.transition, data[ok], d[k], _old)
       }
-      continue
     } else if (v.ref) {
       if (v.ref[0] in data) d[k] = data[v.ref[0]]
-      if (data._old && v.ref[0] in data._old) d._old[k] = data._old[v.ref[0]]
+      if (v.ref[0] in old) d._old[k] = old[v.ref[0]]
     }
   }
+  if (Object.keys(old).length) old.__visited = true
 }
 
 // REVISIT: remove once old database impl is removed
@@ -47,9 +59,9 @@ const _getDataWithAppliedTransitions = (data, req) => {
   // NOTE: there will only be transitions if old database impl is used
   const transition = query._transitions?.find(t => t.queryTarget.name === req.target.name)
   if (transition) {
-    d = { _op: data._op }
+    d = data._op ? { _op: data._op } : {}
     if (data._old) d._old = {}
-    _applyTransitionRecursively(transition, data, d)
+    _applyTransitionRecursively(transition, data, d, data._old)
   }
   return d || data
 }

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@cap-js/audit-logging",
-  "version": "0.5.1",
+  "version": "0.5.2",
   "description": "CDS plugin providing integration to the SAP Audit Log service as well as out-of-the-box personal data-related audit logging based on annotations.",
   "repository": "cap-js/audit-logging",
   "author": "SAP SE (https://www.sap.com)",
@@ -14,7 +14,9 @@
   ],
   "scripts": {
     "lint": "npx eslint .",
-    "test": "npx jest --silent"
+    "test": "npm run test-new-db && npm run test-old-db",
+    "test-new-db": "CDS_ENV='better-sqlite' npx jest --silent",
+    "test-old-db": "npx jest --silent"
   },
   "peerDependencies": {
     "@sap/cds": "^7"
@@ -25,7 +27,8 @@
     "axios": "^1",
     "eslint": "^8",
     "express": "^4",
-    "jest": "^29"
+    "jest": "^29",
+    "sqlite3": "^5.1.6"
   },
   "cds": {
     "requires": {

diff --git a/test/personal-data/crud.test.js b/test/personal-data/crud.test.js
@@ -2046,5 +2046,63 @@ describe('personal data audit logging in CRUD', () => {
         ]
       })
     })
+
+    test('deep', async () => {
+      const c1 = {
+        c_emailAddress: '[email protected]',
+        c_addresses: [
+          {
+            cpa_town: 'moo',
+            cpa_attachments: [
+              {
+                aa_todo: 'boo'
+              },
+              {
+                aa_todo: 'who'
+              }
+            ]
+          },
+          {
+            cpa_town: 'shu'
+          }
+        ]
+      }
+      const { data: r1 } = await POST('/crud-3/C', c1, { auth: ALICE })
+      Object.assign(c1, r1)
+      expect(_logs.length).toBe(5)
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'c_emailAddress', new: '[email protected]' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', new: 'moo' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', new: 'shu' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', new: 'boo' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', new: 'who' }] })
+
+      // reset logs
+      _logs = []
+
+      const c2 = {
+        c_emailAddress: '[email protected]',
+        c_addresses: [
+          {
+            cpa_id: c1.c_addresses[0].cpa_id,
+            cpa_town: 'voo',
+            cpa_attachments: [
+              {
+                aa_id: c1.c_addresses[0].cpa_attachments[0].aa_id,
+                aa_todo: 'doo'
+              }
+            ]
+          }
+        ]
+      }
+      await PATCH(`/crud-3/C/${c1.c_id}`, c2, { auth: ALICE })
+      expect(_logs.length).toBe(5)
+      expect(_logs).toContainMatchObject({
+        attributes: [{ name: 'c_emailAddress', old: '[email protected]', new: '[email protected]' }]
+      })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', old: 'moo', new: 'voo' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'cpa_town', old: 'shu' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', old: 'boo', new: 'doo' }] })
+      expect(_logs).toContainMatchObject({ attributes: [{ name: 'aa_todo', old: 'who' }] })
+    })
   })
 })
diff --git a/test/personal-data/srv/crud-service.cds b/test/personal-data/srv/crud-service.cds
@@ -16,33 +16,27 @@ service CRUD_1 {
   entity LastOne               as projection on db.LastOne;
   entity Notes                 as projection on db.Notes;
 
-  entity AddressAttachment     as projection on db.AddressAttachment {
-    *,
-    address.customer as customer
-  }
+  entity AddressAttachment     as
+    projection on db.AddressAttachment {
+      *,
+      address.customer as customer
+    }
 
-  annotate Orders with @PersonalData: {
-    EntitySemantics: 'Other'
-  } {
+  annotate Orders with @PersonalData: {EntitySemantics: 'Other'} {
     misc @PersonalData.IsPotentiallySensitive;
   }
 
-  annotate OrderHeader with @PersonalData: {
-    EntitySemantics: 'Other'
-  } {
+  annotate OrderHeader with @PersonalData: {EntitySemantics: 'Other'} {
     description @PersonalData.IsPotentiallySensitive;
   }
 
-  annotate OrderHeader.sensitiveData with @PersonalData: {
-    EntitySemantics: 'Other'
-  } {
+  annotate OrderHeader.sensitiveData with @PersonalData: {EntitySemantics: 'Other'} {
     note @PersonalData.IsPotentiallySensitive;
   }
 
-  annotate Pages with @PersonalData       : {
-    EntitySemantics: 'DataSubject'
-    // no DataSubjectRole for testing purposes
-  } {
+  annotate Pages with @PersonalData       : {EntitySemantics: 'DataSubject'
+                                                                           // no DataSubjectRole for testing purposes
+                                                              } {
     ID        @PersonalData.FieldSemantics: 'DataSubjectID';
     sensitive @PersonalData.IsPotentiallySensitive;
     personal  @PersonalData.IsPotentiallyPersonal;
@@ -59,45 +53,33 @@ service CRUD_1 {
     creditCardNo @PersonalData.IsPotentiallySensitive;
   }
 
-  annotate CustomerPostalAddress with @PersonalData: {
-    EntitySemantics: 'DataSubjectDetails'
-  } {
+  annotate CustomerPostalAddress with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} {
     customer @PersonalData.FieldSemantics          : 'DataSubjectID';
     street   @PersonalData.IsPotentiallySensitive;
     town     @PersonalData.IsPotentiallyPersonal;
   }
 
-  annotate CustomerStatus with @PersonalData: {
-    EntitySemantics: 'DataSubjectDetails'
-  } {
+  annotate CustomerStatus with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} {
     description @PersonalData.IsPotentiallySensitive;
     todo        @PersonalData.IsPotentiallyPersonal;
   }
 
-  annotate StatusChange with @PersonalData: {
-    EntitySemantics: 'DataSubjectDetails'
-  } {
+  annotate StatusChange with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} {
     description @PersonalData.IsPotentiallySensitive;
     secondKey   @PersonalData.IsPotentiallyPersonal;
   }
 
-  annotate LastOne with @PersonalData: {
-    EntitySemantics: 'DataSubjectDetails'
-  } {
+  annotate LastOne with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} {
     lastOneField @PersonalData.IsPotentiallySensitive;
   }
 
-  annotate AddressAttachment with @PersonalData: {
-    EntitySemantics: 'DataSubjectDetails'
-  } {
+  annotate AddressAttachment with @PersonalData: {EntitySemantics: 'DataSubjectDetails'} {
     customer    @PersonalData.FieldSemantics   : 'DataSubjectID';
     description @PersonalData.IsPotentiallySensitive;
     todo        @PersonalData.IsPotentiallyPersonal;
   }
 
-  annotate Notes with @PersonalData: {
-    EntitySemantics: 'Other'
-  } {
+  annotate Notes with @PersonalData: {EntitySemantics: 'Other'} {
     note       @PersonalData.IsPotentiallySensitive;
     dummyArray @PersonalData.IsPotentiallyPersonal;
   }
@@ -122,14 +104,13 @@ service CRUD_2 {
   entity CustomerPostalAddress as projection on db.CustomerPostalAddress;
   entity CustomerStatus        as projection on db.CustomerStatus;
 
-  entity AddressAttachment     as projection on db.AddressAttachment {
-    *,
-    address.customer as customer
-  }
+  entity AddressAttachment     as
+    projection on db.AddressAttachment {
+      *,
+      address.customer as customer
+    }
 
-  annotate Customers with @PersonalData   : {
-    EntitySemantics: 'Other'
-  } {
+  annotate Customers with @PersonalData   : {EntitySemantics: 'Other'} {
     addresses @PersonalData.FieldSemantics: 'DataSubjectID';
   }
 
@@ -144,38 +125,61 @@ service CRUD_2 {
   }
 
   // invalid modeling (nothing personal/ sensitive), must have no effect
-  annotate CustomerStatus with @PersonalData: {
-    EntitySemantics: 'DataSubjectDetails'
-  };
+  annotate CustomerStatus with @PersonalData: {EntitySemantics: 'DataSubjectDetails'};
 }
 
 @path    : '/crud-3'
 @requires: 'admin'
 service CRUD_3 {
 
-  entity R1                    as projection on db.RBase {
-    key ID           as r1_ID,
-        emailAddress as r1_emailAddress,
-        firstName    as r1_firstName,
-        lastName     as r1_lastName,
-        creditCardNo as r1_creditCardNo
-  }
+  entity R1                    as
+    projection on db.RBase {
+      key ID           as r1_ID,
+          emailAddress as r1_emailAddress,
+          firstName    as r1_firstName,
+          lastName     as r1_lastName,
+          creditCardNo as r1_creditCardNo
+    }
 
   annotate R1 with @PersonalData: {
     EntitySemantics: 'DataSubject',
     DataSubjectRole: 'Renamed Customer'
   };
 
-  entity R2                    as projection on R1 {
-    key r1_ID           as r2_ID,
-        r1_emailAddress as r2_emailAddress,
-        r1_firstName    as r2_firstName,
-        r1_lastName     as r2_lastName,
-        r1_creditCardNo as r2_creditCardNo
-  }
+  entity R2                    as
+    projection on R1 {
+      key r1_ID           as r2_ID,
+          r1_emailAddress as r2_emailAddress,
+          r1_firstName    as r2_firstName,
+          r1_lastName     as r2_lastName,
+          r1_creditCardNo as r2_creditCardNo
+    }
 
   annotate R2 with @PersonalData: {
     EntitySemantics: 'DataSubject',
     DataSubjectRole: 'Twice Renamed Customer'
   };
+
+  entity C                     as
+    projection on CRUD_1.Customers {
+      key ID           as c_id,
+          emailAddress as c_emailAddress,
+          addresses    as c_addresses
+    };
+
+
+  entity CPA                   as
+    projection on CRUD_1.CustomerPostalAddress {
+      key ID          as cpa_id,
+          town        as cpa_town,
+          customer    as cpa_customer,
+          attachments as cpa_attachments
+    };
+
+  entity AA                    as
+    projection on CRUD_1.AddressAttachment {
+      key ID      as aa_id,
+          todo    as aa_todo,
+          address as aa_address
+    };
 }