Rudiger,
No worries. I took a few days last week as well, and I think Nigel's back
with us now.
On Sun, Aug 28, 2011 at 4:52 PM, Rüdiger zu Dohna
<ruediger.dohna_at_1und1.de>wrote:
> Hi,
>
> sorry for taking so long to reply... I was on vacation with the family,
> i.e. offline for two weeks.
>
> Nigels comments on the AtInject proposal seem quite comprehensive and
> detailled, while the Event Messaging proposal goes quite uncommented, yet,
> so I'll concentrate on that. CDI event binding is obviously a viable
> solution and seems to have support in the EG. As I've stated before, I'd
> prefer to instead bind to a business interface, but I'll leave that
> discussion to a separate thread. Here are my suggestions to improve the
> current proposal, in no particular order:
>
I thought about your business interface binding as well. Assuming CDI 1.1
supports service handlers, I think this is more extension rather than core.
I don't know that it makes sense to standardize, especially when the
arguments are all per application.
>
> * ObjectMessages seem an obvious choice, but I think that they are not a
> good default. Asynchronous messages are very different from synchronous
> calls, also in that there may be intermediate instances that need to read
> them, namely tools to debug e.g. error queues, or content based routers.
> With ObjectMessages containing application specific classes, these tools
> require the proper class files of the message objects being sent. But it's
> often not feasible to share with them all versions of all message objects;
> so it seems to me to be a better default to use generic message formats,
> like xml or mapped messages.
>
I run into class loader issues all the time. I've thought off and on about
how to automate binding, and the only decent way I can think of is to
leverage JAX-B and make it look like to the producer and consumer that it's
the reall object.
>
> * How are the qualifiers mentioned as Annotation[] in the code examples on
> page 5 transported from the sender to the receiver?
>
I don't know that we have to specify how, but just note that it's required
that they do so. This would be transparent to the application developer.
This is a good call out though, since it would require that all annotations
appear on both sides.
>
> * I think it's worth to mention that it's a required change to allow
> MessageListeners in an EE environment.
>
That's one way to look at it. Though I think we still need to explore the
new MDB replacement a bit more before we simply say message listeners (and I
probably made this assumption as well).
>
> * Defining all defaults, e.g. the default factory attribute of the
> JmsConnection annotation, would improve portability, wouldn't it? The spec
> doesn't have to define the exact value in this case, but the exact semantics
> should be set, shouldn't it?
>
At least in my history, all of these are really application specific. I'm
not sure a default would always help, unless it were application
configurable defaults. This would definitely help with readability.
>
> * I think that it would be more consistent with the EJB behavior of
> automatically managing transactions, if the default of the
> JmsConnection#transacted attribute were true.
> * Maybe it would be easier and suffice if "transacted" were not a separate
> attribute but a fourth acknowledge mode (as I understand it, this is what
> Nigel suggests in JMS_SPEC-45).
>
We'll need to mull on it. I set it to false since I am targetting more SE
than EE work. I may have read Nigel's comments wrong then, this would be
cleaner.
>
> * The JmsCredentials annotation (used literal as well as plain) is not
> really an improvement over the current security situation of JMS client
> code. But the only real solution that I can think of may be out of scope, as
> it's a general EE issue: Every application has some identity and required
> privileges to e.g. send messages to some queue. The deployer checks this
> identity and grants it the requested privileges. So the developer only
> defines what the application needs, while the deployer checks and fulfills
> these needs.
>
This sort of crosses both of my proposals. I really only think of
credentials in an SE environment. As I go through this more and more, I
really start wondering if we need a jms.xml deployment descriptor...
>
> * Page 4, 2nd client guarantee, 2nd sentence is unclear to me... and what's
> 3a?
>
> * Page 4 (and later on): The MessageObject type irritates me. Isn't it
> meant to be the StatusIdMessage used before?
>
I believe that's the case. I messed up my docs it would seem. 3a is the
first bullet under #2, when there was another listing there and they were
alphas not bullets. :-)
>
> * Page 4, SomeQueueLiteral: If it's a singleton, the constructor should be
> public... and as it may be serialized it would be better to use a
> one-value-enum (cf. Effective Java). If it's not a singleton, you'd need
> equals and hashCode, so it's a proper value object. Don't have to code it
> but mention it.
>
Doesn't need to be a singleton in this case. I think you mean shouldn't.
Annotation literals are very specific in how they need to be structured to
work with CDI, so need to think if anything's missing.
>
> * Page 5, 2nd (non-code) sentence (and last code snippet and paragraph on
> the same page): The property name is not "someProperty" but "eventType",
> isn't it?
>
Yes
>
> * Page 5, 2nd "contract" bullet point: Do you really suggest that messages
> that can't be converted for some reason be dropped with only an optional
> waring in some logs?!?
>
> Maybe. Would you expect that they generate an object message that has the
serialized object as the payload?
> * Page 5, last paragraph, 2nd to last sentence: I just don't get it. Why is
> the message selector supposed to be an interceptor? How it it supposed to do
> the routing, if there are multiple message selectors? How is
> transactionality related?
>
I suppose a message should only be considered delivered if it found a
matching observer. In this case, an interceptor could be used to evaluate
if the selector matches the message criteria, and only forward to the method
if it's valid. Thoughts?
>
> * Page 6-8, API Changes: I'd prefer to read (eventually commented) code for
> all annotations and enumerations.
>
I was thinking about this as well. I know that the code's sitting in an SVN
repository:
http://java.net/projects/jms-spec/sources/repository/show but
maybe we could take a note from other projects and work in a distributed
SCM? E.g. github or mercurial? I know that some specs are working outside
the box with these respects. Should get some feedback from Nigel about this
first.
>
>
> And I really like the improvement over the 1.1 spec!!!
>
> Rüdiger
John