Skip to content

Commit

Permalink
Fix review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Aleksandr Ivanov <[email protected]>
  • Loading branch information
alexander-e1off committed Nov 19, 2024
1 parent 0455cee commit 801619c
Show file tree
Hide file tree
Showing 11 changed files with 296 additions and 252 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace m_bmqstoragetool {
CompositeSequenceNumber::CompositeSequenceNumber()
: d_leaseId(0)
, d_seqNumber(0)
, d_isUnset(true)
, d_isSet(false)
{
// NOTHING
}
Expand All @@ -42,14 +42,14 @@ CompositeSequenceNumber::CompositeSequenceNumber(
, d_seqNumber(sequenceNumber)
{
BSLS_ASSERT(d_leaseId > 0 && d_seqNumber > 0);
d_isUnset = !(d_leaseId > 0 && d_seqNumber > 0);
d_isSet = d_leaseId > 0 && d_seqNumber > 0;
}

CompositeSequenceNumber&
CompositeSequenceNumber::fromString(bsl::ostream& errorDescription,
const bsl::string& seqNumString)
{
d_isUnset = true;
d_isSet = false;

if (seqNumString.empty()) {
errorDescription << "Invalid input: empty string.";
Expand Down Expand Up @@ -88,7 +88,7 @@ CompositeSequenceNumber::fromString(bsl::ostream& errorDescription,
return *this; // RETURN
}

d_isUnset = false;
d_isSet = true;
}
catch (const bsl::invalid_argument& e) {
errorDescription << "Invalid input: non-numeric values encountered.";
Expand All @@ -110,13 +110,13 @@ bsl::ostream& CompositeSequenceNumber::print(bsl::ostream& stream,

bdlb::Print::indent(stream, level, spacesPerLevel);

if (isUnset()) {
stream << "** UNSET **";
}
else {
if (isSet()) {
stream << "leaseId: " << leaseId()
<< ", sequenceNumber: " << sequenceNumber();
}
else {
stream << "** UNSET **";
}

if (spacesPerLevel >= 0) {
stream << '\n';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ class CompositeSequenceNumber {
// Primary Lease Id
bsls::Types::Uint64 d_seqNumber;
// Sequence Number
bool d_isUnset;
// Set to `true` if the value of this object is not set
bool d_isSet;
// Set to `true` if the value of this object is set

public:
// CREATORS

/// Create an un-initialized CompositeSequenceNumber. Note that
/// `isUnset()` would return true.
/// `isSet()` would return false.
CompositeSequenceNumber();

/// Create CompositeSequenceNumber from the specified `leaseId` and
Expand All @@ -69,16 +69,16 @@ class CompositeSequenceNumber {
/// Initialize this CompositeSequenceNumber from the specified
/// `seqNumString` representation in format `<leaseId>-<sequenceNumber>`.
/// Return a reference offering modifiable access to this object. If
/// convertion is successfull, `isUnset()` would return `true`. Otherwise,
/// `isUnset()` would return `false` and specified `errorDescription` is
/// convertion is successfull, `isSet()` would return `true`. Otherwise,
/// `isSet()` would return `false` and specified `errorDescription` is
/// filled with error description.
CompositeSequenceNumber& fromString(bsl::ostream& errorDescription,
const bsl::string& seqNumString);

// ACCESSORS

/// Return `true` if the value of this object is not set.
bool isUnset() const;
bool isSet() const;

/// Return Primary Lease Id value.
unsigned int leaseId() const;
Expand Down Expand Up @@ -139,9 +139,9 @@ bool operator<=(const CompositeSequenceNumber& lhs,

// ACCESSORS

inline bool CompositeSequenceNumber::isUnset() const
inline bool CompositeSequenceNumber::isSet() const
{
return d_isUnset;
return d_isSet;
}

inline unsigned int CompositeSequenceNumber::leaseId() const
Expand All @@ -167,7 +167,7 @@ inline bsl::ostream& m_bmqstoragetool::operator<<(
const m_bmqstoragetool::CompositeSequenceNumber& rhs)
{
// PRECONDITIONS
BSLS_ASSERT(!rhs.isUnset());
BSLS_ASSERT(rhs.isSet());

return rhs.print(stream, 0, -1);
}
Expand All @@ -177,7 +177,7 @@ inline bool m_bmqstoragetool::operator==(
const m_bmqstoragetool::CompositeSequenceNumber& rhs)
{
// PRECONDITIONS
BSLS_ASSERT(!lhs.isUnset() && !rhs.isUnset());
BSLS_ASSERT(lhs.isSet() && rhs.isSet());

return (lhs.leaseId() == rhs.leaseId() &&
lhs.sequenceNumber() == rhs.sequenceNumber());
Expand All @@ -188,7 +188,7 @@ inline bool m_bmqstoragetool::operator<(
const m_bmqstoragetool::CompositeSequenceNumber& rhs)
{
// PRECONDITIONS
BSLS_ASSERT(!lhs.isUnset() && !rhs.isUnset());
BSLS_ASSERT(lhs.isSet() && rhs.isSet());

// Check leaseId first
if (lhs.leaseId() < rhs.leaseId()) {
Expand All @@ -208,7 +208,7 @@ inline bool m_bmqstoragetool::operator<=(
const m_bmqstoragetool::CompositeSequenceNumber& rhs)
{
// PRECONDITIONS
BSLS_ASSERT(!lhs.isUnset() && !rhs.isUnset());
BSLS_ASSERT(lhs.isSet() && rhs.isSet());

return (lhs < rhs || lhs == rhs);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ static void test1_breathingTest()

{
CompositeSequenceNumber compositeSeqNum;
ASSERT(compositeSeqNum.isUnset());
ASSERT(!compositeSeqNum.isSet());
}

{
CompositeSequenceNumber compositeSeqNum(1, 2);
ASSERT(!compositeSeqNum.isUnset());
ASSERT(compositeSeqNum.isSet());
ASSERT_EQ(compositeSeqNum.leaseId(), 1ul);
ASSERT_EQ(compositeSeqNum.sequenceNumber(), 2ul);
}
Expand Down Expand Up @@ -80,7 +80,7 @@ static void test2_fromStringTest()
bsl::string inputString("123-456", s_allocator_p);

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), false);
ASSERT(compositeSeqNum.isSet());
ASSERT_EQ(compositeSeqNum.leaseId(), 123u);
ASSERT_EQ(compositeSeqNum.sequenceNumber(), 456u);
ASSERT(errorDescription.str().empty());
Expand All @@ -93,7 +93,7 @@ static void test2_fromStringTest()
bsl::string inputString("00123-000456", s_allocator_p);

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), false);
ASSERT(compositeSeqNum.isSet());
ASSERT_EQ(compositeSeqNum.leaseId(), 123u);
ASSERT_EQ(compositeSeqNum.sequenceNumber(), 456u);
ASSERT(errorDescription.str().empty());
Expand All @@ -106,7 +106,7 @@ static void test2_fromStringTest()
bsl::string inputString("", s_allocator_p);

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(), "Invalid input: empty string.");
}
Expand All @@ -119,7 +119,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid format: no '-' separator found.");
Expand All @@ -133,7 +133,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid format: no '-' separator found.");
Expand All @@ -147,7 +147,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid input: non-numeric values encountered.");
Expand All @@ -161,7 +161,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid input: non-numeric values encountered.");
Expand All @@ -175,7 +175,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid input: zero values encountered.");
Expand All @@ -189,7 +189,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid input: zero values encountered.");
Expand All @@ -204,7 +204,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid input: number out of range.");
Expand All @@ -219,7 +219,7 @@ static void test2_fromStringTest()
errorDescription.reset();

compositeSeqNum.fromString(errorDescription, inputString);
ASSERT_EQ(compositeSeqNum.isUnset(), true);
ASSERT_EQ(compositeSeqNum.isSet(), false);
ASSERT(!errorDescription.str().empty());
ASSERT_EQ(errorDescription.str(),
"Invalid input: number out of range.");
Expand Down
51 changes: 25 additions & 26 deletions src/applications/bmqstoragetool/m_bmqstoragetool_filters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,13 @@ namespace m_bmqstoragetool {
// class Filters
// =============

Filters::Filters(const bsl::vector<bsl::string>& queueKeys,
const bsl::vector<bsl::string>& queueUris,
const QueueMap& queueMap,
const Parameters::SearchValueType valueType,
const bsls::Types::Uint64 valueGt,
const bsls::Types::Uint64 valueLt,
const CompositeSequenceNumber seqNumGt,
const CompositeSequenceNumber seqNumLt,
bslma::Allocator* allocator)
Filters::Filters(const bsl::vector<bsl::string>& queueKeys,
const bsl::vector<bsl::string>& queueUris,
const QueueMap& queueMap,
const Parameters::Range& range,
bslma::Allocator* allocator)
: d_queueKeys(allocator)
, d_valueType(valueType)
, d_valueGt(valueGt)
, d_valueLt(valueLt)
, d_seqNumGt(seqNumGt)
, d_seqNumLt(seqNumLt)
, d_range(range)
{
// Fill internal structures
if (!queueKeys.empty()) {
Expand Down Expand Up @@ -76,25 +68,32 @@ bool Filters::apply(const mqbs::MessageRecord& record,
}

// Apply `range` filter
bsls::Types::Uint64 value;
switch (d_valueType) {
case Parameters::e_TIMESTAMP: value = record.header().timestamp(); break;
case Parameters::e_SEQUENCE_NUM: {
bsls::Types::Uint64 value, valueGt, valueLt;
switch (d_range.d_type) {
case Parameters::Range::e_TIMESTAMP:
value = record.header().timestamp();
valueGt = d_range.d_timestampGt;
valueLt = d_range.d_timestampLt;
break;
case Parameters::Range::e_OFFSET:
value = offset;
valueGt = d_range.d_offsetGt;
valueLt = d_range.d_offsetLt;
break;
case Parameters::Range::e_SEQUENCE_NUM: {
CompositeSequenceNumber seqNum(record.header().primaryLeaseId(),
record.header().sequenceNumber());
if ((!d_seqNumGt.isUnset() && seqNum <= d_seqNumGt) ||
(!d_seqNumLt.isUnset() && d_seqNumLt <= seqNum)) {
// Not inside range
return false; // RETURN
}
return !(
(d_range.d_seqNumGt.isSet() && seqNum <= d_range.d_seqNumGt) ||
(d_range.d_seqNumLt.isSet() &&
d_range.d_seqNumLt <= seqNum)); // RETURN
} break;
case Parameters::e_OFFSET: value = offset; break;
default:
// No range filter defined
return true; // RETURN
}
if ((d_valueGt > 0 && value <= d_valueGt) ||
(d_valueLt > 0 && value >= d_valueLt)) {
if ((valueGt > 0 && value <= valueGt) ||
(valueLt > 0 && value >= valueLt)) {
// Not inside range
return false; // RETURN
}
Expand Down
20 changes: 6 additions & 14 deletions src/applications/bmqstoragetool/m_bmqstoragetool_filters.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,17 @@ class Filters {
private:
// DATA
bsl::unordered_set<mqbu::StorageKey> d_queueKeys;
const Parameters::SearchValueType d_valueType;
const bsls::Types::Uint64 d_valueGt;
const bsls::Types::Uint64 d_valueLt;
const CompositeSequenceNumber d_seqNumGt;
const CompositeSequenceNumber d_seqNumLt;
const Parameters::Range d_range;

public:
// CREATORS

/// Constructor using the specified arguments.
explicit Filters(const bsl::vector<bsl::string>& queueKeys,
const bsl::vector<bsl::string>& queueUris,
const QueueMap& queueMap,
const Parameters::SearchValueType valueType,
const bsls::Types::Uint64 valueGt,
const bsls::Types::Uint64 valueLt,
const CompositeSequenceNumber seqNumGt,
const CompositeSequenceNumber seqNumLt,
bslma::Allocator* allocator);
explicit Filters(const bsl::vector<bsl::string>& queueKeys,
const bsl::vector<bsl::string>& queueUris,
const QueueMap& queueMap,
const Parameters::Range& range,
bslma::Allocator* allocator);

// ACCESSORS

Expand Down
Loading

0 comments on commit 801619c

Please sign in to comment.