Sync triggerbindings up to main

README.md

@@ -48,7 +48,7 @@ Azure SQL bindings for Azure Functions are supported for:
       - [Stored Procedure](#stored-procedure)
       - [IAsyncEnumerable](#iasyncenumerable)
     - [Output Binding](#output-binding)
-      - [ICollector<T>/IAsyncCollector<T>](#icollectortiasynccollectort)
+      - [ICollector&lt;T&gt;/IAsyncCollector&lt;T&gt;](#icollectortiasynccollectort)
       - [Array](#array)
       - [Single Row](#single-row)
       - [Primary Keys and Identity Columns](#primary-keys-and-identity-columns)
@@ -240,7 +240,7 @@ Note: This tutorial requires that a SQL database is setup as shown in [Create a
 - Open your app that you created in [Create a Function App](#create-a-function-app) in VSCode
 - Press 'F1' and search for 'Azure Functions: Create Function'
 - Choose HttpTrigger -> (Provide a function name) -> Company.namespace -> anonymous
-- In the file that opens, replace the 'public static async Task< IActionResult > Run' block with the below code.
+- In the file that opens, replace the `public static async Task<IActionResult> Run` block with the below code.
 
     ```csharp
     public static async Task<IActionResult> Run(
@@ -288,7 +288,7 @@ Note: This tutorial requires that a SQL database is setup as shown in [Create a
 - Open your app in VSCode
 - Press 'F1' and search for 'Azure Functions: Create Function'
 - Choose HttpTrigger ->  (Provide a function name) -> Company.namespace is fine -> anonymous
-- In the file that opens, replace the 'public static async Task<IActionResult> Run' block with the below code
+- In the file that opens, replace the `public static async Task<IActionResult> Run` block with the below code
 
     ```csharp
     public static IActionResult Run(
@@ -377,7 +377,7 @@ Note: This tutorial requires that a SQL database is setup as shown in [Create a
 - Open your app that you created in [Create a Function App](#create-a-function-app) in VSCode
 - Press 'F1' and search for 'Azure Functions: Create Function'
 - Choose HttpTrigger -> (Provide a function name) -> anonymous
-- In the file that opens (index.js), replace the 'module.exports = async function (context, req)' block with the below code.
+- In the file that opens (`index.js`), replace the `module.exports = async function (context, req)` block with the below code.
 
     ```javascript
     module.exports = async function (context, req, employee) {
@@ -417,7 +417,7 @@ Note: This tutorial requires that a SQL database is setup as shown in [Create a
 - Open your app in VSCode
 - Press 'F1' and search for 'Azure Functions: Create Function'
 - Choose HttpTrigger ->  (Provide a function name) -> anonymous
-- In the file that opens (index.js), replace the 'module.exports = async function (context, req)' block with the below code.
+- In the file that opens (`index.js`), replace the `module.exports = async function (context, req)` block with the below code.
 
     ```javascript
     module.exports = async function (context, req) {
@@ -472,7 +472,7 @@ Note: This tutorial requires that a SQL database is setup as shown in [Create a
 - Open your app that you created in [Create a Function App](#create-a-function-app) in VSCode
 - Press 'F1' and search for 'Azure Functions: Create Function'
 - Choose HttpTrigger -> (Provide a function name) -> anonymous
-- In the file that opens (__init__.py), replace the 'def main(req: func.HttpRequest) -> func.HttpResponse:' block with the below code.
+- In the file that opens (`__init__.py`), replace the `def main(req: func.HttpRequest) -> func.HttpResponse:` block with the below code.
 
     ```python
     def main(req: func.HttpRequest, employee: func.SqlRowList) -> func.HttpResponse:
@@ -515,7 +515,7 @@ Note: This tutorial requires that a SQL database is setup as shown in [Create a
 - Open your app in VSCode
 - Press 'F1' and search for 'Azure Functions: Create Function'
 - Choose HttpTrigger ->  (Provide a function name) -> anonymous
-- In the file that opens (__init__.py), replace the 'def main(req: func.HttpRequest) -> func.HttpResponse:' block with the below code.
+- In the file that opens (`__init__.py`), replace the `def main(req: func.HttpRequest) -> func.HttpResponse:` block with the below code.
 
     ```python
     def main(req: func.HttpRequest, employee: func.Out[func.SqlRow]) -> func.HttpResponse:
@@ -567,8 +567,8 @@ The input binding takes four [arguments](https://github.com/Azure/azure-function
 
 The following are valid binding types for the result of the query/stored procedure execution:
 
-- **IEnumerable<T>**: Each element is a row of the result represented by `T`, where `T` is a user-defined POCO, or Plain Old C# Object. `T` should follow the structure of a row in the queried table. See the [Query String](#query-string) section for an example of what `T` should look like.
-- **IAsyncEnumerable<T>**: Each element is again a row of the result represented by `T`, but the rows are retrieved "lazily". A row of the result is only retrieved when `MoveNextAsync` is called on the enumerator. This is useful in the case that the query can return a very large amount of rows.
+- **IEnumerable&lt;T&gt;**: Each element is a row of the result represented by `T`, where `T` is a user-defined POCO, or Plain Old C# Object. `T` should follow the structure of a row in the queried table. See the [Query String](#query-string) section for an example of what `T` should look like.
+- **IAsyncEnumerable&lt;T&gt;**: Each element is again a row of the result represented by `T`, but the rows are retrieved "lazily". A row of the result is only retrieved when `MoveNextAsync` is called on the enumerator. This is useful in the case that the query can return a very large amount of rows.
 - **String**: A JSON string representation of the rows of the result (an example is provided [here](https://github.com/Azure/azure-functions-sql-extension/blob/main/samples/samples-csharp/InputBindingSamples/GetProductsString.cs)).
 - **SqlCommand**: The SqlCommand is populated with the appropriate query and parameters, but the associated connection is not opened. It is the responsiblity of the user to execute the command and read in the results. This is useful in the case that the user wants more control over how the results are read in. An example is provided [here](https://github.com/Azure/azure-functions-sql-extension/blob/main/samples/samples-csharp/InputBindingSamples/GetProductsSqlCommand.cs).
 
@@ -708,13 +708,13 @@ The output binding takes two [arguments](https://github.com/Azure/azure-function
 
 The following are valid binding types for the rows to be upserted into the table:
 
-- **ICollector<T>/IAsyncCollector<T>**: Each element is a row represented by `T`, where `T` is a user-defined POCO, or Plain Old C# Object. `T` should follow the structure of a row in the queried table. See the [Query String](#query-string) for an example of what `T` should look like.
+- **ICollector&lt;T&gt;/IAsyncCollector&lt;T&gt;**: Each element is a row represented by `T`, where `T` is a user-defined POCO, or Plain Old C# Object. `T` should follow the structure of a row in the queried table. See the [Query String](#query-string) for an example of what `T` should look like.
 - **T**: Used when just one row is to be upserted into the table.
 - **T[]**: Each element is again a row of the result represented by `T`. This output binding type requires manual instantiation of the array in the function.
 
 The repo contains examples of each of these binding types [here](https://github.com/Azure/azure-functions-sql-extension/tree/main/samples/samples-csharp/OutputBindingSamples). A few examples are also included below.
 
-#### ICollector<T>/IAsyncCollector<T>
+#### ICollector&lt;T&gt;/IAsyncCollector&lt;T&gt;
 
 When using an `ICollector`, it is not necessary to instantiate it. The function can add rows to the `ICollector` directly, and its contents are automatically upserted once the function exits.
 

builds/TSAConfig.gdntsa

@@ -1,7 +1,7 @@
 {
     "codebaseName": "Sql Bindings",
     "notificationAliases": [
-        "sqltools@service.microsoft.com"
+        "sqlbindings@microsoft.com"
     ],
     "codebaseAdmins": [
         "REDMOND\\chlafren",
@@ -14,6 +14,7 @@
     "tools": [
         "BinSkim",
         "RoslynAnalyzers",
-        "CredScan"
+        "CredScan",
+        "Policheck"
     ]
 }

builds/azure-pipelines/template-steps-build-test.yml

@@ -12,6 +12,14 @@ steps:
   inputs:
     useGlobalJson: true
 
+# Run Policheck early to avoid scanning dependency folders
+- task: securedevelopmentteam.vss-secure-development-tools.build-task-policheck.PoliCheck@2
+  displayName: 'Run PoliCheck'
+  inputs:
+    targetType: F
+    result: PoliCheck.xml
+  condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
+
 - script: npm install -g azure-functions-core-tools
   displayName: 'Install Azure Functions Core Tools'
 
@@ -84,12 +92,13 @@ steps:
     arguments: 'analyze $(Build.SourcesDirectory)\src\bin\${{ parameters.configuration }}\* --recurse --verbose'
   condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
 
+# Don't run for PRs since this currently breaks on runs from forks. We run this daily ourselves anyways.
 - task: securedevelopmentteam.vss-secure-development-tools.build-task-roslynanalyzers.RoslynAnalyzers@3
   inputs:
     userProvideBuildInfo: 'autoMsBuildInfo'
   env:
     SYSTEM_ACCESSTOKEN: $(System.AccessToken)
-  condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))
+  condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'), ne(variables['Build.Reason'], 'PullRequest'))
 
 - task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@2
   inputs:
@@ -135,7 +144,7 @@ steps:
   inputs:
     GdnPublishTsaOnboard: true
     GdnPublishTsaConfigFile: '$(Build.SourcesDirectory)\builds\TSAConfig.gdntsa'
-  condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'), ne(variables['Build.Reason'], 'PullRequest'))
+  condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'), eq(variables['TSA_UPLOAD'], 'true'))
 
 # 5.0 isn't supported on Mac yet
 - task: UseDotNet@2
@@ -253,3 +262,9 @@ steps:
   displayName: 'Component Detection'
   inputs:
     failOnAlert: true
+
+- task: securedevelopmentteam.vss-secure-development-tools.build-task-postanalysis.PostAnalysis@2
+  displayName: 'Post Analysis'
+  inputs:
+    GdnBreakPolicyMinSev: Error
+  condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT'))

src/SqlAsyncCollector.cs

@@ -35,6 +35,11 @@ public PrimaryKey(string name, bool isIdentity)
             this.Name = name;
             this.IsIdentity = isIdentity;
         }
+
+        public override string ToString()
+        {
+            return this.Name;
+        }
     }
 
     /// <typeparam name="T">A user-defined POCO that represents a row of the user's table</typeparam>
@@ -305,7 +310,7 @@ private static void GenerateDataQueryForMerge(TableInformation table, IEnumerabl
                     {
                         // SQL Server allows 900 bytes per primary key, so use that as a baseline
                         var combinedPrimaryKey = new StringBuilder(900 * table.PrimaryKeys.Count());
-                        // Look up primary key of T. Because we're going in the same order of fields every time,
+                        // Look up primary key of T. Because we're going in the same order of properties every time,
                         // we can assume that if two rows with the same primary key are in the list, they will collide
                         foreach (PropertyInfo primaryKey in table.PrimaryKeys)
                         {
@@ -622,9 +627,9 @@ public static async Task<TableInformation> RetrieveTableInformationAsync(SqlConn
                     throw ex;
                 }
 
-                // Match SQL Primary Key column names to POCO field/property objects. Ensure none are missing.
+                // Match SQL Primary Key column names to POCO property objects. Ensure none are missing.
                 StringComparison comparison = caseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase;
-                IEnumerable<PropertyInfo> primaryKeyFields = typeof(T).GetProperties().Where(f => primaryKeys.Any(k => string.Equals(k.Name, f.Name, comparison)));
+                IEnumerable<PropertyInfo> primaryKeyProperties = typeof(T).GetProperties().Where(f => primaryKeys.Any(k => string.Equals(k.Name, f.Name, comparison)));
                 IEnumerable<string> primaryKeysFromObject = columnNames.Where(f => primaryKeys.Any(k => string.Equals(k.Name, f, comparison)));
                 IEnumerable<PrimaryKey> missingPrimaryKeysFromItem = primaryKeys
                     .Where(k => !primaryKeysFromObject.Contains(k.Name, comparer));
@@ -654,8 +659,8 @@ public static async Task<TableInformation> RetrieveTableInformationAsync(SqlConn
                 sqlConnProps.Add(TelemetryPropertyName.QueryType, usingInsertQuery ? "insert" : "merge");
                 sqlConnProps.Add(TelemetryPropertyName.HasIdentityColumn, hasIdentityColumnPrimaryKeys.ToString());
                 TelemetryInstance.TrackDuration(TelemetryEventName.GetTableInfoEnd, tableInfoSw.ElapsedMilliseconds, sqlConnProps, durations);
-                logger.LogDebugWithThreadId($"END RetrieveTableInformationAsync Duration={tableInfoSw.ElapsedMilliseconds}ms DB and Table: {sqlConnection.Database}.{fullName}. Primary keys: [{string.Join(",", primaryKeyFields.Select(pk => pk.Name))}]. SQL Column and Definitions:  [{string.Join(",", columnDefinitionsFromSQL)}]");
-                return new TableInformation(primaryKeyFields, columnDefinitionsFromSQL, comparer, query, hasIdentityColumnPrimaryKeys);
+                logger.LogDebugWithThreadId($"END RetrieveTableInformationAsync Duration={tableInfoSw.ElapsedMilliseconds}ms DB and Table: {sqlConnection.Database}.{fullName}. Primary keys: [{string.Join(",", primaryKeyProperties.Select(pk => pk.Name))}]. SQL Column and Definitions:  [{string.Join(",", columnDefinitionsFromSQL)}]");
+                return new TableInformation(primaryKeyProperties, columnDefinitionsFromSQL, comparer, query, hasIdentityColumnPrimaryKeys);
             }
         }
 

src/SqlAsyncEnumerable.cs

@@ -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>
@@ -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
@@ -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);
         }
 
 
@@ -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>
@@ -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);
             }
@@ -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)
                 {
-                    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>