Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
3033edf
connection info telemetry data frm input bindings
MaddyDev Aug 27, 2022
207202c
capture convert event after collecting connection
MaddyDev Aug 30, 2022
61c0d49
remove event
MaddyDev Aug 30, 2022
5fc7e00
first collect telemetry
MaddyDev Aug 30, 2022
b3fb013
fix tests
MaddyDev Aug 30, 2022
cbcfa4e
make convert type optional
MaddyDev Aug 31, 2022
5669021
update xml
MaddyDev Aug 31, 2022
214b718
Merge branch 'main' into maddy/connecInfoTelemetryInputBindings
MaddyDev Aug 31, 2022
9f02a93
add telemetry service
MaddyDev Sep 6, 2022
f2cf20d
Merge branch 'main' into maddy/connecInfoTelemetryInputBindings
MaddyDev Sep 6, 2022
61fb46f
clean up
MaddyDev Sep 7, 2022
da07358
remove ITelemetryService
MaddyDev Sep 7, 2022
48c62b0
undo changes
MaddyDev Sep 7, 2022
8b343e2
merge conflict
MaddyDev Sep 7, 2022
b5d4b83
missed merge conflict
MaddyDev Sep 7, 2022
8a42748
extra line
MaddyDev Sep 7, 2022
85e5e7f
add null check
MaddyDev Sep 7, 2022
d69d896
capture telemetry after opening connection
MaddyDev Sep 7, 2022
0d46238
correct connection
MaddyDev Sep 7, 2022
8f6be53
move telemtery after connection.OpenAsync
MaddyDev Sep 7, 2022
71e5ebd
open connection on initialize
MaddyDev Sep 7, 2022
6559474
add xml
MaddyDev Sep 7, 2022
5afaabe
address comments
MaddyDev Sep 7, 2022
cf89b7e
refactor test
MaddyDev Sep 7, 2022
c69065f
add xml comment
MaddyDev Sep 8, 2022
c08b7dd
update xml
MaddyDev Sep 8, 2022
a627e4d
add check back
MaddyDev Sep 8, 2022
2e35c25
add connection state check
MaddyDev Sep 13, 2022
27e0814
remove explicit checks
MaddyDev Sep 13, 2022
5207365
add isDisposed
MaddyDev Sep 14, 2022
b02ba41
add comments
MaddyDev Sep 14, 2022
df4c464
revert isDisposed changes
MaddyDev Sep 14, 2022
ded66ad
add comment with github issue link
MaddyDev Sep 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions src/SqlAsyncEnumerable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@
using System.Threading.Tasks;
using Microsoft.Data.SqlClient;
using Newtonsoft.Json;

