diff --git a/.github/actions/integration-tests/action.yml b/.github/actions/integration-tests/action.yml index c3f1e894..d85a5718 100644 --- a/.github/actions/integration-tests/action.yml +++ b/.github/actions/integration-tests/action.yml @@ -72,6 +72,7 @@ runs: - name: CDS Versions being used run: cds v -i shell: bash + # Deploy model to HANA - name: Create Object store shell: bash @@ -84,6 +85,7 @@ runs: run: cf create-service hana hdi-shared cap-js-attachments-hana-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA - run: cd tests/incidents-app/ && cds deploy --to hana:cap-js-attachments-hana-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA shell: bash + # Create service key - run: cf create-service-key cap-js-attachments-scanner-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA cap-js-attachments-scanner-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA-key if: ${{ inputs.SCANNER_AUTH == 'basic' }} @@ -91,10 +93,10 @@ runs: - run: cf create-service-key cap-js-attachments-scanner-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA cap-js-attachments-scanner-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA-key -c '{"auth":"mtls"}' if: ${{ inputs.SCANNER_AUTH == 'mtls' }} shell: bash + # Bind against BTP services - 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 shell: bash - - name: Bind object store shell: bash run: | @@ -114,7 +116,7 @@ runs: - run: cds bind --exec npm run test shell: bash - # Cleanup + # Cleanup BTP services - name: Delete Malware Scanner instance if: ${{ always() }} run: cf delete-service cap-js-attachments-scanner-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA -f @@ -127,7 +129,8 @@ runs: if: ${{ always() }} run: cf delete-service-key cap-js-attachments-hana-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA cap-js-attachments-hana-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }}-${{ inputs.SCANNER_AUTH }}-$NODE_VERSION_HANA-key -f shell: bash - # Somehow first delete always fails with a "ongoing operation on service binding" error + + # Note: The initial delete attempt often fails due to an "ongoing operation on service binding" error. - name: Delete HDI Container if: ${{ always() }} shell: bash diff --git a/_i18n/messages.properties b/_i18n/messages.properties new file mode 100644 index 00000000..f7f22e00 --- /dev/null +++ b/_i18n/messages.properties @@ -0,0 +1,4 @@ +UnableToDownloadAttachmentScanStatusNotClean=Unable to download the attachment as scan status is not clean. +InvalidContentSize=Invalid Content Size. +AttachmentSizeExceeded=File Size limit exceeded beyond 400 MB. +MultiUpdateNotSupported=Multi update is not supported. diff --git a/lib/generic-handlers.js b/lib/generic-handlers.js index e0c10689..64ff194e 100644 --- a/lib/generic-handlers.js +++ b/lib/generic-handlers.js @@ -48,7 +48,7 @@ async function validateAttachment(req) { } const scanEnabled = cds.env.requires?.attachments?.scan ?? true if (scanEnabled && status !== 'Clean') { - req.reject(403, 'Unable to download the attachment as scan status is not clean.') + req.reject(403, 'UnableToDownloadAttachmentScanStatusNotClean') } } } @@ -78,10 +78,10 @@ function validateAttachmentSize(req) { fileSizeInBytes = Number(contentLengthHeader) const MAX_FILE_SIZE = 419430400 //400 MB in bytes if (fileSizeInBytes > MAX_FILE_SIZE) { - return req.reject(400, "File Size limit exceeded beyond 400 MB.") + return req.reject(400, "AttachmentSizeExceeded") } } else { - return req.reject(400, "Invalid Content Size") + return req.reject(400, "InvalidContentSize") } } diff --git a/srv/object-store.js b/srv/object-store.js index afb458b9..b7763283 100644 --- a/srv/object-store.js +++ b/srv/object-store.js @@ -35,11 +35,11 @@ module.exports = class RemoteAttachmentsService extends require("./basic") { let metadata = await srv.run(SELECT.from(req.subject).columns('url', ...Object.keys(req.target.keys))) if (metadata.length > 1) { - return req.error(501, 'MULTI_UPDATE_NOT_SUPPORTED') + return req.error(501, 'MultiUpdateNotSupported') } metadata = metadata[0] if (!metadata) { - return req.error(404) + return req.reject(404) } req.data.ID = metadata.ID req.data.url ??= metadata.url diff --git a/tests/integration/attachments-non-draft.test.js b/tests/integration/attachments-non-draft.test.js index 91716313..27281803 100644 --- a/tests/integration/attachments-non-draft.test.js +++ b/tests/integration/attachments-non-draft.test.js @@ -33,13 +33,13 @@ describe("Tests for uploading/deleting and fetching attachments through API call it("unknown extension throws warning", async () => { const response = await axios.post( - `/odata/v4/admin/Incidents(${incidentID})/attachments`, - { filename: 'sample.madeupextension' }, - { headers: { "Content-Type": "application/json" } } + `/odata/v4/admin/Incidents(${incidentID})/attachments`, + { filename: 'sample.madeupextension' }, + { headers: { "Content-Type": "application/json" } } ) expect(response.status).to.equal(201); - expect (log.output.length).to.be.greaterThan(0) - expect (log.output).to.contain('is uploaded whose extension "madeupextension" is not known! Falling back to "application/octet-stream"') + expect(log.output.length).to.be.greaterThan(0) + expect(log.output).to.contain('is uploaded whose extension "madeupextension" is not known! Falling back to "application/octet-stream"') }) it("should list attachments for incident", async () => { @@ -104,20 +104,16 @@ describe("Tests for uploading/deleting and fetching attachments through API call expect(deleteResponse.status).to.equal(204) // Verify the attachment is deleted - try { - await axios.get( - `/odata/v4/admin/Incidents(ID=${incidentID})/attachments(up__ID=${incidentID},ID=${attachmentID})` - ) - // Should not reach here - expect.fail("Expected 404 error") - } catch (err) { - expect(err.response.status).to.equal(404) - } + await axios.get( + `/odata/v4/admin/Incidents(ID=${incidentID})/attachments(up__ID=${incidentID},ID=${attachmentID})` + ).catch(e => { + expect(e.status).to.equal(404) + }) }) it("Updating attachments via srv.run works", async () => { const AdminSrv = await cds.connect.to('AdminService') - + const attachmentsID = cds.utils.uuid(); const doc = await axios.post( `odata/v4/admin/Incidents(ID=${incidentID})/attachments`, @@ -134,7 +130,7 @@ describe("Tests for uploading/deleting and fetching attachments through API call ) const user = new cds.User({ id: 'alice', roles: { admin: 1 } }) const req = new cds.Request({ - query: UPDATE.entity({ref: [{id: 'AdminService.Incidents', where: [{ref: ['ID']}, '=', {val: incidentID}]}, {id: 'attachments', where: [{ref: ['ID']}, '=', {val: doc.data.ID}]}]}).set({ + query: UPDATE.entity({ ref: [{ id: 'AdminService.Incidents', where: [{ ref: ['ID'] }, '=', { val: incidentID }] }, { id: 'attachments', where: [{ ref: ['ID'] }, '=', { val: doc.data.ID }] }] }).set({ filename: "sample.pdf", content: fileContent, mimeType: "application/pdf", @@ -142,9 +138,9 @@ describe("Tests for uploading/deleting and fetching attachments through API call Date.now() - Math.random() * 30 * 24 * 60 * 60 * 1000 ), createdBy: "alice", - }), user: user, headers: {"content-length": 55782} + }), user: user, headers: { "content-length": 55782 } }) - const ctx = cds.EventContext.for ({ id: cds.utils.uuid(), http: { req: null, res: null } }) + const ctx = cds.EventContext.for({ id: cds.utils.uuid(), http: { req: null, res: null } }) ctx.user = user await cds._with(ctx, () => AdminSrv.dispatch(req)) diff --git a/tests/integration/attachments.test.js b/tests/integration/attachments.test.js index ab62c1e9..4cae716d 100644 --- a/tests/integration/attachments.test.js +++ b/tests/integration/attachments.test.js @@ -7,28 +7,9 @@ const { createReadStream } = cds.utils.fs const { join } = cds.utils.path const app = path.join(__dirname, "../incidents-app") -const { test, axios, GET: _GET, POST, DELETE: _DELETE } = cds.test(app) +const { test, axios, GET, POST, DELETE } = cds.test(app) axios.defaults.auth = { username: "alice" } -const DELETE = async function () { - try { - return await _DELETE(...arguments) - } catch (e) { - if (e.response) - return e.response - else - throw e - } -} -const GET = async function () { - try { - return await _GET(...arguments) - } catch (e) { - if (e.response) - return e.response - else - throw e - } -} + let utils = null const incidentID = "3ccf474c-3881-44b7-99fb-59a2a4668418" @@ -203,12 +184,11 @@ describe("Tests for uploading/deleting attachments through API calls", () => { expect(response.status).toEqual(200) expect(response.data.value.length).toEqual(0) - const content = await GET( + await GET( `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)/attachments(up__ID=${incidentID},ID=${sampleDocID},IsActiveEntity=true)/content` - ) - - expect(content).toMatchObject({ - status: 404 + ).catch(e => { + expect(e.status).toEqual(404) + expect(e.response.data.error.message).toMatch(/Not Found/) }) }) @@ -216,22 +196,23 @@ describe("Tests for uploading/deleting attachments through API calls", () => { const response = await DELETE( `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)` ) - expect(response).toMatchObject({ status: 204 }) + expect(response.status).toEqual(204) - const response2 = await DELETE( + await DELETE( `odata/v4/processor/Incidents(ID=${incidentID},IsActiveEntity=true)` - ) - expect(response2.status).toEqual(404) + ).catch(e => { + expect(e.status).toEqual(404) + expect(e.response.data.error.message).toMatch(/Not Found/) + }) }) it("Cancel draft where parent has composed key", async () => { - await POST( `odata/v4/processor/SampleRootWithComposedEntity`, { sampleID: "ABC", gjahr: 2025 - } - ) + }) + const doc = await POST( `odata/v4/processor/SampleRootWithComposedEntity(sampleID='ABC',gjahr=2025,IsActiveEntity=false)/attachments`, { @@ -302,8 +283,39 @@ describe("Tests for uploading/deleting attachments through API calls", () => { ) expect(responseContent.status).toEqual(200) }) + + it("should fail to upload attachment to non-existent entity", async () => { + const fileContent = fs.readFileSync( + path.join(__dirname, "..", "integration", "content/sample.pdf") + ) + await axios.put( + `/odata/v4/admin/Incidents(${incidentID})/attachments(up__ID=${incidentID},ID=${cds.utils.uuid()})/content`, + fileContent, + { + headers: { + "Content-Type": "application/pdf", + "Content-Length": fileContent.length, + }, + } + ).catch(e => { + expect(e.status).toEqual(404) + expect(e.response.data.error.message).toMatch(/Not Found/) + }) + }) + + it("should fail to update note for non-existent attachment", async () => { + await axios.patch( + `/odata/v4/admin/Incidents(${incidentID})/attachments(up__ID=${incidentID},ID=${cds.utils.uuid()})`, + { note: "This should fail" }, + { headers: { "Content-Type": "application/json" } } + ).catch(e => { + expect(e.status).toEqual(404) + expect(e.response.data.error.message).toMatch(/Not Found/) + }) + }) }) + describe("Tests for attachments facet disable", () => { beforeAll(async () => { // Initialize test variables diff --git a/tests/non-draft-request.http b/tests/non-draft-request.http index 3465ef28..92317c99 100644 --- a/tests/non-draft-request.http +++ b/tests/non-draft-request.http @@ -5,13 +5,13 @@ ### Get list of all incidents # @name incidents -GET {{host}}/odata/v4/processor/Incidents +GET {{host}}/odata/v4/admin/Incidents Authorization: {{auth}} ### Creating attachment (metadata request) @incidentsID = {{incidents.response.body.value[2].ID}} # @name createAttachment -POST {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments +POST {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments Authorization: {{auth}} Content-Type: application/json @@ -21,40 +21,40 @@ Content-Type: application/json ### Put attachment content (content request) @attachmentsID = {{createAttachment.response.body.ID}} -PUT {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}})/content +PUT {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}})/content Authorization: {{auth}} Content-Type: image/jpeg < ./integration/content/sample-1.jpg ### Get newly created attachment metadata -GET {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}}) +GET {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}}) Authorization: {{auth}} ### Fetching newly created attachment content -GET {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}})/content +GET {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}})/content Authorization: {{auth}} ### Get list of attachments for a particular incident # @name attachments -GET {{host}}/odata/v4/processor/Incidents(ID={{incidentsID}})/attachments +GET {{host}}/odata/v4/admin/Incidents(ID={{incidentsID}})/attachments Authorization: {{auth}} ### Get attachments content -GET {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}})/content +GET {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}})/content Authorization: {{auth}} ### Get attachments content with up__ID included -GET {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(up__ID={{incidentsID}},ID={{attachmentsID}})/content +GET {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(up__ID={{incidentsID}},ID={{attachmentsID}})/content Authorization: {{auth}} ### Put attachment content (content request) with up__ID included -PUT {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(up__ID={{incidentsID}},ID={{attachmentsID}})/content +PUT {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(up__ID={{incidentsID}},ID={{attachmentsID}})/content Authorization: {{auth}} Content-Type: image/jpeg < ./integration/content/sample-1.jpg ### Delete attachment -DELETE {{host}}/odata/v4/processor/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}}) +DELETE {{host}}/odata/v4/admin/Incidents({{incidentsID}})/attachments(ID={{attachmentsID}}) Authorization: {{auth}} \ No newline at end of file diff --git a/tests/unit/unitTests.test.js b/tests/unit/unitTests.test.js index 8cc7c6c0..8666163d 100644 --- a/tests/unit/unitTests.test.js +++ b/tests/unit/unitTests.test.js @@ -79,11 +79,9 @@ describe('getObjectStoreCredentials', () => { }) it('should throw error if credentials are missing', async () => { - try { - await getObjectStoreCredentials('tenant') - } catch (err) { - expect(err.message).toBe('Service Manager Instance is not bound') - } + await getObjectStoreCredentials('tenant').catch(e => { + expect(e.message).to.equal('Service Manager Instance is not bound') + }) }) }) diff --git a/tests/unit/validateAttachmentSize.test.js b/tests/unit/validateAttachmentSize.test.js index 3622ad51..1774cef0 100644 --- a/tests/unit/validateAttachmentSize.test.js +++ b/tests/unit/validateAttachmentSize.test.js @@ -29,13 +29,13 @@ describe('validateAttachmentSize', () => { req.headers['content-length'] = '20480000000' validateAttachmentSize(req) - expect(req.reject).toHaveBeenCalledWith(400, 'File Size limit exceeded beyond 400 MB.') + expect(req.reject).toHaveBeenCalledWith(400, 'AttachmentSizeExceeded') }) it('should reject when content-length header is missing', () => { validateAttachmentSize(req) - expect(req.reject).toHaveBeenCalledWith(400, 'Invalid Content Size') + expect(req.reject).toHaveBeenCalledWith(400, 'InvalidContentSize') }) })