search

OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
854 stars 476 forks source link

Issue with URLs for Related Entities with Referential Constraints #1190

Open ThomasBarnekow opened 6 years ago

ThomasBarnekow commented 6 years ago

Section 4.3.3 (URLs for Related Entities with Referential Constraints) of OData Version 4.0. Part 2: URL Conventions Plus Errata 03 states that "if a navigation property leading to a related entity type has a partner navigation property that specifies a referential constraint, then those key properties of the related entity that take part in the referential constraint MAY be omitted from URLs". I am trying to support an OData route (see Reproduce steps) that is pretty similar to the one provided in the example. However, the corresponding controller method is not entered and when accessing the metadata endpoint, an exception is thrown (see Actual result).

Assemblies affected

Microsoft.AspNetCore.OData.dll (7.0.0-beta1)

Reproduce steps

Model (simplified)

    public class Product
    {
        [Key]
        [MaxLength(100)]
        public string ProductName { get; set; }

        // Navigation Properties

        [Contained]
        public ICollection<ProductVersion> ProductVersions { get; set; }
    }

    // Key defined in DbContext.OnModelCreating()
    public class ProductVersion
    {
        [Required]
        [MaxLength(100)]
        public string ProductName { get; set; }

        [Required]
        [MaxLength(100)]
        public string ProductVersionName { get; set; }

        // Navigation Properties

        [ForeignKey(nameof(ProductName))]
        public Product Product { get; set; }

        [Contained]
        public ICollection<Instance> Instances { get; set; }
    }

    public class Instance
    {
        [Key]
        public int InstanceId { get; set; }

        [Required]
        [MaxLength(100)]
        public string ProductName { get; set; }

        [Required]
        [MaxLength(100)]
        public string ProductVersionName { get; set; }

        // Navigation Properties

        [ForeignKey("ProductName, ProductVersionName")]
        public ProductVersion ProductVersion { get; set; }
    }

DbContext (simplified)

    public class MyDbContext : DbContext
    {
        public MyDbContext()
        {
        }

        public MyDbContext(DbContextOptions<MyDbContext> options) : base(options)
        {
        }

        public DbSet<Instance> Instances { get; set; }

        public DbSet<Product> Products { get; set; }

        public DbSet<ProductVersion> ProductVersions { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            // ProductVersion entity
            modelBuilder.Entity<ProductVersion>()
                .HasKey(pv => new { pv.ProductName, pv.ProductVersionName });
        }
    }

Startup code (simplified)

    public class Startup
    {
        public void ConfigureServices(IServiceCollection services)
        {
            string connectionString = Configuration.GetConnectionString("DefaultConnection");
            services.AddDbContext<MyDbContext>(options => options.UseSqlServer(connectionString));

            services.AddOData();
            services.AddSingleton(GetEdmModel);

            services.AddMvc();
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            app.UseMvc(routeBuilder =>
            {
                var model = app.ApplicationServices.GetService<IEdmModel>();
                routeBuilder.MapODataServiceRoute("ODataRoute", "odata", model);
            });
        }

        private static IEdmModel GetEdmModel(IServiceProvider serviceProvider)
        {
            var builder = new ODataConventionModelBuilder(serviceProvider)
            {
                Namespace = "MyNamespace",
                ContainerName = "MyContainer"
            };

            builder.EntitySet<Product>("Products");

            builder.EntityType<ProductVersion>()
                .HasKey(pv => new { pv.ProductName, pv.ProductVersionName });

            return builder.GetEdmModel();
        }
    }

Controller (simplified)

    public class ProductsController : ODataController
    {
        [HttpPost]
        [ODataRoute("Products({productName})/ProductVersions(ProductName={productVersionName}, ProductVersionName={productVersionName})/Instances")]
        [ODataRoute("Products({productName})/ProductVersions({productVersionName})/Instances")]
        public IActionResult PostInstance(
            [FromODataUri] string productName,
            [FromODataUri] string productVersionName,
            [FromBody] Instance instance)
        {
            return Created(instance);
        }
    }

Expected result

Having successfully created Product and ProductVersion entities, e.g., "My Product" and "1.0.0", using Postman to POST to:

https://localhost:44356/odata/Products('My Product')/ProductVersions('1.0.0')/Instances

should make my server return 201 Created (and hit a breakpoint set in the above controller method). Further, I should also be able to GET the metadata at https://localhost:44356/odata/$metadata without errors.

Actual result

When posting to the above endpoint using the "shortened key predicate of the related entity", 404 Not Found is returned without hitting my breakpoint.

Further, the following exception is thrown when getting the metadata:

