OData / RESTier

A turn-key library for building RESTful services
http://odata.github.io/RESTier
Other
472 stars 135 forks source link

Support Views #692

Closed cilerler closed 2 years ago

cilerler commented 3 years ago

Having a Keyless attribute (which is a common scenario for Views) causes a System.NullReferenceException on https://github.com/OData/RESTier/blob/1a6182ca41ae762d2071200b4cfa589bb23359ed/src/Microsoft.Restier.EntityFramework.Shared/Model/EFModelBuilder.cs#L90

Error Message

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=Microsoft.Restier.EntityFrameworkCore
  StackTrace:
   at Microsoft.Restier.EntityFrameworkCore.EFModelBuilder.<>c.<GetModel>b__4_8(IEntityType e) in D:\a\1\s\src\Microsoft.Restier.EntityFramework.Shared\Model\EFModelBuilder.cs:line 90

Assemblies affected

<PackageReference Include="Microsoft.Restier.EntityFrameworkCore" Version="1.0.0-CI-20210719-021012" />

Reproduce steps

Create a table on the database.

using Microsoft.EntityFrameworkCore;
using System;

#nullable disable

namespace Microsoft.Restier.Samples.Northwind.AspNetCore
{
    [Keyless]
    public partial class BirthDates
    {
        public string LastName { get; set; }
        public string FirstName { get; set; }
        public DateTime? BirthDate { get; set; }
    }
}

Add it to DbContext

public virtual DbSet<BirthDates> BirthDates{ get; set; }

modelBuilder.Entity<BirthDates>(entity =>
{
    entity.ToView("BirthDates");
    entity.Property(e => e.BirthDate).HasColumnType("datetime");
    entity.Property(e => e.FirstName);
    entity.Property(e => e.LastName);
});

Update the Northwind

CREATE VIEW [dbo].[Birthdates]
    AS SELECT e.FirstName, e.LastName, e.BirthDate FROM dbo.Employees as e
robertmclaws commented 2 years ago

This was due to some issues with how EFCore was originally plumbed. I believe we have this fixed in the latest version. Can you please test with RC8 and see if it is still a problem?

Thanks!

cilerler commented 2 years ago

Tested with RC8, and still has the same issue, can't even get $metadata

<PackageReference Include="Microsoft.Restier.EntityFrameworkCore" Version="1.0.0-rc8.20220714.1" />
robertmclaws commented 2 years ago

Can you get an updated callstack and error line, please? Line 90 is blank in the current codebase.

Thanks!

cilerler commented 2 years ago

This is the place that throws the error due to null value

https://github.com/OData/RESTier/blob/5fa877e274926c74ef2ca15f10c80bfbe673b921/src/Microsoft.Restier.AspNet.Shared/Model/RestierWebApiModelBuilder.cs#L79

StackTrace

image

Reproduce

  1. Update the Microsoft.Restier.Samples.Northwind.AspNetCore.csproj
  2. Update the NorthwindContext.cs
  3. Add SalesByCategory.cs
  4. Run

Microsoft.Restier.Samples.Northwind.AspNetCore.csproj

    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="6.0.7" />
    <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="6.0.7">

NorthwindContext.cs

        public virtual DbSet<SalesByCategory> SalesByCategory { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            //...
            modelBuilder.Entity<SalesByCategory>(entity =>
            {
                entity.ToView("Sales By Category");
                entity.Property(e => e.CategoryId).HasColumnName("CategoryID");
                entity.Property(e => e.CategoryName)
                    .IsRequired()
                    .HasMaxLength(15);
                entity.Property(e => e.ProductName);
                entity.Property(e => e.ProductSales);
            });
            //...
        }

SalesByCategory.cs

using Microsoft.EntityFrameworkCore;

#nullable disable

namespace Microsoft.Restier.Samples.Northwind.AspNetCore
{
    [Keyless]
    public partial class SalesByCategory
    {
        public int CategoryId { get; set; }
        public string CategoryName { get; set; }
        public string ProductName { get; set; }
        public decimal ProductSales { get; set; }
    }
}
robertmclaws commented 2 years ago

I don't know if supporting Views this way is going to be possible. I unblocked the NullReferenceException you were getting, but OData's core libraries now throw an InvalidOperationException, that Entities must have keys. This is expected behavior.

You would have to manually override the ODataConventionModelBuilder and map SalesByCategory as a ComplexType instead, and then create a method on the NorthwindApi that returns SalesbyCategory based on your query.

For deeper Views support, I think you would need to open an issue in the OData core repo and ask them to support keyless entities.

We will include the NullReferenceException fix in the RTM build. Sorry I couldn't unblock this one for you.

robertmclaws commented 2 years ago

I updated the codebase to have a unit test for this issue, and I have also caught the scenario and thrown a more informative exception. You can see the changes in this commit: https://github.com/OData/RESTier/commit/4b15400dbe9d3177c209bf0aea7fce193dfe54c6

cilerler commented 2 years ago

bitmoji

BlagoCuljak commented 1 year ago

Hi guys, maybe someone here can help me with this issue, I do have some keyless views, and I get the error as @robertmclaws referenced in commit, but I would like to kick out that view from my RESTier controller, I don't need that keyless data. I would like to keep using those views in other parts of my app.

Is there some clean and fast way to get rid of all Keyless objects coming from my EF project to RESTier?