* tests/test_lobject.py (LargeObjectTests): add more tests,

including behaviour on closed lobjects and stale lobjects.

	* psycopg/lobject_type.c (psyco_lobj_close): don't mark the
	connection closed here because it is done by
	lobject_close_locked().

	* psycopg/lobject_int.c (lobject_open): mark objects as not closed
	if we successfully open them.
	(lobject_close_locked): mark the lobject closed here.
	(lobject_export): ensure we are in a transaction, since
	lo_export() issues multiple queries.

	* psycopg/lobject_type.c (lobject_setup): make lobjects start closed.

ChangeLog

@@ -1,3 +1,20 @@
+2008-05-06  James Henstridge  <james@jamesh.id.au>
+
+	* tests/test_lobject.py (LargeObjectTests): add more tests,
+	including behaviour on closed lobjects and stale lobjects.
+
+	* psycopg/lobject_type.c (psyco_lobj_close): don't mark the
+	connection closed here because it is done by
+	lobject_close_locked().
+
+	* psycopg/lobject_int.c (lobject_open): mark objects as not closed
+	if we successfully open them.
+	(lobject_close_locked): mark the lobject closed here.
+	(lobject_export): ensure we are in a transaction, since
+	lo_export() issues multiple queries.
+
+	* psycopg/lobject_type.c (lobject_setup): make lobjects start closed.
+
 2008-05-05  James Henstridge  <james@jamesh.id.au>
 
 	* psycopg/lobject.h: don't export the lobjectType symbol.

psycopg/lobject_int.c

@@ -95,11 +95,7 @@ lobject_open(lobjectObject *self, connectionObject *conn,
             retvalue = -1;
             goto end;
         }
-    }
+        self->closed = 0;
-    else {
-        /* this is necessary to make sure no function that needs and
-	   fd is called on unopened lobjects */
-        self->closed = 1;
     }
     /* set the mode for future reference */
     self->mode = mode;
@@ -136,6 +132,7 @@ lobject_close_locked(lobjectObject *self, char **error)
         self->fd == -1)
         return 0;
 
+    self->closed = 1;
     retvalue = lo_close(self->conn->pgconn, self->fd);
     self->fd = -1;
     if (retvalue < 0)
@@ -308,21 +305,26 @@ lobject_export(lobjectObject *self, char *filename)
 {
     PGresult *pgres = NULL;
     char *error = NULL;
-    int res;
+    int retvalue;
 
     Py_BEGIN_ALLOW_THREADS;
     pthread_mutex_lock(&(self->conn->lock));
 
-    res = lo_export(self->conn->pgconn, self->oid, filename);
+    retvalue = pq_begin_locked(self->conn, &pgres, &error);
-    if (res < 0)
+    if (retvalue < 0)
+        goto end;
+
+    retvalue = lo_export(self->conn->pgconn, self->oid, filename);
+    if (retvalue < 0)
         collect_error(self->conn, &error);
 
+ end:
     pthread_mutex_unlock(&(self->conn->lock));
     Py_END_ALLOW_THREADS;
 
-    if (res < 0)
+    if (retvalue < 0)
         pq_complete_error(self->conn, &pgres, &error);
-    return res;
+    return retvalue;
 }
 
 

psycopg/lobject_type.c

@@ -56,11 +56,9 @@ psyco_lobj_close(lobjectObject *self, PyObject *args)
         && self->conn->isolation_level > 0
 	&& self->conn->mark == self->mark)
     {
-        self->closed = 1;
+        Dprintf("psyco_lobj_close: closing lobject at %p", self);
         if (lobject_close(self) < 0)
             return NULL;
-
-        Dprintf("psyco_lobj_close: lobject at %p closed", self);
     }
 
     Py_INCREF(Py_None);
@@ -198,6 +196,8 @@ psyco_lobj_export(lobjectObject *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "s", &filename))
     	return NULL;
 
+    EXC_IF_LOBJ_LEVEL0(self);
+
     if (lobject_export(self, filename) < 0)
     	return NULL;
 
