From b3def740028cd6c8756a6de35d20770d0971a212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Jan=C3=9Fen?= Date: Tue, 10 Nov 2015 17:02:59 +0100 Subject: [PATCH 01/14] Update psycopg1.py --- lib/psycopg1.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/psycopg1.py b/lib/psycopg1.py index 7a24c5f2..95b36bff 100644 --- a/lib/psycopg1.py +++ b/lib/psycopg1.py @@ -28,7 +28,7 @@ old code while porting to psycopg 2. Import it as follows:: # FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public # License for more details. -import _psycopg as _2psycopg +import psycopg2._psycopg as _2psycopg from psycopg2.extensions import cursor as _2cursor from psycopg2.extensions import connection as _2connection From 5fd0f6c4eefecb0d6150179c32c43d16c11b173d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 16 Dec 2015 12:00:52 +0000 Subject: [PATCH 02/14] Fixed race condition on import in errorcodes.lookup Fixes #382. --- NEWS | 1 + lib/errorcodes.py | 10 +++++-- tests/__init__.py | 2 ++ tests/test_errcodes.py | 65 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100755 tests/test_errcodes.py diff --git a/NEWS b/NEWS index 5200c4dd..c1e4152f 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,7 @@ What's new in psycopg 2.6.2 - Raise `!NotSupportedError` on unhandled server response status (:ticket:`#352`). - Fixed `!PersistentConnectionPool` on Python 3 (:ticket:`#348`). +- Fixed `!errorcodes.lookup` initialization thread-safety (:ticket:`#382`). What's new in psycopg 2.6.1 diff --git a/lib/errorcodes.py b/lib/errorcodes.py index 12c300f6..aa5a723c 100644 --- a/lib/errorcodes.py +++ b/lib/errorcodes.py @@ -38,11 +38,17 @@ def lookup(code, _cache={}): return _cache[code] # Generate the lookup map at first usage. + tmp = {} for k, v in globals().iteritems(): if isinstance(v, str) and len(v) in (2, 5): - _cache[v] = k + tmp[v] = k - return lookup(code) + assert tmp + + # Atomic update, to avoid race condition on import (bug #382) + _cache.update(tmp) + + return _cache[code] # autogenerated data: do not edit below this point. diff --git a/tests/__init__.py b/tests/__init__.py index 3e677d85..3e0db779 100755 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -34,6 +34,7 @@ import test_connection import test_copy import test_cursor import test_dates +import test_errcodes import test_extras_dictcursor import test_green import test_lobject @@ -71,6 +72,7 @@ def test_suite(): suite.addTest(test_copy.test_suite()) suite.addTest(test_cursor.test_suite()) suite.addTest(test_dates.test_suite()) + suite.addTest(test_errcodes.test_suite()) suite.addTest(test_extras_dictcursor.test_suite()) suite.addTest(test_green.test_suite()) suite.addTest(test_lobject.test_suite()) diff --git a/tests/test_errcodes.py b/tests/test_errcodes.py new file mode 100755 index 00000000..6cf5ddba --- /dev/null +++ b/tests/test_errcodes.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python + +# test_errcodes.py - unit test for psycopg2.errcodes module +# +# Copyright (C) 2015 Daniele Varrazzo +# +# psycopg2 is free software: you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# In addition, as a special exception, the copyright holders give +# permission to link this program with the OpenSSL library (or with +# modified versions of OpenSSL that use the same license as OpenSSL), +# and distribute linked combinations including the two. +# +# You must obey the GNU Lesser General Public License in all respects for +# all of the code used other than OpenSSL. +# +# psycopg2 is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public +# License for more details. + +from testutils import unittest, ConnectingTestCase + +try: + reload +except NameError: + from imp import reload + +from threading import Thread +from psycopg2 import errorcodes + +class ErrocodeTests(ConnectingTestCase): + def test_lookup_threadsafe(self): + + # Increase if it does not fail with KeyError + MAX_CYCLES = 2000 + + errs = [] + def f(pg_code='40001'): + try: + errorcodes.lookup(pg_code) + except Exception, e: + errs.append(e) + + for __ in xrange(MAX_CYCLES): + reload(errorcodes) + (t1, t2) = (Thread(target=f), Thread(target=f)) + (t1.start(), t2.start()) + (t1.join(), t2.join()) + + if errs: + self.fail( + "raised %s errors in %s cycles (first is %s %s)" % ( + len(errs), MAX_CYCLES, + errs[0].__class__.__name__, errs[0])) + + +def test_suite(): + return unittest.TestLoader().loadTestsFromName(__name__) + +if __name__ == "__main__": + unittest.main() From 3a54e8373733ef574a72b5c592cc6a5033de5c4a Mon Sep 17 00:00:00 2001 From: "Karl O. Pinc" Date: Tue, 2 Feb 2016 12:48:16 -0600 Subject: [PATCH 03/14] Improve sentence. --- doc/src/advanced.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/advanced.rst b/doc/src/advanced.rst index 82754ee0..e63fcff1 100644 --- a/doc/src/advanced.rst +++ b/doc/src/advanced.rst @@ -270,7 +270,7 @@ wasting resources. A simple application could poll the connection from time to time to check if something new has arrived. A better strategy is to use some I/O completion -function such as :py:func:`~select.select` to sleep until awaken from the kernel when there is +function such as :py:func:`~select.select` to sleep until awakened by the kernel when there is some data to read on the connection, thereby using no CPU unless there is something to read:: From 01856333c4783e9053b0cfcb00de8a71146cdc57 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 03:20:11 +0000 Subject: [PATCH 04/14] Some order in the extensions doc Classes, coroutine functions and extra functions grouped under separate headings. --- doc/src/extensions.rst | 117 ++++++++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 43 deletions(-) diff --git a/doc/src/extensions.rst b/doc/src/extensions.rst index d96cca4f..68619e5f 100644 --- a/doc/src/extensions.rst +++ b/doc/src/extensions.rst @@ -12,17 +12,12 @@ The module contains a few objects and function extending the minimum set of functionalities defined by the |DBAPI|_. -.. function:: parse_dsn(dsn) +Classes definitions +------------------- - Parse connection string into a dictionary of keywords and values. - - Uses libpq's ``PQconninfoParse`` to parse the string according to - accepted format(s) and check for supported keywords. - - Example:: - - >>> psycopg2.extensions.parse_dsn('dbname=test user=postgres password=secret') - {'password': 'secret', 'user': 'postgres', 'dbname': 'test'} +Instances of these classes are usually returned by factory functions or +attributes. Their definitions are exposed here to allow subclassing, +introspection etc. .. class:: connection(dsn, async=False) @@ -34,6 +29,7 @@ functionalities defined by the |DBAPI|_. For a complete description of the class, see `connection`. + .. class:: cursor(conn, name=None) It is the class usually returned by the `connection.cursor()` @@ -44,6 +40,7 @@ functionalities defined by the |DBAPI|_. For a complete description of the class, see `cursor`. + .. class:: lobject(conn [, oid [, mode [, new_oid [, new_file ]]]]) Wrapper for a PostgreSQL large object. See :ref:`large-objects` for an @@ -200,39 +197,6 @@ functionalities defined by the |DBAPI|_. server versions. -.. autofunction:: set_wait_callback(f) - - .. versionadded:: 2.2.0 - -.. autofunction:: get_wait_callback() - - .. versionadded:: 2.2.0 - -.. function:: libpq_version() - - Return the version number of the ``libpq`` dynamic library loaded as an - integer, in the same format of `~connection.server_version`. - - Raise `~psycopg2.NotSupportedError` if the ``psycopg2`` module was - compiled with a ``libpq`` version lesser than 9.1 (which can be detected - by the `~psycopg2.__libpq_version__` constant). - - .. seealso:: libpq docs for `PQlibVersion()`__. - - .. __: http://www.postgresql.org/docs/current/static/libpq-misc.html#LIBPQ-PQLIBVERSION - -.. function:: quote_ident(str, scope) - - Return quoted identifier according to PostgreSQL quoting rules. - - The *scope* must be a `connection` or a `cursor`, the underlying - connection encoding is used for any necessary character conversion. - - Requires libpq >= 9.0. - - .. seealso:: libpq docs for `PQescapeIdentifier()`__ - - .. __: http://www.postgresql.org/docs/current/static/libpq-exec.html#LIBPQ-PQESCAPEIDENTIFIER .. _sql-adaptation-objects: @@ -492,6 +456,73 @@ The module exports a few exceptions in addition to the :ref:`standard ones +.. _coroutines-functions: + +Coroutines support functions +---------------------------- + +These functions are used to set and retrieve the callback function for +:ref:`cooperation with coroutine libraries `. + +.. versionadded:: 2.2.0 + +.. autofunction:: set_wait_callback(f) + +.. autofunction:: get_wait_callback() + + + +Other functions +--------------- + +.. function:: libpq_version() + + Return the version number of the ``libpq`` dynamic library loaded as an + integer, in the same format of `~connection.server_version`. + + Raise `~psycopg2.NotSupportedError` if the ``psycopg2`` module was + compiled with a ``libpq`` version lesser than 9.1 (which can be detected + by the `~psycopg2.__libpq_version__` constant). + + .. versionadded:: 2.7 + + .. seealso:: libpq docs for `PQlibVersion()`__. + + .. __: http://www.postgresql.org/docs/current/static/libpq-misc.html#LIBPQ-PQLIBVERSION + + +.. function:: parse_dsn(dsn) + + Parse connection string into a dictionary of keywords and values. + + Uses libpq's ``PQconninfoParse`` to parse the string according to + accepted format(s) and check for supported keywords. + + Example:: + + >>> psycopg2.extensions.parse_dsn('dbname=test user=postgres password=secret') + {'password': 'secret', 'user': 'postgres', 'dbname': 'test'} + + .. versionadded:: 2.7 + + +.. function:: quote_ident(str, scope) + + Return quoted identifier according to PostgreSQL quoting rules. + + The *scope* must be a `connection` or a `cursor`, the underlying + connection encoding is used for any necessary character conversion. + + Requires libpq >= 9.0. + + .. versionadded:: 2.7 + + .. seealso:: libpq docs for `PQescapeIdentifier()`__ + + .. __: http://www.postgresql.org/docs/current/static/libpq-exec.html#LIBPQ-PQESCAPEIDENTIFIER + + + .. index:: pair: Isolation level; Constants From d40f81865f0735a03f80eaa8fbb2db5c036da680 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 03:25:47 +0000 Subject: [PATCH 05/14] Added parse_dsn() docstring --- psycopg/psycopgmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index cf70a4ad..24dd5f75 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -112,7 +112,8 @@ psyco_connect(PyObject *self, PyObject *args, PyObject *keywds) return conn; } -#define psyco_parse_dsn_doc "parse_dsn(dsn) -> dict" +#define psyco_parse_dsn_doc \ +"parse_dsn(dsn) -> dict -- parse a connection string into parameters" static PyObject * psyco_parse_dsn(PyObject *self, PyObject *args, PyObject *kwargs) From 1c4523f0ac685632381a0f4371e93031928326b1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 04:33:59 +0000 Subject: [PATCH 06/14] Implementation of make_dsn in Python This is equivalent to what proposed in #363, but with a much simpler implementation. --- lib/__init__.py | 53 +++++--------------------------------------- lib/extensions.py | 45 ++++++++++++++++++++++++++++++++++--- tests/test_module.py | 38 ++++++++++++++++--------------- 3 files changed, 68 insertions(+), 68 deletions(-) diff --git a/lib/__init__.py b/lib/__init__.py index 994b15a8..608b5d14 100644 --- a/lib/__init__.py +++ b/lib/__init__.py @@ -80,28 +80,9 @@ else: _ext.register_adapter(Decimal, Adapter) del Decimal, Adapter -import re -def _param_escape(s, - re_escape=re.compile(r"([\\'])"), - re_space=re.compile(r'\s')): - """ - Apply the escaping rule required by PQconnectdb - """ - if not s: return "''" - - s = re_escape.sub(r'\\\1', s) - if re_space.search(s): - s = "'" + s + "'" - - return s - -del re - - -def connect(dsn=None, - database=None, user=None, password=None, host=None, port=None, - connection_factory=None, cursor_factory=None, async=False, **kwargs): +def connect(dsn=None, connection_factory=None, cursor_factory=None, + async=False, **kwargs): """ Create a new database connection. @@ -115,7 +96,7 @@ def connect(dsn=None, The basic connection parameters are: - - *dbname*: the database name (only in dsn string) + - *dbname*: the database name - *database*: the database name (only as keyword argument) - *user*: user name used to authenticate - *password*: password used to authenticate @@ -135,32 +116,10 @@ def connect(dsn=None, library: the list of supported parameters depends on the library version. """ - items = [] - if database is not None: - items.append(('dbname', database)) - if user is not None: - items.append(('user', user)) - if password is not None: - items.append(('password', password)) - if host is not None: - items.append(('host', host)) - if port is not None: - items.append(('port', port)) - - items.extend([(k, v) for (k, v) in kwargs.iteritems() if v is not None]) - - if dsn is not None and items: - raise TypeError( - "'%s' is an invalid keyword argument when the dsn is specified" - % items[0][0]) - - if dsn is None: - if not items: - raise TypeError('missing dsn and no parameters') - else: - dsn = " ".join(["%s=%s" % (k, _param_escape(str(v))) - for (k, v) in items]) + if dsn is None and not kwargs: + raise TypeError('missing dsn and no parameters') + dsn = _ext.make_dsn(dsn, **kwargs) conn = _connect(dsn, connection_factory=connection_factory, async=async) if cursor_factory is not None: conn.cursor_factory = cursor_factory diff --git a/lib/extensions.py b/lib/extensions.py index b40e28b8..3024b2fd 100644 --- a/lib/extensions.py +++ b/lib/extensions.py @@ -7,7 +7,7 @@ This module holds all the extensions to the DBAPI-2.0 provided by psycopg. - `lobject` -- the new-type inheritable large object class - `adapt()` -- exposes the PEP-246_ compatible adapting mechanism used by psycopg to adapt Python types to PostgreSQL ones - + .. _PEP-246: http://www.python.org/peps/pep-0246.html """ # psycopg/extensions.py - DBAPI-2.0 extensions specific to psycopg @@ -32,6 +32,9 @@ This module holds all the extensions to the DBAPI-2.0 provided by psycopg. # FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public # License for more details. +import re as _re +import sys as _sys + from psycopg2._psycopg import UNICODE, INTEGER, LONGINTEGER, BOOLEAN, FLOAT from psycopg2._psycopg import TIME, DATE, INTERVAL, DECIMAL from psycopg2._psycopg import BINARYARRAY, BOOLEANARRAY, DATEARRAY, DATETIMEARRAY @@ -56,7 +59,8 @@ try: except ImportError: pass -from psycopg2._psycopg import adapt, adapters, encodings, connection, cursor, lobject, Xid, libpq_version, parse_dsn, quote_ident +from psycopg2._psycopg import adapt, adapters, encodings, connection, cursor +from psycopg2._psycopg import lobject, Xid, libpq_version, parse_dsn, quote_ident from psycopg2._psycopg import string_types, binary_types, new_type, new_array_type, register_type from psycopg2._psycopg import ISQLQuote, Notify, Diagnostics, Column @@ -98,7 +102,6 @@ TRANSACTION_STATUS_INTRANS = 2 TRANSACTION_STATUS_INERROR = 3 TRANSACTION_STATUS_UNKNOWN = 4 -import sys as _sys # Return bytes from a string if _sys.version_info[0] < 3: @@ -108,6 +111,7 @@ else: def b(s): return s.encode('utf8') + def register_adapter(typ, callable): """Register 'callable' as an ISQLQuote adapter for type 'typ'.""" adapters[(typ, ISQLQuote)] = callable @@ -151,6 +155,41 @@ class NoneAdapter(object): return _null +def make_dsn(dsn=None, **kwargs): + """Convert a set of keywords into a connection strings.""" + # Override the dsn with the parameters + if 'database' in kwargs: + if 'dbname' in kwargs: + raise TypeError( + "you can't specify both 'database' and 'dbname' arguments") + kwargs['dbname'] = kwargs.pop('database') + + if dsn is not None: + tmp = parse_dsn(dsn) + tmp.update(kwargs) + kwargs = tmp + + dsn = " ".join(["%s=%s" % (k, _param_escape(str(v))) + for (k, v) in kwargs.iteritems()]) + return dsn + + +def _param_escape(s, + re_escape=_re.compile(r"([\\'])"), + re_space=_re.compile(r'\s')): + """ + Apply the escaping rule required by PQconnectdb + """ + if not s: + return "''" + + s = re_escape.sub(r'\\\1', s) + if re_space.search(s): + s = "'" + s + "'" + + return s + + # Create default json typecasters for PostgreSQL 9.2 oids from psycopg2._json import register_default_json, register_default_jsonb diff --git a/tests/test_module.py b/tests/test_module.py index 62b85ee2..c0e4bf87 100755 --- a/tests/test_module.py +++ b/tests/test_module.py @@ -43,6 +43,9 @@ class ConnectTestCase(unittest.TestCase): def tearDown(self): psycopg2._connect = self._connect_orig + def assertDsnEqual(self, dsn1, dsn2): + self.assertEqual(set(dsn1.split()), set(dsn2.split())) + def test_there_has_to_be_something(self): self.assertRaises(TypeError, psycopg2.connect) self.assertRaises(TypeError, psycopg2.connect, @@ -57,8 +60,8 @@ class ConnectTestCase(unittest.TestCase): self.assertEqual(self.args[2], False) def test_dsn(self): - psycopg2.connect('dbname=blah x=y') - self.assertEqual(self.args[0], 'dbname=blah x=y') + psycopg2.connect('dbname=blah application_name=y') + self.assertDsnEqual(self.args[0], 'dbname=blah application_name=y') self.assertEqual(self.args[1], None) self.assertEqual(self.args[2], False) @@ -90,30 +93,30 @@ class ConnectTestCase(unittest.TestCase): def f(dsn, async=False): pass - psycopg2.connect(database='foo', bar='baz', connection_factory=f) - self.assertEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect(database='foo', application_name='baz', connection_factory=f) + self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') self.assertEqual(self.args[1], f) self.assertEqual(self.args[2], False) - psycopg2.connect("dbname=foo bar=baz", connection_factory=f) - self.assertEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect("dbname=foo application_name=baz", connection_factory=f) + self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') self.assertEqual(self.args[1], f) self.assertEqual(self.args[2], False) def test_async(self): - psycopg2.connect(database='foo', bar='baz', async=1) - self.assertEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect(database='foo', application_name='baz', async=1) + self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') self.assertEqual(self.args[1], None) self.assert_(self.args[2]) - psycopg2.connect("dbname=foo bar=baz", async=True) - self.assertEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect("dbname=foo application_name=baz", async=True) + self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') self.assertEqual(self.args[1], None) self.assert_(self.args[2]) def test_empty_param(self): psycopg2.connect(database='sony', password='') - self.assertEqual(self.args[0], "dbname=sony password=''") + self.assertDsnEqual(self.args[0], "dbname=sony password=''") def test_escape(self): psycopg2.connect(database='hello world') @@ -131,13 +134,12 @@ class ConnectTestCase(unittest.TestCase): psycopg2.connect(database=r"\every thing'") self.assertEqual(self.args[0], r"dbname='\\every thing\''") - def test_no_kwargs_swallow(self): - self.assertRaises(TypeError, - psycopg2.connect, 'dbname=foo', database='foo') - self.assertRaises(TypeError, - psycopg2.connect, 'dbname=foo', user='postgres') - self.assertRaises(TypeError, - psycopg2.connect, 'dbname=foo', no_such_param='meh') + def test_params_merging(self): + psycopg2.connect('dbname=foo', database='bar') + self.assertEqual(self.args[0], 'dbname=bar') + + psycopg2.connect('dbname=foo', user='postgres') + self.assertDsnEqual(self.args[0], 'dbname=foo user=postgres') class ExceptionsTestCase(ConnectingTestCase): From 2c55a1bd5394ef82a49984fdce3c17ce956a9c9e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 15:07:38 +0000 Subject: [PATCH 07/14] Verify that the dsn is not manipulated by make_dsn if not necessary --- lib/__init__.py | 3 --- lib/extensions.py | 7 +++++++ tests/test_module.py | 37 +++++++++++++++++++++++++------------ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/lib/__init__.py b/lib/__init__.py index 608b5d14..4a288197 100644 --- a/lib/__init__.py +++ b/lib/__init__.py @@ -116,9 +116,6 @@ def connect(dsn=None, connection_factory=None, cursor_factory=None, library: the list of supported parameters depends on the library version. """ - if dsn is None and not kwargs: - raise TypeError('missing dsn and no parameters') - dsn = _ext.make_dsn(dsn, **kwargs) conn = _connect(dsn, connection_factory=connection_factory, async=async) if cursor_factory is not None: diff --git a/lib/extensions.py b/lib/extensions.py index 3024b2fd..469f1932 100644 --- a/lib/extensions.py +++ b/lib/extensions.py @@ -157,6 +157,13 @@ class NoneAdapter(object): def make_dsn(dsn=None, **kwargs): """Convert a set of keywords into a connection strings.""" + if dsn is None and not kwargs: + raise TypeError('missing dsn and no parameters') + + # If no kwarg is specified don't mung the dsn + if not kwargs: + return dsn + # Override the dsn with the parameters if 'database' in kwargs: if 'dbname' in kwargs: diff --git a/tests/test_module.py b/tests/test_module.py index c0e4bf87..9f0adcc9 100755 --- a/tests/test_module.py +++ b/tests/test_module.py @@ -31,9 +31,11 @@ from testutils import ConnectingTestCase, skip_copy_if_green, script_to_py3 import psycopg2 + class ConnectTestCase(unittest.TestCase): def setUp(self): self.args = None + def conect_stub(dsn, connection_factory=None, async=False): self.args = (dsn, connection_factory, async) @@ -60,8 +62,8 @@ class ConnectTestCase(unittest.TestCase): self.assertEqual(self.args[2], False) def test_dsn(self): - psycopg2.connect('dbname=blah application_name=y') - self.assertDsnEqual(self.args[0], 'dbname=blah application_name=y') + psycopg2.connect('dbname=blah x=y') + self.assertEqual(self.args[0], 'dbname=blah x=y') self.assertEqual(self.args[1], None) self.assertEqual(self.args[2], False) @@ -93,24 +95,24 @@ class ConnectTestCase(unittest.TestCase): def f(dsn, async=False): pass - psycopg2.connect(database='foo', application_name='baz', connection_factory=f) - self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') + psycopg2.connect(database='foo', bar='baz', connection_factory=f) + self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') self.assertEqual(self.args[1], f) self.assertEqual(self.args[2], False) - psycopg2.connect("dbname=foo application_name=baz", connection_factory=f) - self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') + psycopg2.connect("dbname=foo bar=baz", connection_factory=f) + self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') self.assertEqual(self.args[1], f) self.assertEqual(self.args[2], False) def test_async(self): - psycopg2.connect(database='foo', application_name='baz', async=1) - self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') + psycopg2.connect(database='foo', bar='baz', async=1) + self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') self.assertEqual(self.args[1], None) self.assert_(self.args[2]) - psycopg2.connect("dbname=foo application_name=baz", async=True) - self.assertDsnEqual(self.args[0], 'dbname=foo application_name=baz') + psycopg2.connect("dbname=foo bar=baz", async=True) + self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') self.assertEqual(self.args[1], None) self.assert_(self.args[2]) @@ -141,6 +143,16 @@ class ConnectTestCase(unittest.TestCase): psycopg2.connect('dbname=foo', user='postgres') self.assertDsnEqual(self.args[0], 'dbname=foo user=postgres') + def test_no_dsn_munging(self): + psycopg2.connect('nosuchparam=whatevs') + self.assertEqual(self.args[0], 'nosuchparam=whatevs') + + psycopg2.connect(nosuchparam='whatevs') + self.assertEqual(self.args[0], 'nosuchparam=whatevs') + + self.assertRaises(psycopg2.ProgrammingError, + psycopg2.connect, 'nosuchparam=whatevs', andthis='either') + class ExceptionsTestCase(ConnectingTestCase): def test_attributes(self): @@ -205,7 +217,8 @@ class ExceptionsTestCase(ConnectingTestCase): self.assertEqual(diag.sqlstate, '42P01') del diag - gc.collect(); gc.collect() + gc.collect() + gc.collect() assert(w() is None) @skip_copy_if_green @@ -327,7 +340,7 @@ class TestVersionDiscovery(unittest.TestCase): self.assertTrue(type(psycopg2.__libpq_version__) is int) try: self.assertTrue(type(psycopg2.extensions.libpq_version()) is int) - except NotSupportedError: + except psycopg2.NotSupportedError: self.assertTrue(psycopg2.__libpq_version__ < 90100) From 52087a79d99009e41129601639e022d100d6491b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 15:31:37 +0000 Subject: [PATCH 08/14] Added test suite specific for make_dsn --- tests/test_connection.py | 117 +++++++++++++++++++++++++++------------ 1 file changed, 82 insertions(+), 35 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 68bb6f05..5b296949 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -32,6 +32,7 @@ from StringIO import StringIO import psycopg2 import psycopg2.errorcodes import psycopg2.extensions +ext = psycopg2.extensions from testutils import unittest, decorate_all_tests, skip_if_no_superuser from testutils import skip_before_postgres, skip_after_postgres, skip_before_libpq @@ -125,7 +126,7 @@ class ConnectionTests(ConnectingTestCase): if self.conn.server_version >= 90300: cur.execute("set client_min_messages=debug1") for i in range(0, 100, 10): - sql = " ".join(["create temp table table%d (id serial);" % j for j in range(i, i+10)]) + sql = " ".join(["create temp table table%d (id serial);" % j for j in range(i, i + 10)]) cur.execute(sql) self.assertEqual(50, len(conn.notices)) @@ -151,7 +152,7 @@ class ConnectionTests(ConnectingTestCase): # not limited, but no error for i in range(0, 100, 10): - sql = " ".join(["create temp table table2_%d (id serial);" % j for j in range(i, i+10)]) + sql = " ".join(["create temp table table2_%d (id serial);" % j for j in range(i, i + 10)]) cur.execute(sql) self.assertEqual(len([n for n in conn.notices if 'CREATE TABLE' in n]), @@ -172,7 +173,7 @@ class ConnectionTests(ConnectingTestCase): self.assert_(self.conn.server_version) def test_protocol_version(self): - self.assert_(self.conn.protocol_version in (2,3), + self.assert_(self.conn.protocol_version in (2, 3), self.conn.protocol_version) def test_tpc_unsupported(self): @@ -252,7 +253,7 @@ class ConnectionTests(ConnectingTestCase): t1.start() i = 1 for i in range(1000): - cur.execute("select %s;",(i,)) + cur.execute("select %s;", (i,)) conn.commit() while conn.notices: notices.append((1, conn.notices.pop())) @@ -313,16 +314,15 @@ class ConnectionTests(ConnectingTestCase): class ParseDsnTestCase(ConnectingTestCase): def test_parse_dsn(self): from psycopg2 import ProgrammingError - from psycopg2.extensions import parse_dsn - self.assertEqual(parse_dsn('dbname=test user=tester password=secret'), + self.assertEqual(ext.parse_dsn('dbname=test user=tester password=secret'), dict(user='tester', password='secret', dbname='test'), "simple DSN parsed") - self.assertRaises(ProgrammingError, parse_dsn, + self.assertRaises(ProgrammingError, ext.parse_dsn, "dbname=test 2 user=tester password=secret") - self.assertEqual(parse_dsn("dbname='test 2' user=tester password=secret"), + self.assertEqual(ext.parse_dsn("dbname='test 2' user=tester password=secret"), dict(user='tester', password='secret', dbname='test 2'), "DSN with quoting parsed") @@ -332,7 +332,7 @@ class ParseDsnTestCase(ConnectingTestCase): raised = False try: # unterminated quote after dbname: - parse_dsn("dbname='test 2 user=tester password=secret") + ext.parse_dsn("dbname='test 2 user=tester password=secret") except ProgrammingError, e: raised = True self.assertTrue(str(e).find('secret') < 0, @@ -343,16 +343,14 @@ class ParseDsnTestCase(ConnectingTestCase): @skip_before_libpq(9, 2) def test_parse_dsn_uri(self): - from psycopg2.extensions import parse_dsn - - self.assertEqual(parse_dsn('postgresql://tester:secret@/test'), + self.assertEqual(ext.parse_dsn('postgresql://tester:secret@/test'), dict(user='tester', password='secret', dbname='test'), "valid URI dsn parsed") raised = False try: # extra '=' after port value - parse_dsn(dsn='postgresql://tester:secret@/test?port=1111=x') + ext.parse_dsn(dsn='postgresql://tester:secret@/test?port=1111=x') except psycopg2.ProgrammingError, e: raised = True self.assertTrue(str(e).find('secret') < 0, @@ -362,24 +360,76 @@ class ParseDsnTestCase(ConnectingTestCase): self.assertTrue(raised, "ProgrammingError raised due to invalid URI") def test_unicode_value(self): - from psycopg2.extensions import parse_dsn snowman = u"\u2603" - d = parse_dsn('dbname=' + snowman) + d = ext.parse_dsn('dbname=' + snowman) if sys.version_info[0] < 3: self.assertEqual(d['dbname'], snowman.encode('utf8')) else: self.assertEqual(d['dbname'], snowman) def test_unicode_key(self): - from psycopg2.extensions import parse_dsn snowman = u"\u2603" - self.assertRaises(psycopg2.ProgrammingError, parse_dsn, + self.assertRaises(psycopg2.ProgrammingError, ext.parse_dsn, snowman + '=' + snowman) def test_bad_param(self): - from psycopg2.extensions import parse_dsn - self.assertRaises(TypeError, parse_dsn, None) - self.assertRaises(TypeError, parse_dsn, 42) + self.assertRaises(TypeError, ext.parse_dsn, None) + self.assertRaises(TypeError, ext.parse_dsn, 42) + + +class MakeDsnTestCase(ConnectingTestCase): + def assertDsnEqual(self, dsn1, dsn2): + self.assertEqual(set(dsn1.split()), set(dsn2.split())) + + def test_there_has_to_be_something(self): + self.assertRaises(TypeError, ext.make_dsn) + + def test_empty_param(self): + dsn = ext.make_dsn(database='sony', password='') + self.assertDsnEqual(dsn, "dbname=sony password=''") + + def test_escape(self): + dsn = ext.make_dsn(database='hello world') + self.assertEqual(dsn, "dbname='hello world'") + + dsn = ext.make_dsn(database=r'back\slash') + self.assertEqual(dsn, r"dbname=back\\slash") + + dsn = ext.make_dsn(database="quo'te") + self.assertEqual(dsn, r"dbname=quo\'te") + + dsn = ext.make_dsn(database="with\ttab") + self.assertEqual(dsn, "dbname='with\ttab'") + + dsn = ext.make_dsn(database=r"\every thing'") + self.assertEqual(dsn, r"dbname='\\every thing\''") + + def test_params_merging(self): + dsn = ext.make_dsn('dbname=foo', database='bar') + self.assertEqual(dsn, 'dbname=bar') + + dsn = ext.make_dsn('dbname=foo', user='postgres') + self.assertDsnEqual(dsn, 'dbname=foo user=postgres') + + def test_no_dsn_munging(self): + dsn = ext.make_dsn('nosuchparam=whatevs') + self.assertEqual(dsn, 'nosuchparam=whatevs') + + dsn = ext.make_dsn(nosuchparam='whatevs') + self.assertEqual(dsn, 'nosuchparam=whatevs') + + self.assertRaises(psycopg2.ProgrammingError, + ext.make_dsn, 'nosuchparam=whatevs', andthis='either') + + @skip_before_libpq(9, 2) + def test_url_is_cool(self): + dsn = ext.make_dsn('postgresql://tester:secret@/test') + self.assertEqual(dsn, 'postgresql://tester:secret@/test') + + dsn = ext.make_dsn('postgresql://tester:secret@/test', + application_name='woot') + self.assertDsnEqual(dsn, + 'dbname=test user=tester password=secret application_name=woot') class IsolationLevelsTestCase(ConnectingTestCase): @@ -587,7 +637,7 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn.close() return - gids = [ r[0] for r in cur ] + gids = [r[0] for r in cur] for gid in gids: cur.execute("rollback prepared %s;", (gid,)) cnn.close() @@ -761,13 +811,13 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): def test_status_after_recover(self): cnn = self.connect() self.assertEqual(psycopg2.extensions.STATUS_READY, cnn.status) - xns = cnn.tpc_recover() + cnn.tpc_recover() self.assertEqual(psycopg2.extensions.STATUS_READY, cnn.status) cur = cnn.cursor() cur.execute("select 1") self.assertEqual(psycopg2.extensions.STATUS_BEGIN, cnn.status) - xns = cnn.tpc_recover() + cnn.tpc_recover() self.assertEqual(psycopg2.extensions.STATUS_BEGIN, cnn.status) def test_recovered_xids(self): @@ -789,12 +839,12 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn = self.connect() xids = cnn.tpc_recover() - xids = [ xid for xid in xids if xid.database == dbname ] + xids = [xid for xid in xids if xid.database == dbname] xids.sort(key=attrgetter('gtrid')) # check the values returned self.assertEqual(len(okvals), len(xids)) - for (xid, (gid, prepared, owner, database)) in zip (xids, okvals): + for (xid, (gid, prepared, owner, database)) in zip(xids, okvals): self.assertEqual(xid.gtrid, gid) self.assertEqual(xid.prepared, prepared) self.assertEqual(xid.owner, owner) @@ -825,8 +875,7 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn.close() cnn = self.connect() - xids = [ xid for xid in cnn.tpc_recover() - if xid.database == dbname ] + xids = [x for x in cnn.tpc_recover() if x.database == dbname] self.assertEqual(1, len(xids)) xid = xids[0] self.assertEqual(xid.format_id, fid) @@ -847,8 +896,7 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn.close() cnn = self.connect() - xids = [ xid for xid in cnn.tpc_recover() - if xid.database == dbname ] + xids = [x for x in cnn.tpc_recover() if x.database == dbname] self.assertEqual(1, len(xids)) xid = xids[0] self.assertEqual(xid.format_id, None) @@ -893,8 +941,7 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn.tpc_begin(x1) cnn.tpc_prepare() cnn.reset() - xid = [ xid for xid in cnn.tpc_recover() - if xid.database == dbname ][0] + xid = [x for x in cnn.tpc_recover() if x.database == dbname][0] self.assertEqual(10, xid.format_id) self.assertEqual('uni', xid.gtrid) self.assertEqual('code', xid.bqual) @@ -909,8 +956,7 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn.tpc_prepare() cnn.reset() - xid = [ xid for xid in cnn.tpc_recover() - if xid.database == dbname ][0] + xid = [x for x in cnn.tpc_recover() if x.database == dbname][0] self.assertEqual(None, xid.format_id) self.assertEqual('transaction-id', xid.gtrid) self.assertEqual(None, xid.bqual) @@ -929,7 +975,7 @@ class ConnectionTwoPhaseTests(ConnectingTestCase): cnn.reset() xids = cnn.tpc_recover() - xid = [ xid for xid in xids if xid.database == dbname ][0] + xid = [x for x in xids if x.database == dbname][0] self.assertEqual(None, xid.format_id) self.assertEqual('dict-connection', xid.gtrid) self.assertEqual(None, xid.bqual) @@ -1182,7 +1228,8 @@ class ReplicationTest(ConnectingTestCase): @skip_before_postgres(9, 0) def test_replication_not_supported(self): conn = self.repl_connect() - if conn is None: return + if conn is None: + return cur = conn.cursor() f = StringIO() self.assertRaises(psycopg2.NotSupportedError, From 6893295a8794f332cb9c2a667001ba7dcefb880b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 16:08:09 +0000 Subject: [PATCH 09/14] Added docs about make_dsn connect() docs updated to document the arguments merging. --- doc/src/extensions.rst | 24 +++++++++++++++++++++++- doc/src/module.rst | 20 +++++++++----------- lib/__init__.py | 4 ++-- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/doc/src/extensions.rst b/doc/src/extensions.rst index 68619e5f..594ffc03 100644 --- a/doc/src/extensions.rst +++ b/doc/src/extensions.rst @@ -491,6 +491,27 @@ Other functions .. __: http://www.postgresql.org/docs/current/static/libpq-misc.html#LIBPQ-PQLIBVERSION +.. function:: make_dsn(dsn=None, \*\*kwargs) + + Create a connection string from arguments. + + Put together the arguments in *kwargs* into a connection string. If *dsn* + is specified too, merge the arguments coming from both the sources. If the + same argument is specified in both the sources, the *kwargs* version + overrides the *dsn* version + + At least one param is required (either *dsn* or any keyword). Note that + the empty string is a valid connection string. + + Example:: + + >>> from psycopg2.extensions import make_dsn + >>> make_dsn('dbname=foo host=example.com', password="s3cr3t") + 'host=example.com password=s3cr3t dbname=foo' + + .. versionadded:: 2.7 + + .. function:: parse_dsn(dsn) Parse connection string into a dictionary of keywords and values. @@ -500,7 +521,8 @@ Other functions Example:: - >>> psycopg2.extensions.parse_dsn('dbname=test user=postgres password=secret') + >>> from psycopg2.extensions import parse_dsn + >>> parse_dsn('dbname=test user=postgres password=secret') {'password': 'secret', 'user': 'postgres', 'dbname': 'test'} .. versionadded:: 2.7 diff --git a/doc/src/module.rst b/doc/src/module.rst index 6950b703..25a9ba27 100644 --- a/doc/src/module.rst +++ b/doc/src/module.rst @@ -17,30 +17,25 @@ The module interface respects the standard defined in the |DBAPI|_. single: DSN (Database Source Name) .. function:: - connect(dsn, connection_factory=None, cursor_factory=None, async=False) - connect(\*\*kwargs, connection_factory=None, cursor_factory=None, async=False) + connect(dsn, connection_factory=None, cursor_factory=None, async=False, \*\*kwargs) Create a new database session and return a new `connection` object. - The connection parameters can be specified either as a `libpq connection + The connection parameters can be specified as a `libpq connection string`__ using the *dsn* parameter:: conn = psycopg2.connect("dbname=test user=postgres password=secret") or using a set of keyword arguments:: - conn = psycopg2.connect(database="test", user="postgres", password="secret") + conn = psycopg2.connect(dbname"test", user="postgres", password="secret") - The two call styles are mutually exclusive: you cannot specify connection - parameters as keyword arguments together with a connection string; only - the parameters not needed for the database connection (*i.e.* - *connection_factory*, *cursor_factory*, and *async*) are supported - together with the *dsn* argument. + or using a mix of both: if the same parameter name is specified in both + sources the *kwargs* value will have precedence over the *dsn* value. The basic connection parameters are: - - `!dbname` -- the database name (only in the *dsn* string) - - `!database` -- the database name (only as keyword argument) + - `!dbname` -- the database name (`!database` is a deprecated alias) - `!user` -- user name used to authenticate - `!password` -- password used to authenticate - `!host` -- database host address (defaults to UNIX socket if not provided) @@ -76,6 +71,9 @@ The module interface respects the standard defined in the |DBAPI|_. .. versionchanged:: 2.5 added the *cursor_factory* parameter. + .. versionchanged:: 2.7 + both *dsn* and keyword arguments can be specified. + .. seealso:: - `~psycopg2.extensions.parse_dsn` diff --git a/lib/__init__.py b/lib/__init__.py index 4a288197..698f50d6 100644 --- a/lib/__init__.py +++ b/lib/__init__.py @@ -86,7 +86,7 @@ def connect(dsn=None, connection_factory=None, cursor_factory=None, """ Create a new database connection. - The connection parameters can be specified either as a string: + The connection parameters can be specified as a string: conn = psycopg2.connect("dbname=test user=postgres password=secret") @@ -94,7 +94,7 @@ def connect(dsn=None, connection_factory=None, cursor_factory=None, conn = psycopg2.connect(database="test", user="postgres", password="secret") - The basic connection parameters are: + Or as a mix of both. The basic connection parameters are: - *dbname*: the database name - *database*: the database name (only as keyword argument) From 7155d06cdcc15064d28f8667e72d8f61b9b31e90 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 16:09:33 +0000 Subject: [PATCH 10/14] Test that the empty dsn is a valid make_dsn input --- tests/test_connection.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_connection.py b/tests/test_connection.py index 5b296949..f2ea3d67 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -384,6 +384,10 @@ class MakeDsnTestCase(ConnectingTestCase): def test_there_has_to_be_something(self): self.assertRaises(TypeError, ext.make_dsn) + def test_empty_string(self): + dsn = ext.make_dsn('') + self.assertEqual(dsn, '') + def test_empty_param(self): dsn = ext.make_dsn(database='sony', password='') self.assertDsnEqual(dsn, "dbname=sony password=''") From 7aab934ae5950c6fa1bcd25ed053857538624310 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 16:52:53 +0000 Subject: [PATCH 11/14] Validate output result from make_dsn() The output is not necessarily munged anyway: if no keyword is passed, validate the input but return it untouched. --- doc/src/extensions.rst | 26 +++++++++++++++----- lib/extensions.py | 7 +++++- psycopg/psycopgmodule.c | 2 +- tests/test_connection.py | 51 ++++++++++++++++++++++++---------------- tests/test_module.py | 34 ++++++++++----------------- 5 files changed, 70 insertions(+), 50 deletions(-) diff --git a/doc/src/extensions.rst b/doc/src/extensions.rst index 594ffc03..fa69d628 100644 --- a/doc/src/extensions.rst +++ b/doc/src/extensions.rst @@ -493,15 +493,19 @@ Other functions .. function:: make_dsn(dsn=None, \*\*kwargs) - Create a connection string from arguments. + Create a valid connection string from arguments. Put together the arguments in *kwargs* into a connection string. If *dsn* is specified too, merge the arguments coming from both the sources. If the same argument is specified in both the sources, the *kwargs* version - overrides the *dsn* version + overrides the *dsn* version. - At least one param is required (either *dsn* or any keyword). Note that - the empty string is a valid connection string. + At least one parameter is required (either *dsn* or any keyword). Note + that the empty string is a valid connection string. + + The input arguments are validated: the output should always be a valid + connection string (as far as `parse_dsn()` is concerned). If not raise + `~psycopg2.ProgrammingError`. Example:: @@ -516,17 +520,27 @@ Other functions Parse connection string into a dictionary of keywords and values. - Uses libpq's ``PQconninfoParse`` to parse the string according to - accepted format(s) and check for supported keywords. + Parsing is delegated to the libpq: different versions of the client + library may support different formats or parameters (for example, + `connection URIs`__ are only supported from libpq 9.2). Raise + `~psycopg2.ProgrammingError` if the *dsn* is not valid. + + .. __: http://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING Example:: >>> from psycopg2.extensions import parse_dsn >>> parse_dsn('dbname=test user=postgres password=secret') {'password': 'secret', 'user': 'postgres', 'dbname': 'test'} + >>> parse_dsn("postgresql://someone@example.com/somedb?connect_timeout=10") + {'host': 'example.com', 'user': 'someone', 'dbname': 'somedb', 'connect_timeout': '10'} .. versionadded:: 2.7 + .. seealso:: libpq docs for `PQconninfoParse()`__. + + .. __: http://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-PQCONNINFOPARSE + .. function:: quote_ident(str, scope) diff --git a/lib/extensions.py b/lib/extensions.py index 469f1932..39cc4de6 100644 --- a/lib/extensions.py +++ b/lib/extensions.py @@ -160,8 +160,9 @@ def make_dsn(dsn=None, **kwargs): if dsn is None and not kwargs: raise TypeError('missing dsn and no parameters') - # If no kwarg is specified don't mung the dsn + # If no kwarg is specified don't mung the dsn, but verify it if not kwargs: + parse_dsn(dsn) return dsn # Override the dsn with the parameters @@ -178,6 +179,10 @@ def make_dsn(dsn=None, **kwargs): dsn = " ".join(["%s=%s" % (k, _param_escape(str(v))) for (k, v) in kwargs.iteritems()]) + + # verify that the returned dsn is valid + parse_dsn(dsn) + return dsn diff --git a/psycopg/psycopgmodule.c b/psycopg/psycopgmodule.c index 24dd5f75..55f8ed95 100644 --- a/psycopg/psycopgmodule.c +++ b/psycopg/psycopgmodule.c @@ -133,7 +133,7 @@ psyco_parse_dsn(PyObject *self, PyObject *args, PyObject *kwargs) options = PQconninfoParse(Bytes_AS_STRING(dsn), &err); if (options == NULL) { if (err != NULL) { - PyErr_Format(ProgrammingError, "error parsing the dsn: %s", err); + PyErr_Format(ProgrammingError, "invalid dsn: %s", err); PQfreemem(err); } else { PyErr_SetString(OperationalError, "PQconninfoParse() failed"); diff --git a/tests/test_connection.py b/tests/test_connection.py index f2ea3d67..0158f5cc 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -388,53 +388,64 @@ class MakeDsnTestCase(ConnectingTestCase): dsn = ext.make_dsn('') self.assertEqual(dsn, '') + def test_params_validation(self): + self.assertRaises(psycopg2.ProgrammingError, + ext.make_dsn, 'dbnamo=a') + self.assertRaises(psycopg2.ProgrammingError, + ext.make_dsn, dbnamo='a') + self.assertRaises(psycopg2.ProgrammingError, + ext.make_dsn, 'dbname=a', nosuchparam='b') + def test_empty_param(self): - dsn = ext.make_dsn(database='sony', password='') + dsn = ext.make_dsn(dbname='sony', password='') self.assertDsnEqual(dsn, "dbname=sony password=''") def test_escape(self): - dsn = ext.make_dsn(database='hello world') + dsn = ext.make_dsn(dbname='hello world') self.assertEqual(dsn, "dbname='hello world'") - dsn = ext.make_dsn(database=r'back\slash') + dsn = ext.make_dsn(dbname=r'back\slash') self.assertEqual(dsn, r"dbname=back\\slash") - dsn = ext.make_dsn(database="quo'te") + dsn = ext.make_dsn(dbname="quo'te") self.assertEqual(dsn, r"dbname=quo\'te") - dsn = ext.make_dsn(database="with\ttab") + dsn = ext.make_dsn(dbname="with\ttab") self.assertEqual(dsn, "dbname='with\ttab'") - dsn = ext.make_dsn(database=r"\every thing'") + dsn = ext.make_dsn(dbname=r"\every thing'") self.assertEqual(dsn, r"dbname='\\every thing\''") + def test_database_is_a_keyword(self): + self.assertEqual(ext.make_dsn(database='sigh'), "dbname=sigh") + def test_params_merging(self): - dsn = ext.make_dsn('dbname=foo', database='bar') - self.assertEqual(dsn, 'dbname=bar') + dsn = ext.make_dsn('dbname=foo host=bar', host='baz') + self.assertDsnEqual(dsn, 'dbname=foo host=baz') dsn = ext.make_dsn('dbname=foo', user='postgres') self.assertDsnEqual(dsn, 'dbname=foo user=postgres') def test_no_dsn_munging(self): - dsn = ext.make_dsn('nosuchparam=whatevs') - self.assertEqual(dsn, 'nosuchparam=whatevs') - - dsn = ext.make_dsn(nosuchparam='whatevs') - self.assertEqual(dsn, 'nosuchparam=whatevs') - - self.assertRaises(psycopg2.ProgrammingError, - ext.make_dsn, 'nosuchparam=whatevs', andthis='either') + dsnin = 'dbname=a host=b user=c password=d' + dsn = ext.make_dsn(dsnin) + self.assertEqual(dsn, dsnin) @skip_before_libpq(9, 2) def test_url_is_cool(self): - dsn = ext.make_dsn('postgresql://tester:secret@/test') - self.assertEqual(dsn, 'postgresql://tester:secret@/test') + url = 'postgresql://tester:secret@/test?application_name=wat' + dsn = ext.make_dsn(url) + self.assertEqual(dsn, url) - dsn = ext.make_dsn('postgresql://tester:secret@/test', - application_name='woot') + dsn = ext.make_dsn(url, application_name='woot') self.assertDsnEqual(dsn, 'dbname=test user=tester password=secret application_name=woot') + self.assertRaises(psycopg2.ProgrammingError, + ext.make_dsn, 'postgresql://tester:secret@/test?nosuch=param') + self.assertRaises(psycopg2.ProgrammingError, + ext.make_dsn, url, nosuch="param") + class IsolationLevelsTestCase(ConnectingTestCase): diff --git a/tests/test_module.py b/tests/test_module.py index 9f0adcc9..a6918cb0 100755 --- a/tests/test_module.py +++ b/tests/test_module.py @@ -62,8 +62,8 @@ class ConnectTestCase(unittest.TestCase): self.assertEqual(self.args[2], False) def test_dsn(self): - psycopg2.connect('dbname=blah x=y') - self.assertEqual(self.args[0], 'dbname=blah x=y') + psycopg2.connect('dbname=blah host=y') + self.assertEqual(self.args[0], 'dbname=blah host=y') self.assertEqual(self.args[1], None) self.assertEqual(self.args[2], False) @@ -88,31 +88,31 @@ class ConnectTestCase(unittest.TestCase): self.assertEqual(len(self.args[0].split()), 4) def test_generic_keywords(self): - psycopg2.connect(foo='bar') - self.assertEqual(self.args[0], 'foo=bar') + psycopg2.connect(options='stuff') + self.assertEqual(self.args[0], 'options=stuff') def test_factory(self): def f(dsn, async=False): pass - psycopg2.connect(database='foo', bar='baz', connection_factory=f) - self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect(database='foo', host='baz', connection_factory=f) + self.assertDsnEqual(self.args[0], 'dbname=foo host=baz') self.assertEqual(self.args[1], f) self.assertEqual(self.args[2], False) - psycopg2.connect("dbname=foo bar=baz", connection_factory=f) - self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect("dbname=foo host=baz", connection_factory=f) + self.assertDsnEqual(self.args[0], 'dbname=foo host=baz') self.assertEqual(self.args[1], f) self.assertEqual(self.args[2], False) def test_async(self): - psycopg2.connect(database='foo', bar='baz', async=1) - self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect(database='foo', host='baz', async=1) + self.assertDsnEqual(self.args[0], 'dbname=foo host=baz') self.assertEqual(self.args[1], None) self.assert_(self.args[2]) - psycopg2.connect("dbname=foo bar=baz", async=True) - self.assertDsnEqual(self.args[0], 'dbname=foo bar=baz') + psycopg2.connect("dbname=foo host=baz", async=True) + self.assertDsnEqual(self.args[0], 'dbname=foo host=baz') self.assertEqual(self.args[1], None) self.assert_(self.args[2]) @@ -143,16 +143,6 @@ class ConnectTestCase(unittest.TestCase): psycopg2.connect('dbname=foo', user='postgres') self.assertDsnEqual(self.args[0], 'dbname=foo user=postgres') - def test_no_dsn_munging(self): - psycopg2.connect('nosuchparam=whatevs') - self.assertEqual(self.args[0], 'nosuchparam=whatevs') - - psycopg2.connect(nosuchparam='whatevs') - self.assertEqual(self.args[0], 'nosuchparam=whatevs') - - self.assertRaises(psycopg2.ProgrammingError, - psycopg2.connect, 'nosuchparam=whatevs', andthis='either') - class ExceptionsTestCase(ConnectingTestCase): def test_attributes(self): From c9fd828f8adeeed108a326892394c47e3747b558 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 16:58:24 +0000 Subject: [PATCH 12/14] Allow make_dsn to take no parameter The behaviour of connect() is unchanged: either dsn or params must be specified. --- doc/src/extensions.rst | 7 ++----- lib/__init__.py | 3 +++ lib/extensions.py | 2 +- tests/test_connection.py | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/src/extensions.rst b/doc/src/extensions.rst index fa69d628..b661895d 100644 --- a/doc/src/extensions.rst +++ b/doc/src/extensions.rst @@ -497,11 +497,8 @@ Other functions Put together the arguments in *kwargs* into a connection string. If *dsn* is specified too, merge the arguments coming from both the sources. If the - same argument is specified in both the sources, the *kwargs* version - overrides the *dsn* version. - - At least one parameter is required (either *dsn* or any keyword). Note - that the empty string is a valid connection string. + same argument name is specified in both the sources, the *kwargs* value + overrides the *dsn* value. The input arguments are validated: the output should always be a valid connection string (as far as `parse_dsn()` is concerned). If not raise diff --git a/lib/__init__.py b/lib/__init__.py index 698f50d6..829e29eb 100644 --- a/lib/__init__.py +++ b/lib/__init__.py @@ -116,6 +116,9 @@ def connect(dsn=None, connection_factory=None, cursor_factory=None, library: the list of supported parameters depends on the library version. """ + if dsn is None and not kwargs: + raise TypeError('missing dsn and no parameters') + dsn = _ext.make_dsn(dsn, **kwargs) conn = _connect(dsn, connection_factory=connection_factory, async=async) if cursor_factory is not None: diff --git a/lib/extensions.py b/lib/extensions.py index 39cc4de6..21300985 100644 --- a/lib/extensions.py +++ b/lib/extensions.py @@ -158,7 +158,7 @@ class NoneAdapter(object): def make_dsn(dsn=None, **kwargs): """Convert a set of keywords into a connection strings.""" if dsn is None and not kwargs: - raise TypeError('missing dsn and no parameters') + return '' # If no kwarg is specified don't mung the dsn, but verify it if not kwargs: diff --git a/tests/test_connection.py b/tests/test_connection.py index 0158f5cc..ddec8cfc 100755 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -381,8 +381,8 @@ class MakeDsnTestCase(ConnectingTestCase): def assertDsnEqual(self, dsn1, dsn2): self.assertEqual(set(dsn1.split()), set(dsn2.split())) - def test_there_has_to_be_something(self): - self.assertRaises(TypeError, ext.make_dsn) + def test_empty_arguments(self): + self.assertEqual(ext.make_dsn(), '') def test_empty_string(self): dsn = ext.make_dsn('') From e33073576cc7d238a8a01e6674b1232b928cdd27 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 17:02:47 +0000 Subject: [PATCH 13/14] Brag about make_dsn in the NEWS file --- NEWS | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index c1e4152f..9467fea7 100644 --- a/NEWS +++ b/NEWS @@ -6,7 +6,10 @@ What's new in psycopg 2.7 New features: -- Added `~psycopg2.extensions.parse_dsn()` function (:ticket:`#321`). +- Added `~psycopg2.extensions.parse_dsn()` and + `~psycopg2.extensions.make_dsn()` functions (:tickets:`#321, #363`). + `~psycopg2.connect()` now can take both *dsn* and keyword arguments, merging + them together. - Added `~psycopg2.__libpq_version__` and `~psycopg2.extensions.libpq_version()` to inspect the version of the ``libpq`` library the module was compiled/loaded with From ab5d8f419069bec1c35329fd67b9fe76fbbce4c8 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 3 Mar 2016 17:28:56 +0000 Subject: [PATCH 14/14] Style the dsn arg in connect() as a normal optional parameter Plus some more connect() docs wordsmithing. --- doc/src/module.rst | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/src/module.rst b/doc/src/module.rst index 25a9ba27..97fbdf19 100644 --- a/doc/src/module.rst +++ b/doc/src/module.rst @@ -17,7 +17,7 @@ The module interface respects the standard defined in the |DBAPI|_. single: DSN (Database Source Name) .. function:: - connect(dsn, connection_factory=None, cursor_factory=None, async=False, \*\*kwargs) + connect(dsn=None, connection_factory=None, cursor_factory=None, async=False, \*\*kwargs) Create a new database session and return a new `connection` object. @@ -31,7 +31,9 @@ The module interface respects the standard defined in the |DBAPI|_. conn = psycopg2.connect(dbname"test", user="postgres", password="secret") or using a mix of both: if the same parameter name is specified in both - sources the *kwargs* value will have precedence over the *dsn* value. + sources, the *kwargs* value will have precedence over the *dsn* value. + Note that either the *dsn* or at least one connection-related keyword + argument is required. The basic connection parameters are: @@ -42,7 +44,7 @@ The module interface respects the standard defined in the |DBAPI|_. - `!port` -- connection port number (defaults to 5432 if not provided) Any other connection parameter supported by the client library/server can - be passed either in the connection string or as keywords. The PostgreSQL + be passed either in the connection string or as a keyword. The PostgreSQL documentation contains the complete list of the `supported parameters`__. Also note that the same parameters can be passed to the client library using `environment variables`__. @@ -87,8 +89,8 @@ The module interface respects the standard defined in the |DBAPI|_. .. extension:: - The parameters *connection_factory* and *async* are Psycopg extensions - to the |DBAPI|. + The non-connection-related keyword parameters are Psycopg extensions + to the |DBAPI|_. .. data:: apilevel