Continued fixing connect and disconnect copy and move functions with new unit tests. See #445

README.md

@@ -55,7 +55,6 @@ Hopefully it will be out by the end of 2023. Just a few weeks away!
 - Support for Catch2 v3.x for unit tests (v2.x also still supported).
 - Changed the sample apps to use the newer "mqtt://" schemas.
 - Connect option initializers for v5 and WebSockets.
-
 - [#304](https://github.com/eclipse/paho.mqtt.cpp/issues/304) Missing create_options::DFLT_C_STRUCT symbol when linking with MSVC.
 - [#429] (https://github.com/eclipse/paho.mqtt.cpp/issues/411) Remove declaration of connect_options::to_string() with missing implementation.
 - [#411](https://github.com/eclipse/paho.mqtt.cpp/issues/411) Missing virtual keyword for some client methods

src/connect_options.cpp

@@ -68,9 +68,6 @@ connect_options::connect_options(const connect_options& opt) : opts_(opt.opts_),
 	if (opts_.ssl)
 		set_ssl(opt.ssl_);
 
-	if (opts_.connectProperties)
-		set_properties(opt.props_);
-
 	update_c_struct();
 }
 
@@ -95,12 +92,19 @@ connect_options::connect_options(connect_options&& opt) : opts_(opt.opts_),
 	if (opts_.ssl)
 		opts_.ssl = &ssl_.opts_;
 	
-	if (opts_.connectProperties)
-		opts_.connectProperties = const_cast<MQTTProperties*>(&props_.c_struct());
-
 	update_c_struct();
 }
 
+connect_options connect_options::v3()
+{
+	return connect_options(DFLT_C_STRUCT);
+}
+
+connect_options connect_options::v5()
+{
+	return connect_options(DFLT_C_STRUCT5);
+}
+
 // Unfortunately, with the existing implementation, there's no way to know
 // if the (connect) properties, will and ssl options were set by looking at the C++ structs.
 // In a major update, we can consider using a pointer or optional<> to
@@ -154,7 +158,8 @@ void connect_options::update_c_struct()
 
 	// Connect Properties
 
-	opts_.connectProperties = const_cast<MQTTProperties*>(&props_.c_struct());
+	if (opts_.MQTTVersion >= MQTTVERSION_5)
+		opts_.connectProperties = const_cast<MQTTProperties*>(&props_.c_struct());
 
 	// HTTP & Proxy
 
@@ -180,8 +185,7 @@ connect_options& connect_options::operator=(const connect_options& opt)
 
 	tok_ = opt.tok_;
 	serverURIs_ = opt.serverURIs_;
-	if (opts_.connectProperties)
+	props_ = opt.props_;
-		set_properties(opt.props_);
 
 	httpHeaders_ = opt.httpHeaders_;
 	httpProxy_ = opt.httpProxy_;
@@ -209,8 +213,7 @@ connect_options& connect_options::operator=(connect_options&& opt)
 
 	tok_ = std::move(opt.tok_);
 	serverURIs_ = std::move(opt.serverURIs_);
-	if (opts_.connectProperties)
+	props_ = std::move(opt.props_);
-		set_properties(std::move(opt.props_));
 
 	httpHeaders_ = std::move(opt.httpHeaders_);
 	httpProxy_ = std::move(opt.httpProxy_);

src/disconnect_options.cpp

@@ -7,21 +7,43 @@
 namespace mqtt {
 
 /////////////////////////////////////////////////////////////////////////////
+// disconnect_options
 
-const MQTTAsync_disconnectOptions disconnect_options::DFLT_C_STRUCT = MQTTAsync_disconnectOptions_initializer;
+const MQTTAsync_disconnectOptions disconnect_options::DFLT_C_STRUCT
+		= MQTTAsync_disconnectOptions_initializer;
 
-disconnect_options::disconnect_options() : opts_(DFLT_C_STRUCT)
+const MQTTAsync_disconnectOptions disconnect_options::DFLT_C_STRUCT5
-{
+		= MQTTAsync_disconnectOptions_initializer5;
+
+disconnect_options::disconnect_options() : opts_{DFLT_C_STRUCT} {
 }
 
 disconnect_options::disconnect_options(const disconnect_options& opt)
 			: opts_(opt.opts_), tok_(opt.tok_), props_(opt.props_)
 {
+	update_c_struct();
 }
 
 disconnect_options::disconnect_options(disconnect_options&& opt)
 			: opts_(opt.opts_), tok_(std::move(opt.tok_)), props_(std::move(opt.props_))
 {
+	update_c_struct();
+}
+
+disconnect_options disconnect_options::v3()
+{
+	return disconnect_options { DFLT_C_STRUCT };
+}
+
+disconnect_options disconnect_options::v5()
+{
+	return disconnect_options { DFLT_C_STRUCT5 };
+}
+
+void disconnect_options::update_c_struct()
+{
+	opts_.properties = props_.c_struct();
+	opts_.context = tok_.get();
 }
 
 disconnect_options& disconnect_options::operator=(const disconnect_options& opt)
@@ -29,6 +51,7 @@ disconnect_options& disconnect_options::operator=(const disconnect_options& opt)
 	opts_ = opt.opts_;
 	tok_ = opt.tok_;
 	props_ = opt.props_;
+	update_c_struct();
 	return *this;
 }
 
@@ -37,6 +60,7 @@ disconnect_options& disconnect_options::operator=(disconnect_options&& opt)
 	opts_ = opt.opts_;
 	tok_ = std::move(opt.tok_);
 	props_ = std::move(opt.props_);
+	update_c_struct();
 	return *this;
 }
 
@@ -63,6 +87,19 @@ void disconnect_options::set_token(const token_ptr& tok, int mqttVersion)
 	}
 }
 
