Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to work with Umbraco 9.0.0+ / net5.0+ #30

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

tristanjthompson
Copy link

Upgrade to work with v9+ - WIP

We currently have a v8 project that we're upgrading to v11 - as Umbraco won't be releasing a headless API until v12, we thought it would be worth upgrading HeadRest to work with v11 first, and then downgrade the package versions to v9 to see if it still worked.

This is the work so far - I've got it working on a sample v11 Umbraco project, but we'll be integrating it into our upgraded v11 project in the next few weeks.

I'm raising this draft PR to see if this is something that you might be interested in - no worries if not...especially as headless is coming into core in v12!

{
public void Initialize()
{
RouteTable.Routes.IgnoreStandardExclusions();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a netcore equivalent to this and the package seems to work in v11 without it - not sure if it's needed in netcore?

{
internal class HeadRestAuthorizeAttribute : AuthorizeAttribute
{
protected override void HandleUnauthorizedRequest(AuthorizationContext filterContext)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code has been moved directly into the AuthorizedHeadRestController

_config = config;
}

protected override IPublishedContent FindContent(RequestContext requestContext, UmbracoContext umbracoContext)
Copy link
Author

@tristanjthompson tristanjthompson Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code has been moved directly into the HeadRest.cs class

@@ -76,5 +92,46 @@ private static void ValidateConfig(HeadRestConfig config)
throw new Exception("ViewModelMappings can not be null");
}
}

private static IPublishedContent FindContent(ActionExecutingContext actionExecutingContext)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been moved from the old HeadRestRouteHandler

config.BasePath = basePath;
config.RootNodeXPath = rootNodeXPath;
ConfigureEndpoint(config);
var config = new HeadRestConfig(options)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was quite tricky getting hold of the _mapper at this point, so I changed it to pass in the old options and copy the values in the constructor instead.


namespace Our.Umbraco.HeadRest.Web.Mvc
{
internal class HeadRestResult : JsonNetResult
internal class HeadRestResult : ActionResult
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsonNetResult is no longer available, so I copied the code from it and adapted it for netcore to make HeadRestResult work the same way and serialize using the serializer settings we want it to.


namespace Our.Umbraco.HeadRest.Web.Routing
{
internal class HeadRestUrlProvider : DefaultUrlProvider
internal class HeadRestUrlProvider : IUrlProvider
Copy link
Author

@tristanjthompson tristanjthompson Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DefaultUrlProvider has a different constructor in v9 than it does in v10+ which makes it tricky to inherit from without creating different versions of the code.

Rather than try to have 2 different versions of the code/releases/packages with 2 different constructors, I've updated the code to implement the IUrlProvider instead and injected the DefaultUrlProvider so that we can still fall back to use its methods for non-HeadRest URLs.

@tristanjthompson
Copy link
Author

Update: the site we're migrating from v8 > v11 is in PR and includes this version of HeadRest. It should be going through QA very soon so I'll update this with any bugfixes we make before marking it as ready for review!

context.HttpContext.Response.StatusCode = 401;

// This doesn't exist in netcore - do we need it?
// context.HttpContext.Response.SuppressFormsAuthenticationRedirect = true;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't exist in netcore - do we need it?

@tristanjthompson
Copy link
Author

Another update - our v11 project has been through QA and is going live tomorrow. I'll give it a week to settle in and then submit the PR if everything seems stable

@tristanjthompson
Copy link
Author

Whoops - a bit more than a week...sorry about that! Everything seems to be working on live, so I think it seems good to go (bearing in mind we don't use all the features of HeadRest).

Is this a PR you think would be worth the effort to merge in/maintain? If so I can update some of the docs before submitting...if not, no worries 😄

@luonghongthuan
Copy link

Another update - our v11 project has been through QA and is going live tomorrow. I'll give it a week to settle in and then submit the PR if everything seems stable

could you share how to apply the .net core version to Umbraco 11 project?
many thanks if you have any sample can share

@tristanjthompson
Copy link
Author

tristanjthompson commented Aug 1, 2024

could you share how to apply the .net core version to Umbraco 11 project? many thanks if you have any sample can share

Hi @luonghongthuan - apologies for the delayed reply, I've been away on leave!

It's been quite a while since we did the work, but I've just gone through the readme and updated it with how the configuration has changed - you can see the differences in the PR or view the updated readme here.

You can't install the package from nuget as it only exists in this PR. Instead you'll need to clone/pull down the project locally and either:

  1. Build the project and then copy/reference the DLLs from your own Umbraco project
    or
  2. Copy the project into your VS Solution alongside the Umbraco project and then reference the copied HeadRest project form your Umbraco project (this is what we've done).

Hope that helps!

@tristanjthompson
Copy link
Author

Another update - our v11 project has been through QA and is going live tomorrow. I'll give it a week to settle in and then submit the PR if everything seems stable

Yet another update - we recently upgraded our v11 project to v13 and HeadRest is still working without any changes needed!

I also realised that I made a small change back when we upgraded to v11 which didn't get pushed, so I've just pushed that too.

@tristanjthompson tristanjthompson marked this pull request as ready for review October 29, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants