Skip to content

Commit 75d354c

Browse files
jasmith-hsjasmith-hs
committed
Add algebra Result for better error handling
1 parent ea45395 commit 75d354c

12 files changed

Lines changed: 626 additions & 441 deletions

File tree

‎pom.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
<dep.plugin.jacoco.version>0.8.3</dep.plugin.jacoco.version>
2121
<dep.plugin.javadoc.version>3.0.1</dep.plugin.javadoc.version>
2222
<dep.hubspot-immutables.version>1.9</dep.hubspot-immutables.version>
23+
<dep.algebra.version>1.5</dep.algebra.version>
2324

2425

2526
<basepom.test.add.opens>
@@ -101,6 +102,11 @@
101102
<artifactId>immutables-exceptions</artifactId>
102103
<version>${dep.hubspot-immutables.version}</version>
103104
</dependency>
105+
<dependency>
106+
<groupId>com.hubspot</groupId>
107+
<artifactId>algebra</artifactId>
108+
<version>${dep.algebra.version}</version>
109+
</dependency>
104110
</dependencies>
105111
</dependencyManagement>
106112

@@ -213,6 +219,10 @@
213219
<groupId>com.hubspot.immutables</groupId>
214220
<artifactId>immutables-exceptions</artifactId>
215221
</dependency>
222+
<dependency>
223+
<groupId>com.hubspot</groupId>
224+
<artifactId>algebra</artifactId>
225+
</dependency>
216226
</dependencies>
217227

218228
<build>

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

Lines changed: 77 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package com.hubspot.jinjava.el.ext;
22

33
import com.google.common.collect.ImmutableMap;
4+
import com.hubspot.algebra.Result;
45
import com.hubspot.jinjava.interpret.AutoCloseableSupplier;
56
import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl;
67
import com.hubspot.jinjava.interpret.CallStack;
78
import com.hubspot.jinjava.interpret.DeferredValueException;
89
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
9-
import com.hubspot.jinjava.interpret.MacroTagCycleException;
10+
import com.hubspot.jinjava.interpret.TagCycleException;
1011
import com.hubspot.jinjava.interpret.TemplateError;
1112
import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory;
1213
import com.hubspot.jinjava.lib.fn.MacroFunction;
@@ -20,6 +21,10 @@
2021

2122
public class AstMacroFunction extends AstFunction {
2223

24+
public enum MacroCallError {
25+
CYCLE_DETECTED,
26+
}
27+
2328
public AstMacroFunction(String name, int index, AstParameters params, boolean varargs) {
2429
super(name, index, params, varargs);
2530
}
@@ -43,17 +48,12 @@ public Object eval(Bindings bindings, ELContext context) {
4348
return wrapInvoke(bindings, context, macroFunction);
4449
}
4550
try (
46-
AutoCloseableImpl<Boolean> macroStackPush = checkAndPushMacroStackWithWrapper(
47-
interpreter,
48-
getName()
49-
)
50-
.get()
51+
AutoCloseableImpl<Result<String, MacroCallError>> macroStackPush =
52+
checkAndPushMacroStackWithWrapper(interpreter, getName()).get()
5153
) {
52-
if (macroStackPush.value()) {
53-
return "";
54-
}
55-
56-
return wrapInvoke(bindings, context, macroFunction);
54+
return macroStackPush
55+
.value()
56+
.match(err -> "", path -> wrapInvoke(bindings, context, macroFunction));
5757
}
5858
}
5959

@@ -79,65 +79,75 @@ private Object wrapInvoke(
7979
}
8080
}
8181

