Appending expecting revision 0 does not return an error

[fabriziosestito]

I am trying to integrate spear with commanded by strangling the Extreme library from the extreme_adapter.

Appending works, but I realized some test was not passing due to the expect: 0 being ignored.
The test tries to append 3 events and then it asserts that appending expecting a revision: 0 returns an error.
Other assertions are true with revisions >= 1

Here is the test for reference:

      test "should fail to append to a stream because of wrong expected version", %{
        event_store: event_store,
        event_store_meta: event_store_meta
      } do
        assert :ok == event_store.append_to_stream(event_store_meta, "stream", 0, build_events(3))

        assert {:error, :wrong_expected_version} ==
                 event_store.append_to_stream(event_store_meta, "stream", 0, build_events(1))

        assert {:error, :wrong_expected_version} ==
                 event_store.append_to_stream(event_store_meta, "stream", 1, build_events(1))

        assert {:error, :wrong_expected_version} ==
                 event_store.append_to_stream(event_store_meta, "stream", 2, build_events(1))

        assert :ok == event_store.append_to_stream(event_store_meta, "stream", 3, build_events(1))
      end

where:

  defp expected_version(:any_version), do: :any
  defp expected_version(:no_stream), do: :empty
  defp expected_version(:stream_exists), do: :exists
  defp expected_version(0), do: :empty

This line seems to be the culprit:

spear/lib/spear/writing.ex

Line 72 in 9c8041a

defp map_expectation(revision) when is_integer(revision) and revision >= 1,

Removing the guard seems to do the trick.

Is there any reason why the revision needs to be >= 1 while mapping the expectations?

Thanks in advance!

[the-mikedavis]

I am trying to integrate spear with commanded

Sweet! Thanks for working on that 😀

Is there any reason why the revision needs to be >= 1 while mapping the expectations?

I was probably thinking at the time that if you wanted to expect 0 you could use :empty but I don't see a problem with expecting 0 now that you mention it. Would you like to submit a PR to change that guard? I think having it be revision >= 0 would be my preference rather than just removing it - that field in the protobuf definitions is an unsigned int so I would like to protect against negative values /~https://github.com/EventStore/EventStore/blob/2604dea053a394864d94acdd48d90929cac30601/src/Protos/Grpc/streams.proto#L144

[fabriziosestito]

@the-mikedavis thanks for the fast answer, I baked a small PR as you suggested.
I see the same guard being used for the batch append requests: /~https://github.com/fabriziosestito/spear/blob/85c0d6e9319c2e1b9caa8fd8c4c9d5a42971fa91/lib/spear/writing.ex#L138

Should we change this one as well?

[the-mikedavis]

Oh yeah let's allow 0 there as well 👍

[fabriziosestito]

@the-mikedavis force-pushed, done!