Skip to content

Commit

Permalink
Add an AttributionKey type for local changes (#14465)
Browse files Browse the repository at this point in the history
## Description

Adds an `AttributionKey` type for local changes. This key should be used
while content is un-acked on an attached document.

The PR doesn't update attribution implementations in
SharedString/SharedCell to leverage this key, though #14455 demonstrates
how that might be done.
  • Loading branch information
Abe27342 authored Mar 9, 2023
1 parent 41b095a commit d27bfae
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 25 deletions.
8 changes: 7 additions & 1 deletion api-report/runtime-definitions.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface AttributionInfo {
}

// @alpha
export type AttributionKey = OpAttributionKey | DetachedAttributionKey;
export type AttributionKey = OpAttributionKey | DetachedAttributionKey | LocalAttributionKey;

// @public (undocumented)
export const blobCountPropertyName = "BlobCount";
Expand Down Expand Up @@ -426,6 +426,12 @@ export interface ITelemetryContext {
setMultiple(prefix: string, property: string, values: Record<string, TelemetryEventPropertyType>): void;
}

// @alpha
export interface LocalAttributionKey {
// (undocumented)
type: "local";
}

// @public
export type NamedFluidDataStoreRegistryEntries = Iterable<NamedFluidDataStoreRegistryEntry>;

Expand Down
6 changes: 5 additions & 1 deletion packages/dds/merge-tree/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@
"typescript": "~4.5.5"
},
"typeValidation": {
"broken": {}
"broken": {
"InterfaceDeclaration_SerializedAttributionCollection": {
"backCompat": false
}
}
}
}
2 changes: 2 additions & 0 deletions packages/dds/merge-tree/src/attributionCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ export function areEqualAttributionKeys(a: AttributionKey, b: AttributionKey): b
return a.seq === (b as OpAttributionKey).seq;
case "detached":
return a.id === (b as DetachedAttributionKey).id;
case "local":
return true;
default:
unreachableCase(a, "Unhandled AttributionKey type");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2267,6 +2267,7 @@ declare function get_current_InterfaceDeclaration_SerializedAttributionCollectio
declare function use_old_InterfaceDeclaration_SerializedAttributionCollection(
use: TypeOnly<old.SerializedAttributionCollection>);
use_old_InterfaceDeclaration_SerializedAttributionCollection(
// @ts-expect-error compatibility expected to be broken
get_current_InterfaceDeclaration_SerializedAttributionCollection());

/*
Expand Down
10 changes: 8 additions & 2 deletions packages/framework/attributor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,19 @@ function getAttributionInfo(
attributor: IRuntimeAttributor,
sharedString: SharedString,
pos: number,
): AttributionInfo {
): AttributionInfo | undefined {
const { segment, offset } = sharedString.getContainingSegment(pos);
if (!segment || !offset) {
throw new UsageError("Invalid pos");
}
const attributionKey: AttributionKey = segment.attribution.getAtOffset(offset);
return attributor.getAttributionInfo(attributionKey);
// BEWARE: DDSes may track attribution key with type "detached" and "local", which aren't yet
// supported out-of-the-box in IRuntimeAttributor. The application can recover AttributionInfo
// from these keys if it wants using user information about the creator of the document and the
// current active user, respectively.
if (attributor.has(attributionKey)) {
return attributor.get(attributionKey);
}
}

// Get the user who inserted the text at position 0 in `sharedString` and the timestamp for when they did so.
Expand Down
13 changes: 13 additions & 0 deletions packages/framework/attributor/src/mixinAttributor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,15 @@ class RuntimeAttributor implements IRuntimeAttributor {
throw new Error("Attribution of detached keys is not yet supported.");
}

if (key.type === "local") {
// Note: we can *almost* orchestrate this correctly with internal-only changes by looking up the current
// client id in the audience. However, for read->write client transition, the container might have not yet
// received a client id. This is left as a TODO as it might be more easily solved once the detached case
// is settled (e.g. if it's reasonable for the host to know the current user information at container
// creation time, we could just use that here as well).
throw new Error("Attribution of local keys is not yet supported.");
}

return this.opAttributor.getAttributionInfo(key.seq);
}

Expand All @@ -200,6 +209,10 @@ class RuntimeAttributor implements IRuntimeAttributor {
return false;
}

if (key.type === "local") {
return false;
}

return this.opAttributor?.tryGetAttributionInfo(key.seq) !== undefined;
}

Expand Down
6 changes: 5 additions & 1 deletion packages/runtime/runtime-definitions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
"typescript": "~4.5.5"
},
"typeValidation": {
"broken": {}
"broken": {
"TypeAliasDeclaration_AttributionKey": {
"backCompat": false
}
}
}
}
11 changes: 10 additions & 1 deletion packages/runtime/runtime-definitions/src/attribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,20 @@ export interface DetachedAttributionKey {
id: 0;
}

/**
* AttributionKey associated with content that has been made locally but not yet acked by the server.
*
* @alpha
*/
export interface LocalAttributionKey {
type: "local";
}

