Umbraco-CMS icon indicating copy to clipboard operation
Umbraco-CMS copied to clipboard

IVirtualPageController is not setting the Umbraco context PublishedRequest content via FindContent

Open d-gibbs opened this issue 2 years ago • 1 comments

Which exact Umbraco version are you using? For example: 9.0.1 - don't just write v9

10.1.0

Bug summary

IVirtualPageController is setting up the UmbracoRouteValues object with the content returned by the FindContent interface implementation however subsequent requests which attempt to read the value from IUmbracoContextAccessor.PublishedRequest.PublishedContent will find that either the returned data is null OR the configured 404 page node is there instead, indicating that the content was set correctly for route values but not set correctly for the current umbraco context.

Specifics

According to the documentation, IVirtualPageController can be used to populate the UmbracoRouteValues HTTP feature and allows us to execute a custom route within the Umbraco request context.

Route a custom controller that implements the IVirtualPageController interface, assigning the UmbracoRouteValues to the HTTP requests will then be taken care of for you.

I'm finding that the PublishedContent property of the current umbraco context PublishedRequest instance does not contain the content returned by FindContent in subsequent code - it instead contains the configured 404 page:

image

I should note that the value of this.CurrentPage does contain the correct content. It's only the current Umbraco context which doesn't appear to be set correctly.

Steps to reproduce

Create a controller which implements IVirtualPageController and inherits from UmbracoPageController:

public class ArticleVirtualController : UmbracoPageController, IVirtualPageController
{
    private readonly IUmbracoContextAccessor umbracoContextAccessor;

    public ArticleVirtualController(
        ILogger<ArticleVirtualController> logger,
        ICompositeViewEngine compositeViewEngine,
        IUmbracoContextAccessor umbracoContextAccessor)
        : base(logger, compositeViewEngine)
    {
        this.umbracoContextAccessor = umbracoContextAccessor
    }

    public IActionResult Index()
    {
         return this.View();
    }

    public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext)
    {
       if (this.umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext) &&
            actionExecutingContext.RouteData.Values.TryGetValue("id", out object? id) &&
            int.TryParse(id?.ToString(), out int articleId))
        {
            return umbracoContext.Content?.GetById(articleId);
        }

        return null;
    }
}

Get the current Umbraco context at any point after FindContent has been called:

public IActionResult Index()
{
    if (this.umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext))
    {
        var publishedContent = umbracoContext.PublishedRequest?.PublishedContent;
    }
}

You will find that publishedContent returns the configured 404 page:

Workaround I have a workaround for this issue which involves manually creating the published request with IPublishedRouter:

public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext)
{
    if (this.umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext) &&
        actionExecutingContext.RouteData.Values.TryGetValue("id", out object? id) &&
        int.TryParse(id?.ToString(), out int articleId))
    {
        var article = umbracoContext.Content?.GetById(articleId);

        if (article != null)
        {
            var requestBuilder = Task.Run(async () => await this.publishedRouter.CreateRequestAsync(umbracoContext.CleanedUmbracoUrl)).Result;
            requestBuilder.SetPublishedContent(article);

            umbracoContext.PublishedRequest = requestBuilder.Build();

            return article;
        }

   return null;
 }

Essentially the 'do it completely manually' step from the documentation...

Do it completely manually - This requires that you have a custom route, controller, even middleware, and manually assign the UmbracoRouteValues as an HTTP request feature, however you see fit. To create an UmbracoRouteValues object generally requires: IUmbracoContextAccessor (to access the CleanedUmbracoUrl), IPublishedRouter (to create the IPublishedRequestBuilder), IPublishedRequestBuilder (to set the published content and to build the IPublishedRequest), IPublishedRequest to assign to the UmbracoRouteValues. As you can see this is quite a lot of work, but luckily there's some much easier ways.

https://our.umbraco.com/documentation/reference/routing/custom-routes/#:~:text=Do%20it%20completely,controller%20that%20implements

But this feels wrong...

Expected result / actual result

Expected: IUmbracoContextAccessor.PublishedRequest.PublishedContent should contain the data returned by FindContent just as UmbracoRouteValues does.

Actual: IUmbracoContextAccessor.PublishedRequest.PublishedContent does not contain the data returned by FindContent, it is either null or the will reference the sites configured 404 page.

d-gibbs avatar Aug 15 '22 08:08 d-gibbs

Hi there @d-gibbs!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

  • We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
  • If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
  • We'll replicate the issue to ensure that the problem is as described.
  • We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

github-actions[bot] avatar Aug 15 '22 08:08 github-actions[bot]

I have the same problem. Will this be fixed in the future?

tolpeit avatar Nov 02 '22 10:11 tolpeit

