Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1293,7 +1293,7 @@ private SqlDataReader TryFetchInputParameterEncryptionInfo(
// not present as the rpcName, as is the case with non-_batchRPCMode. So
// input parameters start at parameters[1]. parameters[0] is the actual
// T-SQL Statement. rpcName is sp_executesql.
if (_RPCList[i].systemParams.Length > 1)
if (_RPCList[i].systemParams != null && _RPCList[i].systemParams.Length > 1)
{
_RPCList[i].needsFetchParameterEncryptionMetadata = true;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
// 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 System;
using System.Data;
using System.Threading.Tasks;
using System.Collections.Generic;
using Microsoft.Data.SqlClient;
using Microsoft.Data.SqlClient.ManualTesting.Tests.AlwaysEncrypted.Setup;
using Xunit;

namespace Microsoft.Data.SqlClient.ManualTesting.Tests.AlwaysEncrypted
{
public class SqlDataAdapterBatchUpdateTests : IClassFixture<SQLSetupStrategyCertStoreProvider>
{
private readonly SQLSetupStrategy _fixture;
private readonly Dictionary<string, string> tableNames = new();

public SqlDataAdapterBatchUpdateTests(SQLSetupStrategyCertStoreProvider context)
{
_fixture = context;

// Provide table names to mirror repo patterns.
// If your fixture already exposes specific names for BuyerSeller and procs, wire them here.
// Otherwise use literal names as below.
Comment on lines +24 to +26
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The comment states "Provide table names to mirror repo patterns" but the test is creating its own hardcoded table names that don't exist in the fixture. This comment is misleading as it suggests the fixture provides these names, when it doesn't.

Consider updating the comment to clarify that these are test-specific table names that need to be created, or better yet, define these as constants or properties in the fixture.

Suggested change
// Provide table names to mirror repo patterns.
// If your fixture already exposes specific names for BuyerSeller and procs, wire them here.
// Otherwise use literal names as below.
// The following are test-specific table and procedure names required for these tests.
// The fixture does not provide these names; they must be created and managed within the test.
// For better maintainability, consider defining these as constants or exposing them via the fixture.

Copilot uses AI. Check for mistakes.
tableNames["BuyerSeller"] = "BuyerSeller";
tableNames["ProcInsertBuyerSeller"] = "InsertBuyerSeller";
tableNames["ProcUpdateBuyerSeller"] = "UpdateBuyerSeller";
Comment on lines +27 to +29
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The BuyerSeller table and associated stored procedures (InsertBuyerSeller, UpdateBuyerSeller) referenced in this test are not created by the SQLSetupStrategyCertStoreProvider fixture.

The fixture's CreateTables method (defined in the base SQLSetupStrategy class) does not include the creation of these database objects, which means these tests will fail when executed because the table and stored procedures don't exist in the test database.

To fix this, you should either:

  1. Add the BuyerSeller table and stored procedure creation to the SQLSetupStrategyCertStoreProvider.SetupDatabase() method or the base CreateTables() method, similar to how other test tables like ApiTestTable are created.
  2. Create a dedicated table class like BuyerSellerTable in the AlwaysEncrypted/TestFixtures/Setup/ directory that implements the ICreatable and IDroppable interfaces.

Copilot uses AI. Check for mistakes.
}

// ---------- TESTS ----------

[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsTargetReadyForAeWithKeyStore))]
[ClassData(typeof(AEConnectionStringProvider))]
public async Task dapterUpdate_BatchSizeGreaterThanOne_Succeeds(string connectionString)
{
// Arrange
// Ensure baseline rows exist
TruncateTables("BuyerSeller", connectionString);
PopulateTable("BuyerSeller", new (int id, string s1, string s2)[] {
(1, "123-45-6789", "987-65-4321"),
(2, "234-56-7890", "876-54-3210"),
(3, "345-67-8901", "765-43-2109"),
(4, "456-78-9012", "654-32-1098"),
}, connectionString);

using var conn = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
await conn.OpenAsync();

using var adapter = CreateAdapter(conn, updateBatchSize: 10); // failure repro: > 1
var dataTable = BuildBuyerSellerDataTable();
LoadCurrentRowsIntoDataTable(dataTable, conn);

// Mutate values for update
MutateForUpdate(dataTable);

// Act - This is where NullReferenceException was being thrown previously(which is now fixed)
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Missing space after 'previously' in the comment.

Suggested change
// Act - This is where NullReferenceException was being thrown previously(which is now fixed)
// Act - This is where NullReferenceException was being thrown previously (which is now fixed)

Copilot uses AI. Check for mistakes.
var updated = await Task.Run(() => adapter.Update(dataTable));

// Assert
Assert.Equal(dataTable.Rows.Count, updated);

}

[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsTargetReadyForAeWithKeyStore))]
[ClassData(typeof(AEConnectionStringProvider))]
public async Task AdapterUpdate_BatchSizeOne_Succeeds(string connectionString)
{
// Arrange
TruncateTables("BuyerSeller", connectionString);
PopulateTable("BuyerSeller", new (int id, string s1, string s2)[] {
(1, "123-45-6789", "987-65-4321"),
(2, "234-56-7890", "876-54-3210"),
(3, "345-67-8901", "765-43-2109"),
(4, "456-78-9012", "654-32-1098"),
}, connectionString);

using var conn = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
await conn.OpenAsync();

using var adapter = CreateAdapter(conn, updateBatchSize: 1); // success path
var dataTable = BuildBuyerSellerDataTable();
LoadCurrentRowsIntoDataTable(dataTable, conn);

MutateForUpdate(dataTable);

// Act (should not throw)
var updatedRows = await Task.Run(() => adapter.Update(dataTable));

// Assert
Assert.Equal(dataTable.Rows.Count, updatedRows);
Comment on lines +88 to +92
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The test does not verify that the data was actually updated in the database after the adapter.Update() call. It only checks that the number of rows updated matches the expected count, but doesn't confirm that the values were correctly persisted.

Consider adding an assertion that queries the database after the update and verifies the actual values match the expected mutated values.

Copilot uses AI. Check for mistakes.

}

