From eab8cb4175d62090cc30ff96ac96885d8f683768 Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Thu, 12 Nov 2020 21:26:11 +1100 Subject: [PATCH 1/7] AbstractFilter: handle Number=> Any numeric conversion without string. Null parameter value is treated as missing --- .../jinjava/lib/filter/AbstractFilter.java | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java index 3bbc2fd1f..1ce929772 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java @@ -40,7 +40,7 @@ * @see JinjavaDoc * @see JinjavaParam */ -public abstract class AbstractFilter implements Filter { +public abstract class AbstractFilter implements Filter, AdvancedFilter { private static final Map> NAMED_ARGUMENTS_CACHE = new ConcurrentHashMap<>(); private static final Map> DEFAULT_VALUES_CACHE = new ConcurrentHashMap<>(); @@ -130,28 +130,33 @@ protected Object parseArg( ) { return value; } - String valueString = Objects.toString(value, null); switch (jinjavaParamMetadata.type().toLowerCase()) { case "boolean": return value instanceof Boolean ? (Boolean) value - : BooleanUtils.toBooleanObject(valueString); + : BooleanUtils.toBooleanObject(value.toString()); case "int": - return value instanceof Integer - ? (Integer) value - : NumberUtils.toInt(valueString); + return value instanceof Number + ? ((Number) value).intValue() + : NumberUtils.toInt(value.toString()); case "long": - return value instanceof Long ? (Long) value : NumberUtils.toLong(valueString); + return value instanceof Number + ? ((Number) value).longValue() + : NumberUtils.toLong(value.toString()); case "float": - return value instanceof Float ? (Float) value : NumberUtils.toFloat(valueString); + return value instanceof Number + ? ((Number) value).floatValue() + : NumberUtils.toFloat(value.toString()); case "double": - return value instanceof Double - ? (Double) value - : NumberUtils.toDouble(valueString); + return value instanceof Number + ? ((Number) value).doubleValue() + : NumberUtils.toDouble(value.toString()); case "number": - return value instanceof Number ? (Number) value : new BigDecimal(valueString); + return value instanceof Number + ? (Number) value + : new BigDecimal(value.toString()); case "string": - return valueString; + return value.toString(); default: throw new InvalidInputException( interpreter, @@ -171,7 +176,13 @@ public void validateArgs( Map parsedArgs ) { for (JinjavaParam jinjavaParam : namedArguments.values()) { - if (jinjavaParam.required() && !parsedArgs.containsKey(jinjavaParam.value())) { + if ( + jinjavaParam.required() && + ( + !parsedArgs.containsKey(jinjavaParam.value()) || + parsedArgs.get(jinjavaParam.value()) == null + ) + ) { throw new InvalidInputException( interpreter, "MISSING_REQUIRED_ARG", From 279c03587da25f706ec89c45cf754fa320092849 Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Thu, 12 Nov 2020 21:32:16 +1100 Subject: [PATCH 2/7] regex_replace, replace, round, truncatehtml, urlize filters migrated to AbstractFilter --- .../lib/filter/RegexReplaceFilter.java | 36 ++---- .../jinjava/lib/filter/ReplaceFilter.java | 39 +++--- .../jinjava/lib/filter/RoundFilter.java | 61 +++++---- .../lib/filter/TruncateHtmlFilter.java | 117 ++++-------------- .../jinjava/lib/filter/UrlizeFilter.java | 43 +++---- .../lib/filter/RegexReplaceFilterTest.java | 4 +- .../jinjava/lib/filter/ReplaceFilterTest.java | 4 +- .../lib/filter/TruncateHtmlFilterTest.java | 4 +- 8 files changed, 114 insertions(+), 194 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilter.java index 63f98e212..7005a7193 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilter.java @@ -10,7 +10,7 @@ import com.hubspot.jinjava.interpret.InvalidInputException; import com.hubspot.jinjava.interpret.InvalidReason; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import com.hubspot.jinjava.interpret.TemplateSyntaxException; +import java.util.Map; @JinjavaDoc( value = "Return a copy of the value with all occurrences of a matched regular expression (Java RE2 syntax) " + @@ -23,12 +23,12 @@ ), params = { @JinjavaParam( - value = "regex", + value = RegexReplaceFilter.REGEX_KEY, desc = "The regular expression that you want to match and replace", required = true ), @JinjavaParam( - value = "new", + value = RegexReplaceFilter.REPLACE_WITH, desc = "The new string that you replace the matched substring", required = true ) @@ -40,7 +40,9 @@ ) } ) -public class RegexReplaceFilter implements Filter { +public class RegexReplaceFilter extends AbstractFilter { + public static final String REGEX_KEY = "regex"; + public static final String REPLACE_WITH = "new"; @Override public String getName() { @@ -48,31 +50,19 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { - if (args.length < 2) { - throw new TemplateSyntaxException( - interpreter, - getName(), - "requires 2 arguments (regex string, replacement string)" - ); - } - - if (args[0] == null || args[1] == null) { - throw new TemplateSyntaxException( - interpreter, - getName(), - "requires both a valid regex and new params (not null)" - ); - } - + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { if (var == null) { return null; } if (var instanceof String) { String s = (String) var; - String toReplace = args[0]; - String replaceWith = args[1]; + String toReplace = (String) parsedArgs.get(REGEX_KEY); + String replaceWith = (String) parsedArgs.get(REPLACE_WITH); try { Pattern p = Pattern.compile(toReplace); diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/ReplaceFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/ReplaceFilter.java index ab3e74658..3b21541cc 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/ReplaceFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/ReplaceFilter.java @@ -4,9 +4,8 @@ import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import com.hubspot.jinjava.interpret.TemplateSyntaxException; +import java.util.Map; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.math.NumberUtils; @JinjavaDoc( value = "Return a copy of the value with all occurrences of a substring replaced with a new one. " + @@ -19,18 +18,18 @@ ), params = { @JinjavaParam( - value = "old", + value = ReplaceFilter.OLD_KEY, desc = "The old substring that you want to match and replace", required = true ), @JinjavaParam( - value = "new", + value = ReplaceFilter.REPLACE_WITH_KEY, desc = "The new string that you replace the matched substring", required = true ), @JinjavaParam( - value = "count", - type = "number", + value = ReplaceFilter.COUNT_KEY, + type = "int", desc = "Replace only the first N occurrences" ) }, @@ -45,7 +44,10 @@ ) } ) -public class ReplaceFilter implements Filter { +public class ReplaceFilter extends AbstractFilter { + public static final String OLD_KEY = "old"; + public static final String REPLACE_WITH_KEY = "new"; + public static final String COUNT_KEY = "count"; @Override public String getName() { @@ -53,26 +55,19 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { if (var == null) { return null; } - if (args.length < 2) { - throw new TemplateSyntaxException( - interpreter, - getName(), - "requires 2 arguments (substring to replace, replacement string) or 3 arguments (substring to replace, replacement string, number of occurrences to replace)" - ); - } String s = (String) var; - String toReplace = args[0]; - String replaceWith = args[1]; - Integer count = null; - - if (args.length > 2) { - count = NumberUtils.createInteger(args[2]); - } + String toReplace = (String) parsedArgs.get(OLD_KEY); + String replaceWith = (String) parsedArgs.get(REPLACE_WITH_KEY); + Integer count = (Integer) (parsedArgs.get(COUNT_KEY)); if (count == null) { return StringUtils.replace(s, toReplace, replaceWith); diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/RoundFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/RoundFilter.java index ff3ac56b3..1a8e88c6c 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/RoundFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/RoundFilter.java @@ -8,7 +8,8 @@ import com.hubspot.jinjava.interpret.JinjavaInterpreter; import java.math.BigDecimal; import java.math.RoundingMode; -import org.apache.commons.lang3.math.NumberUtils; +import java.util.Map; +import java.util.Objects; @JinjavaDoc( value = "Round the number to a given precision.", @@ -20,13 +21,13 @@ ), params = { @JinjavaParam( - value = "precision", - type = "number", + value = RoundFilter.PRECISION_KEY, + type = "int", defaultValue = "0", desc = "Specifies the precision of rounding" ), @JinjavaParam( - value = "method", + value = RoundFilter.METHOD_KEY, type = "enum common|ceil|floor", defaultValue = "common", desc = "Method of rounding: 'common' rounds either up or down, 'ceil' always rounds up, and 'floor' always rounds down." @@ -46,7 +47,9 @@ ) } ) -public class RoundFilter implements Filter { +public class RoundFilter extends AbstractFilter { + public static final String PRECISION_KEY = "precision"; + public static final String METHOD_KEY = "method"; @Override public String getName() { @@ -54,7 +57,11 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { if (var == null) { return null; } @@ -71,30 +78,30 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args) ); } - int precision = 0; - if (args.length > 0) { - precision = NumberUtils.toInt(args[0]); - } + int precision = (int) parsedArgs.get(PRECISION_KEY); - String method = "common"; - if (args.length > 1) { - method = args[1]; - } + RoundingMode roundingMode = (RoundingMode) parsedArgs.get(METHOD_KEY); - RoundingMode roundingMode; + return result.setScale(precision, roundingMode); + } - switch (method) { - case "ceil": - roundingMode = RoundingMode.CEILING; - break; - case "floor": - roundingMode = RoundingMode.FLOOR; - break; - case "common": - default: - roundingMode = RoundingMode.HALF_UP; + @Override + protected Object parseArg( + JinjavaInterpreter interpreter, + JinjavaParam jinjavaParamMetadata, + Object value + ) { + if (jinjavaParamMetadata.value().equals(METHOD_KEY)) { + switch (Objects.toString(value, null)) { + case "ceil": + return RoundingMode.CEILING; + case "floor": + return RoundingMode.FLOOR; + case "common": + default: + return RoundingMode.HALF_UP; + } } - - return result.setScale(precision, roundingMode); + return super.parseArg(interpreter, jinjavaParamMetadata, value); } } 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..37fb991e3 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java @@ -1,16 +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.JinjavaInterpreter; import com.hubspot.jinjava.lib.fn.Functions; -import com.hubspot.jinjava.objects.SafeString; import java.util.Map; -import java.util.Objects; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.jsoup.Jsoup; import org.jsoup.nodes.Document; @@ -24,18 +19,18 @@ input = @JinjavaParam(value = "html", desc = "HTML to truncate", required = true), params = { @JinjavaParam( - value = "length", - type = "number", + value = TruncateHtmlFilter.LENGTH_KEY, + type = "int", defaultValue = "255", desc = "Length at which to truncate text (HTML characters not included)" ), @JinjavaParam( - value = "end", + value = TruncateHtmlFilter.END_KEY, defaultValue = "...", desc = "The characters that will be added to indicate where the text was truncated" ), @JinjavaParam( - value = "breakword", + value = TruncateHtmlFilter.BREAKWORD_KEY, type = "boolean", defaultValue = "false", desc = "If set to true, text will be truncated in the middle of words" @@ -48,12 +43,10 @@ ) } ) -public class TruncateHtmlFilter implements AdvancedFilter { - private static final int DEFAULT_TRUNCATE_LENGTH = 255; - private static final String DEFAULT_END = "..."; - private static final String LENGTH_KEY = "length"; - private static final String END_KEY = "end"; - private static final String BREAKWORD_KEY = "breakword"; +public class TruncateHtmlFilter extends AbstractFilter implements AdvancedFilter { + public static final String LENGTH_KEY = "length"; + public static final String END_KEY = "end"; + public static final String BREAKWORD_KEY = "breakword"; @Override public String getName() { @@ -64,87 +57,21 @@ public String getName() { public Object filter( Object var, JinjavaInterpreter interpreter, - Object[] args, - Map kwargs + Map parsedArgs ) { - String length = null; - if (kwargs.containsKey(LENGTH_KEY)) { - length = Objects.toString(kwargs.get(LENGTH_KEY)); - } - String end = null; - if (kwargs.containsKey(END_KEY)) { - end = Objects.toString(kwargs.get(END_KEY)); - } - String breakword = null; - if (kwargs.containsKey(BREAKWORD_KEY)) { - breakword = Objects.toString(kwargs.get(BREAKWORD_KEY)); - } - - String[] newArgs = new String[3]; - for (int i = 0; i < args.length; i++) { - if (i >= newArgs.length) { - break; - } - newArgs[i] = Objects.toString(args[i]); - } - - if (length != null) { - newArgs[0] = length; - } - if (end != null) { - newArgs[1] = end; - } - if (breakword != null) { - newArgs[2] = breakword; - } - - if (var instanceof SafeString) { - return filter((SafeString) var, interpreter, newArgs); - } - - return filter(var, interpreter, newArgs); - } - - @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { - if (var instanceof String) { - int length = DEFAULT_TRUNCATE_LENGTH; - String ends = DEFAULT_END; - - if (args.length > 0) { - 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 - ); - } - } - - if (args.length > 1 && args[1] != null) { - ends = Objects.toString(args[1]); - } - - boolean killwords = false; - if (args.length > 2 && args[2] != null) { - killwords = BooleanUtils.toBoolean(args[2]); - } - - Document dom = Jsoup.parseBodyFragment((String) var); - ContentTruncatingNodeVisitor visitor = new ContentTruncatingNodeVisitor( - length, - ends, - killwords - ); - dom.select("body").traverse(visitor); - dom.select(".__deleteme").remove(); - - return dom.select("body").html(); - } - - return var; + int length = ((int) parsedArgs.get(LENGTH_KEY)); + String end = (String) parsedArgs.get(END_KEY); + boolean breakword = (boolean) parsedArgs.get(BREAKWORD_KEY); + Document dom = Jsoup.parseBodyFragment((String) var); + + ContentTruncatingNodeVisitor visitor = new ContentTruncatingNodeVisitor( + length, + end, + breakword + ); + dom.select("body").traverse(visitor); + dom.select(".__deleteme").remove(); + return dom.select("body").html(); } private static class ContentTruncatingNodeVisitor implements NodeVisitor { diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/UrlizeFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/UrlizeFilter.java index 3b5c3bda4..41e10d641 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/UrlizeFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/UrlizeFilter.java @@ -4,12 +4,11 @@ import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import java.util.Map; import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.math.NumberUtils; @JinjavaDoc( value = "Converts URLs in plain text into clickable links.", @@ -21,17 +20,21 @@ ), params = { @JinjavaParam( - value = "trim_url_limit", - type = "number", - desc = "Sets a character limit" + value = UrlizeFilter.TRIM_URL_LIMIT_KEY, + type = "int", + desc = "Sets a character limit", + defaultValue = "2147483647" ), @JinjavaParam( - value = "nofollow", + value = UrlizeFilter.NO_FOLLOW_KEY, type = "boolean", defaultValue = "False", desc = "Adds nofollow to generated link tag" ), - @JinjavaParam(value = "target", desc = "Adds target attr to generated link tag") + @JinjavaParam( + value = UrlizeFilter.TARGET_KEY, + desc = "Adds target attr to generated link tag" + ) }, snippets = { @JinjavaSnippet( @@ -44,7 +47,10 @@ ) } ) -public class UrlizeFilter implements Filter { +public class UrlizeFilter extends AbstractFilter implements Filter { + public static final String TRIM_URL_LIMIT_KEY = "trim_url_limit"; + public static final String NO_FOLLOW_KEY = "nofollow"; + public static final String TARGET_KEY = "target"; @Override public String getName() { @@ -52,26 +58,21 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { Matcher m = URL_RE.matcher(Objects.toString(var, "")); StringBuffer result = new StringBuffer(); - int trimUrlLimit = Integer.MAX_VALUE; - if (args.length > 0) { - trimUrlLimit = NumberUtils.toInt(args[0], Integer.MAX_VALUE); - } + int trimUrlLimit = ((int) parsedArgs.get(TRIM_URL_LIMIT_KEY)); String fmt = " 1) { - nofollow = BooleanUtils.toBoolean(args[1]); - } + boolean nofollow = (boolean) parsedArgs.get(NO_FOLLOW_KEY); - String target = ""; - if (args.length > 2) { - target = args[2]; - } + String target = (String) parsedArgs.get(TARGET_KEY); if (nofollow) { fmt += " rel=\"nofollow\""; diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilterTest.java index 5c1e98cbc..ac37d11c3 100644 --- a/src/test/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilterTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/filter/RegexReplaceFilterTest.java @@ -20,7 +20,7 @@ public void setup() { @Test public void expects2Args() { assertThatThrownBy(() -> filter.filter("foo", interpreter)) - .hasMessageContaining("requires 2 arguments"); + .hasMessageContaining("Argument named 'regex' is required but missing"); } @Test @@ -28,7 +28,7 @@ public void expectsNotNullArgs() { assertThatThrownBy( () -> filter.filter("foo", interpreter, new String[] { null, null }) ) - .hasMessageContaining("both a valid regex"); + .hasMessageContaining("Argument named 'regex' is required but missing"); } public void noopOnNullExpr() { diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/ReplaceFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/ReplaceFilterTest.java index 316366b12..9dad480ad 100644 --- a/src/test/java/com/hubspot/jinjava/lib/filter/ReplaceFilterTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/filter/ReplaceFilterTest.java @@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import com.hubspot.jinjava.BaseInterpretingTest; -import com.hubspot.jinjava.interpret.InterpretException; +import com.hubspot.jinjava.interpret.InvalidInputException; import com.hubspot.jinjava.objects.SafeString; import org.junit.Before; import org.junit.Test; @@ -16,7 +16,7 @@ public void setup() { filter = new ReplaceFilter(); } - @Test(expected = InterpretException.class) + @Test(expected = InvalidInputException.class) public void expectsAtLeast2Args() { filter.filter("foo", interpreter); } diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java index 3aafe1b19..094150052 100644 --- a/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java @@ -63,7 +63,7 @@ public void itTakesKwargs() { fixture("filter/truncatehtml/long-content-with-tags.html"), interpreter, new Object[] { "35" }, - ImmutableMap.of("breakwords", false) + ImmutableMap.of(TruncateHtmlFilter.BREAKWORD_KEY, false) ); assertThat(result) .isEqualTo( @@ -75,7 +75,7 @@ public void itTakesKwargs() { fixture("filter/truncatehtml/long-content-with-tags.html"), interpreter, new Object[] { "35" }, - ImmutableMap.of("end", "TEST") + ImmutableMap.of(TruncateHtmlFilter.END_KEY, "TEST") ); assertThat(result) .isEqualTo( From a571c8a631eee97026738842029e055ba61c448d Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Thu, 12 Nov 2020 22:22:58 +1100 Subject: [PATCH 3/7] join, slice, sort, split, sum filter migrated to AbstractFilter. join filter: `attr` renamed to `attribute` matching unit test --- .../jinjava/lib/filter/AbstractFilter.java | 1 - .../jinjava/lib/filter/JoinFilter.java | 25 ++++++------- .../jinjava/lib/filter/SliceFilter.java | 35 +++++++++---------- .../jinjava/lib/filter/SortFilter.java | 34 ++++++++++++------ .../jinjava/lib/filter/SplitFilter.java | 32 +++++++++-------- .../hubspot/jinjava/lib/filter/SumFilter.java | 25 ++++++------- 6 files changed, 81 insertions(+), 71 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java index 1ce929772..ee9dd67a9 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java @@ -26,7 +26,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.lang3.BooleanUtils; diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/JoinFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/JoinFilter.java index 210c3f5fe..7f27d6ded 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/JoinFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/JoinFilter.java @@ -12,6 +12,7 @@ import com.hubspot.jinjava.util.ForLoop; import com.hubspot.jinjava.util.LengthLimitingStringBuilder; import com.hubspot.jinjava.util.ObjectIterator; +import java.util.Map; import java.util.Objects; @JinjavaDoc( @@ -19,12 +20,12 @@ input = @JinjavaParam(value = "value", desc = "The values to join", required = true), params = { @JinjavaParam( - value = "d", + value = JoinFilter.SEPARATOR_PARAM, desc = "The separator string used to join the items", defaultValue = "(empty String)" ), @JinjavaParam( - value = "attr", + value = JoinFilter.ATTRIBUTE_PARAM, desc = "Optional dict object attribute to use in joining" ) }, @@ -37,7 +38,9 @@ ) } ) -public class JoinFilter implements Filter { +public class JoinFilter extends AbstractFilter implements Filter { + public static final String SEPARATOR_PARAM = "d"; + public static final String ATTRIBUTE_PARAM = "attribute"; @Override public String getName() { @@ -45,20 +48,18 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { LengthLimitingStringBuilder stringBuilder = new LengthLimitingStringBuilder( interpreter.getConfig().getMaxStringLength() ); - String separator = ""; - if (args.length > 0) { - separator = args[0]; - } + String separator = (String) parsedArgs.get(SEPARATOR_PARAM); - String attr = null; - if (args.length > 1) { - attr = args[1]; - } + String attr = (String) parsedArgs.get(ATTRIBUTE_PARAM); ForLoop loop = ObjectIterator.getLoop(var); boolean first = true; diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/SliceFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/SliceFilter.java index 7f654b00b..6f8f00a77 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/SliceFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/SliceFilter.java @@ -6,12 +6,11 @@ import com.hubspot.jinjava.interpret.InvalidArgumentException; import com.hubspot.jinjava.interpret.InvalidReason; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import com.hubspot.jinjava.interpret.TemplateSyntaxException; import com.hubspot.jinjava.util.ForLoop; import com.hubspot.jinjava.util.ObjectIterator; import java.util.ArrayList; import java.util.List; -import org.apache.commons.lang3.math.NumberUtils; +import java.util.Map; @JinjavaDoc( value = "Slice an iterator and return a list of lists containing those items.", @@ -23,13 +22,13 @@ ), params = { @JinjavaParam( - value = "slices", - type = "number", + value = SliceFilter.SLICES_PARAM, + type = "int", desc = "Specifies how many items will be sliced", required = true ), @JinjavaParam( - value = "fillWith", + value = SliceFilter.FILL_WITH_PARAM, type = "object", desc = "Specifies which object to use to fill missing values on final iteration", required = false @@ -51,7 +50,9 @@ ) } ) -public class SliceFilter implements Filter { +public class SliceFilter extends AbstractFilter implements Filter { + public static final String SLICES_PARAM = "slices"; + public static final String FILL_WITH_PARAM = "fillWith"; @Override public String getName() { @@ -59,25 +60,22 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { ForLoop loop = ObjectIterator.getLoop(var); - if (args.length < 1) { - throw new TemplateSyntaxException( - interpreter, - getName(), - "requires 1 argument (number of slices)" - ); - } - - int slices = NumberUtils.toInt(args[0], 3); + int slices = (int) parsedArgs.get(SLICES_PARAM); + Object fillWith = parsedArgs.get(FILL_WITH_PARAM); if (slices <= 0) { throw new InvalidArgumentException( interpreter, this, InvalidReason.POSITIVE_NUMBER, 0, - args[0] + slices ); } List> result = new ArrayList<>(); @@ -94,8 +92,7 @@ public Object filter(Object var, JinjavaInterpreter interpreter, String... args) i++; } - if (args.length > 1 && currentList != null) { - Object fillWith = args[1]; + if (fillWith != null && currentList != null) { while (currentList.size() < slices) { currentList.add(fillWith); } diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/SortFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/SortFilter.java index ecdbb37ab..20264c1bd 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/SortFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/SortFilter.java @@ -15,8 +15,9 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; -import org.apache.commons.lang3.BooleanUtils; +import org.apache.commons.lang3.StringUtils; @JinjavaDoc( value = "Sort an iterable.", @@ -28,18 +29,21 @@ ), params = { @JinjavaParam( - value = "reverse", + value = SortFilter.REVERSE_PARAM, type = "boolean", defaultValue = "False", desc = "Boolean to reverse the sort order" ), @JinjavaParam( - value = "case_sensitive", + value = SortFilter.CASE_SENSITIVE_PARAM, type = "boolean", defaultValue = "False", desc = "Determines whether or not the sorting is case sensitive" ), - @JinjavaParam(value = "attribute", desc = "Specifies an attribute to sort by") + @JinjavaParam( + value = SortFilter.ATTRIBUTE_PARAM, + desc = "Specifies an attribute to sort by" + ) }, snippets = { @JinjavaSnippet( @@ -54,9 +58,12 @@ ) } ) -public class SortFilter implements Filter { +public class SortFilter extends AbstractFilter implements Filter { private static final Splitter DOT_SPLITTER = Splitter.on('.').omitEmptyStrings(); private static final Joiner DOT_JOINER = Joiner.on('.'); + public static final String REVERSE_PARAM = "reverse"; + public static final String CASE_SENSITIVE_PARAM = "case_sensitive"; + public static final String ATTRIBUTE_PARAM = "attribute"; @Override public String getName() { @@ -64,20 +71,25 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { if (var == null) { return null; } - boolean reverse = args.length > 0 && BooleanUtils.toBoolean(args[0]); - boolean caseSensitive = args.length > 1 && BooleanUtils.toBoolean(args[1]); + boolean reverse = (boolean) parsedArgs.get(REVERSE_PARAM); + boolean caseSensitive = (boolean) parsedArgs.get(CASE_SENSITIVE_PARAM); + String attribute = (String) parsedArgs.get(ATTRIBUTE_PARAM); - if (args.length > 2 && args[2] == null) { + if (parsedArgs.containsKey(ATTRIBUTE_PARAM) && attribute == null) { throw new InvalidArgumentException(interpreter, this, InvalidReason.NULL, 2); } - List attr = args.length > 2 - ? DOT_SPLITTER.splitToList(args[2]) + List attr = StringUtils.isNotEmpty(attribute) + ? DOT_SPLITTER.splitToList(attribute) : Collections.emptyList(); return Lists .newArrayList(ObjectIterator.getLoop(var)) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/SplitFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/SplitFilter.java index f1b81c245..abd39885d 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/SplitFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/SplitFilter.java @@ -7,8 +7,8 @@ import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import java.util.Map; import java.util.Objects; -import org.apache.commons.lang3.math.NumberUtils; /** * split(separator=' ', limit=0) @@ -24,13 +24,13 @@ input = @JinjavaParam(value = "string", desc = "The string to split", required = true), params = { @JinjavaParam( - value = "separator", + value = SplitFilter.SEPARATOR_PARAM, defaultValue = " ", desc = "Specifies the separator to split the variable by" ), @JinjavaParam( - value = "limit", - type = "number", + value = SplitFilter.LIMIT_PARAM, + type = "int", defaultValue = "0", desc = "Limits resulting list by putting remainder of string into last list item" ) @@ -47,7 +47,9 @@ ) } ) -public class SplitFilter implements Filter { +public class SplitFilter extends AbstractFilter implements Filter { + public static final String SEPARATOR_PARAM = "separator"; + public static final String LIMIT_PARAM = "limit"; @Override public String getName() { @@ -55,20 +57,22 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { + String separator = (String) parsedArgs.get(SEPARATOR_PARAM); Splitter splitter; - - if (args.length > 0) { - splitter = Splitter.on(args[0]); + if (separator != null) { + splitter = Splitter.on(separator); } else { splitter = Splitter.on(CharMatcher.whitespace()); } - if (args.length > 1) { - int limit = NumberUtils.toInt(args[1], 0); - if (limit > 0) { - splitter = splitter.limit(limit); - } + int limit = (Integer) parsedArgs.get(LIMIT_PARAM); + if (limit > 0) { + splitter = splitter.limit(limit); } return Lists.newArrayList( diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/SumFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/SumFilter.java index 56c5373a9..46b07cd86 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/SumFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/SumFilter.java @@ -20,13 +20,13 @@ ), params = { @JinjavaParam( - value = "start", + value = SumFilter.START_PARAM, type = "number", defaultValue = "0", desc = "Sets a value to return, if there is nothing in the variable to sum" ), @JinjavaParam( - value = "attribute", + value = SumFilter.ATTRIBUTE_PARAM, desc = "Specify an optional attribute of dict to sum" ) }, @@ -40,7 +40,9 @@ ) } ) -public class SumFilter implements AdvancedFilter { +public class SumFilter extends AbstractFilter implements AdvancedFilter { + public static final String START_PARAM = "start"; + public static final String ATTRIBUTE_PARAM = "attribute"; @Override public String getName() { @@ -51,21 +53,16 @@ public String getName() { public Object filter( Object var, JinjavaInterpreter interpreter, - Object[] args, - Map kwargs + Map parsedArgs ) { ForLoop loop = ObjectIterator.getLoop(var); - BigDecimal sum = BigDecimal.ZERO; - String attr = kwargs.containsKey("attribute") - ? kwargs.get("attribute").toString() - : null; + Number start = (Number) parsedArgs.get(START_PARAM); + String attr = (String) parsedArgs.get(ATTRIBUTE_PARAM); - if (args.length > 0) { - try { - sum = sum.add(new BigDecimal(args[0].toString())); - } catch (NumberFormatException ignored) {} - } + BigDecimal sum = start instanceof BigDecimal + ? (BigDecimal) start + : new BigDecimal(Objects.toString(start.toString(), "0")); while (loop.hasNext()) { Object val = loop.next(); From e5b850bf91634ad6eda9ae9f4b53b9c4069d57ce Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Fri, 13 Nov 2020 00:09:36 +1100 Subject: [PATCH 4/7] datetimeformat, truncate: upgrade to abstract. datetimeformat: fix function jinjavadoc and param defaults --- .../lib/filter/DateTimeFormatFilter.java | 31 +++++++++++++------ .../jinjava/lib/filter/TruncateFilter.java | 25 ++++++++++----- .../com/hubspot/jinjava/lib/fn/Functions.java | 5 +++ 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/DateTimeFormatFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/DateTimeFormatFilter.java index 7aa50f664..1262ec1ec 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/DateTimeFormatFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/DateTimeFormatFilter.java @@ -6,6 +6,7 @@ import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.lib.fn.Functions; import com.hubspot.jinjava.objects.date.StrftimeFormatter; +import java.util.Map; @JinjavaDoc( value = "Formats a date object", @@ -17,19 +18,19 @@ ), params = { @JinjavaParam( - value = "format", + value = DateTimeFormatFilter.FORMAT_PARAM, defaultValue = StrftimeFormatter.DEFAULT_DATE_FORMAT, desc = "The format of the date determined by the directives added to this parameter" ), @JinjavaParam( - value = "timezone", - defaultValue = "utc", + value = DateTimeFormatFilter.TIMEZONE_PARAM, + defaultValue = "UTC", desc = "Time zone of output date" ), @JinjavaParam( - value = "locale", + value = DateTimeFormatFilter.LOCALE_PARAM, type = "string", - defaultValue = "us", + defaultValue = "en-US", desc = "The language code to use when formatting the datetime" ) }, @@ -40,7 +41,10 @@ ) } ) -public class DateTimeFormatFilter implements Filter { +public class DateTimeFormatFilter extends AbstractFilter implements Filter { + public static final String FORMAT_PARAM = "format"; + public static final String TIMEZONE_PARAM = "timezone"; + public static final String LOCALE_PARAM = "locale"; @Override public String getName() { @@ -48,11 +52,18 @@ public String getName() { } @Override - public Object filter(Object var, JinjavaInterpreter interpreter, String... args) { - if (args.length > 0) { - return Functions.dateTimeFormat(var, args); - } else { + public Object filter( + Object var, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { + String format = (String) parsedArgs.get(FORMAT_PARAM); + String timezone = (String) parsedArgs.get(TIMEZONE_PARAM); + String locale = (String) parsedArgs.get(LOCALE_PARAM); + if (format == null && timezone == null && locale == null) { return Functions.dateTimeFormat(var); + } else { + return Functions.dateTimeFormat(var, format, timezone, locale); } } } diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/TruncateFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/TruncateFilter.java index 9b008855b..197c5065d 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/TruncateFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/TruncateFilter.java @@ -20,6 +20,7 @@ import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.interpret.JinjavaInterpreter; import com.hubspot.jinjava.lib.fn.Functions; +import java.util.Map; @JinjavaDoc( value = "Return a truncated copy of the string. The length is specified with the first parameter which defaults to 255. " + @@ -33,19 +34,19 @@ ), params = { @JinjavaParam( - value = "length", - type = "number", + value = TruncateFilter.LENGTH_PARAM, + type = "int", defaultValue = "255", desc = "Specifies the length at which to truncate the text (includes HTML characters)" ), @JinjavaParam( - value = "killwords", + value = TruncateFilter.KILLWORDS_PARAM, type = "boolean", defaultValue = "False", desc = "If true, the string will cut text at length" ), @JinjavaParam( - value = "end", + value = TruncateFilter.END_PARAM, defaultValue = "...", desc = "The characters that will be added to indicate where the text was truncated" ) @@ -61,11 +62,21 @@ ) } ) -public class TruncateFilter implements Filter { +public class TruncateFilter extends AbstractFilter implements Filter { + public static final String LENGTH_PARAM = "length"; + public static final String KILLWORDS_PARAM = "killwords"; + public static final String END_PARAM = "end"; @Override - public Object filter(Object object, JinjavaInterpreter interpreter, String... arg) { - return Functions.truncate(object, arg); + public Object filter( + Object object, + JinjavaInterpreter interpreter, + Map parsedArgs + ) { + int length = (int) parsedArgs.get(LENGTH_PARAM); + boolean killwords = (boolean) parsedArgs.get(KILLWORDS_PARAM); + String end = (String) parsedArgs.get(END_PARAM); + return Functions.truncate(object, length, killwords, end); } @Override 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 5bc5746e3..c2c40c3ef 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/Functions.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/Functions.java @@ -105,6 +105,11 @@ public static ZonedDateTime today(String... var) { value = "timezone", defaultValue = "utc", desc = "Time zone of output date" + ), + @JinjavaParam( + value = "locale", + defaultValue = "us", + desc = "The language code to use when formatting the datetime" ) } ) From f1df1b3355e096a0ee3874b27e3787efdd1bc139 Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Sun, 15 Nov 2020 19:31:36 +1100 Subject: [PATCH 5/7] AbstractFilter: pre-parse default values. Validate input numbers instead of returning default 0 --- .../jinjava/lib/filter/AbstractFilter.java | 83 ++++++++++++------- .../lib/filter/AbstractFilterTest.java | 18 +++- 2 files changed, 70 insertions(+), 31 deletions(-) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java index ee9dd67a9..574956f0d 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java @@ -20,14 +20,15 @@ import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.interpret.InvalidInputException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import java.math.BigDecimal; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; +import javax.annotation.Nullable; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.math.NumberUtils; @@ -118,15 +119,11 @@ public Object filter( } protected Object parseArg( - JinjavaInterpreter interpreter, + @Nullable JinjavaInterpreter interpreter, JinjavaParam jinjavaParamMetadata, Object value ) { - if ( - jinjavaParamMetadata.type() == null || - value == null || - Arrays.asList("object", "dict", "sequence").contains(jinjavaParamMetadata.type()) - ) { + if (jinjavaParamMetadata.type() == null || value == null) { return value; } switch (jinjavaParamMetadata.type().toLowerCase()) { @@ -135,38 +132,64 @@ protected Object parseArg( ? (Boolean) value : BooleanUtils.toBooleanObject(value.toString()); case "int": - return value instanceof Number - ? ((Number) value).intValue() - : NumberUtils.toInt(value.toString()); + return parseNumber(value, int.class); case "long": - return value instanceof Number - ? ((Number) value).longValue() - : NumberUtils.toLong(value.toString()); + return parseNumber(value, long.class); case "float": - return value instanceof Number - ? ((Number) value).floatValue() - : NumberUtils.toFloat(value.toString()); + return parseNumber(value, float.class); case "double": - return value instanceof Number - ? ((Number) value).doubleValue() - : NumberUtils.toDouble(value.toString()); + return parseNumber(value, double.class); case "number": - return value instanceof Number - ? (Number) value - : new BigDecimal(value.toString()); + return parseNumber(value, Number.class); case "string": return value.toString(); + case "object": + case "dict": + case "sequence": + return value; default: - throw new InvalidInputException( - interpreter, - "INVALID_ARG_NAME", + String errorMessage = String.format( + "Argument named '%s' with value '%s' cannot be parsed for filter '%s'", + jinjavaParamMetadata.value(), + value, + getName() + ); + if (interpreter != null) { //Filter runtime vs init + throw new InvalidInputException(interpreter, "INVALID_ARG_NAME", errorMessage); + } else { + throw new IllegalArgumentException(errorMessage); + } + } + } + + public Number parseNumber(Object value, Class numberClass) { + //check if needs to be parsed to number first, and then convert to required type + if (!(value instanceof Number)) { + String str = Objects.toString(value, null); + if (NumberUtils.isCreatable(str)) { + value = NumberUtils.createNumber(str); + } else { + throw new IllegalArgumentException( String.format( - "Argument named '%s' with value '%s' cannot be parsed for filter %s", - jinjavaParamMetadata.value(), + "Input '%s' is not parsable type of '%s' for filter '%s'", value, + numberClass, getName() ) ); + } + } + Number n = (Number) value; + if (numberClass == int.class || numberClass == Integer.class) { + return n.intValue(); + } else if (numberClass == long.class || numberClass == Long.class) { + return n.longValue(); + } else if (numberClass == float.class || numberClass == Float.class) { + return n.floatValue(); + } else if (numberClass == double.class || numberClass == Double.class) { + return n.doubleValue(); + } else { //if nclass == Number.class + return n; } } @@ -245,7 +268,11 @@ public Map initDefaultValues() { .stream() .filter(e -> StringUtils.isNotEmpty(e.getValue().defaultValue())) .collect( - ImmutableMap.toImmutableMap(Map.Entry::getKey, e -> e.getValue().defaultValue()) + // ImmutableMap.toImmutableMap(Map.Entry::getKey, e -> e.getValue().defaultValue()) + ImmutableMap.toImmutableMap( + Map.Entry::getKey, + e -> parseArg(null, e.getValue(), e.getValue().defaultValue()) + ) ); } } diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/AbstractFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/AbstractFilterTest.java index 35a45bb43..ca060b279 100644 --- a/src/test/java/com/hubspot/jinjava/lib/filter/AbstractFilterTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/filter/AbstractFilterTest.java @@ -10,7 +10,6 @@ import com.hubspot.jinjava.interpret.JinjavaInterpreter; import java.math.BigDecimal; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import org.junit.Test; @@ -81,7 +80,7 @@ public void itParsesNumericAndBooleanInput() { .put("long", "2") .put("double", "3") .put("float", "4") - .put("number", "5") + .put("number", "55555555555555555555.555555555555555555") .put("object", new Object()) .put("dict", new Object()) .build(); @@ -95,7 +94,7 @@ public void itParsesNumericAndBooleanInput() { .put("long", 2L) .put("double", 3.0) .put("float", 4.0f) - .put("number", new BigDecimal(5)) + .put("number", new BigDecimal("55555555555555555555.555555555555555555")) .put("object", kwArgs.get("object")) .put("dict", kwArgs.get("dict")) .build(); @@ -146,6 +145,19 @@ public void itErrorsUnknownNamedArg() { .hasMessageContaining("Argument named 'unknown' is invalid"); } + @JinjavaDoc( + params = { + @JinjavaParam(value = "int", type = "int", desc = "int", defaultValue = "?") + } + ) + public static class InvalidDefaultValueFilter extends ArgCapturingFilter {} + + @Test + public void itErrorsInitOnInvalidDefaultValue() { + assertThatThrownBy(() -> new InvalidDefaultValueFilter()) + .hasMessageContaining("Input '?' is not parsable type of 'int' for filter"); + } + public static class ArgCapturingFilter extends AbstractFilter { public Map parsedArgs; From 7ae931d86995119cc3b017cbbc64b4993163cce3 Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Sun, 15 Nov 2020 19:46:26 +1100 Subject: [PATCH 6/7] AbstractFilter: pre-parse default values. Validate input numbers instead of returning default 0. Expose defaultValue for parameter name --- .../java/com/hubspot/jinjava/lib/filter/AbstractFilter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java index 574956f0d..3ded9ac8a 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/AbstractFilter.java @@ -239,6 +239,10 @@ public String getIndexedArgumentName(int position) { .orElse(null); } + public Object getDefaultValue(String argName) { + return defaultValues.get(argName); + } + public Map initNamedArguments() { JinjavaDoc jinjavaDoc = this.getClass().getAnnotation(JinjavaDoc.class); if (jinjavaDoc != null) { From 8de2f0965bf36da2221e6680c44e1ab604b94b12 Mon Sep 17 00:00:00 2001 From: Michael Protopopov Date: Sun, 15 Nov 2020 19:46:56 +1100 Subject: [PATCH 7/7] truncatehtml: add param defaulting as per review comments --- .../jinjava/lib/filter/TruncateHtmlFilter.java | 16 ++++++++++++++++ .../lib/filter/TruncateHtmlFilterTest.java | 15 +++++++++++++++ 2 files changed, 31 insertions(+) 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 37fb991e3..3b4ea1c14 100644 --- a/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java +++ b/src/main/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilter.java @@ -74,6 +74,22 @@ public Object filter( return dom.select("body").html(); } + @Override + protected Object parseArg( + JinjavaInterpreter interpreter, + JinjavaParam jinjavaParamMetadata, + Object value + ) { + if (jinjavaParamMetadata.value().equals(LENGTH_KEY) && interpreter != null) { + try { + return super.parseArg(interpreter, jinjavaParamMetadata, value); + } catch (Exception e) { + return getDefaultValue(LENGTH_KEY); + } + } + return super.parseArg(interpreter, jinjavaParamMetadata, value); + } + private static class ContentTruncatingNodeVisitor implements NodeVisitor { private int maxTextLen; private int textLen; diff --git a/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java b/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java index 094150052..1a2aa8f0f 100644 --- a/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/filter/TruncateHtmlFilterTest.java @@ -83,6 +83,21 @@ public void itTakesKwargs() { ); } + @Test + public void itDefaultsLengthWhenCannotBeParsed() { + String result = (String) filter.filter( + fixture("filter/truncatehtml/long-content-with-tags.html"), + interpreter, + new Object[] { "?" }, + ImmutableMap.of(TruncateHtmlFilter.BREAKWORD_KEY, false) + ); + assertThat(result) + .isEqualTo( + "

HTML Ipsum Presents

\n" + + "

Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum tortor quam, feugiat vitae, ultricies eget, tempor sit amet, ante. Donec eu libero sit amet quam egestas semper. Aenean ultricies...

" + ); + } + private static String fixture(String name) { try { return Resources.toString(Resources.getResource(name), StandardCharsets.UTF_8);