Skip to content

Commit

Permalink
Fix Notification.description polyfill from GqlStatusObject (#1071)
Browse files Browse the repository at this point in the history
Bolt 5.6 introduces the original notification description back at
the protocol level. This avoids the `Notification.description` changing
when connected to GQL aware servers.

This issues was detected during homologation, so the problem won't
happen with any released server since the bolt version missing the
information will not be released.
  • Loading branch information
bigmontz authored Jul 29, 2024
1 parent d6e2fa6 commit 8420fc4
Show file tree
Hide file tree
Showing 13 changed files with 1,465 additions and 35 deletions.
7 changes: 6 additions & 1 deletion src/neo4j/_async/io/_bolt.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ def protocol_handlers(cls, protocol_version=None):
AsyncBolt5x3,
AsyncBolt5x4,
AsyncBolt5x5,
AsyncBolt5x6,
)

handlers = {
Expand All @@ -299,6 +300,7 @@ def protocol_handlers(cls, protocol_version=None):
AsyncBolt5x3.PROTOCOL_VERSION: AsyncBolt5x3,
AsyncBolt5x4.PROTOCOL_VERSION: AsyncBolt5x4,
AsyncBolt5x5.PROTOCOL_VERSION: AsyncBolt5x5,
AsyncBolt5x6.PROTOCOL_VERSION: AsyncBolt5x6,
}

if protocol_version is None:
Expand Down Expand Up @@ -413,7 +415,10 @@ async def open(

# Carry out Bolt subclass imports locally to avoid circular dependency
# issues.
if protocol_version == (5, 5):
if protocol_version == (5, 6):
from ._bolt5 import AsyncBolt5x6
bolt_cls = AsyncBolt5x6
elif protocol_version == (5, 5):
from ._bolt5 import AsyncBolt5x5
bolt_cls = AsyncBolt5x5
elif protocol_version == (5, 4):
Expand Down
38 changes: 35 additions & 3 deletions src/neo4j/_async/io/_bolt5.py
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ def telemetry(self, api: TelemetryAPI, dehydration_hooks=None,
Response(self, "telemetry", hydration_hooks, **handlers),
dehydration_hooks=dehydration_hooks)


class AsyncBolt5x5(AsyncBolt5x4):

PROTOCOL_VERSION = Version(5, 5)
Expand Down Expand Up @@ -783,7 +784,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
("CURRENT_SCHEMA", "/"),
)

