Skip to content

Commit

Permalink
fix(client): send Presence initial values (when added) (microsoft#22515)
Browse files Browse the repository at this point in the history
Values were only being sent after an update or when broadcast refresh
was request (like another client joining).

Now initial values are sent when they are added to the system. A block
when using requestedContent (original schema) or individually when `add`
is called later.

In support of that `value` are no longer strictly required by the value
managers (`NotificationsManager` was always providing dummy values and
we never wanted to send those.)
  • Loading branch information
jason-ha authored Sep 14, 2024
1 parent 42ecec2 commit da0c9fa
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 36 deletions.
2 changes: 1 addition & 1 deletion packages/framework/presence/src/exposedInternalTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export namespace InternalTypes {
key: TKey,
datastoreHandle: StateDatastoreHandle<TKey, TValue>,
) => {
value: TValue;
value?: TValue;
manager: StateValue<TManager>;
};

Expand Down
9 changes: 6 additions & 3 deletions packages/framework/presence/src/internalTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import type { ClientSessionId, ISessionClient } from "./presence.js";
/**
* @internal
*/
export interface ClientRecord<TValue extends InternalTypes.ValueDirectoryOrState<unknown>> {
export interface ClientRecord<
TValue extends InternalTypes.ValueDirectoryOrState<unknown> | undefined,
> {
// Caution: any particular item may or may not exist
// Typescript does not support absent keys without forcing type to also be undefined.
// See https://github.com/microsoft/TypeScript/issues/42810.
[ClientSessionId: ClientSessionId]: TValue;
[ClientSessionId: ClientSessionId]: Exclude<TValue, undefined>;
}

/**
Expand All @@ -24,6 +26,7 @@ export interface ValueManager<
TValueState extends
InternalTypes.ValueDirectoryOrState<TValue> = InternalTypes.ValueDirectoryOrState<TValue>,
> {
get value(): TValueState;
// Most value managers should provide value - implement Required<ValueManager<...>>
readonly value?: TValueState;
update(client: ISessionClient, received: number, value: TValueState): void;
}
4 changes: 3 additions & 1 deletion packages/framework/presence/src/latestMapValueManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ class LatestMapValueManagerImpl<
T,
RegistrationKey extends string,
Keys extends string | number = string | number,
> implements LatestMapValueManager<T, Keys>, ValueManager<T, InternalTypes.MapValueState<T>>
> implements
LatestMapValueManager<T, Keys>,
Required<ValueManager<T, InternalTypes.MapValueState<T>>>
{
public readonly events = createEmitter<LatestMapValueManagerEvents<T, Keys>>();
public readonly controls: LatestValueControl;
Expand Down
4 changes: 3 additions & 1 deletion packages/framework/presence/src/latestValueManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ export interface LatestValueManager<T> {
}

class LatestValueManagerImpl<T, Key extends string>
implements LatestValueManager<T>, ValueManager<T, InternalTypes.ValueRequiredState<T>>
implements
LatestValueManager<T>,
Required<ValueManager<T, InternalTypes.ValueRequiredState<T>>>
{
public readonly events = createEmitter<LatestValueManagerEvents<T>>();
public readonly controls: LatestValueControl;
Expand Down
8 changes: 0 additions & 8 deletions packages/framework/presence/src/notificationsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ class NotificationsManagerImpl<
InternalTypes.ValueRequiredState<InternalTypes.NotificationType>
>,
_initialSubscriptions: NotificationSubscriptions<T>,
public readonly value: InternalTypes.ValueRequiredState<InternalTypes.NotificationType>,
) {}

public update(
Expand All @@ -195,19 +194,13 @@ export function Notifications<
InternalTypes.ValueRequiredState<InternalTypes.NotificationType>,
NotificationsManager<T>
> {
const value: InternalTypes.ValueRequiredState<InternalTypes.NotificationType> = {
rev: 0,
timestamp: Date.now(),
value: { name: "", args: [] },
};
return (
key: Key,
datastoreHandle: InternalTypes.StateDatastoreHandle<
Key,
InternalTypes.ValueRequiredState<InternalTypes.NotificationType>
>,
) => ({
value,
manager: brandIVM<
NotificationsManagerImpl<T, Key>,
InternalTypes.NotificationType,
Expand All @@ -217,7 +210,6 @@ export function Notifications<
key,
datastoreFromHandle(datastoreHandle),
initialSubscriptions,
value,
),
),
});
Expand Down
11 changes: 6 additions & 5 deletions packages/framework/presence/src/presenceDatastoreManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,21 @@ export class PresenceDatastoreManagerImpl implements PresenceDatastoreManager {
}

const localUpdate = (
stateKey: string,
value: ClientUpdateEntry,
states: { [key: string]: ClientUpdateEntry },
forceBroadcast: boolean,
): void => {
// Check for connectivity before sending updates.
if (this.runtime.clientId === undefined) {
return;
}

const updates: GeneralDatastoreMessageContent[string] = {};
for (const [key, value] of Object.entries(states)) {
updates[key] = { [this.clientSessionId]: value };
}
this.localUpdate(
{
[internalWorkspaceAddress]: {
[stateKey]: { [this.clientSessionId]: value },
},
[internalWorkspaceAddress]: updates,
},
forceBroadcast,
);
Expand Down
40 changes: 27 additions & 13 deletions packages/framework/presence/src/presenceStates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export type MapSchemaElement<
export interface PresenceRuntime {
readonly clientSessionId: ClientSessionId;
lookupClient(clientId: ClientConnectionId): ISessionClient;
localUpdate(stateKey: string, value: ClientUpdateEntry, forceBroadcast: boolean): void;
localUpdate(states: { [key: string]: ClientUpdateEntry }, forceBroadcast: boolean): void;
}

type PresenceSubSchemaFromWorkspaceSchema<
Expand Down Expand Up @@ -218,21 +218,32 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
// Prepare initial map content from initial state
{
const clientSessionId = this.runtime.clientSessionId;
let anyInitialValues = false;
// eslint-disable-next-line unicorn/no-array-reduce
const initial = Object.entries(initialContent).reduce(
(acc, [key, nodeFactory]) => {
const newNodeData = nodeFactory(key, handleFromDatastore(this));
acc.nodes[key as keyof TSchema] = newNodeData.manager;
acc.datastore[key] = acc.datastore[key] ?? {};
acc.datastore[key][clientSessionId] = newNodeData.value;
if ("value" in newNodeData) {
acc.datastore[key] = acc.datastore[key] ?? {};
acc.datastore[key][clientSessionId] = newNodeData.value;
acc.newValues[key] = newNodeData.value;
anyInitialValues = true;
}
return acc;
},
{
nodes: {} as unknown as MapEntries<TSchema>,
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
nodes: {} as MapEntries<TSchema>,
datastore,
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions
newValues: {} as { [key: string]: InternalTypes.ValueDirectoryOrState<unknown> },
},
);
this.nodes = initial.nodes;
if (anyInitialValues) {
this.runtime.localUpdate(initial.newValues, false);
}
}
}

Expand All @@ -251,15 +262,15 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
public localUpdate<Key extends keyof TSchema & string>(
key: Key,
value: MapSchemaElement<TSchema, "value", Key> & ClientUpdateEntry,
_forceBroadcast: boolean,
forceBroadcast: boolean,
): void {
this.runtime.localUpdate(key, value, _forceBroadcast);
this.runtime.localUpdate({ [key]: value }, forceBroadcast);
}

public update<Key extends keyof TSchema & string>(
key: Key,
clientId: ClientSessionId,
value: MapSchemaElement<TSchema, "value", Key>,
value: Exclude<MapSchemaElement<TSchema, "value", Key>, undefined>,
): void {
const allKnownState = this.datastore[key];
allKnownState[clientId] = mergeValueDirectory(allKnownState[clientId], value, 0);
Expand All @@ -282,13 +293,16 @@ class PresenceStatesImpl<TSchema extends PresenceStatesSchema>
assert(!(key in this.nodes), "Already have entry for key in map");
const nodeData = nodeFactory(key, handleFromDatastore(this));
this.nodes[key] = nodeData.manager;
if (key in this.datastore) {
// Already have received state from other clients. Kept in `all`.
// TODO: Send current `all` state to state manager.
} else {
this.datastore[key] = {};
if ("value" in nodeData) {
if (key in this.datastore) {
// Already have received state from other clients. Kept in `all`.
// TODO: Send current `all` state to state manager.
} else {
this.datastore[key] = {};
}
this.datastore[key][this.runtime.clientSessionId] = nodeData.value;
this.runtime.localUpdate({ [key]: nodeData.value }, false);
}
this.datastore[key][this.runtime.clientSessionId] = nodeData.value;
}

public ensureContent<TSchemaAdditional extends PresenceStatesSchema>(
Expand Down
13 changes: 9 additions & 4 deletions packages/framework/presence/src/stateDatastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export interface StateDatastoreSchema {
*/
export interface StateDatastore<
TKey extends string,
TValue extends InternalTypes.ValueDirectoryOrState<any>,
TValue extends InternalTypes.ValueDirectoryOrState<any> | undefined,
> {
localUpdate(
key: TKey,
Expand All @@ -55,9 +55,14 @@ export function handleFromDatastore<
// TSchema as `unknown` still provides some type safety.
// TSchema extends StateDatastoreSchema,
TKey extends string /* & keyof TSchema */,
TValue extends InternalTypes.ValueDirectoryOrState<unknown>,
>(datastore: StateDatastore<TKey, TValue>): InternalTypes.StateDatastoreHandle<TKey, TValue> {
return datastore as unknown as InternalTypes.StateDatastoreHandle<TKey, TValue>;
TValue extends InternalTypes.ValueDirectoryOrState<unknown> | undefined,
>(
datastore: StateDatastore<TKey, TValue>,
): InternalTypes.StateDatastoreHandle<TKey, Exclude<TValue, undefined>> {
return datastore as unknown as InternalTypes.StateDatastoreHandle<
TKey,
Exclude<TValue, undefined>
>;
}

/**
Expand Down

0 comments on commit da0c9fa

Please sign in to comment.