AspNetCoreOData icon indicating copy to clipboard operation
AspNetCoreOData copied to clipboard

Odata Generic Controller not treated as odata controller in ver 8.0

Open dvkrish opened this issue 4 years ago • 15 comments

Describe the bug We have upgraded our aspnetcoreodata 7.4 to 8.0 beta. After upgrade our generic controller not treated as odata controller and all the count/expand/select not working.

To Reproduce

in Startup

        services.AddOData(options => options.Count().Filter().Expand().Select().OrderBy().SetMaxTop(100)
            .AddModel("odata", GetEdmModel(), new DefaultODataBatchHandler())
            );
        services.AddMvcCore()
        .ConfigureApplicationPartManager(m => m.FeatureProviders.Add(new GenericFeatureProvider()))

In GenericFeatureProvide

            var typeName = entityType.Name + "Controller";
            if (!feature.Controllers.Any(t => t.Name == typeName))
            {
                var controllerType = typeof(GenericController<>)
                    .MakeGenericType(entityType)
                    .GetTypeInfo();

                feature.Controllers.Add(controllerType);
            }

In GenericController

[GenericControllerName]
////[EnableQuery(MaxNodeCount = 10000, AllowedQueryOptions = AllowedQueryOptions.All, MaxExpansionDepth = 4)]
public class GenericController<T> : ODataController where T : EntityModel
{
    protected IEntityRepository<T> EntityRepository { get; set; }
    public GenericController(IEntityRepository<T> EntityRepo, IConfiguration configuration)
    {
        EntityRepository = EntityRepo;
    }

    [HttpGet]
    public async Task<IQueryable<T>> Get()
    {
         var res = await EntityRepository.GetItemsEnumerableAsync(c => (!c.IsActive || IncludeIsActive));
        return res.AsEnumerable().AsQueryable();
    }

}

dvkrish avatar Feb 25 '21 20:02 dvkrish

@dvkrish Can you share a repo for me with codes?

xuzhg avatar Mar 02 '21 01:03 xuzhg

@dvkrish I was having exact same problem with generics and 8.0, I was making a generic as I wanted to just generate all the controllers automatically and inherit the generic; then I tried to reproduce inside ODataRoutingSample coding it by hand and failed to reproduce.

Figured it out that order versions were forgiving about plural vs singular naming miss match in that this.EntitySet<Category>("Categories"); used to match and find CategoryController (possible because it matched on type? as a secondary match?)

Now only this.EntitySet<Category>("Category"); will match CategoryController

JohnGoldInc avatar Mar 16 '21 13:03 JohnGoldInc

@JohnGoldInc

I am able to re-produce this issue. I have to hardcode the entity in the controller to make it has odata like this attribute [ODataRoutePrefix("EntityName")].

[GenericControllerName] [ODataRoutePrefix("Year")] ////[EnableQuery(MaxNodeCount = 10000, AllowedQueryOptions = AllowedQueryOptions.All, MaxExpansionDepth = 4)] public class GenericController<T> : ODataController where T : EntityModel {

@xuzhg Is it possible to share the code privately with you directly. Please let me know how to.

dvkrish avatar Mar 17 '21 14:03 dvkrish

@dvkrish please send the link or the project to [email protected]

xuzhg avatar Mar 17 '21 17:03 xuzhg

@JohnGoldInc @dvkrish The built-in convention routing is using the "controller name", "action name" and "action parameter", etc to build the routing template. If any of these conditions doesn't meet, you can use the attribute routing. Now, in the latest nightly, we encourage you to use [Route(...)], [HttpGet(...)], etc.

xuzhg avatar Mar 17 '21 17:03 xuzhg

How was this resolved? I need basically the same setup, and have a setup pretty much spot on with this: https://stackoverflow.com/questions/57096991/generic-o-data-controller-does-not-return-expected-result

AndrewZenith avatar Jun 06 '21 21:06 AndrewZenith

The following Blog-Post said that [Route(...)], [HttpGet(...)], etc should be working in RC. https://devblogs.microsoft.com/odata/attribute-routing-in-asp-net-core-odata-8-0-rc/

I am using Microsoft.AspNetCore.OData 8.0.0-rc3 and the RouteAttributes still do not work with ODataControllers. And it also does not help to make the IEdmModel aware of the route by adding any kind of configuration. I also were not able to solve this problem by a custom RoutingConvention. So i had to build a workaround that makes no use of RouteAttributes.

We got like 500 tables and 300 views to serve by OData, so all in all more than 800 entities. Building all the boilerplate code for each entity is no solution. So a generic solution is our way to go.

