Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/2318/migrate pre render service 2 #2980

Merged
merged 28 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
468cf1f
refactor: seems like a working version so far
Aug 11, 2022
cc7750c
refactor: remove now none effective registrations
Aug 11, 2022
2c0b451
refactor: remove logic for reselecting as discussed as this didn't wo…
Aug 11, 2022
2c45dcb
refactor: remove unused getter
Aug 11, 2022
25c7bdf
refactor: remove codeMap.preRender.service and basically deactivate l…
Aug 11, 2022
257cd21
refactor: center map on file selection change again
Aug 11, 2022
e5867f4
Merge branch 'main' into refactor/2318/migrate-pre-render-service-2
MW-Friedrich Aug 12, 2022
99d2496
refactor: make side effect after rendering due to file selection chan…
Aug 12, 2022
eb2d893
fix: re-add loading indicator
Aug 12, 2022
d33b003
fix: trigger codeMapRendered also when rendering wasn't throttled
Aug 12, 2022
8580891
fix: show loading indicator also during file uploading
Aug 12, 2022
78b86bd
refactor: remove unused var
Aug 12, 2022
433b120
fix: handle loading indicator in case of error during file upload
shaman-apprentice Aug 13, 2022
009354a
refactor: remove unnecessary throttling
shaman-apprentice Aug 13, 2022
27469fe
refactor: remove now unnecessary delay
shaman-apprentice Aug 13, 2022
ad4b8c3
fix: only autoFitTo after file selection change
shaman-apprentice Aug 13, 2022
28acf7d
refactor: clean up
shaman-apprentice Aug 14, 2022
3074db9
refactor: unify when to show and hide loading map indicator
shaman-apprentice Aug 14, 2022
deb98b3
refactor: decide to not migrate another service within this migration
shaman-apprentice Aug 14, 2022
fe9281d
refactor: reset default value again
shaman-apprentice Aug 14, 2022
6599cb6
Merge remote-tracking branch 'origin/main' into refactor/2318/migrate…
shaman-apprentice Sep 3, 2022
fe3f419
Merge remote-tracking branch 'origin/main' into refactor/2318/migrate…
shaman-apprentice Sep 11, 2022
48735be
Merge remote-tracking branch 'origin/main' into refactor/2318/migrate…
Sep 13, 2022
7e58527
refactor: extract own center effect
Sep 13, 2022
1da0675
refactor: adjust old tests
Sep 13, 2022
605477c
refactor: add tests for renderCodeMapEffect
Sep 13, 2022
a60de66
docs: add changelog
Sep 13, 2022
9f92eed
test: add tests for autoFitCodeMapOnFileSelectionChangeEffect
Sep 13, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/)

## [unreleased] (Added 🚀 | Changed | Removed 🗑 | Fixed 🐞 | Chore 👨‍💻 👩‍💻)

## Fixed 🐞

