You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2017/03/18 10:12:31 UTC
svn commit: r1787542 - in /jmeter/trunk:
src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
src/core/org/apache/jmeter/save/CSVSaveService.java
test/src/org/apache/jmeter/samplers/TestSampleSaveConfiguration.java
xdocs/changes.xml
Author: pmouawad
Date: Sat Mar 18 10:12:31 2017
New Revision: 1787542
URL: http://svn.apache.org/viewvc?rev=1787542&view=rev
Log:
Bug 60830 - Timestamps in CSV file could be corrupted due to sharing a SimpleDateFormatter across threads
Bugzilla Id: 60830
Modified:
jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
jmeter/trunk/src/core/org/apache/jmeter/save/CSVSaveService.java
jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleSaveConfiguration.java
jmeter/trunk/xdocs/changes.xml
Modified: jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java?rev=1787542&r1=1787541&r2=1787542&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleSaveConfiguration.java Sat Mar 18 10:12:31 2017
@@ -31,6 +31,7 @@ import java.util.Objects;
import java.util.Properties;
import org.apache.commons.lang3.CharUtils;
+import org.apache.commons.lang3.time.FastDateFormat;
import org.apache.jmeter.save.CSVSaveService;
import org.apache.jmeter.testelement.TestPlan;
import org.apache.jmeter.util.JMeterUtils;
@@ -249,8 +250,6 @@ public class SampleSaveConfiguration imp
private static final boolean SAVE_ASSERTION_RESULTS_FAILURE_MESSAGE;
- private static final String TIMESTAMP_FORMAT;
-
private static final int ASSERTIONS_RESULT_TO_SAVE;
// TODO turn into method?
@@ -276,7 +275,7 @@ public class SampleSaveConfiguration imp
private static final boolean SAMPLE_COUNT;
- private static final DateFormat DATE_FORMATTER;
+ private static final String DATE_FORMAT;
/**
* The string used to separate fields when stored to disk, for example, the
@@ -346,19 +345,17 @@ public class SampleSaveConfiguration imp
TIME = TRUE.equalsIgnoreCase(props.getProperty(SAVE_TIME_PROP, TRUE));
- TIMESTAMP_FORMAT = props.getProperty(TIME_STAMP_FORMAT_PROP, MILLISECONDS);
+ String temporaryTimestampFormat = props.getProperty(TIME_STAMP_FORMAT_PROP, MILLISECONDS);
- PRINT_MILLISECONDS = MILLISECONDS.equalsIgnoreCase(TIMESTAMP_FORMAT);
+ PRINT_MILLISECONDS = MILLISECONDS.equalsIgnoreCase(temporaryTimestampFormat);
- // Prepare for a pretty date
- // FIXME Can TIMESTAMP_FORMAT be null ? it does not appear to me .
- if (!PRINT_MILLISECONDS && !NONE.equalsIgnoreCase(TIMESTAMP_FORMAT) && (TIMESTAMP_FORMAT != null)) {
- DATE_FORMATTER = new SimpleDateFormat(TIMESTAMP_FORMAT);
+ if (!PRINT_MILLISECONDS && !NONE.equalsIgnoreCase(temporaryTimestampFormat)) {
+ DATE_FORMAT = validateFormat(temporaryTimestampFormat);
} else {
- DATE_FORMATTER = null;
+ DATE_FORMAT = null;
}
- TIMESTAMP = !NONE.equalsIgnoreCase(TIMESTAMP_FORMAT);// reversed compare allows for null
+ TIMESTAMP = !NONE.equalsIgnoreCase(temporaryTimestampFormat);// reversed compare allows for null
SAVE_ASSERTION_RESULTS_FAILURE_MESSAGE = TRUE.equalsIgnoreCase(props.getProperty(
ASSERTION_RESULTS_FAILURE_MESSAGE_PROP, TRUE));
@@ -394,6 +391,48 @@ public class SampleSaveConfiguration imp
private static final SampleSaveConfiguration STATIC_SAVE_CONFIGURATION = new SampleSaveConfiguration();
+ // for test code only
+ static final String CONFIG_GETTER_PREFIX = "save"; // $NON-NLS-1$
+
+ // for test code only
+ static final String CONFIG_SETTER_PREFIX = "set"; // $NON-NLS-1$
+
+ /**
+ * List of saveXXX/setXXX(boolean) methods which is used to build the Sample Result Save Configuration dialog.
+ * New method names should be added at the end so that existing layouts are not affected.
+ */
+ // The current order is derived from http://jmeter.apache.org/usermanual/listeners.html#csvlogformat
+ // TODO this may not be the ideal order; fix further and update the screenshot(s)
+ public static final List<String> SAVE_CONFIG_NAMES = Collections.unmodifiableList(Arrays.asList(new String[]{
+ "AsXml",
+ "FieldNames", // CSV
+ "Timestamp",
+ "Time", // elapsed
+ "Label",
+ "Code", // Response Code
+ "Message", // Response Message
+ "ThreadName",
+ "DataType",
+ "Success",
+ "AssertionResultsFailureMessage",
+ "Bytes",
+ "SentBytes",
+ "ThreadCounts", // grpThreads and allThreads
+ "Url",
+ "FileName",
+ "Latency",
+ "ConnectTime",
+ "Encoding",
+ "SampleCount", // Sample and Error Count
+ "Hostname",
+ "IdleTime",
+ "RequestHeaders", // XML
+ "SamplerData", // XML
+ "ResponseHeaders", // XML
+ "ResponseData", // XML
+ "Subresults", // XML
+ "Assertions", // XML
+ }));
// N.B. Remember to update the equals and hashCode methods when adding new variables.
// Initialise values from properties
@@ -440,20 +479,22 @@ public class SampleSaveConfiguration imp
// Don't save this, as it is derived from the time format
private boolean printMilliseconds = PRINT_MILLISECONDS;
- /** A formatter for the time stamp. */
- private transient DateFormat formatter = DATE_FORMATTER;
- /* Make transient as we don't want to save the SimpleDataFormat class
+ private String dateFormat = DATE_FORMAT;
+
+ /** A formatter for the time stamp.
+ * Make transient as we don't want to save the FastDateFormat class
* Also, there's currently no way to change the value via the GUI, so changing it
* later means editting the JMX, or recreating the Listener.
*/
-
+ private transient FastDateFormat threadSafeLenientFormatter =
+ dateFormat != null ? FastDateFormat.getInstance(dateFormat) : null;
+
// Don't save this, as not settable via GUI
private String delimiter = DELIMITER;
// Don't save this - only needed for processing CSV headers currently
private transient int varCount = 0;
-
public SampleSaveConfiguration() {
}
@@ -507,10 +548,7 @@ public class SampleSaveConfiguration imp
public static SampleSaveConfiguration staticConfig() {
return STATIC_SAVE_CONFIGURATION;
}
-
- // for test code only
- static final String CONFIG_GETTER_PREFIX = "save"; // $NON-NLS-1$
-
+
/**
* Convert a config name to the method name of the getter.
* The getter method returns a boolean.
@@ -521,9 +559,6 @@ public class SampleSaveConfiguration imp
return CONFIG_GETTER_PREFIX + configName;
}
- // for test code only
- static final String CONFIG_SETTER_PREFIX = "set"; // $NON-NLS-1$
-
/**
* Convert a config name to the method name of the setter
* The setter method requires a boolean parameter.
@@ -535,53 +570,49 @@ public class SampleSaveConfiguration imp
}
/**
- * List of saveXXX/setXXX(boolean) methods which is used to build the Sample Result Save Configuration dialog.
- * New method names should be added at the end so that existing layouts are not affected.
+ * Validate pattern
+ * @param temporaryTimestampFormat DateFormat pattern
+ * @return format if ok or null
*/
- // The current order is derived from http://jmeter.apache.org/usermanual/listeners.html#csvlogformat
- // TODO this may not be the ideal order; fix further and update the screenshot(s)
- public static final List<String> SAVE_CONFIG_NAMES = Collections.unmodifiableList(Arrays.asList(new String[]{
- "AsXml",
- "FieldNames", // CSV
- "Timestamp",
- "Time", // elapsed
- "Label",
- "Code", // Response Code
- "Message", // Response Message
- "ThreadName",
- "DataType",
- "Success",
- "AssertionResultsFailureMessage",
- "Bytes",
- "SentBytes",
- "ThreadCounts", // grpThreads and allThreads
- "Url",
- "FileName",
- "Latency",
- "ConnectTime",
- "Encoding",
- "SampleCount", // Sample and Error Count
- "Hostname",
- "IdleTime",
- "RequestHeaders", // XML
- "SamplerData", // XML
- "ResponseHeaders", // XML
- "ResponseData", // XML
- "Subresults", // XML
- "Assertions", // XML
- }));
+ private static String validateFormat(String temporaryTimestampFormat) {
+ try {
+ new SimpleDateFormat(temporaryTimestampFormat);
+ if(log.isDebugEnabled()) {
+ log.debug("Successfully validated pattern value {} for property {}",
+ temporaryTimestampFormat, TIME_STAMP_FORMAT_PROP);
+ }
+ return temporaryTimestampFormat;
+ } catch(IllegalArgumentException ex) {
+ log.error("Invalid pattern value {} for property {}", temporaryTimestampFormat, TIME_STAMP_FORMAT_PROP);
+ return null;
+ }
+ }
+
private Object readResolve(){
- formatter = DATE_FORMATTER;
- return this;
+ setupDateFormat(DATE_FORMAT);
+ return this;
+ }
+
+ /**
+ * Initialize threadSafeLenientFormatter
+ * @param pDateFormat String date format
+ */
+ private void setupDateFormat(String pDateFormat) {
+ this.dateFormat = pDateFormat;
+ if(dateFormat != null) {
+ this.threadSafeLenientFormatter = FastDateFormat.getInstance(dateFormat);
+ } else {
+ this.threadSafeLenientFormatter = null;
+ }
}
@Override
public Object clone() {
try {
SampleSaveConfiguration clone = (SampleSaveConfiguration)super.clone();
- if(this.formatter != null) {
- clone.formatter = (SimpleDateFormat)this.formatter.clone();
+ if(this.dateFormat != null) {
+ clone.threadSafeLenientFormatter = (FastDateFormat)this.threadSafeLenientFormatter.clone();
}
return clone;
}
@@ -638,7 +669,7 @@ public class SampleSaveConfiguration imp
}
boolean complexValues = false;
if(primitiveValues && stringValues) {
- complexValues = Objects.equals(formatter, s.formatter);
+ complexValues = Objects.equals(dateFormat, s.dateFormat);
}
return primitiveValues && stringValues && complexValues;
@@ -677,7 +708,7 @@ public class SampleSaveConfiguration imp
hash = 31 * hash + (hostname ? 1 : 0);
hash = 31 * hash + (threadCounts ? 1 : 0);
hash = 31 * hash + (delimiter != null ? delimiter.hashCode() : 0);
- hash = 31 * hash + (formatter != null ? formatter.hashCode() : 0);
+ hash = 31 * hash + (dateFormat != null ? dateFormat.hashCode() : 0);
hash = 31 * hash + (sampleCount ? 1 : 0);
hash = 31 * hash + (idleTime ? 1 : 0);
@@ -907,23 +938,38 @@ public class SampleSaveConfiguration imp
///////////////// End of standard field accessors /////////////////////
+
/**
* Intended for use by CsvSaveService (and test cases)
* @param fmt
* format of the date to be saved. If <code>null</code>
* milliseconds since epoch will be printed
*/
- public void setFormatter(DateFormat fmt){
+ public void setDateFormat(String fmt){
printMilliseconds = fmt == null; // maintain relationship
- formatter = fmt;
+ setupDateFormat(fmt);
}
public boolean printMilliseconds() {
return printMilliseconds;
}
- public DateFormat formatter() {
- return formatter;
+ /**
+ * @return {@link DateFormat} non lenient
+ */
+ public DateFormat strictDateFormatter() {
+ if(dateFormat != null) {
+ return new SimpleDateFormat(dateFormat);
+ } else {
+ return null;
+ }
+ }
+
+ /**
+ * @return {@link FastDateFormat} Thread safe lenient formatter
+ */
+ public FastDateFormat threadSafeLenientFormatter() {
+ return threadSafeLenientFormatter;
}
public int assertionsResultsToSave() {
@@ -951,7 +997,7 @@ public class SampleSaveConfiguration imp
// Used by SampleSaveConfigurationConverter.unmarshall()
public void setDefaultTimeStampFormat() {
printMilliseconds=PRINT_MILLISECONDS;
- formatter=DATE_FORMATTER;
+ setupDateFormat(DATE_FORMAT);
}
public boolean saveHostname(){
Modified: jmeter/trunk/src/core/org/apache/jmeter/save/CSVSaveService.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/save/CSVSaveService.java?rev=1787542&r1=1787541&r2=1787542&view=diff
==============================================================================
--- jmeter/trunk/src/core/org/apache/jmeter/save/CSVSaveService.java (original)
+++ jmeter/trunk/src/core/org/apache/jmeter/save/CSVSaveService.java Sat Mar 18 10:12:31 2017
@@ -32,6 +32,7 @@ import java.nio.charset.StandardCharsets
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Date;
import java.util.List;
@@ -211,7 +212,8 @@ public final class CSVSaveService {
try {
timeStamp = Long.parseLong(text); // see if this works
} catch (NumberFormatException e) { // it did not, let's try some other formats
- log.warn("Cannot parse timestamp: '{}'", text);
+ log.warn("Cannot parse timestamp: '{}', will try following formats {}", text,
+ Arrays.asList(DATE_FORMAT_STRINGS));
boolean foundMatch = false;
for(String fmt : DATE_FORMAT_STRINGS) {
SimpleDateFormat dateFormat = new SimpleDateFormat(fmt);
@@ -219,22 +221,20 @@ public final class CSVSaveService {
try {
Date stamp = dateFormat.parse(text);
timeStamp = stamp.getTime();
- // method is only ever called from one thread at a time
- // so it's OK to use a static DateFormat
log.warn("Setting date format to: {}", fmt);
- saveConfig.setFormatter(dateFormat);
+ saveConfig.setDateFormat(fmt);
foundMatch = true;
break;
} catch (ParseException pe) {
- log.info("{} did not match {}", text, fmt);
+ log.info("{} did not match {}, trying next date format", text, fmt);
}
}
if (!foundMatch) {
throw new ParseException("No date-time format found matching "+text,-1);
}
}
- } else if (saveConfig.formatter() != null) {
- Date stamp = saveConfig.formatter().parse(text);
+ } else if (saveConfig.strictDateFormatter() != null) {
+ Date stamp = saveConfig.strictDateFormatter().parse(text);
timeStamp = stamp.getTime();
} else { // can this happen?
final String msg = "Unknown timestamp format";
@@ -802,8 +802,8 @@ public final class CSVSaveService {
if (saveConfig.saveTimestamp()) {
if (saveConfig.printMilliseconds()) {
text.append(sample.getTimeStamp());
- } else if (saveConfig.formatter() != null) {
- String stamp = saveConfig.formatter().format(
+ } else if (saveConfig.threadSafeLenientFormatter() != null) {
+ String stamp = saveConfig.threadSafeLenientFormatter().format(
new Date(sample.getTimeStamp()));
text.append(stamp);
}
Modified: jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleSaveConfiguration.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleSaveConfiguration.java?rev=1787542&r1=1787541&r2=1787542&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleSaveConfiguration.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleSaveConfiguration.java Sat Mar 18 10:12:31 2017
@@ -25,7 +25,6 @@ import static org.junit.Assert.assertNot
import static org.junit.Assert.assertTrue;
import java.lang.reflect.Method;
-import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.List;
@@ -136,16 +135,26 @@ public class TestSampleSaveConfiguration
assertEquals("Hash codes should be equal",a.hashCode(), b.hashCode());
assertTrue("Objects should be equal",a.equals(b));
assertTrue("Objects should be equal",b.equals(a));
- a.setFormatter(null);
- b.setFormatter(null);
+ assertTrue(a.strictDateFormatter() == null);
+ assertTrue(b.strictDateFormatter() == null);
+ assertTrue(a.threadSafeLenientFormatter() == null);
+ assertTrue(b.threadSafeLenientFormatter() == null);
+ a.setDateFormat(null);
+ b.setDateFormat(null);
assertEquals("Hash codes should be equal",a.hashCode(), b.hashCode());
assertTrue("Objects should be equal",a.equals(b));
assertTrue("Objects should be equal",b.equals(a));
- a.setFormatter(new SimpleDateFormat());
- b.setFormatter(new SimpleDateFormat());
+ assertTrue(a.strictDateFormatter() == null);
+ assertTrue(b.strictDateFormatter() == null);
+ assertTrue(a.threadSafeLenientFormatter() == null);
+ assertTrue(b.threadSafeLenientFormatter() == null);
+ a.setDateFormat("dd/MM/yyyy");
+ b.setDateFormat("dd/MM/yyyy");
assertEquals("Hash codes should be equal",a.hashCode(), b.hashCode());
assertTrue("Objects should be equal",a.equals(b));
assertTrue("Objects should be equal",b.equals(a));
+ assertTrue("Objects should be equal",a.strictDateFormatter().equals(b.strictDateFormatter()));
+ assertTrue("Objects should be equal",a.threadSafeLenientFormatter().equals(b.threadSafeLenientFormatter()));
}
@Test
Modified: jmeter/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1787542&r1=1787541&r2=1787542&view=diff
==============================================================================
--- jmeter/trunk/xdocs/changes.xml [utf-8] (original)
+++ jmeter/trunk/xdocs/changes.xml [utf-8] Sat Mar 18 10:12:31 2017
@@ -413,6 +413,7 @@ listeners hold and a rework of the way G
<li><bug>60744</bug>GUI elements are not cleaned up when reused during load of Test Plan which can lead them to be partially initialized with a previous state for a new Test Element</li>
<li><bug>60812</bug>JMeterThread does not honor contract of JMeterStopTestNowException</li>
<li><bug>60857</bug>SaveService omits XML header if _file_encoding is not defined in saveservice.properties</li>
+ <li><bug>60830</bug>Timestamps in CSV file could be corrupted due to sharing a SimpleDateFormatter across threads</li>
</ul>
<!-- =================== Thanks =================== -->