diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java index 50c74ef83..8ce0faac3 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerSetTagStrategy.java @@ -1,6 +1,7 @@ package com.hubspot.jinjava.lib.tag.eager; import com.google.common.annotations.Beta; +import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.lib.tag.SetTag; import com.hubspot.jinjava.lib.tag.eager.importing.AliasedEagerImportingStrategy; @@ -166,6 +167,11 @@ public static String getSuffixToPreserveState( !AliasedEagerImportingStrategy.isTemporaryImportAlias(variables) && !interpreter.getContext().getMetaContextVariables().contains(variables) ) { + if (!interpreter.getContext().containsKey(maybeTemporaryImportAlias.get())) { + throw new DeferredValueException( + "Cannot modify temporary import alias outside of import tag" + ); + } String updateString = getUpdateString(variables); // Don't need to render because the temporary import alias's value is always deferred, and rendering will do nothing diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/AliasedEagerImportingStrategy.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/AliasedEagerImportingStrategy.java index 48c5c6523..9558c4b4b 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/AliasedEagerImportingStrategy.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/importing/AliasedEagerImportingStrategy.java @@ -115,9 +115,17 @@ public void integrateChild(JinjavaInterpreter child) { childBindings.remove(temporaryImportAlias); importingData.getOriginalInterpreter().getContext().remove(temporaryImportAlias); // Remove meta keys - childBindings.remove(Context.GLOBAL_MACROS_SCOPE_KEY); - childBindings.remove(Context.IMPORT_RESOURCE_ALIAS_KEY); - mapForCurrentContextAlias.putAll(childBindings); + childBindings + .entrySet() + .stream() + .filter( + entry -> + !( + entry.getKey().equals(Context.GLOBAL_MACROS_SCOPE_KEY) || + entry.getKey().equals(Context.IMPORT_RESOURCE_ALIAS_KEY) + ) + ) + .forEach(entry -> mapForCurrentContextAlias.put(entry.getKey(), entry.getValue())); } @Override diff --git a/src/test/java/com/hubspot/jinjava/EagerTest.java b/src/test/java/com/hubspot/jinjava/EagerTest.java index 618f08c4b..7d484766a 100644 --- a/src/test/java/com/hubspot/jinjava/EagerTest.java +++ b/src/test/java/com/hubspot/jinjava/EagerTest.java @@ -1499,4 +1499,14 @@ public void itAllowsVariableSharingAliasName() { "allows-variable-sharing-alias-name" ); } + + @Test + public void itFailsOnModificationInAliasedMacro() { + String input = expectedTemplateInterpreter.getFixtureTemplate( + "fails-on-modification-in-aliased-macro" + ); + interpreter.render(input); + // We don't support this + assertThat(interpreter.getContext().getDeferredNodes()).isNotEmpty(); + } } diff --git a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java index 0357a1192..3414d64a2 100644 --- a/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTagTest.java @@ -344,7 +344,7 @@ public void itDefersTripleLayer() { context.put("b_val", "b"); context.put("c_val", "c"); String result = interpreter.render( - "{% import 'import-tree-c.jinja' as c %}{{ c|dictsort(false, 'key') }}" + "{% import 'import-tree-c.jinja' as c %}{{ c.b|dictsort(false, 'key') }}{{ c.foo_b }}{{ c.import_resource_path }}" ); assertThat(interpreter.render("{{ c.b.a.foo_a }}")).isEqualTo("{{ c.b.a.foo_a }}"); assertThat(interpreter.render("{{ c.b.foo_b }}")).isEqualTo("{{ c.b.foo_b }}"); @@ -355,7 +355,7 @@ public void itDefersTripleLayer() { assertThat(interpreter.render(result).trim()) .isEqualTo( interpreter.render( - "{% import 'import-tree-c.jinja' as c %}{{ c|dictsort(false, 'key') }}" + "{% import 'import-tree-c.jinja' as c %}{{ c.b|dictsort(false, 'key') }}{{ c.foo_b }}{{ c.import_resource_path }}" ) ); } diff --git a/src/test/resources/eager/allows-variable-sharing-alias-name.expected.jinja b/src/test/resources/eager/allows-variable-sharing-alias-name.expected.jinja index fe7bb0da9..470103a36 100644 --- a/src/test/resources/eager/allows-variable-sharing-alias-name.expected.jinja +++ b/src/test/resources/eager/allows-variable-sharing-alias-name.expected.jinja @@ -2,6 +2,6 @@ {% set bar = deferred %}{% do __temp_import_alias_854547461__.update({'bar': bar}) %} {% set filters = {} %}{% do __temp_import_alias_854547461__.update({'filters': filters}) %}{% do filters.update(deferred) %} -{% do __temp_import_alias_854547461__.update({'bar': bar,'import_resource_path': 'filters.jinja','filters': filters,'foo': 123}) %}{% endfor %}{% set filters = __temp_import_alias_854547461__ %}{% set current_path = '' %}{% enddo %} +{% do __temp_import_alias_854547461__.update({'bar': bar,'foo': 123,'import_resource_path': 'filters.jinja','filters': filters}) %}{% endfor %}{% set filters = __temp_import_alias_854547461__ %}{% set current_path = '' %}{% enddo %} {{ filters }} diff --git a/src/test/resources/eager/fails-on-modification-in-aliased-macro.jinja b/src/test/resources/eager/fails-on-modification-in-aliased-macro.jinja new file mode 100644 index 000000000..4d01c9c84 --- /dev/null +++ b/src/test/resources/eager/fails-on-modification-in-aliased-macro.jinja @@ -0,0 +1,6 @@ +{% import 'eager/settings.jinja' as shared %} + +{% if deferred %} +{{ shared.load_settings() }} +{% endif %} +{{ shared.settings }} diff --git a/src/test/resources/tags/macrotag/eager/settings.jinja b/src/test/resources/tags/macrotag/eager/settings.jinja new file mode 100644 index 000000000..7c9feac3c --- /dev/null +++ b/src/test/resources/tags/macrotag/eager/settings.jinja @@ -0,0 +1,5 @@ +{% set settings = {} %} + +{% macro load_settings() %} +{% do settings.put('foo', 'bar') %} +{% endmacro %}