diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java index d62314c9d..f1b3b45c3 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java @@ -1,11 +1,11 @@ package com.hubspot.jinjava.lib.filter; -import static com.hubspot.jinjava.util.Logging.ENGINE_LOG; - import com.hubspot.jinjava.doc.annotations.JinjavaDoc; import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; +import com.hubspot.jinjava.interpret.InvalidArgumentException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.lib.fn.Functions; import com.hubspot.jinjava.objects.SafeString; import java.util.Map; @@ -115,16 +115,24 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args) try { length = Integer.parseInt(Objects.toString(args[0])); } catch (Exception e) { - ENGINE_LOG.warn( - "truncatehtml(): error setting length for {}, using default {}", - args[0], - DEFAULT_TRUNCATE_LENGTH + interpreter.addError( + TemplateError.fromInvalidArgumentException( + new InvalidArgumentException( + interpreter, + "truncatehtml", + String.format( + "truncatehtml(): error setting length of %s, using default of %d", + args[0], + DEFAULT_TRUNCATE_LENGTH + ) + ) + ) ); } } if (args.length > 1 && args[1] != null) { - ends = Objects.toString(args[1]); + ends = args[1]; } boolean killwords = false; @@ -148,10 +156,10 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args) } private static class ContentTruncatingNodeVisitor implements NodeVisitor { - private int maxTextLen; + private final int maxTextLen; private int textLen; - private String ending; - private boolean killwords; + private final String ending; + private final boolean killwords; ContentTruncatingNodeVisitor(int maxTextLen, String ending, boolean killwords) { this.maxTextLen = maxTextLen; diff --git a/src/main/java/com/hubspot/jinjava/lib/fn/Functions.java b/src/main/java/com/hubspot/jinjava/lib/fn/Functions.java index f53513516..0b303cd7c 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/Functions.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/Functions.java @@ -1,7 +1,5 @@ package com.hubspot.jinjava.lib.fn; -import static com.hubspot.jinjava.util.Logging.ENGINE_LOG; - import com.google.common.collect.Lists; import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.doc.annotations.JinjavaDoc; @@ -10,7 +8,9 @@ import com.hubspot.jinjava.el.ext.NamedParameter; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.InterpretException; +import com.hubspot.jinjava.interpret.InvalidArgumentException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.mode.ExecutionMode; import com.hubspot.jinjava.objects.Namespace; import com.hubspot.jinjava.objects.date.InvalidDateFormatException; @@ -384,6 +384,8 @@ public static PyishDate stringToDate(String dateString, String dateFormat) { } ) public static Object truncate(Object var, Object... arg) { + JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); + if (var instanceof String) { int length = DEFAULT_TRUNCATE_LENGTH; boolean killwords = false; @@ -393,10 +395,18 @@ public static Object truncate(Object var, Object... arg) { try { length = Integer.parseInt(Objects.toString(arg[0])); } catch (Exception e) { - ENGINE_LOG.debug( - "truncate(): error setting length for {}, using default {}", - arg[0], - DEFAULT_TRUNCATE_LENGTH + interpreter.addError( + TemplateError.fromInvalidArgumentException( + new InvalidArgumentException( + interpreter, + "truncate", + String.format( + "truncate(): error setting length of %s, using default of %d", + arg[0], + DEFAULT_TRUNCATE_LENGTH + ) + ) + ) ); } } @@ -405,7 +415,15 @@ public static Object truncate(Object var, Object... arg) { try { killwords = BooleanUtils.toBoolean(Objects.toString(arg[1])); } catch (Exception e) { - ENGINE_LOG.warn("truncate(); error setting killwords for {}", arg[1]); + interpreter.addError( + TemplateError.fromInvalidArgumentException( + new InvalidArgumentException( + interpreter, + "truncate", + String.format("truncate(): error setting killwords for %s", arg[1]) + ) + ) + ); } } @@ -461,21 +479,29 @@ public static List range(Object arg1, Object... args) { int end = 0; int step = 1; + if (arg1 == null) { + throw new InvalidArgumentException( + JinjavaInterpreter.getCurrent(), + "range", + "Invalid null passed to range function" + ); + } + switch (args.length) { case 0: - if (NumberUtils.isNumber(arg1.toString())) { + if (NumberUtils.isCreatable(arg1.toString())) { end = NumberUtils.toInt(arg1.toString(), rangeLimit); } break; case 1: start = NumberUtils.toInt(arg1.toString()); - if (args[0] != null && NumberUtils.isNumber(args[0].toString())) { + if (args[0] != null && NumberUtils.isCreatable(args[0].toString())) { end = NumberUtils.toInt(args[0].toString(), start + rangeLimit); } break; default: start = NumberUtils.toInt(arg1.toString()); - if (args[0] != null && NumberUtils.isNumber(args[0].toString())) { + if (args[0] != null && NumberUtils.isCreatable(args[0].toString())) { end = NumberUtils.toInt(args[0].toString(), start + rangeLimit); } if (args[1] != null) { diff --git a/src/main/java/com/hubspot/jinjava/util/ForLoop.java b/src/main/java/com/hubspot/jinjava/util/ForLoop.java index 2d364129e..6a0127bb5 100644 --- a/src/main/java/com/hubspot/jinjava/util/ForLoop.java +++ b/src/main/java/com/hubspot/jinjava/util/ForLoop.java @@ -15,8 +15,6 @@ **********************************************************************/ package com.hubspot.jinjava.util; -import static com.hubspot.jinjava.util.Logging.ENGINE_LOG; - import java.util.Iterator; public class ForLoop implements Iterator { @@ -32,7 +30,7 @@ public class ForLoop implements Iterator { private int depth; - private Iterator it; + private final Iterator it; public ForLoop(Iterator ite, int len) { length = len; @@ -111,23 +109,14 @@ public int getRevindex() { } public int getRevindex0() { - if (revindex == NULL_VALUE) { - ENGINE_LOG.warn("can't compute items' length while looping."); - } return revindex; } public int getRevcounter() { - if (revcounter == NULL_VALUE) { - ENGINE_LOG.warn("can't compute items' length while looping."); - } return revcounter; } public int getLength() { - if (revcounter == NULL_VALUE) { - ENGINE_LOG.warn("can't compute items' length while looping."); - } return length; } diff --git a/src/test/java/com/hubspot/jinjava/lib/fn/RangeFunctionTest.java b/src/test/java/com/hubspot/jinjava/lib/fn/RangeFunctionTest.java index 2f5223401..cc6073406 100644 --- a/src/test/java/com/hubspot/jinjava/lib/fn/RangeFunctionTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/fn/RangeFunctionTest.java @@ -6,11 +6,10 @@ import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; -import com.hubspot.jinjava.LegacyOverrides; +import com.hubspot.jinjava.interpret.InvalidArgumentException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import java.util.Arrays; import java.util.Collections; -import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -70,6 +69,12 @@ public void itHandlesBadValues() { assertThat(Functions.range(2, 4, null)).isEqualTo(Arrays.asList(2, 3)); } + @Test + public void itHandlesMissingArg() { + assertThatThrownBy(() -> Functions.range(null)) + .isInstanceOf(InvalidArgumentException.class); + } + @Test public void itTruncatesRangeToDefaultRangeLimit() { int defaultRangeLimit = config.getRangeLimit();