Skip to content

Commit

Permalink
Fix encoding/decoding bugs. Check deleted items through encoding/deco…
Browse files Browse the repository at this point in the history
…ding
  • Loading branch information
point committed Apr 16, 2024
1 parent 0d47038 commit 1b47e58
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 53 deletions.
27 changes: 18 additions & 9 deletions lib/y/decoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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}
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions lib/y/decoder/state.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion lib/y/doc.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 23 additions & 16 deletions lib/y/encoder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 2 additions & 13 deletions lib/y/encoder/buffer.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) <>
# <<byte_size(b.string), b.string::binary>> <>
# <<>>
# Bufferable.dump(b.parent_info) <>
# <<byte_size(type_ref), type_ref::binary>> <>
# Bufferable.dump(b.length) <>
# <<byte_size(b.rest), b.rest::binary>>
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/y/encoder/operations.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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 <> <<cont? ||| neg? ||| (63 &&& num)>>

acc |> do_write_int(floor(num / 64))
Expand Down
2 changes: 1 addition & 1 deletion lib/y/item.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion lib/y/transaction.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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} ->
Expand Down
8 changes: 6 additions & 2 deletions lib/y/type/array.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions lib/y/type/array/array_tree.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit 1b47e58

Please sign in to comment.