BeanInfo & EventListeners

It would be nice if all Vaadin components provided valid BeanInfo descriptors. In particular, I’m noticing that BeanInfo.getEventSetDescriptors() doesn’t return the proper descriptors.

This seems to be because of the non-standard naming of the Listener interfaces. For instance, ItemClickListener is actually
com.vaadin.event.ItemClickEvent.ItemClickListener
. So for Table, the JavaBean Introspector wants to see:
[font=Courier New]

addItemClickEvent$ItemClickListener(ItemClickListener);
removeItemClickEvent$ItemClickListener(ItemClickListener);
[/font]

Here’s why this would be helpful. With groovy, it is possible to write listeners as concise closures. The following is all the code you need to register an item click listener on a Table.


t = new Table();
t.itemClick = { event → println “You clicked!” }

However, for this to work, Introspector.getBeanInfo(Table.class).getEventDescriptors() needs to return descriptors, which it does not.

I was able to patch this up (for Table ItemClick) by:

  1. Implementing
    Table.addItemClickEvent$ItemClickListener(ItemClickListener l) { addListener(l); }
  2. Implementing
    Table.removeItemClickEvent$ItemClickListener(ItemClickListener) { addListener(l); }
  3. Writing a custom BeanInfo, that returns EventSetDescriptors and delegates for the other methods

Of course if there was a way of getting away from using the inner interfaces for the listeners, this problem would go away entirely, but I’m not sure how you would do that without breaking the APIs.
[/font]

I think that it would be a really good idea and might help in the process of creating WYSIWYG editor.

Created a ticket for this:
http://dev.vaadin.com/ticket/3868

Right, I think this is something that has to be resolved somehow even before the WYWIWYG editor reaches 1.0. In general the problem is the API modifications needed to get rid of the inner classes and listener method overloading. 7.0 perhaps?

Those closures look very tempting :slight_smile:

Without thinking if that would be better API or not, we cannot change event handling that much - it would break everything based on Vaadin - badly.

Indeed.

It turns out that original issue can be fixed reasonably easily. I have created a Helper class that introspects the non-standard listeners and creates EventSetDescriptors automatically where the standard Introspector can’t figure it out.

Groovy works like a charm. Requires no Vaadin code changes, just the inclusion of new BeanInfo classes.

I’ll post code shortly.

I posted a self-contained set of code that provides BeanInfo implementations for all the com.vaadin.ui.* classes that implement listeners/handlers.

The code is simple maven project that should build easily.

The code is attached as a tgz to the ticket that you opened.

http://dev.vaadin.com/ticket/3868

More detail there as well.

Great, thanks!

Yes, very nice. This seems very straight-forward implementation and I think it (or something similar) could be made as part of Vaadin.

Btw: The original idea behind using the inner interfaces and classes for events and listeners was to reduce the number of classes. This way only custom BeanInfo classes would be added and we can stick with the inner class approach.

Sweet this was just I was looking for - however the unit tests fail under Groovy 1.7
I tried a quick debugging session but I guess it’s too late in the day for that :wink:

Here’s the stack trace from the unit test (it fails at the very first test - it complains that the are too many “click” listeners - though I only saw your code produce one - but then again it’s late and I might have missed it):


groovy.lang.GroovyRuntimeException: There are multiple listeners for the property click. Please do not use the bean short form to access this listener.
	at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:2402)
	at groovy.lang.MetaClassImpl.setProperty(MetaClassImpl.java:3299)
	at org.codehaus.groovy.runtime.InvokerHelper.setProperty(InvokerHelper.java:183)
	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.setProperty(ScriptBytecodeAdapter.java:483)
	at Script1.run(Script1.groovy:1)
	at groovy.lang.GroovyShell.evaluate(GroovyShell.java:527)
	at groovy.lang.GroovyShell.evaluate(GroovyShell.java:565)
	at groovy.lang.GroovyShell.evaluate(GroovyShell.java:536)
	at com.vaadin.ui.EventSetDescriptorTest.groovyClosureTest(EventSetDescriptorTest.java:155)
	at com.vaadin.ui.EventSetDescriptorTest.assertValidEventSetDescriptors(EventSetDescriptorTest.java:113)
	at com.vaadin.ui.EventSetDescriptorTest.assertValidEventSetDescriptors(EventSetDescriptorTest.java:86)
	at com.vaadin.ui.EventSetDescriptorTest.testEventSetDescriptors(EventSetDescriptorTest.java:39)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:44)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:41)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:28)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:76)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:193)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:52)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:191)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:42)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:184)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:236)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:46)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

The code works against 6.1.5, but there are failures under 6.2. AFAICT, there is nothing particularly wrong with the code that creates the custom BeanInfo classes. It’s just that the Groovy mechanism for implementing this idiom is incompatible with the new APIs.

