Skip to content

Commit 69379e5

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #1089 from HubSpot/preserve-identifier-in-method
Preserve identifiers when reconstructing AstMethod
2 parents 63faa33 + 8c20c26 commit 69379e5

25 files changed

Lines changed: 237 additions & 96 deletions

‎src/main/java/com/hubspot/jinjava/el/ext/DeferredParsingException.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
public class DeferredParsingException extends DeferredValueException {
66
private final String deferredEvalResult;
77
private final Object sourceNode;
8+
private final IdentifierPreservationStrategy identifierPreservationStrategy;
89

910
public DeferredParsingException(String message) {
1011
super(message);
1112
this.deferredEvalResult = message;
1213
this.sourceNode = null;
14+
this.identifierPreservationStrategy = IdentifierPreservationStrategy.RESOLVING;
1315
}
1416

1517
public DeferredParsingException(Object sourceNode, String deferredEvalResult) {
@@ -22,6 +24,24 @@ public DeferredParsingException(Object sourceNode, String deferredEvalResult) {
2224
);
2325
this.deferredEvalResult = deferredEvalResult;
2426
this.sourceNode = sourceNode;
27+
this.identifierPreservationStrategy = IdentifierPreservationStrategy.RESOLVING;
28+
}
29+
30+
public DeferredParsingException(
31+
Object sourceNode,
32+
String deferredEvalResult,
33+
IdentifierPreservationStrategy identifierPreservationStrategy
34+
) {
35+
super(
36+
String.format(
37+
"%s could not be parsed more than: %s",
38+
sourceNode.getClass(),
39+
deferredEvalResult
40+
)
41+
);
42+
this.deferredEvalResult = deferredEvalResult;
43+
this.sourceNode = sourceNode;
44+
this.identifierPreservationStrategy = identifierPreservationStrategy;
2545
}
2646

2747
public String getDeferredEvalResult() {
@@ -31,4 +51,8 @@ public String getDeferredEvalResult() {
3151
public Object getSourceNode() {
3252
return sourceNode;
3353
}
54+
55+
public IdentifierPreservationStrategy getIdentifierPreservationStrategy() {
56+
return identifierPreservationStrategy;
57+
}
3458
}

‎src/main/java/com/hubspot/jinjava/el/ext/IdentifierPreservationStrategy.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package com.hubspot.jinjava.el.ext;
2+
3+
public enum IdentifierPreservationStrategy {
4+
PRESERVING(true),
5+
RESOLVING(false);
6+
7+
public static IdentifierPreservationStrategy preserving(boolean preserveIdentifier) {
8+
return preserveIdentifier ? PRESERVING : RESOLVING;
9+
}
10+
11+
private final boolean preserving;
12+
13+
IdentifierPreservationStrategy(boolean preserving) {
14+
this.preserving = preserving;
15+
}
16+
17+
public boolean isPreserving() {
18+
return preserving;
19+
}
20+
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBinary.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.hubspot.jinjava.el.NoInvokeELContext;
44
import com.hubspot.jinjava.el.ext.DeferredParsingException;
5+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
56
import com.hubspot.jinjava.el.ext.OrOperator;
67
import de.odysseus.el.tree.Bindings;
78
import de.odysseus.el.tree.impl.ast.AstBinary;
@@ -48,15 +49,15 @@ public String getPartiallyResolved(
4849
Bindings bindings,
4950
ELContext context,
5051
DeferredParsingException deferredParsingException,
51-
boolean preserveIdentifier
52+
IdentifierPreservationStrategy identifierPreservationStrategy
5253
) {
5354
return (
5455
EvalResultHolder.reconstructNode(
5556
bindings,
5657
context,
5758
left,
5859
deferredParsingException,
59-
false
60+
IdentifierPreservationStrategy.RESOLVING
6061
) +
6162
String.format(" %s ", operator.toString()) +
6263
EvalResultHolder.reconstructNode(
@@ -66,7 +67,7 @@ public String getPartiallyResolved(
6667
: context,
6768
right,
6869
deferredParsingException,
69-
false
70+
IdentifierPreservationStrategy.RESOLVING
7071
)
7172
);
7273
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstBracket.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.hubspot.jinjava.el.ext.eager;
22

33
import com.hubspot.jinjava.el.ext.DeferredParsingException;
4+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
45
import de.odysseus.el.tree.Bindings;
56
import de.odysseus.el.tree.impl.ast.AstBracket;
67
import de.odysseus.el.tree.impl.ast.AstNode;
@@ -63,7 +64,7 @@ public String getPartiallyResolved(
6364
Bindings bindings,
6465
ELContext context,
6566
DeferredParsingException deferredParsingException,
66-
boolean preserveIdentifier
67+
IdentifierPreservationStrategy identifierPreservationStrategy
6768
) {
6869
return String.format(
6970
"%s[%s]",
@@ -72,14 +73,14 @@ public String getPartiallyResolved(
7273
context,
7374
(EvalResultHolder) prefix,
7475
deferredParsingException,
75-
preserveIdentifier
76+
identifierPreservationStrategy
7677
),
7778
EvalResultHolder.reconstructNode(
7879
bindings,
7980
context,
8081
(EvalResultHolder) property,
8182
deferredParsingException,
82-
false
83+
IdentifierPreservationStrategy.RESOLVING
8384
)
8485
);
8586
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstChoice.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.hubspot.jinjava.el.NoInvokeELContext;
44
import com.hubspot.jinjava.el.ext.DeferredParsingException;
5+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
56
import de.odysseus.el.tree.Bindings;
67
import de.odysseus.el.tree.impl.ast.AstChoice;
78
import de.odysseus.el.tree.impl.ast.AstNode;
@@ -46,7 +47,12 @@ public Object eval(Bindings bindings, ELContext context) throws ELException {
4647
}
4748
throw new DeferredParsingException(
4849
this,
49-
getPartiallyResolved(bindings, context, e, false)
50+
getPartiallyResolved(
51+
bindings,
52+
context,
53+
e,
54+
IdentifierPreservationStrategy.RESOLVING
55+
)
5056
);
5157
}
5258
}
@@ -72,31 +78,31 @@ public String getPartiallyResolved(
7278
Bindings bindings,
7379
ELContext context,
7480
DeferredParsingException deferredParsingException,
75-
boolean preserveIdentifier
81+
IdentifierPreservationStrategy identifierPreservationStrategy
7682
) {
7783
return (
7884
EvalResultHolder.reconstructNode(
7985
bindings,
8086
context,
8187
question,
8288
deferredParsingException,
83-
false
89+
IdentifierPreservationStrategy.RESOLVING
8490
) +
8591
" ? " +
8692
EvalResultHolder.reconstructNode(
8793
bindings,
8894
new NoInvokeELContext(context),
8995
yes,
9096
deferredParsingException,
91-
preserveIdentifier
97+
identifierPreservationStrategy
9298
) +
9399
" : " +
94100
EvalResultHolder.reconstructNode(
95101
bindings,
96102
new NoInvokeELContext(context),
97103
no,
98104
deferredParsingException,
99-
preserveIdentifier
105+
identifierPreservationStrategy
100106
)
101107
);
102108
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDict.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.hubspot.jinjava.el.ext.AstDict;
44
import com.hubspot.jinjava.el.ext.DeferredParsingException;
55
import com.hubspot.jinjava.el.ext.ExtendedParser;
6+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
67
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
78
import com.hubspot.jinjava.util.EagerExpressionResolver;
89
import de.odysseus.el.tree.Bindings;
@@ -34,7 +35,7 @@ public String getPartiallyResolved(
3435
Bindings bindings,
3536
ELContext context,
3637
DeferredParsingException deferredParsingException,
37-
boolean preserveIdentifier
38+
IdentifierPreservationStrategy identifierPreservationStrategy
3839
) {
3940
JinjavaInterpreter interpreter = (JinjavaInterpreter) context
4041
.getELResolver()
@@ -52,7 +53,9 @@ public String getPartiallyResolved(
5253
context,
5354
(EvalResultHolder) key,
5455
deferredParsingException,
55-
!interpreter.getConfig().getLegacyOverrides().isEvaluateMapKeys()
56+
IdentifierPreservationStrategy.preserving(
57+
!interpreter.getConfig().getLegacyOverrides().isEvaluateMapKeys()
58+
)
5659
)
5760
);
5861
} else {
@@ -69,7 +72,7 @@ public String getPartiallyResolved(
6972
context,
7073
(EvalResultHolder) value,
7174
deferredParsingException,
72-
preserveIdentifier
75+
identifierPreservationStrategy
7376
)
7477
);
7578
} else {

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstDot.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.hubspot.jinjava.el.ext.eager;
22

33
import com.hubspot.jinjava.el.ext.DeferredParsingException;
4+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
45
import de.odysseus.el.tree.Bindings;
56
import de.odysseus.el.tree.impl.ast.AstDot;
67
import de.odysseus.el.tree.impl.ast.AstNode;
@@ -52,11 +53,17 @@ public String getPartiallyResolved(
5253
Bindings bindings,
5354
ELContext context,
5455
DeferredParsingException e,
55-
boolean preserveIdentifier
56+
IdentifierPreservationStrategy identifierPreservationStrategy
5657
) {
5758
return String.format(
5859
"%s.%s",
59-
EvalResultHolder.reconstructNode(bindings, context, base, e, preserveIdentifier),
60+
EvalResultHolder.reconstructNode(
61+
bindings,
62+
context,
63+
base,
64+
e,
65+
identifierPreservationStrategy
66+
),
6067
property
6168
);
6269
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstIdentifier.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.hubspot.jinjava.el.ext.eager;
22

33
import com.hubspot.jinjava.el.ext.DeferredParsingException;
4+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
45
import de.odysseus.el.tree.Bindings;
56
import de.odysseus.el.tree.impl.ast.AstIdentifier;
67
import javax.el.ELContext;
@@ -43,7 +44,7 @@ public String getPartiallyResolved(
4344
Bindings bindings,
4445
ELContext context,
4546
DeferredParsingException deferredParsingException,
46-
boolean preserveIdentifier
47+
IdentifierPreservationStrategy identifierPreservationStrategy
4748
) {
4849
return getName();
4950
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstList.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.hubspot.jinjava.el.ext.AstList;
44
import com.hubspot.jinjava.el.ext.DeferredParsingException;
5+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
56
import de.odysseus.el.tree.Bindings;
67
import de.odysseus.el.tree.impl.ast.AstParameters;
78
import java.util.StringJoiner;
@@ -45,7 +46,7 @@ public String getPartiallyResolved(
4546
Bindings bindings,
4647
ELContext context,
4748
DeferredParsingException deferredParsingException,
48-
boolean preserveIdentifier
49+
IdentifierPreservationStrategy identifierPreservationStrategy
4950
) {
5051
StringJoiner joiner = new StringJoiner(", ");
5152
for (int i = 0; i < elements.getCardinality(); i++) {
@@ -55,7 +56,7 @@ public String getPartiallyResolved(
5556
context,
5657
(EvalResultHolder) elements.getChild(i),
5758
deferredParsingException,
58-
preserveIdentifier
59+
identifierPreservationStrategy
5960
)
6061
);
6162
}

‎src/main/java/com/hubspot/jinjava/el/ext/eager/EagerAstMacroFunction.java‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.hubspot.jinjava.el.ext.AstMacroFunction;
44
import com.hubspot.jinjava.el.ext.DeferredParsingException;
55
import com.hubspot.jinjava.el.ext.ExtendedParser;
6+
import com.hubspot.jinjava.el.ext.IdentifierPreservationStrategy;
67
import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
78
import com.hubspot.jinjava.interpret.DeferredValueException;
89
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
@@ -12,7 +13,6 @@
1213
import java.lang.reflect.Array;
1314
import java.lang.reflect.InvocationTargetException;
1415
import java.lang.reflect.Method;
15-
import java.util.StringJoiner;
1616
import javax.el.ELContext;
1717
import javax.el.ELException;
1818

@@ -54,7 +54,13 @@ public Object eval(Bindings bindings, ELContext context) {
5454
);
5555
throw new DeferredParsingException(
5656
this,
57-
getPartiallyResolved(bindings, context, e, true) // Need this to always be true because the macro function may modify the identifier
57+
getPartiallyResolved(
58+
bindings,
59+
context,
60+
e,
61+
IdentifierPreservationStrategy.PRESERVING
62+
), // Need this to always be true because the macro function may modify the identifier
63+
IdentifierPreservationStrategy.PRESERVING
5864
);
5965
}
6066
}
@@ -146,34 +152,28 @@ public String getPartiallyResolved(
146152
Bindings bindings,
147153
ELContext context,
148154
DeferredParsingException deferredParsingException,
149-
boolean preserveIdentifier
155+
IdentifierPreservationStrategy identifierPreservationStrategy
150156
) {
151157
if (
152158
deferredParsingException != null &&
153159
deferredParsingException.getSourceNode() instanceof MacroFunction
154160
) {
155161
return deferredParsingException.getDeferredEvalResult();
156162
}
157-
StringBuilder sb = new StringBuilder();
158-
sb.append(getName());
159-
try {
160-
StringJoiner paramString = new StringJoiner(", ");
161-
for (int i = 0; i < ((AstParameters) params).getCardinality(); i++) {
162-
paramString.add(
163-
EvalResultHolder.reconstructNode(
164-
bindings,
165-
context,
166-
(EvalResultHolder) ((AstParameters) params).getChild(i),
167-
deferredParsingException,
168-
preserveIdentifier
169-
)
163+
String paramString;
164+
if (EvalResultHolder.exceptionMatchesNode(deferredParsingException, params)) {
165+
paramString = deferredParsingException.getDeferredEvalResult();
166+
} else {
167+
paramString =
168+
params.getPartiallyResolved(
169+
bindings,
170+
context,
171+
deferredParsingException,
172+
identifierPreservationStrategy
170173
);
171-
}
172-
sb.append(String.format("(%s)", paramString));
173-
} catch (DeferredParsingException dpe) {
174-
sb.append(String.format("(%s)", dpe.getDeferredEvalResult()));
175174
}
176-
return sb.toString();
175+
176+
return (getName() + String.format("(%s)", paramString));
177177
}
178178

179179
@Override

0 commit comments

Comments
 (0)