Implement QuoteIdentifier and UnquoteIdentifier in OdbcCommandBuilder (#22499)

Implement QuoteIdentifier and UnquoteIdentifier in OdbcCommandBuilder

5 files changed, 146 insertions, 5 deletions

diff --git a/src/System.Data.Odbc/src/Common/System/Data/Common/AdapterUtil.Odbc.cs b/src/System.Data.Odbc/src/Common/System/Data/Common/AdapterUtil.Odbc.cs
index f354ee884c..ee835edbc9 100644
--- a/src/System.Data.Odbc/src/Common/System/Data/Common/AdapterUtil.Odbc.cs
+++ b/src/System.Data.Odbc/src/Common/System/Data/Common/AdapterUtil.Odbc.cs
@@ -306,6 +306,10 @@ namespace System.Data.Common
         {
             return InvalidOperation(SR.GetString(SR.ADP_UninitializedParameterSize, index.ToString(CultureInfo.InvariantCulture), dataType.Name));
         }
+        internal static InvalidOperationException QuotePrefixNotSet(string method)
+        {
+            return InvalidOperation(SR.GetString(SR.ADP_QuotePrefixNotSet, method));
+        }
 
         //
         // : ConnectionUtil
@@ -544,7 +548,6 @@ namespace System.Data.Common
             return Argument(SR.GetString(SR.MDF_UnsupportedVersion, collectionName));
         }
 
-
         // global constant strings
         internal const string BeginTransaction = "BeginTransaction";
         internal const string ChangeDatabase = "ChangeDatabase";
@@ -560,6 +563,8 @@ namespace System.Data.Common
         internal const string ParameterName = "ParameterName";
         internal const string Prepare = "Prepare";
         internal const string RollbackTransaction = "RollbackTransaction";
+        internal const string QuoteIdentifier = "QuoteIdentifier";
+        internal const string UnquoteIdentifier = "UnquoteIdentifier";
 
         internal const int DecimalMaxPrecision = 29;
         internal const int DecimalMaxPrecision28 = 28;  // there are some cases in Odbc where we need that ...
diff --git a/src/System.Data.Odbc/src/Resources/Strings.resx b/src/System.Data.Odbc/src/Resources/Strings.resx
index f90b0484a6..a0981e69ba 100644
--- a/src/System.Data.Odbc/src/Resources/Strings.resx
+++ b/src/System.Data.Odbc/src/Resources/Strings.resx
@@ -201,6 +201,9 @@
   <data name="ADP_NonPooledOpenTimeout" xml:space="preserve">
     <value>Timeout attempting to open the connection.  The time period elapsed prior to attempting to open the connection has been exceeded.  This may have occurred because of too many simultaneous non-pooled connection attempts.</value>
   </data>
+  <data name="ADP_QuotePrefixNotSet" xml:space="preserve">
+    <value>{0} requires an open connection when the quote prefix has not been set.</value>
+  </data>
   <data name="MDF_QueryFailed" xml:space="preserve">
     <value>Unable to build the '{0}' collection because execution of the SQL query failed. See the inner exception for details.</value>
   </data>
@@ -328,7 +331,7 @@
     <value>{0} DeriveParameters only supports CommandType.StoredProcedure, not CommandType.{1}.</value>
   </data>
   <data name="ADP_InvalidCommandTimeout" xml:space="preserve">
-    <value>Invalid CommandTimeout value {0}; the value must be >= 0.</value>
+    <value>Invalid CommandTimeout value {0}; the value must be &gt;= 0.</value>
   </data>
   <data name="ADP_UninitializedParameterSize" xml:space="preserve">
     <value>{1}[{0}]: the Size property has an invalid size of 0.</value>
diff --git a/src/System.Data.Odbc/src/System/Data/Odbc/OdbcCommandBuilder.cs b/src/System.Data.Odbc/src/System/Data/Odbc/OdbcCommandBuilder.cs
index a2fa431498..b1312e0d4b 100644
--- a/src/System.Data.Odbc/src/System/Data/Odbc/OdbcCommandBuilder.cs
+++ b/src/System.Data.Odbc/src/System/Data/Odbc/OdbcCommandBuilder.cs
@@ -244,16 +244,49 @@ namespace System.Data.Odbc
                 }
             }
             retcode = hstmt.CloseCursor();
