Apply API review fixes for MTRPersistentStorageDelegate.h. (#23476)

* Apply API review fixes for MTRPersistentStorageDelegate.h. This is a re-landing of PR #22609 but modified to preserve the old APIs. * Rename the protocol (and header). * Fix some comments. * Address review comment.
project-chip · Nov 3, 2022 · 496ec18 · 496ec18
1 parent 93afba1
commit 496ec18
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 19 deletions.
diff --git a/examples/darwin-framework-tool/commands/common/CHIPCommandStorageDelegate.h b/examples/darwin-framework-tool/commands/common/CHIPCommandStorageDelegate.h
@@ -3,7 +3,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-@interface CHIPToolPersistentStorageDelegate : NSObject <MTRPersistentStorageDelegate>
+@interface CHIPToolPersistentStorageDelegate : NSObject <MTRStorage>
 - (nullable NSData *)storageDataForKey:(NSString *)key;
 - (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key;
 - (BOOL)removeStorageDataForKey:(NSString *)key;

diff --git a/src/darwin/Framework/CHIP/MTRControllerFactory.h b/src/darwin/Framework/CHIP/MTRControllerFactory.h
@@ -24,6 +24,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+@protocol MTRStorage;
 @protocol MTRPersistentStorageDelegate;
 @protocol MTROTAProviderDelegate;
 @protocol MTRKeypair;
@@ -37,7 +38,7 @@ NS_ASSUME_NONNULL_BEGIN
  * controllers.  It is used to store persistent information for the fabrics the
  * controllers ends up interacting with.
  */
-@property (nonatomic, strong, readonly) id<MTRPersistentStorageDelegate> storageDelegate;
+@property (nonatomic, strong, readonly) id<MTRStorage> storage MTR_NEWLY_AVAILABLE;
 
 /*
  * OTA Provider delegate to be called when an OTA Requestor is requesting a software update.
@@ -69,7 +70,7 @@ NS_ASSUME_NONNULL_BEGIN
 @property (nonatomic, assign) BOOL startServer;
 
 - (instancetype)init NS_UNAVAILABLE;
-- (instancetype)initWithStorage:(id<MTRPersistentStorageDelegate>)storageDelegate;
+- (instancetype)initWithStorage:(id<MTRStorage>)storage;
 @end
 
 @interface MTRControllerFactory : NSObject
@@ -128,4 +129,9 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
+@interface MTRControllerFactoryParams (Deprecated)
+@property (nonatomic, strong, readonly) id<MTRPersistentStorageDelegate> storageDelegate MTR_NEWLY_DEPRECATED(
+    "Please use the storage property");
+@end
+
 NS_ASSUME_NONNULL_END
diff --git a/src/darwin/Framework/CHIP/MTRControllerFactory.mm b/src/darwin/Framework/CHIP/MTRControllerFactory.mm
@@ -218,7 +218,7 @@ - (BOOL)startup:(MTRControllerFactoryParams *)startupParams
 
         [MTRControllerAccessControl init];
 
-        _persistentStorageDelegateBridge = new MTRPersistentStorageDelegateBridge(startupParams.storageDelegate);
+        _persistentStorageDelegateBridge = new MTRPersistentStorageDelegateBridge(startupParams.storage);
         if (_persistentStorageDelegateBridge == nil) {
             MTR_LOG_ERROR("Error: %@", kErrorPersistentStorageInit);
             return;
@@ -680,13 +680,13 @@ - (MTRPersistentStorageDelegateBridge *)storageDelegateBridge
 
 @implementation MTRControllerFactoryParams
 
-- (instancetype)initWithStorage:(id<MTRPersistentStorageDelegate>)storageDelegate
+- (instancetype)initWithStorage:(id<MTRStorage>)storage
 {
     if (!(self = [super init])) {
         return nil;
     }
 
-    _storageDelegate = storageDelegate;
+    _storage = storage;
     _otaProviderDelegate = nil;
     _paaCerts = nil;
     _cdCerts = nil;
@@ -697,3 +697,15 @@ - (instancetype)initWithStorage:(id<MTRPersistentStorageDelegate>)storageDelegat
 }
 
 @end
+
+@implementation MTRControllerFactoryParams (Deprecated)
+
+- (id<MTRPersistentStorageDelegate>)storageDelegate
+{
+    // Cast is safe, because MTRPersistentStorageDelegate doesn't add
+    // any selectors to MTRStorage, so anything implementing
+    // MTRStorage also implements MTRPersistentStorageDelegate.
+    return static_cast<id<MTRPersistentStorageDelegate>>(self.storage);
+}
+
+@end
diff --git a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.h b/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.h
@@ -15,7 +15,7 @@
  *    limitations under the License.
  */
 
-#import "MTRPersistentStorageDelegate.h"
+#import "MTRStorage.h"
 
 #import "MTRError_Internal.h"
 #include <lib/core/CHIPPersistentStorageDelegate.h>
@@ -25,7 +25,7 @@ NS_ASSUME_NONNULL_BEGIN
 class MTRPersistentStorageDelegateBridge : public chip::PersistentStorageDelegate
 {
 public:
-    MTRPersistentStorageDelegateBridge(id<MTRPersistentStorageDelegate> delegate);
+    MTRPersistentStorageDelegateBridge(id<MTRStorage> delegate);
     ~MTRPersistentStorageDelegateBridge();
 
     CHIP_ERROR SyncGetKeyValue(const char * key, void * buffer, uint16_t & size) override;
@@ -35,7 +35,7 @@ class MTRPersistentStorageDelegateBridge : public chip::PersistentStorageDelegat
     CHIP_ERROR SyncDeleteKeyValue(const char * key) override;
 
 private:
-    id<MTRPersistentStorageDelegate> mDelegate;
+    id<MTRStorage> mDelegate;
     dispatch_queue_t mWorkQueue;
 };
 

diff --git a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.mm b/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.mm
@@ -19,7 +19,7 @@
 
 #define LOG_DEBUG_PERSISTENT_STORAGE_DELEGATE 0
 
-MTRPersistentStorageDelegateBridge::MTRPersistentStorageDelegateBridge(id<MTRPersistentStorageDelegate> delegate)
+MTRPersistentStorageDelegateBridge::MTRPersistentStorageDelegateBridge(id<MTRStorage> delegate)
     : mDelegate(delegate)
     , mWorkQueue(dispatch_queue_create("com.csa.matter.framework.storage.workqueue", DISPATCH_QUEUE_SERIAL))
 {

diff --git a/...ework/CHIP/MTRPersistentStorageDelegate.h → src/darwin/Framework/CHIP/MTRStorage.h b/...ework/CHIP/MTRPersistentStorageDelegate.h → src/darwin/Framework/CHIP/MTRStorage.h
@@ -20,11 +20,13 @@
 NS_ASSUME_NONNULL_BEGIN
 
 /**
- * The protocol definition for the CHIPPersistenStorageDelegate
+ * This protocol is used by the Matter framework to read and write storage.
  *
- * All delegate methods will be called on the supplied Delegate Queue.
+ * The Matter framework may call storage methods from arbitrary threads, but
+ * will not call storage methods concurrently.
  */
-@protocol MTRPersistentStorageDelegate <NSObject>
+MTR_NEWLY_AVAILABLE
+@protocol MTRStorage <NSObject>
 @required
 
 /**
@@ -47,4 +49,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
+MTR_NEWLY_DEPRECATED("Please use MTRStorage")
+@protocol MTRPersistentStorageDelegate <MTRStorage>
+@end
+
 NS_ASSUME_NONNULL_END
diff --git a/src/darwin/Framework/CHIP/Matter.h b/src/darwin/Framework/CHIP/Matter.h
@@ -51,8 +51,8 @@
 #import <Matter/MTROTAHeader.h>
 #import <Matter/MTROTAProviderDelegate.h>
 #import <Matter/MTROnboardingPayloadParser.h>
-#import <Matter/MTRPersistentStorageDelegate.h>
 #import <Matter/MTRQRCodeSetupPayloadParser.h>
 #import <Matter/MTRSetupPayload.h>
+#import <Matter/MTRStorage.h>
 #import <Matter/MTRStructsObjc.h>
 #import <Matter/MTRThreadOperationalDataset.h>
diff --git a/src/darwin/Framework/CHIPTests/MTRTestStorage.h b/src/darwin/Framework/CHIPTests/MTRTestStorage.h
@@ -19,7 +19,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-@interface MTRTestStorage : NSObject <MTRPersistentStorageDelegate>
+@interface MTRTestStorage : NSObject <MTRStorage>
 - (nullable NSData *)storageDataForKey:(NSString *)key;
 - (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key;
 - (BOOL)removeStorageDataForKey:(NSString *)key;

diff --git a/src/darwin/Framework/Matter.xcodeproj/project.pbxproj b/src/darwin/Framework/Matter.xcodeproj/project.pbxproj
@@ -27,7 +27,7 @@
 		2C5EEEF6268A85C400CAE3D3 /* MTRDeviceConnectionBridge.h in Headers */ = {isa = PBXBuildFile; fileRef = 2C5EEEF4268A85C400CAE3D3 /* MTRDeviceConnectionBridge.h */; };
 		2C5EEEF7268A85C400CAE3D3 /* MTRDeviceConnectionBridge.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2C5EEEF5268A85C400CAE3D3 /* MTRDeviceConnectionBridge.mm */; };
 		2C8C8FC0253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.h in Headers */ = {isa = PBXBuildFile; fileRef = 2C8C8FBD253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.h */; };
-		2C8C8FC1253E0C2100797F05 /* MTRPersistentStorageDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = 2C8C8FBE253E0C2100797F05 /* MTRPersistentStorageDelegate.h */; settings = {ATTRIBUTES = (Public, ); }; };
+		2C8C8FC1253E0C2100797F05 /* MTRStorage.h in Headers */ = {isa = PBXBuildFile; fileRef = 2C8C8FBE253E0C2100797F05 /* MTRStorage.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		2C8C8FC2253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2C8C8FBF253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.mm */; };
 		2CB7163B252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.h in Headers */ = {isa = PBXBuildFile; fileRef = 2CB71638252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.h */; };
 		2CB7163C252E8A7C0026E2BB /* MTRDevicePairingDelegateBridge.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2CB71639252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.mm */; };
