On 22/05/14 15:30, Pavel Bucek wrote:
> On 22/05/14 10:34, Mark Thomas wrote:
>> On 21/05/2014 18:49, Pavel Bucek wrote:
>>> Please provide any feedback by COB Friday - 5/30/2014. We plan to send
>>> this to JCP in the first week of June to start the 30 day review period
>>> for this MR.
>> 1. We should deprecate the existing addMessageHandler() methods.
>
> I would like to do that as well, unfortunately it seems like there is
> a strong push for not using @Deprecated. Let me re-check - I'll keep
> this thread updated.
Seems like the recommendation for all specs which are part of Java EE is
to not directly use @Deprecated, especially when the method still can be
used and won't be removed. And that is exactly our case, so updated
javadoc would need to be enough.
>> 2. We should define and document the behaviour (throw an Exception? If
>> so, what type?) of the existing addMessageHandler() methods if the type
>> can not be determined.
>
> I see the issue with correctly detecting the case when you cannot get
> the type information. Additionally, we would need to define cases
> where we can get the type information safely, which might be also
> problematic (and it might evolve in time, I recall some changes in the
> code which tries to get the type argument because of Java SE 7
> release), so the exception might be then thrown nondeterministically.
> I would let this for implementations to handle this in their own way
> (for example, Tyrus defaults to Object.class when the generic type
> does not seem to be there, other implementation might throw
> IllegalArgumentException or just log it and ignore that message handler).
>
>> The code Tomcat uses to figure out the correct type for these methods
>> makes my head hurt every time I have to work on it. I'd love to be able
>> to throw that code away but that means making the existing
>> addMessageHandler() methods NO-OPs or having them throw an exception -
>> neither of which is good for backward compatibility.
>
> I completely agree, but we cannot do that, especially not for this
> method. It is pretty important in programmatic case and we cannot let
> fail all applications using this method..
>
> Thanks,
> Pavel
>
>>
>> Mark
>>
>>
>>
>>> Thanks,
>>> Pavel
>>>
>>> On 21/05/14 11:22, Pavel Bucek wrote:
>>>> Hi all,
>>>>
>>>> as you might have noticed, I filed blocker bug against WEBSOCKET_SPEC
>>>> project - [1].
>>>>
>>>> The main issue is that current Session.addMessageHandler method cannot
>>>> handle message handlers in form of lambda expressions, because there
>>>> is no information about its generic type parameter available. We
>>>> discussed this issue with Brian Goetz and he pointed out that current
>>>> API is wrong not only for this case, but also for more complicated
>>>> generics usages and is reliable only for anonymous classes created
>>>> directly from MessageHandler.Whole and MessageHandler.Partial (type
>>>> information is in the generated class file), so the issue itself is
>>>> not limited only to Java SE 8.
>>>>
>>>> We think that this issue is important enough to fix it in a
>>>> Maintenance Release as soon as possible, not tied to Java EE 8
>>>> planning or anything else.
>>>>
>>>> Proposed solution is to add two additional Session.addMessageHandler
>>>> methods with explicit type information, please see [2] for more
>>>> complete description. I also attached updated version of the
>>>> specification document. There is only one addition - last paragraph in
>>>> chapter 2.1.3 "Receiving Messages" and the sample code in chapter
>>>> 2.1.4 "Sending Messages" was modified to use the newly introduced
>>>> addMessageHandler method with explicit type.
>>>>
>>>> Complete change diff can be seen here [3] (but it contains lots of
>>>> noise - spec licence etc; changes.txt should be good enough for
>>>> evaluation).
>>>>
>>>> Any feedback would be greatly appreciated!
>>>>
>>>> Thanks and regards,
>>>> Pavel
>>>>
>>>> [1]: https://java.net/jira/browse/WEBSOCKET_SPEC-226
>>>> [2]:
>>>> https://github.com/pavelbucek/websocket-spec/blob/WEBSOCKET_SPEC-226/websocket-1.1-changes.txt
>>>>
>>>>
>>>> [3]: https://github.com/pavelbucek/websocket-spec/pull/1/files
>>
>>
>
>