Bug in transient Fault Handling in EntLib 6

Topics: Transient Fault Handling Application Block ("Topaz")
Apr 26, 2013 at 10:24 AM
Edited Apr 26, 2013 at 10:28 AM
Hi,

With the release of EntLib 6, I have checked if the status of recent bug I discovered in SQL Azure transient fault handling did change or not, and it seems not.

It seems to be an easy bug to fix, and I am glad if I can help in any manner.

I've added 2 units tests as comments which currently fail to demonstrate existence of this bug :
[TestMethod]
public void TestExecuteSimpleCommandWithObjectResult()
{
    SqlCommand command = new SqlCommand("SELECT cast(1 as int)");
    object result = connection.ExecuteCommand<object>(command);

    Assert.IsNotNull(result, "Unexpected null result");

    Type resultType = result.GetType();

    Assert.AreEqual(typeof(int), resultType, "Unexpected result type");
    Assert.AreEqual(1, Convert.ToInt32(result), "Unexpected result");
}

[TestMethod]
public void TextExecuteSelectCommandToGetSqlDataReader()
{
    SqlCommand command = new SqlCommand("SELECT [ProductCategoryID], [Name] FROM [SalesLT].[ProductCategory]");
    using (SqlDataReader reader = connection.ExecuteCommand<SqlDataReader>(command))
    {
        while (reader.Read())
        {
            int id = reader.GetInt32(0);
            string name = reader.GetString(1);

            Trace.WriteLine(string.Format("{0}: {1}", id, name));
        }

        reader.Close();
    }
}
And I can confirm that if you revert types comparison resultType.IsAssignableFrom(typeof(IDataReader)) into typeof(IDataReader).IsAssignableFrom(resultType) (and the same for XmlReader), the both tests succeed (and previous tests too).
May 25, 2013 at 9:18 AM
Hello ? Is there anyone here ?
Coordinator
May 25, 2013 at 3:01 PM
@styx31
Have you checked the updated package we released on 5/24? It contains a fix.
See on NuGet - https://nuget.org/packages/EnterpriseLibrary.TransientFaultHandling.Data/.

Also the source code has been updated.

Grigori
May 28, 2013 at 11:19 AM
Thank you for your answer,

I didn't notice the update because I only subscribed to updates on codeplex discussions.

Thanks again for the reply and the status update of the WI.
Jun 6, 2013 at 9:15 AM
Hi again,

Do you accept pull requests ?
I have a feature that is currently blocked by the way ReliableSqlConnection handle generic parameters and conversions.

In our case my method must use "object" as generic parameter for ExecuteCommand<T>() for all ExecuteScalar calls.

Currently, the ExecuteCommand<T>() method uses Convert.ChangeType in all cases. The good point of this method is that if the resultType and the result.GetType() are the same it does nothing. But if the resultType and the result.GetType are different, it requires that the type implements IConvertible.

So, if you use ExecuteCommand<object>() and the query returns, for example, a varbinary(max) (which is a byte[]), the method fails, despite the fact that byte[] is natively convertible to object.

See this two test cases :
[TestMethod]
public void TestExecuteSimpleCommandWithBinaryResult()
{
    SqlCommand command = new SqlCommand("SELECT cast(0xfe as varbinary(max))");
    var result = connection.ExecuteCommand<byte[]>(command);

    Assert.IsNotNull(result, "Unexpected null result");

    Type resultType = result.GetType();

    Assert.AreEqual(typeof(byte[]), resultType, "Unexpected result type");
    Assert.AreEqual(1, ((byte[])result).Length, "Unexpected binary result length");
    Assert.AreEqual(254, ((byte[])result)[0], "Unexpected binary result datas");
}

[TestMethod]
public void TestExecuteSimpleCommandWithBinaryAsObjectResult()
{
    SqlCommand command = new SqlCommand("SELECT cast(0xfe as varbinary(max))");
    var result = connection.ExecuteCommand<object>(command);

    Assert.IsNotNull(result, "Unexpected null result");

    Type resultType = result.GetType();

    Assert.AreEqual(typeof(byte[]), resultType, "Unexpected result type");
    Assert.AreEqual(1, ((byte[])result).Length, "Unexpected binary result length");
    Assert.AreEqual(254, ((byte[])result)[0], "Unexpected binary result datas");
}
The first test case will work, but not the second one.

My suggestion is to change the method ExecuteCommand<T>() to handle this case : if typeof(T) == typeof(object), then simply return the result as T, and let the caller handling the conversion. It will not break any existing code which use a T different from object and allow callers with specific conversion needs (as me) to handle type conversion by themselves.

The end of ExecuteCommand<T>() is modified this way (starting line 313) :
if (resultType == typeof(NonQueryResult))
{
    var result = new NonQueryResult { RecordsAffected = command.ExecuteNonQuery() };

    closeOpenedConnectionOnSuccess = true;

    return (T)Convert.ChangeType(result, resultType, CultureInfo.InvariantCulture);
}
else
{
    var result = command.ExecuteScalar();

    closeOpenedConnectionOnSuccess = true;

    if (resultType == typeof(object))
    {
        // In case of T as object, we let the caller handle the conversion
        return (T)result;
    }

    if (result != null)
    {
        return (T)Convert.ChangeType(result, resultType, CultureInfo.InvariantCulture);
    }

    return default(T);
}
The two tests cases quoted before will work against this change.

If you want me to create a WI, please do it.
Editor
Jun 12, 2013 at 2:55 AM
Thanks for posting. Enterprise Library is open source and released under MS-PL; the source code can be modified and redistributed subject to the very permissive license. Unfortunately, code contributions to the project are not accepted.

~~
Randy Levy
entlib.support@live.com
Enterprise Library support engineer
Support How-to