Skip to content

Commit

Permalink
Ps/avoid state emit until updated (#7124)
Browse files Browse the repository at this point in the history
* Add a small default time to limit timing failures

* Handle subscription race conditions

* Add Symbols to tracked emission types

This is a bit of a cheat, but Symbols can't be cloned, so
we need to nudge them to something we can handle.
They are rare enough that anyone hitting this is likely to
expect some special handling.

* Ref count state listeners to minimize storage activity

* Ensure statuses are updated

* Remove notes

* Use `test` when gramatically more proper

* Copy race and subscription improvements to single user

* Simplify observer initialization

* Correct parameter names

* Simplify update promises

test we don't accidentally deadlock along the `getFromState` path

* Fix save mock

* WIP: most tests working

* Avoid infinite update loop

* Avoid potential deadlocks with awaiting assigned promises

We were awaiting a promise assigned in a thenable. It turns out that
assignment occurs before all thenables are concatenated, which can cause
deadlocks. Likely, these were not showing up in tests because we're
using very quick memory storage.

* Fix update deadlock test

* Add user update tests

* Assert no double emit for multiple observers

* Add use intent to method name

* Ensure new subscriptions receive only newest data

TODO: is this worth doing for active user state?

* Remove unnecessary design requirement

We don't need to await an executing update promise, we
can support two emissions as long as the observable is
guaranteed to get the new data.

* Cleanup await spam

* test cleanup option behavior

* Remove unnecessary typecast

* Throw over coerce for definition options
  • Loading branch information
MGibson1 authored Dec 12, 2023
1 parent 4d05b00 commit 38c335d
Show file tree
Hide file tree
Showing 10 changed files with 1,127 additions and 206 deletions.
2 changes: 1 addition & 1 deletion libs/common/spec/fake-storage.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class FakeStorageService implements AbstractStorageService {
return Promise.resolve(this.store[key] != null);
}
save<T>(key: string, obj: T, options?: StorageOptions): Promise<void> {
this.mock.save(key, options);
this.mock.save(key, obj, options);
this.store[key] = obj;
this.updatesSubject.next({ key: key, updateType: "save" });
return Promise.resolve();
Expand Down
6 changes: 5 additions & 1 deletion libs/common/spec/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ export function trackEmissions<T>(observable: Observable<T>): T[] {
case "boolean":
emissions.push(value);
break;
case "symbol":
// Cheating types to make symbols work at all
emissions.push(value.toString() as T);
break;
default: {
emissions.push(clone(value));
}
Expand All @@ -85,7 +89,7 @@ function clone(value: any): any {
}
}

export async function awaitAsync(ms = 0) {
export async function awaitAsync(ms = 1) {
if (ms < 1) {
await Promise.resolve();
} else {
Expand Down
Loading

0 comments on commit 38c335d

Please sign in to comment.