From 66ddadc6ebe832ff0aea9ca2b40d059b7d7dcc56 Mon Sep 17 00:00:00 2001 From: Naomi Gassler Date: Wed, 8 Jul 2026 15:30:57 +0200 Subject: [PATCH] Fix infinite route-loader spinner from per-navigation root cache invalidation BrowserInitService invalidated the root /server/api endpoint cache on every NavigationStart. That marks the root request stale (RootDataService .invalidateRootCache -> RequestService.setStaleByHref). HALEndpointService .getEndpointMapAt, used by getEndpoint() for every data request, discards a stale root via filter(rd => !rd.isStale), and its re-fetch can lose the race with the next invalidation, so getEndpoint() never resolves, the route resolver never completes, and the route loader spins forever. The root endpoint map is static between navigations, so invalidating it on every navigation is unnecessary. Remove the per-NavigationStart invalidation and keep the one-time invalidation at app init; backend availability is still established there and surfaces through normal request failures. Fixes #3584, #3697. --- src/modules/app/browser-init.service.spec.ts | 44 ++++++++++++++++++++ src/modules/app/browser-init.service.ts | 26 +++++------- 2 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 src/modules/app/browser-init.service.spec.ts diff --git a/src/modules/app/browser-init.service.spec.ts b/src/modules/app/browser-init.service.spec.ts new file mode 100644 index 00000000000..69487f2e310 --- /dev/null +++ b/src/modules/app/browser-init.service.spec.ts @@ -0,0 +1,44 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +import { NavigationStart } from '@angular/router'; +import { Subject } from 'rxjs'; + +import { BrowserInitService } from './browser-init.service'; + +describe('BrowserInitService', () => { + describe('listenForRouteChanges', () => { + let service: BrowserInitService; + let rootDataServiceSpy; + let routerEvents$: Subject; + + beforeEach(() => { + rootDataServiceSpy = jasmine.createSpyObj('rootDataService', ['invalidateRootCache']); + routerEvents$ = new Subject(); + + service = new BrowserInitService( + null, null, null, null, null, null, null, null, null, null, null, null, null, null, + rootDataServiceSpy, + { events: routerEvents$.asObservable() } as any, + null, null, null, null, + ); + + (service as any).listenForRouteChanges(); + }); + + it('should invalidate the root endpoint cache once, at init', () => { + expect(rootDataServiceSpy.invalidateRootCache).toHaveBeenCalledTimes(1); + }); + + it('should not invalidate the root endpoint cache again on subsequent NavigationStart events', () => { + routerEvents$.next(new NavigationStart(1, '/some-route')); + routerEvents$.next(new NavigationStart(2, '/another-route')); + + expect(rootDataServiceSpy.invalidateRootCache).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/modules/app/browser-init.service.ts b/src/modules/app/browser-init.service.ts index 524229d8bb5..dff19c820d7 100644 --- a/src/modules/app/browser-init.service.ts +++ b/src/modules/app/browser-init.service.ts @@ -10,10 +10,7 @@ import { Injectable, TransferState, } from '@angular/core'; -import { - NavigationStart, - Router, -} from '@angular/router'; +import { Router } from '@angular/router'; import { APP_CONFIG, APP_CONFIG_STATE, @@ -226,21 +223,18 @@ export class BrowserInitService extends InitService { } /** - * Listen to all router events. Every time a new navigation starts, invalidate the cache - * for the root endpoint. That way we retrieve it once per routing operation to ensure the - * backend is not down. But if the guard is called multiple times during the same routing - * operation, the cached version is used. + * Invalidate the cache for the root endpoint once, at startup, so the first request for it + * is guaranteed to hit the backend rather than serve a value cached before the app booted. + * + * This used to also run on every `NavigationStart`, but the root endpoint map does not change + * between navigations, so that repeated invalidation was unnecessary. Worse, it could mark the + * root request stale while {@link HALEndpointService} was still resolving it for an in-flight + * data request; the stale value is filtered out of the endpoint map and the retry for it can + * lose the race with the next invalidation, so `getEndpoint()` never resolves and the route + * that depends on it never finishes loading. */ protected listenForRouteChanges(): void { - // we'll always be too late for the first NavigationStart event with the router subscribe below, - // so this statement is for the very first route operation. this.rootDataService.invalidateRootCache(); - - this.router.events.pipe( - filter(event => event instanceof NavigationStart), - ).subscribe(() => { - this.rootDataService.invalidateRootCache(); - }); } }