From 11ea8be0626d0d8de285ca73b779b074437194e2 Mon Sep 17 00:00:00 2001 From: Jose Martin Rozanec Date: Thu, 26 May 2016 23:40:51 -0300 Subject: [PATCH] Issue #89: regression - NumberFormatException: For input string: '$' not matching original message. --- .../java/com/cronutils/StringValidations.java | 77 +++++++++++++++++++ .../ValidationFieldExpressionVisitor.java | 70 +++-------------- .../com/cronutils/parser/FieldParser.java | 4 +- .../CronParserQuartzIntegrationTest.java | 15 ++++ 4 files changed, 104 insertions(+), 62 deletions(-) create mode 100644 src/main/java/com/cronutils/StringValidations.java diff --git a/src/main/java/com/cronutils/StringValidations.java b/src/main/java/com/cronutils/StringValidations.java new file mode 100644 index 00000000..a87df1ea --- /dev/null +++ b/src/main/java/com/cronutils/StringValidations.java @@ -0,0 +1,77 @@ +/* + * Copyright 2014 jmrozanec + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.cronutils; + +import com.cronutils.model.field.constraint.FieldConstraints; +import com.cronutils.model.field.value.SpecialChar; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Sets; + +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class StringValidations { + private FieldConstraints constraints; + private Pattern stringToIntKeysPattern; + private Pattern numsAndCharsPattern; + private Pattern lwPattern; + private boolean strictRanges; + + public StringValidations(FieldConstraints constraints){ + this.lwPattern = buildLWPattern(constraints.getSpecialChars()); + this.stringToIntKeysPattern = buildStringToIntPattern(constraints.getStringMapping().keySet()); + this.numsAndCharsPattern = Pattern.compile("[#\\?/\\*0-9]"); + } + + @VisibleForTesting + Pattern buildStringToIntPattern(Set strings){ + return buildWordsPattern(strings); + } + + @VisibleForTesting + public String removeValidChars(String exp){ + exp = exp.toUpperCase(); + Matcher numsAndCharsMatcher = numsAndCharsPattern.matcher(exp); + Matcher stringToIntKeysMatcher = stringToIntKeysPattern.matcher(numsAndCharsMatcher.replaceAll("")); + Matcher specialWordsMatcher = lwPattern.matcher(stringToIntKeysMatcher.replaceAll("")); + return specialWordsMatcher.replaceAll("").replaceAll("\\s+", "").replaceAll(",", "").replaceAll("-", ""); + } + + @VisibleForTesting + Pattern buildLWPattern(Set specialChars){ + Set scs = Sets.newHashSet(); + for(SpecialChar sc : new SpecialChar[]{SpecialChar.L, SpecialChar.LW, SpecialChar.W}){ + if(specialChars.contains(sc)){ + scs.add(sc.name()); + } + } + return buildWordsPattern(scs); + } + + @VisibleForTesting + Pattern buildWordsPattern(Set words){ + StringBuilder builder = new StringBuilder("\\b("); + boolean first = true; + for(String word : words){ + if(!first){ + builder.append("|"); + }else{ + first=false; + } + builder.append(word); + } + builder.append(")\\b"); + return Pattern.compile(builder.toString()); + } +} diff --git a/src/main/java/com/cronutils/model/field/expression/visitor/ValidationFieldExpressionVisitor.java b/src/main/java/com/cronutils/model/field/expression/visitor/ValidationFieldExpressionVisitor.java index 7f25a843..17e86500 100644 --- a/src/main/java/com/cronutils/model/field/expression/visitor/ValidationFieldExpressionVisitor.java +++ b/src/main/java/com/cronutils/model/field/expression/visitor/ValidationFieldExpressionVisitor.java @@ -12,6 +12,7 @@ */ package com.cronutils.model.field.expression.visitor; +import com.cronutils.StringValidations; import com.cronutils.model.field.value.FieldValue; import com.cronutils.model.field.value.IntegerFieldValue; import com.cronutils.model.field.value.SpecialChar; @@ -19,31 +20,22 @@ import com.cronutils.model.field.expression.*; import com.cronutils.model.field.value.SpecialCharFieldValue; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Sets; - -import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; public class ValidationFieldExpressionVisitor implements FieldExpressionVisitor { private static final String OORANGE = "Value %s not in range [%s, %s]"; private FieldConstraints constraints; - private Pattern stringToIntKeysPattern; - private Pattern numsAndCharsPattern; - private Pattern lwPattern; + private StringValidations stringValidations; private boolean strictRanges; public ValidationFieldExpressionVisitor(FieldConstraints constraints, boolean strictRanges){ this.constraints = constraints; - this.lwPattern = buildLWPattern(constraints.getSpecialChars()); - this.stringToIntKeysPattern = buildStringToIntPattern(constraints.getStringMapping().keySet()); - this.numsAndCharsPattern = Pattern.compile("[#\\?/\\*0-9]"); + this.stringValidations = new StringValidations(constraints); this.strictRanges = strictRanges; } @Override public FieldExpression visit(FieldExpression expression) { - String unsupportedChars = removeValidChars(expression.asString()).toUpperCase(); + String unsupportedChars = stringValidations.removeValidChars(expression.asString()); if("".equals(unsupportedChars)){ if(expression instanceof Always){ return visit((Always)expression); @@ -64,7 +56,7 @@ public FieldExpression visit(FieldExpression expression) { return visit((QuestionMark)expression); } } - throw new RuntimeException(String.format("Expression contains unsupported chars: %s", unsupportedChars)); + throw new IllegalArgumentException(String.format("Invalid chars in expression! Expression: %s Invalid chars: %s", expression.asString(), unsupportedChars)); } @Override @@ -124,50 +116,12 @@ public QuestionMark visit(QuestionMark questionMark) { return questionMark; } - @VisibleForTesting - Pattern buildStringToIntPattern(Set strings){ - return buildWordsPattern(strings); - } - - @VisibleForTesting - String removeValidChars(String exp){ - Matcher numsAndCharsMatcher = numsAndCharsPattern.matcher(exp); - Matcher stringToIntKeysMatcher = stringToIntKeysPattern.matcher(numsAndCharsMatcher.replaceAll("")); - Matcher specialWordsMatcher = lwPattern.matcher(stringToIntKeysMatcher.replaceAll("")); - return specialWordsMatcher.replaceAll("").replaceAll("\\s+", "").replaceAll(",", "").replaceAll("-", ""); - } - - @VisibleForTesting - Pattern buildLWPattern(Set specialChars){ - Set scs = Sets.newHashSet(); - for(SpecialChar sc : new SpecialChar[]{SpecialChar.L, SpecialChar.LW, SpecialChar.W}){ - if(specialChars.contains(sc)){ - scs.add(sc.name()); - } - } - return buildWordsPattern(scs); - } - @VisibleForTesting - Pattern buildWordsPattern(Set words){ - StringBuilder builder = new StringBuilder("\\b("); - boolean first = true; - for(String word : words){ - if(!first){ - builder.append("|"); - }else{ - first=false; - } - builder.append(word); - } - builder.append(")\\b"); - return Pattern.compile(builder.toString()); - } /** * Check if given number is greater or equal to start range and minor or equal to end range * @param fieldValue - to be validated - * @throws - RuntimeException if not in range + * @throws IllegalArgumentException - if not in range */ @VisibleForTesting void isInRange(FieldValue fieldValue) { @@ -181,16 +135,10 @@ void isInRange(FieldValue fieldValue) { @VisibleForTesting boolean isDefault(FieldValue fieldValue) { - if(fieldValue instanceof IntegerFieldValue){ - return ((IntegerFieldValue)fieldValue).getValue()==-1; - } - return false; + return fieldValue instanceof IntegerFieldValue && ((IntegerFieldValue) fieldValue).getValue() == -1; } - boolean isSpecialCharNotL(FieldValue fieldValue){ - if(fieldValue instanceof SpecialCharFieldValue){ - return !SpecialChar.L.equals(fieldValue.getValue()); - } - return false; + boolean isSpecialCharNotL(FieldValue fieldValue) { + return fieldValue instanceof SpecialCharFieldValue && !SpecialChar.L.equals(fieldValue.getValue()); } } diff --git a/src/main/java/com/cronutils/parser/FieldParser.java b/src/main/java/com/cronutils/parser/FieldParser.java index 97a5b7e1..34a62a9a 100644 --- a/src/main/java/com/cronutils/parser/FieldParser.java +++ b/src/main/java/com/cronutils/parser/FieldParser.java @@ -12,6 +12,7 @@ */ package com.cronutils.parser; +import com.cronutils.StringValidations; import com.cronutils.model.field.constraint.FieldConstraints; import com.cronutils.model.field.expression.*; import com.cronutils.model.field.value.FieldValue; @@ -206,7 +207,8 @@ int stringToInt(String exp) { try{ return Integer.parseInt(exp); }catch (NumberFormatException e){ - throw new IllegalArgumentException(e); + String invalidChars = new StringValidations(fieldConstraints).removeValidChars(exp); + throw new IllegalArgumentException(String.format("Invalid chars in expression! Expression: %s Invalid chars: %s", exp, invalidChars)); } } } diff --git a/src/test/java/com/cronutils/parser/CronParserQuartzIntegrationTest.java b/src/test/java/com/cronutils/parser/CronParserQuartzIntegrationTest.java index ffef6154..877b30b1 100644 --- a/src/test/java/com/cronutils/parser/CronParserQuartzIntegrationTest.java +++ b/src/test/java/com/cronutils/parser/CronParserQuartzIntegrationTest.java @@ -210,6 +210,21 @@ public void testIntervalMinutes() { Assert.assertEquals(assertDate, lastExecution); } + /** + * Issue #89: regression - NumberFormatException: For input string: "$" + */ + @Test + public void testRegressionDifferentMessageForException(){ + boolean asserted = false; + try{ + ExecutionTime.forCron(parser.parse("* * * * $ ?")); + }catch (IllegalArgumentException e){ + assertEquals("Invalid chars in expression! Expression: $ Invalid chars: $", e.getMessage()); + asserted = true; + } + assertTrue(asserted); + } + /** * Issue #90: Reported error contains other expression than the one provided */