RoyDefined / WebDoomer

WebDoomer is a fast and efficient Zandronum and QZandronum server browser as a web app.
GNU General Public License v3.0
4 stars 0 forks source link

Fetching servers with `concatMap` only fetches the initial list when `baseUrl` is fetched by an observable #3

Open RoyDefined opened 7 months ago

RoyDefined commented 7 months ago

Describe the bug The issue is that this concatMap will only trigger once when baseUrl is fetched using an observable: https://github.com/RoyDefined/WebDoomer/blob/09e2beb63d585c1cee263d337416b754a138851f/src/WebDoomerClient/src/app/stores/servers/servers.store.ts#L67

The issue is related to this part of the code: https://github.com/RoyDefined/WebDoomer/blob/09e2beb63d585c1cee263d337416b754a138851f/src/WebDoomerClient/src/app/services/api/servers-api.service.ts#L46

Right now the store fetches the app settings before application load, and therefore can just provide the variable without the observable pattern as it is guaranteed to exist. https://github.com/RoyDefined/WebDoomer/blob/09e2beb63d585c1cee263d337416b754a138851f/src/WebDoomerClient/src/app/stores/appsettings/app-settings.store.ts#L18

Before the refactor the store would look like this:

@Injectable({
    providedIn: 'root',
})
export class AppSettingsStore extends ComponentStore<AppSettingsStoreState> {
    private readonly _item$ = this.select((state) => state.item);
    private readonly _loading$ = this.select((state) => state.loading);
    private readonly _error$ = this.select((state) => state.error);

    private readonly _appSettingsFileFetch = this._http.get('assets/appsettings.json', { observe: 'response', responseType: 'json' });

    public readonly vm$ = this.select({
        item: this._item$,
        loading: this._loading$,
        error: this._error$,
    });

    public readonly fetchAppSettingsFile = this.effect((trigger$) =>
        trigger$.pipe(
            tap(() => this.setLoading(true)),
            combineLatestWith(this._appSettingsFileFetch),
            map((args) => {
                const response = args[1];

                if (!response.ok || !response.body) {
                    this.setError(new Error('The application settings file could not be fetched correctly.'));
                    return;
                }

                const parseResult = appSettingsSchema.safeParse(response.body);
                if (!parseResult.success) {
                    this.setError(new Error('The application settings file could not be parsed correctly.'));
                    return;
                }

                this.setItem(parseResult.data);
                console.log('Application settings loaded.', parseResult.data);
            }),
            catchError((error) => {
                this.setError(error);
                return EMPTY;
            }),
            finalize(() => this.setLoading(false)),
            take(1),
        ),
    );

    private readonly setItem = this.updater((state, item: z.infer<typeof appSettingsSchema>) => {
        return { ...state, item };
    });
    private readonly setLoading = this.updater((state, loading: boolean) => {
        return { ...state, loading };
    });
    private readonly setError = this.updater((state, error: Error) => {
        return { ...state, error };
    });

    constructor(private readonly _http: HttpClient) {
        super({
            item: null,
            loading: false,
            error: null,
        });
    }
}

The main server store would then fetch the base url by combining pipe and switchmap to get the base url for the observable request:

public GetServersPaginated(skip: number, take: number) {
        return this._baseUrl$.pipe(
            switchMap((baseUrl) =>
                this._http.get(baseUrl + `...`, { observe: 'response' }).pipe(
                ...

Finally, updateListedServersByRangeAndDirection, which contains the concatMap, would not fire a second time causing the bug.

Steps to reproduce N/A

Expected behavior Expected is that the servers would fetch more than once when the base url is returned as an observable.

Screenshots N/A

Additional context This report is generally to find out why this was caused in the first place, and is not something that is blocking the application in any way. It would be nice to know the reason for this issue in case it does suddenly happen to something that can't be fixed as easily.