// ---------- HELPERS ----------

private SqlDataAdapter CreateAdapter(SqlConnection connection, int updateBatchSize)
{
// Insert
var insertCmd = new SqlCommand(tableNames["ProcInsertBuyerSeller"], connection)
{
CommandType = CommandType.StoredProcedure
};
insertCmd.Parameters.AddRange(new[]
{
new SqlParameter("@BuyerSellerID", SqlDbType.Int) { SourceColumn = "BuyerSellerID" },
new SqlParameter("@SSN1", SqlDbType.VarChar, 255) { SourceColumn = "SSN1" },
new SqlParameter("@SSN2", SqlDbType.VarChar, 255) { SourceColumn = "SSN2" },
});
insertCmd.UpdatedRowSource = UpdateRowSource.None;

// Update
var updateCmd = new SqlCommand(tableNames["ProcUpdateBuyerSeller"], connection)
{
CommandType = CommandType.StoredProcedure
};
updateCmd.Parameters.AddRange(new[]
{
new SqlParameter("@BuyerSellerID", SqlDbType.Int) { SourceColumn = "BuyerSellerID" },
new SqlParameter("@SSN1", SqlDbType.VarChar, 255) { SourceColumn = "SSN1" },
new SqlParameter("@SSN2", SqlDbType.VarChar, 255) { SourceColumn = "SSN2" },
});
updateCmd.UpdatedRowSource = UpdateRowSource.None;

return new SqlDataAdapter
{
InsertCommand = insertCmd,
UpdateCommand = updateCmd,
UpdateBatchSize = updateBatchSize
};
}

private DataTable BuildBuyerSellerDataTable()
{
var dt = new DataTable(tableNames["BuyerSeller"]);
dt.Columns.AddRange(new[]
{
new DataColumn("BuyerSellerID", typeof(int)),
new DataColumn("SSN1", typeof(string)),
new DataColumn("SSN2", typeof(string)),
});
dt.PrimaryKey = new[] { dt.Columns["BuyerSellerID"] };
return dt;
}

private void LoadCurrentRowsIntoDataTable(DataTable dt, SqlConnection conn)
{
using var cmd = new SqlCommand($"SELECT BuyerSellerID, SSN1, SSN2 FROM [dbo].[{tableNames["BuyerSeller"]}] ORDER BY BuyerSellerID", conn);
using var reader = cmd.ExecuteReader();
while (reader.Read())
{
dt.Rows.Add(reader.GetInt32(0), reader.GetString(1), reader.GetString(2));
}
}

private void MutateForUpdate(DataTable dt)
{
int i = 0;
foreach (DataRow row in dt.Rows)
{
i++;
row["SSN1"] = $"{i:000}-11-{DateTime.Now:HHmm}";
row["SSN2"] = $"{i:000}-22-{DateTime.Now:HHmm}";
}
}

internal void TruncateTables(string tableName, string connectionString)
{
using var connection = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
connection.Open();
SilentRunCommand($@"TRUNCATE TABLE [dbo].[{tableNames[tableName]}]", connection);
}

internal void ExecuteQuery(SqlConnection connection, string commandText)
{
// Mirror AE-enabled command execution style used in repo tests
using var cmd = new SqlCommand(
commandText,
connection: connection,
transaction: null,
columnEncryptionSetting: SqlCommandColumnEncryptionSetting.Enabled);
cmd.ExecuteNonQuery();
}

internal void PopulateTable(string tableName, (int id, string s1, string s2)[] rows, string connectionString)
{
using var connection = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
connection.Open();

foreach (var (id, s1, s2) in rows)
{
ExecuteQuery(connection,
$@"INSERT INTO [dbo].[{tableNames[tableName]}] (BuyerSellerID, SSN1, SSN2) VALUES ({id}, '{s1}', '{s2}')");
}
}

public string GetOpenConnectionString(string baseConnectionString, bool encryptionEnabled)
{
var builder = new SqlConnectionStringBuilder(baseConnectionString)
{
// TrustServerCertificate can be set based on environment; mirror repo’s AE toggling idiom
ColumnEncryptionSetting = encryptionEnabled
? SqlConnectionColumnEncryptionSetting.Enabled
: SqlConnectionColumnEncryptionSetting.Disabled
};
return builder.ToString();
}

internal void SilentRunCommand(string commandText, SqlConnection connection)
{
try
{ ExecuteQuery(connection, commandText); }
catch (SqlException) { /* Swallow for cleanup */ }
}

public void Dispose()
{
foreach (string connectionString in DataTestUtility.AEConnStringsSetup)
{
using var connection = new SqlConnection(GetOpenConnectionString(connectionString, encryptionEnabled: true));
connection.Open();
using var cmd = new SqlCommand($"DELETE FROM [dbo].[{tableNames["BuyerSeller"]}]", connection);
cmd.ExecuteNonQuery();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@
<Compile Include="SQL\ConnectionPoolTest\ConnectionPoolTest.Debug.cs" />
</ItemGroup>
<ItemGroup>
<Compile Include="AlwaysEncrypted\SqlDataAdapterBatchUpdateTests.cs" />
<Compile Include="DataCommon\AADUtility.cs" />
<Compile Include="DataCommon\AssemblyResourceManager.cs" />
<Compile Include="DataCommon\ConnectionTestParameters.cs" />
Expand Down
Loading