C++: proposal to clean up the public API.

[Alan Conway <ac...@redhat.com>]

We need to clean up our public C++ API so that we can:
  - identify when we might be making incompatible changes that will break 
existing user code.
  - reduce our implementation exposure to incompatible changes.
Here are some thoughts in that direction, feedback appreciated:


* Public API design principles

** Rationale
Any change to code that can be seen by the user's compiler can potentially break 
compatibility.

There are two levels of compatibility:
- Binary compatible: user binaries built against old headers/libs can be linked 
with new libs and will work.
- Source compatible: user source code can be re-compiled without change and 
still work.

Future-proofing the API means:
- clearly identifying public header files - those that are installed and can be 
seen by user's compiler.
- minimizing the amount of code in public header files.
- hiding implementation as much as possible in public header files.

** Design Principles

Public header files live under cpp/include, private header files live under cpp/src
- Only headers in cpp/include are installed and visible to the user's compiler.


QPID_*_EXTERN declaration are required on:
  - all member functions intended to be called by users.
  - all member functions used by another qpid library (e.g. common lib functions 
used by client/broker libs)
See 
http://stevehuston.wordpress.com/2009/03/12/lessons-learned-converting-apache-qpid-to-build-dlls-on-windows/ 
for some discussion on this subject.

Public classes should fall into one of the following 3 categories:

Handle: handle to refcounted object (e.g. Connection, Session)
  - pure pointer to impl (PIMPL) idiom: no data except impl pointer, no virtual 
functions, no inlines.
  - qpid::client::Handle provides common base class
  - defined in client lib, namespace ::qpid::client

Interface: abstract base class, intended for user to subclass (e.g. MessageListener)
  - defined in client lib, namespace ::qpid::client

Value: data type with value semantics (e.g. std::string, FieldTable)
  - defined in common lib, namespace ::qpid
  - imported into ::qpid::client namespace with using statements.
  - more on value types below, this is the area that needs most work.

Note type system is defined in common lib, ::qpid namespace so it can be shared 
by client & broker code.
It is imported into ::qpid::client namespace so a typical client need only use 
the qpid::client namespace.


No boost headers included in public .h files: I think this is
feasible. It would the boost-devel requirement for clients and avoid
incompatibilities due to boost version changes.


* Value Types Proposal

Value types have the most exposed implementation so need to be kept as simple 
and clean as possible.
They should:
  - represent strictly the *users* view of the data type: no encode/decode functions
  - have value-semantics - no virtuals, normal copy semantics etc.
  - not be tied to a specific AMQP protocol version - this is the C++ view of 
the type.

Integral types: typedef int8_t Int8; typedef uint16_6 Uint16; ... etc.

String types: typedef std::string String;

Additional classes: class SequenceNumber; class SequenceSet; class Url etc.

Discriminated union type: needed by FieldTable and Url. Currently Url
uses a boost::variant which is not extensible, and FieldTable uses
FieldValue which has a somewhat ad-hoc interface.

Propose replacing both with class qpid::Any, modelled after boost::any
with some additional features described below.  This provides a
standard-ish (boost::any is proposed for a future C++ standard) API
that allows us to store *any* C++ type without modifications.

FieldTable: propose replacing with
   typedef std::map<std::string, Any> Map;
Keep deprecated framing::FieldTable for compatibility.

This allows code like:

   map["foo"] = std::string("abc"); // Creates an any containing std::string

   std::string* s = anyCast<std::string>(&map["bar"]);
   if (s) { do string stuff }

   uint16_t i = anyCast<uint16_t>(&map["x"]); // throw exception if not a uint16_t

as well as all the standard map iterator stuff.


** Relating C++ types to protocol types.

There is not an exact 1-1 mapping between protocol types and the basic
value types described above. E.g. the protocol's str8, str16, and
str32 all map to std::string.

Where we need a 1-1 mapping (e.g. in a Map) we use **type wrappers** e.g.

namespace qpid::amqp_0_10 {
   struct String8 { String value  }; // ctors & conversions to & from String 
omitted
   struct String16 { String value  };
   struct Vbin32 { String value };
   etc.
};

So to insert a value in a map that I want encoded as a string with an 8 bit length:

  map["foo"] = amqp::0_10::String8("bar");

An Any decoded off the wire will always use the wrapped form of a type. e.g.

  if (anyCast<amqp0_10::String8>(&any)) { ... }
  else if (anyCast<amqp0_10::String16>(any)) { ... }
  ...


To allow the user to avoid these protocol specifics we provide anyConvert():

   if (anyConvert<String>(&any)) { ... }

This will work if the any contains any type that can be converted to string.

We use **type traits** to associate protocol-specific type codes with C++ types.

  TypeCode<T>::value == type code for type T.

There is a default mapping for unwrapped types with multiple mappings, e.g.

  TypeCode<std::String>::value == typecode for String32

Finally we add one more function to get the type code from an any:

  anyTypeCode(any) returns the type code for the any.

To support code like:
  switch(anyTypeCode(any)) {
    case STR8_TYPE: ...
    case UINT16_TYPE: ...
  }
}

QUESTION: all of the above should probably be in a
qpid::amqp_<version> namespace?  Can we rely on the 0-10 codes not to
change meaning in future versions? 1.0 type system is simpler but is
it backwards compatible?


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

Re: C++: proposal to clean up the public API.

[Alan Conway <ac...@redhat.com>]

Ted Ross wrote:
> Alan Conway wrote:
>> Public classes should fall into one of the following 3 categories:
>>
>> Handle: handle to refcounted object (e.g. Connection, Session)
>>  - pure pointer to impl (PIMPL) idiom: no data except impl pointer, no 
>> virtual functions, no inlines.
>>  - qpid::client::Handle provides common base class
>>  - defined in client lib, namespace ::qpid::client
>>
> Could you go into a little more detail on this pattern?  I'm 
> particularly interested in the lifecycle of the handle objects and how 
> the ref counts are maintained.
> 
> Thanks,
> 
> -Ted
> 

It  uses refcounts as per boost::intrusive_ptr, but this is not exposed in the 
public  header files.

For an example see qpid/client/Subscription.h. The public header includes 
qpid/client/Handle.h, Subscription.cpp includes qpid/client/HandlePrivate.h.

This pattern is useful for public API, but it's tedious & error prone to write 
all the forwarding functions so I wouldn't recommend it for use on non-API code, 
a qpid::RefCounted and boost::intrusive_ptr will do the job there.



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

Re: C++: proposal to clean up the public API.

[Ted Ross <tr...@redhat.com>]

Alan Conway wrote:
> Public classes should fall into one of the following 3 categories:
>
> Handle: handle to refcounted object (e.g. Connection, Session)
>  - pure pointer to impl (PIMPL) idiom: no data except impl pointer, no 
> virtual functions, no inlines.
>  - qpid::client::Handle provides common base class
>  - defined in client lib, namespace ::qpid::client
>
Could you go into a little more detail on this pattern?  I'm 
particularly interested in the lifecycle of the handle objects and how 
the ref counts are maintained.

Thanks,

-Ted


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