-
Notifications
You must be signed in to change notification settings - Fork 354
Add support for parsing DateTime values #3250
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2fd8ee4
1358975
18d4f90
ecd6403
648b7b4
030ed45
e18b831
80c52c5
2507e0f
889b11e
d62a0b8
c2c0347
43f0ad8
ea07809
ecdca10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -397,9 +397,49 @@ private static bool TryGetPropertyFromParent( | |
| SupportedHotChocolateTypes.SINGLE_TYPE => value is IntValueNode intValueNode ? intValueNode.ToSingle() : ((FloatValueNode)value).ToSingle(), | ||
| SupportedHotChocolateTypes.FLOAT_TYPE => value is IntValueNode intValueNode ? intValueNode.ToDouble() : ((FloatValueNode)value).ToDouble(), | ||
| SupportedHotChocolateTypes.DECIMAL_TYPE => value is IntValueNode intValueNode ? intValueNode.ToDecimal() : ((FloatValueNode)value).ToDecimal(), | ||
| SupportedHotChocolateTypes.DATETIME_TYPE => ParseDateTimeValue(value.Value), | ||
| SupportedHotChocolateTypes.UUID_TYPE => Guid.TryParse(value.Value!.ToString(), out Guid guidValue) ? guidValue : value.Value, | ||
| _ => value.Value | ||
| }; | ||
|
|
||
| static object? ParseDateTimeValue(object? raw) | ||
| { | ||
| if (raw is null) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| if (raw is DateTime dt) | ||
| { | ||
| return dt.Kind switch | ||
| { | ||
| DateTimeKind.Utc => dt, | ||
| DateTimeKind.Unspecified => DateTime.SpecifyKind(dt, DateTimeKind.Utc), | ||
| _ => dt.ToUniversalTime() | ||
| }; | ||
| } | ||
|
|
||
| if (raw is DateTimeOffset dto) | ||
| { | ||
| return dto.UtcDateTime; | ||
| } | ||
|
|
||
| if (raw is string s) | ||
| { | ||
| // HotChocolate DateTime inputs are supplied as strings; parse them so DB providers | ||
| // (notably PostgreSQL) receive a typed UTC parameter instead of text. | ||
| if (DateTimeOffset.TryParse( | ||
| s, | ||
| CultureInfo.InvariantCulture, | ||
| DateTimeStyles.AssumeUniversal, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isnt this forcing UTC culture for all db types? What if the original string had culture information and its losing that while parsing here? This explains why the MSSQL test is failing as per your analysis. If the culture was set to: Similarly, if the test string itself specifies a different timezone wont it fail, since with this change, we are assuming utc?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this path be special cased to PostgreSQL?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think you are right. Please see #3728 for the most up-to-date version of the PR (PostgreSQL is special cased and the parsing logic is moved to a different method). |
||
| out DateTimeOffset parsedDto)) | ||
| { | ||
| return parsedDto.UtcDateTime; | ||
| } | ||
| } | ||
|
|
||
| return raw; | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using Azure.DataApiBuilder.Service.Services; | ||
| using HotChocolate.Execution; | ||
| using HotChocolate.Language; | ||
| using HotChocolate.Types; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
| using Moq; | ||
|
|
||
| namespace Azure.DataApiBuilder.Service.Tests.UnitTests; | ||
|
|
||
| [TestClass] | ||
| public class ExecutionHelperUnitTests | ||
| { | ||
| [TestMethod] | ||
| public void ExtractValueFromIValueNode_DateTimeLiteral_ReturnsUtcDateTime() | ||
| { | ||
| Mock<IInputValueDefinition> argumentSchema = CreateArgumentSchema(new DateTimeType()); | ||
| Mock<IVariableValueCollection> variables = new(); | ||
|
|
||
| object result = ExecutionHelper.ExtractValueFromIValueNode( | ||
| new StringValueNode("2026-04-22T10:15:30-07:00"), | ||
| argumentSchema.Object, | ||
| variables.Object); | ||
|
|
||
| Assert.IsInstanceOfType<DateTime>(result); | ||
| Assert.AreEqual( | ||
| new DateTime(2026, 4, 22, 17, 15, 30, DateTimeKind.Utc), | ||
| (DateTime)result); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public void ExtractValueFromIValueNode_DateTimeVariable_ReturnsUtcDateTime() | ||
| { | ||
| Mock<IInputValueDefinition> argumentSchema = CreateArgumentSchema(new DateTimeType()); | ||
| Mock<IVariableValueCollection> variables = new(); | ||
| variables | ||
| .Setup(v => v.GetValue<IValueNode>("createdAt")) | ||
| .Returns(new StringValueNode("2026-04-22T10:15:30Z")); | ||
|
|
||
| object result = ExecutionHelper.ExtractValueFromIValueNode( | ||
| new VariableNode("createdAt"), | ||
| argumentSchema.Object, | ||
| variables.Object); | ||
|
|
||
| Assert.IsInstanceOfType<DateTime>(result); | ||
| Assert.AreEqual( | ||
| new DateTime(2026, 4, 22, 10, 15, 30, DateTimeKind.Utc), | ||
| (DateTime)result); | ||
| } | ||
|
|
||
| private static Mock<IInputValueDefinition> CreateArgumentSchema(IInputType type) | ||
| { | ||
| Mock<IInputValueDefinition> argumentSchema = new(); | ||
| argumentSchema.SetupGet(a => a.Type).Returns(type); | ||
| return argumentSchema; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not make too much sense to me...if we are dealing with
SupportedHotChocolateTypes.DATETIME_TYPEshouldn't we return aDateTime(not aDateTimeOffset)?