fix(db): Make using pg_service work again.

When I did the merge of schema support I broke pg_service.conf support
by replacing get_database_name with db_schema_split.  This commit
fixes it.

Also this commit returns the schema if one is specified in
pg_service.conf.

back_postgresql.py:

  Replace calls to db_schema_split() with get_database_schema_names()
  (new name for get_database_name()).  Rename db_schema_split to
  _db_schema_split. It now returns a tuple (dbname, schema) rather
  than a list. It is used only by get_database_schema_names() which
  also returns tuples.

  get_database_schema_names() can also get schema info for the service

  (if present) as specified by pg_service.conf.

  Add get_database_user() to get the user from either RDBMS_USER or
  pg_service.conf. (User needed for creating schema, so not needed
  before schema patch.

  import re at the top of file and remove lower import.

  Remove some schema code from db_command as it's not needed. The
  database conection is done to either postgresql or template1
  existing databases. This command never connects to the roundp
  specified db.

test/test_postgresql.py:

  Reorganize top level imports, add import os.  Replace import of
  db_schema_split with get_database_schema_names. Also replace calls
  to db_schema_split.

  Create new Opener for the service file. Set PGSERVICEFILE to point
  to test/pg_service.conf.

  Add three new classes to test Service:

    1) using regular db
    2) using schema within db
    3) Unable to parse schema name from pg_service.conf.

  The last doesn't need a db. Number 1 and 2 reuse the tests in ROTest
  to verify db connectivity.

test/pg_service.conf:

  three service connections for: db only, db and schema, and incorrectly
  specified schema test cases.

doc/upgrading.txt:

  updated to current status. Included example schema definition in
  service file.

Author: rouilj

doc/upgrading.txt

@@ -163,11 +163,31 @@ keyword to ``roundup_database.roundup_schema`` will create
 and use the ``roundup_schema`` in the pre-created
 ``roundup_database``.
 
-Also there is a new configuration keyword in the rdbms section of
-``config.ini``. The ``service`` keyword allows you to define the
-service name for Postgres that will be looked up in the Postgres
-`Connection Service File`_. Setting service to `roundup` with the
-following in the service file::
+Also there is a new configuration keyword in the rdbms
+section of ``config.ini``. The ``service`` keyword allows
+you to define the service name for Postgres that will be
+looked up in the `Connection Service File`_. Any of the
+methods of specifying the file including by using the
+``PGSERVICEFILE`` environment variable are supported.
+
+This is similar to the existing support for MySQL
+option/config files and groups.
+
+If you use services, any settings for the same properties
+(user, name, password ...) that are in the tracker's
+``config.ini`` will override the service settings. So you
+want to leave the ``config.ini`` settings blank. E.G.::
+
+  [rdbms]
+  name = 
+  host =
+  port = 
+  user =
+  password =
+  service =  roundup_roundup
+
+Setting ``service`` to ``roundup_roundup`` with
+the following in the service file::
 
    [roundup_roundup]
    host=127.0.0.1
@@ -176,13 +196,21 @@ following in the service file::
    password=roundup
    dbname=roundup
 
-would use the roundup database with the specified credentials.
+would use the roundup database with the specified
+credentials. It is possible to define a service that
+connects to a specific schema using::
+
+  options=-c search_path=roundup_service_dev
+
+Note that the first schema specified after ``search_path=``
+is created and populated. The schema name
+(``roundup_service_dev``) must be terminated by: a comma,
+whitespace or end of line.
 
-It is possible to define a service that connects to a specific
-schema. However this will require a little fiddling to get things
-working. A future enhancement may make using a schema via this
-mechanism easier. See https://issues.roundup-tracker.org/issue2551299
-for details.
+You can use the command ``psql "service=db_service_name"``
+to verify the settings in the connection file. Inside of
+``psql`` you can verify the ``search_path`` using ``show
+search_path;``.
 
 .. _`Connection Service File`: https://www.postgresql.org/docs/current/libpq-pgservice.html
 

roundup/backends/back_postgresql.py

@@ -48,29 +48,30 @@ def connection_dict(config, dbnamestr=None):
         del d['read_default_file']
     return d
 
