Skip to content

Commit

Permalink
deduplicate stream inserts
Browse files Browse the repository at this point in the history
Also adjusts stream_insert to prepend the insert instead of expensively
appending it.

Fixes #2689.
Closes #3596.
Closes #3598.
  • Loading branch information
SteffenDE committed Dec 22, 2024
1 parent a00ba4f commit b46c102
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
26 changes: 23 additions & 3 deletions lib/phoenix_live_view/live_stream.ex
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ defmodule Phoenix.LiveView.LiveStream do
"stream :dom_id must return a function which accepts each item, got: #{inspect(dom_id)}"
end

items_list = for item <- items, do: {dom_id.(item), -1, item, opts[:limit]}
# We need to go through the items one time to map them into the proper insert tuple format.
# Conveniently, we reverse the list in this pass, which we need to in order to be consistent
# with manually calling stream_insert multiple times, as stream_insert prepends.
items_list =
for item <- items, reduce: [] do
items -> [{dom_id.(item), -1, item, opts[:limit]} | items]
end

%LiveStream{
ref: ref,
Expand Down Expand Up @@ -61,16 +67,30 @@ defmodule Phoenix.LiveView.LiveStream do
def insert_item(%LiveStream{} = stream, item, at, limit) do
item_id = stream.dom_id.(item)

%{stream | inserts: stream.inserts ++ [{item_id, at, item, limit}]}
%{stream | inserts: [{item_id, at, item, limit} | stream.inserts]}
end

defimpl Enumerable, for: LiveStream do
def count(%LiveStream{inserts: inserts}), do: {:ok, length(inserts)}

def member?(%LiveStream{}, _item), do: raise(RuntimeError, "not implemented")

def reduce(%LiveStream{inserts: inserts} = stream, acc, fun) do
def reduce(%LiveStream{} = stream, acc, fun) do
if stream.consumable? do
# the inserts are stored in reverse insert order, so we need to reverse them
# before rendering; we also remove duplicates to only use the most recent
# inserts, which, as the items are reversed, are first
{inserts, _} =
for {id, _, _, _} = insert <- stream.inserts, reduce: {[], MapSet.new()} do
{inserts, ids} ->
if MapSet.member?(ids, id) do
# skip duplicates
{inserts, ids}
else
{[insert | inserts], MapSet.put(ids, id)}
end
end

do_reduce(inserts, acc, fun)
else
raise ArgumentError, """
Expand Down
11 changes: 9 additions & 2 deletions test/phoenix_live_view/live_stream_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ defmodule Phoenix.LiveView.LiveStreamTest do

test "default dom_id" do
stream = LiveStream.new(:users, 0, [%{id: 1}, %{id: 2}], [])
assert stream.inserts == [{"users-1", -1, %{id: 1}, nil}, {"users-2", -1, %{id: 2}, nil}]
assert stream.inserts == [{"users-2", -1, %{id: 2}, nil}, {"users-1", -1, %{id: 1}, nil}]
end

test "custom dom_id" do
stream = LiveStream.new(:users, 0, [%{name: "u1"}, %{name: "u2"}], dom_id: &"u-#{&1.name}")
assert stream.inserts == [{"u-u1", -1, %{name: "u1"}, nil}, {"u-u2", -1, %{name: "u2"}, nil}]
assert stream.inserts == [{"u-u2", -1, %{name: "u2"}, nil}, {"u-u1", -1, %{name: "u1"}, nil}]
end

test "default dom_id without struct or map with :id" do
Expand All @@ -28,4 +28,11 @@ defmodule Phoenix.LiveView.LiveStreamTest do
LiveStream.new(:users, 0, [%{user_id: 1}, %{user_id: 2}], [])
end
end

test "inserts are deduplicated (last insert wins)" do
assert stream = LiveStream.new(:users, 0, [%{id: 1}, %{id: 2}], [])
stream = LiveStream.insert_item(stream, %{id: 2, updated: true}, -1, nil)
stream = %{stream | consumable?: true}
assert Enum.to_list(stream) == [{"users-1", %{id: 1}}, {"users-2", %{id: 2, updated: true}}]
end
end

0 comments on commit b46c102

Please sign in to comment.