From b7682a350b6ff3586f0fd5c5ea6d8939e7165754 Mon Sep 17 00:00:00 2001 From: Alan Conway Date: Wed, 26 Sep 2018 16:11:56 -0400 Subject: [PATCH 1/2] PROTON-1935: [cpp] add accessors for all proton::*_options classes Return an option to query the contents of an options object. --- cpp/include/proton/connection_options.hpp | 23 ++++- cpp/include/proton/delivery_mode.hpp | 5 +- cpp/include/proton/option.hpp | 63 ++++++++++++++ cpp/include/proton/receiver_options.hpp | 15 +++- cpp/include/proton/reconnect_options.hpp | 10 +++ cpp/include/proton/sender_options.hpp | 13 ++- cpp/include/proton/session_options.hpp | 6 ++ cpp/include/proton/source_options.hpp | 14 +++ cpp/include/proton/target_options.hpp | 12 +++ cpp/src/connection_driver.cpp | 2 +- cpp/src/connection_options.cpp | 101 ++++++++++++---------- cpp/src/node_options.cpp | 58 ++++++++----- cpp/src/proactor_container_impl.cpp | 4 +- cpp/src/receiver_options.cpp | 46 +++++----- cpp/src/reconnect_options.cpp | 6 ++ cpp/src/sender_options.cpp | 41 ++++----- cpp/src/session.cpp | 6 +- cpp/src/session_options.cpp | 13 +-- 18 files changed, 295 insertions(+), 143 deletions(-) create mode 100644 cpp/include/proton/option.hpp diff --git a/cpp/include/proton/connection_options.hpp b/cpp/include/proton/connection_options.hpp index 6da610ccaf..695e09b047 100644 --- a/cpp/include/proton/connection_options.hpp +++ b/cpp/include/proton/connection_options.hpp @@ -27,6 +27,7 @@ #include "./internal/config.hpp" #include "./internal/export.hpp" #include "./internal/pn_unique_ptr.hpp" +#include "./option.hpp" #include "./symbol.hpp" #include "./types_fwd.hpp" @@ -171,6 +172,27 @@ class connection_options { /// **Unsettled API** - Set reconnect and failover options. PN_CPP_EXTERN connection_options& reconnect(const reconnect_options &); + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option handler() const; + PN_CPP_EXTERN option max_frame_size() const; + PN_CPP_EXTERN option max_sessions() const; + PN_CPP_EXTERN option idle_timeout() const; + PN_CPP_EXTERN option container_id() const; + PN_CPP_EXTERN option virtual_host() const; + PN_CPP_EXTERN option user() const; + PN_CPP_EXTERN optionssl_client_options() const; + PN_CPP_EXTERN optionssl_server_options() const; + PN_CPP_EXTERN option sasl_enabled() const; + PN_CPP_EXTERN option sasl_allow_insecure_mechs() const; + PN_CPP_EXTERN option sasl_allowed_mechs() const; + PN_CPP_EXTERN option > offered_capabilities() const; + PN_CPP_EXTERN option > desired_capabilities() const; + PN_CPP_EXTERN option sasl_config_name() const; + PN_CPP_EXTERN option sasl_config_path() const; + PN_CPP_EXTERN option reconnect() const; + /// @} + /// Update option values from values set in other. PN_CPP_EXTERN connection_options& update(const connection_options& other); @@ -178,7 +200,6 @@ class connection_options { void apply_unbound(connection&) const; void apply_unbound_client(pn_transport_t*) const; void apply_unbound_server(pn_transport_t*) const; - messaging_handler* handler() const; class impl; internal::pn_unique_ptr impl_; diff --git a/cpp/include/proton/delivery_mode.hpp b/cpp/include/proton/delivery_mode.hpp index 744bd2cbf7..7d1030e593 100644 --- a/cpp/include/proton/delivery_mode.hpp +++ b/cpp/include/proton/delivery_mode.hpp @@ -30,7 +30,8 @@ namespace proton { /// The message delivery policy to establish when opening a link. /// This structure imitates the newer C++11 "enum class" so that /// The enumeration constants are in the delivery_mode namespace. -struct delivery_mode { +class delivery_mode { + public: /// Delivery modes enum modes { /// No set policy. The application must settle messages @@ -47,7 +48,7 @@ struct delivery_mode { }; /// @cond INTERNAL - + delivery_mode() : modes_(NONE) {} delivery_mode(modes m) : modes_(m) {} operator modes() { return modes_; } diff --git a/cpp/include/proton/option.hpp b/cpp/include/proton/option.hpp new file mode 100644 index 0000000000..eccab968f5 --- /dev/null +++ b/cpp/include/proton/option.hpp @@ -0,0 +1,63 @@ +#ifndef PROTON_OPTION_HPP +#define PROTON_OPTION_HPP + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include + +namespace proton { + +/// An option may or may not hold a value. +template class option { + bool is_set_; + T value_; + + public: + /// An empty option - this->is_set() == false + option() : is_set_(false), value_() {} + + /// An option containing x + option(const T& x) : is_set_(true), value_(x) {} + + /// Set option value to x + option& operator=(const T& x) { value_ = x; is_set_ = true; return *this; } + + /// Copy value from another option if it is set, no op otherwise + void update(const option& x) { if (x.is_set()) *this = x.get(); } + + /// @return true if the option has a value + bool is_set() const { return is_set_; } + + /// @return !is_set() + bool empty() const { return !is_set_; } + + /// @return the option value if is_set() + /// @throw std::logic_error if !is_set() + T get() const { + if (!is_set()) throw std::logic_error("proton::option is not set"); + return value_; + } + + /// @return the option value if is_set() or x if not + T get(const T& x) const { return is_set_ ? value_ : x; } +}; + +} +#endif // PROTON_OPTION_HPP diff --git a/cpp/include/proton/receiver_options.hpp b/cpp/include/proton/receiver_options.hpp index 0efe55e6ae..9d6417d3b6 100644 --- a/cpp/include/proton/receiver_options.hpp +++ b/cpp/include/proton/receiver_options.hpp @@ -26,6 +26,7 @@ #include "./internal/export.hpp" #include "./internal/pn_unique_ptr.hpp" #include "./delivery_mode.hpp" +#include "./option.hpp" #include /// @file @@ -76,7 +77,7 @@ class receiver_options { /// Set the delivery mode on the receiver. The default is /// delivery_mode::AT_LEAST_ONCE. - PN_CPP_EXTERN receiver_options& delivery_mode(delivery_mode); + PN_CPP_EXTERN receiver_options& delivery_mode(class delivery_mode); /// Enable or disable automatic acceptance of messages that aren't /// otherwise released, rejected, or modified. It is enabled by @@ -100,11 +101,19 @@ class receiver_options { /// Set the link name. If not set a unique name is generated. PN_CPP_EXTERN receiver_options& name(const std::string& name); + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option handler() const; + PN_CPP_EXTERN option delivery_mode() const; + PN_CPP_EXTERN option auto_accept() const; + PN_CPP_EXTERN option source() const; + PN_CPP_EXTERN option target() const; + PN_CPP_EXTERN option credit_window() const; + PN_CPP_EXTERN option name() const; + /// @} private: void apply(receiver &) const; - const std::string* get_name() const; // Pointer to name if set, else 0 - class impl; internal::pn_unique_ptr impl_; diff --git a/cpp/include/proton/reconnect_options.hpp b/cpp/include/proton/reconnect_options.hpp index bc4e43a3c7..2bd2a3b69b 100644 --- a/cpp/include/proton/reconnect_options.hpp +++ b/cpp/include/proton/reconnect_options.hpp @@ -26,6 +26,7 @@ #include "./internal/pn_unique_ptr.hpp" #include "./duration.hpp" #include "./source.hpp" +#include "./option.hpp" #include #include @@ -79,6 +80,15 @@ class reconnect_options { /// by default. PN_CPP_EXTERN reconnect_options& failover_urls(const std::vector& conn_urls); + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option delay() const; + PN_CPP_EXTERN option delay_multiplier() const; + PN_CPP_EXTERN option max_delay() const; + PN_CPP_EXTERN option max_attempts() const; + PN_CPP_EXTERN option > failover_urls() const; + /// @} + private: class impl; internal::pn_unique_ptr impl_; diff --git a/cpp/include/proton/sender_options.hpp b/cpp/include/proton/sender_options.hpp index 970da7e055..373f0315c6 100644 --- a/cpp/include/proton/sender_options.hpp +++ b/cpp/include/proton/sender_options.hpp @@ -26,6 +26,7 @@ #include "./internal/export.hpp" #include "./internal/pn_unique_ptr.hpp" #include "./delivery_mode.hpp" +#include "./option.hpp" #include /// @file @@ -91,10 +92,18 @@ class sender_options { /// Set the link name. If not set a unique name is generated. PN_CPP_EXTERN sender_options& name(const std::string& name); + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option handler() const; + PN_CPP_EXTERN option delivery_mode() const; + PN_CPP_EXTERN option auto_settle() const; + PN_CPP_EXTERN option source() const; + PN_CPP_EXTERN option target() const; + PN_CPP_EXTERN option name() const; + /// @} + private: void apply(sender&) const; - const std::string* get_name() const; // Pointer to name if set, else 0 - class impl; internal::pn_unique_ptr impl_; diff --git a/cpp/include/proton/session_options.hpp b/cpp/include/proton/session_options.hpp index 515a798700..d5c1e0234d 100644 --- a/cpp/include/proton/session_options.hpp +++ b/cpp/include/proton/session_options.hpp @@ -25,6 +25,7 @@ #include "./fwd.hpp" #include "./internal/export.hpp" #include "./internal/pn_unique_ptr.hpp" +#include "./option.hpp" /// @file /// @copybrief proton::session_options @@ -55,6 +56,11 @@ class session_options { // Other useful session configuration TBD. + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option handler() const; + /// @} + /// @cond INTERNAL private: void apply(session&) const; diff --git a/cpp/include/proton/source_options.hpp b/cpp/include/proton/source_options.hpp index fe1c34a500..1120f8cf45 100644 --- a/cpp/include/proton/source_options.hpp +++ b/cpp/include/proton/source_options.hpp @@ -25,6 +25,7 @@ #include "./internal/export.hpp" #include "./internal/pn_unique_ptr.hpp" #include "./duration.hpp" +#include "./option.hpp" #include "./source.hpp" #include @@ -92,6 +93,19 @@ class source_options { /// Extension capabilities that are supported/requested PN_CPP_EXTERN source_options& capabilities(const std::vector&); + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option address() const; + PN_CPP_EXTERN option dynamic() const; + PN_CPP_EXTERN option anonymous() const; + PN_CPP_EXTERN option distribution_mode() const; + PN_CPP_EXTERN option durability_mode() const; + PN_CPP_EXTERN option timeout() const; + PN_CPP_EXTERN option expiry_policy() const; + PN_CPP_EXTERN option filters() const; + PN_CPP_EXTERN option > capabilities() const; + /// @} + private: void apply(source&) const; diff --git a/cpp/include/proton/target_options.hpp b/cpp/include/proton/target_options.hpp index 834a18563a..448670ffbf 100644 --- a/cpp/include/proton/target_options.hpp +++ b/cpp/include/proton/target_options.hpp @@ -25,6 +25,7 @@ #include "./internal/export.hpp" #include "./internal/pn_unique_ptr.hpp" #include "./duration.hpp" +#include "./option.hpp" #include "./target.hpp" #include @@ -83,6 +84,17 @@ class target_options { /// Extension capabilities that are supported/requested PN_CPP_EXTERN target_options& capabilities(const std::vector&); + /// Get option values, see corresponding set function for details + /// {@ + PN_CPP_EXTERN option address() const; + PN_CPP_EXTERN option dynamic() const; + PN_CPP_EXTERN option anonymous() const; + PN_CPP_EXTERN option durability_mode() const; + PN_CPP_EXTERN option timeout() const; + PN_CPP_EXTERN option expiry_policy() const; + PN_CPP_EXTERN option > capabilities() const; + /// @} + private: void apply(target&) const; diff --git a/cpp/src/connection_driver.cpp b/cpp/src/connection_driver.cpp index 11888bed48..3e8e7ed60f 100644 --- a/cpp/src/connection_driver.cpp +++ b/cpp/src/connection_driver.cpp @@ -69,7 +69,7 @@ void connection_driver::configure(const connection_options& opts, bool server) { opts.apply_unbound_client(driver_.transport); } pn_connection_driver_bind(&driver_); - handler_ = opts.handler(); + handler_ = opts.handler().get(0); } void connection_driver::connect(const connection_options& opts) { diff --git a/cpp/src/connection_options.cpp b/cpp/src/connection_options.cpp index fdc770e753..73fc2144f2 100644 --- a/cpp/src/connection_options.cpp +++ b/cpp/src/connection_options.cpp @@ -40,15 +40,6 @@ namespace proton { -template struct option { - T value; - bool set; - - option() : value(), set(false) {} - option& operator=(const T& x) { value = x; set = true; return *this; } - void update(const option& x) { if (x.set) *this = x.value; } -}; - class connection_options::impl { public: option handler; @@ -84,29 +75,29 @@ class connection_options::impl { bool uninit = c.uninitialized(); if (!uninit) return; - if (reconnect.set) - connection_context::get(pnc).reconnect_context_.reset(new reconnect_context(reconnect.value)); - if (container_id.set) - pn_connection_set_container(pnc, container_id.value.c_str()); - if (virtual_host.set) - pn_connection_set_hostname(pnc, virtual_host.value.c_str()); - if (user.set) - pn_connection_set_user(pnc, user.value.c_str()); - if (password.set) - pn_connection_set_password(pnc, password.value.c_str()); - if (offered_capabilities.set) - value(pn_connection_offered_capabilities(pnc)) = offered_capabilities.value; - if (desired_capabilities.set) - value(pn_connection_desired_capabilities(pnc)) = desired_capabilities.value; + if (reconnect.is_set()) + connection_context::get(pnc).reconnect_context_.reset(new reconnect_context(reconnect.get())); + if (container_id.is_set()) + pn_connection_set_container(pnc, container_id.get().c_str()); + if (virtual_host.is_set()) + pn_connection_set_hostname(pnc, virtual_host.get().c_str()); + if (user.is_set()) + pn_connection_set_user(pnc, user.get().c_str()); + if (password.is_set()) + pn_connection_set_password(pnc, password.get().c_str()); + if (offered_capabilities.is_set()) + value(pn_connection_offered_capabilities(pnc)) = offered_capabilities.get(); + if (desired_capabilities.is_set()) + value(pn_connection_desired_capabilities(pnc)) = desired_capabilities.get(); } void apply_transport(pn_transport_t* pnt) { - if (max_frame_size.set) - pn_transport_set_max_frame(pnt, max_frame_size.value); - if (max_sessions.set) - pn_transport_set_channel_max(pnt, max_sessions.value); - if (idle_timeout.set) - pn_transport_set_idle_timeout(pnt, idle_timeout.value.milliseconds()); + if (max_frame_size.is_set()) + pn_transport_set_max_frame(pnt, max_frame_size.get()); + if (max_sessions.is_set()) + pn_transport_set_channel_max(pnt, max_sessions.get()); + if (idle_timeout.is_set()) + pn_transport_set_idle_timeout(pnt, idle_timeout.get().milliseconds()); } void apply_sasl(pn_transport_t* pnt) { @@ -115,17 +106,17 @@ class connection_options::impl { if (!pnt) return; // Skip entirely if SASL explicitly disabled - if (!sasl_enabled.set || sasl_enabled.value) { - if (sasl_enabled.set) // Explicitly set, not just default behaviour. + if (!sasl_enabled.is_set() || sasl_enabled.get()) { + if (sasl_enabled.is_set()) // Explicitly set, not just default behaviour. pn_sasl(pnt); // Force a sasl instance. Lazily create one otherwise. - if (sasl_allow_insecure_mechs.set) - pn_sasl_set_allow_insecure_mechs(pn_sasl(pnt), sasl_allow_insecure_mechs.value); - if (sasl_allowed_mechs.set) - pn_sasl_allowed_mechs(pn_sasl(pnt), sasl_allowed_mechs.value.c_str()); - if (sasl_config_name.set) - pn_sasl_config_name(pn_sasl(pnt), sasl_config_name.value.c_str()); - if (sasl_config_path.set) - pn_sasl_config_path(pn_sasl(pnt), sasl_config_path.value.c_str()); + if (sasl_allow_insecure_mechs.is_set()) + pn_sasl_set_allow_insecure_mechs(pn_sasl(pnt), sasl_allow_insecure_mechs.get()); + if (sasl_allowed_mechs.is_set()) + pn_sasl_allowed_mechs(pn_sasl(pnt), sasl_allowed_mechs.get().c_str()); + if (sasl_config_name.is_set()) + pn_sasl_config_name(pn_sasl(pnt), sasl_config_name.get().c_str()); + if (sasl_config_path.is_set()) + pn_sasl_config_path(pn_sasl(pnt), sasl_config_path.get().c_str()); } } @@ -135,17 +126,17 @@ class connection_options::impl { // and if there is a pipelined open frame. if (!pnt) return; - if (client && ssl_client_options.set) { + if (client && ssl_client_options.is_set()) { // A side effect of pn_ssl() is to set the ssl peer // hostname to the connection hostname, which has // already been adjusted for the virtual_host option. pn_ssl_t *ssl = pn_ssl(pnt); - if (pn_ssl_init(ssl, ssl_client_options.value.pn_domain(), NULL)) + if (pn_ssl_init(ssl, ssl_client_options.get().pn_domain(), NULL)) throw error(MSG("client SSL/TLS initialization error")); - } else if (!client && ssl_server_options.set) { - pn_ssl_t *ssl = pn_ssl(pnt); - if (pn_ssl_init(ssl, ssl_server_options.value.pn_domain(), NULL)) - throw error(MSG("server SSL/TLS initialization error")); + } else if (!client && ssl_server_options.is_set()) { + pn_ssl_t *ssl = pn_ssl(pnt); + if (pn_ssl_init(ssl, ssl_server_options.get().pn_domain(), NULL)) + throw error(MSG("server SSL/TLS initialization error")); } } @@ -212,10 +203,26 @@ connection_options& connection_options::sasl_allowed_mechs(const std::string &s) connection_options& connection_options::sasl_config_name(const std::string &n) { impl_->sasl_config_name = n; return *this; } connection_options& connection_options::sasl_config_path(const std::string &p) { impl_->sasl_config_path = p; return *this; } +option connection_options::handler() const { return impl_->handler; } +option connection_options::max_frame_size() const { return impl_->max_frame_size; } +option connection_options::max_sessions() const { return impl_->max_sessions; } +option connection_options::idle_timeout() const { return impl_->idle_timeout; } +option connection_options::container_id() const { return impl_->container_id; } +option connection_options::virtual_host() const { return impl_->virtual_host; } +option connection_options::user() const { return impl_->user; } +option connection_options::ssl_client_options() const { return impl_->ssl_client_options; } +option connection_options::ssl_server_options() const { return impl_->ssl_server_options; } +option connection_options::sasl_enabled() const { return impl_->sasl_enabled; } +option connection_options::sasl_allow_insecure_mechs() const { return impl_->sasl_allow_insecure_mechs; } +option connection_options::sasl_allowed_mechs() const { return impl_->sasl_allowed_mechs; } +option > connection_options::offered_capabilities() const { return impl_->offered_capabilities; } +option > connection_options::desired_capabilities() const { return impl_->desired_capabilities; } +option connection_options::sasl_config_name() const { return impl_->sasl_config_name; } +option connection_options::sasl_config_path() const { return impl_->sasl_config_path; } +option connection_options::reconnect() const { return impl_->reconnect; } + void connection_options::apply_unbound(connection& c) const { impl_->apply_unbound(c); } void connection_options::apply_unbound_client(pn_transport_t *t) const { impl_->apply_sasl(t); impl_->apply_ssl(t, true); impl_->apply_transport(t); } void connection_options::apply_unbound_server(pn_transport_t *t) const { impl_->apply_sasl(t); impl_->apply_ssl(t, false); impl_->apply_transport(t); } -messaging_handler* connection_options::handler() const { return impl_->handler.value; } - } // namespace proton diff --git a/cpp/src/node_options.cpp b/cpp/src/node_options.cpp index 3b6d197c75..5dc30b2662 100644 --- a/cpp/src/node_options.cpp +++ b/cpp/src/node_options.cpp @@ -22,6 +22,7 @@ #include "proton/codec/vector.hpp" #include "proton/source.hpp" #include "proton/source_options.hpp" +#include "proton/option.hpp" #include "proton/target.hpp" #include "proton/target_options.hpp" @@ -31,15 +32,6 @@ namespace proton { -template struct option { - T value; - bool set; - - option() : value(), set(false) {} - option& operator=(const T& x) { value = x; set = true; return *this; } - void update(const option& x) { if (x.set) *this = x.value; } -}; - namespace { void timeout(terminus &t, duration d) { @@ -64,23 +56,23 @@ namespace { // Options common to sources and targets void node_address(terminus &t, option &addr, option &dynamic, option &anonymous) { - if (dynamic.set && dynamic.value) { + if (dynamic.is_set() && dynamic.get()) { pn_terminus_set_dynamic(unwrap(t), true); pn_terminus_set_address(unwrap(t), NULL); - } else if (anonymous.set && anonymous.value) { + } else if (anonymous.is_set() && anonymous.get()) { pn_terminus_set_address(unwrap(t), NULL); - } else if (addr.set) { - pn_terminus_set_address(unwrap(t), addr.value.c_str()); + } else if (addr.is_set()) { + pn_terminus_set_address(unwrap(t), addr.get().c_str()); } } void node_durability(terminus &t, option &mode) { - if (mode.set) pn_terminus_set_durability(unwrap(t), pn_durability_t(mode.value)); + if (mode.is_set()) pn_terminus_set_durability(unwrap(t), pn_durability_t(mode.get())); } void node_expiry(terminus &t, option &policy, option &d) { - if (policy.set) pn_terminus_set_expiry_policy(unwrap(t), pn_expiry_policy_t(policy.value)); - if (d.set) timeout(t, d.value); + if (policy.is_set()) pn_terminus_set_expiry_policy(unwrap(t), pn_expiry_policy_t(policy.get())); + if (d.is_set()) timeout(t, d.get()); } } @@ -102,14 +94,14 @@ class source_options::impl { node_address(s, address, dynamic, anonymous); node_durability(s, durability_mode); node_expiry(s, expiry_policy, timeout); - if (distribution_mode.set) - pn_terminus_set_distribution_mode(unwrap(s), pn_distribution_mode_t(distribution_mode.value)); - if (filters.set && !filters.value.empty()) { + if (distribution_mode.is_set()) + pn_terminus_set_distribution_mode(unwrap(s), pn_distribution_mode_t(distribution_mode.get())); + if (filters.is_set() && !filters.get().empty()) { // Applied at most once via source_option. No need to clear. - value(pn_terminus_filter(unwrap(s))) = filters.value; + value(pn_terminus_filter(unwrap(s))) = filters.get(); } - if (capabilities.set) { - value(pn_terminus_capabilities(unwrap(s))) = capabilities.value; + if (capabilities.is_set()) { + value(pn_terminus_capabilities(unwrap(s))) = capabilities.get(); } } }; @@ -137,6 +129,16 @@ source_options& source_options::capabilities(const std::vector& c) { imp void source_options::apply(source& s) const { impl_->apply(s); } +option source_options::address() const { return impl_->address; } +option source_options::dynamic() const { return impl_->dynamic; } +option source_options::anonymous() const { return impl_->anonymous; } +option source_options::distribution_mode() const { return impl_->distribution_mode; } +option source_options::durability_mode() const { return impl_->durability_mode; } +option source_options::timeout() const { return impl_->timeout; } +option source_options::expiry_policy() const { return impl_->expiry_policy; } +option source_options::filters() const { return impl_->filters; } +option > source_options::capabilities() const { return impl_->capabilities; } + // TARGET class target_options::impl { @@ -153,8 +155,8 @@ class target_options::impl { node_address(t, address, dynamic, anonymous); node_durability(t, durability_mode); node_expiry(t, expiry_policy, timeout); - if (capabilities.set) { - value(pn_terminus_capabilities(unwrap(t))) = capabilities.value; + if (capabilities.is_set()) { + value(pn_terminus_capabilities(unwrap(t))) = capabilities.get(); } } }; @@ -180,6 +182,14 @@ target_options& target_options::capabilities(const std::vector& c) { imp void target_options::apply(target& s) const { impl_->apply(s); } +option target_options::address() const { return impl_->address; } +option target_options::dynamic() const { return impl_->dynamic; } +option target_options::anonymous() const { return impl_->anonymous; } +option target_options::durability_mode() const { return impl_->durability_mode; } +option target_options::timeout() const { return impl_->timeout; } +option target_options::expiry_policy() const { return impl_->expiry_policy; } +option > target_options::capabilities() const { return impl_->capabilities; } + } // namespace proton diff --git a/cpp/src/proactor_container_impl.cpp b/cpp/src/proactor_container_impl.cpp index e0057cdbac..6f962ebd7d 100644 --- a/cpp/src/proactor_container_impl.cpp +++ b/cpp/src/proactor_container_impl.cpp @@ -181,7 +181,7 @@ pn_connection_t* container::impl::make_connection_lh( connection_options opts = client_connection_options_; // Defaults opts.update(user_opts); - messaging_handler* mh = opts.handler(); + messaging_handler* mh = opts.handler().get(0); pn_connection_t *pnc = pn_connection(); connection_context& cc(connection_context::get(pnc)); @@ -592,7 +592,7 @@ container::impl::dispatch_result container::impl::dispatch(pn_event_t* event) { connection_context& cc = connection_context::get(c); cc.container = &container_; cc.listener_context_ = lc; - cc.handler = opts.handler(); + cc.handler = opts.handler().get(0); cc.work_queue_ = new container::impl::connection_work_queue(*container_.impl_, c); pn_transport_t* pnt = pn_transport(); pn_transport_set_server(pnt); diff --git a/cpp/src/receiver_options.cpp b/cpp/src/receiver_options.cpp index f7ef5a8b68..34e14b042a 100644 --- a/cpp/src/receiver_options.cpp +++ b/cpp/src/receiver_options.cpp @@ -23,6 +23,7 @@ #include "proton/messaging_handler.hpp" #include "proton/source_options.hpp" #include "proton/target_options.hpp" +#include "proton/option.hpp" #include @@ -33,15 +34,6 @@ namespace proton { -template struct option { - T value; - bool set; - - option() : value(), set(false) {} - option& operator=(const T& x) { value = x; set = true; return *this; } - void update(const option& x) { if (x.set) *this = x.value; } -}; - class receiver_options::impl { static link_context& get_context(receiver l) { return link_context::get(unwrap(l)); @@ -68,26 +60,26 @@ class receiver_options::impl { option auto_settle; option credit_window; option dynamic_address; - option source; - option target; + option source; + option target; option name; void apply(receiver& r) { if (r.uninitialized()) { - if (delivery_mode.set) set_delivery_mode(r, delivery_mode.value); - if (handler.set && handler.value) container::impl::set_handler(r, handler.value); - if (auto_settle.set) get_context(r).auto_settle = auto_settle.value; - if (auto_accept.set) get_context(r).auto_accept = auto_accept.value; - if (credit_window.set) get_context(r).credit_window = credit_window.value; + if (delivery_mode.is_set()) set_delivery_mode(r, delivery_mode.get()); + if (handler.is_set() && handler.get()) container::impl::set_handler(r, handler.get()); + if (auto_settle.is_set()) get_context(r).auto_settle = auto_settle.get(); + if (auto_accept.is_set()) get_context(r).auto_accept = auto_accept.get(); + if (credit_window.is_set()) get_context(r).credit_window = credit_window.get(); - if (source.set) { + if (source.is_set()) { proton::source local_s(make_wrapper(pn_link_source(unwrap(r)))); - source.value.apply(local_s); + source.get().apply(local_s); } - if (target.set) { + if (target.is_set()) { proton::target local_t(make_wrapper(pn_link_target(unwrap(r)))); - target.value.apply(local_t); + target.get().apply(local_t); } } } @@ -123,15 +115,19 @@ receiver_options& receiver_options::handler(class messaging_handler &h) { impl_- receiver_options& receiver_options::delivery_mode(proton::delivery_mode m) {impl_->delivery_mode = m; return *this; } receiver_options& receiver_options::auto_accept(bool b) {impl_->auto_accept = b; return *this; } receiver_options& receiver_options::credit_window(int w) {impl_->credit_window = w; return *this; } -receiver_options& receiver_options::source(source_options &s) {impl_->source = s; return *this; } -receiver_options& receiver_options::target(target_options &s) {impl_->target = s; return *this; } +receiver_options& receiver_options::source(class source_options &s) {impl_->source = s; return *this; } +receiver_options& receiver_options::target(class target_options &s) {impl_->target = s; return *this; } receiver_options& receiver_options::name(const std::string &s) {impl_->name = s; return *this; } void receiver_options::apply(receiver& r) const { impl_->apply(r); } -const std::string* receiver_options::get_name() const { - return impl_->name.set ? &impl_->name.value : 0; -} +option receiver_options::handler() const { return impl_->handler; } +option receiver_options::delivery_mode() const { return impl_->delivery_mode; } +option receiver_options::auto_accept() const { return impl_->auto_accept; } +option receiver_options::source() const { return impl_->source; } +option receiver_options::target() const { return impl_->target; } +option receiver_options::credit_window() const { return impl_->credit_window; } +option receiver_options::name() const { return impl_->name; } // No-op, kept for binary compat but auto_settle is not relevant to receiver only sender. receiver_options& receiver_options::auto_settle(bool b) { return *this; } diff --git a/cpp/src/reconnect_options.cpp b/cpp/src/reconnect_options.cpp index a469d3f151..842c37f095 100644 --- a/cpp/src/reconnect_options.cpp +++ b/cpp/src/reconnect_options.cpp @@ -41,4 +41,10 @@ reconnect_options& reconnect_options::max_delay(duration d) { impl_->max_delay = reconnect_options& reconnect_options::max_attempts(int i) { impl_->max_attempts = i; return *this; } reconnect_options& reconnect_options::failover_urls(const std::vector& urls) { impl_->failover_urls = urls; return *this; } +option reconnect_options::delay() const { return impl_->delay; } +option reconnect_options::delay_multiplier() const { return impl_->delay_multiplier; } +option reconnect_options::max_delay() const { return impl_->max_delay; } +option reconnect_options::max_attempts() const { return impl_->max_attempts; } +option > reconnect_options::failover_urls() const { return impl_->failover_urls; } + } // namespace proton diff --git a/cpp/src/sender_options.cpp b/cpp/src/sender_options.cpp index 1cc0643e57..62fbdb4d44 100644 --- a/cpp/src/sender_options.cpp +++ b/cpp/src/sender_options.cpp @@ -31,15 +31,6 @@ namespace proton { -template struct option { - T value; - bool set; - - option() : value(), set(false) {} - option& operator=(const T& x) { value = x; set = true; return *this; } - void update(const option& x) { if (x.set) *this = x.value; } -}; - class sender_options::impl { static link_context& get_context(sender l) { return link_context::get(unwrap(l)); @@ -63,22 +54,22 @@ class sender_options::impl { option handler; option delivery_mode; option auto_settle; - option source; - option target; + option source; + option target; option name; void apply(sender& s) { if (s.uninitialized()) { - if (delivery_mode.set) set_delivery_mode(s, delivery_mode.value); - if (handler.set && handler.value) container::impl::set_handler(s, handler.value); - if (auto_settle.set) get_context(s).auto_settle = auto_settle.value; - if (source.set) { + if (delivery_mode.is_set()) set_delivery_mode(s, delivery_mode.get()); + if (handler.is_set() && handler.get()) container::impl::set_handler(s, handler.get()); + if (auto_settle.is_set()) get_context(s).auto_settle = auto_settle.get(); + if (source.is_set()) { proton::source local_s(make_wrapper(pn_link_source(unwrap(s)))); - source.value.apply(local_s); + source.get().apply(local_s); } - if (target.set) { + if (target.is_set()) { proton::target local_t(make_wrapper(pn_link_target(unwrap(s)))); - target.value.apply(local_t); + target.get().apply(local_t); } } } @@ -110,13 +101,17 @@ void sender_options::update(const sender_options& x) { impl_->update(*x.impl_); sender_options& sender_options::handler(class messaging_handler &h) { impl_->handler = &h; return *this; } sender_options& sender_options::delivery_mode(proton::delivery_mode m) {impl_->delivery_mode = m; return *this; } sender_options& sender_options::auto_settle(bool b) {impl_->auto_settle = b; return *this; } -sender_options& sender_options::source(const source_options &s) {impl_->source = s; return *this; } -sender_options& sender_options::target(const target_options &s) {impl_->target = s; return *this; } +sender_options& sender_options::source(const class source_options &s) {impl_->source = s; return *this; } +sender_options& sender_options::target(const class target_options &s) {impl_->target = s; return *this; } sender_options& sender_options::name(const std::string &s) {impl_->name = s; return *this; } void sender_options::apply(sender& s) const { impl_->apply(s); } -const std::string* sender_options::get_name() const { - return impl_->name.set ? &impl_->name.value : 0; -} +option sender_options::handler() const { return impl_->handler; } +option sender_options::delivery_mode() const { return impl_->delivery_mode; } +option sender_options::auto_settle() const { return impl_->auto_settle; } +option sender_options::source() const { return impl_->source; } +option sender_options::target() const { return impl_->target; } +option sender_options::name() const { return impl_->name; } + } // namespace proton diff --git a/cpp/src/session.cpp b/cpp/src/session.cpp index fb2e6b346f..581b7c9df0 100644 --- a/cpp/src/session.cpp +++ b/cpp/src/session.cpp @@ -77,7 +77,8 @@ sender session::open_sender(const std::string &addr) { } sender session::open_sender(const std::string &addr, const sender_options &so) { - std::string name = so.get_name() ? *so.get_name() : next_link_name(connection()); + option name_opt = so.name(); + std::string name = name_opt.is_set() ? name_opt.get() : next_link_name(connection()); pn_link_t *lnk = pn_sender(pn_object(), name.c_str()); pn_terminus_set_address(pn_link_target(lnk), addr.c_str()); sender snd(make_wrapper(lnk)); @@ -91,7 +92,8 @@ receiver session::open_receiver(const std::string &addr) { receiver session::open_receiver(const std::string &addr, const receiver_options &ro) { - std::string name = ro.get_name() ? *ro.get_name() : next_link_name(connection()); + option name_opt = ro.name(); + std::string name = name_opt.is_set() ? name_opt.get() : next_link_name(connection()); pn_link_t *lnk = pn_receiver(pn_object(), name.c_str()); pn_terminus_set_address(pn_link_source(lnk), addr.c_str()); receiver rcv(make_wrapper(lnk)); diff --git a/cpp/src/session_options.cpp b/cpp/src/session_options.cpp index fc03ebba86..3a44dc60cb 100644 --- a/cpp/src/session_options.cpp +++ b/cpp/src/session_options.cpp @@ -32,22 +32,13 @@ namespace proton { -template struct option { - T value; - bool set; - - option() : value(), set(false) {} - option& operator=(const T& x) { value = x; set = true; return *this; } - void update(const option& x) { if (x.set) *this = x.value; } -}; - class session_options::impl { public: option handler; void apply(session& s) { if (s.uninitialized()) { - if (handler.set && handler.value) container::impl::set_handler(s, handler.value); + if (handler.is_set() && handler.get()) container::impl::set_handler(s, handler.get()); } } @@ -68,7 +59,7 @@ session_options& session_options::handler(class messaging_handler &h) { impl_->h void session_options::apply(session& s) const { impl_->apply(s); } - +option session_options::handler() const { return impl_->handler; } } // namespace proton From 73af00fd0506db2858a3e979a5a057a84ff14bf7 Mon Sep 17 00:00:00 2001 From: Alan Conway Date: Fri, 28 Sep 2018 14:54:08 -0400 Subject: [PATCH 2/2] PROTON-1935: [cpp] additional tests for connection configuration --- cpp/src/connect_config.cpp | 2 +- cpp/src/connect_config_test.cpp | 48 +++++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/cpp/src/connect_config.cpp b/cpp/src/connect_config.cpp index d85eb046ca..9c8f6da37c 100644 --- a/cpp/src/connect_config.cpp +++ b/cpp/src/connect_config.cpp @@ -156,7 +156,7 @@ std::string parse(std::istream& is, connection_options& opts) { raise(msg() << "'scheme' must be \"amqp\" or \"amqps\""); } - string host = get_string(root, "host", ""); + string host = get_string(root, "host", "localhost"); opts.virtual_host(host.c_str()); Value port = root.get("port", scheme); diff --git a/cpp/src/connect_config_test.cpp b/cpp/src/connect_config_test.cpp index 17f0c4b8c4..d9c07ce9f0 100644 --- a/cpp/src/connect_config_test.cpp +++ b/cpp/src/connect_config_test.cpp @@ -27,6 +27,8 @@ #include "proton/listener.hpp" #include "proton/messaging_handler.hpp" #include "proton/transport.hpp" +#include "proton/ssl.hpp" +#include "proton/sasl.hpp" #include #include @@ -68,8 +70,8 @@ void test_addr() { connection_options opts; ASSERT_EQUAL("foo:bar", configure(opts, "{ \"host\":\"foo\", \"port\":\"bar\" }")); ASSERT_EQUAL("foo:1234", configure(opts, "{ \"host\":\"foo\", \"port\":\"1234\" }")); - ASSERT_EQUAL(":amqps", configure(opts, "{}")); - ASSERT_EQUAL(":amqp", configure(opts, "{\"scheme\":\"amqp\"}")); + ASSERT_EQUAL("localhost:amqps", configure(opts, "{}")); + ASSERT_EQUAL("localhost:amqp", configure(opts, "{\"scheme\":\"amqp\"}")); ASSERT_EQUAL("foo:bar", configure(opts, "{ \"host\":\"foo\", /* inline comment */\"port\":\"bar\" // end of line comment\n}")); ASSERT_THROWS_MSG(error, "'scheme' must be", configure(opts, "{\"scheme\":\"bad\"}")); @@ -78,6 +80,46 @@ void test_addr() { ASSERT_THROWS_MSG(error, "'host' expected string, found boolean", configure(opts, "{\"host\":true}")); } +// Hack to write long strings with embedded '"' and newlines +#define LONG_STRING(...) #__VA_ARGS__ + +const std::string& CONFIG_ALL = LONG_STRING( + { + "host": "foobar.org", + "port": 1234, + "user": "fred", + "password": "secret", + "sasl": { + "mechanisms": ["a", "b", "c"], + "allow_insecure": true + } + } +); + +void test_all() { + connection_options opts; + ASSERT_EQUAL("foobar.org:1234", configure(opts, CONFIG_ALL)); + ASSERT_EQUAL("foobar.org", opts.virtual_host().get()); + ASSERT_EQUAL(true, opts.sasl_enabled().get()); + ASSERT_EQUAL("fred", opts.user().get()); + ASSERT_EQUAL("a b c", opts.sasl_allowed_mechs().get()); + ASSERT(opts.sasl_allow_insecure_mechs().get()); + // TODO aconway 2018-09-21: need to set up real credentials to + // test SSL settings, the ssl_domain is created immediately and + // there will be an exception if the credentials are not valid. +} + +void test_defaults() { + connection_options opts; + ASSERT_EQUAL("localhost:amqps", configure(opts, "{}")); + ASSERT_EQUAL("localhost", opts.virtual_host().get()); + ASSERT_EQUAL(true, opts.sasl_enabled().get()); + ASSERT(opts.user().empty()); + ASSERT(opts.sasl_allowed_mechs().empty()); + ASSERT(opts.sasl_allow_insecure_mechs().empty()); + ASSERT(opts.ssl_client_options().empty()); +} + class test_handler : public messaging_handler { protected: string config_; @@ -137,6 +179,8 @@ int main(int argc, char** argv) { int failed = 0; RUN_ARGV_TEST(failed, test_default_file()); RUN_ARGV_TEST(failed, test_addr()); + RUN_ARGV_TEST(failed, test_all()); + RUN_ARGV_TEST(failed, test_defaults()); RUN_ARGV_TEST(failed, test_default_connect().run()); return failed; }