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.

Author: naomigassler

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<any>;
+
+    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);
+    });
+  });
+});

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();
-    });
   }
 
 }