Limit content item recursions to 20

[deanmarcussen]

Fixes #7451

@jtkech I ended up using both a dictionary and hashset, because comparing by ref wasn't possible, because when it is recursing like this it's creating new shapes everytime, so new objects getting spawned.

I picked 20 as the number, as @jeffolmstead has already experienced the issue you spotted, trying to build multiple display types, on the same content item.

The other option is to remove recursion checking here on this filter completely.

But a bit of bad code, i.e. {{ Model.ContentItem | shape_build_display | shape_render }} does stackoverflow the system.

[jtkech]

@deanmarcussen

Okay for the counters per items, i would not have used an HashSet<string> but only a dictionary which is a kind of hashtable and fast enough, otherwise LGTM.

Not so important but can you remind me why we can't compare the item by ref, i understand if it is a new instance built from a JObject, but i didn't find how to do an infinite recursion on the same item id but never the same item instance. So okay, with the following i can see a recursion but always on the same item instance

{{ Model.ContentItem | shape_build_display | shape_render }}

Then i tried the following with the agency theme, indeed, because built from a JObject i can see different instances, but the recursions count is only equals to the explicit calls that we do, it's not an automatic infinite recursion

        {% for service in Model.ContentItem.Content.Services.ContentItems %}
            {{ service | shape_build_display | shape_render }}
            {{ service | shape_build_display | shape_render }}
            {{ service | shape_build_display | shape_render }}

The only way i can find to do automatically an infinite recursion, is when this is the current shape that renders a JObject item, or its parent shape, that is recursively called, so normally our existing recursion checking, that compares by ref the current model / shape, would have seen that it renders itself (max recursions = 3).

Another way is with the following if rendering the Model.ContentItem also renders child JObject items

{{ Model.ContentItem | shape_build_display | shape_render }}

But as said above your recursion checking would see it by comparing the ContentItem by ref.

[deanmarcussen]

@jtkech

by ref does work! because as you say the only time this can happen is when it is the Model.ContentItem which is always the same object, not serialized as new from JSON.

Validated with both full_text and shape_build_display

Also added as @sebastienros wanted it configurable is a max_recursions argument.

If you can't see any other issues can you merge this tonight, as it is a little broken right now. Cheers.

[jtkech]

@deanmarcussen okay

So, if max_recursions is an argument that may have different values, potentialy the counter may be already greater than a current passed value, so maybe i will replace if (counter == maxRecursions) with if (counter >= maxRecursions)

Or maybe with if (counter > maxRecursions) because the first call is not a first recursion ;)