From 650e8f95ef2e7b37b218c3a2fbeffb8c4bf81e75 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Sat, 22 Apr 2023 00:23:43 +0530 Subject: [PATCH 01/14] enclosing sql db query executions within a transaction --- src/Service/Resolvers/MsSqlQueryBuilder.cs | 8 +- src/Service/Resolvers/SqlMutationEngine.cs | 142 ++++++++++++--------- src/Service/Resolvers/SqlQueryEngine.cs | 37 ++++-- 3 files changed, 114 insertions(+), 73 deletions(-) diff --git a/src/Service/Resolvers/MsSqlQueryBuilder.cs b/src/Service/Resolvers/MsSqlQueryBuilder.cs index 7f7a62a66c..617fa4e808 100644 --- a/src/Service/Resolvers/MsSqlQueryBuilder.cs +++ b/src/Service/Resolvers/MsSqlQueryBuilder.cs @@ -120,16 +120,14 @@ public string Build(SqlUpsertQueryStructure structure) } else { - return $"SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;BEGIN TRANSACTION; UPDATE {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} " + - $"WITH(UPDLOCK) SET {Build(structure.UpdateOperations, ", ")} " + + return $"UPDATE {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} " + + $"SET {Build(structure.UpdateOperations, ", ")} " + $"OUTPUT {MakeOutputColumns(structure.OutputColumns, OutputQualifier.Inserted)} " + $"WHERE {predicates} " + $"IF @@ROWCOUNT = 0 " + - $"BEGIN; " + $"INSERT INTO {QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)} ({Build(structure.InsertColumns)}) " + $"OUTPUT {MakeOutputColumns(structure.OutputColumns, OutputQualifier.Inserted)} " + - $"VALUES ({string.Join(", ", structure.Values)}) " + - $"END; COMMIT TRANSACTION"; + $"VALUES ({string.Join(", ", structure.Values)});"; } } diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 920b82c557..36bf8c6fca 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -11,6 +11,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Threading.Tasks; +using System.Transactions; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Service.Authorization; @@ -91,50 +92,55 @@ public SqlMutationEngine( // If authorization fails, an exception will be thrown and request execution halts. AuthorizeMutationFields(context, parameters, entityName, mutationOperation); - if (mutationOperation is Config.Operation.Delete) + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) { - // compute the mutation result before removing the element, - // since typical GraphQL delete mutations return the metadata of the deleted item. - result = await _queryEngine.ExecuteAsync(context, GetBackingColumnsFromCollection(entityName, parameters)); - - Dictionary? resultProperties = - await PerformDeleteOperation( - entityName, - parameters); - - // If the number of records affected by DELETE were zero, - // and yet the result was not null previously, it indicates this DELETE lost - // a concurrent request race. Hence, empty the non-null result. - if (resultProperties is not null - && resultProperties.TryGetValue(nameof(DbDataReader.RecordsAffected), out object? value) - && Convert.ToInt32(value) == 0 - && result is not null && result.Item1 is not null) + if (mutationOperation is Config.Operation.Delete) { - result = new Tuple( - default(JsonDocument), - PaginationMetadata.MakeEmptyPaginationMetadata()); - } - } - else - { - DbResultSetRow? mutationResultRow = - await PerformMutationOperation( - entityName, - mutationOperation, - parameters, - context); + // compute the mutation result before removing the element, + // since typical GraphQL delete mutations return the metadata of the deleted item. + result = await _queryEngine.ExecuteAsync(context, GetBackingColumnsFromCollection(entityName, parameters)); - if (mutationResultRow is not null && mutationResultRow.Columns.Count > 0 - && !context.Selection.Type.IsScalarType()) + Dictionary? resultProperties = + await PerformDeleteOperation( + entityName, + parameters); + + // If the number of records affected by DELETE were zero, + // and yet the result was not null previously, it indicates this DELETE lost + // a concurrent request race. Hence, empty the non-null result. + if (resultProperties is not null + && resultProperties.TryGetValue(nameof(DbDataReader.RecordsAffected), out object? value) + && Convert.ToInt32(value) == 0 + && result is not null && result.Item1 is not null) + { + result = new Tuple( + default(JsonDocument), + PaginationMetadata.MakeEmptyPaginationMetadata()); + } + } + else { - // Because the GraphQL mutation result set columns were exposed (mapped) column names, - // the column names must be converted to backing (source) column names so the - // PrimaryKeyPredicates created in the SqlQueryStructure created by the query engine - // represent database column names. - result = await _queryEngine.ExecuteAsync( - context, - GetBackingColumnsFromCollection(entityName, mutationResultRow.Columns)); + DbResultSetRow? mutationResultRow = + await PerformMutationOperation( + entityName, + mutationOperation, + parameters, + context); + + if (mutationResultRow is not null && mutationResultRow.Columns.Count > 0 + && !context.Selection.Type.IsScalarType()) + { + // Because the GraphQL mutation result set columns were exposed (mapped) column names, + // the column names must be converted to backing (source) column names so the + // PrimaryKeyPredicates created in the SqlQueryStructure created by the query engine + // represent database column names. + result = await _queryEngine.ExecuteAsync( + context, + GetBackingColumnsFromCollection(entityName, mutationResultRow.Columns)); + } } + + transactionScope.Complete(); } if (result is null) @@ -191,12 +197,19 @@ await PerformMutationOperation( context.ResolvedParameters); string queryText = _queryBuilder.Build(executeQueryStructure); - JsonArray? resultArray = - await _queryExecutor.ExecuteQueryAsync( - queryText, - executeQueryStructure.Parameters, - _queryExecutor.GetJsonArrayAsync, - GetHttpContext()); + JsonArray? resultArray = null; + + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + resultArray = + await _queryExecutor.ExecuteQueryAsync( + queryText, + executeQueryStructure.Parameters, + _queryExecutor.GetJsonArrayAsync, + GetHttpContext()); + + transactionScope.Complete(); + } // A note on returning stored procedure results: // We can't infer what the stored procedure actually did beyond the HasRows and RecordsAffected attributes @@ -271,10 +284,14 @@ await _queryExecutor.ExecuteQueryAsync( if (context.OperationType is Config.Operation.Delete) { - Dictionary? resultProperties = - await PerformDeleteOperation( - context.EntityName, - parameters); + Dictionary? resultProperties = null; + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + resultProperties = await PerformDeleteOperation( + context.EntityName, + parameters); + transactionScope.Complete(); + } // Records affected tells us that item was successfully deleted. // No records affected happens for a DELETE request on nonexistent object @@ -287,10 +304,14 @@ await PerformDeleteOperation( } else if (context.OperationType is Config.Operation.Upsert || context.OperationType is Config.Operation.UpsertIncremental) { - DbResultSet? upsertOperationResult = - await PerformUpsertOperation( - parameters, - context); + DbResultSet? upsertOperationResult = null; + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + upsertOperationResult = await PerformUpsertOperation( + parameters, + context); + transactionScope.Complete(); + } DbResultSetRow? dbResultSetRow = upsertOperationResult is not null ? (upsertOperationResult.Rows.FirstOrDefault() ?? new()) : null; @@ -323,11 +344,16 @@ await PerformUpsertOperation( } else { - DbResultSetRow? mutationResultRow = - await PerformMutationOperation( - context.EntityName, - context.OperationType, - parameters); + DbResultSetRow? mutationResultRow = null; + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + mutationResultRow = + await PerformMutationOperation( + context.EntityName, + context.OperationType, + parameters); + transactionScope.Complete(); + } if (context.OperationType is Config.Operation.Insert) { diff --git a/src/Service/Resolvers/SqlQueryEngine.cs b/src/Service/Resolvers/SqlQueryEngine.cs index b3c4fe043b..3cc6e4f348 100644 --- a/src/Service/Resolvers/SqlQueryEngine.cs +++ b/src/Service/Resolvers/SqlQueryEngine.cs @@ -7,6 +7,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using System.Threading.Tasks; +using System.Transactions; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Service.Configurations; using Azure.DataApiBuilder.Service.Models; @@ -119,12 +120,17 @@ await ExecuteAsync(structure), _gQLFilterParser); string queryString = _queryBuilder.Build(structure); - List? jsonListResult = - await _queryExecutor.ExecuteQueryAsync( + List? jsonListResult = null; + + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + jsonListResult = await _queryExecutor.ExecuteQueryAsync( queryString, structure.Parameters, _queryExecutor.GetJsonResultAsync>, _httpContextAccessor.HttpContext!); + transactionScope.Complete(); + } if (jsonListResult is null) { @@ -300,12 +306,18 @@ public OkObjectResult OkResponse(JsonElement jsonResult) { // Open connection and execute query using _queryExecutor string queryString = _queryBuilder.Build(structure); - JsonDocument? jsonDocument = - await _queryExecutor.ExecuteQueryAsync( + JsonDocument? jsonDocument = null; + + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + jsonDocument = await _queryExecutor.ExecuteQueryAsync( queryString, structure.Parameters, _queryExecutor.GetJsonResultAsync, _httpContextAccessor.HttpContext!); + transactionScope.Complete(); + } + return jsonDocument; } @@ -318,12 +330,17 @@ await _queryExecutor.ExecuteQueryAsync( { string queryString = _queryBuilder.Build(structure); - JsonArray? resultArray = - await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonArrayAsync, - _httpContextAccessor.HttpContext!); + JsonArray? resultArray = null; + + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + resultArray = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonArrayAsync, + _httpContextAccessor.HttpContext!); + transactionScope.Complete(); + } JsonDocument? jsonDocument = null; From f327c62f56c408eec669e85c406f76d74b6f1334 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Sun, 23 Apr 2023 01:19:09 +0530 Subject: [PATCH 02/14] catching transaction related exceptions --- src/Service/Resolvers/SqlMutationEngine.cs | 190 +++++++++++++-------- src/Service/Resolvers/SqlQueryEngine.cs | 77 ++++++--- 2 files changed, 179 insertions(+), 88 deletions(-) diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 36bf8c6fca..eba9bc148d 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -41,6 +41,7 @@ public class SqlMutationEngine : IMutationEngine private readonly IHttpContextAccessor _httpContextAccessor; private readonly GQLFilterParser _gQLFilterParser; public const string IS_FIRST_RESULT_SET = "IsFirstResultSet"; + private const string TRANSACTION_EXCEPTION_ERROR_MSG = "An unexpected error occured"; /// /// Constructor @@ -92,55 +93,65 @@ public SqlMutationEngine( // If authorization fails, an exception will be thrown and request execution halts. AuthorizeMutationFields(context, parameters, entityName, mutationOperation); - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + try { - if (mutationOperation is Config.Operation.Delete) + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) { - // compute the mutation result before removing the element, - // since typical GraphQL delete mutations return the metadata of the deleted item. - result = await _queryEngine.ExecuteAsync(context, GetBackingColumnsFromCollection(entityName, parameters)); - - Dictionary? resultProperties = - await PerformDeleteOperation( - entityName, - parameters); - - // If the number of records affected by DELETE were zero, - // and yet the result was not null previously, it indicates this DELETE lost - // a concurrent request race. Hence, empty the non-null result. - if (resultProperties is not null - && resultProperties.TryGetValue(nameof(DbDataReader.RecordsAffected), out object? value) - && Convert.ToInt32(value) == 0 - && result is not null && result.Item1 is not null) + if (mutationOperation is Config.Operation.Delete) { - result = new Tuple( - default(JsonDocument), - PaginationMetadata.MakeEmptyPaginationMetadata()); - } - } - else - { - DbResultSetRow? mutationResultRow = - await PerformMutationOperation( - entityName, - mutationOperation, - parameters, - context); + // compute the mutation result before removing the element, + // since typical GraphQL delete mutations return the metadata of the deleted item. + result = await _queryEngine.ExecuteAsync(context, GetBackingColumnsFromCollection(entityName, parameters)); - if (mutationResultRow is not null && mutationResultRow.Columns.Count > 0 - && !context.Selection.Type.IsScalarType()) + Dictionary? resultProperties = + await PerformDeleteOperation( + entityName, + parameters); + + // If the number of records affected by DELETE were zero, + // and yet the result was not null previously, it indicates this DELETE lost + // a concurrent request race. Hence, empty the non-null result. + if (resultProperties is not null + && resultProperties.TryGetValue(nameof(DbDataReader.RecordsAffected), out object? value) + && Convert.ToInt32(value) == 0 + && result is not null && result.Item1 is not null) + { + result = new Tuple( + default(JsonDocument), + PaginationMetadata.MakeEmptyPaginationMetadata()); + } + } + else { - // Because the GraphQL mutation result set columns were exposed (mapped) column names, - // the column names must be converted to backing (source) column names so the - // PrimaryKeyPredicates created in the SqlQueryStructure created by the query engine - // represent database column names. - result = await _queryEngine.ExecuteAsync( - context, - GetBackingColumnsFromCollection(entityName, mutationResultRow.Columns)); + DbResultSetRow? mutationResultRow = + await PerformMutationOperation( + entityName, + mutationOperation, + parameters, + context); + + if (mutationResultRow is not null && mutationResultRow.Columns.Count > 0 + && !context.Selection.Type.IsScalarType()) + { + // Because the GraphQL mutation result set columns were exposed (mapped) column names, + // the column names must be converted to backing (source) column names so the + // PrimaryKeyPredicates created in the SqlQueryStructure created by the query engine + // represent database column names. + result = await _queryEngine.ExecuteAsync( + context, + GetBackingColumnsFromCollection(entityName, mutationResultRow.Columns)); + } } - } - transactionScope.Complete(); + transactionScope.Complete(); + } + } + catch (TransactionException) + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); } if (result is null) @@ -199,16 +210,26 @@ await PerformMutationOperation( JsonArray? resultArray = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + try { - resultArray = - await _queryExecutor.ExecuteQueryAsync( - queryText, - executeQueryStructure.Parameters, - _queryExecutor.GetJsonArrayAsync, - GetHttpContext()); + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + resultArray = + await _queryExecutor.ExecuteQueryAsync( + queryText, + executeQueryStructure.Parameters, + _queryExecutor.GetJsonArrayAsync, + GetHttpContext()); - transactionScope.Complete(); + transactionScope.Complete(); + } + } + catch (TransactionException) + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); } // A note on returning stored procedure results: @@ -285,12 +306,24 @@ await _queryExecutor.ExecuteQueryAsync( if (context.OperationType is Config.Operation.Delete) { Dictionary? resultProperties = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + + try { - resultProperties = await PerformDeleteOperation( - context.EntityName, - parameters); - transactionScope.Complete(); + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + resultProperties = await PerformDeleteOperation( + context.EntityName, + parameters); + transactionScope.Complete(); + } + } + catch (TransactionException) + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError + ); } // Records affected tells us that item was successfully deleted. @@ -305,12 +338,24 @@ await _queryExecutor.ExecuteQueryAsync( else if (context.OperationType is Config.Operation.Upsert || context.OperationType is Config.Operation.UpsertIncremental) { DbResultSet? upsertOperationResult = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + + try { - upsertOperationResult = await PerformUpsertOperation( - parameters, - context); - transactionScope.Complete(); + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + upsertOperationResult = await PerformUpsertOperation( + parameters, + context); + transactionScope.Complete(); + } + } + catch (TransactionException) + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError + ); } DbResultSetRow? dbResultSetRow = upsertOperationResult is not null ? @@ -345,14 +390,25 @@ await _queryExecutor.ExecuteQueryAsync( else { DbResultSetRow? mutationResultRow = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + + try { - mutationResultRow = - await PerformMutationOperation( - context.EntityName, - context.OperationType, - parameters); - transactionScope.Complete(); + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + mutationResultRow = + await PerformMutationOperation( + context.EntityName, + context.OperationType, + parameters); + transactionScope.Complete(); + } + } + catch (TransactionException) + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); } if (context.OperationType is Config.Operation.Insert) diff --git a/src/Service/Resolvers/SqlQueryEngine.cs b/src/Service/Resolvers/SqlQueryEngine.cs index 3cc6e4f348..3eed74a28d 100644 --- a/src/Service/Resolvers/SqlQueryEngine.cs +++ b/src/Service/Resolvers/SqlQueryEngine.cs @@ -4,12 +4,14 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net; using System.Text.Json; using System.Text.Json.Nodes; using System.Threading.Tasks; using System.Transactions; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Service.Configurations; +using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.Models; using Azure.DataApiBuilder.Service.Services; using HotChocolate.Resolvers; @@ -35,6 +37,7 @@ public class SqlQueryEngine : IQueryEngine private readonly ILogger _logger; private readonly RuntimeConfigProvider _runtimeConfigProvider; private readonly GQLFilterParser _gQLFilterParser; + private const string TRANSACTION_EXCEPTION_ERROR_MSG = "An unexpected error occured"; // // Constructor. @@ -122,14 +125,25 @@ await ExecuteAsync(structure), string queryString = _queryBuilder.Build(structure); List? jsonListResult = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + try { - jsonListResult = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonResultAsync>, - _httpContextAccessor.HttpContext!); - transactionScope.Complete(); + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + jsonListResult = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonResultAsync>, + _httpContextAccessor.HttpContext!); + transactionScope.Complete(); + } + } + catch (TransactionException) + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError + ); } if (jsonListResult is null) @@ -308,14 +322,25 @@ public OkObjectResult OkResponse(JsonElement jsonResult) string queryString = _queryBuilder.Build(structure); JsonDocument? jsonDocument = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + try + { + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + jsonDocument = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonResultAsync, + _httpContextAccessor.HttpContext!); + transactionScope.Complete(); + } + } + catch (TransactionException) { - jsonDocument = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonResultAsync, - _httpContextAccessor.HttpContext!); - transactionScope.Complete(); + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError + ); } return jsonDocument; @@ -332,14 +357,24 @@ public OkObjectResult OkResponse(JsonElement jsonResult) JsonArray? resultArray = null; - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + try + { + using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + { + resultArray = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonArrayAsync, + _httpContextAccessor.HttpContext!); + transactionScope.Complete(); + } + } + catch (TransactionException) { - resultArray = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonArrayAsync, - _httpContextAccessor.HttpContext!); - transactionScope.Complete(); + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); } JsonDocument? jsonDocument = null; From c962eefe3f760fc1a2c21ba0642dd2039d46b818 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Mon, 24 Apr 2023 15:40:19 +0530 Subject: [PATCH 03/14] using read commited isolation level, adds parallel update test --- .../GraphQLMutationTestBase.cs | 37 +++++++++++++++++ src/Service/Resolvers/SqlMutationEngine.cs | 40 ++++++++++++++++--- src/Service/Resolvers/SqlQueryEngine.cs | 24 +++++++++-- 3 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs index 194aa00504..d06b722f79 100644 --- a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs @@ -677,6 +677,43 @@ public async Task DeleteMutationWithVariablesAndMappings(string dbQuery, string Assert.AreEqual(result.RootElement.GetProperty("count").GetInt64(), 0); } + /// + /// Performs concurrent mutation requests on the same item and validates that consistent response + /// is returned to each of the mutations + /// gQLMutationCyan : Updates the color column of Notebook table to cyan + /// gQLMutationMagenta : Updates the color column of Notebook table to magenta + /// The color field in the responses returned for each of the mutations should be + /// the same value it had written to the table. + /// + [TestMethod] + public async Task TestParallelUpdateMutations() + { + string graphQLMutationName = "updateNotebook"; + string gQLMutationCyan = @" + mutation { + updateNotebook(id: 3, item: { color: ""cyan""} ) { + color + } + } + "; + + string gQLMutationMagenta = @" + mutation { + updateNotebook(id: 3, item: { color: ""magenta""} ) { + color + } + } + "; + + Task cyanResponseTask = ExecuteGraphQLRequestAsync(gQLMutationCyan, graphQLMutationName, isAuthenticated: true); + Task magentaResponseTask = ExecuteGraphQLRequestAsync(gQLMutationMagenta, graphQLMutationName, isAuthenticated: true); + + JsonElement cyanResponse = await cyanResponseTask; + Assert.AreEqual("{\"color\":\"cyan\"}", cyanResponse.ToString()); + JsonElement magentaResponse = await magentaResponseTask; + Assert.AreEqual("{\"color\":\"magenta\"}", magentaResponse.ToString()); + } + #endregion #region Negative Tests diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index eba9bc148d..dba11d0ec1 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -95,7 +95,13 @@ public SqlMutationEngine( try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { if (mutationOperation is Config.Operation.Delete) { @@ -212,7 +218,13 @@ await PerformMutationOperation( try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { resultArray = await _queryExecutor.ExecuteQueryAsync( @@ -309,7 +321,13 @@ await _queryExecutor.ExecuteQueryAsync( try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { resultProperties = await PerformDeleteOperation( context.EntityName, @@ -341,7 +359,13 @@ await _queryExecutor.ExecuteQueryAsync( try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { upsertOperationResult = await PerformUpsertOperation( parameters, @@ -393,7 +417,13 @@ await _queryExecutor.ExecuteQueryAsync( try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { mutationResultRow = await PerformMutationOperation( diff --git a/src/Service/Resolvers/SqlQueryEngine.cs b/src/Service/Resolvers/SqlQueryEngine.cs index 3eed74a28d..f48fad6203 100644 --- a/src/Service/Resolvers/SqlQueryEngine.cs +++ b/src/Service/Resolvers/SqlQueryEngine.cs @@ -127,7 +127,13 @@ await ExecuteAsync(structure), try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { jsonListResult = await _queryExecutor.ExecuteQueryAsync( queryString, @@ -324,7 +330,13 @@ public OkObjectResult OkResponse(JsonElement jsonResult) try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { jsonDocument = await _queryExecutor.ExecuteQueryAsync( queryString, @@ -359,7 +371,13 @@ public OkObjectResult OkResponse(JsonElement jsonResult) try { - using (TransactionScope transactionScope = new(TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new( + TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { resultArray = await _queryExecutor.ExecuteQueryAsync( queryString, From 3de120c1e1059305f75cc8cffd4bb0c97973c599 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Mon, 24 Apr 2023 16:12:18 +0530 Subject: [PATCH 04/14] updates test to run against book entity --- .../GraphQLMutationTestBase.cs | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs index d06b722f79..48f359f289 100644 --- a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs @@ -678,40 +678,40 @@ public async Task DeleteMutationWithVariablesAndMappings(string dbQuery, string } /// - /// Performs concurrent mutation requests on the same item and validates that consistent response - /// is returned to each of the mutations - /// gQLMutationCyan : Updates the color column of Notebook table to cyan - /// gQLMutationMagenta : Updates the color column of Notebook table to magenta - /// The color field in the responses returned for each of the mutations should be + /// Performs concurrent mutation requests on the same item and validates that the responses + /// returned are consistent + /// gQLMutation1 : Updates the title column of Book table to New Title + /// gQLMutation2 : Updates the title column of Book table to Updated Title + /// The title field in the responses returned for each of the mutations should be /// the same value it had written to the table. /// [TestMethod] public async Task TestParallelUpdateMutations() { - string graphQLMutationName = "updateNotebook"; - string gQLMutationCyan = @" + string graphQLMutationName = "updatebook"; + string gQLMutation1 = @" mutation { - updateNotebook(id: 3, item: { color: ""cyan""} ) { - color + updatebook(id : 1, item: { title: ""New Title"" }) + { + title } - } - "; + }"; - string gQLMutationMagenta = @" + string gQLMutation2 = @" mutation { - updateNotebook(id: 3, item: { color: ""magenta""} ) { - color + updatebook(id : 1, item: { title: ""Updated Title"" }) + { + title } - } - "; + }"; - Task cyanResponseTask = ExecuteGraphQLRequestAsync(gQLMutationCyan, graphQLMutationName, isAuthenticated: true); - Task magentaResponseTask = ExecuteGraphQLRequestAsync(gQLMutationMagenta, graphQLMutationName, isAuthenticated: true); + Task responeTask1 = ExecuteGraphQLRequestAsync(gQLMutation1, graphQLMutationName, isAuthenticated: true); + Task responseTask2 = ExecuteGraphQLRequestAsync(gQLMutation2, graphQLMutationName, isAuthenticated: true); - JsonElement cyanResponse = await cyanResponseTask; - Assert.AreEqual("{\"color\":\"cyan\"}", cyanResponse.ToString()); - JsonElement magentaResponse = await magentaResponseTask; - Assert.AreEqual("{\"color\":\"magenta\"}", magentaResponse.ToString()); + JsonElement response1 = await responeTask1; + Assert.AreEqual("{\"title\":\"New Title\"}", response1.ToString()); + JsonElement response2 = await responseTask2; + Assert.AreEqual("{\"title\":\"Updated Title\"}", response2.ToString()); } #endregion From c5188adfb29f6ecad2b181a91f6ec057c88c5774 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Tue, 25 Apr 2023 12:30:28 +0530 Subject: [PATCH 05/14] removing transactionscope for select query execution --- src/Service/Resolvers/SqlQueryEngine.cs | 96 ++++--------------------- 1 file changed, 15 insertions(+), 81 deletions(-) diff --git a/src/Service/Resolvers/SqlQueryEngine.cs b/src/Service/Resolvers/SqlQueryEngine.cs index f48fad6203..5e293aacf1 100644 --- a/src/Service/Resolvers/SqlQueryEngine.cs +++ b/src/Service/Resolvers/SqlQueryEngine.cs @@ -4,14 +4,11 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Net; using System.Text.Json; using System.Text.Json.Nodes; using System.Threading.Tasks; -using System.Transactions; using Azure.DataApiBuilder.Auth; using Azure.DataApiBuilder.Service.Configurations; -using Azure.DataApiBuilder.Service.Exceptions; using Azure.DataApiBuilder.Service.Models; using Azure.DataApiBuilder.Service.Services; using HotChocolate.Resolvers; @@ -37,7 +34,6 @@ public class SqlQueryEngine : IQueryEngine private readonly ILogger _logger; private readonly RuntimeConfigProvider _runtimeConfigProvider; private readonly GQLFilterParser _gQLFilterParser; - private const string TRANSACTION_EXCEPTION_ERROR_MSG = "An unexpected error occured"; // // Constructor. @@ -125,32 +121,11 @@ await ExecuteAsync(structure), string queryString = _queryBuilder.Build(structure); List? jsonListResult = null; - try - { - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) - { - jsonListResult = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonResultAsync>, - _httpContextAccessor.HttpContext!); - transactionScope.Complete(); - } - } - catch (TransactionException) - { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError - ); - } + jsonListResult = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonResultAsync>, + _httpContextAccessor.HttpContext!); if (jsonListResult is null) { @@ -328,32 +303,11 @@ public OkObjectResult OkResponse(JsonElement jsonResult) string queryString = _queryBuilder.Build(structure); JsonDocument? jsonDocument = null; - try - { - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) - { - jsonDocument = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonResultAsync, - _httpContextAccessor.HttpContext!); - transactionScope.Complete(); - } - } - catch (TransactionException) - { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError - ); - } + jsonDocument = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonResultAsync, + _httpContextAccessor.HttpContext!); return jsonDocument; } @@ -369,31 +323,11 @@ public OkObjectResult OkResponse(JsonElement jsonResult) JsonArray? resultArray = null; - try - { - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) - { - resultArray = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonArrayAsync, - _httpContextAccessor.HttpContext!); - transactionScope.Complete(); - } - } - catch (TransactionException) - { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); - } + resultArray = await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonArrayAsync, + _httpContextAccessor.HttpContext!); JsonDocument? jsonDocument = null; From 84ca839465a30077ad766122f9bade7b9adfc95d Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Tue, 25 Apr 2023 15:39:27 +0530 Subject: [PATCH 06/14] adds concurrent tests --- .../GraphQLMutationTestBase.cs | 147 +++++++++++++++++- src/Service/Resolvers/SqlQueryEngine.cs | 32 ++-- 2 files changed, 159 insertions(+), 20 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs index 48f359f289..095cafa1a4 100644 --- a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Linq; using System.Text.Json; using System.Threading.Tasks; @@ -678,7 +679,7 @@ public async Task DeleteMutationWithVariablesAndMappings(string dbQuery, string } /// - /// Performs concurrent mutation requests on the same item and validates that the responses + /// Performs concurrent update mutations on the same item and validates that the responses /// returned are consistent /// gQLMutation1 : Updates the title column of Book table to New Title /// gQLMutation2 : Updates the title column of Book table to Updated Title @@ -709,11 +710,153 @@ public async Task TestParallelUpdateMutations() Task responseTask2 = ExecuteGraphQLRequestAsync(gQLMutation2, graphQLMutationName, isAuthenticated: true); JsonElement response1 = await responeTask1; - Assert.AreEqual("{\"title\":\"New Title\"}", response1.ToString()); JsonElement response2 = await responseTask2; + + Assert.AreEqual("{\"title\":\"New Title\"}", response1.ToString()); Assert.AreEqual("{\"title\":\"Updated Title\"}", response2.ToString()); } + /// + /// Performs concurrent insert mutation on a table where the PK is auto-generated. + /// Since, PK is auto-generated, essentially both the mutations are operating on + /// different items. Therefore, both the mutations should succeed. + /// + [TestMethod] + public async Task TestParallelInsertMutationPKAutoGenerated() + { + string graphQLMutationName = "createbook"; + string graphQLMutation1 = @" + mutation { + createbook(item: { title: ""Awesome Book"", publisher_id: 1234 }) { + title + } + } + "; + + string graphQLMutation2 = @" + mutation { + createbook(item: { title: ""Another Awesome Book"", publisher_id: 1234 }) { + title + } + } + "; + + Task responeTask1 = ExecuteGraphQLRequestAsync(graphQLMutation1, graphQLMutationName, isAuthenticated: true); + Task responseTask2 = ExecuteGraphQLRequestAsync(graphQLMutation2, graphQLMutationName, isAuthenticated: true); + + JsonElement response1 = await responeTask1; + JsonElement response2 = await responseTask2; + + Assert.AreEqual("{\"title\":\"Awesome Book\"}", response1.ToString()); + Assert.AreEqual("{\"title\":\"Another Awesome Book\"}", response2.ToString()); + + } + + /// + /// Performs concurrent insert mutation on a table where the PK is not auto-generated and + /// validates that only one of the mutations is successful. + /// Both the mutations attempt to create an item with the same primary key. The mutation request + /// that runs first at the database layer should succeed and the other request should fail with + /// primary key violation constraint. + /// + [TestMethod] + public async Task TestParallelInsertMutationPKNonAutoGenerated() + { + string graphQLMutationName = "createComic"; + + string graphQLMutation1 = @" + mutation { + createComic (item: { id : 5001, categoryName: ""Fantasy"", title: ""Harry Potter""}){ + id + title + } + } + "; + + string graphQLMutation2 = @" + mutation { + createComic (item: { id : 5001, categoryName: ""Fantasy"", title: ""Lord of the Rings""}){ + id + title + } + } + "; + + Task responeTask1 = ExecuteGraphQLRequestAsync(graphQLMutation1, graphQLMutationName, isAuthenticated: true); + Task responseTask2 = ExecuteGraphQLRequestAsync(graphQLMutation2, graphQLMutationName, isAuthenticated: true); + + JsonElement response1 = await responeTask1; + JsonElement response2 = await responseTask2; + + string responseString1 = response1.ToString(); + string responseString2 = response2.ToString(); + string expectedStatusCode = $"{DataApiBuilderException.SubStatusCodes.DatabaseOperationFailed}"; + + // It is not possible to know beforehand which mutation created the new item. So, validations + // are performed for the cases where either mutation could have succeeded. In each case, + // one of the mutation's reponse will contain a valid repsonse and the other mutation's + // response would contain DatabaseOperationFailed sub-status code as it would've failed at + // the database layer due to primary key violation constraint. + if(responseString1.Contains($"\"code\":\"{expectedStatusCode}\"")) + { + Assert.AreEqual("{\"id\":5001,\"title\":\"Lord of the Rings\"}", responseString2); + } + else if(responseString2.Contains($"\"code\":\"{expectedStatusCode}\"")) + { + Assert.AreEqual("{\"id\":5001,\"title\":\"Harry Potter\"}", responseString1); + } + else + { + Assert.Fail("Unexpected error. Atleast one of the mutations should've succeeded"); + } + } + + /// + /// Performs concurrent delete mutations on the same item and validates that only one of the + /// requests is successful. + /// + [TestMethod] + public async Task TestParallelDeleteMutations() + { + string graphQLMutationName = "deletebook"; + + string graphQLMutation1 = @" + mutation { + deletebook (id: 1){ + id + title + } + } + "; + + Task responeTask1 = ExecuteGraphQLRequestAsync(graphQLMutation1, graphQLMutationName, isAuthenticated: true); + Task responseTask2 = ExecuteGraphQLRequestAsync(graphQLMutation1, graphQLMutationName, isAuthenticated: true); + + JsonElement response1 = await responeTask1; + JsonElement response2 = await responseTask2; + + string responseString1 = response1.ToString(); + string responseString2 = response2.ToString(); + string expectedResponse = "{\"id\":1,\"title\":\"Awesome book\"}"; + + // The mutation request that deletes the item is expected to have a valid response + // and the other mutation is expected to receive an empty response as it + // won't see the item in the table. + if (responseString1.Length == 0) + { + Assert.AreEqual(expectedResponse, responseString2); + } + else if (responseString2.Length == 0) + { + Assert.AreEqual(expectedResponse, responseString1); + } + else + { + Assert.Fail("Unexpected failure. Atleast one of the delete mutations should've succeeded"); + } + + } + #endregion #region Negative Tests diff --git a/src/Service/Resolvers/SqlQueryEngine.cs b/src/Service/Resolvers/SqlQueryEngine.cs index 5e293aacf1..b3c4fe043b 100644 --- a/src/Service/Resolvers/SqlQueryEngine.cs +++ b/src/Service/Resolvers/SqlQueryEngine.cs @@ -119,13 +119,12 @@ await ExecuteAsync(structure), _gQLFilterParser); string queryString = _queryBuilder.Build(structure); - List? jsonListResult = null; - - jsonListResult = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonResultAsync>, - _httpContextAccessor.HttpContext!); + List? jsonListResult = + await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonResultAsync>, + _httpContextAccessor.HttpContext!); if (jsonListResult is null) { @@ -301,14 +300,12 @@ public OkObjectResult OkResponse(JsonElement jsonResult) { // Open connection and execute query using _queryExecutor string queryString = _queryBuilder.Build(structure); - JsonDocument? jsonDocument = null; - - jsonDocument = await _queryExecutor.ExecuteQueryAsync( - queryString, - structure.Parameters, - _queryExecutor.GetJsonResultAsync, - _httpContextAccessor.HttpContext!); - + JsonDocument? jsonDocument = + await _queryExecutor.ExecuteQueryAsync( + queryString, + structure.Parameters, + _queryExecutor.GetJsonResultAsync, + _httpContextAccessor.HttpContext!); return jsonDocument; } @@ -321,9 +318,8 @@ public OkObjectResult OkResponse(JsonElement jsonResult) { string queryString = _queryBuilder.Build(structure); - JsonArray? resultArray = null; - - resultArray = await _queryExecutor.ExecuteQueryAsync( + JsonArray? resultArray = + await _queryExecutor.ExecuteQueryAsync( queryString, structure.Parameters, _queryExecutor.GetJsonArrayAsync, From 7da0ef374917bcf7b67dac53f6a89cc8dd56dc62 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Tue, 25 Apr 2023 15:40:30 +0530 Subject: [PATCH 07/14] fix formatting --- .../GraphQLMutationTestBase.cs | 11 ++++---- src/Service/Resolvers/SqlMutationEngine.cs | 26 +++++++++---------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs index 095cafa1a4..1d3393097f 100644 --- a/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs +++ b/src/Service.Tests/SqlTests/GraphQLMutationTests/GraphQLMutationTestBase.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Linq; using System.Text.Json; using System.Threading.Tasks; @@ -797,17 +796,17 @@ public async Task TestParallelInsertMutationPKNonAutoGenerated() // one of the mutation's reponse will contain a valid repsonse and the other mutation's // response would contain DatabaseOperationFailed sub-status code as it would've failed at // the database layer due to primary key violation constraint. - if(responseString1.Contains($"\"code\":\"{expectedStatusCode}\"")) + if (responseString1.Contains($"\"code\":\"{expectedStatusCode}\"")) { - Assert.AreEqual("{\"id\":5001,\"title\":\"Lord of the Rings\"}", responseString2); + Assert.AreEqual("{\"id\":5001,\"title\":\"Lord of the Rings\"}", responseString2); } - else if(responseString2.Contains($"\"code\":\"{expectedStatusCode}\"")) + else if (responseString2.Contains($"\"code\":\"{expectedStatusCode}\"")) { - Assert.AreEqual("{\"id\":5001,\"title\":\"Harry Potter\"}", responseString1); + Assert.AreEqual("{\"id\":5001,\"title\":\"Harry Potter\"}", responseString1); } else { - Assert.Fail("Unexpected error. Atleast one of the mutations should've succeeded"); + Assert.Fail("Unexpected error. Atleast one of the mutations should've succeeded"); } } diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index dba11d0ec1..92d2de9d1f 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -321,13 +321,12 @@ await _queryExecutor.ExecuteQueryAsync( try { - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new(TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { resultProperties = await PerformDeleteOperation( context.EntityName, @@ -359,13 +358,12 @@ await _queryExecutor.ExecuteQueryAsync( try { - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = new(TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled)) { upsertOperationResult = await PerformUpsertOperation( parameters, From 844346aff8d331b956a24eee1e560f3b5183618d Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Tue, 25 Apr 2023 16:15:14 +0530 Subject: [PATCH 08/14] adds helper method to throw exceptions, comments --- src/Service/Resolvers/SqlMutationEngine.cs | 77 +++++++++++++++------- 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 92d2de9d1f..17fd32ebfb 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -95,6 +95,11 @@ public SqlMutationEngine( try { + // Creating an implicit transaction + // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields + using (TransactionScope transactionScope = new( TransactionScopeOption.Required, new TransactionOptions @@ -152,12 +157,12 @@ await PerformMutationOperation( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope + // derive from TransactionException. Hence, TransactionException acts as a catch-all. + // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. catch (TransactionException) { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + ConstructAndThrowDataApiBuilderException(); } if (result is null) @@ -218,6 +223,10 @@ await PerformMutationOperation( try { + // Creating an implicit transaction + // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields using (TransactionScope transactionScope = new( TransactionScopeOption.Required, new TransactionOptions @@ -236,12 +245,12 @@ await _queryExecutor.ExecuteQueryAsync( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope + // derive from TransactionException. Hence, TransactionException acts as a catch-all. + // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. catch (TransactionException) { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + ConstructAndThrowDataApiBuilderException(); } // A note on returning stored procedure results: @@ -321,6 +330,10 @@ await _queryExecutor.ExecuteQueryAsync( try { + // Creating an implicit transaction + // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields using (TransactionScope transactionScope = new(TransactionScopeOption.Required, new TransactionOptions { @@ -334,13 +347,12 @@ await _queryExecutor.ExecuteQueryAsync( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope + // derive from TransactionException. Hence, TransactionException acts as a catch-all. + // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. catch (TransactionException) { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError - ); + ConstructAndThrowDataApiBuilderException(); } // Records affected tells us that item was successfully deleted. @@ -358,6 +370,10 @@ await _queryExecutor.ExecuteQueryAsync( try { + // Creating an implicit transaction + // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields using (TransactionScope transactionScope = new(TransactionScopeOption.Required, new TransactionOptions { @@ -371,13 +387,12 @@ await _queryExecutor.ExecuteQueryAsync( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope + // derive from TransactionException. Hence, TransactionException acts as a catch-all. + // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. catch (TransactionException) { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError - ); + ConstructAndThrowDataApiBuilderException(); } DbResultSetRow? dbResultSetRow = upsertOperationResult is not null ? @@ -415,6 +430,10 @@ await _queryExecutor.ExecuteQueryAsync( try { + // Creating an implicit transaction + // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields using (TransactionScope transactionScope = new( TransactionScopeOption.Required, new TransactionOptions @@ -431,12 +450,12 @@ await PerformMutationOperation( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope + // derive from TransactionException. Hence, TransactionException acts as a catch-all. + // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. catch (TransactionException) { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + ConstructAndThrowDataApiBuilderException(); } if (context.OperationType is Config.Operation.Insert) @@ -935,5 +954,19 @@ private HttpContext GetHttpContext() { return _httpContextAccessor.HttpContext!; } + + /// + /// Helper method to throw a DataApiBuidlerException when exceptions related + /// to transactions are encountered. + /// + /// + private static void ConstructAndThrowDataApiBuilderException() + { + throw new DataApiBuilderException( + message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + } + } } From 321081e563eae793d93e4a5709bf5970575e1864 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Tue, 25 Apr 2023 16:30:49 +0530 Subject: [PATCH 09/14] updating comments --- src/Service/Resolvers/SqlMutationEngine.cs | 39 +++++++++++++--------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 17fd32ebfb..1c498b1a61 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -97,8 +97,8 @@ public SqlMutationEngine( { // Creating an implicit transaction // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields using (TransactionScope transactionScope = new( TransactionScopeOption.Required, @@ -159,7 +159,8 @@ await PerformMutationOperation( } // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope // derive from TransactionException. Hence, TransactionException acts as a catch-all. - // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. + // When an exception related to Transactions is encountered, the mutation is deemed unsuccessful and + // a DataApiBuilderException is thrown catch (TransactionException) { ConstructAndThrowDataApiBuilderException(); @@ -225,8 +226,8 @@ await PerformMutationOperation( { // Creating an implicit transaction // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields using (TransactionScope transactionScope = new( TransactionScopeOption.Required, new TransactionOptions @@ -245,9 +246,11 @@ await _queryExecutor.ExecuteQueryAsync( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope // derive from TransactionException. Hence, TransactionException acts as a catch-all. - // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. + // When an exception related to Transactions is encountered, the mutation is deemed unsuccessful and + // a DataApiBuilderException is thrown catch (TransactionException) { ConstructAndThrowDataApiBuilderException(); @@ -332,8 +335,8 @@ await _queryExecutor.ExecuteQueryAsync( { // Creating an implicit transaction // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields using (TransactionScope transactionScope = new(TransactionScopeOption.Required, new TransactionOptions { @@ -347,9 +350,11 @@ await _queryExecutor.ExecuteQueryAsync( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope // derive from TransactionException. Hence, TransactionException acts as a catch-all. - // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. + // When an exception related to Transactions is encountered, the mutation is deemed unsuccessful and + // a DataApiBuilderException is thrown catch (TransactionException) { ConstructAndThrowDataApiBuilderException(); @@ -372,8 +377,8 @@ await _queryExecutor.ExecuteQueryAsync( { // Creating an implicit transaction // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields using (TransactionScope transactionScope = new(TransactionScopeOption.Required, new TransactionOptions { @@ -387,9 +392,11 @@ await _queryExecutor.ExecuteQueryAsync( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope // derive from TransactionException. Hence, TransactionException acts as a catch-all. - // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. + // When an exception related to Transactions is encountered, the mutation is deemed unsuccessful and + // a DataApiBuilderException is thrown catch (TransactionException) { ConstructAndThrowDataApiBuilderException(); @@ -432,8 +439,8 @@ await _queryExecutor.ExecuteQueryAsync( { // Creating an implicit transaction // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-7.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-8.0#fields + // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields + // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields using (TransactionScope transactionScope = new( TransactionScopeOption.Required, new TransactionOptions @@ -450,9 +457,11 @@ await PerformMutationOperation( transactionScope.Complete(); } } + // All the exceptions that can be thrown by .Complete() and .Dispose() methods of transactionScope // derive from TransactionException. Hence, TransactionException acts as a catch-all. - // When an exception related to Transactions is encountered, the mutation is simply failed and a DataApiBuilderException is thrown. + // When an exception related to Transactions is encountered, the mutation is deemed unsuccessful and + // a DataApiBuilderException is thrown catch (TransactionException) { ConstructAndThrowDataApiBuilderException(); From 8a49dc99d05016cb059e4dc168fa82f4bce90a80 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Thu, 27 Apr 2023 12:00:54 +0530 Subject: [PATCH 10/14] adds a helper method to create transaction scope --- src/Service/Resolvers/SqlMutationEngine.cs | 74 +++++++--------------- 1 file changed, 24 insertions(+), 50 deletions(-) diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 1c498b1a61..8d69af9218 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -96,17 +96,7 @@ public SqlMutationEngine( try { // Creating an implicit transaction - // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields - - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) { if (mutationOperation is Config.Operation.Delete) { @@ -225,16 +215,7 @@ await PerformMutationOperation( try { // Creating an implicit transaction - // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) { resultArray = await _queryExecutor.ExecuteQueryAsync( @@ -334,15 +315,7 @@ await _queryExecutor.ExecuteQueryAsync( try { // Creating an implicit transaction - // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields - using (TransactionScope transactionScope = new(TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) { resultProperties = await PerformDeleteOperation( context.EntityName, @@ -376,15 +349,7 @@ await _queryExecutor.ExecuteQueryAsync( try { // Creating an implicit transaction - // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields - using (TransactionScope transactionScope = new(TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) { upsertOperationResult = await PerformUpsertOperation( parameters, @@ -437,17 +402,8 @@ await _queryExecutor.ExecuteQueryAsync( try { - // Creating an implicit transaction - // https://learn.microsoft.com/en-us/dotnet/framework/data/transactions/implementing-an-implicit-transaction-using-transaction-scope - // TransactionScopeOption.Required: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeoption?view=net-6.0#fields - // TransactionScopeAsyncFlowOption.Enabled: https://learn.microsoft.com/en-us/dotnet/api/system.transactions.transactionscopeasyncflowoption?view=net-6.0#fields - using (TransactionScope transactionScope = new( - TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled)) + // Creating an implicit transaction + using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) { mutationResultRow = await PerformMutationOperation( @@ -964,6 +920,24 @@ private HttpContext GetHttpContext() return _httpContextAccessor.HttpContext!; } + /// + /// Helper method to construct a transactionscope object at Read Committed isolation level + /// with the TransactionScopeAsyncFlowOption option enabled + /// + /// + /// + /// + /// TransactionScope object set at Read Committed isolation level + private static TransactionScope ConstructReadCommittedTransactionScope() + { + return new(TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted + }, + TransactionScopeAsyncFlowOption.Enabled); + } + /// /// Helper method to throw a DataApiBuidlerException when exceptions related /// to transactions are encountered. From 19701afcca524ed409502d8b6c5fcdeddce0c1f3 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Wed, 3 May 2023 15:27:37 +0530 Subject: [PATCH 11/14] uses repeatable read isolation level for mysql --- src/Service/Resolvers/SqlMutationEngine.cs | 77 +++++++++++----------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 8d69af9218..23abd3d7af 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -41,7 +41,11 @@ public class SqlMutationEngine : IMutationEngine private readonly IHttpContextAccessor _httpContextAccessor; private readonly GQLFilterParser _gQLFilterParser; public const string IS_FIRST_RESULT_SET = "IsFirstResultSet"; - private const string TRANSACTION_EXCEPTION_ERROR_MSG = "An unexpected error occured"; + private const string TRANSACTION_EXCEPTION_ERROR_MSG = "An unexpected error occurred during the transaction execution"; + + private static DataApiBuilderException _dabExceptionWithTransactionErrorMessage = new(message: TRANSACTION_EXCEPTION_ERROR_MSG, + statusCode: HttpStatusCode.InternalServerError, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); /// /// Constructor @@ -96,7 +100,7 @@ public SqlMutationEngine( try { // Creating an implicit transaction - using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) + using (TransactionScope transactionScope = ConstructTransactionScopeBasedOnDbType()) { if (mutationOperation is Config.Operation.Delete) { @@ -153,15 +157,14 @@ await PerformMutationOperation( // a DataApiBuilderException is thrown catch (TransactionException) { - ConstructAndThrowDataApiBuilderException(); + throw _dabExceptionWithTransactionErrorMessage; } if (result is null) { - throw new DataApiBuilderException( - message: "Failed to resolve any query based on the current configuration.", - statusCode: HttpStatusCode.BadRequest, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + throw new DataApiBuilderException(message: "Failed to resolve any query based on the current configuration.", + statusCode: HttpStatusCode.BadRequest, + subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); } return result; @@ -215,7 +218,7 @@ await PerformMutationOperation( try { // Creating an implicit transaction - using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) + using (TransactionScope transactionScope = ConstructTransactionScopeBasedOnDbType()) { resultArray = await _queryExecutor.ExecuteQueryAsync( @@ -234,7 +237,7 @@ await _queryExecutor.ExecuteQueryAsync( // a DataApiBuilderException is thrown catch (TransactionException) { - ConstructAndThrowDataApiBuilderException(); + throw _dabExceptionWithTransactionErrorMessage; } // A note on returning stored procedure results: @@ -315,7 +318,7 @@ await _queryExecutor.ExecuteQueryAsync( try { // Creating an implicit transaction - using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) + using (TransactionScope transactionScope = ConstructTransactionScopeBasedOnDbType()) { resultProperties = await PerformDeleteOperation( context.EntityName, @@ -330,7 +333,7 @@ await _queryExecutor.ExecuteQueryAsync( // a DataApiBuilderException is thrown catch (TransactionException) { - ConstructAndThrowDataApiBuilderException(); + throw _dabExceptionWithTransactionErrorMessage; } // Records affected tells us that item was successfully deleted. @@ -349,7 +352,7 @@ await _queryExecutor.ExecuteQueryAsync( try { // Creating an implicit transaction - using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) + using (TransactionScope transactionScope = ConstructTransactionScopeBasedOnDbType()) { upsertOperationResult = await PerformUpsertOperation( parameters, @@ -364,7 +367,7 @@ await _queryExecutor.ExecuteQueryAsync( // a DataApiBuilderException is thrown catch (TransactionException) { - ConstructAndThrowDataApiBuilderException(); + throw _dabExceptionWithTransactionErrorMessage; } DbResultSetRow? dbResultSetRow = upsertOperationResult is not null ? @@ -403,7 +406,7 @@ await _queryExecutor.ExecuteQueryAsync( try { // Creating an implicit transaction - using (TransactionScope transactionScope = ConstructReadCommittedTransactionScope()) + using (TransactionScope transactionScope = ConstructTransactionScopeBasedOnDbType()) { mutationResultRow = await PerformMutationOperation( @@ -420,7 +423,7 @@ await PerformMutationOperation( // a DataApiBuilderException is thrown catch (TransactionException) { - ConstructAndThrowDataApiBuilderException(); + throw _dabExceptionWithTransactionErrorMessage; } if (context.OperationType is Config.Operation.Insert) @@ -921,35 +924,33 @@ private HttpContext GetHttpContext() } /// - /// Helper method to construct a transactionscope object at Read Committed isolation level - /// with the TransactionScopeAsyncFlowOption option enabled + /// For MySql database type, the isolation level is set at Repeatable Read as it is the default isolation level. Likeweise, for MsSql and PostgreSql + /// database types, the isolation level is set at Read Committed as it is the default. /// - /// - /// - /// - /// TransactionScope object set at Read Committed isolation level - private static TransactionScope ConstructReadCommittedTransactionScope() - { - return new(TransactionScopeOption.Required, - new TransactionOptions - { - IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted - }, - TransactionScopeAsyncFlowOption.Enabled); + /// TransactionScope object with the appropriate isolation level based on the database type + private TransactionScope ConstructTransactionScopeBasedOnDbType() + { + return _sqlMetadataProvider.GetDatabaseType() is DatabaseType.mysql ? ConstructTransactionScopeWithSpecifiedIsolationLevel(isolationLevel: System.Transactions.IsolationLevel.RepeatableRead) + : ConstructTransactionScopeWithSpecifiedIsolationLevel(isolationLevel: System.Transactions.IsolationLevel.ReadCommitted); } /// - /// Helper method to throw a DataApiBuidlerException when exceptions related - /// to transactions are encountered. + /// Helper method to construct a TransactionScope object with the specified isolation level and + /// with the TransactionScopeAsyncFlowOption option enabled. /// - /// - private static void ConstructAndThrowDataApiBuilderException() + /// Transaction isolation level + /// + /// + /// + /// TransactionScope object set at the specified isolation level + private static TransactionScope ConstructTransactionScopeWithSpecifiedIsolationLevel(System.Transactions.IsolationLevel isolationLevel) { - throw new DataApiBuilderException( - message: TRANSACTION_EXCEPTION_ERROR_MSG, - statusCode: HttpStatusCode.InternalServerError, - subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); + return new (TransactionScopeOption.Required, + new TransactionOptions + { + IsolationLevel = isolationLevel + }, + TransactionScopeAsyncFlowOption.Enabled); } - } } From 3f65eebaf65556672aa5702eefd46b82a279df75 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Wed, 3 May 2023 18:19:34 +0530 Subject: [PATCH 12/14] fix formatting --- src/Service/Resolvers/SqlMutationEngine.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Service/Resolvers/SqlMutationEngine.cs b/src/Service/Resolvers/SqlMutationEngine.cs index 23abd3d7af..51a96655cc 100644 --- a/src/Service/Resolvers/SqlMutationEngine.cs +++ b/src/Service/Resolvers/SqlMutationEngine.cs @@ -42,7 +42,7 @@ public class SqlMutationEngine : IMutationEngine private readonly GQLFilterParser _gQLFilterParser; public const string IS_FIRST_RESULT_SET = "IsFirstResultSet"; private const string TRANSACTION_EXCEPTION_ERROR_MSG = "An unexpected error occurred during the transaction execution"; - + private static DataApiBuilderException _dabExceptionWithTransactionErrorMessage = new(message: TRANSACTION_EXCEPTION_ERROR_MSG, statusCode: HttpStatusCode.InternalServerError, subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError); @@ -929,7 +929,7 @@ private HttpContext GetHttpContext() /// /// TransactionScope object with the appropriate isolation level based on the database type private TransactionScope ConstructTransactionScopeBasedOnDbType() - { + { return _sqlMetadataProvider.GetDatabaseType() is DatabaseType.mysql ? ConstructTransactionScopeWithSpecifiedIsolationLevel(isolationLevel: System.Transactions.IsolationLevel.RepeatableRead) : ConstructTransactionScopeWithSpecifiedIsolationLevel(isolationLevel: System.Transactions.IsolationLevel.ReadCommitted); } @@ -945,7 +945,7 @@ private TransactionScope ConstructTransactionScopeBasedOnDbType() /// TransactionScope object set at the specified isolation level private static TransactionScope ConstructTransactionScopeWithSpecifiedIsolationLevel(System.Transactions.IsolationLevel isolationLevel) { - return new (TransactionScopeOption.Required, + return new(TransactionScopeOption.Required, new TransactionOptions { IsolationLevel = isolationLevel From 2f3b41b05896445863fe15d61240b718c83581f4 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Thu, 4 May 2023 12:50:51 +0530 Subject: [PATCH 13/14] removes explicit transaction creating using sql stmnts --- src/Service/Resolvers/MsSqlQueryBuilder.cs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/Service/Resolvers/MsSqlQueryBuilder.cs b/src/Service/Resolvers/MsSqlQueryBuilder.cs index 79594f6df4..2afaa4da31 100644 --- a/src/Service/Resolvers/MsSqlQueryBuilder.cs +++ b/src/Service/Resolvers/MsSqlQueryBuilder.cs @@ -105,12 +105,7 @@ public string Build(SqlExecuteStructure structure) $"{BuildProcedureParameterList(structure.ProcedureParameters)}"; } - /// - /// Avoid redundant check, wrap the sequence in a transaction, - /// and protect the first table access with appropriate locking. - /// - /// - /// Query generated for the PUT(upsert)/PATCH(upsertIncremental) operation. + /// public string Build(SqlUpsertQueryStructure structure) { string tableName = $"{QuoteIdentifier(structure.DatabaseObject.SchemaName)}.{QuoteIdentifier(structure.DatabaseObject.Name)}"; @@ -126,9 +121,7 @@ public string Build(SqlUpsertQueryStructure structure) string queryToGetCountOfRecordWithPK = $"SELECT COUNT(*) as {COUNT_ROWS_WITH_GIVEN_PK} FROM {tableName} WHERE {pkPredicates}"; // Query to initiate transaction and get number of records with given PK. - string prefixQuery = $"SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;" + - $"BEGIN TRANSACTION;" + - $"DECLARE @ROWS_TO_UPDATE int;" + + string prefixQuery = $"DECLARE @ROWS_TO_UPDATE int;" + $"SET @ROWS_TO_UPDATE = ({queryToGetCountOfRecordWithPK}); " + $"{queryToGetCountOfRecordWithPK};"; @@ -137,8 +130,8 @@ public string Build(SqlUpsertQueryStructure structure) // Query to update record (if there exists one for given PK). StringBuilder updateQuery = new( - $"IF @ROWS_TO_UPDATE = 1" + - $"UPDATE {tableName} WITH(UPDLOCK) " + + $"IF @ROWS_TO_UPDATE = 1 " + + $"UPDATE {tableName} " + $"SET {updateOperations} " + $"OUTPUT {outputColumns} " + $"WHERE {updatePredicates};"); @@ -172,9 +165,6 @@ public string Build(SqlUpsertQueryStructure structure) upsertQuery.Append(insertQuery.ToString()); } - // Commit the transaction. - upsertQuery.Append("COMMIT TRANSACTION"); - return upsertQuery.ToString(); } From a99a2b0229f59d6f31605e7fdc518e9d292060b2 Mon Sep 17 00:00:00 2001 From: Shyam Sundar J Date: Thu, 4 May 2023 18:18:43 +0530 Subject: [PATCH 14/14] updating comment --- src/Service/Resolvers/MsSqlQueryBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Service/Resolvers/MsSqlQueryBuilder.cs b/src/Service/Resolvers/MsSqlQueryBuilder.cs index 2afaa4da31..380ee0fbc2 100644 --- a/src/Service/Resolvers/MsSqlQueryBuilder.cs +++ b/src/Service/Resolvers/MsSqlQueryBuilder.cs @@ -120,7 +120,7 @@ public string Build(SqlUpsertQueryStructure structure) string outputColumns = MakeOutputColumns(structure.OutputColumns, OutputQualifier.Inserted); string queryToGetCountOfRecordWithPK = $"SELECT COUNT(*) as {COUNT_ROWS_WITH_GIVEN_PK} FROM {tableName} WHERE {pkPredicates}"; - // Query to initiate transaction and get number of records with given PK. + // Query to get the number of records with a given PK. string prefixQuery = $"DECLARE @ROWS_TO_UPDATE int;" + $"SET @ROWS_TO_UPDATE = ({queryToGetCountOfRecordWithPK}); " + $"{queryToGetCountOfRecordWithPK};";