From aad98950e0b75f66a4b1ce0c0b427c88eb91f241 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 13 Sep 2022 22:16:54 -0400 Subject: [PATCH] Apply API review fixes for MTRPersistentStorageDelegate.h * Rename the protocol (and header). * Fix some comments. Fixes https://github.com/project-chip/connectedhomeip/issues/22542 Addresses part of https://github.com/project-chip/connectedhomeip/issues/22420 --- .../commands/common/CHIPCommandStorageDelegate.h | 2 +- .../CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h | 2 +- .../CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m | 4 ++-- src/darwin/Framework/CHIP/MTRControllerFactory.h | 6 +++--- src/darwin/Framework/CHIP/MTRControllerFactory.mm | 6 +++--- .../Framework/CHIP/MTRPersistentStorageDelegateBridge.h | 6 +++--- .../Framework/CHIP/MTRPersistentStorageDelegateBridge.mm | 2 +- .../CHIP/{MTRPersistentStorageDelegate.h => MTRStorage.h} | 7 ++++--- src/darwin/Framework/CHIP/Matter.h | 2 +- src/darwin/Framework/CHIPTests/MTRTestStorage.h | 2 +- src/darwin/Framework/Matter.xcodeproj/project.pbxproj | 8 ++++---- 11 files changed, 24 insertions(+), 23 deletions(-) rename src/darwin/Framework/CHIP/{MTRPersistentStorageDelegate.h => MTRStorage.h} (82%) diff --git a/examples/darwin-framework-tool/commands/common/CHIPCommandStorageDelegate.h b/examples/darwin-framework-tool/commands/common/CHIPCommandStorageDelegate.h index c2f7ba220e9e50..d5f743d57673c5 100644 --- 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 +@interface CHIPToolPersistentStorageDelegate : NSObject - (nullable NSData *)storageDataForKey:(NSString *)key; - (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key; - (BOOL)removeStorageDataForKey:(NSString *)key; diff --git a/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h b/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h index e4f114bc4ab869..36436bd76a0154 100644 --- a/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h +++ b/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.h @@ -41,7 +41,7 @@ BOOL MTRGetConnectedDeviceWithID(uint64_t deviceId, MTRDeviceConnectionCallback void MTRUnpairDeviceWithID(uint64_t deviceId); MTRBaseDevice * _Nullable MTRGetDeviceBeingCommissioned(void); -@interface CHIPToolPersistentStorageDelegate : NSObject +@interface CHIPToolPersistentStorageDelegate : NSObject - (nullable NSData *)storageDataForKey:(NSString *)key; - (BOOL)setStorageData:(NSData *)value forKey:(NSString *)key; - (BOOL)removeStorageDataForKey:(NSString *)key; diff --git a/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m b/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m index 49578370a62f63..18cb9544c10932 100644 --- a/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m +++ b/src/darwin/CHIPTool/CHIPTool/Framework Helpers/DefaultsUtils.m @@ -205,12 +205,12 @@ void MTRUnpairDeviceWithID(uint64_t deviceId) @implementation CHIPToolPersistentStorageDelegate -// MARK: MTRPersistentStorageDelegate +// MARK: MTRStorage - (nullable NSData *)storageDataForKey:(NSString *)key { NSData * value = MTRGetDomainValueForKey(MTRToolDefaultsDomain, key); - NSLog(@"MTRPersistentStorageDelegate Get Value for Key: %@, value %@", key, value); + NSLog(@"MTRStorage Get Value for Key: %@, value %@", key, value); return value; } diff --git a/src/darwin/Framework/CHIP/MTRControllerFactory.h b/src/darwin/Framework/CHIP/MTRControllerFactory.h index 508cf369374bae..3a9a3d609955f2 100644 --- a/src/darwin/Framework/CHIP/MTRControllerFactory.h +++ b/src/darwin/Framework/CHIP/MTRControllerFactory.h @@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN -@protocol MTRPersistentStorageDelegate; +@protocol MTRStorage; @protocol MTROTAProviderDelegate; @protocol MTRKeypair; @@ -37,7 +37,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 storageDelegate; +@property (nonatomic, strong, readonly) id storage; /* * OTA Provider delegate to be called when an OTA Requestor is requesting a software update. @@ -69,7 +69,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign) BOOL startServer; - (instancetype)init NS_UNAVAILABLE; -- (instancetype)initWithStorage:(id)storageDelegate; +- (instancetype)initWithStorage:(id)storage; @end @interface MTRControllerFactory : NSObject diff --git a/src/darwin/Framework/CHIP/MTRControllerFactory.mm b/src/darwin/Framework/CHIP/MTRControllerFactory.mm index d44a6202386e6b..307e2601b931fb 100644 --- a/src/darwin/Framework/CHIP/MTRControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRControllerFactory.mm @@ -216,7 +216,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; @@ -620,13 +620,13 @@ - (MTRPersistentStorageDelegateBridge *)storageDelegateBridge @implementation MTRControllerFactoryParams -- (instancetype)initWithStorage:(id)storageDelegate +- (instancetype)initWithStorage:(id)storage { if (!(self = [super init])) { return nil; } - _storageDelegate = storageDelegate; + _storage = storage; _otaProviderDelegate = nil; _paaCerts = nil; _cdCerts = nil; diff --git a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.h b/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.h index dfa210c74e31ae..6b5f13452182e9 100644 --- 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 @@ -25,7 +25,7 @@ NS_ASSUME_NONNULL_BEGIN class MTRPersistentStorageDelegateBridge : public chip::PersistentStorageDelegate { public: - MTRPersistentStorageDelegateBridge(id delegate); + MTRPersistentStorageDelegateBridge(id 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 mDelegate; + id mDelegate; dispatch_queue_t mWorkQueue; }; diff --git a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.mm b/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.mm index cf74e08ab028df..bc14ad1020cafe 100644 --- a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.mm +++ b/src/darwin/Framework/CHIP/MTRPersistentStorageDelegateBridge.mm @@ -17,7 +17,7 @@ #import "MTRPersistentStorageDelegateBridge.h" -MTRPersistentStorageDelegateBridge::MTRPersistentStorageDelegateBridge(id delegate) +MTRPersistentStorageDelegateBridge::MTRPersistentStorageDelegateBridge(id delegate) : mDelegate(delegate) , mWorkQueue(dispatch_queue_create("com.csa.matter.framework.storage.workqueue", DISPATCH_QUEUE_SERIAL)) { diff --git a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegate.h b/src/darwin/Framework/CHIP/MTRStorage.h similarity index 82% rename from src/darwin/Framework/CHIP/MTRPersistentStorageDelegate.h rename to src/darwin/Framework/CHIP/MTRStorage.h index e1e36eaaebac2e..2dcba9d330a8c7 100644 --- a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegate.h +++ b/src/darwin/Framework/CHIP/MTRStorage.h @@ -20,11 +20,12 @@ 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. + * All storage methods will be called on a single internal work queue (so the + * implementation does not need to worry about reads and writes racing). */ -@protocol MTRPersistentStorageDelegate +@protocol MTRStorage @required /** diff --git a/src/darwin/Framework/CHIP/Matter.h b/src/darwin/Framework/CHIP/Matter.h index c2040c08a882c4..f9931415d2ecc5 100644 --- a/src/darwin/Framework/CHIP/Matter.h +++ b/src/darwin/Framework/CHIP/Matter.h @@ -41,7 +41,7 @@ #import #import #import -#import #import +#import #import #import diff --git a/src/darwin/Framework/CHIPTests/MTRTestStorage.h b/src/darwin/Framework/CHIPTests/MTRTestStorage.h index 9f8069ccb2a083..ba5e386be23b8c 100644 --- 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 +@interface MTRTestStorage : NSObject - (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 index d643e28707c3f0..26903620532b9f 100644 --- 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 */; }; @@ -166,7 +166,7 @@ 2C5EEEF4268A85C400CAE3D3 /* MTRDeviceConnectionBridge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDeviceConnectionBridge.h; sourceTree = ""; }; 2C5EEEF5268A85C400CAE3D3 /* MTRDeviceConnectionBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDeviceConnectionBridge.mm; sourceTree = ""; }; 2C8C8FBD253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRPersistentStorageDelegateBridge.h; sourceTree = ""; }; - 2C8C8FBE253E0C2100797F05 /* MTRPersistentStorageDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRPersistentStorageDelegate.h; sourceTree = ""; }; + 2C8C8FBE253E0C2100797F05 /* MTRStorage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRStorage.h; sourceTree = ""; }; 2C8C8FBF253E0C2100797F05 /* MTRPersistentStorageDelegateBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRPersistentStorageDelegateBridge.mm; sourceTree = ""; }; 2CB71638252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MTRDevicePairingDelegateBridge.h; sourceTree = ""; }; 2CB71639252E8A7B0026E2BB /* MTRDevicePairingDelegateBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MTRDevicePairingDelegateBridge.mm; sourceTree = ""; }; @@ -387,7 +387,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 */, @@ -542,7 +542,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 */, 1EC4CE6425CC276600D7304F /* MTRBaseClusters.h in Headers */, 2C5EEEF6268A85C400CAE3D3 /* MTRDeviceConnectionBridge.h in Headers */,