My first impression is that there are too many steps and too many objects involved in sending/receiving messages.
Essentially there are 3 operations: - send a message (to somewhere) - pull a message (from somewhere) - register a listener to have messages pushed (from somewhere) We need to define "message" and "somewhere" in a portable way. Your proposal addresses the "somewhere" part with sources, sinks, producers, receivers and listeners. I think we can compress this into fewer classes but I don't have a proposal I'm happy with yet. We also need to cover creating wiring, threading model, flow control, and accepting/acquiring messages (browsing) etc. I think we can evolve rather than replace the existing API in these areas. I'll keep mulling it over this week and we can sit down next week. I'll try to make a start at cleaning up work at cleaning up the mechanics as per my proposal - did you have any comments on it? Here it is again: === 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-subscr...@qpid.apache.org
