From 94b7b37dcf33ad91fd3bcb24389b30b8ba5b28ac Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Wed, 10 Jan 2024 16:56:12 -0500 Subject: [PATCH] Handle deferred modification of an outer scoped variable inside a call tag --- .../jinjava/lib/tag/eager/EagerCallTag.java | 161 ++++++++++-------- .../java/com/hubspot/jinjava/EagerTest.java | 15 ++ ...fication-in-caller.expected.expected.jinja | 2 + ...rred-modification-in-caller.expected.jinja | 2 + ...dles-deferred-modification-in-caller.jinja | 11 ++ 5 files changed, 117 insertions(+), 74 deletions(-) create mode 100644 src/test/resources/eager/handles-deferred-modification-in-caller.expected.expected.jinja create mode 100644 src/test/resources/eager/handles-deferred-modification-in-caller.expected.jinja create mode 100644 src/test/resources/eager/handles-deferred-modification-in-caller.jinja diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCallTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCallTag.java index 363b709c6..b1faab124 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCallTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCallTag.java @@ -39,33 +39,39 @@ public String eagerInterpret( InterpretException e ) { interpreter.getContext().checkNumberOfDeferredTokens(); + MacroFunction caller; + EagerExecutionResult eagerExecutionResult; + PrefixToPreserveState prefixToPreserveState; + LengthLimitingStringJoiner joiner; try (InterpreterScopeClosable c = interpreter.enterNonStackingScope()) { - MacroFunction caller = new MacroFunction( - tagNode.getChildren(), - "caller", - new LinkedHashMap<>(), - true, - interpreter.getContext(), - interpreter.getLineNumber(), - interpreter.getPosition() - ); + caller = + new MacroFunction( + tagNode.getChildren(), + "caller", + new LinkedHashMap<>(), + true, + interpreter.getContext(), + interpreter.getLineNumber(), + interpreter.getPosition() + ); interpreter.getContext().addGlobalMacro(caller); - EagerExecutionResult eagerExecutionResult = EagerContextWatcher.executeInChildContext( - eagerInterpreter -> - EagerExpressionResolver.resolveExpression( - tagNode.getHelpers().trim(), - interpreter - ), - interpreter, - EagerContextWatcher - .EagerChildContextConfig.newBuilder() - .withTakeNewValue(true) - .withPartialMacroEvaluation( - interpreter.getConfig().isNestedInterpretationEnabled() - ) - .build() - ); - PrefixToPreserveState prefixToPreserveState = new PrefixToPreserveState(); + eagerExecutionResult = + EagerContextWatcher.executeInChildContext( + eagerInterpreter -> + EagerExpressionResolver.resolveExpression( + tagNode.getHelpers().trim(), + interpreter + ), + interpreter, + EagerContextWatcher + .EagerChildContextConfig.newBuilder() + .withTakeNewValue(true) + .withPartialMacroEvaluation( + interpreter.getConfig().isNestedInterpretationEnabled() + ) + .build() + ); + prefixToPreserveState = new PrefixToPreserveState(); if ( !eagerExecutionResult.getResult().isFullyResolved() || interpreter.getContext().isDeferredExecutionMode() @@ -93,63 +99,70 @@ public String eagerInterpret( ) ); } + caller.setDeferred(true); + // caller() needs to exist here so that the macro function can be reconstructed EagerReconstructionUtils.hydrateReconstructionFromContextBeforeDeferring( prefixToPreserveState, eagerExecutionResult.getResult().getDeferredWords(), interpreter ); + } - LengthLimitingStringJoiner joiner = new LengthLimitingStringJoiner( - interpreter.getConfig().getMaxOutputSize(), - " " - ); - joiner - .add(tagNode.getSymbols().getExpressionStartWithTag()) - .add(tagNode.getTag().getName()) - .add(eagerExecutionResult.getResult().toString().trim()) - .add(tagNode.getSymbols().getExpressionEndWithTag()); - prefixToPreserveState.withAllInFront( - EagerReconstructionUtils.handleDeferredTokenAndReconstructReferences( - interpreter, - DeferredToken - .builderFromImage(joiner.toString(), tagNode.getMaster()) - .addUsedDeferredWords(eagerExecutionResult.getResult().getDeferredWords()) - .build() - ) - ); - StringBuilder result = new StringBuilder(prefixToPreserveState + joiner.toString()); - interpreter.getContext().setDynamicVariableResolver(s -> DeferredValue.instance()); - if (!tagNode.getChildren().isEmpty()) { - result.append( - EagerContextWatcher - .executeInChildContext( - eagerInterpreter -> - EagerExpressionResult.fromString( - renderChildren(tagNode, eagerInterpreter) - ), - interpreter, - EagerContextWatcher - .EagerChildContextConfig.newBuilder() - .withForceDeferredExecutionMode(true) - .build() - ) - .asTemplateString() - ); - } - if ( - StringUtils.isNotBlank(tagNode.getEndName()) && - ( - !(getTag() instanceof FlexibleTag) || - ((FlexibleTag) getTag()).hasEndTag((TagToken) tagNode.getMaster()) - ) - ) { - result.append(EagerReconstructionUtils.reconstructEnd(tagNode)); - } // Possible set tag in front of this one. - return EagerReconstructionUtils.wrapInAutoEscapeIfNeeded( - result.toString(), - interpreter + // Now preserve those variables from the scope the call tag was called in + prefixToPreserveState.withAllInFront( + new EagerExecutionResult( + eagerExecutionResult.getResult(), + eagerExecutionResult.getSpeculativeBindings() + ) + .getPrefixToPreserveState() + ); + joiner = + new LengthLimitingStringJoiner(interpreter.getConfig().getMaxOutputSize(), " "); + joiner + .add(tagNode.getSymbols().getExpressionStartWithTag()) + .add(tagNode.getTag().getName()) + .add(eagerExecutionResult.getResult().toString().trim()) + .add(tagNode.getSymbols().getExpressionEndWithTag()); + prefixToPreserveState.withAllInFront( + EagerReconstructionUtils.handleDeferredTokenAndReconstructReferences( + interpreter, + DeferredToken + .builderFromImage(joiner.toString(), tagNode.getMaster()) + .addUsedDeferredWords(eagerExecutionResult.getResult().getDeferredWords()) + .build() + ) + ); + + StringBuilder result = new StringBuilder(prefixToPreserveState + joiner.toString()); + interpreter.getContext().setDynamicVariableResolver(s -> DeferredValue.instance()); + if (!tagNode.getChildren().isEmpty()) { + result.append( + EagerContextWatcher + .executeInChildContext( + eagerInterpreter -> + EagerExpressionResult.fromString(renderChildren(tagNode, eagerInterpreter)), + interpreter, + EagerContextWatcher + .EagerChildContextConfig.newBuilder() + .withForceDeferredExecutionMode(true) + .build() + ) + .asTemplateString() ); } + if ( + StringUtils.isNotBlank(tagNode.getEndName()) && + ( + !(getTag() instanceof FlexibleTag) || + ((FlexibleTag) getTag()).hasEndTag((TagToken) tagNode.getMaster()) + ) + ) { + result.append(EagerReconstructionUtils.reconstructEnd(tagNode)); + } // Possible set tag in front of this one. + return EagerReconstructionUtils.wrapInAutoEscapeIfNeeded( + result.toString(), + interpreter + ); } } diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index 7d484766a..9dcba381f 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -1509,4 +1509,19 @@ public void itFailsOnModificationInAliasedMacro() { // We don't support this assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty(); } + + @Test + public void itHandlesDeferredModificationInCaller() { + expectedTemplateInterpreter.assertExpectedOutput( + "handles-deferred-modification-in-caller" + ); + } + + @Test + public void itHandlesDeferredModificationInCallerSecondPass() { + interpreter.getContext().put("deferred", "c"); + expectedTemplateInterpreter.assertExpectedOutput( + "handles-deferred-modification-in-caller.expected" + ); + } } diff --git a/src/test/resources/eager/handles-deferred-modification-in-caller.expected.expected.jinja b/src/test/resources/eager/handles-deferred-modification-in-caller.expected.expected.jinja new file mode 100644 index 000000000..06d144d5b --- /dev/null +++ b/src/test/resources/eager/handles-deferred-modification-in-caller.expected.expected.jinja @@ -0,0 +1,2 @@ +['a', 'b', 'c'] +['a', 'b', 'c', 'd'] diff --git a/src/test/resources/eager/handles-deferred-modification-in-caller.expected.jinja b/src/test/resources/eager/handles-deferred-modification-in-caller.expected.jinja new file mode 100644 index 000000000..4412a84d1 --- /dev/null +++ b/src/test/resources/eager/handles-deferred-modification-in-caller.expected.jinja @@ -0,0 +1,2 @@ +{% set my_list = ['a', 'b'] %}{% set __macro_callerino_729568755_temp_variable_0__ %}{% do my_list.append(deferred) %}{{ my_list }}{% do my_list.append('d') %}{% endset %}{% call __macro_callerino_729568755_temp_variable_0__ %}{% do my_list.append(deferred) %}{{ my_list }}{% endcall %} +{{ my_list }} diff --git a/src/test/resources/eager/handles-deferred-modification-in-caller.jinja b/src/test/resources/eager/handles-deferred-modification-in-caller.jinja new file mode 100644 index 000000000..c0ae33a3c --- /dev/null +++ b/src/test/resources/eager/handles-deferred-modification-in-caller.jinja @@ -0,0 +1,11 @@ +{% macro callerino() -%} + {% do my_list.append('b') -%} + {{ caller() }} + {%- do my_list.append('d') -%} +{%- endmacro %} +{% set my_list = ['a'] %} +{% call callerino() -%} + {%- do my_list.append(deferred) -%} + {{ my_list }} +{%- endcall %} +{{ my_list }}