From 54ff917d605500fdbd8e51e3cbe1640dae127f8b Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Wed, 13 Jan 2021 04:25:44 -0800 Subject: [PATCH 1/2] fixes --- packages/dds/merge-tree/src/base.ts | 2 +- packages/dds/merge-tree/src/client.ts | 52 ++++++----- packages/dds/merge-tree/src/collections.ts | 88 ++++++++++--------- packages/dds/merge-tree/src/mergeTree.ts | 71 +++++++++------ packages/dds/merge-tree/src/properties.ts | 2 +- .../merge-tree/src/segmentGroupCollection.ts | 2 +- .../src/segmentPropertiesManager.ts | 2 +- packages/dds/merge-tree/src/snapshotV1.ts | 41 +++++---- packages/dds/merge-tree/src/text.ts | 6 +- packages/dds/merge-tree/src/textSegment.ts | 46 ++++++---- packages/dds/merge-tree/tsconfig.json | 2 +- 11 files changed, 173 insertions(+), 141 deletions(-) diff --git a/packages/dds/merge-tree/src/base.ts b/packages/dds/merge-tree/src/base.ts index 581ed43e0670..4c36114c15b1 100644 --- a/packages/dds/merge-tree/src/base.ts +++ b/packages/dds/merge-tree/src/base.ts @@ -22,7 +22,7 @@ export type ConflictAction = (key: TKey, currentKey: TKey, data: TData, currentData: TData) => QProperty; export interface Dictionary { - get(key: TKey): Property; + get(key: TKey): Property | undefined; put(key: TKey, data: TData, conflict?: ConflictAction); remove(key: TKey); map(action: PropertyAction, accum?: TAccum); diff --git a/packages/dds/merge-tree/src/client.ts b/packages/dds/merge-tree/src/client.ts index 43fd32b4f864..a0430c498ce1 100644 --- a/packages/dds/merge-tree/src/client.ts +++ b/packages/dds/merge-tree/src/client.ts @@ -51,7 +51,7 @@ export class Client { public accumWindow = 0; public accumOps = 0; public maxWindowTime = 0; - public longClientId: string; + public longClientId: string | undefined; get mergeTreeDeltaCallback(): MergeTreeDeltaCallback { return this.mergeTree.mergeTreeDeltaCallback; } set mergeTreeDeltaCallback(callback: MergeTreeDeltaCallback) { this.mergeTree.mergeTreeDeltaCallback = callback; } @@ -113,7 +113,7 @@ export class Client { public annotateMarkerNotifyConsensus( marker: Marker, props: Properties.PropertySet, - consensusCallback: (m: Marker) => void): ops.IMergeTreeAnnotateMsg { + consensusCallback: (m: Marker) => void): ops.IMergeTreeAnnotateMsg | undefined { const combiningOp: ops.ICombiningOp = { name: "consensus", }; @@ -142,7 +142,7 @@ export class Client { public annotateMarker( marker: Marker, props: Properties.PropertySet, - combiningOp: ops.ICombiningOp): ops.IMergeTreeAnnotateMsg { + combiningOp: ops.ICombiningOp): ops.IMergeTreeAnnotateMsg | undefined { const annotateOp = OpBuilder.createAnnotateMarkerOp(marker, props, combiningOp); @@ -164,7 +164,7 @@ export class Client { start: number, end: number, props: Properties.PropertySet, - combiningOp: ops.ICombiningOp): ops.IMergeTreeAnnotateMsg { + combiningOp: ops.ICombiningOp): ops.IMergeTreeAnnotateMsg | undefined { const annotateOp = OpBuilder.createAnnotateRangeOp( start, end, @@ -198,7 +198,7 @@ export class Client { * @param pos - The position to insert the segment at * @param segment - The segment to insert */ - public insertSegmentLocal(pos: number, segment: ISegment): ops.IMergeTreeInsertMsg { + public insertSegmentLocal(pos: number, segment: ISegment): ops.IMergeTreeInsertMsg | undefined { if (segment.cachedLength <= 0) { return undefined; } @@ -213,7 +213,10 @@ export class Client { * @param refPos - The reference position to insert the segment at * @param segment - The segment to insert */ - public insertAtReferencePositionLocal(refPos: ReferencePosition, segment: ISegment): ops.IMergeTreeInsertMsg { + public insertAtReferencePositionLocal( + refPos: ReferencePosition, + segment: ISegment, + ): ops.IMergeTreeInsertMsg | undefined { const pos = this.mergeTree.referencePositionToLocalPosition( refPos, this.getCurrentSeq(), @@ -227,7 +230,7 @@ export class Client { segment); const opArgs = { op }; - let traceStart: Trace; + let traceStart: Trace | undefined; if (this.measureOps) { traceStart = Trace.start(); } @@ -293,7 +296,7 @@ export class Client { * @param segment - The segment to get the position of */ public getPosition(segment: ISegment): number { - if(segment?.parent === undefined) { + if (segment?.parent === undefined) { return -1; } return this.mergeTree.getPosition(segment, this.getCurrentSeq(), this.getClientId()); @@ -339,7 +342,7 @@ export class Client { this.copy(range, op.register, clientArgs); } - let traceStart: Trace; + let traceStart: Trace | undefined; if (this.measureOps) { traceStart = Trace.start(); } @@ -373,7 +376,7 @@ export class Client { return false; } - let traceStart: Trace; + let traceStart: Trace | undefined; if (this.measureOps) { traceStart = Trace.start(); } @@ -408,7 +411,7 @@ export class Client { return false; } - let segments: ISegment[]; + let segments: ISegment[] | undefined; if (op.seg) { segments = [this.specToSegment(op.seg)]; } else if (op.register) { @@ -430,7 +433,7 @@ export class Client { return false; } - let traceStart: Trace; + let traceStart: Trace | undefined; if (this.measureOps) { traceStart = Trace.start(); } @@ -492,8 +495,8 @@ export class Client { */ private getValidOpRange( op: ops.IMergeTreeAnnotateMsg | ops.IMergeTreeInsertMsg | ops.IMergeTreeRemoveMsg, - clientArgs: IMergeTreeClientSequenceArgs): IIntegerRange { - let start: number = op.pos1; + clientArgs: IMergeTreeClientSequenceArgs): IIntegerRange | undefined { + let start: number | undefined = op.pos1; if (start === undefined && op.relativePos1) { start = this.mergeTree.posFromRelativePos( op.relativePos1, @@ -501,7 +504,7 @@ export class Client { clientArgs.clientId); } - let end: number = op.pos2; + let end: number | undefined = op.pos2; if (end === undefined && op.relativePos2) { end = this.mergeTree.posFromRelativePos( op.relativePos2, @@ -595,7 +598,7 @@ export class Client { private ackPendingSegment(opArgs: IMergeTreeDeltaOpArgs) { const ackOp = (deltaOpArgs: IMergeTreeDeltaOpArgs) => { - let trace: Trace; + let trace: Trace | undefined; if (this.measureOps) { trace = Trace.start(); } @@ -607,7 +610,7 @@ export class Client { } } - if (this.measureOps) { + if (trace) { this.accumTime += elapsedMicroseconds(trace); this.accumOps++; this.accumWindow += (this.getCurrentSeq() - this.getCollabWindow().minSeq); @@ -731,7 +734,7 @@ export class Client { assert(segmentGroup === segmentSegGroup, "Segment group not at head of segment pending queue"); const segmentPosition = this.findReconnectionPostition(segment, segmentGroup.localSeq); - let newOp: ops.IMergeTreeDeltaOp; + let newOp: ops.IMergeTreeDeltaOp | undefined; switch (resetOp.type) { case ops.MergeTreeDeltaType.ANNOTATE: assert(segment.propertyManager.hasPendingProperties(), "Segment has no pending properties"); @@ -995,12 +998,12 @@ export class Client { } updateMinSeq(minSeq: number) { - let trace: Trace; + let trace: Trace | undefined; if (this.measureOps) { trace = Trace.start(); } this.mergeTree.setMinSeq(minSeq); - if (this.measureOps) { + if (trace) { const elapsed = elapsedMicroseconds(trace); this.accumWindowTime += elapsed; if (elapsed > this.maxWindowTime) { @@ -1021,7 +1024,7 @@ export class Client { console.log(`getPropertiesAtPosition cli ${this.getLongClientId(segWindow.clientId)} ref seq ${segWindow.currentSeq}`); } - let propertiesAtPosition: Properties.PropertySet; + let propertiesAtPosition: Properties.PropertySet | undefined; const segoff = this.getContainingSegment(pos); const seg = segoff.segment; if (seg) { @@ -1036,8 +1039,8 @@ export class Client { console.log(`getRangeExtentsOfPosition cli ${this.getLongClientId(segWindow.clientId)} ref seq ${segWindow.currentSeq}`); } - let posStart: number; - let posAfterEnd: number; + let posStart: number | undefined; + let posAfterEnd: number | undefined; const segoff = this.getContainingSegment(pos); const seg = segoff.segment; @@ -1070,7 +1073,8 @@ export class Client { this.getShortClientId(this.longClientId), minSeq, currentSeq, branchId); } else { const oldClientId = this.longClientId; - const oldData = this.clientNameToIds.get(oldClientId).data; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const oldData = this.clientNameToIds.get(oldClientId)!.data; this.longClientId = longClientId; this.clientNameToIds.put(longClientId, oldData); this.shortClientIdMap[oldData.clientId] = longClientId; diff --git a/packages/dds/merge-tree/src/collections.ts b/packages/dds/merge-tree/src/collections.ts index 433f208f2c98..88a5c528756d 100644 --- a/packages/dds/merge-tree/src/collections.ts +++ b/packages/dds/merge-tree/src/collections.ts @@ -29,7 +29,7 @@ export class Stack { } } -export function ListRemoveEntry(entry: List): List { +export function ListRemoveEntry(entry: List): List | undefined { if (entry === undefined) { return undefined; } @@ -44,24 +44,20 @@ export function ListRemoveEntry(entry: List): List { } export function ListMakeEntry(data: U): List { - const entry: List = new List(false, data); - entry.prev = entry; - entry.next = entry; - return entry; + return new List(false, data); } export function ListMakeHead(): List { - const entry: List = new List(true, undefined); - entry.prev = entry; - entry.next = entry; - return entry; + return new List(true, undefined); } export class List { next: List; prev: List; - constructor(public isHead: boolean, public data: T) { + constructor(public isHead: boolean, public data: T | undefined) { + this.prev = this; + this.next = this; } clear(): void { @@ -80,9 +76,10 @@ export class List { return (entry); } - dequeue(): T { + dequeue(): T | undefined { if (!this.empty()) { - const removedEntry = ListRemoveEntry(this.next); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const removedEntry = ListRemoveEntry(this.next)!; return removedEntry.data; } } @@ -125,13 +122,13 @@ export class List { return (i); } - first(): T { + first(): T | undefined { if (!this.empty()) { return (this.next.data); } } - last(): T { + last(): T | undefined { if (!this.empty()) { return (this.prev.data); } @@ -159,12 +156,12 @@ export class List { entry.next.prev = entry; } - popEntry(head: List): List { + popEntry(head: List): List | undefined { if (this.next.isHead) { - return (undefined); + return undefined; } else { - return (ListRemoveEntry(this.next)); + return ListRemoveEntry(this.next); } } @@ -357,8 +354,8 @@ export const enum RBColor { export interface RBNode { key: TKey; data: TData; - left: RBNode; - right: RBNode; + left: RBNode | undefined; + right: RBNode | undefined; color: RBColor; size: number; } @@ -369,8 +366,8 @@ export interface IRBAugmentation { } export interface IRBMatcher { - continueSubtree(node: RBNode, key: TKey): boolean; - matchNode(node: RBNode, key: TKey): boolean; + continueSubtree(node: RBNode | undefined, key: TKey): boolean; + matchNode(node: RBNode | undefined, key: TKey): boolean; } export interface RBNodeActions { @@ -381,7 +378,7 @@ export interface RBNodeActions { } export class RedBlackTree implements Base.SortedDictionary { - root: RBNode; + root: RBNode | undefined; constructor(public compareKeys: Base.KeyComparer, public aug?: IRBAugmentation) { } @@ -394,11 +391,11 @@ export class RedBlackTree implements Base.SortedDictionary) { + isRed(node: RBNode | undefined) { return node && (node.color == RBColor.RED); } - nodeSize(node: RBNode) { + nodeSize(node: RBNode | undefined) { return node ? node.size : 0; } size() { @@ -412,7 +409,7 @@ export class RedBlackTree implements Base.SortedDictionary, key: TKey) { + nodeGet(node: RBNode | undefined, key: TKey) { while (node) { const cmp = this.compareKeys(key, node.key); if (cmp < 0) { @@ -439,7 +436,7 @@ export class RedBlackTree implements Base.SortedDictionary, + node: RBNode | undefined, results: RBNode[], key: TKey, matcher: IRBMatcher) { @@ -468,7 +465,11 @@ export class RedBlackTree implements Base.SortedDictionary, key: TKey, data: TData, conflict?: Base.ConflictAction) { + nodePut( + node: RBNode | undefined, + key: TKey, data: TData, + conflict?: Base.ConflictAction, + ) { if (!node) { return this.makeNode(key, data, RBColor.RED, 1); } @@ -629,7 +630,7 @@ export class RedBlackTree implements Base.SortedDictionary) { + nodeHeight(node: RBNode | undefined) { if (node === undefined) { return -1; } @@ -644,7 +645,7 @@ export class RedBlackTree implements Base.SortedDictionary, key: TKey): RBNode { + nodeFloor(node: RBNode | undefined, key: TKey): RBNode | undefined { if (node) { const cmp = this.compareKeys(key, node.key); if (cmp == 0) { @@ -671,7 +672,7 @@ export class RedBlackTree implements Base.SortedDictionary, key: TKey): RBNode { + nodeCeil(node: RBNode | undefined, key: TKey): RBNode | undefined { if (node) { const cmp = this.compareKeys(key, node.key); if (cmp == 0) { @@ -783,7 +784,8 @@ export class RedBlackTree implements Base.SortedDictionary) { + balance(input: RBNode) { + let node: RBNode | undefined = input; if (this.isRed(node.right)) { node = this.rotateLeft(node); } @@ -982,11 +984,11 @@ export class IntegerRangeTree implements IRBAugmentation implements IRBAugmentation) { - let rbConflict: Base.ConflictAction; + let rbConflict: Base.ConflictAction | undefined; if (conflict) { rbConflict = (key: T, currentKey: T) => { const ival = conflict(key, currentKey); @@ -1076,11 +1078,11 @@ export class IntervalTree implements IRBAugmentation, key: T) { + matchNode(node: IntervalNode | undefined, key: T) { return node && node.key.overlaps(key); } - continueSubtree(node: IntervalNode, key: T) { + continueSubtree(node: IntervalNode | undefined, key: T) { const cont = node && node.data.minmax.overlaps(key); if (this.diag && (!cont)) { if (node) { @@ -1154,7 +1156,7 @@ export class TST { return x.val; } - nodeGet(x: TSTNode, key: string, d: number): TSTNode { + nodeGet(x: TSTNode | undefined, key: string, d: number): TSTNode | undefined { if (x === undefined) { return undefined; } @@ -1179,7 +1181,7 @@ export class TST { // console.log(`put ${key}`); } - nodePut(x: TSTNode, key: string, val: T, d: number) { + nodePut(x: TSTNode | undefined, key: string, val: T, d: number) { const c = key.charAt(d); if (x === undefined) { x = { c }; @@ -1219,7 +1221,7 @@ export class TST { return q; } - collect(x: TSTNode, prefix: TSTPrefix, q: string[]) { + collect(x: TSTNode | undefined, prefix: TSTPrefix, q: string[]) { if (x === undefined) { return; } @@ -1231,7 +1233,7 @@ export class TST { this.collect(x.right, prefix, q); } - mapNode(x: TSTNode, prefix: TSTPrefix, fn: (key: string, val: T) => void) { + mapNode(x: TSTNode | undefined, prefix: TSTPrefix, fn: (key: string, val: T) => void) { if (x === undefined) { return; } @@ -1261,7 +1263,7 @@ export class TST { return q; } - collectPairs(x: TSTNode, prefix: TSTPrefix, q: TSTResult[]) { + collectPairs(x: TSTNode | undefined, prefix: TSTPrefix, q: TSTResult[]) { if (x === undefined) { return; } @@ -1273,7 +1275,7 @@ export class TST { this.collectPairs(x.right, prefix, q); } - patternCollect(x: TSTNode, prefix: TSTPrefix, d: number, pattern: string, q: string[]) { + patternCollect(x: TSTNode | undefined, prefix: TSTPrefix, d: number, pattern: string, q: string[]) { if (x === undefined) { return; } @@ -1296,7 +1298,7 @@ export class TST { } nodeProximity( - x: TSTNode, + x: TSTNode | undefined, prefix: TSTPrefix, d: number, pattern: string, diff --git a/packages/dds/merge-tree/src/mergeTree.ts b/packages/dds/merge-tree/src/mergeTree.ts index 6c75928d8c7b..b27496e75387 100644 --- a/packages/dds/merge-tree/src/mergeTree.ts +++ b/packages/dds/merge-tree/src/mergeTree.ts @@ -95,11 +95,16 @@ export interface ISegment extends IMergeNodeCommon, IRemovalInfo { localRefs?: LocalReferenceCollection; removalsByBranch?: IRemovalInfo[]; properties?: Properties.PropertySet; - addProperties(newProps: Properties.PropertySet, op?: ops.ICombiningOp, seq?: number, collabWindow?: CollaborationWindow): Properties.PropertySet; + addProperties( + newProps: Properties.PropertySet, + op?: ops.ICombiningOp, + seq?: number, + ollabWindow?: CollaborationWindow, + ): Properties.PropertySet | undefined; clone(): ISegment; canAppend(segment: ISegment): boolean; append(segment: ISegment): void; - splitAt(pos: number): ISegment; + splitAt(pos: number): ISegment | undefined; toJSONObject(): any; /** * Acks the current segment against the segment group, op, and merge tree. @@ -123,7 +128,7 @@ export interface IMarkerModifiedAction { export interface ISegmentAction { // eslint-disable-next-line @typescript-eslint/prefer-function-type (segment: ISegment, pos: number, refSeq: number, clientId: number, start: number, - end: number, accum?: TClientData): boolean; + end: number, accum: TClientData): boolean; } export interface ISegmentChanges { @@ -521,7 +526,7 @@ export abstract class BaseSegment extends MergeNode implements ISegment { } } - public splitAt(pos: number): ISegment { + public splitAt(pos: number): ISegment | undefined { if (pos > 0) { const leafSegment = this.createSplitSegmentAt(pos); if (leafSegment) { @@ -569,7 +574,7 @@ export abstract class BaseSegment extends MergeNode implements ISegment { abstract clone(): ISegment; abstract append(segment: ISegment): void; - protected abstract createSplitSegmentAt(pos: number): BaseSegment; + protected abstract createSplitSegmentAt(pos: number): BaseSegment | undefined; } export const reservedTileLabelsKey = "referenceTileLabels"; @@ -1215,7 +1220,7 @@ export class MergeTree { // and update the block's info. for (let childIndex = 0; childIndex < maxChildren && nodeIndex < nodes.length; // While we still have children & nodes left - childIndex++, nodeIndex++ // Advance to next child & node + childIndex++ , nodeIndex++ // Advance to next child & node ) { // Insert the next node into the current block this.addNode(block, nodes[nodeIndex]); @@ -1287,7 +1292,7 @@ export class MergeTree { } private scourNode(node: IMergeBlock, holdNodes: IMergeNode[]) { - let prevSegment: ISegment; + let prevSegment: ISegment | undefined; for (let k = 0; k < node.childCount; k++) { const childNode = node.children[k]; if (childNode.isLeaf()) { @@ -1790,13 +1795,13 @@ export class MergeTree { private search( pos: number, refSeq: number, clientId: number, - actions?: SegmentActions, clientData?: TClientData): ISegment { + actions?: SegmentActions, clientData?: TClientData): ISegment | undefined { return this.searchBlock(this.root, pos, 0, refSeq, clientId, actions, clientData); } private searchBlock( block: IMergeBlock, pos: number, segpos: number, refSeq: number, clientId: number, - actions?: SegmentActions, clientData?: TClientData): ISegment { + actions?: SegmentActions, clientData?: TClientData): ISegment | undefined { const children = block.children; if (actions && actions.pre) { actions.pre(block, segpos, refSeq, clientId, undefined, undefined, clientData); @@ -1830,7 +1835,7 @@ export class MergeTree { private backwardSearch( pos: number, refSeq: number, clientId: number, - actions?: SegmentActions, clientData?: TClientData): ISegment { + actions?: SegmentActions, clientData?: TClientData): ISegment | undefined { const len = this.getLength(refSeq, clientId); if (pos > len) { return undefined; @@ -1840,7 +1845,7 @@ export class MergeTree { private backwardSearchBlock( block: IMergeBlock, pos: number, segEnd: number, refSeq: number, clientId: number, - actions?: SegmentActions, clientData?: TClientData): ISegment { + actions?: SegmentActions, clientData?: TClientData): ISegment | undefined { const children = block.children; if (actions && actions.pre) { actions.pre(block, segEnd, refSeq, clientId, undefined, undefined, clientData); @@ -1965,7 +1970,14 @@ export class MergeTree { return pos; } - public insertSegments(pos: number, segments: ISegment[], refSeq: number, clientId: number, seq: number, opArgs: IMergeTreeDeltaOpArgs) { + public insertSegments( + pos: number, + segments: ISegment[], + refSeq: number, + clientId: number, + seq: number, + opArgs: IMergeTreeDeltaOpArgs | undefined, + ) { // const tt = MergeTree.traceTraversal; // MergeTree.traceTraversal = true; this.ensureIntervalBoundary(pos, refSeq, clientId); @@ -2014,7 +2026,7 @@ export class MergeTree { // and split the blocks until we find a block with // room let block = segment.parent; - let ordinalUpdateNode = block; + let ordinalUpdateNode: IMergeBlock | undefined = block; while (block !== undefined) { if (block.childCount >= MaxNodesInBlock) { const splitNode = this.split(block); @@ -2051,7 +2063,7 @@ export class MergeTree { rebalanceTree(splitSeg.next); startSeg = splitSeg.next; } - this.leftExcursion(startSeg, (backSeg) => { + this.leftExcursion(startSeg, (backSeg) => { if (!backSeg.isLeaf()) { return true; } @@ -2277,8 +2289,7 @@ export class MergeTree { } // Visit segments starting from node's left siblings, then up to node's parent - private leftExcursion(node: IMergeNode, leafAction: ISegmentAction) { - const actions = { leaf: leafAction }; + private leftExcursion(node: IMergeNode, leafAction: ISegmentAction) { let go = true; let startNode = node; let parent = startNode.parent; @@ -2292,10 +2303,10 @@ export class MergeTree { if (matchedStart) { if (!node.isLeaf()) { const childBlock = node; - go = this.nodeMapReverse(childBlock, actions, 0, UniversalSequenceNumber, - this.collabWindow.clientId, undefined); + go = this.nodeMapReverse(childBlock, leafAction, 0, UniversalSequenceNumber, + this.collabWindow.clientId); } else { - go = leafAction(node, 0, UniversalSequenceNumber, this.collabWindow.clientId, 0, 0); + go = leafAction(node, 0, UniversalSequenceNumber, this.collabWindow.clientId, 0, 0, undefined); } if (!go) { return; @@ -2310,7 +2321,7 @@ export class MergeTree { } // Visit segments starting from node's right siblings, then up to node's parent - private rightExcursion(node: IMergeNode, leafAction: ISegmentAction) { + private rightExcursion(node: IMergeNode, leafAction: ISegmentAction) { const actions = { leaf: leafAction }; let go = true; let startNode = node; @@ -2328,7 +2339,7 @@ export class MergeTree { go = this.nodeMap(childBlock, actions, 0, UniversalSequenceNumber, this.collabWindow.clientId, undefined); } else { - go = leafAction(node, 0, UniversalSequenceNumber, this.collabWindow.clientId, 0, 0); + go = leafAction(node, 0, UniversalSequenceNumber, this.collabWindow.clientId, 0, 0, undefined); } if (!go) { return; @@ -2569,7 +2580,7 @@ export class MergeTree { this.ensureIntervalBoundary(end, refSeq, clientId); const deltaSegments: IMergeTreeSegmentDelta[] = []; const localSeq = seq === UnassignedSequenceNumber ? ++this.collabWindow.localSeq : undefined; - let segmentGroup: SegmentGroup; + let segmentGroup: SegmentGroup | undefined; const annotateSegment = (segment: ISegment) => { const propertyDeltas = segment.addProperties(props, combiningOp, seq, this.collabWindow); @@ -2847,7 +2858,9 @@ export class MergeTree { public incrementalBlockMap(stateStack: Collections.Stack>) { while (!stateStack.empty()) { - const state = stateStack.top(); + // We already check the stack is not empty + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const state = stateStack.top()!; if (state.op !== IncrementalExecOp.Go) { return; } @@ -2902,7 +2915,7 @@ export class MergeTree { private nodeMap( node: IMergeBlock, actions: SegmentActions, pos: number, refSeq: number, - clientId: number, accum?: TClientData, start?: number, end?: number) { + clientId: number, accum: TClientData, start?: number, end?: number) { if (start === undefined) { start = 0; } @@ -2983,9 +2996,9 @@ export class MergeTree { } // Straight call every segment; goes until leaf action returns false - private nodeMapReverse( - block: IMergeBlock, actions: SegmentActions, pos: number, refSeq: number, - clientId: number, accum?: TClientData) { + private nodeMapReverse( + block: IMergeBlock, leafAction: ISegmentAction, pos: number, refSeq: number, + clientId: number) { let go = true; const children = block.children; for (let childIndex = block.childCount - 1; childIndex >= 0; childIndex--) { @@ -2994,10 +3007,10 @@ export class MergeTree { // Found entry containing pos if (!child.isLeaf()) { if (go) { - go = this.nodeMapReverse(child, actions, pos, refSeq, clientId, accum); + go = this.nodeMapReverse(child, leafAction, pos, refSeq, clientId); } } else { - go = actions.leaf(child, pos, refSeq, clientId, 0, 0, accum); + go = leafAction(child, pos, refSeq, clientId, 0, 0, undefined); } } if (!go) { diff --git a/packages/dds/merge-tree/src/properties.ts b/packages/dds/merge-tree/src/properties.ts index a5b89e92a55d..36d4075826f8 100644 --- a/packages/dds/merge-tree/src/properties.ts +++ b/packages/dds/merge-tree/src/properties.ts @@ -59,7 +59,7 @@ export function combine(combiningInfo: ops.ICombiningOp, currentValue: any, newV return currentValue; } -export function matchProperties(a: PropertySet, b: PropertySet) { +export function matchProperties(a: PropertySet | undefined, b: PropertySet | undefined) { if (a) { if (!b) { return false; diff --git a/packages/dds/merge-tree/src/segmentGroupCollection.ts b/packages/dds/merge-tree/src/segmentGroupCollection.ts index e749e6a88a20..548cf71279f2 100644 --- a/packages/dds/merge-tree/src/segmentGroupCollection.ts +++ b/packages/dds/merge-tree/src/segmentGroupCollection.ts @@ -26,7 +26,7 @@ export class SegmentGroupCollection { segmentGroup.segments.push(this.segment); } - public dequeue(): SegmentGroup { + public dequeue(): SegmentGroup | undefined { return this.segmentGroups.dequeue(); } diff --git a/packages/dds/merge-tree/src/segmentPropertiesManager.ts b/packages/dds/merge-tree/src/segmentPropertiesManager.ts index bd299c6bafdf..21a2e9c241f0 100644 --- a/packages/dds/merge-tree/src/segmentPropertiesManager.ts +++ b/packages/dds/merge-tree/src/segmentPropertiesManager.ts @@ -36,7 +36,7 @@ export class SegmentPropertiesManager { newProps: Properties.PropertySet, op?: ICombiningOp, seq?: number, - collabWindow?: CollaborationWindow): Properties.PropertySet { + collabWindow?: CollaborationWindow): Properties.PropertySet | undefined { if (!this.segment.properties) { this.pendingRewriteCount = 0; this.segment.properties = Properties.createMap(); diff --git a/packages/dds/merge-tree/src/snapshotV1.ts b/packages/dds/merge-tree/src/snapshotV1.ts index 78dcbe580b43..3330d18dc1b2 100644 --- a/packages/dds/merge-tree/src/snapshotV1.ts +++ b/packages/dds/merge-tree/src/snapshotV1.ts @@ -39,9 +39,9 @@ export class SnapshotV1 { // for very chunky text, blob size can easily be 4x-8x of that number. public static readonly chunkSize: number = 10000; - private header: MergeTreeHeaderMetadata; - private segments: JsonSegmentSpecs[]; - private segmentLengths: number[]; + private readonly header: MergeTreeHeaderMetadata; + private readonly segments: JsonSegmentSpecs[]; + private readonly segmentLengths: number[]; private readonly logger: ITelemetryLogger; private readonly chunkSize: number; @@ -49,9 +49,22 @@ export class SnapshotV1 { public mergeTree: MergeTree.MergeTree, logger: ITelemetryLogger, public filename?: string, - public onCompletion?: () => void) { + public onCompletion?: () => void, + ) { this.logger = ChildLogger.create(logger, "Snapshot"); this.chunkSize = mergeTree?.options?.mergeTreeSnapshotChunkSize ?? SnapshotV1.chunkSize; + + const { currentSeq, minSeq } = mergeTree.getCollabWindow(); + this.header = { + minSequenceNumber: minSeq, + sequenceNumber: currentSeq, + orderedChunkMetadata: [], + totalLength: 0, + totalSegmentCount: 0, + }; + + this.segments = []; + this.segmentLengths = []; } getSeqLengthSegs( @@ -100,7 +113,9 @@ export class SnapshotV1 { this.header.totalLength += chunk.length; } while (this.header.totalSegmentCount < this.segments.length); - const headerChunk = chunks.shift(); + // The do while loop should have added at least one chunk + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const headerChunk = chunks.shift()!; headerChunk.headerMetadata = this.header; headerChunk.headerMetadata.orderedChunkMetadata = [{ id: SnapshotLegacy.header }]; const entries: ITreeEntry[] = chunks.map((chunk, index) => { @@ -150,17 +165,7 @@ export class SnapshotV1 { extractSync() { const mergeTree = this.mergeTree; - const { currentSeq, minSeq } = mergeTree.getCollabWindow(); - this.header = { - minSequenceNumber: minSeq, - sequenceNumber: currentSeq, - orderedChunkMetadata: [], - totalLength: 0, - totalSegmentCount: 0, - }; - - this.segments = []; - this.segmentLengths = []; + const minSeq = this.header.minSequenceNumber; // Helper to add the given `MergeTreeChunkV0SegmentSpec` to the snapshot. const pushSegRaw = (json: JsonSegmentSpecs, length: number) => { @@ -250,7 +255,7 @@ export class SnapshotV1 { storage: IChannelStorageService, path: string, logger: ITelemetryLogger, - options: Properties.PropertySet, + options: Properties.PropertySet | undefined, serializer?: IFluidSerializer, ): Promise { const chunkAsString: string = await storage.read(path); @@ -261,7 +266,7 @@ export class SnapshotV1 { path: string, chunk: string, logger: ITelemetryLogger, - options: Properties.PropertySet, + options: Properties.PropertySet | undefined, serializer?: IFluidSerializer, ): MergeTreeChunkV1 { const utf8 = fromBase64ToUtf8(chunk); diff --git a/packages/dds/merge-tree/src/text.ts b/packages/dds/merge-tree/src/text.ts index 558d7d315ae7..5b0984cb7c39 100644 --- a/packages/dds/merge-tree/src/text.ts +++ b/packages/dds/merge-tree/src/text.ts @@ -24,14 +24,14 @@ export function loadSegments(content: string, segLimit: number, markers: boolean const segments = [] as MergeTree.ISegment[]; for (const paragraph of paragraphs) { - let pgMarker: MergeTree.Marker; + let pgMarker: MergeTree.Marker | undefined; if (markers) { pgMarker = MergeTree.Marker.make(ops.ReferenceType.Tile, { [MergeTree.reservedTileLabelsKey]: ["pg"] }); } if (withProps) { if ((paragraph.includes("Chapter")) || (paragraph.includes("PRIDE AND PREJ"))) { - if (markers) { + if (pgMarker) { pgMarker.addProperties({ header: 2 }); segments.push(new TextSegment(paragraph)); } else { @@ -57,7 +57,7 @@ export function loadSegments(content: string, segLimit: number, markers: boolean } else { segments.push(new TextSegment(paragraph)); } - if (markers) { + if (pgMarker) { segments.push(pgMarker); } } diff --git a/packages/dds/merge-tree/src/textSegment.ts b/packages/dds/merge-tree/src/textSegment.ts index e3f882d9ddd1..73a017b79bbc 100644 --- a/packages/dds/merge-tree/src/textSegment.ts +++ b/packages/dds/merge-tree/src/textSegment.ts @@ -111,22 +111,31 @@ export class TextSegment extends BaseSegment { } } -export interface ITextAccumulator { +interface ITextAccumulator { textSegment: TextSegment; placeholder?: string; parallelArrays?: boolean; - parallelText?: string[]; - parallelMarkers?: Marker[]; - parallelMarkerLabel?: string; - tagsInProgress?: string[]; } +interface ITextAndMarkerAccumulator extends ITextAccumulator { + parallelArrays: true; + parallelText: string[]; + parallelMarkers: Marker[]; + parallelMarkerLabel: string; + tagsInProgress: string[]; +} + +function isTextAndMarkerAccumulator(accum: ITextAccumulator): accum is ITextAndMarkerAccumulator { + return accum.parallelArrays; +} + +type ITextAccumulatorType = ITextAccumulator | ITextAndMarkerAccumulator; export class MergeTreeTextHelper { constructor(private readonly mergeTree: MergeTree) { } public getTextAndMarkers(refSeq: number, clientId: number, label: string, start?: number, end?: number) { const range = this.getValidRange(start, end, refSeq, clientId); - const accum: ITextAccumulator = { + const accum: ITextAndMarkerAccumulator = { parallelArrays: true, parallelMarkerLabel: label, parallelMarkers: [], @@ -140,7 +149,7 @@ export class MergeTreeTextHelper { `get text on cli ${glc(this.mergeTree, this.mergeTree.collabWindow.clientId)} ` + `ref cli ${glc(this.mergeTree, clientId)} refSeq ${refSeq}`); } - this.mergeTree.mapRange( + this.mergeTree.mapRange( { leaf: this.gatherText }, refSeq, clientId, @@ -171,22 +180,21 @@ export class MergeTreeTextHelper { return accum.textSegment.text; } - private getValidRange(start: number, end: number, refSeq: number, clientId: number): IIntegerRange { + private getValidRange( + start: number | undefined, + end: number | undefined, + refSeq: number, + clientId: number, + ): IIntegerRange { const range: IIntegerRange = { - end, - start, + end: end ?? this.mergeTree.getLength(refSeq, clientId), + start: start ?? 0, }; - if (range.start === undefined) { - range.start = 0; - } - if (range.end === undefined) { - range.end = this.mergeTree.getLength(refSeq, clientId); - } return range; } private readonly gatherText = (segment: ISegment, pos: number, refSeq: number, clientId: number, start: number, - end: number, accumText: ITextAccumulator) => { + end: number, accumText: ITextAccumulatorType) => { if (TextSegment.is(segment)) { if (MergeTree.traceGatherText) { console.log( @@ -195,7 +203,7 @@ export class MergeTreeTextHelper { } let beginTags = ""; let endTags = ""; - if (accumText.parallelArrays) { + if (isTextAndMarkerAccumulator(accumText)) { // TODO: let clients pass in function to get tag const tags = [] as string[]; const initTags = [] as string[]; @@ -261,7 +269,7 @@ export class MergeTreeTextHelper { accumText.textSegment.text += accumText.placeholder; } } - } else if (accumText.parallelArrays) { + } else if (isTextAndMarkerAccumulator(accumText)) { const marker = segment as Marker; if (marker.hasTileLabel(accumText.parallelMarkerLabel)) { accumText.parallelMarkers.push(marker); diff --git a/packages/dds/merge-tree/tsconfig.json b/packages/dds/merge-tree/tsconfig.json index 06321fee095c..b8b11d094071 100644 --- a/packages/dds/merge-tree/tsconfig.json +++ b/packages/dds/merge-tree/tsconfig.json @@ -12,4 +12,4 @@ "include": [ "src/**/*" ] -} \ No newline at end of file +} From 4f469334628e53b4cca9adaf3af0c29f19461a5d Mon Sep 17 00:00:00 2001 From: Curtis Man Date: Wed, 13 Jan 2021 13:43:04 -0800 Subject: [PATCH 2/2] Update packages/dds/merge-tree/src/mergeTree.ts Co-authored-by: Tony Murphy --- packages/dds/merge-tree/src/mergeTree.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dds/merge-tree/src/mergeTree.ts b/packages/dds/merge-tree/src/mergeTree.ts index b27496e75387..37b6ff6b8585 100644 --- a/packages/dds/merge-tree/src/mergeTree.ts +++ b/packages/dds/merge-tree/src/mergeTree.ts @@ -99,7 +99,7 @@ export interface ISegment extends IMergeNodeCommon, IRemovalInfo { newProps: Properties.PropertySet, op?: ops.ICombiningOp, seq?: number, - ollabWindow?: CollaborationWindow, + collabWindow?: CollaborationWindow, ): Properties.PropertySet | undefined; clone(): ISegment; canAppend(segment: ISegment): boolean;