Aspnet-api-versioning: How to change the type of an EDM property?

[StevenLiekens]

Hi,

I have an OData endpoint with a _DateTimeOffset_ (Edm.DateTimeOffset) property that I'm trying to change to _DateTime_ (Edm.Date) without renaming it. Or vice versa. I can't seem to make this work with versioning.

Using the SwaggerODataSample to demonstrate the problem.

Order.cs

public class Order
{
    /// <summary>
    /// Gets or sets the date and time when the order was created.
    /// </summary>
    /// <value>The order's creation date.</value>
+    [Obsolete("Replaced by DateTimeOffset in V4.")]
    public DateTime CreatedDate { get; set; } = DateTime.Now;

+    /// <summary>
+    /// Gets or sets the date, time and UTC offset when the order was created.
+    /// </summary>
+    /// <value>The order's creation date.</value>
+    public DateTimeOffset CreatedDateV4 { get; set; } = DateTimeOffset.UtcNow;
}

OrderModelConfiguration.cs

    public class OrderModelConfiguration : IModelConfiguration
    {
        /// <inheritdoc />
        public void Apply( ODataModelBuilder builder, ApiVersion apiVersion, string routePrefix )
        {
            var order = builder.EntitySet<Order>( "Orders" ).EntityType.HasKey( o => o.Id );

+           if ( apiVersion < ApiVersions.V4 )
+           {
+               // CreatedDate as Edm.Date
+               order.Ignore( o => o.CreatedDateV4 );
+               order.Property( o => o.CreatedDate).AsDate();
+           }
+           else
+           {
+               // CreatedDate as Edm.DateTimeOffset
+               order.Ignore( o => o.CreatedDate );
+               order.Property(o => o.CreatedDateV4 ).Name = nameof(Order.CreatedDate);
+           }
        }
    }

Expected behavior

• V1-V3 should return _"createdDate"_ as an Edm.Date
• V4 should return _"createdDate"_ as an Edm.DateTimeOffset

Actual behavior

There seems to be some model caching going on that prevents OData from formatting the response according to the requested API version.

• If I call V1-V3 first then the formatter will only format _"createdDate"_ as Edm.Date when using V4
• If I call V4 first then the formatter will only format _"createdDate"_ as Edm.DateTimeOffset when using any other version

This can result in exceptions because _DateTimeOffset_ can't be converted to _Date_.

