dbapi: make Connection.close invoke rollback, guards for already closed

odeke-em · odeke-em · commit 0b90502d66f7 · 2020-02-05T14:17:45.000-08:00
As per PEP-0249 Connection.close, invoke Connection.close() should rollback any uncommitted operations and also make any future uses of the Connection throw dbapi.Error("attempting to use an already closed connection") Currently Connection.rollback() is noop, but with PR #276, we'll have full commit and rollback of transactions. Also while here, make Cursor.close discard anything in the Cursor and unusable thereafter. Fixes #279
diff --git a/packages/django-google-spanner/spanner/dbapi/connection.py b/packages/django-google-spanner/spanner/dbapi/connection.py
@@ -22,25 +22,30 @@ def __raise_if_already_closed(self):
             raise Error('attempting to use an already closed connection')
 
     def close(self):
-        self.commit()
-        self.__raise_if_already_closed()
+        self.rollback()
         self.__dbhandle = None
         self.__closed = True
 
     def __enter__(self):
         return self
 
     def __exit__(self, etype, value, traceback):
-        return self.close()
+        self.commit()
+        self.close()
 
     def commit(self):
+        self.__raise_if_already_closed()
+
         self.run_prior_DDL_statements()
 
     def rollback(self):
-        # We don't manage transactions.
-        pass
+        self.__raise_if_already_closed()
+
+        # TODO: to be added.
 
     def cursor(self):
+        self.__raise_if_already_closed()
+
         return Cursor(self)
 
     def __handle_update_ddl(self, ddl_statements):
@@ -54,19 +59,29 @@ def __handle_update_ddl(self, ddl_statements):
         Returns:
             google.api_core.operation.Operation.result()
         """
+        self.__raise_if_already_closed()
+
         # Synchronously wait on the operation's completion.
         return self.__dbhandle.update_ddl(ddl_statements).result()
 
     def read_snapshot(self):
+        self.__raise_if_already_closed()
+
         return self.__dbhandle.snapshot()
 
     def in_transaction(self, fn, *args, **kwargs):
+        self.__raise_if_already_closed()
+
         return self.__dbhandle.run_in_transaction(fn, *args, **kwargs)
 
     def append_ddl_statement(self, ddl_statement):
+        self.__raise_if_already_closed()
+
         self.__ddl_statements.append(ddl_statement)
 
     def run_prior_DDL_statements(self):
+        self.__raise_if_already_closed()
+
         if not self.__ddl_statements:
             return
 
diff --git a/packages/django-google-spanner/spanner/dbapi/cursor.py b/packages/django-google-spanner/spanner/dbapi/cursor.py
@@ -6,7 +6,9 @@
 
 import google.api_core.exceptions as grpc_exceptions
 
-from .exceptions import IntegrityError, OperationalError, ProgrammingError
+from .exceptions import (
+    Error, IntegrityError, OperationalError, ProgrammingError,
+)
 from .parse_utils import (
     STMT_DDL, STMT_INSERT, STMT_NON_UPDATING, classify_stmt,
     ensure_where_clause, get_param_types, parse_insert,
@@ -23,6 +25,7 @@ def __init__(self, db_handle=None):
         self.__row_count = _UNSET_COUNT
         self.__db_handle = db_handle
         self.__last_op = None
+        self.__closed = False
 
         # arraysize is a readable and writable property mandated
         # by PEP-0249 https://www.python.org/dev/peps/pep-0249/#arraysize
@@ -44,11 +47,16 @@ def description(self):
     def rowcount(self):
         return self.__row_count
 
-    def close(self):
-        if self.__db_handle is None:
-            return
+    def __raise_if_already_closed(self):
+        """
+        Raises an exception if attempting to use an already closed connection.
+        """
+        if self.__closed:
+            raise Error('attempting to use an already closed connection')
 
-        self.__db_handle = None
+    def close(self):
+        self.__clear()
+        self.__closed = True
 
     def execute(self, sql, args=None):
         """
@@ -62,6 +70,8 @@ def execute(self, sql, args=None):
         Returns:
             None
         """
+        self.__raise_if_already_closed()
+
         if not self.__db_handle:
             raise ProgrammingError('Cursor is not connected to the database')
 
@@ -186,9 +196,14 @@ def __enter__(self):
         return self
 
     def __exit__(self, etype, value, traceback):
-        self.close()
+        self.__clear()
+
+    def __clear(self):
+        self.__db_handle = None
 
     def executemany(self, operation, seq_of_params):
+        self.__raise_if_already_closed()
+
         if not self.__db_handle:
             raise ProgrammingError('Cursor is not connected to the database')
 
@@ -205,12 +220,16 @@ def __iter__(self):
         return self.__itr
 
     def fetchone(self):
+        self.__raise_if_already_closed()
+
         try:
             return next(self)
         except StopIteration:
             return None
 
     def fetchall(self):
+        self.__raise_if_already_closed()
+
         return list(self.__iter__())
 
     def fetchmany(self, size=None):
@@ -226,6 +245,8 @@ def fetchmany(self, size=None):
             Error if the previous call to .execute*() did not produce any result set
             or if no call was issued yet.
         """
+        self.__raise_if_already_closed()
+
         if size is None:
             size = self.arraysize
 
