Fix internal transaction method forwarding (#1017)

Forwarding methods shouldn't be decorated with `functools.wraps` as it not only
copies the docs string and type annotations (desirable), but also `__name__`,
`__qualname__`, and more (undesirable).

While `wraps` can be configured with what properties to copy, it turns out that
there is no need to keep the doc strings in the transaction base class for the
forwarded methods. Instead, they are moved to the public (forwarding) methods.
This still keeps the code DRY.

Author: robsdedude

src/neo4j/_async/work/transaction.py

@@ -18,7 +18,6 @@
 
 import asyncio
 import typing as t
-from functools import wraps
 
 from ..._async_compat.util import AsyncUtil
 from ..._work import Query
@@ -57,7 +56,7 @@ def __init__(self, connection, fetch_size, on_closed, on_error,
         self._on_cancel = on_cancel
         super().__init__()
 
-    async def _enter(self):
+    async def _enter(self) -> te.Self:
         return self
 
     @AsyncNonConcurrentMethodChecker.non_concurrent_method
@@ -113,7 +112,7 @@ async def run(
         parameters: t.Optional[t.Dict[str, t.Any]] = None,
         **kwparameters: t.Any
     ) -> AsyncResult:
-        """ Run a Cypher query within the context of this transaction.
+        """Run a Cypher query within the context of this transaction.
 
         Cypher is typically expressed as a query template plus a
         set of named parameters. In Python, parameters may be expressed
@@ -172,10 +171,6 @@ async def run(
 
     @AsyncNonConcurrentMethodChecker.non_concurrent_method
     async def _commit(self):
-        """Mark this transaction as successful and close in order to trigger a COMMIT.
-
-        :raise TransactionError: if the transaction is already closed
-        """
         if self._closed_flag:
             raise TransactionError(self, "Transaction closed")
         if self._last_error:
@@ -202,10 +197,6 @@ async def _commit(self):
 
     @AsyncNonConcurrentMethodChecker.non_concurrent_method
     async def _rollback(self):
-        """Mark this transaction as unsuccessful and close in order to trigger a ROLLBACK.
-
-        :raise TransactionError: if the transaction is already closed
-        """
         if self._closed_flag:
             raise TransactionError(self, "Transaction closed")
 
@@ -228,88 +219,98 @@ async def _rollback(self):
 
     @AsyncNonConcurrentMethodChecker.non_concurrent_method
     async def _close(self):
-        """Close this transaction, triggering a ROLLBACK if not closed.
-        """
         if self._closed_flag:
             return
         await self._rollback()
 
     if AsyncUtil.is_async_code:
         def _cancel(self) -> None:
-            """Cancel this transaction.
-
-            If the transaction is already closed, this method does nothing.
-            Else, it will close the connection without ROLLBACK or COMMIT in
-            a non-blocking manner.
-
-            The primary purpose of this function is to handle
-            :class:`asyncio.CancelledError`.
-
-            ::
-
-                tx = await session.begin_transaction()
-                try:
-                    ...  # do some work
-                except asyncio.CancelledError:
-                    tx.cancel()
-                    raise
-
-            """
             if self._closed_flag:
                 return
             try:
                 self._on_cancel()
             finally:
                 self._closed_flag = True
 
-    def _closed(self):
-        """Indicate whether the transaction has been closed or cancelled.
-
-        :returns:
-            :data:`True` if closed or cancelled, :data:`False` otherwise.
-        :rtype: bool
-        """
+    def _closed(self) -> bool:
         return self._closed_flag
 
 
 class AsyncTransaction(AsyncTransactionBase):
-    """ Container for multiple Cypher queries to be executed within a single
+    """Fully user-managed transaction.
+
+    Container for multiple Cypher queries to be executed within a single
     context. :class:`AsyncTransaction` objects can be used as a context
-    managers (:py:const:`async with` block) where the transaction is committed
-    or rolled back on based on whether an exception is raised::
+    manager (:py:const:`async with` block) where the transaction is committed
+    or rolled back based on whether an exception is raised::
 
         async with await session.begin_transaction() as tx:
             ...
 
     """
 
-    @wraps(AsyncTransactionBase._enter)
     async def __aenter__(self) -> AsyncTransaction:
         return await self._enter()
 
-    @wraps(AsyncTransactionBase._exit)
-    async def __aexit__(self, exception_type, exception_value, traceback):
+    async def __aexit__(
+        self, exception_type, exception_value, traceback
+    ) -> None:
         await self._exit(exception_type, exception_value, traceback)
 
-    @wraps(AsyncTransactionBase._commit)
     async def commit(self) -> None:
+        """Commit the transaction and close it.
+
+        Marks this transaction as successful and closes in order to trigger a
+        COMMIT.
+
+        :raise TransactionError: if the transaction is already closed
+        """
         return await self._commit()
 
-    @wraps(AsyncTransactionBase._rollback)
     async def rollback(self) -> None:
+        """Rollback the transaction and close it.
+
+        Marks the transaction as unsuccessful and closes in order to trigger
+        a ROLLBACK.
+
+        :raise TransactionError: if the transaction is already closed
+        """
         return await self._rollback()
 
-    @wraps(AsyncTransactionBase._close)
     async def close(self) -> None:
+        """Close this transaction, triggering a ROLLBACK if not closed."""
         return await self._close()
 
-    @wraps(AsyncTransactionBase._closed)
     def closed(self) -> bool:
+        """Indicate whether the transaction has been closed or cancelled.
+
+        :returns:
+            :data:`True` if closed or cancelled, :data:`False` otherwise.
+        :rtype: bool
+        """
         return self._closed()
 
     if AsyncUtil.is_async_code:
-        @wraps(AsyncTransactionBase._cancel)
         def cancel(self) -> None:
+            """Cancel this transaction.
+
+            If the transaction is already closed, this method does nothing.
+            Else, it will close the connection without ROLLBACK or COMMIT in
+            a non-blocking manner.
+
+            The primary purpose of this function is to handle
+            :class:`asyncio.CancelledError`.
+
+            ::
+
+                tx = await session.begin_transaction()
+                try:
+                    ...  # do some work
+                except asyncio.CancelledError:
+                    tx.cancel()
+                    raise
+
+            """
             return self._cancel()
 
 

src/neo4j/_sync/work/transaction.py

@@ -18,7 +18,6 @@
 
 import asyncio
 import typing as t
-from functools import wraps
 
 from ..._async_compat.util import Util
 from ..._work import Query
@@ -57,7 +56,7 @@ def __init__(self, connection, fetch_size, on_closed, on_error,
         self._on_cancel = on_cancel
         super().__init__()
 
-    def _enter(self):
+    def _enter(self) -> te.Self:
         return self
 
     @NonConcurrentMethodChecker.non_concurrent_method
@@ -113,7 +112,7 @@ def run(
         parameters: t.Optional[t.Dict[str, t.Any]] = None,
         **kwparameters: t.Any
     ) -> Result:
-        """ Run a Cypher query within the context of this transaction.
+        """Run a Cypher query within the context of this transaction.
 
         Cypher is typically expressed as a query template plus a
         set of named parameters. In Python, parameters may be expressed
@@ -172,10 +171,6 @@ def run(
 
     @NonConcurrentMethodChecker.non_concurrent_method
     def _commit(self):
-        """Mark this transaction as successful and close in order to trigger a COMMIT.
-
-        :raise TransactionError: if the transaction is already closed
-        """
         if self._closed_flag:
             raise TransactionError(self, "Transaction closed")
         if self._last_error:
@@ -202,10 +197,6 @@ def _commit(self):
 
     @NonConcurrentMethodChecker.non_concurrent_method
     def _rollback(self):
-        """Mark this transaction as unsuccessful and close in order to trigger a ROLLBACK.
-
-        :raise TransactionError: if the transaction is already closed
-        """
         if self._closed_flag:
             raise TransactionError(self, "Transaction closed")
 
@@ -228,88 +219,98 @@ def _rollback(self):
 
     @NonConcurrentMethodChecker.non_concurrent_method
     def _close(self):
-        """Close this transaction, triggering a ROLLBACK if not closed.
-        """
         if self._closed_flag:
             return
         self._rollback()
 
     if Util.is_async_code:
         def _cancel(self) -> None:
-            """Cancel this transaction.
-
-            If the transaction is already closed, this method does nothing.
-            Else, it will close the connection without ROLLBACK or COMMIT in
-            a non-blocking manner.
-
-            The primary purpose of this function is to handle
-            :class:`asyncio.CancelledError`.
-
-            ::
-
-                tx = session.begin_transaction()
-                try:
-                    ...  # do some work
-                except asyncio.CancelledError:
-                    tx.cancel()
-                    raise
-
-            """
             if self._closed_flag:
                 return
             try:
                 self._on_cancel()
             finally:
                 self._closed_flag = True
 
-    def _closed(self):
-        """Indicate whether the transaction has been closed or cancelled.
-
-        :returns:
-            :data:`True` if closed or cancelled, :data:`False` otherwise.
-        :rtype: bool
-        """
+    def _closed(self) -> bool:
         return self._closed_flag
 
 
 class Transaction(TransactionBase):
-    """ Container for multiple Cypher queries to be executed within a single
+    """Fully user-managed transaction.
+
+    Container for multiple Cypher queries to be executed within a single
     context. :class:`Transaction` objects can be used as a context
-    managers (:py:const:`with` block) where the transaction is committed
-    or rolled back on based on whether an exception is raised::
+    manager (:py:const:`with` block) where the transaction is committed
+    or rolled back based on whether an exception is raised::
 
         with session.begin_transaction() as tx:
             ...
 
     """
 
-    @wraps(TransactionBase._enter)
     def __enter__(self) -> Transaction:
         return self._enter()
 
-    @wraps(TransactionBase._exit)
-    def __exit__(self, exception_type, exception_value, traceback):
+    def __exit__(
+        self, exception_type, exception_value, traceback
+    ) -> None:
         self._exit(exception_type, exception_value, traceback)
 
-    @wraps(TransactionBase._commit)
     def commit(self) -> None:
+        """Commit the transaction and close it.
+
+        Marks this transaction as successful and closes in order to trigger a
+        COMMIT.
+
+        :raise TransactionError: if the transaction is already closed
+        """
         return self._commit()
 
-    @wraps(TransactionBase._rollback)
     def rollback(self) -> None:
+        """Rollback the transaction and close it.
+
+        Marks the transaction as unsuccessful and closes in order to trigger
+        a ROLLBACK.
+
+        :raise TransactionError: if the transaction is already closed
+        """
         return self._rollback()
 
-    @wraps(TransactionBase._close)
     def close(self) -> None:
+        """Close this transaction, triggering a ROLLBACK if not closed."""
         return self._close()
 
-    @wraps(TransactionBase._closed)
     def closed(self) -> bool:
+        """Indicate whether the transaction has been closed or cancelled.
+
+        :returns:
+            :data:`True` if closed or cancelled, :data:`False` otherwise.
+        :rtype: bool
+        """
         return self._closed()
 
     if Util.is_async_code:
-        @wraps(TransactionBase._cancel)
         def cancel(self) -> None:
+            """Cancel this transaction.
+
+            If the transaction is already closed, this method does nothing.
+            Else, it will close the connection without ROLLBACK or COMMIT in
+            a non-blocking manner.
+
+            The primary purpose of this function is to handle
+            :class:`asyncio.CancelledError`.
+
+            ::
+
+                tx = session.begin_transaction()
+                try:
+                    ...  # do some work
+                except asyncio.CancelledError:
+                    tx.cancel()
+                    raise
+
+            """
             return self._cancel()