Antti / rust-amqp

AMQP client in pure rust. Corresponds to rabbitmq spec.
MIT License
247 stars 45 forks source link

RFC: Various updates to exchange building. #32

Open alreece45 opened 8 years ago

alreece45 commented 8 years ago

Hi, I'm trying to get some feedback (comments) on an idea for the API before spending too much time working on it.

The number of arguments in declare_exchange() is a bit excessive and not very Rustic. I suggest maybe using a Builder pattern instead, while having the API utilize an Into to allow for more flexibility (e.g: implementing From to allow creating a direct exchange, with just a string).

Consumers would be able to do:

// one doesn't even need to use the builder, just giving a name should work as expected
channel.declare_exchange("this_is_a_direct_exchange");

// one can use the "topic" enum directly
channel.declare_exchange(Exchange::new("some-name", Type::Topic));

// one can also use a custom string for the type, the From implementation will make "topic" into the correct enum variant. If not one of the pre-declared types, it will go under the Custom varient.
channel.declare_exchange(Exchange::new("some-name", "topic"));

// there are factory methods for each different type of exchange, e.g: direct
channel.declare_exchange(Exchange::direct("direct"));

// its simple to keep to declarations to one line, if desired
// underscore in the method name matches the [rust naming conventions](https://doc.rust-lang.org/style/style/naming/README.html)
channel.declare_exchange(Exchange::fan_out("test").durable(true));

// its fairly easy to reason what's being done, what's being declared, and not mess with unused parameters
channel.declare_exchange(Exchange::topic("test");
    .durable(true)
    .auto_delete(true));

The API would probably similar to this, though Cow isn't required, but using the Into<> trait is probably beneficial.

#[derive(Debug)]
pub enum Type<'a> {
    Direct,
    FanOut,
    Headers,
    Topic,
    Custom(Cow<'a, str>)
}

impl<'a> Exchange<'a> {
    pub fn new<N, T>(name: N, _type: T) -> Self
        where N: Into<Cow<'a, str>>,
              T: Into<Type<'a>>;
    pub fn direct<S>(name: S) -> Self where S: Into<Cow<'a, str>>;
    pub fn fan_out<S>>(name: S) -> Self where S: Into<Cow<'a, str>>;
    pub fn headers<S>(name: S) -> Self where S: Into<Cow<'a, str>>;
    pub fn topic<S>(name: S) -> Self  where S: Into<Cow<'a, str>>;
    pub fn durable(mut self, durable: bool) -> Self;
    pub fn passive(mut self, passive: bool) -> Self;
    pub fn exclusive(mut self, exclusive: bool) -> Self;
    pub fn auto_delete(mut self, auto_delete: bool) -> Self;
    pub fn no_wait(mut self, no_wait: bool) -> Self;
}

This API does not attempt to address arguments with the table. When I looked, the usage of the Table wasn't clear and its usage should be more clarified, possibily similar to this one.

If the above API is acceptable, the protocol::Method implementation will need to access the structure members somehow. It seems like these types, and the protocol::Method implementation would be best in their own module, with private members-- but that lead to a more extensive update.

Antti commented 8 years ago

This is a very good idea. Currently, most of the methods on channel are a light wrappers to a corresponding Method struct constructor. This is because I didn't have time to think about the actual "rustic" API, but rather make something low-level, which just works and is familiar to someone who knows AMQP. See: http://www.rabbitmq.com/amqp-0-9-1-reference.html#exchange.declare

It was planned to add a more friendly API on top of the low level protocol implementation. About this api, what would be the default values for those fields (durable, passive, exclusive, auto_delete, no_wait), all false?

alreece45 commented 7 years ago

Yep. All false seems to be reasonable expectations.