guard against updates emitted while populating during first subscription

This came up now because Timeline uses a MappedList to map PendingEvents
to PendingEventEntries. In the map function, we setup links between
entries to support local echo for relations. When opening a timeline
that has unsent relations, the initial populating of the MappedList
will try to emit an update for the target entry in remoteEntries.
This all happens while the ListView of the timeline is calling subscribe
and all collections in the chain are populating themselves based on
their sources.

This usually entails calling subscribe on the source,
and now you are subscribed, iterate over the source (as you're not
allowed to query an unsubscribed observable collection, as it might not
be populated yet, and even if it did, it wouldn't be guaranteed to be
up to date as events aren't flowing yet).

So in this concrete example, TilesCollection hadn't populated its tiles
yet and when the update to the target of the unsent relation reached
TilesCollection, the tiles array was still null and it crashed.

I thought what would be the best way to fix this and have a solid model
for observable collections to ensure they are always compatible with
each other. I considered splitting up the subscription process in two
steps where you'd first populate the source and then explicitly start
events flowing.

I didn't go with this way because it's really only updates that
make sense to be emitted during setup.
A missed update wouldn't usually bring the collections out of sync
like a missed add or remove would. It would just mean the UI isn't
updated (or any subsequent filtered collections are not updated),
but this should be fine to ignore during setup, as you can rely
on the subscribing collections down the chain picking up the update
while populating. If we ever want to support add or remove events
during setup, we would have to explicitly support them, but for now
they are correct to throw.

So for now, just ignore update events that happen during setup
where needed.
This commit is contained in:
Bruno Windels 2021-05-27 10:02:05 +02:00
parent a8e43d4850
commit c6e2607f1f
7 changed files with 30 additions and 4 deletions

View file

@ -143,6 +143,10 @@ export class TilesCollection extends BaseObservableList {
} }
onUpdate(index, entry, params) { onUpdate(index, entry, params) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
if (!this._tiles) {
return;
}
const tileIdx = this._findTileIdx(entry); const tileIdx = this._findTileIdx(entry);
const tile = this._findTileAtIdx(entry, tileIdx); const tile = this._findTileAtIdx(entry, tileIdx);
if (tile) { if (tile) {

View file

@ -33,10 +33,7 @@ export class ConcatList extends BaseObservableList {
} }
onSubscribeFirst() { onSubscribeFirst() {
this._sourceUnsubscribes = []; this._sourceUnsubscribes = this._sourceLists.map(sourceList => sourceList.subscribe(this));
for (const sourceList of this._sourceLists) {
this._sourceUnsubscribes.push(sourceList.subscribe(this));
}
} }
onUnsubscribeLast() { onUnsubscribeLast() {
@ -62,6 +59,11 @@ export class ConcatList extends BaseObservableList {
} }
onUpdate(index, value, params, sourceList) { onUpdate(index, value, params, sourceList) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
// as we are not supposed to call `length` on any uninitialized list
if (!this._sourceUnsubscribes) {
return;
}
this.emitUpdate(this._offsetForSource(sourceList) + index, value, params); this.emitUpdate(this._offsetForSource(sourceList) + index, value, params);
} }

View file

@ -48,6 +48,10 @@ export class MappedList extends BaseObservableList {
} }
onUpdate(index, value, params) { onUpdate(index, value, params) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
if (!this._mappedValues) {
return;
}
const mappedValue = this._mappedValues[index]; const mappedValue = this._mappedValues[index];
if (this._updater) { if (this._updater) {
this._updater(mappedValue, params, value); this._updater(mappedValue, params, value);

View file

@ -70,6 +70,10 @@ export class SortedMapList extends BaseObservableList {
} }
onUpdate(key, value, params) { onUpdate(key, value, params) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
if (!this._sortedPairs) {
return;
}
// TODO: suboptimal for performance, see above for idea with BST to speed this up if we need to // TODO: suboptimal for performance, see above for idea with BST to speed this up if we need to
const oldIdx = this._sortedPairs.findIndex(p => p.key === key); const oldIdx = this._sortedPairs.findIndex(p => p.key === key);
// neccesary to remove pair from array before // neccesary to remove pair from array before

View file

@ -82,6 +82,10 @@ export class FilteredMap extends BaseObservableMap {
} }
onUpdate(key, value, params) { onUpdate(key, value, params) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
if (!this._included) {
return;
}
if (this._filter) { if (this._filter) {
const wasIncluded = this._included.get(key); const wasIncluded = this._included.get(key);
const isIncluded = this._filter(value, key); const isIncluded = this._filter(value, key);

View file

@ -48,6 +48,10 @@ export class JoinedMap extends BaseObservableMap {
} }
onUpdate(source, key, value, params) { onUpdate(source, key, value, params) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
if (!this._subscriptions) {
return;
}
if (!this._isKeyAtSourceOccluded(source, key)) { if (!this._isKeyAtSourceOccluded(source, key)) {
this.emitUpdate(key, value, params); this.emitUpdate(key, value, params);
} }

View file

@ -49,6 +49,10 @@ export class MappedMap extends BaseObservableMap {
} }
onUpdate(key, value, params) { onUpdate(key, value, params) {
// if an update is emitted while calling source.subscribe() from onSubscribeFirst, ignore it
if (!this._mappedValues) {
return;
}
const mappedValue = this._mappedValues.get(key); const mappedValue = this._mappedValues.get(key);
if (mappedValue !== undefined) { if (mappedValue !== undefined) {
// TODO: map params somehow if needed? // TODO: map params somehow if needed?