[Bug Fix][KeySharedPolicy] Fixed bug where KeySharedPolicy::setStickyRanges duplicated ranges. (#242)

* *[fix] Fixed bug where KeySharedPolicy::setStickyRanges appended duplicate ranges to KeySharedPolicy->ranges. The for loop was nested when it shouldn't have been nested.

*Added overloaded KeySharedPolicy::setStickyRanges function that takes the StickyRanges vector. This is convenient in case a developer wants to provide a vector. It is also required for adding KeySharedPolicy to the python pulsar client which uses pybind11. std::initializer_list type conversion is not supported with pybind11. See pybind/pybind11#1302 (comment)

*Added a test that checks to see if the KeySharedPolicy->ranges variable is set as expected after calling KeySharedPolicy::setStickyRanges

*Added a test that checks to see if the KeySharedPolicy->ranges variable is the same when using the two different overloaded KeySharedPolicy::setStickyRanges functions.

* Fixed format. Added depreciated comment. Passed StickRanges as reference. Implicit conversion from std::initializer_list to std::vector

* Removed type

Author: hyperevo

include/pulsar/KeySharedPolicy.h

@@ -95,9 +95,15 @@ class PULSAR_PUBLIC KeySharedPolicy {
 
     /**
      * @param ranges used with sticky mode
+     * @deprecated use the function that takes StickyRanges instead of std::initializer_list
      */
     KeySharedPolicy& setStickyRanges(std::initializer_list<StickyRange> ranges);
 
+    /**
+     * @param ranges used with sticky mode
+     */
+    KeySharedPolicy& setStickyRanges(const StickyRanges& ranges);
+
     /**
      * @return ranges used with sticky mode
      */

lib/KeySharedPolicy.cc

@@ -51,7 +51,11 @@ KeySharedPolicy &KeySharedPolicy::setAllowOutOfOrderDelivery(bool allowOutOfOrde
 bool KeySharedPolicy::isAllowOutOfOrderDelivery() const { return impl_->allowOutOfOrderDelivery; }
 
 KeySharedPolicy &KeySharedPolicy::setStickyRanges(std::initializer_list<StickyRange> ranges) {
-    if (ranges.size() == 0) {
+    return this->setStickyRanges(StickyRanges(ranges));
+}
+
+KeySharedPolicy &KeySharedPolicy::setStickyRanges(const StickyRanges &ranges) {
+    if (ranges.empty()) {
         throw std::invalid_argument("Ranges for KeyShared policy must not be empty.");
     }
     for (StickyRange range : ranges) {
@@ -65,9 +69,9 @@ KeySharedPolicy &KeySharedPolicy::setStickyRanges(std::initializer_list<StickyRa
                 throw std::invalid_argument("Ranges for KeyShared policy with overlap.");
             }
         }
-        for (StickyRange range : ranges) {
-            impl_->ranges.push_back(range);
-        }
+    }
+    for (StickyRange range : ranges) {
+        impl_->ranges.push_back(range);
     }
     return *this;
 }

tests/KeySharedPolicyTest.cc

@@ -201,3 +201,24 @@ TEST_F(KeySharedPolicyTest, InvalidStickyRanges) {
     ASSERT_THROW(ksp.setStickyRanges({StickyRange(0, 65536)}), std::invalid_argument);
     ASSERT_THROW(ksp.setStickyRanges({StickyRange(0, 10), StickyRange(9, 20)}), std::invalid_argument);
 }
+
+TEST_F(KeySharedPolicyTest, testStickyConsumerExpected) {
+    // Test whether the saved range vector is as expected
+    KeySharedPolicy ksp;
+    ksp.setStickyRanges({StickyRange(0, 300), StickyRange(400, 500)});
+
+    std::vector<std::pair<int, int>> expectedStickyRange{{0, 300}, {400, 500}};
+    ASSERT_EQ(ksp.getStickyRanges(), expectedStickyRange);
+}
+
+TEST_F(KeySharedPolicyTest, testStickyConsumerVectors) {
+    // test whether the saved range vector is the same when using initializer_list or vector
+    KeySharedPolicy ksp;
+    ksp.setStickyRanges({StickyRange(0, 300), StickyRange(400, 500)});
+
+    KeySharedPolicy ksp2;
+    std::vector<std::pair<int, int>> stickyRangeVec{{0, 300}, {400, 500}};
+    ksp2.setStickyRanges(stickyRangeVec);
+
+    ASSERT_EQ(ksp.getStickyRanges(), ksp2.getStickyRanges());
+}