Bug/3899/upload same map twice

visualization/CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/)
 - Fix loading cc.json files that contain the 'authors' attribute [#3848](https://github.com/MaibornWolff/codecharta/pull/3897)
 - Fix applying Custom Views [#3898](https://github.com/MaibornWolff/codecharta/pull/3898)
 - The camera is now only reset when the area or the height of the map is changed [#3896](https://github.com/MaibornWolff/codecharta/pull/3896)
+- Fix freezing app on uploading already loaded files [#3901](https://github.com/MaibornWolff/codecharta/pull/3901)
 
 ## [1.131.2] - 2024-12-04
 

visualization/app/codeCharta/services/loadFile/fileParser.ts

@@ -46,7 +46,8 @@ export function enrichFileStatesAndRecentFilesWithValidationResults(
     fileValidationResults: FileValidationResult[],
     currentFilesAreSampleFilesCallback: () => boolean,
     setCurrentFilesAreNotSampleFilesCallback: () => void
-) {
+): boolean {
+    let hasAddedAtLeastOneFile = false
     for (const nameDataPair of nameDataPairs) {
         const fileValidationResult: FileValidationResult = {
             fileName: nameDataPair?.fileName,
@@ -59,14 +60,23 @@ export function enrichFileStatesAndRecentFilesWithValidationResults(
 
         if (fileValidationResult.errors.length === 0) {
             fileValidationResult.warnings.push(...checkWarnings(nameDataPair?.content))
-
-            addFile(fileStates, recentFiles, nameDataPair, currentFilesAreSampleFilesCallback, setCurrentFilesAreNotSampleFilesCallback)
+            const hasAddedFile = addFile(
+                fileStates,
+                recentFiles,
+                nameDataPair,
+                currentFilesAreSampleFilesCallback,
+                setCurrentFilesAreNotSampleFilesCallback
+            )
+            if (hasAddedFile) {
+                hasAddedAtLeastOneFile = true
+            }
         }
 
         if (fileValidationResult.errors.length > 0 || fileValidationResult.warnings.length > 0) {
             fileValidationResults.push(fileValidationResult)
         }
     }
+    return hasAddedAtLeastOneFile
 }
 
 function addFile(
@@ -75,7 +85,7 @@ function addFile(
     file: NameDataPair,
     currentFilesAreSampleFilesCallback: () => boolean,
     setCurrentFilesAreNotSampleFilesCallback: () => void
-) {
+): boolean {
     if (currentFilesAreSampleFilesCallback()) {
         fileStates.length = 0
         setCurrentFilesAreNotSampleFilesCallback()
@@ -98,13 +108,13 @@ function addFile(
     }
     if (isDuplicate) {
         fileStates[storedFileChecksums.get(currentFileChecksum)].file.fileMeta.fileName = currentFileName
-        recentFiles[0] = currentFileName
-        recentFiles.push(currentFileName)
-        return
+        recentFiles.unshift(currentFileName)
+        return false
     }
 
     fileStates.push({ file: ccFile, selectedAs: FileSelectionState.None })
     recentFiles.push(currentFileName)
+    return true
 }
 
 function getFileName(currentFileName: string, storedFileNames: Map<string, string>, currentFileChecksum: string) {

visualization/app/codeCharta/services/loadFile/loadFile.service.spec.ts

@@ -1,5 +1,5 @@
 import { TestBed } from "@angular/core/testing"
-import { LoadFileService } from "./loadFile.service"
+import { FILES_ALREADY_LOADED_ERROR_MESSAGE, LoadFileService } from "./loadFile.service"
 import { TEST_FILE_CONTENT, TEST_FILE_CONTENT_WITH_AUTHORS, TEST_FILE_CONTENT_WITHOUT_AUTHORS } from "../../util/dataMocks"
 import { CCFile, CcState, NodeMetricData, NodeType } from "../../codeCharta.model"
 import { removeFiles, setDeltaReference, setStandard } from "../../state/store/files/files.actions"
@@ -18,6 +18,7 @@ import { State, Store, StoreModule } from "@ngrx/store"
 import { appReducers, setStateMiddleware } from "../../state/store/state.manager"
 import { setCurrentFilesAreSampleFiles } from "../../state/store/appStatus/currentFilesAreSampleFiles/currentFilesAreSampleFiles.actions"
 import { getCCFileAndDecorateFileChecksum } from "../../util/fileHelper"
+import { FileSelectionState, FileState } from "../../model/files/files"
 
 const mockedMetricDataSelector = metricDataSelector as unknown as jest.Mock
 jest.mock("../../state/selectors/accumulatedData/metricData/metricData.selector", () => ({
@@ -27,6 +28,7 @@ jest.mock("../../state/selectors/accumulatedData/metricData/metricData.selector"
 describe("loadFileService", () => {
     let codeChartaService: LoadFileService
     let store: Store<CcState>
+    let storeDispatchSpy: jest.SpyInstance
     let state: State<CcState>
     let dialog: MatDialog
     let validFileContent: ExportCCFile
@@ -43,6 +45,8 @@ describe("loadFileService", () => {
             { name: "mcc", maxValue: 1, minValue: 1, values: [1, 1] },
             { name: "rloc", maxValue: 2, minValue: 1, values: [1, 2] }
         ]
+
+        storeDispatchSpy = jest.spyOn(store, "dispatch")
     })
 
     afterEach(() => {
@@ -179,8 +183,6 @@ describe("loadFileService", () => {
         })
 
         it("should delete sample files when loading new files", () => {
-            const dispatchSpy = jest.spyOn(store, "dispatch")
-
             const valid2ndFileContent = klona(validFileContent)
             valid2ndFileContent.fileChecksum = "hash_1"
 
@@ -191,15 +193,13 @@ describe("loadFileService", () => {
 
             const CCFilesUnderTest = getCCFiles(state.getValue().files)
 
-            expect(dispatchSpy).toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
+            expect(storeDispatchSpy).toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
             expect(CCFilesUnderTest.length).toEqual(1)
             expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("SecondFile")
             expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("hash_1")
         })
 
         it("should keep sample file when loading the same sample file again", () => {
-            const dispatchSpy = jest.spyOn(store, "dispatch")
-
             const valid2ndFileContent = klona(validFileContent)
             valid2ndFileContent.fileChecksum = "hash_1"
 
@@ -227,7 +227,7 @@ describe("loadFileService", () => {
 
             const CCFilesUnderTest = getCCFiles(state.getValue().files)
 
-            expect(dispatchSpy).toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
+            expect(storeDispatchSpy).toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
             expect(CCFilesUnderTest.length).toEqual(3)
             expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("SecondFile")
             expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("hash_1")
@@ -238,8 +238,6 @@ describe("loadFileService", () => {
         })
 
         it("should keep sample files when loading the same sample file and after that another different file", () => {
-            const dispatchSpy = jest.spyOn(store, "dispatch")
-
             const valid2ndFileContent = klona(validFileContent)
             valid2ndFileContent.fileChecksum = "hash_1"
 
@@ -251,17 +249,15 @@ describe("loadFileService", () => {
 
             const CCFilesUnderTest = getCCFiles(state.getValue().files)
 
-            expect(dispatchSpy).toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
+            expect(storeDispatchSpy).toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
             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 keep sample file when loading a invalid file", () => {
-            const dispatchSpy = jest.spyOn(store, "dispatch")
-
+        it("should keep sample file when loading an invalid file", () => {
             codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
             store.dispatch(setCurrentFilesAreSampleFiles({ value: true }))
 
@@ -274,13 +270,13 @@ describe("loadFileService", () => {
 
             const CCFilesUnderTest = getCCFiles(state.getValue().files)
 
-            expect(dispatchSpy).not.toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
+            expect(storeDispatchSpy).not.toHaveBeenCalledWith(setCurrentFilesAreSampleFiles({ value: false }))
             expect(CCFilesUnderTest.length).toEqual(1)
             expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("FirstFile")
             expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("invalid-md5-sample")
         })
 
-        it("should replace files with equal file name and checksum when loading new files", () => {
+        it("should ignore files with equal file name and checksum when loading new files", () => {
             const valid2ndFileContent = klona(validFileContent)
             valid2ndFileContent.fileChecksum = "hash_1"
 
@@ -299,6 +295,92 @@ describe("loadFileService", () => {
             expect(CCFilesUnderTest[1].fileMeta.fileChecksum).toEqual("hash_1")
         })
 
+        it("should only load one file if the same file is loaded twice", () => {
+            codeChartaService.loadFiles([
+                { fileName: "FirstFile", content: validFileContent, fileSize: 42 },
+                { fileName: "SecondFile", content: validFileContent, fileSize: 42 }
+            ])
+
+            const CCFilesUnderTest = getCCFiles(state.getValue().files)
+
+            expect(CCFilesUnderTest.length).toEqual(1)
+        })
+
+        it("should throw error if loaded file was already loaded", () => {
+            codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
+
+            expect(() => {
+                codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
+            }).toThrow(FILES_ALREADY_LOADED_ERROR_MESSAGE)
+        })
+
+        it("should throw error if all loaded files were already loaded", () => {
+            const validFileContent2 = clone({ ...TEST_FILE_CONTENT, fileChecksum: "second-file-checksum" })
+            const validFileContent3 = clone({ ...TEST_FILE_CONTENT, fileChecksum: "third-file-checksum" })
+            codeChartaService.loadFiles([
+                { fileName: "FirstFile", content: validFileContent, fileSize: 42 },
+                { fileName: "SecondFile", content: validFileContent2, fileSize: 42 },
+                { fileName: "ThirdFile", content: validFileContent3, fileSize: 42 }
+            ])
+
+            expect(() => {
+                codeChartaService.loadFiles([
+                    { fileName: "FirstFile", content: validFileContent, fileSize: 42 },
+                    { fileName: "ThirdFile", content: validFileContent3, fileSize: 42 }
+                ])
+            }).toThrow(FILES_ALREADY_LOADED_ERROR_MESSAGE)
+        })
+
+        it("should throw no error if one loaded files was not loaded", () => {
+            const validFileContent2 = clone({ ...TEST_FILE_CONTENT, fileChecksum: "second-file-checksum" })
+            const validFileContent3 = clone({ ...TEST_FILE_CONTENT, fileChecksum: "third-file-checksum" })
+            codeChartaService.loadFiles([
+                { fileName: "FirstFile", content: validFileContent, fileSize: 42 },
+                { fileName: "SecondFile", content: validFileContent2, fileSize: 42 }
+            ])
+
+            expect(() => {
+                codeChartaService.loadFiles([
+                    { fileName: "FirstFile", content: validFileContent, fileSize: 42 },
+                    { fileName: "ThirdFile", content: validFileContent3, fileSize: 42 }
+                ])
+            }).not.toThrow()
+        })
+
+        it("should update name when uploading file that was already uploaded", () => {
+            codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
+
+            try {
+                codeChartaService.loadFiles([{ fileName: "DifferentName", content: validFileContent, fileSize: 42 }])
+            } catch (e) {}
+
+            const CCFilesUnderTest = getCCFiles(state.getValue().files)
+
+            expect(CCFilesUnderTest.length).toEqual(1)
+            expect(CCFilesUnderTest[0].fileMeta.fileName).toEqual("DifferentName")
+            expect(CCFilesUnderTest[0].fileMeta.fileChecksum).toEqual("invalid-md5-sample")
+        })
+
+        it("should update visibility when uploading file that was already uploaded", () => {
+            codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
+            const validFileContent2 = clone({ ...TEST_FILE_CONTENT, fileChecksum: "second-file-checksum" })
+            codeChartaService.loadFiles([{ fileName: "SecondFile", content: validFileContent2, fileSize: 42 }])
+
+            store.dispatch(setStandard({ files: [state.getValue().files[1].file] }))
+
+            try {
+                codeChartaService.loadFiles([{ fileName: "FirstFile", content: validFileContent, fileSize: 42 }])
+            } catch (e) {}
+
+            const filesUnderTest: FileState[] = state.getValue().files
+
+            expect(filesUnderTest.length).toEqual(2)
+            expect(filesUnderTest[0].file.fileMeta.fileName).toEqual("FirstFile")
+            expect(filesUnderTest[0].selectedAs).toEqual(FileSelectionState.Partial)
+            expect(filesUnderTest[1].file.fileMeta.fileName).toEqual("SecondFile")
+            expect(filesUnderTest[1].selectedAs).toEqual(FileSelectionState.None)
+        })
+
         it("should load files with equal file name but different checksum and rename uploaded files ", () => {
             const valid2ndFileContent = klona(validFileContent)
             valid2ndFileContent.fileChecksum = "hash_1"

visualization/app/codeCharta/services/loadFile/loadFile.service.ts

@@ -15,6 +15,7 @@ import { Store, State } from "@ngrx/store"
 import { setCurrentFilesAreSampleFiles } from "../../state/store/appStatus/currentFilesAreSampleFiles/currentFilesAreSampleFiles.actions"
 
 export const NO_FILES_LOADED_ERROR_MESSAGE = "File(s) could not be loaded"
+export const FILES_ALREADY_LOADED_ERROR_MESSAGE = "File(s) are already loaded"
 
 @Injectable({ providedIn: "root" })
 export class LoadFileService implements OnDestroy {
@@ -46,7 +47,7 @@ export class LoadFileService implements OnDestroy {
         const recentFiles: string[] = []
         const fileValidationResults: CCFileValidationResult[] = []
 
-        enrichFileStatesAndRecentFilesWithValidationResults(
+        const hasAddedAtLeastOneFile = enrichFileStatesAndRecentFilesWithValidationResults(
             fileStates,
             recentFiles,
             nameDataPairs,
@@ -71,5 +72,9 @@ export class LoadFileService implements OnDestroy {
         const rootName = this.state.getValue().files.find(f => f.file.fileMeta.fileName === recentFile).file.map.name
         this.store.dispatch(setStandardByNames({ fileNames: recentFiles }))
         fileRoot.updateRoot(rootName)
+
+        if (!hasAddedAtLeastOneFile) {
+            throw new Error(FILES_ALREADY_LOADED_ERROR_MESSAGE)
+        }
     }
 }

visualization/app/codeCharta/ui/toolBar/uploadFilesButton/uploadFiles.service.spec.ts

@@ -0,0 +1,87 @@
+import { TestBed } from "@angular/core/testing"
+import { UploadFilesService } from "./uploadFiles.service"
+import { LoadFileService } from "../../../services/loadFile/loadFile.service"
+import { setIsLoadingFile } from "../../../state/store/appSettings/isLoadingFile/isLoadingFile.actions"
+import { setIsLoadingMap } from "../../../state/store/appSettings/isLoadingMap/isLoadingMap.actions"
+import { createCCFileInput } from "../../../util/uploadFiles/createCCFileInput"
+import { TEST_FILE_CONTENT } from "../../../util/dataMocks"
+import stringify from "safe-stable-stringify"
+import { EffectsModule } from "@ngrx/effects"
+import { Store, StoreModule } from "@ngrx/store"
+import { appReducers, setStateMiddleware } from "../../../state/store/state.manager"
+import { MatDialog } from "@angular/material/dialog"
+import { CcState } from "../../../codeCharta.model"
+import { RenderCodeMapEffect } from "../../../state/effects/renderCodeMapEffect/renderCodeMap.effect"
+import { setFiles, setStandardByNames } from "../../../state/store/files/files.actions"
+import { UnfocusNodesEffect } from "../../../state/effects/unfocusNodes/unfocusNodes.effect"
+
+jest.mock("../../../util/uploadFiles/createCCFileInput")
+
+describe("UploadFilesService", () => {
+    let loadFileService: LoadFileService
+    let uploadFilesService: UploadFilesService
+    let store: Store<CcState>
+    let dispatchSpy: jest.SpyInstance
+    let mockFileInput: HTMLInputElement
+
+    beforeEach(() => {
+        restartSystem()
+        rebuildServices()
+
+        dispatchSpy = jest.spyOn(store, "dispatch")
+
+        mockFileInput = {
+            files: [new File([stringify(TEST_FILE_CONTENT)], "test.cc.json", { type: "application/json" })],
+            click: jest.fn(),
+            addEventListener: jest.fn((event, callback) => {})
+        } as unknown as HTMLInputElement
+        ;(createCCFileInput as jest.Mock).mockReturnValue(mockFileInput)
+    })
+
+    afterEach(() => {
+        loadFileService.referenceFileSubscription.unsubscribe()
+    })
+
+    function restartSystem() {
+        TestBed.configureTestingModule({
+            imports: [
+                StoreModule.forRoot(appReducers, { metaReducers: [setStateMiddleware] }),
+                EffectsModule.forRoot([RenderCodeMapEffect, UnfocusNodesEffect])
+            ],
+            providers: [UploadFilesService, LoadFileService, MatDialog]
+        })
+        store = TestBed.inject(Store)
+    }
+
+    function rebuildServices() {
+        uploadFilesService = TestBed.inject(UploadFilesService)
+        loadFileService = TestBed.inject(LoadFileService)
+    }
+
+    it("should upload file", async () => {
+        uploadFilesService.uploadFiles()
+
+        expect(mockFileInput.click).toHaveBeenCalled()
+        await uploadFilesService["uploadFilesOnEvent"](mockFileInput)
+
+        expect(dispatchSpy).toHaveBeenCalledWith(setFiles({ value: [expect.anything()] }))
+        expect(dispatchSpy).toHaveBeenCalledWith(setStandardByNames({ fileNames: ["test.cc.json"] }))
+    })
+
+    it("should dispatch loading false if already loaded file is uploaded", async () => {
+        uploadFilesService.uploadFiles()
+
+        expect(mockFileInput.click).toHaveBeenCalled()
+        await uploadFilesService["uploadFilesOnEvent"](mockFileInput)
+        dispatchSpy.mockClear()
+
+        uploadFilesService.uploadFiles()
+
+        expect(mockFileInput.click).toHaveBeenCalled()
+        await uploadFilesService["uploadFilesOnEvent"](mockFileInput)
+
+        expect(dispatchSpy).toHaveBeenNthCalledWith(4, setStandardByNames({ fileNames: ["test.cc.json"] }))
+        expect(dispatchSpy).toHaveBeenNthCalledWith(5, setIsLoadingFile({ value: false }))
+        expect(dispatchSpy).toHaveBeenNthCalledWith(6, setIsLoadingMap({ value: false }))
+    })
+})