90252: Ensure calls come through if no subscribers

src/app/core/cache/object-cache.service.spec.ts

@@ -211,8 +211,8 @@ describe('ObjectCacheService', () => {
     });
   });
 
-  describe('has', () => {
-
+  describe('hasByHref', () => {
+    describe('with requestUUID not specified', () => {
       describe('getByHref emits an object', () => {
         beforeEach(() => {
           spyOn(service, 'getByHref').and.returnValue(observableOf(cacheEntry));
@@ -234,6 +234,50 @@ describe('ObjectCacheService', () => {
       });
     });
 
+    describe('with requestUUID specified', () => {
+      describe('getByHref emits an object that includes the specified requestUUID', () => {
+        beforeEach(() => {
+          spyOn(service, 'getByHref').and.returnValue(observableOf(Object.assign(cacheEntry, {
+            requestUUIDs: [
+              'something',
+              'something-else',
+              'specific-request',
+            ]
+          })));
+        });
+
+        it('should return true', () => {
+          expect(service.hasByHref(selfLink, 'specific-request')).toBe(true);
+        });
+      });
+
+      describe('getByHref emits an object that doesn\'t include the specified requestUUID', () => {
+        beforeEach(() => {
+          spyOn(service, 'getByHref').and.returnValue(observableOf(Object.assign(cacheEntry, {
+            requestUUIDs: [
+              'something',
+              'something-else',
+            ]
+          })));
+        });
+
+        it('should return true', () => {
+          expect(service.hasByHref(selfLink, 'specific-request')).toBe(false);
+        });
+      });
+
+      describe('getByHref emits nothing', () => {
+        beforeEach(() => {
+          spyOn(service, 'getByHref').and.returnValue(empty());
+        });
+
+        it('should return false', () => {
+          expect(service.hasByHref(selfLink, 'specific-request')).toBe(false);
+        });
+      });
+    });
+  });
+
   describe('getBySelfLink', () => {
     it('should return the entry returned by the select method', () => {
       const state = Object.assign({}, initialState, {

src/app/core/cache/object-cache.service.ts

@@ -282,7 +282,7 @@ export class ObjectCacheService {
     let result = false;
     this.getByHref(href).subscribe((entry: ObjectCacheEntry) => {
       if (isNotEmpty(requestUUID)) {
-        result = entry.requestUUIDs[0] === requestUUID;  // todo: may make more sense to do entry.requestUUIDs.includes(requestUUID) instead
+        result = entry.requestUUIDs.includes(requestUUID);
       } else {
         result = true;
       }

src/app/core/data/data.service.spec.ts

@@ -864,6 +864,16 @@ describe('DataService', () => {
       });
     });
 
+    it('should call setStaleByUUID even if not subscribing to returned Observable', fakeAsync(() => {
+      service.invalidateByHref('some-href');
+      tick();
+
+      expect(getByHrefSpy).toHaveBeenCalledWith('some-href');
+      expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request1');
+      expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request2');
+      expect(requestService.setStaleByUUID).toHaveBeenCalledWith('request3');
+    }));
+
     it('should return an Observable that only emits true once all requests are stale', () => {
       testScheduler.run(({ cold, expectObservable }) => {
         requestService.setStaleByUUID.and.callFake((uuid) => {
@@ -906,11 +916,11 @@ describe('DataService', () => {
 
     it('should retrieve href by ID and call deleteByHref', () => {
       getIDHrefObsSpy.and.returnValue(observableOf('some-href'));
-      buildFromRequestUUIDSpy.and.returnValue(null);
+      buildFromRequestUUIDSpy.and.returnValue(createSuccessfulRemoteDataObject$({}));
 
-      service.delete('some-id').subscribe(rd => {
+      service.delete('some-id', ['a', 'b', 'c']).subscribe(rd => {
         expect(getIDHrefObsSpy).toHaveBeenCalledWith('some-id');
-        expect(deleteByHrefSpy).toHaveBeenCalledWith('some-href');
+        expect(deleteByHrefSpy).toHaveBeenCalledWith('some-href', ['a', 'b', 'c']);
       });
     });
 
@@ -925,6 +935,15 @@ describe('DataService', () => {
         });
       });
 
+      it('should call invalidateByHref even if not subscribing to returned Observable', fakeAsync(() => {
+        buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_SUCCEEDED_RD));
+
+        service.deleteByHref('some-href');
+        tick();
+
+        expect(invalidateByHrefSpy).toHaveBeenCalled();
+      }));
+
       it('should not call invalidateByHref if the DELETE request fails', (done) => {
         buildFromRequestUUIDSpy.and.returnValue(observableOf(MOCK_FAILED_RD));
 

src/app/core/data/data.service.ts

@@ -1,7 +1,7 @@
 import { HttpClient } from '@angular/common/http';
 import { Store } from '@ngrx/store';
 import { Operation } from 'fast-json-patch';
-import { combineLatest, from, Observable, of as observableOf } from 'rxjs';
+import { AsyncSubject, combineLatest, from as observableFrom, Observable, of as observableOf } from 'rxjs';
 import {
   distinctUntilChanged,
   filter,
@@ -21,6 +21,7 @@ import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
 import { getClassForType } from '../cache/builders/build-decorators';
 import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
 import { RequestParam } from '../cache/models/request-param.model';
+import { ObjectCacheEntry } from '../cache/object-cache.reducer';
 import { ObjectCacheService } from '../cache/object-cache.service';
 import { DSpaceSerializer } from '../dspace-rest/dspace.serializer';
 import { DSpaceObject } from '../shared/dspace-object.model';
@@ -596,18 +597,22 @@ export abstract class DataService<T extends CacheableObject> implements UpdateDa
    * @return  An Observable that will emit `true` once all requests are stale
    */
   invalidateByHref(href: string): Observable<boolean> {
-    return this.objectCache.getByHref(href).pipe(
-      map(oce => oce.requestUUIDs),
-      switchMap(requestUUIDs => {
-        return from(requestUUIDs).pipe(
-          mergeMap(requestUUID => this.requestService.setStaleByUUID(requestUUID)),
+    const done$ = new AsyncSubject<boolean>();
+
+    this.objectCache.getByHref(href).pipe(
+      switchMap((oce: ObjectCacheEntry) => observableFrom(oce.requestUUIDs)),
+      mergeMap((requestUUID: string) => this.requestService.setStaleByUUID(requestUUID)),
       toArray(),
-        );
-      }),
-      map(areRequestsStale => areRequestsStale.every(Boolean)),
+      map((areRequestsStale: boolean[]) => areRequestsStale.every(Boolean)),
       distinctUntilChanged(),
-      takeWhile(allStale => allStale === false, true),
-    );
+    ).subscribe((done: boolean) => {
+      if (done) {
+        done$.next(true);
+        done$.complete();
+      }
+    });
+
+    return done$;
   }
 
   /**
@@ -652,22 +657,25 @@ export abstract class DataService<T extends CacheableObject> implements UpdateDa
 
     const response$ = this.rdbService.buildFromRequestUUID(requestId);
 
-    const invalidated$ = response$.pipe(
+    const invalidated$ = new AsyncSubject<boolean>();
+    response$.pipe(
       getFirstCompletedRemoteData(),
-      switchMap(rd => {
+      switchMap((rd: RemoteData<NoContent>) => {
         if (rd.hasSucceeded) {
           return this.invalidateByHref(href);
         } else {
           return [true];
         }
       })
-    );
+    ).subscribe((invalidated: boolean) => {
+      if (invalidated) {
+        invalidated$.next(true);
+        invalidated$.complete();
+      }
+    });
 
     return combineLatest([response$, invalidated$]).pipe(
       filter(([_, invalidated]) => invalidated),
-      tap(() => {
-        console.log(`DataService.deleteByHref() href=${href} done.`);
-      }),
       map(([response, _]) => response),
     );
   }

src/app/core/data/request.service.ts

@@ -320,8 +320,8 @@ export class RequestService {
     this.store.dispatch(new RequestStaleAction(uuid));
 
     return this.getByUUID(uuid).pipe(
-      map(request => isStale(request.state)),
-      filter(stale => stale === true),
+      map((request: RequestEntry) => isStale(request.state)),
+      filter((stale: boolean) => stale),
       take(1),
     );
   }