ModelBuilder doesn't honor composite key order

[mitselplik]

I have a DbContext on a SQL Server table with a DbSet<PropertyYearLayer> property. The primary key of this table is by Year, SupNumber, and PropertyId. Here is a cut of the DbContext class.

private void OnModelCreating(Microsoft.EntityFrameworkCore.Metadata.Builders.EntityTypeBuilder<PropertyYearLayer> entity)
{
    entity.HasKey(e => new { e.Year, e.SupNumber, e.PropertyId });
    ...
}

The model builder class I use to construct the OData EdmModel similarly defines the key:

public IEdmModel GetEdmModel(IServiceProvider serviceProvider)
{
    var builder = new ODataConventionModelBuilder(serviceProvider);
    EntityTypeConfiguration<PropertyYearLayer> PropertyYearLayerConfiguration 
        = builder.EntitySet<PropertyYearLayer>("PropertyYearLayer").EntityType;
    PropertyYearLayerConfiguration
        .HasKey(r => new { r.Year, r.SupNumber, r.PropertyId });
...
}

My controller class has a Get() method defined to allow retrieving a single record by composite key:

[EnableQuery(PageSize = 100)]
public IQueryable<PropertyYearLayer> Get([FromODataUri] int keyYear, [FromODataUri] int keySupNumber, [FromODataUri] int keyPropertyId)
    => _context.PropertyYearLayer.Where(r => r.Year == keyYear && r.SupNumber == keySupNumber && r.PropertyId == keyPropertyId);

When I test the method by entering the following URL, https://localhost:44308/odata/PropertyYearLayer(2019,0,1061), I expect to see a record for year = 2019, SupNumber=0, and PropertyId = 1061.

However, it appears that the router is ignoring the order of...well...pretty much everything...and deciding that the properties must be in alphabetical order I guess? Is this by convention? Or is this a bug?

[xuzhg]

@mitselplik Thanks for pointing this.

First, ODL processes the composite key value like PropertyYearLayer(2019,0,1061) by its position. That is, the first value (2019) will bind to the first key defined on the entity type, the second (0) will bind to the second key defined on the entity type, etc. It's by design.

The question is the key position when you define the key using .HasKey(r => new { r.Year, r.SupNumber, r.PropertyId });, In Web API OData, by default composite key position is ordered: 1) by its order value 2) then by its name.

By default, the order value for all keys are same value (0), so, your key position by default looks like:

<Key>
     <PropertyRef Name="PropertyId" />
     <PropertyRef Name="SupNumber" />
     <PropertyRef Name="Year" />
</Key>

So, it answers why you got "PropertyId=2019, SupNumber=0, Year=1061" when you filed a request like PropertyYearLayer(2019,0,1061).

Two options to resolve your problem: 1) It's simple to call the API using named value like: (without any code change) PropertyYearLayer(Year=2019,SupNumber=0,PropertyId=1061)

2) Change the property order when building the model.

For example, add the followings codes after call HasKey(....)

PropertyYearLayerConfiguration.Properties.First(c => c.Name == "Year").Order = 1;
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "SupNumber").Order = 2;
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "PropertyId").Order = 3;

Hope it can help, please let me know if it can't work.

[mitselplik]

So, to summarize, unless explicitly told otherwise, the model builder ignores the order in which the properties are declared for the composite key in the anonymous type, it ignores the order in which the properties are defined in the concrete class, and it ignores the order of the key parameters declared for the matching function.

I have to be honest, this feels like a bug, or if not a bug, at the very least a standard that should be changed. The library is smart enough to determine what the key values should be called from examining the anonymous type. Unless some limitation exists that prevents it, (in my opinion) the library should also honor the order in which those properties occur in the anonymous type as well (rather than order them alphabetically) when order value isn't specified.

So, I would propose the following change:

In Web API OData, by default composite key position is ordered:

1. by its order value
2. then by its name. then by the order properties are specified in the anonymous type defining the composite key.

One last note, thank you @xuzhg . This suggestion appears to have worked in the meantime:

PropertyYearLayerConfiguration.Properties.First(c => c.Name == "Year").Order = 1;
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "SupNumber").Order = 2;
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "PropertyId").Order = 3;

I'm going to leave the issue open however in hopes that my suggestion/request will either by approved or declined.

[mitselplik]

For the benefit of anyone who lands on this page from an Internet search, I wanted to post what I did to ultimately resolve the issue. I used @xuzhg's suggestion and simply created a different extension method that essentially does exactly what is needed to have the model setup so that the controller parameter values are in the order I expected:

