rlqs: address nits from PR envoyproxy#37797 (envoyproxy#37851)

Commit Message:
- Nit 1: Lambda capture for the RLQS filter factory callback should move
shared_ptrs instead of copying. This only saves a shared_ptr copy op per
var but is still good practice.
- Nit 2: Replace instances of if-conditions `(!ptr)` / `(ptr)` with
`(ptr == nullptr)` / `(ptr != nullptr)` to conform to existing code.

Risk Level: minimal
Testing: unit testing
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes commit envoyproxy#37797

Signed-off-by: Brian Surber <[email protected]>

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<RateLimitClient> local_client =
         createLocalRateLimitClient(tls_store->global_client_tls, tls_store->buckets_tls);
 

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<CachedBucket> 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<Http::Matching::HttpMatchingDataImpl>(callbacks_->streamInfo());
   }
 
-  if (!matcher_) {
+  if (matcher_ == nullptr) {
     return absl::InternalError("Matcher tree has not been initialized yet.");
   }
   // Populate the request header.

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();
     }
   });

test/extensions/filters/http/rate_limit_quota/client_test.cc

@@ -242,7 +242,7 @@ void setAtomic(uint64_t value, std::atomic<uint64_t>& counter) {
 absl::StatusOr<std::shared_ptr<CachedBucket>>
 tryGetBucket(ThreadLocal::TypedSlot<ThreadLocalBucketsCache>& 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);

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<Event::MockTimer*>(timer);
   }

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());
     }