From 3351615140e2e2c0023b61fbe3c867efda6c0222 Mon Sep 17 00:00:00 2001 From: pcg-kk Date: Mon, 22 Apr 2024 21:08:10 +0200 Subject: [PATCH 1/7] fix: linkName from decorator assign value to the correct property --- src/app/core/cache/builders/link.service.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/app/core/cache/builders/link.service.ts b/src/app/core/cache/builders/link.service.ts index 6265e89d532..df3ccf1c87e 100644 --- a/src/app/core/cache/builders/link.service.ts +++ b/src/app/core/cache/builders/link.service.ts @@ -112,7 +112,16 @@ export class LinkService { * @param linkToFollow the {@link FollowLinkConfig} to resolve */ public resolveLink(model, linkToFollow: FollowLinkConfig): T { - model[linkToFollow.name] = this.resolveLinkWithoutAttaching(model, linkToFollow); + const linkDefinitions = this.getLinkDefinitions(model.constructor as GenericConstructor); + const linkDef = linkDefinitions.get(linkToFollow.name); + + if (isNotEmpty(linkDef)) { + // If link exist in definition we can resolve it and use a real property name + model[linkDef.propertyName] = this.resolveLinkWithoutAttaching(model, linkToFollow); + } else { + // For some links we don't have a definition, so we use the link name as property name + model[linkToFollow.name] = this.resolveLinkWithoutAttaching(model, linkToFollow); + } return model; } From 973fd339da24b0d96ef4484e20f4a2a93224bd04 Mon Sep 17 00:00:00 2001 From: pcg-kk Date: Mon, 22 Apr 2024 22:16:42 +0200 Subject: [PATCH 2/7] fix: add unit test for rename property case --- .../core/cache/builders/link.service.spec.ts | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/src/app/core/cache/builders/link.service.spec.ts b/src/app/core/cache/builders/link.service.spec.ts index d9878913888..122945ab6aa 100644 --- a/src/app/core/cache/builders/link.service.spec.ts +++ b/src/app/core/cache/builders/link.service.spec.ts @@ -31,10 +31,12 @@ class TestModel implements HALResource { self: HALLink; predecessor: HALLink; successor: HALLink; + standardLinkName: HALLink; }; predecessor?: TestModel; successor?: TestModel; + renamedProperty?: TestModel; } const mockDataServiceMap: any = new Map([ @@ -66,6 +68,24 @@ describe('LinkService', () => { testDataService = new TestDataService(); spyOn(testDataService, 'findListByHref').and.callThrough(); spyOn(testDataService, 'findByHref').and.callThrough(); + + const linksDefinitions = new Map(); + linksDefinitions.set('predecessor', { + resourceType: TEST_MODEL, + linkName: 'predecessor', + propertyName: 'predecessor', + }); + linksDefinitions.set('successor', { + resourceType: TEST_MODEL, + linkName: 'successor', + propertyName: 'successor', + }); + linksDefinitions.set('standardLinkName', { + resourceType: TEST_MODEL, + linkName: 'standardLinkName', + propertyName: 'renamedProperty', + }); + TestBed.configureTestingModule({ providers: [ LinkService, @@ -87,18 +107,7 @@ describe('LinkService', () => { }, { provide: LINK_DEFINITION_MAP_FACTORY, - useValue: jasmine.createSpy('getLinkDefinitions').and.returnValue([ - { - resourceType: TEST_MODEL, - linkName: 'predecessor', - propertyName: 'predecessor', - }, - { - resourceType: TEST_MODEL, - linkName: 'successor', - propertyName: 'successor', - }, - ]), + useValue: jasmine.createSpy('getLinkDefinitions').and.returnValue(linksDefinitions), }, ], }); @@ -117,6 +126,15 @@ describe('LinkService', () => { }); }); }); + describe(`when the propertyName is different than linkName`, () => { + beforeEach(() => { + result = service.resolveLink(testModel, followLink('standardLinkName', {})); + }); + it('link should be assign to custom property', () => { + expect(result.renamedProperty).toBeDefined(); + expect(result.standardLinkName).toBeUndefined(); + }); + }); describe(`when the linkdefinition concerns a list`, () => { beforeEach(() => { ((service as any).getLinkDefinition as jasmine.Spy).and.returnValue({ From a862d84738a573c559d18bf523b4cc346000dde2 Mon Sep 17 00:00:00 2001 From: pcg-kk Date: Mon, 12 Aug 2024 21:27:01 +0200 Subject: [PATCH 3/7] fix: use the correct link name for find by href --- src/app/core/data/base/base-data.service.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index d09ee21ee03..56f0c2fc26a 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -28,6 +28,7 @@ import { isNotEmptyOperator, } from '../../../shared/empty.util'; import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; +import { getLinkDefinition } from '../../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { CacheableObject } from '../../cache/cacheable-object.model'; import { RequestParam } from '../../cache/models/request-param.model'; @@ -301,8 +302,9 @@ export class BaseDataService implements HALDataServic tap((remoteDataObject: RemoteData) => { if (hasValue(remoteDataObject?.payload?._links)) { for (const followLinkName of Object.keys(remoteDataObject.payload._links)) { - // only add the followLinks if they are embedded - if (hasValue(remoteDataObject.payload[followLinkName]) && followLinkName !== 'self') { + // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name + const linkDefinition = getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); + if (linkDefinition?.propertyName && hasValue(remoteDataObject.payload[linkDefinition.propertyName]) && followLinkName !== 'self') { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(remoteDataObject.payload._links[followLinkName]); for (const individualFollowLink of followLinksList) { From d9d39b0cb5f1aed5ff7ec9c51c60fb354232a01c Mon Sep 17 00:00:00 2001 From: pcg-kk Date: Tue, 13 Aug 2024 19:36:02 +0200 Subject: [PATCH 4/7] fix: use the correct link name for find by href for findListByHref --- src/app/core/data/base/base-data.service.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index 56f0c2fc26a..b47deb339ca 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -358,8 +358,9 @@ export class BaseDataService implements HALDataServic for (const object of remoteDataObject.payload.page) { if (hasValue(object?._links)) { for (const followLinkName of Object.keys(object._links)) { - // only add the followLinks if they are embedded - if (hasValue(object[followLinkName]) && followLinkName !== 'self') { + // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name + const linkDefinition = getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); + if (linkDefinition?.propertyName && hasValue(remoteDataObject.payload[linkDefinition.propertyName]) && followLinkName !== 'self') { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(object._links[followLinkName]); for (const individualFollowLink of followLinksList) { From 752eb4c57bd9b41f65cae0094006c2e887f37147 Mon Sep 17 00:00:00 2001 From: pcg-kk Date: Wed, 6 Nov 2024 21:29:34 +0100 Subject: [PATCH 5/7] fix: resolve issue with unit tests --- .../core/data/base/base-data.service.spec.ts | 14 ++++++++++++-- src/app/core/data/base/base-data.service.ts | 18 ++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/app/core/data/base/base-data.service.spec.ts b/src/app/core/data/base/base-data.service.spec.ts index 3f44ad5e5ac..7e4443d9a79 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -26,11 +26,13 @@ import { HALEndpointServiceStub } from '../../../shared/testing/hal-endpoint-ser import { ObjectCacheServiceStub } from '../../../shared/testing/object-cache-service.stub'; import { createPaginatedList } from '../../../shared/testing/utils.test'; import { followLink } from '../../../shared/utils/follow-link-config.model'; +import { LinkDefinition } from '../../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; import { ObjectCacheService } from '../../cache/object-cache.service'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { HALLink } from '../../shared/hal-link.model'; +import { HALResource } from '../../shared/hal-resource.model'; import { FindListOptions } from '../find-list-options.model'; import { RemoteData } from '../remote-data'; import { RequestService } from '../request.service'; @@ -417,6 +419,11 @@ describe('BaseDataService', () => { c: remoteDataMocks.ResponsePending, d: remoteDataMocks.Success, })); + spyOn(service, 'getLinkDefinition').and.callFake((source, linkName) => { + return { + propertyName: linkName, + } as any as LinkDefinition; + }); const expected = '--b-c-d'; const values = { b: remoteDataMocks.RequestPending, @@ -625,16 +632,19 @@ describe('BaseDataService', () => { c: remoteDataPageMocks.ResponsePending, d: remoteDataPageMocks.Success, })); + spyOn(service, 'getLinkDefinition').and.callFake((source, linkName) => { + return { + propertyName: linkName, + } as any as LinkDefinition; + }); const expected = '--b-c-d'; const values = { b: remoteDataPageMocks.RequestPending, c: remoteDataPageMocks.ResponsePending, d: remoteDataPageMocks.Success, }; - expectObservable(service.findListByHref(selfLink, findListOptions, false, false, ...linksToFollow)).toBe(expected, values); flush(); - expect(objectCache.addDependency).toHaveBeenCalledTimes(3); }); }); }); diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index b47deb339ca..19e466ee7aa 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -28,14 +28,19 @@ import { isNotEmptyOperator, } from '../../../shared/empty.util'; import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; -import { getLinkDefinition } from '../../cache/builders/build-decorators'; +import { + getLinkDefinition, + LinkDefinition, +} from '../../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { CacheableObject } from '../../cache/cacheable-object.model'; import { RequestParam } from '../../cache/models/request-param.model'; import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; import { ObjectCacheService } from '../../cache/object-cache.service'; +import { GenericConstructor } from '../../shared/generic-constructor'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { HALLink } from '../../shared/hal-link.model'; +import { HALResource } from '../../shared/hal-resource.model'; import { getFirstCompletedRemoteData } from '../../shared/operators'; import { URLCombiner } from '../../url-combiner/url-combiner'; import { FindListOptions } from '../find-list-options.model'; @@ -303,7 +308,7 @@ export class BaseDataService implements HALDataServic if (hasValue(remoteDataObject?.payload?._links)) { for (const followLinkName of Object.keys(remoteDataObject.payload._links)) { // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name - const linkDefinition = getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); + const linkDefinition = this.getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); if (linkDefinition?.propertyName && hasValue(remoteDataObject.payload[linkDefinition.propertyName]) && followLinkName !== 'self') { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(remoteDataObject.payload._links[followLinkName]); @@ -359,8 +364,8 @@ export class BaseDataService implements HALDataServic if (hasValue(object?._links)) { for (const followLinkName of Object.keys(object._links)) { // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name - const linkDefinition = getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); - if (linkDefinition?.propertyName && hasValue(remoteDataObject.payload[linkDefinition.propertyName]) && followLinkName !== 'self') { + const linkDefinition = this.getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); + if (linkDefinition?.propertyName && followLinkName !== 'self' && hasValue(object[linkDefinition.propertyName])) { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(object._links[followLinkName]); for (const individualFollowLink of followLinksList) { @@ -515,4 +520,9 @@ export class BaseDataService implements HALDataServic return done$; } + + getLinkDefinition(source: GenericConstructor, linkName: keyof D['_links']): LinkDefinition { + return getLinkDefinition(source, linkName); + } + } From e4b2ebe7aaaa8afe9ca775bde51679fba20fc09c Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 6 Dec 2024 23:29:21 +0100 Subject: [PATCH 6/7] fix: replace 'any' with specific types --- src/app/core/data/base/base-data.service.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index 2146ba24752..2d4eceb41b3 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -40,7 +40,6 @@ import { ObjectCacheService } from '../../cache/object-cache.service'; import { GenericConstructor } from '../../shared/generic-constructor'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { HALLink } from '../../shared/hal-link.model'; -import { HALResource } from '../../shared/hal-resource.model'; import { getFirstCompletedRemoteData } from '../../shared/operators'; import { URLCombiner } from '../../url-combiner/url-combiner'; import { FindListOptions } from '../find-list-options.model'; @@ -307,9 +306,9 @@ export class BaseDataService implements HALDataServic // Ensure all followLinks from the cached object are automatically invalidated when invalidating the cached object tap((remoteDataObject: RemoteData) => { if (hasValue(remoteDataObject?.payload?._links)) { - for (const followLinkName of Object.keys(remoteDataObject.payload._links)) { + for (const followLinkName of Object.keys(remoteDataObject.payload._links) as (keyof typeof remoteDataObject.payload._links)[]) { // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name - const linkDefinition = this.getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); + const linkDefinition: LinkDefinition = getLinkDefinition(remoteDataObject.payload.constructor as GenericConstructor, followLinkName); if (linkDefinition?.propertyName && hasValue(remoteDataObject.payload[linkDefinition.propertyName]) && followLinkName !== 'self') { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(remoteDataObject.payload._links[followLinkName]); @@ -364,9 +363,9 @@ export class BaseDataService implements HALDataServic if (hasValue(remoteDataObject?.payload?.page)) { for (const object of remoteDataObject.payload.page) { if (hasValue(object?._links)) { - for (const followLinkName of Object.keys(object._links)) { + for (const followLinkName of Object.keys(object._links) as (keyof typeof object._links)[]) { // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name - const linkDefinition = this.getLinkDefinition((remoteDataObject.payload as any).constructor, followLinkName); + const linkDefinition: LinkDefinition> = getLinkDefinition(remoteDataObject.payload.constructor as GenericConstructor>, followLinkName); if (linkDefinition?.propertyName && followLinkName !== 'self' && hasValue(object[linkDefinition.propertyName])) { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(object._links[followLinkName]); @@ -522,9 +521,4 @@ export class BaseDataService implements HALDataServic return done$; } - - getLinkDefinition(source: GenericConstructor, linkName: keyof D['_links']): LinkDefinition { - return getLinkDefinition(source, linkName); - } - } From 7ca4d8f2b13b68cbcedddae5d212caf8f00247f7 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Sat, 7 Dec 2024 02:21:41 +0100 Subject: [PATCH 7/7] fix: ensure findListByHref correctly calls addDependency --- .../core/data/base/base-data.service.spec.ts | 54 ++++++++++++------- src/app/core/data/base/base-data.service.ts | 2 +- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/app/core/data/base/base-data.service.spec.ts b/src/app/core/data/base/base-data.service.spec.ts index b17e024a540..6f323956672 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -5,6 +5,7 @@ * * http://www.dspace.org/license/ */ +// eslint-disable-next-line max-classes-per-file import { fakeAsync, tick, @@ -26,14 +27,20 @@ import { HALEndpointServiceStub } from '../../../shared/testing/hal-endpoint-ser import { ObjectCacheServiceStub } from '../../../shared/testing/object-cache-service.stub'; import { createPaginatedList } from '../../../shared/testing/utils.test'; import { followLink } from '../../../shared/utils/follow-link-config.model'; -import { LinkDefinition } from '../../cache/builders/build-decorators'; +import { + link, + typedObject, +} from '../../cache/builders/build-decorators'; import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; import { ObjectCacheEntry } from '../../cache/object-cache.reducer'; import { ObjectCacheService } from '../../cache/object-cache.service'; +import { BITSTREAM } from '../../shared/bitstream.resource-type'; +import { COLLECTION } from '../../shared/collection.resource-type'; import { HALEndpointService } from '../../shared/hal-endpoint.service'; import { HALLink } from '../../shared/hal-link.model'; -import { HALResource } from '../../shared/hal-resource.model'; +import { ResourceType } from '../../shared/resource-type'; import { FindListOptions } from '../find-list-options.model'; +import { PaginatedList } from '../paginated-list.model'; import { RemoteData } from '../remote-data'; import { RequestService } from '../request.service'; import { RequestEntryState } from '../request-entry-state.model'; @@ -58,6 +65,25 @@ class TestService extends BaseDataService { } } +@typedObject +class BaseData { + static type = new ResourceType('test'); + + foo: string; + + _links: { + followLink1: HALLink; + followLink2: HALLink[]; + self: HALLink; + }; + + @link(COLLECTION) + followLink1: Observable; + + @link(BITSTREAM, true, 'followLink2') + followLink2CustomVariableName: Observable>; +} + describe('BaseDataService', () => { let service: TestService; let requestService; @@ -68,8 +94,8 @@ describe('BaseDataService', () => { let linksToFollow; let testScheduler; let remoteDataTimestamp: number; - let remoteDataMocks: { [responseType: string]: RemoteData }; - let remoteDataPageMocks: { [responseType: string]: RemoteData }; + let remoteDataMocks: { [responseType: string]: RemoteData }; + let remoteDataPageMocks: { [responseType: string]: RemoteData> }; function initTestService(): TestService { requestService = getMockRequestService(); @@ -92,10 +118,10 @@ describe('BaseDataService', () => { // as cached values. remoteDataTimestamp = new Date().getTime() + 60 * 1000; const msToLive = 15 * 60 * 1000; - const payload = { + const payload: BaseData = Object.assign(new BaseData(), { foo: 'bar', - followLink1: {}, - followLink2: {}, + followLink1: observableOf({}), + followLink2CustomVariableName: observableOf(createPaginatedList()), _links: { self: Object.assign(new HALLink(), { href: 'self-test-link', @@ -112,7 +138,7 @@ describe('BaseDataService', () => { }), ], }, - }; + }); const statusCodeSuccess = 200; const statusCodeError = 404; const errorMessage = 'not found'; @@ -439,11 +465,6 @@ describe('BaseDataService', () => { spyOn(rdbService, 'buildSingle').and.returnValue(cold('a', { a: remoteDataMocks.Success, })); - spyOn(service, 'getLinkDefinition').and.callFake((source, linkName) => { - return { - propertyName: linkName, - } as any as LinkDefinition; - }); const expected = 'a'; const values = { a: remoteDataMocks.Success, @@ -670,19 +691,16 @@ describe('BaseDataService', () => { c: remoteDataPageMocks.ResponsePending, d: remoteDataPageMocks.Success, })); - spyOn(service, 'getLinkDefinition').and.callFake((source, linkName) => { - return { - propertyName: linkName, - } as any as LinkDefinition; - }); const expected = '--b-c-d'; const values = { b: remoteDataPageMocks.RequestPending, c: remoteDataPageMocks.ResponsePending, d: remoteDataPageMocks.Success, }; + expectObservable(service.findListByHref(selfLink, findListOptions, false, false, ...linksToFollow)).toBe(expected, values); flush(); + expect(objectCache.addDependency).toHaveBeenCalledTimes(3); }); }); }); diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index 2d4eceb41b3..df4e9b7a487 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -365,7 +365,7 @@ export class BaseDataService implements HALDataServic if (hasValue(object?._links)) { for (const followLinkName of Object.keys(object._links) as (keyof typeof object._links)[]) { // only add the followLinks if they are embedded, and we get only links from the linkMap with the correct name - const linkDefinition: LinkDefinition> = getLinkDefinition(remoteDataObject.payload.constructor as GenericConstructor>, followLinkName); + const linkDefinition: LinkDefinition> = getLinkDefinition(object.constructor as GenericConstructor>, followLinkName); if (linkDefinition?.propertyName && followLinkName !== 'self' && hasValue(object[linkDefinition.propertyName])) { // followLink can be either an individual HALLink or a HALLink[] const followLinksList: HALLink[] = [].concat(object._links[followLinkName]);