From 24ed39430cdc12f8ed1b81195beac19fc999be96 Mon Sep 17 00:00:00 2001 From: YashmeetSAP Date: Wed, 22 May 2024 14:06:18 +0530 Subject: [PATCH] Review comments & UTs --- lib/handler/index.js | 9 +- lib/persistence/index.js | 10 -- lib/sdm.js | 41 +------ lib/util/messageConsts.js | 2 - test/lib/handler/index.test.js | 210 +++++++++++++++++++++++++++------ test/lib/sdm.test.js | 128 ++++++-------------- 6 files changed, 223 insertions(+), 177 deletions(-) diff --git a/lib/handler/index.js b/lib/handler/index.js index a3b779a..dbbf8ed 100644 --- a/lib/handler/index.js +++ b/lib/handler/index.js @@ -57,6 +57,11 @@ async function getFolderIdByPath(req, credentials, token, attachments) { const response = await axios.get(getFolderByPathURL, config); return response.data.properties["cmis:objectId"].value; } catch (error) { + let statusText = "An Error Occurred"; + if (error.response && error.response.statusText) { + statusText = error.response.statusText; + } + console.log(statusText); return null; } } @@ -120,7 +125,7 @@ async function createAttachment( return response; } -async function deleteAttachmentOrFolder(credentials, token, objectId) { +async function deleteAttachmentsOfFolder(credentials, token, objectId) { const { repositoryId } = getConfigurations(); const documentDeleteURL = credentials.uri + "browser/" + repositoryId + "/root"; @@ -175,7 +180,7 @@ module.exports = { getFolderIdByPath, createFolder, createAttachment, - deleteAttachmentOrFolder, + deleteAttachmentsOfFolder, deleteFolderWithAttachments, readAttachment, }; diff --git a/lib/persistence/index.js b/lib/persistence/index.js index 53dd4d7..c972d1b 100644 --- a/lib/persistence/index.js +++ b/lib/persistence/index.js @@ -22,15 +22,6 @@ async function getFolderIdForEntity(attachments, req) { .where({ [up_]: req.data[idValue] }); } -async function getDuplicateAttachments(fileNames, attachments, req) { - const up_ = attachments.keys.up_.keys[0].$generatedFieldName; - const idValue = up_.split("__")[1]; - return await SELECT.distinct(["filename"]) - .from(attachments) - .where({ [up_]: req.data[idValue] }) - .and({ filename: { in: fileNames } }); -} - async function getURLsToDeleteFromAttachments(deletedAttachments, attachments) { return await SELECT.from(attachments) .columns("url") @@ -38,7 +29,6 @@ async function getURLsToDeleteFromAttachments(deletedAttachments, attachments) { } module.exports = { getDraftAttachments, - getDuplicateAttachments, getURLsToDeleteFromAttachments, getURLFromAttachments, getFolderIdForEntity, diff --git a/lib/sdm.js b/lib/sdm.js index b72dfa0..8612771 100644 --- a/lib/sdm.js +++ b/lib/sdm.js @@ -3,14 +3,13 @@ const { getFolderIdByPath, createFolder, createAttachment, - deleteAttachmentOrFolder, + deleteAttachmentsOfFolder, deleteFolderWithAttachments, readAttachment, } = require("../lib/handler"); const { fetchAccessToken } = require("./util/index"); const { getDraftAttachments, - getDuplicateAttachments, getURLsToDeleteFromAttachments, getURLFromAttachments, getFolderIdForEntity, @@ -55,20 +54,11 @@ module.exports = class SDMAttachmentsService extends ( if (duplicateDraftFilesErrMsg != "") { req.reject(409, duplicateDraftFileErr(duplicateDraftFilesErrMsg)); } - //verify if duplicates exist for the drafts - const duplicateFilesErrMsg = await this.isFileNameDuplicate( - attachment_val, - attachments, - req - ); - if (duplicateFilesErrMsg != "") { - req.reject(409, duplicateFileErr(duplicateFilesErrMsg)); - } const token = await fetchAccessToken(this.creds); console.log("Token: ", token); const folderIds = await getFolderIdForEntity(attachments, req); let parentId = ""; - if (folderIds.length == 0) { + if (folderIds?.length == 0) { const folderId = await getFolderIdByPath( req, this.creds, @@ -87,7 +77,7 @@ module.exports = class SDMAttachmentsService extends ( parentId = response.data.succinctProperties["cmis:objectId"]; } } else { - parentId = folderIds[0].folderId; + parentId = folderIds ? folderIds[0].folderId : ""; } const failedReq = await this.onCreate( attachment_val, @@ -121,27 +111,6 @@ module.exports = class SDMAttachmentsService extends ( return duplicates.join(", "); } - async isFileNameDuplicate(attachment_val, attachments, req) { - let fileNames = []; - for (let index in attachment_val) { - fileNames.push(attachment_val[index].filename); - } - const are_duplicates = await getDuplicateAttachments( - fileNames, - attachments, - req - ); - let duplicateFilesErrMsg = ""; - are_duplicates.forEach((file) => { - duplicateFilesErrMsg += "," + file.filename; - }); - // Remove leading comma if any - if (duplicateFilesErrMsg.charAt(0) === ",") { - duplicateFilesErrMsg = duplicateFilesErrMsg.slice(1); - } - return duplicateFilesErrMsg; - } - async attachDeletionData(req) { const attachments = cds.model.definitions[req.query.target.name + ".attachments"]; @@ -165,7 +134,7 @@ module.exports = class SDMAttachmentsService extends ( } if (req.event == "DELETE") { const folderIds = await getFolderIdForEntity(attachments, req); - if (folderIds.length > 0) { + if (folderIds?.length > 0) { const parentId = folderIds[0].folderId; req.parentId = parentId; } @@ -187,7 +156,7 @@ module.exports = class SDMAttachmentsService extends ( } else { const deletePromises = req.attachmentsToDelete.map( async (attachment) => { - const deleteAttachmentResponse = await deleteAttachmentOrFolder( + const deleteAttachmentResponse = await deleteAttachmentsOfFolder( this.creds, token, attachment.url diff --git a/lib/util/messageConsts.js b/lib/util/messageConsts.js index eb3fa7f..8b8d3bb 100644 --- a/lib/util/messageConsts.js +++ b/lib/util/messageConsts.js @@ -1,5 +1,3 @@ -module.exports.duplicateFileErr = (duplicateFiles) => - `The files ${duplicateFiles} are already present in the repository. Please remove them from drafts, rename and try again.`; module.exports.duplicateDraftFileErr = (duplicateDraftFiles) => `The files ${duplicateDraftFiles} are added multiple times. Please remove them from drafts, rename and try again.`; module.exports.emptyFileErr = (fileName) => diff --git a/test/lib/handler/index.test.js b/test/lib/handler/index.test.js index 99866d6..bd700a2 100644 --- a/test/lib/handler/index.test.js +++ b/test/lib/handler/index.test.js @@ -21,56 +21,165 @@ jest.mock("../../../lib/util/index", () => { const FormData = require("form-data"); const { getConfigurations } = require("../../../lib/util/index"); const createAttachment = require("../../../lib/handler/index").createAttachment; -const deleteAttachment = require("../../../lib/handler/index").deleteAttachment; +const deleteAttachmentsOfFolder = + require("../../../lib/handler/index").deleteAttachmentsOfFolder; const readAttachment = require("../../../lib/handler/index").readAttachment; +const getFolderIdByPath = + require("../../../lib/handler/index").getFolderIdByPath; +const createFolder = require("../../../lib/handler/index").createFolder; +const deleteFolderWithAttachments = + require("../../../lib/handler/index").deleteFolderWithAttachments; describe("handlers", () => { - describe('ReadAttachment function', () => { + describe("ReadAttachment function", () => { beforeEach(() => { jest.clearAllMocks(); }); - it('returns document on successful read', async () => { - const mockKey = '123'; - const mockToken = 'a1b2c3'; - const mockCredentials = {uri: 'http://example.com/'}; - const mockRepositoryId = '123'; - - const mockResponse = {data: 'mock pdf file content'}; - const mockBuffer = Buffer.from(mockResponse.data, 'binary'); - + it("returns document on successful read", async () => { + const mockKey = "123"; + const mockToken = "a1b2c3"; + const mockCredentials = { uri: "http://example.com/" }; + //const mockRepositoryId = "123"; + + const mockResponse = { data: "mock pdf file content" }; + const mockBuffer = Buffer.from(mockResponse.data, "binary"); + axios.get.mockResolvedValue(mockResponse); - getConfigurations.mockReturnValue({repositoryId: mockRepositoryId}); - - const document = await readAttachment(mockKey, mockToken, mockCredentials); - - const expectedUrl = mockCredentials.uri+ "browser/" + mockRepositoryId + "/root?objectID=" + mockKey + "&cmisselector=content"; + //getConfigurations.mockReturnValue({ repositoryId: mockRepositoryId }); + + const document = await readAttachment( + mockKey, + mockToken, + mockCredentials + ); + + const expectedUrl = + mockCredentials.uri + + "browser/123/root?objectID=" + + mockKey + + "&cmisselector=content"; expect(axios.get).toHaveBeenCalledWith(expectedUrl, { - headers: {Authorization: `Bearer ${mockToken}`}, - responseType: 'arraybuffer' + headers: { Authorization: `Bearer ${mockToken}` }, + responseType: "arraybuffer", }); expect(document).toEqual(mockBuffer); }); - - it('throws error on unsuccessful read', async () => { - axios.get.mockImplementationOnce(() => Promise.reject({ - response: { - statusText: 'something bad happened' - } - })); - - await expect(readAttachment('123', 'a1b2c3', {uri: 'http://example.com/'})).rejects.toThrow('something bad happened'); + + it("throws error on unsuccessful read", async () => { + axios.get.mockImplementationOnce(() => + Promise.reject({ + response: { + statusText: "something bad happened", + }, + }) + ); + + await expect( + readAttachment("123", "a1b2c3", { uri: "http://example.com/" }) + ).rejects.toThrow("something bad happened"); }); it('throws error with "An Error Occurred" message when statusText is missing', async () => { - axios.get.mockImplementationOnce(() => Promise.reject({ - response: {} - })); - - await expect(readAttachment('123', 'a1b2c3', {uri: 'http://example.com/'})).rejects.toThrow('An Error Occurred'); + axios.get.mockImplementationOnce(() => + Promise.reject({ + response: {}, + }) + ); + + await expect( + readAttachment("123", "a1b2c3", { uri: "http://example.com/" }) + ).rejects.toThrow("An Error Occurred"); + }); + }); + + describe("Test for getFolderIdByPath", () => { + let mockedReq, mockedCredentials, mockedToken, mockedAttachments; + beforeEach(() => { + jest.clearAllMocks(); + mockedReq = { data: { idValue: "testValue" } }; + mockedCredentials = { uri: "mocked_uri/" }; + mockedToken = "mocked_token"; + mockedAttachments = { + keys: { up_: { keys: [{ $generatedFieldName: "__idValue" }] } }, + }; + }); + + it("should return a folderId when axios request is success", async () => { + const mockedResponse = { + data: { properties: { "cmis:objectId": { value: "folderId" } } }, + }; + axios.get.mockResolvedValue(mockedResponse); + //getConfigurations.mockReturnValue({ repositoryId: "123" }); + + const result = await getFolderIdByPath( + mockedReq, + mockedCredentials, + mockedToken, + mockedAttachments + ); + + // assertions + expect(result).toEqual("folderId"); + expect(axios.get).toHaveBeenCalledWith( + "mocked_uri/browser/123/root/testValue?cmisselector=object", + { headers: { Authorization: "Bearer mocked_token" } } + ); + }); + + it("should return null when axios request fails", async () => { + axios.get.mockRejectedValue(new Error("Network error")); + //getConfigurations.mockReturnValue({ repositoryId: "123" }); + + const result = await getFolderIdByPath( + mockedReq, + mockedCredentials, + mockedToken, + mockedAttachments + ); + + // assertions + expect(result).toEqual(null); + expect(axios.get).toHaveBeenCalledWith( + "mocked_uri/browser/123/root/testValue?cmisselector=object", + { headers: { Authorization: "Bearer mocked_token" } } + ); + }); + }); + + describe("createFolder", () => { + it("should create a folder and return expected response when updateServerRequest is successful", async () => { + // arrange + const mockResponse = { data: "some_data" }; + axios.post.mockResolvedValue(mockResponse); + const mockedReq = { data: { field1: "value1" } }; + const mockedCredentials = { uri: "mocked_uri/" }; + const mockedToken = "mocked_token"; + const mockedAttachments = { + keys: { + up_: { + keys: [ + { + $generatedFieldName: "field1__123", + }, + ], + }, + }, + }; + // act + const response = await createFolder( + mockedReq, + mockedCredentials, + mockedToken, + mockedAttachments + ); + // assert + expect(response).toEqual(mockResponse); + expect(axios.post).toHaveBeenCalledTimes(1); + expect(axios.post).toHaveBeenCalled(); }); }); - + describe("createAttachment function", () => { beforeEach(() => { jest.clearAllMocks(); @@ -98,9 +207,10 @@ describe("handlers", () => { }); }); - describe("deleteAttachment()", () => { + describe("deleteAttachmentsOfFolder()", () => { beforeEach(() => { axios.post.mockClear(); + jest.clearAllMocks(); }); it("should perform the delete operation for given attachment", async () => { @@ -110,7 +220,7 @@ describe("handlers", () => { const objectId = "demo-objectId"; const attachments = {}; - const response = await deleteAttachment( + const response = await deleteAttachmentsOfFolder( credentials, token, objectId, @@ -134,8 +244,7 @@ describe("handlers", () => { const token = "demo-token"; const objectId = "demo-objectId"; const attachments = {}; - - const response = await deleteAttachment( + const response = await deleteAttachmentsOfFolder( credentials, token, objectId, @@ -155,4 +264,31 @@ describe("handlers", () => { ); }); }); + + describe("deleteFolderWithAttachments", () => { + beforeEach(() => { + axios.post.mockClear(); + jest.clearAllMocks(); + }); + it("should delete a folder and return expected response when updateServerRequest is successful", async () => { + // arrange + const mockResponse = { data: "some_data" }; + axios.post.mockResolvedValue(mockResponse); + const mockedCredentials = { uri: "mocked_uri/" }; + const mockedToken = "mocked_token"; + const parentId = "mocked_parentId"; + + // act + const response = await deleteFolderWithAttachments( + mockedCredentials, + mockedToken, + parentId + ); + + // assert + expect(response).toEqual(mockResponse); + expect(axios.post).toHaveBeenCalledTimes(1); + expect(axios.post).toHaveBeenCalled(); + }); + }); }); diff --git a/test/lib/sdm.test.js b/test/lib/sdm.test.js index 180e046..da19170 100644 --- a/test/lib/sdm.test.js +++ b/test/lib/sdm.test.js @@ -7,8 +7,11 @@ const getURLsToDeleteFromAttachments = require("../../lib/persistence").getURLsToDeleteFromAttachments; const getURLFromAttachments = require("../../lib/persistence").getURLFromAttachments; +const getFolderIdForEntity = + require("../../lib/persistence").getFolderIdForEntity; const fetchAccessToken = require("../../lib/util").fetchAccessToken; -const deleteAttachment = require("../../lib/handler").deleteAttachment; +const deleteAttachmentsOfFolder = + require("../../lib/handler").deleteAttachmentsOfFolder; const createAttachment = require("../../lib/handler").createAttachment; const readAttachment = require("../../lib/handler").readAttachment; const { duplicateFileErr } = require("../../lib/util/messageConsts"); @@ -18,13 +21,14 @@ jest.mock("../../lib/persistence", () => ({ getDraftAttachments: jest.fn(), getDuplicateAttachments: jest.fn(), getURLsToDeleteFromAttachments: jest.fn(), - getURLFromAttachments: jest.fn() + getURLFromAttachments: jest.fn(), + getFolderIdForEntity: jest.fn(), })); jest.mock("../../lib/util", () => ({ fetchAccessToken: jest.fn(), })); jest.mock("../../lib/handler", () => ({ - deleteAttachment: jest.fn(), + deleteAttachmentsOfFolder: jest.fn(), createAttachment: jest.fn(), readAttachment: jest.fn(), })); @@ -45,49 +49,59 @@ describe("SDMAttachmentsService", () => { service = new SDMAttachmentsService(); service.creds = { uri: "mock_cred" }; }); - + it("should interact with DB, fetch access token and readAttachment with correct parameters", async () => { const attachments = ["attachment1", "attachment2"]; const keys = ["key1", "key2"]; const token = "dummy_token"; - const response = {url:'mockUrl'} - + const response = { url: "mockUrl" }; + fetchAccessToken.mockResolvedValueOnce(token); - getURLFromAttachments.mockResolvedValueOnce(response) - + getURLFromAttachments.mockResolvedValueOnce(response); + await service.get(attachments, keys); - - expect(getURLFromAttachments).toHaveBeenCalledWith(keys,attachments) + + expect(getURLFromAttachments).toHaveBeenCalledWith(keys, attachments); expect(fetchAccessToken).toHaveBeenCalledWith(service.creds); - expect(readAttachment).toHaveBeenCalledWith("mockUrl", token, service.creds); + expect(readAttachment).toHaveBeenCalledWith( + "mockUrl", + token, + service.creds + ); }); it("should throw error if readAttachment fails", async () => { const attachments = ["attachment1", "attachment2"]; const keys = ["key1", "key2"]; const token = "dummy_token"; - const response = {url:'mockUrl'} - + const response = { url: "mockUrl" }; + fetchAccessToken.mockResolvedValueOnce(token); getURLFromAttachments.mockResolvedValueOnce(response); // Make readAttachment to throw error readAttachment.mockImplementationOnce(() => { - throw new Error('Error reading attachment'); + throw new Error("Error reading attachment"); }); - - await expect(service.get(attachments, keys)).rejects.toThrow('Error reading attachment'); - - expect(getURLFromAttachments).toHaveBeenCalledWith(keys,attachments); + + await expect(service.get(attachments, keys)).rejects.toThrow( + "Error reading attachment" + ); + + expect(getURLFromAttachments).toHaveBeenCalledWith(keys, attachments); expect(fetchAccessToken).toHaveBeenCalledWith(service.creds); - expect(readAttachment).toHaveBeenCalledWith("mockUrl", token, service.creds); + expect(readAttachment).toHaveBeenCalledWith( + "mockUrl", + token, + service.creds + ); }); - }); describe("draftSaveHandler", () => { let service; let mockReq; let cds; beforeEach(() => { + jest.clearAllMocks(); cds = require("@sap/cds/lib"); service = new SDMAttachmentsService(); service.creds = { uaa: "mocked uaa" }; @@ -110,17 +124,6 @@ describe("SDMAttachmentsService", () => { }; }); - it("should reject if duplicates exist", async () => { - getDraftAttachments.mockResolvedValue([{ id: 1 }, { id: 2 }]); - service.onCreate = jest.fn().mockResolvedValue([]); - service.isFileNameDuplicate = jest.fn().mockResolvedValue("error"); - await service.draftSaveHandler(mockReq); - expect(mockReq.reject).toHaveBeenCalledWith( - 409, - duplicateFileErr("error") - ); - }); - it("should handle failure in onCreate", async () => { service.isFileNameDuplicate = jest.fn().mockResolvedValue(""); service.onCreate = jest.fn().mockResolvedValue(["ChildTest"]); @@ -151,61 +154,6 @@ describe("SDMAttachmentsService", () => { }); }); - describe("isFileNameDuplicate", () => { - let service; - beforeEach(() => { - service = new SDMAttachmentsService(); - }); - - it("should detect duplicates", async () => { - const attachments = [ - /* array of attachment objects */ - ]; - const attachment_val = [{ filename: "file1" }, { filename: "file2" }]; - getDuplicateAttachments.mockResolvedValue([{ filename: "file1" }]); - - const result = await service.isFileNameDuplicate( - attachment_val, - attachments - ); - - expect(result).toBe("file1"); - }); - - it("should return empty string if no duplicates", async () => { - const attachments = [ - /* array of attachment objects */ - ]; - const attachment_val = [{ filename: "file1" }, { filename: "file2" }]; - getDuplicateAttachments.mockResolvedValue([]); - - const result = await service.isFileNameDuplicate( - attachment_val, - attachments - ); - - expect(result).toBe(""); - }); - - it("should concatenate multiple duplicates", async () => { - const attachments = [ - /* array of attachment objects */ - ]; - const attachment_val = [{ filename: "file1" }, { filename: "file2" }]; - getDuplicateAttachments.mockResolvedValue([ - { filename: "file1" }, - { filename: "file2" }, - ]); - - const result = await service.isFileNameDuplicate( - attachment_val, - attachments - ); - - expect(result).toBe("file1,file2"); - }); - }); - describe("attachDeletionData", () => { let service; beforeEach(() => { @@ -312,7 +260,7 @@ describe("SDMAttachmentsService", () => { cds.model.definitions["testTarget.attachments"] = {}; // Add relevant attachment definition fetchAccessToken.mockResolvedValue("test_token"); - deleteAttachment.mockResolvedValue({}); + deleteAttachmentsOfFolder.mockResolvedValue({}); service.handleRequest = jest .fn() .mockResolvedValueOnce({ message: expectedErrorResponse, ID: "2" }); @@ -320,14 +268,14 @@ describe("SDMAttachmentsService", () => { await service.deleteAttachmentsWithKeys(records, req); expect(fetchAccessToken).toHaveBeenCalledTimes(1); - expect(deleteAttachment).toHaveBeenCalledTimes(2); + expect(deleteAttachmentsOfFolder).toHaveBeenCalledTimes(2); expect(service.handleRequest).toHaveBeenCalledTimes(2); expect(req.attachmentsToDelete).toHaveLength(1); expect(req.attachmentsToDelete[0].ID).toEqual("1"); expect(req.info).toHaveBeenCalledWith(200, "\n" + expectedErrorResponse); }); - it("should not call fetchAccessToken, deleteAttachment, and handleRequest methods if req.attachmentsToDelete is empty", async () => { + it("should not call fetchAccessToken, deleteAttachmentsOfFolder, and handleRequest methods if req.attachmentsToDelete is empty", async () => { const records = []; jest.spyOn(service, "handleRequest"); const req = { @@ -338,7 +286,7 @@ describe("SDMAttachmentsService", () => { await service.deleteAttachmentsWithKeys(records, req); expect(fetchAccessToken).not.toHaveBeenCalled(); - expect(deleteAttachment).not.toHaveBeenCalled(); + expect(deleteAttachmentsOfFolder).not.toHaveBeenCalled(); expect(service.handleRequest).not.toHaveBeenCalled(); }); });