[Serialization] Add support for updating the values of existing keys in associative containers (#6832)

* Add support for updating the values of existing keys in associative containers

Signed-off-by: amzn-sj <srikkant@amazon.com>

* Map/Unordered Map serialization updates values corresponding to existing keys by default. Multimaps always add a new entry for existing keys.

Signed-off-by: amzn-sj <srikkant@amazon.com>

* Fix unused parameter warning

Signed-off-by: amzn-sj <srikkant@amazon.com>

* Update comparison function for test case

Signed-off-by: amzn-sj <srikkant@amazon.com>
This commit is contained in:
amzn-sj
2022-01-28 09:13:24 -08:00
committed by GitHub
parent b34a955c3d
commit 112e310419
3 changed files with 47 additions and 14 deletions
@@ -203,7 +203,7 @@ namespace AZ
JsonSerializationResult::Result JsonMapSerializer::LoadElement(void* outputValue, SerializeContext::IDataContainer* container,
const SerializeContext::ClassElement* pairElement, SerializeContext::IDataContainer* pairContainer,
const SerializeContext::ClassElement* keyElement, const SerializeContext::ClassElement* valueElement,
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context)
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context, bool isMultiMap)
{
namespace JSR = JsonSerializationResult;
@@ -231,8 +231,30 @@ namespace AZ
return context.Report(keyResult, "Failed to read key for associative container.");
}
void* valueAddress = nullptr;
bool keyExists = false;
// For multimaps, we append values to keys instead updating them.
// This is to ensure legacy multimap serialization support.
if (!isMultiMap)
{
auto associativeContainer = container->GetAssociativeContainerInterface();
void* existingKeyValuePair = associativeContainer->GetElementByKey(outputValue, keyElement, keyAddress);
if (existingKeyValuePair)
{
valueAddress = pairContainer->GetElementByIndex(existingKeyValuePair, pairElement, 1);
expectedSize--;
keyExists = true;
}
}
// If the key doesn't exist or it's a multimap, we're adding the new element we reserved above.
if (!keyExists)
{
valueAddress = pairContainer->GetElementByIndex(address, pairElement, 1);
}
// Load value
void* valueAddress = pairContainer->GetElementByIndex(address, pairElement, 1);
AZ_Assert(valueAddress, "Element reserved for associative container, but unable to retrieve address of the value.");
ContinuationFlags valueLoadFlags = ContinuationFlags::LoadAsNewInstance;
if (valueElement->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER)
@@ -257,7 +279,18 @@ namespace AZ
}
else
{
container->StoreElement(outputValue, address);
// Even if the key exists, calling StoreElement will not replace the existing key
// and will free the temporary address as expected. Checking if the key already
// exists and skipping the call to StoreElement if it does, makes the intent more
// clear. The end result is the same either way.
if (!keyExists)
{
container->StoreElement(outputValue, address);
}
else
{
container->FreeReservedElement(outputValue, address, context.GetSerializeContext());
}
if (container->Size(outputValue) != expectedSize)
{
return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unavailable,
@@ -430,7 +463,7 @@ namespace AZ
JsonSerializationResult::Result JsonUnorderedMultiMapSerializer::LoadElement(void* outputValue, SerializeContext::IDataContainer* container,
const SerializeContext::ClassElement* pairElement, SerializeContext::IDataContainer* pairContainer,
const SerializeContext::ClassElement* keyElement, const SerializeContext::ClassElement* valueElement,
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context)
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context, [[maybe_unused]] bool isMultiMap)
{
namespace JSR = JsonSerializationResult;
@@ -440,7 +473,7 @@ namespace AZ
for (auto& entry : value.GetArray())
{
result.Combine(JsonMapSerializer::LoadElement(outputValue, container, pairElement, pairContainer,
keyElement, valueElement, key, entry, context));
keyElement, valueElement, key, entry, context, true));
if (result.GetProcessing() == JSR::Processing::Halted)
{
return context.Report(result, "Unable to process the key or all values in multi-map.");
@@ -451,7 +484,7 @@ namespace AZ
else if (IsExplicitDefault(value))
{
return JsonMapSerializer::LoadElement(outputValue, container, pairElement, pairContainer,
keyElement, valueElement, key, value, context);
keyElement, valueElement, key, value, context, true);
}
else
{
@@ -32,7 +32,7 @@ namespace AZ
virtual JsonSerializationResult::Result LoadElement(void* outputValue, SerializeContext::IDataContainer* container,
const SerializeContext::ClassElement* pairElement, SerializeContext::IDataContainer* pairContainer,
const SerializeContext::ClassElement* keyElement, const SerializeContext::ClassElement* valueElement,
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context);
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context, bool isMultiMap = false);
virtual JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue,
const Uuid& valueTypeId, JsonSerializerContext& context, bool sortResult);
@@ -62,7 +62,7 @@ namespace AZ
JsonSerializationResult::Result LoadElement(void* outputValue, SerializeContext::IDataContainer* container,
const SerializeContext::ClassElement* pairElement, SerializeContext::IDataContainer* pairContainer,
const SerializeContext::ClassElement* keyElement, const SerializeContext::ClassElement* valueElement,
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context) override;
const rapidjson::Value& key, const rapidjson::Value& value, JsonDeserializerContext& context, bool isMultiMap = false) override;
using JsonMapSerializer::Store;
JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue,
@@ -475,7 +475,7 @@ namespace JsonSerializationTests
EXPECT_STRCASEEQ("value_42", worldKey->second.m_value.c_str());
}
TEST_F(JsonMapSerializerTests, Load_DuplicateKey_EntryIgnored)
TEST_F(JsonMapSerializerTests, Load_DuplicateKey_EntryUpdated)
{
using namespace AZ::JsonSerializationResult;
@@ -489,12 +489,12 @@ namespace JsonSerializationTests
StringMap values;
ResultCode result = m_unorderedMapSerializer.Load(&values, azrtti_typeid(&values), *m_jsonDocument, *m_jsonDeserializationContext);
EXPECT_EQ(Processing::PartialAlter, result.GetProcessing());
EXPECT_EQ(Outcomes::Unavailable, result.GetOutcome());
EXPECT_EQ(Processing::Completed, result.GetProcessing());
EXPECT_EQ(Outcomes::Success, result.GetOutcome());
auto entry = values.find("Hello");
ASSERT_NE(values.end(), entry);
EXPECT_STRCASEEQ("World", entry->second.c_str());
EXPECT_EQ("Other", entry->second);
}
TEST_F(JsonMapSerializerTests, Load_DuplicateMultiKey_LoadEverything)
@@ -536,8 +536,8 @@ namespace JsonSerializationTests
ResultCode result = m_unorderedMapSerializer.Load(&values,
azrtti_typeid(&values), *m_jsonDocument, *m_jsonDeserializationContext);
EXPECT_EQ(Processing::Altered, result.GetProcessing());
EXPECT_EQ(Outcomes::Unavailable, result.GetOutcome());
EXPECT_EQ(Processing::Completed, result.GetProcessing());
EXPECT_EQ(Outcomes::Success, result.GetOutcome());
auto entry = values.find("Hello");
ASSERT_NE(values.end(), entry);