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 to use StoreService instead of Store #677

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .eslint/typescript.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ module.exports = {
message:
'You probably want to import this from lodash-es instead.',
},
{
name: '@ngrx/store',
importNames: ['Store'],
message:
'You probably want to use the StoreService instead.',
},
],
patterns: [
{
Expand Down
4 changes: 3 additions & 1 deletion frontend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ In `src/app` and every descending folder, the following guidelines apply:

Commonly used [selectors](https://ngrx.io/guide/store/selectors) should go in [./src/app/state/application/selectors](./src/app/state/application/selectors/).

You can assume that the Store has the current exercise state (either of a live exercise or an exercise in time travel) if you are in `src/app/pages/exercises/exercise`. We use [route guards](https://angular.io/guide/router-tutorial-toh#canactivate-requiring-authentication) for this.
Do not access the ngrx-store directly, use the [StoreService](./src/app/core/store.service.ts) instead.

You can assume that the store has the current exercise state (either of a live exercise or an exercise in time travel) if you are in `src/app/pages/exercises/exercise`. We use [route guards](https://angular.io/guide/router-tutorial-toh#canactivate-requiring-authentication) for this.

If you want to modify the exercise state, do not do it via [reducers](https://ngrx.io/guide/store/reducers) in the store, but propose an action (optimistically) via the [ExerciseService](./src/app/core/exercise.service.ts). The action will automatically be applied to the store.

Expand Down
1 change: 1 addition & 0 deletions frontend/src/app/app.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Component } from '@angular/core';
// eslint-disable-next-line no-restricted-imports
import { Store } from '@ngrx/store';
import { httpOrigin, websocketOrigin } from './core/api-origins';
import { setupCypressTestingValues } from './shared/functions/cypress';
Expand Down
8 changes: 3 additions & 5 deletions frontend/src/app/core/api.service.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
import { HttpClient } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { Store } from '@ngrx/store';
import type {
ExerciseIds,
ExerciseTimeline,
StateExport,
} from 'digital-fuesim-manv-shared';
import { freeze } from 'immer';
import { lastValueFrom } from 'rxjs';
import type { AppState } from '../state/app.state';
import { selectExerciseId } from '../state/application/selectors/application.selectors';
import { selectStateSnapshot } from '../state/get-state-snapshot';
import { httpOrigin } from './api-origins';
import { MessageService } from './messages/message.service';
import { StoreService } from './store.service';

@Injectable({
providedIn: 'root',
})
export class ApiService {
constructor(
private readonly store: Store<AppState>,
private readonly storeService: StoreService,
private readonly messageService: MessageService,
private readonly httpClient: HttpClient
) {}
Expand All @@ -40,7 +38,7 @@ export class ApiService {
}

public async exerciseHistory() {
const exerciseId = selectStateSnapshot(selectExerciseId, this.store);
const exerciseId = this.storeService.select(selectExerciseId);
return lastValueFrom(
this.httpClient.get<ExerciseTimeline>(
`${httpOrigin}/api/exercise/${exerciseId}/history`
Expand Down
15 changes: 6 additions & 9 deletions frontend/src/app/core/application.service.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { Injectable } from '@angular/core';
import { Store } from '@ngrx/store';
import { assertExhaustiveness } from 'digital-fuesim-manv-shared';
import type { AppState } from '../state/app.state';
import {
selectExerciseId,
selectExerciseStateMode,
selectLastClientName,
} from '../state/application/selectors/application.selectors';
import { selectStateSnapshot } from '../state/get-state-snapshot';
import { ExerciseService } from './exercise.service';
import { StoreService } from './store.service';
import { TimeTravelService } from './time-travel.service';

/**
Expand All @@ -22,16 +20,15 @@ export class ApplicationService {
constructor(
private readonly timeTravelService: TimeTravelService,
private readonly exerciseService: ExerciseService,
private readonly store: Store<AppState>
private readonly storeService: StoreService
) {}

/**
* A new mode must be set immediately after this function is called
*/
private stopCurrentMode() {
const currentExerciseStateMode = selectStateSnapshot(
selectExerciseStateMode,
this.store
const currentExerciseStateMode = this.storeService.select(
selectExerciseStateMode
);
switch (currentExerciseStateMode) {
case 'exercise':
Expand Down Expand Up @@ -63,8 +60,8 @@ export class ApplicationService {
*/
public async rejoinExercise() {
return this.joinExercise(
selectStateSnapshot(selectExerciseId, this.store)!,
selectStateSnapshot(selectLastClientName, this.store)!
this.storeService.select(selectExerciseId)!,
this.storeService.select(selectLastClientName)!
);
}

Expand Down
37 changes: 19 additions & 18 deletions frontend/src/app/core/exercise.service.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Injectable } from '@angular/core';
import { Store } from '@ngrx/store';
import type {
ClientToServerEvents,
ExerciseAction,
Expand All @@ -21,7 +20,6 @@ import {
import type { Socket } from 'socket.io-client';
import { io } from 'socket.io-client';
import { handleChanges } from '../shared/functions/handle-changes';
import type { AppState } from '../state/app.state';
import {
createApplyServerActionAction,
createJoinExerciseAction,
Expand All @@ -38,10 +36,10 @@ import {
selectOwnClient,
selectVisibleVehicles,
} from '../state/application/selectors/shared.selectors';
import { selectStateSnapshot } from '../state/get-state-snapshot';
import { websocketOrigin } from './api-origins';
import { MessageService } from './messages/message.service';
import { OptimisticActionHandler } from './optimistic-action-handler';
import { StoreService } from './store.service';

/**
* This Service deals with the state synchronization of a live exercise.
Expand All @@ -68,7 +66,7 @@ export class ExerciseService {
>;

constructor(
private readonly store: Store<AppState>,
private readonly storeService: StoreService,
private readonly messageService: MessageService
) {
this.socket.on('performAction', (action: ExerciseAction) => {
Expand Down Expand Up @@ -138,7 +136,7 @@ export class ExerciseService {
});
return false;
}
this.store.dispatch(
this.storeService.dispatch(
createJoinExerciseAction(
joinResponse.payload,
getStateResponse.payload,
Expand All @@ -153,10 +151,14 @@ export class ExerciseService {
SocketResponse
>(
(exercise) =>
this.store.dispatch(createSetExerciseStateAction(exercise)),
() => selectStateSnapshot(selectExerciseState, this.store),
this.storeService.dispatch(
createSetExerciseStateAction(exercise)
),
() => this.storeService.select(selectExerciseState),
(action) =>
this.store.dispatch(createApplyServerActionAction(action)),
this.storeService.dispatch(
createApplyServerActionAction(action)
),
async (action) => {
const response = await new Promise<SocketResponse>(
(resolve) => {
Expand Down Expand Up @@ -190,7 +192,7 @@ export class ExerciseService {
this.socket.disconnect();
this.stopNotifications();
this.optimisticActionHandler = undefined;
this.store.dispatch(createLeaveExerciseAction());
this.storeService.dispatch(createLeaveExerciseAction());
}

/**
Expand All @@ -200,8 +202,7 @@ export class ExerciseService {
*/
public async proposeAction(action: ExerciseAction, optimistic = false) {
if (
selectStateSnapshot(selectExerciseStateMode, this.store) !==
'exercise' ||
this.storeService.select(selectExerciseStateMode) !== 'exercise' ||
this.optimisticActionHandler === undefined
) {
// Especially during timeTravel, buttons that propose actions are only deactivated via best effort
Expand All @@ -220,11 +221,11 @@ export class ExerciseService {

private startNotifications() {
// If the user is a trainer, display a message for each joined or disconnected client
this.store
.select(selectCurrentRole)
this.storeService
.select$(selectCurrentRole)
.pipe(
filter((role) => role === 'trainer'),
switchMap(() => this.store.select(selectClients)),
switchMap(() => this.storeService.select$(selectClients)),
pairwise(),
takeUntil(this.stopNotifications$)
)
Expand All @@ -249,17 +250,17 @@ export class ExerciseService {
});
});
// If the user is restricted to a viewport, display a message for each vehicle that arrived at this viewport
this.store
.select(selectOwnClient)
this.storeService
.select$(selectOwnClient)
.pipe(
filter(
(client) =>
client?.viewRestrictedToViewportId !== undefined &&
!client.isInWaitingRoom
),
switchMap((client) =>
this.store
.select(selectVisibleVehicles)
this.storeService
.select$(selectVisibleVehicles)
// pipe in here so no pairs of events from different viewports are built
// Do not trigger the message if the vehicle was removed and added again at the same time
.pipe(debounceTime(0), pairwise())
Expand Down
55 changes: 55 additions & 0 deletions frontend/src/app/core/store.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { Injectable } from '@angular/core';
import type { Action, Selector } from '@ngrx/store';
// eslint-disable-next-line no-restricted-imports
import { Store } from '@ngrx/store';
import type { Observable } from 'rxjs';
import { first } from 'rxjs';
import type { AppState } from '../state/app.state';

/**
* This service is a wrapper around the ngrx-store.
* It changes the API to be more convenient to use for this concrete application.
* The normal ngrx-store shouldn't be used directly. Instead, always use this service.
*/
@Injectable({
providedIn: 'root',
})
export class StoreService {
constructor(private readonly store: Store<AppState>) {}

/**
* @returns An observable that on subscription emits the selected part of the current state
* and then emits the new state each time it changes.
*/
public select$<T>(selector: Selector<AppState, T>): Observable<T> {
return this.store.select(selector);
}

/**
* If you want to observe the state and get notified of changes,
* you should use {@link select$} on the store.
*
* @returns a synchronous snapshot of the selected part of the state.
*/
public select<T>(selector: Selector<AppState, T>): T {
// There is sadly currently no other way to get the state synchronously...
let result: T;
// "Subscribing to Store will always be guaranteed to be synchronous" - https://github.com/ngrx/platform/issues/227#issuecomment-431682349
this.store
.select(selector)
.pipe(first())
.subscribe((s) => (result = s));
return result!;
}

/**
* Dispatches an action to the ngrx-store.
*
* Usually you want to propose an action via the {@link ExerciseService} to the backend instead.
*/
// TODO: The action should be a literal union of all our actions.
// NgRx doesn't support this yet, so we will have to get creative ourselves.
public dispatch(action: Action): void {
this.store.dispatch(action);
}
}
19 changes: 7 additions & 12 deletions frontend/src/app/core/time-travel.service.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import { HttpClient } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { Store } from '@ngrx/store';
import type { ExerciseTimeline } from 'digital-fuesim-manv-shared';
import { freeze } from 'immer';
import { lastValueFrom } from 'rxjs';
import type { AppState } from '../state/app.state';
import {
createJumpToTimeAction,
createStartTimeTravelAction,
Expand All @@ -14,9 +12,9 @@ import {
selectTimeConstraints,
} from '../state/application/selectors/application.selectors';
import { selectCurrentTime } from '../state/application/selectors/exercise.selectors';
import { selectStateSnapshot } from '../state/get-state-snapshot';
import { httpOrigin } from './api-origins';
import { MessageService } from './messages/message.service';
import { StoreService } from './store.service';
import { TimeJumpHelper } from './time-jump-helper';

/**
Expand All @@ -35,7 +33,7 @@ export class TimeTravelService {
private activatingTimeTravel = false;

constructor(
private readonly store: Store<AppState>,
private readonly storeService: StoreService,
private readonly httpClient: HttpClient,
private readonly messageService: MessageService
) {}
Expand All @@ -45,7 +43,7 @@ export class TimeTravelService {
*/
public async startTimeTravel() {
this.activatingTimeTravel = true;
const exerciseId = selectStateSnapshot(selectExerciseId, this.store);
const exerciseId = this.storeService.select(selectExerciseId);
const exerciseTimeLine = await lastValueFrom(
this.httpClient.get<ExerciseTimeline>(
`${httpOrigin}/api/exercise/${exerciseId}/history`
Expand All @@ -68,8 +66,8 @@ export class TimeTravelService {
this.timeJumpHelper = new TimeJumpHelper(exerciseTimeLine);
// Travel to the start of the exercise
// TODO: This should be calculated from the timeline (in the TimeJumpHelper - maybe cache states in between)
const endTime = selectStateSnapshot(selectCurrentTime, this.store);
this.store.dispatch(
const endTime = this.storeService.select(selectCurrentTime);
this.storeService.dispatch(
createStartTimeTravelAction(exerciseTimeLine.initialState, endTime)
);
}
Expand All @@ -79,18 +77,15 @@ export class TimeTravelService {
* @param exerciseTime The time to travel to, if it isn't in the timeConstraints, it will be clamped appropriately
*/
public jumpToTime(exerciseTime: number) {
const timeConstraints = selectStateSnapshot(
selectTimeConstraints,
this.store
);
const timeConstraints = this.storeService.select(selectTimeConstraints);
if (!timeConstraints || !this.timeJumpHelper) {
throw new Error('Start the time travel before jumping to a time!');
}
const clampedTime = Math.max(
timeConstraints.start,
Math.min(timeConstraints.end, exerciseTime)
);
this.store.dispatch(
this.storeService.dispatch(
createJumpToTimeAction(
clampedTime,
this.timeJumpHelper.getStateAtTime(clampedTime)
Expand Down
Loading