Properties, defaults, PR 900, etc

Hi,

I'm reviewing PR#900 fix: use loginTimeout in java.sql.DriverManager by
default (see https://github.com/pgjdbc/pgjdbc/pull/900 ), and I think it
requires to be discussed in more details.

The property dance has already been performed multiple times (
https://github.com/pgjdbc/pgjdbc/issues/259,
https://github.com/pgjdbc/pgjdbc/pull/260,
https://github.com/pgjdbc/pgjdbc/pull/438 )

As far as I see, 259 and 438 did teach a isPresent method to *ignore*
"default storage" (e.g. System.properties) while checking for property
presence.

Frankly speaking, I think if a default storage has some value for property
X, then it should be indistinguishable from the case when the property was
provided via connection URL.

Jeremy Whiting, Alexis Meneses, I would appreciate if you agree or disagree
with "there is a case for isPresent to ignore default properties storage".

Here's the change in PR#900 that makes me shiver:

public static Properties parseURL(String url, Properties defaults) {
+ Properties urlProps = new Properties();
+ if (defaults != null) {
+ urlProps.putAll(defaults);

There are two gems in this change:
g1) It mixes existing "new Properties(defaults)" with "copy everything into
final properties" approaches. It would make the code harder to maintain
g2) urlProps.putAll(defaults) would *ignore* the "parent properties" of a
"defaults" object. It happens so that "Properties" does not override
"entrySet"/"putAll", so this putAll would not put all the values, but it
would ignore the ones defined in the "parent" of "defaults".

Just in case, I see the following property sources
"production case":
1) "user" comes from System.properties
2) "org/postgresql/driverconfig.properties" files on the
"pgjdbc.classloader"
3) Properties provided in the "DriverManager.getConnection(...)" call
4) URL parameters
5) fallback values
Note: System.properties is not considered by pgjdbc except "user"

"test mode":
-2) build.properties file
-1) System.properties
... the rest is as in production

Frankly speaking, I would love to make property handling aligned, so we
either always do "new Properties(defaultProps)" kind of thing (do not copy
values, and use proper Properties.getString API to access defaults) or we
do "copy all the values to one final big properties object).

I don't like the mix of the two approaches.

Any thoughts?

Vladimir