saule icon indicating copy to clipboard operation
saule copied to clipboard

Add support for .net core

Open joukevandermaas opened this issue 7 years ago • 14 comments

This is an exploratory issue. I'm assuming there will be quite some work here, so this can serve as something of a todo list.

  • [x] Change the project structure to support multiple targets (done by @laurence79)
  • [x] Convert the 'Common' project to netstandard 2.0 (#171)
  • [ ] Rebase on master (port new features implemented since the start of this work)
  • [ ] Re-enable stylecop/fxcop
  • [ ] Move as much as possible into the 'Common' project (perhaps wrapping some target-specific things in wrappers)
  • [ ] Clean up the tests (currently there is a lot of duplication, and some tests are in the wrong project IMO)

joukevandermaas avatar Jun 12 '17 15:06 joukevandermaas

I've done some exploratory work on this which you can see at laurence79/saule/feature/net-core

I sideways copied all the classes and tests from the existing projects to new netcore projects. The approach I have taken is to end up with 2 separate nuget packages - a classic ASP.net one and an aspnetcore one. I can't see how else we can support both frameworks given their entirely different dependencies - happy to be corrected here though.

Major changes that were required:

  1. Convert PreprocessingDelegatingHandler to an IActionFilter.
  2. Convert JsonApiMediaTypeFormatter to separate input and output formatters.
  3. Convert usage of actionContext.Request.Properties to actionContext.HttpContext.Items.
  4. Make alternative arrangements for surfacing errors - HttpError no longer available
  • Input formatters can't influence responses, only modelstate.
  1. Change the use of HttpConfig to MvcOptions.

All of the old tests have survived with only slight changes, and they all pass.

It would be sensible I think to extract out common functionality from the old and new libraries to a shared common library, probably using netstandard 2.0, although this will need the old library to target .NET Framework 4.6.1.

Let me know your thoughts 😊

laurence79 avatar Sep 06 '17 17:09 laurence79

Ok, further reading shows that we could indeed target both netcore and net4xx in the same package.

http://blogs.microsoft.co.il/iblogger/2017/04/05/easily-supporting-multiple-target-frameworks-tfms-with-vs2017-and-nuget/

Would you prefer this approach?

laurence79 avatar Sep 07 '17 06:09 laurence79

I've decided I also prefer that approach 😊

Latest code includes the following projects (tests not listed)

Saule.Common - Shared Functionality (targets net452 & netcoreapp2.0)

Saule.AspNet - ASP.Net Specific classes (targets net452) (references Saule.Common, System.Web.dll etc.)

Saule.AspNetCore - ASP.Net Core Specific Classes (targets netcoreapp2.0) (references Saule.Common, Microsoft.AspNetCore.Mvc etc.)

Saule - For Nuget Packaging (targets net452 & netcoreapp2.0) (references Saule.AspNet or Saule.AspNetCore depending on target condition)

laurence79 avatar Sep 07 '17 10:09 laurence79

@laurence79 awesome work! I think it makes sense for Saule.Common to target .net standard (probably 1.6?). One thing that worried me was the existing people targeting the Nuget package Saule, but it seems that you have figured out a solution there as well.

I'm guessing some of the actual logic that's currently in the formatter and delegating handler must move into Saule.Common somehow. I think it's important from a contributions point of view that the two ASP specific projects are as small as possible. Some of the integration tests will necessarily need to be duplicated, but that's unavoidable. That said, I think there are some integration tests that could be unit tests.

How do you think we should proceed with getting this new stuff merged into the main Saule repo? Are there any blockers? I would really love to enable you, me and potential others to collaborate on this effort.

joukevandermaas avatar Sep 09 '17 18:09 joukevandermaas

Thanks, I agree.

Perhaps if you start a new branch, say feature/net-core similar to how I have done in my fork. I'll then do a PR into that branch, then you, I, and others can fork that branch and submit PRs?

laurence79 avatar Sep 11 '17 08:09 laurence79

OK, I rebased your branch on master and pushed that in this repo. I also added you as collaborator so you can work in this repo and potentially help reviewing PRs into the feature/net-core branch.

I hope we can get this merged into master pretty smoothly once everything seems ready. I'll try to create a more up-to-date checklist in this issue soon so we know what the status is.

joukevandermaas avatar Sep 11 '17 09:09 joukevandermaas

I've spent a bit of time on this in https://github.com/joukevandermaas/saule/pull/177.

Basically my thoughts are currently like this:

  • I want to move common to netstandard 2, which means bumping the framework version to 4.6.2 and only supporting .Net Core 2.0
  • Since that's a breaking change and we'll need to bump the major version, I also want to remove the 'old' deprecated way of doing setup
  • I also want to slightly clean up the pagination API since it's super confusing

I'll continue in #177 to make the tests and the appveyor build pass.

joukevandermaas avatar Oct 16 '17 21:10 joukevandermaas

Hello Guys!!!

Great library.... perfect..

It is already possible to use the new features for the .net core??

If not there is any release date??

Thanks in advance ..

Paulo

prmces avatar Jan 12 '18 14:01 prmces

Hi Paulo,

I kind of hit a dead end with the migration @laurence79 started in a separate branch. I think we need to prepare the master branch so something similar can be merged into the main releases. Because of my schedule right now I cannot really maintain two separate branches and keep feature parity.

I think the work @laurence79 did informed the direction of this project in a few important ways that open up more contributions:

  1. We need to extract all 'real' logic into Saule-specific classes. In other words, no web api related abstractions should directly do anything, instead they should call into an abstraction. Most of these abstractions are already worked out to some extend in the feature/net-core branch.
  2. We need to extract the web api portion of Saule into a separate project. This effectively means we'll have Saule-Core and Saule-WebApi. Saule-Core will contain all the logic and abstractions, Saule-WebApi will bind these to web api concepts.

All of the above can and should happen in master. After that work is done (or meanwhile, really), we can create .Net core implementations for the same abstractions already in Saule-Core. I cannot commit to a timeline for this as it will largely depend on outside contributions.

All that said, it is definitely possible to use the stuff in feature/net-core. It will take some effort on your part, and it will not have feature parity, but it mostly all works.

joukevandermaas avatar Jan 12 '18 19:01 joukevandermaas

Hi @joukevandermaas, This library is the most complete and easy to use implementation for jsonapi I could find, however, I am working with .NET core and it not being fully supported yet is a shame. I'd like to contribute, at least for feature parity. Do you have any recomendations as to what to do and to not?

SuricateCan avatar Jun 29 '18 11:06 SuricateCan

@suricatecan i would love to have support for core but due to my schedule i just can't put in the time. After the experiment documented above i think the best approach is as follows:

  1. Create a net standard project with dependency on neither asp web api nor core.
  2. Create two projects which implement the public interface for core and framework.
  3. Move as much as possible of the current code into the net standard project. If code is currently implemented as part of a framework api, we can move the implementation out into an interface and implement it for both targets.

This is basically the approach mentioned above. But i think we should do step 1 in master so new contributions work for both targets. Doing this in parallel with new prs seems impossible to manage.

So if you want to take a stab at that in a pr I'm more than happy to review and merge it.

joukevandermaas avatar Jun 29 '18 17:06 joukevandermaas

Great, I'll work on small compatible PRs so it's easier to merge,

SuricateCan avatar Jun 29 '18 18:06 SuricateCan

I have colleagues at work who are using Saule as their json API implementation of choice. As per this thread I understand net core is still not an option. Muy question is, is that because this other implementation works for that matter https://github.com/json-api-dotnet/JsonApiDotNetCore

guillemsola avatar Jul 24 '19 08:07 guillemsola

@guillemsola The reason is, I don't use dotnet core at work and I don't have time to do this outside of work. It is a shitty situation but that's what it is.

However, the fact that there are other implementations that are compatible is good. When I first created Saule, there were no implementations for web api.

joukevandermaas avatar Jul 24 '19 09:07 joukevandermaas