ENT-1361 Enforce Unicode settings for SQL server (#311) (#321)

* Enforce Unicode settings for SQL server (#311)

* Enforce correct unicode setting in the JDBC Url for SQL server

* Avoid cast/null check

* Fixed wrong string literal and added test to check the literal matches what Hikari expects

* Optimise Imports

* Constant for config tag and minor test improvement.

* Constant and case insensitive check

* Import and Capitalisation

* Missed curly brace

node-api/src/main/kotlin/net/corda/nodeapi/internal/persistence/CordaPersistence.kt

@@ -65,6 +65,10 @@ class CordaPersistence(
         }
     }
 
+    object DataSourceConfigTag {
+        const val DATA_SOURCE_URL = "dataSource.url"
+    }
+
     /**
      * Creates an instance of [DatabaseTransaction], with the given transaction isolation level.
      */

node/src/main/kotlin/net/corda/node/services/config/NodeConfiguration.kt

@@ -9,6 +9,7 @@ import net.corda.node.services.messaging.CertificateChainCheckPolicy
 import net.corda.nodeapi.internal.config.NodeSSLConfiguration
 import net.corda.nodeapi.internal.config.User
 import net.corda.nodeapi.internal.config.parseAs
+import net.corda.nodeapi.internal.persistence.CordaPersistence.DataSourceConfigTag
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import java.net.URL
 import java.nio.file.Path
@@ -142,6 +143,12 @@ data class NodeConfigurationImpl(
         if (dataSourceProperties.get("transactionIsolation") == null) {
             dataSourceProperties["transactionIsolation"] = database.transactionIsolationLevel.jdbcString
         }
+
+        // enforce that SQLServer does not get sent all strings as Unicode - hibernate handles this "cleverly"
+        val dataSourceUrl = dataSourceProperties.getProperty(DataSourceConfigTag.DATA_SOURCE_URL, "")
+        if (dataSourceUrl.contains(":sqlserver:") && !dataSourceUrl.contains("sendStringParametersAsUnicode", true)) {
+            dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] = dataSourceUrl + ";sendStringParametersAsUnicode=false"
+        }
     }
 }
 

node/src/test/kotlin/net/corda/node/services/config/NodeConfigurationImplTest.kt

@@ -1,12 +1,17 @@
 package net.corda.node.services.config
 
+import com.zaxxer.hikari.HikariConfig
 import net.corda.core.utilities.NetworkHostAndPort
+import net.corda.nodeapi.internal.persistence.CordaPersistence.DataSourceConfigTag
 import net.corda.testing.ALICE_NAME
 import net.corda.testing.node.MockServices.Companion.makeTestDataSourceProperties
 import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.junit.Test
 import java.nio.file.Paths
+import java.util.*
+import kotlin.test.assertEquals
 import kotlin.test.assertFalse
+import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
 class NodeConfigurationImplTest {
@@ -27,10 +32,41 @@ class NodeConfigurationImplTest {
         assertFalse { configDebugOptions(true, DevModeOptions(true)).shouldCheckCheckpoints() }
     }
 
+    @Test
+    fun `check SQLServer unicode check`() {
+        val dataSourceProperties = Properties()
+        dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] = "jdbc:sqlserver://localhost:10433;databaseName=perftesting;sendStringParametersAsUnicode=false"
+        assertEquals("jdbc:sqlserver://localhost:10433;databaseName=perftesting;sendStringParametersAsUnicode=false", testConfiguration(dataSourceProperties).dataSourceProperties.getProperty(DataSourceConfigTag.DATA_SOURCE_URL))
+
+        dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] = "jdbc:sqlserver://localhost:10433;databaseName=perftesting"
+        assertEquals("jdbc:sqlserver://localhost:10433;databaseName=perftesting;sendStringParametersAsUnicode=false", testConfiguration(dataSourceProperties).dataSourceProperties.getProperty(DataSourceConfigTag.DATA_SOURCE_URL))
+
+        dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] = "jdbc:sqlserver://localhost:10433;databaseName=perftesting;sendStringParametersAsUnicode=true"
+        assertEquals("jdbc:sqlserver://localhost:10433;databaseName=perftesting;sendStringParametersAsUnicode=true", testConfiguration(dataSourceProperties).dataSourceProperties.getProperty(DataSourceConfigTag.DATA_SOURCE_URL))
+
+        dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] = "jdbc:h2:///some/dir/persistence"
+        assertEquals("jdbc:h2:///some/dir/persistence", testConfiguration(dataSourceProperties).dataSourceProperties.getProperty(DataSourceConfigTag.DATA_SOURCE_URL))
+
+        assertNull(testConfiguration(Properties()).dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL])
+    }
+
+    @Test
+    fun `create hikari data source config`() {
+        val dataSourceProperties = Properties()
+        dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] = "jdbc:sqlserver://localhost:10433;databaseName=perftesting"
+        val testConf = testConfiguration(dataSourceProperties)
+        assertEquals("jdbc:sqlserver://localhost:10433;databaseName=perftesting;sendStringParametersAsUnicode=false", testConf.dataSourceProperties.getProperty(DataSourceConfigTag.DATA_SOURCE_URL))
+        HikariConfig(testConf.dataSourceProperties)
+    }
+
     private fun configDebugOptions(devMode: Boolean, devModeOptions: DevModeOptions?) : NodeConfiguration {
         return testConfiguration.copy(devMode = devMode, devModeOptions = devModeOptions)
     }
 
+    private fun testConfiguration(dataSourceProperties: Properties): NodeConfigurationImpl {
+        return testConfiguration.copy(dataSourceProperties = dataSourceProperties)
+    }
+
     private val testConfiguration = NodeConfigurationImpl(
             baseDirectory = Paths.get("."),
             myLegalName = ALICE_NAME,

tools/demobench/src/test/kotlin/net/corda/demobench/model/NodeConfigTest.kt

@@ -7,6 +7,7 @@ import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.node.services.config.parseAsNodeConfiguration
 import net.corda.nodeapi.internal.config.User
 import net.corda.nodeapi.internal.config.toConfig
+import net.corda.nodeapi.internal.persistence.CordaPersistence.DataSourceConfigTag
 import net.corda.webserver.WebServerConfig
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Test
@@ -44,7 +45,7 @@ class NodeConfigTest {
         assertEquals(localPort(40002), fullConfig.rpcAddress)
         assertEquals(localPort(10001), fullConfig.p2pAddress)
         assertEquals(listOf(user("jenny")), fullConfig.rpcUsers)
-        assertThat(fullConfig.dataSourceProperties["dataSource.url"] as String).contains("AUTO_SERVER_PORT=30001")
+        assertThat(fullConfig.dataSourceProperties[DataSourceConfigTag.DATA_SOURCE_URL] as String).contains("AUTO_SERVER_PORT=30001")
         assertTrue(fullConfig.useTestClock)
         assertFalse(fullConfig.detectPublicIp)
     }