Make newly created session return no bookmarks #844
Conversation
robsdedude
force-pushed
the
no-bookmarks-from-new-session
branch
from
8407b47 to
97f3840
Compare
robsdedude
force-pushed
the
no-bookmarks-from-new-session
branch
from
97f3840 to
c93a64d
Compare
| _onCompleteCallback(meta: { bookmark?: string | string[] }): void { | ||
| this._onBookmark(new Bookmark(meta.bookmark)) | ||
| _onCompleteCallback(meta: { bookmark?: string }): void { | ||
| this._onBookmark(meta.bookmark) |
There was a problem hiding this comment.
Why change onBookmark signature?
There was a problem hiding this comment.
Because I want to fail fast should the server send multiple bookmarks. According to the protocol spec, there should only ever be 0 or 1 bookmark in a SUCCESS message.
There was a problem hiding this comment.
Also, thinking further down the call stack, I want _bookmarkOut to be a string. How do you turn a Bookmark (which can contain a string array) into a single string in a type safe way?
| _updateBookmark(newBookmark?: Bookmark): void { | ||
| if (newBookmark && !newBookmark.isEmpty()) { | ||
| this._lastBookmark = newBookmark | ||
| _updateBookmark(newBookmark?: string): void { | ||
| if (newBookmark !== undefined) { | ||
| this._bookmarkIn = new Bookmark(newBookmark) | ||
| this._bookmarkOut = newBookmark |
There was a problem hiding this comment.
Actually, the previous newBookmark will be always defined. So, it could be changed for if(!newBookmark.isEmpty())
There was a problem hiding this comment.
But what was the !newBookmark.isEmpty() there fore before if not to check if the SUCCESS message that triggered this update actually contains a bookmark?
| private _bookmarkIn: Bookmark | ||
| private _bookmarkOut?: string |
There was a problem hiding this comment.
Maybe it could be called currentBookmark and lastReceivedBookmark, or something like this. Because the _bookmarkIn is being changed when a new bookmark arrives.
There was a problem hiding this comment.
I'm fine with that 👌 Will do.
| /** | ||
| * Return the bookmark received following the last completed {@link Transaction}. | ||
| * | ||
| * @return {string[]} A reference to a previous transaction. | ||
| * @return {string | undefined} A reference to a previous transaction if present. | ||
| */ | ||
| lastBookmark(): string[] { | ||
| return this._lastBookmark.values() | ||
| lastBookmark(): string | undefined { | ||
| return this._bookmarkOut | ||
| } |
There was a problem hiding this comment.
Why did the interface changed?
There was a problem hiding this comment.
I saw a recent question from support regarding the Java driver: they wanted to know if they can safely assume that .lastBookmark would only ever return one bookmark. With this signature, there is no doubt about it.
|
Superseded by #848 |
Depends on neo4j-drivers/testkit#364