From 38752d9d71b2441b83bbcb616347df66b7fb5e75 Mon Sep 17 00:00:00 2001
From: Art Lowel <art.lowel@gmail.com>
Date: Wed, 29 Nov 2023 14:25:36 +0100
Subject: [PATCH] ensure HALEndpointService doesn't use stale requests

---
 src/app/core/server-check/server-check.guard.spec.ts | 10 ++++++----
 src/app/core/server-check/server-check.guard.ts      |  6 ++----
 src/app/core/shared/hal-endpoint.service.ts          | 12 ++++++++----
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/app/core/server-check/server-check.guard.spec.ts b/src/app/core/server-check/server-check.guard.spec.ts
index 044609ef427..f65a7deca7c 100644
--- a/src/app/core/server-check/server-check.guard.spec.ts
+++ b/src/app/core/server-check/server-check.guard.spec.ts
@@ -9,7 +9,7 @@ import SpyObj = jasmine.SpyObj;
 describe('ServerCheckGuard', () => {
   let guard: ServerCheckGuard;
   let router: Router;
-  const eventSubject = new ReplaySubject<RouterEvent>(1);
+  let eventSubject: ReplaySubject<RouterEvent>;
   let rootDataServiceStub: SpyObj<RootDataService>;
   let testScheduler: TestScheduler;
   let redirectUrlTree: UrlTree;
@@ -24,6 +24,7 @@ describe('ServerCheckGuard', () => {
       findRoot: jasmine.createSpy('findRoot')
     });
     redirectUrlTree = new UrlTree();
+    eventSubject = new ReplaySubject<RouterEvent>(1);
     router = {
       events: eventSubject.asObservable(),
       navigateByUrl: jasmine.createSpy('navigateByUrl'),
@@ -64,10 +65,10 @@ describe('ServerCheckGuard', () => {
   });
 
   describe(`listenForRouteChanges`, () => {
-    it(`should retrieve the root endpoint, without using the cache, when the method is first called`, () => {
+    it(`should invalidate the root cache, when the method is first called`, () => {
         testScheduler.run(() => {
           guard.listenForRouteChanges();
-          expect(rootDataServiceStub.findRoot).toHaveBeenCalledWith(false);
+          expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(1);
         });
     });
 
@@ -80,7 +81,8 @@ describe('ServerCheckGuard', () => {
           eventSubject.next(new NavigationEnd(2,'', ''));
           eventSubject.next(new NavigationStart(3,''));
         });
-        expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(3);
+        // once when the method is first called, and then 3 times for NavigationStart events
+        expect(rootDataServiceStub.invalidateRootCache).toHaveBeenCalledTimes(1 + 3);
     });
   });
 });
diff --git a/src/app/core/server-check/server-check.guard.ts b/src/app/core/server-check/server-check.guard.ts
index 65ca2b0c498..79c34c36590 100644
--- a/src/app/core/server-check/server-check.guard.ts
+++ b/src/app/core/server-check/server-check.guard.ts
@@ -53,10 +53,8 @@ export class ServerCheckGuard implements CanActivateChild {
    */
   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. A `find` without using the cache,
-    // rather than an invalidateRootCache, because invalidating as the app is bootstrapping can
-    // break other features
-    this.rootDataService.findRoot(false);
+    // so this statement is for the very first route operation.
+    this.rootDataService.invalidateRootCache();
 
     this.router.events.pipe(
       filter(event => event instanceof NavigationStart),
diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts
index 8b6316a6ce2..98ab6a16ea0 100644
--- a/src/app/core/shared/hal-endpoint.service.ts
+++ b/src/app/core/shared/hal-endpoint.service.ts
@@ -1,5 +1,5 @@
 import { Observable } from 'rxjs';
-import { distinctUntilChanged, map, startWith, switchMap, take } from 'rxjs/operators';
+import { distinctUntilChanged, map, startWith, switchMap, take, skipWhile } from 'rxjs/operators';
 import { RequestService } from '../data/request.service';
 import { EndpointMapRequest } from '../data/request.models';
 import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util';
@@ -9,7 +9,7 @@ import { EndpointMap } from '../cache/response.models';
 import { getFirstCompletedRemoteData } from './operators';
 import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
 import { RemoteData } from '../data/remote-data';
-import { UnCacheableObject } from './uncacheable-object.model';
+import { CacheableObject } from '../cache/cacheable-object.model';
 
 @Injectable()
 export class HALEndpointService {
@@ -33,9 +33,13 @@ export class HALEndpointService {
 
     this.requestService.send(request, true);
 
-    return this.rdbService.buildFromHref<UnCacheableObject>(href).pipe(
+    return this.rdbService.buildFromHref<CacheableObject>(href).pipe(
+      // This skip ensures that if a stale object is present in the cache when you do a
+      // call it isn't immediately returned, but we wait until the remote data for the new request
+      // is created.
+      skipWhile((rd: RemoteData<CacheableObject>) => rd.isStale),
       getFirstCompletedRemoteData(),
-      map((response: RemoteData<UnCacheableObject>) => {
+      map((response: RemoteData<CacheableObject>) => {
         if (hasValue(response.payload)) {
           return response.payload._links;
         } else {