-            return rParams.ToArray(); ;
+            return rParams.ToArray();
         }
 
         public override string QuoteIdentifier(string unquotedIdentifier)
         {
             return QuoteIdentifier(unquotedIdentifier, null /* use DataAdapter.SelectCommand.Connection if available */);
         }
+
         public string QuoteIdentifier(string unquotedIdentifier, OdbcConnection connection)
         {
-            throw ADP.NotSupported();
+            ADP.CheckArgumentNull(unquotedIdentifier, nameof(unquotedIdentifier));
+
+            // if the user has specificed a prefix use the user specified  prefix and suffix
+            // otherwise get them from the provider
+            string quotePrefix = QuotePrefix;
+            string quoteSuffix = QuoteSuffix;
+            if (string.IsNullOrEmpty(quotePrefix))
+            {
+                if (connection == null)
+                {
+                    // Use the adapter's connection if QuoteIdentifier was called from 
+                    // DbCommandBuilder instance (which does not have an overload that gets connection object)
+                    connection = DataAdapter?.SelectCommand?.Connection;
+                    if (connection == null)
+                    {
+                        throw ADP.QuotePrefixNotSet(ADP.QuoteIdentifier);
+                    }
+                }
+                quotePrefix = connection.QuoteChar(ADP.QuoteIdentifier);
+                quoteSuffix = quotePrefix;
+            }
+
+            // by the ODBC spec "If the data source does not support quoted identifiers, a blank is returned."
+            // So if a blank is returned the string is returned unchanged. Otherwise the returned string is used
+            // to quote the string
+            if (!string.IsNullOrEmpty(quotePrefix) && quotePrefix != " ")
+            {
+                return ADP.BuildQuotedString(quotePrefix, quoteSuffix, unquotedIdentifier);
+            }
+            else
+            {
+                return unquotedIdentifier;
+            }
         }
 
         protected override void SetRowUpdatingHandler(DbDataAdapter adapter)
@@ -273,9 +306,46 @@ namespace System.Data.Odbc
         {
             return UnquoteIdentifier(quotedIdentifier, null /* use DataAdapter.SelectCommand.Connection if available */);
         }
+
         public string UnquoteIdentifier(string quotedIdentifier, OdbcConnection connection)
         {
-            throw ADP.NotSupported();
+            ADP.CheckArgumentNull(quotedIdentifier, nameof(quotedIdentifier));
+
+            // if the user has specificed a prefix use the user specified  prefix and suffix
+            // otherwise get them from the provider
+            string quotePrefix = QuotePrefix;
+            string quoteSuffix = QuoteSuffix;
+            if (string.IsNullOrEmpty(quotePrefix))
+            {
+                if (connection == null)
+                {
+                    // Use the adapter's connection if UnquoteIdentifier was called from 
+                    // DbCommandBuilder instance (which does not have an overload that gets connection object)
+                    connection = DataAdapter?.SelectCommand?.Connection;
+                    if (connection == null)
+                    {
+                        throw ADP.QuotePrefixNotSet(ADP.UnquoteIdentifier);
+                    }
+                }
+                quotePrefix = connection.QuoteChar(ADP.UnquoteIdentifier);
+                quoteSuffix = quotePrefix;
+            }
+
+            String unquotedIdentifier;
+            // by the ODBC spec "If the data source does not support quoted identifiers, a blank is returned."
+            // So if a blank is returned the string is returned unchanged. Otherwise the returned string is used
+            // to unquote the string
+            if (!string.IsNullOrEmpty(quotePrefix) || quotePrefix != " ")
+            {
+                // ignoring the return value because it is acceptable for the quotedString to not be quoted in this
+                // context.
+                ADP.RemoveStringQuotes(quotePrefix, quoteSuffix, quotedIdentifier, out unquotedIdentifier);
+            }
+            else
+            {
+                unquotedIdentifier = quotedIdentifier;
+            }
+            return unquotedIdentifier;
         }
     }
 }
