OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
457 stars 158 forks source link

[Question] Is it possible to add support for a custom value type? #732

Open julealgon opened 3 years ago

julealgon commented 3 years ago

Suppose I have a custom type that abstracts some concept in a higher level of abstraction than a primitive type does (string, int, etc). I could think of a few such types:

Additionally, some types had support added later, like Spatial types.

What does one need to do to make these custom types available as if they were native constructs in an OData API?

I have not found any documentation around this whatsoever, so I'm asking the owners to see if there is any extension point and how they work.

Sreejithpin commented 3 years ago

One way to do is to use Type definition with an underlying type (say as string) and apply pattern (term) as in the vocab . You can apply regex on it , say for Temperature (regex like length 3, ends with F or C ) refer https://github.com/oasis-tcs/odata-vocabularies/blob/master/vocabularies/Org.OData.Validation.V1.xml#L67

julealgon commented 3 years ago

@Sreejithpin this is very promising, but could you provide a more complete example of what you mean? Would I need to do this inside my EdmModel building logic? How can I create the type definition there and apply the pattern? How do I tie this type definition to my own custom .Net Temperature type?

julealgon commented 3 years ago

I made a few more tests today, but I still couldn't make it work.

First, I created a new TypeDefinition in the EdmModel, somewhat like this:

        IEdmModel GetEdmModel()
        {
            var builder = new ODataConventionModelBuilder();
            var forecastType = builder.EntitySet<WeatherForecast>("Forecasts").EntityType;
            forecastType.Ignore(f => f.Temperature);

            var x = new EdmTypeDefinition("MyCompany.ODataTests.Model", "Temperature", EdmPrimitiveTypeKind.String);

            var model = (EdmModel)builder.GetEdmModel();
            model.AddElement(x);
            var parentType = (EdmEntityType)model.FindDeclaredType("MyCompany.ODataTests.Model.WeatherForecast");
            parentType.AddStructuralProperty("Temperature", new EdmTypeDefinitionReference(x, false));

            return model;
        }

After adding this, I started getting exceptions while deserializing the property with Temperature type. Then I made a few changes to create my own serializer:

Changes on startup:

                    .MapODataRoute("odata", "", builder => builder
                        .AddService(ServiceLifetime.Singleton, (Func<IServiceProvider, IEnumerable<IODataRoutingConvention>>)(sp => ODataRoutingConventions.CreateDefaultWithAttributeRouting("odata", endpoints.ServiceProvider)))
                        .AddService(ServiceLifetime.Singleton, _ => edmModelProvider.GetEdmModel())
                        .AddService<ODataSerializerProvider, SerializerProvider>(ServiceLifetime.Singleton))

Custom serializer provider:

    public class SerializerProvider : DefaultODataSerializerProvider
    {
        public SerializerProvider(IServiceProvider serviceProvider)
            : base(serviceProvider)
        {
        }

        public override ODataEdmTypeSerializer GetEdmTypeSerializer(IEdmTypeReference edmType)
        {
            return edmType.AsTypeDefinition().TypeDefinition().FullTypeName() == "MyCompany.ODataTests.Model.Temperature"
                ? new TemperatureSerializer()
                : base.GetEdmTypeSerializer(edmType);
        }
    }

And the serializer implementation (empty just to place breakpoints):

    public class TemperatureSerializer: ODataEdmTypeSerializer
    {
        public TemperatureSerializer()
            : base(ODataPayloadKind.Property)
        {

        }

        public override void WriteObject(object graph, Type type, ODataMessageWriter messageWriter, ODataSerializerContext writeContext)
        {
            base.WriteObject(graph, type, messageWriter, writeContext);
        }
    }

