address copilot comments

Author: BewareMyPower

include/pulsar/EncryptionContext.h

@@ -32,15 +32,11 @@ namespace proto {
 class MessageMetadata;
 }
 
-class Message;
-
 struct PULSAR_PUBLIC EncryptionKey {
     std::string key;
     std::string value;
     std::unordered_map<std::string, std::string> metadata;
 
-    explicit EncryptionKey() = default;
-
     EncryptionKey(const std::string& key, const std::string& value,
                   const decltype(EncryptionKey::metadata)& metadata)
         : key(key), value(value), metadata(metadata) {}
@@ -95,8 +91,9 @@ class PULSAR_PUBLIC EncryptionContext {
     bool isDecryptionFailed() const noexcept { return isDecryptionFailed_; }
 
     /**
-     * It should be used only internally but it's exposed so that `std::make_optional` can construct the
-     * object in place with this constructor.
+     * This constructor is public to allow in-place construction via std::optional
+     * (e.g., `std::optional<EncryptionContext>(std::in_place, metadata, false)`),
+     * but should not be used directly in application code.
      */
     EncryptionContext(const proto::MessageMetadata&, bool);
 
@@ -109,6 +106,8 @@ class PULSAR_PUBLIC EncryptionContext {
     int32_t batchSize_{-1};
     bool isDecryptionFailed_{false};
 
+    void setDecryptionFailed(bool failed) noexcept { isDecryptionFailed_ = failed; }
+
     friend class ConsumerImpl;
 };
 

lib/ConsumerImpl.cc

@@ -560,17 +560,17 @@ void ConsumerImpl::messageReceived(const ClientConnectionPtr& cnx, const proto::
                                  ? optional<EncryptionContext>(std::in_place, metadata, false)
                                  : std::nullopt;
     const auto decryptionResult = decryptMessageIfNeeded(cnx, msg, encryptionContext, payload);
-    if (decryptionResult == FAILED) {
+    if (decryptionResult == DecryptionResult::FAILED) {
         // Message was discarded or not consumed due to decryption failure
         return;
-    } else if (decryptionResult == CONSUME_ENCRYPTED && encryptionContext.has_value()) {
+    } else if (decryptionResult == DecryptionResult::CONSUME_ENCRYPTED && encryptionContext.has_value()) {
         // Message is encrypted, but we let the application consume it as-is
-        encryptionContext->isDecryptionFailed_ = true;
+        encryptionContext->setDecryptionFailed(true);
     }
 
     auto redeliveryCount = msg.redelivery_count();
     const bool isChunkedMessage = metadata.num_chunks_from_msg() > 1;
-    if (decryptionResult == SUCCESS && !isChunkedMessage) {
+    if (decryptionResult == DecryptionResult::SUCCESS && !isChunkedMessage) {
         if (!uncompressMessageIfNeeded(cnx, msg.message_id(), metadata, payload, true)) {
             // Message was discarded on decompression error
             return;
@@ -615,7 +615,7 @@ void ConsumerImpl::messageReceived(const ClientConnectionPtr& cnx, const proto::
         return;
     }
 
-    if (metadata.has_num_messages_in_batch() && decryptionResult == SUCCESS) {
+    if (metadata.has_num_messages_in_batch() && decryptionResult == DecryptionResult::SUCCESS) {
         BitSet::Data words(msg.ack_set_size());
         for (int i = 0; i < words.size(); i++) {
             words[i] = msg.ack_set(i);
@@ -821,14 +821,14 @@ auto ConsumerImpl::decryptMessageIfNeeded(const ClientConnectionPtr& cnx, const
                                           const optional<EncryptionContext>& context, SharedBuffer& payload)
     -> DecryptionResult {
     if (!context.has_value()) {
-        return SUCCESS;
+        return DecryptionResult::SUCCESS;
     }
 
     // If KeyReader is not configured throw exception based on config param
     if (!config_.isEncryptionEnabled()) {
         if (config_.getCryptoFailureAction() == ConsumerCryptoFailureAction::CONSUME) {
             LOG_WARN(getName() << "CryptoKeyReader is not implemented. Consuming encrypted message.");
-            return CONSUME_ENCRYPTED;
+            return DecryptionResult::CONSUME_ENCRYPTED;
         } else if (config_.getCryptoFailureAction() == ConsumerCryptoFailureAction::DISCARD) {
             LOG_WARN(getName() << "Skipping decryption since CryptoKeyReader is not implemented and config "
                                   "is set to discard");
@@ -839,20 +839,20 @@ auto ConsumerImpl::decryptMessageIfNeeded(const ClientConnectionPtr& cnx, const
             auto messageId = MessageIdBuilder::from(msg.message_id()).build();
             unAckedMessageTrackerPtr_->add(messageId);
         }
-        return FAILED;
+        return DecryptionResult::FAILED;
     }
 
     SharedBuffer decryptedPayload;
     if (msgCrypto_->decrypt(*context, payload, config_.getCryptoKeyReader(), decryptedPayload)) {
         payload = decryptedPayload;
-        return SUCCESS;
+        return DecryptionResult::SUCCESS;
     }
 
     if (config_.getCryptoFailureAction() == ConsumerCryptoFailureAction::CONSUME) {
         // Note, batch message will fail to consume even if config is set to consume
         LOG_WARN(
             getName() << "Decryption failed. Consuming encrypted message since config is set to consume.");
-        return CONSUME_ENCRYPTED;
+        return DecryptionResult::CONSUME_ENCRYPTED;
     } else if (config_.getCryptoFailureAction() == ConsumerCryptoFailureAction::DISCARD) {
         LOG_WARN(getName() << "Discarding message since decryption failed and config is set to discard");
         discardCorruptedMessage(cnx, msg.message_id(), CommandAck_ValidationError_DecryptionError);
@@ -861,7 +861,7 @@ auto ConsumerImpl::decryptMessageIfNeeded(const ClientConnectionPtr& cnx, const
         auto messageId = MessageIdBuilder::from(msg.message_id()).build();
         unAckedMessageTrackerPtr_->add(messageId);
     }
-    return FAILED;
+    return DecryptionResult::FAILED;
 }
 
 bool ConsumerImpl::uncompressMessageIfNeeded(const ClientConnectionPtr& cnx,

lib/ConsumerImpl.h

@@ -195,7 +195,7 @@ class ConsumerImpl : public ConsumerImplBase {
     bool isPriorEntryIndex(int64_t idx);
     void brokerConsumerStatsListener(Result, BrokerConsumerStatsImpl, const BrokerConsumerStatsCallback&);
 
-    enum DecryptionResult
+    enum class DecryptionResult : uint8_t
     {
         SUCCESS,
         CONSUME_ENCRYPTED,