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

This reverts commit d812a16.

Author: woody-apple

.github/workflows/darwin.yaml

@@ -103,8 +103,7 @@ jobs:
               # target versions instead?
               run: |
                   mkdir -p /tmp/darwin/framework-tests
-                  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 > >(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 = [MTRBaseDevice deviceWithNodeID:@(mNodeId) controller:commissioner];
+    auto * device = [MTRDevice 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,7 +19,6 @@
 
 #import <Matter/MTRCluster.h>
 #import <Matter/MTRDefines.h>
-#import <Matter/MTRDiagnosticLogsType.h>
 
 @class MTRSetupPayload;
 @class MTRDeviceController;
@@ -543,26 +542,6 @@ 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,19 +2147,6 @@ + (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,6 +18,7 @@
 #import <Foundation/Foundation.h>
 #import <Matter/MTRBaseDevice.h>
 #import <Matter/MTRDefines.h>
+#import <Matter/MTRDiagnosticLogsType.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -325,6 +326,27 @@ 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

@@ -1749,6 +1749,18 @@ - (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,7 +20,6 @@
 #import <Matter/MTRCluster.h>
 #import <Matter/MTRDefines.h>
 #import <Matter/MTRDeviceController.h>
-#import <Matter/MTRDiagnosticLogsType.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -184,15 +183,6 @@ 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,10 +220,6 @@ + (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,38 +427,6 @@ - (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,6 +165,7 @@ - (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,28 +167,6 @@ - (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
@@ -1887,30 +1865,6 @@ - (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,8 +116,6 @@ @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
 
@@ -276,18 +274,6 @@ - (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];
@@ -314,44 +300,6 @@ - (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;