From 1b47e58755a0427dc09d5e4c2d70962eb95d392b Mon Sep 17 00:00:00 2001 From: point Date: Tue, 16 Apr 2024 13:42:28 +0300 Subject: [PATCH] Fix encoding/decoding bugs. Check deleted items through encoding/decoding --- lib/y/decoder.ex | 27 ++++--- lib/y/decoder/state.ex | 8 +- lib/y/doc.ex | 2 +- lib/y/encoder.ex | 39 +++++---- lib/y/encoder/buffer.ex | 15 +--- lib/y/encoder/operations.ex | 2 +- lib/y/item.ex | 2 +- lib/y/transaction.ex | 2 +- lib/y/type/array.ex | 8 +- lib/y/type/array/array_tree.ex | 21 +++-- test/encoding_decoding_test.exs | 136 ++++++++++++++++++++++++++++++++ 11 files changed, 209 insertions(+), 53 deletions(-) diff --git a/lib/y/decoder.ex b/lib/y/decoder.ex index 2b48cf1..7bb8806 100644 --- a/lib/y/decoder.ex +++ b/lib/y/decoder.ex @@ -223,6 +223,9 @@ defmodule Y.Decoder do ) }) + {:skip, new_transaction} -> + do_integrate(rest_items, items_to_retry, %{internal_state | transaction: new_transaction}) + err -> Logger.warning("Failed to integrate single item", item: item, error: err) do_integrate(rest_items, items_to_retry, internal_state) @@ -312,9 +315,16 @@ defmodule Y.Decoder do {c, s, buf} = if count == 0 do {s, buf} = read_int(buf) - c = 1 - {s, {c, buf}} = if s < 0, do: {-s, read_uint(buf)}, else: {s, {c, buf}} - {c + 2, s, buf} + + {s, {c, buf}} = + if s < 0 do + {c, buf} = read_uint(buf) + {-s, {c + 2, buf}} + else + {s, {1, buf}} + end + + {c, s, buf} else {count, s, buf} end @@ -352,7 +362,7 @@ defmodule Y.Decoder do {c, buf} = read_uint(buf) {c + 2, -s, buf} else - {c, s, buf} + {1, s, buf} end else {c, s, buf} @@ -373,14 +383,13 @@ defmodule Y.Decoder do {diff, buf} = read_int(buf) has_count? = (diff &&& 1) != 0 diff = floor(diff / 2) - c = 1 {c, buf} = if has_count? do {c, buf} = read_uint(buf) {c + 2, buf} else - {c, buf} + {1, buf} end {c, diff, buf} @@ -391,9 +400,9 @@ defmodule Y.Decoder do s = s + diff if which_clock? == :left do - {s, %{state | left_clock: %{state.left_clock | buf: buf, s: s, count: c, diff: diff}}} + {s, %{state | left_clock: %{state.left_clock | buf: buf, s: s, count: c - 1, diff: diff}}} else - {s, %{state | right_clock: %{state.right_clock | buf: buf, s: s, count: c, diff: diff}}} + {s, %{state | right_clock: %{state.right_clock | buf: buf, s: s, count: c - 1, diff: diff}}} end end @@ -422,7 +431,7 @@ defmodule Y.Decoder do # read deleted content defp read_content(1, state) do - {len, state} = State.read_and_advance(state, :rest, &read_uint/1) + {len, state} = read_len(state) {Y.Content.Deleted.new(len), state} end diff --git a/lib/y/decoder/state.ex b/lib/y/decoder/state.ex index 14c13a7..6e67070 100644 --- a/lib/y/decoder/state.ex +++ b/lib/y/decoder/state.ex @@ -58,15 +58,15 @@ defmodule Y.Decoder.State do end def read_ds_clock(%State{} = state, f) do - {clock, state} = f.(Map.fetch!(state, :rest)) + {clock, new_msg} = f.(Map.fetch!(state, :rest)) clock = state.delete_set_cur_val + clock - {clock, %{state | delete_set_cur_val: clock}} + {clock, %{state | delete_set_cur_val: clock, rest: new_msg}} end def read_ds_len(%State{} = state, f) do - {diff, state} = f.(Map.fetch!(state, :rest)) + {diff, new_msg} = f.(Map.fetch!(state, :rest)) diff = diff + 1 - {diff, %{state | delete_set_cur_val: state.delete_set_cur_val + diff}} + {diff, %{state | delete_set_cur_val: state.delete_set_cur_val + diff, rest: new_msg}} end def reset_ds_cur_val(%State{} = state), do: %{state | delete_set_cur_val: 0} diff --git a/lib/y/doc.ex b/lib/y/doc.ex index 53f7abb..50cb0d2 100644 --- a/lib/y/doc.ex +++ b/lib/y/doc.ex @@ -186,7 +186,7 @@ defmodule Y.Doc do |> Map.values() |> Enum.flat_map(fn type -> type - |> Type.to_list(as_items: true) + |> Type.to_list(as_items: true, with_deleted: true) |> Enum.filter(fn %_{id: %ID{client: c}} -> c == client end) |> Enum.sort_by(fn %_{id: %ID{clock: clock}} -> clock end) end) diff --git a/lib/y/encoder.ex b/lib/y/encoder.ex index fc0be17..b4b850a 100644 --- a/lib/y/encoder.ex +++ b/lib/y/encoder.ex @@ -40,22 +40,29 @@ defmodule Y.Encoder do end Enum.reduce(sm, buffer, fn {client, clock}, buffer -> - [%_{id: %ID{clock: f_clock}} | _] = items = Doc.items_of_client!(doc, client) - - clock = max(clock, f_clock) - - buffer - |> write(:rest, write_uint(length(items))) - |> write(:client, client) - |> write(:rest, write_uint(clock)) - |> then(fn buf -> - items - |> Enum.with_index() - |> Enum.reduce(buf, fn - {item, 0}, buf -> write_item(buf, item, clock - f_clock) - {item, _}, buf -> write_item(buf, item) - end) - end) + case Doc.items_of_client!(doc, client) do + [] -> + buffer + |> write(:rest, write_uint(0)) + |> write(:client, client) + |> write(:rest, write_uint(clock)) + + [%_{id: %ID{clock: f_clock}} | _] = items -> + clock = max(clock, f_clock) + + buffer + |> write(:rest, write_uint(length(items))) + |> write(:client, client) + |> write(:rest, write_uint(clock)) + |> then(fn buf -> + items + |> Enum.with_index() + |> Enum.reduce(buf, fn + {item, 0}, buf -> write_item(buf, item, clock - f_clock) + {item, _}, buf -> write_item(buf, item) + end) + end) + end end) end diff --git a/lib/y/encoder/buffer.ex b/lib/y/encoder/buffer.ex index e447e12..a5c3644 100644 --- a/lib/y/encoder/buffer.ex +++ b/lib/y/encoder/buffer.ex @@ -62,7 +62,8 @@ defmodule Y.Encoder.Buffer do end def write(%Buffer{} = b, :ds_length, length) do - %{b | delete_set_cur_val: b.delete_set_cur_val + length} |> write(:rest, length - 1) + %{b | delete_set_cur_val: b.delete_set_cur_val + length} + |> write(:rest, write_uint(length - 1)) end def write(%Buffer{} = b, key, what) do @@ -87,18 +88,6 @@ defmodule Y.Encoder.Buffer do |> Kernel.<>(write_bitstring(type_ref)) |> Kernel.<>(Bufferable.dump(b.length) |> write_bitstring()) |> Kernel.<>(b.rest) - - # <> - # Bufferable.dump(b.client) <> - # Bufferable.dump(b.left_clock) <> - # Bufferable.dump(b.right_clock) <> - # Bufferable.dump(b.info) <> - # <> <> - # <<>> - # Bufferable.dump(b.parent_info) <> - # <> <> - # Bufferable.dump(b.length) <> - # <> end end diff --git a/lib/y/encoder/operations.ex b/lib/y/encoder/operations.ex index 48032df..652b9bd 100644 --- a/lib/y/encoder/operations.ex +++ b/lib/y/encoder/operations.ex @@ -10,9 +10,9 @@ defmodule Y.Encoder.Operations do end def write_int(acc \\ <<>>, num) do - cont? = if num > 63, do: 128, else: 0 neg? = if num < 0, do: 64, else: 0 num = if num < 0, do: -num, else: num + cont? = if num > 63, do: 128, else: 0 acc = acc <> <> acc |> do_write_int(floor(num / 64)) diff --git a/lib/y/item.ex b/lib/y/item.ex index 46f2d66..ecd94e5 100644 --- a/lib/y/item.ex +++ b/lib/y/item.ex @@ -174,7 +174,7 @@ defmodule Y.Item do ) :otherwise -> - {:error, "Cannot integrate item"} + {:error, "Cannot integrate item. Left item or right item missing"} end else {:invalid, transaction} diff --git a/lib/y/transaction.ex b/lib/y/transaction.ex index 47ac920..404b885 100644 --- a/lib/y/transaction.ex +++ b/lib/y/transaction.ex @@ -76,7 +76,7 @@ defmodule Y.Transaction do end defp put_deleted_to_delete_set(delete_set, type, type_before) do - Enum.reduce(type, delete_set, fn + Enum.reduce(Type.to_list(type, as_items: true, with_deleted: true), delete_set, fn %Item{deleted?: true} = item, ds -> case Type.find(type_before, item.id) do %Item{deleted?: false} -> diff --git a/lib/y/type/array.ex b/lib/y/type/array.ex index 8f020ca..4b40d64 100644 --- a/lib/y/type/array.ex +++ b/lib/y/type/array.ex @@ -214,8 +214,12 @@ defmodule Y.Type.Array do end end - def add_before(%Array{tree: tree} = array, nil, %Item{} = item), - do: {:ok, %{array | tree: ArrayTree.put(tree, 0, item)}} + def add_before(%Array{tree: tree} = array, nil, %Item{} = item) do + case ArrayTree.put(tree, 0, item) do + {:ok, tree} -> {:ok, %{array | tree: tree}} + err -> err + end + end def add_before(%Array{tree: tree} = array, %Item{} = before_item, %Item{} = item) do case ArrayTree.add_before(tree, before_item, item) do diff --git a/lib/y/type/array/array_tree.ex b/lib/y/type/array/array_tree.ex index 898d8aa..c596b6f 100644 --- a/lib/y/type/array/array_tree.ex +++ b/lib/y/type/array/array_tree.ex @@ -46,7 +46,14 @@ defmodule Y.Type.Array.ArrayTree do end def put(%ArrayTree{ft: %EmptyTree{} = tree} = array_tree, _index, %Item{} = item) do - {:ok, %ArrayTree{array_tree | ft: FingerTree.cons(tree, item)}} + items = Item.explode(item) |> Enum.reverse() + + tree = + Enum.reduce(items, tree, fn item, tree -> + FingerTree.cons(tree, item) + end) + + {:ok, %ArrayTree{array_tree | ft: tree}} end def put( @@ -137,7 +144,11 @@ defmodule Y.Type.Array.ArrayTree do def to_list(%ArrayTree{ft: tree}), do: FingerTree.to_list(tree) - def find(%ArrayTree{ft: tree}, id, default \\ nil) do + def find(tree, id, default \\ nil) + + def find(%ArrayTree{ft: %EmptyTree{}}, _id, default), do: default + + def find(%ArrayTree{ft: tree}, id, default) do {l, v, _} = FingerTree.split(tree, fn %{highest_clocks: clocks} -> case Map.fetch(clocks, id.client) do @@ -208,12 +219,12 @@ defmodule Y.Type.Array.ArrayTree do end end - def do_transform(nil, left_tree, right_tree, _acc, _fun), do: {left_tree, right_tree} + defp do_transform(nil, left_tree, right_tree, _acc, _fun), do: {left_tree, right_tree} - def do_transform(_, left_tree, nil, _acc, _fun), + defp do_transform(_, left_tree, nil, _acc, _fun), do: {left_tree, nil} - def do_transform(%Item{} = item, left_tree, right_tree, acc, fun) do + defp do_transform(%Item{} = item, left_tree, right_tree, acc, fun) do case fun.(item, acc) do {%Item{} = new_item, new_acc} -> do_transform( diff --git a/test/encoding_decoding_test.exs b/test/encoding_decoding_test.exs index 8901c38..e3fdaf4 100644 --- a/test/encoding_decoding_test.exs +++ b/test/encoding_decoding_test.exs @@ -47,6 +47,38 @@ defmodule Y.EncodingDecodingTest do }} = Doc.get(transaction, "array") end + test "decode js message with delete set" do + msg = + <<0, 0, 6, 229, 230, 215, 150, 3, 1, 2, 8, 2, 0, 5, 8, 0, 129, 0, 136, 7, 5, 97, 114, 114, + 97, 121, 5, 1, 1, 0, 3, 5, 1, 4, 1, 3, 0, 125, 0, 125, 1, 125, 2, 125, 3, 125, 4, 125, 6, + 125, 7, 125, 8, 125, 9, 1, 165, 243, 171, 203, 1, 1, 5, 0>> + + {:ok, doc} = Doc.new(name: :decode_message_from_js_ds) + {:ok, _array} = Doc.get_array(doc, "array") + + assert {:ok, _} = + Doc.transact(doc, fn transaction -> + transaction = Decoder.decode(msg, transaction) + {:ok, transaction} + end) + + {:ok, array} = Doc.get(doc, "array") + assert [0, 1, 2, 3, 4, 6, 7, 8, 9] = Array.to_list(array) + + with_deleted = Array.to_list(array, as_items: true, with_deleted: true) + assert 10 = length(with_deleted) + + assert %Y.Item{ + id: %Y.ID{client: 426_441_125, clock: 5}, + length: 1, + content: %Y.Content.Deleted{len: 1}, + origin: %Y.ID{client: 426_441_125, clock: 4}, + right_origin: nil, + parent_name: "array", + deleted?: true + } = Enum.find(with_deleted, & &1.deleted?) + end + test "decode and integrate into separate array", %{js_msg: js_msg} do {:ok, doc} = Doc.new(name: :decode_into_separate) {:ok, array} = Doc.get_array(doc, "existing_array") @@ -277,4 +309,108 @@ defmodule Y.EncodingDecodingTest do end) |> Array.to_list(as_items: true) end + + test "check delete sets, one element" do + {:ok, doc1} = Doc.new(name: :encode_origin2) + {:ok, doc2} = Doc.new(name: :decode_origin2) + {:ok, array1} = Doc.get_array(doc1, "array") + {:ok, _array2} = Doc.get_array(doc2, "array") + + msg = + doc1 + |> Doc.transact!(fn transaction -> + {:ok, _array, transaction} = + Array.put(array1, transaction, 0, 0) + + {:ok, transaction} + end) + |> Encoder.encode() + + doc2 + |> Doc.transact!(fn transaction -> + transaction = Decoder.decode(msg, transaction) + {:ok, transaction} + end) + + assert {:ok, _array2} = Doc.get(doc2, "array") + + msg2 = + doc1 + |> Doc.transact!(fn transaction -> + {:ok, array1} = Doc.get(transaction, "array") + + {:ok, _array, transaction} = + Array.delete(array1, transaction, 0) + + {:ok, transaction} + end) + |> Encoder.encode() + + doc2 + |> Doc.transact!(fn transaction -> + transaction = Decoder.decode(msg2, transaction) + {:ok, transaction} + end) + + assert {:ok, array2} = Doc.get(doc2, "array") + + assert [%Y.Item{deleted?: true, content: [0]}] = + Array.to_list(array2, as_items: true, with_deleted: true) + end + + test "check delete sets, few elements" do + {:ok, doc1} = Doc.new(name: :ds_few1) + {:ok, doc2} = Doc.new(name: :ds_few2) + {:ok, array1} = Doc.get_array(doc1, "array") + {:ok, _array2} = Doc.get_array(doc2, "array") + + msg = + doc1 + |> Doc.transact!(fn transaction -> + {:ok, _array, transaction} = + Array.put_many(array1, transaction, 0, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]) + + {:ok, transaction} + end) + |> Encoder.encode() + + doc2 + |> Doc.transact!(fn transaction -> + transaction = Decoder.decode(msg, transaction) + {:ok, transaction} + end) + + assert {:ok, array2} = Doc.get(doc2, "array") + assert Enum.to_list(0..9) == Array.to_list(array2) + + msg2 = + doc1 + |> Doc.transact!(fn transaction -> + {:ok, array1} = Doc.get(transaction, "array") + + {:ok, _array1, transaction} = + Array.delete(array1, transaction, 5) + + {:ok, transaction} + end) + |> Encoder.encode() + + doc2 + |> Doc.transact!(fn transaction -> + transaction = Decoder.decode(msg2, transaction) + {:ok, transaction} + end) + + assert {:ok, array2} = Doc.get(doc2, "array") + + {:ok, doc1_instance} = Doc.get_instance(doc1) + client_id = doc1_instance.client_id + + assert %Y.Item{content: [5], id: %{client: ^client_id}} = + array2 + |> Array.to_list(as_items: true, with_deleted: true) + |> Enum.find(& &1.deleted?) + + assert Enum.count(array2) == 9 + end end