Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 327293

Summary:

Event Admin should support Collection<String> type for property event.topics.

Product:

[Eclipse Project] Equinox

Reporter:

John Ross <jwross>

Component:

Compendium

Assignee:

equinox.compendium-inbox <equinox.compendium-inbox>

Status:

RESOLVED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

tjwatson

Version:

unspecified

Target Milestone:

3.7 M3

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch 1	tjwatson: iplog+
Updated patch which creates proper sized array argument	none

Description John Ross

2010-10-07 23:01:01 EDT

Build Identifier: 

The EventHandlerWrapper currently recognizes types String and String[] for the event.topics property. It should also recognize type Collection<String> per 4.3.

Reproducible: Always

Comment 1 John Ross

2010-10-07 23:05:28 EDT

Created attachment 180468 [details]
Patch 1

Tests for instanceof Collection. Attempts to convert to String[]. Catches ArrayStoreException and logs issue.

Comment 2 BJ Hargrave

2010-10-11 23:02:10 EDT

Created attachment 180630 [details]
Updated patch which creates proper sized array argument 

I updated the patch to create the argument to toArray of the proper size. This avoids creating a throw away empty array and requiring toArray to create the proper sized array with Array.newInstance.

Comment 3 John Ross

2010-10-12 11:28:54 EDT

I agree with this approach. 

I did it that way initially because this way left a warning (which you suppressed), and the code was in a section unlikely to present a noticeable performance hit despite the extra array creation. I wasn't comfortable with simply suppressing the warning before receiving some input. It was a close call whether or not to do it your way initially and leave the warning or the way I did it without the warning.

This is actually a good reinforcement of the role of generics and the impact of erasure. When I first looked at the updated patch, I immediately wanted to wail that we needed to catch a ClassCastException instead of an ArrayStoreException before settling down.

I'm going to obsolete Patch 1.

Comment 4 John Ross

2010-10-12 11:29:32 EDT

Comment on attachment 180468 [details]
Patch 1

Obsolete in favor of the updated patch.

Comment 5 Thomas Watson

2010-10-13 15:56:02 EDT

I released the updated patch from BJ (after updating copyright dates).  Thanks for the contribution John, I marked your original patch for the iplog.