Deprecate raw initial_bookmarks for bookmark_manager (#1196)

Deprecate passing raw sting bookmarks as `initial_bookmarks` to
`GraphDatabase.bookmark_manager()`.
Use a `neo4j.Bookmarks` object instead.

Fixup changelog & type hints amending
#1175

Author: robsdedude

CHANGELOG.md

@@ -62,8 +62,13 @@ See also https://github.com/neo4j/neo4j-python-driver/wiki for a full changelog.
     - Change string representation of `Neo4jError` to include GQL error information.
 - Remove deprecated `Record.__getslice__`. This magic method has been removed in Python 3.0.  
   If you were calling it directly, please use `Record.__getitem__(slice(...))` or simply `record[...]` instead.
-- Remove deprecated class `neo4j.Bookmark` in favor of `neo4j.Bookmarks`.
-- Remove deprecated class `session.last_bookmark()` in favor of `last_bookmarks()`.
+- Bookmarks
+  - Remove deprecated class `neo4j.Bookmark` in favor of `neo4j.Bookmarks`.
+  - Remove deprecated class `session.last_bookmark()` in favor of `last_bookmarks()`.
+  - Deprecate passing raw sting bookmarks as `initial_bookmarks` to `GraphDatabase.bookmark_manager()`.  
+    Use a `neo4j.Bookmarks` object instead.
+  - `Driver.session()` no longer accepts raw string bookmarks as `bookmarks` argument.  
+    Use a `neo4j.Bookmarks` object instead.
 - Remove deprecated `ServerInfo.connection_id`.  
   There is no replacement as this is considered internal information.
 - Remove deprecated driver configuration option `trust`.  
@@ -234,17 +239,17 @@ See also https://github.com/neo4j/neo4j-python-driver/wiki for a full changelog.
   manager related methods:
   - `neo4j.BookmarkManger` and `neo4j.AsyncBookmarkManger` abstract base
     classes:
-    - ``update_bookmarks`` has no longer a ``database`` argument.
-    - ``get_bookmarks`` has no longer a ``database`` argument.
-    - The ``get_all_bookmarks`` method was removed.
-    - The ``forget`` method was removed.
+    - `update_bookmarks` has no longer a `database` argument.
+    - `get_bookmarks` has no longer a `database` argument.
+    - The `get_all_bookmarks` method was removed.
+    - The `forget` method was removed.
   - `neo4j.GraphDatabase.bookmark_manager` and
     `neo4j.AsyncGraphDatabase.bookmark_manager` factory methods:
-    - ``initial_bookmarks`` is no longer a mapping from database name
+    - `initial_bookmarks` is no longer a mapping from database name
       to bookmarks but plain bookmarks.
-    - ``bookmarks_supplier`` no longer receives the database name as
+    - `bookmarks_supplier` no longer receives the database name as
       an argument.
-    - ``bookmarks_consumer`` no longer receives the database name as
+    - `bookmarks_consumer` no longer receives the database name as
       an argument.  
 
 

src/neo4j/_async/bookmark_manager.py

@@ -30,18 +30,10 @@
 TBmConsumer = t.Callable[[Bookmarks], None | t.Awaitable[None]]
 
 
-def _bookmarks_to_set(
-    bookmarks: Bookmarks | t.Iterable[str],
-) -> set[str]:
-    if isinstance(bookmarks, Bookmarks):
-        return set(bookmarks.raw_values)
-    return set(map(str, bookmarks))
-
-
 class AsyncNeo4jBookmarkManager(AsyncBookmarkManager):
     def __init__(
         self,
-        initial_bookmarks: Bookmarks | t.Iterable[str] | None = None,
+        initial_bookmarks: Bookmarks | None = None,
         bookmarks_supplier: TBmSupplier | None = None,
         bookmarks_consumer: TBmConsumer | None = None,
     ) -> None:
@@ -51,13 +43,7 @@ def __init__(
         if not initial_bookmarks:
             self._bookmarks = set()
         else:
-            if not hasattr(initial_bookmarks, "raw_values"):
-                initial_bookmarks = Bookmarks.from_raw_values(
-                    t.cast(t.Iterable[str], initial_bookmarks)
-                )
-            self._bookmarks = set(
-                t.cast(Bookmarks, initial_bookmarks).raw_values
-            )
+            self._bookmarks = set(initial_bookmarks.raw_values)
         self._lock = AsyncCooperativeLock()
 
     async def update_bookmarks(

src/neo4j/_async/driver.py

@@ -18,6 +18,7 @@
 
 import asyncio
 import typing as t
+from types import NoneType
 
 
 if t.TYPE_CHECKING:
@@ -51,6 +52,7 @@
 )
 from .._debug import ENABLED as DEBUG_ENABLED
 from .._warnings import (
+    deprecation_warn,
     preview_warn,
     unclosed_resource_warn,
 )
@@ -335,6 +337,11 @@ def bookmark_manager(
         :param initial_bookmarks:
             The initial set of bookmarks. The returned bookmark manager will
             use this to initialize its internal bookmarks.
+
+            .. deprecated:: 6.0
+                Passing raw string bookmarks is deprecated.
+                Use a :class:`.Bookmarks` object instead.
+
         :param bookmarks_supplier:
             Function which will be called every time the default bookmark
             manager's method :meth:`.AsyncBookmarkManager.get_bookmarks`
@@ -367,9 +374,27 @@ def bookmark_manager(
               an argument.
 
         .. versionchanged:: 5.8 Stabilized from experimental.
+
+        .. versionchanged:: 6.0
+            Deprecated passing raw string bookmarks as initial_bookmarks.
         """
+        cast_initial_bookmarks: Bookmarks | None
+        # TODO: 7.0 - remove raw bookmark support
+        if not isinstance(initial_bookmarks, (Bookmarks, NoneType)):
+            deprecation_warn(
+                (
+                    "Passing raw strings as initial_bookmarks is deprecated. "
+                    "Use a Bookmarks object instead."
+                ),
+                stack_level=2,
+            )
+            cast_initial_bookmarks = Bookmarks.from_raw_values(
+                t.cast(t.Iterable[str], initial_bookmarks)
+            )
+        else:
+            cast_initial_bookmarks = initial_bookmarks
         return AsyncNeo4jBookmarkManager(
-            initial_bookmarks=initial_bookmarks,
+            initial_bookmarks=cast_initial_bookmarks,
             bookmarks_supplier=bookmarks_supplier,
             bookmarks_consumer=bookmarks_consumer,
         )
@@ -520,7 +545,7 @@ def session(
             database: str | None = ...,
             fetch_size: int = ...,
             impersonated_user: str | None = ...,
-            bookmarks: t.Iterable[str] | Bookmarks | None = ...,
+            bookmarks: Bookmarks | None = ...,
             default_access_mode: str = ...,
             bookmark_manager: (
                 AsyncBookmarkManager | BookmarkManager | None

src/neo4j/_sync/bookmark_manager.py

@@ -624,7 +624,11 @@ async def new_bookmark_manager(backend, data):
 
     bmm_kwargs = {}
     data.mark_item_as_read("initialBookmarks", recursive=True)
-    bmm_kwargs["initial_bookmarks"] = data.get("initialBookmarks")
+    initial_bookmarks = data.get("initialBookmarks")
+    if initial_bookmarks is not None:
+        bmm_kwargs["initial_bookmarks"] = neo4j.Bookmarks.from_raw_values(
+            initial_bookmarks
+        )
     if data.get("bookmarksSupplierRegistered"):
         bmm_kwargs["bookmarks_supplier"] = bookmarks_supplier(
             backend, bookmark_manager_id

src/neo4j/_sync/driver.py

@@ -17,6 +17,7 @@
 from __future__ import annotations
 
 import typing as t
+from types import NoneType
 
 import pytest
 
@@ -34,6 +35,9 @@
 
 @copy_signature(neo4j.AsyncGraphDatabase.bookmark_manager)
 def bookmark_manager(*args, **kwargs):
+    if not isinstance(kwargs.get("initial_bookmarks"), (NoneType, Bookmarks)):
+        with pytest.warns(DeprecationWarning, match="initial_bookmarks"):
+            return neo4j.AsyncGraphDatabase.bookmark_manager(*args, **kwargs)
     return neo4j.AsyncGraphDatabase.bookmark_manager(*args, **kwargs)