[Matter.framework] Get downloadLogOfType to work over XPC (project-chip#32319)

* [Matter.framework] Move downloadLogOfType from MTRDevice to MTRBaseDevice

* [Matter.framework] Get downloadLogOfType to work over XPC

* Add a simple test to MTRXPCProtocolTests.m to check that the XPC protocol works

* Add a simple test to MTRXPCListenerSampleTests

* [Matter.framework] Do not delete the file containing the log data once the downloadLogOfType completion is done

Author: vivien-apple

.github/workflows/darwin.yaml

@@ -103,7 +103,8 @@ jobs:
               # target versions instead?
               run: |
                   mkdir -p /tmp/darwin/framework-tests
-                  ../../../out/debug/chip-all-clusters-app --interface-id -1 > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
+                  echo "This is a simple log" > /tmp/darwin/framework-tests/end_user_support_log.txt
+                  ../../../out/debug/chip-all-clusters-app --interface-id -1 --end_user_support_log /tmp/darwin/framework-tests/end_user_support_log.txt > >(tee /tmp/darwin/framework-tests/all-cluster-app.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-err.log >&2) &
                   ../../../out/debug/chip-all-clusters-app --interface-id -1 --dac_provider ../../../credentials/development/commissioner_dut/struct_cd_origin_pid_vid_correct/test_case_vector.json --product-id 32768 --discriminator 3839 --secured-device-port 5539 --KVS /tmp/chip-all-clusters-app-kvs2 > >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid.log) 2> >(tee /tmp/darwin/framework-tests/all-cluster-app-origin-vid-err.log >&2) &
                   # Disable BLE (CHIP_IS_BLE=NO) because the app does not have the permission to use it and that may crash the CI.
 

examples/darwin-framework-tool/commands/bdx/DownloadLogCommand.mm

@@ -28,7 +28,7 @@
     ChipLogProgress(chipTool, "Downloading logs from node 0x" ChipLogFormatX64, ChipLogValueX64(mNodeId));
 
     MTRDeviceController * commissioner = CurrentCommissioner();
-    auto * device = [MTRDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
+    auto * device = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
 
     auto logType = static_cast<MTRDiagnosticLogType>(mLogType);
     auto queue = dispatch_queue_create("com.chip.bdx.downloader", DISPATCH_QUEUE_SERIAL);

src/darwin/Framework/CHIP/MTRBaseDevice.h

@@ -19,6 +19,7 @@
 
 #import <Matter/MTRCluster.h>
 #import <Matter/MTRDefines.h>
+#import <Matter/MTRDiagnosticLogsType.h>
 
 @class MTRSetupPayload;
 @class MTRDeviceController;
@@ -542,6 +543,26 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
                           reportHandler:(MTRDeviceResponseHandler)reportHandler
                 subscriptionEstablished:(MTRSubscriptionEstablishedHandler _Nullable)subscriptionEstablished
     MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
+
+/**
+ * Download log of the desired type from the device.
+ *
+ * @param type       The type of log being requested. This should correspond to a value in the enum MTRDiagnosticLogType.
+ * @param timeout    The timeout for getting the log. If the timeout expires, completion will be called with whatever
+ *                   has been retrieved by that point (which might be none or a partial log).
+ *                   If the timeout is set to 0, the request will not expire and completion will not be called until
+ *                   the log is fully retrieved or an error occurs.
+ * @param queue      The queue on which completion will be called.
+ * @param completion The completion handler that is called after attempting to retrieve the requested log.
+ *                     - In case of success, the completion handler is called with a non-nil URL and a nil error.
+ *                     - If there is an error, a non-nil error is used and the url can be non-nil too if some logs have already been downloaded.
+ */
+- (void)downloadLogOfType:(MTRDiagnosticLogType)type
+                  timeout:(NSTimeInterval)timeout
+                    queue:(dispatch_queue_t)queue
+               completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
+    MTR_NEWLY_AVAILABLE;
+
 @end
 
 /**

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -2147,6 +2147,19 @@ + (NSDictionary *)eventReportForHeader:(const chip::app::EventHeader &)header an
 
     return buffer;
 }
+
+- (void)downloadLogOfType:(MTRDiagnosticLogType)type
+                  timeout:(NSTimeInterval)timeout
+                    queue:(dispatch_queue_t)queue
+               completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
+{
+    [_deviceController downloadLogFromNodeWithID:@(_nodeID)
+                                            type:type
+                                         timeout:timeout
+                                           queue:queue
+                                      completion:completion];
+}
+
 @end
 
 @implementation MTRBaseDevice (Deprecated)

src/darwin/Framework/CHIP/MTRDevice.h

@@ -18,7 +18,6 @@
 #import <Foundation/Foundation.h>
 #import <Matter/MTRBaseDevice.h>
 #import <Matter/MTRDefines.h>
-#import <Matter/MTRDiagnosticLogsType.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -326,27 +325,6 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  */
 - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID MTR_UNSTABLE_API;
 
-/**
- * Download log of the desired type from the device.
- *
- * Note: The consumer of this API should move the file that the url points to or open it for reading before the
- * completion handler returns. Otherwise, the file will be deleted, and the data will be lost.
- *
- * @param type       The type of log being requested. This should correspond to a value in the enum MTRDiagnosticLogType.
- * @param timeout    The timeout for getting the log. If the timeout expires, completion will be called with whatever
- *                   has been retrieved by that point (which might be none or a partial log).
- *                   If the timeout is set to 0, the request will not expire and completion will not be called until
- *                   the log is fully retrieved or an error occurs.
- * @param queue      The queue on which completion will be called.
- * @param completion The completion handler that is called after attempting to retrieve the requested log.
- *                     - In case of success, the completion handler is called with a non-nil URL and a nil error.
- *                     - If there is an error, a non-nil error is used and the url can be non-nil too if some logs have already been downloaded.
- */
-- (void)downloadLogOfType:(MTRDiagnosticLogType)type
-                  timeout:(NSTimeInterval)timeout
-                    queue:(dispatch_queue_t)queue
-               completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
-    MTR_NEWLY_AVAILABLE;
 @end
 
 MTR_EXTERN NSString * const MTRPreviousDataKey MTR_NEWLY_AVAILABLE;

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -1713,18 +1713,6 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator
     [baseDevice openCommissioningWindowWithDiscriminator:discriminator duration:duration queue:queue completion:completion];
 }
 
