This is an automated email from the ASF dual-hosted git repository.
xyz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pulsar-client-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new ef25a62 [Bug Fix][KeySharedPolicy] Fixed bug where
KeySharedPolicy::setStickyRanges duplicated ranges. (#242)
ef25a62 is described below
commit ef25a629394b293f73530d989c93bc3b51ec990c
Author: Tommy <ad...@hyperevo.com>
AuthorDate: Thu Apr 20 04:58:38 2023 -0700
[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
https://github.com/pybind/pybind11/issues/1302#issuecomment-369090893
*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
---
include/pulsar/KeySharedPolicy.h | 6 ++++++
lib/KeySharedPolicy.cc | 12 ++++++++----
tests/KeySharedPolicyTest.cc | 21 +++++++++++++++++++++
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/include/pulsar/KeySharedPolicy.h b/include/pulsar/KeySharedPolicy.h
index 08eee77..ee0a979 100644
--- a/include/pulsar/KeySharedPolicy.h
+++ b/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
*/
diff --git a/lib/KeySharedPolicy.cc b/lib/KeySharedPolicy.cc
index 6c3e36a..a1471de 100644
--- a/lib/KeySharedPolicy.cc
+++ b/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;
}
diff --git a/tests/KeySharedPolicyTest.cc b/tests/KeySharedPolicyTest.cc
index ff43102..d032dd4 100644
--- a/tests/KeySharedPolicyTest.cc
+++ b/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());
+}
