Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve Raw Tags config #518

Merged
merged 12 commits into from
Oct 26, 2020
Merged

Preserve Raw Tags config #518

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
f632489
Add preserving raw tags for double-renderings
jasmith-hs Oct 9, 2020
4f120e5
Include image in exception, allow length limiting
jasmith-hs Oct 12, 2020
2798066
Handle preserved raw tag exception in TagNode
jasmith-hs Oct 12, 2020
09f47a2
Test preserving raw tags
jasmith-hs Oct 12, 2020
dd642d0
Exception isn't thrown here
jasmith-hs Oct 12, 2020
cc431d4
Add license
jasmith-hs Oct 12, 2020
229b277
Refactor to remove PreservedRawTagException
jasmith-hs Oct 21, 2020
17c5a1c
Add deferred preserving test
jasmith-hs Oct 21, 2020
bccbc98
Removed hasPreservedRawTags
jasmith-hs Oct 23, 2020
277cc64
Use more generic config flag name: "preserveForSecondPass"
jasmith-hs Oct 23, 2020
1e041b1
Add trailing newline to test jinja resource
jasmith-hs Oct 23, 2020
05b5b9c
Trim before comparing in raw test
jasmith-hs Oct 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/main/java/com/hubspot/jinjava/JinjavaConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class JinjavaConfig {
private TokenScannerSymbols tokenScannerSymbols;
private ELResolver elResolver;
private final boolean iterateOverMapKeys;
private final boolean preserveForSecondPass;

public static Builder newBuilder() {
return new Builder();
Expand Down Expand Up @@ -85,6 +86,7 @@ public JinjavaConfig(InterpreterFactory interpreterFactory) {
interpreterFactory,
new DefaultTokenScannerSymbols(),
JinjavaInterpreterResolver.DEFAULT_RESOLVER_READ_ONLY,
false,
false
);
}
Expand Down Expand Up @@ -114,6 +116,7 @@ public JinjavaConfig(
new JinjavaInterpreterFactory(),
new DefaultTokenScannerSymbols(),
JinjavaInterpreterResolver.DEFAULT_RESOLVER_READ_ONLY,
false,
false
);
}
Expand All @@ -137,7 +140,8 @@ private JinjavaConfig(
InterpreterFactory interpreterFactory,
TokenScannerSymbols tokenScannerSymbols,
ELResolver elResolver,
boolean iterateOverMapKeys
boolean iterateOverMapKeys,
boolean preserveForSecondPass
) {
this.charset = charset;
this.locale = locale;
Expand All @@ -158,6 +162,7 @@ private JinjavaConfig(
this.tokenScannerSymbols = tokenScannerSymbols;
this.elResolver = elResolver;
this.iterateOverMapKeys = iterateOverMapKeys;
this.preserveForSecondPass = preserveForSecondPass;
}

public Charset getCharset() {
Expand Down Expand Up @@ -240,6 +245,10 @@ public boolean isIterateOverMapKeys() {
return iterateOverMapKeys;
}

public boolean isPreserveForSecondPass() {
return preserveForSecondPass;
}

public static class Builder {
private Charset charset = StandardCharsets.UTF_8;
private Locale locale = Locale.ENGLISH;
Expand All @@ -263,6 +272,7 @@ public static class Builder {
private TokenScannerSymbols tokenScannerSymbols = new DefaultTokenScannerSymbols();
private ELResolver elResolver = JinjavaInterpreterResolver.DEFAULT_RESOLVER_READ_ONLY;
private boolean iterateOverMapKeys;
private boolean preserveForSecondPass;

private Builder() {}

Expand Down Expand Up @@ -371,6 +381,11 @@ public Builder withIterateOverMapKeys(boolean iterateOverMapKeys) {
return this;
}

public Builder withPreserveForSecondPass(boolean preserveForSecondPass) {
this.preserveForSecondPass = preserveForSecondPass;
return this;
}

public JinjavaConfig build() {
return new JinjavaConfig(
charset,
Expand All @@ -391,7 +406,8 @@ public JinjavaConfig build() {
interpreterFactory,
tokenScannerSymbols,
elResolver,
iterateOverMapKeys
iterateOverMapKeys,
preserveForSecondPass
);
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/com/hubspot/jinjava/lib/tag/RawTag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ public String getName() {

@Override
public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
if (interpreter.getConfig().isPreserveForSecondPass()) {
return renderNodeRaw(tagNode);
}

LengthLimitingStringBuilder result = new LengthLimitingStringBuilder(
interpreter.getConfig().getMaxOutputSize()
);
Expand Down
19 changes: 19 additions & 0 deletions src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,23 @@ public void itLimitsOutputSizeWhenSumOfNodeSizesExceedsMax() {
assertThat(renderResult.getErrors().get(0).getMessage())
.contains("OutputTooBigException");
}

@Test
public void itCanPreserveRawTags() {
JinjavaConfig preserveConfig = JinjavaConfig
.newBuilder()
.withPreserveForSecondPass(true)
.build();
String input = "1{% raw %}2{% endraw %}3";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a test for deferred values? I thought deferred values will not get evaluated normally. Do they get evaluated inside raw tags?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My testing shows that deferred inside raw does not get evaluated either. https://github.com/HubSpot/jinjava/compare/test-deferred?expand=1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing, including deferred tags, will get evaluated inside raw tags. However without the preserveRawTags flag, on the second pass, the deferred values will get evaluated once the real value is put in the context.

Copy link
Collaborator

@hs-lsong hs-lsong Oct 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my test above, the deferred values do not get evaluated in the second rendering (there is no preserveRawTags) . Maybe we are not talking about the same case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After line 83:

interpreter.getContext().put("deferred", "resolved value");

In the second pass, deferred values will no longer be deferred (such as a contact, which have the actual contact value in the second pass, rather than being a deferred value). If you run the test after putting that line in, I believe that it will get resolved, which is not what is desired.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what Jack meant by second pass is another renderer pass without having DeferredValue.instance() in the context. But in your test case, the test setUp has localContext.put("deferred", DeferredValue.instance()); so it gets deferred in the second pass as well. Yo need to remove that from the context to actually resolve them in the second pass.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what is the use case for those? First round rendering makes it deferred, then second round makes it not deferred, but still don't want to render it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

<div>
Hi {{ contact.firstname }}!
I personalized this with Jinjava:
{% raw %}Hi {{ contact.firstname }}!{% endraw %}
</div>

This should display for someone like:

<div>
Hi Jack!
I personalized this with Jinjava:
Hi {{ contact.firstname }}!
</div>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We wouldn't want it to render like:

<div>
Hi Jack!
I personalized this with Jinjava:
Hi Jack!
</div>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks.

String normalOutput = "123";
String preservedOutput = "1{% raw %}2{% endraw %}3";

RenderResult renderResult = new Jinjava().renderForResult(input, new HashMap<>());
assertThat(renderResult.getOutput()).isEqualTo(normalOutput);
assertThat(renderResult.hasErrors()).isFalse();

renderResult = new Jinjava(preserveConfig).renderForResult(input, new HashMap<>());
assertThat(renderResult.getOutput()).isEqualTo(preservedOutput);
assertThat(renderResult.hasErrors()).isFalse();
}
}
77 changes: 76 additions & 1 deletion src/test/java/com/hubspot/jinjava/lib/tag/RawTagTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import com.google.common.io.Resources;
import com.hubspot.jinjava.Jinjava;
import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.interpret.DeferredValue;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.tree.Node;
import com.hubspot.jinjava.tree.TagNode;
Expand All @@ -17,12 +19,14 @@
import org.junit.Test;

public class RawTagTest {
Jinjava jinjava;
JinjavaInterpreter interpreter;
RawTag tag;

@Before
public void setup() {
interpreter = new Jinjava().newInterpreter();
jinjava = new Jinjava();
interpreter = jinjava.newInterpreter();
tag = new RawTag();
}

Expand Down Expand Up @@ -91,6 +95,77 @@ public void itDoesntProcessJinjaCommentsWithinARawBlock() {
.contains("{{#each people}}");
}

@Test
public void itPreservesRawTags() {
TagNode tagNode = fixture("hubl");
JinjavaInterpreter preserveInterpreter = new JinjavaInterpreter(
jinjava,
jinjava.getGlobalContextCopy(),
JinjavaConfig.newBuilder().withPreserveForSecondPass(true).build()
);
String result = tag.interpret(tagNode, preserveInterpreter);
try {
assertThat(result)
.isEqualTo(
Resources.toString(
Resources.getResource("tags/rawtag/hubl.jinja"),
StandardCharsets.UTF_8
)
);
} catch (IOException e) {
throw new RuntimeException(e);
}
}

@Test
public void itPreservesDeferredWhilePreservingRawTags() {
TagNode tagNode = fixture("deferred");
JinjavaInterpreter preserveInterpreter = new JinjavaInterpreter(
jinjava,
jinjava.getGlobalContextCopy(),
JinjavaConfig.newBuilder().withPreserveForSecondPass(true).build()
);
preserveInterpreter.getContext().put("deferred", DeferredValue.instance());
interpreter.getContext().put("deferred", DeferredValue.instance());

String preservedResult = tag.interpret(tagNode, preserveInterpreter);
String nonPreservedResult = tag.interpret(tagNode, interpreter);
try {
assertThat(preservedResult)
.isEqualTo(
Resources.toString(
Resources.getResource("tags/rawtag/deferred.jinja"),
StandardCharsets.UTF_8
)
);
} catch (IOException e) {
throw new RuntimeException(e);
}
assertThat(nonPreservedResult).isEqualTo("{{ deferred }}");

// Should not get evaluated because it's wrapped in a raw tag.
String deferredRealValue = "Resolved value.";
preserveInterpreter.getContext().put("deferred", deferredRealValue);
interpreter.getContext().put("deferred", deferredRealValue);
String preservedIdempotent = tag.interpret(
(TagNode) new TreeParser(preserveInterpreter, preservedResult)
.buildTree()
.getChildren()
.getFirst(),
preserveInterpreter
);
String secondPass = tag.interpret(
(TagNode) new TreeParser(interpreter, preservedResult)
.buildTree()
.getChildren()
.getFirst(),
interpreter
);

assertThat(preservedIdempotent).isEqualTo(preservedResult);
assertThat(secondPass).isEqualTo("{{ deferred }}");
}

private TagNode fixture(String name) {
return (TagNode) fixtures(name).getFirst();
}
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/tags/rawtag/deferred.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% raw %}{{ deferred }}{% endraw %}