Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IllegalArgumentException for File properties wich allow null value #394

Closed
wants to merge 1 commit into from

Conversation

@Mingun
Copy link

@Mingun Mingun commented Aug 2, 2018

Description

This fix the IllegalArgumentException when TestBean class contains nullable File field (in my case field has name vendorSKPath):

2018-08-02 11:42:54,940 DEBUG o.a.j.t.g.GenericTestBeanCustomizer: Property vendorSKPath has editor class null
2018-08-02 11:42:54,941 DEBUG o.a.j.t.g.WrapperEditor: <-java.lang.String:
2018-08-02 11:42:54,941 DEBUG o.a.j.t.g.WrapperEditor: ->""
2018-08-02 11:42:54,941 DEBUG o.a.j.t.g.GenericTestBeanCustomizer: Property vendorSKPath has property editor org.apache.jmeter.testbeans.gui.FileEditor@1f236f6d
2018-08-02 11:42:54,941 DEBUG o.a.j.t.g.WrapperEditor: <-NULL:null
2018-08-02 11:42:54,941 WARN o.a.j.g.a.Load: Unexpected error. java.lang.IllegalArgumentException
java.lang.IllegalArgumentException: null
	at org.apache.jmeter.testbeans.gui.FieldStringEditor.setValue(FieldStringEditor.java:82) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.testbeans.gui.WrapperEditor.setValue(WrapperEditor.java:349) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.testbeans.gui.FileEditor.setValue(FileEditor.java:214) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.testbeans.gui.GenericTestBeanCustomizer.setEditorValue(GenericTestBeanCustomizer.java:477) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.testbeans.gui.GenericTestBeanCustomizer.<init>(GenericTestBeanCustomizer.java:289) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.testbeans.gui.TestBeanGUI.init(TestBeanGUI.java:417) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.testbeans.gui.TestBeanGUI.configure(TestBeanGUI.java:299) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.tree.JMeterTreeModel.addComponent(JMeterTreeModel.java:159) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.tree.JMeterTreeModel.addSubTree(JMeterTreeModel.java:133) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.tree.JMeterTreeModel.addSubTree(JMeterTreeModel.java:133) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.tree.JMeterTreeModel.addSubTree(JMeterTreeModel.java:133) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.tree.JMeterTreeModel.addSubTree(JMeterTreeModel.java:125) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.GuiPackage.addSubTree(GuiPackage.java:522) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.Load.insertLoadedTree(Load.java:193) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.Load.loadProjectFile(Load.java:130) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.Load.loadProjectFile(Load.java:101) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.LoadRecentProject.doActionAfterCheck(LoadRecentProject.java:67) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.AbstractActionWithNoRunningTest.doAction(AbstractActionWithNoRunningTest.java:45) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.ActionRouter.performAction(ActionRouter.java:88) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at org.apache.jmeter.gui.action.ActionRouter.lambda$actionPerformed$0(ActionRouter.java:70) ~[ApacheJMeter_core.jar:4.1-SNAPSHOT.20180802]
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311) [?:1.8.0_181]
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758) [?:1.8.0_181]
	at java.awt.EventQueue.access$500(EventQueue.java:97) [?:1.8.0_181]
	at java.awt.EventQueue$3.run(EventQueue.java:709) [?:1.8.0_181]
	at java.awt.EventQueue$3.run(EventQueue.java:703) [?:1.8.0_181]
	at java.security.AccessController.doPrivileged(Native Method) [?:1.8.0_181]
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74) [?:1.8.0_181]
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:728) [?:1.8.0_181]
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205) [?:1.8.0_181]
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116) [?:1.8.0_181]
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105) [?:1.8.0_181]
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) [?:1.8.0_181]
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93) [?:1.8.0_181]
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82) [?:1.8.0_181]
public class WithFile implements TestBean {
    File file;
    public void setFile(File file) { this.file = file; }
    public File getFile() { return file; }
}

public class WithFileBeanInfo extends BeanInfoSupport {
    public WithFileBeanInfo() {
        super(WithFile.class);
        property("file");
    }
}

Motivation and Context