internal static class EntityTypeConfigurationExtensions
{
    /// <summary>
    /// Configures the key properties for this entity type where the explicit order given to the component
    /// values of the composite key must be used for any controller routing function parameters.
    /// Otherwise, when not explicitly defined (as is the case with the 
    /// <see cref="EntityTypeConfiguration{TEntityType}.HasKey{TKey}(Expression{Func{TEntityType, TKey}})"/>),
    /// an alphabetical order is assumed for the component values.
    /// </summary>
    /// <typeparam name="TEntityType">The backing CLR type for this Microsoft.OData.Edm.IEdmEntityType.</typeparam>
    /// <typeparam name="TKey">The type of key.</typeparam>
    /// <param name="entityTypeConfiguration"></param>
    /// <param name="keyDefinitionExpression">A lambda expression representing the property to be used as the primary key.
    /// For example, in C# t => new { t.SupplierId, t.ProductId }.</param>
    /// <returns>Returns itself so that multiple calls can be chained.</returns>
    public static EntityTypeConfiguration<TEntityType> HasCompositeKey<TEntityType, TKey>(
        this EntityTypeConfiguration<TEntityType> entityTypeConfiguration,
        Expression<Func<TEntityType, TKey>> keyDefinitionExpression)
        where TEntityType : class
    {
        entityTypeConfiguration.HasKey(keyDefinitionExpression);

        var propertyInfos = keyDefinitionExpression.ReturnType.GetProperties();
        for (int index = 0; index < propertyInfos.Length; index++)
        {
            var propertyInfo = propertyInfos[index];
            entityTypeConfiguration.Properties.First(c => c.Name == propertyInfo.Name).Order = index + 1;
        }

        return entityTypeConfiguration;
    }
}

[xuzhg]

@mitselplik Thanks for sharing your codes. I'd like to update you why we have this orderby logic. You can find the implementation at: https://github.com/OData/WebApi/pull/1762

[mitselplik]

@xuzhg Thank you for sharing that.
The further edification from that issue would indicate that my code needs to be modified to examine TEntityType for any [Column(...)] attributes on its properties and honor the order specified there before attempting to change the default order according to the properties specified in the anonymous type. Does that sound correct?

[xuzhg]

@mitselplik No, i am not recommending you to change using attribute, just sharing why we have this logic. So, it's up to you to make a decision which one you want to pickup. 😄

[bdebaere]

@xuzhg I agree with @mitselplik that the default HasKey extension method should take into account the column ordering specified in order of priority: 1. via the order given by the Order property, 2. via the ColumnAttribute, 3. via the order in which the properties are given to the method.

entity.HasKey(e => new { e.Year, e.SupNumber, e.PropertyId });

Should give an order of e.Year = 0, e.SupNumber = 1, e.PropertyId = 2.

public class PropertyYearLayer
{
    [Column(Order = 1)]
    public int Year { get; set; }

    [Column(Order = 0)]
    public int SupNumber { get; set; }

    [Column(Order = 2)]
    public int PropertyId { get; set; }
}
entity.HasKey(e => new { e.Year, e.SupNumber, e.PropertyId });

Should give an order of e.Year = 1, e.SupNumber = 0, e.PropertyId = 2.

public class PropertyYearLayer
{
    [Column(Order = 1)]
    public int Year { get; set; }

    [Column(Order = 0)]
    public int SupNumber { get; set; }

    [Column(Order = 2)]
    public int PropertyId { get; set; }
}
entity.HasKey(e => new { e.Year, e.SupNumber, e.PropertyId });
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "Year").Order = 3;
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "SupNumber").Order = 2;
PropertyYearLayerConfiguration.Properties.First(c => c.Name == "PropertyId").Order = 1;

Should give an order of e.Year = 3, e.SupNumber = 2, e.PropertyId = 1.

Let me know what you think. I can work on this if you agree.

[fededim]

I have noticed this problem too, @xuzhg would it be possible to preserve primary keys' order (e.g. automatically increment the Order property) while adding an entity through ODataConventionModelBuilder.AddEntitySet ? Or at least allow the manual method EntityTypeConfiguration.HasKey to specify also the order ?

[ericcanadas]

I have the same problem too. (sorry to dig up the topic) @xuzhg, you suggest to specified named value, which I tried but it does not return anything if the order is not respected. Anyway, le Get() method is not fired !

1. It's simple to call the API using named value like: (without any code change) PropertyYearLayer(Year=2019,SupNumber=0,PropertyId=1061)

Should this work ?