Skip to content

Commit 5d98c8b

Browse files
jasmith-hsjasmith-hs
authored
Revert "Track modified variables better in child-scoped eager execution"
1 parent 6895833 commit 5d98c8b

File tree

56 files changed

+721
-1156
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+721
-1156
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,23 @@ public void checkNumberOfDeferredTokens() {
393393

394394
@Beta
395395
public void handleDeferredToken(DeferredToken deferredToken) {
396-
deferredToken.addTo(this);
396+
deferredTokens.add(deferredToken);
397+
398+
if (
399+
deferredToken.getImportResourcePath() == null ||
400+
deferredToken.getImportResourcePath().equals(get(Context.IMPORT_RESOURCE_PATH_KEY))
401+
) {
402+
DeferredValueUtils.findAndMarkDeferredProperties(this, deferredToken);
403+
}
404+
if (getParent() != null) {
405+
Context parent = getParent();
406+
//Ignore global context
407+
if (parent.getParent() != null) {
408+
parent.handleDeferredToken(deferredToken);
409+
} else {
410+
checkNumberOfDeferredTokens();
411+
}
412+
}
397413
}
398414

399415
@Beta

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,21 @@
33
import com.google.common.annotations.Beta;
44

55
@Beta
6-
public class DeferredLazyReferenceSource extends DeferredValueImpl {
6+
public class DeferredLazyReferenceSource implements DeferredValue {
77
private static final DeferredLazyReferenceSource INSTANCE = new DeferredLazyReferenceSource();
88

9+
private Object originalValue;
910
private boolean reconstructed;
1011

1112
private DeferredLazyReferenceSource() {}
1213

1314
private DeferredLazyReferenceSource(Object originalValue) {
14-
super(originalValue);
15+
this.originalValue = originalValue;
16+
}
17+
18+
@Override
19+
public Object getOriginalValue() {
20+
return originalValue;
1521
}
1622

1723
public static DeferredLazyReferenceSource instance() {

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,4 @@ static DeferredValue instance() {
1515
static DeferredValue instance(Object originalValue) {
1616
return DeferredValueImpl.instance(originalValue);
1717
}
18-
19-
static DeferredValueShadow shadowInstance(Object originalValue) {
20-
return DeferredValueShadow.instance(originalValue);
21-
}
2218
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ public class DeferredValueImpl implements DeferredValue {
77

88
private Object originalValue;
99

10-
protected DeferredValueImpl() {}
10+
private DeferredValueImpl() {}
1111

12-
protected DeferredValueImpl(Object originalValue) {
12+
private DeferredValueImpl(Object originalValue) {
1313
this.originalValue = originalValue;
1414
}
1515

@@ -18,11 +18,11 @@ public Object getOriginalValue() {
1818
return originalValue;
1919
}
2020

21-
protected static DeferredValue instance() {
21+
public static DeferredValue instance() {
2222
return INSTANCE;
2323
}
2424

25-
protected static DeferredValue instance(Object originalValue) {
25+
public static DeferredValue instance(Object originalValue) {
2626
return new DeferredValueImpl(originalValue);
2727
}
2828

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

Lines changed: 0 additions & 25 deletions
This file was deleted.

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import com.hubspot.jinjava.util.EagerExpressionResolver;
1515
import com.hubspot.jinjava.util.EagerReconstructionUtils;
1616
import com.hubspot.jinjava.util.Logging;
17-
import com.hubspot.jinjava.util.PrefixToPreserveState;
1817
import java.util.Objects;
1918
import java.util.stream.Collectors;
2019
import org.apache.commons.lang3.StringUtils;
@@ -49,12 +48,9 @@ private String eagerResolveExpression(
4948
.build()
5049
);
5150

52-
PrefixToPreserveState prefixToPreserveState = new PrefixToPreserveState();
53-
if (
54-
!eagerExecutionResult.getResult().isFullyResolved() ||
55-
interpreter.getContext().isDeferredExecutionMode()
56-
) {
57-
prefixToPreserveState.putAll(eagerExecutionResult.getPrefixToPreserveState());
51+
StringBuilder prefixToPreserveState = new StringBuilder();
52+
if (interpreter.getContext().isDeferredExecutionMode()) {
53+
prefixToPreserveState.append(eagerExecutionResult.getPrefixToPreserveState());
5854
} else {
5955
interpreter.getContext().putAll(eagerExecutionResult.getSpeculativeBindings());
6056
}
@@ -64,8 +60,8 @@ private String eagerResolveExpression(
6460
prefixToPreserveState.toString() + postProcessResult(master, result, interpreter)
6561
);
6662
}
67-
prefixToPreserveState.putAll(
68-
EagerReconstructionUtils.reconstructFromContextBeforeDeferringAsMap(
63+
prefixToPreserveState.append(
64+
EagerReconstructionUtils.reconstructFromContextBeforeDeferring(
6965
eagerExecutionResult.getResult().getDeferredWords(),
7066
interpreter
7167
)
@@ -74,7 +70,7 @@ private String eagerResolveExpression(
7470
eagerExecutionResult.getResult().toString(),
7571
interpreter
7672
);
77-
prefixToPreserveState.withAllInFront(
73+
prefixToPreserveState.append(
7874
EagerReconstructionUtils.handleDeferredTokenAndReconstructReferences(
7975
interpreter,
8076
new DeferredToken(

‎src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
package com.hubspot.jinjava.lib.fn;
22

33
import com.hubspot.jinjava.el.ext.AbstractCallableMethod;
4+
import com.hubspot.jinjava.el.ext.DeferredParsingException;
5+
import com.hubspot.jinjava.el.ext.eager.MacroFunctionTempVariable;
46
import com.hubspot.jinjava.interpret.Context;
57
import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
68
import com.hubspot.jinjava.interpret.DeferredValue;
79
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
810
import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable;
911
import com.hubspot.jinjava.tree.Node;
12+
import com.hubspot.jinjava.util.EagerReconstructionUtils;
1013
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
1114
import java.util.HashMap;
1215
import java.util.LinkedHashMap;
1316
import java.util.List;
1417
import java.util.Map;
1518
import java.util.Objects;
1619
import java.util.Optional;
20+
import java.util.concurrent.atomic.AtomicInteger;
1721

1822
/**
1923
* Function definition parsed from a jinjava template, stored in global macros registry in interpreter context.
@@ -22,19 +26,19 @@
2226
*
2327
*/
2428
public class MacroFunction extends AbstractCallableMethod {
25-
public static final String KWARGS_KEY = "kwargs";
26-
public static final String VARARGS_KEY = "varargs";
27-
protected final List<Node> content;
29+
private final List<Node> content;
2830

29-
protected final boolean caller;
31+
private final boolean caller;
3032

31-
protected final Context localContextScope;
33+
private final Context localContextScope;
3234

33-
protected final int definitionLineNumber;
35+
private final int definitionLineNumber;
3436

35-
protected final int definitionStartPosition;
37+
private final int definitionStartPosition;
3638

37-
protected boolean deferred;
39+
private boolean deferred;
40+
41+
private AtomicInteger callCount = new AtomicInteger();
3842

3943
public MacroFunction(
4044
List<Node> content,
@@ -70,10 +74,33 @@ public Object doEvaluate(
7074
Map<String, Object> kwargMap,
7175
List<Object> varArgs
7276
) {
77+
int currentCallCount = callCount.getAndIncrement();
7378
JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent();
7479
Optional<String> importFile = getImportFile(interpreter);
7580
try (InterpreterScopeClosable c = interpreter.enterScope()) {
76-
return getEvaluationResult(argMap, kwargMap, varArgs, interpreter);
81+
String result = getEvaluationResult(argMap, kwargMap, varArgs, interpreter);
82+
if (
83+
!interpreter.getContext().getDeferredNodes().isEmpty() ||
84+
!interpreter.getContext().getDeferredTokens().isEmpty()
85+
) {
86+
if (!interpreter.getContext().isPartialMacroEvaluation()) {
87+
String tempVarName = MacroFunctionTempVariable.getVarName(
88+
getName(),
89+
hashCode(),
90+
currentCallCount
91+
);
92+
interpreter
93+
.getContext()
94+
.getParent()
95+
.put(tempVarName, new MacroFunctionTempVariable(result));
96+
throw new DeferredParsingException(this, tempVarName);
97+
}
98+
if (interpreter.getContext().isDeferredExecutionMode()) {
99+
return EagerReconstructionUtils.wrapInChildScope(result, interpreter);
100+
}
101+
}
102+
103+
return result;
77104
} finally {
78105
importFile.ifPresent(path -> interpreter.getContext().getCurrentPathStack().pop());
79106
}
@@ -138,9 +165,9 @@ public String getEvaluationResult(
138165
interpreter.getContext().put(argEntry.getKey(), argEntry.getValue());
139166
}
140167
// parameter map
141-
interpreter.getContext().put(KWARGS_KEY, kwargMap);
168+
interpreter.getContext().put("kwargs", kwargMap);
142169
// varargs list
143-
interpreter.getContext().put(VARARGS_KEY, varArgs);
170+
interpreter.getContext().put("varargs", varArgs);
144171

145172
LengthLimitingStringBuilder result = new LengthLimitingStringBuilder(
146173
interpreter.getConfig().getMaxOutputSize()

0 commit comments

Comments
 (0)