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

Support NodaTime #1682

Open
bjornharrtell opened this issue Feb 18, 2025 · 10 comments
Open

Support NodaTime #1682

bjornharrtell opened this issue Feb 18, 2025 · 10 comments

Comments

@bjornharrtell
Copy link
Contributor

Goal would be same level of support as for native datetime and DateTimeOffset and ideally it would be possible to use contains filter against a NodaTime interval.

Alternatives I've thought about are custom logic in ResourceDefinition.

@bjornharrtell
Copy link
Contributor Author

Might be that this too (as with #809) would be better served as extension.

@bkoelman
Copy link
Member

Any reason this doesn't already work after adding the EF Core package? NodaTime uses TypeConverter, so RuntimeTypeConverter which is used for query strings should already work.

The contains filter is reserved for substring matching, however lessThan/greaterThan should work. Or you could define your own range function.

Did you actually try to use NodaTime?

@bjornharrtell
Copy link
Contributor Author

@bkoelman no didn't actually give it a shot and made some assumptions which I see might be wrong, sorry. I'll see if I can get some more substantial information here ASAP.

@bjornharrtell
Copy link
Contributor Author

It will certainly eat it, but default serialization of nodatime interval isn't very usable:

{
"start": {},
"hasStart": true,
"end": {},
"hasEnd": true,
"duration": {
"days": 2915343,
"nanosecondOfDay": 86399999999999,
"hours": 23,
"minutes": 59,
"seconds": 59,
"milliseconds": 999,
"subsecondTicks": 9999999,
"subsecondNanoseconds": 999999999,
"bclCompatibleTicks": 2518857216000000000,
"totalDays": 2915344,
"totalHours": 69968256,
"totalMinutes": 4198095360,
"totalSeconds": 251885721600,
"totalMilliseconds": 251885721600000,
"totalTicks": 2518857216000000000,
"totalNanoseconds": 251885721600000000000
}

I'm not sure I can make sense of filtering interval with lessThan / greaterThan.. but I don't think the parser knows what do do with it regardless, resulting in fx. Failed to convert '2025-01-01' of type 'String' to type 'Interval'. Failed at position 22: greaterThan(virkning,^'2025-01-01').

@bkoelman
Copy link
Member

About the serialization, did you call JsonApiOptions.SerializerOptions.ConfigureForNodaTime(DateTimeZoneProviders.Tzdb)?

If so, then apparently this is how NodaTime serializes to JSON. You could always insert your own JsonConverter if you don't like it.

About the conversion, I don't know. Does the library provide something to register type converters to .NET?

It would help if you could share a minimal sample that can be debugged.

@bjornharrtell
Copy link
Contributor Author

Correctly configuring serialization as you suggest definitely makes it look much better:

{
"start": "2018-01-22T00:00:00Z",
"end": "9999-12-31T23:59:59.999999999Z"
}

Sorry again for not doing my homework. I will return with better info if possible.

@bkoelman
Copy link
Member

@bkoelman
Copy link
Member

bkoelman commented Mar 3, 2025

Were you able to get filters to work?

@bjornharrtell
Copy link
Contributor Author

@bkoelman haven't pursued it yet, and not sure when I'll be able to find time for it (or rather, persuade others that it is worthwhile). I suppose this could be closed since basic support is fine, agreed?

@bkoelman
Copy link
Member

bkoelman commented Mar 3, 2025

From what I understand, serialization and model state binding/validation should already work. I'm not entirely sure about query string parsing. I'll close this now, but we can pursue it further in a new issue when more information is available.

If there's no way to get NodaTime type converters working, we could introduce an injectable abstraction over the static RuntimeTypeConverter to allow calling NodaTime conversions from custom code. Only downside is that Identifiable also needs it to convert Id to/from string, where injection is unavailable. But I suppose that limitation isn't much of an actual problem.

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

No branches or pull requests

2 participants