@@ -167,7 +167,7 @@
 		2C5EEEF4268A85C400CAE3D3 /* MTRDeviceConnectionBridge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDeviceConnectionBridge.h; sourceTree = "<group>"; };
 		2C5EEEF5268A85C400CAE3D3 /* MTRDeviceConnectionBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDeviceConnectionBridge.mm; sourceTree = "<group>"; };
 		2C8C8FBD253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRPersistentStorageDelegateBridge.h; sourceTree = "<group>"; };
-		2C8C8FBE253E0C2100797F05 /* MTRPersistentStorageDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRPersistentStorageDelegate.h; sourceTree = "<group>"; };
+		2C8C8FBE253E0C2100797F05 /* MTRStorage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRStorage.h; sourceTree = "<group>"; };
 		2C8C8FBF253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRPersistentStorageDelegateBridge.mm; sourceTree = "<group>"; };
 		2CB71638252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDevicePairingDelegateBridge.h; sourceTree = "<group>"; };
 		2CB71639252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDevicePairingDelegateBridge.mm; sourceTree = "<group>"; };
@@ -389,7 +389,7 @@
 				7596A84C287782E8004DAE0E /* MTRAsyncCallbackWorkQueue_Internal.h */,
 				7596A84628762783004DAE0E /* MTRAsyncCallbackWorkQueue.h */,
 				7596A84728762783004DAE0E /* MTRAsyncCallbackWorkQueue.mm */,
