From bdd7a683a37f042f05a80a1115fedb34f6319034 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Wed, 1 Nov 2023 16:44:56 +0900 Subject: [PATCH] BREAKING `data.model` is synced with Apollo cache #9844 `NaturalAbstractModelService.resolve()` now resolves in two steps. First it asks Apollo to fetch the object from network to force refresh Apollo cache. Then only, the resolving is considered finished, and we emit an observable that watches Apollo cache for the model. So the component can receive the observable from the route, subscribe to it and immediately receive an up-to-date model. And all subsequent updates to the model will be emitted from Apollo cache too. But the real breaking part is that previously the route resolved data shape: ```ts { permission: myPermissions, action: { model: myAction, statuses: myStatuses, priorities: myPriorities, } } ``` Now, because `NaturalAbstractModelService.resolve()` only resolve the model, it becomes the simpler: ```ts { permission: myPermissions, model: myActionObservable, statuses: myStatuses, priorities: myPriorities, } ``` So all overrides of `resolve()` must be migrate outside the model service. Typically, something like: ```ts export const actionResolvers: ResolveData = { action: resolveAction, statuses: () => inject(NaturalEnumService).get('ActionStatus'), priorities: () => inject(NaturalEnumService).get('Priority'), } ``` --- .../src/lib/classes/abstract-detail.ts | 9 - projects/natural/src/lib/classes/rxjs.spec.ts | 259 +----------------- projects/natural/src/lib/classes/rxjs.ts | 140 +--------- .../lib/services/abstract-model.service.ts | 14 +- .../src/lib/services/debounce.service.spec.ts | 46 +++- .../natural/src/lib/testing/item.resolver.ts | 26 +- .../natural/src/lib/testing/item.service.ts | 2 +- projects/natural/src/public-api.ts | 2 +- src/app/detail/detail.component.html | 1 - 9 files changed, 60 insertions(+), 439 deletions(-) diff --git a/projects/natural/src/lib/classes/abstract-detail.ts b/projects/natural/src/lib/classes/abstract-detail.ts index 21a29b55..e938d9c9 100644 --- a/projects/natural/src/lib/classes/abstract-detail.ts +++ b/projects/natural/src/lib/classes/abstract-detail.ts @@ -93,7 +93,6 @@ export class NaturalAbstractDetail< } // Subscribe to model to know when Apollo cache is changed, so we can reflect it into `data.model` - let firstTime = true; this.#modelSub?.unsubscribe(); this.#modelSub = incomingData.model .pipe(takeUntil(this.ngUnsubscribe)) @@ -102,16 +101,8 @@ export class NaturalAbstractDetail< ...incomingData, model: model, } as Data; - // Initialize the form exactly once, and never again when the model is updated in Apollo cache - if (firstTime) { - firstTime = false; - console.log('firstTime', this.data.model); - } }); - console.log('initForm', this.data.model); this.initForm(); - - console.log('FINISHED'); }); } else { this.initForm(); diff --git a/projects/natural/src/lib/classes/rxjs.spec.ts b/projects/natural/src/lib/classes/rxjs.spec.ts index daf6039d..7c45e93a 100644 --- a/projects/natural/src/lib/classes/rxjs.spec.ts +++ b/projects/natural/src/lib/classes/rxjs.spec.ts @@ -1,5 +1,5 @@ -import {cancellableTimeout, debug, emptyResult, spyObservable, waitUntilFirstEmission} from './rxjs'; -import {first, Observable, of, ReplaySubject, Subject} from 'rxjs'; +import {cancellableTimeout} from './rxjs'; +import {ReplaySubject, Subject} from 'rxjs'; import {fakeAsync, tick} from '@angular/core/testing'; describe('cancellableTimeout', () => { @@ -65,258 +65,3 @@ describe('cancellableTimeout', () => { expect(completed).toBe(true); })); }); - -describe('waitUntilFirstEmission', () => { - let source$: Observable; - let sourceSubject: Subject<1 | 2 | 3>; - - beforeEach(() => { - console.error('______________________________'); - sourceSubject = new Subject<1 | 2 | 3>(); - source$ = sourceSubject.pipe(); // For debugging it's convenient to drop `debug('source)` in there - }); - - it('observable is cold, nothing happen without subscription', () => { - const source = spyObservable(source$); - waitUntilFirstEmission(source.observable); - - sourceSubject.next(1); - sourceSubject.next(2); - sourceSubject.next(3); - - expect(source.result).toEqual(emptyResult); - }); - - it('first subscriber will receive exactly 1 emission and be completed automatically, but source is still subscribed', () => { - const source = spyObservable(source$); - const obs = waitUntilFirstEmission(source.observable); - const firstSubscriber = spyObservable(obs); - - firstSubscriber.observable.subscribe(); - - sourceSubject.next(1); - sourceSubject.next(2); - sourceSubject.next(3); - - expect(source.result).toEqual({ - called: 3, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - - expect(firstSubscriber.result).toEqual({ - called: 1, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - }); - - fit('after first subscriber completes, second subscriber will receive first emission (replayed) and all others', done => { - // of(1,2,3,4).pipe(debug('QQQQQQQQQQQ'), first(),debug('QQQ'),).subscribe(); - const source = spyObservable(source$.pipe(debug('source'))); - const obs = waitUntilFirstEmission(source.observable.pipe(debug('resolver'), first())); - const firstSubscriber = spyObservable(obs); - - firstSubscriber.observable.subscribe({ - next: model => { - const secondSubscriber = spyObservable(model); - secondSubscriber.observable.pipe(debug('component')).subscribe({ - next: value => { - switch (secondSubscriber.result.called) { - case 1: - expect(value).toBe(1); - break; - case 2: - expect(value).toBe(2); - sourceSubject.next(3); - break; - case 3: - expect(value).toBe(3); - sourceSubject.complete(); - break; - default: - throw new Error(`should not be called ${secondSubscriber.result.called} times`); - } - }, - complete: () => { - expect(source.result) - .withContext('when second subscriber is completed, that means our source was completed') - .toEqual({ - called: 3, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - - expect(secondSubscriber.result) - .withContext('second subscriber is completed and it received all emission from source') - .toEqual({ - called: 3, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - - done(); - }, - }); - - expect(secondSubscriber.result) - .withContext('second subscriber immediately receives the first emission as a replay') - .toEqual({ - called: 1, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - }, - }); - - sourceSubject.next(1); - sourceSubject.next(2); - - expect(firstSubscriber.result).toEqual({ - called: 1, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - }); - - it('idem but with `of()`', done => { - const source = spyObservable(of(1, 2, 3).pipe(debug('source'))); - const obs = waitUntilFirstEmission(source.observable.pipe(debug('resolver'), first())); - const firstSubscriber = spyObservable(obs); - - firstSubscriber.observable.subscribe({ - next: model => { - const secondSubscriber = spyObservable(model); - secondSubscriber.observable.pipe(debug('component')).subscribe({ - next: value => { - switch (secondSubscriber.result.called) { - case 1: - expect(value).toBe(1); - break; - default: - throw new Error(`should not be called ${secondSubscriber.result.called} times`); - } - }, - complete: () => { - expect(source.result) - .withContext('when second subscriber is completed, that means our source was completed') - .toEqual({ - called: 1, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - - expect(secondSubscriber.result) - .withContext('second subscriber is completed and it received all emission from source') - .toEqual({ - called: 1, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - console.log('SUCESS'); - done(); - }, - }); - - expect(secondSubscriber.result) - .withContext('second subscriber immediately receives the first emission as a replay') - .toEqual({ - called: 1, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - }, - }); - - expect(firstSubscriber.result).toEqual({ - called: 1, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - }); - - it('if first subscriber unsubscribes (before first emission), source is unsubscribed', () => { - const source = spyObservable(source$); - const obs = waitUntilFirstEmission(source.observable); - const firstSubscriber = spyObservable(obs); - - const subscription = firstSubscriber.observable.subscribe(); - subscription.unsubscribe(); - - expect(source.result).toEqual({ - called: 0, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 1, - }); - - expect(firstSubscriber.result).toEqual({ - called: 0, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 1, - }); - }); - - it('if second subscriber unsubscribes, source is unsubscribed', fakeAsync(() => { - const source = spyObservable(source$); - const obs = waitUntilFirstEmission(source.observable); - const firstSubscriber = spyObservable(obs); - - firstSubscriber.observable.subscribe({ - next: model => { - const secondSubscriber = spyObservable(model); - const subscription = secondSubscriber.observable.subscribe(); - subscription.unsubscribe(); - - expect(secondSubscriber.result).toEqual({ - called: 1, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 1, - }); - }, - }); - - sourceSubject.next(1); - - expect(source.result).toEqual({ - called: 1, - completed: 0, - errored: 0, - subscribed: 1, - unsubscribed: 1, - }); - - expect(firstSubscriber.result).toEqual({ - called: 1, - completed: 1, - errored: 0, - subscribed: 1, - unsubscribed: 0, - }); - })); -}); diff --git a/projects/natural/src/lib/classes/rxjs.ts b/projects/natural/src/lib/classes/rxjs.ts index 1796ba96..adbc0cdc 100644 --- a/projects/natural/src/lib/classes/rxjs.ts +++ b/projects/natural/src/lib/classes/rxjs.ts @@ -1,4 +1,4 @@ -import {map, MonoTypeOperatorFunction, Observable, Subscriber, Subscription, take, takeUntil, tap, timer} from 'rxjs'; +import {map, MonoTypeOperatorFunction, Observable, take, takeUntil, tap, timer} from 'rxjs'; /** * Behave like setTimeout(), but with a mandatory cancel mechanism. @@ -51,141 +51,3 @@ export function debug(debugName: string): MonoTypeOperatorFunction { complete: () => console.log('COMPLETE', debugName), }); } - -type SpyResult = { - called: number; - completed: number; - errored: number; - subscribed: number; - unsubscribed: number; -}; - -type ObservableSpy = { - result: SpyResult; - observable: Observable; -}; - -export const emptyResult: Readonly = { - called: 0, - completed: 0, - errored: 0, - subscribed: 0, - unsubscribed: 0, -} as const; - -export function spyObservable(observable: Observable): ObservableSpy { - const result = { - called: 0, - completed: 0, - errored: 0, - subscribed: 0, - unsubscribed: 0, - }; - return { - result: result, - observable: observable.pipe( - tap({ - next: () => result.called++, - complete: () => result.completed++, - error: () => result.errored++, - subscribe: () => result.subscribed++, - unsubscribe: () => result.unsubscribed++, - }), - ), - }; -} - -/** - * This takes a source observable and will return a higher order observable that will emit when the source observable - * emitted for the first time. The value emitted is an observable that forward emissions from the source observable, - * including the original first emission. - * - * The first subscriber will automatically complete after the first emission. All other subsequent subscribers will - * receive the same first value (replayed), and all subsequent values. And they must unsubscribe. - */ -export function waitUntilFirstEmission(source: Observable): Observable> { - let sourceSubscription: Subscription | null = null; - let lastResult: {value: T} | null = null; - const subscribers = new Map, boolean>(); - let isFirstSubscription = true; - let firstSubscriptionIsNexting = false; - let subscriptionCountEver = 0; - - const result = new Observable(subscriber => { - subscriptionCountEver++; - subscribers.set(subscriber, isFirstSubscription); - isFirstSubscription = false; - - if (lastResult) { - console.log('will replay', lastResult); - subscriber.next(lastResult.value); - } - - if (subscribers.get(subscriber)) { - sourceSubscription = source.subscribe({ - next: value => { - lastResult = {value}; - - for (const s of Array.from(subscribers.keys())) { - const isFirst = subscribers.get(s); - if (isFirst) { - firstSubscriptionIsNexting = true; - } - - s.next(value); - - // The first subscriber will complete after the first emission - if (subscribers.get(s)) { - s.complete(); - } - - if (isFirst) { - firstSubscriptionIsNexting = false; - } - } - }, - error: error => { - for (const s of subscribers.keys()) { - s.error(error); - } - }, - complete: () => { - console.log('forward completing'); - console.log('forward completing'); - for (const s of subscribers.keys()) { - s.complete(); - } - }, - }); - } - - return () => { - const wasFirstSubscription = subscribers.get(subscriber); - subscribers.delete(subscriber); - /** - * If we were the last subscriber, then we might unsubscribe the source, if one of the following is true: - * - * - this is the second (or more) subscriber (not the first one which is special) - * - this is the first subscriber, and I have been unsubscribed by outside (typically by Angular cancelled navigation), not by the auto-completion mechanism - * - in the entire lifetime of this observable there's been at least two subscribers - */ - console.log('maybe unsubscribe ?'); - if ( - subscribers.size === 0 && - (!wasFirstSubscription || !firstSubscriptionIsNexting || subscriptionCountEver > 1) - ) { - console.warn('will unsubscribe', { - wasFirstSubscription, - firstSubscriptionIsNexting, - subscriptionCountEver, - }); - sourceSubscription?.unsubscribe(); - sourceSubscription = null; - isFirstSubscription = false; - // lastResult = null; - } - }; - }); - - return result.pipe(map(() => result)); -} diff --git a/projects/natural/src/lib/services/abstract-model.service.ts b/projects/natural/src/lib/services/abstract-model.service.ts index 28a09043..d1e78ce2 100644 --- a/projects/natural/src/lib/services/abstract-model.service.ts +++ b/projects/natural/src/lib/services/abstract-model.service.ts @@ -11,7 +11,6 @@ import {makePlural, mergeOverrideArray, relationsToIds, upperCaseFirstLetter} fr import {PaginatedData} from '../classes/data-source'; import {NaturalDebounceService} from './debounce.service'; import {ApolloQueryResult} from '@apollo/client/core/types'; -import {debug} from '../classes/rxjs'; export interface FormValidators { [key: string]: ValidatorFn[]; @@ -469,15 +468,18 @@ export abstract class NaturalAbstractModelService< } /** - * Resolve model, if the id is provided, or else return default values, in order to show a form + * If the id is provided, resolves an observable model. The observable model will only be emitted after we are sure + * that Apollo cache is fresh and warm. Then the component can subscribe to the observable model to get the model + * immediately from Apollo cache and any subsequents future mutations that may happen to Apollo cache. + * + * Without id, returns default values, in order to show a creation form. */ - public resolve(id: string): Observable> { + public resolve(id: string | undefined): Observable> { if (id) { - const onlyNetwork = this.watchOne('123', 'network-only').pipe(first(), debug('ZZZresolver')); - const onlyCache = this.watchOne('123', 'cache-first').pipe(debug('ZZZcomponent')); + const onlyNetwork = this.watchOne(id, 'network-only').pipe(first()); + const onlyCache = this.watchOne(id, 'cache-first'); return onlyNetwork.pipe(map(() => onlyCache)); - // return this.getOne(id); } else { return of(of(this.getDefaultForServer() as Tone)); } diff --git a/projects/natural/src/lib/services/debounce.service.spec.ts b/projects/natural/src/lib/services/debounce.service.spec.ts index ff3ca6d8..b65349fd 100644 --- a/projects/natural/src/lib/services/debounce.service.spec.ts +++ b/projects/natural/src/lib/services/debounce.service.spec.ts @@ -1,8 +1,50 @@ import {fakeAsync, TestBed, tick} from '@angular/core/testing'; import {TestScheduler} from 'rxjs/testing'; -import {of, throwError} from 'rxjs'; +import {Observable, of, tap, throwError} from 'rxjs'; import {NaturalDebounceService} from './debounce.service'; -import {emptyResult, spyObservable} from '../classes/rxjs'; + +type SpyResult = { + called: number; + completed: number; + errored: number; + subscribed: number; + unsubscribed: number; +}; + +type ObservableSpy = { + result: SpyResult; + observable: Observable; +}; + +const emptyResult: Readonly = { + called: 0, + completed: 0, + errored: 0, + subscribed: 0, + unsubscribed: 0, +} as const; + +function spyObservable(observable: Observable): ObservableSpy { + const result = { + called: 0, + completed: 0, + errored: 0, + subscribed: 0, + unsubscribed: 0, + }; + return { + result: result, + observable: observable.pipe( + tap({ + next: () => result.called++, + complete: () => result.completed++, + error: () => result.errored++, + subscribe: () => result.subscribed++, + unsubscribe: () => result.unsubscribed++, + }), + ), + }; +} describe('NaturalDebounceService', () => { let scheduler: TestScheduler; diff --git a/projects/natural/src/lib/testing/item.resolver.ts b/projects/natural/src/lib/testing/item.resolver.ts index 95ab582d..2f2cda97 100644 --- a/projects/natural/src/lib/testing/item.resolver.ts +++ b/projects/natural/src/lib/testing/item.resolver.ts @@ -1,33 +1,13 @@ -import {concat, first, map, Observable, of, Subject} from 'rxjs'; +import {Observable} from 'rxjs'; import {Item, ItemInput, ItemService} from './item.service'; import {inject} from '@angular/core'; -import {debug, waitUntilFirstEmission} from '@ecodev/natural'; /** * Resolve Item data for router and panels service */ -export function resolveItemSyncrhonous(): Observable> { +export function resolveItem(): Observable> { const itemService = inject(ItemService); console.warn('resolve Item'); - return itemService.resolve('1'); - const onlyNetwork = itemService.watchOne('123', 'network-only').pipe(first()); - const onlyCache = itemService.watchOne('123', 'cache-first'); - - return onlyNetwork.pipe(map(() => onlyCache)); - return waitUntilFirstEmission(concat(of(1 as any, 2 as any, 3 as any)).pipe(debug('source'))); + return itemService.resolve(''); } - -export function resolveDelayedItem(): Observable> { - const itemService = inject(ItemService); - console.warn('resolve delayed Item'); - - let apollo = new Subject(); - setTimeout(() => apollo.next(itemService.getItem()), 2000); - setTimeout(() => apollo.next(itemService.getItem(true)), 4000); - const source = apollo.pipe(debug('source')); - - return waitUntilFirstEmission(source); -} - -export const resolveItem = resolveItemSyncrhonous; diff --git a/projects/natural/src/lib/testing/item.service.ts b/projects/natural/src/lib/testing/item.service.ts index 1e808038..0500da90 100644 --- a/projects/natural/src/lib/testing/item.service.ts +++ b/projects/natural/src/lib/testing/item.service.ts @@ -103,7 +103,7 @@ export class ItemService extends NaturalAbstractModelService< } public override getOne(id: string): Observable { - return of(); + return of(this.getItem(true, 2, id)); } public override watchOne(id: string, fetchPolicy: WatchQueryFetchPolicy = 'cache-and-network'): Observable { diff --git a/projects/natural/src/public-api.ts b/projects/natural/src/public-api.ts index ada6c772..d3c146bb 100644 --- a/projects/natural/src/public-api.ts +++ b/projects/natural/src/public-api.ts @@ -13,7 +13,7 @@ export * from './lib/classes/abstract-navigable-list'; export {createHttpLink} from './lib/classes/apollo-utils'; export * from './lib/classes/data-source'; export * from './lib/classes/query-variable-manager'; -export {cancellableTimeout, debug, waitUntilFirstEmission} from './lib/classes/rxjs'; +export {cancellableTimeout, debug} from './lib/classes/rxjs'; export * from './lib/classes/utility'; export * from './lib/classes/validators'; export {validTlds} from './lib/classes/tld'; diff --git a/src/app/detail/detail.component.html b/src/app/detail/detail.component.html index e1172df6..bfaa0686 100644 --- a/src/app/detail/detail.component.html +++ b/src/app/detail/detail.component.html @@ -1,6 +1,5 @@

NaturalAbstractDetail

- 
{{ data | json }}