When I upgraded to 6.2, I added some exclusions to the unit tests so that the new failures issue warnings instead.

I’ve poked around with it some trying to coax it to work, but haven’t seen any obvious way to fix these new failures by modifying the BeanInfo. If it is possible, I’d like to know.

My $0.02, is that the Vaadin team should try to ensure that additional Listener/Handler interfaces/methods are added to the core components in such a way that this Groovy idiom does not break.

Hi

Let me add a disclaimer first that I have never ever used Groovy :slight_smile:

Anyway, I had a quick look at this an to me it seems the VaadinBeanInfoHelper returns duplicate EventSetDescriptors (at least) for the “click” listener in Panel. Especially the following part seems weird to me:


BeanInfo defaultBeanInfo = Introspector.getBeanInfo(targetClass,
		Introspector.IGNORE_IMMEDIATE_BEANINFO);

// Extract the default EventSetDescriptors
EventSetDescriptor[] parentDescriptors = defaultBeanInfo.getEventSetDescriptors();
if (parentDescriptors != null) {
	descriptorList.addAll(Arrays.asList(parentDescriptors));
}

// Now add all the Listener event sets
descriptorList.addAll(process(defaultBeanInfo, "Listener"));

Here you combine what defaultBeanInfo.getEventSetDescriptors() returns for the class with what process() returns. Both of these include the [public abstract void com.vaadin.event.MouseEvents$ClickListener.click(com.vaadin.event.MouseEvents$ClickEvent)]
method. This is bound to cause confusion later on.

In what way is Groovy mechanism incompatible with the new APIs? And what would be the requirements for the listener/handlers for them to work with Groovy?

Well spotted Artur :slight_smile:

I just did a quick test (changing from a LinkedList to a HashSet) and that removed the duplicates.


protected EventSetDescriptor[] getEventSetDescriptors(Class targetClass) {
		Set<EventSetDescriptor> descriptorList = new HashSet<EventSetDescriptor>();
		try {

			// Get the default BeanInfo
			BeanInfo defaultBeanInfo = Introspector.getBeanInfo(targetClass,
					Introspector.IGNORE_IMMEDIATE_BEANINFO);

/Jeppe

Thanks for spotting this.

My thinking was to add to whatever descriptors might already be present, but since this is being done mechanically, it does indeed cause problems. One could either filter out potential duplicates, or just discard the default EventSetDescriptors. I did the latter, which clears up some failures.

The requirements would be that the Groovy mechanism works for all the listeners. Specifically, that means, I believe, that all of the methods on the listeners for a given class need to be unique. Additionally, there cannot be a property/method on the component with the same name as the Listener method.

If you modify the UnitTest to test some of the new Components, such as PopupDateField, InlineDateField, you will note a failure with the focus event. This is because the listener method has a method called “focus” and there is a method on the component called focus. This naming collision prevents groovy from dynamically adding a property called “focus” to hold the closure reference.

Not a big deal, but would be nice if future API enhancements took this into account so that this “trick” works consistently.

There is a little summary of how this works here at the bottom of the page:

http://groovy.codehaus.org/Groovy+Beans

Here are the “failures” due to naming collisions in the Vaadin API that I mentioned. I have modified the unit test to simply warn instead of fail.

Again, simply naming the FocusListener.focus() method FocusListener.onFocus() or something would have prevented the collision with DateField.focus()


T E S T S

Running com.vaadin.ui.EventSetDescriptorTest
Jan 5, 2010 3:01:25 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: action ‘blur’ on com.vaadin.ui.DateField will not work with groovy using closure idiom for event registration
Jan 5, 2010 3:01:25 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: reason is No such property: blur for class: com.vaadin.ui.DateField
Jan 5, 2010 3:01:25 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: action ‘blur’ on com.vaadin.ui.InlineDateField will not work with groovy using closure idiom for event registration
Jan 5, 2010 3:01:25 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: reason is No such property: blur for class: com.vaadin.ui.InlineDateField
Jan 5, 2010 3:01:26 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: action ‘blur’ on com.vaadin.ui.PopupDateField will not work with groovy using closure idiom for event registration
Jan 5, 2010 3:01:26 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: reason is No such property: blur for class: com.vaadin.ui.PopupDateField
Jan 5, 2010 3:01:26 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: action ‘focus’ on com.vaadin.ui.RichTextArea will not work with groovy using closure idiom for event registration
Jan 5, 2010 3:01:26 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: reason is No such property: focus for class: com.vaadin.ui.RichTextArea
Jan 5, 2010 3:01:26 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: action ‘blur’ on com.vaadin.ui.Select will not work with groovy using closure idiom for event registration
Jan 5, 2010 3:01:26 PM com.vaadin.ui.EventSetDescriptorTest assertValidEventSetDescriptors
WARNING: reason is No such property: blur for class: com.vaadin.ui.Select
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.334 sec