Skip to content

Commit

Permalink
Merge pull request #2972 from pcg-kk/issues/2819/linkName-in-the-link…
Browse files Browse the repository at this point in the history 
…-decorator-doesnt-assign-to-the-value-on-the-correct-property

fix: linkName from decorator assign value to the correct property
  • Loading branch information
@tdonohue
tdonohue authored Dec 19, 2024
2 parents d13d886 + 7ca4d8f commit 5b3da02
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 25 deletions.
42 changes: 30 additions & 12 deletions src/app/core/cache/builders/link.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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,
Expand All @@ -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),
},
],
});
Expand All @@ -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({
Expand Down
11 changes: 10 additions & 1 deletion src/app/core/cache/builders/link.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
40 changes: 34 additions & 6 deletions src/app/core/data/base/base-data.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*
* http://www.dspace.org/license/
*/
// eslint-disable-next-line max-classes-per-file
import {
fakeAsync,
tick,
Expand All @@ -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';
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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',
Expand All @@ -110,7 +138,7 @@ describe('BaseDataService', () => {
}),
],
},
};
});
const statusCodeSuccess = 200;
const statusCodeError = 404;
const errorMessage = 'not found';
Expand Down
19 changes: 13 additions & 6 deletions src/app/core/data/base/base-data.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 5b3da02

Please sign in to comment.