Merge pull request #490 from mspalti/routing_by_id_2

Lookup and redirect by uuid and handle
This commit is contained in:
Tim Donohue
2019-11-14 10:00:12 -06:00
committed by GitHub
19 changed files with 563 additions and 7 deletions

View File

@@ -207,6 +207,7 @@
"error.collection": "Error fetching collection",
"error.collections": "Error fetching collections",
"error.community": "Error fetching community",
"error.identifier": "No item found for the identifier",
"error.default": "Error",
"error.item": "Error fetching item",
"error.items": "Error fetching items",

View File

@@ -0,0 +1,41 @@
import { LookupGuard } from './lookup-guard';
import { NgModule } from '@angular/core';
import { RouterModule, UrlSegment } from '@angular/router';
import { ObjectNotFoundComponent } from './objectnotfound/objectnotfound.component';
import { hasValue, isNotEmpty } from '../shared/empty.util';
@NgModule({
imports: [
RouterModule.forChild([
{
matcher: (url) => {
// The expected path is :idType/:id
const idType = url[0].path;
// Allow for handles that are delimited with a forward slash.
const id = url
.slice(1)
.map((us: UrlSegment) => us.path)
.join('/');
if (isNotEmpty(idType) && isNotEmpty(id)) {
return {
consumed: url,
posParams: {
idType: new UrlSegment(idType, {}),
id: new UrlSegment(id, {})
}
};
}
return null;
},
canActivate: [LookupGuard],
component: ObjectNotFoundComponent }
])
],
providers: [
LookupGuard
]
})
export class LookupRoutingModule {
}

View File

@@ -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 { DsoRedirectDataService } from '../core/data/dso-redirect-data.service';
@NgModule({
imports: [
LookupRoutingModule,
CommonModule,
SharedModule,
],
declarations: [
ObjectNotFoundComponent
],
providers: [
DsoRedirectDataService
]
})
export class LookupIdModule {
}

View File

@@ -0,0 +1,50 @@
import { LookupGuard } from './lookup-guard';
import { of as observableOf } from 'rxjs';
import { IdentifierType } from '../core/data/request.models';
describe('LookupGuard', () => {
let dsoService: any;
let guard: any;
beforeEach(() => {
dsoService = {
findById: jasmine.createSpy('findById').and.returnValue(observableOf({ hasFailed: false,
hasSucceeded: true }))
};
guard = new LookupGuard(dsoService);
});
it('should call findById with handle params', () => {
const scopedRoute = {
params: {
id: '1234',
idType: '123456789'
}
};
guard.canActivate(scopedRoute as any, undefined);
expect(dsoService.findById).toHaveBeenCalledWith('123456789/1234', IdentifierType.HANDLE)
});
it('should call findById with handle params', () => {
const scopedRoute = {
params: {
id: '123456789%2F1234',
idType: 'handle'
}
};
guard.canActivate(scopedRoute as any, undefined);
expect(dsoService.findById).toHaveBeenCalledWith('123456789%2F1234', IdentifierType.HANDLE)
});
it('should call findById with UUID params', () => {
const scopedRoute = {
params: {
id: '34cfed7c-f597-49ef-9cbe-ea351f0023c2',
idType: 'uuid'
}
};
guard.canActivate(scopedRoute as any, undefined);
expect(dsoService.findById).toHaveBeenCalledWith('34cfed7c-f597-49ef-9cbe-ea351f0023c2', IdentifierType.UUID)
});
});

View File