/**
* Can be indexed into the ContainerRuntime in order to retrieve {@link AttributionInfo}.
* @alpha
*/
export type AttributionKey = OpAttributionKey | DetachedAttributionKey;
export type AttributionKey = OpAttributionKey | DetachedAttributionKey | LocalAttributionKey;

/**
* Attribution information associated with a change.
Expand Down
1 change: 1 addition & 0 deletions packages/runtime/runtime-definitions/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export {
AttributionInfo,
AttributionKey,
DetachedAttributionKey,
LocalAttributionKey,
OpAttributionKey,
} from "./attribution";
export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ declare function get_current_TypeAliasDeclaration_AttributionKey():
declare function use_old_TypeAliasDeclaration_AttributionKey(
use: TypeOnly<old.AttributionKey>);
use_old_TypeAliasDeclaration_AttributionKey(
// @ts-expect-error compatibility expected to be broken
get_current_TypeAliasDeclaration_AttributionKey());

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function assertAttributionMatches(
sharedString: SharedString,
position: number,
attributor: IRuntimeAttributor,
expected: Partial<AttributionInfo> | "detached",
expected: Partial<AttributionInfo> | "detached" | "local" | undefined,
): void {
const { segment, offset } = sharedString.getContainingSegment(position);
assert(
Expand All @@ -48,24 +48,42 @@ function assertAttributionMatches(
);
const key = segment.attribution.getAtOffset(offset);

if (expected === "detached") {
assert.deepEqual(
key,
{ type: "detached", id: 0 },
"expected attribution key to be detached",
);
assert.equal(
attributor.has(key),
false,
"Expected RuntimeAttributor to not attribute detached key.",
);
} else {
const { timestamp, user } = attributor.get(key) ?? {};
if (expected.timestamp !== undefined) {
assert.equal(timestamp, expected.timestamp);
}
if (expected.user !== undefined) {
assert.deepEqual(user, expected.user);
switch (expected) {
case "detached":
assert.deepEqual(
key,
{ type: "detached", id: 0 },
"expected attribution key to be detached",
);
assert.equal(
attributor.has(key),
false,
"Expected RuntimeAttributor to not attribute detached key.",
);
break;
case "local":
assert.deepEqual(key, { type: "local" });
assert.equal(
attributor.has(key),
false,
"Expected RuntimeAttributor to not attribute local key.",
);
break;
case undefined:
assert.deepEqual(key, expected);
break;
default: {
if (key === undefined) {
assert.fail("Expected a defined key, but got an undefined one");
}
const { timestamp, user } = attributor.get(key) ?? {};
if (expected.timestamp !== undefined) {
assert.equal(timestamp, expected.timestamp);
}
if (expected.user !== undefined) {
assert.deepEqual(user, expected.user);
}
break;
}
}
}
Expand Down

0 comments on commit d27bfae

Please sign in to comment.