[Design] Redesign member accessors

[sebastienros]

I'd like to get feedback on how member accessor limitations are applied on a model.

It appears that in most cases the model that is passed to the template doesn't need to be restricted. The opt-in behavior, which is already relaxed when the model is passed to the TemplateContext constructor, might not be necessary in most cases and induce some friction during first usages as only the first level properties are accessible. Users then don't understand why something like {{ Name }} works but not {{ Address.City }}.

I propose that these restrictions be removed by default, and optionally defined via either allow or disallow lists.

That means that accessing the model would be configured with 2 layers:

Property accessor

A way to return a value when a property of the model is accessed. Including fake properties.

Something like

var options = new TemplateOptions();

options.MemberAccessors.Add((value, name) => value is IUser user ? user.Name : null);

Value conversion

This example will replace any access that returns an IUser by its name. By returning null further conversions are applied.

options.ValueConverters.Add((value) => value is IUser user ? user.Name : null);

This can also be used to hide all instances of a type, or replace it with a wrapper that could expose other properties.

The following example prevents any instance of IUser from begin accessed.

options.ValueConverters.Add((value) => value is IUser ? NilValue.Instance : null);

Remarks

• We can use a custom value when it is rendered, different than when it is accessed. For instance {{ user }} would return user.Name, but we could still do user.Address.City? Use a value converter to return a custom FluidValue implementation that return the Name property in the ToString() call.
• Verify that all patterns used in Orchard Core are still applicable with this model, since it covers very uncommon cases. If it works in OC it works everywhere.
• Casing would now be defined independently of member accessors. The accessor should get the normalized name such that changing the casing of templates would not impact the delegates.
• Delegates should have access to the context.

[sebastienros]

Note:

• Add IndexerAccessors too.
• Provide generic implementations
• Member and Indexer are only used in ObjectValue.

[lahma]

I think for security (if it can be called that), it might suffice to by default block access to non-public properties, disallow write (is that possible in Fluid?) and block GetType() / Reflection. Then options to allow.

The MemberFilter which was introduced in Jint can also be quite convenient if end user has custom logic for exposing things.

If there a need for FluidPropertyAttribute that would expose with different name, similar to JsonProperty? Drawback is the strict binding to Fluid infrastructure.

[sebastienros]

it might suffice to by default block access to non-public properties, disallow write (is that possible in Fluid?) and block GetType() / Reflection. Then options to allow

That's the current state, I and agree it should not change. I believe that since only reads are allowed, users can just create custom "views/wrappers" to pass as the model if needed. I don't see why this should be implemented in the system, harder to maintain, guarantee, and less performant.

Issue with attributes is that the class needs to be read as a whole to find any property that could have them. Even if a single property needs to be accessed. I assume other "mapping" libraries can do that if necessary.

I think my suggestion for MemberAccessor will also provide the same functionality as MemberFilter. Since you can return Nil.Instance when a specific property should not be accessed.

Thanks for the feedback.