Oleg,
On 10/11/2012 18:34, Oleg Tsal-Tsalko wrote:
> Hi Nigel,
>
> Sorry for late answer.
> Some comments embedded.
>
> 2012/10/30 Nigel Deakin <nigel.deakin_at_oracle.com <mailto:nigel.deakin_at_oracle.com>>
>
> Oleg,
>
> Many thanks for your comments.
>
>
> On 28/10/2012 21:13, Oleg Tsal-Tsalko wrote:
>> Hi Nigel,
>>
>> Was reviewing javadocs for JMS2.0 specification.
>> Have several questions/proposals:
>>
>> *JMSConsumer:*
>>
>> A JMSConsumer object is created by passing a Destination object to one of the createConsumer or
>> createDurableConsumer methods on a JMSContext.
>>
>> This is not precise enough I think, cause JMSConsumer could be created by createSharedConsumer() methods also
>> and createDurableConsumer/createDurableConsumer methods are required passing exactly Topic, but not any
>> Destination...
>
> Yes, you're perfectly correct. How about this?
>
> A client using the simplified JMS API introduced for JMS 2.0 uses a |JMSConsumer| object to receive messages
> from a queue or topic. A |JMSConsumer| object may be created either created by passing a |Queue| or |Topic|
> object to one of the |createConsumer| methods on a |JMSContext|. or by passing a |Topic| object to one of the
> |createSharedConsumer| or |createDurableConsumer| methods on a |JMSContext|.
>
> Thnx for changing javadocs! But probably better to say:
> |JMSConsumer| object introduced in JMS 2.0 as part of simplified API is used to receive messages from a queue or
> topic. A |JMSConsumer| object can be created either by passing a |Destination| object to one of the
> |createConsumer| methods or by passing a |Topic| object to one of the |createSharedConsumer| or
> |createDurableConsumer| methods on a |JMSContext|.
Apart from polishing the wording, were you making any technical points here? (e.g. were you making a point by asking to
change my "queue or topic object" to "destination object"?).
>>
>> *JMSConsumer#get/setMessageListener():*
>>
>> This method must not be used in a Java EE web or EJB application. Doing so may cause a JMSRuntimeException to
>> be thrown though this is not guaranteed
>>
>> I suspect it is because preferred way to use asynchronous delivery in Java EE container is to use MDBs. Might be
>> worth saying it in javadocs. What do you think?
> It does say this in 10.2. "Restrictions on the use of JMS API in the Java EE web or EJB container". The same point
> would apply to same methods on MessageConsumer.
>
> I'm not sure this belongs in the javadoc for these particular methods, though. (We need to draw the line somewhere
> or we'll end up copying the entire spec into the API docs).
>
> Note that I don't know the historical reason for this restriction - this restriction has been in Java EE for a
> very long time.
>>
>> *JMSProducer#get/setPriority():*
>> What's the implications of using getPriority()/setPriority() methods?
>> Does it mean that messages will be placed in priority order on broker's message queue?
>> Or no guarantee and it is up to consumer how to use it?
>
> These methods use the same wording as the existing MessageProducer#set/getPriority methods. These methods are used
> to set the JMSPriority header of the message.
>
> The actual meaning of JMSPriority is defined in section 3.4.10. "JMSPriority", 4.4.10.2. "Order of message sends"
> and in one or two other places.
>
>>
>> *JMSProducer#send():*
>> I noticed that there are number of similar send methods differ only by type of payload to be send.
>> Not sure it is not planned yet, but why can't we introduce:
>> /<T> JMSProducer sendPayload(Destination dest, T payload)/
>> This will be consistent with new methods for receiving payloads.
>
> Are you asking to change the name of these methods from "send" to "sendPayload"?
>
> I think "send" is better as it is shorter and simpler. We can use "send" to send both payloads and messages - the
> compiler can use the parameter type to work out which method to use.
>
> If we're going to aim for consistency we could rename the new receivePayload() methods to be called receive()
>
> So
>
> receivePayload(Class<T> c) becomes receive(Class<T> c)
> receivePayload(Class<T> c, long timeout) becomes receive(Class<T> c, long timeout)
> receivePayloadNoWait(Class<T> c) becomes receiveNoWait(Class<T> c)
>
> There is no clash with the existing
>
> receive()
> receive(long timeout)
> receiveNoWait()
>
> Does anyone think this would be a good idea?
>
> My proposal was first of all to introduce one generic method
> /<T> JMSProducer sendPayload(Destination dest, T payload)/
> instead of number of send methods for different types of message bodies to be send.
> How to name it /send/ or /sendPayload - /it is another question. And I know that you are tending not to use /payload
> /notion anywhere in specification. So, might be:
> /<T> JMSProducer send(Destination dest, T payload)/
> What do you think about adding it to API?
We already have methods that allow a message payload (body) to be sent directly (and which return a JMSProducer to allow
method chaining)
JMSProducer send(Destination dest, Serializable body)
JMSProducer send(Destination dest, String body)
JMSProducer send(Destination dest, Map body)
JMSProducer send(Destination dest, byte[] body)
The application doesn't need to worry about which one to call: they simply pass in an appropriate type and the JVM works
out which one to call. And if you pass in an invalid type (e.g. Float) it won't compile.
You suggest
/<T> JMSProducer send(Destination dest, T payload)
/That would allow you to supply any object as an argument. This doesn't seem to me to be a benefit. If you try to pass
in, say, a Float you will get a runtime error rather than a compile time error, which I think is less good.
Bear in mind that none of these methods (not the JMSProducer method send(), not the JMSConsumer method receiveBody(),
not the Message method getBody()) do anything more than variable assignment. They don't attempt to convert between one
type and another (e.g. between a String and a Float). Is that what you were expecting? If so then think it is safer to
leave this to the application.
Nigel
>
>> *JMSProducer#get/setXXXProperty():*
>> I noticed that there are number of similar methods differ only by type of property used.
>> Why can't we introduce:
>> /<T> T getProperty(String name, Class<T> type)/
>> /<T> void setProperty(String name, T value)/
>> This will be also consistent with new methods for receiving payloads.
>
> Good question.
>
> The methods for setting/getting properties are a bit different from the methods for setting and getting payloads.
>
> The methods for setting/getting properties actually perform conversion between different types (e.g. String to
> int) whereas the methods for setting and getting payloads do not (they simply perform a cast). The conversion of
> message properties is already defined in JMS 1.1 and only a specific set of type conversions is listed. Allowing
> any class to be passed in would be misleading.
>
> In addition, the methods on JMSProducer to set and get properties are intended to be consistent with the existing
> JMS 1.1 methods on Message to set and get properties (bear in mind that setting a property on a JMSProducer is
> simply a convenient way to set a property on a Message). Although I have simplified these methods slightly I think
> it would be confusing to change the API completely and make them very different from the methods on Message.
>
>
>>
>> *JMSProducer#setAsync():*
>>
>> Restrictions on usage in Java EE An asynchronous send is not permitted in a Java EE EJB or web container. If
>> the application component violates this restriction this method may throw a JMSRuntimeException.
>>
>> Again might be worth to explain why. What do you think?
>
> An explanation might be worthwhile, but I think this should be in 4.6.2.5. "Restrictions on usage in Java EE"
> rather than the API docs (since this is not part if the API). I've made a note and I'll think about this.
>
>
>>
>> *JMSProducer#get/setJMSCorrelationIDAsBytes():*
>> What is the reason to support byte[] IDs?
>
> This is needed for consistency with Message.setJMSCorrelationIDAsBytes (which is in JMS 1.1). The API doc for
> Message.setJMSCorrelationIDAsBytes contains a few words about what it is for. I thought it best not to copy and
> paste the same text into JMSProducer.setJMSCorrelationIDAsBytes
>
> Nigel
>
>> Thank you,
>> Oleg
>
> Thank you,
> Oleg