-
Notifications
You must be signed in to change notification settings - Fork 169
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
AST deferred value handling #629
Conversation
…rectly to partially resolve deferred expressions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some clarifications on a couple of things
|
||
TypeConverter converter = new TruthyTypeConverter(); | ||
this.expressionFactory = new ExpressionFactoryImpl(expConfig, converter); | ||
this.eagerExpressionFactory = new ExpressionFactoryImpl(eagerExpConfig, converter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a separate factory allows for one Jinjava
instance to do either Eager Execution or default execution, without needing a separate Jinjava instance.
@@ -7,5 +7,9 @@ default boolean isPreserveRawTags() { | |||
return false; | |||
} | |||
|
|||
default boolean useEagerParser() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flag to either use EagerExtendedParser
or ExtendedParser
. It changes what kind of AstNodes are created when parsing the abstract syntax tree
} | ||
result = | ||
interpreter.resolveELExpression( | ||
String.format("[%s]", value), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we wrap the expression in a list in case there are commas in the expression. Therefore, the result will always be a list wrapping whatever the expression would normally be. We later will unwrap this if needed.
return prefix + toSpaceOut.trim() + suffix; | ||
public static class ResolvedChunks { | ||
private final Object resolvedObject; | ||
private final boolean fullyResolved; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class lets us return either a fully resolved object or a partially resolved string image. Being able to return the exact object helps match the functionality when resolving any non-deferred expressions. (Previously, we could only return a string representation of the object)
The sooner we switch over to handling deferred values in expressions at the syntax tree-level, the better. So I will merge this in. |
This PR is a fleshed-out version of #527
Why do it this way
The current code in the Chunk Resolver is a hand-wavy approach to partially resolving expression and it has a few limitations. A large one is that it requires that the chunks are resolved into string form, even if nothing within the expression is deferred. Certain objects don't have an entirely valid string representation as it only deserializes back into a
SizeLimitingPyMap
, and we can't deserialize back into the original object without knowing its java class. This is a restriction that prevents extending Jinjava and using Eager Execution. An example would be HubSpot's extension of Jinjava (HubL), where HubDB java objects have special methods that would not work in aSizeLimitingPyMap
form.By integrating directly with the abstract syntax tree, deferred values can be handled more effectively. Rather than using a string parser that's attempting to handle all of the Jinjava syntax properly, we can handle the values in the ASTNodes and essentially not worry about parsing. This approach ensures that an expression is parsed into an AST once and evaluated once, which is faster and prevents cases where multiple calls would break the result (such as with the
append
operation). If the syntax tree can be evaluated without encountering any deferred value, then the result will be exactly the same as if it hadn't been done with Eager Execution. (This is pretty big because I keep needing to fix bugs from inconsistent expression resolving even in the absence of any deferred values).How does it work
A core part of this is a new type of exception: DeferredParsingException, which extends
DeferredValueException
. DeferredParsingException works by shooting up through the syntax tree and being handled by new Eager subclasses of the different AstNodes. If a deferred value is pulled from the context when evaluating the syntax tree, this exception will be thrown, signifying the name of the variable that was deferred. It will then be caught by the parent AstNode, which will partially resolve itself (how this happens depends on what kind of node it is), and then it will reconstruct a partially resolved image and rethrow the DeferredParsingException filling in thedeferredValueResult
with it's partially resolved image. The parent will catch and handle it similarly until eventually we make it back to the ChunkResolver, which originally calledresolveELExpression
. It will catch the exception, mark any unresolved variables as deferred and then signify that it's result is a partially resolved string which is the finalDeferredParsingException.deferredValueResult
.Take for example:
val=true
anddeferred
is a deferred value.This will be parsed to a tree that looks like:
EagerAstBinary(EagerAstIdentifier('deferred'), EagerAstIdentifier('val'))
. Call itA(B, C)
.deferred
.hasEvalResult()
, which saves the result when a node has successfully completed. It will be false if it threw a DeferredParsingException).deferredEvaluationResult
and combine it with the&&
operator and the evaluation result from C and throw a DeferredParsingException with the stringdeferred && true
deferred
to its set of deferred words. It will then return aResolvedChunks
object that is marked asfullyResolved=false
. This will wrap the string value that was in the final DeferredParsingException.If instead an expression is evaluated and there are no deferred values, then there won't be an exception to catch and the ChunkResolver can wrap the Object result with the
ResolvedChunks
wrapper marked asfullyResolved=true
. This object will never have been converted to or from a string so it is the exact same object as what would be returned when using default parsing and execution.This is the basics of how it works, but the expression actually is first wrapped in brackets so that we're resolving a list. The reason for this is in case we are handling a cycle tag or a multi-set tag. An expression with commas at the the root level (
#{5, 'go'}
) cannot be parsed so we wrap it as a list (#{[5, 'go']}
).Deferred Filters and Expression Tests
When a filter or expression test is deferred, it's output looks much different than what might be expected. This is because of how a filter or an expression test is parsed to the AST. Take for example
foo is equalto bar
. It is parsed into something like:If written out to a string, this would end up in:
exptest:equalto.evaluate(foo, ____int3rpr3t3r____, bar)
.Similarly a filter like
foo | length
would end up as a string:filter:length.filter(foo, ____int3rpr3t3r____)
.The parser required a little modification to be able to read these back as filters or exptests. It already handled namespace functions so
isPossibleExpTestOrFilter()
let it parse these kinds of strings if they are used in a template.