From c0bc38681188ce86d7f79538eb083dd405fa0819 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 14 Sep 2022 08:52:02 -0400 Subject: [PATCH] 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. --- .../common/CHIPCommandStorageDelegate.h | 2 +- .../Framework/CHIP/MTRControllerFactory.h | 10 ++++++++-- .../Framework/CHIP/MTRControllerFactory.mm | 18 +++++++++++++++--- .../CHIP/MTRPersistentStorageDelegateBridge.h | 6 +++--- .../CHIP/MTRPersistentStorageDelegateBridge.mm | 2 +- ...ersistentStorageDelegate.h => MTRStorage.h} | 12 +++++++++--- src/darwin/Framework/CHIP/Matter.h | 2 +- .../Framework/CHIPTests/MTRTestStorage.h | 2 +- .../Framework/Matter.xcodeproj/project.pbxproj | 8 ++++---- 9 files changed, 43 insertions(+), 19 deletions(-) rename src/darwin/Framework/CHIP/{MTRPersistentStorageDelegate.h => MTRStorage.h} (76%) 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/Framework/CHIP/MTRControllerFactory.h b/src/darwin/Framework/CHIP/MTRControllerFactory.h index 508cf369374bae..efb2edd36670c0 100644 --- 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 storageDelegate; +@property (nonatomic, strong, readonly) id 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)storageDelegate; +- (instancetype)initWithStorage:(id)storage; @end @interface MTRControllerFactory : NSObject @@ -128,4 +129,9 @@ NS_ASSUME_NONNULL_BEGIN @end +@interface MTRControllerFactoryParams (Deprecated) +@property (nonatomic, strong, readonly) id 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 index adf826141ee3ba..0b6926e08ad4b1 100644 --- 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)storageDelegate +- (instancetype)initWithStorage:(id)storage { if (!(self = [super init])) { return nil; } - _storageDelegate = storageDelegate; + _storage = storage; _otaProviderDelegate = nil; _paaCerts = nil; _cdCerts = nil; @@ -697,3 +697,15 @@ - (instancetype)initWithStorage:(id)storageDelegat } @end + +@implementation MTRControllerFactoryParams (Deprecated) + +- (id)storageDelegate +{ + // Cast is safe, because MTRPersistentStorageDelegate doesn't add + // any selectors to MTRStorage, so anything implementing + // MTRStorage also implements MTRPersistentStorageDelegate. + return static_cast>(self.storage); +} + +@end 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 37993200e6251f..fdcde68d6799cc 100644 --- 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 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 76% rename from src/darwin/Framework/CHIP/MTRPersistentStorageDelegate.h rename to src/darwin/Framework/CHIP/MTRStorage.h index 248df57d67b60d..d3aa896f2ab187 100644 --- a/src/darwin/Framework/CHIP/MTRPersistentStorageDelegate.h +++ b/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. + * 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 +MTR_NEWLY_AVAILABLE +@protocol MTRStorage @required /** @@ -47,4 +49,8 @@ NS_ASSUME_NONNULL_BEGIN @end +MTR_NEWLY_DEPRECATED("Please use MTRStorage") +@protocol MTRPersistentStorageDelegate +@end + NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/Matter.h b/src/darwin/Framework/CHIP/Matter.h index 4f06990c07cfb3..35932dbdc4839e 100644 --- a/src/darwin/Framework/CHIP/Matter.h +++ b/src/darwin/Framework/CHIP/Matter.h @@ -51,8 +51,8 @@ #import #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 0a60446632427c..049cd5993aca6c 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 */, @@ -541,7 +541,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 */,