diff --git a/.gitignore b/.gitignore index 01fc9231bc..cbf74702cb 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,5 @@ yarn-error.log *.css package-lock.json + +.java-version diff --git a/src/app/core/breadcrumbs/collection-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/collection-breadcrumb.resolver.ts index d9df7cd767..7384a031db 100644 --- a/src/app/core/breadcrumbs/collection-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/collection-breadcrumb.resolver.ts @@ -21,7 +21,7 @@ export class CollectionBreadcrumbResolver extends DSOBreadcrumbResolver> { return [ - followLink('parentCommunity', undefined, + followLink('parentCommunity', undefined, true, followLink('parentCommunity') ) ]; diff --git a/src/app/core/breadcrumbs/item-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/item-breadcrumb.resolver.ts index 8390c0e001..cd0c23cf82 100644 --- a/src/app/core/breadcrumbs/item-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/item-breadcrumb.resolver.ts @@ -21,8 +21,8 @@ export class ItemBreadcrumbResolver extends DSOBreadcrumbResolver { */ get followLinks(): Array> { return [ - followLink('owningCollection', undefined, - followLink('parentCommunity', undefined, + followLink('owningCollection', undefined, true, + followLink('parentCommunity', undefined, true, followLink('parentCommunity')) ), followLink('bundles'), diff --git a/src/app/core/cache/builders/link.service.spec.ts b/src/app/core/cache/builders/link.service.spec.ts index dbd65eefb5..e9b8447c22 100644 --- a/src/app/core/cache/builders/link.service.spec.ts +++ b/src/app/core/cache/builders/link.service.spec.ts @@ -90,7 +90,7 @@ describe('LinkService', () => { propertyName: 'predecessor' }); spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); - service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))) + service.resolveLink(testModel, followLink('predecessor', {}, true, followLink('successor'))) }); it('should call dataservice.findByHref with the correct href and nested links', () => { expect(testDataService.findByHref).toHaveBeenCalledWith(testModel._links.predecessor.href, followLink('successor')); @@ -105,7 +105,7 @@ describe('LinkService', () => { isList: true }); spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); - service.resolveLink(testModel, followLink('predecessor', { some: 'options '} as any, followLink('successor'))) + service.resolveLink(testModel, followLink('predecessor', { some: 'options '} as any, true, followLink('successor'))) }); it('should call dataservice.findAllByHref with the correct href, findListOptions, and nested links', () => { expect(testDataService.findAllByHref).toHaveBeenCalledWith(testModel._links.predecessor.href, { some: 'options '} as any, followLink('successor')); @@ -119,7 +119,7 @@ describe('LinkService', () => { propertyName: 'predecessor' }); spyOnFunction(decorators, 'getDataServiceFor').and.returnValue(TestDataService); - result = service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))) + result = service.resolveLink(testModel, followLink('predecessor', {}, true, followLink('successor'))) }); it('should call getLinkDefinition with the correct model and link', () => { @@ -144,7 +144,7 @@ describe('LinkService', () => { }); it('should throw an error', () => { expect(() => { - service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))) + service.resolveLink(testModel, followLink('predecessor', {}, true, followLink('successor'))) }).toThrow(); }); }); @@ -160,7 +160,7 @@ describe('LinkService', () => { }); it('should throw an error', () => { expect(() => { - service.resolveLink(testModel, followLink('predecessor', {}, followLink('successor'))) + service.resolveLink(testModel, followLink('predecessor', {}, true, followLink('successor'))) }).toThrow(); }); }); diff --git a/src/app/core/data/base-response-parsing.service.spec.ts b/src/app/core/data/base-response-parsing.service.spec.ts new file mode 100644 index 0000000000..a1d602dc65 --- /dev/null +++ b/src/app/core/data/base-response-parsing.service.spec.ts @@ -0,0 +1,104 @@ +import { BaseResponseParsingService } from './base-response-parsing.service'; +import { GlobalConfig } from '../../../config/global-config.interface'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { CacheableObject } from '../cache/object-cache.reducer'; +import { GetRequest, RestRequest } from './request.models'; +import { DSpaceObject } from '../shared/dspace-object.model'; + +/* tslint:disable:max-classes-per-file */ +class TestService extends BaseResponseParsingService { + toCache = true; + + constructor(protected EnvConfig: GlobalConfig, + protected objectCache: ObjectCacheService) { + super(); + } + + // Overwrite methods to make them public for testing + public process(data: any, request: RestRequest): any { + super.process(data, request); + } + + public cache(obj, request: RestRequest, data: any) { + super.cache(obj, request, data); + } +} + +describe('BaseResponseParsingService', () => { + let service: TestService; + let config: GlobalConfig; + let objectCache: ObjectCacheService; + + const requestUUID = 'request-uuid'; + const requestHref = 'request-href'; + const request = new GetRequest(requestUUID, requestHref); + + beforeEach(() => { + config = Object.assign({}); + objectCache = jasmine.createSpyObj('objectCache', { + add: {} + }); + service = new TestService(config, objectCache); + }); + + describe('cache', () => { + let obj: CacheableObject; + + describe('when the object is undefined', () => { + it('should not throw an error', () => { + expect(() => { service.cache(obj, request, {}) }).not.toThrow(); + }); + + it('should not call objectCache add', () => { + service.cache(obj, request, {}); + expect(objectCache.add).not.toHaveBeenCalled(); + }); + }); + + describe('when the object has a self link', () => { + beforeEach(() => { + obj = Object.assign(new DSpaceObject(), { + _links: { + self: { href: 'obj-selflink' } + } + }); + }); + + it('should call objectCache add', () => { + service.cache(obj, request, {}); + expect(objectCache.add).toHaveBeenCalledWith(obj, request.responseMsToLive, request.uuid); + }); + }); + }); + + describe('process', () => { + let data: any; + let result: any; + + describe('when data is valid, but not a real type', () => { + beforeEach(() => { + data = { + type: 'NotARealType', + _links: { + self: { href: 'data-selflink' } + } + }; + }); + + it('should not throw an error', () => { + expect(() => { result = service.process(data, request) }).not.toThrow(); + }); + + it('should return undefined', () => { + result = service.process(data, request); + expect(result).toBeUndefined(); + }); + + it('should not call objectCache add', () => { + result = service.process(data, request); + expect(objectCache.add).not.toHaveBeenCalled(); + }); + }); + }); +}); +/* tslint:enable:max-classes-per-file */ diff --git a/src/app/core/data/base-response-parsing.service.ts b/src/app/core/data/base-response-parsing.service.ts index 3615ab4023..3d00c5d0bd 100644 --- a/src/app/core/data/base-response-parsing.service.ts +++ b/src/app/core/data/base-response-parsing.service.ts @@ -117,10 +117,12 @@ export abstract class BaseResponseParsingService { const serializer = new this.serializerConstructor(objConstructor); return serializer.deserialize(obj); } else { + console.warn('cannot deserialize type ' + type); return null; } } else { + console.warn('cannot deserialize type ' + type); return null; } } @@ -142,7 +144,8 @@ export abstract class BaseResponseParsingService { } else { dataJSON = JSON.stringify(data); } - throw new Error(`Can't cache incomplete ${type}: ${JSON.stringify(co)}, parsed from (partial) response: ${dataJSON}`); + console.warn(`Can't cache incomplete ${type}: ${JSON.stringify(co)}, parsed from (partial) response: ${dataJSON}`); + return; } this.objectCache.add(co, hasValue(request.responseMsToLive) ? request.responseMsToLive : this.EnvConfig.cache.msToLive.default, request.uuid); } diff --git a/src/app/core/data/data.service.spec.ts b/src/app/core/data/data.service.spec.ts index 347dfa83a4..c5748b05fb 100644 --- a/src/app/core/data/data.service.spec.ts +++ b/src/app/core/data/data.service.spec.ts @@ -1,21 +1,23 @@ -import { DataService } from './data.service'; -import { RequestService } from './request.service'; -import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; -import { CoreState } from '../core.reducers'; +import { HttpClient } from '@angular/common/http'; import { Store } from '@ngrx/store'; -import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { compare, Operation } from 'fast-json-patch'; import { Observable, of as observableOf } from 'rxjs'; -import { FindListOptions } from './request.models'; +import * as uuidv4 from 'uuid/v4'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { createSuccessfulRemoteDataObject$ } from '../../shared/testing/utils'; +import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; +import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { SortDirection, SortOptions } from '../cache/models/sort-options.model'; import { ObjectCacheService } from '../cache/object-cache.service'; -import { compare, Operation } from 'fast-json-patch'; +import { CoreState } from '../core.reducers'; +import { Collection } from '../shared/collection.model'; import { DSpaceObject } from '../shared/dspace-object.model'; -import { ChangeAnalyzer } from './change-analyzer'; -import { HttpClient } from '@angular/common/http'; -import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Item } from '../shared/item.model'; -import * as uuidv4 from 'uuid/v4'; -import { createSuccessfulRemoteDataObject$ } from '../../shared/testing/utils'; +import { ChangeAnalyzer } from './change-analyzer'; +import { DataService } from './data.service'; +import { FindListOptions } from './request.models'; +import { RequestService } from './request.service'; const endpoint = 'https://rest.api/core'; @@ -40,6 +42,7 @@ class TestService extends DataService { return observableOf(endpoint); } } + class DummyChangeAnalyzer implements ChangeAnalyzer { diff(object1: Item, object2: Item): Operation[] { return compare((object1 as any).metadata, (object2 as any).metadata); @@ -50,7 +53,7 @@ class DummyChangeAnalyzer implements ChangeAnalyzer { describe('DataService', () => { let service: TestService; let options: FindListOptions; - const requestService = {generateRequestId: () => uuidv4()} as RequestService; + const requestService = { generateRequestId: () => uuidv4() } as RequestService; const halService = {} as HALEndpointService; const rdbService = {} as RemoteDataBuildService; const notificationsService = {} as NotificationsService; @@ -144,8 +147,143 @@ describe('DataService', () => { (service as any).getFindAllHref(options).subscribe((value) => { expect(value).toBe(expected); }); - }) + }); + + it('should include single linksToFollow as embed', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + }); + const expected = `${endpoint}?embed=bundles`; + + (service as any).getFindAllHref({}, null, mockFollowLinkConfig).subscribe((value) => { + expect(value).toBe(expected); + }); + }); + + it('should include multiple linksToFollow as embed', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + }); + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'templateItemOf' as any, + }); + const expected = `${endpoint}?embed=bundles&embed=owningCollection&embed=templateItemOf`; + + (service as any).getFindAllHref({}, null, mockFollowLinkConfig, mockFollowLinkConfig2, mockFollowLinkConfig3).subscribe((value) => { + expect(value).toBe(expected); + }); + }); + + it('should not include linksToFollow with shouldEmbed = false', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + shouldEmbed: false, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + shouldEmbed: false, + }); + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'templateItemOf' as any, + }); + const expected = `${endpoint}?embed=templateItemOf`; + + (service as any).getFindAllHref({}, null, mockFollowLinkConfig, mockFollowLinkConfig2, mockFollowLinkConfig3).subscribe((value) => { + expect(value).toBe(expected); + }); + }); + + it('should include nested linksToFollow 3lvl', () => { + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'relationships' as any, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'itemtemplate' as any, + linksToFollow: mockFollowLinkConfig3, + }); + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + linksToFollow: mockFollowLinkConfig2, + }); + const expected = `${endpoint}?embed=owningCollection/itemtemplate/relationships`; + + (service as any).getFindAllHref({}, null, mockFollowLinkConfig).subscribe((value) => { + expect(value).toBe(expected); + }); + }); }); + + describe('getIDHref', () => { + const endpointMock = 'https://dspace7-internal.atmire.com/server/api/core/items'; + const resourceIdMock = '003c99b4-d4fe-44b0-a945-e12182a7ca89'; + + it('should return endpoint', () => { + const result = (service as any).getIDHref(endpointMock, resourceIdMock); + expect(result).toEqual(endpointMock + '/' + resourceIdMock); + }); + + it('should include single linksToFollow as embed', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + }); + const expected = `${endpointMock}/${resourceIdMock}?embed=bundles`; + const result = (service as any).getIDHref(endpointMock, resourceIdMock, mockFollowLinkConfig); + expect(result).toEqual(expected); + }); + + it('should include multiple linksToFollow as embed', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + }); + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'templateItemOf' as any, + }); + const expected = `${endpointMock}/${resourceIdMock}?embed=bundles&embed=owningCollection&embed=templateItemOf`; + const result = (service as any).getIDHref(endpointMock, resourceIdMock, mockFollowLinkConfig, mockFollowLinkConfig2, mockFollowLinkConfig3); + expect(result).toEqual(expected); + }); + + it('should not include linksToFollow with shouldEmbed = false', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + shouldEmbed: false, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + shouldEmbed: false, + }); + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'templateItemOf' as any, + }); + const expected = `${endpointMock}/${resourceIdMock}?embed=templateItemOf`; + const result = (service as any).getIDHref(endpointMock, resourceIdMock, mockFollowLinkConfig, mockFollowLinkConfig2, mockFollowLinkConfig3); + expect(result).toEqual(expected); + }); + + it('should include nested linksToFollow 3lvl', () => { + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'relationships' as any, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'itemtemplate' as any, + linksToFollow: mockFollowLinkConfig3, + }); + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + linksToFollow: mockFollowLinkConfig2, + }); + const expected = `${endpointMock}/${resourceIdMock}?embed=owningCollection/itemtemplate/relationships`; + const result = (service as any).getIDHref(endpointMock, resourceIdMock, mockFollowLinkConfig); + expect(result).toEqual(expected); + }); + }); + describe('patch', () => { let operations; let selfLink; diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index 7dcfb6bd6e..3e67675290 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -83,14 +83,15 @@ export abstract class DataService { * @param linkPath The link path for the object * @return {Observable} * Return an observable that emits created HREF + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - protected getFindAllHref(options: FindListOptions = {}, linkPath?: string): Observable { + protected getFindAllHref(options: FindListOptions = {}, linkPath?: string, ...linksToFollow: Array>): Observable { let result$: Observable; const args = []; result$ = this.getBrowseEndpoint(options, linkPath).pipe(distinctUntilChanged()); - return result$.pipe(map((result: string) => this.buildHrefFromFindOptions(result, options, args))); + return result$.pipe(map((result: string) => this.buildHrefFromFindOptions(result, options, args, ...linksToFollow))); } /** @@ -100,8 +101,9 @@ export abstract class DataService { * @param options The [[FindListOptions]] object * @return {Observable} * Return an observable that emits created HREF + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - protected getSearchByHref(searchMethod: string, options: FindListOptions = {}): Observable { + protected getSearchByHref(searchMethod: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable { let result$: Observable; const args = []; @@ -113,7 +115,7 @@ export abstract class DataService { }) } - return result$.pipe(map((result: string) => this.buildHrefFromFindOptions(result, options, args))); + return result$.pipe(map((result: string) => this.buildHrefFromFindOptions(result, options, args, ...linksToFollow))); } /** @@ -124,9 +126,9 @@ export abstract class DataService { * @param extraArgs Array with additional params to combine with query string * @return {Observable} * Return an observable that emits created HREF + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - protected buildHrefFromFindOptions(href: string, options: FindListOptions, extraArgs: string[] = []): string { - + protected buildHrefFromFindOptions(href: string, options: FindListOptions, extraArgs: string[] = [], ...linksToFollow: Array>): string { let args = [...extraArgs]; if (hasValue(options.currentPage) && typeof options.currentPage === 'number') { @@ -142,6 +144,7 @@ export abstract class DataService { if (hasValue(options.startsWith)) { args = [...args, `startsWith=${options.startsWith}`]; } + args = this.addEmbedParams(args, ...linksToFollow); if (isNotEmpty(args)) { return new URLCombiner(href, `?${args.join('&')}`).toString(); } else { @@ -149,6 +152,40 @@ export abstract class DataService { } } + /** + * Adds the embed options to the link for the request + * @param args params for the query string + * @param linksToFollow links we want to embed in query string if shouldEmbed is true + */ + protected addEmbedParams(args: string[], ...linksToFollow: Array>) { + linksToFollow.forEach((linkToFollow: FollowLinkConfig) => { + if (linkToFollow !== undefined && linkToFollow.shouldEmbed) { + const embedString = 'embed=' + String(linkToFollow.name); + const embedWithNestedString = this.addNestedEmbeds(embedString, ...linkToFollow.linksToFollow); + args = [...args, embedWithNestedString]; + } + }); + return args; + } + + /** + * Add the nested followLinks to the embed param, recursively, separated by a / + * @param embedString embedString so far (recursive) + * @param linksToFollow links we want to embed in query string if shouldEmbed is true + */ + protected addNestedEmbeds(embedString: string, ...linksToFollow: Array>): string { + let nestEmbed = embedString; + linksToFollow.forEach((linkToFollow: FollowLinkConfig) => { + if (linkToFollow !== undefined && linkToFollow.shouldEmbed) { + nestEmbed = nestEmbed + '/' + String(linkToFollow.name); + if (linkToFollow.linksToFollow !== undefined) { + nestEmbed = this.addNestedEmbeds(nestEmbed, ...linkToFollow.linksToFollow); + } + } + }); + return nestEmbed; + } + /** * Returns {@link RemoteData} of all object with a list of {@link FollowLinkConfig}, to indicate which embedded * info should be added to the objects @@ -184,12 +221,13 @@ export abstract class DataService { } /** - * Create the HREF for a specific object based on its identifier + * Create the HREF for a specific object based on its identifier; with possible embed query params based on linksToFollow * @param endpoint The base endpoint for the type of object * @param resourceID The identifier for the object + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - getIDHref(endpoint, resourceID): string { - return `${endpoint}/${resourceID}`; + getIDHref(endpoint, resourceID, ...linksToFollow: Array>): string { + return this.buildHrefFromFindOptions(endpoint + '/' + resourceID, {}, [], ...linksToFollow); } /** @@ -199,9 +237,8 @@ export abstract class DataService { * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ findById(id: string, ...linksToFollow: Array>): Observable> { - const hrefObs = this.halService.getEndpoint(this.linkPath).pipe( - map((endpoint: string) => this.getIDHref(endpoint, encodeURIComponent(id)))); + map((endpoint: string) => this.getIDHref(endpoint, encodeURIComponent(id), ...linksToFollow))); hrefObs.pipe( find((href: string) => hasValue(href))) @@ -223,7 +260,7 @@ export abstract class DataService { * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ findByHref(href: string, ...linksToFollow: Array>): Observable> { - const requestHref = this.buildHrefFromFindOptions(href, {}, []); + const requestHref = this.buildHrefFromFindOptions(href, {}, [], ...linksToFollow); const request = new GetRequest(this.requestService.generateRequestId(), requestHref); if (hasValue(this.responseMsToLive)) { request.responseMsToLive = this.responseMsToLive; @@ -240,7 +277,7 @@ export abstract class DataService { * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ findAllByHref(href: string, findListOptions: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { - const requestHref = this.buildHrefFromFindOptions(href, findListOptions, []); + const requestHref = this.buildHrefFromFindOptions(href, findListOptions, [], ...linksToFollow); const request = new GetRequest(this.requestService.generateRequestId(), requestHref); if (hasValue(this.responseMsToLive)) { request.responseMsToLive = this.responseMsToLive; @@ -271,7 +308,7 @@ export abstract class DataService { */ protected searchBy(searchMethod: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { - const hrefObs = this.getSearchByHref(searchMethod, options); + const hrefObs = this.getSearchByHref(searchMethod, options, ...linksToFollow); return hrefObs.pipe( find((href: string) => hasValue(href)), diff --git a/src/app/core/data/dso-redirect-data.service.spec.ts b/src/app/core/data/dso-redirect-data.service.spec.ts index 25a148d92b..4ef5bcb8b4 100644 --- a/src/app/core/data/dso-redirect-data.service.spec.ts +++ b/src/app/core/data/dso-redirect-data.service.spec.ts @@ -1,15 +1,18 @@ +import { HttpClient } from '@angular/common/http'; +import { Store } from '@ngrx/store'; import { cold, getTestScheduler } from 'jasmine-marbles'; import { TestScheduler } from 'rxjs/testing'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import { ObjectCacheService } from '../cache/object-cache.service'; +import { CoreState } from '../core.reducers'; +import { Collection } from '../shared/collection.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { Item } from '../shared/item.model'; +import { DsoRedirectDataService } from './dso-redirect-data.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 { DsoRedirectDataService } from './dso-redirect-data.service'; -import { Store } from '@ngrx/store'; -import { CoreState } from '../core.reducers'; describe('DsoRedirectDataService', () => { let scheduler: TestScheduler; @@ -148,5 +151,71 @@ describe('DsoRedirectDataService', () => { scheduler.flush(); expect(router.navigate).toHaveBeenCalledWith(['communities/' + remoteData.payload.uuid]); }); - }) + }); + + describe('getIDHref', () => { + it('should return endpoint', () => { + const result = (service as any).getIDHref(pidLink, dsoUUID); + expect(result).toEqual(requestUUIDURL); + }); + + it('should include single linksToFollow as embed', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + }); + const expected = `${requestUUIDURL}&embed=bundles`; + const result = (service as any).getIDHref(pidLink, dsoUUID, mockFollowLinkConfig); + expect(result).toEqual(expected); + }); + + it('should include multiple linksToFollow as embed', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + }); + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'templateItemOf' as any, + }); + const expected = `${requestUUIDURL}&embed=bundles&embed=owningCollection&embed=templateItemOf`; + const result = (service as any).getIDHref(pidLink, dsoUUID, mockFollowLinkConfig, mockFollowLinkConfig2, mockFollowLinkConfig3); + expect(result).toEqual(expected); + }); + + it('should not include linksToFollow with shouldEmbed = false', () => { + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'bundles' as any, + shouldEmbed: false, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + shouldEmbed: false, + }); + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'templateItemOf' as any, + }); + const expected = `${requestUUIDURL}&embed=templateItemOf`; + const result = (service as any).getIDHref(pidLink, dsoUUID, mockFollowLinkConfig, mockFollowLinkConfig2, mockFollowLinkConfig3); + expect(result).toEqual(expected); + }); + + it('should include nested linksToFollow 3lvl', () => { + const mockFollowLinkConfig3: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'relationships' as any, + }); + const mockFollowLinkConfig2: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'itemtemplate' as any, + linksToFollow: mockFollowLinkConfig3, + }); + const mockFollowLinkConfig: FollowLinkConfig = Object.assign(new FollowLinkConfig(), { + name: 'owningCollection' as any, + linksToFollow: mockFollowLinkConfig2, + }); + const expected = `${requestUUIDURL}&embed=owningCollection/itemtemplate/relationships`; + const result = (service as any).getIDHref(pidLink, dsoUUID, mockFollowLinkConfig); + expect(result).toEqual(expected); + }); + }); + }); diff --git a/src/app/core/data/dso-redirect-data.service.ts b/src/app/core/data/dso-redirect-data.service.ts index 232fde65d0..87259a4279 100644 --- a/src/app/core/data/dso-redirect-data.service.ts +++ b/src/app/core/data/dso-redirect-data.service.ts @@ -6,6 +6,7 @@ import { Observable } from 'rxjs'; import { take, tap } from 'rxjs/operators'; import { hasValue } from '../../shared/empty.util'; import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../cache/object-cache.service'; import { CoreState } from '../core.reducers'; @@ -45,10 +46,11 @@ export class DsoRedirectDataService extends DataService { } } - getIDHref(endpoint, resourceID): string { + getIDHref(endpoint, resourceID, ...linksToFollow: Array>): string { // Supporting both identifier (pid) and uuid (dso) endpoints - return endpoint.replace(/\{\?id\}/, `?id=${resourceID}`) - .replace(/\{\?uuid\}/, `?uuid=${resourceID}`); + return this.buildHrefFromFindOptions( endpoint.replace(/\{\?id\}/, `?id=${resourceID}`) + .replace(/\{\?uuid\}/, `?uuid=${resourceID}`), + {}, [], ...linksToFollow); } findByIdAndIDType(id: string, identifierType = IdentifierType.UUID): Observable> { diff --git a/src/app/core/data/dspace-object-data.service.ts b/src/app/core/data/dspace-object-data.service.ts index 38e9f8d888..61cc98281e 100644 --- a/src/app/core/data/dspace-object-data.service.ts +++ b/src/app/core/data/dspace-object-data.service.ts @@ -3,6 +3,7 @@ import { Injectable } from '@angular/core'; import { Store } from '@ngrx/store'; import { Observable } from 'rxjs'; import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; import { dataService } from '../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { ObjectCacheService } from '../cache/object-cache.service'; @@ -31,8 +32,9 @@ class DataServiceImpl extends DataService { super(); } - getIDHref(endpoint, resourceID): string { - return endpoint.replace(/\{\?uuid\}/, `?uuid=${resourceID}`); + getIDHref(endpoint, resourceID, ...linksToFollow: Array>): string { + return this.buildHrefFromFindOptions( endpoint.replace(/\{\?uuid\}/, `?uuid=${resourceID}`), + {}, [], ...linksToFollow); } } diff --git a/src/app/core/data/request.service.ts b/src/app/core/data/request.service.ts index 1101c851ac..810b0721ae 100644 --- a/src/app/core/data/request.service.ts +++ b/src/app/core/data/request.service.ts @@ -3,7 +3,7 @@ import { HttpHeaders } from '@angular/common/http'; import { createSelector, MemoizedSelector, select, Store } from '@ngrx/store'; import { Observable, race as observableRace } from 'rxjs'; -import { filter, map, mergeMap, switchMap, take } from 'rxjs/operators'; +import { filter, map, mergeMap, take } from 'rxjs/operators'; import { cloneDeep, remove } from 'lodash'; import { hasValue, isEmpty, isNotEmpty } from '../../shared/empty.util'; import { CacheableObject } from '../cache/object-cache.reducer'; diff --git a/src/app/core/index/index.effects.ts b/src/app/core/index/index.effects.ts index c9f6eace8f..f885db1436 100644 --- a/src/app/core/index/index.effects.ts +++ b/src/app/core/index/index.effects.ts @@ -12,6 +12,7 @@ import { AddToIndexAction, RemoveFromIndexByValueAction } from './index.actions' import { hasValue } from '../../shared/empty.util'; import { IndexName } from './index.reducer'; import { RestRequestMethod } from '../data/rest-request-method'; +import { getUrlWithoutEmbedParams } from './index.selectors'; @Injectable() export class UUIDIndexEffects { @@ -47,7 +48,7 @@ export class UUIDIndexEffects { map((action: RequestConfigureAction) => { return new AddToIndexAction( IndexName.REQUEST, - action.payload.href, + getUrlWithoutEmbedParams(action.payload.href), action.payload.uuid ); }) diff --git a/src/app/core/index/index.selectors.spec.ts b/src/app/core/index/index.selectors.spec.ts new file mode 100644 index 0000000000..02cce4b7d6 --- /dev/null +++ b/src/app/core/index/index.selectors.spec.ts @@ -0,0 +1,32 @@ +import { getUrlWithoutEmbedParams } from './index.selectors'; + +describe(`index selectors`, () => { + + describe(`getUrlWithoutEmbedParams`, () => { + + it(`should return a url without its embed params`, () => { + const source = 'https://rest.api/resource?a=1&embed=2&b=3&embed=4/5&c=6&embed=7/8/9'; + const result = getUrlWithoutEmbedParams(source); + expect(result).toBe('https://rest.api/resource?a=1&b=3&c=6'); + }); + + it(`should return a url without embed params unmodified`, () => { + const source = 'https://rest.api/resource?a=1&b=3&c=6'; + const result = getUrlWithoutEmbedParams(source); + expect(result).toBe(source); + }); + + it(`should return a string that isn't a url unmodified`, () => { + const source = 'a=1&embed=2&b=3&embed=4/5&c=6&embed=7/8/9'; + const result = getUrlWithoutEmbedParams(source); + expect(result).toBe(source); + }); + + it(`should return undefined or null unmodified`, () => { + expect(getUrlWithoutEmbedParams(undefined)).toBe(undefined); + expect(getUrlWithoutEmbedParams(null)).toBe(null); + }); + + }); + +}); diff --git a/src/app/core/index/index.selectors.ts b/src/app/core/index/index.selectors.ts index de4adab09b..b23496c501 100644 --- a/src/app/core/index/index.selectors.ts +++ b/src/app/core/index/index.selectors.ts @@ -1,8 +1,41 @@ import { createSelector, MemoizedSelector } from '@ngrx/store'; -import { hasValue } from '../../shared/empty.util'; +import { hasValue, isNotEmpty } from '../../shared/empty.util'; import { CoreState } from '../core.reducers'; import { coreSelector } from '../core.selectors'; +import { URLCombiner } from '../url-combiner/url-combiner'; import { IndexName, IndexState, MetaIndexState } from './index.reducer'; +import * as parse from 'url-parse'; + +/** + * Return the given url without `embed` params. + * + * E.g. https://rest.api/resource?size=5&embed=subresource&rpp=3 + * becomes https://rest.api/resource?size=5&rpp=3 + * + * When you index a request url you don't want to include + * embed params because embedded data isn't relevant when + * you want to know + * + * @param url The url to use + */ +export const getUrlWithoutEmbedParams = (url: string): string => { + if (isNotEmpty(url)) { + const parsed = parse(url); + if (isNotEmpty(parsed.query)) { + const parts = parsed.query.split(/[?|&]/) + .filter((part: string) => isNotEmpty(part)) + .filter((part: string) => !part.startsWith('embed=')); + let args = ''; + if (isNotEmpty(parts)) { + args = `?${parts.join('&')}`; + } + url = new URLCombiner(parsed.origin, parsed.pathname, args).toString(); + return url; + } + } + + return url; +}; /** * Return the MetaIndexState based on the CoreSate @@ -74,7 +107,7 @@ export const selfLinkFromUuidSelector = export const uuidFromHrefSelector = (href: string): MemoizedSelector => createSelector( requestIndexSelector, - (state: IndexState) => hasValue(state) ? state[href] : undefined + (state: IndexState) => hasValue(state) ? state[getUrlWithoutEmbedParams(href)] : undefined ); /** diff --git a/src/app/core/url-combiner/url-combiner.ts b/src/app/core/url-combiner/url-combiner.ts index ae622ab976..e7468c6107 100644 --- a/src/app/core/url-combiner/url-combiner.ts +++ b/src/app/core/url-combiner/url-combiner.ts @@ -41,8 +41,8 @@ export class URLCombiner { // remove consecutive slashes url = url.replace(/([^:\s])\/+/g, '$1/'); - // remove trailing slash before parameters or hash - url = url.replace(/\/(\?|&|#[^!])/g, '$1'); + // remove trailing slash + url = url.replace(/\/($|\?|&|#[^!])/g, '$1'); // replace ? in parameters with & url = url.replace(/(\?.+)\?/g, '$1&'); diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts index 17792911f7..359d3abcdc 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/claimed-task-search-result/claimed-task-search-result-detail-element.component.ts @@ -49,10 +49,8 @@ export class ClaimedTaskSearchResultDetailElementComponent extends SearchResultD */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink( - 'workflowitem', - null, - followLink('item', null, followLink('bundles')), + this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, + followLink('item', null, true, followLink('bundles')), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; diff --git a/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts b/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts index aeaf314c6d..3094d8a98d 100644 --- a/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts +++ b/src/app/shared/object-detail/my-dspace-result-detail-element/pool-search-result/pool-search-result-detail-element.component.ts @@ -48,10 +48,8 @@ export class PoolSearchResultDetailElementComponent extends SearchResultDetailEl */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink( - 'workflowitem', - null, - followLink('item', null, followLink('bundles')), + this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, + followLink('item', null, true, followLink('bundles')), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; diff --git a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts index c36b45f2f1..d149595514 100644 --- a/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts +++ b/src/app/shared/object-list/my-dspace-result-list-element/claimed-search-result/claimed-search-result-list-element.component.ts @@ -55,11 +55,8 @@ export class ClaimedSearchResultListElementComponent extends SearchResultListEle */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink( - 'workflowitem', - null, - followLink('item'), - followLink('submitter') + this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, + followLink('item'), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; } diff --git a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts index b429ff823d..0953af3c76 100644 --- a/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts +++ b/src/app/shared/object-list/my-dspace-result-list-element/pool-search-result/pool-search-result-list-element.component.ts @@ -58,11 +58,8 @@ export class PoolSearchResultListElementComponent extends SearchResultListElemen */ ngOnInit() { super.ngOnInit(); - this.linkService.resolveLinks(this.dso, followLink( - 'workflowitem', - null, - followLink('item'), - followLink('submitter') + this.linkService.resolveLinks(this.dso, followLink('workflowitem', null, true, + followLink('item'), followLink('submitter') ), followLink('action')); this.workflowitemRD$ = this.dso.workflowitem as Observable>; } diff --git a/src/app/shared/utils/follow-link-config.model.ts b/src/app/shared/utils/follow-link-config.model.ts index 21df288690..87942d8467 100644 --- a/src/app/shared/utils/follow-link-config.model.ts +++ b/src/app/shared/utils/follow-link-config.model.ts @@ -23,6 +23,11 @@ export class FollowLinkConfig { * use on the retrieved object. */ linksToFollow?: Array>; + + /** + * Forward to rest which links we're following, so these can already be embedded + */ + shouldEmbed? = true; } /** @@ -36,15 +41,19 @@ export class FollowLinkConfig { * in a certain way * @param linksToFollow: a list of {@link FollowLinkConfig}s to * use on the retrieved object. + * @param shouldEmbed: boolean to check whether to forward info on followLinks to rest, + * so these can be embedded, default true */ export const followLink = ( linkName: keyof R['_links'], findListOptions?: FindListOptions, + shouldEmbed = true, ...linksToFollow: Array> ): FollowLinkConfig => { return { name: linkName, findListOptions, + shouldEmbed: shouldEmbed, linksToFollow } };