+/////////////////////////////////////////////////////////////////////////////
+// disconnect_options_builder
+
+disconnect_options_builder disconnect_options_builder::v3()
+{
+	return disconnect_options_builder{disconnect_options::DFLT_C_STRUCT};
+}
+
+disconnect_options_builder disconnect_options_builder::v5()
+{
+	return disconnect_options_builder{disconnect_options::DFLT_C_STRUCT5};
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // end namespace 'mqtt'
 }

src/mqtt/connect_options.h

@@ -157,16 +157,12 @@ public:
 	 * Creates default options for an MQTT v3.x connection.
 	 * @return Default options for an MQTT v3.x connection.
 	 */
-	static connect_options v3() {
+	static connect_options v3();
-		return connect_options(DFLT_C_STRUCT);
-	}
 	/**
 	 * Creates default options for an MQTT v5 connection.
 	 * @return Default options for an MQTT v5 connection.
 	 */
-	static connect_options v5() {
+	static connect_options v5();
-		return connect_options(DFLT_C_STRUCT5);
-	}
 	/**
 	 * Creates default options for an MQTT v3.x connection using WebSockets.
 	 *
@@ -316,7 +312,6 @@ public:
 	  *     fails, fall back to 3.1
 	  * @li MQTTVERSION_3_1 (3) = only try version 3.1
 	  * @li MQTTVERSION_3_1_1 (4) = only try version 3.1.1
-	  * @li MQTTVERSION_5 (5) = only try version 5
 	  */
 	int get_mqtt_version() const { return opts_.MQTTVersion; }
 	/**
@@ -510,9 +505,12 @@ public:
 	 * Gets the connect properties.
 	 * @return A const reference to the properties for the connect.
 	 */
