From 12f3d6069c765925608931a0fbc4efcdfc047115 Mon Sep 17 00:00:00 2001 From: Libo Song Date: Fri, 22 Dec 2023 17:10:49 -0500 Subject: [PATCH 1/3] Make null values loop-able. --- .../hubspot/jinjava/interpret/NullValue.java | 19 +++++++++++++++++++ .../com/hubspot/jinjava/lib/tag/ForTag.java | 7 ++++++- .../hubspot/jinjava/lib/tag/ForTagTest.java | 12 ++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 src/main/java/com/hubspot/jinjava/interpret/NullValue.java diff --git a/src/main/java/com/hubspot/jinjava/interpret/NullValue.java b/src/main/java/com/hubspot/jinjava/interpret/NullValue.java new file mode 100644 index 000000000..4c6e61e26 --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/interpret/NullValue.java @@ -0,0 +1,19 @@ +package com.hubspot.jinjava.interpret; + +/** + * Marker object of a `null` value. A null value in the map is usually considered + * the key does not exist. For example map = {"a": null}, if map.get("a") == null, + * we treat it as the there is not key "a" in the map. + */ +public final class NullValue { + public static final NullValue INSTANCE = new NullValue(); + + static NullValue instance() { + return INSTANCE; + } + + @Override + public String toString() { + return "null"; + } +} 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 09e66ffda..00360475a 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java @@ -27,6 +27,7 @@ import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable; +import com.hubspot.jinjava.interpret.NullValue; import com.hubspot.jinjava.interpret.OutputTooBigException; import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.interpret.TemplateSyntaxException; @@ -200,7 +201,11 @@ public String renderForCollection( // set item variables if (loopVars.size() == 1) { - interpreter.getContext().put(loopVars.get(0), val); + if (val == null && interpreter.getContext().get(loopVars.get(0)) != null) { + interpreter.getContext().put(loopVars.get(0), NullValue.INSTANCE); + } else { + interpreter.getContext().put(loopVars.get(0), val); + } } else { for (int loopVarIndex = 0; loopVarIndex < loopVars.size(); loopVarIndex++) { String loopVar = loopVars.get(loopVarIndex); 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 9b5af78f6..bc3a7a515 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java @@ -377,6 +377,18 @@ public void itAllowsCheckingOfWithinForLoop() throws NoSuchMethodException { assertThat(rendered.getOutput()).isEqualTo("false true true false"); } + @Test + public void forLoopWithNullValues() { + context.put("number", -1); + context.put("the_list", Lists.newArrayList(1L, 2L, null, null, null)); + TagNode tagNode = (TagNode) fixture("loop-last-var"); + Document dom = Jsoup.parseBodyFragment(tag.interpret(tagNode, interpreter)); + + assertThat(dom.select("h3")).hasSize(4); + dom.outputSettings().prettyPrint(true).indentAmount(4); + assertThat(dom.html()).contains("seven: null"); + } + public static boolean inForLoop() { JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); return interpreter.getContext().isInForLoop(); From f5c36829613379b8f9ad1d4712e9687e8f5d7511 Mon Sep 17 00:00:00 2001 From: Libo Song Date: Wed, 27 Dec 2023 09:09:51 -0500 Subject: [PATCH 2/3] Explicit test and result. --- .../com/hubspot/jinjava/interpret/NullValue.java | 4 +++- .../com/hubspot/jinjava/lib/tag/ForTagTest.java | 13 +++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/interpret/NullValue.java b/src/main/java/com/hubspot/jinjava/interpret/NullValue.java index 4c6e61e26..ed3d7f0d3 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/NullValue.java +++ b/src/main/java/com/hubspot/jinjava/interpret/NullValue.java @@ -8,7 +8,9 @@ public final class NullValue { public static final NullValue INSTANCE = new NullValue(); - static NullValue instance() { + private NullValue() {} + + public static NullValue instance() { return INSTANCE; } 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 bc3a7a515..ec36d07f5 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/ForTagTest.java @@ -381,12 +381,13 @@ public void itAllowsCheckingOfWithinForLoop() throws NoSuchMethodException { public void forLoopWithNullValues() { context.put("number", -1); context.put("the_list", Lists.newArrayList(1L, 2L, null, null, null)); - TagNode tagNode = (TagNode) fixture("loop-last-var"); - Document dom = Jsoup.parseBodyFragment(tag.interpret(tagNode, interpreter)); - - assertThat(dom.select("h3")).hasSize(4); - dom.outputSettings().prettyPrint(true).indentAmount(4); - assertThat(dom.html()).contains("seven: null"); + String template = "{% for number in the_list %} {{ number }} {% endfor %}"; + TagNode tagNode = (TagNode) new TreeParser(interpreter, template) + .buildTree() + .getChildren() + .getFirst(); + String result = tag.interpret(tagNode, interpreter); + assertThat(result).isEqualTo(" 1 2 null null null "); } public static boolean inForLoop() { From ccb1721d4ebcf8ef9c5d2920dbfcc8afb4e987b8 Mon Sep 17 00:00:00 2001 From: Libo Song Date: Wed, 3 Jan 2024 09:21:28 -0500 Subject: [PATCH 3/3] Make this a legacy override instead of feature. --- .../java/com/hubspot/jinjava/LegacyOverrides.java | 13 +++++++++++++ .../java/com/hubspot/jinjava/lib/tag/ForTag.java | 6 +++++- .../java/com/hubspot/jinjava/BaseJinjavaTest.java | 6 +++++- .../jinjava/lib/tag/eager/EagerForTagTest.java | 6 +++++- 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java index f38e0f681..ce7b8d436 100644 --- a/src/main/java/com/hubspot/jinjava/LegacyOverrides.java +++ b/src/main/java/com/hubspot/jinjava/LegacyOverrides.java @@ -17,6 +17,7 @@ public class LegacyOverrides { .withParseWhitespaceControlStrictly(true) .withAllowAdjacentTextNodes(true) .withUseTrimmingForNotesAndExpressions(true) + .withKeepNullableLoopValues(true) .build(); private final boolean evaluateMapKeys; private final boolean iterateOverMapKeys; @@ -27,6 +28,7 @@ public class LegacyOverrides { private final boolean parseWhitespaceControlStrictly; private final boolean allowAdjacentTextNodes; private final boolean useTrimmingForNotesAndExpressions; + private final boolean keepNullableLoopValues; private LegacyOverrides(Builder builder) { evaluateMapKeys = builder.evaluateMapKeys; @@ -38,6 +40,7 @@ private LegacyOverrides(Builder builder) { parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly; allowAdjacentTextNodes = builder.allowAdjacentTextNodes; useTrimmingForNotesAndExpressions = builder.useTrimmingForNotesAndExpressions; + keepNullableLoopValues = builder.keepNullableLoopValues; } public static Builder newBuilder() { @@ -80,6 +83,10 @@ public boolean isUseTrimmingForNotesAndExpressions() { return useTrimmingForNotesAndExpressions; } + public boolean isKeepNullableLoopValues() { + return keepNullableLoopValues; + } + public static class Builder { private boolean evaluateMapKeys = false; private boolean iterateOverMapKeys = false; @@ -90,6 +97,7 @@ public static class Builder { private boolean parseWhitespaceControlStrictly = false; private boolean allowAdjacentTextNodes = false; private boolean useTrimmingForNotesAndExpressions = false; + private boolean keepNullableLoopValues = false; private Builder() {} @@ -168,5 +176,10 @@ public Builder withUseTrimmingForNotesAndExpressions( this.useTrimmingForNotesAndExpressions = useTrimmingForNotesAndExpressions; return this; } + + public Builder withKeepNullableLoopValues(boolean keepNullableLoopValues) { + this.keepNullableLoopValues = keepNullableLoopValues; + return this; + } } } 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 00360475a..c56008470 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/ForTag.java @@ -201,7 +201,11 @@ public String renderForCollection( // set item variables if (loopVars.size() == 1) { - if (val == null && interpreter.getContext().get(loopVars.get(0)) != null) { + if ( + val == null && + interpreter.getContext().get(loopVars.get(0)) != null && + interpreter.getConfig().getLegacyOverrides().isKeepNullableLoopValues() + ) { interpreter.getContext().put(loopVars.get(0), NullValue.INSTANCE); } else { interpreter.getContext().put(loopVars.get(0), val); diff --git a/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java b/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java index 86fd55ef7..e2a29d4b1 100644 --- a/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java +++ b/src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java @@ -12,7 +12,11 @@ public void baseSetup() { JinjavaConfig .newBuilder() .withLegacyOverrides( - LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() + LegacyOverrides + .newBuilder() + .withUsePyishObjectMapper(true) + .withKeepNullableLoopValues(true) + .build() ) .build() ); 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 f42a0b73f..c1ab9af63 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 @@ -32,7 +32,11 @@ public void eagerSetup() { .withMaxOutputSize(MAX_OUTPUT_SIZE) .withExecutionMode(EagerExecutionMode.instance()) .withLegacyOverrides( - LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build() + LegacyOverrides + .newBuilder() + .withUsePyishObjectMapper(true) + .withKeepNullableLoopValues(true) + .build() ) .build() );