Skip to content

Commit 673ad89

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #864 from HubSpot/defer-set-on-current-scope
[Eager Execution] Improve ForTag with deferred tokens
2 parents c8d14f1 + 17f629d commit 673ad89

20 files changed

Lines changed: 265 additions & 83 deletions

‎src/main/java/com/hubspot/jinjava/interpret/Context.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,7 @@ public void checkNumberOfDeferredTokens() {
388388

389389
public void handleEagerToken(EagerToken eagerToken) {
390390
eagerTokens.add(eagerToken);
391+
391392
if (
392393
eagerToken.getImportResourcePath() == null ||
393394
eagerToken.getImportResourcePath().equals(get(Context.IMPORT_RESOURCE_PATH_KEY))

‎src/main/java/com/hubspot/jinjava/lib/expression/EagerExpressionStrategy.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.hubspot.jinjava.tree.parse.ExpressionToken;
1313
import com.hubspot.jinjava.util.EagerExpressionResolver;
1414
import com.hubspot.jinjava.util.EagerReconstructionUtils;
15+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1516
import com.hubspot.jinjava.util.Logging;
1617
import java.util.stream.Collectors;
1718
import org.apache.commons.lang3.StringUtils;
@@ -38,10 +39,16 @@ private String eagerResolveExpression(
3839
eagerInterpreter ->
3940
EagerExpressionResolver.resolveExpression(master.getExpr(), interpreter),
4041
interpreter,
41-
true,
42-
interpreter.getConfig().isNestedInterpretationEnabled(),
43-
interpreter.getContext().isDeferredExecutionMode()
42+
EagerChildContextConfig
43+
.newBuilder()
44+
.withTakeNewValue(true)
45+
.withPartialMacroEvaluation(
46+
interpreter.getConfig().isNestedInterpretationEnabled()
47+
)
48+
.withCheckForContextChanges(interpreter.getContext().isDeferredExecutionMode())
49+
.build()
4450
);
51+
4552
StringBuilder prefixToPreserveState = new StringBuilder();
4653
if (interpreter.getContext().isDeferredExecutionMode()) {
4754
prefixToPreserveState.append(eagerExecutionResult.getPrefixToPreserveState());

‎src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public Object doEvaluate(
5555
List<Object> varArgs
5656
) {
5757
Optional<String> importFile = macroFunction.getImportFile(interpreter);
58-
try (InterpreterScopeClosable c = interpreter.enterScope()) {
58+
try (InterpreterScopeClosable c = interpreter.enterNonStackingScope()) {
5959
interpreter.getContext().setDeferredExecutionMode(true);
6060
return macroFunction.getEvaluationResult(argMap, kwargMap, varArgs, interpreter);
6161
} finally {
@@ -152,7 +152,7 @@ public String reconstructImage() {
152152
) {
153153
return "";
154154
} else {
155-
try {
155+
try (InterpreterScopeClosable c = interpreter.enterScope()) {
156156
int numEagerTokensStart = interpreter.getContext().getEagerTokens().size();
157157
String evaluation = (String) evaluate(
158158
macroFunction
@@ -162,9 +162,6 @@ public String reconstructImage() {
162162
.toArray()
163163
);
164164

165-
interpreter
166-
.getContext()
167-
.put(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY, currentDeferredImportResource);
168165
if (interpreter.getContext().getEagerTokens().size() > numEagerTokensStart) {
169166
evaluation =
170167
(String) evaluate(
@@ -180,6 +177,9 @@ public String reconstructImage() {
180177
// In case something not eager-supported encountered a deferred value
181178
result = macroFunction.reconstructImage();
182179
} finally {
180+
interpreter
181+
.getContext()
182+
.put(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY, currentDeferredImportResource);
183183
if (!macroFunction.isCaller()) {
184184
interpreter.getContext().getMacroStack().pop();
185185
}

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerBlockSetTagStrategy.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.hubspot.jinjava.util.EagerExpressionResolver.EagerExpressionResult;
1111
import com.hubspot.jinjava.util.EagerExpressionResolver.EagerExpressionResult.ResolutionState;
1212
import com.hubspot.jinjava.util.EagerReconstructionUtils;
13+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1314
import com.hubspot.jinjava.util.LengthLimitingStringJoiner;
1415
import java.util.Collections;
1516
import java.util.Optional;
@@ -46,9 +47,7 @@ protected EagerExecutionResult getEagerExecutionResult(
4647
);
4748
},
4849
interpreter,
49-
true,
50-
false,
51-
false
50+
EagerChildContextConfig.newBuilder().withTakeNewValue(true).build()
5251
);
5352
}
5453

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerCycleTag.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.hubspot.jinjava.tree.parse.TagToken;
99
import com.hubspot.jinjava.util.EagerExpressionResolver;
1010
import com.hubspot.jinjava.util.EagerReconstructionUtils;
11+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1112
import com.hubspot.jinjava.util.HelperStringTokenizer;
1213
import com.hubspot.jinjava.util.WhitespaceUtils;
1314
import java.util.ArrayList;
@@ -46,9 +47,11 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter
4647
eagerInterpreter ->
4748
EagerExpressionResolver.resolveExpression(expression, interpreter),
4849
interpreter,
49-
true,
50-
false,
51-
interpreter.getContext().isDeferredExecutionMode()
50+
EagerChildContextConfig
51+
.newBuilder()
52+
.withTakeNewValue(true)
53+
.withCheckForContextChanges(interpreter.getContext().isDeferredExecutionMode())
54+
.build()
5255
);
5356

5457
StringBuilder prefixToPreserveState = new StringBuilder();

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerForTag.java‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@
77
import com.hubspot.jinjava.interpret.DeferredValueException;
88
import com.hubspot.jinjava.interpret.InterpretException;
99
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
10+
import com.hubspot.jinjava.interpret.OutputTooBigException;
11+
import com.hubspot.jinjava.interpret.TemplateError;
12+
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
1013
import com.hubspot.jinjava.lib.tag.ForTag;
1114
import com.hubspot.jinjava.objects.serialization.PyishObjectMapper;
1215
import com.hubspot.jinjava.tree.TagNode;
1316
import com.hubspot.jinjava.tree.parse.TagToken;
1417
import com.hubspot.jinjava.util.EagerExpressionResolver;
1518
import com.hubspot.jinjava.util.EagerExpressionResolver.EagerExpressionResult;
1619
import com.hubspot.jinjava.util.EagerReconstructionUtils;
20+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1721
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
1822
import com.hubspot.jinjava.util.LengthLimitingStringJoiner;
1923
import java.util.HashSet;
@@ -33,6 +37,43 @@ public EagerForTag(ForTag forTag) {
3337
super(forTag);
3438
}
3539

40+
@Override
41+
public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) {
42+
try {
43+
int numEagerTokensStart = interpreter.getContext().getEagerTokens().size();
44+
EagerExecutionResult result = EagerReconstructionUtils.executeInChildContext(
45+
eagerInterpreter ->
46+
EagerExpressionResult.fromString(
47+
getTag().interpretUnchecked(tagNode, interpreter)
48+
),
49+
interpreter,
50+
EagerChildContextConfig.newBuilder().withCheckForContextChanges(true).build()
51+
);
52+
if (
53+
interpreter.getContext().getEagerTokens().size() > numEagerTokensStart &&
54+
!result.getSpeculativeBindings().isEmpty()
55+
) {
56+
EagerIfTag.resetBindingsForNextBranch(interpreter, result);
57+
throw new DeferredValueException(
58+
"Modification inside partially evaluated for loop"
59+
);
60+
}
61+
return result.getResult().toString(true);
62+
} catch (DeferredValueException | TemplateSyntaxException e) {
63+
try {
64+
return EagerReconstructionUtils.wrapInAutoEscapeIfNeeded(
65+
eagerInterpret(tagNode, interpreter, e),
66+
interpreter
67+
);
68+
} catch (OutputTooBigException e1) {
69+
interpreter.addError(TemplateError.fromOutputTooBigException(e1));
70+
throw new DeferredValueException(
71+
String.format("Output too big for eager execution: %s", e1.getMessage())
72+
);
73+
}
74+
}
75+
}
76+
3677
@Override
3778
public String eagerInterpret(
3879
TagNode tagNode,
@@ -70,9 +111,7 @@ public String eagerInterpret(
70111
)
71112
),
72113
interpreter,
73-
true,
74-
false,
75-
false
114+
EagerChildContextConfig.newBuilder().build()
76115
)
77116
.asTemplateString()
78117
);
@@ -125,9 +164,11 @@ private EagerExecutionResult runLoopOnce(
125164
);
126165
},
127166
interpreter,
128-
false,
129-
false,
130-
true
167+
EagerChildContextConfig
168+
.newBuilder()
169+
.withForceDeferredExecutionMode(true)
170+
.withCheckForContextChanges(true)
171+
.build()
131172
);
132173
}
133174

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerIfTag.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import com.hubspot.jinjava.tree.parse.NoteToken;
1616
import com.hubspot.jinjava.util.EagerExpressionResolver.EagerExpressionResult;
1717
import com.hubspot.jinjava.util.EagerReconstructionUtils;
18+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1819
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
1920
import java.util.HashSet;
2021
import java.util.Map;
@@ -78,9 +79,11 @@ public String eagerInterpret(
7879
eagerRenderBranches(tagNode, eagerInterpreter, e)
7980
),
8081
interpreter,
81-
false,
82-
false,
83-
true
82+
EagerChildContextConfig
83+
.newBuilder()
84+
.withForceDeferredExecutionMode(true)
85+
.withCheckForContextChanges(true)
86+
.build()
8487
)
8588
.asTemplateString()
8689
);
@@ -130,9 +133,11 @@ public String eagerRenderBranches(
130133
evaluateBranch(tagNode, finalBranchStart, branchEnd, interpreter)
131134
),
132135
interpreter,
133-
false,
134-
false,
135-
true
136+
EagerChildContextConfig
137+
.newBuilder()
138+
.withForceDeferredExecutionMode(true)
139+
.withCheckForContextChanges(true)
140+
.build()
136141
);
137142
sb.append(result.getResult());
138143
bindingsToDefer.addAll(resetBindingsForNextBranch(interpreter, result));
@@ -193,7 +198,7 @@ public String eagerRenderBranches(
193198
return sb.toString();
194199
}
195200

196-
private Set<String> resetBindingsForNextBranch(
201+
public static Set<String> resetBindingsForNextBranch(
197202
JinjavaInterpreter interpreter,
198203
EagerExecutionResult result
199204
) {

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerInlineSetTagStrategy.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.hubspot.jinjava.tree.parse.TagToken;
99
import com.hubspot.jinjava.util.EagerExpressionResolver;
1010
import com.hubspot.jinjava.util.EagerReconstructionUtils;
11+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1112
import com.hubspot.jinjava.util.LengthLimitingStringJoiner;
1213
import com.hubspot.jinjava.util.WhitespaceUtils;
1314
import java.util.Arrays;
@@ -34,9 +35,11 @@ public EagerExecutionResult getEagerExecutionResult(
3435
eagerInterpreter ->
3536
EagerExpressionResolver.resolveExpression('[' + expression + ']', interpreter),
3637
interpreter,
37-
true,
38-
false,
39-
interpreter.getContext().isDeferredExecutionMode()
38+
EagerChildContextConfig
39+
.newBuilder()
40+
.withTakeNewValue(true)
41+
.withCheckForContextChanges(interpreter.getContext().isDeferredExecutionMode())
42+
.build()
4043
);
4144
}
4245

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerPrintTag.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.hubspot.jinjava.tree.parse.TagToken;
88
import com.hubspot.jinjava.util.EagerExpressionResolver;
99
import com.hubspot.jinjava.util.EagerReconstructionUtils;
10+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1011
import com.hubspot.jinjava.util.LengthLimitingStringJoiner;
1112
import java.util.stream.Collectors;
1213
import org.apache.commons.lang3.StringUtils;
@@ -55,9 +56,11 @@ public static String interpretExpression(
5556
EagerExecutionResult eagerExecutionResult = EagerReconstructionUtils.executeInChildContext(
5657
eagerInterpreter -> EagerExpressionResolver.resolveExpression(expr, interpreter),
5758
interpreter,
58-
true,
59-
false,
60-
interpreter.getContext().isDeferredExecutionMode()
59+
EagerChildContextConfig
60+
.newBuilder()
61+
.withTakeNewValue(true)
62+
.withCheckForContextChanges(interpreter.getContext().isDeferredExecutionMode())
63+
.build()
6164
);
6265
StringBuilder prefixToPreserveState = new StringBuilder();
6366
if (interpreter.getContext().isDeferredExecutionMode()) {

‎src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerStateChangingTag.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import com.hubspot.jinjava.tree.parse.TagToken;
99
import com.hubspot.jinjava.util.EagerExpressionResolver.EagerExpressionResult;
1010
import com.hubspot.jinjava.util.EagerReconstructionUtils;
11+
import com.hubspot.jinjava.util.EagerReconstructionUtils.EagerChildContextConfig;
1112
import org.apache.commons.lang3.StringUtils;
1213

1314
public class EagerStateChangingTag<T extends Tag> extends EagerTagDecorator<T> {
@@ -36,17 +37,20 @@ public String eagerInterpret(
3637

3738
if (!tagNode.getChildren().isEmpty()) {
3839
result.append(
39-
EagerReconstructionUtils.executeInChildContext(
40-
eagerInterpreter ->
41-
EagerExpressionResult.fromString(renderChildren(tagNode, eagerInterpreter)),
42-
interpreter,
43-
false,
44-
false,
45-
true
46-
)
40+
EagerReconstructionUtils
41+
.executeInChildContext(
42+
eagerInterpreter ->
43+
EagerExpressionResult.fromString(renderChildren(tagNode, eagerInterpreter)),
44+
interpreter,
45+
EagerChildContextConfig
46+
.newBuilder()
47+
.withCheckForContextChanges(true)
48+
.withForceDeferredExecutionMode(true)
49+
.build()
50+
)
51+
.asTemplateString()
4752
);
4853
}
49-
5054
if (
5155
StringUtils.isNotBlank(tagNode.getEndName()) &&
5256
(

0 commit comments

Comments
 (0)