Liquid templates ContentItems property names became case insensitive

[SzymonSel]

After upgrading from 1.9 to 2.0 we've noticed some of our liquid templates failing with a the error

ArgumentException: An item with the same key has already been added. Key: Camelcase (Parameter 'propertyName')

This happend because we are calling {{ field.MyProperty.CamelCase.Text }}

This just so happens that in some time during design and development we misstyped the name of the field and later changed it from Camelcase to CamelCase which left us with some harmless rubish in the JSON content... well at least until we upgraded to OC 2.0

{"CamelCase":{"Text":null},"Camelcase":{"Text":null}}

Is this switch, to case-insensitiveness was done intentionally?

[Piedone]

@MikeAlhayek can you comment on this, please?

[hishamco]

Might the naming policy have been change after introducing STJ

OrchardCore/src/OrchardCore/OrchardCore.Abstractions/Json/JOptions.cs

Lines 44 to 47 in 23ee4f1

	CamelCase = new JsonSerializerOptions(Default)
	{
	PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
	};

[hishamco]

I just notice

OrchardCore/src/OrchardCore/OrchardCore.Abstractions/Json/JOptions.cs

Line 19 in 23ee4f1

PropertyNameCaseInsensitive = true,

@MikeAlhayek it's on your plate while you complete the JT PR

[Piedone]

That's not something we want to change, though. It seems that while the previous de/serialization logic was more forgiving, in the end, the code had a bug that's now surfaced. That's good. We may make the exception more user-friendly though if feasible.

[SzymonSel]

What's the bug in particular?

It is true, that previous de/serialization logic was more forgiving, but the CaseInsesitivity is a mystery for me. I can't see it's purpose.

How do we ensure, this doesn't happen again? The user interface shouldn't allow this.

[Piedone]

The bug is in your code :). I don't think there is one in OC. Identifiers everywhere in C# and OC are case-sensitive apart from selected cases, so I don't see why we would try to allow case insensitivity for part or field names (which are case-sensitive anyway) in Liquid. (To clarify, what this issue is about is case-sensitivity, not case-insensitivity, since your code worked because previously the serializer was case-insensitive.)

[MikeAlhayek]

@hishamco I don't think PropertyNameCaseInsensitive = true, has anything to do with this.

When the PropertyNameCaseInsensitive property is set to true during deserializing, it allows you to map a property from any form back into the object.

For example, if you have this class

public class Person
{
    public string FirstName { get; set; }	
}

you can deserialize this with no problem.

{"FirstName":"Mike"}

But, the following will only be deserialize correctly if PropertyNameCaseInsensitive = true.

{"firstName":"Mike"}

I am not very familiar Fluid project which may be the cause of the issue. @sebastienros

[SzymonSel]

The bug is in your code :). I don't think there is one in OC. Identifiers everywhere in C# and OC are case-sensitive apart from selected cases, so I don't see why we would try to allow case insensitivity for part or field names (which are case-sensitive anyway) in Liquid. (To clarify, what this issue is about is case-sensitivity, not case-insensitivity, since your code worked because previously the serializer was case-insensitive.)

• No, you understood me wrong. It's the other way around. I want the indentifiers to be case sensitive, but they aren't anymore in Liquid Templates. Hence after an update to OC 2.0, we started having the above problem.

We created a field named "Camelcase", then we created some contentItems, the we removed the field "Camelcase" and created a new one named "CamelCase". Everything worked fine until we upgraded to OC 2.0

[Piedone]

Ah OK, got it, sorry about the confusion.

[sebastienros]

ArgumentException: An item with the same key has already been added. Key: Camelcase (Parameter 'propertyName')

What is the full stack trace to understand which component is not happy?

[sebastienros]

This could be from

OrchardCore/src/OrchardCore.Modules/OrchardCore.Liquid/Startup.cs

Line 51 in 9a8aa1c

options.MemberAccessStrategy.Register<JsonObject, object>((source, name) => source[name]);

Where in JSON.NET the accessor would be case insensitive, but STJ might be case-sensitive.

[sebastienros]

They behave the same, case sensitive https://dotnetfiddle.net/CxK4ls

[MikeAlhayek]

@SzymonSel can you please address the last comments from @sebastienros.

[SzymonSel]

Sure, I'll be back shortly with a reply!

[sebastienros]

Having the conflicting property names and case insensitivity support works fine with STJ, so this shouldn't be the problem, we'll wait for the stacktrace to understand where it's coming from:

Example

[sebastienros]

If possible see the liquid template that failed with such data. To get the data, create a TextField then another one with the similar name (might need to remove the previous one, the data will be kept in the json document)

[github-actions]

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

[SzymonSel]

If possible see the liquid template that failed with such data. To get the data, create a TextField then another one with the similar name (might need to remove the previous one, the data will be kept in the json document)

I'm unable to recreate the error (due to time constraints). I played around a bit and couldn't recreate the issue without manualy changing the JSON in the DB, hence this was a distant edge case. When migrating feom some older OC version.

I doubt anyone will ever experience it again.

Case closed!