-
-
Notifications
You must be signed in to change notification settings - Fork 461
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
Added Linq support for FSharp options #3589
Conversation
i have just added a |
… parts of the test suite.
Please do not merge yet, I need to fix failing regression tests (there is a single root cause). Nullable structs are not properly handled (eg. Issue2d?) |
…r using the constructor or a "From" method
…ract presenter in the docs (users can name the builder the way they want)
The problem should now be fixed, tests are passing |
- Required putting the Target type in a separate shared library to avoid a circular dependency - Moved Fsharp acceptance tests to different test classes to avoid affecting other tests due to the specific serializer used for F#
Improved separation between F# test cases and C# test cases to avoid breaking other tests due to differences with the serializer used (newtonsoft has limited support for discriminated unions).
@jeremydmiller Hi Jeremy ! I know I've opened this PR literally just yesterday but I really hope this can make it through the next release as I've implemented this on borrowed time 😅 I was already behind schedule for my project supposed to be used by the customer during the holidays season... |
Summary
This PR adds support for querying against F# options (the F# native equivalent to C#'s nullables).
Because F# options are a generic concrete type and not an interface, there is no covariance. This has required introducing a number of adjustments:
IValueTypeMember
is now genericThis was mainly required because the boxing introduced by ConvertFromWrapperArray caused casting exceptions for F# options since covariance is not possible. Most of the changes are just small API modifications required on components consuming this interface.
Unit tests
A number of unit tests have been created. One noteworthy fact is that comparing F# options (>, >=, <, <=) is invalid in C# (compiler syntax error) while it is a valid construct in F#.
I have therefore defined the comparison lambda expressions in the F# library of the test suite. This required moving the
Target
test type to a standalone library since the F# lambdas need to reference it (otherwise there'd be a circular dependency since the main test project needs to reference the F# lambdas).Nullable struct value types
Nullable struct value types required special treatment (eg.
IssueId?
) to avoid casting issues since IValueType is now strongly typed. Before callingConvertFromWrapperArray(values)
there was a need to unwrap the values from their nullable.This has resulted in the initially daunting
UnwrapIEnumerableOfNullables(this object obj)
method which is actually simple to understand when we take a closer look (we check if it's a IEnumerable<Nullable>, if yes we unwrap and create an instance of the strongTyped id either through the constructor or through the builder method).Some F# tests are kept separate from other tests
Serializing F# options properly requires using System.Text.Json (at least this is the use case I was targetting, since it is the most commonly used serializer nowadays. The most popular f# serialization library uses System.Text.Json. And this is what I use). Using System.Text.Json globally was breaking a couple of tests (this was mainly due to the fact that STJ struggles to deserialize anonymous types). I have therefore created separate F#-friendly stores and created a separate test class using these stores.
Remarks
The implementation assumes that options are stored in the DB unwrapped. While in my opinion this is what makes the most sense (there is either the json property or there is nothing), this sort of puts a constraint on the serialization format (serializers can serialize options in plenty of ways since F# options are discriminated unions).
In a later version, we may look into enabling the user to customize the serialization format somehow (without special handling, just supplying an IMemberSource may not be enough since the built-in FSharpOptionQueryableMember is already registered), but i think this is very reasonable for a default and a starting point
Also in terms of use, users will have to register all the option types variations themselves on startup:This is not ideal, especially since these types are well-known, but I couldn't find a place where value types are auto-registered / built-in. Maybe we can create an extension method on StoreOptionsRegisterFsharpOptionsValueTypes()
?