NFIBrokerage / spear

A sharp EventStoreDB v20+ client backed by Mint :yum:
https://hex.pm/packages/spear
Apache License 2.0
85 stars 14 forks source link

the :from option is actually inclusive #26

Closed the-mikedavis closed 3 years ago

the-mikedavis commented 3 years ago

testing out with volley, ran into that events specified in :from are returned by Spear.read_stream/3

iirc the documentation for :from says that this is exclusive which is not the case

the-mikedavis commented 3 years ago

So it actually appears as though subscriptions are exclusive but reads only through read_stream/3 are inclusive:

Subscriptions:

iex> Stream.iterate(0, &(&1 + 1)) |> Stream.map(&Spear.Event.new("asdf", &1)) |> Stream.take(3) |> Spear.append(conn, "clusivity_test", expect: :empty)
:ok
iex> {:ok, start_sub} = Spear.subscribe(conn, self(), "clusivity_test", from: :start)
{:ok, #Reference<0.3024145066.3358851076.197200>}
iex> event = receive(do: (event -> event))
%Spear.Event{body: 0, ..}
iex> flush
%Spear.Event{body: 1, ..}
%Spear.Event{body: 2, ..}
iex> Spear.cancel_subscription(conn, start_sub)
:ok
iex> 
iex> {:ok, from_0_sub} = Spear.subscribe(conn, self(), "clusivity_test", from: event)
{:ok, #Reference<0.3024145066.3358851073.196545>}
iex> flush
# n.b. body 0 is not sent to the subscriber, so it's exclusive
%Spear.Event{body: 1, ..}
%Spear.Event{body: 2, ..}

Reads:

iex> [event] = Spear.stream!(conn, "clusivity_test", from: :start) |> Enum.take(1)
%Spear.Event{body: 0, ..}
iex> Spear.stream!(conn, "clusivity_test", from: event) |> Enum.take(1)
# n.b. exclusive?!
%Spear.Event{body: 1, ..}
iex> {:ok, events} = Spear.read_stream(conn, "clusivity_test", from: event)
iex> events |> Enum.take(1)
# n.b. inclusive, matches ^event
%Spear.Event{body: 0, ..}

So it appears as though this is inconsistent between stream!/3 and read_stream/3.

the-mikedavis commented 3 years ago

two paths forward:

while it may be more consistent with subscriptions to make read_stream/3 exclusive, I lean towards the first option as you can always Stream.drop(events, 1) to get rid of the first event but you can't 'go back in time' to find the event provided (considering that some consumer may rely on the behavior of inclusivity to read that event

the-mikedavis commented 3 years ago

as long as the documentation is specific, I don't think it actually matters which one we go with

the-mikedavis commented 3 years ago

playing around with the nodejs client looks like inclusive is a consistent choice