#524 Fixed copy and move operations for 'subscribe_options'. Added unit tests.

include/mqtt/response_options.h

@@ -1,9 +1,25 @@
 /////////////////////////////////////////////////////////////////////////////
 /// @file response_options.h
 /// Implementation of the class 'response_options'
-/// @date 26-Aug-2016
+/// @date 26-Aug-2019
 /////////////////////////////////////////////////////////////////////////////
 
+/*******************************************************************************
+ * Copyright (c) 2019-2025 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 v2.0
+ * and Eclipse Distribution License v1.0 which accompany this distribution.
+ *
+ * The Eclipse Public License is available at
+ *    http://www.eclipse.org/legal/epl-v20.html
+ * and the Eclipse Distribution License is available at
+ *   http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * Contributors:
+ *    Frank Pagliughi - initial implementation and documentation
+ *******************************************************************************/
+
 #ifndef __mqtt_response_options_h
 #define __mqtt_response_options_h
 
@@ -70,16 +86,26 @@ public:
      * @param other The other options to copy to this one.
      */
     response_options(const response_options& other);
+    /**
+     * Move constructor.
+     * @param other The other options to move into this one.
+     */
+    response_options(response_options&& other);
     /**
      * Copy operator.
      * @param rhs The other options to copy to this one.
      */
     response_options& operator=(const response_options& rhs);
+    /**
+     * Move operator.
+     * @param rhs The other options to move into this one.
+     */
+    response_options& operator=(response_options&& rhs);
 /**
  * Expose the underlying C struct for the unit tests.
  */
 #if defined(UNIT_TESTS)
-    const MQTTAsync_responseOptions& c_struct() const { return opts_; }
+    const auto& c_struct() const { return opts_; }
 #endif
     /**
      * Sets the MQTT protocol version used for the response.
@@ -112,6 +138,18 @@ public:
         props_ = std::move(props);
         opts_.properties = props_.c_struct();
     }
+    /**
+     * Gets the options for a single topic subscription.
+     * @return The subscribe options.
+     */
+    subscribe_options get_subscribe_options() const {
+        return subscribe_options{opts_.subscribeOptions};
+    }
+    /**
+     * Sets the options for a multi-topic subscription.
+     * @return The vector of the subscribe options.
+     */
+    std::vector<subscribe_options> get_subscribe_many_options() const;
     /**
      * Sets the options for a single topic subscription.
      * @param opts The subscribe options.
@@ -121,7 +159,15 @@ public:
      * Sets the options for a multi-topic subscription.
      * @param opts A vector of the subscribe options.
      */
-    void set_subscribe_options(const std::vector<subscribe_options>& opts);
+    void set_subscribe_many_options(const std::vector<subscribe_options>& opts);
+    /**
+     * Sets the options for a multi-topic subscription.
+     * @param opts A vector of the subscribe options.
+     * @sa set_subscribe_options
+     */
+    void set_subscribe_options(const std::vector<subscribe_options>& opts) {
+        set_subscribe_many_options(opts);
+    }
 };
 
 /////////////////////////////////////////////////////////////////////////////
@@ -183,6 +229,14 @@ public:
         opts_.set_subscribe_options(opts);
         return *this;
     }
+    /**
+     * Sets the options for a multi-topic subscription.
+     * @param opts A vector of the subscribe options.
+     */
+    auto subscribe_many_opts(const std::vector<subscribe_options>& opts) -> self& {
+        opts_.set_subscribe_options(opts);
+        return *this;
+    }
     /**
      * Sets the options for a multi-topic subscription.
      * @param opts A vector of the subscribe options.
@@ -192,8 +246,8 @@ public:
         return *this;
     }
     /**
-     * Finish building the options and return them.
+     * Finish building the response options and return them.
-     * @return The option struct as built.
+     * @return The response option struct as built.
      */
     response_options finalize() { return opts_; }
 };

include/mqtt/subscribe_options.h

@@ -118,12 +118,19 @@ public:
         opts_.retainAsPublished = retainAsPublished ? 1 : 0;
         opts_.retainHandling = (unsigned char)retainHandling;
     }
-/**
+    /**
+     * Creates the set of subscribe options from an underlying C struct.
+     * @param opts The Paho C subscribe options
+     */
+    explicit subscribe_options(MQTTSubscribe_options opts) : opts_{opts} {}
+
+#if defined(UNIT_TESTS)
+    /**
      * Expose the underlying C struct for the unit tests.
      */
-#if defined(UNIT_TESTS)
     const auto& c_struct() const { return opts_; }
 #endif