@tolpeit - We just worked around the issue by using UmbracoRouteValues in the end but it would be good to have a resolution to this. We're using a 3rd party plugin (SEOChecker) which has a bug because of this same issue which we've also had to work around.

d-gibbs avatar Nov 02 '22 14:11 d-gibbs

What should its behaviour be when the FindContent method returns null? In my case it doesn't enter the Index method, but just renders a 404. Not even the configured 404.

joepvtl avatar Nov 30 '22 13:11 joepvtl

Hi @d-gibbs,

Thank you for bringing this to our attention. And sorry for the late response 🙈

I can reproduce this in V10 and V11.

Currently your best bet for working around this is to either use CurrentPage within the controller, or obtain UmbracoRouteValues from the HTTP context features (using HttpContext.Features.Get<UmbracoRouteValues>()) and then use UmbracoRouteValues.PublishedRequest.PublishedContent.

We'd love some help fixing this, so I'm going to mark this as "up for grabs" - perhaps you or someone else would have a go at fixing it?

For anyone making an attempt at this, here are some steps to reproduce:

  1. Create a virtual controller using the code snippet below.
  2. Add the following code to WithEndpoints in Startup.cs:
u.EndpointRouteBuilder.MapControllerRoute(
	"Test Virtual Controller",
	"/test-virtual",
	new {Controller = "TestVirtual", Action = "Index"});

It should look like this: image

  1. Request the virtual controller at the /test-virtual route and verify that "Content from UmbracoContext" lists as "null": image

Code snippet for virtual controller

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.Common.Controllers;
using Umbraco.Cms.Web.Common.Routing;

namespace Issue12834;

public class TestVirtualController : UmbracoPageController, IVirtualPageController
{
    private readonly IUmbracoContextAccessor _umbracoContextAccessor;

    public TestVirtualController(
        ILogger<TestVirtualController> logger,
        ICompositeViewEngine compositeViewEngine,
        IUmbracoContextAccessor umbracoContextAccessor)
        : base(logger, compositeViewEngine)
    {
        _umbracoContextAccessor = umbracoContextAccessor;
    }

    public IActionResult Index()
    {
        if (_umbracoContextAccessor.TryGetUmbracoContext(out var umbracoContext) is false)
        {
            return Content("Error - Could not obtain an Umbraco context");
        }
        var umbracoRouteValues = HttpContext.Features.Get<UmbracoRouteValues>();
        if (umbracoRouteValues is null)
        {
            return Content("Error - Could not obtain an Umbraco route values");
        }
        
        var contentFromUmbracoContext = umbracoContext.PublishedRequest?.PublishedContent;
        var contentFromCurrentPage = CurrentPage;
        var contentFromRouteValues = umbracoRouteValues.PublishedRequest.PublishedContent;

        string ReportContent(IPublishedContent? content)
            => content is null
                ? "(null)"
                : $"{content.Name} ({content.ContentType.Alias}, {content.Id})";

        return Content(@$"
Content from CurrentPage: {ReportContent(contentFromCurrentPage)}
Content from UmbracoContext: {ReportContent(contentFromUmbracoContext)}
Content from UmbracoRouteValues: {ReportContent(contentFromRouteValues)}
");
    }

    public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext)
    {
        // grab the first content item at root no matter which route we're responding to
        return _umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext? umbracoContext) 
            ? umbracoContext.Content?.GetAtRoot().FirstOrDefault() 
            : null;
    }
}

kjac avatar Dec 14 '22 06:12 kjac

@nul800sebastiaan @kjac

I noticed this issue has been open for a while. Is there a plan to address it in the upcoming Hackathon? When is it planned?

In the meantime, I came up with a workaround which might be helpful. Please note that this is a quick fix and might require some adjustments to fit your own project.

public IPublishedContent CurrentPage()
        {
            var umbracoContext = umbracoContextAccessor.GetRequiredUmbracoContext();
            var currentPage = umbracoContext.PublishedRequest.PublishedContent;

            #region Dirty fix for Umbraco issue #12834 (https://github.com/umbraco/Umbraco-CMS/issues/12834)
// Check here first if currentpage is incorrect
if(...)
{
                var httpContext = httpContextAccessor.GetRequiredHttpContext();

                if (httpContext.Request.Path.StartsWithSegments(CategoryConsts.CategoriesUrlSlugHeatersFull, StringComparison.OrdinalIgnoreCase))
                {
                    var categoryRoot = ProductCategoriesPage(StoreEnum.Heaters);

                    var categoryPath = httpContext.Request.Path.ToString().Replace(CategoryConsts.CategoriesUrlSlugHeatersFull, categoryRoot.Id.ToString());

                    return umbracoContext.Content.GetByRoute(categoryPath);
                }
}
            #endregion

            return currentPage;
        }

merijng avatar Aug 17 '23 10:08 merijng