-def db_schema_split(database_name):
+def _db_schema_split(database_name):
     ''' Split database_name into database and schema parts'''
     if '.' in database_name:
         return database_name.split ('.')
-    return [database_name, '']
+    return (database_name, '')
 
 def db_create(config):
     """Clear all database contents and drop database itself"""
-    db_name, schema_name = db_schema_split(config.RDBMS_NAME)
+    db_name, schema_name = get_database_schema_names(config)
     if not schema_name:
         command = "CREATE DATABASE \"%s\" WITH ENCODING='UNICODE'" % db_name
         if config.RDBMS_TEMPLATE:
             command = command + " TEMPLATE=%s" % config.RDBMS_TEMPLATE
         logging.getLogger('roundup.hyperdb').info(command)
         db_command(config, command)
     else:
-        command = "CREATE SCHEMA \"%s\" AUTHORIZATION \"%s\"" % (schema_name, config.RDBMS_USER)
+        command = "CREATE SCHEMA \"%s\" AUTHORIZATION \"%s\"" % (
+            schema_name, get_database_user_name(config))
         logging.getLogger('roundup.hyperdb').info(command)
         db_command(config, command, db_name)
 
 def db_nuke(config):
     """Drop the database (and all its contents) or the schema."""
-    db_name, schema_name = db_schema_split(config.RDBMS_NAME)
+    db_name, schema_name = get_database_schema_names(config)
     if not schema_name:
         command = 'DROP DATABASE "%s"'% db_name
         logging.getLogger('roundup.hyperdb').info(command)
@@ -82,28 +83,86 @@ def db_nuke(config):
     if os.path.exists(config.DATABASE):
         shutil.rmtree(config.DATABASE)
 
-def get_database_name(config):
-    '''Get database name using config.RDBMS_NAME or config.RDBMS_SERVICE.
+def get_database_schema_names(config):
+    '''Get database and schema names using config.RDBMS_NAME or service
+       defined by config.RDBMS_SERVICE.
+
+       If database specifed using RDBMS_SERVICE does not exist, the
+       error message is parsed for the database name. This database
+       can then be created by calling code. Parsing will fail if the
+       error message changes. The alternative is to try to find and
+       parse the .pg_service .ini style file on unix/windows. This is
+       less palatable.
+
+       If the database specified using RDBMS_SERVICE exists, (e.g. we
+       are doing a nuke operation), use
+       psycopg.extensions.ConnectionInfo to get the dbname. Also parse
+       the search_path options setting to get the schema. Only the
+       first element of the search_path is returned.  This requires
+       psycopg2 > 2.8 from 2018.
+    '''
 
-       If database specifed using RDBMS_SERVICE does not exist,
-       the error message is parsed for the database name. This
-       will fail if the error message changes. The alternative is
-       to try to find and parse the .pg_service .ini style file on
-       unix/windows. This is less palatable.
+    if config.RDBMS_NAME:
+        return _db_schema_split(config.RDBMS_NAME)
+
+    template1 = connection_dict(config)
+    try:
+        conn = psycopg2.connect(**template1)
+    except psycopg2.OperationalError as message:
+        # extract db name from error:
+        #  'connection to server at "127.0.0.1", port 5432 failed: \
+        #   FATAL:  database "rounduptest" does not exist\n'
+        # ugh.
+        #
+        # Database name is any character sequence not including a " or
+        # whitespace. Arguably both are allowed by:
+        # 
+        #  https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
+        #
+        # with suitable quoting but ... really.
+        search = re.search(
+            r'FATAL:\s+database\s+"([^"\s]*)"\s+does\s+not\s+exist',
+            message.args[0])
+        if search:
+            dbname = search.groups()[0]
+            # To use a schema, the db has to have been precreated.
+            # So return '' for schema if database does not exist.
+            return (dbname, '')
+
+        raise hyperdb.DatabaseError(
+            "Unable to determine database from service: %s" % message)
+
+    dbname = psycopg2.extensions.ConnectionInfo(conn).dbname
+    schema = ''
+    options = psycopg2.extensions.ConnectionInfo(conn).options
+    conn.close()
+
+    # Assume schema is first word in the search_path spec.
+    # , (for multiple items in path) and whitespace (for another option)
+    # end the schema name.
+    m = re.search(r'search_path=([^,\s]*)', options)
+    if m:
+        schema = m.group(1)
+        if not schema:
+            raise ValueError('Unable to get schema for service: "%s" from options: "%s"' % (template1['service'], options))
+
+    return (dbname, schema)
+
+def get_database_user_name(config):
+    '''Get database username using config.RDBMS_USER or return
+       user from connection created using config.RDBMS_SERVICE.
 
        If the database specified using RDBMS_SERVICE does exist, (i.e. we
        are doing a nuke operation), use psycopg.extensions.ConnectionInfo
