From 98a49b3191dbafad3ac4f429daf74112c09501a2 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 29 Nov 2017 16:47:40 +0100 Subject: [PATCH 01/11] add support for multiple request methods --- src/app/core/cache/object-cache.reducer.ts | 11 +++++++ src/app/core/data/data.service.ts | 29 ++++++++++++++++-- src/app/core/data/request.effects.ts | 30 ++++++++++++++----- src/app/core/data/request.models.ts | 12 ++++++++ .../dspace-rest-v2/dspace-rest-v2.service.ts | 25 +++++++++++++++- 5 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts index 3af7209b24..39c623deed 100644 --- a/src/app/core/cache/object-cache.reducer.ts +++ b/src/app/core/cache/object-cache.reducer.ts @@ -5,6 +5,12 @@ import { import { hasValue } from '../../shared/empty.util'; import { CacheEntry } from './cache-entry'; +export enum DirtyType { + Created = 'Created', + Updated = 'Updated', + Deleted = 'Deleted' +} + /** * An interface to represent objects that can be cached * @@ -13,6 +19,11 @@ import { CacheEntry } from './cache-entry'; export interface CacheableObject { uuid?: string; self: string; + // isNew: boolean; + // dirtyType: DirtyType; + // hasDirtyAttributes: boolean; + // changedAttributes: AttributeDiffh; + // save(): void; } /** diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index e2f41f5962..5751173054 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -6,12 +6,19 @@ import { RemoteDataBuildService } from '../cache/builders/remote-data-build.serv import { CacheableObject } from '../cache/object-cache.reducer'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; +import { DSpaceObject } from '../shared/dspace-object.model'; import { GenericConstructor } from '../shared/generic-constructor'; import { HALEndpointService } from '../shared/hal-endpoint.service'; -import { RemoteData } from './remote-data'; -import { FindAllOptions, FindAllRequest, FindByIDRequest, RestRequest } from './request.models'; -import { RequestService } from './request.service'; import { URLCombiner } from '../url-combiner/url-combiner'; +import { RemoteData } from './remote-data'; +import { + FindAllOptions, + FindAllRequest, + FindByIDRequest, + RestRequest, + RestRequestMethod +} from './request.models'; +import { RequestService } from './request.service'; export abstract class DataService extends HALEndpointService { protected abstract responseCache: ResponseCacheService; @@ -102,4 +109,20 @@ export abstract class DataService return this.rdbService.buildSingle(href, this.normalizedResourceType); } + create(dso: DSpaceObject): Observable> { + const postHrefObs = this.getEndpoint(); + + // TODO ID is unknown at this point + const idHrefObs = postHrefObs.map((href: string) => this.getFindByIDHref(href, dso.id)); + + postHrefObs + .filter((href: string) => hasValue(href)) + .take(1) + .subscribe((href: string) => { + const request = new RestRequest(href, RestRequestMethod.Post, dso); + this.requestService.configure(request); + }); + + return this.rdbService.buildSingle(idHrefObs, this.normalizedResourceType); + } } diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index 84f19679b1..f848d7f0d7 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -1,18 +1,23 @@ import { Inject, Injectable, Injector } from '@angular/core'; +import { Request } from '@angular/http'; +import { RequestArgs } from '@angular/http/src/interfaces'; import { Actions, Effect } from '@ngrx/effects'; // tslint:disable-next-line:import-blacklist import { Observable } from 'rxjs'; import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; +import { isNotEmpty } from '../../shared/empty.util'; import { ErrorResponse, RestResponse } from '../cache/response-cache.models'; import { ResponseCacheService } from '../cache/response-cache.service'; import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; import { DSpaceRESTv2Service } from '../dspace-rest-v2/dspace-rest-v2.service'; import { RequestActionTypes, RequestCompleteAction, RequestExecuteAction } from './request.actions'; -import { RequestError } from './request.models'; +import { RequestError, RestRequest } from './request.models'; import { RequestEntry } from './request.reducer'; import { RequestService } from './request.service'; +import { DSpaceRESTv2Serializer } from '../dspace-rest-v2/dspace-rest-v2.serializer'; +import { NormalizedObjectFactory } from '../cache/models/normalized-object-factory'; @Injectable() export class RequestEffects { @@ -23,15 +28,24 @@ export class RequestEffects { return this.requestService.get(action.payload) .take(1); }) - .flatMap((entry: RequestEntry) => { - return this.restApi.get(entry.request.href) + .map((entry: RequestEntry) => entry.request) + .flatMap((request: RestRequest) => { + const httpRequestConfig: RequestArgs = { + method: request.method, + url: request.href + }; + if (isNotEmpty(request.body)) { + const serializer = new DSpaceRESTv2Serializer(NormalizedObjectFactory.getConstructor(request.body.type)); + httpRequestConfig.body = JSON.stringify(serializer.serialize(request.body)); + } + return this.restApi.request(new Request(httpRequestConfig)) .map((data: DSpaceRESTV2Response) => - this.injector.get(entry.request.getResponseParser()).parse(entry.request, data)) - .do((response: RestResponse) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) - .map((response: RestResponse) => new RequestCompleteAction(entry.request.href)) + this.injector.get(request.getResponseParser()).parse(request, data)) + .do((response: RestResponse) => this.responseCache.add(request.href, response, this.EnvConfig.cache.msToLive)) + .map((response: RestResponse) => new RequestCompleteAction(request.href)) .catch((error: RequestError) => Observable.of(new ErrorResponse(error)) - .do((response: RestResponse) => this.responseCache.add(entry.request.href, response, this.EnvConfig.cache.msToLive)) - .map((response: RestResponse) => new RequestCompleteAction(entry.request.href))); + .do((response: RestResponse) => this.responseCache.add(request.href, response, this.EnvConfig.cache.msToLive)) + .map((response: RestResponse) => new RequestCompleteAction(request.href))); }); constructor( diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index a80eccfaa8..3ab7dc0b8c 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -9,9 +9,21 @@ import { BrowseResponseParsingService } from './browse-response-parsing.service' import { ConfigResponseParsingService } from './config-response-parsing.service'; /* tslint:disable:max-classes-per-file */ +export enum RestRequestMethod { + Get = 'GET', + Post = 'POST', + Put = 'PUT', + Delete = 'DELETE', + Options = 'OPTIONS', + Head = 'HEAD', + Patch = 'PATCH' +} + export class RestRequest { constructor( public href: string, + public method: RestRequestMethod = RestRequestMethod.Get, + public body?: any ) { } getResponseParser(): GenericConstructor { diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts index 6464268201..bc9d5eddaf 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2.service.ts @@ -1,11 +1,12 @@ import { Inject, Injectable } from '@angular/core'; -import { Http, RequestOptionsArgs } from '@angular/http'; +import { Http, RequestOptionsArgs, Request } from '@angular/http'; import { Observable } from 'rxjs/Observable'; import { RESTURLCombiner } from '../url-combiner/rest-url-combiner'; import { DSpaceRESTV2Response } from './dspace-rest-v2-response.model'; import { GLOBAL_CONFIG, GlobalConfig } from '../../../config'; +import { RestRequest } from '../data/request.models'; /** * Service to access DSpace's REST API @@ -36,4 +37,26 @@ export class DSpaceRESTv2Service { }); } + /** + * Performs a request to the REST API. + * + * @param httpRequest + * A Request object + * @return {Observable} + * An Observable containing the response from the server + */ + request(httpRequest: Request): Observable { + // const httpRequest = new Request({ + // method: request.method, + // url: request.href, + // body: request.body + // }); + return this.http.request(httpRequest) + .map((res) => ({ payload: res.json(), statusCode: res.statusText })) + .catch((err) => { + console.log('Error: ', err); + return Observable.throw(err); + }); + } + } From d775467fcb62c5216a99d894a25925a709fa8e3f Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 7 Dec 2017 11:28:44 +0100 Subject: [PATCH 02/11] cleaned up remotedata for a better separation of concerns, moved statuscode and errormsg in to RemoteDataError object, moved pageInfo to PaginatedList object in the payload --- .../collection-page.component.ts | 3 +- ...ty-page-sub-collection-list.component.html | 2 +- ...nity-page-sub-collection-list.component.ts | 5 +- .../top-level-community-list.component.ts | 3 +- .../+search-page/search-page.component.html | 4 +- src/app/+search-page/search-page.component.ts | 3 +- .../search-service/search.service.ts | 66 +++++------------ .../builders/remote-data-build.service.ts | 70 ++++++++++--------- src/app/core/data/data.service.ts | 5 +- src/app/core/data/paginated-list.ts | 42 +++++++++++ src/app/core/data/remote-data-error.ts | 7 ++ src/app/core/data/remote-data.ts | 7 +- src/app/core/data/request.models.ts | 20 ++++-- src/app/core/data/request.service.ts | 28 +++++--- .../core/metadata/metadata.service.spec.ts | 6 +- .../object-list/object-list.component.html | 6 +- src/app/object-list/object-list.component.ts | 13 ++-- 17 files changed, 162 insertions(+), 128 deletions(-) create mode 100644 src/app/core/data/paginated-list.ts create mode 100644 src/app/core/data/remote-data-error.ts diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index 853bd0d154..de7e9a72d4 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -6,6 +6,7 @@ import { Subscription } from 'rxjs/Subscription'; import { SortOptions } from '../core/cache/models/sort-options.model'; import { CollectionDataService } from '../core/data/collection-data.service'; import { ItemDataService } from '../core/data/item-data.service'; +import { PaginatedList } from '../core/data/paginated-list'; import { RemoteData } from '../core/data/remote-data'; import { MetadataService } from '../core/metadata/metadata.service'; @@ -30,7 +31,7 @@ import { PaginationComponentOptions } from '../shared/pagination/pagination-comp }) export class CollectionPageComponent implements OnInit, OnDestroy { collectionRDObs: Observable>; - itemRDObs: Observable>; + itemRDObs: Observable>>; logoRDObs: Observable>; paginationConfig: PaginationComponentOptions; sortConfig: SortOptions; diff --git a/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.html b/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.html index b04e93ff71..8e2d04c5cd 100644 --- a/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.html +++ b/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.html @@ -2,7 +2,7 @@

{{'community.sub-collection-list.head' | translate}}

    -
  • +

  • {{collection.name}}
    {{collection.shortDescription}} diff --git a/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.ts b/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.ts index 8edc275437..cb371617c9 100644 --- a/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.ts +++ b/src/app/+community-page/sub-collection-list/community-page-sub-collection-list.component.ts @@ -1,11 +1,12 @@ import { Component, OnInit } from '@angular/core'; +import { Observable } from 'rxjs/Observable'; import { CollectionDataService } from '../../core/data/collection-data.service'; +import { PaginatedList } from '../../core/data/paginated-list'; import { RemoteData } from '../../core/data/remote-data'; import { Collection } from '../../core/shared/collection.model'; import { fadeIn } from '../../shared/animations/fade'; -import { Observable } from 'rxjs/Observable'; @Component({ selector: 'ds-community-page-sub-collection-list', @@ -14,7 +15,7 @@ import { Observable } from 'rxjs/Observable'; animations:[fadeIn] }) export class CommunityPageSubCollectionListComponent implements OnInit { - subCollectionsRDObs: Observable>; + subCollectionsRDObs: Observable>>; constructor(private cds: CollectionDataService) { diff --git a/src/app/+home-page/top-level-community-list/top-level-community-list.component.ts b/src/app/+home-page/top-level-community-list/top-level-community-list.component.ts index b364985fc1..1b71220382 100644 --- a/src/app/+home-page/top-level-community-list/top-level-community-list.component.ts +++ b/src/app/+home-page/top-level-community-list/top-level-community-list.component.ts @@ -2,6 +2,7 @@ import { ChangeDetectionStrategy, Component } from '@angular/core'; import { Observable } from 'rxjs/Observable'; import { SortOptions } from '../../core/cache/models/sort-options.model'; import { CommunityDataService } from '../../core/data/community-data.service'; +import { PaginatedList } from '../../core/data/paginated-list'; import { RemoteData } from '../../core/data/remote-data'; import { Community } from '../../core/shared/community.model'; @@ -17,7 +18,7 @@ import { PaginationComponentOptions } from '../../shared/pagination/pagination-c animations: [fadeInOut] }) export class TopLevelCommunityListComponent { - communitiesRDObs: Observable>; + communitiesRDObs: Observable>>; config: PaginationComponentOptions; sortConfig: SortOptions; diff --git a/src/app/+search-page/search-page.component.html b/src/app/+search-page/search-page.component.html index c4d679f72b..953de02ab4 100644 --- a/src/app/+search-page/search-page.component.html +++ b/src/app/+search-page/search-page.component.html @@ -8,7 +8,7 @@ [query]="query" [scope]="(scopeObjectRDObs | async)?.payload" [currentParams]="currentParams" - [scopes]="(scopeListRDObs | async)?.payload"> + [scopes]="(scopeListRDObs | async)?.payload?.page">

    -
diff --git a/src/app/+search-page/search-page.component.ts b/src/app/+search-page/search-page.component.ts index 153402d11f..3dcd0ccccf 100644 --- a/src/app/+search-page/search-page.component.ts +++ b/src/app/+search-page/search-page.component.ts @@ -2,6 +2,7 @@ import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/ import { ActivatedRoute } from '@angular/router'; import { Observable } from 'rxjs/Observable'; import { CommunityDataService } from '../core/data/community-data.service'; +import { PaginatedList } from '../core/data/paginated-list'; import { RemoteData } from '../core/data/remote-data'; import { Community } from '../core/shared/community.model'; import { DSpaceObject } from '../core/shared/dspace-object.model'; @@ -36,7 +37,7 @@ export class SearchPageComponent implements OnInit, OnDestroy { resultsRDObs: Observable>>>; currentParams = {}; searchOptions: SearchOptions; - scopeListRDObs: Observable>; + scopeListRDObs: Observable>>; isMobileView: Observable; constructor(private service: SearchService, diff --git a/src/app/+search-page/search-service/search.service.ts b/src/app/+search-page/search-service/search.service.ts index 4b5ba7b702..a6648fedd7 100644 --- a/src/app/+search-page/search-service/search.service.ts +++ b/src/app/+search-page/search-service/search.service.ts @@ -1,6 +1,8 @@ import { Injectable, OnDestroy } from '@angular/core'; +import { PaginatedList } from '../../core/data/paginated-list'; import { RemoteData } from '../../core/data/remote-data'; import { Observable } from 'rxjs/Observable'; +import { RemoteDataError } from '../../core/data/remote-data-error'; import { SearchResult } from '../search-result.model'; import { ItemDataService } from '../../core/data/item-data.service'; import { PageInfo } from '../../core/shared/page-info.model'; @@ -100,26 +102,7 @@ export class SearchService implements OnDestroy { } search(query: string, scopeId?: string, searchOptions?: SearchOptions): Observable>>> { - this.searchOptions = this.searchOptions; - let self = `https://dspace7.4science.it/dspace-spring-rest/api/search?query=${query}`; - if (hasValue(scopeId)) { - self += `&scope=${scopeId}`; - } - if (isNotEmpty(searchOptions) && hasValue(searchOptions.pagination.currentPage)) { - self += `&page=${searchOptions.pagination.currentPage}`; - } - if (isNotEmpty(searchOptions) && hasValue(searchOptions.pagination.pageSize)) { - self += `&pageSize=${searchOptions.pagination.pageSize}`; - } - if (isNotEmpty(searchOptions) && hasValue(searchOptions.sort.direction)) { - self += `&sortDirection=${searchOptions.sort.direction}`; - } - if (isNotEmpty(searchOptions) && hasValue(searchOptions.sort.field)) { - self += `&sortField=${searchOptions.sort.field}`; - } - - const errorMessage = undefined; - const statusCode = '200'; + const error = new RemoteDataError('200', undefined); const returningPageInfo = new PageInfo(); if (isNotEmpty(searchOptions)) { @@ -137,13 +120,12 @@ export class SearchService implements OnDestroy { }); return itemsObs - .filter((rd: RemoteData) => rd.hasSucceeded) - .map((rd: RemoteData) => { + .filter((rd: RemoteData>) => rd.hasSucceeded) + .map((rd: RemoteData>) => { - const totalElements = rd.pageInfo.totalElements > 20 ? 20 : rd.pageInfo.totalElements; - const pageInfo = Object.assign({}, rd.pageInfo, { totalElements: totalElements }); + const totalElements = rd.payload.totalElements > 20 ? 20 : rd.payload.totalElements; - const payload = shuffle(rd.payload) + const page = shuffle(rd.payload.page) .map((item: Item, index: number) => { const mockResult: SearchResult = new ItemSearchResult(); mockResult.dspaceObject = item; @@ -154,24 +136,20 @@ export class SearchService implements OnDestroy { return mockResult; }); + const payload = Object.assign({}, rd.payload, { totalElements: totalElements, page }); + return new RemoteData( - self, rd.isRequestPending, rd.isResponsePending, rd.hasSucceeded, - errorMessage, - statusCode, - pageInfo, + error, payload ) }).startWith(new RemoteData( - '', true, false, undefined, undefined, - undefined, - undefined, undefined )); } @@ -180,17 +158,12 @@ export class SearchService implements OnDestroy { const requestPending = false; const responsePending = false; const isSuccessful = true; - const errorMessage = undefined; - const statusCode = '200'; - const returningPageInfo = new PageInfo(); + const error = new RemoteDataError('200', undefined); return Observable.of(new RemoteData( - 'https://dspace7.4science.it/dspace-spring-rest/api/search', requestPending, responsePending, isSuccessful, - errorMessage, - statusCode, - returningPageInfo, + error, this.config )); } @@ -198,12 +171,12 @@ export class SearchService implements OnDestroy { getFacetValuesFor(searchFilterConfigName: string): Observable> { const filterConfig = this.config.find((config: SearchFilterConfig) => config.name === searchFilterConfigName); return this.routeService.getQueryParameterValues(filterConfig.paramName).map((selectedValues: string[]) => { - const values: FacetValue[] = []; + const payload: FacetValue[] = []; const totalFilters = 13; for (let i = 0; i < totalFilters; i++) { const value = searchFilterConfigName + ' ' + (i + 1); if (!selectedValues.includes(value)) { - values.push({ + payload.push({ value: value, count: Math.floor(Math.random() * 20) + 20 * (totalFilters - i), // make sure first results have the highest (random) count search: decodeURI(this.router.url) + (this.router.url.includes('?') ? '&' : '?') + filterConfig.paramName + '=' + value @@ -213,18 +186,13 @@ export class SearchService implements OnDestroy { const requestPending = false; const responsePending = false; const isSuccessful = true; - const errorMessage = undefined; - const statusCode = '200'; - const returningPageInfo = new PageInfo(); + const error = new RemoteDataError('200', undefined);; return new RemoteData( - 'https://dspace7.4science.it/dspace-spring-rest/api/search', requestPending, responsePending, isSuccessful, - errorMessage, - statusCode, - returningPageInfo, - values + error, + payload ) } ) 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 2e3fc01b52..eee6808d86 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -1,5 +1,7 @@ import { Injectable } from '@angular/core'; import { Observable } from 'rxjs/Observable'; +import { PaginatedList } from '../../data/paginated-list'; +import { RemoteDataError } from '../../data/remote-data-error'; import { CacheableObject } from '../object-cache.reducer'; import { ObjectCacheService } from '../object-cache.service'; @@ -88,32 +90,18 @@ export class RemoteDataBuildService { const requestPending = hasValue(reqEntry.requestPending) ? reqEntry.requestPending : true; const responsePending = hasValue(reqEntry.responsePending) ? reqEntry.responsePending : false; let isSuccessFul: boolean; - let errorMessage: string; - let statusCode: string; - let pageInfo: PageInfo; + let error: RemoteDataError; if (hasValue(resEntry) && hasValue(resEntry.response)) { isSuccessFul = resEntry.response.isSuccessful; - errorMessage = isSuccessFul === false ? (resEntry.response as ErrorResponse).errorMessage : undefined; - statusCode = resEntry.response.statusCode; - - if (hasValue((resEntry.response as DSOSuccessResponse).pageInfo)) { - const resPageInfo = (resEntry.response as DSOSuccessResponse).pageInfo; - if (isNotEmpty(resPageInfo) && resPageInfo.currentPage >= 0) { - pageInfo = Object.assign({}, resPageInfo, { currentPage: resPageInfo.currentPage + 1 }); - } else { - pageInfo = resPageInfo; - } - } + const errorMessage = isSuccessFul === false ? (resEntry.response as ErrorResponse).errorMessage : undefined; + error = new RemoteDataError(resEntry.response.statusCode, errorMessage); } return new RemoteData( - href, requestPending, responsePending, isSuccessFul, - errorMessage, - statusCode, - pageInfo, + error, payload ); }); @@ -122,7 +110,7 @@ export class RemoteDataBuildService { buildList( hrefObs: string | Observable, normalizedType: GenericConstructor - ): Observable> { + ): Observable>> { if (typeof hrefObs === 'string') { hrefObs = Observable.of(hrefObs); } @@ -132,7 +120,7 @@ export class RemoteDataBuildService { const responseCacheObs = hrefObs.flatMap((href: string) => this.responseCache.get(href)) .filter((entry) => hasValue(entry)); - const payloadObs = responseCacheObs + const tDomainListObs = responseCacheObs .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) .map((entry: ResponseCacheEntry) => (entry.response as DSOSuccessResponse).resourceSelfLinks) .flatMap((resourceUUIDs: string[]) => { @@ -146,6 +134,27 @@ export class RemoteDataBuildService { .startWith([]) .distinctUntilChanged(); + const pageInfoObs = responseCacheObs + .filter((entry: ResponseCacheEntry) => entry.response.isSuccessful) + .map((entry: ResponseCacheEntry) => { + if (hasValue((entry.response as DSOSuccessResponse).pageInfo)) { + const resPageInfo = (entry.response as DSOSuccessResponse).pageInfo; + if (isNotEmpty(resPageInfo) && resPageInfo.currentPage >= 0) { + return Object.assign({}, resPageInfo, { currentPage: resPageInfo.currentPage + 1 }); + } else { + return resPageInfo; + } + } + }); + + const payloadObs = Observable.combineLatest(tDomainListObs, pageInfoObs, (tDomainList, pageInfo) => { + if (hasValue(pageInfo)) { + return new PaginatedList(pageInfo, tDomainList); + } else { + return tDomainList; + } + }); + return this.toRemoteDataObservable(hrefObs, requestObs, responseCacheObs, payloadObs); } @@ -209,35 +218,32 @@ export class RemoteDataBuildService { .every((b: boolean) => b === true); const errorMessage: string = arr - .map((d: RemoteData) => d.errorMessage) - .map((e: string, idx: number) => { + .map((d: RemoteData) => d.error) + .map((e: RemoteDataError, idx: number) => { if (hasValue(e)) { - return `[${idx}]: ${e}`; + return `[${idx}]: ${e.message}`; } }).filter((e: string) => hasValue(e)) .join(', '); const statusCode: string = arr - .map((d: RemoteData) => d.statusCode) - .map((c: string, idx: number) => { - if (hasValue(c)) { - return `[${idx}]: ${c}`; + .map((d: RemoteData) => d.error) + .map((e: RemoteDataError, idx: number) => { + if (hasValue(e)) { + return `[${idx}]: ${e.statusCode}`; } }).filter((c: string) => hasValue(c)) .join(', '); - const pageInfo = undefined; + const error = new RemoteDataError(statusCode, errorMessage); const payload: T[] = arr.map((d: RemoteData) => d.payload); return new RemoteData( - `dspace-angular://aggregated/object/${new Date().getTime()}`, requestPending, responsePending, isSuccessFul, - errorMessage, - statusCode, - pageInfo, + error, payload ); }) diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 5751173054..68fa2657f5 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -10,6 +10,7 @@ import { DSpaceObject } from '../shared/dspace-object.model'; import { GenericConstructor } from '../shared/generic-constructor'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { URLCombiner } from '../url-combiner/url-combiner'; +import { PaginatedList } from './paginated-list'; import { RemoteData } from './remote-data'; import { FindAllOptions, @@ -70,7 +71,7 @@ export abstract class DataService } } - findAll(options: FindAllOptions = {}): Observable> { + findAll(options: FindAllOptions = {}): Observable>> { const hrefObs = this.getEndpoint().filter((href: string) => isNotEmpty(href)) .flatMap((endpoint: string) => this.getFindAllHref(endpoint, options)); @@ -82,7 +83,7 @@ export abstract class DataService this.requestService.configure(request); }); - return this.rdbService.buildList(hrefObs, this.normalizedResourceType); + return this.rdbService.buildList(hrefObs, this.normalizedResourceType) as Observable>>; } getFindByIDHref(endpoint, resourceID): string { diff --git a/src/app/core/data/paginated-list.ts b/src/app/core/data/paginated-list.ts new file mode 100644 index 0000000000..f1d076927d --- /dev/null +++ b/src/app/core/data/paginated-list.ts @@ -0,0 +1,42 @@ +import { PageInfo } from '../shared/page-info.model'; + +export class PaginatedList { + + constructor( + private pageInfo: PageInfo, + public page: T[] + ) { + } + + get elementsPerPage(): number { + return this.pageInfo.elementsPerPage; + } + + set elementsPerPage(value: number) { + this.pageInfo.elementsPerPage = value; + } + + get totalElements(): number { + return this.pageInfo.totalElements; + } + + set totalElements(value: number) { + this.pageInfo.totalElements = value; + } + + get totalPages(): number { + return this.pageInfo.totalPages; + } + + set totalPages(value: number) { + this.pageInfo.totalPages = value; + } + + get currentPage(): number { + return this.pageInfo.currentPage; + } + + set currentPage(value: number) { + this.pageInfo.currentPage = value; + } +} diff --git a/src/app/core/data/remote-data-error.ts b/src/app/core/data/remote-data-error.ts new file mode 100644 index 0000000000..a2ff27a073 --- /dev/null +++ b/src/app/core/data/remote-data-error.ts @@ -0,0 +1,7 @@ +export class RemoteDataError { + constructor( + public statusCode: string, + public message: string + ) { + } +} diff --git a/src/app/core/data/remote-data.ts b/src/app/core/data/remote-data.ts index d8a2f79e66..41953260ac 100644 --- a/src/app/core/data/remote-data.ts +++ b/src/app/core/data/remote-data.ts @@ -1,5 +1,5 @@ -import { PageInfo } from '../shared/page-info.model'; import { hasValue } from '../../shared/empty.util'; +import { RemoteDataError } from './remote-data-error'; export enum RemoteDataState { RequestPending = 'RequestPending', @@ -13,13 +13,10 @@ export enum RemoteDataState { */ export class RemoteData { constructor( - public self: string, private requestPending: boolean, private responsePending: boolean, private isSuccessFul: boolean, - public errorMessage: string, - public statusCode: string, - public pageInfo: PageInfo, + public error: RemoteDataError, public payload: T ) { } diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 3ab7dc0b8c..3a7141d22d 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -9,14 +9,24 @@ import { BrowseResponseParsingService } from './browse-response-parsing.service' import { ConfigResponseParsingService } from './config-response-parsing.service'; /* tslint:disable:max-classes-per-file */ + +/** + * Represents a Request Method. + * + * I didn't reuse the RequestMethod enum in @angular/http because + * it uses numbers. The string values here are more clear when + * debugging. + * + * The ones commented out are still unsupported in the rest of the codebase + */ export enum RestRequestMethod { Get = 'GET', Post = 'POST', - Put = 'PUT', - Delete = 'DELETE', - Options = 'OPTIONS', - Head = 'HEAD', - Patch = 'PATCH' + // Put = 'PUT', + // Delete = 'DELETE', + // Options = 'OPTIONS', + // Head = 'HEAD', + // Patch = 'PATCH' } export class RestRequest { diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 0eee771a52..a075244648 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -12,7 +12,7 @@ import { ResponseCacheService } from '../cache/response-cache.service'; import { coreSelector, CoreState } from '../core.reducers'; import { keySelector } from '../shared/selectors'; import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; -import { RestRequest } from './request.models'; +import { RestRequest, RestRequestMethod } from './request.models'; import { RequestEntry, RequestState } from './request.reducer'; @@ -30,11 +30,9 @@ export function requestStateSelector(): MemoizedSelector - ) { + constructor(private objectCache: ObjectCacheService, + private responseCache: ResponseCacheService, + private store: Store) { } isPending(href: string): boolean { @@ -59,6 +57,12 @@ export class RequestService { } configure(request: RestRequest): void { + if (request.method !== RestRequestMethod.Get || !this.isCachedOrPending(request)) { + this.dispatchRequest(request); + } + } + + private isCachedOrPending(request: RestRequest) { let isCached = this.objectCache.hasBySelfLink(request.href); if (!isCached && this.responseCache.has(request.href)) { const [successResponse, errorResponse] = this.responseCache.get(request.href) @@ -84,11 +88,13 @@ export class RequestService { const isPending = this.isPending(request.href); - if (!(isCached || isPending)) { - this.store.dispatch(new RequestConfigureAction(request)); - this.store.dispatch(new RequestExecuteAction(request.href)); - this.trackRequestsOnTheirWayToTheStore(request.href); - } + return isCached || isPending; + } + + private dispatchRequest(request: RestRequest) { + this.store.dispatch(new RequestConfigureAction(request)); + this.store.dispatch(new RequestExecuteAction(request.href)); + this.trackRequestsOnTheirWayToTheStore(request.href); } /** diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 4c8775fcfb..110a46c1b9 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -11,6 +11,7 @@ import { TranslateModule, TranslateLoader } from '@ngx-translate/core'; import { Store, StoreModule } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; +import { RemoteDataError } from '../data/remote-data-error'; import { MetadataService } from './metadata.service'; @@ -178,13 +179,10 @@ describe('MetadataService', () => { const mockRemoteData = (mockItem: Item): Observable> => { return Observable.of(new RemoteData( - '', false, false, true, - '', - '200', - {} as PageInfo, + new RemoteDataError('200', ''), MockItem )); } diff --git a/src/app/object-list/object-list.component.html b/src/app/object-list/object-list.component.html index 0d488b5298..897342ae0d 100644 --- a/src/app/object-list/object-list.component.html +++ b/src/app/object-list/object-list.component.html @@ -1,7 +1,7 @@
    -
  • +
diff --git a/src/app/object-list/object-list.component.ts b/src/app/object-list/object-list.component.ts index 0f7decadd7..b298522ebc 100644 --- a/src/app/object-list/object-list.component.ts +++ b/src/app/object-list/object-list.component.ts @@ -8,13 +8,12 @@ import { } from '@angular/core'; import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; +import { PaginatedList } from '../core/data/paginated-list'; import { RemoteData } from '../core/data/remote-data'; -import { PageInfo } from '../core/shared/page-info.model'; -import { ListableObject } from '../object-list/listable-object/listable-object.model'; +import { ListableObject } from './listable-object/listable-object.model'; import { fadeIn } from '../shared/animations/fade'; -import { hasValue } from '../shared/empty.util'; import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model'; @@ -32,13 +31,9 @@ export class ObjectListComponent { @Input() sortConfig: SortOptions; @Input() hideGear = false; @Input() hidePagerWhenSinglePage = true; - private _objects: RemoteData; - pageInfo: PageInfo; - @Input() set objects(objects: RemoteData) { + private _objects: RemoteData>; + @Input() set objects(objects: RemoteData>) { this._objects = objects; - if (hasValue(objects)) { - this.pageInfo = objects.pageInfo; - } } get objects() { return this._objects; From 5f5d9eaeeedc03c3adcd8c88f0b0e067faa55b66 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Tue, 12 Dec 2017 17:45:32 +0100 Subject: [PATCH 03/11] Switched to storing requests based on UUID, generalized UUIDIndexReducer to work with any type of index --- package.json | 2 + .../+search-page/search-page.component.html | 2 +- src/app/core/browse/browse.service.spec.ts | 7 +- src/app/core/browse/browse.service.ts | 2 +- .../builders/remote-data-build.service.ts | 10 +-- src/app/core/cache/object-cache.service.ts | 11 +-- src/app/core/cache/response-cache.service.ts | 6 +- src/app/core/config/config.service.spec.ts | 11 ++- src/app/core/config/config.service.ts | 8 +-- src/app/core/core.effects.ts | 2 +- src/app/core/core.module.ts | 2 + src/app/core/core.reducers.ts | 6 +- .../browse-response-parsing.service.spec.ts | 2 +- src/app/core/data/comcol-data.service.spec.ts | 7 +- src/app/core/data/comcol-data.service.ts | 2 +- .../config-response-parsing.service.spec.ts | 2 +- src/app/core/data/data.service.ts | 8 +-- src/app/core/data/request.actions.ts | 18 +++-- src/app/core/data/request.effects.ts | 6 +- src/app/core/data/request.models.ts | 22 +++--- src/app/core/data/request.reducer.spec.ts | 39 ++++++----- src/app/core/data/request.reducer.ts | 4 +- src/app/core/data/request.service.ts | 46 ++++++++---- src/app/core/index/index.actions.ts | 69 ++++++++++++++++++ src/app/core/index/index.effects.ts | 61 ++++++++++++++++ src/app/core/index/index.reducer.spec.ts | 58 +++++++++++++++ src/app/core/index/index.reducer.ts | 62 ++++++++++++++++ src/app/core/index/uuid-index.actions.ts | 60 ---------------- src/app/core/index/uuid-index.effects.ts | 34 --------- src/app/core/index/uuid-index.reducer.spec.ts | 56 --------------- src/app/core/index/uuid-index.reducer.ts | 46 ------------ .../core/shared/hal-endpoint.service.spec.ts | 5 +- src/app/core/shared/hal-endpoint.service.ts | 2 +- src/app/core/shared/selectors.ts | 24 ++++--- src/app/core/shared/uuid.service.ts | 9 +++ src/app/shared/mocks/mock-request.service.ts | 8 +++ yarn.lock | 70 ++++++++++--------- 37 files changed, 449 insertions(+), 340 deletions(-) create mode 100644 src/app/core/index/index.actions.ts create mode 100644 src/app/core/index/index.effects.ts create mode 100644 src/app/core/index/index.reducer.spec.ts create mode 100644 src/app/core/index/index.reducer.ts delete mode 100644 src/app/core/index/uuid-index.actions.ts delete mode 100644 src/app/core/index/uuid-index.effects.ts delete mode 100644 src/app/core/index/uuid-index.reducer.spec.ts delete mode 100644 src/app/core/index/uuid-index.reducer.ts create mode 100644 src/app/core/shared/uuid.service.ts create mode 100644 src/app/shared/mocks/mock-request.service.ts diff --git a/package.json b/package.json index f2bb074ff4..60bb3c8d09 100644 --- a/package.json +++ b/package.json @@ -107,6 +107,7 @@ "reflect-metadata": "0.1.10", "rxjs": "5.4.3", "ts-md5": "1.2.2", + "uuid": "^3.1.0", "webfontloader": "1.6.28", "zone.js": "0.8.18" }, @@ -126,6 +127,7 @@ "@types/node": "8.0.34", "@types/serve-static": "1.7.32", "@types/source-map": "0.5.1", + "@types/uuid": "^3.4.3", "@types/webfontloader": "1.6.29", "ajv": "5.2.3", "ajv-keywords": "2.1.0", diff --git a/src/app/+search-page/search-page.component.html b/src/app/+search-page/search-page.component.html index 953de02ab4..08fae7d19a 100644 --- a/src/app/+search-page/search-page.component.html +++ b/src/app/+search-page/search-page.component.html @@ -29,7 +29,7 @@ | translate}} - diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index 65b32b3c0b..30429c5a8c 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -1,3 +1,4 @@ +import { initMockRequestService } from '../../shared/mocks/mock-request.service'; import { BrowseService } from './browse.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { RequestService } from '../data/request.service'; @@ -85,10 +86,6 @@ describe('BrowseService', () => { }); } - function initMockRequestService() { - return jasmine.createSpyObj('requestService', ['configure']); - } - function initTestService() { return new BrowseService( responseCache, @@ -157,7 +154,7 @@ describe('BrowseService', () => { it('should configure a new BrowseEndpointRequest', () => { const metadatumKey = 'dc.date.issued'; const linkName = 'items'; - const expected = new BrowseEndpointRequest(browsesEndpointURL); + const expected = new BrowseEndpointRequest(requestService.generateRequestId(), browsesEndpointURL); scheduler.schedule(() => service.getBrowseURLFor(metadatumKey, linkName).subscribe()); scheduler.flush(); diff --git a/src/app/core/browse/browse.service.ts b/src/app/core/browse/browse.service.ts index 6d8d504b82..a321e14706 100644 --- a/src/app/core/browse/browse.service.ts +++ b/src/app/core/browse/browse.service.ts @@ -40,7 +40,7 @@ export class BrowseService extends HALEndpointService { return this.getEndpoint() .filter((href: string) => isNotEmpty(href)) .distinctUntilChanged() - .map((endpointURL: string) => new BrowseEndpointRequest(endpointURL)) + .map((endpointURL: string) => new BrowseEndpointRequest(this.requestService.generateRequestId(), endpointURL)) .do((request: RestRequest) => this.requestService.configure(request)) .flatMap((request: RestRequest) => { const [successResponse, errorResponse] = this.responseCache.get(request.href) 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 eee6808d86..6fb24328c2 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -39,10 +39,10 @@ export class RemoteDataBuildService { this.objectCache.getRequestHrefBySelfLink(href)); const requestObs = Observable.race( - hrefObs.flatMap((href: string) => this.requestService.get(href)) + hrefObs.flatMap((href: string) => this.requestService.getByHref(href)) .filter((entry) => hasValue(entry)), requestHrefObs.flatMap((requestHref) => - this.requestService.get(requestHref)).filter((entry) => hasValue(entry)) + this.requestService.getByHref(requestHref)).filter((entry) => hasValue(entry)) ); const responseCacheObs = Observable.race( @@ -115,7 +115,7 @@ export class RemoteDataBuildService { hrefObs = Observable.of(hrefObs); } - const requestObs = hrefObs.flatMap((href: string) => this.requestService.get(href)) + const requestObs = hrefObs.flatMap((href: string) => this.requestService.getByHref(href)) .filter((entry) => hasValue(entry)); const responseCacheObs = hrefObs.flatMap((href: string) => this.responseCache.get(href)) .filter((entry) => hasValue(entry)); @@ -169,7 +169,7 @@ export class RemoteDataBuildService { const resourceConstructor = NormalizedObjectFactory.getConstructor(resourceType); if (Array.isArray(normalized[relationship])) { normalized[relationship].forEach((href: string) => { - this.requestService.configure(new RestRequest(href)) + this.requestService.configure(new RestRequest(this.requestService.generateRequestId(), href)) }); const rdArr = []; @@ -183,7 +183,7 @@ export class RemoteDataBuildService { links[relationship] = rdArr[0]; } } else { - this.requestService.configure(new RestRequest(normalized[relationship])); + this.requestService.configure(new RestRequest(this.requestService.generateRequestId(), normalized[relationship])); // The rest API can return a single URL to represent a list of resources (e.g. /items/:id/bitstreams) // in that case only 1 href will be stored in the normalized obj (so the isArray above fails), diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 31eb2d0b6a..ae41c38fbe 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -2,20 +2,21 @@ import { Injectable } from '@angular/core'; import { MemoizedSelector, Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; +import { IndexName } from '../index/index.reducer'; import { ObjectCacheEntry, CacheableObject } from './object-cache.reducer'; import { AddToObjectCacheAction, RemoveFromObjectCacheAction } from './object-cache.actions'; import { hasNoValue } from '../../shared/empty.util'; import { GenericConstructor } from '../shared/generic-constructor'; -import { CoreState } from '../core.reducers'; -import { keySelector } from '../shared/selectors'; +import { coreSelector, CoreState } from '../core.reducers'; +import { pathSelector } from '../shared/selectors'; function selfLinkFromUuidSelector(uuid: string): MemoizedSelector { - return keySelector('index/uuid', uuid); + return pathSelector(coreSelector, 'index', IndexName.OBJECT, uuid); } function entryFromSelfLinkSelector(selfLink: string): MemoizedSelector { - return keySelector('data/object', selfLink); + return pathSelector(coreSelector, 'data/object', selfLink); } /** @@ -60,7 +61,7 @@ export class ObjectCacheService { * the cached plain javascript object in to an instance of * a class. * - * e.g. get('c96588c6-72d3-425d-9d47-fa896255a695', Item) + * e.g. getByUUID('c96588c6-72d3-425d-9d47-fa896255a695', Item) * * @param uuid * The UUID of the object to get diff --git a/src/app/core/cache/response-cache.service.ts b/src/app/core/cache/response-cache.service.ts index 65ce8b2bac..77a2402043 100644 --- a/src/app/core/cache/response-cache.service.ts +++ b/src/app/core/cache/response-cache.service.ts @@ -7,11 +7,11 @@ import { ResponseCacheEntry } from './response-cache.reducer'; import { hasNoValue } from '../../shared/empty.util'; import { ResponseCacheRemoveAction, ResponseCacheAddAction } from './response-cache.actions'; import { RestResponse } from './response-cache.models'; -import { CoreState } from '../core.reducers'; -import { keySelector } from '../shared/selectors'; +import { coreSelector, CoreState } from '../core.reducers'; +import { pathSelector } from '../shared/selectors'; function entryFromKeySelector(key: string): MemoizedSelector { - return keySelector('data/response', key); + return pathSelector(coreSelector, 'data/response', key); } /** diff --git a/src/app/core/config/config.service.spec.ts b/src/app/core/config/config.service.spec.ts index c0d02be82a..111e1d3eaa 100644 --- a/src/app/core/config/config.service.spec.ts +++ b/src/app/core/config/config.service.spec.ts @@ -1,6 +1,7 @@ import { cold, getTestScheduler, hot } from 'jasmine-marbles'; import { TestScheduler } from 'rxjs/Rx'; import { GlobalConfig } from '../../../config'; +import { initMockRequestService } from '../../shared/mocks/mock-request.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { ConfigService } from './config.service'; import { RequestService } from '../data/request.service'; @@ -38,10 +39,6 @@ describe('ConfigService', () => { const scopedEndpoint = `${serviceEndpoint}/${scopeName}`; const searchEndpoint = `${serviceEndpoint}/${BROWSE}?uuid=${scopeID}`; - function initMockRequestService(): RequestService { - return jasmine.createSpyObj('requestService', ['configure']); - } - function initMockResponseCacheService(isSuccessful: boolean): ResponseCacheService { return jasmine.createSpyObj('responseCache', { get: cold('c-', { @@ -70,7 +67,7 @@ describe('ConfigService', () => { describe('getConfigByHref', () => { it('should configure a new ConfigRequest', () => { - const expected = new ConfigRequest(scopedEndpoint); + const expected = new ConfigRequest(requestService.generateRequestId(), scopedEndpoint); scheduler.schedule(() => service.getConfigByHref(scopedEndpoint).subscribe()); scheduler.flush(); @@ -81,7 +78,7 @@ describe('ConfigService', () => { describe('getConfigByName', () => { it('should configure a new ConfigRequest', () => { - const expected = new ConfigRequest(scopedEndpoint); + const expected = new ConfigRequest(requestService.generateRequestId(), scopedEndpoint); scheduler.schedule(() => service.getConfigByName(scopeName).subscribe()); scheduler.flush(); @@ -93,7 +90,7 @@ describe('ConfigService', () => { it('should configure a new ConfigRequest', () => { findOptions.scopeID = scopeID; - const expected = new ConfigRequest(searchEndpoint); + const expected = new ConfigRequest(requestService.generateRequestId(), searchEndpoint); scheduler.schedule(() => service.getConfigBySearch(findOptions).subscribe()); scheduler.flush(); diff --git a/src/app/core/config/config.service.ts b/src/app/core/config/config.service.ts index 55c4055ed7..9ad4684300 100644 --- a/src/app/core/config/config.service.ts +++ b/src/app/core/config/config.service.ts @@ -75,14 +75,14 @@ export abstract class ConfigService extends HALEndpointService { return this.getEndpoint() .filter((href: string) => isNotEmpty(href)) .distinctUntilChanged() - .map((endpointURL: string) => new ConfigRequest(endpointURL)) + .map((endpointURL: string) => new ConfigRequest(this.requestService.generateRequestId(), endpointURL)) .do((request: RestRequest) => this.requestService.configure(request)) .flatMap((request: RestRequest) => this.getConfig(request)) .distinctUntilChanged(); } public getConfigByHref(href: string): Observable { - const request = new ConfigRequest(href); + const request = new ConfigRequest(this.requestService.generateRequestId(), href); this.requestService.configure(request); return this.getConfig(request); @@ -93,7 +93,7 @@ export abstract class ConfigService extends HALEndpointService { .map((endpoint: string) => this.getConfigByNameHref(endpoint, name)) .filter((href: string) => isNotEmpty(href)) .distinctUntilChanged() - .map((endpointURL: string) => new ConfigRequest(endpointURL)) + .map((endpointURL: string) => new ConfigRequest(this.requestService.generateRequestId(), endpointURL)) .do((request: RestRequest) => this.requestService.configure(request)) .flatMap((request: RestRequest) => this.getConfig(request)) .distinctUntilChanged(); @@ -104,7 +104,7 @@ export abstract class ConfigService extends HALEndpointService { .map((endpoint: string) => this.getConfigSearchHref(endpoint, options)) .filter((href: string) => isNotEmpty(href)) .distinctUntilChanged() - .map((endpointURL: string) => new ConfigRequest(endpointURL)) + .map((endpointURL: string) => new ConfigRequest(this.requestService.generateRequestId(), endpointURL)) .do((request: RestRequest) => this.requestService.configure(request)) .flatMap((request: RestRequest) => this.getConfig(request)) .distinctUntilChanged(); diff --git a/src/app/core/core.effects.ts b/src/app/core/core.effects.ts index f144f773a1..7cda10b4ae 100644 --- a/src/app/core/core.effects.ts +++ b/src/app/core/core.effects.ts @@ -1,7 +1,7 @@ import { ObjectCacheEffects } from './cache/object-cache.effects'; import { ResponseCacheEffects } from './cache/response-cache.effects'; -import { UUIDIndexEffects } from './index/uuid-index.effects'; +import { UUIDIndexEffects } from './index/index.effects'; import { RequestEffects } from './data/request.effects'; export const coreEffects = [ diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index f4c7e2bbcc..768f05f24b 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -37,6 +37,7 @@ import { RouteService } from '../shared/route.service'; import { SubmissionDefinitionsConfigService } from './config/submission-definitions-config.service'; import { SubmissionFormsConfigService } from './config/submission-forms-config.service'; import { SubmissionSectionsConfigService } from './config/submission-sections-config.service'; +import { UUIDService } from './shared/uuid.service'; const IMPORTS = [ CommonModule, @@ -75,6 +76,7 @@ const PROVIDERS = [ SubmissionDefinitionsConfigService, SubmissionFormsConfigService, SubmissionSectionsConfigService, + UUIDService, { provide: NativeWindowService, useFactory: NativeWindowFactory } ]; diff --git a/src/app/core/core.reducers.ts b/src/app/core/core.reducers.ts index 493c9e96d9..d2898eb3c3 100644 --- a/src/app/core/core.reducers.ts +++ b/src/app/core/core.reducers.ts @@ -2,21 +2,21 @@ import { ActionReducerMap, createFeatureSelector } from '@ngrx/store'; import { responseCacheReducer, ResponseCacheState } from './cache/response-cache.reducer'; import { objectCacheReducer, ObjectCacheState } from './cache/object-cache.reducer'; -import { uuidIndexReducer, UUIDIndexState } from './index/uuid-index.reducer'; +import { indexReducer, IndexState } from './index/index.reducer'; import { requestReducer, RequestState } from './data/request.reducer'; export interface CoreState { 'data/object': ObjectCacheState, 'data/response': ResponseCacheState, 'data/request': RequestState, - 'index/uuid': UUIDIndexState + 'index': IndexState } export const coreReducers: ActionReducerMap = { 'data/object': objectCacheReducer, 'data/response': responseCacheReducer, 'data/request': requestReducer, - 'index/uuid': uuidIndexReducer + 'index': indexReducer }; export const coreSelector = createFeatureSelector('core'); diff --git a/src/app/core/data/browse-response-parsing.service.spec.ts b/src/app/core/data/browse-response-parsing.service.spec.ts index 5f27519a93..6579621610 100644 --- a/src/app/core/data/browse-response-parsing.service.spec.ts +++ b/src/app/core/data/browse-response-parsing.service.spec.ts @@ -11,7 +11,7 @@ describe('BrowseResponseParsingService', () => { }); describe('parse', () => { - const validRequest = new BrowseEndpointRequest('https://rest.api/discover/browses'); + const validRequest = new BrowseEndpointRequest('clients/b186e8ce-e99c-4183-bc9a-42b4821bdb78', 'https://rest.api/discover/browses'); const validResponse = { payload: { diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index 0d78a9fa8d..d34ffc2277 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -2,6 +2,7 @@ import { Store } from '@ngrx/store'; import { cold, getTestScheduler, hot } from 'jasmine-marbles'; import { TestScheduler } from 'rxjs/Rx'; import { GlobalConfig } from '../../../config'; +import { initMockRequestService } from '../../shared/mocks/mock-request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { NormalizedCommunity } from '../cache/models/normalized-community.model'; import { CacheableObject } from '../cache/object-cache.reducer'; @@ -62,10 +63,6 @@ describe('ComColDataService', () => { }); } - function initMockRequestService(): RequestService { - return jasmine.createSpyObj('requestService', ['configure']); - } - function initMockResponseCacheService(isSuccessful: boolean): ResponseCacheService { return jasmine.createSpyObj('responseCache', { get: cold('c-', { @@ -110,7 +107,7 @@ describe('ComColDataService', () => { responseCache = initMockResponseCacheService(true); service = initTestService(); - const expected = new FindByIDRequest(communityEndpoint, scopeID); + const expected = new FindByIDRequest(requestService.generateRequestId(), communityEndpoint, scopeID); scheduler.schedule(() => service.getScopedEndpoint(scopeID).subscribe()); scheduler.flush(); diff --git a/src/app/core/data/comcol-data.service.ts b/src/app/core/data/comcol-data.service.ts index 17d2fb313c..68981121c1 100644 --- a/src/app/core/data/comcol-data.service.ts +++ b/src/app/core/data/comcol-data.service.ts @@ -33,7 +33,7 @@ export abstract class ComColDataService isNotEmpty(href)) .take(1) .do((href: string) => { - const request = new FindByIDRequest(href, scopeID); + const request = new FindByIDRequest(this.requestService.generateRequestId(), href, scopeID); this.requestService.configure(request); }); diff --git a/src/app/core/data/config-response-parsing.service.spec.ts b/src/app/core/data/config-response-parsing.service.spec.ts index 46e6d61f8f..dc5c42cbd5 100644 --- a/src/app/core/data/config-response-parsing.service.spec.ts +++ b/src/app/core/data/config-response-parsing.service.spec.ts @@ -21,7 +21,7 @@ describe('ConfigResponseParsingService', () => { }); describe('parse', () => { - const validRequest = new ConfigRequest('https://rest.api/config/submissiondefinitions/traditional'); + const validRequest = new ConfigRequest('69f375b5-19f4-4453-8c7a-7dc5c55aafbb', 'https://rest.api/config/submissiondefinitions/traditional'); const validResponse = { payload: { diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 68fa2657f5..3e86bb21b0 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -79,7 +79,7 @@ export abstract class DataService .filter((href: string) => hasValue(href)) .take(1) .subscribe((href: string) => { - const request = new FindAllRequest(href, options); + const request = new FindAllRequest(this.requestService.generateRequestId(), href, options); this.requestService.configure(request); }); @@ -98,7 +98,7 @@ export abstract class DataService .filter((href: string) => hasValue(href)) .take(1) .subscribe((href: string) => { - const request = new FindByIDRequest(href, id); + const request = new FindByIDRequest(this.requestService.generateRequestId(), href, id); this.requestService.configure(request); }); @@ -106,7 +106,7 @@ export abstract class DataService } findByHref(href: string): Observable> { - this.requestService.configure(new RestRequest(href)); + this.requestService.configure(new RestRequest(this.requestService.generateRequestId(), href)); return this.rdbService.buildSingle(href, this.normalizedResourceType); } @@ -120,7 +120,7 @@ export abstract class DataService .filter((href: string) => hasValue(href)) .take(1) .subscribe((href: string) => { - const request = new RestRequest(href, RestRequestMethod.Post, dso); + const request = new RestRequest(this.requestService.generateRequestId(), href, RestRequestMethod.Post, dso); this.requestService.configure(request); }); diff --git a/src/app/core/data/request.actions.ts b/src/app/core/data/request.actions.ts index 31f0dc5996..436c365caa 100644 --- a/src/app/core/data/request.actions.ts +++ b/src/app/core/data/request.actions.ts @@ -27,8 +27,14 @@ export class RequestExecuteAction implements Action { type = RequestActionTypes.EXECUTE; payload: string; - constructor(key: string) { - this.payload = key + /** + * Create a new RequestExecuteAction + * + * @param uuid + * the request's uuid + */ + constructor(uuid: string) { + this.payload = uuid } } @@ -42,11 +48,11 @@ export class RequestCompleteAction implements Action { /** * Create a new RequestCompleteAction * - * @param key - * the key under which this request is stored, + * @param uuid + * the request's uuid */ - constructor(key: string) { - this.payload = key; + constructor(uuid: string) { + this.payload = uuid; } } /* tslint:enable:max-classes-per-file */ diff --git a/src/app/core/data/request.effects.ts b/src/app/core/data/request.effects.ts index f848d7f0d7..815878b7bf 100644 --- a/src/app/core/data/request.effects.ts +++ b/src/app/core/data/request.effects.ts @@ -25,7 +25,7 @@ export class RequestEffects { @Effect() execute = this.actions$ .ofType(RequestActionTypes.EXECUTE) .flatMap((action: RequestExecuteAction) => { - return this.requestService.get(action.payload) + return this.requestService.getByUUID(action.payload) .take(1); }) .map((entry: RequestEntry) => entry.request) @@ -42,10 +42,10 @@ export class RequestEffects { .map((data: DSpaceRESTV2Response) => this.injector.get(request.getResponseParser()).parse(request, data)) .do((response: RestResponse) => this.responseCache.add(request.href, response, this.EnvConfig.cache.msToLive)) - .map((response: RestResponse) => new RequestCompleteAction(request.href)) + .map((response: RestResponse) => new RequestCompleteAction(request.uuid)) .catch((error: RequestError) => Observable.of(new ErrorResponse(error)) .do((response: RestResponse) => this.responseCache.add(request.href, response, this.EnvConfig.cache.msToLive)) - .map((response: RestResponse) => new RequestCompleteAction(request.href))); + .map((response: RestResponse) => new RequestCompleteAction(request.uuid))); }); constructor( diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 3a7141d22d..46f40b0abf 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -31,10 +31,12 @@ export enum RestRequestMethod { export class RestRequest { constructor( + public uuid: string, public href: string, public method: RestRequestMethod = RestRequestMethod.Get, public body?: any - ) { } + ) { + } getResponseParser(): GenericConstructor { return DSOResponseParsingService; @@ -43,10 +45,11 @@ export class RestRequest { export class FindByIDRequest extends RestRequest { constructor( + uuid: string, href: string, public resourceID: string ) { - super(href); + super(uuid, href); } } @@ -59,17 +62,18 @@ export class FindAllOptions { export class FindAllRequest extends RestRequest { constructor( + uuid: string, href: string, public options?: FindAllOptions, ) { - super(href); + super(uuid, href); } } export class RootEndpointRequest extends RestRequest { - constructor(EnvConfig: GlobalConfig) { + constructor(uuid: string, EnvConfig: GlobalConfig) { const href = new RESTURLCombiner(EnvConfig, '/').toString(); - super(href); + super(uuid, href); } getResponseParser(): GenericConstructor { @@ -78,8 +82,8 @@ export class RootEndpointRequest extends RestRequest { } export class BrowseEndpointRequest extends RestRequest { - constructor(href: string) { - super(href); + constructor(uuid: string, href: string) { + super(uuid, href); } getResponseParser(): GenericConstructor { @@ -88,8 +92,8 @@ export class BrowseEndpointRequest extends RestRequest { } export class ConfigRequest extends RestRequest { - constructor(href: string) { - super(href); + constructor(uuid: string, href: string) { + super(uuid, href); } getResponseParser(): GenericConstructor { diff --git a/src/app/core/data/request.reducer.spec.ts b/src/app/core/data/request.reducer.spec.ts index a6d84ffe80..138d3ea5dd 100644 --- a/src/app/core/data/request.reducer.spec.ts +++ b/src/app/core/data/request.reducer.spec.ts @@ -16,11 +16,13 @@ class NullAction extends RequestCompleteAction { } describe('requestReducer', () => { + const id1 = 'clients/eca2ea1d-6a6a-4f62-8907-176d5fec5014'; + const id2 = 'clients/eb7cde2e-a03f-4f0b-ac5d-888a4ef2b4eb'; const link1 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/567a639f-f5ff-4126-807c-b7d0910808c8'; const link2 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/1911e8a4-6939-490c-b58b-a5d70f8d91fb'; const testState: RequestState = { - [link1]: { - request: new RestRequest(link1), + [id1]: { + request: new RestRequest(id1, link1), requestPending: false, responsePending: false, completed: false @@ -44,37 +46,40 @@ describe('requestReducer', () => { it('should add the new RestRequest and set \'requestPending\' to true, \'responsePending\' to false and \'completed\' to false for the given RestRequest in the state, in response to a CONFIGURE action', () => { const state = testState; - const request = new RestRequest(link2); + const request = new RestRequest(id2, link2); const action = new RequestConfigureAction(request); const newState = requestReducer(state, action); - expect(newState[link2].request.href).toEqual(link2); - expect(newState[link2].requestPending).toEqual(true); - expect(newState[link2].responsePending).toEqual(false); - expect(newState[link2].completed).toEqual(false); + expect(newState[id2].request.uuid).toEqual(id2); + expect(newState[id2].request.href).toEqual(link2); + expect(newState[id2].requestPending).toEqual(true); + expect(newState[id2].responsePending).toEqual(false); + expect(newState[id2].completed).toEqual(false); }); 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', () => { const state = testState; - const action = new RequestExecuteAction(link1); + const action = new RequestExecuteAction(id1); const newState = requestReducer(state, action); - expect(newState[link1].request.href).toEqual(link1); - expect(newState[link1].requestPending).toEqual(false); - expect(newState[link1].responsePending).toEqual(true); - expect(newState[link1].completed).toEqual(state[link1].completed); + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].requestPending).toEqual(false); + expect(newState[id1].responsePending).toEqual(true); + expect(newState[id1].completed).toEqual(state[id1].completed); }); 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(link1); + const action = new RequestCompleteAction(id1); const newState = requestReducer(state, action); - expect(newState[link1].request.href).toEqual(link1); - expect(newState[link1].requestPending).toEqual(state[link1].requestPending); - expect(newState[link1].responsePending).toEqual(false); - expect(newState[link1].completed).toEqual(true); + expect(newState[id1].request.uuid).toEqual(id1); + expect(newState[id1].request.href).toEqual(link1); + expect(newState[id1].requestPending).toEqual(state[id1].requestPending); + expect(newState[id1].responsePending).toEqual(false); + expect(newState[id1].completed).toEqual(true); }); }); diff --git a/src/app/core/data/request.reducer.ts b/src/app/core/data/request.reducer.ts index 628725f745..3ac35d2741 100644 --- a/src/app/core/data/request.reducer.ts +++ b/src/app/core/data/request.reducer.ts @@ -12,7 +12,7 @@ export class RequestEntry { } export interface RequestState { - [key: string]: RequestEntry + [uuid: string]: RequestEntry } // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) @@ -41,7 +41,7 @@ export function requestReducer(state = initialState, action: RequestAction): Req function configureRequest(state: RequestState, action: RequestConfigureAction): RequestState { return Object.assign({}, state, { - [action.payload.href]: { + [action.payload.uuid]: { request: action.payload, requestPending: true, responsePending: false, diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index a075244648..6cfbedb5cb 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -10,14 +10,20 @@ import { DSOSuccessResponse, RestResponse } from '../cache/response-cache.models import { ResponseCacheEntry } from '../cache/response-cache.reducer'; import { ResponseCacheService } from '../cache/response-cache.service'; import { coreSelector, CoreState } from '../core.reducers'; -import { keySelector } from '../shared/selectors'; +import { IndexName } from '../index/index.reducer'; +import { pathSelector } from '../shared/selectors'; +import { UUIDService } from '../shared/uuid.service'; import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; import { RestRequest, RestRequestMethod } from './request.models'; import { RequestEntry, RequestState } from './request.reducer'; -function entryFromHrefSelector(href: string): MemoizedSelector { - return keySelector('data/request', href); +function entryFromUUIDSelector(uuid: string): MemoizedSelector { + return pathSelector(coreSelector, 'data/request', uuid); +} + +function uuidFromHrefSelector(href: string): MemoizedSelector { + return pathSelector(coreSelector, 'index', IndexName.REQUEST, href); } export function requestStateSelector(): MemoizedSelector { @@ -32,18 +38,23 @@ export class RequestService { constructor(private objectCache: ObjectCacheService, private responseCache: ResponseCacheService, + private uuidService: UUIDService, private store: Store) { } - isPending(href: string): boolean { + generateRequestId(): string { + return `client/${this.uuidService.generate()}`; + } + + isPending(uuid: string): boolean { // first check requests that haven't made it to the store yet - if (this.requestsOnTheirWayToTheStore.includes(href)) { + if (this.requestsOnTheirWayToTheStore.includes(uuid)) { return true; } // then check the store let isPending = false; - this.store.select(entryFromHrefSelector(href)) + this.store.select(entryFromUUIDSelector(uuid)) .take(1) .subscribe((re: RequestEntry) => { isPending = (hasValue(re) && !re.completed) @@ -52,8 +63,13 @@ export class RequestService { return isPending; } - get(href: string): Observable { - return this.store.select(entryFromHrefSelector(href)); + getByUUID(uuid: string): Observable { + return this.store.select(entryFromUUIDSelector(uuid)); + } + + getByHref(href: string): Observable { + return this.store.select(uuidFromHrefSelector(href)) + .flatMap((uuid: string) => this.getByUUID(uuid)); } configure(request: RestRequest): void { @@ -86,15 +102,15 @@ export class RequestService { ).subscribe((c) => isCached = c); } - const isPending = this.isPending(request.href); + const isPending = this.isPending(request.uuid); return isCached || isPending; } private dispatchRequest(request: RestRequest) { this.store.dispatch(new RequestConfigureAction(request)); - this.store.dispatch(new RequestExecuteAction(request.href)); - this.trackRequestsOnTheirWayToTheStore(request.href); + this.store.dispatch(new RequestExecuteAction(request.uuid)); + this.trackRequestsOnTheirWayToTheStore(request.uuid); } /** @@ -104,13 +120,13 @@ export class RequestService { * This method will store the href of every request that gets configured in a local variable, and * remove it as soon as it can be found in the store. */ - private trackRequestsOnTheirWayToTheStore(href: string) { - this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, href]; - this.store.select(entryFromHrefSelector(href)) + private trackRequestsOnTheirWayToTheStore(uuid: string) { + this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, uuid]; + this.store.select(entryFromUUIDSelector(uuid)) .filter((re: RequestEntry) => hasValue(re)) .take(1) .subscribe((re: RequestEntry) => { - this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((pendingHref: string) => pendingHref !== href) + this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((pendingUUID: string) => pendingUUID !== uuid) }); } } diff --git a/src/app/core/index/index.actions.ts b/src/app/core/index/index.actions.ts new file mode 100644 index 0000000000..014b6561a3 --- /dev/null +++ b/src/app/core/index/index.actions.ts @@ -0,0 +1,69 @@ +import { Action } from '@ngrx/store'; + +import { type } from '../../shared/ngrx/type'; +import { IndexName } from './index.reducer'; + +/** + * The list of HrefIndexAction type definitions + */ +export const IndexActionTypes = { + ADD: type('dspace/core/index/ADD'), + REMOVE_BY_VALUE: type('dspace/core/index/REMOVE_BY_VALUE') +}; + +/* tslint:disable:max-classes-per-file */ +/** + * An ngrx action to add an value to the index + */ +export class AddToIndexAction implements Action { + type = IndexActionTypes.ADD; + payload: { + name: IndexName; + value: string; + key: string; + }; + + /** + * Create a new AddToIndexAction + * + * @param name + * the name of the index to add to + * @param key + * the key to add + * @param value + * the self link of the resource the key belongs to + */ + constructor(name: IndexName, key: string, value: string) { + this.payload = { name, key, value }; + } +} + +/** + * An ngrx action to remove an value from the index + */ +export class RemoveFromIndexByValueAction implements Action { + type = IndexActionTypes.REMOVE_BY_VALUE; + payload: { + name: IndexName, + value: string + }; + + /** + * Create a new RemoveFromIndexByValueAction + * + * @param name + * the name of the index to remove from + * @param value + * the value to remove the UUID for + */ + constructor(name: IndexName, value: string) { + this.payload = { name, value }; + } + +} +/* tslint:enable:max-classes-per-file */ + +/** + * A type to encompass all HrefIndexActions + */ +export type IndexAction = AddToIndexAction | RemoveFromIndexByValueAction; diff --git a/src/app/core/index/index.effects.ts b/src/app/core/index/index.effects.ts new file mode 100644 index 0000000000..05ae529c8e --- /dev/null +++ b/src/app/core/index/index.effects.ts @@ -0,0 +1,61 @@ +import { Injectable } from '@angular/core'; +import { Effect, Actions } from '@ngrx/effects'; + +import { + ObjectCacheActionTypes, AddToObjectCacheAction, + RemoveFromObjectCacheAction +} from '../cache/object-cache.actions'; +import { RequestActionTypes, RequestConfigureAction } from '../data/request.actions'; +import { RestRequestMethod } from '../data/request.models'; +import { AddToIndexAction, RemoveFromIndexByValueAction } from './index.actions'; +import { hasValue } from '../../shared/empty.util'; +import { IndexName } from './index.reducer'; + +@Injectable() +export class UUIDIndexEffects { + + @Effect() addObject$ = this.actions$ + .ofType(ObjectCacheActionTypes.ADD) + .filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)) + .map((action: AddToObjectCacheAction) => { + return new AddToIndexAction( + IndexName.OBJECT, + action.payload.objectToCache.uuid, + action.payload.objectToCache.self + ); + }); + + @Effect() removeObject$ = this.actions$ + .ofType(ObjectCacheActionTypes.REMOVE) + .map((action: RemoveFromObjectCacheAction) => { + return new RemoveFromIndexByValueAction( + IndexName.OBJECT, + action.payload + ); + }); + + @Effect() addRequest$ = this.actions$ + .ofType(RequestActionTypes.CONFIGURE) + .filter((action: RequestConfigureAction) => action.payload.method === RestRequestMethod.Get) + .map((action: RequestConfigureAction) => { + return new AddToIndexAction( + IndexName.REQUEST, + action.payload.href, + action.payload.uuid + ); + }); + + // @Effect() removeRequest$ = this.actions$ + // .ofType(ObjectCacheActionTypes.REMOVE) + // .map((action: RemoveFromObjectCacheAction) => { + // return new RemoveFromIndexByValueAction( + // IndexName.OBJECT, + // action.payload + // ); + // }); + + constructor(private actions$: Actions) { + + } + +} diff --git a/src/app/core/index/index.reducer.spec.ts b/src/app/core/index/index.reducer.spec.ts new file mode 100644 index 0000000000..a1cf92aeb3 --- /dev/null +++ b/src/app/core/index/index.reducer.spec.ts @@ -0,0 +1,58 @@ +import * as deepFreeze from 'deep-freeze'; + +import { IndexName, indexReducer, IndexState } from './index.reducer'; +import { AddToIndexAction, RemoveFromIndexByValueAction } from './index.actions'; + +class NullAction extends AddToIndexAction { + type = null; + payload = null; + + constructor() { + super(null, null, null); + } +} + +describe('requestReducer', () => { + const key1 = '567a639f-f5ff-4126-807c-b7d0910808c8'; + const key2 = '1911e8a4-6939-490c-b58b-a5d70f8d91fb'; + const val1 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/567a639f-f5ff-4126-807c-b7d0910808c8'; + const val2 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/1911e8a4-6939-490c-b58b-a5d70f8d91fb'; + const testState: IndexState = { + [IndexName.OBJECT]: { + [key1]: val1 + } + }; + deepFreeze(testState); + + it('should return the current state when no valid actions have been made', () => { + const action = new NullAction(); + const newState = indexReducer(testState, action); + + expect(newState).toEqual(testState); + }); + + it('should start with an empty state', () => { + const action = new NullAction(); + const initialState = indexReducer(undefined, action); + + expect(initialState).toEqual(Object.create(null)); + }); + + it('should add the \'key\' with the corresponding \'value\' to the correct substate, in response to an ADD action', () => { + const state = testState; + + const action = new AddToIndexAction(IndexName.REQUEST, key2, val2); + const newState = indexReducer(state, action); + + expect(newState[IndexName.REQUEST][key2]).toEqual(val2); + }); + + it('should remove the given \'value\' from its corresponding \'key\' in the correct substate, in response to a REMOVE_BY_VALUE action', () => { + const state = testState; + + const action = new RemoveFromIndexByValueAction(IndexName.OBJECT, val1); + const newState = indexReducer(state, action); + + expect(newState[IndexName.OBJECT][key1]).toBeUndefined(); + }); +}); diff --git a/src/app/core/index/index.reducer.ts b/src/app/core/index/index.reducer.ts new file mode 100644 index 0000000000..869dee9e51 --- /dev/null +++ b/src/app/core/index/index.reducer.ts @@ -0,0 +1,62 @@ +import { + IndexAction, + IndexActionTypes, + AddToIndexAction, + RemoveFromIndexByValueAction +} from './index.actions'; + +export enum IndexName { + OBJECT = 'object/uuid-to-self-link', + REQUEST = 'get-request/href-to-uuid' +} + +export interface IndexState { + // TODO this should be `[name in IndexName]: {` but that's currently broken, + // see https://github.com/Microsoft/TypeScript/issues/13042 + [name: string]: { + [key: string]: string + } +} + +// Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) +const initialState: IndexState = Object.create(null); + +export function indexReducer(state = initialState, action: IndexAction): IndexState { + switch (action.type) { + + case IndexActionTypes.ADD: { + return addToIndex(state, action as AddToIndexAction); + } + + case IndexActionTypes.REMOVE_BY_VALUE: { + return removeFromIndexByValue(state, action as RemoveFromIndexByValueAction) + } + + default: { + return state; + } + } +} + +function addToIndex(state: IndexState, action: AddToIndexAction): IndexState { + const subState = state[action.payload.name]; + const newSubState = Object.assign({}, subState, { + [action.payload.key]: action.payload.value + }); + return Object.assign({}, state, { + [action.payload.name]: newSubState + }) +} + +function removeFromIndexByValue(state: IndexState, action: RemoveFromIndexByValueAction): IndexState { + const subState = state[action.payload.name]; + const newSubState = Object.create(null); + for (const value in subState) { + if (subState[value] !== action.payload.value) { + newSubState[value] = subState[value]; + } + } + return Object.assign({}, state, { + [action.payload.name]: newSubState + }); +} diff --git a/src/app/core/index/uuid-index.actions.ts b/src/app/core/index/uuid-index.actions.ts deleted file mode 100644 index 0bea11204c..0000000000 --- a/src/app/core/index/uuid-index.actions.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { Action } from '@ngrx/store'; - -import { type } from '../../shared/ngrx/type'; - -/** - * The list of HrefIndexAction type definitions - */ -export const UUIDIndexActionTypes = { - ADD: type('dspace/core/index/uuid/ADD'), - REMOVE_HREF: type('dspace/core/index/uuid/REMOVE_HREF') -}; - -/* tslint:disable:max-classes-per-file */ -/** - * An ngrx action to add an href to the index - */ -export class AddToUUIDIndexAction implements Action { - type = UUIDIndexActionTypes.ADD; - payload: { - href: string; - uuid: string; - }; - - /** - * Create a new AddToUUIDIndexAction - * - * @param uuid - * the uuid to add - * @param href - * the self link of the resource the uuid belongs to - */ - constructor(uuid: string, href: string) { - this.payload = { href, uuid }; - } -} - -/** - * An ngrx action to remove an href from the index - */ -export class RemoveHrefFromUUIDIndexAction implements Action { - type = UUIDIndexActionTypes.REMOVE_HREF; - payload: string; - - /** - * Create a new RemoveHrefFromUUIDIndexAction - * - * @param href - * the href to remove the UUID for - */ - constructor(href: string) { - this.payload = href; - } - -} -/* tslint:enable:max-classes-per-file */ - -/** - * A type to encompass all HrefIndexActions - */ -export type UUIDIndexAction = AddToUUIDIndexAction | RemoveHrefFromUUIDIndexAction; diff --git a/src/app/core/index/uuid-index.effects.ts b/src/app/core/index/uuid-index.effects.ts deleted file mode 100644 index 2f5900ed04..0000000000 --- a/src/app/core/index/uuid-index.effects.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { Injectable } from '@angular/core'; -import { Effect, Actions } from '@ngrx/effects'; - -import { - ObjectCacheActionTypes, AddToObjectCacheAction, - RemoveFromObjectCacheAction -} from '../cache/object-cache.actions'; -import { AddToUUIDIndexAction, RemoveHrefFromUUIDIndexAction } from './uuid-index.actions'; -import { hasValue } from '../../shared/empty.util'; - -@Injectable() -export class UUIDIndexEffects { - - @Effect() add$ = this.actions$ - .ofType(ObjectCacheActionTypes.ADD) - .filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)) - .map((action: AddToObjectCacheAction) => { - return new AddToUUIDIndexAction( - action.payload.objectToCache.uuid, - action.payload.objectToCache.self - ); - }); - - @Effect() remove$ = this.actions$ - .ofType(ObjectCacheActionTypes.REMOVE) - .map((action: RemoveFromObjectCacheAction) => { - return new RemoveHrefFromUUIDIndexAction(action.payload); - }); - - constructor(private actions$: Actions) { - - } - -} diff --git a/src/app/core/index/uuid-index.reducer.spec.ts b/src/app/core/index/uuid-index.reducer.spec.ts deleted file mode 100644 index e477d8df2e..0000000000 --- a/src/app/core/index/uuid-index.reducer.spec.ts +++ /dev/null @@ -1,56 +0,0 @@ -import * as deepFreeze from 'deep-freeze'; - -import { uuidIndexReducer, UUIDIndexState } from './uuid-index.reducer'; -import { AddToUUIDIndexAction, RemoveHrefFromUUIDIndexAction } from './uuid-index.actions'; - -class NullAction extends AddToUUIDIndexAction { - type = null; - payload = null; - - constructor() { - super(null, null); - } -} - -describe('requestReducer', () => { - const link1 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/567a639f-f5ff-4126-807c-b7d0910808c8'; - const link2 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/1911e8a4-6939-490c-b58b-a5d70f8d91fb'; - const uuid1 = '567a639f-f5ff-4126-807c-b7d0910808c8'; - const uuid2 = '1911e8a4-6939-490c-b58b-a5d70f8d91fb'; - const testState: UUIDIndexState = { - [uuid1]: link1 - }; - deepFreeze(testState); - - it('should return the current state when no valid actions have been made', () => { - const action = new NullAction(); - const newState = uuidIndexReducer(testState, action); - - expect(newState).toEqual(testState); - }); - - it('should start with an empty state', () => { - const action = new NullAction(); - const initialState = uuidIndexReducer(undefined, action); - - expect(initialState).toEqual(Object.create(null)); - }); - - it('should add the \'uuid\' with the corresponding \'href\' to the state, in response to an ADD action', () => { - const state = testState; - - const action = new AddToUUIDIndexAction(uuid2, link2); - const newState = uuidIndexReducer(state, action); - - expect(newState[uuid2]).toEqual(link2); - }); - - it('should remove the given \'href\' from its corresponding \'uuid\' in the state, in response to a REMOVE_HREF action', () => { - const state = testState; - - const action = new RemoveHrefFromUUIDIndexAction(link1); - const newState = uuidIndexReducer(state, action); - - expect(newState[uuid1]).toBeUndefined(); - }); -}); diff --git a/src/app/core/index/uuid-index.reducer.ts b/src/app/core/index/uuid-index.reducer.ts deleted file mode 100644 index 191dd8f463..0000000000 --- a/src/app/core/index/uuid-index.reducer.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { - UUIDIndexAction, - UUIDIndexActionTypes, - AddToUUIDIndexAction, - RemoveHrefFromUUIDIndexAction -} from './uuid-index.actions'; - -export interface UUIDIndexState { - [uuid: string]: string -} - -// Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) -const initialState: UUIDIndexState = Object.create(null); - -export function uuidIndexReducer(state = initialState, action: UUIDIndexAction): UUIDIndexState { - switch (action.type) { - - case UUIDIndexActionTypes.ADD: { - return addToUUIDIndex(state, action as AddToUUIDIndexAction); - } - - case UUIDIndexActionTypes.REMOVE_HREF: { - return removeHrefFromUUIDIndex(state, action as RemoveHrefFromUUIDIndexAction) - } - - default: { - return state; - } - } -} - -function addToUUIDIndex(state: UUIDIndexState, action: AddToUUIDIndexAction): UUIDIndexState { - return Object.assign({}, state, { - [action.payload.uuid]: action.payload.href - }); -} - -function removeHrefFromUUIDIndex(state: UUIDIndexState, action: RemoveHrefFromUUIDIndexAction): UUIDIndexState { - const newState = Object.create(null); - for (const uuid in state) { - if (state[uuid] !== action.payload) { - newState[uuid] = state[uuid]; - } - } - return newState; -} diff --git a/src/app/core/shared/hal-endpoint.service.spec.ts b/src/app/core/shared/hal-endpoint.service.spec.ts index f7adc1eccf..22d9a15fd7 100644 --- a/src/app/core/shared/hal-endpoint.service.spec.ts +++ b/src/app/core/shared/hal-endpoint.service.spec.ts @@ -1,5 +1,6 @@ import { cold, hot } from 'jasmine-marbles'; import { GlobalConfig } from '../../../config/global-config.interface'; +import { initMockRequestService } from '../../shared/mocks/mock-request.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { RootEndpointRequest } from '../data/request.models'; import { RequestService } from '../data/request.service'; @@ -38,7 +39,7 @@ describe('HALEndpointService', () => { }) }); - requestService = jasmine.createSpyObj('requestService', ['configure']); + requestService = initMockRequestService(); envConfig = { rest: { baseUrl: 'https://rest.api/' } @@ -53,7 +54,7 @@ describe('HALEndpointService', () => { it('should configure a new RootEndpointRequest', () => { (service as any).getEndpointMap(); - const expected = new RootEndpointRequest(envConfig); + const expected = new RootEndpointRequest(requestService.generateRequestId(), envConfig); expect(requestService.configure).toHaveBeenCalledWith(expected); }); diff --git a/src/app/core/shared/hal-endpoint.service.ts b/src/app/core/shared/hal-endpoint.service.ts index fa11fed308..84587f1eea 100644 --- a/src/app/core/shared/hal-endpoint.service.ts +++ b/src/app/core/shared/hal-endpoint.service.ts @@ -14,7 +14,7 @@ export abstract class HALEndpointService { protected abstract EnvConfig: GlobalConfig; protected getEndpointMap(): Observable { - const request = new RootEndpointRequest(this.EnvConfig); + const request = new RootEndpointRequest(this.requestService.generateRequestId(), this.EnvConfig); this.requestService.configure(request); return this.responseCache.get(request.href) .map((entry: ResponseCacheEntry) => entry.response) diff --git a/src/app/core/shared/selectors.ts b/src/app/core/shared/selectors.ts index 06f444b2e6..7bd35d39c1 100644 --- a/src/app/core/shared/selectors.ts +++ b/src/app/core/shared/selectors.ts @@ -1,13 +1,17 @@ import { createSelector, MemoizedSelector } from '@ngrx/store'; -import { coreSelector, CoreState } from '../core.reducers'; -import { hasValue } from '../../shared/empty.util'; +import { hasNoValue, isEmpty } from '../../shared/empty.util'; -export function keySelector(subState: string, key: string): MemoizedSelector { - return createSelector(coreSelector, (state: CoreState) => { - if (hasValue(state[subState])) { - return state[subState][key]; - } else { - return undefined; - } - }); +export function pathSelector(selector: MemoizedSelector, ...path: string[]): MemoizedSelector { + return createSelector(selector, (state: any) => getSubState(state, path)); +} + +function getSubState(state: any, path: string[]) { + const current = path[0]; + const remainingPath = path.slice(1); + const subState = state[current]; + if (hasNoValue(subState) || isEmpty(remainingPath)) { + return subState; + } else { + return getSubState(subState, remainingPath); + } } diff --git a/src/app/core/shared/uuid.service.ts b/src/app/core/shared/uuid.service.ts new file mode 100644 index 0000000000..6c02facbac --- /dev/null +++ b/src/app/core/shared/uuid.service.ts @@ -0,0 +1,9 @@ +import { Injectable } from '@angular/core'; +import * as uuidv4 from 'uuid/v4'; + +@Injectable() +export class UUIDService { + generate(): string { + return uuidv4(); + } +} diff --git a/src/app/shared/mocks/mock-request.service.ts b/src/app/shared/mocks/mock-request.service.ts new file mode 100644 index 0000000000..9ece96da3e --- /dev/null +++ b/src/app/shared/mocks/mock-request.service.ts @@ -0,0 +1,8 @@ +import { RequestService } from '../../core/data/request.service'; + +export function initMockRequestService(): RequestService { + return jasmine.createSpyObj('requestService', { + configure: () => false, + generateRequestId: () => 'clients/b186e8ce-e99c-4183-bc9a-42b4821bdb78' + }); +} diff --git a/yarn.lock b/yarn.lock index 91b2a787e2..6fc8d3ab0f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -225,6 +225,12 @@ version "0.5.1" resolved "https://registry.yarnpkg.com/@types/source-map/-/source-map-0.5.1.tgz#7e74db5d06ab373a712356eebfaea2fad0ea2367" +"@types/key@^3.4.3": + version "3.4.3" + resolved "https://registry.yarnpkg.com/@types/key/-/key-3.4.3.tgz#121ace265f5569ce40f4f6d0ff78a338c732a754" + dependencies: + "@types/node" "*" + "@types/webfontloader@1.6.29": version "1.6.29" resolved "https://registry.yarnpkg.com/@types/webfontloader/-/webfontloader-1.6.29.tgz#c6b5f6eb8ca31d0aae6b02b6c1300349dd93ea8e" @@ -1009,7 +1015,7 @@ cache-base@^1.0.1: dependencies: collection-visit "^1.0.0" component-emitter "^1.2.1" - get-value "^2.0.6" + getByUUID-value "^2.0.6" has-value "^1.0.0" isobject "^3.0.1" set-value "^2.0.0" @@ -1751,7 +1757,7 @@ dateformat@^1.0.11, dateformat@^1.0.6: version "1.0.12" resolved "https://registry.yarnpkg.com/dateformat/-/dateformat-1.0.12.tgz#9f124b67594c937ff706932e4a642cca8dbbfee9" dependencies: - get-stdin "^4.0.1" + getByUUID-stdin "^4.0.1" meow "^3.3.0" debug@2, debug@2.6.9, debug@^2.2.0, debug@^2.3.3, debug@^2.6.6, debug@^2.6.8: @@ -2317,7 +2323,7 @@ execa@^0.7.0: resolved "https://registry.yarnpkg.com/execa/-/execa-0.7.0.tgz#944becd34cc41ee32a63a9faf27ad5a65fc59777" dependencies: cross-spawn "^5.0.1" - get-stream "^3.0.0" + getByUUID-stream "^3.0.0" is-stream "^1.1.0" npm-run-path "^2.0.0" p-finally "^1.0.0" @@ -2731,25 +2737,25 @@ generate-object-property@^1.1.0: dependencies: is-property "^1.0.0" -get-caller-file@^1.0.1: +getByUUID-caller-file@^1.0.1: version "1.0.2" - resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-1.0.2.tgz#f702e63127e7e231c160a80c1554acb70d5047e5" + resolved "https://registry.yarnpkg.com/getByUUID-caller-file/-/getByUUID-caller-file-1.0.2.tgz#f702e63127e7e231c160a80c1554acb70d5047e5" -get-stdin@^4.0.1: +getByUUID-stdin@^4.0.1: version "4.0.1" - resolved "https://registry.yarnpkg.com/get-stdin/-/get-stdin-4.0.1.tgz#b968c6b0a04384324902e8bf1a5df32579a450fe" + resolved "https://registry.yarnpkg.com/getByUUID-stdin/-/getByUUID-stdin-4.0.1.tgz#b968c6b0a04384324902e8bf1a5df32579a450fe" -get-stdin@^5.0.1: +getByUUID-stdin@^5.0.1: version "5.0.1" - resolved "https://registry.yarnpkg.com/get-stdin/-/get-stdin-5.0.1.tgz#122e161591e21ff4c52530305693f20e6393a398" + resolved "https://registry.yarnpkg.com/getByUUID-stdin/-/getByUUID-stdin-5.0.1.tgz#122e161591e21ff4c52530305693f20e6393a398" -get-stream@^3.0.0: +getByUUID-stream@^3.0.0: version "3.0.0" - resolved "https://registry.yarnpkg.com/get-stream/-/get-stream-3.0.0.tgz#8e943d1358dc37555054ecbe2edb05aa174ede14" + resolved "https://registry.yarnpkg.com/getByUUID-stream/-/getByUUID-stream-3.0.0.tgz#8e943d1358dc37555054ecbe2edb05aa174ede14" -get-value@^2.0.3, get-value@^2.0.6: +getByUUID-value@^2.0.3, getByUUID-value@^2.0.6: version "2.0.6" - resolved "https://registry.yarnpkg.com/get-value/-/get-value-2.0.6.tgz#dc15ca1c672387ca76bd37ac0a395ba2042a2c28" + resolved "https://registry.yarnpkg.com/getByUUID-value/-/getByUUID-value-2.0.6.tgz#dc15ca1c672387ca76bd37ac0a395ba2042a2c28" getpass@^0.1.1: version "0.1.7" @@ -2842,7 +2848,7 @@ got@^6.7.1: dependencies: create-error-class "^3.0.0" duplexer3 "^0.1.4" - get-stream "^3.0.0" + getByUUID-stream "^3.0.0" is-redirect "^1.0.0" is-retry-allowed "^1.0.0" is-stream "^1.0.0" @@ -2974,7 +2980,7 @@ has-value@^0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/has-value/-/has-value-0.3.1.tgz#7b1f58bada62ca827ec0a2078025654845995e1f" dependencies: - get-value "^2.0.3" + getByUUID-value "^2.0.3" has-values "^0.1.4" isobject "^2.0.0" @@ -2982,7 +2988,7 @@ has-value@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/has-value/-/has-value-1.0.0.tgz#18b281da585b1c5c51def24c930ed29a0be6b177" dependencies: - get-value "^2.0.6" + getByUUID-value "^2.0.6" has-values "^1.0.0" isobject "^3.0.0" @@ -4652,7 +4658,7 @@ node-sass@4.5.3: chalk "^1.1.1" cross-spawn "^3.0.0" gaze "^1.0.0" - get-stdin "^4.0.1" + getByUUID-stdin "^4.0.1" glob "^7.0.3" in-publish "^2.0.0" lodash.assign "^4.2.0" @@ -5223,7 +5229,7 @@ postcss-cli@4.1.1: chokidar "^1.6.1" dependency-graph "^0.5.0" fs-extra "^4.0.1" - get-stdin "^5.0.1" + getByUUID-stdin "^5.0.1" globby "^6.1.0" ora "^1.1.0" postcss "^6.0.1" @@ -5802,7 +5808,7 @@ protractor-istanbul-plugin@2.0.0: fs-extra "^0.22.1" merge "^1.2.0" q "^1.4.1" - uuid "^2.0.1" + key "^2.0.1" protractor@5.1.2: version "5.1.2" @@ -6224,7 +6230,7 @@ request@2, request@^2.78.0, request@^2.79.0: stringstream "~0.0.5" tough-cookie "~2.3.3" tunnel-agent "^0.6.0" - uuid "^3.1.0" + key "^3.1.0" request@2.79.0: version "2.79.0" @@ -6249,7 +6255,7 @@ request@2.79.0: stringstream "~0.0.4" tough-cookie "~2.3.0" tunnel-agent "~0.4.1" - uuid "^3.0.0" + key "^3.0.0" request@2.81.0, request@~2.81.0: version "2.81.0" @@ -6276,7 +6282,7 @@ request@2.81.0, request@~2.81.0: stringstream "~0.0.4" tough-cookie "~2.3.0" tunnel-agent "^0.6.0" - uuid "^3.0.0" + key "^3.0.0" require-directory@^2.1.1: version "2.1.1" @@ -6783,7 +6789,7 @@ sockjs@0.3.18: resolved "https://registry.yarnpkg.com/sockjs/-/sockjs-0.3.18.tgz#d9b289316ca7df77595ef299e075f0f937eb4207" dependencies: faye-websocket "^0.10.0" - uuid "^2.0.2" + key "^2.0.2" sort-keys@^1.0.0: version "1.1.2" @@ -7056,7 +7062,7 @@ strip-indent@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/strip-indent/-/strip-indent-1.0.1.tgz#0c7962a6adefa7bbd4ac366460a638552ae1a0a2" dependencies: - get-stdin "^4.0.1" + getByUUID-stdin "^4.0.1" strip-json-comments@^2.0.0, strip-json-comments@~2.0.1: version "2.0.1" @@ -7459,7 +7465,7 @@ union-value@^1.0.0: resolved "https://registry.yarnpkg.com/union-value/-/union-value-1.0.0.tgz#5c71c34cb5bad5dcebe3ea0cd08207ba5aa1aea4" dependencies: arr-union "^3.1.0" - get-value "^2.0.6" + getByUUID-value "^2.0.6" is-extendable "^0.1.1" set-value "^0.4.3" @@ -7604,13 +7610,13 @@ utils-merge@1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/utils-merge/-/utils-merge-1.0.1.tgz#9f95710f50a267947b2ccc124741c1028427e713" -uuid@^2.0.1, uuid@^2.0.2: +key@^2.0.1, key@^2.0.2: version "2.0.3" - resolved "https://registry.yarnpkg.com/uuid/-/uuid-2.0.3.tgz#67e2e863797215530dff318e5bf9dcebfd47b21a" + resolved "https://registry.yarnpkg.com/key/-/key-2.0.3.tgz#67e2e863797215530dff318e5bf9dcebfd47b21a" -uuid@^3.0.0, uuid@^3.1.0: +key@^3.0.0, key@^3.1.0: version "3.1.0" - resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.1.0.tgz#3dd3d3e790abc24d7b0d3a034ffababe28ebbc04" + resolved "https://registry.yarnpkg.com/key/-/key-3.1.0.tgz#3dd3d3e790abc24d7b0d3a034ffababe28ebbc04" v8flags@^3.0.0: version "3.0.1" @@ -7999,7 +8005,7 @@ yargs@^6.6.0: camelcase "^3.0.0" cliui "^3.2.0" decamelize "^1.1.1" - get-caller-file "^1.0.1" + getByUUID-caller-file "^1.0.1" os-locale "^1.4.0" read-pkg-up "^1.0.1" require-directory "^2.1.1" @@ -8017,7 +8023,7 @@ yargs@^7.0.0: camelcase "^3.0.0" cliui "^3.2.0" decamelize "^1.1.1" - get-caller-file "^1.0.1" + getByUUID-caller-file "^1.0.1" os-locale "^1.4.0" read-pkg-up "^1.0.1" require-directory "^2.1.1" @@ -8035,7 +8041,7 @@ yargs@^8.0.1, yargs@^8.0.2: camelcase "^4.1.0" cliui "^3.2.0" decamelize "^1.1.1" - get-caller-file "^1.0.1" + getByUUID-caller-file "^1.0.1" os-locale "^2.0.0" read-pkg-up "^2.0.0" require-directory "^2.1.1" From 0010f9d704d8e2ff6bf777cc3d8ed2ace565972e Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 10 Jan 2018 14:02:58 +0100 Subject: [PATCH 04/11] fixed find-and-replace mistakes in yarn.lock --- yarn.lock | 68 +++++++++++++++++++++++++++---------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/yarn.lock b/yarn.lock index 6fc8d3ab0f..3955f0a44b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -225,9 +225,9 @@ version "0.5.1" resolved "https://registry.yarnpkg.com/@types/source-map/-/source-map-0.5.1.tgz#7e74db5d06ab373a712356eebfaea2fad0ea2367" -"@types/key@^3.4.3": +"@types/uuid@^3.4.3": version "3.4.3" - resolved "https://registry.yarnpkg.com/@types/key/-/key-3.4.3.tgz#121ace265f5569ce40f4f6d0ff78a338c732a754" + resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-3.4.3.tgz#121ace265f5569ce40f4f6d0ff78a338c732a754" dependencies: "@types/node" "*" @@ -1015,7 +1015,7 @@ cache-base@^1.0.1: dependencies: collection-visit "^1.0.0" component-emitter "^1.2.1" - getByUUID-value "^2.0.6" + get-value "^2.0.6" has-value "^1.0.0" isobject "^3.0.1" set-value "^2.0.0" @@ -1757,7 +1757,7 @@ dateformat@^1.0.11, dateformat@^1.0.6: version "1.0.12" resolved "https://registry.yarnpkg.com/dateformat/-/dateformat-1.0.12.tgz#9f124b67594c937ff706932e4a642cca8dbbfee9" dependencies: - getByUUID-stdin "^4.0.1" + get-stdin "^4.0.1" meow "^3.3.0" debug@2, debug@2.6.9, debug@^2.2.0, debug@^2.3.3, debug@^2.6.6, debug@^2.6.8: @@ -2323,7 +2323,7 @@ execa@^0.7.0: resolved "https://registry.yarnpkg.com/execa/-/execa-0.7.0.tgz#944becd34cc41ee32a63a9faf27ad5a65fc59777" dependencies: cross-spawn "^5.0.1" - getByUUID-stream "^3.0.0" + get-stream "^3.0.0" is-stream "^1.1.0" npm-run-path "^2.0.0" p-finally "^1.0.0" @@ -2737,25 +2737,25 @@ generate-object-property@^1.1.0: dependencies: is-property "^1.0.0" -getByUUID-caller-file@^1.0.1: +get-caller-file@^1.0.1: version "1.0.2" - resolved "https://registry.yarnpkg.com/getByUUID-caller-file/-/getByUUID-caller-file-1.0.2.tgz#f702e63127e7e231c160a80c1554acb70d5047e5" + resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-1.0.2.tgz#f702e63127e7e231c160a80c1554acb70d5047e5" -getByUUID-stdin@^4.0.1: +get-stdin@^4.0.1: version "4.0.1" - resolved "https://registry.yarnpkg.com/getByUUID-stdin/-/getByUUID-stdin-4.0.1.tgz#b968c6b0a04384324902e8bf1a5df32579a450fe" + resolved "https://registry.yarnpkg.com/get-stdin/-/get-stdin-4.0.1.tgz#b968c6b0a04384324902e8bf1a5df32579a450fe" -getByUUID-stdin@^5.0.1: +get-stdin@^5.0.1: version "5.0.1" - resolved "https://registry.yarnpkg.com/getByUUID-stdin/-/getByUUID-stdin-5.0.1.tgz#122e161591e21ff4c52530305693f20e6393a398" + resolved "https://registry.yarnpkg.com/get-stdin/-/get-stdin-5.0.1.tgz#122e161591e21ff4c52530305693f20e6393a398" -getByUUID-stream@^3.0.0: +get-stream@^3.0.0: version "3.0.0" - resolved "https://registry.yarnpkg.com/getByUUID-stream/-/getByUUID-stream-3.0.0.tgz#8e943d1358dc37555054ecbe2edb05aa174ede14" + resolved "https://registry.yarnpkg.com/get-stream/-/get-stream-3.0.0.tgz#8e943d1358dc37555054ecbe2edb05aa174ede14" -getByUUID-value@^2.0.3, getByUUID-value@^2.0.6: +get-value@^2.0.3, get-value@^2.0.6: version "2.0.6" - resolved "https://registry.yarnpkg.com/getByUUID-value/-/getByUUID-value-2.0.6.tgz#dc15ca1c672387ca76bd37ac0a395ba2042a2c28" + resolved "https://registry.yarnpkg.com/get-value/-/get-value-2.0.6.tgz#dc15ca1c672387ca76bd37ac0a395ba2042a2c28" getpass@^0.1.1: version "0.1.7" @@ -2848,7 +2848,7 @@ got@^6.7.1: dependencies: create-error-class "^3.0.0" duplexer3 "^0.1.4" - getByUUID-stream "^3.0.0" + get-stream "^3.0.0" is-redirect "^1.0.0" is-retry-allowed "^1.0.0" is-stream "^1.0.0" @@ -2980,7 +2980,7 @@ has-value@^0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/has-value/-/has-value-0.3.1.tgz#7b1f58bada62ca827ec0a2078025654845995e1f" dependencies: - getByUUID-value "^2.0.3" + get-value "^2.0.3" has-values "^0.1.4" isobject "^2.0.0" @@ -2988,7 +2988,7 @@ has-value@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/has-value/-/has-value-1.0.0.tgz#18b281da585b1c5c51def24c930ed29a0be6b177" dependencies: - getByUUID-value "^2.0.6" + get-value "^2.0.6" has-values "^1.0.0" isobject "^3.0.0" @@ -4658,7 +4658,7 @@ node-sass@4.5.3: chalk "^1.1.1" cross-spawn "^3.0.0" gaze "^1.0.0" - getByUUID-stdin "^4.0.1" + get-stdin "^4.0.1" glob "^7.0.3" in-publish "^2.0.0" lodash.assign "^4.2.0" @@ -5229,7 +5229,7 @@ postcss-cli@4.1.1: chokidar "^1.6.1" dependency-graph "^0.5.0" fs-extra "^4.0.1" - getByUUID-stdin "^5.0.1" + get-stdin "^5.0.1" globby "^6.1.0" ora "^1.1.0" postcss "^6.0.1" @@ -5808,7 +5808,7 @@ protractor-istanbul-plugin@2.0.0: fs-extra "^0.22.1" merge "^1.2.0" q "^1.4.1" - key "^2.0.1" + uuid "^2.0.1" protractor@5.1.2: version "5.1.2" @@ -6230,7 +6230,7 @@ request@2, request@^2.78.0, request@^2.79.0: stringstream "~0.0.5" tough-cookie "~2.3.3" tunnel-agent "^0.6.0" - key "^3.1.0" + uuid "^3.1.0" request@2.79.0: version "2.79.0" @@ -6255,7 +6255,7 @@ request@2.79.0: stringstream "~0.0.4" tough-cookie "~2.3.0" tunnel-agent "~0.4.1" - key "^3.0.0" + uuid "^3.0.0" request@2.81.0, request@~2.81.0: version "2.81.0" @@ -6282,7 +6282,7 @@ request@2.81.0, request@~2.81.0: stringstream "~0.0.4" tough-cookie "~2.3.0" tunnel-agent "^0.6.0" - key "^3.0.0" + uuid "^3.0.0" require-directory@^2.1.1: version "2.1.1" @@ -6789,7 +6789,7 @@ sockjs@0.3.18: resolved "https://registry.yarnpkg.com/sockjs/-/sockjs-0.3.18.tgz#d9b289316ca7df77595ef299e075f0f937eb4207" dependencies: faye-websocket "^0.10.0" - key "^2.0.2" + uuid "^2.0.2" sort-keys@^1.0.0: version "1.1.2" @@ -7062,7 +7062,7 @@ strip-indent@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/strip-indent/-/strip-indent-1.0.1.tgz#0c7962a6adefa7bbd4ac366460a638552ae1a0a2" dependencies: - getByUUID-stdin "^4.0.1" + get-stdin "^4.0.1" strip-json-comments@^2.0.0, strip-json-comments@~2.0.1: version "2.0.1" @@ -7465,7 +7465,7 @@ union-value@^1.0.0: resolved "https://registry.yarnpkg.com/union-value/-/union-value-1.0.0.tgz#5c71c34cb5bad5dcebe3ea0cd08207ba5aa1aea4" dependencies: arr-union "^3.1.0" - getByUUID-value "^2.0.6" + get-value "^2.0.6" is-extendable "^0.1.1" set-value "^0.4.3" @@ -7610,13 +7610,13 @@ utils-merge@1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/utils-merge/-/utils-merge-1.0.1.tgz#9f95710f50a267947b2ccc124741c1028427e713" -key@^2.0.1, key@^2.0.2: +uuid@^2.0.1, uuid@^2.0.2: version "2.0.3" - resolved "https://registry.yarnpkg.com/key/-/key-2.0.3.tgz#67e2e863797215530dff318e5bf9dcebfd47b21a" + resolved "https://registry.yarnpkg.com/uuid/-/uuid-2.0.3.tgz#67e2e863797215530dff318e5bf9dcebfd47b21a" -key@^3.0.0, key@^3.1.0: +uuid@^3.0.0, uuid@^3.1.0: version "3.1.0" - resolved "https://registry.yarnpkg.com/key/-/key-3.1.0.tgz#3dd3d3e790abc24d7b0d3a034ffababe28ebbc04" + resolved "https://registry.yarnpkg.com/uuid/-/uuid-3.1.0.tgz#3dd3d3e790abc24d7b0d3a034ffababe28ebbc04" v8flags@^3.0.0: version "3.0.1" @@ -8005,7 +8005,7 @@ yargs@^6.6.0: camelcase "^3.0.0" cliui "^3.2.0" decamelize "^1.1.1" - getByUUID-caller-file "^1.0.1" + get-caller-file "^1.0.1" os-locale "^1.4.0" read-pkg-up "^1.0.1" require-directory "^2.1.1" @@ -8023,7 +8023,7 @@ yargs@^7.0.0: camelcase "^3.0.0" cliui "^3.2.0" decamelize "^1.1.1" - getByUUID-caller-file "^1.0.1" + get-caller-file "^1.0.1" os-locale "^1.4.0" read-pkg-up "^1.0.1" require-directory "^2.1.1" @@ -8041,7 +8041,7 @@ yargs@^8.0.1, yargs@^8.0.2: camelcase "^4.1.0" cliui "^3.2.0" decamelize "^1.1.1" - getByUUID-caller-file "^1.0.1" + get-caller-file "^1.0.1" os-locale "^2.0.0" read-pkg-up "^2.0.0" require-directory "^2.1.1" From eb3748829631dad0dae3a08490bb0f7aaa95548c Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 11 Jan 2018 10:40:31 +0100 Subject: [PATCH 05/11] commented out the create method until the spec for POST responses is final --- src/app/core/data/data.service.ts | 33 ++++++++++++++++--------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 3e86bb21b0..8cdcac1796 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -110,20 +110,21 @@ export abstract class DataService return this.rdbService.buildSingle(href, this.normalizedResourceType); } - create(dso: DSpaceObject): Observable> { - const postHrefObs = this.getEndpoint(); - - // TODO ID is unknown at this point - const idHrefObs = postHrefObs.map((href: string) => this.getFindByIDHref(href, dso.id)); - - postHrefObs - .filter((href: string) => hasValue(href)) - .take(1) - .subscribe((href: string) => { - const request = new RestRequest(this.requestService.generateRequestId(), href, RestRequestMethod.Post, dso); - this.requestService.configure(request); - }); - - return this.rdbService.buildSingle(idHrefObs, this.normalizedResourceType); - } + // TODO implement, after the structure of the REST server's POST response is finalized + // create(dso: DSpaceObject): Observable> { + // const postHrefObs = this.getEndpoint(); + // + // // TODO ID is unknown at this point + // const idHrefObs = postHrefObs.map((href: string) => this.getFindByIDHref(href, dso.id)); + // + // postHrefObs + // .filter((href: string) => hasValue(href)) + // .take(1) + // .subscribe((href: string) => { + // const request = new RestRequest(this.requestService.generateRequestId(), href, RestRequestMethod.Post, dso); + // this.requestService.configure(request); + // }); + // + // return this.rdbService.buildSingle(idHrefObs, this.normalizedResourceType); + // } } From 6711b63827ab9f25a0f1c92dcb2b343882e15232 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Mon, 15 Jan 2018 10:48:50 +0100 Subject: [PATCH 06/11] Removed errors with status code 200 in mock search service and tests --- src/app/+search-page/search-service/search.service.ts | 6 +++--- src/app/core/metadata/metadata.service.spec.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/+search-page/search-service/search.service.ts b/src/app/+search-page/search-service/search.service.ts index a6648fedd7..64d76afc54 100644 --- a/src/app/+search-page/search-service/search.service.ts +++ b/src/app/+search-page/search-service/search.service.ts @@ -102,7 +102,7 @@ export class SearchService implements OnDestroy { } search(query: string, scopeId?: string, searchOptions?: SearchOptions): Observable>>> { - const error = new RemoteDataError('200', undefined); + const error = undefined; const returningPageInfo = new PageInfo(); if (isNotEmpty(searchOptions)) { @@ -158,7 +158,7 @@ export class SearchService implements OnDestroy { const requestPending = false; const responsePending = false; const isSuccessful = true; - const error = new RemoteDataError('200', undefined); + const error = undefined; return Observable.of(new RemoteData( requestPending, responsePending, @@ -186,7 +186,7 @@ export class SearchService implements OnDestroy { const requestPending = false; const responsePending = false; const isSuccessful = true; - const error = new RemoteDataError('200', undefined);; + const error = undefined; return new RemoteData( requestPending, responsePending, diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index 110a46c1b9..fe783bb678 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -182,7 +182,7 @@ describe('MetadataService', () => { false, false, true, - new RemoteDataError('200', ''), + undefined, MockItem )); } From 334b07ac26f69b7ae4cd63eed8b3943903463dfa Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 17 Jan 2018 15:58:40 +0100 Subject: [PATCH 07/11] added a RestRequest class for each http method --- .../builders/remote-data-build.service.ts | 23 +++--- src/app/core/data/data.service.ts | 11 +-- src/app/core/data/request.models.ts | 82 +++++++++++++++++-- src/app/core/data/request.reducer.spec.ts | 6 +- 4 files changed, 92 insertions(+), 30 deletions(-) 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 6fb24328c2..a986754b0d 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -1,22 +1,21 @@ import { Injectable } from '@angular/core'; import { Observable } from 'rxjs/Observable'; +import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { PaginatedList } from '../../data/paginated-list'; +import { RemoteData } from '../../data/remote-data'; import { RemoteDataError } from '../../data/remote-data-error'; +import { GetRequest } from '../../data/request.models'; +import { RequestEntry } from '../../data/request.reducer'; +import { RequestService } from '../../data/request.service'; +import { GenericConstructor } from '../../shared/generic-constructor'; +import { NormalizedObjectFactory } from '../models/normalized-object-factory'; import { CacheableObject } from '../object-cache.reducer'; import { ObjectCacheService } from '../object-cache.service'; -import { RequestService } from '../../data/request.service'; -import { ResponseCacheService } from '../response-cache.service'; -import { RequestEntry } from '../../data/request.reducer'; -import { hasValue, isNotEmpty } from '../../../shared/empty.util'; +import { DSOSuccessResponse, ErrorResponse } from '../response-cache.models'; import { ResponseCacheEntry } from '../response-cache.reducer'; -import { ErrorResponse, DSOSuccessResponse } from '../response-cache.models'; -import { RemoteData } from '../../data/remote-data'; -import { GenericConstructor } from '../../shared/generic-constructor'; +import { ResponseCacheService } from '../response-cache.service'; import { getMapsTo, getRelationMetadata, getRelationships } from './build-decorators'; -import { NormalizedObjectFactory } from '../models/normalized-object-factory'; -import { RestRequest } from '../../data/request.models'; -import { PageInfo } from '../../shared/page-info.model'; @Injectable() export class RemoteDataBuildService { @@ -169,7 +168,7 @@ export class RemoteDataBuildService { const resourceConstructor = NormalizedObjectFactory.getConstructor(resourceType); if (Array.isArray(normalized[relationship])) { normalized[relationship].forEach((href: string) => { - this.requestService.configure(new RestRequest(this.requestService.generateRequestId(), href)) + this.requestService.configure(new GetRequest(this.requestService.generateRequestId(), href)) }); const rdArr = []; @@ -183,7 +182,7 @@ export class RemoteDataBuildService { links[relationship] = rdArr[0]; } } else { - this.requestService.configure(new RestRequest(this.requestService.generateRequestId(), normalized[relationship])); + this.requestService.configure(new GetRequest(this.requestService.generateRequestId(), normalized[relationship])); // The rest API can return a single URL to represent a list of resources (e.g. /items/:id/bitstreams) // in that case only 1 href will be stored in the normalized obj (so the isArray above fails), diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 8cdcac1796..2d003d6fd1 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -6,19 +6,12 @@ import { RemoteDataBuildService } from '../cache/builders/remote-data-build.serv import { CacheableObject } from '../cache/object-cache.reducer'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; -import { DSpaceObject } from '../shared/dspace-object.model'; import { GenericConstructor } from '../shared/generic-constructor'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { URLCombiner } from '../url-combiner/url-combiner'; import { PaginatedList } from './paginated-list'; import { RemoteData } from './remote-data'; -import { - FindAllOptions, - FindAllRequest, - FindByIDRequest, - RestRequest, - RestRequestMethod -} from './request.models'; +import { FindAllOptions, FindAllRequest, FindByIDRequest, GetRequest } from './request.models'; import { RequestService } from './request.service'; export abstract class DataService extends HALEndpointService { @@ -106,7 +99,7 @@ export abstract class DataService } findByHref(href: string): Observable> { - this.requestService.configure(new RestRequest(this.requestService.generateRequestId(), href)); + this.requestService.configure(new GetRequest(this.requestService.generateRequestId(), href)); return this.rdbService.buildSingle(href, this.normalizedResourceType); } diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 46f40b0abf..fbf394cc01 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -22,14 +22,14 @@ import { ConfigResponseParsingService } from './config-response-parsing.service' export enum RestRequestMethod { Get = 'GET', Post = 'POST', - // Put = 'PUT', - // Delete = 'DELETE', - // Options = 'OPTIONS', - // Head = 'HEAD', - // Patch = 'PATCH' + Put = 'PUT', + Delete = 'DELETE', + Options = 'OPTIONS', + Head = 'HEAD', + Patch = 'PATCH' } -export class RestRequest { +export abstract class RestRequest { constructor( public uuid: string, public href: string, @@ -43,6 +43,76 @@ export class RestRequest { } } +export class GetRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Get, body) + } +} + +export class PostRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Post, body) + } +} + +export class PutRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Put, body) + } +} + +export class DeleteRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Delete, body) + } +} + +export class OptionsRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Options, body) + } +} + +export class HeadRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Head, body) + } +} + +export class PatchRequest extends RestRequest { + constructor( + public uuid: string, + public href: string, + public body?: any + ) { + super(uuid, href, RestRequestMethod.Patch, body) + } +} + export class FindByIDRequest extends RestRequest { constructor( uuid: string, diff --git a/src/app/core/data/request.reducer.spec.ts b/src/app/core/data/request.reducer.spec.ts index 138d3ea5dd..bd8fad5de7 100644 --- a/src/app/core/data/request.reducer.spec.ts +++ b/src/app/core/data/request.reducer.spec.ts @@ -4,7 +4,7 @@ import { requestReducer, RequestState } from './request.reducer'; import { RequestCompleteAction, RequestConfigureAction, RequestExecuteAction } from './request.actions'; -import { RestRequest } from './request.models'; +import { GetRequest, RestRequest } from './request.models'; class NullAction extends RequestCompleteAction { type = null; @@ -22,7 +22,7 @@ describe('requestReducer', () => { const link2 = 'https://dspace7.4science.it/dspace-spring-rest/api/core/items/1911e8a4-6939-490c-b58b-a5d70f8d91fb'; const testState: RequestState = { [id1]: { - request: new RestRequest(id1, link1), + request: new GetRequest(id1, link1), requestPending: false, responsePending: false, completed: false @@ -46,7 +46,7 @@ describe('requestReducer', () => { it('should add the new RestRequest and set \'requestPending\' to true, \'responsePending\' to false and \'completed\' to false for the given RestRequest in the state, in response to a CONFIGURE action', () => { const state = testState; - const request = new RestRequest(id2, link2); + const request = new GetRequest(id2, link2); const action = new RequestConfigureAction(request); const newState = requestReducer(state, action); From f349f27002209c911f72e7ad4a77e38a8e56f6bd Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Wed, 17 Jan 2018 16:29:12 +0100 Subject: [PATCH 08/11] fixed type issues in tests --- src/app/core/data/config-response-parsing.service.spec.ts | 1 + src/app/core/dspace-rest-v2/dspace-rest-v2-response.model.ts | 2 +- src/app/core/metadata/metadata.service.spec.ts | 5 ++++- src/app/core/shared/item.model.spec.ts | 3 --- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/app/core/data/config-response-parsing.service.spec.ts b/src/app/core/data/config-response-parsing.service.spec.ts index dc5c42cbd5..3a09de6e4c 100644 --- a/src/app/core/data/config-response-parsing.service.spec.ts +++ b/src/app/core/data/config-response-parsing.service.spec.ts @@ -1,4 +1,5 @@ import { ConfigSuccessResponse, ErrorResponse } from '../cache/response-cache.models'; +import { DSpaceRESTV2Response } from '../dspace-rest-v2/dspace-rest-v2-response.model'; import { ConfigResponseParsingService } from './config-response-parsing.service'; import { ObjectCacheService } from '../cache/object-cache.service'; import { GlobalConfig } from '../../../config/global-config.interface'; diff --git a/src/app/core/dspace-rest-v2/dspace-rest-v2-response.model.ts b/src/app/core/dspace-rest-v2/dspace-rest-v2-response.model.ts index cb39fc718e..d225eadcc4 100644 --- a/src/app/core/dspace-rest-v2/dspace-rest-v2-response.model.ts +++ b/src/app/core/dspace-rest-v2/dspace-rest-v2-response.model.ts @@ -1,6 +1,6 @@ export interface DSpaceRESTV2Response { payload: { - [name: string]: string; + [name: string]: any; _embedded?: any; _links?: any; page?: any; diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index fe783bb678..4182587cc7 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -12,6 +12,7 @@ import { Store, StoreModule } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; import { RemoteDataError } from '../data/remote-data-error'; +import { UUIDService } from '../shared/uuid.service'; import { MetadataService } from './metadata.service'; @@ -65,6 +66,7 @@ describe('MetadataService', () => { let objectCacheService: ObjectCacheService; let responseCacheService: ResponseCacheService; let requestService: RequestService; + let uuidService: UUIDService; let remoteDataBuildService: RemoteDataBuildService; let itemDataService: ItemDataService; @@ -83,7 +85,8 @@ describe('MetadataService', () => { objectCacheService = new ObjectCacheService(store); responseCacheService = new ResponseCacheService(store); - requestService = new RequestService(objectCacheService, responseCacheService, store); + uuidService = new UUIDService(); + requestService = new RequestService(objectCacheService, responseCacheService, uuidService, store); remoteDataBuildService = new RemoteDataBuildService(objectCacheService, responseCacheService, requestService); TestBed.configureTestingModule({ diff --git a/src/app/core/shared/item.model.spec.ts b/src/app/core/shared/item.model.spec.ts index 1e962f7038..c020cd3454 100644 --- a/src/app/core/shared/item.model.spec.ts +++ b/src/app/core/shared/item.model.spec.ts @@ -104,13 +104,10 @@ describe('Item', () => { function createRemoteDataObject(object: any) { return Observable.of(new RemoteData( - '', false, false, true, undefined, - '200', - new PageInfo(), object )); From 4c361d25b6f1250c80400b46e0b1326509588da3 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 18 Jan 2018 13:51:01 +0100 Subject: [PATCH 09/11] fixed an issue where certain GET requests would be performed more often then needed and added tests for the requestservice (not everything tested yet) --- src/app/core/data/request.models.ts | 10 +- src/app/core/data/request.service.spec.ts | 271 ++++++++++++++++++ src/app/core/data/request.service.ts | 54 ++-- .../shared/mocks/mock-object-cache.service.ts | 7 + src/app/shared/mocks/mock-store.ts | 26 +- src/app/shared/mocks/mock-uuid.service.ts | 9 + 6 files changed, 323 insertions(+), 54 deletions(-) create mode 100644 src/app/core/data/request.service.spec.ts create mode 100644 src/app/shared/mocks/mock-object-cache.service.ts create mode 100644 src/app/shared/mocks/mock-uuid.service.ts diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index fbf394cc01..ee37f9c3d4 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -113,7 +113,7 @@ export class PatchRequest extends RestRequest { } } -export class FindByIDRequest extends RestRequest { +export class FindByIDRequest extends GetRequest { constructor( uuid: string, href: string, @@ -130,7 +130,7 @@ export class FindAllOptions { sort?: SortOptions; } -export class FindAllRequest extends RestRequest { +export class FindAllRequest extends GetRequest { constructor( uuid: string, href: string, @@ -140,7 +140,7 @@ export class FindAllRequest extends RestRequest { } } -export class RootEndpointRequest extends RestRequest { +export class RootEndpointRequest extends GetRequest { constructor(uuid: string, EnvConfig: GlobalConfig) { const href = new RESTURLCombiner(EnvConfig, '/').toString(); super(uuid, href); @@ -151,7 +151,7 @@ export class RootEndpointRequest extends RestRequest { } } -export class BrowseEndpointRequest extends RestRequest { +export class BrowseEndpointRequest extends GetRequest { constructor(uuid: string, href: string) { super(uuid, href); } @@ -161,7 +161,7 @@ export class BrowseEndpointRequest extends RestRequest { } } -export class ConfigRequest extends RestRequest { +export class ConfigRequest extends GetRequest { constructor(uuid: string, href: string) { super(uuid, href); } diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts new file mode 100644 index 0000000000..afc4209d67 --- /dev/null +++ b/src/app/core/data/request.service.spec.ts @@ -0,0 +1,271 @@ +import { Store } from '@ngrx/store'; +import { cold, hot } from 'jasmine-marbles'; +import { Observable } from 'rxjs/Observable'; +import { initMockObjectCacheService } from '../../shared/mocks/mock-object-cache.service'; +import { initMockStore } from '../../shared/mocks/mock-store'; +import { defaultUUID, initMockUUIDService } from '../../shared/mocks/mock-uuid.service'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { ResponseCacheService } from '../cache/response-cache.service'; +import { CoreState } from '../core.reducers'; +import { UUIDService } from '../shared/uuid.service'; +import { + DeleteRequest, GetRequest, + HeadRequest, + OptionsRequest, + PatchRequest, + PostRequest, + PutRequest, RestRequest +} from './request.models'; +import { RequestService } from './request.service'; + +describe('RequestService', () => { + let service: RequestService; + let objectCache: ObjectCacheService; + let responseCache: ResponseCacheService; + let uuidService: UUIDService; + let store: Store; + + const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb'; + const testHref = 'https://rest.api/endpoint/selfLink'; + + function initMockResponseCacheService() { + return jasmine.createSpyObj('responseCache', { + has: true, + get: cold('b-', { + b: { + response: {} + } + }) + }); + } + + function initTestService() { + return new RequestService( + objectCache, + responseCache, + uuidService, + store + ); + } + + function initServices(selectResult: any) { + objectCache = initMockObjectCacheService(); + responseCache = initMockResponseCacheService(); + uuidService = initMockUUIDService(); + store = initMockStore(selectResult); + service = initTestService(); + } + + describe('generateRequestId', () => { + beforeEach(() => { + initServices(Observable.of(undefined)); + }); + + it('should generate a new request ID', () => { + const result = service.generateRequestId(); + const expected = `client/${defaultUUID}`; + + expect(result).toBe(expected); + }); + }); + + describe('isPending', () => { + let testRequest: GetRequest; + describe('before the request is configured', () => { + beforeEach(() => { + initServices(Observable.of(undefined)); + testRequest = new GetRequest(testUUID, testHref) + }); + + it('should return false', () => { + const result = service.isPending(testRequest); + const expected = false; + + expect(result).toBe(expected); + }); + }); + + describe('when the request has been configured but hasn\'t reached the store yet', () => { + beforeEach(() => { + initServices(Observable.of(undefined)); + }); + + it('should return true', () => { + (service as any).requestsOnTheirWayToTheStore = [testHref]; + const result = service.isPending(testRequest); + const expected = true; + + expect(result).toBe(expected); + }); + }); + + describe('when the request has reached the store, before the server responds', () => { + beforeEach(() => { + initServices(Observable.of({ + completed: false + })); + }); + + it('should return true', () => { + const result = service.isPending(testRequest); + const expected = true; + + expect(result).toBe(expected); + }); + }); + + describe('after the server responds', () => { + beforeEach(() => { + initServices(Observable.of({ + completed: true + })); + }); + + it('should return false', () => { + const result = service.isPending(testRequest); + const expected = false; + + expect(result).toBe(expected); + }); + }); + + describe('getByUUID', () => { + describe('if the request with the specified UUID exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: { + completed: true + } + })); + + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: { + completed: true + } + }); + + expect(result).toBeObservable(expected); + }); + }); + + describe('if the request with the specified UUID doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); + + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: undefined + }); + + expect(result).toBeObservable(expected); + }); + }); + + }); + + describe('getByHref', () => { + describe('if the request with the specified href exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: testUUID + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: { + completed: true + } + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: { + completed: true + } + }); + + expect(result).toBeObservable(expected); + }); + }); + + describe('if the request with the specified href doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: undefined + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: undefined + }); + + expect(result).toBeObservable(expected); + }); + }); + }); + + describe('configure', () => { + describe('if the request is a GET request', () => { + describe('and it isn\'t already cached', () => { + it('should dispatch the request', () => { + initServices(Observable.of(undefined)); + spyOn((service as any), 'dispatchRequest'); + spyOn((service as any), 'isCachedOrPending').and.returnValue(false); + + const request = new GetRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + }); + }); + describe('and it is already cached or pending', () => { + it('shouldn\'t dispatch the request', () => { + initServices(Observable.of(undefined)); + spyOn((service as any), 'dispatchRequest'); + spyOn((service as any), 'isCachedOrPending').and.returnValue(true); + + const request = new GetRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).not.toHaveBeenCalled(); + }); + }); + }); + + describe('if the request isn\'t a GET request', () => { + it('should dispatch the request', () => { + initServices(Observable.of(undefined)); + spyOn((service as any), 'dispatchRequest'); + + let request = new PostRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new PutRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new DeleteRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new OptionsRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new HeadRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + + request = new PatchRequest(testUUID, testHref); + service.configure(request); + expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); + }); + }); + }); + + }); + +}); diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 6cfbedb5cb..f589221e63 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -14,24 +14,10 @@ import { IndexName } from '../index/index.reducer'; import { pathSelector } from '../shared/selectors'; import { UUIDService } from '../shared/uuid.service'; import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; -import { RestRequest, RestRequestMethod } from './request.models'; +import { GetRequest, RestRequest, RestRequestMethod } from './request.models'; import { RequestEntry, RequestState } from './request.reducer'; -function entryFromUUIDSelector(uuid: string): MemoizedSelector { - return pathSelector(coreSelector, 'data/request', uuid); -} - -function uuidFromHrefSelector(href: string): MemoizedSelector { - return pathSelector(coreSelector, 'index', IndexName.REQUEST, href); -} - -export function requestStateSelector(): MemoizedSelector { - return createSelector(coreSelector, (state: CoreState) => { - return state['data/request'] as RequestState; - }); -} - @Injectable() export class RequestService { private requestsOnTheirWayToTheStore: string[] = []; @@ -42,19 +28,27 @@ export class RequestService { private store: Store) { } + private entryFromUUIDSelector(uuid: string): MemoizedSelector { + return pathSelector(coreSelector, 'data/request', uuid); + } + + private uuidFromHrefSelector(href: string): MemoizedSelector { + return pathSelector(coreSelector, 'index', IndexName.REQUEST, href); + } + generateRequestId(): string { return `client/${this.uuidService.generate()}`; } - isPending(uuid: string): boolean { + isPending(request: GetRequest): boolean { // first check requests that haven't made it to the store yet - if (this.requestsOnTheirWayToTheStore.includes(uuid)) { + if (this.requestsOnTheirWayToTheStore.includes(request.href)) { return true; } // then check the store let isPending = false; - this.store.select(entryFromUUIDSelector(uuid)) + this.getByHref(request.href) .take(1) .subscribe((re: RequestEntry) => { isPending = (hasValue(re) && !re.completed) @@ -64,11 +58,11 @@ export class RequestService { } getByUUID(uuid: string): Observable { - return this.store.select(entryFromUUIDSelector(uuid)); + return this.store.select(this.entryFromUUIDSelector(uuid)); } getByHref(href: string): Observable { - return this.store.select(uuidFromHrefSelector(href)) + return this.store.select(this.uuidFromHrefSelector(href)) .flatMap((uuid: string) => this.getByUUID(uuid)); } @@ -78,7 +72,7 @@ export class RequestService { } } - private isCachedOrPending(request: RestRequest) { + private isCachedOrPending(request: GetRequest) { let isCached = this.objectCache.hasBySelfLink(request.href); if (!isCached && this.responseCache.has(request.href)) { const [successResponse, errorResponse] = this.responseCache.get(request.href) @@ -102,7 +96,7 @@ export class RequestService { ).subscribe((c) => isCached = c); } - const isPending = this.isPending(request.uuid); + const isPending = this.isPending(request); return isCached || isPending; } @@ -110,23 +104,25 @@ export class RequestService { private dispatchRequest(request: RestRequest) { this.store.dispatch(new RequestConfigureAction(request)); this.store.dispatch(new RequestExecuteAction(request.uuid)); - this.trackRequestsOnTheirWayToTheStore(request.uuid); + if (request.method === RestRequestMethod.Get) { + this.trackRequestsOnTheirWayToTheStore(request); + } } /** * ngrx action dispatches are asynchronous. But this.isPending needs to return true as soon as the - * configure method for a request has been executed, otherwise certain requests will happen multiple times. + * configure method for a GET request has been executed, otherwise certain requests will happen multiple times. * - * This method will store the href of every request that gets configured in a local variable, and + * This method will store the href of every GET request that gets configured in a local variable, and * remove it as soon as it can be found in the store. */ - private trackRequestsOnTheirWayToTheStore(uuid: string) { - this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, uuid]; - this.store.select(entryFromUUIDSelector(uuid)) + private trackRequestsOnTheirWayToTheStore(request: GetRequest) { + this.requestsOnTheirWayToTheStore = [...this.requestsOnTheirWayToTheStore, request.href]; + this.store.select(this.entryFromUUIDSelector(request.href)) .filter((re: RequestEntry) => hasValue(re)) .take(1) .subscribe((re: RequestEntry) => { - this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((pendingUUID: string) => pendingUUID !== uuid) + this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((pendingHref: string) => pendingHref !== request.href) }); } } diff --git a/src/app/shared/mocks/mock-object-cache.service.ts b/src/app/shared/mocks/mock-object-cache.service.ts new file mode 100644 index 0000000000..80f7b102fa --- /dev/null +++ b/src/app/shared/mocks/mock-object-cache.service.ts @@ -0,0 +1,7 @@ +import { ObjectCacheService } from '../../core/cache/object-cache.service'; + +export function initMockObjectCacheService(): ObjectCacheService { + return jasmine.createSpyObj('objectCacheService', { + hasBySelfLink: true, + }); +} diff --git a/src/app/shared/mocks/mock-store.ts b/src/app/shared/mocks/mock-store.ts index c619b5aa77..54b521458d 100644 --- a/src/app/shared/mocks/mock-store.ts +++ b/src/app/shared/mocks/mock-store.ts @@ -1,23 +1,9 @@ -import { Action } from '@ngrx/store'; +import { Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; -import { BehaviorSubject } from 'rxjs/BehaviorSubject'; - -export class MockStore extends BehaviorSubject { - - constructor(private _initialState: T) { - super(_initialState); - } - - dispatch = (action: Action): void => { - console.info(); - } - - select = (pathOrMapFn: any): Observable => { - return Observable.of(this.getValue()); - } - - nextState(_newState: T) { - this.next(_newState); - } +export function initMockStore(selectResult: Observable): Store { + return jasmine.createSpyObj('store', { + dispatch: null, + select: selectResult, + }); } diff --git a/src/app/shared/mocks/mock-uuid.service.ts b/src/app/shared/mocks/mock-uuid.service.ts new file mode 100644 index 0000000000..b82e007ed2 --- /dev/null +++ b/src/app/shared/mocks/mock-uuid.service.ts @@ -0,0 +1,9 @@ +import { UUIDService } from '../../core/shared/uuid.service'; + +export const defaultUUID = 'c4ce6905-290b-478f-979d-a333bbd7820f'; + +export function initMockUUIDService(uuid = defaultUUID): UUIDService { + return jasmine.createSpyObj('uuidService', { + generate: uuid, + }); +} From b771503cf7c692df40684099ba41ebb969089dd8 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Fri, 19 Jan 2018 10:40:04 +0100 Subject: [PATCH 10/11] more tests for request service --- .../builders/remote-data-build.service.ts | 12 +- src/app/core/data/remote-data.ts | 6 +- src/app/core/data/request.service.spec.ts | 463 ++++++++++++------ src/app/shared/mocks/mock-item.ts | 8 +- .../shared/mocks/mock-object-cache.service.ts | 15 +- 5 files changed, 343 insertions(+), 161 deletions(-) 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 a986754b0d..9ed43c242b 100644 --- a/src/app/core/cache/builders/remote-data-build.service.ts +++ b/src/app/core/cache/builders/remote-data-build.service.ts @@ -88,18 +88,18 @@ export class RemoteDataBuildService { (href: string, reqEntry: RequestEntry, resEntry: ResponseCacheEntry, payload: T) => { const requestPending = hasValue(reqEntry.requestPending) ? reqEntry.requestPending : true; const responsePending = hasValue(reqEntry.responsePending) ? reqEntry.responsePending : false; - let isSuccessFul: boolean; + let isSuccessful: boolean; let error: RemoteDataError; if (hasValue(resEntry) && hasValue(resEntry.response)) { - isSuccessFul = resEntry.response.isSuccessful; - const errorMessage = isSuccessFul === false ? (resEntry.response as ErrorResponse).errorMessage : undefined; + isSuccessful = resEntry.response.isSuccessful; + const errorMessage = isSuccessful === false ? (resEntry.response as ErrorResponse).errorMessage : undefined; error = new RemoteDataError(resEntry.response.statusCode, errorMessage); } return new RemoteData( requestPending, responsePending, - isSuccessFul, + isSuccessful, error, payload ); @@ -212,7 +212,7 @@ export class RemoteDataBuildService { .map((d: RemoteData) => d.isResponsePending) .every((b: boolean) => b === true); - const isSuccessFul: boolean = arr + const isSuccessful: boolean = arr .map((d: RemoteData) => d.hasSucceeded) .every((b: boolean) => b === true); @@ -241,7 +241,7 @@ export class RemoteDataBuildService { return new RemoteData( requestPending, responsePending, - isSuccessFul, + isSuccessful, error, payload ); diff --git a/src/app/core/data/remote-data.ts b/src/app/core/data/remote-data.ts index 41953260ac..2aa3227d12 100644 --- a/src/app/core/data/remote-data.ts +++ b/src/app/core/data/remote-data.ts @@ -15,16 +15,16 @@ export class RemoteData { constructor( private requestPending: boolean, private responsePending: boolean, - private isSuccessFul: boolean, + private isSuccessful: boolean, public error: RemoteDataError, public payload: T ) { } get state(): RemoteDataState { - if (this.isSuccessFul === true && hasValue(this.payload)) { + if (this.isSuccessful === true && hasValue(this.payload)) { return RemoteDataState.Success - } else if (this.isSuccessFul === false) { + } else if (this.isSuccessful === false) { return RemoteDataState.Failed } else if (this.requestPending === true) { return RemoteDataState.RequestPending diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index afc4209d67..c4e5eb136e 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -8,18 +8,21 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; import { UUIDService } from '../shared/uuid.service'; +import { RequestConfigureAction, RequestExecuteAction } from './request.actions'; import { - DeleteRequest, GetRequest, + DeleteRequest, + GetRequest, HeadRequest, OptionsRequest, PatchRequest, PostRequest, - PutRequest, RestRequest + PutRequest } from './request.models'; import { RequestService } from './request.service'; describe('RequestService', () => { let service: RequestService; + let serviceAsAny: any; let objectCache: ObjectCacheService; let responseCache: ResponseCacheService; let uuidService: UUIDService; @@ -27,15 +30,23 @@ describe('RequestService', () => { const testUUID = '5f2a0d2a-effa-4d54-bd54-5663b960f9eb'; const testHref = 'https://rest.api/endpoint/selfLink'; + const testGetRequest = new GetRequest(testUUID, testHref); + const testPostRequest = new PostRequest(testUUID, testHref); + const testPutRequest = new PutRequest(testUUID, testHref); + const testDeleteRequest = new DeleteRequest(testUUID, testHref); + const testOptionsRequest = new OptionsRequest(testUUID, testHref); + const testHeadRequest = new HeadRequest(testUUID, testHref); + const testPatchRequest = new PatchRequest(testUUID, testHref); - function initMockResponseCacheService() { + const defaultSelectResult = Observable.of(undefined); + const defaultHasResponse = false; + const defaultGetResponse = Observable.of(undefined); + const defaultHasObjectCache = false; + + function initMockResponseCacheService(hasResponse, getResponse) { return jasmine.createSpyObj('responseCache', { - has: true, - get: cold('b-', { - b: { - response: {} - } - }) + has: hasResponse, + get: getResponse }); } @@ -48,17 +59,22 @@ describe('RequestService', () => { ); } - function initServices(selectResult: any) { + function initServices(selectResult = defaultSelectResult, hasResponse = defaultHasResponse, getResponse = defaultGetResponse, hasObjectCache: boolean | boolean[] = defaultHasObjectCache) { + if (!Array.isArray(hasObjectCache)) { + hasObjectCache = [hasObjectCache]; + } objectCache = initMockObjectCacheService(); - responseCache = initMockResponseCacheService(); + (objectCache.hasBySelfLink as any).and.returnValues(...hasObjectCache); + responseCache = initMockResponseCacheService(hasResponse, getResponse); uuidService = initMockUUIDService(); store = initMockStore(selectResult); service = initTestService(); + serviceAsAny = service as any; } describe('generateRequestId', () => { beforeEach(() => { - initServices(Observable.of(undefined)); + initServices(); }); it('should generate a new request ID', () => { @@ -70,15 +86,13 @@ describe('RequestService', () => { }); describe('isPending', () => { - let testRequest: GetRequest; describe('before the request is configured', () => { beforeEach(() => { - initServices(Observable.of(undefined)); - testRequest = new GetRequest(testUUID, testHref) + initServices(); }); it('should return false', () => { - const result = service.isPending(testRequest); + const result = service.isPending(testGetRequest); const expected = false; expect(result).toBe(expected); @@ -87,12 +101,12 @@ describe('RequestService', () => { describe('when the request has been configured but hasn\'t reached the store yet', () => { beforeEach(() => { - initServices(Observable.of(undefined)); + initServices(); }); it('should return true', () => { - (service as any).requestsOnTheirWayToTheStore = [testHref]; - const result = service.isPending(testRequest); + serviceAsAny.requestsOnTheirWayToTheStore = [testHref]; + const result = service.isPending(testGetRequest); const expected = true; expect(result).toBe(expected); @@ -107,7 +121,7 @@ describe('RequestService', () => { }); it('should return true', () => { - const result = service.isPending(testRequest); + const result = service.isPending(testGetRequest); const expected = true; expect(result).toBe(expected); @@ -122,150 +136,309 @@ describe('RequestService', () => { }); it('should return false', () => { - const result = service.isPending(testRequest); + const result = service.isPending(testGetRequest); const expected = false; expect(result).toBe(expected); }); }); - describe('getByUUID', () => { - describe('if the request with the specified UUID exists in the store', () => { - it('should return an Observable of the RequestEntry', () => { - initServices(hot('a', { - a: { - completed: true - } - })); + }); - const result = service.getByUUID(testUUID); - const expected = cold('b', { - b: { - completed: true - } - }); + describe('getByUUID', () => { + describe('if the request with the specified UUID exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: { + completed: true + } + })); - expect(result).toBeObservable(expected); + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: { + completed: true + } }); - }); - describe('if the request with the specified UUID doesn\'t exist in the store', () => { - it('should return an Observable of undefined', () => { - initServices(hot('a', { - a: undefined - })); - - const result = service.getByUUID(testUUID); - const expected = cold('b', { - b: undefined - }); - - expect(result).toBeObservable(expected); - }); - }); - - }); - - describe('getByHref', () => { - describe('if the request with the specified href exists in the store', () => { - it('should return an Observable of the RequestEntry', () => { - initServices(hot('a', { - a: testUUID - })); - spyOn(service, 'getByUUID').and.returnValue(cold('b', { - b: { - completed: true - } - })); - - const result = service.getByHref(testHref); - const expected = cold('c', { - c: { - completed: true - } - }); - - expect(result).toBeObservable(expected); - }); - }); - - describe('if the request with the specified href doesn\'t exist in the store', () => { - it('should return an Observable of undefined', () => { - initServices(hot('a', { - a: undefined - })); - spyOn(service, 'getByUUID').and.returnValue(cold('b', { - b: undefined - })); - - const result = service.getByHref(testHref); - const expected = cold('c', { - c: undefined - }); - - expect(result).toBeObservable(expected); - }); + expect(result).toBeObservable(expected); }); }); - describe('configure', () => { - describe('if the request is a GET request', () => { - describe('and it isn\'t already cached', () => { - it('should dispatch the request', () => { - initServices(Observable.of(undefined)); - spyOn((service as any), 'dispatchRequest'); - spyOn((service as any), 'isCachedOrPending').and.returnValue(false); + describe('if the request with the specified UUID doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); - const request = new GetRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - }); + const result = service.getByUUID(testUUID); + const expected = cold('b', { + b: undefined }); - describe('and it is already cached or pending', () => { - it('shouldn\'t dispatch the request', () => { - initServices(Observable.of(undefined)); - spyOn((service as any), 'dispatchRequest'); - spyOn((service as any), 'isCachedOrPending').and.returnValue(true); - const request = new GetRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).not.toHaveBeenCalled(); - }); - }); - }); - - describe('if the request isn\'t a GET request', () => { - it('should dispatch the request', () => { - initServices(Observable.of(undefined)); - spyOn((service as any), 'dispatchRequest'); - - let request = new PostRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new PutRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new DeleteRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new OptionsRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new HeadRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - - request = new PatchRequest(testUUID, testHref); - service.configure(request); - expect((service as any).dispatchRequest).toHaveBeenCalledWith(request); - }); + expect(result).toBeObservable(expected); }); }); }); + describe('getByHref', () => { + describe('if the request with the specified href exists in the store', () => { + it('should return an Observable of the RequestEntry', () => { + initServices(hot('a', { + a: testUUID + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: { + completed: true + } + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: { + completed: true + } + }); + + expect(result).toBeObservable(expected); + }); + }); + + describe('if the request with the specified href doesn\'t exist in the store', () => { + it('should return an Observable of undefined', () => { + initServices(hot('a', { + a: undefined + })); + spyOn(service, 'getByUUID').and.returnValue(cold('b', { + b: undefined + })); + + const result = service.getByHref(testHref); + const expected = cold('c', { + c: undefined + }); + + expect(result).toBeObservable(expected); + }); + }); + }); + + describe('configure', () => { + describe('if the request is a GET request', () => { + describe('and it isn\'t already cached', () => { + it('should dispatch the request', () => { + initServices(); + spyOn(serviceAsAny, 'dispatchRequest'); + spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(false); + + service.configure(testGetRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testGetRequest); + }); + }); + describe('and it is already cached or pending', () => { + it('shouldn\'t dispatch the request', () => { + initServices(); + spyOn(serviceAsAny, 'dispatchRequest'); + spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(true); + + service.configure(testGetRequest); + expect(serviceAsAny.dispatchRequest).not.toHaveBeenCalled(); + }); + }); + }); + + describe('if the request isn\'t a GET request', () => { + it('should dispatch the request', () => { + initServices(); + spyOn(serviceAsAny, 'dispatchRequest'); + + service.configure(testPostRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPostRequest); + + service.configure(testPutRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPutRequest); + + service.configure(testDeleteRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testDeleteRequest); + + service.configure(testOptionsRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testOptionsRequest); + + service.configure(testHeadRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testHeadRequest); + + service.configure(testPatchRequest); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPatchRequest); + }); + }); + }); + + describe('isCachedOrPending', () => { + describe('when the request is cached', () => { + describe('in the ObjectCache', () => { + it('should return true', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true + } + } + ), true); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + }); + describe('in the responseCache', () => { + describe('and it\'s a DSOSuccessResponse', () => { + it('should return true if all top level links in the response are cached in the object cache', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true, + resourceSelfLinks: [ + 'https://rest.api/endpoint/selfLink1', + 'https://rest.api/endpoint/selfLink2' + ] + } + } + ), [false, true, true]); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + it('should return false if not all top level links in the response are cached in the object cache', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true, + resourceSelfLinks: [ + 'https://rest.api/endpoint/selfLink1', + 'https://rest.api/endpoint/selfLink2' + ] + } + } + ), [false, true, false]); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = false; + + expect(result).toEqual(expected); + }); + }); + describe('and it isn\'t a DSOSuccessResponse', () => { + it('should return true', () => { + initServices(defaultSelectResult, true, Observable.of({ + response: { + isSuccessful: true + } + } + ), false); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + }); + }); + }); + + describe('when the request is pending', () => { + it('should return true', () => { + initServices(); + spyOn(service, 'isPending').and.returnValue(true); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = true; + + expect(result).toEqual(expected); + }); + }); + + describe('when the request is neither cached nor pending', () => { + it('should return false', () => { + initServices(); + + const result = serviceAsAny.isCachedOrPending(testGetRequest); + const expected = false; + + expect(result).toEqual(expected); + }); + }); + }); + + describe('dispatchRequest', () => { + beforeEach(() => { + initServices(); + }); + it('should dispatch a RequestConfigureAction', () => { + const request = testGetRequest; + serviceAsAny.dispatchRequest(request); + expect(store.dispatch).toHaveBeenCalledWith(new RequestConfigureAction(request)); + }); + it('should dispatch a RequestExecuteAction', () => { + const request = testGetRequest; + serviceAsAny.dispatchRequest(request); + expect(store.dispatch).toHaveBeenCalledWith(new RequestExecuteAction(request.uuid)); + }); + describe('when it\'s a GET request', () => { + it('should track it on it\'s way to the store', () => { + spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore'); + const request = testGetRequest; + serviceAsAny.dispatchRequest(request); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).toHaveBeenCalledWith(request); + }); + }); + describe('when it\'s not a GET request', () => { + it('shouldn\'t track it', () => { + spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore'); + + serviceAsAny.dispatchRequest(testPostRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testPutRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testDeleteRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testOptionsRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testHeadRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + + serviceAsAny.dispatchRequest(testPatchRequest); + expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).not.toHaveBeenCalled(); + }); + }); + }); + + describe('trackRequestsOnTheirWayToTheStore', () => { + let request: GetRequest; + + beforeEach(() => { + initServices(); + request = testGetRequest; + }); + + describe('when the method is called with a new request', () => { + it('should start tracking the request', () => { + expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy(); + serviceAsAny.trackRequestsOnTheirWayToTheStore(request); + expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeTruthy(); + }); + }); + + describe('when the request is added to the store', () => { + it('should stop tracking the request', () => { + (store.select as any).and.returnValue(Observable.of({ request })); + serviceAsAny.trackRequestsOnTheirWayToTheStore(request); + expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy(); + }); + }); + }); }); diff --git a/src/app/shared/mocks/mock-item.ts b/src/app/shared/mocks/mock-item.ts index 6b34a31888..81c1fcf26c 100644 --- a/src/app/shared/mocks/mock-item.ts +++ b/src/app/shared/mocks/mock-item.ts @@ -13,7 +13,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'dspace-angular://aggregated/object/1507836003548', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, @@ -25,7 +25,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/10', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, @@ -60,7 +60,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/bitstreamformats/4', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, @@ -191,7 +191,7 @@ export const MockItem: Item = Object.assign(new Item(), { self: 'https://dspace7.4science.it/dspace-spring-rest/api/core/collections/1c11f3f1-ba1f-4f36-908a-3f1ea9a557eb', requestPending: false, responsePending: false, - isSuccessFul: true, + isSuccessful: true, errorMessage: '', statusCode: '202', pageInfo: {}, diff --git a/src/app/shared/mocks/mock-object-cache.service.ts b/src/app/shared/mocks/mock-object-cache.service.ts index 80f7b102fa..4fd311465e 100644 --- a/src/app/shared/mocks/mock-object-cache.service.ts +++ b/src/app/shared/mocks/mock-object-cache.service.ts @@ -1,7 +1,16 @@ import { ObjectCacheService } from '../../core/cache/object-cache.service'; export function initMockObjectCacheService(): ObjectCacheService { - return jasmine.createSpyObj('objectCacheService', { - hasBySelfLink: true, - }); + return jasmine.createSpyObj('objectCacheService', [ + 'add', + 'remove', + 'getByUUID', + 'getBySelfLink', + 'getRequestHrefBySelfLink', + 'getRequestHrefByUUID', + 'getList', + 'hasByUUID', + 'hasBySelfLink' + ]); + } From 1d48172ecd44482f199860dc1681547142c33ce9 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Fri, 19 Jan 2018 12:47:29 +0100 Subject: [PATCH 11/11] refactored tests --- src/app/core/browse/browse.service.spec.ts | 27 +-- src/app/core/config/config.service.spec.ts | 4 +- src/app/core/data/comcol-data.service.spec.ts | 10 +- src/app/core/data/request.service.spec.ts | 198 +++++++++--------- .../core/shared/hal-endpoint.service.spec.ts | 4 +- .../shared/mocks/mock-object-cache.service.ts | 2 +- src/app/shared/mocks/mock-request.service.ts | 2 +- .../mocks/mock-response-cache.service.ts | 10 + src/app/shared/mocks/mock-store.ts | 16 +- src/app/shared/mocks/mock-uuid.service.ts | 2 +- 10 files changed, 147 insertions(+), 128 deletions(-) create mode 100644 src/app/shared/mocks/mock-response-cache.service.ts diff --git a/src/app/core/browse/browse.service.spec.ts b/src/app/core/browse/browse.service.spec.ts index 30429c5a8c..764107837b 100644 --- a/src/app/core/browse/browse.service.spec.ts +++ b/src/app/core/browse/browse.service.spec.ts @@ -1,4 +1,5 @@ -import { initMockRequestService } from '../../shared/mocks/mock-request.service'; +import { getMockRequestService } from '../../shared/mocks/mock-request.service'; +import { getMockResponseCacheService } from '../../shared/mocks/mock-response-cache.service'; import { BrowseService } from './browse.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { RequestService } from '../data/request.service'; @@ -74,16 +75,16 @@ describe('BrowseService', () => { ]; function initMockResponseCacheService(isSuccessful: boolean) { - return jasmine.createSpyObj('responseCache', { - get: cold('b-', { - b: { - response: { - isSuccessful, - browseDefinitions, - } + const rcs = getMockResponseCacheService(); + (rcs.get as any).and.returnValue(cold('b-', { + b: { + response: { + isSuccessful, + browseDefinitions, } - }) - }); + } + })); + return rcs; } function initTestService() { @@ -103,7 +104,7 @@ describe('BrowseService', () => { describe('if getEndpoint fires', () => { beforeEach(() => { responseCache = initMockResponseCacheService(true); - requestService = initMockRequestService(); + requestService = getMockRequestService(); service = initTestService(); spyOn(service, 'getEndpoint').and .returnValue(hot('--a-', { a: browsesEndpointURL })); @@ -168,7 +169,7 @@ describe('BrowseService', () => { describe('if getEndpoint doesn\'t fire', () => { it('should return undefined', () => { responseCache = initMockResponseCacheService(true); - requestService = initMockRequestService(); + requestService = getMockRequestService(); service = initTestService(); spyOn(service, 'getEndpoint').and .returnValue(hot('----')); @@ -185,7 +186,7 @@ describe('BrowseService', () => { describe('if the browses endpoint can\'t be retrieved', () => { it('should throw an error', () => { responseCache = initMockResponseCacheService(false); - requestService = initMockRequestService(); + requestService = getMockRequestService(); service = initTestService(); spyOn(service, 'getEndpoint').and .returnValue(hot('--a-', { a: browsesEndpointURL })); diff --git a/src/app/core/config/config.service.spec.ts b/src/app/core/config/config.service.spec.ts index 111e1d3eaa..b0c364a86e 100644 --- a/src/app/core/config/config.service.spec.ts +++ b/src/app/core/config/config.service.spec.ts @@ -1,7 +1,7 @@ import { cold, getTestScheduler, hot } from 'jasmine-marbles'; import { TestScheduler } from 'rxjs/Rx'; import { GlobalConfig } from '../../../config'; -import { initMockRequestService } from '../../shared/mocks/mock-request.service'; +import { getMockRequestService } from '../../shared/mocks/mock-request.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { ConfigService } from './config.service'; import { RequestService } from '../data/request.service'; @@ -57,7 +57,7 @@ describe('ConfigService', () => { beforeEach(() => { responseCache = initMockResponseCacheService(true); - requestService = initMockRequestService(); + requestService = getMockRequestService(); service = initTestService(); scheduler = getTestScheduler(); spyOn(service, 'getEndpoint').and diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index d34ffc2277..fefe7d3730 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -2,7 +2,7 @@ import { Store } from '@ngrx/store'; import { cold, getTestScheduler, hot } from 'jasmine-marbles'; import { TestScheduler } from 'rxjs/Rx'; import { GlobalConfig } from '../../../config'; -import { initMockRequestService } from '../../shared/mocks/mock-request.service'; +import { getMockRequestService } from '../../shared/mocks/mock-request.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { NormalizedCommunity } from '../cache/models/normalized-community.model'; import { CacheableObject } from '../cache/object-cache.reducer'; @@ -102,7 +102,7 @@ describe('ComColDataService', () => { it('should configure a new FindByIDRequest for the scope Community', () => { cds = initMockCommunityDataService(); - requestService = initMockRequestService(); + requestService = getMockRequestService(); objectCache = initMockObjectCacheService(); responseCache = initMockResponseCacheService(true); service = initTestService(); @@ -118,7 +118,7 @@ describe('ComColDataService', () => { describe('if the scope Community can be found', () => { beforeEach(() => { cds = initMockCommunityDataService(); - requestService = initMockRequestService(); + requestService = getMockRequestService(); objectCache = initMockObjectCacheService(); responseCache = initMockResponseCacheService(true); service = initTestService(); @@ -141,7 +141,7 @@ describe('ComColDataService', () => { describe('if the scope Community can\'t be found', () => { beforeEach(() => { cds = initMockCommunityDataService(); - requestService = initMockRequestService(); + requestService = getMockRequestService(); objectCache = initMockObjectCacheService(); responseCache = initMockResponseCacheService(false); service = initTestService(); @@ -158,7 +158,7 @@ describe('ComColDataService', () => { describe('if the scope is not specified', () => { beforeEach(() => { cds = initMockCommunityDataService(); - requestService = initMockRequestService(); + requestService = getMockRequestService(); objectCache = initMockObjectCacheService(); responseCache = initMockResponseCacheService(true); service = initTestService(); diff --git a/src/app/core/data/request.service.spec.ts b/src/app/core/data/request.service.spec.ts index c4e5eb136e..17d4a89d05 100644 --- a/src/app/core/data/request.service.spec.ts +++ b/src/app/core/data/request.service.spec.ts @@ -1,9 +1,10 @@ import { Store } from '@ngrx/store'; import { cold, hot } from 'jasmine-marbles'; import { Observable } from 'rxjs/Observable'; -import { initMockObjectCacheService } from '../../shared/mocks/mock-object-cache.service'; -import { initMockStore } from '../../shared/mocks/mock-store'; -import { defaultUUID, initMockUUIDService } from '../../shared/mocks/mock-uuid.service'; +import { getMockObjectCacheService } from '../../shared/mocks/mock-object-cache.service'; +import { getMockResponseCacheService } from '../../shared/mocks/mock-response-cache.service'; +import { getMockStore } from '../../shared/mocks/mock-store'; +import { defaultUUID, getMockUUIDService } from '../../shared/mocks/mock-uuid.service'; import { ObjectCacheService } from '../cache/object-cache.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; @@ -16,7 +17,7 @@ import { OptionsRequest, PatchRequest, PostRequest, - PutRequest + PutRequest, RestRequest } from './request.models'; import { RequestService } from './request.service'; @@ -38,45 +39,29 @@ describe('RequestService', () => { const testHeadRequest = new HeadRequest(testUUID, testHref); const testPatchRequest = new PatchRequest(testUUID, testHref); - const defaultSelectResult = Observable.of(undefined); - const defaultHasResponse = false; - const defaultGetResponse = Observable.of(undefined); - const defaultHasObjectCache = false; + beforeEach(() => { + objectCache = getMockObjectCacheService(); + (objectCache.hasBySelfLink as any).and.returnValue(false); - function initMockResponseCacheService(hasResponse, getResponse) { - return jasmine.createSpyObj('responseCache', { - has: hasResponse, - get: getResponse - }); - } + responseCache = getMockResponseCacheService(); + (responseCache.has as any).and.returnValue(false); + (responseCache.get as any).and.returnValue(Observable.of(undefined)); - function initTestService() { - return new RequestService( + uuidService = getMockUUIDService(); + + store = getMockStore(); + (store.select as any).and.returnValue(Observable.of(undefined)); + + service = new RequestService( objectCache, responseCache, uuidService, store ); - } - - function initServices(selectResult = defaultSelectResult, hasResponse = defaultHasResponse, getResponse = defaultGetResponse, hasObjectCache: boolean | boolean[] = defaultHasObjectCache) { - if (!Array.isArray(hasObjectCache)) { - hasObjectCache = [hasObjectCache]; - } - objectCache = initMockObjectCacheService(); - (objectCache.hasBySelfLink as any).and.returnValues(...hasObjectCache); - responseCache = initMockResponseCacheService(hasResponse, getResponse); - uuidService = initMockUUIDService(); - store = initMockStore(selectResult); - service = initTestService(); serviceAsAny = service as any; - } + }); describe('generateRequestId', () => { - beforeEach(() => { - initServices(); - }); - it('should generate a new request ID', () => { const result = service.generateRequestId(); const expected = `client/${defaultUUID}`; @@ -88,7 +73,7 @@ describe('RequestService', () => { describe('isPending', () => { describe('before the request is configured', () => { beforeEach(() => { - initServices(); + spyOn(service, 'getByHref').and.returnValue(Observable.of(undefined)); }); it('should return false', () => { @@ -101,11 +86,11 @@ describe('RequestService', () => { describe('when the request has been configured but hasn\'t reached the store yet', () => { beforeEach(() => { - initServices(); + spyOn(service, 'getByHref').and.returnValue(Observable.of(undefined)); + serviceAsAny.requestsOnTheirWayToTheStore = [testHref]; }); it('should return true', () => { - serviceAsAny.requestsOnTheirWayToTheStore = [testHref]; const result = service.isPending(testGetRequest); const expected = true; @@ -115,9 +100,9 @@ describe('RequestService', () => { describe('when the request has reached the store, before the server responds', () => { beforeEach(() => { - initServices(Observable.of({ + spyOn(service, 'getByHref').and.returnValue(Observable.of({ completed: false - })); + })) }); it('should return true', () => { @@ -130,7 +115,7 @@ describe('RequestService', () => { describe('after the server responds', () => { beforeEach(() => { - initServices(Observable.of({ + spyOn(service, 'getByHref').and.returnValues(Observable.of({ completed: true })); }); @@ -147,13 +132,15 @@ describe('RequestService', () => { describe('getByUUID', () => { describe('if the request with the specified UUID exists in the store', () => { - it('should return an Observable of the RequestEntry', () => { - initServices(hot('a', { + beforeEach(() => { + (store.select as any).and.returnValues(hot('a', { a: { completed: true } })); + }); + it('should return an Observable of the RequestEntry', () => { const result = service.getByUUID(testUUID); const expected = cold('b', { b: { @@ -166,11 +153,13 @@ describe('RequestService', () => { }); describe('if the request with the specified UUID doesn\'t exist in the store', () => { - it('should return an Observable of undefined', () => { - initServices(hot('a', { + beforeEach(() => { + (store.select as any).and.returnValues(hot('a', { a: undefined })); + }); + it('should return an Observable of undefined', () => { const result = service.getByUUID(testUUID); const expected = cold('b', { b: undefined @@ -183,9 +172,9 @@ describe('RequestService', () => { }); describe('getByHref', () => { - describe('if the request with the specified href exists in the store', () => { - it('should return an Observable of the RequestEntry', () => { - initServices(hot('a', { + describe('when the request with the specified href exists in the store', () => { + beforeEach(() => { + (store.select as any).and.returnValues(hot('a', { a: testUUID })); spyOn(service, 'getByUUID').and.returnValue(cold('b', { @@ -193,7 +182,9 @@ describe('RequestService', () => { completed: true } })); + }); + it('should return an Observable of the RequestEntry', () => { const result = service.getByHref(testHref); const expected = cold('c', { c: { @@ -205,15 +196,17 @@ describe('RequestService', () => { }); }); - describe('if the request with the specified href doesn\'t exist in the store', () => { - it('should return an Observable of undefined', () => { - initServices(hot('a', { + describe('when the request with the specified href doesn\'t exist in the store', () => { + beforeEach(() => { + (store.select as any).and.returnValues(hot('a', { a: undefined })); spyOn(service, 'getByUUID').and.returnValue(cold('b', { b: undefined })); + }); + it('should return an Observable of undefined', () => { const result = service.getByHref(testHref); const expected = cold('c', { c: undefined @@ -225,34 +218,41 @@ describe('RequestService', () => { }); describe('configure', () => { - describe('if the request is a GET request', () => { - describe('and it isn\'t already cached', () => { - it('should dispatch the request', () => { - initServices(); - spyOn(serviceAsAny, 'dispatchRequest'); - spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(false); + beforeEach(() => { + spyOn(serviceAsAny, 'dispatchRequest'); + }); - service.configure(testGetRequest); - expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testGetRequest); + describe('when the request is a GET request', () => { + let request: RestRequest; + + beforeEach(() => { + request = testGetRequest; + }); + + describe('and it isn\'t cached or pending', () => { + beforeEach(() => { + spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(false); + }); + + it('should dispatch the request', () => { + service.configure(request); + expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(request); }); }); describe('and it is already cached or pending', () => { - it('shouldn\'t dispatch the request', () => { - initServices(); - spyOn(serviceAsAny, 'dispatchRequest'); + beforeEach(() => { spyOn(serviceAsAny, 'isCachedOrPending').and.returnValue(true); + }); - service.configure(testGetRequest); + it('shouldn\'t dispatch the request', () => { + service.configure(request); expect(serviceAsAny.dispatchRequest).not.toHaveBeenCalled(); }); }); }); - describe('if the request isn\'t a GET request', () => { + describe('when the request isn\'t a GET request', () => { it('should dispatch the request', () => { - initServices(); - spyOn(serviceAsAny, 'dispatchRequest'); - service.configure(testPostRequest); expect(serviceAsAny.dispatchRequest).toHaveBeenCalledWith(testPostRequest); @@ -277,14 +277,11 @@ describe('RequestService', () => { describe('isCachedOrPending', () => { describe('when the request is cached', () => { describe('in the ObjectCache', () => { - it('should return true', () => { - initServices(defaultSelectResult, true, Observable.of({ - response: { - isSuccessful: true - } - } - ), true); + beforeEach(() => { + (objectCache.hasBySelfLink as any).and.returnValues(true); + }); + it('should return true', () => { const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = true; @@ -292,9 +289,13 @@ describe('RequestService', () => { }); }); describe('in the responseCache', () => { + beforeEach(() => { + (responseCache.has as any).and.returnValues(true); + }); + describe('and it\'s a DSOSuccessResponse', () => { - it('should return true if all top level links in the response are cached in the object cache', () => { - initServices(defaultSelectResult, true, Observable.of({ + beforeEach(() => { + (responseCache.get as any).and.returnValues(Observable.of({ response: { isSuccessful: true, resourceSelfLinks: [ @@ -303,7 +304,11 @@ describe('RequestService', () => { ] } } - ), [false, true, true]); + )); + }); + + it('should return true if all top level links in the response are cached in the object cache', () => { + (objectCache.hasBySelfLink as any).and.returnValues(false, true, true); const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = true; @@ -311,16 +316,7 @@ describe('RequestService', () => { expect(result).toEqual(expected); }); it('should return false if not all top level links in the response are cached in the object cache', () => { - initServices(defaultSelectResult, true, Observable.of({ - response: { - isSuccessful: true, - resourceSelfLinks: [ - 'https://rest.api/endpoint/selfLink1', - 'https://rest.api/endpoint/selfLink2' - ] - } - } - ), [false, true, false]); + (objectCache.hasBySelfLink as any).and.returnValues(false, true, false); const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = false; @@ -329,14 +325,18 @@ describe('RequestService', () => { }); }); describe('and it isn\'t a DSOSuccessResponse', () => { - it('should return true', () => { - initServices(defaultSelectResult, true, Observable.of({ + beforeEach(() => { + (objectCache.hasBySelfLink as any).and.returnValues(false); + (responseCache.has as any).and.returnValues(true); + (responseCache.get as any).and.returnValues(Observable.of({ response: { isSuccessful: true } } - ), false); + )); + }); + it('should return true', () => { const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = true; @@ -347,10 +347,11 @@ describe('RequestService', () => { }); describe('when the request is pending', () => { - it('should return true', () => { - initServices(); + beforeEach(() => { spyOn(service, 'isPending').and.returnValue(true); + }); + it('should return true', () => { const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = true; @@ -360,8 +361,6 @@ describe('RequestService', () => { describe('when the request is neither cached nor pending', () => { it('should return false', () => { - initServices(); - const result = serviceAsAny.isCachedOrPending(testGetRequest); const expected = false; @@ -371,27 +370,31 @@ describe('RequestService', () => { }); describe('dispatchRequest', () => { - beforeEach(() => { - initServices(); - }); it('should dispatch a RequestConfigureAction', () => { const request = testGetRequest; serviceAsAny.dispatchRequest(request); expect(store.dispatch).toHaveBeenCalledWith(new RequestConfigureAction(request)); }); + it('should dispatch a RequestExecuteAction', () => { const request = testGetRequest; serviceAsAny.dispatchRequest(request); expect(store.dispatch).toHaveBeenCalledWith(new RequestExecuteAction(request.uuid)); }); + describe('when it\'s a GET request', () => { + let request: RestRequest; + beforeEach(() => { + request = testGetRequest; + }); + it('should track it on it\'s way to the store', () => { spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore'); - const request = testGetRequest; serviceAsAny.dispatchRequest(request); expect(serviceAsAny.trackRequestsOnTheirWayToTheStore).toHaveBeenCalledWith(request); }); }); + describe('when it\'s not a GET request', () => { it('shouldn\'t track it', () => { spyOn(serviceAsAny, 'trackRequestsOnTheirWayToTheStore'); @@ -421,7 +424,6 @@ describe('RequestService', () => { let request: GetRequest; beforeEach(() => { - initServices(); request = testGetRequest; }); @@ -435,7 +437,7 @@ describe('RequestService', () => { describe('when the request is added to the store', () => { it('should stop tracking the request', () => { - (store.select as any).and.returnValue(Observable.of({ request })); + (store.select as any).and.returnValues(Observable.of({ request })); serviceAsAny.trackRequestsOnTheirWayToTheStore(request); expect(serviceAsAny.requestsOnTheirWayToTheStore.includes(request.href)).toBeFalsy(); }); diff --git a/src/app/core/shared/hal-endpoint.service.spec.ts b/src/app/core/shared/hal-endpoint.service.spec.ts index 22d9a15fd7..a47bfd745c 100644 --- a/src/app/core/shared/hal-endpoint.service.spec.ts +++ b/src/app/core/shared/hal-endpoint.service.spec.ts @@ -1,6 +1,6 @@ import { cold, hot } from 'jasmine-marbles'; import { GlobalConfig } from '../../../config/global-config.interface'; -import { initMockRequestService } from '../../shared/mocks/mock-request.service'; +import { getMockRequestService } from '../../shared/mocks/mock-request.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { RootEndpointRequest } from '../data/request.models'; import { RequestService } from '../data/request.service'; @@ -39,7 +39,7 @@ describe('HALEndpointService', () => { }) }); - requestService = initMockRequestService(); + requestService = getMockRequestService(); envConfig = { rest: { baseUrl: 'https://rest.api/' } diff --git a/src/app/shared/mocks/mock-object-cache.service.ts b/src/app/shared/mocks/mock-object-cache.service.ts index 4fd311465e..9e35a519ff 100644 --- a/src/app/shared/mocks/mock-object-cache.service.ts +++ b/src/app/shared/mocks/mock-object-cache.service.ts @@ -1,6 +1,6 @@ import { ObjectCacheService } from '../../core/cache/object-cache.service'; -export function initMockObjectCacheService(): ObjectCacheService { +export function getMockObjectCacheService(): ObjectCacheService { return jasmine.createSpyObj('objectCacheService', [ 'add', 'remove', diff --git a/src/app/shared/mocks/mock-request.service.ts b/src/app/shared/mocks/mock-request.service.ts index 9ece96da3e..ed8ffa028d 100644 --- a/src/app/shared/mocks/mock-request.service.ts +++ b/src/app/shared/mocks/mock-request.service.ts @@ -1,6 +1,6 @@ import { RequestService } from '../../core/data/request.service'; -export function initMockRequestService(): RequestService { +export function getMockRequestService(): RequestService { return jasmine.createSpyObj('requestService', { configure: () => false, generateRequestId: () => 'clients/b186e8ce-e99c-4183-bc9a-42b4821bdb78' diff --git a/src/app/shared/mocks/mock-response-cache.service.ts b/src/app/shared/mocks/mock-response-cache.service.ts new file mode 100644 index 0000000000..95b4e7aca0 --- /dev/null +++ b/src/app/shared/mocks/mock-response-cache.service.ts @@ -0,0 +1,10 @@ +import { ResponseCacheService } from '../../core/cache/response-cache.service'; + +export function getMockResponseCacheService(): ResponseCacheService { + return jasmine.createSpyObj('ResponseCacheService', [ + 'add', + 'get', + 'has', + ]); + +} diff --git a/src/app/shared/mocks/mock-store.ts b/src/app/shared/mocks/mock-store.ts index 54b521458d..73c87d324a 100644 --- a/src/app/shared/mocks/mock-store.ts +++ b/src/app/shared/mocks/mock-store.ts @@ -1,9 +1,15 @@ import { Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; -export function initMockStore(selectResult: Observable): Store { - return jasmine.createSpyObj('store', { - dispatch: null, - select: selectResult, - }); +export function getMockStore(): Store { + return jasmine.createSpyObj('store', [ + 'select', + 'dispatch', + 'lift', + 'next', + 'error', + 'complete', + 'addReducer', + 'removeReducer' + ]); } diff --git a/src/app/shared/mocks/mock-uuid.service.ts b/src/app/shared/mocks/mock-uuid.service.ts index b82e007ed2..1acf3e1f3e 100644 --- a/src/app/shared/mocks/mock-uuid.service.ts +++ b/src/app/shared/mocks/mock-uuid.service.ts @@ -2,7 +2,7 @@ import { UUIDService } from '../../core/shared/uuid.service'; export const defaultUUID = 'c4ce6905-290b-478f-979d-a333bbd7820f'; -export function initMockUUIDService(uuid = defaultUUID): UUIDService { +export function getMockUUIDService(uuid = defaultUUID): UUIDService { return jasmine.createSpyObj('uuidService', { generate: uuid, });