[GitHub] jmeter issue #438: Try running on java-ea for travis builds.

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/438
  
> Hi Felix,
> Since you're working on this.
> I see that when when we use openjdk11, we have those warnings:
> 
> warning: [options] bootstrap class path not set in conjunction with 
-source 8
> 
> Shouldn't we set source and target to 11 ?
> Regards

Well, as long as we are on Java 8 as minimum version, we should compile 
with something, that is *compatible*. That probably means:

 * set the bootstrap class to a JDK from version 8 as shown on 
https://ant.apache.org/manual/Tasks/javac.html#bootstrap
 * use a JDK 8 to compile and run the the tests with newer versions

But for this PR I am happy to have included the EA builds (even if they are 
failing right now)


---

[GitHub] jmeter pull request #438: Try running on java-ea for travis builds.

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/438

Try running on java-ea for travis builds.

## Description
Enable builds for Java EA on travis ci

## Motivation and Context
We should get early warnings, when JMeter doesn't run on EA versions of 
Java.

## How Has This Been Tested?
Will get tested by travis :)

## Screenshots (if appropriate):

## Types of changes
- New feature (non-breaking change which adds functionality)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter java-ea-for-travis

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/438.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #438


commit 81f80e459e84eaa338b1bdc5906837aaea9e953c
Author: Felix Schumacher 
Date:   2019-02-03T11:39:27Z

Try running on java-ea for travis builds.




---

[GitHub] jmeter pull request #437: Native properties

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/437

Native properties

## Description
Use native encoding for properties files in the source directories

## Motivation and Context
Currently the properties files are encoded using the native2ascii program 
as properties file read by java have to be in ASCII. Those encoded files are 
hard to read and error prone. By converting them to UTF-8 it should be easier 
to maintain.
The properties files will be encoded when the corresponding sources are 
compiled. 

## How Has This Been Tested?
Checked the generated properties file by hand and run JMeter in GUI mode.

## Types of changes

- New feature (non-breaking change which adds functionality)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter native-properties

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/437.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #437


commit e3efb950bde2f31270023833cf6dc87973a4b698
Author: Felix Schumacher 
Date:   2019-01-11T17:38:51Z

Use native2ascii while compiling the sources.

commit 53571eedd60ad2e3080ee666b7e41cc6fc71c488
Author: Felix Schumacher 
Date:   2019-01-11T17:41:44Z

Converted properties to utf8

Used the tool native2ascii to convert the ascii encoded properties
to utf-8. It should now be easier for language translators to edit those
files.

$ find src/ -name "*.properties"  | while read i
  do
native2ascii -reverse $i $i.new && mv $i.new $i
  done




---

[GitHub] jmeter pull request #431: Class#newInstance deprecation with Java 9

[FSchumacher]

Github user FSchumacher closed the pull request at:

https://github.com/apache/jmeter/pull/431


---

[GitHub] jmeter issue #431: Class#newInstance deprecation with Java 9

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/431
  
> According to 
https://docs.oracle.com/javase/9/docs/api/java/lang/Class.html
> It should be replaced by
> ` clazz.getDeclaredConstructor().newInstance()`
> see also 
https://stackoverflow.com/questions/53257073/java-9-replace-class-newinstance

Thanks for the info, I read the same article before I started and then made 
such a mess :) I prepared a new pull request #435 and will close this one.


---

[GitHub] jmeter pull request #435: Get rid of deprecated new instance calls

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/435

Get rid of deprecated new instance calls

## Description
This basically is an update to #431 

Get rid of calls to `Class#newInstance()` by replacing them with 
`Class#getDeclaredConstructor()#newInstance()`

And while I edited the classes I cleaned them up a bit, when eclipse showed 
warnings. I think those snippets should be committed separately, but I left 
them in here, so that they can be discussed if necessary.

## Motivation and Context
Java 9 deprecates Class#newInstance(). 

## How Has This Been Tested?
Tests should run when github merges this for testing.

## Screenshots (if appropriate):

## Types of changes

- cleanup

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter 
get-rid-of-deprecated-newInstance-calls

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/435.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #435


commit 9dabe9afee1af634f731226caede65c2b1fcac74
Author: Felix Schumacher 
Date:   2018-11-27T19:11:02Z

Use StringUtils.isNotBlank to make intent clearer

commit 5db20ea5a86fed37e3bb523df86bb48fceb69fe4
Author: Felix Schumacher 
Date:   2018-11-27T19:15:06Z

Replace deprecated newInstance calls

commit 9ea1b7b469892bd761083f22da33183f06f53a97
Author: Felix Schumacher 
Date:   2018-11-27T19:16:09Z

Use log format string

commit c21bbd2a0776dab9cf20e19b574a503cc7df7ffc
Author: Felix Schumacher 
Date:   2018-11-27T19:21:08Z

Get rid of deprecated newInstance call

commit c4a32b8a92b63aacd23785f6c8292f0aeaf4a72d
Author: Felix Schumacher 
Date:   2018-11-27T19:25:45Z

Get rid of deprecated newInstance call

Clarify the thrown exceptions of the createThinkTime method, while we
are here

commit 90ac82643bb8ceb56184c0037fac18eee1232d23
Author: Felix Schumacher 
Date:   2018-11-27T19:27:55Z

Get rid of deprecated newInstance call

commit dc04575bf94388ccf1eaef3d88a82ced10643ed2
Author: Felix Schumacher 
Date:   2018-11-27T19:30:49Z

Get rid of deprecated newInstance call

commit 99dfb4999d5528d087256c7b95edf26fd8402b9d
Author: Felix Schumacher 
Date:   2018-11-27T19:32:32Z

Use log format strings

commit 632a30f999d70fc6436f6f9b6b46fa390f941175
Author: Felix Schumacher 
Date:   2018-11-27T19:35:34Z

Simplify code.

With the introduction of varargs this can be written simpler. We don't
have to construct arrays here anymore.

commit 59646b49f1dc4d8c92b9691c957678dd688e583a
Author: Felix Schumacher 
Date:   2018-11-27T19:36:19Z

Use Java conventions for names of parameters

commit 100bf933a31fa61e19b310f83d2d8ead5469a983
Author: Felix Schumacher 
Date:   2018-11-27T19:37:53Z

Use StringUtils#isNotBlank to make intent clearer

commit 08413fb9a3c4730773cbff6eccbbdb5b262f33c3
Author: Felix Schumacher 
Date:   2018-11-27T19:40:46Z

Get rid of deprecated newInstance call

commit f10891f630e1539c71f42b4ce786465a332c4e45
Author: Felix Schumacher 
Date:   2018-11-27T19:48:25Z

Extract code to create BackendListenerClient from a classname

commit e4e23ed9e546eb85c7e258124142d4a54e0a73ff
Author: Felix Schumacher 
Date:   2018-11-27T19:56:51Z

Extract code into private methods to make intent clearer

commit 78f08709d44f10ec2871ab4120f3b805fba4d021
Author: Felix Schumacher 
Date:   2018-11-27T19:59:23Z

Use StringUtils.isNotBlank to make intent of code clearer

commit 2d41ba0abd738c02b9188136cfb4b04715698ce7
Author: Felix Schumacher 
Date:   2018-11-27T20:02:22Z

Get rid of deprecated newInstance call

commit cc882dfb64d1f354ed6bfc4204271ccdee75229f
Author: Felix Schumacher 
Date:   2018-11-27T20:04:38Z

Get rid of deprecated newInstance calls

commit 3e7030da36783fd140c3a3a7ba33ae4615c2b865
Author: Felix Schumacher 
Date:   2018-11-27T20:06:23Z

Get rid of deprecated newInstance calls

commit ee0536082eadb703f97f304420dc4d18e5aaa1d8
Author: Felix Schumacher 
Date:   2018-11-27T20:07:07Z

Use log string formats

commit d78b7c490611483d8717126c8f56279f7f2212ef
Author: Felix Schumacher 
Date:   2018-11-27T20:11:23Z

Get rid of deprecated newInstance call

commit 39b104c6a31b22727b23faa7dcedeb56a8e2e4e1
Author: Felix Schumacher 
Date:   2018-11-27T20:12:49Z

Get rid of deprecated newInstance call

commit 92530c4ae1e5d5755d2b5e4c988b8acf3bb3f380
Author: Felix Schumacher 
Date:   2018-11-27T20:14:20Z

Get rid of deprecated newInstance call

commit 9f74643e9169748a045299e2cf4ee948d7ce9547
Author: Felix Schumacher 

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r236800696
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -137,67 +160,138 @@ private void checkDirtyAndLoad(final ActionEvent 
actionEvent)
 if (template == null) {
 return;
 }
+
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
 // reload the templates before loading
+
 final boolean isTestPlan = template.isTestPlan();
 // Check if the user wants to drop any changes
-if (isTestPlan) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.CHECK_DIRTY));
-GuiPackage guiPackage = GuiPackage.getInstance();
-if (guiPackage.isDirty()) {
-// Check if the user wants to create from template
-int response = 
JOptionPane.showConfirmDialog(GuiPackage.getInstance().getMainFrame(),
-
JMeterUtils.getResString("cancel_new_from_template"), // $NON-NLS-1$
-JMeterUtils.getResString("template_load?"),  // 
$NON-NLS-1$
-JOptionPane.YES_NO_CANCEL_OPTION,
-JOptionPane.QUESTION_MESSAGE);
-if(response == JOptionPane.YES_OPTION) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), ActionNames.SAVE));
-}
-if (response == JOptionPane.CLOSED_OPTION || response == 
JOptionPane.CANCEL_OPTION) {
-return; // Don't clear the plan
-}
-}
+if (isTestPlan && !checkDirty(actionEvent)) {
+return;
 }
 ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.STOP_THREAD));
 final File parent = template.getParent();
-final File fileToCopy = parent != null 
+File fileToCopy = parent != null 
   ? new File(parent, template.getFileName())
