Skip to content

Commit

Permalink
Fix/2480/loading formerly loaded files (#2548)
Browse files Browse the repository at this point in the history
* Update validation of two equal files including file name and md5 hash
#2480

* Add new logic for adding files with different conditions
#2480

* Remove validation method for testing files equality
#2480

* Update addFile method for loading files properly when same file name occurs
#2480

* Change some tests to async functions and update test for loading files with equal file name und fileChecksum
#2480

* Remove storeService from fileValidator test cases
#2480

* Update e2e test for loading files correctly
#2480

* Add tests to ensure the correct upload of files when file names and or checksums are equal
#2480

* Revision of addFile method and correct implementation of renaming files
#2480

* Remove redundant code for adding files
#2480

* Add test that ensures uploading of previously manually fixed files is possible
Refactor some variables
#2480

* Update CHANGELOG.md
#2480

* Refactor condition in getFileName method
#2480

* Rename values of fileChecksum for better understanding in test cases
#2480
  • Loading branch information
Hall-Ma committed Dec 10, 2021
1 parent 6402fa7 commit ab399b9
Show file tree
Hide file tree
Showing 9 changed files with 193 additions and 102 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
- Use icon tag instead of font awesome icon [#2537](https://github.com/MaibornWolff/codecharta/pull/2537).
- Rename text for placeholder of metric chooser [#2547](https://github.com/MaibornWolff/codecharta/pull/2547)
- Fix attribute type selector of primary edge metric not shown [#2528](https://github.com/MaibornWolff/codecharta/issues/2528).
- Identical files and files with identical file names but different hashes can be loaded [#2548](https://github.com/MaibornWolff/codecharta/pull/2548)

### Chore 👨‍💻 👩‍💻

Expand Down
2 changes: 1 addition & 1 deletion visualization/app/codeCharta/assets/sample1.cc.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"projectName": "Sample Project with Edges",
"apiVersion": "1.2",
"fileChecksum": "invalid-md5-sample",
"fileChecksum": "valid-md5-sample1",
"nodes": [
{
"name": "root",
Expand Down
2 changes: 1 addition & 1 deletion visualization/app/codeCharta/assets/sample2.cc.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"projectName": "Sample Project",
"apiVersion": "1.2",
"fileChecksum": "invalid-md5-sample",
"fileChecksum": "valid-md5-sample2",
"nodes": [
{
"name": "root",
Expand Down
2 changes: 1 addition & 1 deletion visualization/app/codeCharta/assets/sample3.cc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"projectName": "Sample Project",
"fileChecksum": "invalid-md5-sample",
"fileChecksum": "valid-md5-sample",
"apiVersion": "1.2",
"nodes": [
{
Expand Down
2 changes: 1 addition & 1 deletion visualization/app/codeCharta/assets/sample4.cc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"projectName": "Sample Project 4",
"fileChecksum": "invalid-md5-sample",
"fileChecksum": "valid-md5-sample-1",
"apiVersion": "1.2",
"nodes": [
{
Expand Down
162 changes: 116 additions & 46 deletions visualization/app/codeCharta/codeCharta.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { CCValidationResult, ERROR_MESSAGES } from "./util/fileValidator"
import packageJson from "../../package.json"
import { clone } from "./util/clone"
import { nodeMetricDataSelector } from "./state/selectors/accumulatedData/metricData/nodeMetricData.selector"
import { klona } from "klona"

const mockedNodeMetricDataSelector = nodeMetricDataSelector as unknown as jest.Mock
jest.mock("./state/selectors/accumulatedData/metricData/nodeMetricData.selector", () => ({
Expand All @@ -32,6 +33,7 @@ describe("codeChartaService", () => {
withMockedDialogService()

validFileContent = clone(TEST_FILE_CONTENT)

metricData = clone([
{ name: "mcc", maxValue: 1, minValue: 1 },
{ name: "rloc", maxValue: 2, minValue: 1 }
Expand All @@ -57,6 +59,8 @@ describe("codeChartaService", () => {
}

describe("loadFiles", () => {
mockedNodeMetricDataSelector.mockImplementation(() => metricData)

const expected: CCFile = {
fileMeta: {
apiVersion: packageJson.codecharta.apiVersion,
Expand Down Expand Up @@ -121,10 +125,10 @@ describe("codeChartaService", () => {
}
}

it("should load a file without edges", () => {
it("should load a file without edges", async () => {
validFileContent.edges = undefined

codeChartaService.loadFiles([
await codeChartaService.loadFiles([
{
fileName,
content: validFileContent,
Expand All @@ -136,8 +140,8 @@ describe("codeChartaService", () => {
expect(isSingleState(storeService.getState().files)).toBeTruthy()
})

it("should load a valid file and update root data", () => {
codeChartaService.loadFiles([
it("should load a valid file and update root data", async () => {
await codeChartaService.loadFiles([
{
fileName,
content: validFileContent,
Expand All @@ -154,39 +158,106 @@ describe("codeChartaService", () => {
expect(CodeChartaService.ROOT_PATH).toEqual(`/${expected.map.name}`)
})

it("should not replace the last files when loading new files", () => {
codeChartaService.loadFiles([
{
fileName: "FirstFile",
content: validFileContent,
fileSize: 42
}
it("should replace files with equal file name and checksum when loading new files", async () => {
const valid2ndFileContent = klona(validFileContent)
valid2ndFileContent.fileChecksum = "hash_1"

await codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])

await codeChartaService.loadFiles([
{ fileName: "FirstFile", content: validFileContent, fileSize: 42 },
{ fileName: "SecondFile", content: valid2ndFileContent, fileSize: 42 }
])

const CCFilesUnderTest = getCCFiles(storeService.getState().files)

expect(CCFilesUnderTest.length).toEqual(2)
expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("FirstFile")
expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("invalid-md5-sample")
expect(CCFilesUnderTest[1].fileMeta.fileName).toEqual("SecondFile")
expect(CCFilesUnderTest[1].fileMeta.fileChecksum).toEqual("hash_1")
})

it("should load files with equal file name but different checksum and rename uploaded files ", async () => {
const valid2ndFileContent = klona(validFileContent)
valid2ndFileContent.fileChecksum = "hash_1"
const valid3rdFileContent = klona(validFileContent)
valid3rdFileContent.fileChecksum = "hash_2"

await codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])

await codeChartaService.loadFiles([
{ fileName: "FirstFile", content: valid2ndFileContent, fileSize: 42 },
{ fileName: "FirstFile", content: valid3rdFileContent, fileSize: 42 }
])

codeChartaService.loadFiles([
{ fileName: "SecondFile", content: validFileContent, fileSize: 42 },
{ fileName: "ThirdFile", content: validFileContent, fileSize: 42 }
const CCFilesUnderTest = getCCFiles(storeService.getState().files)

expect(CCFilesUnderTest.length).toEqual(3)
expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("FirstFile")
expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("invalid-md5-sample")
expect(CCFilesUnderTest[1].fileMeta.fileName).toEqual("FirstFile_1")
expect(CCFilesUnderTest[1].fileMeta.fileChecksum).toEqual("hash_1")
expect(CCFilesUnderTest[2].fileMeta.fileName).toEqual("FirstFile_2")
expect(CCFilesUnderTest[2].fileMeta.fileChecksum).toEqual("hash_2")
})

it("should not load files with equal file name and checksum when there was a renaming of file names in previous uploads", async () => {
const valid2ndFileContent = klona(validFileContent)
valid2ndFileContent.fileChecksum = "hash_1"
const valid3rdFileContent = klona(validFileContent)
valid3rdFileContent.fileChecksum = "hash_2"

await codeChartaService.loadFiles([
{ fileName: "FirstFile", content: validFileContent, fileSize: 42 },
{ fileName: "FirstFile", content: valid2ndFileContent, fileSize: 42 }
])

await codeChartaService.loadFiles([
{ fileName: "FirstFile", content: valid3rdFileContent, fileSize: 42 },
{ fileName: "FirstFile", content: valid2ndFileContent, fileSize: 42 }
])

expect(getCCFiles(storeService.getState().files).length).toEqual(3)
expect(getCCFiles(storeService.getState().files)[0].fileMeta.fileName).toEqual("FirstFile")
expect(getCCFiles(storeService.getState().files)[1].fileMeta.fileName).toEqual("SecondFile")
expect(getCCFiles(storeService.getState().files)[2].fileMeta.fileName).toEqual("ThirdFile")
const CCFilesUnderTest = getCCFiles(storeService.getState().files)

expect(CCFilesUnderTest.length).toEqual(3)
expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("FirstFile")
expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("invalid-md5-sample")
expect(CCFilesUnderTest[1].fileMeta.fileName).toEqual("FirstFile_1")
expect(CCFilesUnderTest[1].fileMeta.fileChecksum).toEqual("hash_1")
expect(CCFilesUnderTest[2].fileMeta.fileName).toEqual("FirstFile_2")
expect(CCFilesUnderTest[2].fileMeta.fileChecksum).toEqual("hash_2")
})

it("should select the newly added file", () => {
codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
it("should not load broken file but after fixing this problem manually it should possible to load this file again", async () => {
const invalidFileContent = klona(validFileContent)
invalidFileContent.apiVersion = ""

codeChartaService.loadFiles([{ fileName: "SecondFile", content: validFileContent, fileSize: 42 }])
await codeChartaService.loadFiles([{ fileName: "FirstFile", content: invalidFileContent, fileSize: 42 }])

await codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])

const CCFilesUnderTest = getCCFiles(storeService.getState().files)

expect(CCFilesUnderTest.length).toEqual(1)
expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("FirstFile")
expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("invalid-md5-sample")
})

it("should select the newly added file", async () => {
const valid2ndFileContent = klona(validFileContent)
valid2ndFileContent.fileChecksum = "hash_1"

await codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])

await codeChartaService.loadFiles([{ fileName: "SecondFile", content: valid2ndFileContent, fileSize: 42 }])

expect(storeService.getState().files[0].selectedAs).toEqual("None")
expect(storeService.getState().files[1].selectedAs).toEqual("Single")
})

it("should load the default scenario after loading a valid file", () => {
mockedNodeMetricDataSelector.mockImplementation(() => metricData)

codeChartaService.loadFiles([
it("should load the default scenario after loading a valid file", async () => {
await codeChartaService.loadFiles([
{
fileName,
content: validFileContent,
Expand All @@ -199,11 +270,10 @@ describe("codeChartaService", () => {
expect(storeService.getState().dynamicSettings.colorMetric).toEqual("mcc")
})

it("should not load the default scenario after loading a valid file, that does not have the required metrics", () => {
it("should not load the default scenario after loading a valid file, that does not have the required metrics", async () => {
metricData.pop()
mockedNodeMetricDataSelector.mockImplementation(() => metricData)

codeChartaService.loadFiles([
await codeChartaService.loadFiles([
{
fileName,
content: validFileContent,
Expand All @@ -216,48 +286,48 @@ describe("codeChartaService", () => {
expect(storeService.getState().dynamicSettings.colorMetric).toBeNull()
})

it("should show error on invalid file", () => {
it("should show error on invalid file", async () => {
const expectedError: CCValidationResult = {
error: [ERROR_MESSAGES.fileIsInvalid],
warning: []
}

codeChartaService.loadFiles([{ fileName, content: null, fileSize: 0 }])
await codeChartaService.loadFiles([{ fileName, content: null, fileSize: 0 }])

expect(storeService.getState().files).toHaveLength(0)
expect(dialogService.showValidationErrorDialog).toHaveBeenCalledWith(expectedError)
})

it("should show error on a random string", () => {
it("should show error on a random string", async () => {
const expectedError: CCValidationResult = {
error: [ERROR_MESSAGES.apiVersionIsInvalid],
warning: []
}

codeChartaService.loadFiles([{ fileName, fileSize: 42, content: "string" as unknown as ExportCCFile }])
await codeChartaService.loadFiles([{ fileName, fileSize: 42, content: "string" as unknown as ExportCCFile }])

expect(storeService.getState().files).toHaveLength(0)
expect(dialogService.showValidationErrorDialog).toHaveBeenCalledWith(expectedError)
})

it("should show error if a file is missing a required property", () => {
it("should show error if a file is missing a required property", async () => {
const expectedError: CCValidationResult = {
error: ["Required error: should have required property 'projectName'"],
warning: []
}

const invalidFileContent = validFileContent
delete invalidFileContent.projectName
codeChartaService.loadFiles([{ fileName, fileSize: 42, content: invalidFileContent }])
await codeChartaService.loadFiles([{ fileName, fileSize: 42, content: invalidFileContent }])

expect(storeService.getState().files).toHaveLength(0)
expect(dialogService.showValidationErrorDialog).toHaveBeenCalledWith(expectedError)
})

it("should convert old blacklist type", () => {
it("should convert old blacklist type", async () => {
validFileContent.blacklist = [{ path: "foo", type: ExportBlacklistType.hide }]

codeChartaService.loadFiles([
await codeChartaService.loadFiles([
{
fileName,
content: validFileContent,
Expand All @@ -269,7 +339,7 @@ describe("codeChartaService", () => {
expect(getCCFiles(storeService.getState().files)[0].settings.fileSettings.blacklist).toEqual(blacklist)
})

it("should break the loop after the first invalid file was validated", () => {
it("should break the loop after the first invalid file was validated", async () => {
const expectedError: CCValidationResult = {
error: ["Required error: should have required property 'projectName'"],
warning: []
Expand All @@ -278,7 +348,7 @@ describe("codeChartaService", () => {
const invalidFileContent = validFileContent
delete invalidFileContent.projectName

codeChartaService.loadFiles([
await codeChartaService.loadFiles([
{ fileName, content: invalidFileContent, fileSize: 42 },
{ fileName, content: invalidFileContent, fileSize: 42 }
])
Expand All @@ -287,44 +357,44 @@ describe("codeChartaService", () => {
expect(dialogService.showValidationErrorDialog).toHaveBeenCalledWith(expectedError)
})

it("should not show a validation error if filenames are duplicated when their path is different", () => {
it("should not show a validation error if filenames are duplicated when their path is different", async () => {
validFileContent.nodes[0].children[0].name = "duplicate"
validFileContent.nodes[0].children[1].children[0].name = "duplicate"

codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])
await codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])

expect(dialogService.showValidationErrorDialog).toHaveBeenCalledTimes(0)
expect(storeService.getState().files).toHaveLength(1)
})

it("should show a validation error if two files in a folder have the same name", () => {
it("should show a validation error if two files in a folder have the same name", async () => {
validFileContent.nodes[0].children[1].children[0].name = "duplicate"
validFileContent.nodes[0].children[1].children[1].name = "duplicate"
const expectedError: CCValidationResult = {
error: [`${ERROR_MESSAGES.nodesNotUnique} Found duplicate of File with path: /root/Parent Leaf/duplicate`],
warning: []
}

codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])
await codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])

expect(dialogService.showValidationErrorDialog).toHaveBeenCalledTimes(1)
expect(dialogService.showValidationErrorDialog).toHaveBeenCalledWith(expectedError)
})

it("should not show a validation error if two files in a folder have the same name but different type", () => {
it("should not show a validation error if two files in a folder have the same name but different type", async () => {
validFileContent.nodes[0].children[1].children[0].name = "duplicate"
validFileContent.nodes[0].children[1].children[0].type = NodeType.FILE
validFileContent.nodes[0].children[1].children[1].name = "duplicate"
validFileContent.nodes[0].children[1].children[1].type = NodeType.FOLDER

codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])
await codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])

expect(dialogService.showValidationErrorDialog).toHaveBeenCalledTimes(0)
expect(storeService.getState().files).toHaveLength(1)
})

it("should remove a file", () => {
codeChartaService.loadFiles([
it("should remove a file", async () => {
await codeChartaService.loadFiles([
{ fileName: "FirstFile", content: validFileContent, fileSize: 42 },
{ fileName: "SecondFile", content: validFileContent, fileSize: 42 }
])
Expand Down

0 comments on commit ab399b9

Please sign in to comment.