search

skttl / umbraco-fulltextsearch8

Full Text indexing and searching for Umbraco 8 and Examine.
MIT License
19 stars 21 forks source link

Is hijacked routes supported? #80

Closed enkelmedia closed 1 year ago

enkelmedia commented 1 year ago

Hi! I'm trying out this package but are having issues with pages that have a custom RenderController. Initialy the "FullTextContent"-field in the index was just empty and no errors in the log but I made a custom ICacheServer to look closer and turns out that the RenderTemplateAsync-method returned a error:

<!-- Error rendering template with id 1057: 'Umbraco.Cms.Web.Common.ModelBinders.ModelBindingException: Cannot bind source type site.Web.Models.Cms.CmsStartPage to model type site.Web.Pages.StartPage.ViewModels.StartPageViewModel.
   at Umbraco.Cms.Web.Common.ModelBinders.ContentModelBinder.ThrowModelBindingException(Boolean sourceContent, Boolean modelContent, Type sourceType, Type modelType)
   at Umbraco.Cms.Web.Common.ModelBinders.ContentModelBinder.BindModel(ModelBindingContext bindingContext, Object source, Type modelType)
   at Umbraco.Cms.Web.Common.Views.UmbracoViewPage`1.BindViewData(ContentModelBinder contentModelBinder, ViewDataDictionary viewData)
   at Umbraco.Cms.Web.Common.Views.UmbracoViewPage`1.set_ViewContext(ViewContext value)
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageCoreAsync(IRazorPage page, ViewContext context)
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderPageAsync(IRazorPage page, ViewContext context, Boolean invokeViewStarts)
   at Microsoft.AspNetCore.Mvc.Razor.RazorView.RenderAsync(ViewContext context)
   at Umbraco.Cms.Web.Common.Templates.TemplateRenderer.ExecuteTemplateRendering(TextWriter sw, IPublishedRequest request)
   at Umbraco.Cms.Web.Common.Templates.TemplateRenderer.RenderAsync(Int32 pageId, Nullable`1 altTemplateId, StringWriter writer)
   at Umbraco.Cms.Core.Templates.UmbracoComponentRenderer.RenderTemplateAsync(Int32 contentId, Nullable`1 altTemplateId)' -->

Since the "GetTextFromHtml"-method below strips the comments from the string only the empty text was stored in the index. So one thing might be to log any errors in the rendering.

But, my "real" question is about the support for the RenderControllers, it looks like the template rendering is not using the controller hence getting the wrong model passed. Is this by design?

Cheers!

skttl commented 1 year ago

Not completely, sorry.

If the hijacked route is using the ModelsBuilder model, it will work sometimes (depending on what changes you do to the model), but if you are supplying your own model, it will fail. Apparently the RenderTemplate method in Umbraco, doesn't take route hijacking into account.

I have been thinking of adding a Http mode, where the content of the page is fetched by using a HttpClient instead to mitigate this.

enkelmedia commented 1 year ago

Hi!

I totally understand! This made me understand I have the same issue with one of my packages.

I have made a custom CacheService with HttpClient that works well. I would be happy to create a PR if you want to? I’m guessing that this could be an option with the current template rendering as default as this is probably better for performance?

Maybe we could extract the "render this node to a HTML-string"-part into it's own thing something like a `IPageRenderer" or something similar with two different implementations.

skttl commented 1 year ago

I think the IPageRenderer route sounds like the right way to go. Then it's responsible for rendering, while cacheservice is responsible for caching. I've also had requests for being able to put the cache in external services like Relewise or Algolia, but that is for another issue :)

enkelmedia commented 1 year ago

Lovely! I'll create PR and then we can take from there =D

skttl commented 1 year ago

Just released 4.0.0-alpha001 with the HttpPageRenderer as the default :)