Summary to my solution

  1. I created a generic ODataController.
  2. I generate a Controller for each Entity making use of my generic ODataController each time I add entities again.
  3. I used reflection to build the IEdmModel from our existing EF Core model.

My solution

Not the perfect solution but one that works.

The Generic ODataController using EF Core DbContext to implement the basic CRUD operations:

	public class ODataController<TDbContext, TEntity> : ODataController
		where TDbContext : DbContext
		where TEntity : class
	{
		[EnableQuery]
		public IActionResult Get()
		{
			var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
			var dbSet = dbContext.Set<TEntity>();

			return Ok(dbSet);
		}

		[EnableQuery]
		public async Task<IActionResult> Get(long key)
		{
			var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
			var dbSet = dbContext.Set<TEntity>();

			var entity = await dbSet.FindAsync(key).ConfigureAwait(true);
			if (entity == null)
			{
				return NotFound();
			}

			return Ok(entity);
		}

		[EnableQuery]
		public async Task<IActionResult> Post([FromBody] TEntity entity)
		{
			var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
			await dbContext.AddAsync<TEntity>(entity).ConfigureAwait(false);
			await dbContext.SaveChangesAsync().ConfigureAwait(false);

			return Created(entity);
		}

		[EnableQuery]
		public async Task<IActionResult> Put(long key, [FromBody] Delta<TEntity> entity)
		{
			var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
			var dbSet = dbContext.Set<TEntity>();

			var originalEntity = await dbSet.FindAsync(key).ConfigureAwait(true);
			if (originalEntity == null)
			{
				return NotFound();
			}

			entity.Put(originalEntity);
			await dbContext.SaveChangesAsync().ConfigureAwait(false);

			return Updated(entity);
		}

		[EnableQuery]
		public async Task<IActionResult> Patch(long key, [FromBody] Delta<TEntity> entity)
		{
			var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
			var dbSet = dbContext.Set<TEntity>();

			var originalEntity = await dbSet.FindAsync(key).ConfigureAwait(true);
			if (originalEntity == null)
			{
				return NotFound();
			}

			entity.Patch(originalEntity);
			await dbContext.SaveChangesAsync().ConfigureAwait(false);

			return Updated(entity);
		}

		[EnableQuery]
		public async Task<IActionResult> Delete(long key)
		{
			var dbContext = HttpContext.RequestServices.GetService<TDbContext>();
			var dbSet = dbContext.Set<TEntity>();

			var entity = await dbSet.FindAsync(key).ConfigureAwait(true);
			if (entity == null)
			{
				return NotFound();
			}

			dbContext.Remove<TEntity>(entity);
			await dbContext.SaveChangesAsync().ConfigureAwait(false);

			return Ok();
		}
	}

I created a generator that creates a Controller for each Entity into a single file:

public class ArticleController: ODataController<MyContext, Article> { }
public class BrandController: ODataController<MyContext, Brand> { }
public class ColorController: ODataController<MyContext, Color> { }
...
...
...

Build IEdmModel by reflection from EF Core DbContext:

		public static IEdmModel GetEdmModel()
		{
			ODataConventionModelBuilder builder = new ODataConventionModelBuilder();
			AddEntities(builder);
			return builder.GetEdmModel();
		}

		public static void AddEntities(ODataConventionModelBuilder builder)
		{
			var contextType = typeof(MyContext);
			var properties = contextType.GetProperties(System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance)
				.Where(x => x.PropertyType.IsGenericType)
				.ToArray();

			var entitySetMethod = typeof(ODataConventionModelBuilder).GetMethod(nameof(ODataConventionModelBuilder.EntitySet));

			var dbSetProperties = properties.Where(x => x.PropertyType.GetGenericTypeDefinition() == typeof(DbSet<>)).ToArray();
			foreach (var property in dbSetProperties)
			{
				var dbSetType = property.PropertyType.GetGenericArguments().FirstOrDefault();
				var isKeyless = dbSetType.GetCustomAttributes(typeof(KeylessAttribute), false).Any();
				if (isKeyless == false)
				{
					var entitySetMethodGeneric = entitySetMethod.MakeGenericMethod(dbSetType);
					entitySetMethodGeneric?.Invoke(builder, new[] { dbSetType.Name });
				}
				else
				{
					var keyProperty = DetermineKeyProperty(dbSetType);
					if (keyProperty != null)
					{
						EntityTypeConfiguration entityType = builder.AddEntityType(dbSetType);
						entityType.HasKey(keyProperty);
						builder.AddEntitySet(dbSetType.Name, entityType);
					}
				}
			}
		}

