Skip to content

Commit

Permalink
Enable persistent storage on Darwin in Server.h. (#14342)
Browse files Browse the repository at this point in the history
There were two remaining issues in the Darwin KVS impl that needed
to be fixed to get this to pass:

1) The errors returned did not match the API documentation, which
   caused some consumers to fail.  The most important problem here was
   returning CHIP_ERROR_KEY_NOT_FOUND instead of
   CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND from _Get, but I went
   ahead and aligned the other error cases (not initialized, buffer
   passed to _Get too small, deleting unknown key, failures to save to
   persistent storage) with the API documentation.

2) In _Put, if the item already existed the change of "value" was not
   being picked up by the NSManagedObject machinery, so updating the
   value of an already-existing key did not work.  The fix for this is
   to use @dynamic instead of @synthesize for the properties of
   KeyValueItem (shamelessly copy/pasting from the documentation at
   https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CoreData/LifeofaManagedObject.html),
   which I _think_ causes the getter/setter to be provided by the
   NSMnagedObject bits, which then know about the set we perform and
   know to update the data store.

Fixes #12174
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Aug 29, 2023
1 parent 0d5812d commit 9201620
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 25 deletions.
7 changes: 0 additions & 7 deletions src/app/server/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ config("server_config") {
if (chip_enable_group_messaging_tests) {
defines += [ "CHIP_ENABLE_GROUP_MESSAGING_TESTS" ]
}

if (current_os == "mac" || current_os == "ios") {
# Using Non persistent storage delegate since
# persistence is also disable in Unit test for linux and MacOS
# Issue #12174 for Darwin
defines += [ "CHIP_USE_NON_PERSISTENT_STORAGE_DELEGATE" ]
}
}

static_library("server") {
Expand Down
7 changes: 0 additions & 7 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <inet/InetConfig.h>
#include <lib/core/CHIPConfig.h>
#include <lib/support/SafeInt.h>
#include <lib/support/TestPersistentStorageDelegate.h>
#include <messaging/ExchangeMgr.h>
#include <platform/KeyValueStoreManager.h>
#include <protocols/secure_channel/CASEServer.h>
Expand Down Expand Up @@ -190,13 +189,7 @@ class Server

// Both PersistentStorageDelegate, and GroupDataProvider should be injected by the applications
// See: https://github.com/project-chip/connectedhomeip/issues/12276
// Currently, the GroupDataProvider cannot use KeyValueStoreMgr() due to
// (https://github.com/project-chip/connectedhomeip/issues/12174)
#ifdef CHIP_USE_NON_PERSISTENT_STORAGE_DELEGATE
TestPersistentStorageDelegate mDeviceStorage;
#else
DeviceStorageDelegate mDeviceStorage;
#endif
Credentials::GroupDataProviderImpl mGroupsProvider;
app::DefaultAttributePersistenceProvider mAttributePersister;
GroupDataProviderListener mListener;
Expand Down
42 changes: 31 additions & 11 deletions src/platform/Darwin/KeyValueStoreManagerImpl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,14 @@ @interface KeyValueItem : NSManagedObject

@implementation KeyValueItem

@synthesize key;
@synthesize value;
@dynamic key;
@dynamic value;

- (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context
key:(nonnull NSString *)key_
value:(nonnull NSData *)value_
- (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context key:(nonnull NSString *)key value:(nonnull NSData *)value
{
if (self = [super initWithContext:context]) {
key = key_;
value = value_;
self.key = key;
self.value = value;
}
return self;
}
Expand Down Expand Up @@ -209,10 +207,11 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context
{
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorCodeIf(offset != 0, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_WELL_UNINITIALIZED);

KeyValueItem * item = FindItemForKey([[NSString alloc] initWithUTF8String:key], nil, true);
if (!item) {
return CHIP_ERROR_KEY_NOT_FOUND;
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}

if (read_bytes_size != nullptr) {
Expand All @@ -221,6 +220,17 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context

if (value != nullptr) {
memcpy(value, item.value.bytes, std::min<size_t>((item.value.length), value_size));
#if CHIP_CONFIG_DARWIN_STORAGE_VERBOSE_LOGGING
fprintf(stderr, "GETTING VALUE FOR: '%s': ", key);
for (size_t i = 0; i < std::min<size_t>((item.value.length), value_size); ++i) {
fprintf(stderr, "%02x ", static_cast<uint8_t *>(value)[i]);
}
fprintf(stderr, "\n");
#endif
}

if (item.value.length > value_size) {
return CHIP_ERROR_BUFFER_TOO_SMALL;
}

return CHIP_NO_ERROR;
Expand All @@ -229,10 +239,11 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context
CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key)
{
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_WELL_UNINITIALIZED);

KeyValueItem * item = FindItemForKey([[NSString alloc] initWithUTF8String:key], nil);
if (!item) {
return CHIP_NO_ERROR;
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
}

__block BOOL success = NO;
Expand All @@ -244,7 +255,7 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context

if (!success) {
ChipLogError(DeviceLayer, "Error saving context: %s", error.localizedDescription.UTF8String);
return CHIP_ERROR_INTERNAL;
return CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}

return CHIP_NO_ERROR;
Expand All @@ -253,6 +264,7 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context
CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size)
{
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_WELL_UNINITIALIZED);

NSData * data = [[NSData alloc] initWithBytes:value length:value_size];

Expand All @@ -274,9 +286,17 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context

if (!success) {
ChipLogError(DeviceLayer, "Error saving context: %s", error.localizedDescription.UTF8String);
return CHIP_ERROR_INTERNAL;
return CHIP_ERROR_PERSISTED_STORAGE_FAILED;
}

#if CHIP_CONFIG_DARWIN_STORAGE_VERBOSE_LOGGING
fprintf(stderr, "PUT VALUE FOR: '%s': ", key);
for (size_t i = 0; i < value_size; ++i) {
fprintf(stderr, "%02x ", static_cast<const uint8_t *>(value)[i]);
}
fprintf(stderr, "\n");
#endif

return CHIP_NO_ERROR;
}

Expand Down

0 comments on commit 9201620

Please sign in to comment.