@@ -0,0 +1,53 @@
import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from '@angular/router';
import { Injectable } from '@angular/core';
import { IdentifierType } from '../core/data/request.models';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { RemoteData } from '../core/data/remote-data';
import { FindByIDRequest } from '../core/data/request.models';
import { DsoRedirectDataService } from '../core/data/dso-redirect-data.service';
interface LookupParams {
type: IdentifierType;
id: string;
}
@Injectable()
export class LookupGuard implements CanActivate {
constructor(private dsoService: DsoRedirectDataService) {
}
canActivate(route: ActivatedRouteSnapshot, state:RouterStateSnapshot): Observable<boolean> {
const params = this.getLookupParams(route);
return this.dsoService.findById(params.id, params.type).pipe(
map((response: RemoteData<FindByIDRequest>) => 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}/${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
};
}
}

View File

@@ -0,0 +1,8 @@
<div class="object-not-found container">
<h1>{{"error.identifier" | translate}}</h1>
<h2><small><em>{{missingItem}}</em></small></h2>
<br />
<p class="text-center">
<a routerLink="/home" class="btn btn-primary">{{"404.link.home-page" | translate}}</a>
</p>
</div>

View File

@@ -0,0 +1,79 @@
import { NO_ERRORS_SCHEMA } from '@angular/core';
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { TranslateModule } from '@ngx-translate/core';
import { ObjectNotFoundComponent } from './objectnotfound.component';
import { ActivatedRouteStub } from '../../shared/testing/active-router-stub';
import { of as observableOf } from 'rxjs';
import { ActivatedRoute } from '@angular/router';
describe('ObjectNotFoundComponent', () => {
let comp: ObjectNotFoundComponent;
let fixture: ComponentFixture<ObjectNotFoundComponent>;
const testUUID = '34cfed7c-f597-49ef-9cbe-ea351f0023c2';
const uuidType = 'uuid';
const handlePrefix = '123456789';
const handleId = '22';
const activatedRouteStub = Object.assign(new ActivatedRouteStub(), {
params: observableOf({id: testUUID, idType: uuidType})
});
const activatedRouteStubHandle = Object.assign(new ActivatedRouteStub(), {
params: observableOf({id: handleId, idType: handlePrefix})
});
describe('uuid request', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [
TranslateModule.forRoot()
], providers: [
{provide: ActivatedRoute, useValue: activatedRouteStub}
],
declarations: [ObjectNotFoundComponent],
schemas: [NO_ERRORS_SCHEMA]
}).compileComponents();
}));
beforeEach(() => {
fixture = TestBed.createComponent(ObjectNotFoundComponent);
comp = fixture.componentInstance;
fixture.detectChanges();
});
it('should create instance', () => {
expect(comp).toBeDefined()
});
it('should have id and idType', () => {
expect(comp.id).toEqual(testUUID);
expect(comp.idType).toEqual(uuidType);
expect(comp.missingItem).toEqual('uuid: ' + testUUID);
});
});
describe( 'legacy handle request', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [
TranslateModule.forRoot()
], providers: [
{provide: ActivatedRoute, useValue: activatedRouteStubHandle}
],
declarations: [ObjectNotFoundComponent],
schemas: [NO_ERRORS_SCHEMA]
}).compileComponents();
}));
beforeEach(() => {
fixture = TestBed.createComponent(ObjectNotFoundComponent);
comp = fixture.componentInstance;
fixture.detectChanges();
});
it('should have handle prefix and id', () => {
expect(comp.id).toEqual(handleId);
expect(comp.idType).toEqual(handlePrefix);
expect(comp.missingItem).toEqual('handle: ' + handlePrefix + '/' + handleId);
});
});
});

View File

@@ -0,0 +1,43 @@
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.idType.startsWith('uuid')) {
this.missingItem = this.idType + ': ' + this.id;
} else {
this.missingItem = 'handle: ' + this.idType + '/' + this.id;
}
}
}

View File

@@ -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' },

View File

