OData / WebApi

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

Moving code to OData.Core #1054

Open raffaeler opened 7 years ago

raffaeler commented 7 years ago

There are two features that are currently part of the OData/WebApi set of assemblies but can be moved away to OData.Core since they can be used outside the web world.

  1. The FilterQueryOption class should not stay here and it is super-precious because it is the one that allows converting the odata query in a Linq Expression (via FilterBinder class). The same apply for the other Query Options. Please note that the ODataUriParser is already in the OData.Core BUT it is useless to extract a predicate (Func<T, bool>) or a projection.

  2. The ConventionalModel Builder is the simpler way to create an EDM Model but is not available in OData.Core. It is instead in the WebApi set of classes. It definitely makes sense to build an EDM Model without web dependencies since an EDM Model must be created when transforming OData clauses in Expression Trees. By the way, I created a model by hand using reflection with only the OData.Core assemblies but it is a pointless and useless work).

Please move these pieces out from here in OData.Core or a new neststandard library. This should never require any dependency from the asp.net/webapi set of libraries.

schmidtaa commented 7 years ago

Second that, expression tree walkers are the most complicated to implement and any reuse would benefit both worlds.

NetTecture commented 7 years ago

You can add attributes.

System.Web.OData.Builder

has for example the Contained attribute. Now, I love to keep my models in a separate DLL - but now this DLL pulls in WebApi JUST BECAUSE of attributes. And I would question the namespace... ComponentModel or ODataAnnotations would be more inline with the .NET framework for attributes that go on models (a la System.ComponentModel or System.DataAnnotations). Some cleanup would be appreciated.

raffaeler commented 6 years ago

I still can't see anything different on this request in the feature/netcore branch. @robward-ms could you please tell us if this is something happening at some point? TIA

xuzhg commented 6 years ago

@raffaeler I totally agree with what you proposed. I originially think the structure of ASP.NET Core OData will be same as ASP.NET Core structure, it includes several small packages and develop/build/publish one by one or as a whole metapackage.

For the ASP.NET Core Web API OData, just like you mentioned, it can be include parts seperatly as:

  1. Model builder. it can be "Microsoft.OData.Builder"
  2. Query. It can be "Microsoft.OData.Query"
  3. Routing. It can be "Microsoft.OData.Routing" ...

Maybe we can consider this structure in the next 7.1 release.

raffaeler commented 6 years ago

@xuzhg thanks for the feedback Sam. I really hope to see the new structure coming soon :)

raffaeler commented 6 years ago

@xuzhg I want to ask you to reconsider putting the refactoring in the 7.0 release for these reasons:

  1. Dependencies. Currently the Microsoft.AspNetCore.OData needs Microsoft.AspNetCore.Mvc.Core which basically means getting almost all of asp.net core.

  2. Refactoring the namespaces in a minor release can lead to troubles in many projects

Thanks