Skip to content

Commit

Permalink
Merge branch 'w2p-107155_Performance-re-request-embeds-7.6' into w2p-…
Browse files Browse the repository at this point in the history
…107155_Performance-re-request-embeds-main

# Conflicts:
#	src/app/collection-page/collection-page.component.ts
#	src/app/core/breadcrumbs/item-breadcrumb.resolver.ts
#	src/app/core/browse/browse.service.spec.ts
#	src/app/core/browse/browse.service.ts
#	src/app/core/data/relationship-data.service.spec.ts
#	src/app/core/data/relationship-data.service.ts
#	src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts
#	src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.spec.ts
#	src/app/item-page/edit-item-page/item-relationships/edit-relationship-list/edit-relationship-list.component.ts
#	src/app/item-page/item.resolver.ts
#	src/app/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.ts
#	src/app/shared/utils/relation-query.utils.ts
  • Loading branch information
alexandrevryghem committed Oct 11, 2024
2 parents ecb0de7 + b709ee0 commit 1975c63
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 53 deletions.
4 changes: 2 additions & 2 deletions src/app/core/breadcrumbs/item-breadcrumb.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import { Observable } from 'rxjs';

import { BreadcrumbConfig } from '../../breadcrumbs/breadcrumb/breadcrumb-config.model';
import { ITEM_PAGE_LINKS_TO_FOLLOW } from '../../item-page/item.resolver';
import { getItemPageLinksToFollow } from '../../item-page/item.resolver';
import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model';
import { ItemDataService } from '../data/item-data.service';
import { DSpaceObject } from '../shared/dspace-object.model';
Expand All @@ -24,7 +24,7 @@ export const itemBreadcrumbResolver: ResolveFn<BreadcrumbConfig<Item>> = (
breadcrumbService: DSOBreadcrumbsService = inject(DSOBreadcrumbsService),
dataService: ItemDataService = inject(ItemDataService),
): Observable<BreadcrumbConfig<Item>> => {
const linksToFollow: FollowLinkConfig<DSpaceObject>[] = ITEM_PAGE_LINKS_TO_FOLLOW as FollowLinkConfig<DSpaceObject>[];
const linksToFollow: FollowLinkConfig<DSpaceObject>[] = getItemPageLinksToFollow() as FollowLinkConfig<DSpaceObject>[];
return DSOBreadcrumbResolver(
route,
state,
Expand Down
12 changes: 0 additions & 12 deletions src/app/core/browse/browse.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { of as observableOf } from 'rxjs';
import { TestScheduler } from 'rxjs/testing';

import { getMockHrefOnlyDataService } from '../../shared/mocks/href-only-data.service.mock';
import { getMockRemoteDataBuildService } from '../../shared/mocks/remote-data-build.service.mock';
import { getMockRequestService } from '../../shared/mocks/request.service.mock';
import {
createSuccessfulRemoteDataObject,
Expand All @@ -18,7 +17,6 @@ import {
createPaginatedList,
getFirstUsedArgumentOfSpyMethod,
} from '../../shared/testing/utils.test';
import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
import { RequestService } from '../data/request.service';
import { RequestEntry } from '../data/request-entry.model';
import { FlatBrowseDefinition } from '../shared/flat-browse-definition.model';
Expand All @@ -31,7 +29,6 @@ describe('BrowseService', () => {
let scheduler: TestScheduler;
let service: BrowseService;
let requestService: RequestService;
let rdbService: RemoteDataBuildService;

const browsesEndpointURL = 'https://rest.api/browses';
const halService: any = new HALEndpointServiceStub(browsesEndpointURL);
Expand Down Expand Up @@ -129,7 +126,6 @@ describe('BrowseService', () => {
halService,
browseDefinitionDataService,
hrefOnlyDataService,
rdbService,
);
}

Expand All @@ -141,11 +137,9 @@ describe('BrowseService', () => {

beforeEach(() => {
requestService = getMockRequestService(getRequestEntry$(true));
rdbService = getMockRemoteDataBuildService();
service = initTestService();
spyOn(halService, 'getEndpoint').and
.returnValue(hot('--a-', { a: browsesEndpointURL }));
spyOn(rdbService, 'buildList').and.callThrough();
});

it('should call BrowseDefinitionDataService to create the RemoteData Observable', () => {
Expand All @@ -162,9 +156,7 @@ describe('BrowseService', () => {

beforeEach(() => {
requestService = getMockRequestService(getRequestEntry$(true));
rdbService = getMockRemoteDataBuildService();
service = initTestService();
spyOn(rdbService, 'buildList').and.callThrough();
});

describe('when getBrowseEntriesFor is called with a valid browse definition id', () => {
Expand Down Expand Up @@ -215,7 +207,6 @@ describe('BrowseService', () => {
describe('if getBrowseDefinitions fires', () => {
beforeEach(() => {
requestService = getMockRequestService(getRequestEntry$(true));
rdbService = getMockRemoteDataBuildService();
service = initTestService();
spyOn(service, 'getBrowseDefinitions').and
.returnValue(hot('--a-', {
Expand Down Expand Up @@ -270,7 +261,6 @@ describe('BrowseService', () => {
describe('if getBrowseDefinitions doesn\'t fire', () => {
it('should return undefined', () => {
requestService = getMockRequestService(getRequestEntry$(true));
rdbService = getMockRemoteDataBuildService();
service = initTestService();
spyOn(service, 'getBrowseDefinitions').and
.returnValue(hot('----'));
Expand All @@ -288,9 +278,7 @@ describe('BrowseService', () => {
describe('getFirstItemFor', () => {
beforeEach(() => {
requestService = getMockRequestService();
rdbService = getMockRemoteDataBuildService();
service = initTestService();
spyOn(rdbService, 'buildList').and.callThrough();
});

describe('when getFirstItemFor is called with a valid browse definition id', () => {
Expand Down
19 changes: 12 additions & 7 deletions src/app/core/browse/browse.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
startWith,
} from 'rxjs/operators';

import { environment } from '../../../environments/environment';
import {
hasValue,
hasValueOperator,
Expand All @@ -16,7 +17,6 @@ import {
followLink,
FollowLinkConfig,
} from '../../shared/utils/follow-link-config.model';
import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';
import { SortDirection } from '../cache/models/sort-options.model';
import { HrefOnlyDataService } from '../data/href-only-data.service';
import { PaginatedList } from '../data/paginated-list.model';
Expand All @@ -38,9 +38,15 @@ import { URLCombiner } from '../url-combiner/url-combiner';
import { BrowseDefinitionDataService } from './browse-definition-data.service';
import { BrowseEntrySearchOptions } from './browse-entry-search-options.model';

export const BROWSE_LINKS_TO_FOLLOW: FollowLinkConfig<BrowseEntry | Item>[] = [
followLink('thumbnail'),
];
export function getBrowseLinksToFollow(): FollowLinkConfig<BrowseEntry | Item>[] {
const followLinks = [

Check warning on line 42 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L42

Added line #L42 was not covered by tests
followLink('thumbnail'),
];
if (environment.item.showAccessStatuses) {
followLinks.push(followLink('accessStatus'));

Check warning on line 46 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L46

Added line #L46 was not covered by tests
}
return followLinks;

Check warning on line 48 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L48

Added line #L48 was not covered by tests
}

/**
* The service handling all browse requests
Expand All @@ -67,7 +73,6 @@ export class BrowseService {
protected halService: HALEndpointService,
private browseDefinitionDataService: BrowseDefinitionDataService,
private hrefOnlyDataService: HrefOnlyDataService,
private rdb: RemoteDataBuildService,
) {
}

Expand Down Expand Up @@ -117,7 +122,7 @@ export class BrowseService {
}),
);
if (options.fetchThumbnail ) {
return this.hrefOnlyDataService.findListByHref<BrowseEntry>(href$, {}, undefined, undefined, ...BROWSE_LINKS_TO_FOLLOW);
return this.hrefOnlyDataService.findListByHref<BrowseEntry>(href$, {}, undefined, undefined, ...getBrowseLinksToFollow());

Check warning on line 125 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L125

Added line #L125 was not covered by tests
}
return this.hrefOnlyDataService.findListByHref<BrowseEntry>(href$);
}
Expand Down Expand Up @@ -165,7 +170,7 @@ export class BrowseService {
}),
);
if (options.fetchThumbnail) {
return this.hrefOnlyDataService.findListByHref<Item>(href$, {}, undefined, undefined, ...BROWSE_LINKS_TO_FOLLOW);
return this.hrefOnlyDataService.findListByHref<Item>(href$, {}, undefined, undefined, ...getBrowseLinksToFollow());

Check warning on line 173 in src/app/core/browse/browse.service.ts

View check run for this annotation

Codecov / codecov/patch

src/app/core/browse/browse.service.ts#L173

Added line #L173 was not covered by tests
}
return this.hrefOnlyDataService.findListByHref<Item>(href$);
}
Expand Down
5 changes: 4 additions & 1 deletion src/app/core/data/relationship-data.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { Store } from '@ngrx/store';
import { provideMockStore } from '@ngrx/store/testing';
import { of as observableOf } from 'rxjs';

import { APP_CONFIG } from '../../../config/app-config.interface';
import { environment } from '../../../environments/environment.test';
import { PAGINATED_RELATIONS_TO_ITEMS_OPERATOR } from '../../item-page/simple/item-types/shared/item-relationships-utils';
import { getMockRemoteDataBuildServiceHrefMap } from '../../shared/mocks/remote-data-build.service.mock';
import { getMockRequestService } from '../../shared/mocks/request.service.mock';
Expand Down Expand Up @@ -150,14 +152,15 @@ describe('RelationshipDataService', () => {
{ provide: RequestService, useValue: requestService },
{ provide: PAGINATED_RELATIONS_TO_ITEMS_OPERATOR, useValue: jasmine.createSpy('paginatedRelationsToItems').and.returnValue((v) => v) },
{ provide: Store, useValue: provideMockStore() },
{ provide: APP_CONFIG, useValue: environment },
RelationshipDataService,
],
});
service = TestBed.inject(RelationshipDataService);
});

describe('composition', () => {
const initService = () => new RelationshipDataService(null, null, null, null, null, null, null, null);
const initService = () => new RelationshipDataService(null, null, null, null, null, null, null, null, environment);

testSearchDataImplementation(initService);
});
Expand Down
7 changes: 6 additions & 1 deletion src/app/core/data/relationship-data.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import {
tap,
} from 'rxjs/operators';

import {
APP_CONFIG,
AppConfig,
} from '../../../config/app-config.interface';
import {
AppState,
keySelector,
Expand Down Expand Up @@ -133,6 +137,7 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
protected itemService: ItemDataService,
protected appStore: Store<AppState>,
@Inject(PAGINATED_RELATIONS_TO_ITEMS_OPERATOR) private paginatedRelationsToItems: (thisId: string) => (source: Observable<RemoteData<PaginatedList<Relationship>>>) => Observable<RemoteData<PaginatedList<Item>>>,
@Inject(APP_CONFIG) private appConfig: AppConfig,
) {
super('relationships', requestService, rdbService, objectCache, halService, 15 * 60 * 1000);

Expand Down Expand Up @@ -319,7 +324,7 @@ export class RelationshipDataService extends IdentifiableDataService<Relationshi
* @param options
*/
getRelatedItemsByLabel(item: Item, label: string, options?: FindListOptions): Observable<RemoteData<PaginatedList<Item>>> {
const linksToFollow: FollowLinkConfig<Relationship>[] = itemLinksToFollow(options.fetchThumbnail);
const linksToFollow: FollowLinkConfig<Relationship>[] = itemLinksToFollow(options.fetchThumbnail, this.appConfig.item.showAccessStatuses);
linksToFollow.push(followLink('relationshipType'));

return this.getItemRelationshipsByLabel(item, label, options, true, true, ...linksToFollow).pipe(this.paginatedRelationsToItems(item.uuid));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ export class RecentItemListComponent implements OnInit, OnDestroy {
if (this.appConfig.browseBy.showThumbnails) {
linksToFollow.push(followLink('thumbnail'));
}
if (this.appConfig.item.showAccessStatuses) {
linksToFollow.push(followLink('accessStatus'));

Check warning on line 102 in src/app/home-page/recent-item-list/recent-item-list.component.ts

View check run for this annotation

Codecov / codecov/patch

src/app/home-page/recent-item-list/recent-item-list.component.ts#L102

Added line #L102 was not covered by tests
}

this.itemRD$ = this.searchService.search(
new PaginatedSearchOptions({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { getAllSucceededRemoteData } from '../../../core/shared/operators';
import { hasValue } from '../../../shared/empty.util';
import { NotificationsService } from '../../../shared/notifications/notifications.service';
import { AbstractTrackableComponent } from '../../../shared/trackable/abstract-trackable.component';
import { ITEM_PAGE_LINKS_TO_FOLLOW } from '../../item.resolver';
import { getItemPageLinksToFollow } from '../../item.resolver';
import { getItemPageRoute } from '../../item-page-routing-paths';

@Component({
Expand Down Expand Up @@ -92,7 +92,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl
this.item = rd.payload;
}),
switchMap((rd: RemoteData<Item>) => {
return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...ITEM_PAGE_LINKS_TO_FOLLOW);
return this.itemService.findByHref(rd.payload._links.self.href, true, true, ...getItemPageLinksToFollow());
}),
getAllSucceededRemoteData(),
).subscribe((rd: RemoteData<Item>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ export class ItemDeleteComponent
this.linkService.resolveLinks(
relationship,
followLink('relationshipType'),
followLink('leftItem'),
followLink('rightItem'),
followLink('leftItem', undefined, followLink<Item>('accessStatus')),
followLink('rightItem', undefined, followLink<Item>('accessStatus')),
);
return relationship.relationshipType.pipe(
getFirstSucceededRemoteData(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ export class EditRelationshipListComponent implements OnInit, OnDestroy {
);

// this adds thumbnail images when required by configuration
const linksToFollow: FollowLinkConfig<Relationship>[] = itemLinksToFollow(this.fetchThumbnail);
const linksToFollow: FollowLinkConfig<Relationship>[] = itemLinksToFollow(this.fetchThumbnail, this.appConfig.item.showAccessStatuses);

this.subs.push(
observableCombineLatest([
Expand Down
4 changes: 2 additions & 2 deletions src/app/item-page/item-page.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { redirectOn4xx } from '../core/shared/authorized.operators';
import { Item } from '../core/shared/item.model';
import { getFirstCompletedRemoteData } from '../core/shared/operators';
import { hasValue } from '../shared/empty.util';
import { ITEM_PAGE_LINKS_TO_FOLLOW } from './item.resolver';
import { getItemPageLinksToFollow } from './item.resolver';
import { getItemPageRoute } from './item-page-routing-paths';

/**
Expand All @@ -44,7 +44,7 @@ export const itemPageResolver: ResolveFn<RemoteData<Item>> = (
route.params.id,
true,
false,
...ITEM_PAGE_LINKS_TO_FOLLOW,
...getItemPageLinksToFollow(),
).pipe(
getFirstCompletedRemoteData(),
redirectOn4xx(router, authService),
Expand Down
27 changes: 17 additions & 10 deletions src/app/item-page/item.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
import { Store } from '@ngrx/store';
import { Observable } from 'rxjs';

import { environment } from '../../environments/environment';
import { AppState } from '../app.reducer';
import { ItemDataService } from '../core/data/item-data.service';
import { RemoteData } from '../core/data/remote-data';
Expand All @@ -22,15 +23,21 @@ import {
* The self links defined in this list are expected to be requested somewhere in the near future
* Requesting them as embeds will limit the number of requests
*/
export const ITEM_PAGE_LINKS_TO_FOLLOW: FollowLinkConfig<Item>[] = [
followLink('owningCollection', {},
followLink('parentCommunity', {},
followLink('parentCommunity')),
),
followLink('relationships'),
followLink('version', {}, followLink('versionhistory')),
followLink('thumbnail'),
];
export function getItemPageLinksToFollow(): FollowLinkConfig<Item>[] {
const followLinks: FollowLinkConfig<Item>[] = [
followLink('owningCollection', {},
followLink('parentCommunity', {},
followLink('parentCommunity')),
),
followLink('relationships'),
followLink('version', {}, followLink('versionhistory')),
followLink('thumbnail'),
];
if (environment.item.showAccessStatuses) {
followLinks.push(followLink('accessStatus'));

Check warning on line 37 in src/app/item-page/item.resolver.ts

View check run for this annotation

Codecov / codecov/patch

src/app/item-page/item.resolver.ts#L37

Added line #L37 was not covered by tests
}
return followLinks;
}

export const itemResolver: ResolveFn<RemoteData<Item>> = (
route: ActivatedRouteSnapshot,
Expand All @@ -42,7 +49,7 @@ export const itemResolver: ResolveFn<RemoteData<Item>> = (
route.params.id,
true,
false,
...ITEM_PAGE_LINKS_TO_FOLLOW,
...getItemPageLinksToFollow(),
).pipe(
getFirstCompletedRemoteData(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ export class DsDynamicFormControlContainerComponent extends DynamicFormControlCo
const relationship$ = this.relationshipService.findById(this.metadataService.virtualValue(this.value),
true,
true,
... itemLinksToFollow(this.fetchThumbnail)).pipe(
... itemLinksToFollow(this.fetchThumbnail, this.appConfig.item.showAccessStatuses)).pipe(
getAllSucceededRemoteData(),
getRemoteDataPayload());
this.relationshipValue$ = observableCombineLatest([this.item$.pipe(take(1)), relationship$]).pipe(
Expand Down
28 changes: 16 additions & 12 deletions src/app/shared/utils/relation-query.utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Item } from '../../core/shared/item.model';
import { Relationship } from '../../core/shared/item-relationships/relationship.model';
import {
followLink,
Expand All @@ -24,19 +25,22 @@ export function getFilterByRelation(relationType: string, itemUUID: string): str
}

/**
* Creates links to follow for the leftItem and rightItem. Links will include
* @param showThumbnail thumbnail image configuration
* @returns followLink array
* Creates links to follow for the leftItem and rightItem. Optionally additional links for `thumbnail` & `accessStatus`
* can be embedded as well.
*
* @param showThumbnail Whether the `thumbnail` needs to be embedded on the {@link Item}
* @param showAccessStatus Whether the `accessStatus` needs to be embedded on the {@link Item}
*/
export function itemLinksToFollow(showThumbnail: boolean): FollowLinkConfig<Relationship>[] {
let linksToFollow: FollowLinkConfig<Relationship>[];
export function itemLinksToFollow(showThumbnail: boolean, showAccessStatus: boolean): FollowLinkConfig<Relationship>[] {
const conditionalLinksToFollow: FollowLinkConfig<Item>[] = [];
if (showThumbnail) {
linksToFollow = [
followLink('leftItem',{}, followLink('thumbnail')),
followLink('rightItem',{}, followLink('thumbnail')),
];
} else {
linksToFollow = [followLink('leftItem'), followLink('rightItem')];
conditionalLinksToFollow.push(followLink<Item>('thumbnail'));
}
return linksToFollow;
if (showAccessStatus) {
conditionalLinksToFollow.push(followLink<Item>('accessStatus'));

Check warning on line 40 in src/app/shared/utils/relation-query.utils.ts

View check run for this annotation

Codecov / codecov/patch

src/app/shared/utils/relation-query.utils.ts#L40

Added line #L40 was not covered by tests
}
return [
followLink('leftItem', undefined, ...conditionalLinksToFollow),
followLink('rightItem', undefined, ...conditionalLinksToFollow),
];
}

0 comments on commit 1975c63

Please sign in to comment.