Skip to content
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

Adding DateTimeConverter which will handle binding of DateTime/DateTimeOffset type parameters #852

Merged
merged 7 commits into from
May 4, 2022

Conversation

kshyju
Copy link
Member

@kshyju kshyju commented Apr 11, 2022

Fixes #851

Adding a new IConverter implementation to handle binding the below types.

  1. DateTime
  2. DateTime?
  3. DateTimeOffset
  4. DateTimeOffset?

public ValueTask<ConversionResult> ConvertAsync(ConverterContext context)
{
if (
context.TargetType == typeof(DateTime) || context.TargetType == typeof(DateTime?)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should DateTimeOffset be handled as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Yes, we should support DateTimeOffset as well. I pushed a new iteration with that support.

@kshyju kshyju changed the title Adding DateTimeConverter which will handle binding of DateTime/DateOnly/TimeOnly type parameters Adding DateTimeConverter which will handle binding of DateTime/DateTimeOffset type parameters Apr 14, 2022
@kshyju kshyju requested a review from liliankasem April 25, 2022 19:55
return new ValueTask<ConversionResult>(ConversionResult.Unhandled());
}

if ((context.TargetType == typeof(DateTimeOffset) || context.TargetType == typeof(DateTimeOffset?))
Copy link
Member

Choose a reason for hiding this comment

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

In what scenario would we pass null to a DateTimeOffset? or DateTime?? Is there any?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's say customer has a function definition like below.

public static HttpResponseData Run(
    [HttpTrigger(AuthorizationLevel.Anonymous, "get")] HttpRequestData req,
    DateTime startDate,
    DateTime? endDate)

where endDate param is nullable type / optional. The below 2 requests will work for this function definition.

api/Function4?startdate=2021/10/10&enddate=2021/12/12
api/Function4?startdate=2021/12/12

For the second request where endDate is not part of the request's input data/ trigger metadata, so, we do not have a source data to use (ie: source is null). So, the converter will return Unhandled result and the default value of the parameterValues array (of type object[]) entry will be used, which is null. We will end up passing null as that argument's value when invoking the function.

Copy link
Member Author

Choose a reason for hiding this comment

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

We do similar thing for Guid converter where Guid? is supported.

@kshyju
Copy link
Member Author

kshyju commented May 4, 2022

/check-enforcer evaluate

@kshyju kshyju merged commit 3650b65 into main May 4, 2022
@kshyju kshyju deleted the shkr/datetime-converter branch May 4, 2022 21:09
@kshyju
Copy link
Member Author

kshyju commented May 17, 2022

Microsoft.Azure.Functions.Worker 1.8.0-preview3 includes this change.

@viktorasmickunas
Copy link

I have a question: how come this converter converts timestamps in strings, when the TargetType isn't even DateTime or DateTimeOffset?

See issue: https://github.com/Azure/azure-functions-dotnet-worker/issues/2731

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.NET Isolated DateTime Route parameter issue
5 participants