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

F# discriminated union support for strongly-typed identifiers #3279

Merged
merged 11 commits into from
Jun 27, 2024

Conversation

nkosi23
Copy link

@nkosi23 nkosi23 commented Jun 21, 2024

Implements #3280

nkosi23 added 5 commits June 20, 2024 19:55
…eat F# discriminated unions (DUs) as candidates

- Modified the RegistereValueTypes method in StoreOptions.MemberFactory.cs to support registering types representing single case F# discriminated unions matching naming policy
- Removing the struct constraint to the TOuter generic parameter on the class StrongTypedIdSelectClause to support reference types (since F# DUs are represents by abstract classes) caused an obscure runtime cast issue in the BuildHandler<TResult> method when value types are used.

The cast error was related to the fact that value type is wrapped into a nullable when the struct constraint is removed.I therefore introduced the intermediary types INullableQueryHandler<T> where T : struct and NullableListQueryHandler<T> and implementing it to facilitate this cast for value types.

To bypass compiler limitations to the inferred type of TResult in BuildHandler<TResult>, NullableListQueryHandler<TResult> lists (only used for value types) are instanciated dynamically.

- Added test cases to existing tests in applicability_of_identity_types.cs to verify that classes representing F# DUs are correctly registered and detected as candidates
- Added a test class for F# DUs but for example the tests are just a copy-and-paste of other test cases

Status: awaiting implementation of all tests in F#
…identifiers

- Added a couple of utility methods

Used a wrong reference for the class representing F# single-case DUs (there is no nested class in this case). Awaiting changes in an upcoming commit
…e f# compiler does not generate nested classes for single case discriminated), the implementation has been adjusted (and simplified) accordingly
… made the necessary adjustments

- Added missing test cases
- Added F# project to test against actual F# types
- All tests are (finally) passing!!!
@nkosi23 nkosi23 changed the title [WIPI]: F# discriminated union support for strongly-typed identifiers F# discriminated union support for strongly-typed identifiers Jun 22, 2024
@nkosi23
Copy link
Author

nkosi23 commented Jun 22, 2024

One thing that I've not done though is Comb's sequential Guid id generation. This is the kind of thing I'd love to use as I see a lot of value in it, but I'm not sure how I'd surface such an id generated by marten considering that:

  1. The FSharp DU id generation strategy is essentially a no-op strategy
  2. F# objects are immutable by default

My understanding is that these Ids cannot be generated externally but must be generated from the internals of Marten since it requires access to the database context (having raw access to the table etc...)

Edit: okay, was tired and read too fast the code about Comb Guids. just figured comb guids can be generated externally from my code as they actually only depend on a timestamp. I guess I'll just create and inject a Guid generation service in my project that will use Marten's CombGuidIdGeneration.NewGuid() method behind the scenes.

@jeremydmiller
Copy link
Member

@nkosi23 Can you reverse the rename of ValueTypeInfo to StrongTypedIdInfo? The earlier name was to reflect that that was used within LINQ and possibly independently of the strong typed ids

@nkosi23
Copy link
Author

nkosi23 commented Jun 24, 2024

@jeremydmiller Thanks for the feedback, I've just reverted it

@jeremydmiller jeremydmiller merged commit 224af3f into JasperFx:master Jun 27, 2024
9 of 11 checks passed
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.

None yet

2 participants