Skip to content

Commit f5975fa

Browse files
jasmith-hsjasmith-hs
committed
Replace isIgnoreParseErrors and isThrowInterpreterErrors with
ErrorHandlingStrategy
1 parent 4e144d8 commit f5975fa

7 files changed

Lines changed: 182 additions & 110 deletions

File tree

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

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.common.collect.ImmutableSet;
2222
import com.google.common.collect.SetMultimap;
2323
import com.google.common.collect.Sets;
24+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy;
2425
import com.hubspot.jinjava.lib.Importable;
2526
import com.hubspot.jinjava.lib.expression.ExpressionStrategy;
2627
import com.hubspot.jinjava.lib.exptest.ExpTest;
@@ -769,13 +770,64 @@ public TemporaryValueClosable<Boolean> withDeferLargeObjects(
769770
return temporaryValueClosable;
770771
}
771772

773+
@Deprecated
772774
public boolean getThrowInterpreterErrors() {
773-
return contextConfiguration.isThrowInterpreterErrors();
775+
ErrorHandlingStrategy errorHandlingStrategy = getErrorHandlingStrategy();
776+
return (
777+
errorHandlingStrategy.getFatalErrorStrategy() ==
778+
TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION
779+
);
774780
}
775781

782+
@Deprecated
776783
public void setThrowInterpreterErrors(boolean throwInterpreterErrors) {
777784
contextConfiguration =
778-
contextConfiguration.withThrowInterpreterErrors(throwInterpreterErrors);
785+
contextConfiguration.withErrorHandlingStrategy(
786+
ErrorHandlingStrategy
787+
.builder()
788+
.setFatalErrorStrategy(
789+
throwInterpreterErrors
790+
? TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION
791+
: TemplateErrorTypeHandlingStrategy.ADD_ERROR
792+
)
793+
.setNonFatalErrorStrategy(
794+
throwInterpreterErrors
795+
? TemplateErrorTypeHandlingStrategy.IGNORE // Deprecated, warnings are ignored when doing eager expression resolving
796+
: TemplateErrorTypeHandlingStrategy.ADD_ERROR
797+
)
798+
.build()
799+
);
800+
}
801+
802+
@Deprecated
803+
public TemporaryValueClosable<Boolean> withThrowInterpreterErrors() {
804+
TemporaryValueClosable<Boolean> temporaryValueClosable = new TemporaryValueClosable<>(
805+
getThrowInterpreterErrors(),
806+
this::setThrowInterpreterErrors
807+
);
808+
setThrowInterpreterErrors(true);
809+
return temporaryValueClosable;
810+
}
811+
812+
public ErrorHandlingStrategy getErrorHandlingStrategy() {
813+
return contextConfiguration.getErrorHandlingStrategy();
814+
}
815+
816+
public void setErrorHandlingStrategy(ErrorHandlingStrategy errorHandlingStrategy) {
817+
contextConfiguration =
818+
contextConfiguration.withErrorHandlingStrategy(errorHandlingStrategy);
819+
}
820+
821+
public TemporaryValueClosable<ErrorHandlingStrategy> withErrorHandlingStrategy(
822+
ErrorHandlingStrategy errorHandlingStrategy
823+
) {
824+
TemporaryValueClosable<ErrorHandlingStrategy> temporaryValueClosable =
825+
new TemporaryValueClosable<>(
826+
getErrorHandlingStrategy(),
827+
this::setErrorHandlingStrategy
828+
);
829+
setErrorHandlingStrategy(errorHandlingStrategy);
830+
return temporaryValueClosable;
779831
}
780832

