diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp index 196ce28216..f7e41c0c00 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp @@ -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 { diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.h index 937c8389ff..74d7e81ac9 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.h @@ -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, diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp index 7ffe3ffee0..fe97d36f08 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp @@ -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);