diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobOperator.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobOperator.java index f2369f04e..8f747458d 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobOperator.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/JobOperator.java @@ -91,7 +91,8 @@ public interface JobOperator { Set getRunningExecutions(String jobName) throws NoSuchJobException; /** - * Get the {@link JobParameters} as an easily readable String. + * Get the {@link JobParameters} as a human readable String (new line separated + * key=value pairs). * @param executionId the id of an existing {@link JobExecution} * @return the job parameters that were used to launch the associated instance * @throws NoSuchJobExecutionException if the id was not associated with any @@ -102,8 +103,8 @@ public interface JobOperator { /** * Start a new instance of a job with the parameters specified. * @param jobName the name of the {@link Job} to launch - * @param parameters the parameters to launch it with (comma or newline separated - * name=value pairs) + * @param parameters the parameters to launch it with (new line separated key=value + * pairs) * @return the id of the {@link JobExecution} that is launched * @throws NoSuchJobException if there is no {@link Job} with the specified name * @throws JobInstanceAlreadyExistsException if a job instance with this name and diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java index ca67fa9aa..b73811aa1 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/SimpleJobOperator.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2023 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. @@ -60,9 +60,9 @@ import org.springframework.batch.core.step.StepLocator; import org.springframework.batch.core.step.tasklet.StoppableTasklet; import org.springframework.batch.core.step.tasklet.Tasklet; import org.springframework.batch.core.step.tasklet.TaskletStep; +import org.springframework.batch.support.PropertiesConverter; import org.springframework.beans.factory.InitializingBean; import org.springframework.lang.Nullable; -import org.springframework.transaction.annotation.Transactional; import org.springframework.util.Assert; /** @@ -222,11 +222,7 @@ public class SimpleJobOperator implements JobOperator, InitializingBean { Properties properties = this.jobParametersConverter.getProperties(jobExecution.getJobParameters()); - List keyValuePairs = new ArrayList<>(); - for (Map.Entry entry : properties.entrySet()) { - keyValuePairs.add(entry.getKey() + "=" + entry.getValue()); - } - return String.join(" ", keyValuePairs); + return PropertiesConverter.propertiesToString(properties); } /* @@ -319,14 +315,7 @@ public class SimpleJobOperator implements JobOperator, InitializingBean { logger.info("Checking status of job with name=" + jobName); } - Properties properties = new Properties(); - if (!parameters.isEmpty()) { - String[] keyValuePairs = parameters.split(" "); - for (String string : keyValuePairs) { - String[] keyValuePair = string.split("="); - properties.setProperty(keyValuePair[0], keyValuePair[1]); - } - } + Properties properties = PropertiesConverter.stringToProperties(parameters); JobParameters jobParameters = jobParametersConverter.getJobParameters(properties); if (jobRepository.isJobInstanceExists(jobName, jobParameters)) { diff --git a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/PropertiesConverter.java b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/PropertiesConverter.java index 926b8ec4c..041026717 100644 --- a/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/PropertiesConverter.java +++ b/spring-batch-infrastructure/src/main/java/org/springframework/batch/support/PropertiesConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2023 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. @@ -16,122 +16,74 @@ package org.springframework.batch.support; -import java.io.IOException; -import java.io.StringReader; -import java.io.StringWriter; -import java.util.Arrays; +import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Properties; -import org.springframework.util.DefaultPropertiesPersister; -import org.springframework.util.PropertiesPersister; +import org.springframework.lang.NonNull; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** - * Utility to convert a Properties object to a String and back. Ideally this utility - * should have been used to convert to string in order to convert that string back to a - * Properties Object. Attempting to convert a string obtained by calling - * Properties.toString() will return an invalid Properties object. The format of - * Properties is that used by {@link PropertiesPersister} from the Spring Core, so a - * String in the correct format for a Spring property editor is fine (key=value pairs - * separated by new lines). + * Utility to convert a Properties object to a String and back. The format of properties + * is new line separated key=value pairs. * * @author Lucas Ward * @author Dave Syer - * @see PropertiesPersister + * @author Mahmoud Ben Hassine */ public final class PropertiesConverter { - private static final PropertiesPersister propertiesPersister = new DefaultPropertiesPersister(); - - private static final String LINE_SEPARATOR = System.getProperty("line.separator"); + private static final String LINE_SEPARATOR = "\n"; // prevents the class from being instantiated private PropertiesConverter() { } /** - * Parse a String to a Properties object. If string is null, an empty Properties - * object will be returned. The input String is a set of name=value pairs, delimited - * by either newline or comma (for brevity). If the input String contains a newline it - * is assumed that the separator is newline, otherwise comma. - * @param stringToParse String to parse. - * @return Properties parsed from each string. - * @see PropertiesPersister + * Parse a String to a Properties object. If string is empty, an empty Properties + * object will be returned. The input String should be a set of key=value pairs, + * separated by a new line. + * @param stringToParse String to parse. Must not be {@code null}. + * @return Properties parsed from each key=value pair. */ - public static Properties stringToProperties(String stringToParse) { - - if (stringToParse == null) { + public static Properties stringToProperties(@NonNull String stringToParse) { + Assert.notNull(stringToParse, "stringToParse must not be null"); + if (!StringUtils.hasText(stringToParse)) { return new Properties(); } - - if (!contains(stringToParse, "\n")) { - stringToParse = StringUtils - .arrayToDelimitedString(StringUtils.commaDelimitedListToStringArray(stringToParse), "\n"); - } - - StringReader stringReader = new StringReader(stringToParse); - Properties properties = new Properties(); - - try { - propertiesPersister.load(properties, stringReader); - // Exception is only thrown by StringReader after it is closed, - // so never in this case. + String[] keyValuePairs = stringToParse.split(LINE_SEPARATOR); + for (String string : keyValuePairs) { + if (!string.contains("=")) { + throw new IllegalArgumentException(string + "is not a valid key=value pair"); + } + String[] keyValuePair = string.split("="); + properties.setProperty(keyValuePair[0], keyValuePair[1]); } - catch (IOException ex) { - throw new IllegalStateException( - "Error while trying to parse String to java.util.Properties," + " given String: " + properties); - } - return properties; } /** - * Convert Properties object to String. This is only necessary for compatibility with - * converting the String back to a properties object. If an empty properties object is - * passed in, a blank string is returned, otherwise it's string representation is - * returned. - * @param propertiesToParse contains the properties be converted. - * @return String representation of properties object + * Convert a Properties object to a String. This is only necessary for compatibility + * with converting the String back to a properties object. If an empty properties + * object is passed in, a blank string is returned, otherwise it's string + * representation is returned. + * @param propertiesToParse contains the properties to be converted. Must not be + * {@code null}. + * @return String representation of the properties object */ - public static String propertiesToString(Properties propertiesToParse) { - - // If properties is empty, return a blank string. - if (propertiesToParse == null || propertiesToParse.size() == 0) { + public static String propertiesToString(@NonNull Properties propertiesToParse) { + Assert.notNull(propertiesToParse, "propertiesToParse must not be null"); + if (propertiesToParse.isEmpty()) { return ""; } - - StringWriter stringWriter = new StringWriter(); - - try { - propertiesPersister.store(propertiesToParse, stringWriter, null); + List keyValuePairs = new ArrayList<>(); + for (Map.Entry entry : propertiesToParse.entrySet()) { + keyValuePairs.add(entry.getKey() + "=" + entry.getValue()); } - catch (IOException ex) { - // Exception is never thrown by StringWriter - throw new IllegalStateException("Error while trying to convert properties to string"); - } - - // If the value is short enough (and doesn't contain commas), convert to - // comma-separated... - String value = stringWriter.toString(); - if (value.length() < 160) { - List list = Arrays - .asList(StringUtils.delimitedListToStringArray(value, LINE_SEPARATOR, LINE_SEPARATOR)); - String shortValue = StringUtils.collectionToCommaDelimitedString(list.subList(1, list.size())); - int count = StringUtils.countOccurrencesOf(shortValue, ","); - if (count == list.size() - 2) { - value = shortValue; - } - if (value.endsWith(",")) { - value = value.substring(0, value.length() - 1); - } - } - return value; - } - - private static boolean contains(String str, String searchStr) { - return str.indexOf(searchStr) != -1; + return String.join(LINE_SEPARATOR, keyValuePairs); } } diff --git a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/PropertiesConverterTests.java b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/PropertiesConverterTests.java index 791373c62..43e537c79 100644 --- a/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/PropertiesConverterTests.java +++ b/spring-batch-infrastructure/src/test/java/org/springframework/batch/support/PropertiesConverterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2022 the original author or authors. + * Copyright 2006-2023 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. @@ -16,121 +16,79 @@ package org.springframework.batch.support; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.util.Properties; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; + import org.springframework.util.StringUtils; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + /** - * Unit tests for {@link PropertiesConverter} + * Unit tests for {@link PropertiesConverter}. * * @author Robert Kasanicky + * @author Mahmoud Ben Hassine */ class PropertiesConverterTests { - // convenience attributes for storing results of conversions - private Properties props = null; - - /** - * Check that Properties can be converted to String and back correctly. - */ @Test - void testTwoWayRegularConversion() { + void testStringToPropertiesConversion() { + String stringToParse = "key1=value1\nkey2=value2"; + Properties expectedProperties = new Properties(); + expectedProperties.setProperty("key1", "value1"); + expectedProperties.setProperty("key2", "value2"); - Properties storedProps = new Properties(); - storedProps.setProperty("key1", "value1"); - storedProps.setProperty("key2", "value2"); + Properties props = PropertiesConverter.stringToProperties(stringToParse); - props = PropertiesConverter.stringToProperties(PropertiesConverter.propertiesToString(storedProps)); - - assertEquals(storedProps, props); + assertEquals(expectedProperties, props); } - /** - * Check that Properties can be comma delimited. - */ @Test - void testRegularConversionWithComma() { + void testPropertiesToStringConversion() { + Properties properties = new Properties(); + properties.setProperty("key1", "value1"); + properties.setProperty("key2", "value2"); - Properties storedProps = new Properties(); - storedProps.setProperty("key1", "value1"); - storedProps.setProperty("key2", "value2"); - - props = PropertiesConverter.stringToProperties("key1=value1,key2=value2"); - - assertEquals(storedProps, props); - } - - /** - * Check that Properties can be comma delimited with extra whitespace. - */ - @Test - void testRegularConversionWithCommaAndWhitespace() { - - Properties storedProps = new Properties(); - storedProps.setProperty("key1", "value1"); - storedProps.setProperty("key2", "value2"); - - props = PropertiesConverter.stringToProperties("key1=value1, key2=value2"); - - assertEquals(storedProps, props); - } - - /** - * Check that Properties can be comma delimited with extra whitespace. - */ - @Test - void testShortConversionWithCommas() { - - Properties storedProps = new Properties(); - storedProps.setProperty("key1", "value1"); - storedProps.setProperty("key2", "value2"); - - String value = PropertiesConverter.propertiesToString(storedProps); + String value = PropertiesConverter.propertiesToString(properties); assertTrue(value.contains("key1=value1"), "Wrong value: " + value); assertTrue(value.contains("key2=value2"), "Wrong value: " + value); - assertEquals(1, StringUtils.countOccurrencesOf(value, ",")); + assertEquals(1, StringUtils.countOccurrencesOf(value, "\n")); } - /** - * Check that Properties can be newline delimited. - */ @Test - void testRegularConversionWithCommaAndNewline() { - + void testTwoWayRegularConversion() { Properties storedProps = new Properties(); storedProps.setProperty("key1", "value1"); storedProps.setProperty("key2", "value2"); - props = PropertiesConverter.stringToProperties("key1=value1\n key2=value2"); + Properties props = PropertiesConverter.stringToProperties(PropertiesConverter.propertiesToString(storedProps)); assertEquals(storedProps, props); } - /** - * Null String should be converted to empty Properties - */ @Test - void testStringToPropertiesNull() { - props = PropertiesConverter.stringToProperties(null); - assertNotNull(props); - assertEquals(0, props.size(), "properties are empty"); + void nullStringShouldNotBeAccepted() { + Assertions.assertThrows(IllegalArgumentException.class, () -> PropertiesConverter.stringToProperties(null)); } - /** - * Null or empty properties should be converted to empty String - */ @Test - void testPropertiesToStringNull() { - String string = PropertiesConverter.propertiesToString(null); - assertEquals("", string); + void emptyStringShouldBeConvertedToEmptyProperties() { + Properties properties = PropertiesConverter.stringToProperties(""); + Assertions.assertTrue(properties.isEmpty()); + } - string = PropertiesConverter.propertiesToString(new Properties()); + @Test + void nullPropertiesShouldNotBeAccepted() { + Assertions.assertThrows(IllegalArgumentException.class, () -> PropertiesConverter.propertiesToString(null)); + } + + @Test + void emptyPropertiesShouldBeConvertedToEmptyString() { + String string = PropertiesConverter.propertiesToString(new Properties()); assertEquals("", string); } diff --git a/spring-batch-test/src/main/java/org/springframework/batch/test/MetaDataInstanceFactory.java b/spring-batch-test/src/main/java/org/springframework/batch/test/MetaDataInstanceFactory.java index 94c2c5e4d..38411be19 100644 --- a/spring-batch-test/src/main/java/org/springframework/batch/test/MetaDataInstanceFactory.java +++ b/spring-batch-test/src/main/java/org/springframework/batch/test/MetaDataInstanceFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2021 the original author or authors. + * Copyright 2006-2023 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. @@ -112,7 +112,7 @@ public class MetaDataInstanceFactory { * @param jobName the name of the job * @param instanceId the Id of the {@link JobInstance} * @param executionId the id for the {@link JobExecution} - * @param jobParameters comma or new line separated name=value pairs + * @param jobParameters new line separated key=value pairs * @return a {@link JobExecution} */ public static JobExecution createJobExecution(String jobName, Long instanceId, Long executionId,