On 19 February 2015 at 16:11, Rajith Muditha Attapattu
<rajit...@gmail.com> wrote:
> On Thu, Feb 19, 2015 at 5:41 AM, Robbie Gemmell <robbie.gemm...@gmail.com>
> wrote:
>> On 19 February 2015 at 04:22, Rafael Schloming <r...@alum.mit.edu> wrote:
>> > On Wed, Feb 18, 2015 at 3:58 PM, Rajith Muditha Attapattu <
>> > rajit...@gmail.com> wrote:
>> >
>> >> Setting the message body for an o.a.q.proton.message.Message is slightly
>> >> awkward.
>> >> You have to create a AmqpValue. AmqpSequence or a Data object that
>> >> encapsulates the underlying data type.
>> >>
>> >> Given that one of our goals is to expose an API that works with standard
>> >> Java types instead of AMQP defined types, I suggest we simply use Object
>> >> for setBody and getBody methods.
>> >> The implementation can then handle the conversion back and forth
>> >> underneath.
>> >>
>> >> What do you think?
>> >>
>> >
>> > Makes sense to me.
>> >
>> > --Rafael
>> Adding additional functionality to set/get a 'body object' like this
>> certainly might be useful in cases, but I dont think it should be the
>> only way. I also wouldnt change the existing methods to do this unless
>> the idea is to let you continue setting/getting the existing Section
>> objects unchanged (it would obviously likely break all existing user
>> code sending payloads otherwise). I can see that being easy enough for
>> the setter as it works now, though I dont see how it could work for
>> the getter without adding a control toggle.
> The Message interface is part of the public API and I understand the impact
> on existing code out there.
> **I don't plan to remove anything or even change any existing public API.
> At least not for the upcoming release.**

Yes, I understand that this isnt aimed at 0.9. When it lands doesnt
particularly change my perspective however.

> So far I have sketched out a parallel API for certain pieces of the public
> API. Message handling is one of them.
> What I would like to have is a discussion based on that, as it appears the
> existing API had little discussion or scrutiny.
> An API should be easy to use and work with standard Java types as much as
> possible. The existing API is awkward to use in that respect.

I'm not saying it has been scrutinized, but in regards to the setBody
specifically, I dont think it was chosen particularly randomly. There
are certainly other AMQP 1.0 API's that exist which work quite
similarly in that particular regard.

Easy to use doesn't necessarily mean using the Java types. Where it
makes sense, sure. Where they can't fully support the basic
functionality, using additional types seems fine and there are
obviously many which do that.

Most of the things I'd consider most awkward about about the current
codebase have little to do with the Message, but rather all the stuff
you need before you get that far. The main thing that would seem
confusing about Message to me are bits like the encode/decode vs
load/save stuff, and all the related methods for the latter that dont
really say what they do, but involve gems like the 'setMessageFormat'
method that doesnt set the 'Message Format' field of the AMQP message
transfer but rather has some other unspecified behaviour you need to
read the code to figure out.

Mostly, I find other areas being much more awkard, like many of the
types required to set up and operate connections, sessions, links etc
being in a bunch of different packages that arent as obvious as they
could be, and perhaps even interfaces and implementations with the
same name (e.g Source). That is all possibly in part because there was
only impl initially, then it got split to completely separate
API+Impl, then it all got pushed back together again. There were
efforts at having a bunch of separate factory objects, then the
aggregating Proton factory object, then yet-different seperate factory
objects, and also new factory objects in the interfaces. Figuring out
those types of things are far more annoying to get your head around in
my view. If you get as far as needing to set a body section, you have
done much harder work. I might have added a body specific interface
for set/getBody rather than just using the Section interface, since
that permits the other non-body sections.

> If I want to send a String or a Map, I have to wrap it with an AMQP type
> first. Which is awkward at best.

As I said, I can definitely see there also being merit in having an
API that you set/get simple 'body objects' such as String with. I just
dont think it should be the only way as outlined, particularly since
it hasnt been from the start.

>> Doing this would simply some use cases, but also further restrict the
>> ability to send the bodies you want or know what you really received,
>> e.g is a List an AmqpvValue+List or an AmqpSequence?
> From a typical application programmers perspective,  a List wrapped in an
> AmqpValue or an AmqpSequence makes little difference.

It makes a fairly big difference if you e.g want to send sequences.

> What the user cares about is that it's a List, not the underlying AMQP type.
> Unless I have missed some subtlety, I don't see much of a benefit in
> figuring out what the underlying AMQP type is.
> If they care about that level of detail, then perhaps they are working at
> the wrong level of granularity.
> They should instead use the codec API for that.
> Robbie, can you please give us a use case to further explain your point?

I'm mainly getting at that by making the proposed change, we would be
saying that the Message API probably wont actually support sending
lots of varieties of AMQP message content, or perhaps not receiving
them. Sequences allow you to do things that value+list do not, and
data allows you to do things that value+binary do not (I can only
presume thats part of the reason they both exist?).  The JMS mapping
for example uses sequences and data to allow for things that wouldnt
be possible using value+list or value+binary. Even if it didnt use
them as a matter of course for its basic message types though, it is
also a goal to let people send/recv fairly arbitrary AMQP message
through the JMS API, which could involve wanting to use them.

Again, I'm all for adding a simpler way to do things which people can
use where those are sufficient, but it seems weird that it would be
made the only way, which would make Message be unable to do reasonable
things it can currently do, and will break most existing code over
something that isnt particularly hard to use as is, particularly when
compared to the rest of the codebase.

It also feels a little weird that something [originally] written
mostly by the key authors of the protocol, as a toolkit allowing
others to leverage the protocol without digging deep into a protocol
implementation, wouldnt actually let you use certain basic
functionality of the messages without doing so. Perhaps such people
should go and use the codec, but will it become similarly hobbled at
some point?

>> Is <some-object>
>> an AmqpValue+Binary, or is it a Data?
> Again the same question. For 99% of the use cases out there, the user will
> only care about the underlying byte[].
> Could you pls let us know a use case where such a distinction is useful ?
>> There is then things like how to
> support e.g multiple Data and AmqpSequence sections (something it
>> should already support but doesnt).
>> Yea I noticed this was missing. More thoughts on that later.
>> Robbie

Reply via email to