JasperFx / marten

.NET Transactional Document DB and Event Store on PostgreSQL
https://martendb.io
MIT License
2.81k stars 445 forks source link

Overriding Identity Types + Overriding Property Types in queries. #3257

Closed nkosi23 closed 3 months ago

nkosi23 commented 3 months ago

Another hello from the F# world :)

In F# it is common to wrap identity types in Single Case discriminated unions such as:

type OrderId = Id of Guid

The goal and culture are to encode the domain and business rules in the type system as much as possible to leverage the power of compile-time checks to design and evolve models safely. Many folks are also starting to do it in C# (as part of DDD), but it is much more common in F# due to the ease and conciseness of designing such types.

The above type is not exactly a Guid, it is instantiated like so: OrderId.Id(Guid.NewGuid()). The most commonly used F# to JSON serializer unwraps such types and stores them as plain strings. Therefore the problem is not with the persistence format, but with the .NET type constraints introduced by Marten (ie. trying to use such a type as a primary key throws an exception, as explained in the documentation)

In this context, it would be very useful to be able to instruct Marten to override the type of primary keys, but also of properties in queries.

First of all by adding the following overload to the identity function to have something like the below to tell Marten that it should consider that the primary key has the provided generic type in the database:

storeOptions.Schema.For<Order>()
        .Identity<Order,Guid>(fun x -> x.Id)

Then another thing that could be useful is a Where<T1,T2> overload on IMartenQueryable so that we can do something like the below, for example when querying an Invoice document having an OrderId property (OrderId cannot be converted to Guid by default. Moreover, even if such a cast could be translated to SQL by the Linq provider, it would be quite cumbersome to define casts explicitly on all such types - especially since they are supposed to be one-liners):

documentSession.Query<Invoice>()
      .Where<Invoice,Guid>(x => x.OrderId, x => x == orderId)

The first lambda would be used to select the property, while the second lambda would be the one actually being turned into SQL using the type of the second generic parameter.

I think this would be useful beyond F#, as it would give users full flexibility to decouple the JSON representation of data from their representation in .NET. It would empower users to model their domain in the way making the most sense for them from a business perspective and use custom JsonConverters for serialization. These two methods would enable quite a lot of use cases.

What are your thoughts on this idea?

nkosi23 commented 3 months ago

Just found out that @oskardudycz also brought up this topic that was apparently already a frequent request a couple of years ago in this blog post.

I feel my proposal is a simpler and more versatile approach to address this need than the workaround suggested in the blog post (as for example, it wouldn't work well with F# as inheritance is not idiomatic and even discouraged. And this workaround also seems to have the drawback of duplicating the id in two different properties - which consumes meaningful additional storage space at scale).

I am about to unwrap all our types for time being since such a change would be backward compatible, but I'm a bit sad to forfeit this type safety convenience. Quickly defining these id wrapper types is a second nature when writing F#..

I'm just stumbling across this because I was initially defining separate DTOs objects for every document type to decouple serialization concerns, but the development experience really sucked. Too many layers of indirection, massive productivity and joy killer. I just discovered that the F# to JSON serializer is actually quite powerful such that persisting the domain entities directly is more viable than I imagined. The only show stopper is this identity type thing.

nkosi23 commented 3 months ago

Just added a PR to illustrate the approach I have in mind, the final version should not contain much more code, just needing to do the same for the Query part but haven't dug into this code yet. Just wanted to post my thinking in the meantime.

haefele commented 3 months ago

Seems very similar to the Typed-ID work from jeremy the last days. See these discord messages and this branch.

But could also be different situation because of F#.

nkosi23 commented 3 months ago

Thanks a lot for the pointer! This looks close indeed, but is also different in spirit from what I can tell. @jeremydmiller is doing it the right way by taking ownership of the creation of type resolvers / value converters, because he aims to add more general support for arbitrary value objects (which is not what I had in mind).

My approach that is aiming to keep the work needed to support this use case to a minimum (but I'm not sure it's truly viable as a bunch of tests are failing in the Github Pipeline) - is basically about :

  1. keeping the supported id types intact (not expanding them),
  2. letting the developer handle serialization to the supported id types themselves through JSON value converters, and
  3. giving type hints to Marten so that code compiles.

If Jeremy is able to get his approach work, it is the superior one but way more complex to implement.

However, one thing that would still be a blocker for F# folks is the situation with Linq querying. We do not use inheritance and F# is very strict about type coercion (implicit conversions are not really a thing). Moreover, deconstructing our type OrderId = Id of Guid wrapper types is not as easy as accessing a property. It requires a pattern matching expression which will interfere with the Linq to SQL translation (in addition to the fact that having to write pattern matching code for every linq query would be a PITA)

Therefore, for querying a method similar to the one I have proposed in my PR would still be needed I'm afraid, something like:

public static IMartenQueryable<T1> Where<T1, T2>(
        this IMartenQueryable<T1> queryable,
        Expression<Func<T1, object>> propertySelector,
        Expression<Func<T2, bool>> predicate)
jeremydmiller commented 3 months ago

Duplicate with https://github.com/JasperFx/marten/issues/2487, and I'm unenthusiastic about the proposed solution (which isn't really simpler mechanically and would actually require more intrusive changes to the LINQ support).

We're going to accommodate value types such that Marten's LINQ support will see "SpecialValueTypeThatIsReallyWrapperAround Guid" and treat it as a Guid for the JSON & database parameter support. And we'll pull that off w/o having to require any marker interfaces or abstract classes or whatever else that makes F# devs get all snooty.