Merge pull request DSpace#3095 from tdonohue/port_2761_to_7x (#667)

[Port dspace-7_x] Fix caching issues for versioning

Co-authored-by: Tim Donohue <tim.donohue@lyrasis.org>

Author: milanmajchrak

src/app/core/data/base/identifiable-data.service.spec.ts

@@ -5,23 +5,23 @@
  *
  * http://www.dspace.org/license/
  */
-import { FindListOptions } from '../find-list-options.model';
 import { getMockRequestService } from '../../../shared/mocks/request.service.mock';
 import { HALEndpointServiceStub } from '../../../shared/testing/hal-endpoint-service.stub';
 import { getMockRemoteDataBuildService } from '../../../shared/mocks/remote-data-build.service.mock';
 import { followLink } from '../../../shared/utils/follow-link-config.model';
 import { TestScheduler } from 'rxjs/testing';
 import { RemoteData } from '../remote-data';
 import { RequestEntryState } from '../request-entry-state.model';
-import { Observable, of as observableOf } from 'rxjs';
+import { of as observableOf } from 'rxjs';
 import { RequestService } from '../request.service';
 import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service';
 import { HALEndpointService } from '../../shared/hal-endpoint.service';
 import { ObjectCacheService } from '../../cache/object-cache.service';
 import { IdentifiableDataService } from './identifiable-data.service';
 import { EMBED_SEPARATOR } from './base-data.service';
 
-const endpoint = 'https://rest.api/core';
+const base = 'https://rest.api/core';
+const endpoint = 'test';
 
 class TestService extends IdentifiableDataService<any> {
   constructor(
@@ -30,11 +30,7 @@ class TestService extends IdentifiableDataService<any> {
     protected objectCache: ObjectCacheService,
     protected halService: HALEndpointService,
   ) {
-    super(undefined, requestService, rdbService, objectCache, halService);
-  }
-
-  public getBrowseEndpoint(options: FindListOptions = {}, linkPath: string = this.linkPath): Observable<string> {
-    return observableOf(endpoint);
+    super(endpoint, requestService, rdbService, objectCache, halService);
   }
 }
 
@@ -51,7 +47,7 @@ describe('IdentifiableDataService', () => {
 
   function initTestService(): TestService {
     requestService = getMockRequestService();
-    halService = new HALEndpointServiceStub('url') as any;
+    halService = new HALEndpointServiceStub(base) as any;
     rdbService = getMockRemoteDataBuildService();
     objectCache = {
 
@@ -143,4 +139,12 @@ describe('IdentifiableDataService', () => {
       expect(result).toEqual(expected);
     });
   });
+
+  describe('invalidateById', () => {
+    it('should invalidate the correct resource by href', () => {
+      spyOn(service, 'invalidateByHref').and.returnValue(observableOf(true));
+      service.invalidateById('123');
+      expect(service.invalidateByHref).toHaveBeenCalledWith(`${base}/${endpoint}/123`);
+    });
+  });
 });

src/app/core/data/base/identifiable-data.service.ts

@@ -8,7 +8,7 @@
 import { CacheableObject } from '../../cache/cacheable-object.model';
 import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model';
 import { Observable } from 'rxjs';
-import { map } from 'rxjs/operators';
+import { map, switchMap, take } from 'rxjs/operators';
 import { RemoteData } from '../remote-data';
 import { BaseDataService } from './base-data.service';
 import { RequestService } from '../request.service';
@@ -80,4 +80,19 @@ export class IdentifiableDataService<T extends CacheableObject> extends BaseData
     return this.getEndpoint().pipe(
       map((endpoint: string) => this.getIDHref(endpoint, resourceID, ...linksToFollow)));
   }
+
+  /**
+   * Invalidate a cached resource by its identifier
+   * @param resourceID  the identifier of the resource to invalidate
+   */
+  invalidateById(resourceID: string): Observable<boolean> {
+    const ok$ = this.getIDHrefObs(resourceID).pipe(
+      take(1),
+      switchMap((href) => this.invalidateByHref(href))
+    );
+
+    ok$.subscribe();
+
+    return ok$;
+  }
 }

src/app/core/data/version-history-data.service.spec.ts

@@ -65,6 +65,10 @@ describe('VersionHistoryDataService', () => {
       },
     },
   });