Explainations to my solution

The generic ODataController may not use Constructor DependencyInjection, so you need to query the ServiceProvider explicitly for the required dependencies. Or maybe i just did that to reduce the code to be generated, not sure anymore.

The ODataConventionModelBuilder does not offer any easy way to configure the IEdmModel generically. So i had to use some reflection on the ODataConventionModelBuilder to add the entities as EntitySet.

IEdmModel requires a key for each Entity configuration, even if there is no key on the entity. The method DetermineKeyProperty(dbSetType) is my implementation to determine a property as key by our conventions.

Luckily most of our tables and views use an Id as PK of type BIGINT. But if you also need something generic for the key, just add TKey and generate Controllers with the correct type for TKey. If you got composed Primary Keys, then you might want to create controllers for them manually. If you got a lot of composed Primary Keys ... dont ask me :P

Hope this helps.

JanKotschenreuther avatar Jun 11 '21 16:06 JanKotschenreuther

Thanks for the response there @JanKotschenreuther, fortunately for me my EdmModel creation comes from the ORM (using DevExpress XPO), but that's an interesting idea to create from the the controllers!

Seems you have the same issue as I do though, the generic controller works as long as you have them all in code, but not if you use the parts feature to add them:

I created a generator that creates a Controller for each Entity into a single file:

public class ArticleController: ODataController<MyContext, Article> { }
public class BrandController: ODataController<MyContext, Brand> { }
public class ColorController: ODataController<MyContext, Color> { }

I would have preferred to have added these dynamically rather than generating the code. I'm adding other REST controllers dynamically, but as mentioned if I do this for my generic OData Controller it appears, but it's NOT an ODataController, it's a (simple) REST controller. The following code works for other controllers, but no longer for OData:

public class GenericTypeControllerFeatureProvider :IApplicationFeatureProvider<ControllerFeature> { public void PopulateFeature(IEnumerable<ApplicationPart> parts, ControllerFeature feature) { var currentAssembly = typeof(GenericTypeControllerFeatureProvider).Assembly; var candidates = currentAssembly.GetExportedTypes().Where(x => x.GetCustomAttributes<GeneratedControllerAttribute>().Any());

            foreach (var candidate in candidates)
            {

                feature.Controllers.Add(typeof(CustomODataController<>).MakeGenericType(candidate).GetTypeInfo());

            }
        }
    }
    

The route however is working perfectly for me, which it wasn't doing so in earlier versions so a bit thumbs up for that.

AndrewZenith avatar Jun 12 '21 16:06 AndrewZenith

@AndrewZenith I also implemented some generic ApiControllers the ASP.NET Core way as mentioned here https://github.com/OData/AspNetCoreOData/issues/76#issuecomment-859910236. ODataQueryOptions allow working with OData on those controllers.

Also spent some time making generic controllers work with ASP.NET Core OData but that was a very frustrating experience because i could not get it to work. My hope was that a custom ODataConvention could solve my problem, but i were not able to figure out how ODataConventions could do me that favour. If that is possible then it just failed because of missing or bad docs.

I am wondering why ASP.NET Core OData has to serve its own implementation of RouteConventions. Maybe it would be easier if ASP.NET Core OData makes more use of ASP.NET Core features and APIs. That would result in better documentation because ASP.NET Core is very well documented and got a way larger community which results in more help from that community.

I can see some commitment by making use of ASP.NET Core RouteAttributes. But at the moment that does not work. So I am in hope future releases become easier to use.

JanKotschenreuther avatar Jun 14 '21 07:06 JanKotschenreuther

This is still an issue

closem90 avatar Nov 03 '22 18:11 closem90

to dispense with exposition, the thing that's still an issue is how to implement a single odata endpoint that serves arbitrary entities

at some point the issue resolves to the mismatch between

  • the expected name of an entityset endpoint
  • the full name of the generic controller, which will include type information and fail the odata route convention grep operation

