From 26efd91ff4c2c36b5f9691ffd88342a9f517111b Mon Sep 17 00:00:00 2001 From: Edwin Wu Date: Thu, 9 Apr 2026 22:52:26 -0700 Subject: [PATCH] Introduce factory patterns for BLE managers in BleMedium. PiperOrigin-RevId: 897492346 --- .../apple/Tests/ble_medium_test.mm | 67 +++++++++++++++---- .../implementation/apple/ble_medium.h | 9 +++ .../implementation/apple/ble_medium.mm | 42 +++++++----- 3 files changed, 89 insertions(+), 29 deletions(-) diff --git a/internal/platform/implementation/apple/Tests/ble_medium_test.mm b/internal/platform/implementation/apple/Tests/ble_medium_test.mm index f8e53d2e..491ee93f 100644 --- a/internal/platform/implementation/apple/Tests/ble_medium_test.mm +++ b/internal/platform/implementation/apple/Tests/ble_medium_test.mm @@ -47,11 +47,13 @@ namespace apple { class BleMediumPeer { public: - static void SetSocketCentralManager(BleMedium *ble_medium, GNSCentralManager *manager) { - ble_medium->socketCentralManager_ = manager; + static void SetPeripheralManagerFactory(BleMedium *ble_medium, + BleMedium::PeripheralManagerFactory factory) { + ble_medium->peripheral_manager_factory_ = std::move(factory); } - static void SetSocketPeripheralManager(BleMedium *ble_medium, GNSPeripheralManager *manager) { - ble_medium->socketPeripheralManager_ = manager; + static void SetCentralManagerFactory(BleMedium *ble_medium, + BleMedium::CentralManagerFactory factory) { + ble_medium->central_manager_factory_ = std::move(factory); } static GNSPeripheralServiceManager *GetSocketPeripheralServiceManager(BleMedium *ble_medium) { return ble_medium->socketPeripheralServiceManager_; @@ -88,6 +90,28 @@ static const char *const kTestServiceID = "TestServiceID"; [super tearDown]; } +- (void)testOpenServerSocket_UsesFactoryForInitialization { + __block BOOL factoryWasCalled = NO; + id mockPeripheralManager = OCMClassMock([GNSPeripheralManager class]); + OCMStub([mockPeripheralManager addPeripheralServiceManager:[OCMArg any] + bleServiceAddedCompletion:[OCMArg any]]) + .andDo(^(GNSPeripheralManager *localSelf, GNSPeripheralServiceManager *manager, + void (^completion)(NSError *error)) { + completion(nil); + }); + + nearby::apple::BleMediumPeer::SetPeripheralManagerFactory(_medium.get(), ^() { + factoryWasCalled = YES; + return mockPeripheralManager; + }); + + // This call should trigger the factory inside BleMedium. + auto server_socket = _medium->OpenServerSocket(kTestServiceID); + + XCTAssertTrue(factoryWasCalled, @"BleMedium should have requested the manager from the factory."); + XCTAssertNotEqual(server_socket.get(), nullptr); +} + #pragma mark - Advertising Tests - (void)testStartAdvertising_Success { @@ -431,7 +455,9 @@ static const char *const kTestServiceID = "TestServiceID"; id mockCentralManager = OCMClassMock([GNSCentralManager class]); OCMStub([mockCentralManager retrieveCentralPeerWithIdentifier:fakePeripheral.identifier]) .andReturn(nil); - nearby::apple::BleMediumPeer::SetSocketCentralManager(_medium.get(), mockCentralManager); + nearby::apple::BleMediumPeer::SetCentralManagerFactory(_medium.get(), ^(CBUUID *uuid) { + return mockCentralManager; + }); auto socket = _medium->Connect(kTestServiceID, nearby::api::ble::TxPowerLevel::kUltraLow, fakePeripheral.identifier.hash, nullptr); @@ -469,7 +495,9 @@ static const char *const kTestServiceID = "TestServiceID"; id mockCentralManager = OCMClassMock([GNSCentralManager class]); OCMStub([mockCentralManager retrieveCentralPeerWithIdentifier:fakePeripheral.identifier]) .andReturn(mockCentralPeerManager); - nearby::apple::BleMediumPeer::SetSocketCentralManager(_medium.get(), mockCentralManager); + nearby::apple::BleMediumPeer::SetCentralManagerFactory(_medium.get(), ^(CBUUID *uuid) { + return mockCentralManager; + }); auto socket = _medium->Connect(kTestServiceID, nearby::api::ble::TxPowerLevel::kUltraLow, fakePeripheral.identifier.hash, nullptr); @@ -490,7 +518,9 @@ static const char *const kTestServiceID = "TestServiceID"; void (^completion)(NSError *error)) { completion(nil); }); - nearby::apple::BleMediumPeer::SetSocketPeripheralManager(_medium.get(), mockPeripheralManager); + nearby::apple::BleMediumPeer::SetPeripheralManagerFactory(_medium.get(), ^() { + return mockPeripheralManager; + }); auto server_socket = _medium->OpenServerSocket(kTestServiceID); @@ -512,7 +542,9 @@ static const char *const kTestServiceID = "TestServiceID"; void (^completion)(NSError *error)) { completion(nil); }); - nearby::apple::BleMediumPeer::SetSocketPeripheralManager(_medium.get(), mockPeripheralManager); + nearby::apple::BleMediumPeer::SetPeripheralManagerFactory(_medium.get(), ^() { + return mockPeripheralManager; + }); auto server_socket = _medium->OpenServerSocket(kTestServiceID); @@ -534,7 +566,9 @@ static const char *const kTestServiceID = "TestServiceID"; void (^completion)(NSError *error)) { completion(nil); }); - nearby::apple::BleMediumPeer::SetSocketPeripheralManager(_medium.get(), mockPeripheralManager); + nearby::apple::BleMediumPeer::SetPeripheralManagerFactory(_medium.get(), ^() { + return mockPeripheralManager; + }); auto server_socket = _medium->OpenServerSocket(kTestServiceID); XCTAssertNotEqual(server_socket.get(), nullptr); @@ -570,9 +604,10 @@ static const char *const kTestServiceID = "TestServiceID"; // Since we use dispatch_async internally for connection callback, give it a small amount of time // to process so we know it didn't crash. XCTestExpectation *expectation2 = [self expectationWithDescription:@"Wait for async execution"]; - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ - [expectation2 fulfill]; - }); + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(0.1 * NSEC_PER_SEC)), + dispatch_get_main_queue(), ^{ + [expectation2 fulfill]; + }); [self waitForExpectations:@[ expectation2 ] timeout:1.0]; } @@ -584,7 +619,9 @@ static const char *const kTestServiceID = "TestServiceID"; void (^completion)(NSError *error)) { completion([NSError errorWithDomain:@"test" code:0 userInfo:nil]); }); - nearby::apple::BleMediumPeer::SetSocketPeripheralManager(_medium.get(), mockPeripheralManager); + nearby::apple::BleMediumPeer::SetPeripheralManagerFactory(_medium.get(), ^() { + return mockPeripheralManager; + }); auto server_socket = _medium->OpenServerSocket(kTestServiceID); @@ -599,7 +636,9 @@ static const char *const kTestServiceID = "TestServiceID"; void (^completion)(NSError *error)){ // Do not call completion to simulate timeout. }); - nearby::apple::BleMediumPeer::SetSocketPeripheralManager(_medium.get(), mockPeripheralManager); + nearby::apple::BleMediumPeer::SetPeripheralManagerFactory(_medium.get(), ^() { + return mockPeripheralManager; + }); auto server_socket = _medium->OpenServerSocket(kTestServiceID); diff --git a/internal/platform/implementation/apple/ble_medium.h b/internal/platform/implementation/apple/ble_medium.h index 7bbf2856..28438113 100644 --- a/internal/platform/implementation/apple/ble_medium.h +++ b/internal/platform/implementation/apple/ble_medium.h @@ -21,6 +21,7 @@ #import +#include #include #include #include @@ -50,6 +51,10 @@ class BleMedium : public api::ble::BleMedium { friend class BleMediumPeer; public: + // Define factory types for managers. + using PeripheralManagerFactory = std::function; + using CentralManagerFactory = std::function; + BleMedium(); // For testing only. explicit BleMedium(GNCBLEMedium *medium); @@ -217,6 +222,10 @@ class BleMedium : public api::ble::BleMedium { // The executor for handling callbacks. apple::SingleThreadExecutor callback_executor_; + // Factories for lazy initialization + PeripheralManagerFactory peripheral_manager_factory_ = nullptr; + CentralManagerFactory central_manager_factory_ = nullptr; + GNCBLEMedium *medium_; PeripheralsMap peripherals_; diff --git a/internal/platform/implementation/apple/ble_medium.mm b/internal/platform/implementation/apple/ble_medium.mm index 27a65563..dcb1d19f 100644 --- a/internal/platform/implementation/apple/ble_medium.mm +++ b/internal/platform/implementation/apple/ble_medium.mm @@ -193,7 +193,11 @@ std::unique_ptr BleMedium::StartScanning( peripherals_.Clear(); ClearAdvertisementPacketsMap(); - socketCentralManager_ = [[GNSCentralManager alloc] initWithSocketServiceUUID:serviceUUID]; + if (central_manager_factory_) { + socketCentralManager_ = central_manager_factory_(serviceUUID); + } else { + socketCentralManager_ = [[GNSCentralManager alloc] initWithSocketServiceUUID:serviceUUID]; + } [socketCentralManager_ startNoScanModeWithAdvertisedServiceUUIDs:@[ serviceUUID ]]; dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); @@ -247,7 +251,11 @@ bool BleMedium::StartScanning(const Uuid &service_uuid, api::ble::TxPowerLevel t peripherals_.Clear(); ClearAdvertisementPacketsMap(); - socketCentralManager_ = [[GNSCentralManager alloc] initWithSocketServiceUUID:serviceUUID]; + if (central_manager_factory_) { + socketCentralManager_ = central_manager_factory_(serviceUUID); + } else { + socketCentralManager_ = [[GNSCentralManager alloc] initWithSocketServiceUUID:serviceUUID]; + } [socketCentralManager_ startNoScanModeWithAdvertisedServiceUUIDs:@[ serviceUUID ]]; dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); @@ -294,7 +302,11 @@ bool BleMedium::StartMultipleServicesScanning(const std::vector &service_u peripherals_.Clear(); ClearAdvertisementPacketsMap(); - socketCentralManager_ = [[GNSCentralManager alloc] initWithSocketServiceUUID:serviceUUIDs[0]]; + if (central_manager_factory_) { + socketCentralManager_ = central_manager_factory_(serviceUUIDs[0]); + } else { + socketCentralManager_ = [[GNSCentralManager alloc] initWithSocketServiceUUID:serviceUUIDs[0]]; + } [socketCentralManager_ startNoScanModeWithAdvertisedServiceUUIDs:@[ serviceUUIDs[0] ]]; dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); @@ -460,12 +472,12 @@ std::unique_ptr BleMedium::OpenServerSocketWithDeadlo auto server_socket = std::make_unique(); if (socketPeripheralManager_ == nil) { - socketPeripheralManager_ = [[GNSPeripheralManager alloc] initWithAdvertisedName:nil - restoreIdentifier:nil]; - } - if (socketPeripheralManager_ == nil) { - GNCLoggerError(@"Failed to create peripheral manager."); - return nullptr; + if (peripheral_manager_factory_) { + socketPeripheralManager_ = peripheral_manager_factory_(); + } else { + socketPeripheralManager_ = [[GNSPeripheralManager alloc] initWithAdvertisedName:nil + restoreIdentifier:nil]; + } } // Fix for b/494335036 (Registry + Background Queue) @@ -543,12 +555,12 @@ std::unique_ptr BleMedium::OpenServerSocketLegacy( auto server_socket = std::make_unique(); if (socketPeripheralManager_ == nil) { - socketPeripheralManager_ = [[GNSPeripheralManager alloc] initWithAdvertisedName:nil - restoreIdentifier:nil]; - } - if (socketPeripheralManager_ == nil) { - GNCLoggerError(@"Failed to create peripheral manager."); - return nullptr; + if (peripheral_manager_factory_) { + socketPeripheralManager_ = peripheral_manager_factory_(); + } else { + socketPeripheralManager_ = [[GNSPeripheralManager alloc] initWithAdvertisedName:nil + restoreIdentifier:nil]; + } } // Raw pointer for closure capture in the legacy path (risks use-after-free).