Feedback and proposal for new notifications

[enkelmedia]

Hi man!

Great progress on the v4-version!

I was looking at the hacks that I applied to our projects and most of these can now be facilitated with the core features of the package which is awesome! I still have one thing that I want to do and that I solved in my CustomCacheService.

Basically, we have a true/false-property on most nodes that says "Exclude from indexing" we use this recursively so if a parent node has this property checked any descendants will be excluded.

The implementation in my HackCacheService looks something like this:

Since I can't exclude all nodes of this type my solution here was to just store an empty string in the examine index and don't fetch the content using the HTTP-client.

In my case, this logic was added where the most left arrow is and if the page should be excluded I just did not perform the call to page renderer and just stored the fullHtml as an empty string.

I know that the new notifications might be a good fit for this but at the point where the notification fire I don't have access to the IPublishedContent to check the properties, I would have to re-fetch them to check the "no index"-property.

So I was thinking that there might be something that could be improved here, either:

1. Maybe a notification that is fired before the indexing for a certain IPublishedContent is starting to that we can return "Ignore" or something. Something like "Rendering" or similar.

2. Use another type in the current events that include the IPublishedContent? So that we could replace the content with an empty string. But this option is not so good since it would send an HTTP-request and then ignore the content, so would be better to be able to abort before the request is fired.

Looking forward to a new alpha with the notification stuff included =D

Cheers!

[enkelmedia]

Hmm... I remember that I made the implementation Render in HttpPageRenderer virtual to be able to support this requirement.

It could just be solved like this:

public class HttpPageRendererNoIndex : HttpPageRenderer
{
    public HttpPageRendererNoIndex(
        IOptions<FullTextSearchOptions> options, 
        IHttpClientFactory httpClientFactory, 
        ILogger<HttpPageRenderer> logger) : base(options, httpClientFactory, logger)
    {
    }

    public override async Task<string> Render(IPublishedContent publishedContent, PublishedCultureInfo culture)
    {
        bool shouldBeExcluded = publishedContent.Value<bool>(Core.Constants.ContentProperties.NoIndexNoFollowAlias) ||
                                publishedContent.Ancestors().Any(x => x.Value<bool>(Core.Constants.ContentProperties.NoIndexNoFollowAlias));

        if (shouldBeExcluded)
            return string.Empty;

        var html = await base.Render(publishedContent, culture);

        return html;
    }
}

If you think that there is no need for a notification at the point where I was proposing, I think we can just close this issue. The solution above works well.

[skttl]

I think implementing your own Renderer here is the right choice. Thanks for the thoughts and suggestion. We should probably add this to the documentation :)