Merge pull request #586 from v-afrafi/JDBC4.3

Update JDBC4.3 branch with latest dev changes

CHANGELOG.md

@@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/) 
 
+## [6.3.6] Preview Release
+### Added
+- Added support for using database name as part of the key for handle cache [#561](https://github.com/Microsoft/mssql-jdbc/pull/561)
+- Updated ADAL4J version to 1.3.0 and also added it into README file [#564](https://github.com/Microsoft/mssql-jdbc/pull/564)
+
+### Fixed Issues 
+- Fixed issues with static loggers being set by every constructor invocation  [#563](https://github.com/Microsoft/mssql-jdbc/pull/563)
+
 ## [6.3.5] Preview Release
 ### Added
 - Added handle for Account Locked Exception 18486 during login in SQLServerConnection [#522](https://github.com/Microsoft/mssql-jdbc/pull/522)

README.md

@@ -90,7 +90,7 @@ To get the latest preview version of the driver, add the following to your POM f
 <dependency>
 	<groupId>com.microsoft.sqlserver</groupId>
 	<artifactId>mssql-jdbc</artifactId>
-	<version>6.3.5.jre8-preview</version>
+	<version>6.3.6.jre8-preview</version>
 </dependency>
 ```
 
@@ -115,12 +115,28 @@ mvn dependency:tree
 ### Azure Key Vault and Azure Active Directory Authentication Dependencies
 Projects that require either of the two features need to explicitly declare the dependency in their pom file.
 
+***For Example:*** If you are using *Azure Active Directory Authentication feature* then you need to redeclare *adal4j* dependency in your project's pom file. Please see the following snippet: 
+```xml
+<dependency>
+	<groupId>com.microsoft.sqlserver</groupId>
+	<artifactId>mssql-jdbc</artifactId>
+	<version>6.3.6.jre8-preview</version>
+	<scope>compile</scope>
+</dependency>
+
+<dependency>
+	<groupId>com.microsoft.azure</groupId>
+	<artifactId>adal4j</artifactId>
+	<version>1.3.0</version>
+</dependency>
+```
+
 ***For Example:*** If you are using *Azure Key Vault feature* then you need to redeclare *azure-keyvault* dependency in your project's pom file. Please see the following snippet: 
 ```xml
 <dependency>
 	<groupId>com.microsoft.sqlserver</groupId>
 	<artifactId>mssql-jdbc</artifactId>
-	<version>6.3.5.jre8-preview</version>
+	<version>6.3.6.jre8-preview</version>
 	<scope>compile</scope>
 </dependency>
 

appveyor.yml

@@ -1,13 +1,16 @@
 version: '{build}'
 
+init:
+  - cmd: net start MSSQL$%SQL_Instance%
+
 environment:
-  mssql_jdbc_test_connection_properties: jdbc:sqlserver://localhost:1433;instanceName=SQL2016;databaseName=master;username=sa;password=Password12!;
+  JAVA_HOME: C:\Program Files\Java\jdk1.8.0
+  mssql_jdbc_test_connection_properties: jdbc:sqlserver://localhost:1433;instanceName=%SQL_Instance%;databaseName=master;username=sa;password=Password12!;
+
   matrix:
-    - JAVA_HOME: C:\Program Files\Java\jdk1.8.0 
+    - SQL_Instance: SQL2008R2SP2
+    - SQL_Instance: SQL2016
 
-services:
-  - mssql2016
-
 install:
   - ps: Write-Host 'Installing JCE with powershell'
   - ps: cd AppVeyorJCE

pom.xml

@@ -4,7 +4,7 @@
 
 	<groupId>com.microsoft.sqlserver</groupId>
 	<artifactId>mssql-jdbc</artifactId>
-	<version>6.3.6-SNAPSHOT.${jreVersion}-preview</version>
+	<version>6.3.6.${jreVersion}-preview</version>
 	<packaging>jar</packaging>
 
 	<name>Microsoft JDBC Driver for SQL Server</name>

src/main/java/com/microsoft/sqlserver/jdbc/SQLJdbcVersion.java

@@ -11,6 +11,6 @@
 final class SQLJdbcVersion {
     static final int major = 6;
     static final int minor = 3;
-    static final int patch = 5;
+    static final int patch = 6;
     static final int build = 0;
 }

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerClob.java

@@ -34,6 +34,9 @@
 
 public class SQLServerClob extends SQLServerClobBase implements Clob {
     private static final long serialVersionUID = 2872035282200133865L;
+
+    // Loggers should be class static to avoid lock contention with multiple threads
+    private static final Logger logger = Logger.getLogger("com.microsoft.sqlserver.jdbc.internals.SQLServerClob");
 
     /**
      * Create a new CLOB
@@ -47,19 +50,19 @@ public class SQLServerClob extends SQLServerClobBase implements Clob {
     @Deprecated
     public SQLServerClob(SQLServerConnection connection,
             String data) {
-        super(connection, data, (null == connection) ? null : connection.getDatabaseCollation(), null);
+        super(connection, data, (null == connection) ? null : connection.getDatabaseCollation(), logger, null);
 
         if (null == data)
             throw new NullPointerException(SQLServerException.getErrString("R_cantSetNull"));
     }
 
     SQLServerClob(SQLServerConnection connection) {
-        super(connection, "", connection.getDatabaseCollation(), null);
+        super(connection, "", connection.getDatabaseCollation(), logger, null);
     }
 
     SQLServerClob(BaseInputStream stream,
             TypeInfo typeInfo) throws SQLServerException, UnsupportedEncodingException {
-        super(null, stream, typeInfo.getSQLCollation(), typeInfo);
+        super(null, stream, typeInfo.getSQLCollation(), logger, typeInfo);
     }
 
     final JDBCType getJdbcType() {
@@ -89,7 +92,7 @@ abstract class SQLServerClobBase implements Serializable {
 
     transient SQLServerConnection con;
 
-    private final Logger logger = Logger.getLogger(getClass().getName());
+    private final Logger logger;
 
     final private String traceID = getClass().getName().substring(1 + getClass().getName().lastIndexOf('.')) + ":" + nextInstanceID();
 
@@ -128,6 +131,7 @@ private String getDisplayClassName() {
     SQLServerClobBase(SQLServerConnection connection,
             Object data,
             SQLCollation collation,
+            Logger logger,
             TypeInfo typeInfo) {
         this.con = connection;
         if (data instanceof BaseInputStream) {
@@ -137,8 +141,8 @@ private String getDisplayClassName() {
             this.value = (String) data;
         }
         this.sqlCollation = collation;
+        this.logger = logger;
         this.typeInfo = typeInfo;
-
         if (logger.isLoggable(Level.FINE)) {
             String loggingInfo = (null != connection) ? connection.toString() : "null connection";
             logger.fine(toString() + " created by (" + loggingInfo + ")");

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

@@ -123,8 +123,10 @@ public class SQLServerConnection implements ISQLServerConnection {
     static class Sha1HashKey {
         private byte[] bytes;
 
-        Sha1HashKey(String sql, String parametersDefinition) {
-            this(String.format("%s%s", sql, parametersDefinition));
+        Sha1HashKey(String sql,
+                String parametersDefinition,
+                String dbName) {
+            this(String.format("%s%s%s", sql, parametersDefinition, dbName));
         }
 
         Sha1HashKey(String s) {
@@ -3098,6 +3100,10 @@ final void poolCloseEventNotify() throws SQLServerException {
         return sCatalog;
     }
 
+    String getSCatalog() throws SQLServerException {
+        return sCatalog;
+    }
+
     /* L0 */ public void setTransactionIsolation(int level) throws SQLServerException {
         if (loggerExternal.isLoggable(Level.FINER)) {
             loggerExternal.entering(getClassNameLogging(), "setTransactionIsolation", level);

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDatabaseMetaData.java

@@ -267,7 +267,11 @@ private CallableStatement getCallableStatementHandle(CallableHandles request,
             hassoc = new HandleAssociation(catalog, CS);
             HandleAssociation previous = handleMap.put(request, hassoc);
             if (null != previous) {
+                ((SQLServerCallableStatement) previous.stmt).handleDBName = previous.databaseName;
+
                 previous.close();
+
+                ((SQLServerCallableStatement) previous.stmt).handleDBName = null;
             }
         }
         return hassoc.stmt;

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerNClob.java

@@ -10,6 +10,7 @@
 
 import java.io.UnsupportedEncodingException;
 import java.sql.NClob;
+import java.util.logging.Logger;
 
 /**
  * SQLServerNClob represents a National Character Set LOB object and implements java.sql.NClob.
@@ -19,13 +20,16 @@ public final class SQLServerNClob extends SQLServerClobBase implements NClob {
 
 	private static final long serialVersionUID = 1L;
 
+    // Loggers should be class static to avoid lock contention with multiple threads
+    private static final Logger logger = Logger.getLogger("com.microsoft.sqlserver.jdbc.internals.SQLServerNClob");
+
 	SQLServerNClob(SQLServerConnection connection) {
-        super(connection, "", connection.getDatabaseCollation(), null);
+        super(connection, "", connection.getDatabaseCollation(), logger, null);
     }
 
     SQLServerNClob(BaseInputStream stream,
             TypeInfo typeInfo) throws SQLServerException, UnsupportedEncodingException {
-        super(null, stream, typeInfo.getSQLCollation(), typeInfo);
+        super(null, stream, typeInfo.getSQLCollation(), logger, typeInfo);
     }
 
     final JDBCType getJdbcType() {

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java

@@ -100,6 +100,7 @@ public class SQLServerPreparedStatement extends SQLServerStatement implements IS
 
     /** The prepared statement handle returned by the server */
     private int prepStmtHandle = 0;
+    String handleDBName = null;
 
     /** Statement used for getMetadata(). Declared as a field to facilitate closing the statement. */
     private SQLServerStatement internalStmt = null;
@@ -184,7 +185,9 @@ String getClassNameInternal() {
         // Parse or fetch SQL metadata from cache.
         ParsedSQLCacheItem parsedSQL = getCachedParsedSQL(sqlTextCacheKey);
         if(null != parsedSQL) {
-            isExecutedAtLeastOnce = true;
+            if(null != connection && connection.isStatementPoolingEnabled()) {
+                isExecutedAtLeastOnce = true;
+            }
         }
         else {
             parsedSQL = parseAndCacheSQL(sqlTextCacheKey, sql);
@@ -535,7 +538,8 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE
             hasNewTypeDefinitions = buildPreparedStrings(inOutParam, true);
         }
 
-        if (reuseCachedHandle(hasNewTypeDefinitions, false)) {
+        String dbName = connection.getSCatalog();
+        if (reuseCachedHandle(hasNewTypeDefinitions, false, dbName)) {
             hasNewTypeDefinitions = false;
         }
 
@@ -581,8 +585,14 @@ boolean onRetValue(TDSReader tdsReader) throws SQLServerException {
                 setPreparedStatementHandle(param.getInt(tdsReader));
 
                 // Cache the reference to the newly created handle, NOT for cursorable handles.
-                if (null == cachedPreparedStatementHandle && !isCursorable(executeMethod)) {                
-                    cachedPreparedStatementHandle = connection.registerCachedPreparedStatementHandle(new Sha1HashKey(preparedSQL, preparedTypeDefinitions), prepStmtHandle, executedSqlDirectly);
+                if (null == cachedPreparedStatementHandle && !isCursorable(executeMethod)) {
+                    String dbName = connection.getSCatalog();
+                    if (null != handleDBName) {
+                        dbName = handleDBName;
+                    }
+
+                    cachedPreparedStatementHandle = connection.registerCachedPreparedStatementHandle(
+                            new Sha1HashKey(preparedSQL, preparedTypeDefinitions, dbName), prepStmtHandle, executedSqlDirectly);
                 }
 
                 param.skipValue(tdsReader, true);
@@ -904,7 +914,7 @@ private void getParameterEncryptionMetadata(Parameter[] params) throws SQLServer
     }
 
 	/** Manage re-using cached handles */
-	private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discardCurrentCacheItem) {
+	private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discardCurrentCacheItem, String dbName) {
         if (definitionChanged || connection.contextChanged) {
             prepStmtHandle = -1; // so that hasPreparedStatementHandle() also returns false
 
@@ -945,7 +955,8 @@ private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discard
 
         // Check for new cache reference.
         if (null == cachedPreparedStatementHandle) {
-            PreparedStatementHandle cachedHandle = connection.getCachedPreparedStatementHandle(new Sha1HashKey(preparedSQL, preparedTypeDefinitions));
+            PreparedStatementHandle cachedHandle = connection
+                    .getCachedPreparedStatementHandle(new Sha1HashKey(preparedSQL, preparedTypeDefinitions, dbName));
 
             // If handle was found then re-use, only if AE is not on and is not a batch query with new type definitions (We shouldn't reuse handle
             // if it is batch query and has new type definition, or if it is on, make sure encryptionMetadataIsRetrieved is retrieved.
@@ -2615,8 +2626,9 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
                         batchParam[i].cryptoMeta = cryptoMetaBatch.get(i);
                     }
                 }
-
-                if (reuseCachedHandle(hasNewTypeDefinitions, false)) {
+
+                String dbName = connection.getSCatalog();
+                if (reuseCachedHandle(hasNewTypeDefinitions, false, dbName)) {
                     hasNewTypeDefinitions = false;
                 }
 

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResultSet.java

@@ -965,6 +965,7 @@ public boolean next() throws SQLServerException {
                 moveFirst();
             else
                 moveForward(1);
+
             boolean value = hasCurrentRow();
             loggerExternal.exiting(getClassNameLogging(), "next", value);
             return value;

src/test/java/com/microsoft/sqlserver/jdbc/datatypes/SQLVariantResultSetTest.java

@@ -386,8 +386,9 @@ public void readvarBinary8000() throws SQLException, SecurityException, IOExcept
     public void readSQLVariantProperty() throws SQLException, SecurityException, IOException {
         String value = "hi";
         createAndPopulateTable("binary(8000)", "'" + value + "'");
-        rs = (SQLServerResultSet) stmt.executeQuery("SELECT SQL_VARIANT_PROPERTY(col1,'BaseType') AS 'Base Type',"
-                + " SQL_VARIANT_PROPERTY(col1,'Precision') AS 'Precision' from " + tableName);
+        rs = (SQLServerResultSet) stmt.executeQuery(
+                "SELECT SQL_VARIANT_PROPERTY(col1,'BaseType') AS 'Base Type', SQL_VARIANT_PROPERTY(col1,'Precision') AS 'Precision' from "
+                        + tableName);
         rs.next();
         assertTrue(rs.getString(1).equalsIgnoreCase("binary"), "unexpected baseType, expected: binary, retrieved:" + rs.getString(1));
     }

src/test/java/com/microsoft/sqlserver/jdbc/preparedStatement/BatchExecutionWithNullTest.java

@@ -117,16 +117,12 @@ public void testSetup() throws TestAbortedException, Exception {
     }
 
     @AfterAll
-    public static void terminateVariation() throws TestAbortedException, Exception {
-
-        assumeTrue(13 <= new DBConnection(connectionString).getServerVersion(),
-                "Aborting test case as SQL Server version is not compatible with Always encrypted ");
+    public static void terminateVariation() throws SQLException {
+        connection = DriverManager.getConnection(connectionString);
+
         SQLServerStatement stmt = (SQLServerStatement) connection.createStatement();
         Utils.dropTableIfExists("esimple", stmt);
 
-        if (null != connection) {
-            connection.close();
-        }
         if (null != pstmt) {
             pstmt.close();
         }
@@ -139,5 +135,8 @@ public static void terminateVariation() throws TestAbortedException, Exception {
         if (null != rs) {
             rs.close();
         }
+        if (null != connection) {
+            connection.close();
+        }
     }
 }

src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java

@@ -545,4 +545,54 @@ public void testStatementPoolingPreparedStatementExecAndUnprepareConfig() throws
             assertSame(0, con.getDiscardedServerPreparedStatementCount());
         }
     }
+
+    /**
+     * Validate the right behavior for EnablePrepareOnFirstPreparedStatementCall with 
+     * statement pooling turned off.
+     *
+     * @throws SQLException
+     */
+    @Test
+    public void testEnablePrepareOnFirstPreparedStatementCallWithStatementPoolingOff() throws SQLException {
+
+        try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) {
+
+            // Turn off use of prepared statement cache.
+            con.setStatementPoolingCacheSize(0);
+            // Disable EnablePrepareOnFirstPreparedStatementCall (default)
+            con.setEnablePrepareOnFirstPreparedStatementCall(false);
+
+            String query = "/*testEnablePrepareOnFirstPreparedStatementCallWithStatementPoolingOff*/SELECT * FROM sys.objects;";
+
+            // Verify first use is never prepared.
+            for(int i = 0; i < 10; ++i)
+            {
+                try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(query)) {
+                    pstmt.execute(); // sp_executesql
+                    pstmt.getMoreResults(); // Make sure handle is updated.
+
+                    // Validate no handle was created.
+                    assertTrue(0 >= pstmt.getPreparedStatementHandle());
+                }
+            }
+
+            // Verify second use is prepared.
+            for(int i = 0; i < 10; ++i)
+            {
+                try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(query)) {
+                    pstmt.execute(); // sp_executesql
+                    pstmt.getMoreResults(); // Make sure handle is updated.
+
+                    // Validate no handle was created.
+                    assertTrue(0 >= pstmt.getPreparedStatementHandle());
+
+                    pstmt.execute(); // sp_prepexec
+                    pstmt.getMoreResults(); // Make sure handle is updated.
+
+                    // Validate handle was created.
+                    assertTrue(0 < pstmt.getPreparedStatementHandle());
+                }
+            }
+        }
+    }
 }