diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index 047613d6d1..b76c0a7520 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core'; import { ActivatedRoute } from '@angular/router'; -import { Observable, Subscription } from 'rxjs'; +import { Observable, Subscription } from 'rxjs'; import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; import { CollectionDataService } from '../core/data/collection-data.service'; import { PaginatedList } from '../core/data/paginated-list'; @@ -15,7 +15,7 @@ import { Item } from '../core/shared/item.model'; import { fadeIn, fadeInOut } from '../shared/animations/fade'; import { hasValue, isNotEmpty } from '../shared/empty.util'; import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model'; -import { filter, first, flatMap, map } from 'rxjs/operators'; +import { filter, flatMap, map, tap } from 'rxjs/operators'; import { SearchService } from '../+search-page/search-service/search.service'; import { PaginatedSearchOptions } from '../+search-page/paginated-search-options.model'; import { toDSpaceObjectListRD } from '../core/shared/operators'; diff --git a/src/app/+search-page/search-service/search.service.spec.ts b/src/app/+search-page/search-service/search.service.spec.ts index 028a3fa446..4af0ffcb2e 100644 --- a/src/app/+search-page/search-service/search.service.spec.ts +++ b/src/app/+search-page/search-service/search.service.spec.ts @@ -84,8 +84,8 @@ describe('SearchService', () => { const remoteDataBuildService = { toRemoteDataObservable: (requestEntryObs: Observable, payloadObs: Observable) => { return observableCombineLatest(requestEntryObs, payloadObs).pipe( - map(([req, res, pay]) => { - return { req, res, pay }; + map(([req, pay]) => { + return { req, pay }; }) ); }, @@ -175,9 +175,6 @@ describe('SearchService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((searchService as any).requestService.getByHref).toHaveBeenCalledWith(endPoint); }); - it('should call get on the request service with the correct request url', () => { - expect((searchService as any).responseCache.get).toHaveBeenCalledWith(endPoint); - }); }); describe('when getConfig is called without a scope', () => { @@ -203,9 +200,6 @@ describe('SearchService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((searchService as any).requestService.getByHref).toHaveBeenCalledWith(endPoint); }); - it('should call get on the request service with the correct request url', () => { - expect((searchService as any).responseCache.get).toHaveBeenCalledWith(endPoint); - }); }); describe('when getConfig is called with a scope', () => { @@ -233,9 +227,6 @@ describe('SearchService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((searchService as any).requestService.getByHref).toHaveBeenCalledWith(requestUrl); }); - it('should call get on the request service with the correct request url', () => { - expect((searchService as any).responseCache.get).toHaveBeenCalledWith(requestUrl); - }); }); }); }); diff --git a/src/app/+search-page/search-service/search.service.ts b/src/app/+search-page/search-service/search.service.ts index c628fd350e..275b0b3340 100644 --- a/src/app/+search-page/search-service/search.service.ts +++ b/src/app/+search-page/search-service/search.service.ts @@ -7,7 +7,7 @@ import { Router, UrlSegmentGroup } from '@angular/router'; -import { filter, flatMap, map, switchMap } from 'rxjs/operators'; +import { map, switchMap, tap } from 'rxjs/operators'; import { RemoteDataBuildService } from '../../core/cache/builders/remote-data-build.service'; import { FacetConfigSuccessResponse, @@ -47,7 +47,6 @@ import { CommunityDataService } from '../../core/data/community-data.service'; import { ViewMode } from '../../core/shared/view-mode.model'; import { ResourceType } from '../../core/shared/resource-type'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; -import { RequestEntry } from '../../core/data/request.reducer'; /** * Service that performs all general actions that have to do with the search page @@ -100,7 +99,7 @@ export class SearchService implements OnDestroy { configureRequest(this.requestService) ); const requestEntryObs = requestObs.pipe( - flatMap((request: RestRequest) => this.requestService.getByHref(request.href)) + switchMap((request: RestRequest) => this.requestService.getByHref(request.href)) ); // get search results from response cache @@ -113,10 +112,11 @@ export class SearchService implements OnDestroy { // Turn list of observable remote data DSO's into observable remote data object with list of DSO const dsoObs: Observable> = sqrObs.pipe( map((sqr: SearchQueryResponse) => { - return sqr.objects.map((nsr: NormalizedSearchResult) => - this.rdb.buildSingle(nsr.dspaceObject)); + return sqr.objects.map((nsr: NormalizedSearchResult) => { + return this.rdb.buildSingle(nsr.dspaceObject); + }) }), - flatMap((input: Array>>) => this.rdb.aggregate(input)) + switchMap((input: Array>>) => this.rdb.aggregate(input)), ); // Create search results again with the correct dso objects linked to each result @@ -180,7 +180,7 @@ export class SearchService implements OnDestroy { ); const requestEntryObs = requestObs.pipe( - flatMap((request: RestRequest) => this.requestService.getByHref(request.href)) + switchMap((request: RestRequest) => this.requestService.getByHref(request.href)) ); // get search results from response cache @@ -223,7 +223,7 @@ export class SearchService implements OnDestroy { ); const requestEntryObs = requestObs.pipe( - flatMap((request: RestRequest) => this.requestService.getByHref(request.href)) + switchMap((request: RestRequest) => this.requestService.getByHref(request.href)) ); // get search results from response cache diff --git a/src/app/core/auth/auth-response-parsing.service.spec.ts b/src/app/core/auth/auth-response-parsing.service.spec.ts index e23e3ac56b..a4131db489 100644 --- a/src/app/core/auth/auth-response-parsing.service.spec.ts +++ b/src/app/core/auth/auth-response-parsing.service.spec.ts @@ -1,7 +1,6 @@ import { AuthStatusResponse } from '../cache/response.models'; import { ObjectCacheService } from '../cache/object-cache.service'; -import { GlobalConfig } from '../../../config/global-config.interface'; import { AuthStatus } from './models/auth-status.model'; import { AuthResponseParsingService } from './auth-response-parsing.service'; import { AuthGetRequest, AuthPostRequest } from '../data/request.models'; @@ -11,7 +10,7 @@ import { ObjectCacheState } from '../cache/object-cache.reducer'; describe('AuthResponseParsingService', () => { let service: AuthResponseParsingService; - const EnvConfig = { cache: { msToLive: 1000 } } as GlobalConfig; + const EnvConfig = { cache: { msToLive: 1000 } } as any; const store = new MockStore({}); const objectCacheService = new ObjectCacheService(store as any); diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index 4465eae1ee..da75e1a877 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -8,6 +8,8 @@ import { BrowseEndpointRequest, BrowseEntriesRequest, BrowseItemsRequest } from import { RequestService } from '../data/request.service'; import { BrowseDefinition } from '../shared/browse-definition.model'; import { BrowseService } from './browse.service'; +import { RequestEntry } from '../data/request.reducer'; +import { of as observableOf } from 'rxjs'; describe('BrowseService', () => { let scheduler: TestScheduler; @@ -76,7 +78,11 @@ describe('BrowseService', () => { }) ]; - + const getRequestEntry$ = (successful: boolean) => { + return observableOf({ + response: { isSuccessful: successful, payload: browseDefinitions } as any + } as RequestEntry) + }; function initTestService() { return new BrowseService( @@ -93,7 +99,7 @@ describe('BrowseService', () => { describe('getBrowseDefinitions', () => { beforeEach(() => { - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(true)); rdbService = getMockRemoteDataBuildService(); service = initTestService(); spyOn(halService, 'getEndpoint').and @@ -131,7 +137,7 @@ describe('BrowseService', () => { const mockAuthorName = 'Donald Smith'; beforeEach(() => { - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(true)); rdbService = getMockRemoteDataBuildService(); service = initTestService(); spyOn(service, 'getBrowseDefinitions').and @@ -204,7 +210,7 @@ describe('BrowseService', () => { describe('if getBrowseDefinitions fires', () => { beforeEach(() => { - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(true)); rdbService = getMockRemoteDataBuildService(); service = initTestService(); spyOn(service, 'getBrowseDefinitions').and @@ -259,7 +265,7 @@ describe('BrowseService', () => { describe('if getBrowseDefinitions doesn\'t fire', () => { it('should return undefined', () => { - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(true)); rdbService = getMockRemoteDataBuildService(); service = initTestService(); spyOn(service, 'getBrowseDefinitions').and diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index c1c893ec27..ca56c0d267 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -1,8 +1,8 @@ import { Injectable } from '@angular/core'; import { Observable } from 'rxjs'; -import { distinctUntilChanged, filter, map, startWith, tap } from 'rxjs/operators'; +import { distinctUntilChanged, map, startWith } from 'rxjs/operators'; import { - ensureArrayHasValue, hasValue, + ensureArrayHasValue, hasValueOperator, isEmpty, isNotEmpty, @@ -34,7 +34,6 @@ import { import { URLCombiner } from '../url-combiner/url-combiner'; import { Item } from '../shared/item.model'; import { DSpaceObject } from '../shared/dspace-object.model'; -import { RequestEntry } from '../data/request.reducer'; @Injectable() export class BrowseService { diff --git a/src/app/core/cache/builders/remote-data-build.service.ts b/src/app/core/cache/builders/remote-data-build.service.ts index 27b5ddf50d..ad5707bbf6 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -5,7 +5,7 @@ import { race as observableRace } from 'rxjs'; import { Injectable } from '@angular/core'; -import { distinctUntilChanged, flatMap, map, startWith, switchMap, tap } from 'rxjs/operators'; +import { distinctUntilChanged, first, flatMap, map, startWith, switchMap } from 'rxjs/operators'; import { hasValue, hasValueOperator, isEmpty, isNotEmpty } from '../../../shared/empty.util'; import { PaginatedList } from '../../data/paginated-list'; import { RemoteData } from '../../data/remote-data'; @@ -32,6 +32,8 @@ export class RemoteDataBuildService { } buildSingle(href$: string | Observable): Observable> { + console.log('call buildSingle', href$); + if (typeof href$ === 'string') { href$ = observableOf(href$); } @@ -42,19 +44,18 @@ export class RemoteDataBuildService { const requestEntry$ = observableRace( href$.pipe(getRequestFromSelflink(this.requestService)), - requestHref$.pipe(getRequestFromSelflink(this.requestService)) - ); + requestHref$.pipe(getRequestFromSelflink(this.requestService)), + ).pipe(first()); // always use self link if that is cached, only if it isn't, get it via the response. const payload$ = observableCombineLatest( href$.pipe( - flatMap((href: string) => this.objectCache.getBySelfLink(href)), - startWith(undefined) - ), + switchMap((href: string) => this.objectCache.getBySelfLink(href)), + startWith(undefined)), requestEntry$.pipe( getResourceLinksFromResponse(), - flatMap((resourceSelfLinks: string[]) => { + switchMap((resourceSelfLinks: string[]) => { if (isNotEmpty(resourceSelfLinks)) { return this.objectCache.getBySelfLink(resourceSelfLinks[0]); } else { @@ -83,7 +84,7 @@ export class RemoteDataBuildService { } toRemoteDataObservable(requestEntry$: Observable, payload$: Observable) { - return observableCombineLatest(requestEntry$, requestEntry$.pipe(startWith(undefined)), payload$).pipe( + return observableCombineLatest(requestEntry$, payload$).pipe( map(([reqEntry, payload]) => { const requestPending = hasValue(reqEntry.requestPending) ? reqEntry.requestPending : true; const responsePending = hasValue(reqEntry.responsePending) ? reqEntry.responsePending : false; @@ -124,9 +125,8 @@ export class RemoteDataBuildService { })); }), startWith([]), - distinctUntilChanged() + distinctUntilChanged(), ); - // tDomainList$.subscribe((t) => {console.log('domainlist', t)}); const pageInfo$ = requestEntry$.pipe( filterSuccessfulResponses(), map((response: DSOSuccessResponse) => { @@ -152,7 +152,6 @@ export class RemoteDataBuildService { build(normalized: TNormalized): TDomain { const links: any = {}; - const relationships = getRelationships(normalized.constructor) || []; relationships.forEach((relationship: string) => { @@ -196,8 +195,6 @@ export class RemoteDataBuildService { } }); const domainModel = getMapsTo(normalized.constructor); - // console.log('domain model', normalized); - return Object.assign(new domainModel(), normalized, links); } diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index 2125aff797..867d559c70 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -12,6 +12,8 @@ import { FindAllOptions, FindByIDRequest } from './request.models'; import { RequestService } from './request.service'; import { NormalizedObject } from '../cache/models/normalized-object.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { RequestEntry } from './request.reducer'; +import { of as observableOf } from 'rxjs'; const LINK_NAME = 'test'; @@ -34,6 +36,7 @@ class TestService extends ComColDataService { super(); } } + /* tslint:enable:max-classes-per-file */ describe('ComColDataService', () => { @@ -52,6 +55,11 @@ describe('ComColDataService', () => { const options = Object.assign(new FindAllOptions(), { scopeID: scopeID }); + const getRequestEntry$ = (successful: boolean) => { + return observableOf({ + response: { isSuccessful: successful } as any + } as RequestEntry) + }; const communitiesEndpoint = 'https://rest.api/core/communities'; const communityEndpoint = `${communitiesEndpoint}/${scopeID}`; @@ -97,7 +105,7 @@ describe('ComColDataService', () => { it('should configure a new FindByIDRequest for the scope Community', () => { cds = initMockCommunityDataService(); - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(true)); objectCache = initMockObjectCacheService(); service = initTestService(); @@ -112,7 +120,7 @@ describe('ComColDataService', () => { describe('if the scope Community can be found', () => { beforeEach(() => { cds = initMockCommunityDataService(); - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(true)); objectCache = initMockObjectCacheService(); service = initTestService(); }); @@ -125,16 +133,16 @@ describe('ComColDataService', () => { it('should return the endpoint to fetch resources within the given scope', () => { const result = service.getBrowseEndpoint(options); - const expected = cold('--e-', { e: scopedEndpoint }); + const expected = '--e-'; - expect(result).toBeObservable(expected); + scheduler.expectObservable(result).toBe(expected, { e: scopedEndpoint }); }); }); describe('if the scope Community can\'t be found', () => { beforeEach(() => { cds = initMockCommunityDataService(); - requestService = getMockRequestService(); + requestService = getMockRequestService(getRequestEntry$(false)); objectCache = initMockObjectCacheService(); service = initTestService(); }); diff --git a/src/app/core/data/request.reducer.spec.ts b/src/app/core/data/request.reducer.spec.ts index bd8fad5de7..57fbb01ce1 100644 --- a/src/app/core/data/request.reducer.spec.ts +++ b/src/app/core/data/request.reducer.spec.ts @@ -2,16 +2,20 @@ import * as deepFreeze from 'deep-freeze'; import { requestReducer, RequestState } from './request.reducer'; import { - RequestCompleteAction, RequestConfigureAction, RequestExecuteAction + RequestCompleteAction, + RequestConfigureAction, + RequestExecuteAction, ResetResponseTimestampsAction } from './request.actions'; -import { GetRequest, RestRequest } from './request.models'; +import { GetRequest } from './request.models'; +import { RestResponse } from '../cache/response.models'; +const response = new RestResponse(true, 'OK'); class NullAction extends RequestCompleteAction { type = null; payload = null; constructor() { - super(null); + super(null, null); } } @@ -25,7 +29,8 @@ describe('requestReducer', () => { request: new GetRequest(id1, link1), requestPending: false, responsePending: false, - completed: false + completed: false, + response: undefined } }; deepFreeze(testState); @@ -56,6 +61,7 @@ describe('requestReducer', () => { expect(newState[id2].requestPending).toEqual(true); expect(newState[id2].responsePending).toEqual(false); expect(newState[id2].completed).toEqual(false); + expect(newState[id2].response).toEqual(undefined); }); it('should set \'requestPending\' to false, \'responsePending\' to true and leave \'completed\' untouched for the given RestRequest in the state, in response to an EXECUTE action', () => { @@ -69,11 +75,13 @@ describe('requestReducer', () => { expect(newState[id1].requestPending).toEqual(false); expect(newState[id1].responsePending).toEqual(true); expect(newState[id1].completed).toEqual(state[id1].completed); + expect(newState[id1].response).toEqual(undefined) }); + it('should leave \'requestPending\' untouched, set \'responsePending\' to false and \'completed\' to true for the given RestRequest in the state, in response to a COMPLETE action', () => { const state = testState; - const action = new RequestCompleteAction(id1); + const action = new RequestCompleteAction(id1, response); const newState = requestReducer(state, action); expect(newState[id1].request.uuid).toEqual(id1); @@ -81,5 +89,25 @@ describe('requestReducer', () => { expect(newState[id1].requestPending).toEqual(state[id1].requestPending); expect(newState[id1].responsePending).toEqual(false); expect(newState[id1].completed).toEqual(true); + expect(newState[id1].response.isSuccessful).toEqual(response.isSuccessful) + expect(newState[id1].response.statusCode).toEqual(response.statusCode) + expect(newState[id1].response.timeAdded).toBeTruthy() + }); + + it('should leave \'requestPending\' untouched, should leave \'responsePending\' untouched and leave \'completed\' untouched, but update the response\'s timeAdded for the given RestRequest in the state, in response to a COMPLETE action', () => { + const update = Object.assign({}, testState[id1], {response}); + const state = Object.assign({}, testState, {[id1]: update}); + const timeStamp = 1000; + const action = new ResetResponseTimestampsAction(timeStamp); + const newState = requestReducer(state, action); + + expect(newState[id1].request.uuid).toEqual(state[id1].request.uuid); + expect(newState[id1].request.href).toEqual(state[id1].request.href); + expect(newState[id1].requestPending).toEqual(state[id1].requestPending); + expect(newState[id1].responsePending).toEqual(state[id1].responsePending); + expect(newState[id1].completed).toEqual(state[id1].completed); + expect(newState[id1].response.isSuccessful).toEqual(response.isSuccessful); + expect(newState[id1].response.statusCode).toEqual(response.statusCode); + expect(newState[id1].response.timeAdded).toBe(timeStamp); }); }); diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index b0875f37b3..a680de2d6b 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -56,12 +56,13 @@ function configureRequest(state: RequestState, action: RequestConfigureAction): } function executeRequest(state: RequestState, action: RequestExecuteAction): RequestState { - return Object.assign({}, state, { + const obs = Object.assign({}, state, { [action.payload]: Object.assign({}, state[action.payload], { requestPending: false, responsePending: true }) }); + return obs; } /** @@ -76,16 +77,13 @@ function executeRequest(state: RequestState, action: RequestExecuteAction): Requ */ function completeRequest(state: RequestState, action: RequestCompleteAction): RequestState { const time = new Date().getTime(); - - const ob = Object.assign({}, state, { + return Object.assign({}, state, { [action.payload.uuid]: Object.assign({}, state[action.payload.uuid], { responsePending: false, completed: true, response: Object.assign({}, action.payload.response, { timeAdded: time }) }) }); - console.log(ob); - return ob; } function resetResponseTimestamps(state: RequestState, action: ResetResponseTimestampsAction) { diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index 5953d43c9f..5232d7efab 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -46,13 +46,10 @@ describe('RequestService', () => { objectCache = getMockObjectCacheService(); (objectCache.hasBySelfLink as any).and.returnValue(false); - (responseCache.has as any).and.returnValue(false); - (responseCache.get as any).and.returnValue(observableOf(undefined)); - uuidService = getMockUUIDService(); store = new Store(new BehaviorSubject({}), new ActionsSubject(), null); - selectSpy = spyOnProperty(ngrx, 'select') + selectSpy = spyOnProperty(ngrx, 'select'); selectSpy.and.callFake(() => { return () => { return () => cold('a', { a: undefined }); @@ -322,7 +319,7 @@ describe('RequestService', () => { describe('when the request is cached', () => { describe('in the ObjectCache', () => { beforeEach(() => { - (objectCache.hasBySelfLink as any).and.returnValues(true); + (objectCache.hasBySelfLink as any).and.returnValue(true); }); it('should return true', () => { @@ -334,12 +331,13 @@ describe('RequestService', () => { }); describe('in the responseCache', () => { beforeEach(() => { - (responseCache.has as any).and.returnValues(true); + spyOn(serviceAsAny, 'isReusable').and.returnValue(observableOf(true)); + spyOn(serviceAsAny, 'getByHref').and.returnValue(observableOf(undefined)); }); describe('and it\'s a DSOSuccessResponse', () => { beforeEach(() => { - (responseCache.get as any).and.returnValues(observableOf({ + (serviceAsAny.getByHref as any).and.returnValue(observableOf({ response: { isSuccessful: true, resourceSelfLinks: [ @@ -361,6 +359,7 @@ describe('RequestService', () => { }); it('should return false if not all top level links in the response are cached in the object cache', () => { (objectCache.hasBySelfLink as any).and.returnValues(false, true, false); + spyOn(service, 'isPending').and.returnValue(false); const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = false; @@ -368,11 +367,12 @@ describe('RequestService', () => { expect(result).toEqual(expected); }); }); + describe('and it isn\'t a DSOSuccessResponse', () => { beforeEach(() => { - (objectCache.hasBySelfLink as any).and.returnValues(false); - (responseCache.has as any).and.returnValues(true); - (responseCache.get as any).and.returnValues(observableOf({ + (objectCache.hasBySelfLink as any).and.returnValue(false); + (service as any).isReusable.and.returnValue(observableOf(true)); + (serviceAsAny.getByHref as any).and.returnValue(observableOf({ response: { isSuccessful: true } diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index cc8c9816f8..564b19de75 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -1,5 +1,6 @@ import { merge as observableMerge, Observable, of as observableOf } from 'rxjs'; import { + distinctUntilChanged, filter, find, first, @@ -109,10 +110,10 @@ export class RequestService { map((resourceSelfLinks: string[]) => resourceSelfLinks .every((selfLink) => this.objectCache.hasBySelfLink(selfLink)) )); + const otherSuccessResponses = responses.pipe(filter((response) => response.isSuccessful && !hasValue((response as DSOSuccessResponse).resourceSelfLinks)), map(() => true)); observableMerge(errorResponses, otherSuccessResponses, dsoSuccessResponses).subscribe((c) => isCached = c); - const isPending = this.isPending(request); return isCached || isPending; } @@ -144,7 +145,7 @@ export class RequestService { } /** - * Check whether a ResponseCacheEntry should still be cached + * Check whether a Response should still be cached * * @param entry * the entry to check diff --git a/src/app/core/registry/registry.service.spec.ts b/src/app/core/registry/registry.service.spec.ts index adb2ed8b05..c87597cffc 100644 --- a/src/app/core/registry/registry.service.spec.ts +++ b/src/app/core/registry/registry.service.spec.ts @@ -124,10 +124,10 @@ describe('RegistryService', () => { }; const rdbStub = { - toRemoteDataObservable: (requestEntryObs: Observable, responseCacheObs: Observable, payloadObs: Observable) => { + toRemoteDataObservable: (requestEntryObs: Observable, payloadObs: Observable) => { return observableCombineLatest(requestEntryObs, - responseCacheObs, payloadObs).pipe(map(([req, res, pay]) => { - return { req, res, pay }; + payloadObs).pipe(map(([req, pay]) => { + return { req, pay }; }) ); }, @@ -160,10 +160,10 @@ describe('RegistryService', () => { page: pageInfo }); const response = new RegistryMetadataschemasSuccessResponse(queryResponse, '200', pageInfo); - const responseEntry = Object.assign(new ResponseCacheEntry(), { response: response }); + const responseEntry = Object.assign(new RequestEntry(), { response: response }); beforeEach(() => { - (registryService as any).responseCache.get.and.returnValue(observableOf(responseEntry)); + (registryService as any).requestService.getByHref.and.returnValue(observableOf(responseEntry)); /* tslint:disable:no-empty */ registryService.getMetadataSchemas(pagination).subscribe((value) => { }); @@ -181,10 +181,6 @@ describe('RegistryService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((registryService as any).requestService.getByHref).toHaveBeenCalledWith(endpointWithParams); }); - - it('should call get on the request service with the correct request url', () => { - expect((registryService as any).responseCache.get).toHaveBeenCalledWith(endpointWithParams); - }); }); describe('when requesting metadataschema by name', () => { @@ -193,10 +189,10 @@ describe('RegistryService', () => { page: pageInfo }); const response = new RegistryMetadataschemasSuccessResponse(queryResponse, '200', pageInfo); - const responseEntry = Object.assign(new ResponseCacheEntry(), { response: response }); + const responseEntry = Object.assign(new RequestEntry(), { response: response }); beforeEach(() => { - (registryService as any).responseCache.get.and.returnValue(observableOf(responseEntry)); + (registryService as any).requestService.getByHref.and.returnValue(observableOf(responseEntry)); /* tslint:disable:no-empty */ registryService.getMetadataSchemaByName(mockSchemasList[0].prefix).subscribe((value) => { }); @@ -214,10 +210,6 @@ describe('RegistryService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((registryService as any).requestService.getByHref.calls.argsFor(0)[0]).toContain(endpoint); }); - - it('should call get on the request service with the correct request url', () => { - expect((registryService as any).responseCache.get.calls.argsFor(0)[0]).toContain(endpoint); - }); }); describe('when requesting metadatafields', () => { @@ -226,10 +218,10 @@ describe('RegistryService', () => { page: pageInfo }); const response = new RegistryMetadatafieldsSuccessResponse(queryResponse, '200', pageInfo); - const responseEntry = Object.assign(new ResponseCacheEntry(), { response: response }); + const responseEntry = Object.assign(new RequestEntry(), { response: response }); beforeEach(() => { - (registryService as any).responseCache.get.and.returnValue(observableOf(responseEntry)); + (registryService as any).requestService.getByHref.and.returnValue(observableOf(responseEntry)); /* tslint:disable:no-empty */ registryService.getMetadataFieldsBySchema(mockSchemasList[0], pagination).subscribe((value) => { }); @@ -247,10 +239,6 @@ describe('RegistryService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((registryService as any).requestService.getByHref).toHaveBeenCalledWith(endpointWithParams); }); - - it('should call get on the request service with the correct request url', () => { - expect((registryService as any).responseCache.get).toHaveBeenCalledWith(endpointWithParams); - }); }); describe('when requesting bitstreamformats', () => { @@ -259,10 +247,10 @@ describe('RegistryService', () => { page: pageInfo }); const response = new RegistryBitstreamformatsSuccessResponse(queryResponse, '200', pageInfo); - const responseEntry = Object.assign(new ResponseCacheEntry(), { response: response }); + const responseEntry = Object.assign(new RequestEntry(), { response: response }); beforeEach(() => { - (registryService as any).responseCache.get.and.returnValue(observableOf(responseEntry)); + (registryService as any).requestService.getByHref.and.returnValue(observableOf(responseEntry)); /* tslint:disable:no-empty */ registryService.getBitstreamFormats(pagination).subscribe((value) => { }); @@ -280,9 +268,5 @@ describe('RegistryService', () => { it('should call getByHref on the request service with the correct request url', () => { expect((registryService as any).requestService.getByHref).toHaveBeenCalledWith(endpointWithParams); }); - - it('should call get on the request service with the correct request url', () => { - expect((registryService as any).responseCache.get).toHaveBeenCalledWith(endpointWithParams); - }); }); }); diff --git a/src/app/core/shared/hal-endpoint.service.spec.ts b/src/app/core/shared/hal-endpoint.service.spec.ts index d36da207ca..b65b3f905b 100644 --- a/src/app/core/shared/hal-endpoint.service.spec.ts +++ b/src/app/core/shared/hal-endpoint.service.spec.ts @@ -1,33 +1,44 @@ -import { cold, hot } from 'jasmine-marbles'; +import { cold, getTestScheduler, hot } from 'jasmine-marbles'; import { GlobalConfig } from '../../../config/global-config.interface'; import { getMockRequestService } from '../../shared/mocks/mock-request.service'; import { RequestService } from '../data/request.service'; import { HALEndpointService } from './hal-endpoint.service'; import { EndpointMapRequest } from '../data/request.models'; +import { RequestEntry } from '../data/request.reducer'; +import { of as observableOf } from 'rxjs'; describe('HALEndpointService', () => { let service: HALEndpointService; let requestService: RequestService; let envConfig: GlobalConfig; + let requestEntry; const endpointMap = { test: 'https://rest.api/test', }; const linkPath = 'test'; + beforeEach(() => { + requestEntry = { + request: { responseMsToLive: 1000 } as any, + requestPending: false, + responsePending: false, + completed: true, + response: { endpointMap: endpointMap } as any + } as RequestEntry; + requestService = getMockRequestService(observableOf(requestEntry)); + + envConfig = { + rest: { baseUrl: 'https://rest.api/' } + } as any; + + service = new HALEndpointService( + requestService, + envConfig + ); + }); + describe('getRootEndpointMap', () => { - beforeEach(() => { - requestService = getMockRequestService(); - - envConfig = { - rest: { baseUrl: 'https://rest.api/' } - } as any; - - service = new HALEndpointService( - requestService, - envConfig - ); - }); it('should configure a new EndpointMapRequest', () => { (service as any).getRootEndpointMap(); @@ -37,8 +48,8 @@ describe('HALEndpointService', () => { it('should return an Observable of the endpoint map', () => { const result = (service as any).getRootEndpointMap(); - const expected = cold('b-', { b: endpointMap }); - expect(result).toBeObservable(expected); + const expected = '(b|)'; + getTestScheduler().expectObservable(result).toBe(expected, { b: endpointMap }); }); }); diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index 8ef65c4dd1..3a5da84e13 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -1,9 +1,8 @@ -import { Observable, of as observableOf } from 'rxjs'; +import { Observable, of as observableOf, combineLatest as observableCombineLatest } from 'rxjs'; import { distinctUntilChanged, - filter, - flatMap, map, + mergeMap, startWith, switchMap, tap @@ -11,12 +10,13 @@ import { import { RequestService } from '../data/request.service'; import { GlobalConfig } from '../../../config/global-config.interface'; import { EndpointMapRequest } from '../data/request.models'; -import { hasValue, isEmpty, isNotEmpty, isNotUndefined } from '../../shared/empty.util'; +import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; import { Inject, Injectable } from '@angular/core'; import { GLOBAL_CONFIG } from '../../../config'; import { EndpointMap, EndpointMapSuccessResponse } from '../cache/response.models'; import { getResponseFromEntry } from './operators'; +import { URLCombiner } from '../url-combiner/url-combiner'; @Injectable() export class HALEndpointService { @@ -36,48 +36,41 @@ export class HALEndpointService { private getEndpointMapAt(href): Observable { const request = new EndpointMapRequest(this.requestService.generateRequestId(), href); - this.requestService.getByUUID(request.uuid).pipe( - getResponseFromEntry(), - map((response: EndpointMapSuccessResponse) => response.endpointMap), - distinctUntilChanged()).subscribe((t) => console.log('uuid', t)); - this.requestService.getByHref(request.href).pipe( - getResponseFromEntry(), - map((response: EndpointMapSuccessResponse) => response.endpointMap), - distinctUntilChanged()).subscribe((t) => console.log('href', t)); - this.requestService.configure(request); - return this.requestService.getByHref(request.href).pipe( /*<-- changing this to UUID breaks it */ + return this.requestService.getByHref(request.href).pipe( getResponseFromEntry(), map((response: EndpointMapSuccessResponse) => response.endpointMap), distinctUntilChanged()); - } public getEndpoint(linkPath: string): Observable { - return this.getEndpointAt(...linkPath.split('/')); + return this.getEndpointAt(this.getRootHref(), ...linkPath.split('/')); } - private getEndpointAt(...path: string[]): Observable { - if (isEmpty(path)) { - path = ['/']; + private getEndpointAt(href: string, ...halNames: string[]): Observable { + if (isEmpty(halNames)) { + throw new Error('cant\'t fetch the URL without the HAL link names') + } + + const nextHref$ = this.getEndpointMapAt(href).pipe( + map((endpointMap: EndpointMap): string => { + /*TODO remove if/else block once the rest response contains _links for facets*/ + const nextName = halNames[0]; + if (hasValue(endpointMap) && hasValue(endpointMap[nextName])) { + return endpointMap[nextName]; + } else { + return new URLCombiner(href, nextName).toString(); + } + }) + ) as Observable; + + if (halNames.length === 1) { + return nextHref$; + } else { + return nextHref$.pipe( + switchMap((nextHref) => this.getEndpointAt(nextHref, ...halNames.slice(1))) + ); } - let currentPath; - const pipeArguments = path - .map((subPath: string, index: number) => [ - switchMap((href: string) => this.getEndpointMapAt(href)), - map((endpointMap: EndpointMap) => { - if (hasValue(endpointMap) && hasValue(endpointMap[subPath])) { - currentPath = endpointMap[subPath]; - return endpointMap[subPath]; - } else { - /*TODO remove if/else block once the rest response contains _links for facets*/ - currentPath += '/' + subPath; - return currentPath; - } - }), - ]) - .reduce((combined, thisElement) => [...combined, ...thisElement], []); - return observableOf(this.getRootHref()).pipe(...pipeArguments, distinctUntilChanged()); } public isEnabledOnRestApi(linkPath: string): Observable { diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 0684308fe9..5f29de9e93 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -25,6 +25,14 @@ describe('Core Module - RxJS Operators', () => { e: { response: { isSuccessful: 1, resourceSelfLinks: [] } } }; + const testResponses = { + a: testRCEs.a.response, + b: testRCEs.b.response, + c: testRCEs.c.response, + d: testRCEs.d.response, + e: testRCEs.e.response + }; + beforeEach(() => { scheduler = getTestScheduler(); }); @@ -66,7 +74,7 @@ describe('Core Module - RxJS Operators', () => { it('should only return responses for which isSuccessful === true', () => { const source = hot('abcde', testRCEs); const result = source.pipe(filterSuccessfulResponses()); - const expected = cold('a--d-', testRCEs); + const expected = cold('a--d-', testResponses); expect(result).toBeObservable(expected) }); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index e9ab8794ff..a6335ebb5d 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -58,6 +58,7 @@ export const getSucceededRemoteData = () => export const toDSpaceObjectListRD = () => (source: Observable>>>): Observable>> => source.pipe( + filter((rd: RemoteData>>) => rd.hasSucceeded), map((rd: RemoteData>>) => { const dsoPage: T[] = rd.payload.page.map((searchResult: SearchResult) => searchResult.dspaceObject); const payload = Object.assign(rd.payload, { page: dsoPage }) as PaginatedList;