def _make_enrich_diagnostic_record_handler(self, wrapped_handler=None):
def _make_enrich_statuses_handler(self, wrapped_handler=None):
async def handler(metadata):
def enrich(metadata_):
if not isinstance(metadata_, dict):
Expand All @@ -794,6 +795,7 @@ def enrich(metadata_):
for status in statuses:
if not isinstance(status, dict):
continue
status["description"] = status.get("status_description")
diag_record = status.setdefault("diagnostic_record", {})
if not isinstance(diag_record, dict):
log.info("[#%04X] _: <CONNECTION> Server supplied an "
Expand All @@ -810,14 +812,44 @@ def enrich(metadata_):

def discard(self, n=-1, qid=-1, dehydration_hooks=None,
hydration_hooks=None, **handlers):
handlers["on_success"] = self._make_enrich_diagnostic_record_handler(
handlers["on_success"] = self._make_enrich_statuses_handler(
wrapped_handler=handlers.get("on_success")
)
super().discard(n, qid, dehydration_hooks, hydration_hooks, **handlers)

def pull(self, n=-1, qid=-1, dehydration_hooks=None, hydration_hooks=None,
**handlers):
handlers["on_success"] = self._make_enrich_diagnostic_record_handler(
handlers["on_success"] = self._make_enrich_statuses_handler(
wrapped_handler=handlers.get("on_success")
)
super().pull(n, qid, dehydration_hooks, hydration_hooks, **handlers)


class AsyncBolt5x6(AsyncBolt5x5):

PROTOCOL_VERSION = Version(5, 6)

def _make_enrich_statuses_handler(self, wrapped_handler=None):
async def handler(metadata):
def enrich(metadata_):
if not isinstance(metadata_, dict):
return
statuses = metadata_.get("statuses")
if not isinstance(statuses, list):
return
for status in statuses:
if not isinstance(status, dict):
continue
diag_record = status.setdefault("diagnostic_record", {})
if not isinstance(diag_record, dict):
log.info("[#%04X] _: <CONNECTION> Server supplied an "
"invalid diagnostic record (%r).",
self.local_port, diag_record)
continue
for key, value in self.DEFAULT_DIAGNOSTIC_RECORD:
diag_record.setdefault(key, value)

enrich(metadata)
await AsyncUtil.callback(wrapped_handler, metadata)

return handler
7 changes: 6 additions & 1 deletion src/neo4j/_sync/io/_bolt.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ def protocol_handlers(cls, protocol_version=None):
Bolt5x3,
Bolt5x4,
Bolt5x5,
Bolt5x6,
)

handlers = {
Expand All @@ -299,6 +300,7 @@ def protocol_handlers(cls, protocol_version=None):
Bolt5x3.PROTOCOL_VERSION: Bolt5x3,
Bolt5x4.PROTOCOL_VERSION: Bolt5x4,
Bolt5x5.PROTOCOL_VERSION: Bolt5x5,
Bolt5x6.PROTOCOL_VERSION: Bolt5x6,
}

if protocol_version is None:
Expand Down Expand Up @@ -413,7 +415,10 @@ def open(

# Carry out Bolt subclass imports locally to avoid circular dependency
# issues.
if protocol_version == (5, 5):
if protocol_version == (5, 6):
from ._bolt5 import Bolt5x6
bolt_cls = Bolt5x6
elif protocol_version == (5, 5):
from ._bolt5 import Bolt5x5
bolt_cls = Bolt5x5
elif protocol_version == (5, 4):
Expand Down
38 changes: 35 additions & 3 deletions src/neo4j/_sync/io/_bolt5.py
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ def telemetry(self, api: TelemetryAPI, dehydration_hooks=None,
Response(self, "telemetry", hydration_hooks, **handlers),
dehydration_hooks=dehydration_hooks)


class Bolt5x5(Bolt5x4):

PROTOCOL_VERSION = Version(5, 5)
Expand Down Expand Up @@ -783,7 +784,7 @@ def begin(self, mode=None, bookmarks=None, metadata=None, timeout=None,
("CURRENT_SCHEMA", "/"),
)

def _make_enrich_diagnostic_record_handler(self, wrapped_handler=None):
def _make_enrich_statuses_handler(self, wrapped_handler=None):
def handler(metadata):
def enrich(metadata_):
if not isinstance(metadata_, dict):
Expand All @@ -794,6 +795,7 @@ def enrich(metadata_):
for status in statuses:
if not isinstance(status, dict):
continue
status["description"] = status.get("status_description")
diag_record = status.setdefault("diagnostic_record", {})
if not isinstance(diag_record, dict):
log.info("[#%04X] _: <CONNECTION> Server supplied an "
Expand All @@ -810,14 +812,44 @@ def enrich(metadata_):

def discard(self, n=-1, qid=-1, dehydration_hooks=None,
hydration_hooks=None, **handlers):
handlers["on_success"] = self._make_enrich_diagnostic_record_handler(
handlers["on_success"] = self._make_enrich_statuses_handler(
wrapped_handler=handlers.get("on_success")
)
super().discard(n, qid, dehydration_hooks, hydration_hooks, **handlers)

def pull(self, n=-1, qid=-1, dehydration_hooks=None, hydration_hooks=None,
**handlers):
handlers["on_success"] = self._make_enrich_diagnostic_record_handler(
handlers["on_success"] = self._make_enrich_statuses_handler(
wrapped_handler=handlers.get("on_success")
)
super().pull(n, qid, dehydration_hooks, hydration_hooks, **handlers)


class Bolt5x6(Bolt5x5):

PROTOCOL_VERSION = Version(5, 6)

def _make_enrich_statuses_handler(self, wrapped_handler=None):
def handler(metadata):
def enrich(metadata_):
if not isinstance(metadata_, dict):
return
statuses = metadata_.get("statuses")
if not isinstance(statuses, list):
return
for status in statuses:
if not isinstance(status, dict):
continue
diag_record = status.setdefault("diagnostic_record", {})
if not isinstance(diag_record, dict):
log.info("[#%04X] _: <CONNECTION> Server supplied an "
"invalid diagnostic record (%r).",
self.local_port, diag_record)
continue
for key, value in self.DEFAULT_DIAGNOSTIC_RECORD:
diag_record.setdefault(key, value)

enrich(metadata)
Util.callback(wrapped_handler, metadata)

return handler
2 changes: 1 addition & 1 deletion src/neo4j/_work/summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def _set_notifications(self):
for notification_key, status_key in (
("title", "title"),
("code", "neo4j_code"),
("description", "status_description"),
("description", "description"),
):
value = status.get(status_key)
if not isinstance(value, str) or not value:
Expand Down
1 change: 1 addition & 0 deletions testkitbackend/test_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"Feature:Bolt:5.3": true,
"Feature:Bolt:5.4": true,
"Feature:Bolt:5.5": true,
"Feature:Bolt:5.6": true,
"Feature:Bolt:Patch:UTC": true,
"Feature:Impersonation": true,
"Feature:TLS:1.1": "Driver blocks TLS 1.1 for security reasons.",
Expand Down
12 changes: 7 additions & 5 deletions tests/unit/async_/io/test_class_bolt.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_class_method_protocol_handlers():
expected_handlers = {
(3, 0),
(4, 1), (4, 2), (4, 3), (4, 4),
(5, 0), (5, 1), (5, 2), (5, 3), (5, 4), (5, 5),
(5, 0), (5, 1), (5, 2), (5, 3), (5, 4), (5, 5), (5, 6),
}

protocol_handlers = AsyncBolt.protocol_handlers()
Expand Down Expand Up @@ -65,7 +65,8 @@ def test_class_method_protocol_handlers():
((5, 3), 1),
((5, 4), 1),
((5, 5), 1),
((5, 6), 0),
((5, 6), 1),
((5, 7), 0),
((6, 0), 0),
]
)
Expand All @@ -85,7 +86,7 @@ def test_class_method_protocol_handlers_with_invalid_protocol_version():
# [bolt-version-bump] search tag when changing bolt version support
def test_class_method_get_handshake():
handshake = AsyncBolt.get_handshake()
assert (b"\x00\x05\x05\x05\x00\x02\x04\x04\x00\x00\x01\x04\x00\x00\x00\x03"
assert (b"\x00\x06\x06\x05\x00\x02\x04\x04\x00\x00\x01\x04\x00\x00\x00\x03"
== handshake)


Expand Down Expand Up @@ -134,6 +135,7 @@ async def test_cancel_hello_in_open(mocker, none_auth):
((5, 3), "neo4j._async.io._bolt5.AsyncBolt5x3"),
((5, 4), "neo4j._async.io._bolt5.AsyncBolt5x4"),
((5, 5), "neo4j._async.io._bolt5.AsyncBolt5x5"),
((5, 6), "neo4j._async.io._bolt5.AsyncBolt5x6"),
),
)
@mark_async_test
Expand Down Expand Up @@ -166,14 +168,14 @@ async def test_version_negotiation(
(2, 0),
(4, 0),
(3, 1),
(5, 6),
(5, 7),
(6, 0),
))
@mark_async_test
async def test_failing_version_negotiation(mocker, bolt_version, none_auth):
supported_protocols = (
"('3.0', '4.1', '4.2', '4.3', '4.4', "
"'5.0', '5.1', '5.2', '5.3', '5.4', '5.5')"
"'5.0', '5.1', '5.2', '5.3', '5.4', '5.5', '5.6')"
)

address = ("localhost", 7687)
Expand Down
10 changes: 7 additions & 3 deletions tests/unit/async_/io/test_class_bolt5x5.py
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ async def test_tracks_last_database(fake_socket_pair, actions):
)
@pytest.mark.parametrize("method", ("pull", "discard"))
@mark_async_test
async def test_enriches_diagnostic_record(
async def test_enriches_statuses(
sent_diag_records,
method,
fake_socket_pair,
Expand All @@ -628,7 +628,9 @@ async def test_enriches_diagnostic_record(

sent_metadata = {
"statuses": [
{"diagnostic_record": r} if r is not ... else {}
{"status_description": "the status description", "diagnostic_record": r}
if r is not ...
else { "status_description": "the status description" }
for r in sent_diag_records
]
}
Expand All @@ -654,7 +656,9 @@ def extend_diag_record(r):
expected_diag_records = [extend_diag_record(r) for r in sent_diag_records]
expected_metadata = {
"statuses": [
{"diagnostic_record": r} if r is not ... else {}
{"status_description": "the status description", "description": "the status description", "diagnostic_record": r}
if r is not ...
else { "status_description": "the status description", "description": "the status description" }
for r in expected_diag_records
]
}
Expand Down
Loading

0 comments on commit 8420fc4

Please sign in to comment.