Skip to content

Commit

Permalink
Support concurrent formatting of Text (#642)
Browse files Browse the repository at this point in the history
Currently, we are unable to check for concurrent cases when applying
the Text.setStyle operation. This pull request introduces a map called
latestCreatedAtMapByActor to track the causality between the
operations of the two clients and ensures that the results converge
into one.
  • Loading branch information
MoonGyu1 authored Sep 13, 2023
1 parent 37c332f commit 1594345
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 13 deletions.
9 changes: 9 additions & 0 deletions src/api/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,10 @@ function toOperation(operation: Operation): PbOperation {
);
pbStyleOperation.setFrom(toTextNodePos(styleOperation.getFromPos()));
pbStyleOperation.setTo(toTextNodePos(styleOperation.getToPos()));
const pbCreatedAtMapByActor = pbStyleOperation.getCreatedAtMapByActorMap();
for (const [key, value] of styleOperation.getMaxCreatedAtMapByActor()) {
pbCreatedAtMapByActor.set(key, toTimeTicket(value)!);
}
const pbAttributes = pbStyleOperation.getAttributesMap();
for (const [key, value] of styleOperation.getAttributes()) {
pbAttributes.set(key, value);
Expand Down Expand Up @@ -1073,6 +1077,10 @@ function fromOperations(pbOperations: Array<PbOperation>): Array<Operation> {
);
} else if (pbOperation.hasStyle()) {
const pbStyleOperation = pbOperation.getStyle();
const createdAtMapByActor = new Map();
pbStyleOperation!.getCreatedAtMapByActorMap().forEach((value, key) => {
createdAtMapByActor.set(key, fromTimeTicket(value));
});
const attributes = new Map();
pbStyleOperation!.getAttributesMap().forEach((value, key) => {
attributes.set(key, value);
Expand All @@ -1081,6 +1089,7 @@ function fromOperations(pbOperations: Array<PbOperation>): Array<Operation> {
fromTimeTicket(pbStyleOperation!.getParentCreatedAt())!,
fromTextNodePos(pbStyleOperation!.getFrom()!),
fromTextNodePos(pbStyleOperation!.getTo()!),
createdAtMapByActor,
attributes,
fromTimeTicket(pbStyleOperation!.getExecutedAt())!,
);
Expand Down
1 change: 1 addition & 0 deletions src/api/yorkie/v1/resources.proto
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ message Operation {
TextNodePos to = 3;
map<string, string> attributes = 4;
TimeTicket executed_at = 5;
map<string, TimeTicket> created_at_map_by_actor = 6;
}
message Increase {
TimeTicket parent_created_at = 1;
Expand Down
4 changes: 4 additions & 0 deletions src/api/yorkie/v1/resources_pb.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ export namespace Operation {
hasExecutedAt(): boolean;
clearExecutedAt(): Style;

getCreatedAtMapByActorMap(): jspb.Map<string, TimeTicket>;
clearCreatedAtMapByActorMap(): Style;

serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): Style.AsObject;
static toObject(includeInstance: boolean, msg: Style): Style.AsObject;
Expand All @@ -494,6 +497,7 @@ export namespace Operation {
to?: TextNodePos.AsObject,
attributesMap: Array<[string, string]>,
executedAt?: TimeTicket.AsObject,
createdAtMapByActorMap: Array<[string, TimeTicket.AsObject]>,
}
}

Expand Down
35 changes: 34 additions & 1 deletion src/api/yorkie/v1/resources_pb.js
Original file line number Diff line number Diff line change
Expand Up @@ -4224,7 +4224,8 @@ proto.yorkie.v1.Operation.Style.toObject = function(includeInstance, msg) {
from: (f = msg.getFrom()) && proto.yorkie.v1.TextNodePos.toObject(includeInstance, f),
to: (f = msg.getTo()) && proto.yorkie.v1.TextNodePos.toObject(includeInstance, f),
attributesMap: (f = msg.getAttributesMap()) ? f.toObject(includeInstance, undefined) : [],
executedAt: (f = msg.getExecutedAt()) && proto.yorkie.v1.TimeTicket.toObject(includeInstance, f)
executedAt: (f = msg.getExecutedAt()) && proto.yorkie.v1.TimeTicket.toObject(includeInstance, f),
createdAtMapByActorMap: (f = msg.getCreatedAtMapByActorMap()) ? f.toObject(includeInstance, proto.yorkie.v1.TimeTicket.toObject) : []
};

if (includeInstance) {
Expand Down Expand Up @@ -4287,6 +4288,12 @@ proto.yorkie.v1.Operation.Style.deserializeBinaryFromReader = function(msg, read
reader.readMessage(value,proto.yorkie.v1.TimeTicket.deserializeBinaryFromReader);
msg.setExecutedAt(value);
break;
case 6:
var value = msg.getCreatedAtMapByActorMap();
reader.readMessage(value, function(message, reader) {
jspb.Map.deserializeBinary(message, reader, jspb.BinaryReader.prototype.readString, jspb.BinaryReader.prototype.readMessage, proto.yorkie.v1.TimeTicket.deserializeBinaryFromReader, "", new proto.yorkie.v1.TimeTicket());
});
break;
default:
reader.skipField();
break;
Expand Down Expand Up @@ -4352,6 +4359,10 @@ proto.yorkie.v1.Operation.Style.serializeBinaryToWriter = function(message, writ
proto.yorkie.v1.TimeTicket.serializeBinaryToWriter
);
}
f = message.getCreatedAtMapByActorMap(true);
if (f && f.getLength() > 0) {
f.serializeBinary(6, writer, jspb.BinaryWriter.prototype.writeString, jspb.BinaryWriter.prototype.writeMessage, proto.yorkie.v1.TimeTicket.serializeBinaryToWriter);
}
};


Expand Down Expand Up @@ -4525,6 +4536,28 @@ proto.yorkie.v1.Operation.Style.prototype.hasExecutedAt = function() {
};


/**
* map<string, TimeTicket> created_at_map_by_actor = 6;
* @param {boolean=} opt_noLazyCreate Do not create the map if
* empty, instead returning `undefined`
* @return {!jspb.Map<string,!proto.yorkie.v1.TimeTicket>}
*/
proto.yorkie.v1.Operation.Style.prototype.getCreatedAtMapByActorMap = function(opt_noLazyCreate) {
return /** @type {!jspb.Map<string,!proto.yorkie.v1.TimeTicket>} */ (
jspb.Message.getMapField(this, 6, opt_noLazyCreate,
proto.yorkie.v1.TimeTicket));
};


/**
* Clears values from the map. The map will be non-null.
* @return {!proto.yorkie.v1.Operation.Style} returns this
*/
proto.yorkie.v1.Operation.Style.prototype.clearCreatedAtMapByActorMap = function() {
this.getCreatedAtMapByActorMap().clear();
return this;};





Expand Down
10 changes: 10 additions & 0 deletions src/document/crdt/rga_tree_split.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,16 @@ export class RGATreeSplitNode<
);
}

/**
* `canStyle` checks if node is able to set style.
*/
public canStyle(editedAt: TimeTicket, latestCreatedAt: TimeTicket): boolean {
return (
!this.getCreatedAt().after(latestCreatedAt) &&
(!this.removedAt || editedAt.after(this.removedAt))
);
}

/**
* `remove` removes node of given edited time.
*/
Expand Down
34 changes: 31 additions & 3 deletions src/document/crdt/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@
* limitations under the License.
*/

import { TimeTicket } from '@yorkie-js-sdk/src/document/time/ticket';
import {
InitialTimeTicket,
MaxTimeTicket,
TimeTicket,
} from '@yorkie-js-sdk/src/document/time/ticket';
import { Indexable } from '@yorkie-js-sdk/src/document/document';
import { RHT } from '@yorkie-js-sdk/src/document/crdt/rht';
import { CRDTGCElement } from '@yorkie-js-sdk/src/document/crdt/element';
import {
RGATreeSplit,
RGATreeSplitNode,
RGATreeSplitPosRange,
ValueChange,
} from '@yorkie-js-sdk/src/document/crdt/rga_tree_split';
Expand Down Expand Up @@ -233,7 +238,8 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTGCElement {
range: RGATreeSplitPosRange,
attributes: Record<string, string>,
editedAt: TimeTicket,
): Array<TextChange<A>> {
latestCreatedAtMapByActor?: Map<string, TimeTicket>,
): [Map<string, TimeTicket>, Array<TextChange<A>>] {
// 01. split nodes with from and to
const [, toRight] = this.rgaTreeSplit.findNodeWithSplit(range[1], editedAt);
const [, fromRight] = this.rgaTreeSplit.findNodeWithSplit(
Expand All @@ -244,7 +250,29 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTGCElement {
// 02. style nodes between from and to
const changes: Array<TextChange<A>> = [];
const nodes = this.rgaTreeSplit.findBetween(fromRight, toRight);
const createdAtMapByActor = new Map<string, TimeTicket>();
const toBeStyleds: Array<RGATreeSplitNode<CRDTTextValue>> = [];

for (const node of nodes) {
const actorID = node.getCreatedAt().getActorID()!;

const latestCreatedAt = latestCreatedAtMapByActor?.size
? latestCreatedAtMapByActor!.has(actorID!)
? latestCreatedAtMapByActor!.get(actorID!)!
: InitialTimeTicket
: MaxTimeTicket;

if (node.canStyle(editedAt, latestCreatedAt)) {
const latestCreatedAt = createdAtMapByActor.get(actorID);
const createdAt = node.getCreatedAt();
if (!latestCreatedAt || createdAt.after(latestCreatedAt)) {
createdAtMapByActor.set(actorID, createdAt);
}
toBeStyleds.push(node);
}
}

for (const node of toBeStyleds) {
if (node.isRemoved()) {
continue;
}
Expand All @@ -267,7 +295,7 @@ export class CRDTText<A extends Indexable = Indexable> extends CRDTGCElement {
}
}

return changes;
return [createdAtMapByActor, changes];
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/document/json/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,14 @@ export class Text<A extends Indexable = Indexable> {

const attrs = stringifyObjectValues(attributes);
const ticket = this.context.issueTimeTicket();
this.text.setStyle(range, attrs, ticket);
const [maxCreatedAtMapByActor] = this.text.setStyle(range, attrs, ticket);

this.context.push(
new StyleOperation(
this.text.getCreatedAt(),
range[0],
range[1],
maxCreatedAtMapByActor,
new Map(Object.entries(attrs)),
ticket,
),
Expand Down
16 changes: 15 additions & 1 deletion src/document/operation/style_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,21 @@ import { Indexable } from '../document';
export class StyleOperation extends Operation {
private fromPos: RGATreeSplitPos;
private toPos: RGATreeSplitPos;
private maxCreatedAtMapByActor: Map<string, TimeTicket>;
private attributes: Map<string, string>;

constructor(
parentCreatedAt: TimeTicket,
fromPos: RGATreeSplitPos,
toPos: RGATreeSplitPos,
maxCreatedAtMapByActor: Map<string, TimeTicket>,
attributes: Map<string, string>,
executedAt: TimeTicket,
) {
super(parentCreatedAt, executedAt);
this.fromPos = fromPos;
this.toPos = toPos;
this.maxCreatedAtMapByActor = maxCreatedAtMapByActor;
this.attributes = attributes;
}

Expand All @@ -53,13 +56,15 @@ export class StyleOperation extends Operation {
parentCreatedAt: TimeTicket,
fromPos: RGATreeSplitPos,
toPos: RGATreeSplitPos,
maxCreatedAtMapByActor: Map<string, TimeTicket>,
attributes: Map<string, string>,
executedAt: TimeTicket,
): StyleOperation {
return new StyleOperation(
parentCreatedAt,
fromPos,
toPos,
maxCreatedAtMapByActor,
attributes,
executedAt,
);
Expand All @@ -77,10 +82,11 @@ export class StyleOperation extends Operation {
logger.fatal(`fail to execute, only Text can execute edit`);
}
const text = parentObject as CRDTText<A>;
const changes = text.setStyle(
const [, changes] = text.setStyle(
[this.fromPos, this.toPos],
this.attributes ? Object.fromEntries(this.attributes) : {},
this.getExecutedAt(),
this.maxCreatedAtMapByActor,
);
return changes.map(({ from, to, value }) => {
return {
Expand Down Expand Up @@ -131,4 +137,12 @@ export class StyleOperation extends Operation {
public getAttributes(): Map<string, string> {
return this.attributes;
}

/**
* `getMaxCreatedAtMapByActor` returns the map that stores the latest creation time
* by actor for the nodes included in the editing range.
*/
public getMaxCreatedAtMapByActor(): Map<string, TimeTicket> {
return this.maxCreatedAtMapByActor;
}
}
47 changes: 40 additions & 7 deletions test/integration/text_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,38 @@ describe('Text', function () {
}, this.test!.title);
});

it('should handle concurrent insertion and deletion', async function () {
await withTwoClientsAndDocuments<{ k1: Text }>(async (c1, d1, c2, d2) => {
d1.update((root) => {
root.k1 = new Text();
root.k1.edit(0, 0, 'AB');
}, 'set new text by c1');
await c1.sync();
await c2.sync();
assert.equal(d1.toSortedJSON(), `{"k1":[{"val":"AB"}]}`);
assert.equal(d1.toSortedJSON(), d2.toSortedJSON());

d1.update((root) => {
root['k1'].edit(0, 2, '');
});
assert.equal(d1.toSortedJSON(), `{"k1":[]}`);
d2.update((root) => {
root['k1'].edit(1, 1, 'C');
});
assert.equal(
d2.toSortedJSON(),
`{"k1":[{"val":"A"},{"val":"C"},{"val":"B"}]}`,
);

await c1.sync();
await c2.sync();
await c1.sync();
assert.equal(d1.toSortedJSON(), `{"k1":[{"val":"C"}]}`);
assert.equal(d2.toSortedJSON(), `{"k1":[{"val":"C"}]}`);
assert.equal(d1.toSortedJSON(), d2.toSortedJSON());
}, this.test!.title);
});

it('should handle concurrent block deletions', async function () {
await withTwoClientsAndDocuments<{ k1: Text }>(async (c1, d1, c2, d2) => {
d1.update((root) => {
Expand Down Expand Up @@ -412,7 +444,7 @@ describe('peri-text example: text concurrent edit', function () {
}, this.test!.title);
});

it.skip('ex2. concurrent formatting and insertion', async function () {
it('ex2. concurrent formatting and insertion', async function () {
await withTwoClientsAndDocuments<{ k1: Text }>(async (c1, d1, c2, d2) => {
d1.update((root) => {
root.k1 = new Text();
Expand Down Expand Up @@ -440,17 +472,18 @@ describe('peri-text example: text concurrent edit', function () {
await c1.sync();
await c2.sync();
await c1.sync();
// NOTE(chacha912): d1 and d2 should have the same content
assert.equal(
d1.toSortedJSON(),
'{"k1":[{"attrs":{"bold":true},"val":"The "},{"val":"brown "},{"attrs":{"bold":true},"val":"fox jumped."}]}',
'd1',
);
assert.equal(
d2.toSortedJSON(),
'{"k1":[{"attrs":{"bold":true},"val":"The "},{"attrs":{"bold":true},"val":"brown "},{"attrs":{"bold":true},"val":"fox jumped."}]}',
'd2',
);
// TODO(MoonGyu1): d1 and d2 should have the result below after applying mark operation
// assert.equal(
// d1.toSortedJSON(),
// '{"k1":[{"attrs":{"bold":true},"val":"The "},{"attrs":{"bold":true},"val":"brown "},{"attrs":{"bold":true},"val":"fox jumped."}]}',
// 'd1',
// );
assert.equal(d2.toSortedJSON(), d1.toSortedJSON());
}, this.test!.title);
});

Expand Down

0 comments on commit 1594345

Please sign in to comment.