-       to get the dbname. This requires psycopg2 > 2.8 from 2018.
+       to get the user. This requires psycopg2 > 2.8 from 2018.
     '''
-
-    if config.RDBMS_NAME:
-        return config.RDBMS_NAME
+    if config.RDBMS_USER:
+        return config.RDBMS_USER
 
     template1 = connection_dict(config)
     try:
         conn = psycopg2.connect(**template1)
     except psycopg2.OperationalError as message:
-        import re
         # extract db name from error:
         #  'connection to server at "127.0.0.1", port 5432 failed: \
         #   FATAL:  database "rounduptest" does not exist\n'
@@ -120,14 +179,17 @@ def get_database_name(config):
             message.args[0])
         if search:
             dbname = search.groups()[0]
-            return dbname
+            # To have a user, the db has to exist already.
+            # so return '' for user.
+            return ''
 
         raise hyperdb.DatabaseError(
             "Unable to determine database from service: %s" % message)
 
-    dbname = psycopg2.extensions.ConnectionInfo(conn).dbname
+    user = psycopg2.extensions.ConnectionInfo(conn).user
     conn.close()
-    return dbname
+
+    return user
 
 def db_command(config, command, database='postgres'):
     '''Perform some sort of database-level command. Retry 10 times if we
@@ -138,15 +200,13 @@ def db_command(config, command, database='postgres'):
     Compare to issue2550543.
     '''
     template1 = connection_dict(config, 'database')
-    db_name, schema_name = db_schema_split(template1['database'])
     template1['database'] = database
 
     try:
         conn = psycopg2.connect(**template1)
     except psycopg2.OperationalError as message:
-        if not schema_name:
-            if re.search(r'database ".+" does not exist', str(message)):
-                return db_command(config, command, database='template1')
+        if re.search(r'database ".+" does not exist', str(message)):
+            return db_command(config, command, database='template1')
         raise hyperdb.DatabaseError(message)
 
     conn.set_isolation_level(0)
@@ -186,7 +246,7 @@ def pg_command(cursor, command, args=()):
 def db_exists(config):
     """Check if database or schema already exists"""
     db = connection_dict(config, 'database')
-    db_name, schema_name = db_schema_split(db['database'])
+    db_name, schema_name = get_database_schema_names(config)
     if schema_name:
         db['database'] = db_name
     try:
@@ -253,7 +313,7 @@ class Database(rdbms_common.Database):
 
     def sql_open_connection(self):
         db = connection_dict(self.config, 'database')
-        db_name, schema_name = db_schema_split (db['database'])
+        db_name, schema_name = get_database_schema_names(self.config)
         if schema_name:
             db['database'] = db_name
 

test/pg_service.conf

@@ -0,0 +1,23 @@
+[roundup_test_db]
+host=127.0.0.1
+port=5432
+user=rounduptest
+password=rounduptest
+dbname=rounduptest
+
+[roundup_test_schema]
+host=127.0.0.1
+port=5432
+user=rounduptest_schema
+password=rounduptest
+dbname=rounduptest_schema
+options=-c search_path=roundup_service_dev
+
+[roundup_test_schema_bad]
+host=127.0.0.1
+port=5432
+user=rounduptest_schema
+password=rounduptest
+dbname=rounduptest_schema
+options=-c search_path= 
+