Persistence API: add CurrentLastSequenceNumberByPersistenceIdQuery #1773
Conversation
| trait CurrentLastKnownSequenceNumberByPersistenceIdQuery extends ReadJournal { | ||
|
|
||
| /** | ||
| * Returns the last known sequence number for the given `persistenceId`. Empty if the `persistenceId` is unknown. |
There was a problem hiding this comment.
Optional should use the value is absent instead of Empty?
There was a problem hiding this comment.
I think 'empty' makes since this is the terminology used by scala.Option and java Optional.
| * A trait that enables querying the current last known sequence number for a given `persistenceId`. | ||
| * @since 1.2.0 | ||
| */ | ||
| trait CurrentLastKnownSequenceNumberByPersistenceIdQuery extends ReadJournal { |
There was a problem hiding this comment.
LastKnownSequenceNumberQuery ?
There was a problem hiding this comment.
Just to highlight the intent behind the Current in name: It seemed to me that "final" queries like CurrentEventsByPersistenceIdQuery are prefixed with 'Current', while continuous corresponding queries like EventsByPersistenceIdQuery drop this prefix. The ByPersistenceId clarifies the scope of query. Even though a LastKnownSequenceNumberByPersistenceIdQuery doesn't exist yet, one could imagine a query that continuously emits the last known sequence number for persistence id in future and thus I had preferred to 1. not block the name already 2. align to existing naming schema.
There was a problem hiding this comment.
I just think the name is too long, maybe CurrentLastKnownSequenceNumberQuery, otherwise , if we need add more methods to this trait, then we need another trait.
There was a problem hiding this comment.
How about dropping the Known from the name? Current already implies it in a way. EventSourcedBehavior.lastSequenceNumber already exist, so naming would align.
There was a problem hiding this comment.
Would CurrentLastSequenceNumberQuery work?
There was a problem hiding this comment.
I think CurrentLastSequenceNumberByPersistenceIdQuery is good if we are sticking to the style of having a trait per least amount of methods, which does make sense - for example each journal might want to implement CurrentLastSequenceNumberByPersistenceIdQuery, but not necessarily CurrentLastSequenceNumberBySliceQuery or CurrentLastSequenceNumberByTagQuery if we ever implement them.
pjfanning
changed the title
Based on apache/pekko-persistence-jdbc#267