@@ -259,7 +259,7 @@ lobject_setup(lobjectObject *self, connectionObject *conn,
 
     Py_INCREF((PyObject*)self->conn);
 
-    self->closed = 0;
+    self->closed = 1;
     self->oid = InvalidOid;
     self->fd = -1;
 

tests/test_lobject.py

@@ -1,3 +1,6 @@
+import os
+import shutil
+import tempfile
 import unittest
 
 import psycopg2
@@ -10,10 +13,19 @@ class LargeObjectTests(unittest.TestCase):
     def setUp(self):
         self.conn = psycopg2.connect(tests.dsn)
         self.lo_oid = None
+        self.tmpdir = None
 
     def tearDown(self):
+        if self.tmpdir:
+            shutil.rmtree(self.tmpdir, ignore_errors=True)
         if self.lo_oid is not None:
-            self.conn.lobject(self.lo_oid).unlink()
+            self.conn.rollback()
+            try:
+                lo = self.conn.lobject(self.lo_oid, "n")
+            except psycopg2.OperationalError:
+                pass
+            else:
+                lo.unlink()
 
     def test_create(self):
         lo = self.conn.lobject()
@@ -21,8 +33,11 @@ class LargeObjectTests(unittest.TestCase):
         self.assertEqual(lo.mode, "w")
 
     def test_open_non_existent(self):
-        # This test will give a false negative if Oid 42 is a large object.
+        # By creating then removing a large object, we get an Oid that
-        self.assertRaises(psycopg2.OperationalError, self.conn.lobject, 42)
+        # should be unused.
+        lo = self.conn.lobject()
+        lo.unlink()
+        self.assertRaises(psycopg2.OperationalError, self.conn.lobject, lo.oid)
 
     def test_open_existing(self):
         lo = self.conn.lobject()
@@ -31,6 +46,47 @@ class LargeObjectTests(unittest.TestCase):
         self.assertEqual(lo2.oid, lo.oid)
         self.assertEqual(lo2.mode, "r")
 
+    def test_open_for_write(self):
+        lo = self.conn.lobject()
+        lo2 = self.conn.lobject(lo.oid, "w")
+        self.assertEqual(lo2.mode, "w")
+        lo2.write("some data")
+
+    def test_open_mode_n(self):
+        # Openning an object in mode "n" gives us a closed lobject.
+        lo = self.conn.lobject()
+        lo.close()
+
+        lo2 = self.conn.lobject(lo.oid, "n")
+        self.assertEqual(lo2.oid, lo.oid)
+        self.assertEqual(lo2.closed, True)
+
+    def test_create_with_oid(self):
+        # Create and delete a large object to get an unused Oid.
+        lo = self.conn.lobject()
+        oid = lo.oid
+        lo.unlink()
+
+        lo = self.conn.lobject(0, "w", oid)
+        self.assertEqual(lo.oid, oid)
+
+    def test_create_with_existing_oid(self):
+        lo = self.conn.lobject()
+        lo.close()
+
+        self.assertRaises(psycopg2.OperationalError,
+                          self.conn.lobject, 0, "w", lo.oid)
+
+    def test_import(self):
+        self.tmpdir = tempfile.mkdtemp()
+        filename = os.path.join(self.tmpdir, "data.txt")
+        fp = open(filename, "wb")
+        fp.write("some data")
+        fp.close()
+
+        lo = self.conn.lobject(0, "r", 0, filename)
+        self.assertEqual(lo.read(), "some data")
+
     def test_close(self):
         lo = self.conn.lobject()
         self.assertEqual(lo.closed, False)
@@ -41,23 +97,6 @@ class LargeObjectTests(unittest.TestCase):
         lo = self.conn.lobject()
         self.assertEqual(lo.write("some data"), len("some data"))
 
-    def test_seek_tell(self):
-        lo = self.conn.lobject()
-        length = lo.write("some data")
-        self.assertEqual(lo.tell(), length)
-        lo.close()
-        lo = self.conn.lobject(lo.oid)
-
-        self.assertEqual(lo.seek(5, 0), 5)
-        self.assertEqual(lo.tell(), 5)
-
-        # SEEK_CUR: relative current location
-        self.assertEqual(lo.seek(2, 1), 7)
-        self.assertEqual(lo.tell(), 7)
-
-        # SEEK_END: relative to end of file
-        self.assertEqual(lo.seek(-2, 2), length - 2)
-
     def test_read(self):
         lo = self.conn.lobject()
         length = lo.write("some data")
@@ -67,13 +106,142 @@ class LargeObjectTests(unittest.TestCase):
         self.assertEqual(lo.read(4), "some")
         self.assertEqual(lo.read(), " data")
 
+    def test_seek_tell(self):
+        lo = self.conn.lobject()
+        length = lo.write("some data")
+        self.assertEqual(lo.tell(), length)
+        lo.close()
+        lo = self.conn.lobject(lo.oid)
+
+        self.assertEqual(lo.seek(5, 0), 5)
+        self.assertEqual(lo.tell(), 5)
+        self.assertEqual(lo.read(), "data")
+
+        # SEEK_CUR: relative current location
+        lo.seek(5)
+        self.assertEqual(lo.seek(2, 1), 7)
+        self.assertEqual(lo.tell(), 7)
+        self.assertEqual(lo.read(), "ta")
+
+        # SEEK_END: relative to end of file
+        self.assertEqual(lo.seek(-2, 2), length - 2)
+        self.assertEqual(lo.read(), "ta")
+
     def test_unlink(self):
         lo = self.conn.lobject()
-        lo.close()
         lo.unlink()
 
         # the object doesn't exist now, so we can't reopen it.
         self.assertRaises(psycopg2.OperationalError, self.conn.lobject, lo.oid)
+        # And the object has been closed.
+        self.assertEquals(lo.closed, True)
+
+    def test_export(self):
+        lo = self.conn.lobject()
+        lo.write("some data")
+
+        self.tmpdir = tempfile.mkdtemp()
+        filename = os.path.join(self.tmpdir, "data.txt")
+        lo.export(filename)
+        self.assertTrue(os.path.exists(filename))
+        self.assertEqual(open(filename, "rb").read(), "some data")
+
+    def test_close_twice(self):
+        lo = self.conn.lobject()
+        lo.close()
+        lo.close()
+
+    def test_write_after_close(self):
+        lo = self.conn.lobject()
+        lo.close()
+        self.assertRaises(psycopg2.InterfaceError, lo.write, "some data")
+
+    def test_read_after_close(self):
+        lo = self.conn.lobject()
+        lo.close()
+        self.assertRaises(psycopg2.InterfaceError, lo.read, 5)
+
+    def test_seek_after_close(self):
+        lo = self.conn.lobject()
+        lo.close()
+        self.assertRaises(psycopg2.InterfaceError, lo.seek, 0)
+
+    def test_tell_after_close(self):
+        lo = self.conn.lobject()
+        lo.close()
+        self.assertRaises(psycopg2.InterfaceError, lo.tell)
+
+    def test_unlink_after_close(self):
+        lo = self.conn.lobject()
+        lo.close()
+        # Unlink works on closed files.
+        lo.unlink()
+
+    def test_export_after_close(self):
+        lo = self.conn.lobject()
+        lo.write("some data")
+        lo.close()
+
+        self.tmpdir = tempfile.mkdtemp()
+        filename = os.path.join(self.tmpdir, "data.txt")
+        lo.export(filename)
+        self.assertTrue(os.path.exists(filename))
+        self.assertEqual(open(filename, "rb").read(), "some data")
+
+    def test_close_after_commit(self):
+        lo = self.conn.lobject()
+        self.lo_oid = lo.oid
+        self.conn.commit()
+
+        # Closing outside of the transaction is okay.
+        lo.close()
+
+    def test_write_after_commit(self):
+        lo = self.conn.lobject()
+        self.lo_oid = lo.oid
+        self.conn.commit()
+
+        self.assertRaises(psycopg2.ProgrammingError, lo.write, "some data")
+
+    def test_read_after_commit(self):
+        lo = self.conn.lobject()
+        self.lo_oid = lo.oid
+        self.conn.commit()
+
+        self.assertRaises(psycopg2.ProgrammingError, lo.read, 5)
+
+    def test_seek_after_commit(self):
+        lo = self.conn.lobject()
+        self.lo_oid = lo.oid
+        self.conn.commit()
+
+        self.assertRaises(psycopg2.ProgrammingError, lo.seek, 0)
+
+    def test_tell_after_commit(self):
+        lo = self.conn.lobject()
+        self.lo_oid = lo.oid
+        self.conn.commit()
+
+        self.assertRaises(psycopg2.ProgrammingError, lo.tell)
+
+    def test_unlink_after_commit(self):
+        lo = self.conn.lobject()
+        self.lo_oid = lo.oid
+        self.conn.commit()
+
+        # Unlink of stale lobject is okay
+        lo.unlink()
+
+    def test_export_after_commit(self):
+        lo = self.conn.lobject()
+        lo.write("some data")
+        self.conn.commit()
+
+        self.tmpdir = tempfile.mkdtemp()
+        filename = os.path.join(self.tmpdir, "data.txt")
+        lo.export(filename)
+        self.assertTrue(os.path.exists(filename))
+        self.assertEqual(open(filename, "rb").read(), "some data")
 
 
 def test_suite():