diff --git a/src/main/java/com/hubspot/jinjava/el/ext/DeferredParsingException.java b/src/main/java/com/hubspot/jinjava/el/ext/DeferredParsingException.java index c8842308e..a58df5984 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/DeferredParsingException.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/DeferredParsingException.java @@ -5,11 +5,13 @@ public class DeferredParsingException extends DeferredValueException { private final String deferredEvalResult; private final Object sourceNode; + private final IdentifierPreservationStrategy identifierPreservationStrategy; public DeferredParsingException(String message) { super(message); this.deferredEvalResult = message; this.sourceNode = null; + this.identifierPreservationStrategy = IdentifierPreservationStrategy.RESOLVING; } public DeferredParsingException(Object sourceNode, String deferredEvalResult) { @@ -22,6 +24,24 @@ public DeferredParsingException(Object sourceNode, String deferredEvalResult) { ); this.deferredEvalResult = deferredEvalResult; this.sourceNode = sourceNode; + this.identifierPreservationStrategy = IdentifierPreservationStrategy.RESOLVING; + } + + public DeferredParsingException( + Object sourceNode, + String deferredEvalResult, + IdentifierPreservationStrategy identifierPreservationStrategy + ) { + super( + String.format( + "%s could not be parsed more than: %s", + sourceNode.getClass(), + deferredEvalResult + ) + ); + this.deferredEvalResult = deferredEvalResult; + this.sourceNode = sourceNode; + this.identifierPreservationStrategy = identifierPreservationStrategy; } public String getDeferredEvalResult() { @@ -31,4 +51,8 @@ public String getDeferredEvalResult() { public Object getSourceNode() { return sourceNode; } + + public IdentifierPreservationStrategy getIdentifierPreservationStrategy() { + return identifierPreservationStrategy; + } } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/IdentifierPreservationStrategy.java b/src/main/java/com/hubspot/jinjava/el/ext/IdentifierPreservationStrategy.java new file mode 100644 index 000000000..d3e6536dd --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/el/ext/IdentifierPreservationStrategy.java @@ -0,0 +1,20 @@ +package com.hubspot.jinjava.el.ext; + +public enum IdentifierPreservationStrategy { + PRESERVING(true), + RESOLVING(false); + + public static IdentifierPreservationStrategy preserving(boolean preserveIdentifier) { + return preserveIdentifier ? PRESERVING : RESOLVING; + } + + private final boolean preserving; + + IdentifierPreservationStrategy(boolean preserving) { + this.preserving = preserving; + } + + public boolean isPreserving() { + return preserving; + } +} diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBinary.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBinary.java index 95d92e917..63611056a 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBinary.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBinary.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.NoInvokeELContext; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.el.ext.OrOperator; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstBinary; @@ -48,7 +49,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return ( EvalResultHolder.reconstructNode( @@ -56,7 +57,7 @@ public String getPartiallyResolved( context, left, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) + String.format(" %s ", operator.toString()) + EvalResultHolder.reconstructNode( @@ -66,7 +67,7 @@ public String getPartiallyResolved( : context, right, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBracket.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBracket.java index 85bd00346..fd72a726d 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBracket.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBracket.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstBracket; import de.odysseus.el.tree.impl.ast.AstNode; @@ -63,7 +64,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return String.format( "%s[%s]", @@ -72,14 +73,14 @@ public String getPartiallyResolved( context, (EvalResultHolder) prefix, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ), EvalResultHolder.reconstructNode( bindings, context, (EvalResultHolder) property, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstChoice.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstChoice.java index dd5d0472f..17c709bb3 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstChoice.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstChoice.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.NoInvokeELContext; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstChoice; import de.odysseus.el.tree.impl.ast.AstNode; @@ -46,7 +47,12 @@ public Object eval(Bindings bindings, ELContext context) throws ELException { } throw new DeferredParsingException( this, - getPartiallyResolved(bindings, context, e, false) + getPartiallyResolved( + bindings, + context, + e, + IdentifierPreservationStrategy.RESOLVING + ) ); } } @@ -72,7 +78,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return ( EvalResultHolder.reconstructNode( @@ -80,7 +86,7 @@ public String getPartiallyResolved( context, question, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) + " ? " + EvalResultHolder.reconstructNode( @@ -88,7 +94,7 @@ public String getPartiallyResolved( new NoInvokeELContext(context), yes, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) + " : " + EvalResultHolder.reconstructNode( @@ -96,7 +102,7 @@ public String getPartiallyResolved( new NoInvokeELContext(context), no, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java index 48302a4a1..ea4ebd339 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java @@ -3,6 +3,7 @@ import com.hubspot.jinjava.el.ext.AstDict; import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.util.EagerExpressionResolver; import de.odysseus.el.tree.Bindings; @@ -34,7 +35,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { JinjavaInterpreter interpreter = (JinjavaInterpreter) context .getELResolver() @@ -52,7 +53,9 @@ public String getPartiallyResolved( context, (EvalResultHolder) key, deferredParsingException, - !interpreter.getConfig().getLegacyOverrides().isEvaluateMapKeys() + IdentifierPreservationStrategy.preserving( + !interpreter.getConfig().getLegacyOverrides().isEvaluateMapKeys() + ) ) ); } else { @@ -69,7 +72,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) value, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) ); } else { diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDot.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDot.java index 862d9620d..73e56ab3a 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDot.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDot.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstDot; import de.odysseus.el.tree.impl.ast.AstNode; @@ -52,11 +53,17 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException e, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return String.format( "%s.%s", - EvalResultHolder.reconstructNode(bindings, context, base, e, preserveIdentifier), + EvalResultHolder.reconstructNode( + bindings, + context, + base, + e, + identifierPreservationStrategy + ), property ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstIdentifier.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstIdentifier.java index a7b631e4a..ee72b3f7e 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstIdentifier.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstIdentifier.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstIdentifier; import javax.el.ELContext; @@ -43,7 +44,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return getName(); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstList.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstList.java index 6ca90e4a1..d0b80658a 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstList.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstList.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.ext.AstList; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstParameters; import java.util.StringJoiner; @@ -45,7 +46,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { StringJoiner joiner = new StringJoiner(", "); for (int i = 0; i < elements.getCardinality(); i++) { @@ -55,7 +56,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) elements.getChild(i), deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java index 8b5fc23dd..80f809dda 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java @@ -3,6 +3,7 @@ import com.hubspot.jinjava.el.ext.AstMacroFunction; import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; @@ -12,7 +13,6 @@ import java.lang.reflect.Array; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.StringJoiner; import javax.el.ELContext; import javax.el.ELException; @@ -54,7 +54,13 @@ public Object eval(Bindings bindings, ELContext context) { ); throw new DeferredParsingException( this, - getPartiallyResolved(bindings, context, e, true) // Need this to always be true because the macro function may modify the identifier + getPartiallyResolved( + bindings, + context, + e, + IdentifierPreservationStrategy.PRESERVING + ), // Need this to always be true because the macro function may modify the identifier + IdentifierPreservationStrategy.PRESERVING ); } } @@ -146,7 +152,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { if ( deferredParsingException != null && @@ -154,26 +160,20 @@ public String getPartiallyResolved( ) { return deferredParsingException.getDeferredEvalResult(); } - StringBuilder sb = new StringBuilder(); - sb.append(getName()); - try { - StringJoiner paramString = new StringJoiner(", "); - for (int i = 0; i < ((AstParameters) params).getCardinality(); i++) { - paramString.add( - EvalResultHolder.reconstructNode( - bindings, - context, - (EvalResultHolder) ((AstParameters) params).getChild(i), - deferredParsingException, - preserveIdentifier - ) + String paramString; + if (EvalResultHolder.exceptionMatchesNode(deferredParsingException, params)) { + paramString = deferredParsingException.getDeferredEvalResult(); + } else { + paramString = + params.getPartiallyResolved( + bindings, + context, + deferredParsingException, + identifierPreservationStrategy ); - } - sb.append(String.format("(%s)", paramString)); - } catch (DeferredParsingException dpe) { - sb.append(String.format("(%s)", dpe.getDeferredEvalResult())); } - return sb.toString(); + + return (getName() + String.format("(%s)", paramString)); } @Override diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethod.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethod.java index 366195214..413536cd7 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethod.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethod.java @@ -1,11 +1,10 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.DeferredValueException; -import com.hubspot.jinjava.util.EagerExpressionResolver; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstMethod; -import de.odysseus.el.tree.impl.ast.AstNode; import de.odysseus.el.tree.impl.ast.AstParameters; import de.odysseus.el.tree.impl.ast.AstProperty; import javax.el.ELContext; @@ -43,7 +42,13 @@ public Object eval(Bindings bindings, ELContext context) { ); throw new DeferredParsingException( this, - getPartiallyResolved(bindings, context, e, true) // Need this to always be true because the method may modify the identifier + getPartiallyResolved( + bindings, + context, + e, + IdentifierPreservationStrategy.PRESERVING + ), // Need this to always be true because the method may modify the identifier + IdentifierPreservationStrategy.PRESERVING ); } } @@ -73,7 +78,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { String propertyResult; propertyResult = @@ -81,25 +86,19 @@ public String getPartiallyResolved( bindings, context, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ); String paramString; - if ( - deferredParsingException != null && - deferredParsingException.getSourceNode() == params - ) { + if (EvalResultHolder.exceptionMatchesNode(deferredParsingException, params)) { paramString = deferredParsingException.getDeferredEvalResult(); } else { - try { - paramString = - EagerExpressionResolver.getValueAsJinjavaStringSafe( - ((AstNode) params).eval(bindings, context) - ); - // remove brackets so they can get replaced with parentheses - paramString = paramString.substring(1, paramString.length() - 1); - } catch (DeferredParsingException e) { - paramString = e.getDeferredEvalResult(); - } + paramString = + params.getPartiallyResolved( + bindings, + context, + deferredParsingException, + identifierPreservationStrategy + ); } return (propertyResult + String.format("(%s)", paramString)); diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNamedParameter.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNamedParameter.java index 8d1a23c61..5f70202cd 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNamedParameter.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNamedParameter.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.ext.AstNamedParameter; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstIdentifier; import de.odysseus.el.tree.impl.ast.AstNode; @@ -39,7 +40,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return String.format( "%s=%s", @@ -49,7 +50,7 @@ public String getPartiallyResolved( context, value, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNested.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNested.java index 317167c11..749006117 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNested.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNested.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.Node; import de.odysseus.el.tree.impl.ast.AstNode; @@ -71,7 +72,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return String.format( "(%s)", @@ -80,7 +81,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) child, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNodeDecorator.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNodeDecorator.java index 2b34b3071..7ea532357 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNodeDecorator.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstNodeDecorator.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.Node; import de.odysseus.el.tree.impl.ast.AstNode; @@ -64,7 +65,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return astNode.toString(); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java index 920bafff0..f6deaf6cd 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstParameters.java @@ -2,7 +2,9 @@ import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; +import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstNode; @@ -11,6 +13,7 @@ import java.util.StringJoiner; import java.util.stream.Collectors; import javax.el.ELContext; +import javax.el.ELException; public class EagerAstParameters extends AstParameters implements EvalResultHolder { protected Object evalResult; @@ -43,11 +46,24 @@ public Object[] eval(Bindings bindings, ELContext context) { ).getContext() .withPartialMacroEvaluation(false) ) { - return (Object[]) EvalResultHolder.super.eval( - () -> super.eval(bindings, context), - bindings, - context - ); + try { + setEvalResult(super.eval(bindings, context)); + return (Object[]) checkEvalResultSize(context); + } catch (DeferredValueException | ELException originalException) { + DeferredParsingException e = EvalResultHolder.convertToDeferredParsingException( + originalException + ); + throw new DeferredParsingException( + this, + getPartiallyResolved( + bindings, + context, + e, + IdentifierPreservationStrategy.PRESERVING + ), // Need this to always be true because a function may modify the identifier + IdentifierPreservationStrategy.PRESERVING + ); + } } } @@ -56,7 +72,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { StringJoiner joiner = new StringJoiner(", "); nodes @@ -70,7 +86,7 @@ public String getPartiallyResolved( context, node, deferredParsingException, - false + identifierPreservationStrategy ) ) ); diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstRangeBracket.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstRangeBracket.java index 0f4f8d4f6..c2fe057e3 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstRangeBracket.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstRangeBracket.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.ext.AstRangeBracket; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstNode; import javax.el.ELContext; @@ -42,7 +43,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return ( EvalResultHolder.reconstructNode( @@ -50,7 +51,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) prefix, deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) + "[" + EvalResultHolder.reconstructNode( @@ -58,7 +59,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) property, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) + ":" + EvalResultHolder.reconstructNode( @@ -66,7 +67,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) rangeMax, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) + "]" ); diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstTuple.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstTuple.java index e0c4b86b4..ab731cbee 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstTuple.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstTuple.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.ext.AstTuple; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstParameters; import java.util.StringJoiner; @@ -29,7 +30,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { StringJoiner joiner = new StringJoiner(", "); for (int i = 0; i < elements.getCardinality(); i++) { @@ -39,7 +40,7 @@ public String getPartiallyResolved( context, (EvalResultHolder) elements.getChild(i), deferredParsingException, - preserveIdentifier + identifierPreservationStrategy ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstUnary.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstUnary.java index ffdbdfc36..344f92701 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstUnary.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstUnary.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.el.ext.eager; import com.hubspot.jinjava.el.ext.DeferredParsingException; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import de.odysseus.el.tree.Bindings; import de.odysseus.el.tree.impl.ast.AstNode; import de.odysseus.el.tree.impl.ast.AstUnary; @@ -36,7 +37,7 @@ public String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ) { return ( operator.toString() + @@ -45,7 +46,7 @@ public String getPartiallyResolved( context, child, deferredParsingException, - false + IdentifierPreservationStrategy.RESOLVING ) ); } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java b/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java index 30de67e77..921289f7a 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/eager/EvalResultHolder.java @@ -2,6 +2,7 @@ import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.util.EagerExpressionResolver; @@ -34,7 +35,12 @@ default Object eval( ); throw new DeferredParsingException( this, - getPartiallyResolved(bindings, context, e, false) + getPartiallyResolved( + bindings, + context, + e, + IdentifierPreservationStrategy.RESOLVING + ) ); } } @@ -60,7 +66,7 @@ String getPartiallyResolved( Bindings bindings, ELContext context, DeferredParsingException deferredParsingException, - boolean preserveIdentifier + IdentifierPreservationStrategy identifierPreservationStrategy ); static String reconstructNode( @@ -68,19 +74,23 @@ static String reconstructNode( ELContext context, EvalResultHolder astNode, DeferredParsingException exception, - boolean preserveIdentifier + IdentifierPreservationStrategy preserveIdentifier ) { if (astNode == null) { return ""; } - preserveIdentifier |= - astNode instanceof AstIdentifier && - ExtendedParser.INTERPRETER.equals(((AstIdentifier) astNode).getName()); + preserveIdentifier = + IdentifierPreservationStrategy.preserving( + preserveIdentifier.isPreserving() || + astNode instanceof AstIdentifier && + ExtendedParser.INTERPRETER.equals(((AstIdentifier) astNode).getName()) + ); if ( - preserveIdentifier && + preserveIdentifier.isPreserving() && !astNode.hasEvalResult() && - !(exception != null && exception.getSourceNode() == astNode) + !(exceptionMatchesNode(exception, astNode)) ) { + // Evaluate to determine if the result is primitive. If so, we don't need to preserve the identifier try { EagerExpressionResolver.getValueAsJinjavaStringSafe( ((AstNode) astNode).eval(bindings, context) @@ -89,10 +99,16 @@ static String reconstructNode( } Object evalResult = astNode.getEvalResult(); if ( - !preserveIdentifier || + exceptionMatchesNode(exception, astNode) && + exception.getIdentifierPreservationStrategy().isPreserving() + ) { + return exception.getDeferredEvalResult(); + } + if ( + !preserveIdentifier.isPreserving() || (astNode.hasEvalResult() && EagerExpressionResolver.isPrimitive(evalResult)) ) { - if (exception != null && exception.getSourceNode() == astNode) { + if (exceptionMatchesNode(exception, astNode)) { return exception.getDeferredEvalResult(); } if (!astNode.hasEvalResult()) { @@ -106,7 +122,12 @@ static String reconstructNode( return EagerExpressionResolver.getValueAsJinjavaStringSafe(evalResult); } catch (DeferredValueException ignored) {} } - return astNode.getPartiallyResolved(bindings, context, exception, true); + return astNode.getPartiallyResolved( + bindings, + context, + exception, + IdentifierPreservationStrategy.PRESERVING + ); } static DeferredParsingException convertToDeferredParsingException( @@ -127,4 +148,14 @@ static DeferredParsingException convertToDeferredParsingException( } return null; } + + static boolean exceptionMatchesNode( + DeferredParsingException deferredParsingException, + Object astNode + ) { + return ( + deferredParsingException != null && + deferredParsingException.getSourceNode() == astNode + ); + } } diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index 65b12639d..a39842ec5 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -1053,7 +1053,10 @@ public void itReconstructsWithMultipleLoops() { @Test public void itFullyDefersFilteredMacro() { - expectedTemplateInterpreter.assertExpectedOutput("fully-defers-filtered-macro"); + // Macro and set tag reconstruction are flipped so not exactly idempotent, but functionally identical + expectedTemplateInterpreter.assertExpectedOutputNonIdempotent( + "fully-defers-filtered-macro" + ); } @Test diff --git a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java index 98c31584f..2f65c6e49 100644 --- a/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java +++ b/src/test/java/com/hubspot/jinjava/el/ext/eager/EagerAstMethodTest.java @@ -69,6 +69,27 @@ public void itPreservesIdentifier() { } } + @Test + public void itPreservesNonDeferredIdentifier() { + try { + interpreter.resolveELExpression("deferred.modify(foo_map)", -1); + fail("Should throw DeferredParsingException"); + } catch (DeferredParsingException e) { + assertThat(e.getDeferredEvalResult()).isEqualTo("deferred.modify(foo_map)"); + } + } + + @Test + public void itPreservesNonDeferredIdentifierWhenSecondParamIsDeferred() { + try { + interpreter.resolveELExpression("foo_list.modify(foo_map, deferred)", -1); + fail("Should throw DeferredParsingException"); + } catch (DeferredParsingException e) { + assertThat(e.getDeferredEvalResult()) + .isEqualTo("foo_list.modify(foo_map, deferred)"); + } + } + @Test public void itPreservesAstDot() { try { diff --git a/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java b/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java index 95d4cf0ee..b14b56a33 100644 --- a/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunctionTest.java @@ -81,7 +81,7 @@ public void itReconstructsForAliasedName() { public void itResolvesFromSet() { String template = "{% macro foo(foobar, other) %}" + - " {% do foobar.update({'a': 'b'} ) %} " + + " {% do foobar.update({'a': 'b'}) %} " + " {{ foobar }} and {{ other }}" + "{% endmacro %}" + "{% set bar = {} %}" + diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerForTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerForTagTest.java index bbfb3c523..f42a0b73f 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerForTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerForTagTest.java @@ -173,7 +173,7 @@ public void itDoesntAllowChangesInDeferredForWithSameHashCode() { "{% set foo = {'a': 'a'} %}{% for i in range(0, deferred) %}\n" + "bar{{ foo }}\n" + "{% do foo.clear() %}\n" + - "{% do foo.update({'b': 'b'} ) %}\n" + + "{% do foo.update({'b': 'b'}) %}\n" + "{% endfor %}\n" + "{{ foo }}" ); diff --git a/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java b/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java index 6f8dac047..2a2b22605 100644 --- a/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java +++ b/src/test/java/com/hubspot/jinjava/util/EagerExpressionResolverTest.java @@ -782,7 +782,7 @@ public void itHandlesToday() { @Test public void itHandlesRandom() { assertThat(eagerResolveExpression("range(1)|random").toString()) - .isEqualTo("filter:random.filter([0], ____int3rpr3t3r____)"); + .isEqualTo("filter:random.filter(range(1), ____int3rpr3t3r____)"); } @Test diff --git a/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja b/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja index 3fd6290f0..6eff41e08 100644 --- a/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja +++ b/src/test/resources/eager/fully-defers-filtered-macro.expected.jinja @@ -1,5 +1,6 @@ {% macro flashy(foo) %}{{ filter:upper.filter(foo, ____int3rpr3t3r____) }} - A flashy {{ deferred }}.{% endmacro %}{{ flashy(flashy('bar')) }} + A flashy {{ deferred }}.{% endmacro %}{% set __macro_flashy_1625622909_temp_variable_0__ %}BAR + A flashy {{ deferred }}.{% endset %}{{ flashy(__macro_flashy_1625622909_temp_variable_0__) }} --- -{% set __macro_silly_2092874071_temp_variable_0__ %}A silly {{ deferred }}.{% endset %}{{ filter:upper.filter(__macro_silly_2092874071_temp_variable_0__, ____int3rpr3t3r____) }} +{% set __macro_silly_2092874071_temp_variable_0__ %}A silly {{ deferred }}.{% endset %}{{ filter:upper.filter(__macro_silly_2092874071_temp_variable_0__, ____int3rpr3t3r____) }} \ No newline at end of file