-  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());   
-Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, false);
-this.setVisible(false);
+  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());
+replaceTemplateParametersAndLoad(actionEvent, template, 
isTestPlan, fileToCopy);
+}
+
+/**
+ * @param actionEvent {@link ActionEvent}
+ * @param template {@link Template} definition
+ * @param isTestPlan If it's a full test plan or a part
+ * @param templateFile Template file to load
+ */
+void replaceTemplateParametersAndLoad(final ActionEvent actionEvent, 
final Template template,
+final boolean isTestPlan, File templateFile) {
+File temporaryGeneratedFile = null;
+try {
+// handle customized templates (the .jmx.fmkr files)
+if (template.getParameters() != null && 
!template.getParameters().isEmpty()) {
+File jmxFile = new File(templateFile.getAbsolutePath());
+Map userParameters = getUserParameters();
+Configuration templateCfg = 
TemplateUtil.getTemplateConfig();
+try {
+temporaryGeneratedFile = 
File.createTempFile(template.getName(), ".output");
+templateFile = temporaryGeneratedFile;
+TemplateUtil.processTemplate(jmxFile, 
temporaryGeneratedFile, templateCfg, userParameters);
+} catch (IOException | TemplateException ex) {
+log.error("Error generating output file {} from 
template {}", temporaryGeneratedFile, jmxFile, ex);
+return;
+}
+}
+Load.loadProjectFile(actionEvent, templateFile, !isTestPlan, 
false);
+this.dispose();
+} finally {
+if (temporaryGeneratedFile != null && 
!temporaryGeneratedFile.delete()) {
+log.warn("Could not delete generated output file {} from 
template {}", temporaryGeneratedFile, templateFile);
+}
+}
+}
+
+/**
+ * @param actionEvent {@link ActionEvent}
+ * @return true if plan is not dirty or has been saved 
+ */
+boolean checkDirty(final ActionEvent actionEvent) {
--- End diff --

Any reason for not using `private`?


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r236801357
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -137,67 +160,138 @@ private void checkDirtyAndLoad(final ActionEvent 
actionEvent)
 if (template == null) {
 return;
 }
+
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
 // reload the templates before loading
+
 final boolean isTestPlan = template.isTestPlan();
 // Check if the user wants to drop any changes
-if (isTestPlan) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.CHECK_DIRTY));
-GuiPackage guiPackage = GuiPackage.getInstance();
-if (guiPackage.isDirty()) {
-// Check if the user wants to create from template
-int response = 
JOptionPane.showConfirmDialog(GuiPackage.getInstance().getMainFrame(),
-
JMeterUtils.getResString("cancel_new_from_template"), // $NON-NLS-1$
-JMeterUtils.getResString("template_load?"),  // 
$NON-NLS-1$
-JOptionPane.YES_NO_CANCEL_OPTION,
-JOptionPane.QUESTION_MESSAGE);
-if(response == JOptionPane.YES_OPTION) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), ActionNames.SAVE));
-}
-if (response == JOptionPane.CLOSED_OPTION || response == 
JOptionPane.CANCEL_OPTION) {
-return; // Don't clear the plan
-}
-}
+if (isTestPlan && !checkDirty(actionEvent)) {
+return;
 }
 ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.STOP_THREAD));
 final File parent = template.getParent();
-final File fileToCopy = parent != null 
+File fileToCopy = parent != null 
   ? new File(parent, template.getFileName())
-  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());   
-Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, false);
-this.setVisible(false);
+  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());
+replaceTemplateParametersAndLoad(actionEvent, template, 
isTestPlan, fileToCopy);
+}
+
+/**
+ * @param actionEvent {@link ActionEvent}
+ * @param template {@link Template} definition
+ * @param isTestPlan If it's a full test plan or a part
+ * @param templateFile Template file to load
+ */
+void replaceTemplateParametersAndLoad(final ActionEvent actionEvent, 
final Template template,
+final boolean isTestPlan, File templateFile) {
+File temporaryGeneratedFile = null;
+try {
+// handle customized templates (the .jmx.fmkr files)
+if (template.getParameters() != null && 
!template.getParameters().isEmpty()) {
+File jmxFile = new File(templateFile.getAbsolutePath());
+Map userParameters = getUserParameters();
+Configuration templateCfg = 
TemplateUtil.getTemplateConfig();
+try {
+temporaryGeneratedFile = 
File.createTempFile(template.getName(), ".output");
+templateFile = temporaryGeneratedFile;
+TemplateUtil.processTemplate(jmxFile, 
temporaryGeneratedFile, templateCfg, userParameters);
+} catch (IOException | TemplateException ex) {
+log.error("Error generating output file {} from 
template {}", temporaryGeneratedFile, jmxFile, ex);
+return;
+}
+}
+Load.loadProjectFile(actionEvent, templateFile, !isTestPlan, 
false);
+this.dispose();
+} finally {
+if (temporaryGeneratedFile != null && 
!temporaryGeneratedFile.delete()) {
+log.warn("Could not delete generated output file {} from 
template {}", temporaryGeneratedFile, templateFile);
+}
+}
+}
+
+/**
+ * @param actionEvent {@link ActionEvent}
+ * @return true if plan is not dirty or has been saved 
--- End diff --

Somehow I think a method `checkDirty` would return `true` if plan is 
*dirty* and has not been saved. Can you think of a better name?


---

[GitHub] jmeter pull request #434: Truncate response message to avoid too large udp p...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/434#discussion_r236044166
  
--- Diff: 
src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java
 ---
@@ -196,12 +196,18 @@ private void addMetrics(String transaction, 
SamplerMetric metric) {
 }
 
 private void addErrorMetric(String transaction, String responseCode, 
String responseMessage, long count) {
+final int MAX_RES_CODE_LENGTH_FOR_UDP = 50;
--- End diff --

The rounding should probably done in a different pull request and it should 
probably take the magnitude of the number into account. That is the method 
taken by HdrHistogram as suggested to use in #380.


---

[GitHub] jmeter pull request #434: Truncate response message to avoid too large udp p...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/434#discussion_r236039975
  
--- Diff: 
src/components/org/apache/jmeter/visualizers/backend/influxdb/InfluxdbBackendListenerClient.java
 ---
@@ -196,12 +196,18 @@ private void addMetrics(String transaction, 
SamplerMetric metric) {
 }
 
 private void addErrorMetric(String transaction, String responseCode, 
String responseMessage, long count) {
+final int MAX_RES_CODE_LENGTH_FOR_UDP = 50;
--- End diff --

We seem to waste a lot of characters with the number conversion of the 
percentage values. They seem to be exact to about two digits behind the comma, 
but are written out to much more. Maybe we could use a better 
formatting/rounding there, too?


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235119001
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -207,19 +290,35 @@ private void init() { // WARNING: called from ctor so 
must not be overridden (i.
 public void actionPerformed(ActionEvent e) {
 final Object source = e.getSource();
 if (source == cancelButton) {
-this.setVisible(false);
-return;
+resetJDialog();
+this.dispose();
 } else if (source == applyTemplateButton) {
-checkDirtyAndLoad(e);
-} else if (source == reloadTemplateButton) {
-
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
+String selectedTemplate = templateList.getText();
+Template template = 
TemplateManager.getInstance().getTemplateByName(selectedTemplate);
+if(template.getParameters() != null && 
!template.getParameters().isEmpty()) {
+
this.setContentPane(configureParametersPanel(template.getParameters()));
+this.revalidate();
+}else {
+checkDirtyAndLoad(e);
+}
+} else if (source == reloadTemplateButton || source == previous) {
+resetJDialog();
+} else if(source == validateButton) {
--- End diff --

Same as with `for`. Spacepolice is calling :)


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235117129
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -137,67 +160,127 @@ private void checkDirtyAndLoad(final ActionEvent 
actionEvent)
 if (template == null) {
 return;
 }
+
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
 // reload the templates before loading
+
 final boolean isTestPlan = template.isTestPlan();
 // Check if the user wants to drop any changes
-if (isTestPlan) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.CHECK_DIRTY));
-GuiPackage guiPackage = GuiPackage.getInstance();
-if (guiPackage.isDirty()) {
-// Check if the user wants to create from template
-int response = 
JOptionPane.showConfirmDialog(GuiPackage.getInstance().getMainFrame(),
-
JMeterUtils.getResString("cancel_new_from_template"), // $NON-NLS-1$
-JMeterUtils.getResString("template_load?"),  // 
$NON-NLS-1$
-JOptionPane.YES_NO_CANCEL_OPTION,
-JOptionPane.QUESTION_MESSAGE);
-if(response == JOptionPane.YES_OPTION) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), ActionNames.SAVE));
-}
-if (response == JOptionPane.CLOSED_OPTION || response == 
JOptionPane.CANCEL_OPTION) {
-return; // Don't clear the plan
-}
-}
+if (isTestPlan && !checkDirty(actionEvent)) {
+return;
 }
 ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.STOP_THREAD));
 final File parent = template.getParent();
-final File fileToCopy = parent != null 
+File fileToCopy = parent != null 
   ? new File(parent, template.getFileName())
-  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());   
-Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, false);
-this.setVisible(false);
+  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());
+File temporaryGeneratedFile = null;
--- End diff --

Could this and the following try block be extracted into a method? That 
would enable us to show the intent of the code block by using the method name.


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235125307
  
--- Diff: 
src/core/org/apache/jmeter/gui/action/template/TemplateManager.java ---
@@ -149,19 +109,107 @@ public TemplateManager reset() {
 } else {
 if (log.isWarnEnabled()) {
 log.warn("Ignoring template file:'{}' as it 
does not exist or is not readable",
-f.getAbsolutePath());
+file.getAbsolutePath());
 }
 }
 } catch(Exception ex) {
 if (log.isWarnEnabled()) {
-log.warn("Ignoring template file:'{}', an error 
occurred parsing the file", f.getAbsolutePath(),
+log.warn("Ignoring template file:'{}', an error 
occurred parsing the file", file.getAbsolutePath(),
 ex);
 }
 } 
 }
 }
 return temps;
 }
+
+public final class LoggingErrorHandler implements ErrorHandler {
+private Logger logger;
+
+public LoggingErrorHandler(Logger logger) {
+this.logger = logger;
+}
+@Override
+public void error(SAXParseException ex) throws SAXException {
+throw ex;
+}
+
+@Override
+public void fatalError(SAXParseException ex) throws SAXException {
+throw ex;
+}
+
+@Override
+public void warning(SAXParseException ex) throws SAXException {
+logger.warn("Warning", ex);
+}
+}
+
+public static class DefaultEntityResolver implements EntityResolver {
+public DefaultEntityResolver() {
+super();
+}
+
+public InputSource resolveEntity(String publicId, String systemId) 
throws SAXException, IOException {
+if(systemId.endsWith("templates.dtd")) {
+return new 
InputSource(TemplateManager.class.getResourceAsStream("/org/apache/jmeter/gui/action/template/templates.dtd"));
+} else {
+return null;
+}
+}
+}
+
+public Map parseTemplateFile(File file) throws 
IOException, SAXException, ParserConfigurationException{
+DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+dbf.setValidating(true);
+dbf.setNamespaceAware(true);
+
dbf.setFeature("http://xml.org/sax/features/external-general-entities";, false);
+
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities";, 
false);
+DocumentBuilder bd = dbf.newDocumentBuilder();
+bd.setEntityResolver(new DefaultEntityResolver());
+LoggingErrorHandler errorHandler = new LoggingErrorHandler(log);
--- End diff --

We could hand the filename to the error handler, so that it could be used 
in the error messages.


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235118443
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -137,67 +160,127 @@ private void checkDirtyAndLoad(final ActionEvent 
actionEvent)
 if (template == null) {
 return;
 }
+
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
 // reload the templates before loading
+
 final boolean isTestPlan = template.isTestPlan();
 // Check if the user wants to drop any changes
-if (isTestPlan) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.CHECK_DIRTY));
-GuiPackage guiPackage = GuiPackage.getInstance();
-if (guiPackage.isDirty()) {
-// Check if the user wants to create from template
-int response = 
JOptionPane.showConfirmDialog(GuiPackage.getInstance().getMainFrame(),
-
JMeterUtils.getResString("cancel_new_from_template"), // $NON-NLS-1$
-JMeterUtils.getResString("template_load?"),  // 
$NON-NLS-1$
-JOptionPane.YES_NO_CANCEL_OPTION,
-JOptionPane.QUESTION_MESSAGE);
-if(response == JOptionPane.YES_OPTION) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), ActionNames.SAVE));
-}
-if (response == JOptionPane.CLOSED_OPTION || response == 
JOptionPane.CANCEL_OPTION) {
-return; // Don't clear the plan
-}
-}
+if (isTestPlan && !checkDirty(actionEvent)) {
+return;
 }
 ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.STOP_THREAD));
 final File parent = template.getParent();
-final File fileToCopy = parent != null 
+File fileToCopy = parent != null 
   ? new File(parent, template.getFileName())
-  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());   
-Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, false);
-this.setVisible(false);
+  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());
+File temporaryGeneratedFile = null;
+try {
+// handle customized templates (the .jmx.fmkr files)
+if (template.getParameters() != null && 
!template.getParameters().isEmpty()) {
+File jmxFile = new File(fileToCopy.getAbsolutePath());
+Map userParameters = getUserParameters();
+Configuration templateCfg = 
TemplateUtil.getTemplateConfig();
+try {
+temporaryGeneratedFile = 
File.createTempFile(template.getName(), ".output");
+fileToCopy = temporaryGeneratedFile;
+TemplateUtil.processTemplate(jmxFile, 
temporaryGeneratedFile, templateCfg, userParameters);
+} catch (IOException | TemplateException ex) {
+log.error("Error generating output file {} from 
template {}", temporaryGeneratedFile, jmxFile, ex);
+return;
+}
+}
+Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, 
false);
+this.dispose();
+} finally {
+if(temporaryGeneratedFile != null && 
!temporaryGeneratedFile.delete()) {
+log.warn("Could not delete generated output file {} from 
template {}", temporaryGeneratedFile, fileToCopy);
+}
+}
+}
+
+/**
+ * @param actionEvent
+ * @return true if we can continue
+ */
+boolean checkDirty(final ActionEvent actionEvent) {
+ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.CHECK_DIRTY));
+GuiPackage guiPackage = GuiPackage.getInstance();
+if (guiPackage.isDirty()) {
+// Check if the user wants to create from template
+int response = 
JOptionPane.showConfirmDialog(GuiPackage.getInstance().getMainFrame(),
+JMeterUtils.getResString("cancel_new_from_template"), 
// $NON-NLS-1$
+JMeterUtils.getResString("template_load?"),  // 
$NON-NLS-1$
+JOptionPane.YES_NO_

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235125815
  
--- Diff: 
src/core/org/apache/jmeter/gui/action/template/TemplateManager.java ---
@@ -149,19 +109,107 @@ public TemplateManager reset() {
 } else {
 if (log.isWarnEnabled()) {
 log.warn("Ignoring template file:'{}' as it 
does not exist or is not readable",
-f.getAbsolutePath());
+file.getAbsolutePath());
 }
 }
 } catch(Exception ex) {
 if (log.isWarnEnabled()) {
-log.warn("Ignoring template file:'{}', an error 
occurred parsing the file", f.getAbsolutePath(),
+log.warn("Ignoring template file:'{}', an error 
occurred parsing the file", file.getAbsolutePath(),
 ex);
 }
 } 
 }
 }
 return temps;
 }
