infinite loop / missing data when converting to a DataSet

[mgrandi]

I am using the IDataReader.ToDataset() extension method provided by the Microsoft.Azure.Kusto.Cloud.Platform nuget package to convert a IDataReader to a DataSet.

It seems that with the MockDataTable that I am setting up using this nuget package, it seems to be going into a infinite loop. I took a look today, and it seems that it might be caused by the fact that this package isn't mocking IDataReader.IsClosed and IDataReader.NextResult(),

I got around the infinite loop temporarily by doing

            mockDataReader.SetupAllProperties();
            mockDataReader.SetupSequence(m => m.IsClosed).Returns(false).Returns(true).Returns(true).Returns(true);
            mockDataReader.SetupSequence(m => m.NextResult()).Returns(false);

but that seems to not be enough, because the DataSet is returned but has no columns or rows

the extension method provided by the Kusto nuget package seems to be using DataTable.Load() to load the data, which the source code is here: https://github.com/microsoft/referencesource/blob/master/System.Data/System/Data/DataTable.cs#L4459 , but is fairly complicated, so i would need some time to see what is happening

I'm wondering if it would honestly be easier to just implement IDataReader itself rather than using Moq to mock the individual methods, as that would still provide immense value as implementing IDataReader is complicated for a single developer to do, and having the extension method is handy, and implementing it normally without mock would make it so all edge cases would be less likely to happen. Honestly i think the API could still stay the same, as you still have the MockDataTable object that you pass into SetupReturns(), and no would needs to know or care that it is an actual IDataReader implementation

I am willing to help contribute either way, however i am busy at the moment so I wanted to file this issue before i forget

[alfusinigoj]

Hello @mgrandi , thanks for the details. I did have a look at the call stack and figured out that it is much complicated or even not possible, because of the new() objects like DataTable being created on the fly. I don't think that the code is that easily unit testable.

Here is the call stack (methods), that i could trace out.

using System.Data;

public static DataSet ToDataSet(this IDataReader reader)
{
    return reader.FromDataReader("DataSet");
}

using System.Data;
using Kusto.Cloud.Platform.Utils;

public static DataSet FromDataReader(this IDataReader reader, string name)
{
    using (reader)
    {
        DataSet dataSet = new DataSet(name);
        int num = 0;
        bool moreData = true;
        while (moreData)
        {
            DataTable dataTable = ExtendedDataTable.FromDataReader(reader, out moreData);
            if (string.IsNullOrWhiteSpace(dataTable.TableName))
            {
                dataTable.TableName = $"{name}_Table_{num++}";
            }
            dataSet.Tables.Add(dataTable);
        }
        dataSet.AcceptChanges();
        return dataSet;
    }
}

using System;
using System.Data;
using Kusto.Cloud.Platform.Utils;

public static DataTable FromDataReader(IDataReader reader, out bool moreData)
{
    moreData = false;
    try
    {
        DataTable dataTable = new DataTable();
        DataTable schemaTable = reader.GetSchemaTable();
        dataTable.Columns.CollectionChanged += Columns_CollectionChanged;
        dataTable.Locale = ExtendedCultureInfo.InvariantCulture;
        dataTable.Load(reader);
        dataTable.Columns.CollectionChanged -= Columns_CollectionChanged;
        DataColumn dataColumn = schemaTable.Columns["ColumnType"];
        DataColumn dataColumn2 = schemaTable.Columns["ColumnOrdinal"];
        if (dataColumn != null && dataColumn2 != null)
        {
            foreach (DataRow row in schemaTable.Rows)
            {
                int index = Convert.ToInt32(row[dataColumn2]);
                dataTable.Columns[index].ExtendedProperties.Add("ColumnType", Convert.ToString(row[dataColumn]));
            }
        }
        moreData = !reader.IsClosed;
        return dataTable;
    }
    finally
    {
        if (!moreData)
        {
            reader.Dispose();
        }
    }
}

Which then calls the code you referred, DataTable.Load(reader, ...).

As you mentioned, I done see a real value in trying to make this work. However, I came across A Test Helper that could help in Mocking this, if there is some time to spend. Please have a look.

[mgrandi]

thanks for taking a look!

would it be worth it to change the logic or add a second set of classes to actually implement a full IDataReader? (dependingon if doing that breaks any existing usage of the current code)

it seems that the current set of classes work for some use cases but there are others where like Kusto are expecting a full IDataReader and it might be worth to implement one, and it might actually be easier to do rather than using Moq methods alone like the current code does