qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rajith Attapattu" <rajit...@gmail.com>
Subject Re: on how to use the qpid java client
Date Mon, 04 Jun 2007 17:03:30 GMT
Gordon,

Thank you very much for the feedback !!!!!
The API certainly needs some tweaking and I wasn't able to put out a design
document as promised.
I still have some unfinished work, especially the message factories. Thats
why I didn't make an official announcement.
I checked in bcos, the toronto office had a hardware failure and we don't
have access to our home directories for about a week now.
My comments are inline and thanks again for the feedback.

Regards,

Rajith

On 6/4/07, Gordon Sim <gsim@redhat.com> wrote:
>
> Gordon Sim wrote:
> > rajith@apache.org wrote:
> >> Author: rajith
> >> Date: Fri Jun  1 13:40:34 2007
> >> New Revision: 543602
> >>
> >> URL: http://svn.apache.org/viewvc?view=rev&rev=543602
> >> Log:
> >> more enchancements for the Qpid java client. Also I have checked in a
> >> sample client(QpidTestClient) on how to use the qpid java client.
> >
> > Rajith,
> >
> > Great to see this coming together! The sample was perfect for getting a
> > quick overview of how the API is to work. It will also be very useful in
> > focusing the discussion on more uniformity across the qpid clients.
> >
> > I've attached some comments on that sample. I have to confess though
> > that I didn't investigate the APIs directly so some of these comments
> > may be based on ignorance of what is already there. A lot of them are
> > also probably based on prejudices acquired in connection with the c++ or
> > python clients!
> >
> > --Gordon.
> >
>
> Many apologies; comments are now attached as originally promised!
>
>
>
>
>     QpidConnection con = new QpidConnectionImpl();
>     con.connect("amqp://guest:guest@test
> /test?brokerlist='tcp://localhost:5672?'");
>
> Rather than having to remember the format of this URL scheme, I'd
> prefer (overloaded) method arguments for the constitutent parts. E.g.
> con.connect("localhost", 5672, "test", "guest", "guest"). Or perhaps a
> properties object with some defined keys (i.e. similar to how JDBC
> connections are often configured).


+1,  I will add that additional method.  JMS was the focus and they usually
use a url.
But u have a very valid point and I am happy to add that.

    QpidSession session = con.createSession(
> QpidConstants.SESSION_EXPIRY_TIED_TO_CHANNEL);
>     session.open();
>
> Though a minor point, there should be a createSession() with no
> argument that assumes the expiry is tied to the channel. That keeps
> the simple case simple.


+1 again. Will add.

    QpidExchangeHelper exchangeHelper = session.getExchangeHelper();
>     exchangeHelper.declareExchange(false, false,
> QpidConstants.DIRECT_EXCHANGE_NAME, false, false, false,
> QpidConstants.DIRECT_EXCHANGE_CLASS);
>
>     QpidQueueHelper queueHelper = session.getQueueHelper();
>     queueHelper.declareQueue(false, false, false, false, false,
> "myQueue");
>     queueHelper.bindQueue(QpidConstants.DIRECT_EXCHANGE_NAME, false,
> "myQueue", "RH");
>
> Perhaps a bit controversial, but how about e.g.
>
> session.exchange.declare(false, false ...);
> session.queue.declare(false, false, ...);
> session.queue.bind(QpidConstants.DIRECT_EXCHANGE_NAME, ...);
>
> or (probably even more controversially!):
>
> session.exchangeDeclare(false, false ...);
> session.queueDeclare(false, false, ...);
> session.queueBind(QpidConstants.DIRECT_EXCHANGE_NAME, ...);
>
> I thought hard about option 1 & 3 (didn't think about option 2).
I stayed away from option 3 bcos,
a) The session class becomes more complex and lengthy with all the methods
added (a problem with the current code)
b) If we hide Queue, Exchange and Message stuff behind interfaces then we
can clearly isolate these areas and changes. modifications are easy managed.

I thought if I compartmentalize these functional areas, it will make the
code more simple, manageable and readable.
However I agree that option 3 looks more natural and elegant, but I choose
to compromise them for the sake of above properties, keeping in mind that it
will help with more mundane tasks of maintenance and modifications.

Option 2 looks like a nice compromise, however it is a bit controversial. If
u guys like that approach, I can promote that.
thoughts ???


Ideally the method fields would be in the order defined in the spec or
> the order of relevance; in queue.declare for example, having the name
> first would be more 'appealing' to me. We should also expose the
> 'arguments' field table.


Gordon,  the argument order is in the exact order from the generated classes
from the spec.
I will look closely again to see if I have made any mistakes. But I agree
that the name as the first argument is more natural.

Very minor point: should it not be DIRECT_EXCHANGE_TYPE, rather than
> _CLASS?


Agreed. This was a direct  (blind) copy from the existing code :)
Will change this.

    MessageHeaders msgHeaders = new MessageHeaders();
>     msgHeaders.setRoutingKey(new AMQShortString("RH"));
>     msgHeaders.setExchange(new AMQShortString(
> QpidConstants.DIRECT_EXCHANGE_NAME));
>     AMQPApplicationMessage msg = new
> AMQPApplicationMessage(msgHeaders,"test".getBytes());
>
> The application should not have to care about AMQShortString; it
> should be able to pass in a normal java string and have the client
> wrap it if required.


This is purely laziness on my part.  I just wanted to test end to end so
this is a big hack :)
If u look at the other code (Connection, session, queue, exchange) I have
wrapped all the AMQ specifics like AMQString etc with java.lang.string to
make it more natural. Only thing that I am stuck with is the field table.

I want to introduce a message factory that will take in a few parameters and
produce a message.
//ex: MessageFactory.createTextMessage(msgText,exchangeName,routingKey);
//ex: MessageFactory.createXMLMessage(xmlText,exchangeName,routingKey);
//ex: MessageFactory.createBytesMessage(bytes,exchangeName,routingKey);

The discussions with Tomas and Jonathon also indicated that they would like
message factories.

My view on the exchange is that the application wouldn't directly set
> a message header, but would specify the exchange to which a publish
> was directed. I think I would do the same for routing key also.


Gordon, see my above comment. Creating messages can be hidden behind
factories.

    QpidMessageProducer messageProducer = session.createProducer();
>     messageProducer.open();
>     messageProducer.send(false, true, msg);
>
> I would directly expose e.g. the message.transfer method, rather than
> wrapping it behind a more generic producer interface.


Two things were on my mind.  First was the JMS  API and the second and more
important point was the current thinking reflected by
Rafi and Robert Godfrey about breaking the transfer into a send and receive
(may not be the exact terminology).

    QpidMessageConsumer messageConsumer = session.createConsumer("myQueue",
> false, false);
>     messageConsumer.open();
>
>     AMQPApplicationMessage msg2 = messageConsumer.receive();
>
> Again, I think I would lean towards more direct expression of the AMQP
> methods. So a message.consume method, and then the ability to set a
> listener or poll from a queue for that consumer. e.g.
>
> QpidMessageConsumer consumer = session.message.consume("myQueue", false,
> false);
> //either
> consumer.setListener(...);
> //or
> AMQPApplicationMessage msg = consumer.receive()


Gordon, I wanted the API methods to communicate  to the end user, as much
information as possible.
session.message.consume method would give the impression that u are
referencing an already created object.
While session.createConsumer will clearly communicate that you are creating
a new consumer.

I can do consumer.open inside the create method, but an explicit
consumer.open will remind them to close the resource too.
Or else it will only be closed when the session is closed.

There is always a fine line in deciding an API. Lots of things to balance
:). But your feedback is very much appreciated.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message