Skip to content

Commit 8a32f3f

Browse files
seantleonardAniruddh25abhishekkumams
authored
Fix MySQL test execution time + Fix DateTime GraphQL input handling (#2265)
## Why make this change? I discovered another bug while working on this change #2268, which will be addressed separately. ## What is this change? - Fixes a bug where GraphQL DateTime input with Timezone (TZ) offset was not honored. - e.g. `12-31-2024T12:00:00+03:00` was stored as `12-31-2024T12:00:00` in the backend database instead of `12-31-2024T09:00:00` - This bug was discovered because some SupportedTypes tests involving dates always failed locally but not in the integration test pipeline. My local machine in UTC-8 while pipelines are UTC. Additionally, some test input `9999-12-31 23:59:59.9999999` was run for MySql where that value is an invalid date (max decimal is .499999). - Fixes false negative (passing) test cases in **GraphQLSupportedTypesTests** by adding more comprehensive asserts, updating DataRow test input, and ensuring tests specific to a database type are correctly executed. - As a result, MySQL tests in this specific test suite run way faster (~minutes down to less than 10 seconds because the conclusion of each data row execution doesn't reset the MySql database contents which takes a long time via `ResetDbStateAsync()`.) - Updates test code queries and mutations to include the primary key (typeid) of the table **type_table** so that tests interact with determinate records: ```csharp string gqlQuery = "{ supportedType_by_pk(typeid: " + id + ") { typeid, " + field + " } }"; ``` - A bug in the test ```csharp // Old test [DataRow(TIME_TYPE, "\"23:59:59.9999999\"")] // Fixed Test [DataRow(TIME_TYPE, "23:59:59")] public async Task InsertIntoTypeColumnWithArgument(string type, object value) ``` This test shows as green in main branch. However, looking at the log of the test, I saw this error: > GraphQL error: [{"message":"The variable `param` is not compatible with the type of the current location.","locations":[{"line":1,"column":65}],"path":["createSupportedType"],"extensions":{"variable":"param","variableType":"Time","locationType":"LocalTime","specifiedBy":"http://spec.graphql.org/October2021/#sec-All-Variable-Usages-are-Allowed"}}] There are two layers of changes here: 1. TIME_TYPE is technically not a GraphQL data type recognized or defined by DAB's generated GraphQL schema/ HotChocolate -> LocalTime is. So when the data row for TIME_TYPE comes in this code resolves the expected type: ```csharp private static string TypeNameToGraphQLType(string typeName) { return typeName switch { DATETIMEOFFSET_TYPE => DATETIME_TYPE, TIME_TYPE => LOCALTIME_TYPE, _ => typeName }; } ``` 2. Leaving quotes on the value -> `\"23:59:59.9999999\"` in results in the error: > GraphQL error: [{"message":"Unable to deserialize string to LocalTime","path":["param"]}] why keep data row as "TIME_TYPE"? well, our test code resolves that type name not only as the GraphQL type but also as a way to determine which column in the data source to select which is named time_types. So we'd need to decouple the GraphQL Data Type value from being used to determine the SQL table column name. ## How was this tested? - [x] Integration Tests - Fixed tests in GraphQLSupportedTypesTests --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com> Co-authored-by: Abhishek Kumar <102276754+abhishekkumams@users.noreply.github.com> Co-authored-by: Abhishek Kumar <abhishekkuma@microsoft.com>
1 parent 689c3e7 commit 8a32f3f

File tree

6 files changed

+326
-79
lines changed

6 files changed

+326
-79
lines changed

src/Core/Resolvers/Sql Query Structures/BaseSqlQueryStructure.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,14 @@ protected static object ParseParamAsSystemType(string param, Type systemType)
439439
"Double" => double.Parse(param),
440440
"Decimal" => decimal.Parse(param),
441441
"Boolean" => bool.Parse(param),
442-
"DateTime" => DateTimeOffset.Parse(param, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal).DateTime,
442+
// When GraphQL input specifies a TZ offset "12-31-2024T12:00:00+03:00"
443+
// and DAB has resolved the ColumnDefinition.SystemType to DateTime,
444+
// DAB converts the input to UTC because:
445+
// - DAB assumes that values without timezone offset are UTC.
446+
// - DAB shouldn't store values in the backend database which the user did not intend
447+
// - e.g. Storing this value for the original example would be incorrect: "12-31-2024T12:00:00"
448+
// - The correct value to store would be "12-31-2024T09:00:00" (UTC)
449+
"DateTime" => DateTimeOffset.Parse(param, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal).UtcDateTime,
443450
"DateTimeOffset" => DateTimeOffset.Parse(param, DateTimeFormatInfo.InvariantInfo, DateTimeStyles.AssumeUniversal),
444451
"Date" => DateOnly.Parse(param),
445452
"Guid" => Guid.Parse(param),
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
namespace Azure.DataApiBuilder.Service.Tests.SqlTests.GraphQLSupportedTypesTests;
5+
6+
/// <summary>
7+
/// Encapsulates field name metadata which is used when
8+
/// creating database queries to validate test results
9+
/// in the GraphQLSupportedTypesTestBase.
10+
/// </summary>
11+
public class DabField
12+
{
13+
/// <summary>
14+
/// Mapped (aliased) column name defined in DAB runtime config.
15+
/// </summary>
16+
public string Alias { get; set; }
17+
18+
/// <summary>
19+
/// Database column name.
20+
/// </summary>
21+
public string BackingColumnName { get; set; }
22+
23+
/// <summary>
24+
/// Creates a new DabField instance with both alias and backing column name.
25+
/// </summary>
26+
/// <param name="alias">Mapped (aliased) column name defined in DAB runtime config.</param>
27+
/// <param name="backingColumnName">Database column name.</param>
28+
public DabField(string alias, string backingColumnName)
29+
{
30+
Alias = alias;
31+
BackingColumnName = backingColumnName;
32+
}
33+
34+
/// <summary>
35+
/// Creates a new DabField instance with only the backing column name
36+
/// where the alias is the same as the backing column name.
37+
/// </summary>
38+
/// <param name="backingColumnName">Database column name.</param>
39+
public DabField(string backingColumnName)
40+
{
41+
Alias = backingColumnName;
42+
BackingColumnName = backingColumnName;
43+
}
44+
}

0 commit comments

Comments
 (0)