+  const version1WithDraft = Object.assign(new Version(), {
+    ...version1,
+    versionhistory: createSuccessfulRemoteDataObject$(versionHistoryDraft),
+  });
   const versions = [version1, version2];
   versionHistory.versions = createSuccessfulRemoteDataObject$(createPaginatedList(versions));
   const item1 = Object.assign(new Item(), {
@@ -186,21 +190,18 @@ describe('VersionHistoryDataService', () => {
   });
 
   describe('hasDraftVersion$', () => {
-    beforeEach(waitForAsync(() => {
-      versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$<Version>(version1));
-    }));
     it('should return false if draftVersion is false', fakeAsync(() => {
-      versionService.getHistoryFromVersion.and.returnValue(of(versionHistory));
+      versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$<Version>(version1));
       service.hasDraftVersion$('href').subscribe((res) => {
         expect(res).toBeFalse();
       });
     }));
+
     it('should return true if draftVersion is true', fakeAsync(() => {
-      versionService.getHistoryFromVersion.and.returnValue(of(versionHistoryDraft));
+      versionService.findByHref.and.returnValue(createSuccessfulRemoteDataObject$<Version>(version1WithDraft));
       service.hasDraftVersion$('href').subscribe((res) => {
         expect(res).toBeTrue();
       });
     }));
   });
-
 });

src/app/core/data/version-history-data.service.ts

@@ -6,23 +6,21 @@ import { ObjectCacheService } from '../cache/object-cache.service';
 import { HALEndpointService } from '../shared/hal-endpoint.service';
 import { HttpHeaders } from '@angular/common/http';
 import { PostRequest } from './request.models';
-import { Observable, of } from 'rxjs';
+import { combineLatest, Observable, of as observableOf } from 'rxjs';
 import { PaginatedSearchOptions } from '../../shared/search/models/paginated-search-options.model';
 import { RemoteData } from './remote-data';
 import { PaginatedList } from './paginated-list.model';
 import { Version } from '../shared/version.model';
