Remove session accessors on non-concrete MTRDeviceController. (#36317)

CASE/PASE sessions only make sense for concrete controllers.

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -233,11 +233,6 @@ - (void)onDone
 
 @end
 
-@interface MTRBaseDevice ()
-// Will return nil if our controller is not in fact a concrete controller.
-@property (nullable, nonatomic, strong, readonly) MTRDeviceController_Concrete * concreteController;
-@end
-
 @implementation MTRBaseDevice
 
 - (instancetype)initWithPASEDevice:(chip::DeviceProxy *)device controller:(MTRDeviceController *)controller

src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h

@@ -19,6 +19,7 @@
 #import <Foundation/Foundation.h>
 
 #import "MTRDefines_Internal.h"
+#import "MTRDeviceController_Concrete.h"
 
 #include <app/AttributePathParams.h>
 #include <app/ConcreteAttributePath.h>
@@ -210,6 +211,11 @@ static inline MTRTransportType MTRMakeTransportType(chip::Transport::Type type)
                                 queue:(dispatch_queue_t)queue
                            completion:(MTRDeviceResponseHandler)completion;
 
+/**
+ * Will return nil if our controller is not in fact a concrete controller.
+ */
+@property (nullable, nonatomic, strong, readonly) MTRDeviceController_Concrete * concreteController;
+
 @end
 
 @interface MTRClusterPath ()

src/darwin/Framework/CHIP/MTRCallbackBridgeBase.h

@@ -18,8 +18,10 @@
 #import <Foundation/Foundation.h>
 
 #import "MTRBaseDevice_Internal.h"
+#import "MTRDeviceController_Concrete.h"
 #import "MTRDeviceController_Internal.h"
 #import "MTRError_Internal.h"
+#import "MTRLogging_Internal.h"
 #import "zap-generated/MTRBaseClusters.h"
 
 #include <app/data-model/NullObject.h>
@@ -36,17 +38,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 class MTRCallbackBridgeBase {
 public:
-    /**
-     * Run the given MTRActionBlock on the Matter thread, after getting a CASE
-     * session (possibly pre-existing) to the given node ID on the fabric
-     * represented by the given MTRDeviceController.  On success, convert the
-     * success value to whatever type it needs to be to call the callback type
-     * we're templated over.  Once this function has been called, on a callback
-     * bridge allocated with `new`, the bridge object must not be accessed by
-     * the caller.  The action block will handle deleting the bridge.
-     */
-    void DispatchAction(chip::NodeId nodeID, MTRDeviceController * controller) && { ActionWithNodeID(nodeID, controller); }
-
     /**
      * Run the given MTRActionBlock on the Matter thread after getting a secure
      * session corresponding to the given MTRBaseDevice.  On success, convert
@@ -60,7 +51,7 @@ class MTRCallbackBridgeBase {
         if (device.isPASEDevice) {
             ActionWithPASEDevice(device);
         } else {
-            ActionWithNodeID(device.nodeID, device.deviceController);
+            ActionWithNodeID(device.nodeID, device.concreteController);
         }
     }
 
@@ -81,22 +72,30 @@ class MTRCallbackBridgeBase {
     {
         LogRequestStart();
 
-        // TODO: Figure out whether we can usefully get an MTRDeviceController_Concrete in here, so
-        // we can move getSessionForCommissioneeDevice off of MTRDeviceController_Internal.  Ideally
-        // without bloating this inline method too much.
-        [device.deviceController getSessionForCommissioneeDevice:device.nodeID
-                                                      completion:^(chip::Messaging::ExchangeManager * exchangeManager,
-                                                          const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
-                                                          MaybeDoAction(exchangeManager, session, error);
-                                                      }];
+        MTRDeviceController_Concrete * _Nullable controller = device.concreteController;
+        if (!controller) {
+            MTR_LOG_ERROR("Trying to perform action with PASE device with a non-concrete controller");
+            MaybeDoAction(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
+            return;
+        }
+
+        [controller getSessionForCommissioneeDevice:device.nodeID
+                                         completion:^(chip::Messaging::ExchangeManager * exchangeManager,
+                                             const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {
+                                             MaybeDoAction(exchangeManager, session, error);
+                                         }];
     }
 
-    void ActionWithNodeID(chip::NodeId nodeID, MTRDeviceController * controller)
+    void ActionWithNodeID(chip::NodeId nodeID, MTRDeviceController_Concrete * _Nullable controller)
     {
         LogRequestStart();
 
-        // TODO: Figure out whether we can usefully get an MTRDeviceController_Concrete in here, so
-        // we can move getSessionForNode off of MTRDeviceController_Internal.
+        if (!controller) {
+            MTR_LOG_ERROR("Trying to perform action on node ID 0x%016llX (%llu) with a non-concrete controller", nodeID, nodeID);
+            MaybeDoAction(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE]);
+            return;
+        }
+
         [controller getSessionForNode:nodeID
                            completion:^(chip::Messaging::ExchangeManager * _Nullable exchangeManager,
                                const chip::Optional<chip::SessionHandle> & session, NSError * _Nullable error, NSNumber * _Nullable retryDelay) {

src/darwin/Framework/CHIP/MTRDeviceController.mm

@@ -535,18 +535,6 @@ - (BOOL)checkIsRunning:(NSError * __autoreleasing *)error
     return NO;
 }
 
-- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion
-{
-    MTR_ABSTRACT_METHOD();
-    completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
-}
-
-- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion
-{
-    MTR_ABSTRACT_METHOD();
-    completion(nullptr, chip::NullOptional, [MTRError errorForCHIPErrorCode:CHIP_ERROR_INCORRECT_STATE], nil);
-}
-
 - (void)asyncDispatchToMatterQueue:(dispatch_block_t)block errorHandler:(nullable MTRDeviceErrorHandler)errorHandler
 {
     // TODO: Figure out how to get callsites to have an MTRDeviceController_Concrete.

src/darwin/Framework/CHIP/MTRDeviceController_Concrete.h

@@ -120,13 +120,47 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)clearPendingShutdown;
 
+/**
+ * Ensure we have a CASE session to the given node ID and then call the provided
+ * connection callback.  This may be called on any queue (including the Matter
+ * event queue) and on success will always call the provided connection callback
+ * on the Matter queue, asynchronously.  Consumers must be prepared to run on
+ * the Matter queue (an in particular must not use any APIs that will try to do
+ * sync dispatch to the Matter queue).
+ *
+ * If the controller is not running when this function is called, it will
+ * synchronously invoke the completion with an error, on whatever queue
+ * getSessionForNode was called on.
+ *
+ * If the controller is not running when the async dispatch on the Matter queue
+ * happens, the completion will be invoked with an error on the Matter queue.
+ */
+- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
+
 /**
  * Since getSessionForNode now enqueues by the subscription pool for Thread
  * devices, MTRDevice_Concrete needs a direct non-queued access because it already
  * makes use of the subscription pool.
  */
 - (void)directlyGetSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
 
+/**
+ * Get a session for the commissionee device with the given device id.  This may
+ * be called on any queue (including the Matter event queue) and on success will
+ * always call the provided connection callback on the Matter queue,
+ * asynchronously.  Consumers must be prepared to run on the Matter queue (an in
+ * particular must not use any APIs that will try to do sync dispatch to the
+ * Matter queue).
+ *
+ * If the controller is not running when this function is called, it will
+ * synchronously invoke the completion with an error, on whatever queue
+ * getSessionForCommissioneeDevice was called on.
+ *
+ * If the controller is not running when the async dispatch on the Matter queue
+ * happens, the completion will be invoked with an error on the Matter queue.
+ */
+- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion;
+
 /**
  * Notify the controller that a new operational instance with the given node id
  * and a compressed fabric id that matches this controller has been observed.

src/darwin/Framework/CHIP/MTRDeviceController_Internal.h

@@ -121,40 +121,6 @@ NS_ASSUME_NONNULL_BEGIN
  */
 @property (nonatomic, retain, nullable) NSData * rootPublicKey;
 
-/**
- * Ensure we have a CASE session to the given node ID and then call the provided
- * connection callback.  This may be called on any queue (including the Matter
- * event queue) and on success will always call the provided connection callback
- * on the Matter queue, asynchronously.  Consumers must be prepared to run on
- * the Matter queue (an in particular must not use any APIs that will try to do
- * sync dispatch to the Matter queue).
- *
- * If the controller is not running when this function is called, it will
- * synchronously invoke the completion with an error, on whatever queue
- * getSessionForNode was called on.
- *
- * If the controller is not running when the async dispatch on the Matter queue
- * happens, the completion will be invoked with an error on the Matter queue.
- */
-- (void)getSessionForNode:(chip::NodeId)nodeID completion:(MTRInternalDeviceConnectionCallback)completion;
-
-/**
- * Get a session for the commissionee device with the given device id.  This may
- * be called on any queue (including the Matter event queue) and on success will
- * always call the provided connection callback on the Matter queue,
- * asynchronously.  Consumers must be prepared to run on the Matter queue (an in
- * particular must not use any APIs that will try to do sync dispatch to the
- * Matter queue).
- *
- * If the controller is not running when this function is called, it will
- * synchronously invoke the completion with an error, on whatever queue
- * getSessionForCommissioneeDevice was called on.
- *
- * If the controller is not running when the async dispatch on the Matter queue
- * happens, the completion will be invoked with an error on the Matter queue.
- */
-- (void)getSessionForCommissioneeDevice:(chip::NodeId)deviceID completion:(MTRInternalDeviceConnectionCallback)completion;
-
 /**
  * Try to asynchronously dispatch the given block on the Matter queue.  If the
  * controller is not running either at call time or when the block would be

src/darwin/Framework/CHIPTests/MTRPerControllerStorageTests.m

@@ -2706,8 +2706,8 @@ - (void)doTestSubscriptionPoolWithSize:(NSInteger)subscriptionPoolSize deviceOnb
 
             // Given the base device read is happening on the 5th device, at the completion
             // time of the first [pool size] subscriptions, the BaseDevice's request to
-            // read can't have completed, as it should be gated on its call to the
-            // MTRDeviceController's getSessionForNode: call.
+            // read can't have completed, as it should be gated on its call to
+            // MTRDeviceController_Concrete's getSessionForNode:.
             if (subscriptionDequeueCount <= (orderedDeviceIDs.count - subscriptionPoolSize)) {
                 XCTAssertFalse(baseDeviceReadCompleted);
             }