From efc91a4591344761620f34c7befa4c1191f331be Mon Sep 17 00:00:00 2001 From: Michael W Spalti Date: Wed, 25 Sep 2019 17:37:12 -0700 Subject: [PATCH] Updated work on routing by id. Fixed unit tests. Updated to use pid REST endpoint. Minor change in data.service and unit test update. Updated the objectnotfound page with new text and go home button. --- .../lookup-by-id-routing.module.ts | 19 +++ src/app/+lookup-by-id/lookup-by-id.module.ts | 23 ++++ src/app/+lookup-by-id/lookup-guard.ts | 52 ++++++++ .../objectnotfound.component.html | 8 ++ .../objectnotfound.component.scss | 0 .../objectnotfound.component.ts | 45 +++++++ src/app/app-routing.module.ts | 2 + src/app/app.module.ts | 2 +- src/app/core/cache/object-cache.reducer.ts | 1 + src/app/core/cache/object-cache.service.ts | 16 +-- src/app/core/core.effects.ts | 4 +- src/app/core/data/comcol-data.service.spec.ts | 4 +- src/app/core/data/comcol-data.service.ts | 2 +- src/app/core/data/data.service.ts | 18 ++- .../data/dso-data-redirect.service.spec.ts | 112 ++++++++++++++++++ .../core/data/dso-data-redirect.service.ts | 78 ++++++++++++ .../data/dspace-object-data.service.spec.ts | 3 +- src/app/core/data/request.models.ts | 6 +- src/app/core/data/request.service.ts | 18 ++- src/app/core/index/index.actions.ts | 14 +-- src/app/core/index/index.effects.ts | 38 ++++-- src/app/core/index/index.reducer.spec.ts | 40 +++++-- src/app/core/index/index.reducer.ts | 41 ++++--- src/app/core/index/index.selectors.ts | 34 ++++-- 24 files changed, 498 insertions(+), 82 deletions(-) create mode 100644 src/app/+lookup-by-id/lookup-by-id-routing.module.ts create mode 100644 src/app/+lookup-by-id/lookup-by-id.module.ts create mode 100644 src/app/+lookup-by-id/lookup-guard.ts create mode 100644 src/app/+lookup-by-id/objectnotfound/objectnotfound.component.html create mode 100644 src/app/+lookup-by-id/objectnotfound/objectnotfound.component.scss create mode 100644 src/app/+lookup-by-id/objectnotfound/objectnotfound.component.ts create mode 100644 src/app/core/data/dso-data-redirect.service.spec.ts create mode 100644 src/app/core/data/dso-data-redirect.service.ts diff --git a/src/app/+lookup-by-id/lookup-by-id-routing.module.ts b/src/app/+lookup-by-id/lookup-by-id-routing.module.ts new file mode 100644 index 0000000000..012345e791 --- /dev/null +++ b/src/app/+lookup-by-id/lookup-by-id-routing.module.ts @@ -0,0 +1,19 @@ +import { LookupGuard } from './lookup-guard'; +import { NgModule } from '@angular/core'; +import { RouterModule } from '@angular/router'; +import { ObjectNotFoundComponent } from './objectnotfound/objectnotfound.component'; + +@NgModule({ + imports: [ + RouterModule.forChild([ + { path: ':idType/:id', canActivate: [LookupGuard], component: ObjectNotFoundComponent } + ]) + ], + providers: [ + LookupGuard + ] +}) + +export class LookupRoutingModule { + +} diff --git a/src/app/+lookup-by-id/lookup-by-id.module.ts b/src/app/+lookup-by-id/lookup-by-id.module.ts new file mode 100644 index 0000000000..4620f57824 --- /dev/null +++ b/src/app/+lookup-by-id/lookup-by-id.module.ts @@ -0,0 +1,23 @@ +import { NgModule } from '@angular/core'; +import { CommonModule } from '@angular/common'; +import { SharedModule } from '../shared/shared.module'; +import { LookupRoutingModule } from './lookup-by-id-routing.module'; +import { ObjectNotFoundComponent } from './objectnotfound/objectnotfound.component'; +import { DsoDataRedirectService } from '../core/data/dso-data-redirect.service'; + +@NgModule({ + imports: [ + LookupRoutingModule, + CommonModule, + SharedModule, + ], + declarations: [ + ObjectNotFoundComponent + ], + providers: [ + DsoDataRedirectService + ] +}) +export class LookupIdModule { + +} diff --git a/src/app/+lookup-by-id/lookup-guard.ts b/src/app/+lookup-by-id/lookup-guard.ts new file mode 100644 index 0000000000..61e3688ee2 --- /dev/null +++ b/src/app/+lookup-by-id/lookup-guard.ts @@ -0,0 +1,52 @@ +import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from '@angular/router'; +import { DsoDataRedirectService } from '../core/data/dso-data-redirect.service'; +import { Injectable } from '@angular/core'; +import { IdentifierType } from '../core/index/index.reducer'; +import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; +import { RemoteData } from '../core/data/remote-data'; +import { FindByIDRequest } from '../core/data/request.models'; + +interface LookupParams { + type: IdentifierType; + id: string; +} + +@Injectable() +export class LookupGuard implements CanActivate { + constructor(private dsoService: DsoDataRedirectService, private router: Router) { + } + + canActivate(route: ActivatedRouteSnapshot, state:RouterStateSnapshot): Observable { + const params = this.getLookupParams(route); + return this.dsoService.findById(params.id, params.type).pipe( + map((response: RemoteData) => response.hasFailed) + ); + } + + private getLookupParams(route: ActivatedRouteSnapshot): LookupParams { + let type; + let id; + const idType = route.params.idType; + + // If the idType is not recognized, assume a legacy handle request (handle/prefix/id) + if (idType !== IdentifierType.HANDLE && idType !== IdentifierType.UUID) { + type = IdentifierType.HANDLE; + const prefix = route.params.idType; + const handleId = route.params.id; + id = `${prefix}%2F${handleId}`; + + } else if (route.params.idType === IdentifierType.HANDLE) { + type = IdentifierType.HANDLE; + id = route.params.id; + + } else { + type = IdentifierType.UUID; + id = route.params.id; + } + return { + type: type, + id: id + }; + } +} diff --git a/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.html b/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.html new file mode 100644 index 0000000000..662d3cde52 --- /dev/null +++ b/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.html @@ -0,0 +1,8 @@ +
+

{{"error.item" | translate}}

+

{{missingItem}}

+
+

+ {{"404.link.home-page" | translate}} +

+
diff --git a/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.scss b/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.ts b/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.ts new file mode 100644 index 0000000000..0116575154 --- /dev/null +++ b/src/app/+lookup-by-id/objectnotfound/objectnotfound.component.ts @@ -0,0 +1,45 @@ + +import { Component, ChangeDetectionStrategy, OnInit } from '@angular/core'; +import { ActivatedRoute } from '@angular/router'; + +/** + * This component representing the `PageNotFound` DSpace page. + */ +@Component({ + selector: 'ds-objnotfound', + styleUrls: ['./objectnotfound.component.scss'], + templateUrl: './objectnotfound.component.html', + changeDetection: ChangeDetectionStrategy.Default +}) +export class ObjectNotFoundComponent implements OnInit { + + idType: string; + + id: string; + + missingItem: string; + + /** + * Initialize instance variables + * + * @param {AuthService} authservice + * @param {ServerResponseService} responseService + */ + constructor(private route: ActivatedRoute) { + route.params.subscribe((params) => { + this.idType = params.idType; + this.id = params.id; + }) + } + + ngOnInit(): void { + if (this.idType.startsWith('handle')) { + this.missingItem = 'handle: ' + this.id; + } else if (this.idType.startsWith('uuid')) { + this.missingItem = 'uuid: ' + this.id; + } else { + this.missingItem = 'handle: ' + this.idType + '/' + this.id; + } + } + +} diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index e1ddc2b889..5085633a5b 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -27,6 +27,8 @@ export function getAdminModulePath() { RouterModule.forRoot([ { path: '', redirectTo: '/home', pathMatch: 'full' }, { path: 'home', loadChildren: './+home-page/home-page.module#HomePageModule' }, + { path: 'id', loadChildren: './+lookup-by-id/lookup-by-id.module#LookupIdModule' }, + { path: 'handle', loadChildren: './+lookup-by-id/lookup-by-id.module#LookupIdModule' }, { path: COMMUNITY_MODULE_PATH, loadChildren: './+community-page/community-page.module#CommunityPageModule' }, { path: COLLECTION_MODULE_PATH, loadChildren: './+collection-page/collection-page.module#CollectionPageModule' }, { path: ITEM_MODULE_PATH, loadChildren: './+item-page/item-page.module#ItemPageModule' }, diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 916788df8c..3d8bf0ed43 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -128,7 +128,7 @@ const EXPORTS = [ ...PROVIDERS ], declarations: [ - ...DECLARATIONS, + ...DECLARATIONS ], exports: [ ...EXPORTS diff --git a/src/app/core/cache/object-cache.reducer.ts b/src/app/core/cache/object-cache.reducer.ts index f41151fd90..afc040bf59 100644 --- a/src/app/core/cache/object-cache.reducer.ts +++ b/src/app/core/cache/object-cache.reducer.ts @@ -44,6 +44,7 @@ export abstract class TypedObject { */ export class CacheableObject extends TypedObject { uuid?: string; + handle?: string; self: string; // isNew: boolean; // dirtyType: DirtyType; diff --git a/src/app/core/cache/object-cache.service.ts b/src/app/core/cache/object-cache.service.ts index 11f3a6ce3e..95e96db0c8 100644 --- a/src/app/core/cache/object-cache.service.ts +++ b/src/app/core/cache/object-cache.service.ts @@ -4,7 +4,7 @@ import { applyPatch, Operation } from 'fast-json-patch'; import { combineLatest as observableCombineLatest, Observable } from 'rxjs'; import { distinctUntilChanged, filter, map, mergeMap, take, } from 'rxjs/operators'; -import { hasNoValue, hasValue, isNotEmpty } from '../../shared/empty.util'; +import { hasNoValue, isNotEmpty } from '../../shared/empty.util'; import { CoreState } from '../core.reducers'; import { coreSelector } from '../core.selectors'; import { RestRequestMethod } from '../data/rest-request-method'; @@ -21,6 +21,7 @@ import { import { CacheableObject, ObjectCacheEntry, ObjectCacheState } from './object-cache.reducer'; import { AddToSSBAction } from './server-sync-buffer.actions'; import { getMapsToType } from './builders/build-decorators'; +import { IdentifierType } from '../index/index.reducer'; /** * The base selector function to select the object cache in the store @@ -75,14 +76,15 @@ export class ObjectCacheService { /** * Get an observable of the object with the specified UUID * - * @param uuid + * @param id * The UUID of the object to get * @return Observable> * An observable of the requested object in normalized form */ - getObjectByUUID(uuid: string): Observable> { + getObjectByID(id: string, identifierType: IdentifierType = IdentifierType.UUID): + Observable> { return this.store.pipe( - select(selfLinkFromUuidSelector(uuid)), + select(selfLinkFromUuidSelector(id, identifierType)), mergeMap((selfLink: string) => this.getObjectBySelfLink(selfLink) ) ) @@ -188,17 +190,17 @@ export class ObjectCacheService { /** * Check whether the object with the specified UUID is cached * - * @param uuid + * @param id * The UUID of the object to check * @return boolean * true if the object with the specified UUID is cached, * false otherwise */ - hasByUUID(uuid: string): boolean { + hasById(id: string, identifierType: IdentifierType = IdentifierType.UUID): boolean { let result: boolean; this.store.pipe( - select(selfLinkFromUuidSelector(uuid)), + select(selfLinkFromUuidSelector(id, identifierType)), take(1) ).subscribe((selfLink: string) => result = this.hasBySelfLink(selfLink)); diff --git a/src/app/core/core.effects.ts b/src/app/core/core.effects.ts index 0eabfc5dc8..ae6f7f3cfa 100644 --- a/src/app/core/core.effects.ts +++ b/src/app/core/core.effects.ts @@ -1,5 +1,5 @@ import { ObjectCacheEffects } from './cache/object-cache.effects'; -import { UUIDIndexEffects } from './index/index.effects'; +import { IdentifierIndexEffects } from './index/index.effects'; import { RequestEffects } from './data/request.effects'; import { AuthEffects } from './auth/auth.effects'; import { JsonPatchOperationsEffects } from './json-patch/json-patch-operations.effects'; @@ -10,7 +10,7 @@ import { RouteEffects } from './services/route.effects'; export const coreEffects = [ RequestEffects, ObjectCacheEffects, - UUIDIndexEffects, + IdentifierIndexEffects, AuthEffects, JsonPatchOperationsEffects, ServerSyncBufferEffects, diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index b5232b0bff..de17d7a39f 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -95,7 +95,7 @@ describe('ComColDataService', () => { function initMockObjectCacheService(): ObjectCacheService { return jasmine.createSpyObj('objectCache', { - getObjectByUUID: cold('d-', { + getObjectByID: cold('d-', { d: { _links: { [LINK_NAME]: scopedEndpoint @@ -160,7 +160,7 @@ describe('ComColDataService', () => { it('should fetch the scope Community from the cache', () => { scheduler.schedule(() => service.getBrowseEndpoint(options).subscribe()); scheduler.flush(); - expect(objectCache.getObjectByUUID).toHaveBeenCalledWith(scopeID); + expect(objectCache.getObjectByID).toHaveBeenCalledWith(scopeID); }); it('should return the endpoint to fetch resources within the given scope', () => { diff --git a/src/app/core/data/comcol-data.service.ts b/src/app/core/data/comcol-data.service.ts index 68eb3e4880..3059d568df 100644 --- a/src/app/core/data/comcol-data.service.ts +++ b/src/app/core/data/comcol-data.service.ts @@ -49,7 +49,7 @@ export abstract class ComColDataService extends DataS ); const successResponses = responses.pipe( filter((response) => response.isSuccessful), - mergeMap(() => this.objectCache.getObjectByUUID(options.scopeID)), + mergeMap(() => this.objectCache.getObjectByID(options.scopeID)), map((nc: NormalizedCommunity) => nc._links[linkPath]), filter((href) => isNotEmpty(href)) ); diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index ad0db51980..0f7ca74d15 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -37,6 +37,7 @@ import { NormalizedObjectBuildService } from '../cache/builders/normalized-objec import { ChangeAnalyzer } from './change-analyzer'; import { RestRequestMethod } from './rest-request-method'; import { getMapsToType } from '../cache/builders/build-decorators'; +import { IdentifierType } from '../index/index.reducer'; export abstract class DataService { protected abstract requestService: RequestService; @@ -146,14 +147,21 @@ export abstract class DataService { return `${endpoint}/${resourceID}`; } - findById(id: string): Observable> { - const hrefObs = this.halService.getEndpoint(this.linkPath).pipe( - map((endpoint: string) => this.getIDHref(endpoint, id))); - + findById(id: string, identifierType: IdentifierType = IdentifierType.UUID): Observable> { + let hrefObs; + if (identifierType === IdentifierType.UUID) { + hrefObs = this.halService.getEndpoint(this.linkPath).pipe( + map((endpoint: string) => this.getIDHref(endpoint, id))); + } else if (identifierType === IdentifierType.HANDLE) { + hrefObs = this.halService.getEndpoint(this.linkPath).pipe( + map((endpoint: string) => { + return this.getIDHref(endpoint, encodeURIComponent(id)); + })); + } hrefObs.pipe( find((href: string) => hasValue(href))) .subscribe((href: string) => { - const request = new FindByIDRequest(this.requestService.generateRequestId(), href, id); + const request = new FindByIDRequest(this.requestService.generateRequestId(), href, id, identifierType); this.requestService.configure(request, this.forceBypassCache); }); diff --git a/src/app/core/data/dso-data-redirect.service.spec.ts b/src/app/core/data/dso-data-redirect.service.spec.ts new file mode 100644 index 0000000000..ece3c242fc --- /dev/null +++ b/src/app/core/data/dso-data-redirect.service.spec.ts @@ -0,0 +1,112 @@ +import { cold, getTestScheduler } from 'jasmine-marbles'; +import { TestScheduler } from 'rxjs/testing'; +import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { DSpaceObject } from '../shared/dspace-object.model'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { FindByIDRequest } from './request.models'; +import { RequestService } from './request.service'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; +import { NormalizedObjectBuildService } from '../cache/builders/normalized-object-build.service'; +import { IdentifierType } from '../index/index.reducer'; +import { DsoDataRedirectService } from './dso-data-redirect.service'; +import { Router } from '@angular/router'; +import { Store } from '@ngrx/store'; +import { CoreState } from '../core.reducers'; + +describe('DsoDataRedirectService', () => { + let scheduler: TestScheduler; + let service: DsoDataRedirectService; + let halService: HALEndpointService; + let requestService: RequestService; + let rdbService: RemoteDataBuildService; + let objectCache: ObjectCacheService; + let router: Router; + let remoteData; + const dsoUUID = '9b4f22f4-164a-49db-8817-3316b6ee5746'; + const dsoHandle = '1234567789/22'; + const encodedHandle = encodeURIComponent(dsoHandle); + const pidLink = 'https://rest.api/rest/api/pid/find{?id}'; + const requestHandleURL = `https://rest.api/rest/api/pid/find?id=${encodedHandle}`; + const requestUUIDURL = `https://rest.api/rest/api/pid/find?id=${dsoUUID}`; + const requestUUID = '34cfed7c-f597-49ef-9cbe-ea351f0023c2'; + const testObject = { + uuid: dsoUUID + } as DSpaceObject; + + beforeEach(() => { + scheduler = getTestScheduler(); + + halService = jasmine.createSpyObj('halService', { + getEndpoint: cold('a', { a: pidLink }) + }); + requestService = jasmine.createSpyObj('requestService', { + generateRequestId: requestUUID, + configure: true + }); + rdbService = jasmine.createSpyObj('rdbService', { + buildSingle: cold('a', { + a: { + payload: testObject + } + }) + }); + router = jasmine.createSpyObj('router', { + navigate: () => true + }); + remoteData = { + isSuccessful: true, + error: undefined, + hasSucceeded: true, + payload: { + type: 'item', + id: '123456789' + } + }; + objectCache = {} as ObjectCacheService; + const store = {} as Store; + const notificationsService = {} as NotificationsService; + const http = {} as HttpClient; + const comparator = {} as any; + const dataBuildService = {} as NormalizedObjectBuildService; + + service = new DsoDataRedirectService( + requestService, + rdbService, + dataBuildService, + store, + objectCache, + halService, + notificationsService, + http, + comparator, + router + ); + }); + + describe('findById', () => { + it('should call HALEndpointService with the path to the dso endpoint', () => { + scheduler.schedule(() => service.findById(dsoUUID)); + scheduler.flush(); + + expect(halService.getEndpoint).toHaveBeenCalledWith('pid'); + }); + + it('should configure the proper FindByIDRequest for uuid', () => { + scheduler.schedule(() => service.findById(dsoUUID, IdentifierType.UUID)); + scheduler.flush(); + + expect(requestService.configure).toHaveBeenCalledWith(new FindByIDRequest(requestUUID, requestUUIDURL, dsoUUID, IdentifierType.UUID), false); + }); + + it('should configure the proper FindByIDRequest for handle', () => { + scheduler.schedule(() => service.findById(dsoHandle, IdentifierType.HANDLE)); + scheduler.flush(); + + expect(requestService.configure).toHaveBeenCalledWith(new FindByIDRequest(requestUUID, requestHandleURL, dsoHandle, IdentifierType.HANDLE), false); + }); + + // TODO: test for router.navigate + }); +}); diff --git a/src/app/core/data/dso-data-redirect.service.ts b/src/app/core/data/dso-data-redirect.service.ts new file mode 100644 index 0000000000..b568a23c8a --- /dev/null +++ b/src/app/core/data/dso-data-redirect.service.ts @@ -0,0 +1,78 @@ +import { DataService } from './data.service'; +import { NormalizedObjectBuildService } from '../cache/builders/normalized-object-build.service'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { HttpClient } from '@angular/common/http'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { RequestService } from './request.service'; +import { Store } from '@ngrx/store'; +import { CoreState } from '../core.reducers'; +import { FindAllOptions, FindByIDRequest } from './request.models'; +import { Observable, of } from 'rxjs'; +import { IdentifierType } from '../index/index.reducer'; +import { RemoteData } from './remote-data'; +import { DSOChangeAnalyzer } from './dso-change-analyzer.service'; +import { Injectable } from '@angular/core'; +import { map, tap } from 'rxjs/operators'; +import { hasValue } from '../../shared/empty.util'; +import { getFinishedRemoteData, getSucceededRemoteData } from '../shared/operators'; +import { Router } from '@angular/router'; + +@Injectable() +export class DsoDataRedirectService extends DataService { + + protected linkPath = 'pid'; + protected forceBypassCache = false; + + constructor( + protected requestService: RequestService, + protected rdbService: RemoteDataBuildService, + protected dataBuildService: NormalizedObjectBuildService, + protected store: Store, + protected objectCache: ObjectCacheService, + protected halService: HALEndpointService, + protected notificationsService: NotificationsService, + protected http: HttpClient, + protected comparator: DSOChangeAnalyzer, + private router: Router) { + super(); + } + + getBrowseEndpoint(options: FindAllOptions = {}, linkPath: string = this.linkPath): Observable { + return this.halService.getEndpoint(linkPath); + } + + getIDHref(endpoint, resourceID): string { + return endpoint.replace(/\{\?id\}/,`?id=${resourceID}`); + } + + findById(id: string, identifierType = IdentifierType.UUID): Observable> { + return super.findById(id, identifierType).pipe( + getFinishedRemoteData(), + tap((response) => { + if (response.hasSucceeded) { + const uuid = response.payload.uuid; + // Is there an existing method somewhere that converts dso type to endpoint? + // This will not work for all endpoints! + const dsoType = this.getEndpointFromDSOType(response.payload.type); + if (hasValue(uuid) && hasValue(dsoType)) { + this.router.navigate([dsoType + '/' + uuid]); + } + } + }) + ); + } + + getEndpointFromDSOType(dsoType: string): string { + if (dsoType.startsWith('item')) { + return 'items' + } else if (dsoType.startsWith('community')) { + return 'communities'; + } else if (dsoType.startsWith('collection')) { + return 'collections' + } else { + return ''; + } + } +} diff --git a/src/app/core/data/dspace-object-data.service.spec.ts b/src/app/core/data/dspace-object-data.service.spec.ts index a0bba214ae..b411028420 100644 --- a/src/app/core/data/dspace-object-data.service.spec.ts +++ b/src/app/core/data/dspace-object-data.service.spec.ts @@ -10,6 +10,7 @@ import { ObjectCacheService } from '../cache/object-cache.service'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { HttpClient } from '@angular/common/http'; import { NormalizedObjectBuildService } from '../cache/builders/normalized-object-build.service'; +import { IdentifierType } from '../index/index.reducer'; describe('DSpaceObjectDataService', () => { let scheduler: TestScheduler; @@ -72,7 +73,7 @@ describe('DSpaceObjectDataService', () => { scheduler.schedule(() => service.findById(testObject.uuid)); scheduler.flush(); - expect(requestService.configure).toHaveBeenCalledWith(new FindByIDRequest(requestUUID, requestURL, testObject.uuid), false); + expect(requestService.configure).toHaveBeenCalledWith(new FindByIDRequest(requestUUID, requestURL, testObject.uuid, IdentifierType.UUID), false); }); it('should return a RemoteData for the object with the given ID', () => { diff --git a/src/app/core/data/request.models.ts b/src/app/core/data/request.models.ts index 43ab9e58e7..45e8fdea16 100644 --- a/src/app/core/data/request.models.ts +++ b/src/app/core/data/request.models.ts @@ -19,6 +19,7 @@ import { MetadatafieldParsingService } from './metadatafield-parsing.service'; import { URLCombiner } from '../url-combiner/url-combiner'; import { TaskResponseParsingService } from '../tasks/task-response-parsing.service'; import { MappedCollectionsReponseParsingService } from './mapped-collections-reponse-parsing.service'; +import { IdentifierType } from '../index/index.reducer'; /* tslint:disable:max-classes-per-file */ @@ -49,7 +50,7 @@ export class GetRequest extends RestRequest { public uuid: string, public href: string, public body?: any, - public options?: HttpOptions, + public options?: HttpOptions ) { super(uuid, href, RestRequestMethod.GET, body, options) } @@ -125,7 +126,8 @@ export class FindByIDRequest extends GetRequest { constructor( uuid: string, href: string, - public resourceID: string + public resourceID: string, + public identifierType?: IdentifierType ) { super(uuid, href); } diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 0980d48537..ac65042238 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -11,7 +11,7 @@ import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { CacheableObject } from '../cache/object-cache.reducer'; import { ObjectCacheService } from '../cache/object-cache.service'; import { CoreState } from '../core.reducers'; -import { IndexName, IndexState, MetaIndexState } from '../index/index.reducer'; +import { IdentifierType, IndexState, MetaIndexState, REQUEST, UUID_MAPPING } from '../index/index.reducer'; import { originalRequestUUIDFromRequestUUIDSelector, requestIndexSelector, @@ -19,7 +19,7 @@ import { } from '../index/index.selectors'; import { UUIDService } from '../shared/uuid.service'; import { RequestConfigureAction, RequestExecuteAction, RequestRemoveAction } from './request.actions'; -import { GetRequest, RestRequest } from './request.models'; +import { FindByIDRequest, GetRequest, RestRequest } from './request.models'; import { RequestEntry, RequestState } from './request.reducer'; import { CommitSSBAction } from '../cache/server-sync-buffer.actions'; import { RestRequestMethod } from './rest-request-method'; @@ -162,7 +162,7 @@ export class RequestService { filter((entry) => hasValue(entry)), take(1) ).subscribe((entry) => { - return this.store.dispatch(new AddToIndexAction(IndexName.UUID_MAPPING, request.uuid, entry.request.uuid)) + return this.store.dispatch(new AddToIndexAction(UUID_MAPPING, request.uuid, entry.request.uuid)) } ) } @@ -206,7 +206,7 @@ export class RequestService { } }); this.requestsOnTheirWayToTheStore = this.requestsOnTheirWayToTheStore.filter((reqHref: string) => reqHref.indexOf(href) < 0); - this.indexStore.dispatch(new RemoveFromIndexBySubstringAction(IndexName.REQUEST, href)); + this.indexStore.dispatch(new RemoveFromIndexBySubstringAction(REQUEST, href)); } /** @@ -225,8 +225,14 @@ export class RequestService { private isCachedOrPending(request: GetRequest): boolean { const inReqCache = this.hasByHref(request.href); const inObjCache = this.objectCache.hasBySelfLink(request.href); - const isCached = inReqCache || inObjCache; - + let inObjIdCache = false; + if (request instanceof FindByIDRequest) { + const req = request as FindByIDRequest; + if (hasValue(req.identifierType && hasValue(req.resourceID))) { + inObjIdCache = this.objectCache.hasById(req.resourceID, req.identifierType) + } + } + const isCached = inReqCache || inObjCache || inObjIdCache; const isPending = this.isPending(request); return isCached || isPending; } diff --git a/src/app/core/index/index.actions.ts b/src/app/core/index/index.actions.ts index 42804dbe26..24f031b33c 100644 --- a/src/app/core/index/index.actions.ts +++ b/src/app/core/index/index.actions.ts @@ -1,7 +1,7 @@ import { Action } from '@ngrx/store'; import { type } from '../../shared/ngrx/type'; -import { IndexName } from './index.reducer'; +import { } from './index.reducer'; /** * The list of HrefIndexAction type definitions @@ -19,7 +19,7 @@ export const IndexActionTypes = { export class AddToIndexAction implements Action { type = IndexActionTypes.ADD; payload: { - name: IndexName; + name: string; value: string; key: string; }; @@ -34,7 +34,7 @@ export class AddToIndexAction implements Action { * @param value * the self link of the resource the key belongs to */ - constructor(name: IndexName, key: string, value: string) { + constructor(name: string, key: string, value: string) { this.payload = { name, key, value }; } } @@ -45,7 +45,7 @@ export class AddToIndexAction implements Action { export class RemoveFromIndexByValueAction implements Action { type = IndexActionTypes.REMOVE_BY_VALUE; payload: { - name: IndexName, + name: string, value: string }; @@ -57,7 +57,7 @@ export class RemoveFromIndexByValueAction implements Action { * @param value * the value to remove the UUID for */ - constructor(name: IndexName, value: string) { + constructor(name: string, value: string) { this.payload = { name, value }; } @@ -69,7 +69,7 @@ export class RemoveFromIndexByValueAction implements Action { export class RemoveFromIndexBySubstringAction implements Action { type = IndexActionTypes.REMOVE_BY_SUBSTRING; payload: { - name: IndexName, + name: string, value: string }; @@ -81,7 +81,7 @@ export class RemoveFromIndexBySubstringAction implements Action { * @param value * the value to remove the UUID for */ - constructor(name: IndexName, value: string) { + constructor(name: string, value: string) { this.payload = { name, value }; } diff --git a/src/app/core/index/index.effects.ts b/src/app/core/index/index.effects.ts index 61cf313ab1..0cdf6bea6c 100644 --- a/src/app/core/index/index.effects.ts +++ b/src/app/core/index/index.effects.ts @@ -10,43 +10,67 @@ import { import { RequestActionTypes, RequestConfigureAction } from '../data/request.actions'; import { AddToIndexAction, RemoveFromIndexByValueAction } from './index.actions'; import { hasValue } from '../../shared/empty.util'; -import { IndexName } from './index.reducer'; +import { getIdentiferByIndexName, IdentifierType, REQUEST } from './index.reducer'; import { RestRequestMethod } from '../data/rest-request-method'; @Injectable() -export class UUIDIndexEffects { +export class IdentifierIndexEffects { - @Effect() addObject$ = this.actions$ + @Effect() addObjectByUUID$ = this.actions$ .pipe( ofType(ObjectCacheActionTypes.ADD), filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.uuid)), map((action: AddToObjectCacheAction) => { return new AddToIndexAction( - IndexName.OBJECT, + getIdentiferByIndexName(IdentifierType.UUID), action.payload.objectToCache.uuid, action.payload.objectToCache.self ); }) ); - @Effect() removeObject$ = this.actions$ + @Effect() addObjectByHandle$ = this.actions$ + .pipe( + ofType(ObjectCacheActionTypes.ADD), + filter((action: AddToObjectCacheAction) => hasValue(action.payload.objectToCache.handle)), + map((action: AddToObjectCacheAction) => { + return new AddToIndexAction( + getIdentiferByIndexName(IdentifierType.HANDLE), + action.payload.objectToCache.handle, + action.payload.objectToCache.self + ); + }) + ); + + @Effect() removeObjectByUUID$ = this.actions$ .pipe( ofType(ObjectCacheActionTypes.REMOVE), map((action: RemoveFromObjectCacheAction) => { return new RemoveFromIndexByValueAction( - IndexName.OBJECT, + getIdentiferByIndexName(IdentifierType.UUID), action.payload ); }) ); + @Effect() removeObjectByHandle$ = this.actions$ + .pipe( + ofType(ObjectCacheActionTypes.REMOVE), + map((action: RemoveFromObjectCacheAction) => { + return new RemoveFromIndexByValueAction( + getIdentiferByIndexName(IdentifierType.HANDLE), + action.payload + ); + }) + ); + @Effect() addRequest$ = this.actions$ .pipe( ofType(RequestActionTypes.CONFIGURE), filter((action: RequestConfigureAction) => action.payload.method === RestRequestMethod.GET), map((action: RequestConfigureAction) => { return new AddToIndexAction( - IndexName.REQUEST, + REQUEST, action.payload.href, action.payload.uuid ); diff --git a/src/app/core/index/index.reducer.spec.ts b/src/app/core/index/index.reducer.spec.ts index ef46c760c6..35460a9ef5 100644 --- a/src/app/core/index/index.reducer.spec.ts +++ b/src/app/core/index/index.reducer.spec.ts @@ -1,6 +1,6 @@ import * as deepFreeze from 'deep-freeze'; -import { IndexName, indexReducer, MetaIndexState } from './index.reducer'; +import { getIdentiferByIndexName, IdentifierType, indexReducer, MetaIndexState, REQUEST, } from './index.reducer'; import { AddToIndexAction, RemoveFromIndexBySubstringAction, RemoveFromIndexByValueAction } from './index.actions'; class NullAction extends AddToIndexAction { @@ -15,14 +15,19 @@ class NullAction extends AddToIndexAction { describe('requestReducer', () => { const key1 = '567a639f-f5ff-4126-807c-b7d0910808c8'; const key2 = '1911e8a4-6939-490c-b58b-a5d70f8d91fb'; + const key3 = '123456789/22'; 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 uuidIndex = getIdentiferByIndexName(IdentifierType.UUID); + const handleIndex = getIdentiferByIndexName(IdentifierType.HANDLE); const testState: MetaIndexState = { - [IndexName.OBJECT]: { + 'object/uuid-to-self-link/uuid': { [key1]: val1 - },[IndexName.REQUEST]: { + },'object/uuid-to-self-link/handle': { + [key3]: val1 + },'get-request/href-to-uuid': { [key1]: val1 - },[IndexName.UUID_MAPPING]: { + },'get-request/configured-to-cache-uuid': { [key1]: val1 } }; @@ -45,27 +50,38 @@ describe('requestReducer', () => { 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 action = new AddToIndexAction(REQUEST, key2, val2); const newState = indexReducer(state, action); - expect(newState[IndexName.REQUEST][key2]).toEqual(val2); + expect(newState[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); + let action = new RemoveFromIndexByValueAction(uuidIndex, val1); + let newState = indexReducer(state, action); + + expect(newState[uuidIndex][key1]).toBeUndefined(); + + action = new RemoveFromIndexByValueAction(handleIndex, val1); + newState = indexReducer(state, action); + + expect(newState[handleIndex][key3]).toBeUndefined(); - expect(newState[IndexName.OBJECT][key1]).toBeUndefined(); }); it('should remove the given \'value\' from its corresponding \'key\' in the correct substate, in response to a REMOVE_BY_SUBSTRING action', () => { const state = testState; - const action = new RemoveFromIndexBySubstringAction(IndexName.OBJECT, key1); - const newState = indexReducer(state, action); + let action = new RemoveFromIndexBySubstringAction(uuidIndex, key1); + let newState = indexReducer(state, action); - expect(newState[IndexName.OBJECT][key1]).toBeUndefined(); + expect(newState[uuidIndex][key1]).toBeUndefined(); + + action = new RemoveFromIndexBySubstringAction(handleIndex, key3); + newState = indexReducer(state, action); + + expect(newState[uuidIndex][key3]).toBeUndefined(); }); }); diff --git a/src/app/core/index/index.reducer.ts b/src/app/core/index/index.reducer.ts index b4cd8aa84b..631d579911 100644 --- a/src/app/core/index/index.reducer.ts +++ b/src/app/core/index/index.reducer.ts @@ -6,24 +6,26 @@ import { RemoveFromIndexByValueAction } from './index.actions'; -/** - * An enum containing all index names - */ -export enum IndexName { - // Contains all objects in the object cache indexed by UUID - OBJECT = 'object/uuid-to-self-link', - - // contains all requests in the request cache indexed by UUID - REQUEST = 'get-request/href-to-uuid', - - /** - * Contains the UUIDs of requests that were sent to the server and - * have their responses cached, indexed by the UUIDs of requests that - * weren't sent because the response they requested was already cached - */ - UUID_MAPPING = 'get-request/configured-to-cache-uuid' +export enum IdentifierType { + UUID ='uuid', + HANDLE = 'handle' } +/** + * Contains the UUIDs of requests that were sent to the server and + * have their responses cached, indexed by the UUIDs of requests that + * weren't sent because the response they requested was already cached + */ +export const UUID_MAPPING = 'get-request/configured-to-cache-uuid'; + +// contains all requests in the request cache indexed by UUID +export const REQUEST = 'get-request/href-to-uuid'; + +// returns the index for the provided id type (uuid, handle) +export const getIdentiferByIndexName = (idType: IdentifierType): string => { + return `object/uuid-to-self-link/${idType}`; +}; + /** * The state of a single index */ @@ -34,8 +36,11 @@ export interface IndexState { /** * The state that contains all indices */ -export type MetaIndexState = { - [name in IndexName]: IndexState +export interface MetaIndexState { + 'get-request/configured-to-cache-uuid': IndexState, + 'get-request/href-to-uuid': IndexState, + 'object/uuid-to-self-link/uuid': IndexState, + 'object/uuid-to-self-link/handle': IndexState } // Object.create(null) ensures the object has no default js properties (e.g. `__proto__`) diff --git a/src/app/core/index/index.selectors.ts b/src/app/core/index/index.selectors.ts index 3c7b331a92..80a7c0d46a 100644 --- a/src/app/core/index/index.selectors.ts +++ b/src/app/core/index/index.selectors.ts @@ -3,7 +3,14 @@ import { AppState } from '../../app.reducer'; import { hasValue } from '../../shared/empty.util'; import { CoreState } from '../core.reducers'; import { coreSelector } from '../core.selectors'; -import { IndexName, IndexState, MetaIndexState } from './index.reducer'; +import { + getIdentiferByIndexName, + IdentifierType, + IndexState, + MetaIndexState, + REQUEST, + UUID_MAPPING +} from './index.reducer'; /** * Return the MetaIndexState based on the CoreSate @@ -20,13 +27,17 @@ export const metaIndexSelector: MemoizedSelector = cre * Return the object index based on the MetaIndexState * It contains all objects in the object cache indexed by UUID * + * @param identifierType the type of index, used to select index from state + * * @returns * a MemoizedSelector to select the object index */ -export const objectIndexSelector: MemoizedSelector = createSelector( - metaIndexSelector, - (state: MetaIndexState) => state[IndexName.OBJECT] -); +export const objectIndexSelector = (identifierType: IdentifierType = IdentifierType.UUID): MemoizedSelector => { + return createSelector( + metaIndexSelector, + (state: MetaIndexState) => state[getIdentiferByIndexName(identifierType)] + ); +} /** * Return the request index based on the MetaIndexState @@ -36,7 +47,7 @@ export const objectIndexSelector: MemoizedSelector = creat */ export const requestIndexSelector: MemoizedSelector = createSelector( metaIndexSelector, - (state: MetaIndexState) => state[IndexName.REQUEST] + (state: MetaIndexState) => state[REQUEST] ); /** @@ -47,21 +58,22 @@ export const requestIndexSelector: MemoizedSelector = crea */ export const requestUUIDIndexSelector: MemoizedSelector = createSelector( metaIndexSelector, - (state: MetaIndexState) => state[IndexName.UUID_MAPPING] + (state: MetaIndexState) => state[UUID_MAPPING] ); /** * Return the self link of an object in the object-cache based on its UUID * - * @param uuid + * @param id * the UUID for which you want to find the matching self link + * @param identifierType the type of index, used to select index from state * @returns * a MemoizedSelector to select the self link */ export const selfLinkFromUuidSelector = - (uuid: string): MemoizedSelector => createSelector( - objectIndexSelector, - (state: IndexState) => hasValue(state) ? state[uuid] : undefined + (id: string, identifierType: IdentifierType = IdentifierType.UUID): MemoizedSelector => createSelector( + objectIndexSelector(identifierType), + (state: IndexState) => hasValue(state) ? state[id] : undefined ); /**