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

Add full_text_aspect liquid filter #7357

Merged
merged 3 commits into from
Oct 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ namespace OrchardCore.Contents.Handlers
/// <summary>
/// Provides the content for FullTextAspect based on FullTextAspectSettings
/// </summary>
public class FullTextAspectSettingsHandler : ContentHandlerBase
public class FullTextAspectContentHandler : ContentHandlerBase
deanmarcussen marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly IContentDefinitionManager _contentDefinitionManager;
private readonly ILiquidTemplateManager _liquidTemplateManager;
private readonly IServiceProvider _serviceProvider;

public FullTextAspectSettingsHandler(
public FullTextAspectContentHandler(
IContentDefinitionManager contentDefinitionManager,
ILiquidTemplateManager liquidTemplateManager,
IServiceProvider serviceProvider
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Threading.Tasks;
using OrchardCore.ContentManagement;
using OrchardCore.ContentManagement.Models;
Expand All @@ -21,10 +22,13 @@ public async Task BuildIndexAsync(BuildIndexContext context)
// Index each segment as a new value to prevent from allocation a new string
foreach (var segment in result.Segments)
{
context.DocumentIndex.Set(
IndexingConstants.FullTextKey,
segment,
DocumentIndexOptions.Analyze | DocumentIndexOptions.Sanitize);
if (!String.IsNullOrEmpty(segment))
deanmarcussen marked this conversation as resolved.
Show resolved Hide resolved
{
context.DocumentIndex.Set(
IndexingConstants.FullTextKey,
segment,
DocumentIndexOptions.Analyze | DocumentIndexOptions.Sanitize);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,19 @@ static async ValueTask<FluidValue> Awaited(Task<IShape> task)
throw new ArgumentException("Services missing while invoking 'shape_build_display'");
}

var serviceProvider = (IServiceProvider)services;

var buildDisplayRecursionHelper = serviceProvider.GetRequiredService<IContentItemRecursionHelper<BuildDisplayFilter>>();

// When {{ Model.ContentItem | shape_build_display | shape_render }} is called prevent recursion.
if (buildDisplayRecursionHelper.IsRecursive(contentItem))
{
return new ValueTask<FluidValue>(NilValue.Instance);
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure here that it can't be called more than once, not necessarily through the same model / shape, but on the same item, this in a given request scope?

}

var displayType = arguments["type"].Or(arguments.At(0)).ToStringValue();
var displayManager = ((IServiceProvider)services).GetRequiredService<IContentItemDisplayManager>();
var updateModelAccessor = ((IServiceProvider)services).GetRequiredService<IUpdateModelAccessor>();
var displayManager = serviceProvider.GetRequiredService<IContentItemDisplayManager>();
var updateModelAccessor = serviceProvider.GetRequiredService<IUpdateModelAccessor>();

var task = displayManager.BuildDisplayAsync(contentItem, updateModelAccessor.ModelUpdater, displayType);
if (task.IsCompletedSuccessfully)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
using System;
using System.Collections.Generic;
using OrchardCore.ContentManagement;
using OrchardCore.Liquid;

namespace OrchardCore.Contents.Liquid
{
/// <summary>
/// Prevents a content item being called by an implementation of a <see cref="ILiquidFilter"/> recursivly.
/// </summary>
public interface IContentItemRecursionHelper<T> where T : ILiquidFilter
{
/// <summary>
/// Returns <see langword="True"/> when the <see cref="ContentItem"/> has already been evaluated during this request by the particular filter./>
/// </summary>
bool IsRecursive(ContentItem contentItem);
}

/// <inheritdocs />
public class ContentItemRecursionHelper<T> : IContentItemRecursionHelper<T> where T : ILiquidFilter
{
private HashSet<string> _contentItemIds = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

/// <inheritdocs />
public bool IsRecursive(ContentItem contentItem)
{
if (_contentItemIds.Contains(contentItem.ContentItemId))
{
return true;
Copy link
Member

Choose a reason for hiding this comment

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

Sometimes i just cache the whole object and contains uses a by ref comparison

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I couldn't because there is too much JObject.ToObject<ContentItem> which breaks by ref

Copy link
Member

Choose a reason for hiding this comment

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

Cool then

}

_contentItemIds.Add(contentItem.ContentItemId);

return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Fluid;
using Fluid.Values;
using Microsoft.Extensions.DependencyInjection;
using Newtonsoft.Json.Linq;
using OrchardCore.ContentManagement;
using OrchardCore.ContentManagement.Models;
using OrchardCore.Liquid;

namespace OrchardCore.Contents.Liquid
{
public class FullTextFilter : ILiquidFilter
{
public async ValueTask<FluidValue> ProcessAsync(FluidValue input, FilterArguments arguments, TemplateContext ctx)
{
if (!ctx.AmbientValues.TryGetValue("Services", out var services))
{
throw new ArgumentException("Services missing while invoking 'full_text_aspect'");
}

var serviceProvider = (IServiceProvider)services;

var contentManager = serviceProvider.GetRequiredService<IContentManager>();
var fullTextRecursionHelper = serviceProvider.GetRequiredService<IContentItemRecursionHelper<FullTextFilter>>();

if (input.Type == FluidValues.Array)
{
var contentItems = new List<ContentItem>();
foreach(var objValue in input.Enumerate())
{
var contentItem = GetContentItem(objValue);
if (contentItem != null)
{
if (!fullTextRecursionHelper.IsRecursive(contentItem))
{
contentItems.Add(contentItem);
}
}
}

if (!contentItems.Any())
{
return NilValue.Instance;
}

var aspects = new List<FullTextAspect>();

foreach(var contentItem in contentItems)
{
aspects.Add(await contentManager.PopulateAspectAsync<FullTextAspect>(contentItem));
}

// When returning segments seperate them so multiple segments are indexed individually.
return new ArrayValue(aspects.SelectMany(x => x.Segments).Where(x => !String.IsNullOrEmpty(x)).Select(x => new StringValue(x + ' ')));
}
else
{
var contentItem = GetContentItem(input);

if (contentItem == null || fullTextRecursionHelper.IsRecursive(contentItem))
{
return NilValue.Instance;
}

var fullTextAspect = await contentManager.PopulateAspectAsync<FullTextAspect>(contentItem);

// Remove empty strings as display text is often unused in contained content items.
return new ArrayValue(fullTextAspect.Segments.Where(x => !String.IsNullOrEmpty(x)).Select(x => new StringValue(x)));
}
}

private static ContentItem GetContentItem(FluidValue input)
{
var obj = input.ToObjectValue();

if (!(obj is ContentItem contentItem))
{
contentItem = null;

if (obj is JObject jObject)
{
contentItem = jObject.ToObject<ContentItem>();
// If input is a 'JObject' but which not represents a 'ContentItem',
// a 'ContentItem' is still created but with some null properties.
if (contentItem?.ContentItemId == null)
{
return null;
}
}
}

return contentItem;
}
}
}
6 changes: 5 additions & 1 deletion src/OrchardCore.Modules/OrchardCore.Contents/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public override void ConfigureServices(IServiceCollection services)

// FullTextAspect
services.AddScoped<IContentTypeDefinitionDisplayDriver, FullTextAspectSettingsDisplayDriver>();
services.AddScoped<IContentHandler, FullTextAspectSettingsHandler>();
services.AddScoped<IContentHandler, FullTextAspectContentHandler>();

services.AddTagHelpers<ContentLinkTagHelper>();
services.AddTagHelpers<ContentItemTagHelper>();
Expand Down Expand Up @@ -138,6 +138,10 @@ public override void ConfigureServices(IServiceCollection services)
services.AddLiquidFilter<ContentItemFilter>("content_item_id");
services.AddLiquidFilter<DisplayTextFilter>("display_text");
services.AddLiquidFilter<DisplayUrlFilter>("display_url");
services.AddLiquidFilter<FullTextFilter>("full_text");

services.AddScoped<IContentItemRecursionHelper<FullTextFilter>, ContentItemRecursionHelper<FullTextFilter>>();
services.AddScoped<IContentItemRecursionHelper<BuildDisplayFilter>, ContentItemRecursionHelper<BuildDisplayFilter>>();
Copy link
Member

Choose a reason for hiding this comment

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

I would have registered a generic one

services.AddScoped(typeof(IContentItemRecursionHelper<>), typeof(ContentItemRecursionHelper<>));

So that another one could be resolved on demand without having to do another registration

Copy link
Member Author

Choose a reason for hiding this comment

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

I like that @jtkech didn't even know about it.

Will do some docs shortly, so will change that then.

}

public override void Configure(IApplicationBuilder builder, IEndpointRouteBuilder routes, IServiceProvider serviceProvider)
Expand Down