namespace Microsoft.Azure.WebJobs.Extensions.Sql
{
/// <typeparam name="T">A user-defined POCO that represents a row of the user's table</typeparam>
internal class SqlAsyncEnumerable<T> : IAsyncEnumerable<T>
{
private readonly SqlConnection _connection;
public SqlConnection Connection { get; private set; }
private readonly SqlAttribute _attribute;

/// <summary>
Expand All @@ -26,8 +25,9 @@ internal class SqlAsyncEnumerable<T> : IAsyncEnumerable<T>
/// </exception>
public SqlAsyncEnumerable(SqlConnection connection, SqlAttribute attribute)
{
this._connection = connection ?? throw new ArgumentNullException(nameof(connection));
this.Connection = connection ?? throw new ArgumentNullException(nameof(connection));
this._attribute = attribute ?? throw new ArgumentNullException(nameof(attribute));
this.Connection.Open();
}
/// <summary>
/// Returns the enumerator associated with this enumerable. The enumerator will execute the query specified
Expand All @@ -38,7 +38,7 @@ public SqlAsyncEnumerable(SqlConnection connection, SqlAttribute attribute)
/// <returns>The enumerator</returns>
public IAsyncEnumerator<T> GetAsyncEnumerator(CancellationToken cancellationToken = default)
{
return new SqlAsyncEnumerator(this._connection, this._attribute);
return new SqlAsyncEnumerator(this.Connection, this._attribute);
}


Expand All @@ -47,7 +47,6 @@ private class SqlAsyncEnumerator : IAsyncEnumerator<T>
private readonly SqlConnection _connection;
private readonly SqlAttribute _attribute;
private SqlDataReader _reader;

/// <summary>
/// Initializes a new instance of the <see cref="SqlAsyncEnumerator<typeparamref name="T"/>"/> class.
/// </summary>
Expand Down Expand Up @@ -77,7 +76,7 @@ public SqlAsyncEnumerator(SqlConnection connection, SqlAttribute attribute)
public ValueTask DisposeAsync()
{
// Doesn't seem like there's an async version of closing the reader/connection
this._reader.Close();
this._reader?.Close();
this._connection.Close();
return new ValueTask(Task.CompletedTask);
}
Expand All @@ -101,23 +100,24 @@ public ValueTask<bool> MoveNextAsync()
/// </returns>
private async Task<bool> GetNextRowAsync()
{
if (this._reader == null)
// check connection state before trying to access the reader
// if DisposeAsync has already closed it due to the issue described here https://github.com/Azure/azure-functions-sql-extension/issues/350
if (this._connection.State != System.Data.ConnectionState.Closed)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the tests pass please create an issue for investigating this and then add a comment explaining why we're doing this with a link to the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran locally and they passed (fingers crossed), will add the comment, thanks

{
using (SqlCommand command = SqlBindingUtilities.BuildCommand(this._attribute, this._connection))
if (this._reader == null)
{
await command.Connection.OpenAsync();
this._reader = await command.ExecuteReaderAsync();
using (SqlCommand command = SqlBindingUtilities.BuildCommand(this._attribute, this._connection))
{
this._reader = await command.ExecuteReaderAsync();
}
}
if (await this._reader.ReadAsync())
{
this.Current = JsonConvert.DeserializeObject<T>(this.SerializeRow());
return true;
}
}
if (await this._reader.ReadAsync())
{
this.Current = JsonConvert.DeserializeObject<T>(this.SerializeRow());
return true;
}
else
{
return false;
}
return false;
}

/// <summary>
Expand Down
22 changes: 13 additions & 9 deletions src/SqlConverters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,11 @@ public SqlGenericsConverter(IConfiguration configuration, ILogger logger)
/// <returns>An IEnumerable containing the rows read from the user's database in the form of the user-defined POCO</returns>
public async Task<IEnumerable<T>> ConvertAsync(SqlAttribute attribute, CancellationToken cancellationToken)
{
TelemetryInstance.TrackConvert(ConvertType.IEnumerable);
this._logger.LogDebugWithThreadId("BEGIN ConvertAsync (IEnumerable)");
var sw = Stopwatch.StartNew();
try
{
string json = await this.BuildItemFromAttributeAsync(attribute);
string json = await this.BuildItemFromAttributeAsync(attribute, ConvertType.IEnumerable);
IEnumerable<T> result = JsonConvert.DeserializeObject<IEnumerable<T>>(json);
this._logger.LogDebugWithThreadId($"END ConvertAsync (IEnumerable) Duration={sw.ElapsedMilliseconds}ms");
return result;
Expand Down Expand Up @@ -140,12 +139,11 @@ public async Task<IEnumerable<T>> ConvertAsync(SqlAttribute attribute, Cancellat
/// </returns>
async Task<string> IAsyncConverter<SqlAttribute, string>.ConvertAsync(SqlAttribute attribute, CancellationToken cancellationToken)
{
TelemetryInstance.TrackConvert(ConvertType.Json);
this._logger.LogDebugWithThreadId("BEGIN ConvertAsync (Json)");
var sw = Stopwatch.StartNew();
try
{
string result = await this.BuildItemFromAttributeAsync(attribute);
string result = await this.BuildItemFromAttributeAsync(attribute, ConvertType.Json);
this._logger.LogDebugWithThreadId($"END ConvertAsync (Json) Duration={sw.ElapsedMilliseconds}ms");
return result;
}
Expand All @@ -167,8 +165,11 @@ async Task<string> IAsyncConverter<SqlAttribute, string>.ConvertAsync(SqlAttribu
/// <param name="attribute">
/// The binding attribute that contains the name of the connection string app setting and query.
/// </param>
/// <param name="type">
/// The type of conversion being performed by the input binding.
/// </param>
/// <returns></returns>
public virtual async Task<string> BuildItemFromAttributeAsync(SqlAttribute attribute)
public virtual async Task<string> BuildItemFromAttributeAsync(SqlAttribute attribute, ConvertType type)
{
using (SqlConnection connection = SqlBindingUtilities.BuildConnection(attribute.ConnectionStringSetting, this._configuration))
// Ideally, we would like to move away from using SqlDataAdapter both here and in the
Expand All @@ -178,6 +179,8 @@ public virtual async Task<string> BuildItemFromAttributeAsync(SqlAttribute attri
{
adapter.SelectCommand = command;
await connection.OpenAsync();
Dictionary<TelemetryPropertyName, string> props = connection.AsConnectionProps();
TelemetryInstance.TrackConvert(type, props);
var dataTable = new DataTable();
adapter.Fill(dataTable);
this._logger.LogInformation($"{dataTable.Rows.Count} row(s) queried from database: {connection.Database} using Command: {command.CommandText}");
Expand All @@ -188,10 +191,12 @@ public virtual async Task<string> BuildItemFromAttributeAsync(SqlAttribute attri

IAsyncEnumerable<T> IConverter<SqlAttribute, IAsyncEnumerable<T>>.Convert(SqlAttribute attribute)
{
TelemetryInstance.TrackConvert(ConvertType.IAsyncEnumerable);
try
{
return new SqlAsyncEnumerable<T>(SqlBindingUtilities.BuildConnection(attribute.ConnectionStringSetting, this._configuration), attribute);
var asyncEnumerable = new SqlAsyncEnumerable<T>(SqlBindingUtilities.BuildConnection(attribute.ConnectionStringSetting, this._configuration), attribute);
Dictionary<TelemetryPropertyName, string> props = asyncEnumerable.Connection.AsConnectionProps();
TelemetryInstance.TrackConvert(ConvertType.IAsyncEnumerable, props);
return asyncEnumerable;
}
catch (Exception ex)
{
Expand All @@ -214,10 +219,9 @@ IAsyncEnumerable<T> IConverter<SqlAttribute, IAsyncEnumerable<T>>.Convert(SqlAtt
/// <returns>JArray containing the rows read from the user's database in the form of the user-defined POCO</returns>
async Task<JArray> IAsyncConverter<SqlAttribute, JArray>.ConvertAsync(SqlAttribute attribute, CancellationToken cancellationToken)
{
TelemetryInstance.TrackConvert(ConvertType.JArray);
try
{
string json = await this.BuildItemFromAttributeAsync(attribute);
string json = await this.BuildItemFromAttributeAsync(attribute, ConvertType.JArray);
return JArray.Parse(json);
}
catch (Exception ex)
Expand Down
20 changes: 11 additions & 9 deletions test/Unit/SqlInputBindingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Moq;
using Xunit;
using Microsoft.Azure.WebJobs.Extensions.Sql.Tests.Common;
using Microsoft.Azure.WebJobs.Extensions.Sql.Telemetry;

namespace Microsoft.Azure.WebJobs.Extensions.Sql.Tests.Unit
{
Expand Down Expand Up @@ -60,17 +61,18 @@ public void TestNullCommand()
[Fact]
public void TestNullArgumentsSqlAsyncEnumerableConstructor()
{

Assert.Throws<ArgumentNullException>(() => new SqlAsyncEnumerable<string>(connection, null));
Assert.Throws<ArgumentNullException>(() => new SqlAsyncEnumerable<string>(null, new SqlAttribute("")));
}

/// <summary>
/// SqlAsyncEnumerable should throw InvalidOperationExcepion when invoked with an invalid connection
/// string setting and It should fail here since we're passing an empty connection string.
/// <summary>
[Fact]
public void TestNullCurrentValueEnumerator()
public void TestInvalidOperationSqlAsyncEnumerableConstructor()
{
var enumerable = new SqlAsyncEnumerable<string>(connection, new SqlAttribute(""));
IAsyncEnumerator<string> enumerator = enumerable.GetAsyncEnumerator();
Assert.Null(enumerator.Current);
Assert.Throws<InvalidOperationException>(() => new SqlAsyncEnumerable<string>(connection, new SqlAttribute("")));
}

[Fact]
Expand Down Expand Up @@ -230,7 +232,7 @@ public async void TestWellformedDeserialization()
var converter = new Mock<SqlGenericsConverter<TestData>>(config.Object, logger.Object);
string json = "[{ \"ID\":1,\"Name\":\"Broom\",\"Cost\":32.5,\"Timestamp\":\"2019-11-22T06:32:15\"},{ \"ID\":2,\"Name\":\"Brush\",\"Cost\":12.3," +
"\"Timestamp\":\"2017-01-27T03:13:11\"},{ \"ID\":3,\"Name\":\"Comb\",\"Cost\":100.12,\"Timestamp\":\"1997-05-03T10:11:56\"}]";
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg)).ReturnsAsync(json);
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg, ConvertType.IEnumerable)).ReturnsAsync(json);
var list = new List<TestData>();
var data1 = new TestData
{
Expand Down Expand Up @@ -268,7 +270,7 @@ public async void TestMalformedDeserialization()

// SQL data is missing a field
string json = "[{ \"ID\":1,\"Name\":\"Broom\",\"Timestamp\":\"2019-11-22T06:32:15\"}]";
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg)).ReturnsAsync(json);
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg, ConvertType.IEnumerable)).ReturnsAsync(json);
var list = new List<TestData>();
var data = new TestData
{
Expand All @@ -283,7 +285,7 @@ public async void TestMalformedDeserialization()

// SQL data's columns are named differently than the POCO's fields
json = "[{ \"ID\":1,\"Product Name\":\"Broom\",\"Price\":32.5,\"Timessstamp\":\"2019-11-22T06:32:15\"}]";
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg)).ReturnsAsync(json);
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg, ConvertType.IEnumerable)).ReturnsAsync(json);
list = new List<TestData>();
data = new TestData
{
Expand All @@ -297,7 +299,7 @@ public async void TestMalformedDeserialization()

// Confirm that the JSON fields are case-insensitive (technically malformed string, but still works)
json = "[{ \"id\":1,\"nAme\":\"Broom\",\"coSt\":32.5,\"TimEStamp\":\"2019-11-22T06:32:15\"}]";
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg)).ReturnsAsync(json);
converter.Setup(_ => _.BuildItemFromAttributeAsync(arg, ConvertType.IEnumerable)).ReturnsAsync(json);
list = new List<TestData>();
data = new TestData
{
Expand Down