Prefer $connect_settings over explicit parameters

This is the outcome of a large refactoring in #1450 and discussions on
IRC and slack. We noticed that the behaviour of `$connect_settings` is
currently inconsistent. Sometimes it's preferred over explicity
parameters, sometimes not.

Reminder: The idea of `$connect_settings` is to provide a hash with
environment variable to tell puppet to manage a remote database instead
of a local instance.

One example is: https://github.com/puppetlabs/puppetlabs-postgresql/blob/93386b48861ff41d5f47033afee592e0506526c5/manifests/server/grant.pp#L80-L86

```
if $port {
  $port_override = $port
} elsif 'PGPORT' in $connect_settings {
  $port_override = undef
} else {
  $port_override = $postgresql::server::port
}
```

Here the local $port parameter is preferred over `$connect_settings`.
The problem is that we now cannot set a default in the resource for
`$port`. The default is hardcoded to `$postgresql::server::port`. This
becomes. Other classes handle it in a different way:

https://github.com/puppetlabs/puppetlabs-postgresql/blob/93386b48861ff41d5f47033afee592e0506526c5/manifests/server/database.pp#L41-L46

```
if 'PGPORT' in $connect_settings {
  $port_override = undef
} else {
  $port_override = $port
}
```

Here `$connct_settings` is checked first. It defaults to an empty hash.
If `PGPORT` isn't in it, `$port` is used. The logic is shorter and
enables us to expose $port as a parameter with a default value. With
this approach users can decide if they pass `$port` or
`$connect_settings`.

At the moment the remote database support is broken because the logic to
parse `$connect_settings` isn't consistent. The goal of this PR is to
unify the logic and prefer `$connect_settings` if it's provided by the
user.

Author: bastelfreak

README.md

@@ -175,11 +175,15 @@ class { 'postgresql::server':
 
 ### Manage remote users, roles, and permissions
 
-Remote SQL objects are managed using the same Puppet resources as local SQL objects, along with a [`connect_settings`](#connect_settings) hash. This provides control over how Puppet connects to the remote Postgres instances and which version is used for generating SQL commands.
+Remote SQL objects are managed using the same Puppet resources as local SQL objects, along with a `$connect_settings` hash. This provides control over how Puppet connects to the remote Postgres instances and which version is used for generating SQL commands.
 
-The `connect_settings` hash can contain environment variables to control Postgres client connections, such as 'PGHOST', 'PGPORT', 'PGPASSWORD', and 'PGSSLKEY'. See the [PostgreSQL Environment Variables](http://www.postgresql.org/docs/9.4/static/libpq-envars.html)  documentation for a complete list of variables.
+The `connect_settings` hash can contain environment variables to control Postgres client connections, such as 'PGHOST', 'PGPORT', 'PGPASSWORD', 'PGUSER' and 'PGSSLKEY'. See the [PostgreSQL Environment Variables](https://www.postgresql.org/docs/current/libpq-envars.html)  documentation for a complete list of variables.
 
-Additionally, you can specify the target database version with the special value of 'DBVERSION'. If the `connect_settings` hash is omitted or empty, then Puppet connects to the local PostgreSQL instance.
+Additionally, you can specify the target database version with the special value of 'DBVERSION'. If the `$connect_settings` hash is omitted or empty, then Puppet connects to the local PostgreSQL instance.
+
+**The $connect_settings hash has priority over the explicit variables like $port and $user**
+
+When a user provides only the `$port` parameter to a resource and no `$connect_settings`, `$port` will be used. When `$connect_settings` contains `PGPORT` and `$port` is set, `$connect_settings['PGPORT']` will be used.
 
 You can provide a `connect_settings` hash for each of the Puppet resources, or you can set a default `connect_settings` hash in `postgresql::globals`. Configuring `connect_settings` per resource allows SQL objects to be created on multiple databases by multiple users.
 

REFERENCE.md

@@ -2240,11 +2240,11 @@ Default value: `$postgresql::server::user`
 
 ##### <a name="-postgresql--server--grant--port"></a>`port`
 
-Data type: `Optional[Stdlib::Port]`
+Data type: `Stdlib::Port`
 
 Port to use when connecting.
 
-Default value: `undef`
+Default value: `$postgresql::server::port`
 
 ##### <a name="-postgresql--server--grant--onlyif_exists"></a>`onlyif_exists`
 

manifests/server/grant.pp

@@ -41,7 +41,7 @@
   Array[String[1],0]                             $object_arguments  = [],
   String                                         $psql_db           = $postgresql::server::default_database,
   String                                         $psql_user         = $postgresql::server::user,
-  Optional[Stdlib::Port] $port = undef,
+  Stdlib::Port $port = $postgresql::server::port,
   Boolean                                        $onlyif_exists     = false,
   Hash                                           $connect_settings  = $postgresql::server::default_connect_settings,
   Enum['present', 'absent']                      $ensure            = 'present',
@@ -75,14 +75,11 @@
   }
 
   #
-  # Port, order of precedence: $port parameter, $connect_settings[PGPORT], $postgresql::server::port
   # We don't use pick() here because that would introduce a hard dependency to the postgresql::server class
-  if $port {
-    $port_override = $port
-  } elsif 'PGPORT' in $connect_settings {
-    $port_override = undef
+  if $connect_settings['PGPORT'] {
+    $port_override = $connect_settings['PGPORT']
   } else {
-    $port_override = $postgresql::server::port
+    $port_override = $port
   }
 
   ## Munge the input values