-import { filter, map, switchMap, take } from 'rxjs/operators';
+import { filter, find, map, switchMap, take } from 'rxjs/operators';
 import { VERSION_HISTORY } from '../shared/version-history.resource-type';
 import { followLink, FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
 import { VersionDataService } from './version-data.service';
 import { HttpOptions } from '../dspace-rest/dspace-rest.service';
 import { getAllSucceededRemoteData, getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload, getRemoteDataPayload } from '../shared/operators';
 import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model';
-import { hasValueOperator } from '../../shared/empty.util';
+import { hasValue, hasValueOperator } from '../../shared/empty.util';
 import { Item } from '../shared/item.model';
 import { FindListOptions } from './find-list-options.model';
-import { sendRequest } from '../shared/request.operators';
-import { RestRequest } from './rest-request.model';
 import { IdentifiableDataService } from './base/identifiable-data.service';
 import { dataService } from './base/data-service.decorator';
 
@@ -86,19 +84,31 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
    * @param summary the summary of the new version
    */
   createVersion(itemHref: string, summary: string): Observable<RemoteData<Version>> {
+    const requestId = this.requestService.generateRequestId();
     const requestOptions: HttpOptions = Object.create({});
     let requestHeaders = new HttpHeaders();
     requestHeaders = requestHeaders.append('Content-Type', 'text/uri-list');
     requestOptions.headers = requestHeaders;
 
-    return this.halService.getEndpoint(this.versionsEndpoint).pipe(
+    this.halService.getEndpoint(this.versionsEndpoint).pipe(
       take(1),
       map((endpointUrl: string) => (summary?.length > 0) ? `${endpointUrl}?summary=${summary}` : `${endpointUrl}`),
-      map((endpointURL: string) => new PostRequest(this.requestService.generateRequestId(), endpointURL, itemHref, requestOptions)),
-      sendRequest(this.requestService),
-      switchMap((restRequest: RestRequest) => this.rdbService.buildFromRequestUUID(restRequest.uuid)),
-      getFirstCompletedRemoteData()
-    ) as Observable<RemoteData<Version>>;
+      find((href: string) => hasValue(href)),
+    ).subscribe((href) => {
+      const request = new PostRequest(requestId, href, itemHref, requestOptions);
+      if (hasValue(this.responseMsToLive)) {
+        request.responseMsToLive = this.responseMsToLive;
+      }
+
+      this.requestService.send(request);
+    });
+
+    return this.rdbService.buildFromRequestUUIDAndAwait<Version>(requestId, (versionRD) => combineLatest([
+      this.requestService.setStaleByHrefSubstring(versionRD.payload._links.self.href),
+      this.requestService.setStaleByHrefSubstring(versionRD.payload._links.versionhistory.href),
+    ])).pipe(
+      getFirstCompletedRemoteData(),
+    );
   }
 
   /**
@@ -137,7 +147,7 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
       switchMap((res) => res.versionhistory),
       getFirstSucceededRemoteDataPayload(),
       switchMap((versionHistoryRD) => this.getLatestVersionFromHistory$(versionHistoryRD)),
-    ) : of(null);
+    ) : observableOf(null);
   }
 
   /**
@@ -148,8 +158,8 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
   isLatest$(version: Version): Observable<boolean> {
     return version ? this.getLatestVersion$(version).pipe(
       take(1),
-      switchMap((latestVersion) => of(version.version === latestVersion.version))
-    ) : of(null);
+      switchMap((latestVersion) => observableOf(version.version === latestVersion.version))
+    ) : observableOf(null);
   }
 
   /**
@@ -158,17 +168,22 @@ export class VersionHistoryDataService extends IdentifiableDataService<VersionHi
    * @returns `true` if a workspace item exists, `false` otherwise, or `null` if a version history does not exist
    */
   hasDraftVersion$(versionHref: string): Observable<boolean> {
-    return this.versionDataService.findByHref(versionHref, true, true, followLink('versionhistory')).pipe(
+    return this.versionDataService.findByHref(versionHref, false, true, followLink('versionhistory')).pipe(
       getFirstCompletedRemoteData(),
-      switchMap((res) => {
-        if (res.hasSucceeded && !res.hasNoContent) {
-          return of(res).pipe(
-            getFirstSucceededRemoteDataPayload(),
-            switchMap((version) => this.versionDataService.getHistoryFromVersion(version)),
-            map((versionHistory) => versionHistory ? versionHistory.draftVersion : false),
+      switchMap((versionRD: RemoteData<Version>) => {
+        if (versionRD.hasSucceeded && !versionRD.hasNoContent) {
+          return versionRD.payload.versionhistory.pipe(
+            getFirstCompletedRemoteData(),
+            map((versionHistoryRD: RemoteData<VersionHistory>) => {
+              if (versionHistoryRD.hasSucceeded && !versionHistoryRD.hasNoContent) {
+                return versionHistoryRD.payload.draftVersion;
+              } else {
+                return false;
+              }
+            }),
           );
         } else {
-          return of(false);
+          return observableOf(false);
         }
       }),
     );

src/app/core/submission/workflowitem-data.service.ts

@@ -13,7 +13,6 @@ import { RemoteData } from '../data/remote-data';
 import { NoContent } from '../shared/NoContent.model';
 import { getFirstCompletedRemoteData } from '../shared/operators';
 import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
-import { WorkspaceItem } from './models/workspaceitem.model';
 import { RequestParam } from '../cache/models/request-param.model';
 import { FindListOptions } from '../data/find-list-options.model';
 import { IdentifiableDataService } from '../data/base/identifiable-data.service';
@@ -28,7 +27,6 @@ import { dataService } from '../data/base/data-service.decorator';
 @Injectable()
 @dataService(WorkflowItem.type)
 export class WorkflowItemDataService extends IdentifiableDataService<WorkflowItem> implements SearchData<WorkflowItem>, DeleteData<WorkflowItem> {
-  protected linkPath = 'workflowitems';
   protected searchByItemLinkPath = 'item';
   protected responseMsToLive = 10 * 1000;
 
@@ -42,7 +40,7 @@ export class WorkflowItemDataService extends IdentifiableDataService<WorkflowIte
     protected halService: HALEndpointService,
     protected notificationsService: NotificationsService,
   ) {
-    super('workspaceitems', requestService, rdbService, objectCache, halService);
+    super('workflowitems', requestService, rdbService, objectCache, halService);
 
     this.searchData = new SearchDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, this.responseMsToLive);
     this.deleteData = new DeleteDataImpl(this.linkPath, requestService, rdbService, objectCache, halService, notificationsService, this.responseMsToLive, this.constructIdEndpoint);
@@ -105,7 +103,7 @@ export class WorkflowItemDataService extends IdentifiableDataService<WorkflowIte
    * @param options        The {@link FindListOptions} object
    * @param linksToFollow  List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved
    */
-  public findByItem(uuid: string, useCachedVersionIfAvailable = false, reRequestOnStale = true, options: FindListOptions = {}, ...linksToFollow: FollowLinkConfig<WorkspaceItem>[]): Observable<RemoteData<WorkspaceItem>> {
+  public findByItem(uuid: string, useCachedVersionIfAvailable = false, reRequestOnStale = true, options: FindListOptions = {}, ...linksToFollow: FollowLinkConfig<WorkflowItem>[]): Observable<RemoteData<WorkflowItem>> {
     const findListOptions = new FindListOptions();
     findListOptions.searchParams = [new RequestParam('uuid', encodeURIComponent(uuid))];
     const href$ = this.searchData.getSearchByHref(this.searchByItemLinkPath, findListOptions, ...linksToFollow);
@@ -126,7 +124,7 @@ export class WorkflowItemDataService extends IdentifiableDataService<WorkflowIte
    * @return {Observable<RemoteData<PaginatedList<T>>}
    *    Return an observable that emits response from the server
    */
-  public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<WorkspaceItem>[]): Observable<RemoteData<PaginatedList<WorkspaceItem>>> {
+  public searchBy(searchMethod: string, options?: FindListOptions, useCachedVersionIfAvailable?: boolean, reRequestOnStale?: boolean, ...linksToFollow: FollowLinkConfig<WorkflowItem>[]): Observable<RemoteData<PaginatedList<WorkflowItem>>> {
     return this.searchData.searchBy(searchMethod, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow);
   }
 

src/app/shared/dso-page/dso-versioning-modal-service/dso-versioning-modal.service.ts

@@ -1,7 +1,7 @@
 import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../../../core/shared/operators';
 import { RemoteData } from '../../../core/data/remote-data';
 import { Version } from '../../../core/shared/version.model';
-import { map, startWith, switchMap, tap } from 'rxjs/operators';
+import { map, switchMap, tap } from 'rxjs/operators';
 import { Item } from '../../../core/shared/item.model';
 import { WorkspaceItem } from '../../../core/submission/models/workspaceitem.model';
 import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
@@ -13,9 +13,7 @@ import { ItemDataService } from '../../../core/data/item-data.service';
 import { Injectable } from '@angular/core';
 import { Observable, of } from 'rxjs';
 import { ItemVersionsSharedService } from '../../../item-page/versions/item-versions-shared.service';
-import {
-  ItemVersionsSummaryModalComponent
-} from '../../../item-page/versions/item-versions-summary-modal/item-versions-summary-modal.component';
+import { ItemVersionsSummaryModalComponent } from '../../../item-page/versions/item-versions-summary-modal/item-versions-summary-modal.component';
 
 /**
  * Service to take care of all the functionality related to the version creation modal
@@ -86,7 +84,6 @@ export class DsoVersioningModalService {
       // button is disabled if hasDraftVersion = true, and enabled if hasDraftVersion = false or null
       // (hasDraftVersion is null when a version history does not exist)
       map((res) => Boolean(res)),
-      startWith(true),
     );
   }
 

src/app/shared/mydspace-actions/claimed-task/approve/claimed-task-actions-approve.component.spec.ts

@@ -2,7 +2,7 @@ import { ChangeDetectionStrategy, Injector, NO_ERRORS_SCHEMA } from '@angular/co
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
 import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
-import { of, of as observableOf } from 'rxjs';
+import { of as observableOf } from 'rxjs';
 
 import { ClaimedTaskActionsApproveComponent } from './claimed-task-actions-approve.component';
 import { TranslateLoaderMock } from '../../../mocks/translate-loader.mock';
@@ -18,6 +18,7 @@ import { Router } from '@angular/router';
 import { RouterStub } from '../../../testing/router.stub';
 import { SearchService } from '../../../../core/shared/search/search.service';
 import { RequestService } from '../../../../core/data/request.service';
+import { WorkflowItemDataService } from '../../../../core/submission/workflowitem-data.service';
 
 let component: ClaimedTaskActionsApproveComponent;
 let fixture: ComponentFixture<ClaimedTaskActionsApproveComponent>;
@@ -27,6 +28,7 @@ const searchService = getMockSearchService();
 const requestService = getMockRequestService();
 
 let mockPoolTaskDataService: PoolTaskDataService;
+let mockWorkflowItemDataService: WorkflowItemDataService;
 
 describe('ClaimedTaskActionsApproveComponent', () => {
   const object = Object.assign(new ClaimedTask(), { id: 'claimed-task-1' });
@@ -36,6 +38,10 @@ describe('ClaimedTaskActionsApproveComponent', () => {
 
   beforeEach(waitForAsync(() => {
     mockPoolTaskDataService = new PoolTaskDataService(null, null, null, null);
+    mockWorkflowItemDataService = jasmine.createSpyObj('WorkflowItemDataService', {
+      'invalidateByHref': observableOf(false),
+    });
+
     TestBed.configureTestingModule({
       imports: [
         TranslateModule.forRoot({
@@ -53,6 +59,7 @@ describe('ClaimedTaskActionsApproveComponent', () => {
         { provide: SearchService, useValue: searchService },
         { provide: RequestService, useValue: requestService },
         { provide: PoolTaskDataService, useValue: mockPoolTaskDataService },
+        { provide: WorkflowItemDataService, useValue: mockWorkflowItemDataService },
       ],
       declarations: [ClaimedTaskActionsApproveComponent],
       schemas: [NO_ERRORS_SCHEMA]
@@ -89,7 +96,7 @@ describe('ClaimedTaskActionsApproveComponent', () => {
 
     beforeEach(() => {
       spyOn(component.processCompleted, 'emit');
-      spyOn(component, 'startActionExecution').and.returnValue(of(null));
+      spyOn(component, 'startActionExecution').and.returnValue(observableOf(null));
 
       expectedBody = {
         [component.option]: 'true'