Feature/urlparser improve3 pr1 (#2641)

* refactoring: PGPropertyPasswordParser, PGPropertyServiceParser moved to package org.postgresql.jdbcurlresolver

* fix: loglevel -> loggerLevel (there is no property loglevel)

* remove usage of environment variables (it eliminates need for cleanup)

* replace hard-coded property values with references to PGProperty enum

* rename: PGProperty.get() -> getOrDefault() because it returns value and fallback is default value. get() is added to return value without fallback (preparation for urlparser)

* remove references to logLevel and loggerLevel

appveyor.yml

@@ -5,8 +5,6 @@ configuration: Release
 clone_depth: 1
 environment:
   JAVA_HOME: 'C:\Program Files\Java\jdk1.8.0'
-  PGUSER: postgres
-  PGPASSWORD: Password12!
   matrix:
   - pg: 9.6.11-1
     PlatformToolset: v120
@@ -62,29 +60,21 @@ before_build:
 - ps: Add-Content -PATH "$env:pgroot\data\pg_hba.conf" "host    replication     all             127.0.0.1/32            trust"
 - net start postgresql%x64%-%pgversion%
 - path %pgroot%\bin;%PATH%
-- SET PGUSER=postgres
-- SET PGPASSWORD=Password12!
+- mkdir %APPDATA%\postgresql
+- echo *:*:*:postgres:Password12!> %APPDATA%\postgresql\pgpass.conf
 - createuser -U postgres test
 - psql -U postgres -c "alter user test with password 'test'" postgres
 - psql -U postgres -c "alter user test with replication" postgres
 - createuser -U postgres testsspi
 - createdb -U postgres -O test test
+- del %APPDATA%\postgresql\pgpass.conf
 
 build_script:
   - gradlew assemble
 
 test_script:
   - echo redirect escape ^> foo.bar
   - echo privilegedPassword=Password12!>c:\projects\pgjdbc\build.local.properties
-  - set PGDATABASE=
-  - set PGHOST=
-  - set PGPASSFILE=
-  - set PGPASSWORD=
-  - set PGPORT=
-  - set PGSERVICE=
-  - set PGSERVICEFILE=
-  - set PGSYSCONFDIR=
-  - set PGUSER=
   - gradlew test
 
 cache:

build.properties

@@ -1,6 +1,5 @@
 # Default build parameters.  These may be overridden by local configuration
 # settings in build.local.properties.
-# loggerLevel can be OFF, DEBUG, TRACE
 #
 
 server=localhost
@@ -16,7 +15,5 @@ privilegedUser=postgres
 privilegedPassword=
 sspiusername=testsspi
 preparethreshold=5
-loggerLevel=OFF
-loggerFile=target/pgjdbc-tests.log
 protocolVersion=0
 sslpassword=sslpwd

packaging/rpm/postgresql-jdbc.spec.tpl

@@ -111,9 +111,9 @@ find -type f \( -name "*.jar" -or -name "*.class" \) | xargs rm -f
 # Remove the test files depending on system-stubs-jupiter
 rm src/test/java/org/postgresql/test/jdbc2/DriverTest.java \
    src/test/java/org/postgresql/util/OSUtilTest.java \
-   src/test/java/org/postgresql/util/PGPropertyPasswordParserTest.java \
    src/test/java/org/postgresql/util/StubEnvironmentAndProperties.java \
-   src/test/java/org/postgresql/util/PGPropertyServiceParserTest.java
+   src/test/java/org/postgresql/jdbcurlresolver/PgPassParserTest.java \
+   src/test/java/org/postgresql/jdbcurlresolver/PgServiceConfParserTest.java
 
 # compat symlink: requested by dtardon (libreoffice), reverts part of
 # 0af97ce32de877 commit.
@@ -145,7 +145,6 @@ password=test
 privilegedUser=$PGTESTS_ADMIN
 privilegedPassword=$PGTESTS_ADMINPASS
 preparethreshold=5
-loglevel=0
 protocolVersion=0
 EOF
 

pgjdbc-osgi-test/src/test/java/org/postgresql/test/osgi/DataSourceFactoryTest.java

@@ -120,7 +120,6 @@ private String getUrl() {
         + p.get("server") + ":"
         + p.get("port") + "/"
         + p.get("database")
-        + "?loglevel=" + p.get("loglevel")
         ;
   }
 

pgjdbc/src/main/java/org/postgresql/Driver.java

@@ -8,11 +8,11 @@
 import static org.postgresql.util.internal.Nullness.castNonNull;
 
 import org.postgresql.jdbc.PgConnection;
+import org.postgresql.jdbcurlresolver.PgPassParser;
+import org.postgresql.jdbcurlresolver.PgServiceConfParser;
 import org.postgresql.util.DriverInfo;
 import org.postgresql.util.GT;
 import org.postgresql.util.HostSpec;
-import org.postgresql.util.PGPropertyPasswordParser;
-import org.postgresql.util.PGPropertyServiceParser;
 import org.postgresql.util.PGPropertyUtil;
 import org.postgresql.util.PSQLException;
 import org.postgresql.util.PSQLState;
@@ -629,7 +629,7 @@ public boolean jdbcCompliant() {
     // load pg_service.conf
     if (serviceName != null) {
       LOGGER.log(Level.FINE, "Processing option [?service={0}]", serviceName);
-      Properties result = PGPropertyServiceParser.getServiceProperties(serviceName);
+      Properties result = PgServiceConfParser.getServiceProperties(serviceName);
       if (result == null) {
         LOGGER.log(Level.WARNING, "Definition of service [{0}] not found", serviceName);
         return null;
@@ -652,8 +652,8 @@ public boolean jdbcCompliant() {
     // priority 5 - PGProperty defaults for PGHOST, PGPORT, PGDBNAME
     result.putIfAbsent(PGProperty.PG_PORT.getName(), castNonNull(PGProperty.PG_PORT.getDefaultValue()));
     result.putIfAbsent(PGProperty.PG_HOST.getName(), castNonNull(PGProperty.PG_HOST.getDefaultValue()));
-    if (PGProperty.USER.get(result) != null) {
-      result.putIfAbsent(PGProperty.PG_DBNAME.getName(), castNonNull(PGProperty.USER.get(result)));
+    if (PGProperty.USER.getOrDefault(result) != null) {
+      result.putIfAbsent(PGProperty.PG_DBNAME.getName(), castNonNull(PGProperty.USER.getOrDefault(result)));
     }
 
     // consistency check
@@ -662,9 +662,9 @@ public boolean jdbcCompliant() {
     }
 
     // try to load .pgpass if password is missing
-    if (PGProperty.PASSWORD.get(result) == null) {
-      String password = PGPropertyPasswordParser.getPassword(
-          PGProperty.PG_HOST.get(result), PGProperty.PG_PORT.get(result), PGProperty.PG_DBNAME.get(result), PGProperty.USER.get(result)
+    if (PGProperty.PASSWORD.getOrDefault(result) == null) {
+      String password = PgPassParser.getPassword(
+          PGProperty.PG_HOST.getOrDefault(result), PGProperty.PG_PORT.getOrDefault(result), PGProperty.PG_DBNAME.getOrDefault(result), PGProperty.USER.getOrDefault(result)
       );
       if (password != null && !password.isEmpty()) {
         PGProperty.PASSWORD.set(result, password);
@@ -688,9 +688,9 @@ public boolean jdbcCompliant() {
    * @return the address portion of the URL
    */
   private static HostSpec[] hostSpecs(Properties props) {
-    String[] hosts = castNonNull(PGProperty.PG_HOST.get(props)).split(",");
-    String[] ports = castNonNull(PGProperty.PG_PORT.get(props)).split(",");
-    String localSocketAddress = PGProperty.LOCAL_SOCKET_ADDRESS.get(props);
+    String[] hosts = castNonNull(PGProperty.PG_HOST.getOrDefault(props)).split(",");
+    String[] ports = castNonNull(PGProperty.PG_PORT.getOrDefault(props)).split(",");
+    String localSocketAddress = PGProperty.LOCAL_SOCKET_ADDRESS.getOrDefault(props);
     HostSpec[] hostSpecs = new HostSpec[hosts.length];
     for (int i = 0; i < hostSpecs.length; ++i) {
       hostSpecs[i] = new HostSpec(hosts[i], Integer.parseInt(ports[i]), localSocketAddress);
@@ -702,7 +702,7 @@ private static HostSpec[] hostSpecs(Properties props) {
    * @return the timeout from the URL, in milliseconds
    */
   private static long timeout(Properties props) {
-    String timeout = PGProperty.LOGIN_TIMEOUT.get(props);
+    String timeout = PGProperty.LOGIN_TIMEOUT.getOrDefault(props);
     if (timeout != null) {
       try {
         return (long) (Float.parseFloat(timeout) * 1000);

pgjdbc/src/main/java/org/postgresql/PGProperty.java

@@ -317,17 +317,19 @@ public enum PGProperty {
 
   /**
    * This property is no longer used by the driver and will be ignored.
-   * Logging is configured via java.util.logging.
+   * @deprecated Logging is configured via java.util.logging.
    */
+  @Deprecated
   LOGGER_FILE(
       "loggerFile",
       null,
       "File name output of the Logger"),
 
   /**
    * This property is no longer used by the driver and will be ignored.
-   * Logging is configured via java.util.logging.
+   * @deprecated Logging is configured via java.util.logging.
    */
+  @Deprecated
   LOGGER_LEVEL(
       "loggerLevel",
       null,
@@ -855,10 +857,20 @@ public String getDescription() {
    * @param properties properties to take actual value from
    * @return evaluated value for this connection parameter
    */
-  public @Nullable String get(Properties properties) {
+  public @Nullable String getOrDefault(Properties properties) {
     return properties.getProperty(name, defaultValue);
   }
 
+  /**
+   * Returns the value of the connection parameters according to the given {@code Properties}
+   *
+   * @param properties properties to take actual value from
+   * @return evaluated value for this connection parameter or null
+   */
+  public @Nullable String get(Properties properties) {
+    return properties.getProperty(name);
+  }
+
   /**
    * Set the value for this connection parameter in the given {@code Properties}.
    *
@@ -880,7 +892,7 @@ public void set(Properties properties, @Nullable String value) {
    * @return evaluated value for this connection parameter converted to boolean
    */
   public boolean getBoolean(Properties properties) {
-    return Boolean.parseBoolean(get(properties));
+    return Boolean.parseBoolean(getOrDefault(properties));
   }
 
   /**
@@ -893,7 +905,7 @@ public boolean getBoolean(Properties properties) {
    */
   @SuppressWarnings("nullness:argument.type.incompatible")
   public int getIntNoCheck(Properties properties) {
-    String value = get(properties);
+    String value = getOrDefault(properties);
     //noinspection ConstantConditions
     return Integer.parseInt(value);
   }
@@ -907,7 +919,7 @@ public int getIntNoCheck(Properties properties) {
    */
   @SuppressWarnings("nullness:argument.type.incompatible")
   public int getInt(Properties properties) throws PSQLException {
-    String value = get(properties);
+    String value = getOrDefault(properties);
     try {
       //noinspection ConstantConditions
       return Integer.parseInt(value);
@@ -925,7 +937,7 @@ public int getInt(Properties properties) throws PSQLException {
    * @throws PSQLException if unable to parse property as integer
    */
   public @Nullable Integer getInteger(Properties properties) throws PSQLException {
-    String value = get(properties);
+    String value = getOrDefault(properties);
     if (value == null) {
       return null;
     }
@@ -975,7 +987,7 @@ public boolean isPresent(Properties properties) {
    * @return a DriverPropertyInfo representing this connection parameter
    */
   public DriverPropertyInfo toDriverPropertyInfo(Properties properties) {
-    DriverPropertyInfo propertyInfo = new DriverPropertyInfo(name, get(properties));
+    DriverPropertyInfo propertyInfo = new DriverPropertyInfo(name, getOrDefault(properties));
     propertyInfo.required = required;
     propertyInfo.description = description;
     propertyInfo.choices = choices;

pgjdbc/src/main/java/org/postgresql/core/ConnectionFactory.java

@@ -42,7 +42,7 @@ public abstract class ConnectionFactory {
    */
   public static QueryExecutor openConnection(HostSpec[] hostSpecs,
       Properties info) throws SQLException {
-    String protoName = PGProperty.PROTOCOL_VERSION.get(info);
+    String protoName = PGProperty.PROTOCOL_VERSION.getOrDefault(info);
 
     if (protoName == null || protoName.isEmpty() || "3".equals(protoName)) {
       ConnectionFactory connectionFactory = new ConnectionFactoryImpl();

pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java

@@ -69,17 +69,17 @@ public abstract class QueryExecutorBase implements QueryExecutor {
   @SuppressWarnings({"assignment.type.incompatible", "argument.type.incompatible"})
   protected QueryExecutorBase(PGStream pgStream, int cancelSignalTimeout, Properties info) throws SQLException {
     this.pgStream = pgStream;
-    this.user = PGProperty.USER.get(info);
-    this.database = PGProperty.PG_DBNAME.get(info);
+    this.user = PGProperty.USER.getOrDefault(info);
+    this.database = PGProperty.PG_DBNAME.getOrDefault(info);
     this.cancelSignalTimeout = cancelSignalTimeout;
     this.reWriteBatchedInserts = PGProperty.REWRITE_BATCHED_INSERTS.getBoolean(info);
     this.columnSanitiserDisabled = PGProperty.DISABLE_COLUMN_SANITISER.getBoolean(info);
-    String callMode = PGProperty.ESCAPE_SYNTAX_CALL_MODE.get(info);
+    String callMode = PGProperty.ESCAPE_SYNTAX_CALL_MODE.getOrDefault(info);
     this.escapeSyntaxCallMode = EscapeSyntaxCallMode.of(callMode);
     this.quoteReturningIdentifiers = PGProperty.QUOTE_RETURNING_IDENTIFIERS.getBoolean(info);
-    String preferMode = PGProperty.PREFER_QUERY_MODE.get(info);
+    String preferMode = PGProperty.PREFER_QUERY_MODE.getOrDefault(info);
     this.preferQueryMode = PreferQueryMode.of(preferMode);
-    this.autoSave = AutoSave.of(PGProperty.AUTOSAVE.get(info));
+    this.autoSave = AutoSave.of(PGProperty.AUTOSAVE.getOrDefault(info));
     this.logServerErrorDetail = PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info);
     // assignment.type.incompatible, argument.type.incompatible
     this.cachedQueryCreateAction = new CachedQueryCreateAction(this);

pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java

@@ -31,13 +31,13 @@ public class SocketFactoryFactory {
    */
   public static SocketFactory getSocketFactory(Properties info) throws PSQLException {
     // Socket factory
-    String socketFactoryClassName = PGProperty.SOCKET_FACTORY.get(info);
+    String socketFactoryClassName = PGProperty.SOCKET_FACTORY.getOrDefault(info);
     if (socketFactoryClassName == null) {
       return SocketFactory.getDefault();
     }
     try {
       return ObjectFactory.instantiate(SocketFactory.class, socketFactoryClassName, info, true,
-          PGProperty.SOCKET_FACTORY_ARG.get(info));
+          PGProperty.SOCKET_FACTORY_ARG.getOrDefault(info));
     } catch (Exception e) {
       throw new PSQLException(
           GT.tr("The SocketFactory class provided {0} could not be instantiated.",
@@ -54,15 +54,15 @@ public static SocketFactory getSocketFactory(Properties info) throws PSQLExcepti
    * @throws PSQLException if something goes wrong
    */
   public static SSLSocketFactory getSslSocketFactory(Properties info) throws PSQLException {
-    String classname = PGProperty.SSL_FACTORY.get(info);
+    String classname = PGProperty.SSL_FACTORY.getOrDefault(info);
     if (classname == null
         || "org.postgresql.ssl.jdbc4.LibPQFactory".equals(classname)
         || "org.postgresql.ssl.LibPQFactory".equals(classname)) {
       return new LibPQFactory(info);
     }
     try {
       return ObjectFactory.instantiate(SSLSocketFactory.class, classname, info, true,
-          PGProperty.SSL_FACTORY_ARG.get(info));
+          PGProperty.SSL_FACTORY_ARG.getOrDefault(info));
     } catch (Exception e) {
       throw new PSQLException(
           GT.tr("The SSLSocketFactory class provided {0} could not be instantiated.", classname),

pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java

@@ -55,11 +55,11 @@ public static <T> T withPassword(AuthenticationRequestType type, Properties info
       PasswordAction<char @Nullable [], T> action) throws PSQLException, IOException {
     char[] password = null;
 
-    String authPluginClassName = PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info);
+    String authPluginClassName = PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.getOrDefault(info);
 
     if (authPluginClassName == null || authPluginClassName.equals("")) {
       // Default auth plugin simply pulls password directly from connection properties
-      String passwordText = PGProperty.PASSWORD.get(info);
+      String passwordText = PGProperty.PASSWORD.getOrDefault(info);
       if (passwordText != null) {
         password = passwordText.toCharArray();
       }
@@ -108,7 +108,7 @@ public static <T> T withEncodedPassword(AuthenticationRequestType type, Properti
       if (password == null) {
         throw new PSQLException(
             GT.tr("The server requested password-based authentication, but no password was provided by plugin {0}",
-                PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info)),
+                PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.getOrDefault(info)),
             PSQLState.CONNECTION_REJECTED);
       }
       ByteBuffer buf = StandardCharsets.UTF_8.encode(CharBuffer.wrap(password));