nextExpectedVersion & actualVersion weird when appending to empty stream

[daMupfel]

While using the client i found some weird corner cases that around appendToSTream when used on an empty stream.

Basically it does not seem to be possible based on the return message to identify that the stream was empty.

There are basically two different cases.

1. When appending an "empty" iterable on an empty stream:

    var result = client.appendToStream("foo", AppendToStreamOptions.get(), Collections.emptyIterator())
        .get();
    System.out.printf("NextExpectedRevision: %s%n", result.getNextExpectedRevision());
   // NextExpectedRevision: 1

2. When appending an event and provide an expected version on an empty stream:

    try {
      client
          .appendToStream(
              "foo",
              AppendToStreamOptions.get().expectedRevision(ExpectedRevision.expectedRevision(10)),
              EventDataBuilder.binary("bar", new byte[] {}).build())
          .get();
    } catch (ExecutionException e) {
      if (e.getCause() instanceof WrongExpectedVersionException) {
        System.out
            .printf("ActualRevision: %s%n", ((WrongExpectedVersionException) e.getCause()).getActualVersion());
      }
    }
// ActualRevision: 2

This seems rather odd to me since i would expect a token indicating that we are at the beginning of the stream (which is currently a bit problematic because a StreamVersion currently only holds an "unsigned" long.

By digging a bit deeper it looks like the two values are arbitrarily set by the client (in AppendToStream.java):

// Handling NO_STREAM result from EventStore. Around line nr. 46.
StreamRevision nextExpectedRevision;
if (success.getCurrentRevisionOptionCase() == StreamsOuterClass.AppendResp.Success.CurrentRevisionOptionCase.NO_STREAM) {
    nextExpectedRevision = new StreamRevision(1); // NO_STREAM
} else {
    nextExpectedRevision = new StreamRevision(success.getCurrentRevision());
}

and

// Handling NO_STREAM result from EventStore when expectation failed. Around line nr. 74
StreamRevision currentRevision;
if (wev.getCurrentRevisionOptionCase() == StreamsOuterClass.AppendResp.WrongExpectedVersion.CurrentRevisionOptionCase.NO_STREAM) {
    // TODO(jen20): This feels very wrong?
    currentRevision = new StreamRevision(2); //StreamState.Constants.NoStream
} else {
    currentRevision = new StreamRevision(wev.getCurrentRevision());
}

[YoEight]

On hold, as we are reviewing an RFC on this very subject.

[daMupfel]

Are there any news on this? We are currently waiting on a few of the open issues in order to migrate to a newer eventstore version (the other client is currently not an option for us).

[michael-schnell]

Any progress here?

[daMupfel]

It seems that some progress was made now. I did some tests with the latest snapshot version.

Regarding point 1 (see initial message): This still seems to be broken. It will return now 0 instead of 1 which is still not an empty stream. I assume we need to change the type of getNextExpectedRevision to the new ExpectedRevision type so we can represent an empty stream.

Regarding point 2: This works now as intended. Within the exception we now have the ExpectedRevision type which will return an ExpectedRevision.noStream() / NoStreamExpectedRevision.

[YoEight]

Change will happen at that line: https://github.com/EventStore/EventStoreDB-Client-Java/blob/trunk/db-client-java/src/main/java/com/eventstore/dbclient/AppendToStream.java#L52

The protobuf api does descriminate between NoStream and actual stream revision. A patch will come soon.

[dosomder]

@YoEight Any updates when the patch is coming?

[daMupfel]

Hi @YoEight :wave:. I was wondering if you guys would be up for this to be contributed via a PR?

I wasn't sure since it might contain breaking changes (depending on how it is implemented) and it seemed as it would be done soon anyway.

Best regards, David

[michael-schnell]

Unfortunately I'm a Java developer (very little C# knowledge)

[YoEight]

Sorry for the late response, the patch is there: https://github.com/EventStore/EventStoreDB-Client-Java/pull/196

[dosomder]

Thank you @YoEight for the patch. Would you know when we can roughly expect a release with this fix included?

[YoEight]

Once reviewed, soon-ish. I need to double-check a few things but I don't think there is anything preventing the v4 release.

[daMupfel]

Thank you very much! We are looking forward to this landing :).

[YoEight]

Client version 4.0.0 is available on Maven: https://central.sonatype.dev/artifact/com.eventstore/db-client-java/4.0.0