System.InvalidOperationException: The path template 'Products({productName})/ProductVersions({productVersionName})/Instances' on the action 'PostInstance' in controller 'Products' is not a valid OData path template. The number of keys specified in the URI does not match number of key properties for the resource 'Models.ProductVersion'.
   at Microsoft.AspNet.OData.Routing.Conventions.AttributeRoutingConvention.GetODataPathTemplate(String prefix, String pathTemplate, IServiceProvider requestContainer, String controllerName, String actionName)
   at Microsoft.AspNet.OData.Routing.Conventions.AttributeRoutingConvention.<>c__DisplayClass11_0.<GetODataPathTemplates>b__0(ODataRouteAttribute route)
   at System.Linq.Enumerable.SelectArrayIterator`2.MoveNext()
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at Microsoft.AspNet.OData.Routing.Conventions.AttributeRoutingConvention.BuildAttributeMappings(IEnumerable`1 controllerActions)
   at Microsoft.AspNet.OData.Routing.Conventions.AttributeRoutingConvention.get_AttributeMappings()
   at Microsoft.AspNet.OData.Routing.Conventions.AttributeRoutingConvention.SelectAction(RouteContext routeContext)
   at Microsoft.AspNet.OData.Routing.ODataActionSelector.SelectCandidates(RouteContext context)
   at Microsoft.AspNetCore.Mvc.Internal.MvcRouteHandler.RouteAsync(RouteContext context)
   at Microsoft.AspNetCore.Routing.Route.OnRouteMatched(RouteContext context)
   at Microsoft.AspNetCore.Routing.RouteBase.RouteAsync(RouteContext context)
   at Microsoft.AspNetCore.Routing.RouteCollection.<RouteAsync>d__9.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.<Invoke>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.<Invoke>d__3.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__7.MoveNext()

Additional detail

When removing the shortened OData route

[ODataRoute("Products({productName})/ProductVersions({productVersionName})/Instances")]

from the controller, no exception is thrown and I obviously still can't use that route. When posting to an endpoint based on the full route (which is not the preferred one as per OData)

[ODataRoute("Products({productName})/ProductVersions(ProductName={productVersionName}, ProductVersionName={productVersionName})/Instances")]

everything works just fine.

robward-ms commented 6 years ago

Note sure it's NetCore related yet but certainly looks like a bug. There is code to handle this scenario in ODL (https://github.com/OData/odata.net/blob/75df8f44f2b81f984589790be4885b6ee8946ad0/src/Microsoft.OData.Core/UriParser/Parsers/KeyFinder.cs#L55-L73) and it appears you have it modeled correctly. I'll continue to investigate to understand why.

I can explain the metadata error: in the ASP.NET Core version of OData WebApi, the attribute routing convention is initialized at the first request. Because of the bug, that convention throws an exception and fails the request. If you comment out the problematic route attribute, i.e. the second one that has only the version key, the metadata request works as expected. I could change thius behaviour but I think it might make it harder to find faulty route attributes.

robward-ms commented 6 years ago

Not NetCore related per se, i.e. should repro on WebApi 6.x as well. The root of the issue with where the referential constraint for the model is defined: on the ProductVersions or Product navigation property. ODL is looking for it on ProductVersions where as this model, which matches our documentation, puts it on Products. Still investigating as to whether this should be fixed in the model & docs, WebApi or ODL. Stay tuned.

robward-ms commented 6 years ago

I've consulted with our OData committee member and we have decided that the ODL implementation for this scenario is incorrect. I filed an ODL issue here: https://github.com/OData/odata.net/issues/1071

I attempted to inject the constraint into the "ProductVersions" navigation property without success using this:

            builder.EntityType<Product>()
                .HasKey(p => p.ProductName)
                .HasRequired(p => p.ProductVersions, (p, pv) => p.ProductName == pv.First().ProductName);

WebApi (correctly) fails to parse the expression "p.ProductName == pv.First().ProductName" because .First() is not a property of pv, which is of type ICollection. You'd want to put "pv.ProductName" but the compiler knows it's a collection and it won't let you,

Since WebApi will not build the model that ODL will parse of this scenario, I can only think of two work-arounds, both involving XML and neither is clean: 1.) Use an XML chunk for the model. 2.) Use WebApi to construct the model, export to XML, insert the referential constraint, read the XML back into an EdmModel and return it.

The metadata generated by your model now is:

      <EntityType Name="Product">
        <Key>
          <PropertyRef Name="ProductName"/>
        </Key>
        <Property Name="ProductName" MaxLength="100" Nullable="false" Type="Edm.String"/>
        <NavigationProperty Name="ProductVersions" Type="Collection(OData_Test.ProductVersion)" ContainsTarget="true"/>
      </EntityType>

For ODL to parse the path as you expect it to, the metadata would need to look like this:

      <EntityType Name="Product">
        <Key>
          <PropertyRef Name="ProductName"/>
        </Key>
        <Property Name="ProductName" MaxLength="100" Nullable="false" Type="Edm.String"/>
        <NavigationProperty Name="ProductVersions" Type="Collection(OData_Test.ProductVersion)" ContainsTarget="true">
          <ReferentialConstraint ReferencedProperty="ProductName" Property="ProductName"/>
        </NavigationProperty>
      </EntityType>
mikepizzo commented 6 years ago

This appears to be an issue in ODL. @robward-ms has opened OData/odata.net#1071 to track. We will wait to close this issue until the ODL issue is resolved and we can verify that it works correctly in WebAPI.