781833
public boolean isPartialMacroEvaluation() {
@@ -819,23 +871,6 @@ public TemporaryValueClosable<Boolean> withUnwrapRawOverride() {
819871
return temporaryValueClosable;
820872
}
821873

822-
public boolean isIgnoreParseErrors() {
823-
return contextConfiguration.isIgnoreParseErrors();
824-
}
825-
826-
private void setIgnoreParseErrors(boolean ignoreParseErrors) {
827-
contextConfiguration = contextConfiguration.withIgnoreParseErrors(ignoreParseErrors);
828-
}
829-
830-
public TemporaryValueClosable<Boolean> withIgnoreParseErrors() {
831-
TemporaryValueClosable<Boolean> temporaryValueClosable = new TemporaryValueClosable<>(
832-
isIgnoreParseErrors(),
833-
this::setIgnoreParseErrors
834-
);
835-
setIgnoreParseErrors(true);
836-
return temporaryValueClosable;
837-
}
838-
839874
public static class TemporaryValueClosable<T> implements AutoCloseable {
840875

841876
private final T previousValue;

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

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ default boolean isDeferLargeObjects() {
3333
return false;
3434
}
3535

36-
@Default
37-
default boolean isThrowInterpreterErrors() {
38-
return false;
39-
}
40-
4136
@Default
4237
default boolean isPartialMacroEvaluation() {
4338
return false;
@@ -48,11 +43,42 @@ default boolean isUnwrapRawOverride() {
4843
return false;
4944
}
5045

51-
/**
52-
* When trying nested interpretation, parsing errors are likely. This flag avoids propogating to differentiate between static and dynamic parsing errors.
53-
*/
5446
@Default
55-
default boolean isIgnoreParseErrors() {
56-
return false;
47+
default ErrorHandlingStrategy getErrorHandlingStrategy() {
48+
return ErrorHandlingStrategy.of();
49+
}
50+
51+
@Immutable(singleton = true)
52+
@HubSpotImmutableStyle
53+
interface ErrorHandlingStrategyIF {
54+
@Default
55+
default TemplateErrorTypeHandlingStrategy getFatalErrorStrategy() {
56+
return TemplateErrorTypeHandlingStrategy.ADD_ERROR;
57+
}
58+
59+
@Default
60+
default TemplateErrorTypeHandlingStrategy getNonFatalErrorStrategy() {
61+
return TemplateErrorTypeHandlingStrategy.ADD_ERROR;
62+
}
63+
64+
enum TemplateErrorTypeHandlingStrategy {
65+
IGNORE,
66+
ADD_ERROR,
67+
THROW_EXCEPTION,
68+
}
69+
70+
static ErrorHandlingStrategy throwAll() {
71+
return ErrorHandlingStrategy
72+
.of()
73+
.withFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION)
74+
.withNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.THROW_EXCEPTION);
75+
}
76+
77+
static ErrorHandlingStrategy ignoreAll() {
78+
return ErrorHandlingStrategy
79+
.of()
80+
.withFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE)
81+
.withNonFatalErrorStrategy(TemplateErrorTypeHandlingStrategy.IGNORE);
82+
}
5783
}
5884
}

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

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import com.hubspot.jinjava.el.ext.DeferredParsingException;
3131
import com.hubspot.jinjava.el.ext.ExtendedParser;
3232
import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
33+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF;
34+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy;
3335
import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
3436
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
3537
import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
@@ -262,27 +264,33 @@ public String renderFlat(String template) {
262264
public String renderFlat(String template, long renderLimit) {
263265
int depth = context.getRenderDepth();
264266

265-
try (TemporaryValueClosable<Boolean> c = ignoreParseErrorsIfActivated()) {
267+
try {
266268
if (depth > config.getMaxRenderDepth()) {
267269
ENGINE_LOG.warn("Max render depth exceeded: {}", Integer.toString(depth));
268270
return template;
269271
} else {
270272
context.setRenderDepth(depth + 1);
271-
return render(parse(template), false, renderLimit);
273+
Node parsedNode;
274+
try (
275+
TemporaryValueClosable<ErrorHandlingStrategy> c = ignoreParseErrorsIfActivated()
276+
) {
277+
parsedNode = parse(template);
278+
}
279+
return render(parsedNode, false, renderLimit);
272280
}
273281
} finally {
274282
context.setRenderDepth(depth);
275283
}
276284
}
277285

278-
private TemporaryValueClosable<Boolean> ignoreParseErrorsIfActivated() {
286+
private TemporaryValueClosable<ErrorHandlingStrategy> ignoreParseErrorsIfActivated() {
279287
return config
280288
.getFeatures()
281289
.getActivationStrategy(
282290
JinjavaInterpreter.IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS
283291
)
284292
.isActive(context)
285-
? context.withIgnoreParseErrors()
293+
? context.withErrorHandlingStrategy(ErrorHandlingStrategyIF.ignoreAll())
286294
: TemporaryValueClosable.noOp();
287295
}
288296

@@ -826,46 +834,50 @@ public void addError(TemplateError templateError) {
826834
if (templateError == null) {
827835
return;
828836
}
829-
830-
if (context.getThrowInterpreterErrors()) {
831-
if (templateError.getSeverity() == ErrorType.FATAL) {
832-
// Throw fatal errors when locating deferred words.
837+
ErrorHandlingStrategy errorHandlingStrategy = context.getErrorHandlingStrategy();
838+
TemplateErrorTypeHandlingStrategy errorTypeHandlingStrategy =
839+
templateError.getSeverity() == ErrorType.FATAL
840+
? errorHandlingStrategy.getFatalErrorStrategy()
841+
: errorHandlingStrategy.getNonFatalErrorStrategy();
842+
switch (errorTypeHandlingStrategy) {
843+
case IGNORE:
844+
return;
845+
case THROW_EXCEPTION:
833846
throw new TemplateSyntaxException(
834847
this,
835848
templateError.getFieldName(),
836849
templateError.getMessage()
837850
);
838-
} else {
839-
// Hide warning errors when locating deferred words.
840-
return;
841-
}
842-
}
843-
// fix line numbers not matching up with source template
844-
if (!context.getCurrentPathStack().isEmpty()) {
845-
if (
846-
!templateError.getSourceTemplate().isPresent() &&
847-
context.getCurrentPathStack().peek().isPresent()
848-
) {
849-
templateError.setMessage(
850-
getWrappedErrorMessage(
851-
context.getCurrentPathStack().peek().get(),
852-
templateError
853-
)
854-
);
855-
templateError.setSourceTemplate(context.getCurrentPathStack().peek().get());
856-
}
857-
templateError.setStartPosition(context.getCurrentPathStack().getTopStartPosition());
858-
templateError.setLineno(context.getCurrentPathStack().getTopLineNumber());
859-
}
851+
case ADD_ERROR:
852+
// fix line numbers not matching up with source template
853+
if (!context.getCurrentPathStack().isEmpty()) {
854+
if (
855+
!templateError.getSourceTemplate().isPresent() &&
856+
context.getCurrentPathStack().peek().isPresent()
857+
) {
858+
templateError.setMessage(
859+
getWrappedErrorMessage(
860+
context.getCurrentPathStack().peek().get(),
861+
templateError
862+
)
863+
);
864+
templateError.setSourceTemplate(context.getCurrentPathStack().peek().get());
865+
}
866+
templateError.setStartPosition(
867+
context.getCurrentPathStack().getTopStartPosition()
868+
);
869+
templateError.setLineno(context.getCurrentPathStack().getTopLineNumber());
870+
}
860871

861-
// Limit the number of errors and filter duplicates
862-
if (errors.size() < MAX_ERROR_SIZE) {
863-
templateError = templateError.withScopeDepth(scopeDepth);
864-
int errorCode = templateError.hashCode();
865-
if (!errorSet.contains(errorCode)) {
866-
this.errors.add(templateError);
867-
this.errorSet.add(errorCode);
868-
}
872+
// Limit the number of errors and filter duplicates
873+
if (errors.size() < MAX_ERROR_SIZE) {
874+
templateError = templateError.withScopeDepth(scopeDepth);
875+
int errorCode = templateError.hashCode();
876+
if (!errorSet.contains(errorCode)) {
877+
this.errors.add(templateError);
878+
this.errorSet.add(errorCode);
879+
}
880+
}
869881
}
870882
}
871883

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

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
import com.google.common.annotations.Beta;
44
import com.hubspot.jinjava.JinjavaConfig;
5+
import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable;
6+
import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF;
7+
import com.hubspot.jinjava.interpret.ErrorHandlingStrategy;
58
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
6-
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
9+
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
710
import com.hubspot.jinjava.lib.filter.EscapeFilter;
811
import com.hubspot.jinjava.lib.tag.eager.DeferredToken;
912
import com.hubspot.jinjava.lib.tag.eager.EagerExecutionResult;
@@ -14,7 +17,6 @@
1417
import com.hubspot.jinjava.util.EagerReconstructionUtils;
1518
import com.hubspot.jinjava.util.Logging;
1619
import com.hubspot.jinjava.util.PrefixToPreserveState;
17-
import java.util.Objects;
1820
import org.apache.commons.lang3.StringUtils;
1921

2022
@Beta
@@ -103,17 +105,20 @@ public static String postProcessResult(
103105
StringUtils.contains(result, master.getSymbols().getExpressionStartWithTag()))
104106
) {
105107
if (interpreter.getConfig().isNestedInterpretationEnabled()) {
106-
long errorSizeStart = getParsingErrorsCount(interpreter);
107-
108-
interpreter.parse(result);
109-
110-
if (getParsingErrorsCount(interpreter) == errorSizeStart) {
108+
try {
109+
try (
110+
TemporaryValueClosable<ErrorHandlingStrategy> c = interpreter
111+
.getContext()
112+
.withErrorHandlingStrategy(ErrorHandlingStrategyIF.throwAll())
113+
) {
114+
interpreter.parse(result);
115+
}
111116
try {
112117
result = interpreter.renderFlat(result);
113118
} catch (Exception e) {
114119
Logging.ENGINE_LOG.warn("Error rendering variable node result", e);
115120
}
116-
}
121+
} catch (TemplateSyntaxException ignored) {}
117122
} else {
118123
result = EagerReconstructionUtils.wrapInRawIfNeeded(result, interpreter);
119124
}
@@ -125,18 +130,6 @@ public static String postProcessResult(
125130
return result;
126131
}
127132

128-
private static long getParsingErrorsCount(JinjavaInterpreter interpreter) {
129-
return interpreter
130-
.getErrors()
131-
.stream()
132-
.filter(Objects::nonNull)
133-
.filter(error ->
134-
"Unclosed comment".equals(error.getMessage()) ||
135-
error.getReason() == ErrorReason.DISABLED
136-
)
137-
.count();
138-
}
139-
140133
private static String wrapInExpression(String output, JinjavaInterpreter interpreter) {
141134
JinjavaConfig config = interpreter.getConfig();
142135
return String.format(

0 commit comments

Comments
 (0)