Split DefaultStorageKeyAllocator into Keys and Allocator (#23422)

* Separate out key allocation from storage keys. Make storage keys generally const-safe and immutable

* Convert a lot of usage of default storage key allocator into using the new format. Persisted counter is the only one remaining

* Fix some compilations

* More compile fixes. Minmdns compiles

* Restyle

* Fix binding table test to compile

* One more compile fix for tests

* ran `rg '\WStorageKey\W' | grep -v third_party/ | cut -f1 -d: | uniq | xargs sd '(\W)StorageKey(\W)' '${1}StorageKeyName${2}'\`

* Restyle

* Also update StorageKey at the beginning of the line

examples/platform/linux/testing/CustomCSRResponseOperationalKeyStore.cpp

@@ -68,11 +68,10 @@ CHIP_ERROR CustomCSRResponseOperationalKeyStore::ReuseOpKeypair(FabricIndex fabr
 
  // Load up the operational key structure from storage
  uint16_t size = static_cast<uint16_t>(buf.Capacity());
- DefaultStorageKeyAllocator keyAlloc;
 
  // In order to retrieve a keypair that has already been registered, assume the device
  // as already been commissioned and fabric index 1 is the registered fabric.
- CHIP_ERROR err = mStorage->SyncGetKeyValue(keyAlloc.FabricOpKey(1 /* fabricIndex */), buf.Bytes(), size);
+ CHIP_ERROR err = mStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::FabricOpKey(1 /* fabricIndex */), buf.Bytes(), size);
  if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
  {
  err = CHIP_ERROR_INVALID_FABRIC_INDEX;

examples/providers/DeviceInfoProviderImpl.cpp

@@ -120,24 +120,21 @@ bool DeviceInfoProviderImpl::FixedLabelIteratorImpl::Next(FixedLabelType & outpu
 
 CHIP_ERROR DeviceInfoProviderImpl::SetUserLabelLength(EndpointId endpoint, size_t val)
 {
- DefaultStorageKeyAllocator keyAlloc;
-
- return mStorage->SyncSetKeyValue(keyAlloc.UserLabelLengthKey(endpoint), &val, static_cast<uint16_t>(sizeof(val)));
+ return mStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::UserLabelLengthKey(endpoint), &val,
+ static_cast<uint16_t>(sizeof(val)));
 }
 
 CHIP_ERROR DeviceInfoProviderImpl::GetUserLabelLength(EndpointId endpoint, size_t & val)
 {
- DefaultStorageKeyAllocator keyAlloc;
  uint16_t len = static_cast<uint16_t>(sizeof(val));
 
- return mStorage->SyncGetKeyValue(keyAlloc.UserLabelLengthKey(endpoint), &val, len);
+ return mStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::UserLabelLengthKey(endpoint), &val, len);
 }
 
 CHIP_ERROR DeviceInfoProviderImpl::SetUserLabelAt(EndpointId endpoint, size_t index, const UserLabelType & userLabel)
 {
  VerifyOrReturnError(CanCastTo<uint32_t>(index), CHIP_ERROR_INVALID_ARGUMENT);
 
- DefaultStorageKeyAllocator keyAlloc;
  uint8_t buf[UserLabelTLVMaxSize()];
  TLV::TLVWriter writer;
  writer.Init(buf);
@@ -148,15 +145,13 @@ CHIP_ERROR DeviceInfoProviderImpl::SetUserLabelAt(EndpointId endpoint, size_t in
  ReturnErrorOnFailure(writer.PutString(kLabelValueTag, userLabel.value));
  ReturnErrorOnFailure(writer.EndContainer(outerType));
 
- return mStorage->SyncSetKeyValue(keyAlloc.UserLabelIndexKey(endpoint, static_cast<uint32_t>(index)), buf,
+ return mStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::UserLabelIndexKey(endpoint, static_cast<uint32_t>(index)), buf,
  static_cast<uint16_t>(writer.GetLengthWritten()));
 }
 
 CHIP_ERROR DeviceInfoProviderImpl::DeleteUserLabelAt(EndpointId endpoint, size_t index)
 {
- DefaultStorageKeyAllocator keyAlloc;
-
- return mStorage->SyncDeleteKeyValue(keyAlloc.UserLabelIndexKey(endpoint, static_cast<uint32_t>(index)));
+ return mStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::UserLabelIndexKey(endpoint, static_cast<uint32_t>(index)));
 }
 
 DeviceInfoProvider::UserLabelIterator * DeviceInfoProviderImpl::IterateUserLabel(EndpointId endpoint)
@@ -181,11 +176,11 @@ bool DeviceInfoProviderImpl::UserLabelIteratorImpl::Next(UserLabelType & output)
  VerifyOrReturnError(mIndex < mTotal, false);
  VerifyOrReturnError(CanCastTo<uint32_t>(mIndex), false);
 
- DefaultStorageKeyAllocator keyAlloc;
  uint8_t buf[UserLabelTLVMaxSize()];
  uint16_t len = static_cast<uint16_t>(sizeof(buf));
 
- err = mProvider.mStorage->SyncGetKeyValue(keyAlloc.UserLabelIndexKey(mEndpoint, static_cast<uint32_t>(mIndex)), buf, len);
+ err = mProvider.mStorage->SyncGetKeyValue(
+ DefaultStorageKeyAllocator::UserLabelIndexKey(mEndpoint, static_cast<uint32_t>(mIndex)), buf, len);
  VerifyOrReturnError(err == CHIP_NO_ERROR, false);
 
  TLV::ContiguousBufferTLVReader reader;

src/app/DefaultAttributePersistenceProvider.cpp

@@ -30,24 +30,23 @@ CHIP_ERROR DefaultAttributePersistenceProvider::WriteValue(const ConcreteAttribu
  // TODO: we may want to have a small cache for values that change a lot, so
  // we only write them once a bunch of changes happen or on timer or
  // shutdown.
- DefaultStorageKeyAllocator key;
  if (!CanCastTo<uint16_t>(aValue.size()))
  {
  return CHIP_ERROR_BUFFER_TOO_SMALL;
  }
- return mStorage->SyncSetKeyValue(key.AttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), aValue.data(),
- static_cast<uint16_t>(aValue.size()));
+ return mStorage->SyncSetKeyValue(
+ DefaultStorageKeyAllocator::AttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), aValue.data(),
+ static_cast<uint16_t>(aValue.size()));
 }
 
 CHIP_ERROR DefaultAttributePersistenceProvider::ReadValue(const ConcreteAttributePath & aPath,
  const EmberAfAttributeMetadata * aMetadata, MutableByteSpan & aValue)
 {
  VerifyOrReturnError(mStorage != nullptr, CHIP_ERROR_INCORRECT_STATE);
 
- DefaultStorageKeyAllocator key;
  uint16_t size = static_cast<uint16_t>(min(aValue.size(), static_cast<size_t>(UINT16_MAX)));
- ReturnErrorOnFailure(mStorage->SyncGetKeyValue(key.AttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId),
-   aValue.data(), size));
+ ReturnErrorOnFailure(mStorage->SyncGetKeyValue(
+ DefaultStorageKeyAllocator::AttributeValue(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId), aValue.data(), size));
  EmberAfAttributeType type = aMetadata->attributeType;
  if (emberAfIsStringAttributeType(type))
  {

src/app/clusters/access-control-server/access-control-server.cpp

@@ -192,16 +192,15 @@ CHIP_ERROR AccessControlAttribute::ReadAcl(AttributeValueEncoder & aEncoder)
 CHIP_ERROR AccessControlAttribute::ReadExtension(AttributeValueEncoder & aEncoder)
 {
  auto & storage = Server::GetInstance().GetPersistentStorage();
- DefaultStorageKeyAllocator key;
-
  auto & fabrics = Server::GetInstance().GetFabricTable();
 
  return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR {
  for (auto & fabric : fabrics)
  {
  uint8_t buffer[kExtensionDataMaxLength] = { 0 };
  uint16_t size = static_cast<uint16_t>(sizeof(buffer));
- CHIP_ERROR errStorage = storage.SyncGetKeyValue(key.AccessControlExtensionEntry(fabric.GetFabricIndex()), buffer, size);
+ CHIP_ERROR errStorage = storage.SyncGetKeyValue(
+ DefaultStorageKeyAllocator::AccessControlExtensionEntry(fabric.GetFabricIndex()), buffer, size);
  ReturnErrorCodeIf(errStorage == CHIP_ERROR_BUFFER_TOO_SMALL, CHIP_ERROR_INCORRECT_STATE);
  if (errStorage == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
  {
@@ -295,13 +294,13 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP
 CHIP_ERROR AccessControlAttribute::WriteExtension(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
 {
  auto & storage = Server::GetInstance().GetPersistentStorage();
- DefaultStorageKeyAllocator key;
 
  FabricIndex accessingFabricIndex = aDecoder.AccessingFabricIndex();
 
  uint8_t buffer[kExtensionDataMaxLength] = { 0 };
  uint16_t size = static_cast<uint16_t>(sizeof(buffer));
- CHIP_ERROR errStorage = storage.SyncGetKeyValue(key.AccessControlExtensionEntry(accessingFabricIndex), buffer, size);
+ CHIP_ERROR errStorage =
+ storage.SyncGetKeyValue(DefaultStorageKeyAllocator::AccessControlExtensionEntry(accessingFabricIndex), buffer, size);
  ReturnErrorCodeIf(errStorage == CHIP_ERROR_BUFFER_TOO_SMALL, CHIP_ERROR_INCORRECT_STATE);
  ReturnErrorCodeIf(errStorage != CHIP_NO_ERROR && errStorage != CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND, errStorage);
 
@@ -316,7 +315,8 @@ CHIP_ERROR AccessControlAttribute::WriteExtension(const ConcreteDataAttributePat
  if (count == 0)
  {
  ReturnErrorCodeIf(errStorage == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND, CHIP_NO_ERROR);
- ReturnErrorOnFailure(storage.SyncDeleteKeyValue(key.AccessControlExtensionEntry(accessingFabricIndex)));
+ ReturnErrorOnFailure(
+ storage.SyncDeleteKeyValue(DefaultStorageKeyAllocator::AccessControlExtensionEntry(accessingFabricIndex)));
  AccessControlCluster::Structs::ExtensionEntry::Type item = {
  .data = ByteSpan(buffer, size),
  .fabricIndex = accessingFabricIndex,
@@ -339,8 +339,9 @@ CHIP_ERROR AccessControlAttribute::WriteExtension(const ConcreteDataAttributePat
 
  ReturnErrorOnFailure(CheckExtensionEntryDataFormat(item.data));
 
- ReturnErrorOnFailure(storage.SyncSetKeyValue(key.AccessControlExtensionEntry(accessingFabricIndex), item.data.data(),
- static_cast<uint16_t>(item.data.size())));
+ ReturnErrorOnFailure(
+ storage.SyncSetKeyValue(DefaultStorageKeyAllocator::AccessControlExtensionEntry(accessingFabricIndex),
+ item.data.data(), static_cast<uint16_t>(item.data.size())));
  ReturnErrorOnFailure(LogExtensionChangedEvent(item, aDecoder.GetSubjectDescriptor(),
  errStorage == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND
  ? AccessControlCluster::ChangeTypeEnum::kAdded
@@ -361,8 +362,8 @@ CHIP_ERROR AccessControlAttribute::WriteExtension(const ConcreteDataAttributePat
 
  ReturnErrorOnFailure(CheckExtensionEntryDataFormat(item.data));
 
- ReturnErrorOnFailure(storage.SyncSetKeyValue(key.AccessControlExtensionEntry(accessingFabricIndex), item.data.data(),
- static_cast<uint16_t>(item.data.size())));
+ ReturnErrorOnFailure(storage.SyncSetKeyValue(DefaultStorageKeyAllocator::AccessControlExtensionEntry(accessingFabricIndex),
+ item.data.data(), static_cast<uint16_t>(item.data.size())));
  ReturnErrorOnFailure(
  LogExtensionChangedEvent(item, aDecoder.GetSubjectDescriptor(), AccessControlCluster::ChangeTypeEnum::kAdded));
  }

src/app/clusters/ota-requestor/DefaultOTARequestorStorage.cpp

@@ -40,7 +40,6 @@ constexpr size_t kProviderListMaxSerializedSize = kProviderMaxSerializedSize * C
 
 CHIP_ERROR DefaultOTARequestorStorage::StoreDefaultProviders(const ProviderLocationList & providers)
 {
- DefaultStorageKeyAllocator key;
  uint8_t buffer[kProviderListMaxSerializedSize];
  TLV::TLVWriter writer;
  TLV::TLVType outerType;
@@ -56,16 +55,16 @@ CHIP_ERROR DefaultOTARequestorStorage::StoreDefaultProviders(const ProviderLocat
 
  ReturnErrorOnFailure(writer.EndContainer(outerType));
 
- return mPersistentStorage->SyncSetKeyValue(key.OTADefaultProviders(), buffer, static_cast<uint16_t>(writer.GetLengthWritten()));
+ return mPersistentStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::OTADefaultProviders(), buffer,
+ static_cast<uint16_t>(writer.GetLengthWritten()));
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::LoadDefaultProviders(ProviderLocationList & providers)
 {
- DefaultStorageKeyAllocator key;
  uint8_t buffer[kProviderListMaxSerializedSize];
  MutableByteSpan bufferSpan(buffer);
 
- ReturnErrorOnFailure(Load(key.OTADefaultProviders(), bufferSpan));
+ ReturnErrorOnFailure(Load(DefaultStorageKeyAllocator::OTADefaultProviders(), bufferSpan));
 
  TLV::TLVReader reader;
  TLV::TLVType outerType;
@@ -88,30 +87,27 @@ CHIP_ERROR DefaultOTARequestorStorage::LoadDefaultProviders(ProviderLocationList
 
 CHIP_ERROR DefaultOTARequestorStorage::StoreCurrentProviderLocation(const ProviderLocationType & provider)
 {
- DefaultStorageKeyAllocator key;
  uint8_t buffer[kProviderMaxSerializedSize];
  TLV::TLVWriter writer;
 
  writer.Init(buffer);
  ReturnErrorOnFailure(provider.EncodeForRead(writer, TLV::AnonymousTag(), provider.fabricIndex));
 
- return mPersistentStorage->SyncSetKeyValue(key.OTACurrentProvider(), buffer, static_cast<uint16_t>(writer.GetLengthWritten()));
+ return mPersistentStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::OTACurrentProvider(), buffer,
+ static_cast<uint16_t>(writer.GetLengthWritten()));
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::ClearCurrentProviderLocation()
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncDeleteKeyValue(key.OTACurrentProvider());
+ return mPersistentStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::OTACurrentProvider());
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::LoadCurrentProviderLocation(ProviderLocationType & provider)
 {
- DefaultStorageKeyAllocator key;
  uint8_t buffer[kProviderMaxSerializedSize];
  MutableByteSpan bufferSpan(buffer);
 
- ReturnErrorOnFailure(Load(key.OTACurrentProvider(), bufferSpan));
+ ReturnErrorOnFailure(Load(DefaultStorageKeyAllocator::OTACurrentProvider(), bufferSpan));
 
  TLV::TLVReader reader;
 
@@ -124,67 +120,54 @@ CHIP_ERROR DefaultOTARequestorStorage::LoadCurrentProviderLocation(ProviderLocat
 
 CHIP_ERROR DefaultOTARequestorStorage::StoreUpdateToken(ByteSpan updateToken)
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncSetKeyValue(key.OTAUpdateToken(), updateToken.data(), static_cast<uint16_t>(updateToken.size()));
+ return mPersistentStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::OTAUpdateToken(), updateToken.data(),
+ static_cast<uint16_t>(updateToken.size()));
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::ClearUpdateToken()
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncDeleteKeyValue(key.OTAUpdateToken());
+ return mPersistentStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::OTAUpdateToken());
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::LoadUpdateToken(MutableByteSpan & updateToken)
 {
- DefaultStorageKeyAllocator key;
-
- return Load(key.OTAUpdateToken(), updateToken);
+ return Load(DefaultStorageKeyAllocator::OTAUpdateToken(), updateToken);
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::StoreCurrentUpdateState(OTAUpdateStateEnum currentUpdateState)
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncSetKeyValue(key.OTACurrentUpdateState(), &currentUpdateState, sizeof(currentUpdateState));
+ return mPersistentStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::OTACurrentUpdateState(), &currentUpdateState,
+ sizeof(currentUpdateState));
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::LoadCurrentUpdateState(OTAUpdateStateEnum & currentUpdateState)
 {
- DefaultStorageKeyAllocator key;
  uint16_t size = static_cast<uint16_t>(sizeof(currentUpdateState));
 
- return mPersistentStorage->SyncGetKeyValue(key.OTACurrentUpdateState(), &currentUpdateState, size);
+ return mPersistentStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::OTACurrentUpdateState(), &currentUpdateState, size);
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::ClearCurrentUpdateState()
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncDeleteKeyValue(key.OTACurrentUpdateState());
+ return mPersistentStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::OTACurrentUpdateState());
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::StoreTargetVersion(uint32_t targetVersion)
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncSetKeyValue(key.OTATargetVersion(), &targetVersion, sizeof(targetVersion));
+ return mPersistentStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::OTATargetVersion(), &targetVersion,
+ sizeof(targetVersion));
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::LoadTargetVersion(uint32_t & targetVersion)
 {
- DefaultStorageKeyAllocator key;
  uint16_t size = static_cast<uint16_t>(sizeof(targetVersion));
 
- return mPersistentStorage->SyncGetKeyValue(key.OTATargetVersion(), &targetVersion, size);
+ return mPersistentStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::OTATargetVersion(), &targetVersion, size);
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::ClearTargetVersion()
 {
- DefaultStorageKeyAllocator key;
-
- return mPersistentStorage->SyncDeleteKeyValue(key.OTATargetVersion());
+ return mPersistentStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::OTATargetVersion());
 }
 
 CHIP_ERROR DefaultOTARequestorStorage::Load(const char * key, MutableByteSpan & buffer)

src/app/server/DefaultAclStorage.cpp

@@ -75,8 +75,6 @@ class : public EntryListener
  {
  CHIP_ERROR err;
 
- DefaultStorageKeyAllocator key;
-
  uint8_t buffer[kEncodedEntryTotalBytes] = { 0 };
 
  VerifyOrExit(mPersistentStorage != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
@@ -87,16 +85,19 @@ class : public EntryListener
  while (true)
  {
  uint16_t size = static_cast<uint16_t>(sizeof(buffer));
- err = mPersistentStorage->SyncGetKeyValue(key.AccessControlAclEntry(fabric, index + 1), buffer, size);
+ err = mPersistentStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::AccessControlAclEntry(fabric, index + 1),
+ buffer, size);
  if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
  {
  break;
  }
  SuccessOrExit(err);
- SuccessOrExit(err = mPersistentStorage->SyncSetKeyValue(key.AccessControlAclEntry(fabric, index), buffer, size));
+ SuccessOrExit(err = mPersistentStorage->SyncSetKeyValue(
+ DefaultStorageKeyAllocator::AccessControlAclEntry(fabric, index), buffer, size));
  index++;
  }
- SuccessOrExit(err = mPersistentStorage->SyncDeleteKeyValue(key.AccessControlAclEntry(fabric, index)));
+ SuccessOrExit(
+ err = mPersistentStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::AccessControlAclEntry(fabric, index)));
  }
  else
  {
@@ -106,8 +107,9 @@ class : public EntryListener
  writer.Init(buffer);
  EncodableEntry encodableEntry(*entry);
  SuccessOrExit(err = encodableEntry.EncodeForWrite(writer, TLV::AnonymousTag()));
- SuccessOrExit(err = mPersistentStorage->SyncSetKeyValue(key.AccessControlAclEntry(fabric, index), buffer,
- static_cast<uint16_t>(writer.GetLengthWritten())));
+ SuccessOrExit(err =
+ mPersistentStorage->SyncSetKeyValue(DefaultStorageKeyAllocator::AccessControlAclEntry(fabric, index),
+ buffer, static_cast<uint16_t>(writer.GetLengthWritten())));
  }
 
  return;
@@ -136,8 +138,6 @@ CHIP_ERROR DefaultAclStorage::Init(PersistentStorageDelegate & persistentStorage
 
  CHIP_ERROR err;
 
- DefaultStorageKeyAllocator key;
-
  size_t count = 0;
 
  for (auto it = first; it != last; ++it)
@@ -147,7 +147,7 @@ CHIP_ERROR DefaultAclStorage::Init(PersistentStorageDelegate & persistentStorage
  {
  uint8_t buffer[kEncodedEntryTotalBytes] = { 0 };
  uint16_t size = static_cast<uint16_t>(sizeof(buffer));
- err = persistentStorage.SyncGetKeyValue(key.AccessControlAclEntry(fabric, index), buffer, size);
+ err = persistentStorage.SyncGetKeyValue(DefaultStorageKeyAllocator::AccessControlAclEntry(fabric, index), buffer, size);
  if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
  {
  break;