below is a bit of code that assumes the implementer

  • wants to 'fix' routing to a not-sealed generic controller that at least partially serves responses (let's say json without the odata.context)
  • is comfortable introducing dynamic code generation of TEntityController : GenericController<TEntity>
  • has a method of getting the TypeInfo of the custom generic controller
  • has a method of generating an edm model dynamically
    public class DynamicControllerFeatureProvider : IApplicationFeatureProvider<ControllerFeature>
    {
        // these items assist with rendering a class that inherits from the user's generic controller
        AssemblyBuilder ab;
        ModuleBuilder mb;

        // this is here to get a class reference in play that hopefully prevents the generated types from going out of scope
        // this needs testing to understand what's needed to prevent null object references
        // since the generated classes are never rendered physically
        List<TypeInfo> GeneratedTypes = new List<TypeInfo>();

        public void PopulateFeature(IEnumerable<ApplicationPart> parts, ControllerFeature feature)
        {
            // find THE GENERIC ODATA CONTROLLER implemantion's assembly
            // this needs to be customized to your own scheme, according to 
            // the contents of your associated assemblies
            var currentAssembly = typeof(DynamicControllerFeatureProvider ).Assembly;
            var candidates = currentAssembly.GetExportedTypes().Where(x => 
                x.GetCustomAttributes<HorselessGraphConventionAttribute>().Any());

            // find the custom entities
            // the below code uses the semantics used by the author to solve this part of the problem
            var assemblyLoader = new YourAssemblyLoader();
            var solutionAssemblies = assemblyLoader.GetSolutionAssemblies(f => true);
            var deployableEntities = assemblyLoader.GetContentModelEntities<IDeployableContentEntity>(solutionAssemblies);

            // arrange the dynamic assembly renderer
           AssemblyName aName = new AssemblyName("DynamicTypes");
            ab =
                AssemblyBuilder.DefineDynamicAssembly(
                    aName,
                    AssemblyBuilderAccess.Run);

            mb =
               ab.DefineDynamicModule(aName.Name);

            // arrange the entities we want to generate classes for
            var deployableList = new List<Type>();
            deployableList.AddRange(deployableEntities);

            // for each generic controller you want to 
            // treat as the base class for a dynamically
            // rendered odata controller with the name
            // appropriate to its entityset
            foreach (var candidate in candidates)
            {
                // https://stackoverflow.com/questions/36680933/discovering-generic-controllers-in-asp-net-core
                // There's no 'real' controller for this entity, just a generic proxy, so add the generic version.
  
                 // for each entity you need to generate a controller 
                // populate controller feature for deployable entities
                foreach (var entityType in deployableList)
                {
                    // arrange the base class
                    var entityTypeInfo = entityType.GetTypeInfo();
                    var controllerType = typeof(YourGenericController<>) // assumes Controller<TEntityType> 
                        .MakeGenericType(entityTypeInfo).GetTypeInfo();

                    // arrange sanity check conditions for rendering the class
                    if (!feature.Controllers.Any(t => t.Name == $"{entityType.Name}Controller"))
                    {
                        // render a controller class named TEntityController that inherits from YourGenericController<TEntity>
                        TypeBuilder tb = mb.DefineType(
                                $"{entityType.Name}Controller",
                         TypeAttributes.Public, controllerType);

                        // arrange to prevent the new object from going out of scope
                        var inheritedControllerType = tb.CreateTypeInfo();
                        this.GeneratedTypes.Add(inheritedControllerType);

                        // finis - you're adding TEntityController : YourGenericController<TEntity>
                        feature.Controllers.Add(inheritedControllerType);

                    }
                }

            }
        }
    }

vigouredelaruse avatar Mar 16 '23 03:03 vigouredelaruse

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

xuzhg avatar Jan 30 '24 17:01 xuzhg

https://github.com/OData/AspNetCoreOData/issues/737

xuzhg avatar Jan 30 '24 17:01 xuzhg

@dvkrish @AndrewZenith @closem90 Can you check out the sample shared by Sam here https://github.com/odata/aspnetcoreodata/issues/98#issuecomment-1917579319 and confirm if that helps?

gathogojr avatar Feb 02 '24 06:02 gathogojr

Hi John,

Thanks for the update. Please give sometime I will check and revert back to you

Thanks

On Thu, Feb 1, 2024 at 10:10 PM John Gathogo @.***> wrote:

@dvkrish https://github.com/dvkrish @AndrewZenith https://github.com/AndrewZenith @closem90 https://github.com/closem90 Can you check out the sample shared by Sam here #98 (comment) https://github.com/OData/AspNetCoreOData/issues/98#issuecomment-1917579319 and confirm if that helps?

— Reply to this email directly, view it on GitHub https://github.com/OData/AspNetCoreOData/issues/98#issuecomment-1922920061, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD63VRDX2QLTYPYNLRGONOLYRR7OLAVCNFSM4YHERIJKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJSGI4TEMBQGYYQ . You are receiving this because you were mentioned.Message ID: @.***>

dvkrish avatar Feb 02 '24 06:02 dvkrish