Add assertions to reject null values in JobParameter

Resolves #3913
spring-projects · Sep 9, 2021 · 497438a · 497438a
1 parent 6c619b9
commit 497438a
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 201 deletions.
diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/JobParameter.java b/spring-batch-core/src/main/java/org/springframework/batch/core/JobParameter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2013 the original author or authors.
+ * Copyright 2006-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,6 +19,9 @@
 import java.io.Serializable;
 import java.util.Date;
 
+import org.springframework.lang.NonNull;
+import org.springframework.util.Assert;
+
 /**
  * Domain representation of a parameter to a batch job. Only the following types
  * can be parameters: String, Long, Date, and Double.  The identifying flag is
@@ -28,10 +31,10 @@
  * @author Lucas Ward
  * @author Dave Syer
  * @author Michael Minella
+ * @author Mahmoud Ben Hassine
  * @since 2.0
  *
  */
-@SuppressWarnings("serial")
 public class JobParameter implements Serializable {
 
 	private final Object parameter;
@@ -42,61 +45,57 @@ public class JobParameter implements Serializable {
 
 	/**
 	 * Construct a new JobParameter as a String.
-	 * @param parameter {@link String} instance.
+	 * @param parameter {@link String} instance. Must not be {@code null}.
 	 * @param identifying true if JobParameter should be identifying.
 	 */
-	public JobParameter(String parameter, boolean identifying) {
-		this.parameter = parameter;
-		parameterType = ParameterType.STRING;
-		this.identifying = identifying;
+	public JobParameter(@NonNull String parameter, boolean identifying) {
+		this(parameter, identifying, ParameterType.STRING);
 	}
 
 	/**
 	 * Construct a new JobParameter as a Long.
 	 *
-	 * @param parameter {@link Long} instance.
+	 * @param parameter {@link Long} instance. Must not be {@code null}.
 	 * @param identifying true if JobParameter should be identifying.
 	 */
-	public JobParameter(Long parameter, boolean identifying) {
-		this.parameter = parameter;
-		parameterType = ParameterType.LONG;
-		this.identifying = identifying;
+	public JobParameter(@NonNull Long parameter, boolean identifying) {
+		this(parameter, identifying, ParameterType.LONG);
 	}
 
 	/**
 	 * Construct a new JobParameter as a Date.
 	 *
-	 * @param parameter {@link Date} instance.
+	 * @param parameter {@link Date} instance. Must not be {@code null}.
 	 * @param identifying true if JobParameter should be identifying.
 	 */
-	public JobParameter(Date parameter, boolean identifying) {
-		this.parameter = parameter;
-		parameterType = ParameterType.DATE;
-		this.identifying = identifying;
+	public JobParameter(@NonNull Date parameter, boolean identifying) {
+		this(parameter, identifying, ParameterType.DATE);
 	}
 
 	/**
 	 * Construct a new JobParameter as a Double.
 	 *
-	 * @param parameter {@link Double} instance.
+	 * @param parameter {@link Double} instance. Must not be {@code null}.
 	 * @param identifying true if JobParameter should be identifying.
 	 */
-	public JobParameter(Double parameter, boolean identifying) {
+	public JobParameter(@NonNull Double parameter, boolean identifying) {
+		this(parameter, identifying, ParameterType.DOUBLE);
+	}
+
+	private JobParameter(Object parameter, boolean identifying, ParameterType parameterType) {
+		Assert.notNull(parameter, "parameter must not be null");
 		this.parameter = parameter;
-		parameterType = ParameterType.DOUBLE;
+		this.parameterType = parameterType;
 		this.identifying = identifying;
 	}
 
-
 	/**
 	 * Construct a new JobParameter as a String.
 	 *
 	 * @param parameter {@link String} instance.
 	 */
 	public JobParameter(String parameter) {
-		this.parameter = parameter;
-		parameterType = ParameterType.STRING;
-		this.identifying = true;
+		this(parameter, true);
 	}
 
 	/**
@@ -105,9 +104,7 @@ public JobParameter(String parameter) {
 	 * @param parameter {@link Long} instance.
 	 */
 	public JobParameter(Long parameter) {
-		this.parameter = parameter;
-		parameterType = ParameterType.LONG;
-		this.identifying = true;
+		this(parameter, true);
 	}
 
 	/**
@@ -116,9 +113,7 @@ public JobParameter(Long parameter) {
 	 * @param parameter {@link Date} instance.
 	 */
 	public JobParameter(Date parameter) {
-		this.parameter = parameter;
-		parameterType = ParameterType.DATE;
-		this.identifying = true;
+		this(parameter, true);
 	}
 
 	/**
@@ -127,9 +122,7 @@ public JobParameter(Date parameter) {
 	 * @param parameter {@link Double} instance.
 	 */
 	public JobParameter(Double parameter) {
-		this.parameter = parameter;
-		parameterType = ParameterType.DOUBLE;
-		this.identifying = true;
+		this(parameter, true);
 	}
 
 	public boolean isIdentifying() {
@@ -140,13 +133,7 @@ public boolean isIdentifying() {
 	 * @return the value contained within this JobParameter.
 	 */
 	public Object getValue() {
-
-		if (parameter != null && parameter.getClass().isInstance(Date.class)) {
-			return new Date(((Date) parameter).getTime());
-		}
-		else {
-			return parameter;
-		}
+		return parameter;
 	}
 
 	/**
@@ -158,7 +145,7 @@ public ParameterType getType() {
 
 	@Override
 	public boolean equals(Object obj) {
-		if (obj instanceof JobParameter == false) {
+		if (!(obj instanceof JobParameter)) {
 			return false;
 		}
 
@@ -167,18 +154,17 @@ public boolean equals(Object obj) {
 		}
 
 		JobParameter rhs = (JobParameter) obj;
-		return parameter==null ? rhs.parameter==null && parameterType==rhs.parameterType: parameter.equals(rhs.parameter);
+		return parameterType == rhs.parameterType && parameter.equals(rhs.parameter);
 	}
 
 	@Override
 	public String toString() {
-		return parameter == null ? null : (parameterType == ParameterType.DATE ? "" + ((Date) parameter).getTime()
-				: parameter.toString());
+		return  parameterType == ParameterType.DATE ? "" + ((Date) parameter).getTime() : parameter.toString();
 	}
 
 	@Override
 	public int hashCode() {
-		return 7 + 21 * (parameter == null ? parameterType.hashCode() : parameter.hashCode());
+		return 7 + 21 * parameter.hashCode();
 	}
 
 	/**

diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/JobParametersBuilder.java b/spring-batch-core/src/main/java/org/springframework/batch/core/JobParametersBuilder.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2019 the original author or authors.
+ * Copyright 2006-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,8 @@
 import java.util.Properties;
 
 import org.springframework.batch.core.explore.JobExplorer;
+import org.springframework.lang.NonNull;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
@@ -103,10 +105,10 @@ public JobParametersBuilder(JobParameters jobParameters, JobExplorer jobExplorer
 	 * Add a new identifying String parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addString(String key, String parameter) {
+	public JobParametersBuilder addString(String key, @NonNull String parameter) {
 		this.parameterMap.put(key, new JobParameter(parameter, true));
 		return this;
 	}
@@ -115,11 +117,11 @@ public JobParametersBuilder addString(String key, String parameter) {
 	 * Add a new String parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @param identifying - indicates if the parameter is used as part of identifying a job instance
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addString(String key, String parameter, boolean identifying) {
+	public JobParametersBuilder addString(String key, @NonNull String parameter, boolean identifying) {
 		this.parameterMap.put(key, new JobParameter(parameter, identifying));
 		return this;
 	}
@@ -128,10 +130,10 @@ public JobParametersBuilder addString(String key, String parameter, boolean iden
 	 * Add a new identifying {@link Date} parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addDate(String key, Date parameter) {
+	public JobParametersBuilder addDate(String key, @NonNull Date parameter) {
 		this.parameterMap.put(key, new JobParameter(parameter, true));
 		return this;
 	}
@@ -140,11 +142,11 @@ public JobParametersBuilder addDate(String key, Date parameter) {
 	 * Add a new {@link Date} parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @param identifying - indicates if the parameter is used as part of identifying a job instance
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addDate(String key, Date parameter, boolean identifying) {
+	public JobParametersBuilder addDate(String key, @NonNull Date parameter, boolean identifying) {
 		this.parameterMap.put(key, new JobParameter(parameter, identifying));
 		return this;
 	}
@@ -153,10 +155,10 @@ public JobParametersBuilder addDate(String key, Date parameter, boolean identify
 	 * Add a new identifying Long parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addLong(String key, Long parameter) {
+	public JobParametersBuilder addLong(String key, @NonNull Long parameter) {
 		this.parameterMap.put(key, new JobParameter(parameter, true));
 		return this;
 	}
@@ -165,11 +167,11 @@ public JobParametersBuilder addLong(String key, Long parameter) {
 	 * Add a new Long parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @param identifying - indicates if the parameter is used as part of identifying a job instance
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addLong(String key, Long parameter, boolean identifying) {
+	public JobParametersBuilder addLong(String key, @NonNull Long parameter, boolean identifying) {
 		this.parameterMap.put(key, new JobParameter(parameter, identifying));
 		return this;
 	}
@@ -178,10 +180,10 @@ public JobParametersBuilder addLong(String key, Long parameter, boolean identify
 	 * Add a new identifying Double parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addDouble(String key, Double parameter) {
+	public JobParametersBuilder addDouble(String key, @NonNull Double parameter) {
 		this.parameterMap.put(key, new JobParameter(parameter, true));
 		return this;
 	}
@@ -190,11 +192,11 @@ public JobParametersBuilder addDouble(String key, Double parameter) {
 	 * Add a new Double parameter for the given key.
 	 *
 	 * @param key - parameter accessor.
-	 * @param parameter - runtime parameter
+	 * @param parameter - runtime parameter. Must not be {@code null}.
 	 * @param identifying - indicates if the parameter is used as part of identifying a job instance
 	 * @return a reference to this object.
 	 */
-	public JobParametersBuilder addDouble(String key, Double parameter, boolean identifying) {
+	public JobParametersBuilder addDouble(String key, @NonNull Double parameter, boolean identifying) {
 		this.parameterMap.put(key, new JobParameter(parameter, identifying));
 		return this;
 	}

diff --git a/...-batch-core/src/test/java/org/springframework/batch/core/DefaultJobKeyGeneratorTests.java b/...-batch-core/src/test/java/org/springframework/batch/core/DefaultJobKeyGeneratorTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2013 the original author or authors.
+ * Copyright 2013-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -54,17 +54,6 @@ public void testCreateJobKey() {
 		assertEquals(32, key.length());
 	}
 
-	@Test
-	public void testCreateJobKeyWithNullParameter() {
-		JobParameters jobParameters1 = new JobParametersBuilder().addString(
-				"foo", "bar").addString("bar", null).toJobParameters();
-		JobParameters jobParameters2 = new JobParametersBuilder().addString(
-				"foo", "bar").addString("bar", "").toJobParameters();
-		String key1 = jobKeyGenerator.generateKey(jobParameters1);
-		String key2 = jobKeyGenerator.generateKey(jobParameters2);
-		assertEquals(key1, key2);
-	}
-
 	@Test
 	public void testCreateJobKeyOrdering() {
 		JobParameters jobParameters1 = new JobParametersBuilder().addString(

diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/JobParameterTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2008-2013 the original author or authors.
+ * Copyright 2008-2021 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -37,10 +37,9 @@ public void testStringParameter(){
 		assertEquals("test", jobParameter.getValue());
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void testNullStringParameter(){
 		jobParameter = new JobParameter((String)null, true);
-		assertEquals(null, jobParameter.getValue());
 	}
 
 	@Test
@@ -62,10 +61,9 @@ public void testDateParameter(){
 		assertEquals(new Date(0L), jobParameter.getValue());
 	}
 
-	@Test
+	@Test(expected = IllegalArgumentException.class)
 	public void testNullDateParameter(){
 		jobParameter = new JobParameter((Date)null, true);
-		assertEquals(null, jobParameter.getValue());
 	}
 
 	@Test
@@ -89,25 +87,4 @@ public void testHashcode(){
 		assertEquals(testParameter.hashCode(), jobParameter.hashCode());
 	}
 
-	@Test
-	public void testEqualsWithNull(){
-		jobParameter = new JobParameter((String)null, true);
-		JobParameter testParameter = new JobParameter((String)null, true);
-		assertTrue(jobParameter.equals(testParameter));
-	}
-
-	@Test
-	public void testEqualsWithNullAndDifferentType(){
-		jobParameter = new JobParameter((String)null, true);
-		JobParameter testParameter = new JobParameter((Date)null, true);
-		assertFalse(jobParameter.equals(testParameter));
-	}
-
-	@Test
-	public void testHashcodeWithNull(){
-		jobParameter = new JobParameter((String)null, true);
-		JobParameter testParameter = new JobParameter((String)null, true);
-		assertEquals(testParameter.hashCode(), jobParameter.hashCode());
-	}
-
 }