Skip to content

Fix bookmark comparison #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions neo4j/v1/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,26 @@
RETRY_DELAY_MULTIPLIER = 2.0
RETRY_DELAY_JITTER_FACTOR = 0.2

BOOKMARK_PREFIX = "neo4j:bookmark:v1:tx"


def last_bookmark(b0, b1):
""" Return the latest of two bookmarks by looking for the maximum
integer value following the last colon in the bookmark string.
""" Return the latest of two bookmarks.
"""
n = [None, None]
_, _, n[0] = b0.rpartition(":")
_, _, n[1] = b1.rpartition(":")
for i in range(2):
try:
n[i] = int(n[i])
except ValueError:
raise ValueError("Invalid bookmark: {}".format(b0))
return b0 if n[0] > n[1] else b1
return b0 if _bookmark_value(b0) > _bookmark_value(b1) else b1


def _bookmark_value(b):
"""Return the int value of the given bookmark.
"""
if b is None or not b.startswith(BOOKMARK_PREFIX):
raise ValueError("Invalid bookmark: {}".format(b))

value_string = b[len(BOOKMARK_PREFIX):]
try:
return int(value_string)
except ValueError:
raise ValueError("Invalid bookmark: {}".format(b))


def retry_delay_generator(initial_delay, multiplier, jitter_factor):
Expand Down
8 changes: 4 additions & 4 deletions test/stub/scripts/bookmark_chain.script
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
!: AUTO INIT
!: AUTO RESET

C: RUN "BEGIN" {"bookmark": "bookmark:1", "bookmarks": ["bookmark:0", "bookmark:1"]}
C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx0", "neo4j:bookmark:v1:tx1"]}
PULL_ALL
S: SUCCESS {}
SUCCESS {}
C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:2", "bookmarks": ["bookmark:2"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx2", "bookmarks": ["neo4j:bookmark:v1:tx2"]}
SUCCESS {}

C: RUN "BEGIN" {"bookmark": "bookmark:2", "bookmarks": ["bookmark:2"]}
C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx2", "bookmarks": ["neo4j:bookmark:v1:tx2"]}
PULL_ALL
S: SUCCESS {}
SUCCESS {}
C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:3", "bookmarks": ["bookmark:3"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx3", "bookmarks": ["neo4j:bookmark:v1:tx3"]}
SUCCESS {}
10 changes: 5 additions & 5 deletions test/stub/scripts/bookmark_chain_with_autocommit.script
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
!: AUTO INIT
!: AUTO RESET

C: RUN "BEGIN" {"bookmark": "bookmark:1", "bookmarks": ["bookmark:1"]}
C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx1"]}
PULL_ALL
S: SUCCESS {}
SUCCESS {}
C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:2", "bookmarks": ["bookmark:2"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx2", "bookmarks": ["neo4j:bookmark:v1:tx2"]}
SUCCESS {}

C: RUN "RETURN 1" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:x", "bookmarks": ["bookmark:x"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx42", "bookmarks": ["neo4j:bookmark:v1:tx42"]}
SUCCESS {}

C: RUN "BEGIN" {"bookmark": "bookmark:2", "bookmarks": ["bookmark:2"]}
C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx2", "bookmarks": ["neo4j:bookmark:v1:tx2"]}
PULL_ALL
S: SUCCESS {}
SUCCESS {}
C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:3", "bookmarks": ["bookmark:3"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx3", "bookmarks": ["neo4j:bookmark:v1:tx3"]}
SUCCESS {}
2 changes: 1 addition & 1 deletion test/stub/scripts/return_1_in_tx.script
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ S: SUCCESS {"fields": ["1"]}

C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:1", "bookmarks": ["bookmark:1"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx1"]}
SUCCESS {}
6 changes: 3 additions & 3 deletions test/stub/scripts/return_1_in_tx_twice.script
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ S: SUCCESS {"fields": ["1"]}

C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:1", "bookmarks": ["bookmark:1"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx1"]}
SUCCESS {}

C: RUN "BEGIN" {"bookmark": "bookmark:1", "bookmarks": ["bookmark:1"]}
C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx1"]}
PULL_ALL
S: SUCCESS {"fields": []}
SUCCESS {}
Expand All @@ -30,5 +30,5 @@ S: SUCCESS {"fields": ["1"]}

C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:2", "bookmarks": ["bookmark:2"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx2", "bookmarks": ["neo4j:bookmark:v1:tx2"]}
SUCCESS {}
2 changes: 1 addition & 1 deletion test/stub/scripts/return_1_twice_in_tx.script
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ S: SUCCESS {"fields": ["x"]}

C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:1", "bookmarks": ["bookmark:1"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx1"]}
SUCCESS {}
4 changes: 2 additions & 2 deletions test/stub/scripts/return_2_in_tx.script
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
!: AUTO INIT
!: AUTO RESET

C: RUN "BEGIN" {"bookmark": "bookmark:1", "bookmarks": ["bookmark:1"]}
C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx1", "bookmarks": ["neo4j:bookmark:v1:tx1"]}
PULL_ALL
S: SUCCESS {"fields": []}
SUCCESS {}
Expand All @@ -14,5 +14,5 @@ S: SUCCESS {"fields": ["2"]}

C: RUN "COMMIT" {}
PULL_ALL
S: SUCCESS {"bookmark": "bookmark:2", "bookmarks": ["bookmark:2"]}
S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx2", "bookmarks": ["neo4j:bookmark:v1:tx2"]}
SUCCESS {}
18 changes: 9 additions & 9 deletions test/stub/test_bookmarking.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,31 +44,31 @@ def test_should_be_able_to_set_multiple_bookmarks(self):
with StubCluster({9001: "router.script"}):
uri = "bolt+routing://localhost:9001"
with GraphDatabase.driver(uri, auth=self.auth_token, encrypted=False) as driver:
with driver.session(bookmarks=[":1", ":2"]) as session:
assert session.last_bookmark() == ":2"
with driver.session(bookmarks=["neo4j:bookmark:v1:tx1", "neo4j:bookmark:v1:tx2"]) as session:
assert session.last_bookmark() == "neo4j:bookmark:v1:tx2"

def test_should_automatically_chain_bookmarks(self):
with StubCluster({9001: "router.script", 9004: "bookmark_chain.script"}):
uri = "bolt+routing://localhost:9001"
with GraphDatabase.driver(uri, auth=self.auth_token, encrypted=False) as driver:
with driver.session(access_mode=READ_ACCESS, bookmarks=["bookmark:0", "bookmark:1"]) as session:
with driver.session(access_mode=READ_ACCESS, bookmarks=["neo4j:bookmark:v1:tx0", "neo4j:bookmark:v1:tx1"]) as session:
with session.begin_transaction():
pass
assert session.last_bookmark() == "bookmark:2"
assert session.last_bookmark() == "neo4j:bookmark:v1:tx2"
with session.begin_transaction():
pass
assert session.last_bookmark() == "bookmark:3"
assert session.last_bookmark() == "neo4j:bookmark:v1:tx3"

def test_autocommit_transaction_does_not_break_chain(self):
with StubCluster({9001: "router.script", 9004: "bookmark_chain_with_autocommit.script"}):
uri = "bolt+routing://localhost:9001"
with GraphDatabase.driver(uri, auth=self.auth_token, encrypted=False) as driver:
with driver.session(access_mode=READ_ACCESS, bookmark="bookmark:1") as session:
with driver.session(access_mode=READ_ACCESS, bookmark="neo4j:bookmark:v1:tx1") as session:
with session.begin_transaction():
pass
assert session.last_bookmark() == "bookmark:2"
assert session.last_bookmark() == "neo4j:bookmark:v1:tx2"
session.run("RETURN 1").consume()
assert session.last_bookmark() == "bookmark:2"
assert session.last_bookmark() == "neo4j:bookmark:v1:tx2"
with session.begin_transaction():
pass
assert session.last_bookmark() == "bookmark:3"
assert session.last_bookmark() == "neo4j:bookmark:v1:tx3"