Merge pull request #2972 from pcg-kk/issues/2819/linkName-in-the-link-decorator-doesnt-assign-to-the-value-on-the-correct-property

fix: linkName from decorator assign value to the correct property

Author: tdonohue

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({

src/app/core/cache/builders/link.service.ts

@@ -112,7 +112,16 @@ export class LinkService {
    * @param linkToFollow the {@link FollowLinkConfig} to resolve
    */
   public resolveLink<T extends HALResource>(model, linkToFollow: FollowLinkConfig<T>): T {
-    model[linkToFollow.name] = this.resolveLinkWithoutAttaching(model, linkToFollow);
+    const linkDefinitions = this.getLinkDefinitions(model.constructor as GenericConstructor<T>);
+    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;
   }
 

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,12 +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 {
+  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 { 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';
@@ -56,6 +65,25 @@ class TestService extends BaseDataService<any> {
   }
 }
 
+@typedObject
+class BaseData {
+  static type = new ResourceType('test');
+
+  foo: string;
+
+  _links: {
+    followLink1: HALLink;
+    followLink2: HALLink[];
+    self: HALLink;
+  };
+
+  @link(COLLECTION)
+  followLink1: Observable<any>;
+
+  @link(BITSTREAM, true, 'followLink2')
+  followLink2CustomVariableName: Observable<PaginatedList<any>>;
+}
+
 describe('BaseDataService', () => {
   let service: TestService;
   let requestService;
@@ -66,8 +94,8 @@ describe('BaseDataService', () => {
   let linksToFollow;
   let testScheduler;
   let remoteDataTimestamp: number;
-  let remoteDataMocks: { [responseType: string]: RemoteData<any> };
-  let remoteDataPageMocks: { [responseType: string]: RemoteData<any> };
+  let remoteDataMocks: { [responseType: string]: RemoteData<BaseData> };
+  let remoteDataPageMocks: { [responseType: string]: RemoteData<PaginatedList<BaseData>> };
 
   function initTestService(): TestService {
     requestService = getMockRequestService();
@@ -90,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',
@@ -110,7 +138,7 @@ describe('BaseDataService', () => {
           }),
         ],
       },
-    };
+    });
     const statusCodeSuccess = 200;
     const statusCodeError = 404;
     const errorMessage = 'not found';

src/app/core/data/base/base-data.service.ts

@@ -28,11 +28,16 @@ import {
   isNotEmptyOperator,
 } from '../../../shared/empty.util';
 import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model';
+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 { getFirstCompletedRemoteData } from '../../shared/operators';
@@ -301,9 +306,10 @@ export class BaseDataService<T extends CacheableObject> implements HALDataServic
       // Ensure all followLinks from the cached object are automatically invalidated when invalidating the cached object
       tap((remoteDataObject: RemoteData<T>) => {
         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') {
+          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: LinkDefinition<T> = getLinkDefinition(remoteDataObject.payload.constructor as GenericConstructor<T>, 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) {
@@ -357,9 +363,10 @@ export class BaseDataService<T extends CacheableObject> 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)) {
-                // only add the followLinks if they are embedded
-                if (hasValue(object[followLinkName]) && followLinkName !== 'self') {
+              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<PaginatedList<T>> = getLinkDefinition(object.constructor as GenericConstructor<PaginatedList<T>>, 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) {