C++ API issues with ValueType et. al.

[John Dennis <jd...@redhat.com>]

I recently attempted to write code which would serialize AMQP data into 
different formats and ran into a number of issues with the Qpid C++ API. 
Serialization code unlike application specific code does not have a 
prori knowledge of the format of the data, it must rely soley on what it 
finds in the data object presented (e.g. inspection).

1) Let start with a FieldTable which is a map whose values are 
FieldValue objects. The FieldTable object provides a number of accessors 
which return the value of a key as a specific type (e.g. getAsUInt64()). 
However if you're iterating through a FieldTable and have a reference to 
a FieldValue you run into problems. First you begin by calling getType() 
to learn the type of the value, but a FieldValue has a very limited set 
of type specific accessors (either on the FieldValue object or on the 
nested Data object which encapsulates the value). Here are a few quick 
observations/questions related to FieldValues:

* Shouldn't a FieldValue have an accessor method for each basic data type?

* There is a limited set of specialized template get() functions. But 
you have to know which ones are actually implemented or the generalized 
template throws a InvalidConversionException.

* It feels wrong from an API perspective to use templated functions 
rather than accessor methods, one has to know the right way to 
parameterize the template and which ones are implemented.

* If you know enough about how the code is implemented you can reach 
into the nested Data object in some circumstances, but from an API 
perspective this seems wrong, one is exposing too much implementation 
detail.

* You can't get basic types such as float or double, this requires one 
to return back to the FieldTable you're iterating through and use it's 
accessor method by performing a needless and somewhat expensive lookup 
by field name which internally just gets you back to the exact same 
FieldValue reference you already have in hand but can't use directly. 
Plus this won't work if and when lists are implemented (see below).

* There appears to be no direct support in either FieldTables nor 
FieldValues for unsigned integers whose width is less than 64 bits.


2) Version 10 of the AMQP protocol specifies two types of sequences, 
lists and arrays. Arrays are homogeneous, lists are heterogeneous.

* Qpid only implements arrays, lists appear to be unimplemented.

* Qpid's implementation of arrays appears to be incorrect, or at least 
inefficient. Arrays in Qpid are sequences of FieldValues, FieldValues 
embed the type information, but in an array the type is a property of 
the array, not it's individual elements. As a consequence one could end 
up with a element whose type does not match the type bound to the array 
(I didn't find any code which enforces the consistency of elements, but 
maybe I missed it).

* Shouldn't what Qpid 0.10 currently calls an Array actually be called a 
List and then add an implementation for Arrays?


-- 
John Dennis <jd...@redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:users-subscribe@qpid.apache.org

Re: C++ API issues with ValueType et. al.

[Gordon Sim <gs...@redhat.com>]

Apologies for the delayed response, I had a few other issues that 
distracted my from this task...

John Dennis wrote:
> On 07/16/2009 07:03 AM, Gordon Sim wrote:
>> John,
>>
>> I agree with every point you make. The FieldTable and Array classes are
>> pretty horrible to use except for the simple cases and lack consistency
>> in design. They also expose boost which we are trying to move away
>> from[1] and tie the encoding details to the data structure[2] and
>> application visible API. In short, though we will obviously continue to
>> support it for existing users, I don't think that FieldTable in its
>> current form is really suitable as part of the exposed API in the long
>> term.
> 
> Thanks for the follow-up Gordon. I think you answered my high order bit 
> question. Do we fix what we have or do we re-implement it? 

I've gone back and forth on that question. I think we need to keep
FieldTable, and ideally not break existing applications or ABI.

What I've done so far is to define a new Variant type and have maps and 
lists of those. This is part of a proposed higher level api for 
messaging (see separate mail).

In terms of the implementation I've currently mapped these onto 
FieldValues for encode/decode so that I can use the existing 0-10 client 
code beneath the new api. Doing this I hit all the issues you discussed 
and so have modified FieldValue and FieldTable to address them (at least 
to a degree). I have also add a List type corresponding to the amqp0-10 
list.

> I was going 
> to offer to send a set of patches for the current implementation but in 
> a conversation with Ted Ross he cautioned patience because of the 
> possibility this area of the API might be reworked.

Attached is a patch that contains just the changes to FieldTable and 
FieldValue, plus the addition of the List class.

