Skip to content

Commit

Permalink
fix: adapt to template processor API change (#110)
Browse files Browse the repository at this point in the history
Co-authored-by: D050513 <[email protected]>
  • Loading branch information
arleytm and sjvans authored Sep 12, 2024
1 parent f4c91b4 commit 523b7c0
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 183 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ jobs:
fail-fast: false
matrix:
node-version: [22.x, 20.x]
cds-version: [8, 7]
steps:
- uses: actions/checkout@v2
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node-version }}
- run: npm i -g @sap/cds-dk
- run: npm i -g @sap/cds-dk@${{ matrix.cds-version }}
- run: npm i
- run: npm i @sap/cds@${{ matrix.cds-version }}
- run: cds v
- run: npm run lint
- run: npm run test
Expand Down
13 changes: 10 additions & 3 deletions lib/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const cds = require('@sap/cds')

// REVISIT: don't require internal stuff
const getTemplate = require('@sap/cds/libx/_runtime/common/utils/template')
const templateProcessor = require('@sap/cds/libx/_runtime/common/utils/templateProcessor')

const {
getRootEntity,
Expand Down Expand Up @@ -56,8 +55,16 @@ const auditAccess = async function (data, req) {
if (!template.elements.size) return

const accessLogs = {}
const _data = Array.isArray(data) ? data : [data]
_data.forEach(row => templateProcessor({ processFn: _processorFnAccess(accessLogs, this.model, req), row, template }))
const processFn = _processorFnAccess(accessLogs, this.model, req)
// cds internal templating mechanism api changed in 8.2.0 -> polyfill
if (!template.process) {
module.exports._templateProcessor ??= require('@sap/cds/libx/_runtime/common/utils/templateProcessor')
template.process = (data, processFn) => {
const _data = Array.isArray(data) ? data : [data]
_data.forEach(row => module.exports._templateProcessor({ processFn, row, template }))
}
}
template.process(data, processFn)

for (const each of Object.keys(accessLogs)) if (!accessLogs[each].attributes.length) delete accessLogs[each]
if (!Object.keys(accessLogs).length) return
Expand Down
10 changes: 8 additions & 2 deletions lib/modification.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const cds = require('@sap/cds')

// REVISIT: don't require internal stuff
const getTemplate = require('@sap/cds/libx/_runtime/common/utils/template')
const templateProcessor = require('@sap/cds/libx/_runtime/common/utils/templateProcessor')

const {
getMapKeyForCurrentRequest,
Expand Down Expand Up @@ -166,7 +165,14 @@ const _getDataModificationLogs = (req, tx, diff, beforeWrite) => {

const modificationLogs = {}
const processFn = _processorFnModification(modificationLogs, tx.model, req, beforeWrite)
templateProcessor({ processFn, row: diff, template })
// cds internal templating mechanism api changed in 8.2.0 -> polyfill
if (!template.process) {
module.exports._templateProcessor ??= require('@sap/cds/libx/_runtime/common/utils/templateProcessor')
template.process = (data, processFn) => {
module.exports._templateProcessor({ processFn, row: data, template })
}
}
template.process(diff, processFn)

return modificationLogs
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"test-old-db": "npx jest --silent"
},
"peerDependencies": {
"@sap/cds": ">=7 || ^8.0.0-beta"
"@sap/cds": ">=7"
},
"devDependencies": {
"@cap-js/audit-logging": "file:.",
Expand Down
218 changes: 42 additions & 176 deletions test/personal-data/crud.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,8 @@ describe('personal data audit logging in CRUD', () => {
{ auth: ALICE }
)
expect(response).toMatchObject({ status: 200 })
expect(_logs.length).toBe(3)
// NOTE: cds^8 only returns root on update requests -> no data access logs for children
expect(_logs.length).toBeGreaterThanOrEqual(1)
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
Expand All @@ -397,40 +398,6 @@ describe('personal data audit logging in CRUD', () => {
{ name: 'town', new: 'updated town', old: 'shu' }
]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_2.CustomerPostalAddress',
id: { ID: '1ab71292-ef69-4571-8cfb-10b9d5d1459e' }
},
data_subject: {
type: 'CRUD_2.CustomerPostalAddress',
role: 'Address',
id: {
ID: '1ab71292-ef69-4571-8cfb-10b9d5d1459e',
street: 'updated',
town: 'updated town'
}
},
attributes: [{ name: 'someOtherField' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_2.CustomerPostalAddress',
id: { ID: '285225db-6eeb-4b4f-9439-dbe5fcb4ce82' }
},
data_subject: {
type: 'CRUD_2.CustomerPostalAddress',
role: 'Address',
id: {
ID: '285225db-6eeb-4b4f-9439-dbe5fcb4ce82',
street: 'sue',
town: 'lou'
}
},
attributes: [{ name: 'someOtherField' }]
})
})

test('create Customer - flat', async () => {
Expand Down Expand Up @@ -867,9 +834,19 @@ describe('personal data audit logging in CRUD', () => {
}

response = await PATCH(`/crud-1/Customers(${CUSTOMER_ID})`, customer, { auth: ALICE })

expect(response).toMatchObject({ status: 200 })
expect(_logs.length).toBe(12)

// NOTE: cds^8 only returns root on update requests -> no data access logs for children
expect(_logs.length).toBeGreaterThanOrEqual(9)

// augment response with data (specifically keys in children) not returned in cds^8
if (!response.data.addresses) {
const {
data: { addresses, status }
} = await GET(`/crud-1/Customers(${CUSTOMER_ID})?$select=ID&$expand=addresses,status`, { auth: ALICE })
response.data.addresses = addresses
response.data.status = status
}

const newAddresses = response.data.addresses
const newStatus = response.data.status
Expand Down Expand Up @@ -943,33 +920,6 @@ describe('personal data audit logging in CRUD', () => {
data_subject: DATA_SUBJECT,
attributes: [{ name: 'creditCardNo' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.CustomerPostalAddress',
id: { ID: newAddresses[0].ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'street' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.CustomerPostalAddress',
id: { ID: newAddresses[1].ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'street' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.CustomerStatus',
id: { ID: newStatus.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'description' }]
})
})

test('update Customer - deep with reusing notes', async () => {
Expand Down Expand Up @@ -1028,15 +978,26 @@ describe('personal data audit logging in CRUD', () => {
_logs = []

response = await PATCH(`/crud-1/Customers(${CUSTOMER_ID})`, customer, { auth: ALICE })

expect(response).toMatchObject({ status: 200 })
expect(_logs.length).toBe(16)

// NOTE: cds^8 only returns root on update requests -> no data access logs for children
expect(_logs.length).toBeGreaterThanOrEqual(10)

// augment response with data (specifically keys in children) not returned in cds^8
if (!response.data.addresses) {
const {
data: { addresses, status }
} = await GET(
`/crud-1/Customers(${CUSTOMER_ID})?$select=ID&$expand=addresses($expand=attachments($expand=notes)),status`,
{ auth: ALICE }
)
response.data.addresses = addresses
response.data.status = status
}

const newAddresses = response.data.addresses
const newStatus = response.data.status
const newAttachments = response.data.addresses[0].attachments
const newAttachmentNote = response.data.addresses[0].attachments[0].notes[0]
const newStatusNote = response.data.status.notes[0]

expect(_logs).toContainMatchObject({
user: 'alice',
Expand Down Expand Up @@ -1131,15 +1092,6 @@ describe('personal data audit logging in CRUD', () => {
{ name: 'todo', old: oldStatus.todo, new: newStatus.todo }
]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.AddressAttachment',
id: { ID: newAttachments[0].ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'description' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
Expand All @@ -1149,51 +1101,6 @@ describe('personal data audit logging in CRUD', () => {
data_subject: DATA_SUBJECT,
attributes: [{ name: 'creditCardNo' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.CustomerPostalAddress',
id: { ID: newAddresses[0].ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'street' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.CustomerPostalAddress',
id: { ID: newAddresses[1].ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'street' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.CustomerStatus',
id: { ID: newStatus.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'description' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.Notes',
id: { ID: newStatusNote.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'note' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.Notes',
id: { ID: newAttachmentNote.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'note' }]
})
})

test('delete Customer - flat', async () => {
Expand Down Expand Up @@ -1745,8 +1652,7 @@ describe('personal data audit logging in CRUD', () => {
],
misc: 'abc'
}
const r1 = await POST(`/crud-1/Orders`, order, { auth: ALICE })
expect(r1)
await POST(`/crud-1/Orders`, order, { auth: ALICE })
const {
data: {
header_ID,
Expand All @@ -1772,9 +1678,12 @@ describe('personal data audit logging in CRUD', () => {
},
items
}

_logs = []

await PATCH(`/crud-1/Orders(${order.ID})`, updatedOrder, { auth: ALICE })
expect(_logs.length).toBe(6)
// NOTE: cds^8 only returns root on update requests -> no data access logs for children
expect(_logs.length).toBeGreaterThanOrEqual(4)
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
Expand Down Expand Up @@ -1811,62 +1720,19 @@ describe('personal data audit logging in CRUD', () => {
data_subject: DATA_SUBJECT,
attributes: [{ name: 'misc' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.OrderHeader',
id: { ID: header_ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'description' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.OrderHeader.sensitiveData',
id: { ID: sensitiveData.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'note' }]
})
const r2 = await DELETE(`/crud-1/Orders(${order.ID})`, { auth: ALICE })
expect(r2).toMatchObject({ status: 204 })
expect(_logs.length).toBe(9)
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.Orders',
id: { ID: order.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'misc', old: '***', new: '***' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.OrderHeader',
id: { ID: header_ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'description', old: '***', new: '***' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.OrderHeader.sensitiveData',
id: { ID: sensitiveData.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'note', old: '***', new: '***' }]
})

_logs = []

await DELETE(`/crud-1/Orders(${order.ID})`, { auth: ALICE })
expect(_logs.length).toBe(3)
expect(_logs).toContainMatchObject({
user: 'alice',
object: {
type: 'CRUD_1.Orders',
id: { ID: order.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'misc', old: '***', new: '***' }]
attributes: [{ name: 'misc', old: '***' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
Expand All @@ -1875,7 +1741,7 @@ describe('personal data audit logging in CRUD', () => {
id: { ID: header_ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'description', old: '***', new: '***' }]
attributes: [{ name: 'description', old: '***' }]
})
expect(_logs).toContainMatchObject({
user: 'alice',
Expand All @@ -1884,7 +1750,7 @@ describe('personal data audit logging in CRUD', () => {
id: { ID: sensitiveData.ID }
},
data_subject: DATA_SUBJECT,
attributes: [{ name: 'note', old: '***', new: '***' }]
attributes: [{ name: 'note', old: '***' }]
})
})

Expand Down

0 comments on commit 523b7c0

Please sign in to comment.