Do not set up subscription when setting a delegate for a MTRDevice wi… (

project-chip#33559)

* Do not set up subscription when setting a delegate for a MTRDevice with a remote controller over XPC

- Add tests for testing the behavior when a MTRDevice is created with an XPC controller

* Restyled by whitespace

* Restyled by clang-format

* Clean up

* Restyled by clang-format

* Add an API for checking if subscriptions are allowed and check that in _setUpSubscription

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <[email protected]>

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -685,11 +685,17 @@ - (void)_setDSTOffsets:(NSArray<MTRTimeSynchronizationClusterDSTOffsetStruct *>
 #define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MIN (10 * 60) // 10 minutes (for now)
 #define MTR_DEVICE_SUBSCRIPTION_MAX_INTERVAL_MAX (60 * 60) // 60 minutes
 
+- (BOOL)_subscriptionsAllowed
+{
+    return ![_deviceController isKindOfClass:MTRDeviceControllerOverXPC.class];
+}
+
 - (void)setDelegate:(id<MTRDeviceDelegate>)delegate queue:(dispatch_queue_t)queue
 {
     MTR_LOG("%@ setDelegate %@", self, delegate);
 
-    BOOL setUpSubscription = YES;
+    // We should not set up a subscription for device controllers over XPC.
+    BOOL setUpSubscription = [self _subscriptionsAllowed];
 
     // For unit testing only
 #ifdef DEBUG
@@ -933,6 +939,14 @@ - (void)_changeInternalState:(MTRInternalDeviceState)state
     }
 }
 
+#ifdef DEBUG
+- (MTRInternalDeviceState)_getInternalState
+{
+    std::lock_guard lock(self->_lock);
+    return _internalDeviceState;
+}
+#endif
+
 // First Time Sync happens 2 minutes after reachability (this can be changed in the future)
 #define MTR_DEVICE_TIME_UPDATE_INITIAL_WAIT_TIME_SEC (60 * 2)
 - (void)_handleSubscriptionEstablished
@@ -1680,6 +1694,11 @@ - (void)_setupSubscription
 {
     os_unfair_lock_assert_owner(&self->_lock);
 
+    if (![self _subscriptionsAllowed]) {
+        MTR_LOG("_setupSubscription: Subscriptions not allowed. Do not set up subscription");
+        return;
+    }
+
 #ifdef DEBUG
     id delegate = _weakDelegate.strongObject;
     Optional<System::Clock::Seconds32> maxIntervalOverride;

src/darwin/Framework/CHIPTests/MTRDeviceTests.m

@@ -3618,6 +3618,36 @@ - (void)test034_TestMTRDeviceHistoricalEvents
     XCTAssertTrue(eventReportsReceived > 0);
 }
 
+- (void)test035_TestMTRDeviceSubscriptionNotEstablishedOverXPC
+{
+    NSString * const MTRDeviceControllerId = @"MTRController";
+    __auto_type remoteController = [MTRDeviceController
+        sharedControllerWithID:MTRDeviceControllerId
+               xpcConnectBlock:^NSXPCConnection * _Nonnull {
+                   return nil;
+               }];
+
+    __auto_type * device = [MTRDevice deviceWithNodeID:kDeviceId deviceController:remoteController];
+    dispatch_queue_t queue = dispatch_get_main_queue();
+
+    // We should not set up a subscription when creating a MTRDevice with a remote controller.
+    XCTestExpectation * subscriptionExpectation = [self expectationWithDescription:@"Subscription has been set up"];
+    subscriptionExpectation.inverted = YES;
+
+    __auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
+
+    XCTAssertTrue([device _getInternalState] == MTRInternalDeviceStateUnsubscribed);
+
+    delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * attributeReport) {
+        [subscriptionExpectation fulfill];
+    };
+
+    [device setDelegate:delegate queue:queue];
+    [self waitForExpectations:@[ subscriptionExpectation ] timeout:30];
+
+    XCTAssertTrue([device _getInternalState] == MTRInternalDeviceStateUnsubscribed);
+}
+
 @end
 
 @interface MTRDeviceEncoderTests : XCTestCase

src/darwin/Framework/CHIPTests/TestHelpers/MTRTestDeclarations.h

@@ -59,10 +59,31 @@ NS_ASSUME_NONNULL_BEGIN
 @end
 
 @interface MTRDevice (TestDebug)
+typedef NS_ENUM(NSUInteger, MTRInternalDeviceState) {
+    // Unsubscribed means we do not have a subscription and are not trying to set one up.
+    MTRInternalDeviceStateUnsubscribed = 0,
+    // Subscribing means we are actively trying to establish our initial subscription (e.g. doing
+    // DNS-SD discovery, trying to establish CASE to the peer, getting priming reports, etc).
+    MTRInternalDeviceStateSubscribing = 1,
+    // InitialSubscriptionEstablished means we have at some point finished setting up a
+    // subscription.  That subscription may have dropped since then, but if so it's the ReadClient's
+    // responsibility to re-establish it.
+    MTRInternalDeviceStateInitialSubscriptionEstablished = 2,
+    // Resubscribing means we had established a subscription, but then
+    // detected a subscription drop due to not receiving a report on time. This
+    // covers all the actions that happen when re-subscribing (discovery, CASE,
+    // getting priming reports, etc).
+    MTRInternalDeviceStateResubscribing = 3,
+    // LaterSubscriptionEstablished meant that we had a subscription drop and
+    // then re-created a subscription.
+    MTRInternalDeviceStateLaterSubscriptionEstablished = 4,
+};
+
 - (void)unitTestInjectEventReport:(NSArray<NSDictionary<NSString *, id> *> *)eventReport;
 - (void)unitTestInjectAttributeReport:(NSArray<NSDictionary<NSString *, id> *> *)attributeReport fromSubscription:(BOOL)isFromSubscription;
 - (NSUInteger)unitTestAttributesReportedSinceLastCheck;
 - (void)unitTestClearClusterData;
+- (MTRInternalDeviceState)_getInternalState;
 @end
 #endif