However, even though the serializer provider and the actual serializer are built correctly (I can see they are hit while debugging), I'm now getting an exception inside the ToEdmTypeReference method in the EdmLibHelpers class. It seems that it is reaching an unsupported branch because the TypeDefinition type is not handled:

        public static IEdmTypeReference ToEdmTypeReference(this IEdmType edmType, bool isNullable)
        {
            switch (edmType.TypeKind)
            {
            case EdmTypeKind.Collection:
                return new EdmCollectionTypeReference(edmType as IEdmCollectionType);
            case EdmTypeKind.Complex:
                return new EdmComplexTypeReference(edmType as IEdmComplexType, isNullable);
            case EdmTypeKind.Entity:
                return new EdmEntityTypeReference(edmType as IEdmEntityType, isNullable);
            case EdmTypeKind.EntityReference:
                return new EdmEntityReferenceTypeReference(edmType as IEdmEntityReferenceType, isNullable);
            case EdmTypeKind.Enum:
                return new EdmEnumTypeReference(edmType as IEdmEnumType, isNullable);
            case EdmTypeKind.Primitive:
                return _coreModel.GetPrimitive((edmType as IEdmPrimitiveType).PrimitiveKind, isNullable);
            default:
                throw Error.NotSupported(SRResources.EdmTypeNotSupported, edmType.ToTraceString());
            }
        }

EDIT:

Here is the stacktrace I'm getting now:

System.NotSupportedException: MyCompany.ODataTests.Model.Temperature is not a supported EDM type. at Microsoft.AspNet.OData.Formatter.EdmLibHelpers.ToEdmTypeReference(IEdmType edmType, Boolean isNullable) at Microsoft.AspNet.OData.Formatter.EdmLibHelpers.GetEdmTypeReference(IEdmModel edmModel, Type clrType) at Microsoft.AspNet.OData.Formatter.ClrTypeCache.GetEdmType(Type clrType, IEdmModel model) at Microsoft.AspNet.OData.Formatter.Serialization.ODataSerializerContext.GetEdmType(Object instance, Type type) at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.CreateStructuralProperty(IEdmStructuralProperty structuralProperty, ResourceContext resourceContext) at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.CreateStructuralPropertyBag(SelectExpandNode selectExpandNode, ResourceContext resourceContext) at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.CreateResource(SelectExpandNode selectExpandNode, ResourceContext resourceContext) at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteResourceAsync(Object graph, ODataWriter writer, ODataSerializerContext writeContext, IEdmTypeReference expectedType) at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSetSerializer.WriteResourceSetAsync(IEnumerable enumerable, IEdmTypeReference resourceSetType, ODataWriter writer, ODataSerializerContext writeContext) at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSetSerializer.WriteObjectAsync(Object graph, Type type, ODataMessageWriter messageWriter, ODataSerializerContext writeContext) at Microsoft.AspNet.OData.Formatter.ODataOutputFormatterHelper.WriteToStreamAsync(Type type, Object value, IEdmModel model, ODataVersion version, Uri baseAddress, MediaTypeHeaderValue contentType, IWebApiUrlHelper internaUrlHelper, IWebApiRequestMessage internalRequest, IWebApiHeaders internalRequestHeaders, Func2 getODataMessageWrapper, Func2 getEdmTypeSerializer, Func2 getODataPayloadSerializer, Func1 getODataSerializerContext)

The TypeKind for my type returns TypeDefinition.

Any extra help here is appreciated.

Sreejithpin commented 3 years ago

Thanks for the update. We will check on this and update you

julealgon commented 3 years ago

@Sreejithpin do you have any news on this topic?

zejji commented 3 years ago

I would also find this helpful. I am using Ulids, which are structs, as primary keys for my entities but cannot find any documentation re how to set up custom primitive types.

julealgon commented 3 years ago

@xuzhg is there anybody that can provide further guidance on this?

Sreejithpin commented 3 years ago

@julealgon Sorry it looks like it was off the radar, will check if we need to update ToEdmTypeReference and will update you this week itself

pkanavos commented 3 years ago

@Sreejithpin this is a bit more urgent now that .NET 6 adds DateOnly and TimeOnly types.

zejji commented 3 years ago

@Sreejithpin - Are you able to provide any update on this? Thanks!

Sreejithpin commented 3 years ago

Were looking for making the update, but looked like there will be more pieces involved if we make the change. Can we sit sometime to see how it would affect you and what kind of scenarios you are looking . Please let me know

zejji commented 3 years ago

@Sreejithpin - Many thanks for the reply.

I can only speak for my use case, but the issue is that I have a DbContext which uses Ulids as primary keys (stored as a byte array in Postgres via a ValueConverter). Ulids are structs which are used in a similar manner to Guids, but have the additional advantage that they are sortable - for more information as to the reasoning behind using Ulids please see this article.

