Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: linkName from decorator assign value to the correct property #2972

42 changes: 30 additions & 12 deletions src/app/core/cache/builders/link.service.spec.ts
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
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,16 @@
* @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);

Check warning on line 123 in src/app/core/cache/builders/link.service.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L123 was not covered by tests
}
return model;
}

Expand Down
40 changes: 34 additions & 6 deletions src/app/core/data/base/base-data.service.spec.ts
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
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
Loading