82-
public static AutoCloseableSupplier<Boolean> checkAndPushMacroStackWithWrapper(
82+
public static AutoCloseableSupplier<Result<String, MacroCallError>> checkAndPushMacroStackWithWrapper(
8383
JinjavaInterpreter interpreter,
8484
String name
8585
) {
8686
CallStack macroStack = interpreter.getContext().getMacroStack();
87-
try {
88-
if (interpreter.getConfig().isEnableRecursiveMacroCalls()) {
89-
if (interpreter.getConfig().getMaxMacroRecursionDepth() != 0) {
90-
return macroStack
91-
.closeablePushWithMaxDepth(
92-
name,
93-
interpreter.getConfig().getMaxMacroRecursionDepth(),
94-
interpreter.getLineNumber(),
95-
interpreter.getPosition()
96-
)
97-
.map(macro -> false);
98-
} else {
99-
return macroStack
100-
.closeablePushWithoutCycleCheck(
101-
name,
102-
interpreter.getLineNumber(),
103-
interpreter.getPosition()
104-
)
105-
.map(macro -> false);
106-
}
107-
}
108-
return macroStack.closeablePush(name, -1, -1).map(macro -> false);
109-
} catch (MacroTagCycleException e) {
110-
int maxDepth = interpreter.getConfig().getMaxMacroRecursionDepth();
111-
if (maxDepth != 0 && interpreter.getConfig().isValidationMode()) {
112-
// validation mode is only concerned with syntax
113-
return AutoCloseableSupplier.of(true);
87+
if (interpreter.getConfig().isEnableRecursiveMacroCalls()) {
88+
if (interpreter.getConfig().getMaxMacroRecursionDepth() != 0) {
89+
return macroStack
90+
.closeablePushWithMaxDepth(
91+
name,
92+
interpreter.getConfig().getMaxMacroRecursionDepth(),
93+
interpreter.getLineNumber(),
94+
interpreter.getPosition()
95+
)
96+
.map(result ->
97+
result.mapErr(err -> {
98+
handleMacroCycleError(interpreter, name, err);
99+
return MacroCallError.CYCLE_DETECTED;
100+
})
101+
);
102+
} else {
103+
return macroStack
104+
.closeablePushWithoutCycleCheck(
105+
name,
106+
interpreter.getLineNumber(),
107+
interpreter.getPosition()
108+
)
109+
.map(Result::ok);
114110
}
115-
116-
String message = maxDepth == 0
117-
? String.format("Cycle detected for macro '%s'", name)
118-
: String.format(
119-
"Max recursion limit of %d reached for macro '%s'",
120-
maxDepth,
121-
name
122-
);
123-
124-
interpreter.addError(
125-
new TemplateError(
126-
TemplateError.ErrorType.WARNING,
127-
TemplateError.ErrorReason.EXCEPTION,
128-
TemplateError.ErrorItem.TAG,
129-
message,
130-
null,
131-
e.getLineNumber(),
132-
e.getStartPosition(),
133-
e,
134-
BasicTemplateErrorCategory.CYCLE_DETECTED,
135-
ImmutableMap.of("name", name)
136-
)
111+
}
112+
return macroStack
113+
.closeablePush(name, -1, -1)
114+
.map(result ->
115+
result.mapErr(err -> {
116+
handleMacroCycleError(interpreter, name, err);
117+
return MacroCallError.CYCLE_DETECTED;
118+
})
137119
);
120+
}
138121

139-
return AutoCloseableSupplier.of(true);
122+
private static void handleMacroCycleError(
123+
JinjavaInterpreter interpreter,
124+
String name,
125+
TagCycleException e
126+
) {
127+
int maxDepth = interpreter.getConfig().getMaxMacroRecursionDepth();
128+
if (maxDepth != 0 && interpreter.getConfig().isValidationMode()) {
129+
// validation mode is only concerned with syntax
130+
return;
140131
}
132+
133+
String message = maxDepth == 0
134+
? String.format("Cycle detected for macro '%s'", name)
135+
: String.format("Max recursion limit of %d reached for macro '%s'", maxDepth, name);
136+
137+
interpreter.addError(
138+
new TemplateError(
139+
TemplateError.ErrorType.WARNING,
140+
TemplateError.ErrorReason.EXCEPTION,
141+
TemplateError.ErrorItem.TAG,
142+
message,
143+
null,
144+
e.getLineNumber(),
145+
e.getStartPosition(),
146+
e,
147+
BasicTemplateErrorCategory.CYCLE_DETECTED,
148+
ImmutableMap.of("name", name)
149+
)
150+
);
141151
}
142152

143153
@Deprecated
@@ -146,6 +156,10 @@ public static boolean checkAndPushMacroStack(
146156
String name
147157
) {
148158
return checkAndPushMacroStackWithWrapper(interpreter, name)
149-
.dangerouslyGetWithoutClosing();
159+
.dangerouslyGetWithoutClosing()
160+
.match(
161+
err -> true, // cycle detected
162+
ok -> false // no cycle
163+
);
150164
}
151165
}

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

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,49 @@
11
package com.hubspot.jinjava.interpret;
22

3+
import com.google.common.base.Suppliers;
34
import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl;
45
import java.util.function.Consumer;
6+
import java.util.function.Function;
57
import java.util.function.Supplier;
68

79
public class AutoCloseableSupplier<T> implements Supplier<AutoCloseableImpl<T>> {
810

9-
public interface GenericThrowingFunction<T, R, E extends Exception> {
10-
R apply(T t) throws E;
11+
public static <T> AutoCloseableSupplier<T> of(T tSupplier) {
12+
return of(() -> tSupplier, ignored -> {});
1113
}
1214

13-
public static <T> AutoCloseableSupplier<T> of(T t) {
14-
return new AutoCloseableSupplier<>(new AutoCloseableImpl<>(t, ignored -> {}));
15-
}
16-
17-
public static <T> AutoCloseableSupplier<T> of(T t, Consumer<T> closeConsumer) {
18-
return new AutoCloseableSupplier<>(new AutoCloseableImpl<>(t, closeConsumer));
15+
public static <T> AutoCloseableSupplier<T> of(
16+
Supplier<T> tSupplier,
17+
Consumer<T> closeConsumer
18+
) {
19+
return new AutoCloseableSupplier<>(
20+
Suppliers.memoize(() -> new AutoCloseableImpl<>(tSupplier.get(), closeConsumer))
21+
);
1922
}
2023

21-
private final AutoCloseableImpl<T> autoCloseableImplWrapper;
24+
private final Supplier<AutoCloseableImpl<T>> autoCloseableImplWrapper;
2225

23-
private AutoCloseableSupplier(AutoCloseableImpl<T> autoCloseableImplWrapper) {
26+
private AutoCloseableSupplier(Supplier<AutoCloseableImpl<T>> autoCloseableImplWrapper) {
2427
this.autoCloseableImplWrapper = autoCloseableImplWrapper;
2528
}
2629

2730
@Override
2831
public AutoCloseableImpl<T> get() {
29-
return autoCloseableImplWrapper;
32+
return autoCloseableImplWrapper.get();
3033
}
3134

3235
public T dangerouslyGetWithoutClosing() {
33-
return autoCloseableImplWrapper.value();
36+
return autoCloseableImplWrapper.get().value();
3437
}
3538

36-
public <R, E extends Exception> AutoCloseableSupplier<R> map(
37-
GenericThrowingFunction<T, R, E> mapper
38-
) throws E {
39-
T t = autoCloseableImplWrapper.value();
40-
return new AutoCloseableSupplier<>(
41-
new AutoCloseableImpl<>(
39+
public <R> AutoCloseableSupplier<R> map(Function<T, R> mapper) {
40+
return new AutoCloseableSupplier<>(() -> {
41+
T t = autoCloseableImplWrapper.get().value();
42+
return new AutoCloseableImpl<>(
4243
mapper.apply(t),
43-
r -> autoCloseableImplWrapper.closeConsumer.accept(t)
44-
)
45-
);
44+
r -> autoCloseableImplWrapper.get().closeConsumer.accept(t)
45+
);
46+
});
4647
}
4748

4849
public static class AutoCloseableImpl<T> implements java.lang.AutoCloseable {

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

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.hubspot.jinjava.interpret;
22

3+
import com.hubspot.algebra.Result;
34
import java.util.Optional;
45
import java.util.Stack;
56

@@ -111,32 +112,55 @@ public boolean isEmpty() {
111112
return stack.empty() && (parent == null || parent.isEmpty());
112113
}
113114

114-
public AutoCloseableSupplier<String> closeablePush(
115+
public AutoCloseableSupplier<Result<String, TagCycleException>> closeablePush(
115116
String path,
116117
int lineNumber,
117118
int startPosition
118119
) {
119-
push(path, lineNumber, startPosition);
120-
return AutoCloseableSupplier.of(path, ignored -> pop());
120+
return AutoCloseableSupplier.of(
121+
() -> {
122+
try {
123+
push(path, lineNumber, startPosition);
124+
return Result.ok(path);
125+
} catch (TagCycleException e) {
126+
return Result.err(e);
127+
}
128+
},
129+
result -> result.ifOk(ok -> pop())
130+
);
121131
}
122132

123133
public AutoCloseableSupplier<String> closeablePushWithoutCycleCheck(
124134
String path,
125135
int lineNumber,
126136
int startPosition
127137
) {
128-
pushWithoutCycleCheck(path, lineNumber, startPosition);
129-
return AutoCloseableSupplier.of(path, ignored -> pop());
138+
return AutoCloseableSupplier.of(
139+
() -> {
140+
pushWithoutCycleCheck(path, lineNumber, startPosition);
141+
return path;
142+
},
143+
ignored -> pop()
144+
);
130145
}
131146

132-
public AutoCloseableSupplier<String> closeablePushWithMaxDepth(
147+
public AutoCloseableSupplier<Result<String, TagCycleException>> closeablePushWithMaxDepth(
133148
String path,
134149
int maxDepth,
135150
int lineNumber,
136151
int startPosition
137152
) {
138-
pushWithMaxDepth(path, maxDepth, lineNumber, startPosition);
139-
return AutoCloseableSupplier.of(path, ignored -> pop());
153+
return AutoCloseableSupplier.of(
154+
() -> {
155+
try {
156+
pushWithMaxDepth(path, maxDepth, lineNumber, startPosition);
157+
return Result.ok(path);
158+
} catch (TagCycleException e) {
159+
return Result.err(e);
160+
}
161+
},
162+
result -> result.ifOk(ok -> pop())
163+
);
140164
}
141165

142166
private void pushToStack(String path, int lineNumber, int startPosition) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ public void setRenderDepth(int renderDepth) {
726726

727727
public AutoCloseableSupplier<String> closeablePushRenderStack(String template) {
728728
renderStack.push(template);
729-
return AutoCloseableSupplier.of(template, t -> renderStack.pop());
729+
return AutoCloseableSupplier.of(() -> template, t -> renderStack.pop());
730730
}
731731

732732
@Deprecated

0 commit comments

Comments
 (0)