From f521fc8f8185c24bd2430ba667b6e16c0da8e6d4 Mon Sep 17 00:00:00 2001 From: Brian Surber Date: Thu, 2 Jan 2025 18:37:54 +0000 Subject: [PATCH] Address nits from PR #37797 Signed-off-by: Brian Surber --- .../filters/http/rate_limit_quota/config.cc | 4 ++-- .../filters/http/rate_limit_quota/filter.cc | 8 ++++---- .../http/rate_limit_quota/global_client_impl.cc | 16 ++++++++-------- .../filters/http/rate_limit_quota/client_test.cc | 2 +- .../http/rate_limit_quota/client_test_utils.h | 2 +- .../http/rate_limit_quota/integration_test.cc | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/source/extensions/filters/http/rate_limit_quota/config.cc b/source/extensions/filters/http/rate_limit_quota/config.cc index 0ef4e6532aa0..eb005e4e5cde 100644 --- a/source/extensions/filters/http/rate_limit_quota/config.cc +++ b/source/extensions/filters/http/rate_limit_quota/config.cc @@ -83,8 +83,8 @@ Http::FilterFactoryCb RateLimitQuotaFilterFactory::createFilterFactoryFromProtoT matcher = matcher_factory.create(config->bucket_matchers())(); } - return [&, config = std::move(config), config_with_hash_key, tls_store, - matcher](Http::FilterChainFactoryCallbacks& callbacks) -> void { + return [&, config = std::move(config), config_with_hash_key, tls_store = std::move(tls_store), + matcher = std::move(matcher)](Http::FilterChainFactoryCallbacks& callbacks) -> void { std::unique_ptr local_client = createLocalRateLimitClient(tls_store->global_client_tls, tls_store->buckets_tls); diff --git a/source/extensions/filters/http/rate_limit_quota/filter.cc b/source/extensions/filters/http/rate_limit_quota/filter.cc index de3141e3a317..d1b70a5abfe7 100644 --- a/source/extensions/filters/http/rate_limit_quota/filter.cc +++ b/source/extensions/filters/http/rate_limit_quota/filter.cc @@ -95,7 +95,7 @@ Http::FilterHeadersStatus RateLimitQuotaFilter::decodeHeaders(Http::RequestHeade callbacks_->streamInfo().setDynamicMetadata(kBucketMetadataNamespace, bucket_log); std::shared_ptr cached_bucket = client_->getBucket(bucket_id); - if (cached_bucket) { + if (cached_bucket != nullptr) { // Found the cached bucket entry. return processCachedBucket(*cached_bucket); } @@ -162,14 +162,14 @@ RateLimitQuotaFilter::requestMatching(const Http::RequestHeaderMap& headers) { // Initialize the data pointer on first use and reuse it for subsequent // requests. This avoids creating the data object for every request, which // is expensive. - if (!data_ptr_) { - if (!callbacks_) { + if (data_ptr_ == nullptr) { + if (callbacks_ == nullptr) { return absl::InternalError("Filter callback has not been initialized successfully yet."); } data_ptr_ = std::make_unique(callbacks_->streamInfo()); } - if (!matcher_) { + if (matcher_ == nullptr) { return absl::InternalError("Matcher tree has not been initialized yet."); } // Populate the request header. diff --git a/source/extensions/filters/http/rate_limit_quota/global_client_impl.cc b/source/extensions/filters/http/rate_limit_quota/global_client_impl.cc index 3b4f78b67a1f..426f4546790f 100644 --- a/source/extensions/filters/http/rate_limit_quota/global_client_impl.cc +++ b/source/extensions/filters/http/rate_limit_quota/global_client_impl.cc @@ -203,7 +203,7 @@ void GlobalRateLimitClientImpl::createBucketImpl(const BucketId& bucket_id, size sendUsageReportImpl(initial_report); writeBucketsToTLS(); - if (callbacks_) { + if (callbacks_ != nullptr) { callbacks_->onBucketCreated(buckets_cache_[id]->bucket_id, id); } } @@ -257,7 +257,7 @@ createTokenBucketFromAction(const RateLimitStrategy& strategy, TimeSource& time_ } void GlobalRateLimitClientImpl::onReceiveMessage(RateLimitQuotaResponsePtr&& response) { - if (!response) { + if (response == nullptr) { return; } main_dispatcher_.post( @@ -359,7 +359,7 @@ void GlobalRateLimitClientImpl::onQuotaResponseImpl(const RateLimitQuotaResponse } // Push updates to TLS. writeBucketsToTLS(); - if (callbacks_) { + if (callbacks_ != nullptr) { callbacks_->onQuotaResponseProcessed(); } } @@ -388,13 +388,13 @@ bool GlobalRateLimitClientImpl::startStreamImpl() { } void GlobalRateLimitClientImpl::startSendReportsTimerImpl() { - if (send_reports_timer_) { + if (send_reports_timer_ != nullptr) { return; } ENVOY_LOG(debug, "Start the usage reporting timer for the RLQS stream."); send_reports_timer_ = main_dispatcher_.createTimer([&]() { onSendReportsTimer(); - if (callbacks_) { + if (callbacks_ != nullptr) { callbacks_->onUsageReportsSent(); } send_reports_timer_->enableTimer(send_reports_interval_); @@ -420,7 +420,7 @@ void GlobalRateLimitClientImpl::startActionExpirationTimer(CachedBucket* cached_ // Pointer safety as all writes are against the source-of-truth. cached_bucket->action_expiration_timer = main_dispatcher_.createTimer([&, id, cached_bucket]() { onActionExpirationTimer(cached_bucket, id); - if (callbacks_) { + if (callbacks_ != nullptr) { callbacks_->onActionExpiration(); } }); @@ -442,7 +442,7 @@ void GlobalRateLimitClientImpl::onActionExpirationTimer(CachedBucket* bucket, si // Without a fallback action, the cached action will be deleted and the bucket // will revert to its default action. - if (!cached_bucket->fallback_action) { + if (cached_bucket->fallback_action == nullptr) { ENVOY_LOG(debug, "No fallback action is configured for bucket id {}, reverting to " "its default action.", @@ -509,7 +509,7 @@ void GlobalRateLimitClientImpl::startFallbackExpirationTimer(CachedBucket* cache // Pointer safety as all writes are against the source-of-truth. cached_bucket->fallback_expiration_timer = main_dispatcher_.createTimer([&, id, cached_bucket]() { onFallbackExpirationTimer(cached_bucket, id); - if (callbacks_) { + if (callbacks_ != nullptr) { callbacks_->onFallbackExpiration(); } }); diff --git a/test/extensions/filters/http/rate_limit_quota/client_test.cc b/test/extensions/filters/http/rate_limit_quota/client_test.cc index bfb54df48a4f..8f01704f7373 100644 --- a/test/extensions/filters/http/rate_limit_quota/client_test.cc +++ b/test/extensions/filters/http/rate_limit_quota/client_test.cc @@ -242,7 +242,7 @@ void setAtomic(uint64_t value, std::atomic& counter) { absl::StatusOr> tryGetBucket(ThreadLocal::TypedSlot& buckets_tls, size_t id) { auto cache_ref = buckets_tls.get(); - if (!cache_ref.has_value() || !cache_ref->quota_buckets_) + if (!cache_ref.has_value() || cache_ref->quota_buckets_ == nullptr) return absl::NotFoundError("Bucket TLS not initialized"); auto bucket_it = cache_ref->quota_buckets_->find(id); diff --git a/test/extensions/filters/http/rate_limit_quota/client_test_utils.h b/test/extensions/filters/http/rate_limit_quota/client_test_utils.h index 75e1bc3c001b..c03c7c340e3a 100644 --- a/test/extensions/filters/http/rate_limit_quota/client_test_utils.h +++ b/test/extensions/filters/http/rate_limit_quota/client_test_utils.h @@ -119,7 +119,7 @@ class RateLimitTestClient { } inline static Event::MockTimer* assertMockTimer(Event::Timer* timer) { - if (!timer) + if (timer == nullptr) return nullptr; return dynamic_cast(timer); } diff --git a/test/extensions/filters/http/rate_limit_quota/integration_test.cc b/test/extensions/filters/http/rate_limit_quota/integration_test.cc index c68f28b62a8d..e6fda8491ba0 100644 --- a/test/extensions/filters/http/rate_limit_quota/integration_test.cc +++ b/test/extensions/filters/http/rate_limit_quota/integration_test.cc @@ -196,7 +196,7 @@ class RateLimitQuotaIntegrationTest : public Event::TestUsingSimulatedTime, } void cleanUp() { - if (rlqs_connection_) { + if (rlqs_connection_ != nullptr) { ASSERT_TRUE(rlqs_connection_->close()); ASSERT_TRUE(rlqs_connection_->waitForDisconnect()); }