+
     /**
      * Gets the value of the "no local" flag.
      * @return Whether the server should send back our own publications, if

src/response_options.cpp

@@ -1,7 +1,7 @@
 // response_options.cpp
 
 /*******************************************************************************
- * Copyright (c) 2019-2024 Frank Pagliughi <fpagliughi@mindspring.com>
+ * Copyright (c) 2019-2025 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 v2.0
@@ -30,18 +30,43 @@ response_options::
 }
 
 response_options::response_options(const response_options& other)
-    : opts_(other.opts_), tok_(other.tok_), props_(other.props_)
+    : opts_{other.opts_}, tok_{other.tok_}, props_{other.props_}, subOpts_{other.subOpts_}
+{
+    update_c_struct();
+}
+
+response_options::response_options(response_options&& other)
+    : opts_{other.opts_},
+      tok_{std::move(other.tok_)},
+      props_{std::move(other.props_)},
+      subOpts_{std::move(other.subOpts_)}
 {
     update_c_struct();
 }
 
 response_options& response_options::operator=(const response_options& rhs)
 {
+    if (&rhs != this) {
         opts_ = rhs.opts_;
         tok_ = rhs.tok_;
         props_ = rhs.props_;
+        subOpts_ = rhs.subOpts_;
 
         update_c_struct();
+    }
+    return *this;
+}
+
+response_options& response_options::operator=(response_options&& rhs)
+{
+    if (&rhs != this) {
+        opts_ = rhs.opts_;
+        tok_ = std::move(rhs.tok_);
+        props_ = std::move(rhs.props_);
+        subOpts_ = std::move(rhs.subOpts_);
+
+        update_c_struct();
+    }
     return *this;
 }
 
@@ -80,7 +105,14 @@ void response_options::set_subscribe_options(const subscribe_options& opts)
     opts_.subscribeOptions = opts.opts_;
 }
 
-void response_options::set_subscribe_options(const std::vector<subscribe_options>& opts)
+std::vector<subscribe_options> response_options::get_subscribe_many_options() const
+{
+    std::vector<subscribe_options> opts;
+    for (const auto& opt : subOpts_) opts.push_back(subscribe_options{opt});
+    return opts;
+}
+
+void response_options::set_subscribe_many_options(const std::vector<subscribe_options>& opts)
 {
     subOpts_.clear();
     for (const auto& opt : opts) subOpts_.push_back(opt.opts_);

test/unit/test_response_options.cpp

@@ -38,66 +38,143 @@ using namespace mqtt;
 
 static constexpr token::Type TOKEN_TYPE = token::Type::CONNECT;
 
+// The struct_id for the Paho C MQTTSubscribe_options struct.
+static constexpr const char* STRUCT_ID = "MQTR";
+
+const properties PROPS{
+    {property::PAYLOAD_FORMAT_INDICATOR, 42}, {property::MESSAGE_EXPIRY_INTERVAL, 70000}
+};
+
+const std::vector<subscribe_options> SUB_OPTS{
+    3, subscribe_options{subscribe_options::NO_LOCAL}
+};
+
 static mock_async_client cli;
 
 // ----------------------------------------------------------------------
 // Test default constructor
 // ----------------------------------------------------------------------
 
-TEST_CASE("response_options dflt constructor", "[options]")
+TEST_CASE("response_options dflt ctor", "[options]")
 {
     response_options opts;
-    const auto& c_struct = opts.c_struct();
+    const auto& copts = opts.c_struct();
 
-    REQUIRE(c_struct.context == nullptr);
+    REQUIRE(0 == memcmp(copts.struct_id, STRUCT_ID, 4));
+    REQUIRE(copts.context == nullptr);
 
-    // Make sure the callback functions are set during object construction
+    // Make sure the v3 callback functions are set during object construction
-    REQUIRE(c_struct.onSuccess != nullptr);
+    REQUIRE(copts.onSuccess != nullptr);
-    REQUIRE(c_struct.onFailure != nullptr);
+    REQUIRE(copts.onFailure != nullptr);
+    REQUIRE(copts.onSuccess5 == nullptr);
+    REQUIRE(copts.onFailure5 == nullptr);
 }
 
 // ----------------------------------------------------------------------
 // Test user constructor
 // ----------------------------------------------------------------------
 
-TEST_CASE("response_options user constructor", "[options]")
+TEST_CASE("response_options user ctor", "[options]")
 {
     token_ptr token{token::create(TOKEN_TYPE, cli)};
     response_options opts{token};
-    const auto& c_struct = opts.c_struct();
+    const auto& copts = opts.c_struct();
 
-    REQUIRE(c_struct.context == token.get());
+    REQUIRE(0 == memcmp(copts.struct_id, STRUCT_ID, 4));
+    REQUIRE(copts.context == token.get());
 
-    // Make sure the callback functions are set during object construction
+    // Make sure the v3 callback functions are set during object construction
-    REQUIRE(c_struct.onSuccess != nullptr);
+    REQUIRE(copts.onSuccess != nullptr);
-    REQUIRE(c_struct.onFailure != nullptr);
+    REQUIRE(copts.onFailure != nullptr);
+    REQUIRE(copts.onSuccess5 == nullptr);
+    REQUIRE(copts.onFailure5 == nullptr);
+}
+
+// ----------------------------------------------------------------------
+// Test user constructor for v5
+// ----------------------------------------------------------------------
+
+TEST_CASE("response_options user v5 ctor", "[options]")
+{
+    token_ptr token{token::create(TOKEN_TYPE, cli)};
+    response_options opts{token, 5};
+    const auto& copts = opts.c_struct();
+
+    REQUIRE(0 == memcmp(copts.struct_id, STRUCT_ID, 4));
+    REQUIRE(copts.context == token.get());
+
+    // Make sure the v5 callback functions are set during object construction
+    REQUIRE(copts.onSuccess == nullptr);
+    REQUIRE(copts.onFailure == nullptr);
+    REQUIRE(copts.onSuccess5 != nullptr);
+    REQUIRE(copts.onFailure5 != nullptr);
 }
 
 // ----------------------------------------------------------------------
 // Test copy constructor
 // ----------------------------------------------------------------------
 
-TEST_CASE("response_options copy constructor", "[options]")
+TEST_CASE("response_options copy ctor", "[options]")
 {
     token_ptr token{token::create(TOKEN_TYPE, cli)};
-    response_options opts_org{token};
 
-    properties props{
+    response_options optsOrg{token, 5};
-        {property::PAYLOAD_FORMAT_INDICATOR, 42}, {property::MESSAGE_EXPIRY_INTERVAL, 70000}
+    optsOrg.set_properties(PROPS);
-    };
+    optsOrg.set_subscribe_many_options(SUB_OPTS);
-    opts_org.set_properties(props);
-
-    response_options opts{opts_org};
 
+    response_options opts{optsOrg};
     const auto& copts = opts.c_struct();
 
+    REQUIRE(0 == memcmp(copts.struct_id, STRUCT_ID, 4));
     REQUIRE(copts.context == token.get());
 
-    // Make sure the callback functions are set during object construction
+    // Make sure the v5 callback functions are set during object construction
-    REQUIRE(copts.onSuccess != nullptr);
+    REQUIRE(copts.onSuccess == nullptr);
-    REQUIRE(copts.onFailure != nullptr);
+    REQUIRE(copts.onFailure == nullptr);
+    REQUIRE(copts.onSuccess5 != nullptr);
+    REQUIRE(copts.onFailure5 != nullptr);
 
-    REQUIRE(opts.get_properties().size() == 2);
+    REQUIRE(opts.get_properties().size() == PROPS.size());
+
+    auto subOpts = opts.get_subscribe_many_options();
+    REQUIRE(subOpts.size() == SUB_OPTS.size());
+    REQUIRE(subOpts[0].get_no_local());
+    REQUIRE(subOpts[1].get_no_local());
+}
+
+// ----------------------------------------------------------------------
+// Test move constructor
+// ----------------------------------------------------------------------
+
+TEST_CASE("response_options move ctor", "[options]")
+{
+    token_ptr token{token::create(TOKEN_TYPE, cli)};
+
+    response_options optsOrg{token, 5};
+    optsOrg.set_properties(PROPS);
+    optsOrg.set_subscribe_many_options(SUB_OPTS);
+
+    response_options opts{std::move(optsOrg)};
+    const auto& copts = opts.c_struct();
+
+    REQUIRE(0 == memcmp(copts.struct_id, STRUCT_ID, 4));
+    REQUIRE(copts.context == token.get());
+
+    // Make sure the v3 callback functions are set during object construction
+    REQUIRE(copts.onSuccess == nullptr);
+    REQUIRE(copts.onFailure == nullptr);
+    REQUIRE(copts.onSuccess5 != nullptr);
+    REQUIRE(copts.onFailure5 != nullptr);
+
+    REQUIRE(opts.get_properties().size() == PROPS.size());
+
+    auto subOpts = opts.get_subscribe_many_options();
+    REQUIRE(subOpts.size() == SUB_OPTS.size());
+    REQUIRE(subOpts[0].get_no_local());
+    REQUIRE(subOpts[1].get_no_local());
+
+    auto subOptsOrg = optsOrg.get_subscribe_many_options();
+    REQUIRE(subOptsOrg.size() == 0);
 }
 
 // ----------------------------------------------------------------------
@@ -108,12 +185,30 @@ TEST_CASE("response_options builder", "[options]")
 {
     token_ptr token{token::create(TOKEN_TYPE, cli)};
 
-    properties props{
+    auto opts = response_options_builder()
-        {property::PAYLOAD_FORMAT_INDICATOR, 42}, {property::MESSAGE_EXPIRY_INTERVAL, 70000}
+                    .mqtt_version(5)
-    };
+                    .token(token)
+                    .properties(PROPS)
+                    .subscribe_opts(SUB_OPTS)
+                    .finalize();
 
-    auto opts =
+    const auto& copts = opts.c_struct();
-        response_options_builder().mqtt_version(5).token(token).properties(props).finalize();
+
+    REQUIRE(0 == memcmp(copts.struct_id, STRUCT_ID, 4));
+    REQUIRE(copts.context == token.get());
+
+    // Make sure the v5 callback functions are set during object construction
+    REQUIRE(copts.onSuccess == nullptr);
+    REQUIRE(copts.onFailure == nullptr);
+    REQUIRE(copts.onSuccess5 != nullptr);
+    REQUIRE(copts.onFailure5 != nullptr);
+
+    REQUIRE(opts.get_properties().size() == PROPS.size());
+
+    auto subOpts = opts.get_subscribe_many_options();
+    REQUIRE(subOpts.size() == SUB_OPTS.size());
+    REQUIRE(subOpts[0].get_no_local());
+    REQUIRE(subOpts[1].get_no_local());
 }
 
 // ----------------------------------------------------------------------
@@ -123,12 +218,12 @@ TEST_CASE("response_options builder", "[options]")
 TEST_CASE("response_options set token", "[options]")
 {
     response_options opts;
-    const auto& c_struct = opts.c_struct();
+    const auto& copts = opts.c_struct();
 
-    REQUIRE(c_struct.context == nullptr);
+    REQUIRE(copts.context == nullptr);
     token_ptr token{token::create(TOKEN_TYPE, cli)};
     opts.set_token(token);
-    REQUIRE(c_struct.context == token.get());
+    REQUIRE(copts.context == token.get());
 }
 
 /////////////////////////////////////////////////////////////////////////////
@@ -139,35 +234,37 @@ TEST_CASE("response_options set token", "[options]")
 // Test default constructor
 // ----------------------------------------------------------------------
 
-TEST_CASE("delivery_response_options dflt constructor", "[options]")
+TEST_CASE("delivery_response_options dflt ctor", "[options]")
 {
     delivery_response_options opts;
-    const auto& c_struct = opts.c_struct();
+    const auto& copts = opts.c_struct();
 
-    REQUIRE(c_struct.context == nullptr);
+    REQUIRE(copts.context == nullptr);
 
-    // Make sure the callback functions are set during object construction
+    // Make sure the v3 callback functions are set during object construction
-    REQUIRE(c_struct.onSuccess != nullptr);
+    REQUIRE(copts.onSuccess != nullptr);
-    REQUIRE(c_struct.onFailure != nullptr);
+    REQUIRE(copts.onFailure != nullptr);
+    REQUIRE(copts.onSuccess5 == nullptr);
+    REQUIRE(copts.onFailure5 == nullptr);
 }
 
 // ----------------------------------------------------------------------
 // Test user constructor
 // ----------------------------------------------------------------------
 
-TEST_CASE("delivery_response_options user constructor", "[options]")
+TEST_CASE("delivery_response_options user ctor", "[options]")
 {
-    mock_async_client cli;
+    //    mock_async_client cli;
 
     delivery_token_ptr token{new delivery_token{cli}};
     delivery_response_options opts{token};
-    const auto& c_struct = opts.c_struct();
+    const auto& copts = opts.c_struct();
 
-    REQUIRE(c_struct.context == token.get());
+    REQUIRE(copts.context == token.get());
 
     // Make sure the callback functions are set during object construction
-    REQUIRE(c_struct.onSuccess != nullptr);
+    REQUIRE(copts.onSuccess != nullptr);
-    REQUIRE(c_struct.onFailure != nullptr);
+    REQUIRE(copts.onFailure != nullptr);
 }
 
 // ----------------------------------------------------------------------
@@ -177,12 +274,12 @@ TEST_CASE("delivery_response_options user constructor", "[options]")
 TEST_CASE("delivery_response_options set token", "[options]")
 {
     delivery_response_options opts;
-    const auto& c_struct = opts.c_struct();
+    const auto& copts = opts.c_struct();
 
-    REQUIRE(c_struct.context == nullptr);
+    REQUIRE(copts.context == nullptr);
 
     mock_async_client cli;
     delivery_token_ptr token{new delivery_token{cli}};
     opts.set_token(token);
-    REQUIRE(c_struct.context == token.get());
+    REQUIRE(copts.context == token.get());
 }