Though in the comment to the class FieldStringEditor it is told that it processes only non-null strings:

/**
* This class implements a property editor for non-null String properties that
* supports custom editing (i.e.: provides a GUI component) based on a text
* field.
* <p>
* The provided GUI is a simple text field.
*
*/
class FieldStringEditor extends PropertyEditorSupport implements ActionListener, FocusListener {

the class WrappedEditor can call setValue with null:

if (value == null) {
if (!acceptsNull) {
throw new IllegalArgumentException("Null is not allowed");
}
text = null;
} else if (acceptsExpressions && isExpression(value)) {
text = (String) value;
} else {
// Not an expression (isn't or can't be), not null.
typeEditor.setValue(value); // may throw IllegalArgumentExc.
text = fixGetAsTextBug(typeEditor.getAsText());
if (!acceptsOther && !isATag(text)) {
throw new IllegalArgumentException("Value not allowed: '" + text + "' is not in " + Arrays.toString(getTags()));
}
}
guiEditor.setValue(text);

So I think that comment also need to be updated, but I am not sure. It is possible that this class as envisioned shall not allow null and it is necessary to do fix in other place, but in case of my testing this fix works fine.

How Has This Been Tested?

Create plugin with any TestBean class with nullable File field as show above

Types of changes

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

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
@Mingun
Copy link
Author

@Mingun Mingun commented Aug 7, 2018

Any news?

@FSchumacher
Copy link
Contributor

@FSchumacher FSchumacher commented Aug 8, 2018

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.

@Mingun
Copy link
Author

@Mingun Mingun commented Aug 8, 2018

In my plugin it is filepath that enable optional feature, so when it is undefined the feature is off.

In the next 2 weeks I will not be able to work on PR since I am on vacation, therefore if any additional work are required, then be not surprised that I will answer not soon.

@Mingun Mingun force-pushed the fix-null-in-field branch from 7495471 to 33d461f Sep 17, 2018
@Mingun
Copy link
Author

@Mingun Mingun commented Sep 17, 2018

Now its ready to merge. Travis fail is unrelated to this changes

@FSchumacher
Copy link
Contributor

@FSchumacher FSchumacher commented Sep 22, 2018

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.

@asfgit asfgit closed this in 413bab2 Sep 23, 2018
@Mingun
Copy link
Author

@Mingun Mingun commented Sep 27, 2018

Thanks!

I was tested with 2c82bf3 (trunk branch). The exception does not arise any more though to set Undefined value for File property is impossible now, even though the flag {{NOT_UNDEFINED}} is not set for property. I attach 2 screenshots showing behavior of the same plugin file in versions 3.1 and 5.1:
jmeter 3 x
jmeter 5 x

Therefore this fix so far not really helps me

asfgit pushed a commit that referenced this issue Sep 28, 2018
@FSchumacher
Copy link
Contributor

@FSchumacher FSchumacher commented Sep 28, 2018

OK, so we really have to explicitly allow null to help you there, right? Does the last change help you?

@Mingun
Copy link
Author

@Mingun Mingun commented Oct 1, 2018

Does the last change help you?

No. Actually, the problem isn't connected with this task any more, the editor has changed as a result of the сommit fc4781d which is linked to bug 61625, at first sight absolutely not relating to it

After reverting this change editor look as I expect:
fc4781d-reverted

@pmouawad, can you look at this? Change that you made is necessary? What strange behavior that you talk about? In my opinion, combobox works as expected and I don't represent at all how it can influence perfomance

@Mingun Mingun deleted the fix-null-in-field branch Oct 1, 2018
StorDm pushed a commit to etnetera/jmeter that referenced this issue Jan 7, 2021
Based on patch by Mingun (alexander_sergey at mail.ru)
Closes apache#394


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1841688 13f79535-47bb-0310-9956-ffa450edef68

Former-commit-id: 413bab2
StorDm pushed a commit to etnetera/jmeter that referenced this issue Jan 7, 2021
Relates to apache#394 on github.


git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1842263 13f79535-47bb-0310-9956-ffa450edef68

Former-commit-id: 4f2b6d2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants