From b2e9fb2d3fb76a98ef4a9327e7c479c22dbe57e8 Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Fri, 23 Aug 2024 11:56:53 -0400 Subject: [PATCH 1/2] Fix PrefixToPreserveState creation by operating against DeferredLazyReference and not LazyReference. This was the intended functionality to make sure that the order of reconstructed values was such that {% set foo = [] %}{% set bar = foo %}, rather than {% set bar = foo %}{% set foo = [] %} would happen --- .../jinjava/lib/tag/eager/EagerExecutionResult.java | 10 ++++++---- src/test/java/com/hubspot/jinjava/EagerTest.java | 9 ++++++++- .../test.expected.jinja | 5 +++++ .../test.jinja | 8 ++++++++ .../test.expected.jinja} | 0 .../test.jinja} | 0 6 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.expected.jinja create mode 100644 src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.jinja rename src/test/resources/eager/{handles-duplicate-variable-reference-modification.expected.jinja => handles-duplicate-variable-reference-modification/test.expected.jinja} (100%) rename src/test/resources/eager/{handles-duplicate-variable-reference-modification.jinja => handles-duplicate-variable-reference-modification/test.jinja} (100%) diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerExecutionResult.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerExecutionResult.java index 2637ec3dd..1d9c1cf4e 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerExecutionResult.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerExecutionResult.java @@ -3,10 +3,10 @@ import static com.hubspot.jinjava.util.EagerReconstructionUtils.buildSetTag; import com.google.common.annotations.Beta; +import com.hubspot.jinjava.interpret.DeferredLazyReference; import com.hubspot.jinjava.interpret.DeferredLazyReferenceSource; import com.hubspot.jinjava.interpret.DeferredValueShadow; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import com.hubspot.jinjava.interpret.LazyReference; import com.hubspot.jinjava.objects.serialization.PyishObjectMapper; import com.hubspot.jinjava.util.EagerExpressionResolver.EagerExpressionResult; import com.hubspot.jinjava.util.EagerReconstructionUtils; @@ -65,7 +65,7 @@ public PrefixToPreserveState getPrefixToPreserveState() { .collect(Collectors.toList()); filteredEntries .stream() - .filter(entry -> !(entry.getValue() instanceof LazyReference)) + .filter(entry -> !(entry.getValue() instanceof DeferredLazyReference)) .forEach(entry -> EagerReconstructionUtils.hydrateBlockOrInlineSetTagRecursively( prefixToPreserveState, @@ -76,11 +76,13 @@ public PrefixToPreserveState getPrefixToPreserveState() { ); filteredEntries .stream() - .filter(entry -> (entry.getValue() instanceof LazyReference)) + .filter(entry -> (entry.getValue() instanceof DeferredLazyReference)) .map(entry -> new AbstractMap.SimpleImmutableEntry<>( entry.getKey(), - PyishObjectMapper.getAsPyishString(entry.getValue()) + PyishObjectMapper.getAsPyishString( + ((DeferredLazyReference) entry.getValue()).getOriginalValue() + ) ) ) .sorted((a, b) -> diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index e333c827f..edccf5642 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -1158,7 +1158,14 @@ public void itDefersCallTagWithDeferredArgumentSecondPass() { @Test public void itHandlesDuplicateVariableReferenceModification() { expectedTemplateInterpreter.assertExpectedOutputNonIdempotent( - "handles-duplicate-variable-reference-modification" + "handles-duplicate-variable-reference-modification/test" + ); + } + + @Test + public void itHandlesDuplicateVariableReferenceModification2() { + expectedTemplateInterpreter.assertExpectedOutputNonIdempotent( + "handles-duplicate-variable-reference-modification-2/test" ); } diff --git a/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.expected.jinja b/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.expected.jinja new file mode 100644 index 000000000..2a13936f5 --- /dev/null +++ b/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.expected.jinja @@ -0,0 +1,5 @@ +{% set foo = ['a', 1] %}{% set bar = foo %}{% if deferred %} +{% do bar.append(2) %} +{% endif %} +{% do bar.append(3) %} +{{ foo ~ 'and' ~ bar }} diff --git a/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.jinja b/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.jinja new file mode 100644 index 000000000..ecf1af7be --- /dev/null +++ b/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.jinja @@ -0,0 +1,8 @@ +{% set foo = ['a'] -%} +{%- set bar = foo -%} +{% do bar.append(1) %} +{% if deferred %} +{% do bar.append(2) %} +{% endif %} +{% do bar.append(3) %} +{{ foo ~ 'and' ~ bar }} diff --git a/src/test/resources/eager/handles-duplicate-variable-reference-modification.expected.jinja b/src/test/resources/eager/handles-duplicate-variable-reference-modification/test.expected.jinja similarity index 100% rename from src/test/resources/eager/handles-duplicate-variable-reference-modification.expected.jinja rename to src/test/resources/eager/handles-duplicate-variable-reference-modification/test.expected.jinja diff --git a/src/test/resources/eager/handles-duplicate-variable-reference-modification.jinja b/src/test/resources/eager/handles-duplicate-variable-reference-modification/test.jinja similarity index 100% rename from src/test/resources/eager/handles-duplicate-variable-reference-modification.jinja rename to src/test/resources/eager/handles-duplicate-variable-reference-modification/test.jinja From 63c995087302086340515fd5dd4c7700b3d28e9e Mon Sep 17 00:00:00 2001 From: Jack Smith Date: Fri, 23 Aug 2024 12:07:19 -0400 Subject: [PATCH 2/2] Use more descriptive test name --- src/test/java/com/hubspot/jinjava/EagerTest.java | 4 ++-- .../test.expected.jinja | 0 .../test.jinja | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/test/resources/eager/{handles-duplicate-variable-reference-modification-2 => handles-duplicate-variable-reference-speculative-modification}/test.expected.jinja (100%) rename src/test/resources/eager/{handles-duplicate-variable-reference-modification-2 => handles-duplicate-variable-reference-speculative-modification}/test.jinja (100%) diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index edccf5642..0bfdc6fa1 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -1163,9 +1163,9 @@ public void itHandlesDuplicateVariableReferenceModification() { } @Test - public void itHandlesDuplicateVariableReferenceModification2() { + public void itHandlesDuplicateVariableReferenceSpeculativeModification() { expectedTemplateInterpreter.assertExpectedOutputNonIdempotent( - "handles-duplicate-variable-reference-modification-2/test" + "handles-duplicate-variable-reference-speculative-modification/test" ); } diff --git a/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.expected.jinja b/src/test/resources/eager/handles-duplicate-variable-reference-speculative-modification/test.expected.jinja similarity index 100% rename from src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.expected.jinja rename to src/test/resources/eager/handles-duplicate-variable-reference-speculative-modification/test.expected.jinja diff --git a/src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.jinja b/src/test/resources/eager/handles-duplicate-variable-reference-speculative-modification/test.jinja similarity index 100% rename from src/test/resources/eager/handles-duplicate-variable-reference-modification-2/test.jinja rename to src/test/resources/eager/handles-duplicate-variable-reference-speculative-modification/test.jinja