> It's good to know 
> there is a consensus the existing implementation needs some love and I 
> wasn't just out in left field somewhere due to misunderstandings.

No, it's definitely an area in need of some attention! My view is still 
that the best solution is an alternative api however as described I've 
ended up wanting to make some additions to the existing api as well (I 
believe these are all backwards compatible).

>> Some time ago the desirability of a higher level API for the c++ client
>> was discussed and I started some work on that. For the past couple of
>> months my attention has been diverted by other work but I hope to return
>> to it now. This API will include a variant type and a corresponding map
>> of these through which message properties would be accessed. The
>> intention is also to provide direct support for messages whose content
>> is an encoded form of a map or list. I still have some details to work
>> through of course, including how to avoid breaking the existing API
>> while avoiding expensive conversions.
> 
> At the moment I'm fine with waiting for the new implementation, I can 
> work around the issues from my end. I'll look forward to a cleaner 
> design when you have it ready. If you would like to bounce some design 
> ideas off me I'd be happy to provide feedback, or if there is a design 
> proposal on a wiki or some such you might want to provide a pointer to 
> that works as well.

That would be great! You can have a look at the framing namespace 
changes in the attached patch (unless anyone objects I'll probably check 
this part in shortly); for the alternative see the other mail on the 
messaging api.

--Gordon.

Re: C++ API issues with ValueType et. al.

[John Dennis <jd...@redhat.com>]

On 07/16/2009 07:03 AM, Gordon Sim wrote:
> John,
>
> I agree with every point you make. The FieldTable and Array classes are
> pretty horrible to use except for the simple cases and lack consistency
> in design. They also expose boost which we are trying to move away
> from[1] and tie the encoding details to the data structure[2] and
> application visible API. In short, though we will obviously continue to
> support it for existing users, I don't think that FieldTable in its
> current form is really suitable as part of the exposed API in the long
> term.

Thanks for the follow-up Gordon. I think you answered my high order bit 
question. Do we fix what we have or do we re-implement it? I was going 
to offer to send a set of patches for the current implementation but in 
a conversation with Ted Ross he cautioned patience because of the 
possibility this area of the API might be reworked. It's good to know 
there is a consensus the existing implementation needs some love and I 
wasn't just out in left field somewhere due to misunderstandings.


> Some time ago the desirability of a higher level API for the c++ client
> was discussed and I started some work on that. For the past couple of
> months my attention has been diverted by other work but I hope to return
> to it now. This API will include a variant type and a corresponding map
> of these through which message properties would be accessed. The
> intention is also to provide direct support for messages whose content
> is an encoded form of a map or list. I still have some details to work
> through of course, including how to avoid breaking the existing API
> while avoiding expensive conversions.

At the moment I'm fine with waiting for the new implementation, I can 
work around the issues from my end. I'll look forward to a cleaner 
design when you have it ready. If you would like to bounce some design 
ideas off me I'd be happy to provide feedback, or if there is a design 
proposal on a wiki or some such you might want to provide a pointer to 
that works as well.

Thanks for your good work and attention.

John


-- 
John Dennis <jd...@redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:users-subscribe@qpid.apache.org

Re: C++ API issues with ValueType et. al.

[Gordon Sim <gs...@redhat.com>]

John,

I agree with every point you make. The FieldTable and Array classes are 
pretty horrible to use except for the simple cases and lack consistency 
in design. They also expose boost which we are trying to move away 
from[1] and tie the encoding details to the data structure[2] and 
application visible API. In short, though we will obviously continue to 
support it for existing users, I don't think that FieldTable in its 
current form is really suitable as part of the exposed API in the long term.

Some time ago the desirability of a higher level API for the c++ client 
was discussed and I started some work on that. For the past couple of 
months my attention has been diverted by other work but I hope to return 
to it now. This API will include a variant type and a corresponding map 
of these through which message properties would be accessed. The 
intention is also to provide direct support for messages whose content 
is an encoded form of a map or list. I still have some details to work 
through of course, including how to avoid breaking the existing API 
while avoiding expensive conversions.

--Gordon

[1] https://issues.apache.org/jira/browse/QPID-1780

[2] I see we now have two encodings, with qmfEncode/qmfDecode having 
been added, but I think the point still holds.




---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:users-subscribe@qpid.apache.org