+
+public final class LoggingErrorHandler implements ErrorHandler {
+private Logger logger;
+
+public LoggingErrorHandler(Logger logger) {
+this.logger = logger;
+}
+@Override
+public void error(SAXParseException ex) throws SAXException {
+throw ex;
+}
+
+@Override
+public void fatalError(SAXParseException ex) throws SAXException {
+throw ex;
+}
+
+@Override
+public void warning(SAXParseException ex) throws SAXException {
+logger.warn("Warning", ex);
+}
+}
+
+public static class DefaultEntityResolver implements EntityResolver {
+public DefaultEntityResolver() {
+super();
+}
+
+public InputSource resolveEntity(String publicId, String systemId) 
throws SAXException, IOException {
+if(systemId.endsWith("templates.dtd")) {
+return new 
InputSource(TemplateManager.class.getResourceAsStream("/org/apache/jmeter/gui/action/template/templates.dtd"));
+} else {
+return null;
+}
+}
+}
+
+public Map parseTemplateFile(File file) throws 
IOException, SAXException, ParserConfigurationException{
+DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+dbf.setValidating(true);
+dbf.setNamespaceAware(true);
+
dbf.setFeature("http://xml.org/sax/features/external-general-entities";, false);
+
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities";, 
false);
+DocumentBuilder bd = dbf.newDocumentBuilder();
+bd.setEntityResolver(new DefaultEntityResolver());
+LoggingErrorHandler errorHandler = new LoggingErrorHandler(log);
+bd.setErrorHandler(errorHandler);
+Document document = bd.parse(file.getAbsolutePath());
+document.getDocumentElement().normalize();
+Map templates = new TreeMap<>();
+NodeList templateNodes = document.getElementsByTagName("template");
+for (int i = 0; i < templateNodes.getLength(); i++) {
+Node node = templateNodes.item(i);
+parseTemplateNode(templates, node);
+}
+return templates;
+}
+
+/**
+ * @param templates
--- End diff --

either fill in the javadoc, or leave it out completely.


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235127351
  
--- Diff: 
src/core/org/apache/jmeter/gui/action/template/TemplateManager.java ---
@@ -149,19 +109,107 @@ public TemplateManager reset() {
 } else {
 if (log.isWarnEnabled()) {
 log.warn("Ignoring template file:'{}' as it 
does not exist or is not readable",
-f.getAbsolutePath());
+file.getAbsolutePath());
 }
 }
 } catch(Exception ex) {
 if (log.isWarnEnabled()) {
-log.warn("Ignoring template file:'{}', an error 
occurred parsing the file", f.getAbsolutePath(),
+log.warn("Ignoring template file:'{}', an error 
occurred parsing the file", file.getAbsolutePath(),
 ex);
 }
 } 
 }
 }
 return temps;
 }
+
+public final class LoggingErrorHandler implements ErrorHandler {
+private Logger logger;
+
+public LoggingErrorHandler(Logger logger) {
+this.logger = logger;
+}
+@Override
+public void error(SAXParseException ex) throws SAXException {
+throw ex;
+}
+
+@Override
+public void fatalError(SAXParseException ex) throws SAXException {
+throw ex;
+}
+
+@Override
+public void warning(SAXParseException ex) throws SAXException {
+logger.warn("Warning", ex);
+}
+}
+
+public static class DefaultEntityResolver implements EntityResolver {
+public DefaultEntityResolver() {
+super();
+}
+
+public InputSource resolveEntity(String publicId, String systemId) 
throws SAXException, IOException {
+if(systemId.endsWith("templates.dtd")) {
+return new 
InputSource(TemplateManager.class.getResourceAsStream("/org/apache/jmeter/gui/action/template/templates.dtd"));
+} else {
+return null;
+}
+}
+}
+
+public Map parseTemplateFile(File file) throws 
IOException, SAXException, ParserConfigurationException{
+DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+dbf.setValidating(true);
+dbf.setNamespaceAware(true);
+
dbf.setFeature("http://xml.org/sax/features/external-general-entities";, false);
+
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities";, 
false);
+DocumentBuilder bd = dbf.newDocumentBuilder();
+bd.setEntityResolver(new DefaultEntityResolver());
+LoggingErrorHandler errorHandler = new LoggingErrorHandler(log);
+bd.setErrorHandler(errorHandler);
+Document document = bd.parse(file.getAbsolutePath());
+document.getDocumentElement().normalize();
+Map templates = new TreeMap<>();
+NodeList templateNodes = document.getElementsByTagName("template");
+for (int i = 0; i < templateNodes.getLength(); i++) {
+Node node = templateNodes.item(i);
+parseTemplateNode(templates, node);
+}
+return templates;
+}
+
+/**
+ * @param templates
+ * @param templateNode
+ */
+void parseTemplateNode(Map templates, Node 
templateNode) {
+if (templateNode.getNodeType() == Node.ELEMENT_NODE) {
+Template template = new Template();
+Element element =  (Element) templateNode;
+
template.setTestPlan("true".equals(element.getAttribute("isTestPlan")));
+
template.setName(element.getElementsByTagName("name").item(0).getTextContent());
--- End diff --

maybe introduce a helper method, that extracts the text content for the 
first element found:
`template.setName(textOfFirstTag(element, "name"))`
This seems to be repeated three times.


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235121173
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -207,19 +290,35 @@ private void init() { // WARNING: called from ctor so 
must not be overridden (i.
 public void actionPerformed(ActionEvent e) {
 final Object source = e.getSource();
 if (source == cancelButton) {
-this.setVisible(false);
-return;
+resetJDialog();
+this.dispose();
 } else if (source == applyTemplateButton) {
-checkDirtyAndLoad(e);
-} else if (source == reloadTemplateButton) {
-
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
+String selectedTemplate = templateList.getText();
+Template template = 
TemplateManager.getInstance().getTemplateByName(selectedTemplate);
+if(template.getParameters() != null && 
!template.getParameters().isEmpty()) {
--- End diff --

I find negations harder to read. I would prefer to use `params == null || 
params.isEmpty()` and inverse the code blocks and/or introduce a private method 
`isEmpty(template.getParameters)`.


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235114098
  
--- Diff: bin/templates/recording.jmx ---
@@ -90,32 +101,32 @@
   
 
 
-  .*toolbar\.live\.com.*
   (?i).*\.(bmp|css|js|gif|ico|jpe?g|png|swf|eot|otf|ttf|mp4|woff|woff2)
-  update\.microsoft\.com.*
-  toolbarqueries\.google\..*
-  clients.*\.google.*
-  api\.bing\.com.*
+  www\.download\.windowsupdate\.com.*
--- End diff --

I don't think these changes have anything to do with the feature, would you 
like to commit these in an extra step?


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235117924
  
--- Diff: src/core/org/apache/jmeter/gui/action/SelectTemplatesDialog.java 
---
@@ -137,67 +160,127 @@ private void checkDirtyAndLoad(final ActionEvent 
actionEvent)
 if (template == null) {
 return;
 }
+
templateList.setValues(TemplateManager.getInstance().reset().getTemplateNames());
 // reload the templates before loading
+
 final boolean isTestPlan = template.isTestPlan();
 // Check if the user wants to drop any changes
-if (isTestPlan) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.CHECK_DIRTY));
-GuiPackage guiPackage = GuiPackage.getInstance();
-if (guiPackage.isDirty()) {
-// Check if the user wants to create from template
-int response = 
JOptionPane.showConfirmDialog(GuiPackage.getInstance().getMainFrame(),
-
JMeterUtils.getResString("cancel_new_from_template"), // $NON-NLS-1$
-JMeterUtils.getResString("template_load?"),  // 
$NON-NLS-1$
-JOptionPane.YES_NO_CANCEL_OPTION,
-JOptionPane.QUESTION_MESSAGE);
-if(response == JOptionPane.YES_OPTION) {
-ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), ActionNames.SAVE));
-}
-if (response == JOptionPane.CLOSED_OPTION || response == 
JOptionPane.CANCEL_OPTION) {
-return; // Don't clear the plan
-}
-}
+if (isTestPlan && !checkDirty(actionEvent)) {
+return;
 }
 ActionRouter.getInstance().doActionNow(new 
ActionEvent(actionEvent.getSource(), actionEvent.getID(), 
ActionNames.STOP_THREAD));
 final File parent = template.getParent();
-final File fileToCopy = parent != null 
+File fileToCopy = parent != null 
   ? new File(parent, template.getFileName())
-  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());   
-Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, false);
-this.setVisible(false);
+  : new File(JMeterUtils.getJMeterHome(), 
template.getFileName());
+File temporaryGeneratedFile = null;
+try {
+// handle customized templates (the .jmx.fmkr files)
+if (template.getParameters() != null && 
!template.getParameters().isEmpty()) {
+File jmxFile = new File(fileToCopy.getAbsolutePath());
+Map userParameters = getUserParameters();
+Configuration templateCfg = 
TemplateUtil.getTemplateConfig();
+try {
+temporaryGeneratedFile = 
File.createTempFile(template.getName(), ".output");
+fileToCopy = temporaryGeneratedFile;
+TemplateUtil.processTemplate(jmxFile, 
temporaryGeneratedFile, templateCfg, userParameters);
+} catch (IOException | TemplateException ex) {
+log.error("Error generating output file {} from 
template {}", temporaryGeneratedFile, jmxFile, ex);
+return;
+}
+}
+Load.loadProjectFile(actionEvent, fileToCopy, !isTestPlan, 
false);
+this.dispose();
+} finally {
+if(temporaryGeneratedFile != null && 
!temporaryGeneratedFile.delete()) {
+log.warn("Could not delete generated output file {} from 
template {}", temporaryGeneratedFile, fileToCopy);
+}
+}
+}
+
+/**
+ * @param actionEvent
+ * @return true if we can continue
--- End diff --

This comment seems odd to me. If it returns `true` I would think, that we 
found the state to be `dirty`. Why would I want to continue in such a state? 


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235127891
  
--- Diff: src/core/org/apache/jmeter/resources/messages.properties ---
@@ -1206,6 +1206,7 @@ teardown_on_shutdown=Run tearDown Thread Groups after 
shutdown of main threads
 template_choose=Select Template
 template_create_from=Create
 template_field=Template ($i$ where i is capturing group number, starts at 
1):
+template_fill_parameters=Fill your parameters \:
--- End diff --

In English typography there is no space before the colon :)


---

[GitHub] jmeter pull request #432: Bug 62870 / Templates : Add ability to provide par...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/432#discussion_r235128465
  
--- Diff: src/core/org/apache/jmeter/util/TemplateUtil.java ---
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.jmeter.util;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+
+import freemarker.template.Configuration;
+import freemarker.template.TemplateException;
+import freemarker.template.TemplateExceptionHandler;
+
+/**
+ * Class used to process freemarkers templates
+ * @since 5.1
+ */
+public final class TemplateUtil {
+
+private static Configuration templateConfiguration = init();
+
+private TemplateUtil() {
+super();
+}
+
+private static Configuration init() {
+Configuration templateConfiguration = new 
Configuration(Configuration.getVersion());
+
templateConfiguration.setDefaultEncoding(StandardCharsets.UTF_8.name());
+
templateConfiguration.setInterpolationSyntax(Configuration.SQUARE_BRACKET_INTERPOLATION_SYNTAX);
+
templateConfiguration.setTemplateExceptionHandler(TemplateExceptionHandler.RETHROW_HANDLER);
+return templateConfiguration;
+}
+
+/**
+ * Give a basic templateConfiguration
+ * @return a Configuration
+ */
+public static Configuration getTemplateConfig() {
+return templateConfiguration;
+}
+
+/**
+ * Process a given freemarker template and put its result in a new 
folder.
+ * 
+ * @param template : file that contains the freemarker template to 
process
--- End diff --

Is the colon needed here? What does the rendered javadoc look like? 


---

[GitHub] jmeter issue #432: Bug 62870 / Templates : Add ability to provide parameters

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/432
  
This seems to be quite a big patch. Do you think two days are enough to 
have a good look at it?


---

[GitHub] jmeter issue #431: Class#newInstance deprecation with Java 9

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/431
  
Well, I don't know, if we can generalize it, but it is surely the first 
hits, only. There will be more, when I have slept a bit.


---

[GitHub] jmeter pull request #431: Class#newInstance deprecation with Java 9

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/431

Class#newInstance deprecation with Java 9

## Description
Change invocation of Class#newInstance to Class#getConstructor#newInstance
And do some cleanup using StringUtils.isNotBlank

## Motivation and Context
Java 9 deprecates Class#newInstance, so this will get rid of the 
deprecation warnings

## How Has This Been Tested?
ran tests

## Screenshots (if appropriate):

## Types of changes

- Bug fix (non-breaking change which fixes an issue)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter newinstance-deprecation

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/431.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #431


commit 3d8aa9c075770a669c9e8eb441e2d30d211b8fa9
Author: Felix Schumacher 
Date:   2018-11-11T21:16:06Z

Class#newInstance will be deprecated with Java 9

commit 224c7f41ff0c282baef55e6c5658a543aa4ee308
Author: Felix Schumacher 
Date:   2018-11-11T21:18:22Z

Get rid of deprecation warning of Class#newInstance

commit 9807e46ecc40bbb655fc847f429467bc3657d366
Author: Felix Schumacher 
Date:   2018-11-11T21:23:16Z

Get rid of deprecation warning for Class#newInstance

commit b36e2202d48b427d1dca7ecf62ccc23f9ae24f48
Author: Felix Schumacher 
Date:   2018-11-11T21:27:22Z

Get rid of deprecation warnings for Class#newInstance




---

[GitHub] jmeter issue #401: Bug 62257 - Expand/Collapse short key doesn't work with n...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/401
  
Do you think, we should support both key combinations, or should we rather 
fix the documentation to use the working combinations?


---

[GitHub] jmeter issue #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/396
  
Thanks for your contribution.

I still don't know, why we get `null` for `CTRL`+`6` on windows, but the 
workaround seems to work, so I have included it and hope somebody can explain 
the real bug to me.


---

[GitHub] jmeter issue #412: add i18nEdit zh-cn

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/412
  
Thank you very much for your contribution.

I wonder what is the difference between the different characters for a 
comma that you used, for example `beanshell、javascript、jexl` versus 
`beanshell， javascript， jexl`.

Also I didn't commit the change to `xpath_assertion_test` as I think the 
whitespace at the end of the line was deliberately there and should not be 
removed.


---

[GitHub] jmeter pull request #409: Cleanup saveservice

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/409

Cleanup saveservice

## Description
Cleanup code in `SaveService`.

## Motivation and Context
I always like to look at the warnings in the IDE to see, where the code has 
problems. If there are too many warnings, I tend to oversee the real dangerous 
ones. So this pull request tries to get rid of the ones that are easy to fix 
and hopefully clearer to read.
 
## How Has This Been Tested?
Ran the unit tests.

## Screenshots (if appropriate):

## Types of changes
- Code cleanup / refactoring

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter cleanup-saveservice

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/409.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #409


commit 7bfc83f90d7dd0ed3f279e83b77a1470c6ff6b97
Author: Felix Schumacher 
Date:   2018-10-14T11:00:50Z

Clarify that we want a single class constructor.

commit dc4b0746b46e8c30bfecd72dd695837cdb83c7c3
Author: Felix Schumacher 
Date:   2018-10-14T11:05:18Z

Refactor nested try block to named function

commit bba5780e04614c923f78511aad4098b6e320d681
Author: Felix Schumacher 
Date:   2018-10-14T11:09:22Z

Reduce duplicated code by using a variable

commit 54476f922d17b46cf3ec5f9dc57db5fd543df659
Author: Felix Schumacher 
Date:   2018-10-14T11:10:10Z

Use correct order of keywords as specified in JLS

commit 6ae11df28b77b6655c4af8b340273d91f5413eef
Author: Felix Schumacher 
Date:   2018-10-14T11:11:55Z

Return result directly.




---

[GitHub] jmeter issue #408: Display an info message, when no JavaFX is found and Rend...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/408
  
The message from a `ClassNotFoundException` is checked for `javafx`. I 
think it is safe to assume that any message on that kind of error hints to a 
missing JavaFX installation, when it contains the string `javafx`.


---

[GitHub] jmeter issue #401: Bug 62257 - Expand/Collapse short key doesn't work with n...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/401
  
Thanks for your contribution and for the correction of your name ;)

Have you tried `shift`+`left cursor` and `shift`+`right cursor`? They work 
fine on my install. Why would we want to have more than one keyboard short cut 
for the same functionality?


---

[GitHub] jmeter pull request #408: Display an info message, when no JavaFX is found a...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/408

Display an info message, when no JavaFX is found and RenderInBrowser …

…is used

## Description
Currently we will log a full exception stack trace in warning level, when no
JavaFX was found and `RenderInBrowser` is initialized.

Reduce the log level to info and display a short message that the user 
should
install JavaFX in their Java environment, if they want to use the renderer.

## Motivation and Context
The user is probably scared when an exception stack trace is shown in their 
log file. This change should help the user to take the needed steps more easily.

## How Has This Been Tested?
Tested through GUI.

## Types of changes
- Bug fix (non-breaking change which fixes an issue)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter no-exceptions-on-browser

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/408.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #408


commit 002d427c8e4553511be2c85ade4c2ffc3c2e1eae
Author: Felix Schumacher 
Date:   2018-10-13T14:49:46Z

Display an info message, when no JavaFX is found and RenderInBrowser is used

Currently we will log a full exception stack trace in warning leve, when no
JavaFX was found and RenderInBrowser is initialized.

Reduce the log level to info and display a short message that the user 
should
install JavaFX in their Java environment, if they want to use the renderer.




---

[GitHub] jmeter pull request #407: Render min and max values in Summary Report as `#N...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/407

Render min and max values in Summary Report as `#N/A`

## Description
Render min and max values in Summary Report as `#N/A`

## Motivation and context
In the Summary Report min and max values are initialized with
`Long#MAX_VALUE` and `Long#MIN_VALUE` to work easily with them.

Those initial values are looking weird in the rendered view.
Hide them by replacing the extrema by the text `#N/A`.

Bugzilla Id: 62822

## How Has This Been Tested?
Used the GUI to display `Summary Report` and run a simple test.

## Types of changes

- Bug fix (non-breaking change which fixes an issue)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter 
bug-62822-default-values-in-summary-report

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/407.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #407


commit 6fd61d3da36167d577ba313bdea61eebadec
Author: Felix Schumacher 
Date:   2018-10-13T13:37:11Z

Render min an max values in Summary Report as `#N/A`

In the Summary Report min and max values are initialized with
`Long#MAX_VALUE` and `Long#MIN_VALUE` to work easily with them.

Those initial values are looking weird in the rendered view.
Hide them by replacing the extrema by the text `#N/A`.

Bugzilla Id: 62822




---

[GitHub] jmeter pull request #406: Document usage of a security manager for remote te...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/406

Document usage of a security manager for remote tests

## Description
Remote tests can be secured even further, when running with enabled 
security manager.

## Motivation and Context
Especially RMI has gained more attraction by attackers, so give users 
another tool to defend against attacks.

## How Has This Been Tested?
A simple test plan has been run remotely with the documented steps. To be 
honest the documentation has been done after the tests. So it would be great if 
more people can follow the documentation and try their tests.

## Screenshots (if appropriate):

## Types of changes

- Documentation

## Checklist:


- [x] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter security-manager

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/406.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #406


commit 5514943ac9c8c54277ab92ad4913ed379d27590d
Author: Felix Schumacher 
Date:   2018-10-13T11:15:51Z

Document usage of a security manager for remote tests




---

[GitHub] jmeter pull request #405: Use SHA-512 checksums instead of MD5 to verify jar...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/405

Use SHA-512 checksums instead of MD5 to verify jar downloads

## Description
Change the checksums for the downloaded jars from MD5 to SHA-512.

## Motivation and Context
MD5 is considered broken, so we should verify downloaded artefacts for our 
build process with a non broken checksum. SHA-512 is considered safe -- at the 
moment.
## How Has This Been Tested?
`ant download_jars` and other download targets have been run without 
problems.
 
## Screenshots (if appropriate):

## Types of changes

- Bug fix (non-breaking change which fixes an issue)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.
 No documentation found for the old md5 checksums construct.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter sha-for-downloads

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/405.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #405


commit 0341693b24d868e9d5e70a121463feed375934fa
Author: Felix Schumacher 
Date:   2018-10-11T19:00:52Z

Use SHA-512 checksums instead of MD5 to verify jar downloads




---

[GitHub] jmeter pull request #404: Use log string format instead of doing the concate...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/404

Use log string format instead of doing the concatenation ourselves

## Description
Simplify code.

## Motivation and Context
Make code a bit more readable by using placeholders for the
parameters. We can get rid of a few `if` statements, where
parameters are used directly in the log statements.

## How Has This Been Tested?
No functional change.

## Screenshots (if appropriate):

## Types of changes
- code cleanup

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter use-log-string-format

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/404.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #404


commit bf424d461e4fc16d2142865f92bbf8ac0e72c807
Author: Felix Schumacher 
Date:   2018-10-10T20:21:43Z

Use log string format instead of doing the concatenation ourselves

Make code a bit more readable by using placeholders for the
parameters. We can get rid of a few `if` statements, where
parameters are used directly in the log statements.




---

[GitHub] jmeter pull request #403: Use simpler code for "static" String concatenation

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/403

Use simpler code for "static" String concatenation

## Description
Simplify code for String concatenation

## Motivation and Context


Java is smart enough to compile simple string concatenation `"something" + 
x` to
the same code as `new StringBuilder("something").append(x).toString()`.
The latter is really hard to read, so use the simpler one.

While at it, use string formats for the logger (as it is some sort of string
concatenation, too). And since we use the string formatter, we don't have
to guard the log level anymore for the simple case.


## How Has This Been Tested?




## Screenshots (if appropriate):

## Types of changes

- code simplification, no functional change

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter 
simple-string-instead-stringbuilder

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/403.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #403


commit 79890d4312a6797a96624498545e02c1c35f98c1
Author: Felix Schumacher 
Date:   2018-10-10T19:43:01Z

Use simpler code for "static" String concatenation

Java is smart enough to compile simple string concatenation to
the same code as `new StringBuilder("something").append("other 
thing").toString()`.
The latter is really hard to read, so use the simpler one.

While at it, use string formats for the logger (as it is some sort of string
concatenation, too). And since we use the string formatter, we don't have
to guard the log level anymore for the simple case.




---

[GitHub] jmeter pull request #402: Use isEmpty instead of comparing size of a collect...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/402

Use isEmpty instead of comparing size of a collection to zero

## Description
Use `isEmpty()` on collections instead of comparing their size to zero

## Motivation and Context
The `isEmpty()` check can be done a lot faster than calculating the size of 
a collection. This is mainly a performance optimiziation, but it does make the 
intention of the code a bit clearer, too.

## How Has This Been Tested?
No functional change has been done, so the normal test suite should catch 
any unintended changes.

## Screenshots (if appropriate):

## Types of changes
- make code a bit clearer and perhaps a bit faster

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter use-isempty-insteadof-size

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/402.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #402


commit 32d125eabfc5f926a4d364cd7641b60899902305
Author: Felix Schumacher 
Date:   2018-10-06T13:11:26Z

Use isEmpty instead of comparing size of a collection to zero




---

[GitHub] jmeter issue #394: Fix IllegalArgumentException for File properties wich all...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/394
  
OK, so we really have to explicitly allow `null` to help you there, right? 
Does the last change help you?


---

[GitHub] jmeter issue #394: Fix IllegalArgumentException for File properties wich all...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/394
  
I have modified your code slightly where I think it helps readability. It 
should contain no functional difference to your code. It would be nice, if you 
could test the next nightly or current trunk.

Thanks again for your contribution.


---

[GitHub] jmeter issue #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/396
  
Just wanted to let you know, that you are not forgotten. I have now access 
to a windows 10 system and can reproduce the issue. But before I implement your 
fix, I would like to understand why `ctrl`+`6`doesn't work, as all other keys 
seem to work OK.


---

[GitHub] jmeter issue #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/396
  
Right, the events will never be the same, but still... I wonder why 
`ActionEvent` is `null` in the first place. This seems to be a bug in the JDK, 
it doesn't make sense to call an action listener without a valid event.
Can you post a stacktrace when the event is `null` (and maybe return early 
from the function and see, if the next event in the action listener is the 
*correct* one)?


---

[GitHub] jmeter pull request #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/396#discussion_r212836512
  
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
 
 @Override
 public void actionPerformed(ActionEvent actionEvent) {
-String propname = "gui.quick_" + 
actionEvent.getActionCommand();
+//Bug 62336
+AWTEvent current_event = EventQueue.getCurrentEvent();
+String key_text = "";
+if(current_event instanceof KeyEvent) {
+KeyEvent key_event = (KeyEvent)current_event;
--- End diff --

Here I would put a space after the closing parenthesis as this is a cast.


---

[GitHub] jmeter pull request #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/396#discussion_r212836402
  
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
 
 @Override
 public void actionPerformed(ActionEvent actionEvent) {
-String propname = "gui.quick_" + 
actionEvent.getActionCommand();
+//Bug 62336
+AWTEvent current_event = EventQueue.getCurrentEvent();
--- End diff --

Have you checked what `actionEvent` is here? If I read the Swing-API 
correctly, it should be the same object as `current_event`. If `actionEvent` is 
really `null` here, is it only sometimes `null` and the NPE stops the AWT 
thread?
As a minor note: `current_event` should be written in camel case as 
`currentEvent` to match the other names. 


---

[GitHub] jmeter pull request #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/396#discussion_r212836472
  
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
 
 @Override
 public void actionPerformed(ActionEvent actionEvent) {
-String propname = "gui.quick_" + 
actionEvent.getActionCommand();
+//Bug 62336
+AWTEvent current_event = EventQueue.getCurrentEvent();
+String key_text = "";
+if(current_event instanceof KeyEvent) {
+KeyEvent key_event = (KeyEvent)current_event;
+key_text = KeyEvent.getKeyText( key_event.getKeyCode() 
);
--- End diff --

Again a minor nit: No space needed after the opening and before the closing 
parenthesis as this is a function call.
And as above variable names should be written in camel case: `keyEvent` 
instead of `key_event`.


---

[GitHub] jmeter pull request #396: Fix to 62336

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/396#discussion_r212836415
  
--- Diff: src/core/org/apache/jmeter/gui/MainFrame.java ---
@@ -680,7 +683,15 @@ private void addQuickComponentHotkeys(JTree treevar) {
 
 @Override
 public void actionPerformed(ActionEvent actionEvent) {
-String propname = "gui.quick_" + 
actionEvent.getActionCommand();
+//Bug 62336
+AWTEvent current_event = EventQueue.getCurrentEvent();
+String key_text = "";
+if(current_event instanceof KeyEvent) {
--- End diff --

I would place a space between `if` and the opening parenthesis as `if` is 
not a function call.


---

[GitHub] jmeter issue #395: Fixed ant version

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/395
  
Thanks for your contribution.


---

[GitHub] jmeter issue #394: Fix IllegalArgumentException for File properties wich all...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/394
  
Thanks for your contribution and the detailed description.
Is there any reason, that you don't want to specify a default value for a 
property that can be undefined? Look for example at `Example3BeanInfo.java`.
Other than that it seems OK to include the patch, as there is a code path 
into `FieldStringEditor` that would allow `null` values.


---

[GitHub] jmeter issue #392: Fixed typo in url encoding documentation

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/392
  
Thanks for the report. The correct source of the bug is in 
``xdocs/usermanual/functions.xml``. The docs are generated from those sources.


---

[GitHub] jmeter issue #390: Update mongo-java-driver version

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/390
  
I always build with Oracle JDK 8 (u171 at the moment). ant version is 
1.10.3. 
I suspect that you have an old mongo db jar lying around in your build path.


---

[GitHub] jmeter issue #390: Update mongo-java-driver version

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/390
  
You can reach me (and other JMeter users/developers) on the [JMeter mailing 
list](https://jmeter.apache.org/mail2.html#JMeterDev)

I hacked together an updated version of the Mongo Sampler at 
https://github.com/FSchumacher/jmeter/tree/pr-390-mongo, but I am not sure, 
that it is a good idea, as it will break all old test plans.

And note, that I don't use MongoDB, so I am not sure, if it is correct at 
all.


---

[GitHub] jmeter issue #390: Update mongo-java-driver version

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/390
  
When I compile your PR with ```ant download_jars clean install```, I get 
the following:
```
compile-mongodb:
[javac] Compiling 9 source files to /xxx/jmeter/build/protocol/mongodb
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/config/MongoSourceElement.java:117:
 error: cannot find symbol
[javac] .autoConnectRetry(getAutoConnectRetry())
[javac] ^
[javac]   symbol:   method autoConnectRetry(boolean)
[javac]   location: class Builder
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/config/MongoSourceElement.java:130:
 error: no suitable constructor found for 
WriteConcern(int,int,boolean,boolean,boolean)
[javac] builder.writeConcern(new WriteConcern(
[javac]  ^
[javac] constructor WriteConcern.WriteConcern() is not applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor WriteConcern.WriteConcern(int) is not applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor WriteConcern.WriteConcern(String) is not 
applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor WriteConcern.WriteConcern(int,int) is not 
applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor WriteConcern.WriteConcern(boolean) is not 
applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor WriteConcern.WriteConcern(int,int,boolean) is 
not applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor 
WriteConcern.WriteConcern(int,int,boolean,boolean) is not applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor 
WriteConcern.WriteConcern(String,int,boolean,boolean) is not applicable
[javac]   (actual and formal argument lists differ in length)
[javac] constructor 
WriteConcern.WriteConcern(Object,Integer,Boolean,Boolean) is not applicable
[javac]   (actual and formal argument lists differ in length)
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/mongo/MongoDB.java:53:
 error: cannot find symbol
[javac] boolean authenticated = db.isAuthenticated();
[javac]   ^
[javac]   symbol:   method isAuthenticated()
[javac]   location: variable db of type DB
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/mongo/MongoDB.java:57:
 error: cannot find symbol
[javac] authenticated = db.authenticate(username, 
password.toCharArray());
[javac]   ^
[javac]   symbol:   method authenticate(String,char[])
[javac]   location: variable db of type DB
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/sampler/MongoScriptRunner.java:55:
 error: cannot find symbol
[javac] db.requestStart();
[javac]   ^
[javac]   symbol:   method requestStart()
[javac]   location: variable db of type DB
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/sampler/MongoScriptRunner.java:57:
 error: cannot find symbol
[javac] db.requestEnsureConnection();
[javac]   ^
[javac]   symbol:   method requestEnsureConnection()
[javac]   location: variable db of type DB
[javac] 
/xxx/jmeter/src/protocol/mongodb/org/apache/jmeter/protocol/mongodb/sampler/MongoScriptRunner.java:66:
 error: cannot find symbol
[javac] db.requestDone();
[javac]   ^
[javac]   symbol:   method requestDone()
[javac]   location: variable db of type DB
[javac] Note: Some input files use or override a deprecated API.
[javac] Note: Recompile with -Xlint:deprecation for details.
[javac] 7 errors
```
That means, that the current code doesn't work with the new jars.

If you want to update the code the new jars, go ahead, but as of JMeter 3.0 
the sampler has been deprecated and it should have been removed with version 
3.1 (which obviously didn't happen).

It would be interesting to see your JSR223 sampler. I suspect it handles 
all of the connection setup and client usage itself. Maybe it would be a good 
idea to discuss your use case and solution on the mailing list.


---

[GitHub] jmeter issue #390: Update mongo-java-driver version

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/390
  
Thanks for your contribution, but I don't think this is the correct 
solution.
If we only update the jars, the code will not compile anymore. There are a 
lot of missing and deprecated methods and classes. I tried to update our usage 
of the api to the current one, but that is not possible without a lot of work. 
(The authentication mechanism has changed and the eval mechanism, that we 
depend on has been deprecated/removed).
As the mongo sampler has been deprecated with JMeter 3.0. So a better patch 
would be to remove the client jars altogether.


---

[GitHub] jmeter pull request #389: Open different RemoteObjects on different ports.

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/389

Open different RemoteObjects on different ports.

## Description


When using RMI over SSL with fixed `client.rmi.localport` JMeter currently 
throws an exception as it has problems to bind RemoteObjects to the same port 
more than once.
This change will use offsets for the different RemoteObjects, so that it 
will be possible again to use a fixed port for those RMI objects.
There is another bug in JMeter, that prevents a complete fix: Some of the 
remote listeners will get initialized twice in GUI mode. That will lead to some 
similar problems.

## Motivation and Context


This bug was reported on the [users mailing 
list](https://lists.apache.org/thread.html/2de9386fb0f0c073eb9602837338c60981f1d22ded85661424ffa83e@%3Cuser.jmeter.apache.org%3E)
  and on [SO as "unable to start jmeter 4.0 client with ssl 
rmi"](https://stackoverflow.com/questions/50752126/unable-to-start-jmeter-4-0-client-with-ssl-rmi).

## How Has This Been Tested?



Ran the tests and started a simple distributed setup with RMI over SSL.

## Screenshots (if appropriate):

## Types of changes

- Bug fix (non-breaking change which fixes an issue)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [x] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter fix-rmi-localports

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/389.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #389


commit 6825ad4f1632681ce221daa1d468694294081b05
Author: Felix Schumacher 
Date:   2018-06-16T14:11:33Z

Open different RemoteObjects on different ports.




---

[GitHub] jmeter issue #388: Fix minor typo

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/388
  
Thanks for the contribution.


---

[GitHub] jmeter issue #387: Add delegation for SPNEGO back to JMeter

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/387
  
I thought I hat to overwrite the method `createGSSContext`, which is only 
possible on a subclass in the same package. I changed it now explicitly to call 
another method and placed the classes in JMeter's package namespace. Will check 
on Monday, if it is still working.


---

[GitHub] jmeter pull request #387: Add delegation for SPNEGO back to JMeter

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/387

Add delegation for SPNEGO back to JMeter

## Description


Newer httpclient versions (at least in the 4.x branch) have dropped the 
support for delegation of kerberos tickets over SPNEGO. This patch brings back 
the support to delegation back to JMeter.

## Motivation and Context
Older versions of JMeter always tried to get forwardable tickets when using 
SPNEGO. As I need this functionality for a service, I would like to have the 
forwarding back. As the default forwarding could be seen critical from a 
security standpoint, the patch disables it by default. It can be enabled by a 
system property.



## How Has This Been Tested?



Tested it on a service that depends on delegation of tickets.

## Screenshots (if appropriate):

## Types of changes

- Bug fix (non-breaking change which fixes an issue) (At least I see this 
as a regression)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [x] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter delegate-krb

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/387.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #387


commit 94b369b64f931b75564e4df47231754602639972
Author: Felix Schumacher 
Date:   2018-06-07T17:54:03Z

Add delegation for SPNEGO back to JMeter




---

[GitHub] jmeter issue #386: Adds parameter supports for RMI keystore creation scripts

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/386
  
Thanks for your contribution.
Would you care to update the docs for the changes and create a new pull 
request :)


---

[GitHub] jmeter issue #384: Minor wording correction

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/384
  
Thanks for your contribution.

Please note, that the documentation is generated from xml files in `xdocs` 
and those should be patched instead of the generated ones.

But apart from that I am not sure the old wording is wrong and that the new 
wording is better. If I skip the "load test," part it would read: 

> ... designed to functional behaviour and measure performance.

That doesn't sound right to me.


---

[GitHub] jmeter issue #383: Docs 4.0

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/383
  
I don't see any changes in here and there is no real description to this 
pull request.


---

[GitHub] jmeter issue #382: fixed Gradle typo in README.md

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/382
  
Thanks for your contribution.

The test TestRedirectionPolicies is flaky and happens to fail sometimes 
with no apparent reason. This has nothing to do with your spelling fix.


---

[GitHub] jmeter pull request #380: Hdrhistogram

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/380

Hdrhistogram

## Description


First try to replace percentile calculations with HdrHistogram.
This is just to get an impression, what we can gain by using another 
implementation and should not  be incorporated yet.

## Motivation and Context


Currently we have two different algorithms for calculating percentiles. The 
results differ and the algorithm is not documented. This should be changed 
(although it is not addressed yet by this pr).

Another point is the efficiency of the used algorithms. As the swing gui 
will be refreshed every 300 ms (or so), when data arrives and the mean, average 
and other percentiles are re-caculated on every refresh, the used algorithm 
should be very efficient. The currently used one is iterating over every bucket 
to calculate the mean (and std deviation).

The proposed implementation uses a different approach and doesn't need to 
iterate to calculate the mean, etc. The percentiles are calculated by 
HdrHistogram and the locking has been simplified by transferring all logic that 
modifies the data into the awt thread.

In my tests the samples per second went up from 200 per second over 1.400 
per second.

## How Has This Been Tested?




Tested by using a simple JSR223 Sampler that slept a bit (and by a JSR223 
Sampler, that pretended to sleep a bit)

## Screenshots (if appropriate):

## Types of changes

- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing functionality 
to not work as expected)

A bit of everything

## Checklist:


- [ ] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter hdrhistogram

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/380.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #380


commit 4280996464a4adbed427672f7c6f0eda82c020e1
Author: fschumacher 
Date:   2018-03-30T17:17:19Z

HdrHistogram, first try

commit 492a869b705d36d6421d42899fdc73a2d052b88c
Author: fschumacher 
Date:   2018-03-31T07:05:19Z

Use more efficient algorithm to calculate mean and stddev




---

[GitHub] jmeter issue #377: fixed wrong example for loop controller iteration variabl...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/377
  
Thanks for the contribution.


---

[GitHub] jmeter issue #376: Logging of JUnitSampler exceptions

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/376
  
Some discussion about the performance impact of the log messages has taken 
place on the dev mailing list.

Would this patch be still useful for you, if we logged the messages at 
debug or info level?


---

[GitHub] jmeter issue #376: Logging of JUnitSampler exceptions

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/376
  
Thanks for your contribution.


---

[GitHub] jmeter pull request #356: Sonar fixes

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/356#discussion_r167627478
  
--- Diff: src/components/org/apache/jmeter/assertions/HTMLAssertion.java ---
@@ -328,17 +304,13 @@ public void setXML() {
 
 /**
  * Check if xml validation mode is set
- * 
- * @return boolean
  */
 public boolean isXML() {
 return getPropertyAsLong(FORMAT_KEY) == 2;
 }
 
 /**
- * Sets the name of the file where tidy writes the output to
- * 
- * @return name of file
+ * Gets the name of the file where tidy writes the output to
--- End diff --

If we remove the return tag, javadoc will complain.


---

[GitHub] jmeter pull request #356: Sonar fixes

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/356#discussion_r167626506
  
--- Diff: src/components/org/apache/jmeter/assertions/BSFAssertion.java ---
@@ -57,8 +57,4 @@ public AssertionResult getResult(SampleResult response) {
 return result;
 }
 
-@Override
-public Object clone() {
--- End diff --

If we remove this method sonar will complain about the missing method, as 
the class is marked Clonable


---

[GitHub] jmeter issue #365: Expanded Checkstyle to files in src and test; fixed newly...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/365
  
Thanks for your contribution.


---

[GitHub] jmeter issue #374: Add a 'go to top' button that gets shown, when page is sc...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/374
  
I didn't want to introduce a possible breakage into the RC and it will 
affect the web page, only. So we can "release" these kind of changes at a later 
point.


---

[GitHub] jmeter pull request #374: Add a 'go to top' button that gets shown, when pag...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/374

Add a 'go to top' button that gets shown, when page is scrolled down

## Description
The JMeter html documentation contains rather long pages, which take a lot 
of time to scroll down and a long time to scroll back up again. This patch adds 
a button (link really) to the bottom of the screen, that gets shown, when a 
page is scrolled down. A click on the link will take the user back up to the 
top.

## How Has This Been Tested?
A test has been made with firefox (v58) on the locally generated pages (ant 
docs-site)

## Types of changes
- New feature (non-breaking change which adds functionality)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [No documentation needed] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter go-to-top-button

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/374.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #374


commit 8fe98f385bd7174a66d7a02ee2db0e779e088863
Author: Felix Schumacher 
Date:   2018-02-01T21:03:32Z

Add a 'go to top' button that gets shown, when page is scrolled down




---

[GitHub] jmeter issue #343: Reduced the size of all screenshots.

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/343
  
Thanks for your contribution. I ran the shell commands on current trunk, 
inspected the changed images and committed them.


---

[GitHub] jmeter issue #371: Minor fix in 'XPath assertion' description

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/371
  
Thanks for your contribution.


---

[GitHub] jmeter issue #350: Parallelised unit tests

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/350
  
If we initialize the string text to an empty string in the anonymous inner 
class, the tests pass. 


---

[GitHub] jmeter issue #350: Parallelised unit tests

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/350
  
Maybe this is the culprit:

URL: http://svn.apache.org/viewvc?rev=1817421&view=rev
Log:
Make mock react correctly on setText/getText


---

[GitHub] jmeter issue #343: Reduced the size of all screenshots.

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/343
  
Some of those images have been reduced already in the past. In my tests 
pngquant will happily compress those again, but that would probably be at 
0.75*0.75 quality.


---

[GitHub] jmeter issue #341: Test isIgnore after post processor and assertions

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/341
  
Yes, but also to enable us to filter in view results tree on those meta 
information. It could be something like a comment, a thread name or anything 
else.


---

[GitHub] jmeter issue #342: More edge cases for changeCase function and slight behavi...

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/342
  
@pmouawad do you mean trim for all cases (upper, lower and capitalize)?


---

[GitHub] jmeter issue #341: Test isIgnore after post processor and assertions

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/341
  
I wonder, whether instead of or additionally to the new `setIgnore` method, 
we should add and expose a `setMetadata(type, content)` method.
I believe it would be useful for example to:
* mark samples to be ignored by listeners and such
* use the information later on to group by (a use case would be to group by 
some user given metadata in the view results tree)


---

[GitHub] jmeter issue #332: Added Spock framework and some tests, both old and new

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/332
  
Thanks for your contribution.


---

[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/342#discussion_r153048358
  
--- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java 
---
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.jmeter.functions;
+
+import static org.junit.Assert.*;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.jmeter.engine.util.CompoundVariable;
+import org.apache.jmeter.functions.ChangeCase.ChangeCaseMode;
+import org.apache.jmeter.junit.JMeterTestCase;
+import org.apache.jmeter.samplers.SampleResult;
+import org.apache.jmeter.threads.JMeterContext;
+import org.apache.jmeter.threads.JMeterContextService;
+import org.apache.jmeter.threads.JMeterVariables;
+import org.hamcrest.CoreMatchers;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class TestChangeCaseExamples extends JMeterTestCase {
+
+@Parameters
+public static Collection data() {
+return Arrays.asList(new Object[][] { { " spaces before and after 
",
+new String[] { " SPACES BEFORE AND AFTER ",
--- End diff --

This would be a perfect showcase for spock, don't you think?


---

[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/342#discussion_r153048353
  
--- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java 
---
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.jmeter.functions;
+
+import static org.junit.Assert.*;
--- End diff --

Right, but we don't enforce it yet and there are more in our test code base.

I will have a look at my eclipse settings.


---

[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...

[FSchumacher]

GitHub user FSchumacher opened a pull request:

https://github.com/apache/jmeter/pull/342

More edge cases for changeCase function and slight behaviour changes.

## Description
Addition of more examples to test with the different change case modes.
We now really split on all non alpha numeric characters - even unicode ones.
The input strings for the camel case modes get trimmed before further 
actions.

A further question is, whether we should trim the input in the other modes 
as well?
For example "` space in front`" + `capitalize` = "`Space in front`" or "` 
space in front`"?


## Motivation and Context
The handling of whitespace and other non alphanumeric characters was not 
well defined.

## How Has This Been Tested?
unit tests are added and run

## Screenshots (if appropriate):

## Types of changes
- Bug fix (non-breaking change which fixes an issue)

## Checklist:


- [x] My code follows the [code style][style-guide] of this project.
- [x] I have updated the documentation accordingly.

[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/FSchumacher/jmeter camel-case-examples

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/jmeter/pull/342.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #342


commit 3d576bfca9eb2503f7ba04c083f7e3d9167751b9
Author: Felix Schumacher 
Date:   2017-11-25T13:47:53Z

More edge cases for changeCase function and slight behaviour changes.

Addition of more examples to test with the different change case modes.
We now really split on all non alpha numeric characters - even unicode ones.
The input strings for the camel case modes get trimmed before further 
actions.




---

[GitHub] jmeter pull request #305: Add increase font / decrease font / clear in LogPa...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/305#discussion_r135325828
  
--- Diff: src/core/org/apache/jmeter/gui/LoggerPanel.java ---
@@ -18,14 +18,11 @@
 
 package org.apache.jmeter.gui;
 
-import java.awt.BorderLayout;
-import java.awt.Insets;
+import java.awt.*;
--- End diff --

Don't use wildcard import.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #305: Add increase font / decrease font / clear in LogPa...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/305#discussion_r135327597
  
--- Diff: src/core/org/apache/jmeter/resources/messages_ja.properties ---
@@ -451,3 +451,5 @@ web_testing_title=HTTP \u30EA\u30AF\u30A8\u30B9\u30C8
 workbench_title=\u30EF\u30FC\u30AF\u30D9\u30F3\u30C1
 xml_assertion_title=XML \u30A2\u30B5\u30FC\u30B7\u30E7\u30F3
 
you_must_enter_a_valid_number=\u9069\u5207\u306A\u6570\u5024\u3092\u5165\u529B\u3057\u3066\u304F\u3060\u3055\u3044
+font.decrease=Decrease font to %d pt
--- End diff --

Well, ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #305: Add increase font / decrease font / clear in LogPa...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/305#discussion_r135326024
  
--- Diff: src/core/org/apache/jmeter/gui/LoggerPanel.java ---
@@ -90,6 +91,21 @@ private JTextArea init() { // WARNING: called from ctor 
so must not be overridde
 jSyntaxTextArea.setLineWrap(false);
 jSyntaxTextArea.setLanguage("text");
 jSyntaxTextArea.setMargin(new Insets(2, 2, 2, 2)); // space 
between borders and text
+int fontSize = jSyntaxTextArea.getFont().getSize();
+increase = new 
JMenuItem(String.format(JMeterUtils.getResString("font.increase"), (fontSize + 
1)));
--- End diff --

The parenthesis around `fontSize + 1` is not needed and should be removed.
The other scaling used in JMeter uses a multiplication scheme. Perhaps this 
one should follow the other one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #305: Add increase font / decrease font / clear in LogPa...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/305#discussion_r135327448
  
--- Diff: src/core/org/apache/jmeter/resources/messages_es.properties ---
@@ -1003,3 +1003,5 @@ xpath_tidy_show_warnings=Mostrar advertencias
 you_must_enter_a_valid_number=Debe introducir un n\u00FAmero v\u00E1lido
 zh_cn=Chino (Simplificado)
 zh_tw=Chino (Tradicional)
+font.decrease=Decrease font to %d pt
--- End diff --

Same here, but I have no spanish foo.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #305: Add increase font / decrease font / clear in LogPa...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/305#discussion_r135327543
  
--- Diff: src/core/org/apache/jmeter/resources/messages_fr.properties ---
@@ -1364,3 +1364,5 @@ xpath_tidy_show_warnings=Afficher les alertes
 you_must_enter_a_valid_number=Vous devez entrer un nombre valide
 zh_cn=Chinois (simplifi\u00E9)
 zh_tw=Chinois (traditionnel)
+font.decrease=Decrease font to %d pt
--- End diff --

And again. But this one has to be translated, it will be checked.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #305: Add increase font / decrease font / clear in LogPa...

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/305#discussion_r135327348
  
--- Diff: src/core/org/apache/jmeter/resources/messages_de.properties ---
@@ -536,3 +536,5 @@ warning=Warnung\!
 web_server_domain=Server Name oder IP\:
 web_testing_retrieve_images=Hole alle Bilder und Java Applets (nur HTML 
Dateien)
 you_must_enter_a_valid_number=Sie m\u00FCssen ein g\u00FCltige Nummer 
eingeben
+font.decrease=Decrease font to %d pt
--- End diff --

This is not a german sentence :)
If you are not sure, leave it out, or use `Verkleinere Font auf %d pt` (The 
next one would be `Vergrößere Font auf %d pt` - the `ö` and `ß` would have 
to be translated to \u... sequences, still.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter issue #301: First stab at a whitelist for the xstream library.

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/301
  
Now both the whitelist should be checked first and if the class is **not** 
allowed a warning will be logged (as info).
To support dynamic additions by plugins, we could have a look at Emi's 
suggestion of a ServiceLoader.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #301: First stab at a whitelist for the xstream library.

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/301#discussion_r131525468
  
--- Diff: src/core/org/apache/jmeter/util/JMeterUtils.java ---
@@ -1261,10 +1265,39 @@ public static final void refreshUI() {
  */
 public static void setupXStreamSecurityPolicy(XStream xstream) {
 // This will lift the insecure warning
+// disallow any class
 xstream.addPermission(NoTypePermission.NONE);
-// We reapply very permissive policy
-// See 
https://groups.google.com/forum/#!topic/xstream-user/wiKfdJPL8aY
-// TODO : How much are we concerned by CVE-2013-7285 
-xstream.addPermission(AnyTypePermission.ANY);
+// allow some classes that are hopefully secure
+for (TypePermission perm : Arrays.asList(NullPermission.NULL,
+ArrayTypePermission.ARRAYS,
+PrimitiveTypePermission.PRIMITIVES)) {
+xstream.addPermission(perm);
+}
+for (Class allowHierarchy : 
Arrays.asList(java.util.Collection.class,
+org.apache.jmeter.testelement.TestElement.class,
+org.apache.jorphan.collections.HashTree.class,
+org.apache.jmeter.samplers.SampleSaveConfiguration.class)) 
{
+xstream.allowTypeHierarchy(allowHierarchy);
--- End diff --

Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r131245265
  
--- Diff: src/core/org/apache/jmeter/gui/UndoHistoryItem.java ---
@@ -35,31 +36,48 @@
 private final HashTree tree;
 // TODO: find a way to show this comment in menu item and toolbar 
tooltip
 private final String comment;
+private final TreeState treeState;
+private final boolean dirty;
 
 /**
  * This constructor is for Unit test purposes only
  * @deprecated DO NOT USE
  */
 @Deprecated
 public UndoHistoryItem() {
-tree = null;
-comment = null;
+this(null, null, null, false);
 }
 
 /**
  * @param copy HashTree
  * @param acomment String
  */
-public UndoHistoryItem(HashTree copy, String acomment) {
+public UndoHistoryItem(HashTree copy, String acomment, TreeState 
treeState, boolean dirty) {
 tree = copy;
 comment = acomment;
--- End diff --

Fair enough.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r131244988
  
--- Diff: src/core/org/apache/jmeter/gui/SimpleCompoundEdit.java ---
@@ -0,0 +1,14 @@
+package org.apache.jmeter.gui;
+
+import javax.swing.undo.CompoundEdit;
+
+public class SimpleCompoundEdit extends CompoundEdit {
+
+public boolean isEmpty() {
+return size() == 0;
--- End diff --

My reasoning was that `isEmpty()` can be implemented more efficient than 
`size() == 0`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter issue #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on the issue:

https://github.com/apache/jmeter/pull/300
  
I think I can reproduce Philippes problem. And it will probably look really 
strange to a user. My steps where:
* start a new test plan `Ctrl+L`
* add a thread group `Ctrl+0`
* add a http sampler `Ctrl+1`
* add a view results tree `Ctrl+9`
* move view results tree above the http sampler
* undo (I wish I could have pressed `Ctrl+Z`)
I end up with two view results trees.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130236104
  
--- Diff: src/core/org/apache/jmeter/gui/UndoHistory.java ---
@@ -118,8 +87,12 @@ public void clear() {
 return;
 }
 log.debug("Clearing undo history");
-history.clear();
-position = INITIAL_POS;
+manager.discardAllEdits();
+if (isTransaction()) {
+log.warn("Clearing undo history with " + transactions.size() + 
" unfinished transactions");
--- End diff --

I like the logs to use the formatting/placeholder feature `{}` together 
with parameters.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130236542
  
--- Diff: src/core/org/apache/jmeter/gui/action/SearchTreeDialog.java ---
@@ -258,6 +259,7 @@ private void doSearch(ActionEvent e) {
 jTree.expandPath(new TreePath(jMeterTreeNode.getPath()));
 }
 }
+guiPackage.endUndoTransaction();
--- End diff --

No `try { ... } finally { guiPackage.endUndoTransaction() }` here? Why 
above?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130236022
  
--- Diff: src/core/org/apache/jmeter/gui/TreeState.java ---
@@ -0,0 +1,84 @@
+package org.apache.jmeter.gui;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.swing.JTree;
+
+public interface TreeState {
+
+/**
+ * Restore tree expanded and selected state
+ *
+ * @param guiInstance GuiPackage to be used
+ */
+void restore(GuiPackage guiInstance);
+
+final static TreeState NOTHING = new TreeState() {
+@Override
+public void restore(GuiPackage guiInstance) {
+//nothing
+}
+};
+
+/**
+ * Save tree expanded and selected state
+ *
+ * @param guiPackage {@link GuiPackage} to be used
+ */
+public static TreeState from(GuiPackage guiPackage) {
+if (guiPackage == null) {
+return NOTHING;
+}
+
+MainFrame mainframe = guiPackage.getMainFrame();
+if (mainframe != null) {
+final JTree tree = mainframe.getTree();
+int savedSelected = tree.getMinSelectionRow();
+ArrayList savedExpanded = new ArrayList<>();
+
+for (int rowN = 0; rowN < tree.getRowCount(); rowN++) {
+if (tree.isExpanded(rowN)) {
+savedExpanded.add(rowN);
+}
+}
+
+return new TreeStateImpl(savedSelected, savedExpanded);
+}
+
+return NOTHING;
+}
+
+static final class TreeStateImpl implements TreeState {
+
+// GUI tree expansion state
+private final List savedExpanded;
+
+// GUI tree selected row
+private final int savedSelected;
+
+public TreeStateImpl(int savedSelected, List 
savedExpanded) {
+this.savedSelected = savedSelected;
+this.savedExpanded = savedExpanded;
+}
+
+@Override
+public void restore(GuiPackage guiInstance) {
+MainFrame mainframe = guiInstance.getMainFrame();
+if (mainframe == null) {
+//log?
+return;
+}
+
+final JTree tree = mainframe.getTree();
+
+if (savedExpanded.size() > 0) {
--- End diff --

use `isEmpty()` can be more efficient


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130236378
  
--- Diff: src/core/org/apache/jmeter/gui/action/ActionRouter.java ---
@@ -67,6 +72,9 @@ public void actionPerformed(final ActionEvent e) {
 
 private void performAction(final ActionEvent e) {
 String actionCommand = e.getActionCommand();
+if(!NO_TRANSACTION_ACTIONS.contains(actionCommand)) {
--- End diff --

Perhaps a small helper method `needsTransaction(actionCommand)` would help 
readability, as it would get rid of the double negation (`!NO..`)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130235928
  
--- Diff: src/core/org/apache/jmeter/gui/TreeState.java ---
@@ -0,0 +1,84 @@
+package org.apache.jmeter.gui;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.swing.JTree;
+
+public interface TreeState {
+
+/**
+ * Restore tree expanded and selected state
+ *
+ * @param guiInstance GuiPackage to be used
+ */
+void restore(GuiPackage guiInstance);
+
+final static TreeState NOTHING = new TreeState() {
--- End diff --

`static final` to conform with java conventions and the anonymous class 
implementing `NOTHING` could be rewritten as a lambda function.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130236483
  
--- Diff: src/core/org/apache/jmeter/gui/action/CheckDirty.java ---
@@ -64,11 +64,12 @@
 public CheckDirty() {
 previousGuiItems = new HashMap<>();
 ActionRouter.getInstance().addPreActionListener(ExitCommand.class, 
this);
+
ActionRouter.getInstance().addPostActionListener(UndoCommand.class, this);
 }
 
 @Override
 public void actionPerformed(ActionEvent e) {
-if (e.getActionCommand().equals(ActionNames.EXIT)) {
+if (e.getActionCommand().equals(ActionNames.EXIT) || 
e.getActionCommand().equals(ActionNames.UNDO) || 
e.getActionCommand().equals(ActionNames.REDO)) {
--- End diff --

Maybe we should define one set of names that identify the commands that 
should call `doAction` than this would be probably more readable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130235880
  
--- Diff: src/core/org/apache/jmeter/gui/SimpleCompoundEdit.java ---
@@ -0,0 +1,14 @@
+package org.apache.jmeter.gui;
+
+import javax.swing.undo.CompoundEdit;
+
+public class SimpleCompoundEdit extends CompoundEdit {
+
+public boolean isEmpty() {
+return size() == 0;
--- End diff --

Wouldn't it be nicer and more efficient to delegate `isEmpty` to edit?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] jmeter pull request #300: Bug 57039 - Inconsistency with the undo/redo log

[FSchumacher]

Github user FSchumacher commented on a diff in the pull request:

https://github.com/apache/jmeter/pull/300#discussion_r130235898
  
--- Diff: src/core/org/apache/jmeter/gui/SimpleCompoundEdit.java ---
@@ -0,0 +1,14 @@
+package org.apache.jmeter.gui;
+
+import javax.swing.undo.CompoundEdit;
+
+public class SimpleCompoundEdit extends CompoundEdit {
--- End diff --

This is a serializable class but a serial version uid field is missing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---