-- (void)downloadLogOfType:(MTRDiagnosticLogType)type
-                  timeout:(NSTimeInterval)timeout
-                    queue:(dispatch_queue_t)queue
-               completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
-{
-    [_deviceController downloadLogFromNodeWithID:_nodeID
-                                            type:type
-                                         timeout:timeout
-                                           queue:queue
-                                      completion:completion];
-}
-
 #pragma mark - Cache management
 
 // assume lock is held

src/darwin/Framework/CHIP/MTRDeviceController+XPC.h

@@ -20,6 +20,7 @@
 #import <Matter/MTRCluster.h>
 #import <Matter/MTRDefines.h>
 #import <Matter/MTRDeviceController.h>
+#import <Matter/MTRDiagnosticLogsType.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -183,6 +184,15 @@ typedef void (^MTRValuesHandler)(id _Nullable values, NSError * _Nullable error)
                              attributeId:(NSNumber * _Nullable)attributeId
                               completion:(MTRValuesHandler)completion;
 
+/**
+ * Requests downloading some logs
+ */
+- (void)downloadLogWithController:(id _Nullable)controller
+                           nodeId:(uint64_t)nodeId
+                             type:(MTRDiagnosticLogType)type
+                          timeout:(NSTimeInterval)timeout
+                       completion:(void (^)(NSString * _Nullable url, NSError * _Nullable error))completion;
+
 @end
 
 /**

src/darwin/Framework/CHIP/MTRDeviceController+XPC.mm

@@ -220,6 +220,10 @@ + (NSXPCInterface *)xpcInterfaceForServerProtocol
                  forSelector:@selector(readAttributeCacheWithController:nodeId:endpointId:clusterId:attributeId:completion:)
                argumentIndex:0
                      ofReply:YES];
+    [xpcInterface setClasses:GetXPCAllowedClasses()
+                 forSelector:@selector(downloadLogWithController:nodeId:type:timeout:completion:)
+               argumentIndex:0
+                     ofReply:YES];
     return xpcInterface;
 }
 

src/darwin/Framework/CHIP/MTRDeviceOverXPC.mm

@@ -427,6 +427,38 @@ - (void)openCommissioningWindowWithDiscriminator:(NSNumber *)discriminator
     });
 }
 
+- (void)downloadLogOfType:(MTRDiagnosticLogType)type
+                  timeout:(NSTimeInterval)timeout
+                    queue:(dispatch_queue_t)queue
+               completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion
+{
+    MTR_LOG_DEBUG("Downloading log ...");
+
+    __auto_type workBlock = ^(MTRDeviceControllerXPCProxyHandle * _Nullable handle, NSError * _Nullable error) {
+        if (error != nil) {
+            completion(nil, error);
+            return;
+        }
+
+        [handle.proxy downloadLogWithController:self.controllerID
+                                         nodeId:self.nodeID.unsignedLongLongValue
+                                           type:type
+                                        timeout:timeout
+                                     completion:^(NSString * _Nullable url, NSError * _Nullable error) {
+                                         dispatch_async(queue, ^{
+                                             MTR_LOG_DEBUG("Download log");
+                                             completion([NSURL URLWithString:url], error);
+                                             // The following captures the proxy handle in the closure so that the
+                                             // handle won't be released prior to block call.
+                                             __auto_type handleRetainer = handle;
+                                             (void) handleRetainer;
+                                         });
+                                     }];
+    };
+
+    [self fetchProxyHandleWithQueue:queue completion:workBlock];
+}
+
 - (void)fetchProxyHandleWithQueue:(dispatch_queue_t)queue completion:(MTRFetchProxyHandleCompletion)completion
 {
     if (self.controllerID != nil) {

src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm

@@ -165,7 +165,6 @@ - (instancetype)initWithType:(MTRDiagnosticLogType)type
                     // data in the logs that the caller may find useful. For this reason, fileURL is passed in even
                     // when there is an error but fileHandle is not nil.
                     completion(strongSelf->_fileHandle ? fileURL : nil, bdxError);
-                    [strongSelf deleteFile];
 
                     done(strongSelf);
                 }

src/darwin/Framework/CHIPTests/MTRXPCListenerSampleTests.m

@@ -167,6 +167,28 @@ - (void)getAnyDeviceControllerWithCompletion:(void (^)(id _Nullable controller,
     completion(MTRDeviceControllerId, nil);
 }
 
+- (void)downloadLogWithController:(id)controller
+                           nodeId:(uint64_t)nodeId
+                             type:(MTRDiagnosticLogType)type
+                          timeout:(NSTimeInterval)timeout
+                       completion:(void (^)(NSString * _Nullable url, NSError * _Nullable error))completion
+{
+    (void) controller;
+    __auto_type sharedController = sController;
+    if (sharedController) {
+        __auto_type device = [MTRBaseDevice deviceWithNodeID:@(nodeId) controller:sharedController];
+        [device downloadLogOfType:type
+                          timeout:timeout
+                            queue:dispatch_get_main_queue()
+                       completion:^(NSURL * _Nullable url, NSError * _Nullable error) {
+                           completion([url absoluteString], error);
+                       }];
+    } else {
+        NSLog(@"Failed to get shared controller");
+        completion(nil, [NSError errorWithDomain:MTRErrorDomain code:MTRErrorCodeGeneralError userInfo:nil]);
+    }
+}
+
 - (void)readAttributeWithController:(id)controller
                              nodeId:(uint64_t)nodeId
                          endpointId:(NSNumber * _Nullable)endpointId
@@ -1865,6 +1887,30 @@ - (void)test015_MTRDeviceInteraction
         }, @(NO));
 }
 
+- (void)test016_DownloadLog
+{
+    XCTestExpectation * expectation =
+        [self expectationWithDescription:@"Download EndUserSupport log"];
+
+    MTRBaseDevice * device = GetConnectedDevice();
+    dispatch_queue_t queue = dispatch_get_main_queue();
+
+    [device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport
+                      timeout:10
+                        queue:queue
+                   completion:^(NSURL * _Nullable url, NSError * _Nullable error) {
+                       NSLog(@"downloadLogOfType: url: %@, error: %@", url, error);
+                       XCTAssertNil(error);
+
+                       NSError * readError;
+                       NSString * fileContent = [NSString stringWithContentsOfURL:url encoding:NSUTF8StringEncoding error:&readError];
+                       XCTAssertNil(readError);
+                       XCTAssertTrue([fileContent isEqualToString:@"This is a simple log\n"]);
+                       [expectation fulfill];
+                   }];
+    [self waitForExpectationsWithTimeout:kTimeoutInSeconds handler:nil];
+}
+
 - (void)test900_SubscribeClusterStateCache
 {
     XCTestExpectation * expectation = [self expectationWithDescription:@"subscribe attributes by cache"];

src/darwin/Framework/CHIPTests/MTRXPCProtocolTests.m

@@ -116,6 +116,8 @@ @interface MTRXPCProtocolTests<NSXPCListenerDelegate, CHIPRemoteDeviceProtocol>
 @property (readwrite, strong) void (^handleReadClusterStateCache)
     (id controller, NSNumber * nodeId, NSNumber * _Nullable endpointId, NSNumber * _Nullable clusterId,
         NSNumber * _Nullable attributeId, void (^completion)(id _Nullable values, NSError * _Nullable error));
+@property (readwrite, strong) void (^handleDownloadLog)(id controller, NSNumber * nodeId, MTRDiagnosticLogType type, NSTimeInterval timeout,
+    void (^completion)(NSString * _Nullable url, NSError * _Nullable error));
 
 @end
 
@@ -274,6 +276,18 @@ - (void)readAttributeCacheWithController:(id _Nullable)controller
     });
 }
 
+- (void)downloadLogWithController:(id)controller
+                           nodeId:(uint64_t)nodeId
+                             type:(MTRDiagnosticLogType)type
+                          timeout:(NSTimeInterval)timeout
+                       completion:(void (^)(NSString * _Nullable url, NSError * _Nullable error))completion
+{
+    dispatch_async(dispatch_get_main_queue(), ^{
+        XCTAssertNotNil(self.handleDownloadLog);
+        self.handleDownloadLog(controller, @(nodeId), type, timeout, completion);
+    });
+}
+
 - (void)setUp
 {
     [self setContinueAfterFailure:NO];
@@ -300,6 +314,44 @@ - (void)tearDown
     _xpcDisconnectExpectation = nil;
 }
 
+- (void)testDownloadLogSuccess
+{
+    uint64_t myNodeId = 9876543210;
+    NSString * myBdxURL = @"bdx://foo";
+    NSTimeInterval myTimeout = 10;
+
+    XCTestExpectation * callExpectation = [self expectationWithDescription:@"XPC call received"];
+    XCTestExpectation * responseExpectation = [self expectationWithDescription:@"XPC response received"];
+
+    __auto_type uuid = self.controllerUUID;
+    _handleDownloadLog = ^(id controller, NSNumber * nodeId, MTRDiagnosticLogType type, NSTimeInterval timeout,
+        void (^completion)(NSString * _Nullable url, NSError * _Nullable error)) {
+        XCTAssertTrue([controller isEqualToString:uuid]);
+        XCTAssertEqual([nodeId unsignedLongLongValue], myNodeId);
+        [callExpectation fulfill];
+        completion(myBdxURL, nil);
+    };
+
+    __auto_type * device = [MTRBaseDevice deviceWithNodeID:@(myNodeId) controller:_remoteDeviceController];
+    NSLog(@"Device acquired. Downloading...");
+    [device downloadLogOfType:MTRDiagnosticLogTypeEndUserSupport
+                      timeout:myTimeout
+                        queue:dispatch_get_main_queue()
+                   completion:^(NSURL * _Nullable url, NSError * _Nullable error) {
+                       NSLog(@"Read url: %@", url);
+                       XCTAssertNotNil(url);
+                       XCTAssertNil(error);
+                       [responseExpectation fulfill];
+                       self.xpcDisconnectExpectation = [self expectationWithDescription:@"XPC Disconnected"];
+                   }];
+
+    [self waitForExpectations:[NSArray arrayWithObjects:callExpectation, responseExpectation, nil] timeout:kTimeoutInSeconds];
+
+    // When download is done, connection should have been released
+    [self waitForExpectations:[NSArray arrayWithObject:_xpcDisconnectExpectation] timeout:kTimeoutInSeconds];
+    XCTAssertNil(_xpcConnection);
+}
+
 - (void)testReadAttributeSuccess
 {
     uint64_t myNodeId = 9876543210;