Skip to content

Commit e1aba41

Browse files
schiwekMKoblerS
andauthored
Leverage generic handlers to allow for extension and feature toggles (#292)
Register handlers in a generic fashion so the plugin can be put behind a feature toggle or extension. The scenario is that `cds.model` only has the base model but that might not contain attachments. This currently leads to attachment handlers not being registered and then once a feature toggle is activated the handlers are missing to handle the added attachment. Using generic handlers fixes this. --------- Co-authored-by: Simon Kobler <32038731+KoblerS@users.noreply.github.com>
1 parent 4440559 commit e1aba41

File tree

11 files changed

+350
-392
lines changed

11 files changed

+350
-392
lines changed

.github/actions/integration-tests/action.yml

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,19 @@ runs:
8989
# Bind against BTP services
9090
- run: cds bind db -2 cap-js-attachments-hana-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA -o package.json
9191
shell: bash
92-
- run: cds bind objectStore -2 cap-js-attachments-object-store-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA -o package.json
92+
93+
- name: Bind object store
9394
shell: bash
95+
run: |
96+
for i in {1..3}; do
97+
cds bind objectStore -2 cap-js-attachments-object-store-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA -o package.json && break
98+
echo "cds bind objectStore failed, retrying ($i/3)..."
99+
sleep 100
100+
if [ "$i" -eq 3 ]; then
101+
echo "❌ cds bind objectStore failed after 3 attempts."
102+
exit 1
103+
fi
104+
done
94105
- run: cds bind malware-scanner -2 cap-js-attachments-scanner-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA -o package.json
95106
shell: bash
96107

lib/aws-s3.js

Lines changed: 1 addition & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind
99
const separateObjectStore = isMultitenacyEnabled && objectStoreKind === "separate"
1010

1111
const s3ClientsCache = {}
12-
module.exports = class AWSAttachmentsService extends require("./basic") {
12+
module.exports = class AWSAttachmentsService extends require("./object-store.js") {
1313
/**
1414
* Initializes the AWS S3 Attachments Service
1515
*/
@@ -80,20 +80,6 @@ module.exports = class AWSAttachmentsService extends require("./basic") {
8080
} else {
8181
logConfig.info('Separate object store mode enabled - clients will be created per tenant')
8282
}
83-
84-
this.on('DeleteAttachment', async msg => {
85-
await this.delete(msg.data.url)
86-
})
87-
88-
this.on('DeleteInfectedAttachment', async msg => {
89-
const { target, hash, keys } = msg.data
90-
const attachment = await SELECT.one.from(target).where(Object.assign({ hash }, keys)).columns('url')
91-
if (attachment) { //Might happen that a draft object is the target
92-
await this.delete(attachment.url)
93-
} else {
94-
logConfig.warn(`Cannot delete malware file with the hash ${hash} for attachment ${target}, keys: ${keys}`)
95-
}
96-
})
9783
}
9884

9985
/**
@@ -410,50 +396,6 @@ module.exports = class AWSAttachmentsService extends require("./basic") {
410396
}
411397
}
412398

413-
/**
414-
* @inheritdoc
415-
*/
416-
registerUpdateHandlers(srv, mediaElements) {
417-
for (const mediaElement of mediaElements) {
418-
srv.prepend(() => {
419-
srv.on(
420-
"PUT",
421-
mediaElement,
422-
this.updateContentHandler.bind(this)
423-
)
424-
})
425-
}
426-
}
427-
428-
/**
429-
* @inheritdoc
430-
*/
431-
registerDraftUpdateHandlers(srv, entity, mediaElements) {
432-
for (const mediaElement of mediaElements) {
433-
srv.prepend(() => {
434-
if (mediaElement.drafts) {
435-
srv.on(
436-
"PUT",
437-
mediaElement.drafts,
438-
this.updateContentHandler.bind(this)
439-
)
440-
441-
// case: attachments uploaded in draft and deleted before saving
442-
srv.before(
443-
"DELETE",
444-
mediaElement.drafts,
445-
this.attachDraftDeletionData.bind(this)
446-
)
447-
srv.after(
448-
"DELETE",
449-
mediaElement.drafts,
450-
this.deleteAttachmentsWithKeys.bind(this)
451-
)
452-
}
453-
})
454-
}
455-
}
456-
457399
/**
458400
* Deletes a file from S3 based on the provided key
459401
* @param {string} Key - The key of the file to delete

lib/azure-blob-storage.js

Lines changed: 1 addition & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ const objectStoreKind = cds.env.requires?.attachments?.objectStore?.kind
99
const separateObjectStore = isMultitenacyEnabled && objectStoreKind === "separate"
1010

1111
const azureClientsCache = {}
12-
module.exports = class AzureAttachmentsService extends require("./basic") {
12+
module.exports = class AzureAttachmentsService extends require("./object-store") {
1313
/**
1414
* Initializes the Azure Blob Storage Attachments Service
1515
*/
@@ -70,20 +70,6 @@ module.exports = class AzureAttachmentsService extends require("./basic") {
7070
} else {
7171
logConfig.info('Separate object store mode enabled - clients will be created per tenant')
7272
}
73-
74-
this.on('DeleteAttachment', async msg => {
75-
await this.delete(msg.url)
76-
})
77-
78-
this.on('DeleteInfectedAttachment', async msg => {
79-
const { target, hash, keys } = msg.data
80-
const attachment = await SELECT.one.from(target).where(Object.assign({ hash }, keys)).columns('url')
81-
if (attachment) { //Might happen that a draft object is the target
82-
await this.delete(attachment.url)
83-
} else {
84-
logConfig.warn(`Cannot delete malware file with the hash ${hash} for attachment ${target}, keys: ${keys}`)
85-
}
86-
})
8773
}
8874

8975
/**
@@ -393,50 +379,6 @@ module.exports = class AzureAttachmentsService extends require("./basic") {
393379
}
394380
}
395381

396-
/**
397-
* @inheritdoc
398-
*/
399-
registerUpdateHandlers(srv, mediaElements) {
400-
for (const mediaElement of mediaElements) {
401-
srv.prepend(() => {
402-
srv.on(
403-
"PUT",
404-
mediaElement,
405-
this.updateContentHandler.bind(this)
406-
)
407-
})
408-
}
409-
}
410-
411-
/**
412-
* @inheritdoc
413-
*/
414-
registerDraftUpdateHandlers(srv, entity, mediaElements) {
415-
for (const mediaElement of mediaElements) {
416-
srv.prepend(() => {
417-
if (mediaElement.drafts) {
418-
srv.on(
419-
"PUT",
420-
mediaElement.drafts,
421-
this.updateContentHandler.bind(this)
422-
)
423-
424-
// case: attachments uploaded in draft and deleted before saving
425-
srv.before(
426-
"DELETE",
427-
mediaElement.drafts,
428-
this.attachDraftDeletionData.bind(this)
429-
)
430-
srv.after(
431-
"DELETE",
432-
mediaElement.drafts,
433-
this.deleteAttachmentsWithKeys.bind(this)
434-
)
435-
}
436-
})
437-
}
438-
}
439-
440382
/**
441383
* Deletes a file from Azure Blob Storage
442384
* @param {string} Key - The key of the file to delete

lib/basic.js

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,22 @@ const { computeHash } = require('./helper')
66
class AttachmentsService extends cds.Service {
77

88
init() {
9+
this.on('DeleteAttachment', async msg => {
10+
await this.delete(msg.url)
11+
})
12+
913
this.on('DeleteInfectedAttachment', async msg => {
1014
const { target, hash, keys } = msg.data
11-
await UPDATE(target).where(Object.assign({ hash }, keys)).with({ content: null })
15+
const attachment = await SELECT.one.from(target).where(Object.assign({ hash }, keys)).columns('url')
16+
if (attachment) { //Might happen that a draft object is the target
17+
await this.delete(attachment.url)
18+
} else {
19+
logConfig.warn(`Cannot delete malware file with the hash ${hash} for attachment ${target}, keys: ${keys}`)
20+
}
1221
})
22+
return super.init()
1323
}
24+
1425
/**
1526
* Uploads attachments to the database and initiates malware scans for database-stored files
1627
* @param {cds.Entity} attachments - Attachments entity definition
@@ -64,19 +75,17 @@ class AttachmentsService extends cds.Service {
6475
}
6576

6677
// Initiate malware scanning for database-stored files
67-
if (this.kind === 'db') {
68-
logConfig.debug('Initiating malware scans for database-stored files', {
69-
fileCount: data.length,
70-
fileIds: data.map(d => d.ID)
71-
})
78+
logConfig.debug('Initiating malware scans for database-stored files', {
79+
fileCount: data.length,
80+
fileIds: data.map(d => d.ID)
81+
})
7282

73-
const MalwareScanner = await cds.connect.to('malwareScanner')
74-
await Promise.all(
75-
data.map(async (d) => {
76-
await MalwareScanner.emit('ScanFile', { target: attachments.name, keys: { ID: d.ID } })
77-
})
78-
)
79-
}
83+
const MalwareScanner = await cds.connect.to('malwareScanner')
84+
await Promise.all(
85+
data.map(async (d) => {
86+
await MalwareScanner.emit('ScanFile', { target: attachments.name, keys: { ID: d.ID } })
87+
})
88+
)
8089

8190
return res
8291
}
@@ -162,28 +171,31 @@ class AttachmentsService extends cds.Service {
162171
/**
163172
* Registers handlers for attachment entities in the service
164173
* @param {cds.Service} srv - The CDS service instance
165-
* @param {cds.Entity} entity - The entity containing attachment associations
166-
* @param {cds.Entity} target - Attachments entity definition to register handlers for
167174
*/
168-
registerUpdateHandlers(srv, targets) {
169-
for (const target of targets) {
170-
srv.after("PUT", target, async (req) => {
171-
await this.nonDraftHandler(req, target, srv)
172-
})
173-
}
175+
registerUpdateHandlers(srv) {
176+
srv.after("PUT", async (res, req) => {
177+
if (!req.target._attachments.isAttachmentsEntity) return;
178+
await this.nonDraftHandler(res, req.target, srv)
179+
})
174180
}
175181

176182
/**
177183
* Registers draft save handler for attachment entities in the service
178184
* @param {cds.Service} srv - The CDS service instance
179-
* @param {cds.Entity} entity - The entity containing attachment associations
180-
* @param {cds.Entity} target - Attachments entity definition to register handlers for
181185
*/
182-
registerDraftUpdateHandlers(srv, entity, targets) {
183-
for (const target of targets) {
184-
srv.after("SAVE", entity, this.draftSaveHandler(target))
185-
}
186-
return
186+
registerDraftUpdateHandlers(srv) {
187+
srv.after("SAVE", async function saveDraftAttachments(res, req) {
188+
if (
189+
req.target.isDraft ||
190+
!req.target._attachments.hasAttachmentsComposition ||
191+
!req.target._attachments.attachmentCompositions
192+
) return;
193+
await Promise.all(
194+
Object.keys(req.target._attachments.attachmentCompositions).map(attachmentsEle =>
195+
this.draftSaveHandler(req.target.elements[attachmentsEle]._target)(res, req)
196+
)
197+
)
198+
}.bind(this))
187199
}
188200

189201
/**
@@ -340,6 +352,15 @@ class AttachmentsService extends cds.Service {
340352
req.attachmentsToDelete = attachmentsToDelete
341353
}
342354
}
355+
356+
/**
357+
* Deletes a file from the database. Does not delete metadata
358+
* @param {string} url - The url of the file to delete
359+
* @returns {Promise} - Promise resolving when deletion is complete
360+
*/
361+
async delete(url, target) {
362+
return await UPDATE(target).where({url}).with({ content: null })
363+
}
343364
}
344365

345366

lib/csn-runtime-extension.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
const cds = require('@sap/cds')
2+
const { LinkedDefinitions } = require("@sap/cds/lib/core/linked-csn")
3+
4+
Object.defineProperty(cds.builtin.classes.entity.prototype, '_attachments', {
5+
get() {
6+
const entity = this;
7+
return {
8+
get hasAttachmentsComposition() {
9+
return entity.compositions && Object.keys(entity.compositions).some(ele => entity.compositions[ele]._target?.["@_is_media_data"] || entity.compositions[ele]._target?._attachments.hasAttachmentsComposition)
10+
},
11+
get attachmentCompositions() {
12+
const resultSet = new LinkedDefinitions()
13+
if (!entity.compositions) return resultSet
14+
for (const ele of Object.keys(entity.compositions).filter(ele => entity.compositions[ele]._target?.["@_is_media_data"] || entity.compositions[ele]._target?._attachments.hasAttachmentsComposition)) {
15+
resultSet[ele] = entity.compositions[ele]
16+
};
17+
return resultSet;
18+
},
19+
get isAttachmentsEntity() {
20+
return !!entity?.["@_is_media_data"]
21+
}
22+
}
23+
},
24+
})

0 commit comments

Comments
 (0)