From 987ea5104adda0f7887a4fef710a11b6fa165989 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 19 Nov 2024 16:35:45 +0100 Subject: [PATCH 1/3] 120109: Fixed BaseDataService not emitting when the request is too fast and the ResponsePending are not emitted (cherry picked from commit 0f4d71eb58d63c9c73af2e098126437da0f266c4) --- .../core/data/base/base-data.service.spec.ts | 69 +++++++++++++++---- src/app/core/data/base/base-data.service.ts | 6 +- 2 files changed, 60 insertions(+), 15 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 098f075c10..9ec31b376e 100644 --- a/src/app/core/data/base/base-data.service.spec.ts +++ b/src/app/core/data/base/base-data.service.spec.ts @@ -50,6 +50,7 @@ describe('BaseDataService', () => { let selfLink; let linksToFollow; let testScheduler; + let remoteDataTimestamp: number; let remoteDataMocks; function initTestService(): TestService { @@ -86,19 +87,21 @@ describe('BaseDataService', () => { expect(actual).toEqual(expected); }); - const timeStamp = new Date().getTime(); + // The response's lastUpdated equals the time of 60 seconds after the test started, ensuring they are not perceived + // as cached values. + remoteDataTimestamp = new Date().getTime() + 60 * 1000; const msToLive = 15 * 60 * 1000; const payload = { foo: 'bar' }; const statusCodeSuccess = 200; const statusCodeError = 404; const errorMessage = 'not found'; remoteDataMocks = { - RequestPending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.RequestPending, undefined, undefined, undefined), - ResponsePending: new RemoteData(undefined, msToLive, timeStamp, RequestEntryState.ResponsePending, undefined, undefined, undefined), - Success: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Success, undefined, payload, statusCodeSuccess), - SuccessStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.SuccessStale, undefined, payload, statusCodeSuccess), - Error: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError), - ErrorStale: new RemoteData(timeStamp, msToLive, timeStamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError), + RequestPending: new RemoteData(undefined, msToLive, remoteDataTimestamp, RequestEntryState.RequestPending, undefined, undefined, undefined), + ResponsePending: new RemoteData(undefined, msToLive, remoteDataTimestamp, RequestEntryState.ResponsePending, undefined, undefined, undefined), + Success: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.Success, undefined, payload, statusCodeSuccess), + SuccessStale: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.SuccessStale, undefined, payload, statusCodeSuccess), + Error: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.Error, errorMessage, undefined, statusCodeError), + ErrorStale: new RemoteData(remoteDataTimestamp, msToLive, remoteDataTimestamp, RequestEntryState.ErrorStale, errorMessage, undefined, statusCodeError), }; return new TestService( @@ -330,11 +333,15 @@ describe('BaseDataService', () => { spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source); }); - - it(`should not emit a cached completed RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + it('should not emit a cached completed RemoteData', () => { + // Old cached value from 1 minute before the test started + const oldCachedSucceededData: RemoteData = Object.assign({}, remoteDataMocks.Success, { + timeCompleted: remoteDataTimestamp - 2 * 60 * 1000, + lastUpdated: remoteDataTimestamp - 2 * 60 * 1000, + } as RemoteData); testScheduler.run(({ cold, expectObservable }) => { spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', { - a: remoteDataMocks.Success, + a: oldCachedSucceededData, b: remoteDataMocks.RequestPending, c: remoteDataMocks.ResponsePending, d: remoteDataMocks.Success, @@ -352,6 +359,22 @@ describe('BaseDataService', () => { }); }); + it('should emit the first completed RemoteData since the request was made', () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b', { + a: remoteDataMocks.Success, + b: remoteDataMocks.SuccessStale, + })); + const expected = 'a-b'; + const values = { + a: remoteDataMocks.Success, + b: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findByHref(selfLink, false, true, ...linksToFollow)).toBe(expected, values); + }); + }); + it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(rdbService, 'buildSingle').and.returnValue(cold('a-b-c-d-e', { @@ -514,11 +537,15 @@ describe('BaseDataService', () => { spyOn(service as any, 'reRequestStaleRemoteData').and.callFake(() => (source) => source); }); - - it(`should not emit a cached completed RemoteData, but only start emitting after the state first changes to RequestPending`, () => { + it('should not emit a cached completed RemoteData', () => { testScheduler.run(({ cold, expectObservable }) => { + // Old cached value from 1 minute before the test started + const oldCachedSucceededData: RemoteData = Object.assign({}, remoteDataMocks.Success, { + timeCompleted: remoteDataTimestamp - 2 * 60 * 1000, + lastUpdated: remoteDataTimestamp - 2 * 60 * 1000, + } as RemoteData); spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { - a: remoteDataMocks.Success, + a: oldCachedSucceededData, b: remoteDataMocks.RequestPending, c: remoteDataMocks.ResponsePending, d: remoteDataMocks.Success, @@ -536,6 +563,22 @@ describe('BaseDataService', () => { }); }); + it('should emit the first completed RemoteData since the request was made', () => { + testScheduler.run(({ cold, expectObservable }) => { + spyOn(rdbService, 'buildList').and.returnValue(cold('a-b', { + a: remoteDataMocks.Success, + b: remoteDataMocks.SuccessStale, + })); + const expected = 'a-b'; + const values = { + a: remoteDataMocks.Success, + b: remoteDataMocks.SuccessStale, + }; + + expectObservable(service.findListByHref(selfLink, findListOptions, false, true, ...linksToFollow)).toBe(expected, values); + }); + }); + it(`should not emit a cached stale RemoteData, but only start emitting after the state first changes to RequestPending`, () => { testScheduler.run(({ cold, expectObservable }) => { spyOn(rdbService, 'buildList').and.returnValue(cold('a-b-c-d-e', { diff --git a/src/app/core/data/base/base-data.service.ts b/src/app/core/data/base/base-data.service.ts index edd6d9e2a4..18131dea63 100644 --- a/src/app/core/data/base/base-data.service.ts +++ b/src/app/core/data/base/base-data.service.ts @@ -266,6 +266,7 @@ export class BaseDataService implements HALDataServic map((href: string) => this.buildHrefFromFindOptions(href, {}, [], ...linksToFollow)), ); + const startTime: number = new Date().getTime(); this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable); return this.rdbService.buildSingle(requestHref$, ...linksToFollow).pipe( @@ -273,7 +274,7 @@ export class BaseDataService implements HALDataServic // call it isn't immediately returned, but we wait until the remote data for the new request // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a // cached completed object - skipWhile((rd: RemoteData) => useCachedVersionIfAvailable ? rd.isStale : rd.hasCompleted), + skipWhile((rd: RemoteData) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)), this.reRequestStaleRemoteData(reRequestOnStale, () => this.findByHref(href$, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); @@ -300,6 +301,7 @@ export class BaseDataService implements HALDataServic map((href: string) => this.buildHrefFromFindOptions(href, options, [], ...linksToFollow)), ); + const startTime: number = new Date().getTime(); this.createAndSendGetRequest(requestHref$, useCachedVersionIfAvailable); return this.rdbService.buildList(requestHref$, ...linksToFollow).pipe( @@ -307,7 +309,7 @@ export class BaseDataService implements HALDataServic // call it isn't immediately returned, but we wait until the remote data for the new request // is created. If useCachedVersionIfAvailable is false it also ensures you don't get a // cached completed object - skipWhile((rd: RemoteData>) => useCachedVersionIfAvailable ? rd.isStale : rd.hasCompleted), + skipWhile((rd: RemoteData>) => rd.isStale || (!useCachedVersionIfAvailable && rd.lastUpdated < startTime)), this.reRequestStaleRemoteData(reRequestOnStale, () => this.findListByHref(href$, options, useCachedVersionIfAvailable, reRequestOnStale, ...linksToFollow)), ); From 91acc957b13675ab8c1fdab141579b8794afd2d2 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 19 Nov 2024 18:14:12 +0100 Subject: [PATCH 2/3] 120109: Fixed "no elements in sequence" sometimes being thrown on the item bitstream & relationship tabs (cherry picked from commit decacec40437e891c672d1842598dfabd55f204a) --- .../abstract-item-update/abstract-item-update.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts index 80002f614b..3745b972d6 100644 --- a/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts +++ b/src/app/item-page/edit-item-page/abstract-item-update/abstract-item-update.component.ts @@ -6,7 +6,7 @@ import { ObjectUpdatesService } from '../../../core/data/object-updates/object-u import { ActivatedRoute, Router, Data } from '@angular/router'; import { NotificationsService } from '../../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; -import { first, map, switchMap, tap } from 'rxjs/operators'; +import { take, map, switchMap, tap } from 'rxjs/operators'; import { RemoteData } from '../../../core/data/remote-data'; import { AbstractTrackableComponent } from '../../../shared/trackable/abstract-trackable.component'; import { environment } from '../../../../environments/environment'; @@ -85,7 +85,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl if (this.url.indexOf('?') > 0) { this.url = this.url.substr(0, this.url.indexOf('?')); } - this.hasChanges().pipe(first()).subscribe((hasChanges) => { + this.hasChanges().pipe(take(1)).subscribe((hasChanges) => { if (!hasChanges) { this.initializeOriginalFields(); } else { @@ -170,7 +170,7 @@ export class AbstractItemUpdateComponent extends AbstractTrackableComponent impl */ private checkLastModified() { const currentVersion = this.item.lastModified; - this.objectUpdatesService.getLastModified(this.url).pipe(first()).subscribe( + this.objectUpdatesService.getLastModified(this.url).pipe(take(1)).subscribe( (updateVersion: Date) => { if (updateVersion.getDate() !== currentVersion.getDate()) { this.notificationsService.warning(this.getNotificationTitle('outdated'), this.getNotificationContent('outdated')); From ce17d23c08c2e5eef9f318c4095e95abbe626108 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 19 Nov 2024 18:45:16 +0100 Subject: [PATCH 3/3] 120109: Updated the route configuration to only resolve the dsoEditMenuResolver on pages who use the DsoEditMenuComponent (cherry picked from commit 5c9f494f7692ae7d664cf89d970c330528878abe) --- src/app/collection-page/collection-page-routing.module.ts | 4 +++- src/app/community-page/community-page-routing.module.ts | 4 +++- src/app/item-page/item-page-routing.module.ts | 7 ++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/app/collection-page/collection-page-routing.module.ts b/src/app/collection-page/collection-page-routing.module.ts index 9dc25b778e..6d07c467e6 100644 --- a/src/app/collection-page/collection-page-routing.module.ts +++ b/src/app/collection-page/collection-page-routing.module.ts @@ -36,7 +36,6 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; resolve: { dso: CollectionPageResolver, breadcrumb: CollectionBreadcrumbResolver, - menu: DSOEditMenuResolver }, runGuardsAndResolvers: 'always', children: [ @@ -66,6 +65,9 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; path: '', component: ThemedCollectionPageComponent, pathMatch: 'full', + resolve: { + menu: DSOEditMenuResolver, + }, } ], data: { diff --git a/src/app/community-page/community-page-routing.module.ts b/src/app/community-page/community-page-routing.module.ts index c37f8832f8..6412eb9f8e 100644 --- a/src/app/community-page/community-page-routing.module.ts +++ b/src/app/community-page/community-page-routing.module.ts @@ -29,7 +29,6 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; resolve: { dso: CommunityPageResolver, breadcrumb: CommunityBreadcrumbResolver, - menu: DSOEditMenuResolver }, runGuardsAndResolvers: 'always', children: [ @@ -49,6 +48,9 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; path: '', component: ThemedCommunityPageComponent, pathMatch: 'full', + resolve: { + menu: DSOEditMenuResolver, + }, } ], data: { diff --git a/src/app/item-page/item-page-routing.module.ts b/src/app/item-page/item-page-routing.module.ts index 0c855ab34d..5fb11f7056 100644 --- a/src/app/item-page/item-page-routing.module.ts +++ b/src/app/item-page/item-page-routing.module.ts @@ -28,7 +28,6 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; resolve: { dso: ItemPageResolver, breadcrumb: ItemBreadcrumbResolver, - menu: DSOEditMenuResolver }, runGuardsAndResolvers: 'always', children: [ @@ -36,10 +35,16 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver'; path: '', component: ThemedItemPageComponent, pathMatch: 'full', + resolve: { + menu: DSOEditMenuResolver, + }, }, { path: 'full', component: ThemedFullItemPageComponent, + resolve: { + menu: DSOEditMenuResolver, + }, }, { path: ITEM_EDIT_PATH,