Repository: pgjdbc/pgjdbc
Pull request #765 (2017-02-27 20:17:42)

refactor: simplify Encoding class
Little refactor to simplify the Encoding class.

codecov-io: # [Codecov](https://codecov.io/gh/pgjdbc/pgjdbc/pull/765?src=pr&el=h1) Report
> Merging [#765](https://codecov.io/gh/pgjdbc/pgjdbc/pull/765?src=pr&el=desc) into [master](https://codecov.io/gh/pgjdbc/pgjdbc/commit/bd0497dee741e92de1cc3eeabb58a82608ed6070?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `79.03%`.



```diff
@@             Coverage Diff              @@
##             master     #765      +/-   ##
============================================
+ Coverage     65.24%   65.25%   +<.01%     
  Complexity     3495     3495              
============================================
  Files           164      164              
  Lines         15120    15121       +1     
  Branches       2450     2451       +1     
============================================
+ Hits           9865     9867       +2     
  Misses         4077     4077              
+ Partials       1178     1177       -1
```

------

[Continue to review full report at Codecov](https://codecov.io/gh/pgjdbc/pgjdbc/pull/765?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pgjdbc/pgjdbc/pull/765?src=pr&el=footer). Last update [bd0497d...50ac0a1](https://codecov.io/gh/pgjdbc/pgjdbc/compare/bd0497dee741e92de1cc3eeabb58a82608ed6070...50ac0a10280f2d20b3692c4ecaf8bae2d334312c?src=pr&el=footer&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
davecramer: +1

Patch:
diff --git a/docs/documentation/head/callproc.md b/docs/documentation/head/callproc.md
index c2d91c59..bf5e14a5 100644
--- a/docs/documentation/head/callproc.md
+++ b/docs/documentation/head/callproc.md
@@ -22,7 +22,7 @@ This example shows how to call a PostgreSQL™ built in function, `upper`, which
 simply converts the supplied string argument to uppercase.
 
 ```java
-CallableStatement upperProc = conn.prepareCall("{ ? = call upper( ? ) }");
+CallableStatement upperProc = conn.prepareCall("{? = call upper( ? ) }");
 upperProc.registerOutParameter(1, Types.VARCHAR);
 upperProc.setString(2, "lowercase to uppercase");
 upperProc.execute();
@@ -95,7 +95,7 @@ stmt.close();
 conn.setAutoCommit(false);
 
 // Procedure call.
-CallableStatement proc = conn.prepareCall("{ ? = call refcursorfunc() }");
+CallableStatement proc = conn.prepareCall("{? = call refcursorfunc() }");
 proc.registerOutParameter(1, Types.OTHER);
 proc.execute();
 ResultSet results = (ResultSet) proc.getObject(1);
@@ -116,7 +116,7 @@ you are free to directly use cursor commands on it, such as `FETCH` and `MOVE`.
 
 ```java
 conn.setAutoCommit(false);
-CallableStatement proc = conn.prepareCall("{ ? = call refcursorfunc() }");
+CallableStatement proc = conn.prepareCall("{? = call refcursorfunc() }");
 proc.registerOutParameter(1, Types.OTHER);
 proc.execute();
 String cursorName = proc.getString(1);
diff --git a/pgjdbc/pom.xml b/pgjdbc/pom.xml
index 0d2ba622..f78fa4f6 100644
--- a/pgjdbc/pom.xml
+++ b/pgjdbc/pom.xml
@@ -266,6 +266,7 @@
           </dependencies>
           <configuration>
             <configLocation>src/main/checkstyle/checks.xml</configLocation>
+	    <suppressionsLocation>src/main/checkstyle/suppressions.xml</suppressionsLocation>
             <violationSeverity>error</violationSeverity>
             <failOnViolation>true</failOnViolation>
             <failsOnError>true</failsOnError>
diff --git a/pgjdbc/src/main/checkstyle/suppressions.xml b/pgjdbc/src/main/checkstyle/suppressions.xml
new file mode 100644
index 00000000..76e50708
--- /dev/null
+++ b/pgjdbc/src/main/checkstyle/suppressions.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE suppressions PUBLIC
+        "-//Puppy Crawl//DTD Suppressions 1.1//EN"
+        "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
+
+<suppressions>
+    <suppress files="WriterHandler\.java" checks="RegexpHeader"/>
+</suppressions>
diff --git a/pgjdbc/src/main/java/org/postgresql/Driver.java b/pgjdbc/src/main/java/org/postgresql/Driver.java
index d471f4d9..27c6b486 100644
--- a/pgjdbc/src/main/java/org/postgresql/Driver.java
+++ b/pgjdbc/src/main/java/org/postgresql/Driver.java
@@ -6,12 +6,14 @@
 package org.postgresql;
 
 import org.postgresql.jdbc.PgConnection;
+
 import org.postgresql.util.ExpressionProperties;
 import org.postgresql.util.GT;
 import org.postgresql.util.HostSpec;
 import org.postgresql.util.PSQLException;
 import org.postgresql.util.PSQLState;
 import org.postgresql.util.SharedTimer;
+import org.postgresql.util.WriterHandler;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -29,8 +31,11 @@ import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.Properties;
 import java.util.Set;
+import java.util.logging.Formatter;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.logging.SimpleFormatter;
+import java.util.logging.StreamHandler;
 
 /**
  * The Java SQL framework allows for multiple database drivers. Each driver should supply a class
@@ -308,7 +313,7 @@ public class Driver implements java.sql.Driver {
       loggerHandlerFile = null;
     }
 
-    java.util.logging.Handler handler = new java.util.logging.ConsoleHandler();
+    java.util.logging.Handler handler = null;
     if (driverLogFile != null) {
       try {
         handler = new java.util.logging.FileHandler(driverLogFile);
@@ -318,7 +323,21 @@ public class Driver implements java.sql.Driver {
       }
     }
 
-    handler.setFormatter(new java.util.logging.SimpleFormatter());
+    Formatter formatter = new SimpleFormatter();
+
+    if ( handler == null ) {
+      if (DriverManager.getLogWriter() != null) {
+        handler = new WriterHandler(DriverManager.getLogWriter());
+      } else if ( DriverManager.getLogStream() != null) {
+        handler = new StreamHandler(DriverManager.getLogStream(), formatter);
+      } else {
+        handler = new StreamHandler(System.err, formatter);
+      }
+    } else {
+      handler.setFormatter(formatter);
+    }
+
+    handler.setLevel(PARENT_LOGGER.getLevel());
     PARENT_LOGGER.setUseParentHandlers(false);
     PARENT_LOGGER.addHandler(handler);
   }
diff --git a/pgjdbc/src/main/java/org/postgresql/core/Encoding.java b/pgjdbc/src/main/java/org/postgresql/core/Encoding.java
index 557470f2..51fc774a 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/Encoding.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/Encoding.java
@@ -12,13 +12,20 @@ import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Reader;
 import java.io.Writer;
+import java.nio.charset.Charset;
 import java.util.HashMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Representation of a particular character encoding.
  */
 public class Encoding {
-  private static final Encoding DEFAULT_ENCODING = new Encoding(null);
+
+  private static final Logger LOGGER = Logger.getLogger(Encoding.class.getName());
+
+  private static final Encoding DEFAULT_ENCODING = new Encoding();
+  private static final Encoding UTF8_ENCODING = new Encoding("UTF-8");
 
   /*
    * Preferred JVM encodings for backend encodings.
@@ -28,10 +35,9 @@ public class Encoding {
   static {
     //Note: this list should match the set of supported server
     // encodings found in backend/util/mb/encnames.c
-    encodings.put("SQL_ASCII", new String[]{"ASCII", "us-ascii"});
+    encodings.put("SQL_ASCII", new String[]{"ASCII", "US-ASCII"});
     encodings.put("UNICODE", new String[]{"UTF-8", "UTF8"});
-    encodings.put("UTF8",
-        new String[]{"UTF-8", "UTF8"}); // 8.1's canonical name for UNICODE changed.
+    encodings.put("UTF8", new String[]{"UTF-8", "UTF8"});
     encodings.put("LATIN1", new String[]{"ISO8859_1"});
     encodings.put("LATIN2", new String[]{"ISO8859_2"});
     encodings.put("LATIN3", new String[]{"ISO8859_3"});
@@ -73,9 +79,28 @@ public class Encoding {
   private final String encoding;
   private final boolean fastASCIINumbers;
 
+  /**
+   * Uses the default charset of the JVM.
+   */
+  private Encoding() {
+    this(Charset.defaultCharset().name());
+  }
+
+  /**
+   * Use the charset passed as parameter.
+   *
+   * @param encoding charset name to use
+   */
   protected Encoding(String encoding) {
+    if (encoding == null) {
+      throw new NullPointerException("Null encoding charset not supported");
+    }
     this.encoding = encoding;
     fastASCIINumbers = testAsciiNumbers();
+    if (LOGGER.isLoggable(Level.FINEST)) {
+      LOGGER.log(Level.FINEST, "Creating new Encoding {0} with fastASCIINumbers {1}",
+          new Object[]{encoding, fastASCIINumbers});
+    }
   }
 
   /**
@@ -96,14 +121,13 @@ public class Encoding {
    * default JVM encoding if the specified encoding is unavailable.
    */
   public static Encoding getJVMEncoding(String jvmEncoding) {
-    if (isAvailable(jvmEncoding)) {
-      if (jvmEncoding.equals("UTF-8") || jvmEncoding.equals("UTF8")) {
-        return new UTF8Encoding(jvmEncoding);
-      } else {
-        return new Encoding(jvmEncoding);
-      }
+    if ("UTF-8".equals(jvmEncoding)) {
+      return new UTF8Encoding(jvmEncoding);
+    }
+    if (Charset.isSupported(jvmEncoding)) {
+      return new Encoding(jvmEncoding);
     } else {
-      return defaultEncoding();
+      return DEFAULT_ENCODING;
     }
   }
 
@@ -115,14 +139,17 @@ public class Encoding {
    * default JVM encoding if the specified encoding is unavailable.
    */
   public static Encoding getDatabaseEncoding(String databaseEncoding) {
+    if ("UTF8".equals(databaseEncoding)) {
+      return UTF8_ENCODING;
+    }
     // If the backend encoding is known and there is a suitable
     // encoding in the JVM we use that. Otherwise we fall back
     // to the default encoding of the JVM.
-
     String[] candidates = encodings.get(databaseEncoding);
     if (candidates != null) {
       for (String candidate : candidates) {
-        if (isAvailable(candidate)) {
+        LOGGER.log(Level.FINEST, "Search encoding candidate {0}", candidate);
+        if (Charset.isSupported(candidate)) {
           return new Encoding(candidate);
         }
       }
@@ -130,12 +157,13 @@ public class Encoding {
 
     // Try the encoding name directly -- maybe the charset has been
     // provided by the user.
-    if (isAvailable(databaseEncoding)) {
+    if (Charset.isSupported(databaseEncoding)) {
       return new Encoding(databaseEncoding);
     }
 
     // Fall back to default JVM encoding.
-    return defaultEncoding();
+    LOGGER.log(Level.FINEST, "{0} encoding not found, returning default encoding", databaseEncoding);
+    return DEFAULT_ENCODING;
   }
 
   /**
@@ -144,7 +172,7 @@ public class Encoding {
    * @return the JVM encoding name used by this instance.
    */
   public String name() {
-    return encoding;
+    return Charset.isSupported(encoding) ? Charset.forName(encoding).name() : encoding;
   }
 
   /**
@@ -159,10 +187,6 @@ public class Encoding {
       return null;
     }
 
-    if (encoding == null) {
-      return s.getBytes();
-    }
-
     return s.getBytes(encoding);
   }
 
@@ -177,10 +201,6 @@ public class Encoding {
    * @throws IOException if something goes wrong
    */
   public String decode(byte[] encodedString, int offset, int length) throws IOException {
-    if (encoding == null) {
-      return new String(encodedString, offset, length);
-    }
-
     return new String(encodedString, offset, length, encoding);
   }
 
@@ -203,10 +223,6 @@ public class Encoding {
    * @throws IOException if something goes wrong
    */
   public Reader getDecodingReader(InputStream in) throws IOException {
-    if (encoding == null) {
-      return new InputStreamReader(in);
-    }
-
     return new InputStreamReader(in, encoding);
   }
 
@@ -218,10 +234,6 @@ public class Encoding {
    * @throws IOException if something goes wrong
    */
   public Writer getEncodingWriter(OutputStream out) throws IOException {
-    if (encoding == null) {
-      return new OutputStreamWriter(out);
-    }
-
     return new OutputStreamWriter(out, encoding);
   }
 
@@ -234,23 +246,8 @@ public class Encoding {
     return DEFAULT_ENCODING;
   }
 
-  /**
-   * Test if an encoding is available in the JVM.
-   *
-   * @param encodingName the JVM encoding name to test
-   * @return true iff the encoding is supported
-   */
-  private static boolean isAvailable(String encodingName) {
-    try {
-      "DUMMY".getBytes(encodingName); //NOSONAR
-      return true;
-    } catch (java.io.UnsupportedEncodingException e) {
-      return false;
-    }
-  }
-
   public String toString() {
-    return (encoding == null ? "<default JVM encoding>" : encoding);
+    return encoding;
   }
 
   /**
diff --git a/pgjdbc/src/main/java/org/postgresql/util/WriterHandler.java b/pgjdbc/src/main/java/org/postgresql/util/WriterHandler.java
new file mode 100644
index 00000000..822f9536
--- /dev/null
+++ b/pgjdbc/src/main/java/org/postgresql/util/WriterHandler.java
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+
+package org.postgresql.util;
+
+import java.io.Writer;
+
+import java.util.logging.ErrorManager;
+import java.util.logging.Formatter;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import java.util.logging.SimpleFormatter;
+
+public class WriterHandler extends Handler {
+
+  private boolean doneHeader;
+
+  private Writer writer;
+
+  public WriterHandler(Writer inWriter) {
+    super();
+    setLevel(Level.INFO);
+    setFilter(null);
+    setFormatter(new SimpleFormatter());
+    writer = inWriter;
+  }
+
+  public WriterHandler(Writer inWriter, Formatter formatter) {
+    super();
+    setLevel(Level.INFO);
+    setFilter(null);
+    setFormatter(formatter);
+    writer = inWriter;
+
+  }
+
+  public synchronized void publish(final LogRecord record) {
+    if (!isLoggable(record)) {
+      return;
+    }
+    String msg;
+    try {
+      msg = getFormatter().format(record);
+    } catch (Exception ex) {
+      // We don't want to throw an exception here, but we
+      // report the exception to any registered ErrorManager.
+      reportError(null, ex, ErrorManager.FORMAT_FAILURE);
+      return;
+    }
+
+    try {
+      if (!doneHeader) {
+        writer.write(getFormatter().getHead(this));
+        doneHeader = true;
+      }
+      writer.write(msg);
+    } catch (Exception ex) {
+      // We don't want to throw an exception here, but we
+      // report the exception to any registered ErrorManager.
+      reportError(null, ex, ErrorManager.WRITE_FAILURE);
+    }
+  }
+
+  public boolean isLoggable(final LogRecord record) {
+    if (writer == null || record == null) {
+      return false;
+    }
+    return super.isLoggable(record);
+  }
+
+  public synchronized void flush() {
+    if (writer != null) {
+      try {
+        writer.flush();
+      } catch (Exception ex) {
+        // We don't want to throw an exception here, but we
+        // report the exception to any registered ErrorManager.
+        reportError(null, ex, ErrorManager.FLUSH_FAILURE);
+      }
+    }
+  }
+
+  private synchronized void flushAndClose() throws SecurityException {
+
+    if (writer != null) {
+      try {
+        if (!doneHeader) {
+          writer.write(getFormatter().getHead(this));
+          doneHeader = true;
+        }
+        writer.write(getFormatter().getTail(this));
+        writer.flush();
+        writer.close();
+      } catch (Exception ex) {
+        // We don't want to throw an exception here, but we
+        // report the exception to any registered ErrorManager.
+        reportError(null, ex, ErrorManager.CLOSE_FAILURE);
+      }
+      writer = null;
+
+    }
+  }
+
+  public synchronized void close() throws SecurityException {
+    flushAndClose();
+  }
+}


Test patch:
diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
index 9d26beaf..07416d81 100644
--- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
+++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
@@ -13,15 +13,20 @@ import static org.junit.Assert.fail;
 
 import org.postgresql.Driver;
 import org.postgresql.test.TestUtil;
+import org.postgresql.util.NullOutputStream;
+import org.postgresql.util.WriterHandler;
 
 import org.junit.Test;
 
+import java.io.PrintWriter;
 import java.lang.reflect.Method;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Properties;
+import java.util.logging.Handler;
+import java.util.logging.Logger;
 
 /*
  * Tests the dynamically created class org.postgresql.Driver
@@ -180,4 +185,74 @@ public class DriverTest {
     }
     fail("Driver has not been found in DriverManager's list but it should be registered");
   }
+
+  @Test
+  public void testSetLogWriter() throws Exception {
+
+    // this is a dummy to make sure TestUtil is initialized
+    Connection con = DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
+    con.close();
+    String loggerLevel = System.getProperty("loggerLevel");
+    String loggerFile = System.getProperty("loggerFile");
+
+    try {
+
+      PrintWriter printWriter = new PrintWriter(new NullOutputStream(System.err));
+      DriverManager.setLogWriter(printWriter);
+      assertEquals(DriverManager.getLogWriter(), printWriter);
+      System.clearProperty("loggerFile");
+      System.clearProperty("loggerLevel");
+      Properties props = new Properties();
+      props.setProperty("user", TestUtil.getUser());
+      props.setProperty("password", TestUtil.getPassword());
+      props.setProperty("loggerLevel", "DEBUG");
+      con = DriverManager.getConnection(TestUtil.getURL(), props);
+
+      Logger logger = Logger.getLogger("org.postgresql");
+      Handler[] handlers = logger.getHandlers();
+      assertTrue(handlers[0] instanceof WriterHandler );
+      con.close();
+    } finally {
+      DriverManager.setLogWriter(null);
+      System.setProperty("loggerLevel", loggerLevel);
+      System.setProperty("loggerFile", loggerFile);
+
+    }
+
+  }
+
+  @Test
+  public void testSetLogStream() throws Exception {
+
+    // this is a dummy to make sure TestUtil is initialized
+    Connection con = DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
+    con.close();
+    String loggerLevel = System.getProperty("loggerLevel");
+    String loggerFile = System.getProperty("loggerFile");
+
+    try {
+
+      DriverManager.setLogStream(new NullOutputStream(System.err));
+      System.clearProperty("loggerFile");
+      System.clearProperty("loggerLevel");
+      Properties props = new Properties();
+      props.setProperty("user", TestUtil.getUser());
+      props.setProperty("password", TestUtil.getPassword());
+      props.setProperty("loggerLevel", "DEBUG");
+      con = DriverManager.getConnection(TestUtil.getURL(), props);
+
+      Logger logger = Logger.getLogger("org.postgresql");
+      Handler []handlers = logger.getHandlers();
+      assertTrue( handlers[0] instanceof WriterHandler );
+      con.close();
+    } finally {
+      DriverManager.setLogStream(null);
+      System.setProperty("loggerLevel", loggerLevel);
+      System.setProperty("loggerFile", loggerFile);
+
+
+    }
+
+  }
+
 }
diff --git a/pgjdbc/src/test/java/org/postgresql/util/NullOutputStream.java b/pgjdbc/src/test/java/org/postgresql/util/NullOutputStream.java
new file mode 100644
index 00000000..764dc1ac
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/util/NullOutputStream.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2017, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.util;
+
+import java.io.OutputStream;
+import java.io.PrintStream;
+
+/**
+ * Created by davec on 3/14/17.
+ */
+public class NullOutputStream extends PrintStream {
+
+  public NullOutputStream(OutputStream out) {
+    super(out);
+  }
+
+  @Override
+  public void write(int b) {
+
+  }
+
+  @Override
+  public void write(byte[] buf, int off, int len) {
+
+  }
+}