-				2C8C8FBE253E0C2100797F05 /* MTRPersistentStorageDelegate.h */,
+				2C8C8FBE253E0C2100797F05 /* MTRStorage.h */,
 				5ACDDD7927CD129700EFD68A /* MTRAttributeCacheContainer.h */,
 				5A60370727EA1FF60020DB79 /* MTRAttributeCacheContainer+XPC.h */,
 				5ACDDD7B27CD14AF00EFD68A /* MTRAttributeCacheContainer_Internal.h */,
@@ -544,7 +544,7 @@
 				1ED276E426C5832500547A89 /* MTRCluster.h in Headers */,
 				5A6FEC9A27B5C89300F25F42 /* MTRDeviceControllerXPCConnection.h in Headers */,
 				5129BCFD26A9EE3300122DDF /* MTRError.h in Headers */,
-				2C8C8FC1253E0C2100797F05 /* MTRPersistentStorageDelegate.h in Headers */,
+				2C8C8FC1253E0C2100797F05 /* MTRStorage.h in Headers */,
 				AF1CB8702874B04C00865A96 /* MTROTAProviderDelegateBridge.h in Headers */,
 				B2E0D7B5245B0B5C003C5B48 /* MTRQRCodeSetupPayloadParser.h in Headers */,
 				1EC4CE6425CC276600D7304F /* MTRBaseClusters.h in Headers */,