-	const properties& get_properties() const {
+	const properties& get_properties() const { return props_; }
-		return props_;
+	/**
-	}
+	 * Gets a mutable reference to the connect properties.
+	 * @return A reference to the properties for the connect.
+	 */
+	properties& get_properties() { return props_; }
 	/**
 	 * Sets the properties for the connect.
 	 * @param props The properties to place into the message.

src/mqtt/disconnect_options.h

@@ -41,6 +41,9 @@ class disconnect_options
 	/** The default C struct */
 	static const MQTTAsync_disconnectOptions DFLT_C_STRUCT;
 
+	/** The default C struct */
+	static const MQTTAsync_disconnectOptions DFLT_C_STRUCT5;
+
 	/** The underlying C disconnect options */
 	MQTTAsync_disconnectOptions opts_;
 
@@ -53,6 +56,17 @@ class disconnect_options
 	/** The client has special access */
 	friend class async_client;
 
+	/** The options builder has special access */
+	friend class disconnect_options_builder;
+
+	/**
+	 * Updates the underlying C structure to match our cached data.
+	 */
+	void update_c_struct();
+
+	/** Construct options from a C struct */
+	disconnect_options(const MQTTAsync_disconnectOptions& copts) : opts_{copts} {}
+
 public:
 	/**
 	 * Create an empty delivery response object.
@@ -84,6 +98,16 @@ public:
 	 * @param opt Another object to move into this new one.
 	 */
 	disconnect_options(disconnect_options&& opt);
+	/**
+	 * Creates default options for an MQTT v3.x connection.
+	 * @return Default options for an MQTT v3.x connection.
+	 */
+	static disconnect_options v3();
+	/**
+	 * Creates default options for an MQTT v5 connection.
+	 * @return Default options for an MQTT v5 connection.
+	 */
+	static disconnect_options v5();
 	/**
 	 * Copy assignment.
 	 * @param opt Another object to copy.
@@ -108,16 +132,15 @@ public:
 		return std::chrono::milliseconds(opts_.timeout);
 	}
 	/**
-	 * Sets the timeout for disconnecting.
+	 * Sets the disconnect timeout, in milliseconds.
 	 * This allows for any remaining in-flight messages to be delivered.
-	 * @param timeout The timeout (in milliseconds).
+	 * @param timeout The disconnect timeout (in milliseconds).
 	 */
 	void set_timeout(int timeout) { opts_.timeout = timeout; }
 	/**
-	 * Sets the connect timeout with a chrono duration.
+	 * Sets the disconnect timeout with a duration.
-	 * This is the maximum time that the underlying library will wait for a
+	 * This allows for any remaining in-flight messages to be delivered.
-	 * connection before failing.
+	 * @param to The disconnect connect timeout.
-	 * @param to The connect timeout in seconds.
 	 */
 	template <class Rep, class Period>
 	void set_timeout(const std::chrono::duration<Rep, Period>& to) {
@@ -137,10 +160,15 @@ public:
 	 */
 	token_ptr get_token() const { return tok_; }
 	/**
-	 * Gets the connect properties.
+	 * Gets the disconnect properties.
-	 * @return A const reference to the properties for the connect.
+	 * @return A const reference to the properties for the disconnect.
 	 */
 	const properties& get_properties() const { return props_; }
+	/**
+	 * Gets a mutable reference to the disconnect properties.
+	 * @return A mutable reference to the properties for the disconnect.
+	 */
+	properties& get_properties() { return props_; }
 	/**
 	 * Sets the properties for the connect.
 	 * @param props The properties to place into the message.
@@ -173,7 +201,6 @@ public:
 	}
 };
 
-
 /////////////////////////////////////////////////////////////////////////////
 
 /**
@@ -184,6 +211,9 @@ class disconnect_options_builder
 	/** The underlying options */
 	disconnect_options opts_;
 
+	/** Construct options builder from a C struct */
+	disconnect_options_builder(const MQTTAsync_disconnectOptions& copts) : opts_{copts} {}
+
 public:
 	/** This class */
 	using self = disconnect_options_builder;
@@ -191,6 +221,16 @@ public:
 	 * Default constructor.
 	 */
 	disconnect_options_builder() {}
+	/**
+	 * Creates default options builder for an MQTT v3.x connection.
+	 * @return Default options builder for an MQTT v3.x connection.
+	 */
+	static disconnect_options_builder v3();
+	/**
+	 * Creates default options builder for an MQTT v5 connection.
+	 * @return Default options builder for an MQTT v5 connection.
+	 */
+	static disconnect_options_builder v5();
 	/**
 	 * Sets the properties for the disconnect message.
 	 * @param props The properties for the disconnect message.
@@ -208,10 +248,9 @@ public:
 		return *this;
 	}
 	/**
-	 * Sets the connect timeout with a chrono duration.
+	 * Sets the disconnect connect timeout.
-	 * This is the maximum time that the underlying library will wait for a
+	 * This allows for any remaining in-flight messages to be delivered.
-	 * connection before failing.
+	 * @param to The disconnect timeout.
-	 * @param to The connect timeout in seconds.
 	 */
 	template <class Rep, class Period>
 	auto timeout(const std::chrono::duration<Rep, Period>& to) -> self&{

src/mqtt/properties.h

@@ -253,6 +253,9 @@ inline string_pair get<string_pair>(const property& prop) {
  */
 class properties
 {
+	/** The default C struct */
+	static const MQTTProperties DFLT_C_STRUCT;
+
 	/** The underlying C properties struct  */
 	MQTTProperties props_;
 
@@ -267,9 +270,7 @@ public:
 	 * Default constructor.
 	 * Creates an empty properties list.
 	 */
-	properties() {
+	properties();
-		std::memset(&props_, 0, sizeof(MQTTProperties));
-	}
 	/**
 	 * Copy constructor.
 	 * @param other The property list to copy.
@@ -309,26 +310,13 @@ public:
 	 * @param rhs The other property list to copy into this one
 	 * @return A reference to this object.
 	 */
-	properties& operator=(const properties& rhs) {
+	properties& operator=(const properties& rhs);
-		if (&rhs != this) {
-			::MQTTProperties_free(&props_);
-			props_ = ::MQTTProperties_copy(&rhs.props_);
-		}
-		return *this;
-	}
 	/**
 	 * Move assignment.
 	 * @param rhs The property list to move to this one.
 	 * @return A reference to this object.
 	 */
-	properties& operator=(properties&& rhs) {
+	properties& operator=(properties&& rhs);
-		if (&rhs != this) {
-			::MQTTProperties_free(&props_);
-			props_ = rhs.props_;
-			std::memset(&rhs.props_, 0, sizeof(MQTTProperties));
-		}
-		return *this;
-	}
 	/**
 	 * Determines if the property list is empty.
 	 * @return @em true if there are no properties in the list, @em false if
@@ -350,7 +338,9 @@ public:
 	/**
 	 * Removes all the items from the property list.
 	 */
-	void clear();
+	void clear() {
+		::MQTTProperties_free(&props_);
+	}
 	/**
 	 * Determines if the list contains a specific property.
 	 * @param propid The property ID (code).

src/properties.cpp

@@ -154,17 +154,35 @@ property& property::operator=(property&& rhs)
 
 /////////////////////////////////////////////////////////////////////////////
 
-properties::properties(std::initializer_list<property> props)
+const MQTTProperties properties::DFLT_C_STRUCT = MQTTProperties_initializer;
+
+properties::properties() : props_{DFLT_C_STRUCT}
+{
+}
+
+properties::properties(std::initializer_list<property> props) : props_{DFLT_C_STRUCT}
 {
-	std::memset(&props_, 0, sizeof(properties));
 	for (const auto& prop : props)
 		::MQTTProperties_add(&props_, &prop.c_struct());
 }
 
-void properties::clear()
+properties& properties::operator=(const properties& rhs)
 {
-	::MQTTProperties_free(&props_);
+	if (&rhs != this) {
-	memset(&props_, 0, sizeof(MQTTProperties));
+		::MQTTProperties_free(&props_);
+		props_ = ::MQTTProperties_copy(&rhs.props_);
+	}
+	return *this;
+}
+
+properties& properties::operator=(properties&& rhs)
+{
+	if (&rhs != this) {
+		::MQTTProperties_free(&props_);
+		props_ = rhs.props_;
+		rhs.props_ = DFLT_C_STRUCT;
+	}
+	return *this;
 }
 
 property properties::get(property::code propid, size_t idx /*=0*/)

test/unit/test_connect_options.cpp

@@ -4,7 +4,7 @@
 //
 
 /*******************************************************************************
- * Copyright (c) 2016-2020 Frank Pagliughi <fpagliughi@mindspring.com>
+ * Copyright (c) 2016-2023 Frank Pagliughi <fpagliughi@mindspring.com>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -62,7 +62,7 @@ static const std::string HTTPS_PROXY { "https://localhost:443" };
 
 TEST_CASE("connect_options default ctor", "[options]")
 {
-	mqtt::connect_options opts;
+	connect_options opts;
 
 	REQUIRE(EMPTY_STR == opts.get_user_name());
 	REQUIRE(EMPTY_STR == opts.get_password_str());
@@ -113,7 +113,7 @@ TEST_CASE("connect_options default ctor", "[options]")
 
 TEST_CASE("connect_options user_constructor", "[options]")
 {
-	mqtt::connect_options opts { USER, PASSWD };
+	connect_options opts { USER, PASSWD };
 
 	REQUIRE(opts.get_http_proxy().empty());
 	REQUIRE(opts.get_https_proxy().empty());
@@ -144,10 +144,10 @@ TEST_CASE("connect_options user_constructor", "[options]")
 
 TEST_CASE("connect_options copy ctor", "[options]")
 {
-	mqtt::connect_options orgOpts { USER, PASSWD };
+	connect_options orgOpts { USER, PASSWD };
 
 	SECTION("simple options") {
-    	mqtt::connect_options opts { orgOpts };
+    	connect_options opts { orgOpts };
 
     	REQUIRE(USER == opts.get_user_name());
     	REQUIRE(PASSWD == opts.get_password_str());
@@ -172,7 +172,7 @@ TEST_CASE("connect_options copy ctor", "[options]")
 	SECTION("proxy options") {
 		orgOpts.set_http_proxy(HTTP_PROXY);
 
-		mqtt::connect_options opts { orgOpts };
+		connect_options opts { orgOpts };
 		REQUIRE(HTTP_PROXY == opts.get_http_proxy());
 		REQUIRE(opts.get_https_proxy().empty());
 	}
@@ -180,17 +180,25 @@ TEST_CASE("connect_options copy ctor", "[options]")
 	SECTION("secure proxy options") {
 		orgOpts.set_https_proxy(HTTPS_PROXY);
 
-		mqtt::connect_options opts { orgOpts };
+		connect_options opts { orgOpts };
 		REQUIRE(HTTPS_PROXY == opts.get_https_proxy());
 		REQUIRE(opts.get_http_proxy().empty());
 	}
 
 	SECTION("properties") {
-		orgOpts.set_properties({{ mqtt::property::SESSION_EXPIRY_INTERVAL, 0 }});
+		orgOpts.set_properties({{ property::SESSION_EXPIRY_INTERVAL, 0 }});
 
-		mqtt::connect_options opts { orgOpts };
+		connect_options opts { orgOpts };
 
-		REQUIRE(opts.get_properties().contains(mqtt::property::SESSION_EXPIRY_INTERVAL));
+		const auto& copts = opts.c_struct();
+		const auto& orgCopts = orgOpts.c_struct();
+
+		// Make sure it's an actual copy
+		REQUIRE(copts.connectProperties->array != orgCopts.connectProperties->array);
+		orgOpts.get_properties().clear();
+
+		REQUIRE(1 == opts.get_properties().size());
+		REQUIRE(opts.get_properties().contains(property::SESSION_EXPIRY_INTERVAL));
 		REQUIRE(opts.c_struct().connectProperties == &opts.get_properties().c_struct());
 	}
 }
@@ -201,10 +209,124 @@ TEST_CASE("connect_options copy ctor", "[options]")
 
 TEST_CASE("connect_options move_constructor", "[options]")
 {
-	mqtt::connect_options orgOpts { USER, PASSWD };
+	connect_options orgOpts { USER, PASSWD };
 
 	SECTION("simple options") {
-		mqtt::connect_options opts { std::move(orgOpts) };
+		connect_options opts { std::move(orgOpts) };
+
+		REQUIRE(USER == opts.get_user_name());
+		REQUIRE(PASSWD == opts.get_password_str());
+
+		const auto& copts = opts.c_struct();
+
+		REQUIRE(0 == memcmp(&copts.struct_id, CSIG, CSIG_LEN));
+
+		REQUIRE(0 == strcmp(USER.c_str(), copts.username));
+		REQUIRE(copts.password == nullptr);
+		REQUIRE(PASSWD.size() == size_t(copts.binarypwd.len));
+		REQUIRE(0 == memcmp(PASSWD.data(), copts.binarypwd.data, PASSWD.size()));
+		REQUIRE(nullptr == copts.connectProperties);
+
+		// Make sure it's a true copy, not linked to the original
+		orgOpts.set_user_name(EMPTY_STR);
+		orgOpts.set_password(EMPTY_STR);
+
+		REQUIRE(USER == opts.get_user_name());
+		REQUIRE(PASSWD == opts.get_password_str());
+
+		// Check that the original was moved
+		REQUIRE(EMPTY_STR == orgOpts.get_user_name());
+		REQUIRE(EMPTY_STR == orgOpts.get_password_str());
+	}
+
+	SECTION("properties") {
+		orgOpts.set_properties({{ property::SESSION_EXPIRY_INTERVAL, 42 }});
+
+		connect_options opts { std::move(orgOpts) };
+
+		const auto& copts = opts.c_struct();
+
+		// Check that the original was moved
+		REQUIRE(orgOpts.get_properties().empty());
+
+		// Check that we got the correct properties
+		REQUIRE(1 == opts.get_properties().size());
+		REQUIRE(opts.get_properties().contains(property::SESSION_EXPIRY_INTERVAL));
+		REQUIRE(42 == get<int>(opts.get_properties(), property::SESSION_EXPIRY_INTERVAL));
+
+		REQUIRE(copts.connectProperties == &opts.get_properties().c_struct());
+	}
+}
+
+// ----------------------------------------------------------------------
+// Test the copy assignment operator=(const&)
+// ----------------------------------------------------------------------
+
+TEST_CASE("connect_options copy_assignment", "[options]")
+{
+	SECTION("v3") {
+		connect_options orgOpts { USER, PASSWD };
+		connect_options opts;
+
+		opts = orgOpts;
+
+		REQUIRE(USER == opts.get_user_name());
+		REQUIRE(PASSWD == opts.get_password_str());
+
+		const auto& copts = opts.c_struct();
+
+		REQUIRE(0 == memcmp(&copts.struct_id, CSIG, CSIG_LEN));
+
+		REQUIRE(0 == strcmp(USER.c_str(), copts.username));
+		REQUIRE(copts.password == nullptr);
+		REQUIRE(PASSWD.size() == size_t(copts.binarypwd.len));
+		REQUIRE(0 == memcmp(PASSWD.data(), copts.binarypwd.data, PASSWD.size()));
+		REQUIRE(nullptr == copts.connectProperties);
+
+		// Make sure it's a true copy, not linked to the original
+		orgOpts.set_user_name(EMPTY_STR);
+		orgOpts.set_password(EMPTY_STR);
+
+		REQUIRE(USER == opts.get_user_name());
+		REQUIRE(PASSWD == opts.get_password_str());
+
+		// Self assignment should cause no harm
+		opts = opts;
+
+		REQUIRE(USER == opts.get_user_name());
+		REQUIRE(PASSWD == opts.get_password_str());
+	}
+
+	SECTION("v5") {
+		auto orgOpts = connect_options::v5();
+		orgOpts.set_properties({{ property::SESSION_EXPIRY_INTERVAL, 42 }});
+
+		connect_options opts = orgOpts;
+		const auto& copts = opts.c_struct();
+		const auto& orgCopts = orgOpts.c_struct();
+
+		// Make sure it's an actual copy
+		REQUIRE(copts.connectProperties->array != orgCopts.connectProperties->array);
+		orgOpts.get_properties().clear();
+
+		// Check that we got the correct properties
+		REQUIRE(1 == opts.get_properties().size());
+		REQUIRE(opts.get_properties().contains(property::SESSION_EXPIRY_INTERVAL));
+		REQUIRE(42 == get<int>(opts.get_properties(), property::SESSION_EXPIRY_INTERVAL));
+
+		REQUIRE(copts.connectProperties == &opts.get_properties().c_struct());
+	}
+}
+
+// ----------------------------------------------------------------------
+// Test the move assignment, operator=(&&)
+// ----------------------------------------------------------------------
+
+TEST_CASE("connect_options move_assignment", "[options]")
+{
+	SECTION("v3") {
+		connect_options orgOpts { USER, PASSWD };
+		connect_options opts { std::move(orgOpts) };
 
 		REQUIRE(USER == opts.get_user_name());
 		REQUIRE(PASSWD == opts.get_password_str());
@@ -228,99 +350,36 @@ TEST_CASE("connect_options move_constructor", "[options]")
 		// Check that the original was moved
 		REQUIRE(EMPTY_STR == orgOpts.get_user_name());
 		REQUIRE(EMPTY_STR == orgOpts.get_password_str());
-		}
+
+		// Self assignment should cause no harm
+		// (clang++ is smart enough to warn about this)
+		#if !defined(__clang__)
+			opts = std::move(opts);
+			REQUIRE(USER == opts.get_user_name());
+			REQUIRE(PASSWD == opts.get_password_str());
+		#endif
+	}
 
 	SECTION("properties") {
-		orgOpts.set_properties({{ mqtt::property::SESSION_EXPIRY_INTERVAL, 0 }});
+		auto orgOpts = connect_options::v5();
+		orgOpts.set_properties({{ property::SESSION_EXPIRY_INTERVAL, 42 }});
 
-		mqtt::connect_options opts { std::move(orgOpts) };
+		connect_options opts = std::move(orgOpts);
 
-		REQUIRE(opts.get_properties().contains(mqtt::property::SESSION_EXPIRY_INTERVAL));
+		const auto& copts = opts.c_struct();
-		REQUIRE(opts.c_struct().connectProperties == &opts.get_properties().c_struct());
 
 		// Check that the original was moved
 		REQUIRE(orgOpts.get_properties().empty());
+
+		// Check that we got the correct properties
+		REQUIRE(1 == opts.get_properties().size());
+		REQUIRE(opts.get_properties().contains(property::SESSION_EXPIRY_INTERVAL));
+		REQUIRE(42 == get<int>(opts.get_properties(), property::SESSION_EXPIRY_INTERVAL));
+
+		REQUIRE(copts.connectProperties == &opts.get_properties().c_struct());
 	}
 }
 
-// ----------------------------------------------------------------------
-// Test the copy assignment operator=(const&)
-// ----------------------------------------------------------------------
-
-TEST_CASE("connect_options copy_assignment", "[options]")
-{
-	mqtt::connect_options orgOpts { USER, PASSWD };
-	mqtt::connect_options opts;
-
-	opts = orgOpts;
-
-	REQUIRE(USER == opts.get_user_name());
-	REQUIRE(PASSWD == opts.get_password_str());
-
-	const auto& c_struct = opts.c_struct();
-
-	REQUIRE(0 == memcmp(&c_struct.struct_id, CSIG, CSIG_LEN));
-
-	REQUIRE(0 == strcmp(USER.c_str(), c_struct.username));
-	REQUIRE(c_struct.password == nullptr);
-	REQUIRE(PASSWD.size() == size_t(c_struct.binarypwd.len));
-	REQUIRE(0 == memcmp(PASSWD.data(), c_struct.binarypwd.data, PASSWD.size()));
-
-	// Make sure it's a true copy, not linked to the original
-	orgOpts.set_user_name(EMPTY_STR);
-	orgOpts.set_password(EMPTY_STR);
-
-	REQUIRE(USER == opts.get_user_name());
-	REQUIRE(PASSWD == opts.get_password_str());
-
-	// Self assignment should cause no harm
-	opts = opts;
-
-	REQUIRE(USER == opts.get_user_name());
-	REQUIRE(PASSWD == opts.get_password_str());
-}
-
-// ----------------------------------------------------------------------
-// Test the move assignment, operator=(&&)
-// ----------------------------------------------------------------------
-
-TEST_CASE("connect_options move_assignment", "[options]")
-{
-	mqtt::connect_options orgOpts { USER, PASSWD };
-	mqtt::connect_options opts { std::move(orgOpts) };
-
-	REQUIRE(USER == opts.get_user_name());
-	REQUIRE(PASSWD == opts.get_password_str());
-
-	const auto& c_struct = opts.c_struct();
-
-	REQUIRE(0 == memcmp(&c_struct.struct_id, CSIG, CSIG_LEN));
-
-	REQUIRE(0 == strcmp(USER.c_str(), c_struct.username));
-	REQUIRE(c_struct.password == nullptr);
-	REQUIRE(PASSWD.size() == size_t(c_struct.binarypwd.len));
-	REQUIRE(0 == memcmp(PASSWD.data(), c_struct.binarypwd.data, PASSWD.size()));
-
-	// Make sure it's a true copy, not linked to the original
-	orgOpts.set_user_name(EMPTY_STR);
-	orgOpts.set_password(EMPTY_STR);
-
-	REQUIRE(USER == opts.get_user_name());
-	REQUIRE(PASSWD == opts.get_password_str());
-
-	// Check that the original was moved
-	REQUIRE(EMPTY_STR == orgOpts.get_user_name());
-	REQUIRE(EMPTY_STR == orgOpts.get_password_str());
-
-	// Self assignment should cause no harm
-	// (clang++ is smart enough to warn about this)
-	#if !defined(__clang__)
-		opts = std::move(opts);
-		REQUIRE(USER == opts.get_user_name());
-		REQUIRE(PASSWD == opts.get_password_str());
-	#endif
-}
-
 
 // ----------------------------------------------------------------------
 // Test set/get of the user and password.
@@ -328,7 +387,7 @@ TEST_CASE("connect_options move_assignment", "[options]")
 
 TEST_CASE("connect_options set_user", "[options]")
 {
-	mqtt::connect_options opts;
+	connect_options opts;
 	const auto& c_struct = opts.c_struct();
 
 	opts.set_user_name(USER);
@@ -359,7 +418,7 @@ TEST_CASE("connect_options set_long_user", "[options]")
 		passwd.push_back(by);
 	}
 
-	mqtt::connect_options orgOpts;
+	connect_options orgOpts;
 
 	orgOpts.set_user_name(user);
 	orgOpts.set_password(passwd);
@@ -367,7 +426,7 @@ TEST_CASE("connect_options set_long_user", "[options]")
 	REQUIRE(user == orgOpts.get_user_name());
 	REQUIRE(passwd == orgOpts.get_password_str());
 
-	mqtt::connect_options opts;
+	connect_options opts;
 	opts = orgOpts;
 
 	REQUIRE(user == opts.get_user_name());
@@ -387,11 +446,11 @@ TEST_CASE("connect_options set_long_user", "[options]")
 
 TEST_CASE("connect_options set_will", "[options]")
 {
-	mqtt::connect_options opts;
+	connect_options opts;
 	const auto& c_struct = opts.c_struct();
 
 	REQUIRE(nullptr == c_struct.will);
-	mqtt::will_options willOpts;
+	will_options willOpts;
 	opts.set_will(willOpts);
 	REQUIRE(nullptr != c_struct.will);
 	//REQUIRE(&opts.will_.opts_ == c_struct.will);
@@ -403,11 +462,11 @@ TEST_CASE("connect_options set_will", "[options]")
 
 TEST_CASE("connect_options set_ssl", "[options]")
 {
-	mqtt::connect_options opts;
+	connect_options opts;
 	const auto& c_struct = opts.c_struct();
 
 	REQUIRE(nullptr == c_struct.ssl);
-	mqtt::ssl_options sslOpts;
+	ssl_options sslOpts;
 	opts.set_ssl(sslOpts);
 	REQUIRE(nullptr != c_struct.ssl);
 	//REQUIRE(&opts.ssl_.opts_ == c_struct.ssl);
@@ -419,7 +478,7 @@ TEST_CASE("connect_options set_ssl", "[options]")
 
 TEST_CASE("set_token", "[options]")
 {
-	mqtt::connect_options opts;
+	connect_options opts;
 	const auto& c_struct = opts.c_struct();
 
 	REQUIRE(nullptr == c_struct.context);
@@ -509,39 +568,39 @@ TEST_CASE("set_token", "[options]")
 
 TEST_CASE("connect_options_builder default generator", "[options]")
 {
-	mqtt::connect_options opts;
+	connect_options opts;
 
 	// Default is v3.x
 
-	opts = mqtt::connect_options_builder().finalize();
+	opts = connect_options_builder().finalize();
 
 	REQUIRE(MQTTVERSION_DEFAULT == opts.get_mqtt_version());
 	REQUIRE(DFLT_KEEP_ALIVE == (int) opts.get_keep_alive_interval().count());
 
 	// Explicit v3
 
-	opts = mqtt::connect_options_builder::v3().finalize();
+	opts = connect_options_builder::v3().finalize();
 
 	REQUIRE(MQTTVERSION_DEFAULT == opts.get_mqtt_version());
 	REQUIRE(DFLT_KEEP_ALIVE == (int) opts.get_keep_alive_interval().count());
 
 	// v5
 
-	opts = mqtt::connect_options_builder::v5().finalize();
+	opts = connect_options_builder::v5().finalize();
 
 	REQUIRE(MQTTVERSION_5 == opts.get_mqtt_version());
 	REQUIRE(DFLT_KEEP_ALIVE == (int) opts.get_keep_alive_interval().count());
 
 	// WebSocket
 
-	opts = mqtt::connect_options_builder::ws().finalize();
+	opts = connect_options_builder::ws().finalize();
 
 	REQUIRE(MQTTVERSION_DEFAULT == opts.get_mqtt_version());
 	REQUIRE(DFLT_WS_KEEP_ALIVE == (int) opts.get_keep_alive_interval().count());
 
 	// Explicit WebSocket v5
 
-	opts = mqtt::connect_options_builder::v5_ws().finalize();
+	opts = connect_options_builder::v5_ws().finalize();
 
 	REQUIRE(MQTTVERSION_5 == opts.get_mqtt_version());
 	REQUIRE(DFLT_WS_KEEP_ALIVE == (int) opts.get_keep_alive_interval().count());
@@ -555,10 +614,10 @@ TEST_CASE("connect_options_builder set", "[options]")
 {
 	const uint32_t INTERVAL = 80000;
 
-	mqtt::properties conn_props{
+	properties conn_props{
-		mqtt::property{mqtt::property::SESSION_EXPIRY_INTERVAL, INTERVAL}};
+		property{property::SESSION_EXPIRY_INTERVAL, INTERVAL}};
 
-	auto opts = mqtt::connect_options_builder()
+	auto opts = connect_options_builder()
 		.properties(conn_props)
 		.finalize();
 
@@ -566,7 +625,7 @@ TEST_CASE("connect_options_builder set", "[options]")
 
 	REQUIRE(!props.empty());
 	REQUIRE(1 == props.size());
-	REQUIRE(INTERVAL == get<uint32_t>(props, mqtt::property::SESSION_EXPIRY_INTERVAL));
+	REQUIRE(INTERVAL == get<uint32_t>(props, property::SESSION_EXPIRY_INTERVAL));
 
 	const auto& copts = opts.c_struct();
 	REQUIRE(nullptr != copts.connectProperties);

test/unit/test_disconnect_options.cpp

@@ -3,6 +3,7 @@
 
 /*******************************************************************************
  * Copyright (c) 2016 Guilherme M. Ferreira <guilherme.maciel.ferreira@gmail.com>
+ * Copyright (c) 2016-2023 Frank Pagliughi <fpagliughi@mindspring.com>
  *
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
@@ -42,7 +43,7 @@ static mock_async_client cli;
 
 TEST_CASE("disconnect_options dflt constructor", "[options]")
 {
-	mqtt::disconnect_options opts;
+	disconnect_options opts;
 
 	REQUIRE(DFLT_TIMEOUT == (int) opts.get_timeout().count());
 	REQUIRE(!opts.get_token());
@@ -64,7 +65,7 @@ TEST_CASE("disconnect_options user constructor", "[options]")
 	const int TIMEOUT = 10;
 
 	auto tok = token::create(TOKEN_TYPE, cli);
-	mqtt::disconnect_options opts { TIMEOUT };
+	disconnect_options opts { TIMEOUT };
 	opts.set_token(tok, MQTTVERSION_DEFAULT);
 
 	const auto& c_struct = opts.c_struct();
@@ -84,10 +85,10 @@ TEST_CASE("disconnect_options copy ctor", "[options]")
 {
 	constexpr std::chrono::milliseconds TIMEOUT { 50 };
 
-	mqtt::disconnect_options orgOpts { TIMEOUT };
+	disconnect_options orgOpts { TIMEOUT };
 
 	SECTION("simple options") {
-    	mqtt::disconnect_options opts { orgOpts };
+    	disconnect_options opts { orgOpts };
 
     	REQUIRE(TIMEOUT == opts.get_timeout());
 
@@ -99,12 +100,29 @@ TEST_CASE("disconnect_options copy ctor", "[options]")
 	}
 
 	SECTION("properties") {
-		orgOpts.set_properties({{ mqtt::property::SESSION_EXPIRY_INTERVAL, 0 }});
+		orgOpts.set_properties({{ property::SESSION_EXPIRY_INTERVAL, 42 }});
 
-		mqtt::disconnect_options opts { orgOpts };
+		disconnect_options opts { orgOpts };
+
+		const auto& copts = opts.c_struct();
+		const auto& orgCopts = orgOpts.c_struct();
+
+		// Make sure we copied the properties
+		REQUIRE(orgCopts.properties.array != copts.properties.array);
+		orgOpts.get_properties().clear();
+
+		// Check that the properties transferred over
+		REQUIRE(1 == opts.get_properties().size());
+		REQUIRE(opts.get_properties().contains(property::SESSION_EXPIRY_INTERVAL));
+		REQUIRE(42 == get<int>(opts.get_properties(), property::SESSION_EXPIRY_INTERVAL));
 
-		REQUIRE(opts.get_properties().contains(mqtt::property::SESSION_EXPIRY_INTERVAL));
 		REQUIRE(1 == opts.c_struct().properties.count);
+		REQUIRE(1 == MQTTProperties_propertyCount(
+			const_cast<MQTTProperties*>(&copts.properties),
+			MQTTPROPERTY_CODE_SESSION_EXPIRY_INTERVAL));
+		REQUIRE(42 == MQTTProperties_getNumericValue(
+			const_cast<MQTTProperties*>(&copts.properties),
+			        MQTTPROPERTY_CODE_SESSION_EXPIRY_INTERVAL));
 	}
 }
 
@@ -116,23 +134,25 @@ TEST_CASE("disconnect_options move_constructor", "[options]")
 {
 	constexpr std::chrono::milliseconds TIMEOUT { 50 };
 
-	mqtt::disconnect_options orgOpts { TIMEOUT };
+	disconnect_options orgOpts { TIMEOUT };
 
 	SECTION("simple options") {
-		mqtt::disconnect_options opts { std::move(orgOpts) };
+		disconnect_options opts { std::move(orgOpts) };
 
 		REQUIRE(TIMEOUT == opts.get_timeout());
-
     	REQUIRE(opts.get_properties().empty());
 	}
 
 	SECTION("properties") {
-		orgOpts.set_properties({{ mqtt::property::SESSION_EXPIRY_INTERVAL, 0 }});
+		orgOpts.set_properties({{ property::SESSION_EXPIRY_INTERVAL, 42 }});
 
-		mqtt::disconnect_options opts { std::move(orgOpts) };
+		disconnect_options opts { std::move(orgOpts) };
 
-		REQUIRE(opts.get_properties().contains(mqtt::property::SESSION_EXPIRY_INTERVAL));
+		const auto& copts = opts.c_struct();
-		REQUIRE(1 == opts.c_struct().properties.count);
+
+		REQUIRE(1 == opts.get_properties().size());
+		REQUIRE(opts.get_properties().contains(property::SESSION_EXPIRY_INTERVAL));
+		REQUIRE(1 == copts.properties.count);
 
 		// Check that the original was moved
 		REQUIRE(orgOpts.get_properties().empty());
@@ -145,7 +165,7 @@ TEST_CASE("disconnect_options move_constructor", "[options]")
 
 TEST_CASE("disconnect_options set timeout", "[options]")
 {
-	mqtt::disconnect_options opts;
+	disconnect_options opts;
 	const auto& c_struct = opts.c_struct();
 
 	const int TIMEOUT = 5000;	// ms
@@ -168,14 +188,14 @@ TEST_CASE("disconnect_options set timeout", "[options]")
 TEST_CASE("disconnect_options set token", "[options]")
 {
 	auto tok = token::create(TOKEN_TYPE, cli);
-	mqtt::disconnect_options opts;
+	disconnect_options opts;
 
 	const auto& c_struct = opts.c_struct();
 
 	REQUIRE(nullptr == c_struct.onSuccess);
 	REQUIRE(nullptr == c_struct.onFailure);
 
-	opts.set_token(mqtt::token_ptr(), MQTTVERSION_DEFAULT);
+	opts.set_token(token_ptr(), MQTTVERSION_DEFAULT);
 	REQUIRE(nullptr == c_struct.onSuccess);
 	REQUIRE(nullptr == c_struct.onFailure);
 

test/unit/test_properties.cpp

@@ -538,6 +538,10 @@ TEST_CASE("properties copy and move", "[properties]") {
 		properties props { orgProps };
 
 		// Make sure it's a real copy, not a reference to org
+		const auto& cprops = props.c_struct();
+		const auto& orgCprops = orgProps.c_struct();
+		REQUIRE(orgCprops.array != cprops.array);
+
 		orgProps.clear();
 
 		REQUIRE(get<uint8_t>(props, property::PAYLOAD_FORMAT_INDICATOR) == FMT_IND);
@@ -579,14 +583,20 @@ TEST_CASE("properties copy and move", "[properties]") {
 
 		REQUIRE(orgProps.empty());
 		REQUIRE(0 == orgProps.size());
-	}
 
+		const auto& orgCprops = orgProps.c_struct();
+		REQUIRE(nullptr == orgCprops.array);
+	}
 
 	SECTION("copy assignment") {
 		properties props;
 		props = orgProps;
 
 		// Make sure it's a real copy, not a reference to org
+		const auto& cprops = props.c_struct();
+		const auto& orgCprops = orgProps.c_struct();
+		REQUIRE(orgCprops.array != cprops.array);
+
 		orgProps.clear();
 
 		REQUIRE(get<uint8_t>(props, property::PAYLOAD_FORMAT_INDICATOR) == FMT_IND);