diff options
| -rw-r--r-- | ChangeLog | 17 | ||||
| -rw-r--r-- | psycopg/lobject_int.c | 22 | ||||
| -rw-r--r-- | psycopg/lobject_type.c | 52 | ||||
| -rw-r--r-- | tests/test_lobject.py | 190 |
4 files changed, 234 insertions, 47 deletions
@@ -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. diff --git a/psycopg/lobject_int.c b/psycopg/lobject_int.c index 9058523..476f236 100644 --- a/psycopg/lobject_int.c +++ b/psycopg/lobject_int.c @@ -95,11 +95,7 @@ lobject_open(lobjectObject *self, connectionObject *conn, retvalue = -1; goto end; } - } - else { - /* this is necessary to make sure no function that needs and - fd is called on unopened lobjects */ - self->closed = 1; + self->closed = 0; } /* 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); - if (res < 0) + retvalue = pq_begin_locked(self->conn, &pgres, &error); + 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; } diff --git a/psycopg/lobject_type.c b/psycopg/lobject_type.c index ebbcb1d..690e4b6 100644 --- a/psycopg/lobject_type.c +++ b/psycopg/lobject_type.c @@ -48,19 +48,17 @@ static PyObject * psyco_lobj_close(lobjectObject *self, PyObject *args) { if (!PyArg_ParseTuple(args, "")) return NULL; - + /* file-like objects can be closed multiple times and remember that - closing the current transaction is equivalent to close all the + closing the current transaction is equivalent to close all the opened large objects */ if (!self->closed && 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); @@ -135,14 +133,14 @@ psyco_lobj_seek(lobjectObject *self, PyObject *args) if (!PyArg_ParseTuple(args, "i|i", &offset, &whence)) return NULL; - + EXC_IF_LOBJ_CLOSED(self); EXC_IF_LOBJ_LEVEL0(self); EXC_IF_LOBJ_UNMARKED(self); - + if ((pos = lobject_seek(self, offset, whence)) < 0) return NULL; - + return PyInt_FromLong((long)pos); } @@ -157,14 +155,14 @@ psyco_lobj_tell(lobjectObject *self, PyObject *args) int pos; if (!PyArg_ParseTuple(args, "")) return NULL; - + EXC_IF_LOBJ_CLOSED(self); EXC_IF_LOBJ_LEVEL0(self); EXC_IF_LOBJ_UNMARKED(self); - + if ((pos = lobject_tell(self)) < 0) return NULL; - + return PyInt_FromLong((long)pos); } @@ -177,10 +175,10 @@ static PyObject * psyco_lobj_unlink(lobjectObject *self, PyObject *args) { if (!PyArg_ParseTuple(args, "")) return NULL; - + if (lobject_unlink(self) < 0) return NULL; - + Py_INCREF(Py_None); return Py_None; } @@ -197,12 +195,14 @@ 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; - + Py_INCREF(Py_None); - return Py_None; + return Py_None; } @@ -247,9 +247,9 @@ lobject_setup(lobjectObject *self, connectionObject *conn, Oid oid, int mode, Oid new_oid, char *new_file) { Dprintf("lobject_setup: init lobject object at %p", self); - + if (conn->isolation_level == 0) { - psyco_set_error(ProgrammingError, (PyObject*)self, + psyco_set_error(ProgrammingError, (PyObject*)self, "can't use a lobject outside of transactions", NULL, NULL); return -1; } @@ -258,14 +258,14 @@ lobject_setup(lobjectObject *self, connectionObject *conn, self->mark = conn->mark; Py_INCREF((PyObject*)self->conn); - - self->closed = 0; + + self->closed = 1; self->oid = InvalidOid; self->fd = -1; if (lobject_open(self, conn, oid, mode, new_oid, new_file) == -1) return -1; - + Dprintf("lobject_setup: good lobject object at %p, refcnt = %d", self, ((PyObject *)self)->ob_refcnt); Dprintf("lobject_setup: oid = %d, fd = %d", self->oid, self->fd); @@ -294,7 +294,7 @@ lobject_init(PyObject *obj, PyObject *args, PyObject *kwds) int mode=0; char *new_file = NULL; PyObject *conn; - + if (!PyArg_ParseTuple(args, "O|iiis", &conn, &oid, &mode, &new_oid, &new_file)) return -1; @@ -335,7 +335,7 @@ PyTypeObject lobjectType = { sizeof(lobjectObject), 0, lobject_dealloc, /*tp_dealloc*/ - 0, /*tp_print*/ + 0, /*tp_print*/ 0, /*tp_getattr*/ 0, /*tp_setattr*/ 0, /*tp_compare*/ @@ -353,7 +353,7 @@ PyTypeObject lobjectType = { Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE|Py_TPFLAGS_HAVE_ITER, /*tp_flags*/ lobjectType_doc, /*tp_doc*/ - + 0, /*tp_traverse*/ 0, /*tp_clear*/ @@ -370,11 +370,11 @@ PyTypeObject lobjectType = { 0, /*tp_getset*/ 0, /*tp_base*/ 0, /*tp_dict*/ - + 0, /*tp_descr_get*/ 0, /*tp_descr_set*/ 0, /*tp_dictoffset*/ - + lobject_init, /*tp_init*/ 0, /*tp_alloc Will be set to PyType_GenericAlloc in module init*/ lobject_new, /*tp_new*/ diff --git a/tests/test_lobject.py b/tests/test_lobject.py index 130cea8..7f97f8f 100644 --- a/tests/test_lobject.py +++ b/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. - self.assertRaises(psycopg2.OperationalError, self.conn.lobject, 42) + # By creating then removing a large object, we get an Oid that + # 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,6 +97,15 @@ class LargeObjectTests(unittest.TestCase): lo = self.conn.lobject() self.assertEqual(lo.write("some data"), len("some data")) + def test_read(self): + lo = self.conn.lobject() + length = lo.write("some data") + lo.close() + + lo = self.conn.lobject(lo.oid) + 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") @@ -50,30 +115,133 @@ class LargeObjectTests(unittest.TestCase): 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_read(self): + def test_unlink(self): lo = self.conn.lobject() - length = lo.write("some data") + 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() - lo = self.conn.lobject(lo.oid) - self.assertEqual(lo.read(4), "some") - self.assertEqual(lo.read(), " data") + def test_write_after_close(self): + lo = self.conn.lobject() + lo.close() + self.assertRaises(psycopg2.InterfaceError, lo.write, "some data") - def test_unlink(self): + 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() - # the object doesn't exist now, so we can't reopen it. - self.assertRaises(psycopg2.OperationalError, self.conn.lobject, lo.oid) + 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(): |