diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java index 825a240b8..2905728c1 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java @@ -58,7 +58,7 @@ * @author anysome */ @JinjavaDoc( - value = "Outputs the inner content for each item in the given iterable", + value = "Outputs the inner content for each item in the given iterable, or the else block if the iterable is empty", params = { @JinjavaParam( value = "items_to_iterate", @@ -156,6 +156,19 @@ public String renderForCollection( Object collection ) { ForLoop loop = ObjectIterator.getLoop(collection); + boolean empty = !loop.hasNext(); + List children = tagNode.getChildren(); + int elseIndex = children.size(); + for (int i = 0; i < children.size(); i++) { + if ( + children.get(i) instanceof TagNode && + ElseTag.TAG_NAME.equals(children.get(i).getName()) + ) { + elseIndex = i; + break; + } + } + String result; try (InterpreterScopeClosable c = interpreter.enterScope()) { if (interpreter.isValidationMode() && !loop.hasNext()) { @@ -257,7 +270,7 @@ public String renderForCollection( } } - for (Node node : tagNode.getChildren()) { + for (Node node : children.subList(0, elseIndex)) { if (interpreter.getContext().isValidationMode()) { node.render(interpreter); } else { @@ -284,8 +297,35 @@ public String renderForCollection( throw new DeferredValueException(TOO_LARGE_EXCEPTION_MESSAGE); } } - return checkLoopVariable(interpreter, buff); + result = checkLoopVariable(interpreter, buff); } + if (elseIndex < children.size() && (empty || interpreter.isValidationMode())) { + try (InterpreterScopeClosable c = interpreter.enterScope()) { + if (!empty) { + interpreter.getContext().setValidationMode(true); + } + LengthLimitingStringBuilder buff = new LengthLimitingStringBuilder( + interpreter.getConfig().getMaxOutputSize() + ); + for (Node node : children.subList(elseIndex + 1, children.size())) { + if (interpreter.getContext().isValidationMode()) { + node.render(interpreter); + } else { + try { + buff.append(node.render(interpreter)); + } catch (OutputTooBigException e) { + if (interpreter.getConfig().getExecutionMode().useEagerParser()) { + throw new DeferredValueException(TOO_LARGE_EXCEPTION_MESSAGE); + } + interpreter.addError(TemplateError.fromOutputTooBigException(e)); + break; + } + } + } + result += buff.toString(); + } + } + return result; } private String checkLoopVariable( diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java index f87be2e68..8768b75cb 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java @@ -7,7 +7,9 @@ import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.lib.tag.ElseTag; import com.hubspot.jinjava.lib.tag.ForTag; +import com.hubspot.jinjava.tree.Node; import com.hubspot.jinjava.tree.TagNode; import com.hubspot.jinjava.tree.parse.TagToken; import com.hubspot.jinjava.util.EagerContextWatcher; @@ -153,6 +155,13 @@ public String eagerInterpret( ); } + for (int i = 0; i < tagNode.getChildren().size(); i++) { + Node child = tagNode.getChildren().get(i); + if (child instanceof TagNode && ElseTag.TAG_NAME.equals(child.getName())) { + return renderDeferredForElse(tagNode, interpreter, result, i); + } + } + EagerExecutionResult firstRunResult = runLoopOnce(tagNode, interpreter, true); PrefixToPreserveState prefixToPreserveState = firstRunResult .getPrefixToPreserveState() @@ -186,23 +195,123 @@ private EagerExecutionResult runLoopOnce( TagNode tagNode, JinjavaInterpreter interpreter, boolean clearDeferredWords + ) { + return runBranchOnce( + tagNode, + interpreter, + clearDeferredWords, + tagNode.getChildren(), + true + ); + } + + private String renderDeferredForElse( + TagNode tagNode, + JinjavaInterpreter interpreter, + LengthLimitingStringBuilder result, + int elseIndex + ) { + List body = tagNode.getChildren().subList(0, elseIndex); + List otherwise = tagNode + .getChildren() + .subList(elseIndex + 1, tagNode.getChildren().size()); + EagerExecutionResult firstBody = runBranchOnce( + tagNode, + interpreter, + true, + body, + true + ); + PrefixToPreserveState prefix = firstBody.getPrefixToPreserveState(); + Set bindingsToDefer = new HashSet<>( + EagerReconstructionUtils.resetSpeculativeBindings(interpreter, firstBody) + ); + EagerExecutionResult firstElse = runBranchOnce( + tagNode, + interpreter, + true, + otherwise, + false + ); + prefix.withAll(firstElse.getPrefixToPreserveState()); + bindingsToDefer.addAll( + EagerReconstructionUtils.resetSpeculativeBindings(interpreter, firstElse) + ); + prefix.withAllInFront( + EagerReconstructionUtils.deferWordsAndReconstructReferences( + interpreter, + bindingsToDefer + ) + ); + + // Re-evaluate both alternatives after all potentially modified values are deferred. + // Reset each branch before evaluating the other one, just as for eager if branches. + EagerExecutionResult secondBody = runBranchOnce( + tagNode, + interpreter, + false, + body, + true + ); + if (!bindingsToDefer.containsAll(secondBody.getSpeculativeBindings().keySet())) { + throw new DeferredValueException("Modified values in deferred for loop"); + } + result.append(secondBody.asTemplateString()); + prefix.withAllInFront( + EagerReconstructionUtils.resetAndDeferSpeculativeBindings(interpreter, secondBody) + ); + result.append(tagNode.getChildren().get(elseIndex).getMaster().getImage()); + EagerExecutionResult secondElse = runBranchOnce( + tagNode, + interpreter, + false, + otherwise, + false + ); + if (!bindingsToDefer.containsAll(secondElse.getSpeculativeBindings().keySet())) { + throw new DeferredValueException("Modified values in deferred for else block"); + } + result.append(secondElse.asTemplateString()); + prefix.withAllInFront( + EagerReconstructionUtils.resetAndDeferSpeculativeBindings(interpreter, secondElse) + ); + result.append(EagerReconstructionUtils.reconstructEnd(tagNode)); + return prefix.toString() + result; + } + + private EagerExecutionResult runBranchOnce( + TagNode tagNode, + JinjavaInterpreter interpreter, + boolean clearDeferredWords, + List children, + boolean loopBody ) { return EagerContextWatcher.executeInChildContext( eagerInterpreter -> { - if (!(eagerInterpreter.getContext().get(ForTag.LOOP) instanceof DeferredValue)) { + if ( + loopBody && + !(eagerInterpreter.getContext().get(ForTag.LOOP) instanceof DeferredValue) + ) { eagerInterpreter.getContext().put(ForTag.LOOP, DeferredValue.instance()); } - List loopVars = getTag() - .getLoopVarsAndExpression((TagToken) tagNode.getMaster()) - .getLeft(); + List loopVars = loopBody + ? getTag().getLoopVarsAndExpression((TagToken) tagNode.getMaster()).getLeft() + : Collections.emptyList(); interpreter.getContext().addNonMetaContextVariables(loopVars); loopVars.forEach(var -> interpreter.getContext().put(var, DeferredValue.instance()) ); try { - return EagerExpressionResult.fromString( - renderChildren(tagNode, eagerInterpreter) - ); + if (children == tagNode.getChildren()) { + return EagerExpressionResult.fromString( + renderChildren(tagNode, eagerInterpreter) + ); + } + StringBuilder output = new StringBuilder(); + for (Node child : children) { + output.append(child.render(eagerInterpreter).getValue()); + } + return EagerExpressionResult.fromString(output.toString()); } finally { interpreter.getContext().removeNonMetaContextVariables(loopVars); if (clearDeferredWords) { diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java index 1895a25a4..067184d49 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java @@ -18,6 +18,7 @@ import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.RenderResult; import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.interpret.UnknownTokenException; import com.hubspot.jinjava.lib.fn.ELFunctionDefinition; import com.hubspot.jinjava.objects.date.PyishDate; import com.hubspot.jinjava.tree.Node; @@ -25,6 +26,7 @@ import com.hubspot.jinjava.tree.TreeParser; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import java.util.Date; import java.util.List; import java.util.Map; @@ -64,6 +66,188 @@ public void baseSetup() { } } + @Test + public void itRendersElseForAnEmptyLoop() { + assertThat( + interpreter.render("{% for item in [] %}{{ item }}{% else %}EMPTY{% endfor %}") + ) + .isEqualTo("EMPTY"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itSkipsElseForANonEmptyLoop() { + assertThat( + interpreter.render( + "{% for item in ['a', 'b'] %}{{ item }}{% else %}EMPTY{% endfor %}" + ) + ) + .isEqualTo("ab"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itRendersElseForEmptyCollectionsAndNull() { + for (Object values : Arrays.asList( + null, + List.of(), + Map.of(), + new String[0], + List.of().iterator() + )) { + context.put("values", values); + assertThat( + interpreter.render("{% for item in values %}BODY{% else %}EMPTY{% endfor %}") + ) + .isEqualTo("EMPTY"); + } + assertThat( + interpreter.render( + "{% for item in ['a']|select('equalto', 'b') %}BODY{% else %}EMPTY{% endfor %}" + ) + ) + .isEqualTo("EMPTY"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itHandlesForElseWithArraysIteratorsAndMultipleLoopVariables() { + for (Object values : List.of( + new String[] { "a", "b" }, + List.of("a", "b").iterator() + )) { + TagNode loop = (TagNode) new TreeParser( + interpreter, + "{% for item in values %}{{ item }}{% else %}EMPTY{% endfor %}" + ) + .buildTree() + .getChildren() + .get(0); + assertThat( + new ForTag().renderForCollection(loop, interpreter, List.of("item"), values) + ) + .isEqualTo("ab"); + } + assertThat( + interpreter.render( + "{% for key, value in {'a': 1}.items() %}{{ key }}{{ value }}{% else %}EMPTY{% endfor %}" + ) + ) + .isEqualTo("a1"); + assertThat( + interpreter.render( + "{% for key, value in {}.items() %}BODY{% else %}EMPTY{% endfor %}" + ) + ) + .isEqualTo("EMPTY"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itKeepsNestedElseBlocksSeparate() { + assertThat( + interpreter.render( + "{% for item in [0, 1] %}{% if item %}Y{% else %}N{% endif %}" + + "{% for inner in [] %}INNER{% else %}E{% endfor %}" + + "{% else %}OUTER{% endfor %}" + ) + ) + .isEqualTo("NEYE"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itDoesNotRunElseAfterBreakOrContinue() { + for (String control : List.of("break", "continue")) { + assertThat( + interpreter.render( + "{% for item in [1, 2] %}{% " + control + " %}BODY{% else %}EMPTY{% endfor %}" + ) + ) + .isEmpty(); + } + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itKeepsElseAssignmentsLocalAndUsesOuterLoopVariables() { + assertThat( + interpreter.render( + "{% set item = 'outer' %}{% set value = 'before' %}" + + "{% for outer in [1, 2] %}" + + "{% for item in [] %}BODY{% else %}{{ loop.index }}{{ item }}{% set value = 'inside' %}{% endfor %}" + + "{% endfor %}{{ value }}" + ) + ) + .isEqualTo("1outer2outerbefore"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itSkipsSideEffectsAndErrorsInTheInactiveBranch() { + assertThat( + interpreter.render( + "{% set values = [] %}{% for item in [] %}{% do values.append('body') %}{{ badBody( }}" + + "{% else %}{% do values.append('else') %}{% endfor %}{{ values|join(',') }}" + ) + ) + .isEqualTo("else"); + assertThat( + interpreter.render( + "{% set values = [] %}{% for item in [1] %}{% do values.append('body') %}" + + "{% else %}{% do values.append('else') %}{{ badElse( }}{% endfor %}{{ values|join(',') }}" + ) + ) + .isEqualTo("body"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itPreservesElseWhitespaceAndAutoescaping() { + assertThat( + interpreter.render( + "A {%- for item in [] -%} BODY {%- else -%} EMPTY {%- endfor -%} Z" + ) + ) + .isEqualTo("AEMPTYZ"); + assertThat( + interpreter.render( + "{% autoescape true %}{% for item in [] %}BODY{% else %}{{ '' }}{% endfor %}{% endautoescape %}" + ) + ) + .isEqualTo("<b>"); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itRetainsUnknownVariableHandlingForForElse() { + assertThat( + interpreter.render("{% for item in missing %}BODY{% else %}EMPTY{% endfor %}") + ) + .isEqualTo("EMPTY"); + JinjavaInterpreter strict = new Jinjava( + BaseJinjavaTest.newConfigBuilder().withFailOnUnknownTokens(true).build() + ) + .newInterpreter(); + assertThatThrownBy(() -> + strict.render("{% for item in missing %}BODY{% else %}EMPTY{% endfor %}") + ) + .isInstanceOf(UnknownTokenException.class); + } + + @Test + public void itLimitsElseOutput() { + JinjavaInterpreter limited = new Jinjava( + BaseJinjavaTest.newConfigBuilder().withMaxOutputSize(5).build() + ) + .newInterpreter(); + assertThat(limited.render("{% for item in [] %}BODY{% else %}12345{% endfor %}")) + .isEqualTo("12345"); + assertThat(limited.getErrors()).isEmpty(); + limited.render("{% for item in [] %}BODY{% else %}123456{% endfor %}"); + assertThat(limited.getErrors()).isNotEmpty(); + } + @Test public void forLoopUsingLoopLastVar() { context.put("the_list", Lists.newArrayList(1L, 2L, 3L, 7L)); diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java index 3e0b1438f..a0c4eea92 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/ValidationModeTest.java @@ -70,6 +70,41 @@ public void setup() { ); } + @Test + public void itValidatesBothForElseBranchesWithoutRenderingTheInactiveBranch() { + for (String values : List.of("[]", "[1]")) { + JinjavaInterpreter validating = new Jinjava( + BaseJinjavaTest.newConfigBuilder().withValidationMode(true).build() + ) + .newInterpreter(); + String output = validating.render( + "{% for item in " + + values + + " %}BODY{{ badBody( }}{% else %}EMPTY{{ badElse( }}{% endfor %}" + ); + assertThat(output).isEqualTo(values.equals("[]") ? "EMPTY" : "BODY"); + assertThat(validating.getErrors()).hasSize(2); + } + } + + @Test + public void itDoesNotApplyChangesFromAValidatedForElseBranch() { + for (String values : List.of("[]", "[1]")) { + JinjavaInterpreter validating = new Jinjava( + BaseJinjavaTest.newConfigBuilder().withValidationMode(true).build() + ) + .newInterpreter(); + String output = validating.render( + "{% set result = [] %}{% for item in " + + values + + " %}{% do result.append('body') %}" + + "{% else %}{% do result.append('else') %}{% endfor %}{{ result|join(',') }}" + ); + assertThat(output).isEqualTo(values.equals("[]") ? "else" : "body"); + assertThat(validating.getErrors()).isEmpty(); + } + } + @Test public void itResolvesAllIfExpressionsInValidationMode() { validatingInterpreter.render( 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 ed256fda3..8bfce4c73 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 @@ -54,6 +54,110 @@ public void teardown() { JinjavaInterpreter.popCurrent(); } + @Test + public void itPreservesElseForADeferredLoop() { + String result = interpreter.render( + "{% for item in deferred %}{{ item }}{% else %}EMPTY{% endfor %}" + ); + assertThat(result).contains("{% else %}"); + for (List values : List.of(List.of(), List.of("a", "b"))) { + JinjavaInterpreter secondPass = jinjava.newInterpreter(); + secondPass.getContext().put("deferred", values); + assertThat(secondPass.render(result)).isEqualTo(values.isEmpty() ? "EMPTY" : "ab"); + assertThat(secondPass.getErrors()).isEmpty(); + } + } + + @Test + public void itKeepsDeferredForElseStateChangesInSeparateBranches() { + String template = + "{% set values = ['start'] %}{% set item = 'outer' %}" + + "{% for item in deferred %}{{ values|join(',') }}{% do values.append(item) %}{% set local = 'body' %}" + + "{% else %}{{ item }}:{{ values|join(',') }}{% do values.append('empty') %}{% set local = 'else' %}" + + "{% endfor %}|{{ values|join(',') }}|{{ local|default('missing') }}"; + String result = interpreter.render(template); + assertThat(result).contains("{% else %}"); + assertThat(interpreter.getErrors()).isEmpty(); + for (List values : List.of(List.of(), List.of("a", "b"))) { + assertSecondPassMatchesDirect(template, result, values); + } + } + + @Test + public void itResolvesDeferredValuesInTheSelectedElseBranch() { + String template = "{% for item in [] %}BODY{% else %}{{ deferred }}{% endfor %}"; + String result = interpreter.render(template); + assertThat(interpreter.getErrors()).isEmpty(); + assertSecondPassMatchesDirect(template, result, "resolved"); + } + + @Test + public void itPreservesNestedForElseAndOuterLoopValuesWhenDeferred() { + String template = + "{% for outer in ['a', 'b'] %}{% for item in deferred %}{{ outer }}{{ item }}" + + "{% else %}{{ outer }}{{ loop.index }}{% endfor %}{% endfor %}"; + String result = interpreter.render(template); + assertThat(interpreter.getErrors()).isEmpty(); + assertSecondPassMatchesDirect(template, result, List.of()); + assertSecondPassMatchesDirect(template, result, List.of("x", "y")); + } + + @Test + public void itPreservesDeferredLoopControlWithoutExecutingElse() { + for (String control : List.of("break", "continue")) { + String template = + "{% for item in [1, 2] %}{% if deferred %}{% " + + control + + " %}{% endif %}{{ item }}{% else %}EMPTY{% endfor %}"; + String result = interpreter.render(template); + assertSecondPassMatchesDirect(template, result, true); + assertSecondPassMatchesDirect(template, result, false); + } + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itPreservesDeferredElseWhitespaceAndAutoescaping() { + String template = + "{% autoescape true %}A {%- for item in deferred -%}{{ item }}" + + "{%- else -%}{{ '' }}{%- endfor -%} Z{% endautoescape %}"; + String result = interpreter.render(template); + assertSecondPassMatchesDirect(template, result, List.of()); + assertSecondPassMatchesDirect(template, result, List.of("")); + assertThat(interpreter.getErrors()).isEmpty(); + } + + @Test + public void itDefersElseWhenOutputIsTooLarge() { + String template = + "{% for item in [] %}BODY{% else %}" + + "a".repeat((int) MAX_OUTPUT_SIZE) + + "{% endfor %}"; + assertThat(interpreter.render(template)).hasSize((int) MAX_OUTPUT_SIZE); + assertThat(interpreter.getErrors()).isEmpty(); + String tooBig = + "{% for item in [] %}BODY{% else %}{% for i in range(501) %}1234567890{% endfor %}{% endfor %}"; + String deferred = interpreter.render(tooBig); + assertThat(deferred).contains("{% for i in range(501) %}", "{% endfor %}"); + assertThat(jinjava.newInterpreter().render(deferred)) + .isEqualTo("1234567890".repeat(501)); + assertThat(interpreter.getErrors()).isEmpty(); + } + + private void assertSecondPassMatchesDirect( + String template, + String partiallyRendered, + Object value + ) { + JinjavaInterpreter direct = jinjava.newInterpreter(); + direct.getContext().put("deferred", value); + JinjavaInterpreter secondPass = jinjava.newInterpreter(); + secondPass.getContext().put("deferred", value); + assertThat(secondPass.render(partiallyRendered)).isEqualTo(direct.render(template)); + assertThat(direct.getErrors()).isEmpty(); + assertThat(secondPass.getErrors()).isEmpty(); + } + @Test public void itRegistersDeferredToken() { expectedNodeInterpreter.assertExpectedOutput("registers-eager-token");