@@ -128,7 +128,7 @@ const EXPORTS = [
...PROVIDERS
],
declarations: [
...DECLARATIONS,
...DECLARATIONS
],
exports: [
...EXPORTS

View File

@@ -44,6 +44,7 @@ export abstract class TypedObject {
*/
export class CacheableObject extends TypedObject {
uuid?: string;
handle?: string;
self: string;
// isNew: boolean;
// dirtyType: DirtyType;

View File

@@ -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';
@@ -80,7 +80,8 @@ export class ObjectCacheService {
* @return Observable<NormalizedObject<T>>
* An observable of the requested object in normalized form
*/
getObjectByUUID<T extends CacheableObject>(uuid: string): Observable<NormalizedObject<T>> {
getObjectByUUID<T extends CacheableObject>(uuid: string):
Observable<NormalizedObject<T>> {
return this.store.pipe(
select(selfLinkFromUuidSelector(uuid)),
mergeMap((selfLink: string) => this.getObjectBySelfLink(selfLink)

View File

@@ -153,8 +153,9 @@ export abstract class DataService<T extends CacheableObject> {
}
findById(id: string): Observable<RemoteData<T>> {
const hrefObs = this.halService.getEndpoint(this.linkPath).pipe(
map((endpoint: string) => this.getIDHref(endpoint, id)));
map((endpoint: string) => this.getIDHref(endpoint, encodeURIComponent(id))));
hrefObs.pipe(
find((href: string) => hasValue(href)))

View File

@@ -0,0 +1,155 @@
import { cold, getTestScheduler } from 'jasmine-marbles';
import { TestScheduler } from 'rxjs/testing';
import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
import { HALEndpointService } from '../shared/hal-endpoint.service';
import { FindByIDRequest, IdentifierType } 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 { DsoRedirectDataService } from './dso-redirect-data.service';
import { Store } from '@ngrx/store';
import { CoreState } from '../core.reducers';
describe('DsoRedirectDataService', () => {
let scheduler: TestScheduler;
let service: DsoRedirectDataService;
let halService: HALEndpointService;
let requestService: RequestService;
let rdbService: RemoteDataBuildService;
let 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 store = {} as Store<CoreState>;
const notificationsService = {} as NotificationsService;
const http = {} as HttpClient;
const comparator = {} as any;
const dataBuildService = {} as NormalizedObjectBuildService;
const objectCache = {} as ObjectCacheService;
let setup;
beforeEach(() => {
scheduler = getTestScheduler();
halService = jasmine.createSpyObj('halService', {
getEndpoint: cold('a', {a: pidLink})
});
requestService = jasmine.createSpyObj('requestService', {
generateRequestId: requestUUID,
configure: true
});
router = {
navigate: jasmine.createSpy('navigate')
};
remoteData = {
isSuccessful: true,
error: undefined,
hasSucceeded: true,
isLoading: false,
payload: {
type: 'item',
uuid: '123456789'
}
};
setup = () => {
rdbService = jasmine.createSpyObj('rdbService', {
buildSingle: cold('a', {
a: remoteData
})
});
service = new DsoRedirectDataService(
requestService,
rdbService,
dataBuildService,
store,
objectCache,
halService,
notificationsService,
http,
comparator,
router
);
}
});
describe('findById', () => {
it('should call HALEndpointService with the path to the pid endpoint', () => {
setup();
scheduler.schedule(() => service.findById(dsoHandle, IdentifierType.HANDLE));
scheduler.flush();
expect(halService.getEndpoint).toHaveBeenCalledWith('pid');
});
it('should call HALEndpointService with the path to the dso endpoint', () => {
setup();
scheduler.schedule(() => service.findById(dsoUUID, IdentifierType.UUID));
scheduler.flush();
expect(halService.getEndpoint).toHaveBeenCalledWith('dso');
});
it('should call HALEndpointService with the path to the dso endpoint when identifier type not specified', () => {
setup();
scheduler.schedule(() => service.findById(dsoUUID));
scheduler.flush();
expect(halService.getEndpoint).toHaveBeenCalledWith('dso');
});
it('should configure the proper FindByIDRequest for uuid', () => {
setup();
scheduler.schedule(() => service.findById(dsoUUID, IdentifierType.UUID));
scheduler.flush();
expect(requestService.configure).toHaveBeenCalledWith(new FindByIDRequest(requestUUID, requestUUIDURL, dsoUUID), false);
});
it('should configure the proper FindByIDRequest for handle', () => {
setup();
scheduler.schedule(() => service.findById(dsoHandle, IdentifierType.HANDLE));
scheduler.flush();
expect(requestService.configure).toHaveBeenCalledWith(new FindByIDRequest(requestUUID, requestHandleURL, dsoHandle), false);
});
it('should navigate to item route', () => {
remoteData.payload.type = 'item';
setup();
const redir = service.findById(dsoHandle, IdentifierType.HANDLE);
// The framework would normally subscribe but do it here so we can test navigation.
redir.subscribe();
scheduler.schedule(() => redir);
scheduler.flush();
expect(router.navigate).toHaveBeenCalledWith([remoteData.payload.type + 's/' + remoteData.payload.uuid]);
});
it('should navigate to collections route', () => {
remoteData.payload.type = 'collection';
setup();
const redir = service.findById(dsoHandle, IdentifierType.HANDLE);
redir.subscribe();
scheduler.schedule(() => redir);
scheduler.flush();
expect(router.navigate).toHaveBeenCalledWith([remoteData.payload.type + 's/' + remoteData.payload.uuid]);
});
it('should navigate to communities route', () => {
remoteData.payload.type = 'community';
setup();
const redir = service.findById(dsoHandle, IdentifierType.HANDLE);
redir.subscribe();
scheduler.schedule(() => redir);
scheduler.flush();
expect(router.navigate).toHaveBeenCalledWith(['communities/' + remoteData.payload.uuid]);
});
})
});

View File

@@ -0,0 +1,90 @@
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, IdentifierType } from './request.models';
import { Observable } from 'rxjs';
import { RemoteData } from './remote-data';
import { DSOChangeAnalyzer } from './dso-change-analyzer.service';
import { Injectable } from '@angular/core';
import { filter, take, tap } from 'rxjs/operators';
import { hasValue } from '../../shared/empty.util';
import { getFinishedRemoteData } from '../shared/operators';
import { Router } from '@angular/router';
@Injectable()
export class DsoRedirectDataService extends DataService<any> {
// Set the default link path to the identifier lookup endpoint.
protected linkPath = 'pid';
protected forceBypassCache = false;
private uuidEndpoint = 'dso';
constructor(
protected requestService: RequestService,
protected rdbService: RemoteDataBuildService,
protected dataBuildService: NormalizedObjectBuildService,
protected store: Store<CoreState>,
protected objectCache: ObjectCacheService,
protected halService: HALEndpointService,
protected notificationsService: NotificationsService,
protected http: HttpClient,
protected comparator: DSOChangeAnalyzer<any>,
private router: Router) {
super();
}
getBrowseEndpoint(options: FindAllOptions = {}, linkPath: string = this.linkPath): Observable<string> {
return this.halService.getEndpoint(linkPath);
}
setLinkPath(identifierType: IdentifierType) {
// The default 'pid' endpoint for identifiers does not support uuid lookups.
// For uuid lookups we need to change the linkPath.
if (identifierType === IdentifierType.UUID) {
this.linkPath = this.uuidEndpoint;
}
}
getIDHref(endpoint, resourceID): string {
// Supporting both identifier (pid) and uuid (dso) endpoints
return endpoint.replace(/\{\?id\}/, `?id=${resourceID}`)
.replace(/\{\?uuid\}/, `?uuid=${resourceID}`);
}
findById(id: string, identifierType = IdentifierType.UUID): Observable<RemoteData<FindByIDRequest>> {
this.setLinkPath(identifierType);
return super.findById(id).pipe(
getFinishedRemoteData(),
take(1),
tap((response) => {
if (response.hasSucceeded) {
const uuid = response.payload.uuid;
const newRoute = this.getEndpointFromDSOType(response.payload.type);
if (hasValue(uuid) && hasValue(newRoute)) {
this.router.navigate([newRoute + '/' + uuid]);
}
}
})
);
}
// Is there an existing method somewhere else that converts dso type to route?
getEndpointFromDSOType(dsoType: string): string {
// Are there other types to consider?
if (dsoType.startsWith('item')) {
return 'items'
} else if (dsoType.startsWith('community')) {
return 'communities';
} else if (dsoType.startsWith('collection')) {
return 'collections'
} else {
return '';
}
}
}

View File

@@ -22,6 +22,12 @@ import { MappedCollectionsReponseParsingService } from './mapped-collections-rep
/* tslint:disable:max-classes-per-file */
// uuid and handle requests have separate endpoints
export enum IdentifierType {
UUID ='uuid',
HANDLE = 'handle'
}
export abstract class RestRequest {
public responseMsToLive = 10 * 1000;
public forceBypassCache = false;
@@ -50,7 +56,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)
}

View File

@@ -298,10 +298,11 @@ describe('RequestService', () => {
describe('in the ObjectCache', () => {
beforeEach(() => {
(objectCache.hasBySelfLink as any).and.returnValue(true);
(objectCache.hasByUUID as any).and.returnValue(true);
spyOn(serviceAsAny, 'hasByHref').and.returnValue(false);
});
it('should return true', () => {
it('should return true for GetRequest', () => {
const result = serviceAsAny.isCachedOrPending(testGetRequest);
const expected = true;

View File

@@ -53,8 +53,9 @@ export const requestUUIDIndexSelector: MemoizedSelector<AppState, IndexState> =
/**
* 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
*/