code review changes

Author: fmbenhassine

spring-batch-core/src/main/java/org/springframework/batch/core/DefaultJobKeyGenerator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2013 the original author or authors.
+ * Copyright 2006-2018 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.
@@ -24,13 +24,16 @@
 import java.util.List;
 import java.util.Map;
 
+import org.springframework.util.Assert;
+
 /**
  * Default implementation of the {@link JobKeyGenerator} interface.
  * This implementation provides a single hash value based on the JobParameters
  * passed in.  Only identifying parameters (per {@link JobParameter#isIdentifying()})
  * are used in the calculation of the key.
  *
  * @author Michael Minella
+ * @author Mahmoud Ben Hassine
  * @since 2.2
  */
 public class DefaultJobKeyGenerator implements JobKeyGenerator<JobParameters> {
@@ -42,6 +45,7 @@ public class DefaultJobKeyGenerator implements JobKeyGenerator<JobParameters> {
 	@Override
 	public String generateKey(JobParameters source) {
 
+		Assert.notNull(source, "source must not be null");
 		Map<String, JobParameter> props = source.getParameters();
 		StringBuilder stringBuffer = new StringBuilder();
 		List<String> keys = new ArrayList<String>(props.keySet());

spring-batch-core/src/main/java/org/springframework/batch/core/ItemReadListener.java

@@ -17,7 +17,6 @@
 
 import org.springframework.batch.item.ItemReader;
 import org.springframework.batch.item.ItemWriter;
-import org.springframework.lang.Nullable;
 
 /**
  * Listener interface around the reading of an item.
@@ -34,11 +33,13 @@ public interface ItemReadListener<T> extends StepListener {
 	void beforeRead();
 
 	/**
-	 * Called after {@link ItemReader#read()}
+	 * Called after {@link ItemReader#read()}.
+	 * This method is called only for actual items (ie it is not called when the
+	 * reader returns null).
 	 * 
 	 * @param item returned from read()
 	 */
-	void afterRead(@Nullable T item);
+	void afterRead(T item);
 
 	/**
 	 * Called if an error occurs while trying to read.

spring-batch-core/src/main/java/org/springframework/batch/core/JobKeyGenerator.java

@@ -15,8 +15,6 @@
  */
 package org.springframework.batch.core;
 
-import org.springframework.lang.Nullable;
-
 /**
  * Strategy interface for the generation of the key used in identifying
  * unique {@link JobInstance}.
@@ -32,10 +30,10 @@ public interface JobKeyGenerator<T> {
 	/**
 	 * Method to generate the unique key used to identify a job instance.
 	 *
-	 * @param source Source information used to generate the key (can be {@code null})
+	 * @param source Source information used to generate the key (must not be {@code null})
 	 *
 	 * @return a unique string identifying the job based on the information
 	 * supplied
 	 */
-	String generateKey(@Nullable T source);
+	String generateKey(T source);
 }

spring-batch-core/src/main/java/org/springframework/batch/core/step/StepLocator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009 the original author or authors.
+ * Copyright 2009-2018 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.
@@ -18,17 +18,20 @@
 import java.util.Collection;
 
 import org.springframework.batch.core.Step;
+import org.springframework.lang.Nullable;
 
 /**
  * Interface for locating a {@link Step} instance by name.
  * 
  * @author Dave Syer
+ * @author Mahmoud Ben Hassine
  *
  */
 public interface StepLocator {
 
 	Collection<String> getStepNames();
-	
-	Step getStep(String stepName) throws NoSuchStepException;
+
+	@Nullable
+	Step getStep(String stepName);
 
 }

spring-batch-core/src/main/java/org/springframework/batch/core/step/item/SimpleChunkProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2013 the original author or authors.
+ * Copyright 2006-2018 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.
@@ -24,6 +24,7 @@
 import org.springframework.batch.item.ItemProcessor;
 import org.springframework.batch.item.ItemWriter;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
 /**
@@ -42,19 +43,22 @@ public class SimpleChunkProcessor<I, O> implements ChunkProcessor<I>, Initializi
 	private final MulticasterBatchListener<I, O> listener = new MulticasterBatchListener<I, O>();
 
 	/**
-	 * Default constructor for ease of configuration (both itemWriter and
-	 * itemProcessor are mandatory).
+	 * Default constructor for ease of configuration.
 	 */
 	@SuppressWarnings("unused")
 	private SimpleChunkProcessor() {
 		this(null, null);
 	}
 
-	public SimpleChunkProcessor(ItemProcessor<? super I, ? extends O> itemProcessor, ItemWriter<? super O> itemWriter) {
+	public SimpleChunkProcessor(@Nullable ItemProcessor<? super I, ? extends O> itemProcessor, ItemWriter<? super O> itemWriter) {
 		this.itemProcessor = itemProcessor;
 		this.itemWriter = itemWriter;
 	}
 
+	public SimpleChunkProcessor(ItemWriter<? super O> itemWriter) {
+		this(null, itemWriter);
+	}
+
 	/**
 	 * @param itemProcessor the {@link ItemProcessor} to set
 	 */
@@ -77,7 +81,6 @@ public void setItemWriter(ItemWriter<? super O> itemWriter) {
 	@Override
 	public void afterPropertiesSet() throws Exception {
 		Assert.notNull(itemWriter, "ItemWriter must be set");
-		Assert.notNull(itemProcessor, "ItemProcessor must be set");
 	}
 
 	/**

spring-batch-core/src/test/java/org/springframework/batch/core/DefaultJobKeyGeneratorTests.java

@@ -29,6 +29,11 @@ public void setUp() throws Exception {
 		jobKeyGenerator = new DefaultJobKeyGenerator();
 	}
 
+	@Test(expected = IllegalArgumentException.class)
+	public void testNullParameters() {
+		jobKeyGenerator.generateKey(null);
+	}
+
 	@Test
 	public void testMixedParameters() {
 		JobParameters jobParameters1 = new JobParametersBuilder().addString(