Microsoft.OData.ODataException: An incompatible primitive type 'Edm.DateTimeOffset[Nullable=False]' was found for an item that was expected to be of type 'Edm.Date[Nullable=False]'.
   at Microsoft.OData.ValidationUtils.ValidateMetadataPrimitiveType(IEdmTypeReference expectedTypeReference, IEdmTypeReference typeReferenceFromValue)
   at Microsoft.OData.ValidationUtils.ValidateIsExpectedPrimitiveType(Object value, IEdmPrimitiveTypeReference valuePrimitiveTypeReference, IEdmTypeReference expectedTypeReference)
   at Microsoft.OData.WriterValidator.ValidateIsExpectedPrimitiveType(Object value, IEdmPrimitiveTypeReference valuePrimitiveTypeReference, IEdmTypeReference expectedTypeReference)
   at Microsoft.OData.JsonLight.ODataJsonLightValueSerializer.WritePrimitiveValue(Object value, IEdmTypeReference actualTypeReference, IEdmTypeReference expectedTypeReference)
   at Microsoft.OData.JsonLight.ODataJsonLightPropertySerializer.WritePrimitiveProperty(ODataPrimitiveValue primitiveValue, Boolean isOpenPropertyType)
   at Microsoft.OData.JsonLight.ODataJsonLightPropertySerializer.WriteProperty(ODataProperty property, IEdmStructuredType owningType, Boolean isTopLevel, IDuplicatePropertyNameChecker duplicatePropertyNameChecker, ODataResourceMetadataBuilder metadataBuilder)
   at Microsoft.OData.JsonLight.ODataJsonLightPropertySerializer.WriteProperties(IEdmStructuredType owningType, IEnumerable`1 properties, Boolean isComplexValue, IDuplicatePropertyNameChecker duplicatePropertyNameChecker, ODataResourceMetadataBuilder metadataBuilder)
   at Microsoft.OData.JsonLight.ODataJsonLightWriter.StartResource(ODataResource resource)
   at Microsoft.OData.ODataWriterCore.<>c__DisplayClass121_0.<WriteStartResourceImplementation>b__0()
   at Microsoft.OData.ODataWriterCore.InterceptException(Action action)
   at Microsoft.OData.ODataWriterCore.WriteStartResourceImplementation(ODataResource resource)
   at Microsoft.OData.ODataWriterCore.WriteStart(ODataResource resource)
   at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteResource(Object graph, ODataWriter writer, ODataSerializerContext writeContext, IEdmTypeReference expectedType)
   at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteObjectInline(Object graph, IEdmTypeReference expectedType, ODataWriter writer, ODataSerializerContext writeContext)
   at Microsoft.AspNet.OData.Formatter.Serialization.ODataResourceSerializer.WriteObject(Object graph, Type type, ODataMessageWriter messageWriter, ODataSerializerContext writeContext)
   at Microsoft.AspNet.OData.Formatter.ODataOutputFormatterHelper.WriteToStream(Type type, Object value, IEdmModel model, ODataVersion version, Uri baseAddress, MediaTypeHeaderValue contentType, IWebApiUrlHelper internaUrlHelper, IWebApiRequestMessage internalRequest, IWebApiHeaders internalRequestHeaders, Func`2 getODataMessageWrapper, Func`2 getEdmTypeSerializer, Func`2 getODataPayloadSerializer, Func`1 getODataSerializerContext)
   at Microsoft.AspNet.OData.Formatter.ODataOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding)
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextResultFilterAsync>g__Awaited|29_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeResultFilters()
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeFilterPipelineAsync>g__Awaited|19_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

PS: I was able to produce this exception in both v4.1.1 and v5.0.0-rc.1

[commonsensesoftware]

What you're doing looks correct to me. This is exactly the scenario it was meant to solve. I'm 98% sure it's not working as expected due to the bug you reported in #675. The _Model Substitution_ feature will take your code model and make it look the way the EDM says it should. However, if the wrong EDM model is selected, then it all falls down. Once I get the other fix out, I suspect this will be resolved.

[StevenLiekens]

Thanks for the quick follow-up. Can this be the same bug if I originally discovered this problem in v4.1.1?

I'm starting to suspect that this might not even be a problem with OData.Versioning. As far as I can tell, the right model is selected whenever I do a request after a cold start.

e.g.

1. ✔ When I start my API and do a GET with api-version=1.0 then I get { "createdDate": "2020-10-17" }
2. ❌ The next GET with api-version=4.0 results in { "createdDate": "2020-10-17T12:12:12.121+02:00" }
   
   
   
   • (this is wrong because the actual offset should be +00:00 but OData converts it to a local date time)
   
   
3. ✔ If I then restart my API and repeat the GET with api-version=4.0 then I get { "createdDate": "2020-10-17T12:12:12.121Z" }
4. ❌ The next GET with api-version=1.0 results in the exception from above

[commonsensesoftware]

Interesting. I've setup your scenario for a repro, but it seems to be working. I used the OData Basic Sample as the baseline.

I changed Order.cs as follows:

```c#
public class Order
{
public int Id { get; set; }
public DateTime CreatedDate { get; set; } = DateTime.Now;
public DateTimeOffset CreatedDateV2 { get; set; } = DateTimeOffset.Now;
public DateTimeOffset EffectiveDate { get; set; } = DateTimeOffset.Now;
[Required]
public string Customer { get; set; }
}

I changed the **OrderConfiguration.cs** as follows:

```c#
var order = ConfigureCurrent( builder );

if ( apiVersion == V1 )
{
    order.Property( o => o.CreatedDate ).AsDate();
    order.Ignore( o => o.CreatedDateV2 );
}
else
{
    order.Ignore( o => o.CreatedDate );
    order.Property( o => o.CreatedDateV2 ).Name = nameof( Order.CreatedDate );
}

api/v1/$metadata reports:

<EntityType Name="Order">
  <Key>
    <PropertyRef Name="id"/>
  </Key>
  <Property Name="id" Type="Edm.Int32" Nullable="false"/>
  <Property Name="createdDate" Type="Edm.Date" Nullable="false"/>
  <Property Name="effectiveDate" Type="Edm.DateTimeOffset" Nullable="false"/>
  <Property Name="customer" Type="Edm.String" Nullable="false"/>
</EntityType>

api/v2/$metadata reports:

<EntityType Name="Order">
  <Key>
    <PropertyRef Name="id"/>
  </Key>
  <Property Name="id" Type="Edm.Int32" Nullable="false"/>
  <Property Name="createdDate" Type="Edm.DateTimeOffset" Nullable="false"/>
  <Property Name="effectiveDate" Type="Edm.DateTimeOffset" Nullable="false"/>
  <Property Name="customer" Type="Edm.String" Nullable="false"/>
</EntityType>

api/v1/orders/42 yields:

{
  "@odata.context": "http://localhost:5000/api/v1/$metadata#Orders/$entity",
  "id": 42,
  "createdDate": "2020-10-24",
  "effectiveDate": "2020-10-24T09:15:17.5542017-07:00",
  "customer": "Bill Mei"
}

api/v2/orders/42 yields:

{
  "@odata.context": "http://localhost:5000/api/v2/$metadata#Orders/$entity",
  "id": 42,
  "createdDate": "2020-10-24T09:15:57.0782421-07:00",
  "effectiveDate": "2020-10-24T09:15:57.0782912-07:00",
  "customer": "Bill Mei"
}

Full transparency, this does include a couple of fixes going into RC.2, but beyond the previous bug, I don't see how it would affect this. Those changes will be rolling out soon.

What type of operation are you performing? I was trying GET operations only. Based on what you've said, I _think_ it may be related to OData itself. The difference in formats is definitely strange. If I know the operations you're trying, I can try to repro those as well.

[StevenLiekens]

Maybe it was the same bug after all. I can still reproduce the problem with your exact setup except I changed the version back to RC.1: https://github.com/StevenLiekens/odata-versioning-issue-676

[commonsensesoftware]

It's definitely _possible_ it is a bug in both. 4.0 to 5.0 is a massive change under the hood. It may have already been in there. I've merged the latest changes, but I don't have RC.2 out just yet. If you're working from the source, you might give it a whirl.

[StevenLiekens]

I added a branch that shows the same problem using 4.1.1
https://github.com/StevenLiekens/odata-versioning-issue-676/tree/v4.1.1

[commonsensesoftware]

I was thinking about it. It _could_ be a bug in OData that was fixed. API Versioning takes the lowest version dependency possible. What happens in the v4.1.1 version if you bump up OData to the latest, supported version explicitly?

[StevenLiekens]

Good idea, but unfortunately the result is the same. Tested with Microsoft.AspNetCore.OData 7.5.1.

[commonsensesoftware]

That seems to point back that the issue is, in fact, with in API versioning. =/ We're definitely eliminating what it's not.

[StevenLiekens]

The last clue I can give you is that the problem disappears when I remove this line from the model configuration:

-order.Property( o => o.CreatedDateV2 ).Name = nameof( Order.CreatedDate );

So... there must be something that compares properties by EDM name somewhere and then makes wrong decisions.

[commonsensesoftware]

I put out RC.2. See if that makes a difference. It _should_ work. 🤞🏽

[StevenLiekens]

Unfortunately I still get the same behavior. 🤔

I think the code in your previous comment only _seems_ to work because you used Now for both properties, which sort of hides the problem.

It's much more obvious when you use Today and UtcNow.

public class Order
{
    public int Id { get; set;  }
    public DateTime CreatedDate { get; set; } = DateTime.Today;
    public DateTimeOffset CreatedDateV2 { get; set; } = DateTimeOffset.UtcNow;
    [Required]
    public string Customer { get; set; }
}

With that setup, you'll see a missing timestamp and a nonzero UTC offset when you call v2 after v1, or a crash when you call v1 after v2.

[commonsensesoftware]

That is strange. Maybe it is OData after all. I'll try this combination and post the results. If it works, I'll attach the repro

[commonsensesoftware]

Interesting development...

I _think_ you may be correct that something is being cached, but I'm not sure what or where - yet. I'm finally able to repro the scenario using my original steps (e.g. Today vs UtcNow doesn't matter - which is good).

Issuing requests in this order:

• api/v1/orders/42
• api/v2/orders/42

Yields the correct and expected behavior. However...

Issuing requests in this order:

• api/v2/orders/42
• api/v1/orders/42

Repros the scenario. As best as I can tell from the API versioning code, the correct model is being selected. I _suspect_ that something in the OData stack is caching or otherwise holding on to something. They won't consider it a bug because their design intrinsically expects a 1:1 mapping between route and EDM. That simply doesn't work for API versioning.

I need to study the stack trace of the exception and examine all of the types. _"One of them is doing their own thing..."_. This is the first time I've seen this, but it should be flushed out because I won't be surprised if other edge cases related to caching information about the EDM could lead to similar problems.

If you happen to find anything, please share.

[commonsensesoftware]

UPDATE: After a lot of digging, our assumptions were right. It is a caching issue. It was very painful to track down, but here's how/where it happens:

ODataResourceSerializer.WriteResource → ResourceContext.AsEdmResourceObject → TypedEdmStructuredObject → TryGetPropertyValue → _propertyGetterCache

So what happens is that if you request things in the order of:

• api/v2/orders/42
• api/v1/orders/42

On the first pass, a mapping of createdDate is made to CreateDateV2. On the 2nd+ pass, createdDate still maps to CreatedDateV2 when it should map to CreatedDate. The correct EDM property in the model is selected, but the property CLR property is selected and a DateTimeOffset value is retrieved instead of a DateTime. I believe the inverse order works because DateTime can upconvert to DateTimeOffset or there's no applicable validation. I'm not 100% sure.

I _think_ in the strictest sense, this _may_ be fixable; however, it's so far down in the OData stack, it would require a tremendous amount of code replacement/substitution. There is not an easy way to get in front of this.

I've tried a couple of different alternate solutions, but I can't get them to work the way you would want. The only way I think you can make it work would be to use String in the mapping. There is no formal date-encoding in JSON. It's modeled as a string in a specific format. You _could_ force it to work by reading and writing to the CreatedDate property as a String. You can then use model method, extension methods, etc to get the typed form. This could even occur when the property is written to. In order to preserve validation in an idiomatic way, you could implement IValidatableObject with special validation code for the CreatedDate property modeled as a string.

Of course all that craziness is trying to use a single data model. It's less than ideal, but if you version the Order type itself as two different types, you get the desired result. You can _abuse_ inheritance to remove some of the duplication. This configuration will work:

```c#
public abstract class OrderBase
{
protected OrderBase() { }
public int Id { get; set; }
public DateTimeOffset EffectiveDate { get; set; } = DateTimeOffset.Now;
[Required]
public string Customer { get; set; }
}

public class Order : OrderBase
{
public DateTime CreatedDate { get; set; } = DateTime.Now;
}

public class OrderV2 : OrderBase
{
public DateTimeOffset CreatedDate { get; set; } = DateTimeOffset.Now;
}

public class OrderModelConfiguration : IModelConfiguration
{
private static readonly ApiVersion V1 = new ApiVersion( 1, 0 );

public void Apply( ODataModelBuilder builder, ApiVersion apiVersion, string routePrefix )
{
    if ( routePrefix != "api/v{version:apiVersion}" )
    {
        return;
    }

    if ( apiVersion == V1 )
    {
        var order = builder.EntitySet<Order>( "Orders" ).EntityType;

        order.HasKey( p => p.Id );
        order.Property( o => o.CreatedDate ).AsDate();
    }
    else
    {
        var order = builder.EntitySet<OrderV2>( "Orders" ).EntityType;

        order.Name = nameof( Order );
        order.HasKey( p => p.Id );
    }
}

}
```

Ultimately, this issue is down in the OData stack. On one hand, that's _good_ because API Versioning is doing the right thing. On the other hand, it's _bad_ because it's a PIA to deal with and, honestly, a POLA violation (IMHO).

[commonsensesoftware]

I believe this issue has be resolved and has reached its conclusion. If none of these solution worked for you, we can re-open the issue can discuss further. Thanks.