Internal Refactor.

PiperOrigin-RevId: 609768025

connections/implementation/BUILD

@@ -144,6 +144,8 @@ cc_library(
         "@com_google_absl//absl/functional:any_invocable",
         "@com_google_absl//absl/functional:bind_front",
         "@com_google_absl//absl/log:check",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/time",
@@ -255,6 +257,7 @@ cc_test(
         "@com_github_protobuf_matchers//protobuf-matchers",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/container:flat_hash_set",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/synchronization",

connections/implementation/ble_advertisement.cc

@@ -16,9 +16,15 @@
 
 #include <inttypes.h>
 
+#include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/escaping.h"
+#include "absl/strings/str_cat.h"
 #include "connections/implementation/base_pcp_handler.h"
+#include "connections/implementation/pcp.h"
 #include "internal/platform/base_input_stream.h"
+#include "internal/platform/bluetooth_utils.h"
+#include "internal/platform/byte_array.h"
 #include "internal/platform/logging.h"
 
 namespace nearby {
@@ -87,26 +93,22 @@ void BleAdvertisement::DoInitialize(bool fast_advertisement, Version version,
   }
 }
 
-BleAdvertisement::BleAdvertisement(bool fast_advertisement,
-                                   const ByteArray& ble_advertisement_bytes) {
-  fast_advertisement_ = fast_advertisement;
-
+absl::StatusOr<BleAdvertisement> BleAdvertisement::CreateBleAdvertisement(
+    bool fast_advertisement, const ByteArray& ble_advertisement_bytes) {
   if (ble_advertisement_bytes.Empty()) {
-    NEARBY_LOG(ERROR,
-               "Cannot deserialize BleAdvertisement: null bytes passed in.");
-    return;
+    return absl::InvalidArgumentError(
+        "Cannot deserialize BleAdvertisement: null bytes passed in.");
   }
 
-  int min_advertisement_length = fast_advertisement_
+  int min_advertisement_length = fast_advertisement
                                      ? kMinFastAdvertisementLength
                                      : kMinAdvertisementLength;
 
   if (ble_advertisement_bytes.size() < min_advertisement_length) {
-    NEARBY_LOG(ERROR,
-               "Cannot deserialize BleAdvertisement: expecting min %d raw "
-               "bytes, got %" PRIu64,
-               kMinAdvertisementLength, ble_advertisement_bytes.size());
-    return;
+    return absl::InvalidArgumentError(
+        absl::StrCat("Cannot deserialize BleAdvertisement: expecting min ",
+                     min_advertisement_length, " raw bytes, got ",
+                     ble_advertisement_bytes.size()));
   }
 
   ByteArray advertisement_bytes{ble_advertisement_bytes};
@@ -114,98 +116,105 @@ BleAdvertisement::BleAdvertisement(bool fast_advertisement,
   // The first 1 byte is supposed to be the version and pcp.
   auto version_and_pcp_byte = static_cast<char>(base_input_stream.ReadUint8());
   // The upper 3 bits are supposed to be the version.
-  version_ =
+  Version version =
       static_cast<Version>((version_and_pcp_byte & kVersionBitmask) >> 5);
-  if (version_ != Version::kV1) {
-    NEARBY_LOG(INFO,
-               "Cannot deserialize BleAdvertisement: unsupported Version %d",
-               version_);
-    return;
+  if (version != Version::kV1) {
+    return absl::InvalidArgumentError(absl::StrCat(
+        "Cannot deserialize BleAdvertisement: unsupported Version: ", version));
   }
+
   // The lower 5 bits are supposed to be the Pcp.
-  pcp_ = static_cast<Pcp>(version_and_pcp_byte & kPcpBitmask);
-  switch (pcp_) {
+  Pcp pcp = static_cast<Pcp>(version_and_pcp_byte & kPcpBitmask);
+  switch (pcp) {
     case Pcp::kP2pCluster:  // Fall through
     case Pcp::kP2pStar:     // Fall through
     case Pcp::kP2pPointToPoint:
       break;
     default:
-      NEARBY_LOG(INFO,
-                 "Cannot deserialize BleAdvertisement: unsupported V1 PCP %d",
-                 pcp_);
+      return absl::InvalidArgumentError(absl::StrCat(
+          "Cannot deserialize BleAdvertisement: unsupported V1 PCP ", pcp));
   }
 
   // The next 3 bytes are supposed to be the service_id_hash if not fast
   // advertisement.
-  if (!fast_advertisement_)
-    service_id_hash_ = base_input_stream.ReadBytes(kServiceIdHashLength);
+  ByteArray service_id_hash;
+  if (!fast_advertisement) {
+    service_id_hash = base_input_stream.ReadBytes(kServiceIdHashLength);
+  }
 
   // The next 4 bytes are supposed to be the endpoint_id.
-  endpoint_id_ = std::string{base_input_stream.ReadBytes(kEndpointIdLength)};
+  std::string endpoint_id =
+      std::string{base_input_stream.ReadBytes(kEndpointIdLength)};
 
   // The next 1 byte is supposed to be the length of the endpoint_info.
-  std::uint32_t expected_endpoint_info_length = base_input_stream.ReadUint8();
+  auto expected_endpoint_info_length = base_input_stream.ReadUint8();
 
   // The next x bytes are the endpoint info. (Max length is 131 bytes or 17
   // bytes as fast_advertisement being true).
-  endpoint_info_ = base_input_stream.ReadBytes(expected_endpoint_info_length);
+  auto endpoint_info =
+      base_input_stream.ReadBytes(expected_endpoint_info_length);
   const int max_endpoint_info_length =
-      fast_advertisement_ ? kMaxFastEndpointInfoLength : kMaxEndpointInfoLength;
-  if (endpoint_info_.Empty() ||
-      endpoint_info_.size() != expected_endpoint_info_length ||
-      endpoint_info_.size() > max_endpoint_info_length) {
-    NEARBY_LOG(INFO,
-               "Cannot deserialize BleAdvertisement(fast advertisement=%d): "
-               "expected endpointInfo to be %d bytes, got %" PRIu64,
-               fast_advertisement_, expected_endpoint_info_length,
-               endpoint_info_.size());
-
-    // Clear endpoint_id for validity.
-    endpoint_id_.clear();
-    return;
+      fast_advertisement ? kMaxFastEndpointInfoLength : kMaxEndpointInfoLength;
+  if (endpoint_info.Empty() ||
+      endpoint_info.size() != expected_endpoint_info_length ||
+      endpoint_info.size() > max_endpoint_info_length) {
+    return absl::InvalidArgumentError(absl::StrCat(
+        "Cannot deserialize BleAdvertisement(fast advertisement=",
+        fast_advertisement, "): expected endpointInfo to be ",
+        expected_endpoint_info_length, " bytes, got ", endpoint_info.size()));
   }
 
   // The next 6 bytes are the bluetooth mac address if not fast advertisement.
-  if (!fast_advertisement_) {
+  std::string bluetooth_mac_address;
+  if (!fast_advertisement) {
     auto bluetooth_mac_address_bytes =
         base_input_stream.ReadBytes(BluetoothUtils::kBluetoothMacAddressLength);
-    bluetooth_mac_address_ =
+    bluetooth_mac_address =
         BluetoothUtils::ToString(bluetooth_mac_address_bytes);
   }
 
   // The next 1 byte is supposed to be the length of the uwb_address. If the
   // next byte is not available then it should be a fast advertisement and skip
   // it for remaining bytes.
+  ByteArray uwb_address;
+  BleAdvertisement ble_advertisement;
   if (base_input_stream.IsAvailable(1)) {
-    std::uint32_t expected_uwb_address_length = base_input_stream.ReadUint8();
+    auto expected_uwb_address_length = base_input_stream.ReadUint8();
     // If the length of uwb_address is not zero, then retrieve it.
     if (expected_uwb_address_length != 0) {
-      uwb_address_ = base_input_stream.ReadBytes(expected_uwb_address_length);
-      if (uwb_address_.Empty() ||
-          uwb_address_.size() != expected_uwb_address_length) {
-        NEARBY_LOG(INFO,
-                   "Cannot deserialize BleAdvertisement: "
-                   "expected uwbAddress size to be %d bytes, got %" PRIu64,
-                   expected_uwb_address_length, uwb_address_.size());
-
-        // Clear endpoint_id for validity.
-        endpoint_id_.clear();
-        return;
+      uwb_address = base_input_stream.ReadBytes(expected_uwb_address_length);
+      if (uwb_address.Empty() ||
+          uwb_address.size() != expected_uwb_address_length) {
+        return absl::InvalidArgumentError(absl::StrCat(
+            "Cannot deserialize BleAdvertisement: expected uwbAddress size to "
+            "be ",
+            expected_uwb_address_length, " bytes, got ", uwb_address.size()));
       }
     }
 
     // The next 1 byte is extra field.
-    if (!fast_advertisement_) {
+    if (!fast_advertisement) {
       if (base_input_stream.IsAvailable(kExtraFieldLength)) {
         auto extra_field = static_cast<char>(base_input_stream.ReadUint8());
-        web_rtc_state_ = (extra_field & kWebRtcConnectableFlagBitmask) == 1
-                             ? WebRtcState::kConnectable
-                             : WebRtcState::kUnconnectable;
+        ble_advertisement.web_rtc_state_ =
+            (extra_field & kWebRtcConnectableFlagBitmask) == 1
+                ? WebRtcState::kConnectable
+                : WebRtcState::kUnconnectable;
       }
     }
   }
 
   base_input_stream.Close();
+
+  ble_advertisement.fast_advertisement_ = fast_advertisement;
+  ble_advertisement.version_ = version;
+  ble_advertisement.pcp_ = pcp;
+  ble_advertisement.service_id_hash_ = service_id_hash;
+  ble_advertisement.endpoint_id_ = endpoint_id;
+  ble_advertisement.endpoint_info_ = endpoint_info;
+  ble_advertisement.bluetooth_mac_address_ = bluetooth_mac_address;
+  ble_advertisement.uwb_address_ = uwb_address;
+  return ble_advertisement;
 }
 
 BleAdvertisement::operator ByteArray() const {

connections/implementation/ble_advertisement.h

@@ -15,6 +15,7 @@
 #ifndef CORE_INTERNAL_BLE_ADVERTISEMENT_H_
 #define CORE_INTERNAL_BLE_ADVERTISEMENT_H_
 
+#include "absl/status/statusor.h"
 #include "connections/implementation/base_pcp_handler.h"
 #include "connections/implementation/pcp.h"
 #include "internal/platform/bluetooth_utils.h"
@@ -77,8 +78,8 @@ class BleAdvertisement {
                    const ByteArray& endpoint_info,
                    const std::string& bluetooth_mac_address,
                    const ByteArray& uwb_address, WebRtcState web_rtc_state);
-  BleAdvertisement(bool fast_advertisement,
-                   const ByteArray& ble_advertisement_bytes);
+  static absl::StatusOr<BleAdvertisement> CreateBleAdvertisement(
+      bool fast_advertisement, const ByteArray& ble_advertisement_bytes);
   BleAdvertisement(const BleAdvertisement&) = default;
   BleAdvertisement& operator=(const BleAdvertisement&) = default;
   BleAdvertisement(BleAdvertisement&&) = default;

connections/implementation/ble_advertisement_test.cc

@@ -14,8 +14,12 @@
 
 #include "connections/implementation/ble_advertisement.h"
 
+#include "gmock/gmock.h"
+#include "protobuf-matchers/protocol-buffer-matchers.h"
 #include "gtest/gtest.h"
+#include "absl/status/status.h"
 #include "connections/implementation/base_pcp_handler.h"
+#include "internal/platform/byte_array.h"
 
 namespace nearby {
 namespace connections {
@@ -31,6 +35,9 @@ constexpr absl::string_view kFastAdvertisementEndpointName{"Fast Advertise"};
 constexpr absl::string_view kBluetoothMacAddress{"00:00:E6:88:64:13"};
 constexpr WebRtcState kWebRtcState = WebRtcState::kConnectable;
 
+using ::absl::StatusCode::kInvalidArgument;
+using ::testing::status::StatusIs;
+
 // TODO(b/169550050): Implement UWBAddress.
 TEST(BleAdvertisementTest, ConstructionWorks) {
   ByteArray service_id_hash{std::string(kServiceIdHashBytes)};
@@ -269,7 +276,10 @@ TEST(BleAdvertisementTest, ConstructionFromBytesWorks) {
       ByteArray{},     kWebRtcState};
   ByteArray ble_advertisement_bytes(org_ble_advertisement);
 
-  BleAdvertisement ble_advertisement{false, ble_advertisement_bytes};
+  auto ble_status_or =
+      BleAdvertisement::CreateBleAdvertisement(false, ble_advertisement_bytes);
+  ASSERT_OK(ble_status_or.status());
+  auto ble_advertisement = ble_status_or.value();
 
   EXPECT_TRUE(ble_advertisement.IsValid());
   EXPECT_FALSE(ble_advertisement.IsFastAdvertisement());
@@ -290,7 +300,10 @@ TEST(BleAdvertisementTest, ConstructionFromBytesWorksForFastAdvertisement) {
                                          fast_endpoint_info, ByteArray{}};
   ByteArray ble_advertisement_bytes(org_ble_advertisement);
 
-  BleAdvertisement ble_advertisement{true, ble_advertisement_bytes};
+  auto ble_status_or =
+      BleAdvertisement::CreateBleAdvertisement(true, ble_advertisement_bytes);
+  ASSERT_OK(ble_status_or.status());
+  auto ble_advertisement = ble_status_or.value();
 
   EXPECT_TRUE(ble_advertisement.IsValid());
   EXPECT_TRUE(ble_advertisement.IsFastAdvertisement());
@@ -322,7 +335,10 @@ TEST(BleAdvertisementTest, ConstructionFromLongLengthBytesWorks) {
   memcpy(long_ble_advertisement_bytes.data(), ble_advertisement_bytes.data(),
          ble_advertisement_bytes.size());
 
-  BleAdvertisement long_ble_advertisement{false, long_ble_advertisement_bytes};
+  auto ble_status_or = BleAdvertisement::CreateBleAdvertisement(
+      false, long_ble_advertisement_bytes);
+  ASSERT_OK(ble_status_or.status());
+  auto long_ble_advertisement = ble_status_or.value();
 
   EXPECT_TRUE(long_ble_advertisement.IsValid());
   EXPECT_EQ(kVersion, long_ble_advertisement.GetVersion());
@@ -336,15 +352,13 @@ TEST(BleAdvertisementTest, ConstructionFromLongLengthBytesWorks) {
 }
 
 TEST(BleAdvertisementTest, ConstructionFromNullBytesFails) {
-  BleAdvertisement ble_advertisement{false, ByteArray{}};
-
-  EXPECT_FALSE(ble_advertisement.IsValid());
+  EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(false, ByteArray()),
+              StatusIs(kInvalidArgument));
 }
 
 TEST(BleAdvertisementTest, ConstructionFromNullBytesFailsForFastAdvertisement) {
-  BleAdvertisement ble_advertisement{true, ByteArray{}};
-
-  EXPECT_FALSE(ble_advertisement.IsValid());
+  EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(true, ByteArray()),
+              StatusIs(kInvalidArgument));
 }
 
 TEST(BleAdvertisementTest, ConstructionFromShortLengthBytesFails) {
@@ -363,10 +377,9 @@ TEST(BleAdvertisementTest, ConstructionFromShortLengthBytesFails) {
       ble_advertisement_bytes.data(),
       BleAdvertisement::kMinAdvertisementLength - 1};
 
-  BleAdvertisement short_ble_advertisement{false,
-                                           short_ble_advertisement_bytes};
-
-  EXPECT_FALSE(short_ble_advertisement.IsValid());
+  EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(
+                  false, short_ble_advertisement_bytes),
+              StatusIs(kInvalidArgument));
 }
 
 TEST(BleAdvertisementTest,
@@ -382,9 +395,9 @@ TEST(BleAdvertisementTest,
       ble_advertisement_bytes.data(),
       BleAdvertisement::kMinAdvertisementLength - 1};
 
-  BleAdvertisement short_ble_advertisement{true, short_ble_advertisement_bytes};
-
-  EXPECT_FALSE(short_ble_advertisement.IsValid());
+  EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(
+                  true, short_ble_advertisement_bytes),
+              StatusIs(kInvalidArgument));
 }
 
 TEST(BleAdvertisementTest,
@@ -404,10 +417,9 @@ TEST(BleAdvertisementTest,
   corrupt_ble_advertisement_string[8] ^= 0x0FF;
   ByteArray corrupt_ble_advertisement_bytes(corrupt_ble_advertisement_string);
 
-  BleAdvertisement corrupt_ble_advertisement{false,
-                                             corrupt_ble_advertisement_bytes};
-
-  EXPECT_FALSE(corrupt_ble_advertisement.IsValid());
+  EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(
+                  false, corrupt_ble_advertisement_bytes),
+              StatusIs(kInvalidArgument));
 }
 
 TEST(BleAdvertisementTest,
@@ -423,10 +435,9 @@ TEST(BleAdvertisementTest,
   corrupt_ble_advertisement_string[5] ^= 0x0FF;
   ByteArray corrupt_ble_advertisement_bytes(corrupt_ble_advertisement_string);
 
-  BleAdvertisement corrupt_ble_advertisement{true,
-                                             corrupt_ble_advertisement_bytes};
-
-  EXPECT_FALSE(corrupt_ble_advertisement.IsValid());
+  EXPECT_THAT(BleAdvertisement::CreateBleAdvertisement(
+                  true, corrupt_ble_advertisement_bytes),
+              StatusIs(kInvalidArgument));
 }
 
 }  // namespace

connections/implementation/p2p_cluster_pcp_handler.cc

@@ -441,19 +441,6 @@ void P2pClusterPcpHandler::BluetoothDeviceLostHandler(
 bool P2pClusterPcpHandler::IsRecognizedBleEndpoint(
     const std::string& service_id,
     const BleAdvertisement& advertisement) const {
-  if (!advertisement.IsValid()) {
-    NEARBY_LOGS(INFO)
-        << "BleAdvertisement doesn't conform to the format, discarding.";
-    return false;
-  }
-
-  if (advertisement.GetVersion() != kBleAdvertisementVersion) {
-    NEARBY_LOGS(INFO) << "BleAdvertisement has an unknown version; expected "
-                      << static_cast<int>(kBleAdvertisementVersion)
-                      << ", found "
-                      << static_cast<int>(advertisement.GetVersion());
-    return false;
-  }
 
   if (advertisement.GetPcp() != GetPcp()) {
     NEARBY_LOGS(INFO) << "BleAdvertisement doesn't match on Pcp; expected "
@@ -499,8 +486,13 @@ void P2pClusterPcpHandler::BlePeripheralDiscoveredHandler(
           return;
         }
 
-        // Parse the BLE advertisement bytes.
-        BleAdvertisement advertisement(fast_advertisement, advertisement_bytes);
+        auto ble_status_or = BleAdvertisement::CreateBleAdvertisement(
+            fast_advertisement, advertisement_bytes);
+        if (!ble_status_or.ok()) {
+          NEARBY_LOGS(ERROR) << ble_status_or.status().ToString();
+          return;
+        }
+        auto advertisement = ble_status_or.value();
 
         // Make sure the BLE advertisement points to a valid
         // endpoint we're discovering.
@@ -671,8 +663,13 @@ void P2pClusterPcpHandler::BleV2PeripheralDiscoveredHandler(
           return;
         }
 
-        // Parse the BLE advertisement bytes.
-        BleAdvertisement advertisement(fast_advertisement, advertisement_bytes);
+        auto ble_status_or = BleAdvertisement::CreateBleAdvertisement(
+            fast_advertisement, advertisement_bytes);
+        if (!ble_status_or.ok()) {
+          NEARBY_LOGS(ERROR) << ble_status_or.status();
+          return;
+        }
+        auto advertisement = ble_status_or.value();
 
         // Make sure the BLE advertisement points to a valid
         // endpoint we're discovering.
@@ -761,8 +758,13 @@ void P2pClusterPcpHandler::BleV2PeripheralLostHandler(
           return;
         }
 
-        // Parse the BLE advertisement bytes.
-        BleAdvertisement advertisement(fast_advertisement, advertisement_bytes);
+        auto ble_status_or = BleAdvertisement::CreateBleAdvertisement(
+            fast_advertisement, advertisement_bytes);
+        if (!ble_status_or.ok()) {
+          NEARBY_LOGS(ERROR) << ble_status_or.status();
+          return;
+        }
+        auto advertisement = ble_status_or.value();
 
         // Make sure the BLE advertisement points to a valid
         // endpoint we're discovering.