From efc9a14a2b5ec0c9c6032af382a186be98ccd94f Mon Sep 17 00:00:00 2001 From: Elad Alon Date: Fri, 8 Feb 2019 23:35:59 +0100 Subject: [PATCH] Make UniqueNumberGenerator::AddKnownId() return a value Make AddKnownId() return a value to indicate whether the ID was known before, or has only been made known now. This allows users of the class to RTC_DCHECK that no collisions existed in their seed set, for instance. This change is done for the following classes: 1. UniqueNumberGenerator 2. UniqueRandomIdGenerator 3. UniqueStringGenerator Bug: None Change-Id: I627d2821cb76aa333075e36575088d76dbeb3665 Reviewed-on: https://webrtc-review.googlesource.com/c/121780 Commit-Queue: Elad Alon Reviewed-by: Amit Hilbuch Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#26621} --- rtc_base/unique_id_generator.cc | 11 ++++--- rtc_base/unique_id_generator.h | 13 +++++--- rtc_base/unique_id_generator_unittest.cc | 40 ++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/rtc_base/unique_id_generator.cc b/rtc_base/unique_id_generator.cc index 62cd068f11..d41fa8d186 100644 --- a/rtc_base/unique_id_generator.cc +++ b/rtc_base/unique_id_generator.cc @@ -26,8 +26,8 @@ UniqueRandomIdGenerator::UniqueRandomIdGenerator(ArrayView known_ids) UniqueRandomIdGenerator::~UniqueRandomIdGenerator() = default; uint32_t UniqueRandomIdGenerator::GenerateId() { + RTC_CHECK_LT(known_ids_.size(), std::numeric_limits::max() - 1); while (true) { - RTC_CHECK_LT(known_ids_.size(), std::numeric_limits::max()); auto pair = known_ids_.insert(CreateRandomNonZeroId()); if (pair.second) { return *pair.first; @@ -35,8 +35,8 @@ uint32_t UniqueRandomIdGenerator::GenerateId() { } } -void UniqueRandomIdGenerator::AddKnownId(uint32_t value) { - known_ids_.insert(value); +bool UniqueRandomIdGenerator::AddKnownId(uint32_t value) { + return known_ids_.insert(value).second; } UniqueStringGenerator::UniqueStringGenerator() : unique_number_generator_() {} @@ -52,13 +52,14 @@ std::string UniqueStringGenerator::GenerateString() { return ToString(unique_number_generator_.GenerateNumber()); } -void UniqueStringGenerator::AddKnownId(const std::string& value) { +bool UniqueStringGenerator::AddKnownId(const std::string& value) { absl::optional int_value = StringToNumber(value); // The underlying generator works for uint32_t values, so if the provided // value is not a uint32_t it will never be generated anyway. if (int_value.has_value()) { - unique_number_generator_.AddKnownId(int_value.value()); + return unique_number_generator_.AddKnownId(int_value.value()); } + return false; } } // namespace rtc diff --git a/rtc_base/unique_id_generator.h b/rtc_base/unique_id_generator.h index 849daa9758..836dc70b61 100644 --- a/rtc_base/unique_id_generator.h +++ b/rtc_base/unique_id_generator.h @@ -43,7 +43,8 @@ class UniqueNumberGenerator { TIntegral operator()() { return GenerateNumber(); } // Adds an id that this generator should no longer generate. - void AddKnownId(TIntegral value); + // Return value indicates whether the ID was hitherto unknown. + bool AddKnownId(TIntegral value); private: static_assert(std::is_integral::value, "Must be integral type."); @@ -71,7 +72,8 @@ class UniqueRandomIdGenerator { uint32_t operator()() { return GenerateId(); } // Adds an id that this generator should no longer generate. - void AddKnownId(uint32_t value); + // Return value indicates whether the ID was hitherto unknown. + bool AddKnownId(uint32_t value); private: std::set known_ids_; @@ -93,7 +95,8 @@ class UniqueStringGenerator { std::string operator()() { return GenerateString(); } // Adds an id that this generator should no longer generate. - void AddKnownId(const std::string& value); + // Return value indicates whether the ID was hitherto unknown. + bool AddKnownId(const std::string& value); private: // This implementation will be simple and will generate "0", "1", ... @@ -123,8 +126,8 @@ TIntegral UniqueNumberGenerator::GenerateNumber() { } template -void UniqueNumberGenerator::AddKnownId(TIntegral value) { - known_ids_.insert(value); +bool UniqueNumberGenerator::AddKnownId(TIntegral value) { + return known_ids_.insert(value).second; } } // namespace rtc diff --git a/rtc_base/unique_id_generator_unittest.cc b/rtc_base/unique_id_generator_unittest.cc index 1634d56148..7ce192c05e 100644 --- a/rtc_base/unique_id_generator_unittest.cc +++ b/rtc_base/unique_id_generator_unittest.cc @@ -29,6 +29,7 @@ class UniqueIdGeneratorTest : public Test {}; using test_types = ::testing::Types, UniqueNumberGenerator, UniqueNumberGenerator, + UniqueNumberGenerator, UniqueRandomIdGenerator, UniqueStringGenerator>; @@ -107,4 +108,43 @@ TYPED_TEST(UniqueIdGeneratorTest, AddedElementsAreNotGenerated) { EXPECT_THAT(intersection, IsEmpty()); } +TYPED_TEST(UniqueIdGeneratorTest, AddKnownIdOnNewIdReturnsTrue) { + typedef TypeParam Generator; + + rtc::InitRandom(0); + Generator generator1; + const typename Generator::value_type id = generator1(); + + rtc::InitRandom(0); + Generator generator2; + EXPECT_TRUE(generator2.AddKnownId(id)); +} + +TYPED_TEST(UniqueIdGeneratorTest, AddKnownIdCalledAgainForSameIdReturnsFalse) { + typedef TypeParam Generator; + + rtc::InitRandom(0); + Generator generator1; + const typename Generator::value_type id = generator1(); + + rtc::InitRandom(0); + Generator generator2; + ASSERT_TRUE(generator2.AddKnownId(id)); + EXPECT_FALSE(generator2.AddKnownId(id)); +} + +TYPED_TEST(UniqueIdGeneratorTest, + AddKnownIdOnIdProvidedAsKnownToCtorReturnsFalse) { + typedef TypeParam Generator; + + rtc::InitRandom(0); + Generator generator1; + const typename Generator::value_type id = generator1(); + std::vector known_values = {id}; + + rtc::InitRandom(0); + Generator generator2(known_values); + EXPECT_FALSE(generator2.AddKnownId(id)); +} + } // namespace rtc