provide basic parameter parsing validation.

It would be nice, but not included here, if the parameters were checked against:
- supported parameter types
- actual project parameter names

Author: jwmach1

src/main/java/org/jenkinsci/plugins/parameterizedschedular/ParameterParser.java

@@ -2,23 +2,50 @@
 
 import java.util.Map;
 
+import org.apache.commons.lang.StringUtils;
+
+import com.google.common.base.Splitter;
 import com.google.common.collect.Maps;
 
 public class ParameterParser {
+	/**
+	 * if ever changed, documentation and messages will need to be updated as well
+	 */
+	private static final String PARAMETER_SEPARATOR = "%";
+	private static final String NAME_VALUE_SEPARATOR = "=";
+	private static final String PAIR_SEPARATOR = ";";
+
+	/**
+	 * 
+	 * @param nameValuePairFormattedString of name=value;other=value name value pairs
+	 * @return
+	 */
+	public Map<String, String> parse(String nameValuePairFormattedString) {
+		if (StringUtils.isBlank(nameValuePairFormattedString)) {
+			return Maps.<String, String> newHashMap();
+		}
+		String clean = nameValuePairFormattedString.trim();
+		if (nameValuePairFormattedString.endsWith(PAIR_SEPARATOR)) {
+			//the default splitter message in this scenario is not user friendly, so snip a trailing semicolon
+			clean = clean.substring(0, clean.length() - 1);
+		}
+		return Splitter.on(PAIR_SEPARATOR).withKeyValueSeparator(NAME_VALUE_SEPARATOR).split(clean);
+	}
 
-	public Map<String, String> parse(String formattedString) {
-		Map<String, String> result = Maps.newHashMap();
-		if (formattedString == null) {
-			return result;
+	public String checkSanity(String cronTabSpec) {
+		String[] split = cronTabSpec.split(PARAMETER_SEPARATOR);
+		if (split.length < 2) {
+			return null;
 		}
-		String[] parameters = formattedString.split(";");
-		for (String parameter : parameters) {
-			String[] keyvalue = parameter.split("=");
-			if (keyvalue.length == 2) {
-				result.put(keyvalue[0].trim(), keyvalue[1].trim());
-			}
+		if (split.length > 2) {
+			return Messages.ParameterizedTimerTrigger_MoreThanOnePercent();
 		}
 
-		return result;
+		try {
+			parse(split[1]);
+		} catch (IllegalArgumentException e) {
+			return e.getMessage();
+		}
+		return null;
 	}
 }

src/main/java/org/jenkinsci/plugins/parameterizedschedular/ParameterizedTimerTrigger.java

@@ -129,11 +129,18 @@ public FormValidation doCheckParameterizedSpecification(@QueryParameter String v
 				if (msg != null) {
 					return FormValidation.warning(msg);
 				}
+				msg = new ParameterParser().checkSanity(value);
+				if (msg != null) {
+					return FormValidation.warning(msg);
+				}
+
 				return FormValidation.ok();
 			} catch (ANTLRException e) {
 				if (value.trim().indexOf('\n') == -1 && value.contains("**"))
 					return FormValidation.error(Messages.ParameterizedTimerTrigger_MissingWhitespace());
 				return FormValidation.error(e.getMessage());
+			} catch (IllegalArgumentException e) {
+				return FormValidation.error(e.getMessage());
 			}
 		}
 	}

src/main/resources/org/jenkinsci/plugins/parameterizedschedular/Messages.properties

@@ -1,3 +1,5 @@
 ParameterizedTimerTrigger.DisplayName=Build periodically with parameters
 ParameterizedTimerTrigger.MissingWhitespace=You appear to be missing whitespace between * and *.
+ParameterizedTimerTrigger.MoreThanOnePercent=You can only use one percent sign to separate the cron from the parameters.
+ParameterizedTimerTrigger.TrailingSemicolon=I need you to remove that semicolon from the end of the line.
 ParameterizedTimerTrigger.TimerTriggerCause.ShortDescription=Started by timer with parameters:

src/test/java/org/jenkinsci/plugins/parameterizedschedular/ParameterParserTest.java

@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.parameterizedschedular;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 import java.util.HashMap;
 
@@ -20,15 +21,17 @@ public void test_EmptyStringReturns_emptyMap() {
 		ParameterParser testObject = new ParameterParser();
 
 		assertEquals(new HashMap<String, String>(), testObject.parse(""));
+		assertEquals(new HashMap<String, String>(), testObject.parse("     "));
 	}
-	
-	@Test
+
+	@Test(expected = IllegalArgumentException.class)
 	public void test_Malformed_NoEquals_StringReturns_emptyMap() {
 		ParameterParser testObject = new ParameterParser();
-		
+
 		assertEquals(new HashMap<String, String>(), testObject.parse("namevalue"));
 	}
-	@Test
+
+	@Test(expected = IllegalArgumentException.class)
 	public void test_Malformed_ExtraSemicolon_StringReturns_emptyMap() {
 		ParameterParser testObject = new ParameterParser();
 
@@ -45,14 +48,14 @@ public void test_OneParamStringReturns_emptyMap() {
 		expected.put("name", "value");
 		assertEquals(expected, testObject.parse("name=value"));
 	}
-	
-	@Test
+
+	@Test(expected = IllegalArgumentException.class)
 	public void test_TrimsSpacesStringReturns_emptyMap() {
 		ParameterParser testObject = new ParameterParser();
-		
+
 		HashMap<String, String> expected = new HashMap<String, String>();
 		expected.put("name", "value");
-		assertEquals(expected, testObject.parse(" name = value; ;;"));
+		testObject.parse(" name = value; ;;");
 	}
 
 	@Test
@@ -65,4 +68,48 @@ public void test_TwoParamsStringReturns_emptyMap() {
 		assertEquals(expected, testObject.parse("name2=value2;name=value"));
 	}
 
+	@Test
+	public void checkSanity_HappyPath() throws Exception {
+		ParameterParser testObject = new ParameterParser();
+
+		assertNull(testObject.checkSanity("* * * * *%name=value"));
+	}
+
+	@Test
+	public void checkSanity_TrailingSemiColon_IsTrimmed() throws Exception {
+		ParameterParser testObject = new ParameterParser();
+
+		assertNull(testObject.checkSanity("* * * * *%env=eight;freckled=flase;frecked=false;"));
+	}
+
+	@Test
+	public void checkSanity_MoreThanOnePercent() throws Exception {
+		ParameterParser testObject = new ParameterParser();
+
+		assertEquals(Messages.ParameterizedTimerTrigger_MoreThanOnePercent(),
+				testObject.checkSanity("* * * * *%name=value;%fred=barney"));
+	}
+
+	@Test
+	public void checkSanity_NoParaetersIsNoBigDeal() throws Exception {
+		ParameterParser testObject = new ParameterParser();
+
+		assertNull(testObject.checkSanity("* * * * *%"));
+		assertNull(testObject.checkSanity("* * * * *"));
+	}
+
+	@Test
+	public void checkSanity_duplicateParamName() throws Exception {
+		ParameterParser testObject = new ParameterParser();
+
+		testObject.checkSanity("* * * * *%name=value;name=value2");
+	}
+
+	@Test
+	public void checkSanity_UnmatchedEquals() throws Exception {
+		ParameterParser testObject = new ParameterParser();
+
+		testObject.checkSanity("* * * * *%name=value;name2=");
+	}
+
 }