Accordingly, the lack of support in OData for the custom value types currently makes it completely impossible to use OData in my project.

julealgon commented 2 years ago

Folks, bump here. There should be some way to add support for custom value types, even if it is somewhat complex to do at first.

@xuzhg have you seen this particular issue? I saw several blog posts from you about OData v8 so copying you here as well.

julealgon commented 2 years ago

My apologies for bumping this yet again, but to me this is a very important aspect that we are glossing over. Can someone please respond on this? The same question on SO also doesn't have an answer and is getting traction from other people having the same concern.

@habbes / @mikepizzo / @xuzhg

mikepizzo commented 2 years ago

The right answer should be the use of type definitions. If that isn't working, we should fix it (and provide some examples showing how to make it work).

Sorry this got dropped; Sreejith is no longer on the team, but I will add this to our backlog.

mikepizzo commented 2 years ago

To verify, creating type definitions in core odata library works, it's just not correctly handled in WebAPI OData libraries, correct? If so, I'll move this to the proper repo. -- thanks!

mikepizzo commented 2 years ago

TypeDefinitions are intended to solve this type of scenario. They are supported in ODL/modelbuilder, but not in webapi odata v7. V8 adds support for custom type mapping, which could provide a solution. @xuzhg will look at building a sample for this.

julealgon commented 2 years ago

To verify, creating type definitions in core odata library works, it's just not correctly handled in WebAPI OData libraries, correct? If so, I'll move this to the proper repo. -- thanks!

This is indeed the case.

Thanks for your support on this one @mikepizzo , much appreciated.

xuzhg commented 2 years ago

@julealgon Let me know your thoughts about this sample.

https://github.com/xuzhg/WebApiSample/tree/main/v8.x/CreateNewTypeSample

julealgon commented 2 years ago

@julealgon Let me know your thoughts about this sample.

https://github.com/xuzhg/WebApiSample/tree/main/v8.x/CreateNewTypeSample

Damn that looks really promising, but at the same time unexpectedly convoluted 😆

I'll take a deeper look at it later, but I already appreciate the effort and the sample responses in the readme. That's exactly what I wanted to do at one point when I originally asked this question ages ago.

This called my attention however:

https://github.com/xuzhg/WebApiSample/blob/8d1248a41055fa5ac73d1efc96b1e912074b1098/v8.x/CreateNewTypeSample/CreateNewTypeSample/Extensions/MyPrimivieReserializer.cs#L35-L47

Is that even doing anything (my assumption is "no" but I just wanted to double check)?

xuzhg commented 2 years ago

@julealgon Let me know your thoughts about this sample. https://github.com/xuzhg/WebApiSample/tree/main/v8.x/CreateNewTypeSample

Damn that looks really promising, but at the same time unexpectedly convoluted 😆

I'll take a deeper look at it later, but I already appreciate the effort and the sample responses in the readme. That's exactly what I wanted to do at one point when I originally asked this question ages ago.

This called my attention however:

https://github.com/xuzhg/WebApiSample/blob/8d1248a41055fa5ac73d1efc96b1e912074b1098/v8.x/CreateNewTypeSample/CreateNewTypeSample/Extensions/MyPrimivieReserializer.cs#L35-L47

Is that even doing anything (my assumption is "no" but I just wanted to double check)?

Do nothing except let the OData lib to skip the validation.

mikepizzo commented 2 years ago

Note that the service should NOT be defining new Edm.* types, as the Temperature sample shows. This is against the protocol and will break common client apps.

TypeDefinitions, as used in the Dist example, provides a cleaner, compatible way to extend the type system.

kooshan commented 1 year ago

@mikepizzo @xuzhg

Great solution! Thanks. It would be nice to see this incorporated into the lib API for a more eloquent and gentle experience when working with value objects. I assume EFCore team has already announced some ideas to explore on value types for upcoming .Net 8 release as well.

In my case, some of these value types are actually the entity keys themselves. Is there any workaround to introduce value type keys into the model, since .HasKey() does not work even after introducing new TypeDefinitions.

awbacker commented 1 year ago

I'm still struggling with this too.

I can't deserialize a JsonDocument, nor a custom type to wrap it, nor a Dictionary<string,string>. I'm happy to write custom serializers, I usually have to and it is easy elsewhere.