diff --git a/src/System.Data.Odbc/tests/CommandBuilderTests.cs b/src/System.Data.Odbc/tests/CommandBuilderTests.cs
new file mode 100644
index 0000000000..2b19b46a38
--- /dev/null
+++ b/src/System.Data.Odbc/tests/CommandBuilderTests.cs
@@ -0,0 +1,59 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Xunit;
+
+namespace System.Data.Odbc.Tests
+{
+    public class CommandBuilderTests : IntegrationTestBase
+    {
+        [Fact(Skip = "Native dependencies missing in CI. See https://github.com/dotnet/corefx/issues/15776.")]
+        public void QuoteIdentifier_UseConnection()
+        {
+            var commandBuilder = new OdbcCommandBuilder();
+
+            // Get quote string
+            var quotedIdentifier = commandBuilder.QuoteIdentifier("Test", connection);
+            var qs = quotedIdentifier.Remove(quotedIdentifier.IndexOf("Test"));
+            Assert.NotEmpty(qs);
+
+            // Test -> 'Test'
+            var quotedTestString = commandBuilder.QuoteIdentifier("Source", connection);
+            Assert.Equal($"{qs}Source{qs}", quotedTestString);
+            // 'Test' -> Test
+            Assert.Equal("Source", commandBuilder.UnquoteIdentifier(quotedTestString, connection));
+
+            // Test' -> 'Test'''
+            quotedTestString = commandBuilder.QuoteIdentifier($"Test identifier{qs}", connection);
+            Assert.Equal($"{qs}Test identifier{qs}{qs}{qs}", quotedTestString);
+            // 'Test''' -> Test'
+            Assert.Equal($"Test identifier{qs}", commandBuilder.UnquoteIdentifier(quotedTestString, connection));
+
+            // Needs an active connection
+            Assert.Throws<InvalidOperationException>(() => commandBuilder.QuoteIdentifier("Test", null));
+            Assert.Throws<InvalidOperationException>(() => commandBuilder.QuoteIdentifier("Test"));
+            Assert.Throws<InvalidOperationException>(() => commandBuilder.UnquoteIdentifier("Test", null));
+            Assert.Throws<InvalidOperationException>(() => commandBuilder.UnquoteIdentifier("Test"));
+        }
+
+        [Fact(Skip = "Native dependencies missing in CI. See https://github.com/dotnet/corefx/issues/15776.")]
+        public void QuoteIdentifier_CustomPrefixSuffix()
+        {
+            var commandBuilder = new OdbcCommandBuilder();
+
+            // Custom prefix & suffix
+            commandBuilder.QuotePrefix = "'";
+            commandBuilder.QuoteSuffix = "'";
+
+            Assert.Equal("'Test'", commandBuilder.QuoteIdentifier("Test", connection));
+            Assert.Equal("'Te''st'", commandBuilder.QuoteIdentifier("Te'st", connection));
+            Assert.Equal("Test", commandBuilder.UnquoteIdentifier("'Test'", connection));
+            Assert.Equal("Te'st", commandBuilder.UnquoteIdentifier("'Te''st'", connection));
+            
+            // Ensure we don't need active connection:
+            Assert.Equal("'Test'", commandBuilder.QuoteIdentifier("Test", null));
+            Assert.Equal("Test", commandBuilder.UnquoteIdentifier("'Test'", null));
+        }
+    }
+}
diff --git a/src/System.Data.Odbc/tests/System.Data.Odbc.Tests.csproj b/src/System.Data.Odbc/tests/System.Data.Odbc.Tests.csproj
index 462090fec9..d31912d784 100644
--- a/src/System.Data.Odbc/tests/System.Data.Odbc.Tests.csproj
+++ b/src/System.Data.Odbc/tests/System.Data.Odbc.Tests.csproj
@@ -10,8 +10,12 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Windows_NT-Release|AnyCPU'" />
   <ItemGroup>
     <Compile Include="IntegrationTestBase.cs" />
+    <Compile Include="CommandBuilderTests.cs" />
     <Compile Include="ReaderTests.cs" />
     <Compile Include="SmokeTest.cs" />
+    <Compile Include="$(CommonTestPath)\System\PlatformDetection.cs">
+      <Link>Common\System\PlatformDetection.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsWindows)' == 'true'">
     <Compile Include="ConnectionStrings.Windows.cs" />