- Show loading spinners while loading a file [#2980](https://github.com/MaibornWolff/codecharta/pull/2980)

## Chore 👨‍💻 👩‍💻

- Throttle rendering and migrate codeMap.preRender.service to Angular [#2980](https://github.com/MaibornWolff/codecharta/pull/2980)

## [1.106.0] - 2022-09-13

### Added 🚀
Expand Down
6 changes: 5 additions & 1 deletion visualization/app/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ import { ColorSettingsPanelModule } from "./codeCharta/ui/ribbonBar/colorSetting
import { SplitStateActionsEffect } from "./codeCharta/state/effects/splitStateActionsEffect/splitStateActions.effect"
import { ViewCubeModule } from "./codeCharta/ui/viewCube/viewCube.module"
import { ToolBarModule } from "./codeCharta/ui/toolBar/toolBar.module"
import { RenderCodeMapEffect } from "./codeCharta/state/effects/renderCodeMapEffect/renderCodeMap.effect"
import { AutoFitCodeMapOnFileSelectionChangeEffect } from "./codeCharta/state/effects/autoFitCodeMapOnFileSelectionChange/autoFitCodeMapOnFileSelectionChange.effect"

@NgModule({
imports: [
Expand All @@ -64,7 +66,9 @@ import { ToolBarModule } from "./codeCharta/ui/toolBar/toolBar.module"
ResetColorRangeEffect,
ResetDynamicMarginEffect,
ResetChosenMetricsEffect,
UpdateEdgePreviewsEffect
UpdateEdgePreviewsEffect,
RenderCodeMapEffect,
AutoFitCodeMapOnFileSelectionChangeEffect
]),
SliderModule,
AttributeSideBarModule,
Expand Down
20 changes: 15 additions & 5 deletions visualization/app/codeCharta/codeCharta.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ describe("codeChartaService", () => {
const invalidFileContent = klona(validFileContent)
invalidFileContent.apiVersion = ""

await codeChartaService.loadFiles([{ fileName: "FirstFile", content: invalidFileContent, fileSize: 42 }])
await expect(async () =>
codeChartaService.loadFiles([{ fileName: "FirstFile", content: invalidFileContent, fileSize: 42 }])
).rejects.toThrow("No files could be uploaded")

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

Expand Down Expand Up @@ -269,7 +271,9 @@ describe("codeChartaService", () => {
}
]

await codeChartaService.loadFiles([{ fileName, content: null, fileSize: 0 }])
await expect(async () => codeChartaService.loadFiles([{ fileName, content: null, fileSize: 0 }])).rejects.toThrow(
"No files could be uploaded"
)

expect(storeService.getState().files).toHaveLength(0)
expect(dialogService.showValidationDialog).toHaveBeenCalledWith(expectedError)
Expand All @@ -284,7 +288,9 @@ describe("codeChartaService", () => {
}
]

await codeChartaService.loadFiles([{ fileName, fileSize: 42, content: "string" as unknown as ExportCCFile }])
expect(async () =>
codeChartaService.loadFiles([{ fileName, fileSize: 42, content: "string" as unknown as ExportCCFile }])
).rejects.toThrow("No files could be uploaded")

expect(storeService.getState().files).toHaveLength(0)
expect(dialogService.showValidationDialog).toHaveBeenCalledWith(expectedError)
Expand All @@ -301,7 +307,9 @@ describe("codeChartaService", () => {

const invalidFileContent = validFileContent
delete invalidFileContent.projectName
await codeChartaService.loadFiles([{ fileName, fileSize: 42, content: invalidFileContent }])
await expect(async () =>
codeChartaService.loadFiles([{ fileName, fileSize: 42, content: invalidFileContent }])
).rejects.toThrow("No files could be uploaded")

expect(storeService.getState().files).toHaveLength(0)
expect(dialogService.showValidationDialog).toHaveBeenCalledWith(expectedError)
Expand Down Expand Up @@ -343,7 +351,9 @@ describe("codeChartaService", () => {
}
]

await codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])
expect(async () => codeChartaService.loadFiles([{ fileName, content: validFileContent, fileSize: 42 }])).rejects.toThrow(
"No files could be uploaded"
)
shaman-apprentice marked this conversation as resolved.
Show resolved Hide resolved

expect(dialogService.showValidationDialog).toHaveBeenCalledWith(expectedError)
})
Expand Down
8 changes: 5 additions & 3 deletions visualization/app/codeCharta/codeCharta.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { NodeDecorator } from "./util/nodeDecorator"
import { StoreService } from "./state/store.service"
import { setFiles, setStandardByNames } from "./state/store/files/files.actions"
import { DialogService } from "./ui/dialog/dialog.service"
import { setIsLoadingFile } from "./state/store/appSettings/isLoadingFile/isLoadingFile.actions"
import { FileSelectionState, FileState } from "./model/files/files"
import { getCCFile } from "./util/fileHelper"
import { NameDataPair } from "./codeCharta.model"
Expand All @@ -22,8 +21,11 @@ export class CodeChartaService {
}
})

static instance: CodeChartaService

constructor(private storeService: StoreService, private dialogService: DialogService) {
"ngInject"
CodeChartaService.instance = this
}

async loadFiles(nameDataPairs: NameDataPair[]) {
Expand All @@ -32,8 +34,6 @@ export class CodeChartaService {

this.getValidationResults(nameDataPairs, fileValidationResults)

this.storeService.dispatch(setIsLoadingFile(false))

if (fileValidationResults.length > 0) {
await this.dialogService.showValidationDialog(fileValidationResults)
}
Expand All @@ -49,6 +49,8 @@ export class CodeChartaService {

this.fileStates = []
this.recentFiles = []
} else {
throw new Error("No files could be uploaded")
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { ApplicationInitStatus } from "@angular/core"
import { TestBed } from "@angular/core/testing"
import { Subject } from "rxjs"
import { mocked } from "ts-jest/utils"
import { ThreeOrbitControlsService } from "../../../ui/codeMap/threeViewer/threeOrbitControlsService"
import { EffectsModule } from "../../angular-redux/effects/effects.module"
import { Store } from "../../angular-redux/store"
import { visibleFileStatesSelector } from "../../selectors/visibleFileStates.selector"
import { resetCameraIfNewFileIsLoadedSelector } from "../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector"
import { RenderCodeMapEffect } from "../renderCodeMapEffect/renderCodeMap.effect"
import { AutoFitCodeMapOnFileSelectionChangeEffect } from "./autoFitCodeMapOnFileSelectionChange.effect"

jest.mock("../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector", () => ({
resetCameraIfNewFileIsLoadedSelector: jest.fn()
}))
const mockedResetCameraIfNewFileIsLoadedSelector = mocked(resetCameraIfNewFileIsLoadedSelector)

describe("autoFitCodeMapOnFileSelectionChangeEffect", () => {
const mockedRenderCodeMap$ = new Subject()
const mockedVisibleFileStates$ = new Subject()
const mockedStore = {
select: (selector: unknown) => {
switch (selector) {
case visibleFileStatesSelector:
return mockedVisibleFileStates$
default:
throw new Error("selector is not mocked")
}
},
dispatch: jest.fn()
}

beforeEach(async () => {
ThreeOrbitControlsService.instance = { autoFitTo: jest.fn() } as unknown as ThreeOrbitControlsService

TestBed.configureTestingModule({
imports: [EffectsModule.forRoot([AutoFitCodeMapOnFileSelectionChangeEffect])],
providers: [
{ provide: RenderCodeMapEffect, useValue: { renderCodeMap$: mockedRenderCodeMap$ } },
{ provide: Store, useValue: mockedStore }
]
})
await TestBed.inject(ApplicationInitStatus).donePromise
})

it("should auto fit map after first render after selected files changed", () => {
mockedResetCameraIfNewFileIsLoadedSelector.mockImplementation(() => true)
mockedVisibleFileStates$.next("")
mockedVisibleFileStates$.next("")
expect(ThreeOrbitControlsService.instance.autoFitTo).not.toHaveBeenCalled()

mockedRenderCodeMap$.next("")
expect(ThreeOrbitControlsService.instance.autoFitTo).toHaveBeenCalledTimes(1)

mockedRenderCodeMap$.next("")
expect(ThreeOrbitControlsService.instance.autoFitTo).toHaveBeenCalledTimes(1)
})

it("should skip first change", () => {
mockedResetCameraIfNewFileIsLoadedSelector.mockImplementation(() => true)
mockedVisibleFileStates$.next("")
mockedRenderCodeMap$.next("")
expect(ThreeOrbitControlsService.instance.autoFitTo).toHaveBeenCalledTimes(1)
})

it("should do nothing when reset camera if new file is loaded is deactivated", () => {
mockedResetCameraIfNewFileIsLoadedSelector.mockImplementation(() => false)
mockedVisibleFileStates$.next("")
mockedVisibleFileStates$.next("")
mockedRenderCodeMap$.next("")
expect(ThreeOrbitControlsService.instance.autoFitTo).not.toHaveBeenCalled()
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { Inject, Injectable } from "@angular/core"
import { concatMap, filter, skip, take, tap } from "rxjs"
import { ThreeOrbitControlsService } from "../../../ui/codeMap/threeViewer/threeOrbitControlsService"
import { createEffect } from "../../angular-redux/effects/createEffect"
import { State } from "../../angular-redux/state"
import { Store } from "../../angular-redux/store"
import { visibleFileStatesSelector } from "../../selectors/visibleFileStates.selector"
import { resetCameraIfNewFileIsLoadedSelector } from "../../store/appSettings/resetCameraIfNewFileIsLoaded/resetCameraIfNewFileIsLoaded.selector"
import { RenderCodeMapEffect } from "../renderCodeMapEffect/renderCodeMap.effect"

// don't inject AngularJS services, as AngularJS is not yet bootstrapped when Effects are bootstrapped
@Injectable()
export class AutoFitCodeMapOnFileSelectionChangeEffect {
constructor(
@Inject(Store) private store: Store,
@Inject(State) private state: State,
@Inject(RenderCodeMapEffect) private renderCodeMapEffect: RenderCodeMapEffect
) {}

autoFitCodeMapOnFileSelectionChange$ = createEffect(
() =>
this.store.select(visibleFileStatesSelector).pipe(
skip(1), // initial map load is already fitted
filter(() => resetCameraIfNewFileIsLoadedSelector(this.state.getValue())),
concatMap(() => this.renderCodeMapEffect.renderCodeMap$.pipe(take(1))),
tap(() => {
ThreeOrbitControlsService.instance?.autoFitTo()
})
),
{ dispatch: false }
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { AmountOfEdgePreviewsActions } from "../../store/appSettings/amountOfEdgePreviews/amountOfEdgePreviews.actions"
import { AmountOfTopLabelsActions } from "../../store/appSettings/amountOfTopLabels/amountOfTopLabels.actions"
import { ColorLabelsActions } from "../../store/appSettings/colorLabels/colorLabels.actions"
import { EdgeHeightActions } from "../../store/appSettings/edgeHeight/edgeHeight.actions"
import { HideFlatBuildingsActions } from "../../store/appSettings/hideFlatBuildings/hideFlatBuildings.actions"
import { InvertAreaActions } from "../../store/appSettings/invertArea/invertArea.actions"
import { InvertHeightActions } from "../../store/appSettings/invertHeight/invertHeight.actions"
import { IsWhiteBackgroundActions } from "../../store/appSettings/isWhiteBackground/isWhiteBackground.actions"
import { LayoutAlgorithmActions } from "../../store/appSettings/layoutAlgorithm/layoutAlgorithm.actions"
import { MapColorsActions } from "../../store/appSettings/mapColors/mapColors.actions"
import { MaxTreeMapFilesActions } from "../../store/appSettings/maxTreeMapFiles/maxTreeMapFiles.actions"
import { ScalingActions } from "../../store/appSettings/scaling/scaling.actions"
import { SharpnessModeActions } from "../../store/appSettings/sharpnessMode/sharpnessMode.actions"
import { ShowMetricLabelNameValueActions } from "../../store/appSettings/showMetricLabelNameValue/showMetricLabelNameValue.actions"
import { ShowMetricLabelNodeNameActions } from "../../store/appSettings/showMetricLabelNodeName/showMetricLabelNodeName.actions"
import { ShowOnlyBuildingsWithEdgesActions } from "../../store/appSettings/showOnlyBuildingsWithEdges/showOnlyBuildingsWithEdges.actions"
import { AreaMetricActions } from "../../store/dynamicSettings/areaMetric/areaMetric.actions"
import { ColorMetricActions } from "../../store/dynamicSettings/colorMetric/colorMetric.actions"
import { ColorModeActions } from "../../store/dynamicSettings/colorMode/colorMode.actions"
import { ColorRangeActions } from "../../store/dynamicSettings/colorRange/colorRange.actions"
import { EdgeMetricActions } from "../../store/dynamicSettings/edgeMetric/edgeMetric.actions"
import { FocusedNodePathActions } from "../../store/dynamicSettings/focusedNodePath/focusedNodePath.actions"
import { HeightMetricActions } from "../../store/dynamicSettings/heightMetric/heightMetric.actions"
import { MarginActions } from "../../store/dynamicSettings/margin/margin.actions"
import { SearchPatternActions } from "../../store/dynamicSettings/searchPattern/searchPattern.actions"

export const actionsRequiringRerender = [
ColorLabelsActions,
MapColorsActions,
ShowMetricLabelNodeNameActions,
ShowMetricLabelNameValueActions,
IsWhiteBackgroundActions,
InvertAreaActions,
InvertHeightActions,
HideFlatBuildingsActions,
ScalingActions,
EdgeHeightActions,
AmountOfEdgePreviewsActions,
AmountOfTopLabelsActions,
LayoutAlgorithmActions,
MaxTreeMapFilesActions,
SharpnessModeActions,
ColorModeActions,
EdgeMetricActions,
ColorRangeActions,
MarginActions,
SearchPatternActions,
FocusedNodePathActions,
HeightMetricActions,
AreaMetricActions,
ColorMetricActions,
ShowOnlyBuildingsWithEdgesActions
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { ApplicationInitStatus } from "@angular/core"
import { TestBed } from "@angular/core/testing"
import { Action } from "redux"
import { BehaviorSubject, Subject } from "rxjs"
import { Vector3 } from "three"
import { CodeMapRenderService } from "../../../ui/codeMap/codeMap.render.service"
import { ThreeRendererService } from "../../../ui/codeMap/threeViewer/threeRendererService"
import { UploadFilesService } from "../../../ui/toolBar/uploadFilesButton/uploadFiles.service"
import { wait } from "../../../util/testUtils/wait"
import { EffectsModule } from "../../angular-redux/effects/effects.module"
import { Store } from "../../angular-redux/store"
import { accumulatedDataSelector } from "../../selectors/accumulatedData/accumulatedData.selector"
import { setInvertArea } from "../../store/appSettings/invertArea/invertArea.actions"
import { setIsLoadingFile } from "../../store/appSettings/isLoadingFile/isLoadingFile.actions"
import { setIsLoadingMap } from "../../store/appSettings/isLoadingMap/isLoadingMap.actions"
import { setScaling } from "../../store/appSettings/scaling/scaling.actions"
import { maxFPS, RenderCodeMapEffect } from "./renderCodeMap.effect"

describe("renderCodeMapEffect", () => {
let mockedStore

beforeEach(async () => {
mockedStore = {
select: (selector: unknown) => {
switch (selector) {
case accumulatedDataSelector:
return new BehaviorSubject({ unifiedMapNode: {} })
default:
throw new Error("selector is not mocked")
}
},
dispatch: jest.fn()
}

CodeMapRenderService.instance = { render: jest.fn(), scaleMap: jest.fn() } as unknown as CodeMapRenderService
ThreeRendererService.instance = { render: jest.fn() } as unknown as ThreeRendererService
EffectsModule.actions$ = new Subject<Action>()

TestBed.configureTestingModule({
imports: [EffectsModule.forRoot([RenderCodeMapEffect])],
providers: [
{ provide: Store, useValue: mockedStore },
{ provide: UploadFilesService, useValue: { isUploading: false } }
]
})
await TestBed.inject(ApplicationInitStatus).donePromise
})

afterEach(() => {
EffectsModule.actions$.complete()
})

it("should render once throttled after actions requiring rerender, but not scale map", async () => {
EffectsModule.actions$.next(setInvertArea(true))
EffectsModule.actions$.next(setInvertArea(true))
expect(CodeMapRenderService.instance.render).toHaveBeenCalledTimes(0)
expect(ThreeRendererService.instance.render).toHaveBeenCalledTimes(0)

await wait(maxFPS)
expect(CodeMapRenderService.instance.render).toHaveBeenCalledTimes(1)
expect(ThreeRendererService.instance.render).toHaveBeenCalledTimes(1)
expect(CodeMapRenderService.instance.scaleMap).not.toHaveBeenCalled()
})

it("should scale map when scaling changes", async () => {
EffectsModule.actions$.next(setScaling(new Vector3(1, 1, 1)))
await wait(maxFPS)
expect(CodeMapRenderService.instance.scaleMap).toHaveBeenCalledTimes(1)
})

it("should remove loading indicators after render", async () => {
EffectsModule.actions$.next(setInvertArea(true))
await wait(maxFPS)
expect(mockedStore.dispatch).toHaveBeenCalledWith(setIsLoadingFile(false))
expect(mockedStore.dispatch).toHaveBeenCalledWith(setIsLoadingMap(false))
})

it("should not remove loading indicators after render when a file is still being uploaded", async () => {
const uploadFileService = TestBed.inject(UploadFilesService)
uploadFileService.isUploading = true
EffectsModule.actions$.next(setInvertArea(true))
await wait(maxFPS)
expect(mockedStore.dispatch).not.toHaveBeenCalledWith(setIsLoadingFile(false))
expect(mockedStore.dispatch).not.toHaveBeenCalledWith(setIsLoadingMap(false))
})
})