fluid icon indicating copy to clipboard operation
fluid copied to clipboard

Validate context model calls when rendering

Open gluhovds opened this issue 7 years ago • 11 comments

If I access a non-existing context property in a template, like this:

{{ nonExistingProperty }}

then I don't get any errors when calling FluidTemplate.Render. Such objects are just ignored and not rendered.

Is there a way to validate them and either throw exception or render error text in a place of invalid call?

Thanks in advance

gluhovds avatar Feb 28 '18 10:02 gluhovds

We could have a strict mode option that would throw an exception in this case.

sebastienros avatar Mar 27 '19 17:03 sebastienros

A strict mode would be great.

blue56 avatar Apr 26 '19 19:04 blue56

Any chance to have this feature soon?

HamedFathi avatar Dec 03 '21 19:12 HamedFathi

Should be done soon yes. I recently discovered these settings in Shopify and I think they are great. https://github.com/Shopify/liquid#undefined-variables-and-filters

sebastienros avatar Dec 03 '21 19:12 sebastienros

@sebastienros

You did an amazing job, honestly, I made a tricky way with dotLiquid here. It works but it is not good. I prefer to switch to Fluid but when this feature is available.

Thanks for your quick reply.

HamedFathi avatar Dec 03 '21 19:12 HamedFathi

We could have a strict mode option that would throw an exception in this case.

@sebastienros can such option goes into TemplateOptions?

hishamco avatar Dec 05 '21 16:12 hishamco

Could this be split into three different flags? I'm thinking something like this:

StrictMembers=true access of any non-existing property should result in error "undefined property '<name>' at <row>:<col>"
StrictMembers=false access of any non-existing property should result in null

StrictTraversal=true access of any property on null should result in error "cannot access property '<name>' of null at <row>:<col>"
StrictTraversal=false access of any property on null should result in null

StrictSecurity=true access of any inaccessible property should result in error "access to property '<name>' denied at <row>:<col>"
StrictSecurity=false access of any inaccessible property should result in null

This would be useful for debugging while allowing looser rules on production - for example we'd rather an e-mail based on template was sent slightly incomplete than not at all (StrictTraversal=false), but while debugging I'd like this validation enabled.

Samples: https://gist.github.com/Kukkimonsuta/9654183b63a31a53c8dedead2f384f52

Kukkimonsuta avatar Dec 28 '21 12:12 Kukkimonsuta

Should be done soon yes. I recently discovered these settings in Shopify and I think they are great. https://github.com/Shopify/liquid#undefined-variables-and-filters

Is there a possible workaround until this feature is implemented?

Edited: This is a POC that can be improved and extended on for anybody facing the same issue:

     class NullAssertionMemberAccessStrategy: DefaultMemberAccessStrategy
        {
            public override IMemberAccessor GetAccessor(Type type, string name)
            {
                var accessor = base.GetAccessor(type, name);

                if (accessor == null)
                {
                    throw new Exception($"Accessor for {name} not found");
                }

                return accessor;
            }
        }
var templateOptions = new TemplateOptions { 
                MemberAccessStrategy = new NullMemberAccessStrategy(),
            };

WSeegers avatar Feb 16 '22 09:02 WSeegers

A feature like this would be great. I'm not super familiar with Fluid yet but I'm a fan and have just started using it for a new project. Here is some more context from me as a user of this library:

A use case I am working on right now is validating Liquid templates prior to storing in a database. At that point we don't even have a populated model for the parameters, but we do have a parameter model class that can be used to indicate which parameters are acceptable. It would work just fine for us to try to "render" with an empty object and just collect any errors from a "strict mode" for properties that don't exist on the model, but another option for this use case would be to have a "ValidateBindings" method that could be run separately from Parse.

There are three types of validation I am working on running: Parseable (valid Liquid), bindable (valid parameter properties referenced in the Liquid template), and valid HTML. I think bindable is the only tricky part since parsing will just print empty strings for unfound properties at this time.

I tried to make due with using the workaround mentioned above which works in simple scenarios but doesn't check some situations such as things nested in for loops (only checks top level). (We have layers of nested objects/lists of objects in our parameter type that we need to handle.) I also tried to use the Handlebars library as a workaround just for this validation since it has a ThrowOnUnresolvedBindingException option, but that did not handle nested for loops. I'm still open to finding ways for me to use someone else's work on this. If this is expected to come out within the next month please let me know and maybe I can help review/test it a bit if you want.

If my team still thinks it's important enough to spend the time to develop this now one way I was thinking of accomplishing this is starting with

List<Statement> statements = _parser.Grammar.Parse(source)

Then looping through the statements using polymorphism (to handle different types of statements like OutputStatements and ForStatements appropriately) and recursion (because things like ForStatements can have statements within) to check all the Identifier fields against our model. I'd also have to collect additional identifiers in the appropriate scopes because of assign statements, variable names in for loops, etc. It's a bit tricky and there are multiple cases to think through. Maybe there is a better way - I'll have to investigate the existing parsing logic more before considering whether to go forward.

Just wanted to share as additional context and also some additional support for the case that this feature would be useful for users of this library. If anyone has suggestions that's also welcome. Thanks!

EDIT: I am going to go with the workaround I think. Validating the advanced cases (nested forloops and such) aren't important enough to worry about right now for us, but we plan to use that if it becomes a part of Fluid.

CameronJayBarton avatar Mar 29 '22 15:03 CameronJayBarton

Should be done soon yes. I recently discovered these settings in Shopify and I think they are great. https://github.com/Shopify/liquid#undefined-variables-and-filters

@sebastienros Has this feature been implemented into fluid yet?

patsfan247 avatar Mar 06 '23 17:03 patsfan247

same question from here - will this ever get done? it is a sorely missing feature

mbje-saxo avatar Nov 08 '23 12:11 mbje-saxo