JustGivingArchive / JustGiving.EventStore.Http

An ES .Net HTTP client, with some subscriber stuff too
Apache License 2.0
27 stars 12 forks source link

manipulated sequence number does not work when projected streams with linked events are subscribed #12

Closed humblelistener closed 8 years ago

humblelistener commented 8 years ago

Current sequence number logic,

public int SequenceNumber
{
      get
      {
            var idString = Title.Split('@')[0];
            return int.Parse(idString);
      }
}

When subscribing to a stream that was created with linked events from several streams, the manipulation of sequence number based on title is not working.

The title in such cases point to title of the original event, which causes the StreamPositionRepository to go crazy as the Sequence number is used to persist the read position.

image

humblelistener commented 8 years ago

As also seen in the picture above, event store provides positionEventNumber field which is the actual sequence number. Using it instead of manipulation will solve the problem.

Here is the diff https://github.com/JustGiving/JustGiving.EventStore.Http/compare/master...PageUpPeopleOrg:feature/position-number

spadger commented 8 years ago

Good Spot. I don't want to pull your code because it could break any instances of IHandleEventsAndMetadataOf<> that reference SequenceNumber. I'd be happy to accept a pull request with PositionEventNumber, if you keep SequenceNumber availabe as well (proxying PositionEventNumber). Otherwise, I can just make the change myself today.

Thanks

humblelistener commented 8 years ago

I will check that.. also I am still fixing some unit tests. Can you point me to the place where IHandleEventsAndMetadataOf uses SequenceNumber?

spadger commented 8 years ago

It doesn't per se - it is an interface you can use to create event handlers, which also receives some metadata, as well as the event. I think we use it internally, for example in one of our project's event handlers

humblelistener commented 8 years ago

gotcha.. you don't want that contract to break. Makes sense. Update on the way

humblelistener commented 8 years ago

All set!