JDBC-197 Fix broken tests + add TODOs regarding resolving to NONE if …

…nothing is specified
FirebirdSQL · Aug 5, 2013 · 5f14dd5 · 5f14dd5
1 parent dcf192c
commit 5f14dd5
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 9 deletions.
diff --git a/src/main/org/firebirdsql/encodings/ConnectionEncodingFactory.java b/src/main/org/firebirdsql/encodings/ConnectionEncodingFactory.java
@@ -104,6 +104,12 @@ public CharacterTranslator getCharacterTranslator(final String mappingPath) thro
 
     @Override
     public EncodingDefinition getEncodingDefinition(final String firebirdEncodingName, final String javaCharsetAlias) {
+        // TODO Consider returning getDefaultEncodingDefinition() if null
         return factory.getEncodingDefinition(firebirdEncodingName, javaCharsetAlias);
     }
+
+    @Override
+    public IEncodingFactory withDefaultEncodingDefinition(EncodingDefinition encodingDefinition) {
+        return factory.withDefaultEncodingDefinition(encodingDefinition != null ? encodingDefinition : getDefaultEncodingDefinition());
+    }
 }
diff --git a/src/main/org/firebirdsql/encodings/EncodingFactory.java b/src/main/org/firebirdsql/encodings/EncodingFactory.java
@@ -242,6 +242,7 @@ public EncodingDefinition getEncodingDefinition(final String firebirdEncodingNam
         try {
             EncodingDefinition encodingDefinition = null;
             Charset charset = null;
+            // TODO Consider returning getDefaultEncodingDefinition() if both firebirdEncodingName and javaCharsetAlias are NULL
             if (firebirdEncodingName != null) {
                 encodingDefinition = getEncodingDefinitionByFirebirdName(firebirdEncodingName);
                 if (javaCharsetAlias != null) {
@@ -257,32 +258,33 @@ public EncodingDefinition getEncodingDefinition(final String firebirdEncodingNam
             }
 
             if (encodingDefinition == null) {
+                // TODO Consider throwing exception if no EncodingDefinition is found
                 return null;
             }
             if (!encodingDefinition.isInformationOnly() && (charset == null || encodingDefinition.getJavaCharset().equals(charset))) {
+                // Normal encoding definition
                 return encodingDefinition;
             }
             if (charset != null) {
+                /* Construct non-standard combination of Firebird encoding + Java character set
+                 * This allows for special purpose combinations like Firebird ISO8859_3 with Java ISO-8859-1
+                 * But is mostly intended for using Firebird NONE with a specific java character set
+                 */
                 return new DefaultEncodingDefinition(encodingDefinition.getFirebirdEncodingName(), charset, encodingDefinition.getMaxBytesPerChar(),
                         encodingDefinition.getFirebirdCharacterSetId(), false);
             }
-            if (firebirdEncodingName != null && firebirdEncodingName.equalsIgnoreCase("NONE")) {
+            if ("NONE".equalsIgnoreCase(firebirdEncodingName)) {
                 return getDefaultEncodingDefinition();
             }
+            // TODO Consider throwing exception if no EncodingDefinition is found
             return null;
         } catch (Exception e) {
             log.debug(String.format("Exception looking up encoding definition for firebirdEncodingName %s, javaCharsetAlias %s", firebirdEncodingName, javaCharsetAlias), e);
             return null;
         }
     }
 
-    /**
-     * Returns an {@link IEncodingFactory} that uses <code>encodingDefinition</code> as the default.
-     *
-     * @param encodingDefinition
-     *         The default encoding to use (or <code>null</code> for the value of {@link #getDefaultEncoding()}
-     * @return IEncoding instance with the specified default.
-     */
+    @Override
     public IEncodingFactory withDefaultEncodingDefinition(EncodingDefinition encodingDefinition) {
         return new ConnectionEncodingFactory(this, encodingDefinition != null ? encodingDefinition : getDefaultEncodingDefinition());
     }

diff --git a/src/main/org/firebirdsql/encodings/IEncodingFactory.java b/src/main/org/firebirdsql/encodings/IEncodingFactory.java
@@ -183,4 +183,13 @@ public interface IEncodingFactory {
      * @return An EncodingDefinition or null if both parameters are null, no encoding was found or if an exception occurred.
      */
     EncodingDefinition getEncodingDefinition(String firebirdEncodingName, String javaCharsetAlias);
+
+    /**
+     * Returns an {@link org.firebirdsql.encodings.IEncodingFactory} that uses <code>encodingDefinition</code> as the default.
+     *
+     * @param encodingDefinition
+     *         The default encoding to use (or <code>null</code> for the value of {@link #getDefaultEncoding()}
+     * @return IEncoding instance with the specified default.
+     */
+    IEncodingFactory withDefaultEncodingDefinition(EncodingDefinition encodingDefinition);
 }
diff --git a/src/main/org/firebirdsql/gds/ng/wire/WireConnection.java b/src/main/org/firebirdsql/gds/ng/wire/WireConnection.java
@@ -110,12 +110,13 @@ public WireConnection(IConnectionProperties connectionProperties, IEncodingFacto
     public WireConnection(IConnectionProperties connectionProperties, IEncodingFactory encodingFactory, ProtocolCollection protocols) throws SQLException {
         this.connectionProperties = new FbConnectionProperties(connectionProperties);
         this.protocols = protocols;
-        this.encodingFactory = encodingFactory;
         encodingDefinition = encodingFactory.getEncodingDefinition(connectionProperties.getEncoding(), connectionProperties.getCharSet());
         if (encodingDefinition == null || encodingDefinition.isInformationOnly()) {
+            // TODO Don't throw exception if encoding/charSet is null (see also TODO inside EncodingFactory.getEncodingDefinition)
             throw new SQLNonTransientConnectionException(String.format("No valid encoding definition for Firebird encoding %s and/or Java charset %s",
                     connectionProperties.getEncoding(), connectionProperties.getCharSet()), FBSQLException.SQL_STATE_CONNECTION_ERROR);
         }
+        this.encodingFactory = encodingFactory.withDefaultEncodingDefinition(encodingDefinition);
     }
 
     public boolean isConnected() {

diff --git a/src/test/org/firebirdsql/gds/ng/TestFbConnectionProperties.java b/src/test/org/firebirdsql/gds/ng/TestFbConnectionProperties.java
@@ -188,6 +188,9 @@ public void testAsImmutable() throws Exception {
             } else if (parameterType == String.class) {
                 method.invoke(info, method.getName());
                 testValues.put(descriptor.getName(), method.getName());
+            } else if (parameterType == boolean.class) {
+                method.invoke(info, true);
+                testValues.put(descriptor.getName(), true);
             } else {
                 throw new IllegalStateException("Unexpected setter type: " + parameterType);
             }

diff --git a/src/test/org/firebirdsql/gds/ng/wire/TestWireConnection.java b/src/test/org/firebirdsql/gds/ng/wire/TestWireConnection.java
@@ -58,6 +58,8 @@ public class TestWireConnection extends FBJUnit4TestBase {
         connectionInfo.setServerName(FBTestProperties.DB_SERVER_URL);
         connectionInfo.setPortNumber(FBTestProperties.DB_SERVER_PORT);
         connectionInfo.setDatabaseName(FBTestProperties.getDatabasePath());
+        // TODO consider keeping NONE the default in WireConnection if not specified
+        connectionInfo.setEncoding("NONE");
     }
 
     @BeforeClass