From 7c3af93fa9dcf5a7f4ca9b6a5b47fc4bc16a684d Mon Sep 17 00:00:00 2001 From: Bartosz Sypytkowski Date: Fri, 2 Jun 2023 20:02:28 +0200 Subject: [PATCH] simplified model for weak links --- src/structs/ContentLink.js | 144 +++++++++---------------------------- src/structs/Item.js | 16 ++--- src/types/AbstractType.js | 4 +- src/types/WeakLink.js | 53 +++++--------- tests/weakLinks.tests.js | 12 +--- 5 files changed, 65 insertions(+), 164 deletions(-) diff --git a/src/structs/ContentLink.js b/src/structs/ContentLink.js index 1bdba6c7..a8d5dcbe 100644 --- a/src/structs/ContentLink.js +++ b/src/structs/ContentLink.js @@ -15,12 +15,14 @@ import { export class ContentLink { /** - * @param {WeakLink|null} link - * @param {{type:ID|null,tname:string|null,item:ID|null,key:string|null}|null} raw + * @param {WeakLink} link */ - constructor (link, raw) { + constructor (link) { this.link = link - this.raw = raw + /** + * @type {Item|null} + */ + this._item = null } /** @@ -48,7 +50,7 @@ import { * @return {ContentLink} */ copy () { - return new ContentLink(this.link, this.raw) + return new ContentLink(this.link) } /** @@ -72,40 +74,24 @@ import { * @param {Item} item */ integrate (transaction, item) { - if (this.raw !== null) { - const { type, tname, key, item } = this.raw - let parent = null - if (type !== null) { - const parentItem = find(transaction.doc.store, type) - if (parentItem.constructor === Item) { - parent = /** @type {ContentType} */ (parentItem.content).type - } else { - parent = null - } - } else { - parent = /** @type {AbstractType} */ (transaction.doc.share.get(/** @type {string} */ (tname))) + let sourceItem = this.link.item !== null ? this.link.item : getItemCleanStart(transaction, this.link.id) + if (sourceItem.constructor === Item && sourceItem.parentSub !== null) { + // for maps, advance to most recent item + while (sourceItem.right !== null) { + sourceItem = sourceItem.right } - - let target = null - if (item !== null) { - target = getItemCleanStart(transaction, item) - if (target.length > 1) { - target = getItemCleanEnd(transaction, transaction.doc.store, createID(target.id.client, target.id.clock + 1)) - } - } else if (parent !== null) { - target = parent._map.get(/** @type {string} */ (key)) || null - } - const source = (parent !== null && target !== null) ? {parent: parent, item: target, key: key} : null - this.link = new WeakLink(source) - this.raw = null } - - const linked = /** @type {WeakLink} */ (this.link).linkedItem() - if (linked !== undefined && linked.constructor === Item) { - if (linked.linkedBy === null) { - linked.linkedBy = new Set() + if (!sourceItem.deleted && sourceItem.length > 1) { + sourceItem = getItemCleanEnd(transaction, transaction.doc.store, createID(sourceItem.id.client, sourceItem.id.clock + 1)) + } + this.link.item = sourceItem + this._item = item + if (!sourceItem.deleted) { + const src = /** @type {Item} */ (sourceItem) + if (src.linkedBy === null) { + src.linkedBy = new Set() } - linked.linkedBy.add(/** @type {WeakLink} */ (this.link)) + src.linkedBy.add(item) } } @@ -113,14 +99,12 @@ import { * @param {Transaction} transaction */ delete (transaction) { - if (this.link !== null && this.link.source !== null) { - const item = this.link.source.item - if (item !== null && item.constructor === Item) { - if (item.linkedBy !== null) { - item.linkedBy.delete(this.link) - } + if (this._item !== null && this.link !== null && this.link.item !== null && !this.link.item.deleted) { + const item = /** @type {Item} */ (this.link.item) + if (item.linkedBy !== null) { + item.linkedBy.delete(this._item) } - this.link.source = null + this.link.item = null } } @@ -134,49 +118,9 @@ import { * @param {number} offset */ write (encoder, offset) { - let type = null - let tname = null - let item = null - let key = null - let flags = 0 - if (this.raw !== null) { - type = this.raw.type - tname = this.raw.tname - key = this.raw.key - item = this.raw.item - } else { - const source = /** @type {WeakLink} */ (this.link).source - if (source !== null) { - if (source.parent._item !== null) { - type = source.parent._item.id - } else { - tname = findRootTypeKey(source.parent) - } - if (source.item !== null) { - item = source.item.id - } else { - key = source.key - } - } - } - - if (type !== null) { - flags |= 1 - } - if (item !== null) { - flags |= 2 - } - encoding.writeVarUint(encoder.restEncoder, flags) - if (type !== null) { - encoder.writeLeftID(type) - } else { - encoder.writeString(/** @type {string} */ (tname)) - } - if (item !== null) { - encoder.writeLeftID(item) - } else { - encoder.writeString(/** @type {string} */ (key)) - } + const flags = 0 // flags that could be used in the future + encoding.writeUint8(encoder.restEncoder, flags) + encoder.writeLeftID(this.link.id) } /** @@ -192,22 +136,9 @@ import { * @return {ContentLink} */ export const readContentWeakLink = decoder => { - const flags = decoding.readVarUint(decoder.restDecoder) - let type = null - let tname = null - let item = null - let key = null - if ((flags & 1) !== 0) { - type = decoder.readLeftID() - } else { - tname = decoder.readString() - } - if ((flags & 2) !== 0) { - item = decoder.readLeftID() - } else { - key = decoder.readString() - } - return new ContentLink(null, { type, tname, item, key }) + const flags = decoding.readUint8(decoder.restDecoder) + const id = decoder.readLeftID() + return new ContentLink(new WeakLink(id, null)) } const lengthExceeded = error.create('Length exceeded!') @@ -221,12 +152,7 @@ const lengthExceeded = error.create('Length exceeded!') * @return {WeakLink} */ export const arrayWeakLink = (transaction, parent, index) => { - const marker = findMarker(parent, index) let item = parent._start - if (marker !== null) { - item = marker.p - index -= marker.index - } for (; item !== null; item = item.right) { if (!item.deleted && item.countable) { if (index < item.length) { @@ -236,7 +162,7 @@ export const arrayWeakLink = (transaction, parent, index) => { if (item.length > 1) { item = getItemCleanEnd(transaction, transaction.doc.store, createID(item.id.client, item.id.clock + 1)) } - return new WeakLink({parent, item, key: null}) + return new WeakLink(item.id, item) } index -= item.length } @@ -255,7 +181,7 @@ export const arrayWeakLink = (transaction, parent, index) => { export const mapWeakLink = (parent, key) => { const item = parent._map.get(key) if (item !== undefined) { - return new WeakLink({parent, item, key}) + return new WeakLink(item.id, item) } else { return undefined } diff --git a/src/structs/Item.js b/src/structs/Item.js index ce06345d..196e9179 100644 --- a/src/structs/Item.js +++ b/src/structs/Item.js @@ -302,7 +302,7 @@ export class Item extends AbstractStruct { * If this item was referenced by other weak links, here we keep the references * to these weak refs. * - * @type {Set> | null} + * @type {Set | null} */ this.linkedBy = null /** @@ -388,13 +388,8 @@ export class Item extends AbstractStruct { } if (this.content.constructor === ContentLink) { const content = /** @type {ContentLink} */ (this.content) - if (content.raw !== null) { - const { type, item } = content.raw - if (type !== null && type.client !== this.id.client) { - return type.client - } else if (item !== null && item.client !== this.id.client) { - return item.client - } + if (content.link.id.client !== this.id.client) { + return content.link.id.client } } @@ -543,6 +538,11 @@ export class Item extends AbstractStruct { this.content.integrate(transaction, this) // add parent to transaction.changed addChangedTypeToTransaction(transaction, /** @type {AbstractType} */ (this.parent), this.parentSub) + if (this.linkedBy !== null) { + for (let link of this.linkedBy) { + addChangedTypeToTransaction(transaction, /** @type {AbstractType} */ (link.parent), this.parentSub) + } + } if ((/** @type {AbstractType} */ (this.parent)._item !== null && /** @type {AbstractType} */ (this.parent)._item.deleted) || (this.parentSub !== null && this.right !== null)) { // delete if parent is deleted or if this is not the current attribute value of parent this.delete(transaction) diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js index 9aaa95c9..4f9830c8 100644 --- a/src/types/AbstractType.js +++ b/src/types/AbstractType.js @@ -670,7 +670,7 @@ export const typeListInsertGenericsAfter = (transaction, parent, referenceItem, left.integrate(transaction, 0) break case WeakLink: - left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentLink(/** @type {WeakLink} */ (c), null)) + left = new Item(createID(ownClientId, getState(store, ownClientId)), left, left && left.lastId, right, right && right.id, parent, null, new ContentLink(/** @type {WeakLink} */ (c))) left.integrate(transaction, 0) break default: @@ -856,7 +856,7 @@ export const typeMapSet = (transaction, parent, key, value) => { content = new ContentDoc(/** @type {Doc} */ (value)) break case WeakLink: - content = new ContentLink(/** @type {WeakLink} */ (value), null) + content = new ContentLink(/** @type {WeakLink} */ (value)) break; default: if (value instanceof AbstractType) { diff --git a/src/types/WeakLink.js b/src/types/WeakLink.js index 2029b01c..c1b25a18 100644 --- a/src/types/WeakLink.js +++ b/src/types/WeakLink.js @@ -1,5 +1,4 @@ -import { AbstractType, GC, Item, createID } from "yjs" -import { typeMapGet } from "./AbstractType.js" +import { AbstractType, GC, ID, Item } from "yjs" /** * @template T @@ -8,10 +7,12 @@ import { typeMapGet } from "./AbstractType.js" */ export class WeakLink { /** - * @param {{parent:AbstractType,item:Item|GC|null,key:string|null}|null} source + * @param {ID} id + * @param {Item|GC|null} item */ - constructor(source) { - this.source = source + constructor(id, item) { + this.id = id + this.item = item } /** @@ -20,37 +21,19 @@ export class WeakLink { * @return {T|undefined} */ deref() { - const item = this.linkedItem() - if (item !== undefined && !item.deleted) { - return /** @type {Item} */ (item).content.getContent()[0] - } else { - return undefined - } - } - - /** - * Returns currently linked item to an underlying value existing somewhere on in the document. - * - * @return {Item|GC|undefined} - */ - linkedItem() { - if (this.source !== null) { - const source = this.source - if (source.key !== null) { - return source.parent._map.get(source.key) - } else if (source.item !== null && !source.item.deleted) { - return source.item + if (this.item !== null && this.item.constructor === Item) { + let item = this.item + if (item.parentSub !== null) { + // for map types go to the most recent one + while (item.right !== null) { + item = item.right + } + this.item = item + } + if (!item.deleted) { + return item.content.getContent()[0] } } - return undefined - } - - /** - * Checks if a linked content source has been deleted. - * - * @return {boolean} - */ - get deleted() { - return this.source === null || this.source.item === null || this.source.item.deleted + return undefined; } } diff --git a/tests/weakLinks.tests.js b/tests/weakLinks.tests.js index c5fd4f25..017ba631 100644 --- a/tests/weakLinks.tests.js +++ b/tests/weakLinks.tests.js @@ -5,7 +5,7 @@ import { init, compare } from './testHelper.js' /** * @param {t.TestCase} tc */ -export const testBasicMap = tc => { +const testBasicMap = tc => { const doc = new Y.Doc() const map = doc.getMap('map') @@ -24,7 +24,7 @@ export const testBasicMap = tc => { /** * @param {t.TestCase} tc */ -export const testBasicArray = tc => { +const testBasicArray = tc => { const { testConnector, array0, array1 } = init(tc, {users:2}) array0.insert(0, [1,2,3]) array0.insert(3, [array0.link(1)]) @@ -81,16 +81,12 @@ export const testDeleteWeakLink = tc => { const l1 = /** @type {Y.Map} */ (link1.deref()) const l0 = /** @type {Y.Map} */ (link0.deref()) t.compare(l1.get('a1'), l0.get('a1')) - t.compare(link0.deleted, false) - t.compare(link1.deleted, false) map1.delete('b') // delete links testConnector.flushAllMessages() // since links have been deleted, they no longer refer to any content - t.compare(link0.deleted, true) - t.compare(link1.deleted, true) t.compare(link0.deref(), undefined) t.compare(link1.deref(), undefined) } @@ -108,8 +104,6 @@ export const testDeleteSource = tc => { const link1 = /** @type {Y.WeakLink>} */ (map1.get('b')) let l1 = /** @type {Y.Map} */ (link1.deref()) let l0 = /** @type {Y.Map} */ (link0.deref()) - t.compare(link0.deleted, false) - t.compare(link1.deleted, false) t.compare(l1.get('a1'), l0.get('a1')) map1.delete('a') // delete source of the link @@ -117,8 +111,6 @@ export const testDeleteSource = tc => { testConnector.flushAllMessages() // since source have been deleted, links no longer refer to any content - t.compare(link0.deleted, true) - t.compare(link1.deleted, true) t.compare(link0.deref(), undefined) t.compare(link1.deref(), undefined) }