diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp index f2e9bb866f..8d0da9e54a 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp @@ -97,7 +97,8 @@ namespace AZ return context.Report(Tasks::RetrieveInfo, Outcomes::Unknown, AZStd::string::format("Failed to retrieve rtti information for %s.", classData->m_name)); } - AZ_Assert(classData->m_azRtti->GetTypeId() == typeId, "Type id mismatch during deserialization of a json file. (%s vs %s)"); + AZ_Assert(classData->m_azRtti->GetTypeId() == typeId, "Type id mismatch during deserialization of a json file. (%s vs %s)", + classData->m_azRtti->GetTypeId().ToString().c_str(), typeId.ToString().c_str()); void** objectPtr = reinterpret_cast(object); bool isNull = *objectPtr == nullptr; @@ -512,27 +513,24 @@ namespace AZ if (*object) { const AZ::Uuid& actualClassId = rtti.GetActualUuid(*object); - if (actualClassId != objectType) + const SerializeContext::ClassData* actualClassData = context.GetSerializeContext()->FindClassData(actualClassId); + if (!actualClassData) { - const SerializeContext::ClassData* actualClassData = context.GetSerializeContext()->FindClassData(actualClassId); - if (!actualClassData) - { - status = context.Report(Tasks::RetrieveInfo, Outcomes::Unknown, - AZStd::string::format("Unable to find serialization information for type %s.", actualClassId.ToString().c_str())); - return ResolvePointerResult::FullyProcessed; - } + status = context.Report(Tasks::RetrieveInfo, Outcomes::Unknown, + AZStd::string::format("Unable to find serialization information for type %s.", actualClassId.ToString().c_str())); + return ResolvePointerResult::FullyProcessed; + } - if (actualClassData->m_factory) - { - actualClassData->m_factory->Destroy(*object); - *object = nullptr; - } - else - { - status = context.Report(Tasks::RetrieveInfo, Outcomes::Catastrophic, - "Unable to find the factory needed to clear out the default value."); - return ResolvePointerResult::FullyProcessed; - } + if (actualClassData->m_factory) + { + actualClassData->m_factory->Destroy(*object); + *object = nullptr; + } + else + { + status = context.Report(Tasks::RetrieveInfo, Outcomes::Catastrophic, + "Unable to find the factory needed to clear out the default value."); + return ResolvePointerResult::FullyProcessed; } } status = ResultCode(Tasks::ReadField, Outcomes::Success); diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.h b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.h index 5746005832..cf73d8dc56 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.h @@ -38,6 +38,20 @@ namespace AZ }; //! Core class to handle serialization to and from json documents. + //! The Json Serialization works by taking a default constructed object and then apply the information found in the JSON document + //! on top of that object. This allows the Json Serialization to avoid storing default values and helps guarantee that the final + //! object is in a valid state even if non-fatal issues are encountered. + //! Note on containers: Containers such as vector or map are always considered to be empty even if there's entries in the provided + //! default object. During deserialization entries will be appended to any existing values. A flag is provided to automatically + //! clear containers during deserialization. + //! Note on maps: If the key for map containers such as unordered_map can be interpret as a string the Json Serialization will use + //! a JSON Object to store the data in instead of an array with key/value objects. + //! Note on pointers: The Json Serialization assumes that are always constructed, so a default JSON value of "{}" is interpret as + //! creating a new default instance even if the default value is a null pointer. A JSON Null needs to be explicitly stored in + //! the JSON Document in order to default or explicitly set a pointer to null. + //! Note on pointer memory: Objects created/destroyed by the Json Serialization for pointers require that the AZ_CLASS_ALLOCATOR is + //! declared and the object is created using aznew or memory is allocated using azmalloc. Without these the application may + //! crash if the Json Serialization tries to create or destroy an object pointed to by a pointer. class JsonSerialization final { public: diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/SmartPointerSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/SmartPointerSerializer.cpp index 0180e99592..9e707f8644 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/SmartPointerSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/SmartPointerSerializer.cpp @@ -23,13 +23,6 @@ namespace AZ { namespace JSR = JsonSerializationResult; - if (IsExplicitDefault(inputValue)) - { - // Do nothing if the input is an explicit default. - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, - "Default value for smart pointer requested so no change was made."); - } - const SerializeContext::ClassData* containerClass = context.GetSerializeContext()->FindClassData(outputValueTypeId); if (!containerClass) { @@ -153,8 +146,7 @@ namespace AZ if (defaultValue) { - bool typesMatch = false; - auto defaultInputCallback = [&defaultValue, &inputPtrType, &typesMatch] + auto defaultInputCallback = [&defaultValue] (void* elementPtr, const Uuid&, const SerializeContext::ClassData*, const SerializeContext::ClassElement*) { defaultValue = elementPtr; @@ -164,11 +156,6 @@ namespace AZ } JSR::ResultCode result = ContinueStoring(outputValue, inputValue, defaultValue, inputPtrType, context, Flags::ResolvePointer); - if (result.GetOutcome() == JSR::Outcomes::DefaultsUsed) - { - outputValue = GetExplicitDefault(); - return context.Report(result, "Smart pointer used all defaults."); - } return context.Report(result, result.GetProcessing() != JSR::Processing::Halted ? "Successfully processed smart pointer." : "A problem occurred while processing a smart pointer."); } diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/BaseJsonSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/BaseJsonSerializerTests.cpp index 417cb10c8c..08de21b54f 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/BaseJsonSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/BaseJsonSerializerTests.cpp @@ -110,7 +110,7 @@ namespace JsonSerializationTests EXPECT_EQ(42, value); } - TEST_F(BaseJsonSerializerTests, ContinueLoading_PointerInstance_ValueLoadedCorrectly) + TEST_F(BaseJsonSerializerTests, ContinueLoading_ToPointerInstance_ValueLoadedCorrectly) { using namespace AZ::JsonSerializationResult; @@ -126,6 +126,51 @@ namespace JsonSerializationTests EXPECT_EQ(42, value); } + TEST_F(BaseJsonSerializerTests, ContinueLoading_ToNullPointer_ValueLoadedCorrectly) + { + using namespace AZ::JsonSerializationResult; + + rapidjson::Value json; + json.Set(42); + int* ptrValue = nullptr; + + ResultCode result = ContinueLoading(&ptrValue, azrtti_typeid(), json, *m_jsonDeserializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_NE(nullptr, ptrValue); + EXPECT_EQ(42, *ptrValue); + + azfree(ptrValue, AZ::SystemAllocator, sizeof(int), alignof(int)); + } + + TEST_F(BaseJsonSerializerTests, ContinueLoading_DefaultToNullPointer_ValueLoadedCorrectly) + { + using namespace AZ::JsonSerializationResult; + + rapidjson::Value json(rapidjson::kObjectType); + int* ptrValue = nullptr; + + ResultCode result = ContinueLoading(&ptrValue, azrtti_typeid(), json, *m_jsonDeserializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_NE(nullptr, ptrValue); + + azfree(ptrValue, AZ::SystemAllocator, sizeof(int), alignof(int)); + } + + TEST_F(BaseJsonSerializerTests, ContinueLoading_NullDeletesObject_ValueLoadedCorrectly) + { + using namespace AZ::JsonSerializationResult; + + rapidjson::Value json(rapidjson::kNullType); + int* ptrValue = reinterpret_cast(azmalloc(sizeof(int), alignof(int), AZ::SystemAllocator)); + + ResultCode result = ContinueLoading(&ptrValue, azrtti_typeid(), json, *m_jsonDeserializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_EQ(nullptr, ptrValue); + } + // // ContinueStoring // @@ -156,6 +201,64 @@ namespace JsonSerializationTests Expect_DocStrEq("42"); } + TEST_F(BaseJsonSerializerTests, ContinueStoring_StorePointerToFullDefaultedInstance_ValueStoredCorrectly) + { + using namespace AZ::JsonSerializationResult; + + int value = 42; + int* ptrValue = &value; + int value2 = 42; + int* defaultPtrValue = &value2; + + ResultCode result = + ContinueStoring(*m_jsonDocument, &ptrValue, &defaultPtrValue, azrtti_typeid(), *m_jsonSerializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + Expect_DocStrEq("{}"); + } + + TEST_F(BaseJsonSerializerTests, ContinueStoring_StorePointerToNullptr_ValueStoredCorrectly) + { + using namespace AZ::JsonSerializationResult; + + int* ptrValue = nullptr; + + ResultCode result = ContinueStoring( + *m_jsonDocument, &ptrValue, nullptr, azrtti_typeid(), *m_jsonSerializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + Expect_DocStrEq("null"); + } + + TEST_F(BaseJsonSerializerTests, ContinueStoring_StorePointerToNullptrWithValueDefault_ValueStoredCorrectly) + { + using namespace AZ::JsonSerializationResult; + + int* ptrValue = nullptr; + int value2 = 42; + int* defaultPtrValue = &value2; + + ResultCode result = + ContinueStoring(*m_jsonDocument, &ptrValue, &defaultPtrValue, azrtti_typeid(), *m_jsonSerializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + Expect_DocStrEq("null"); + } + + TEST_F(BaseJsonSerializerTests, ContinueStoring_StorePointerToNullptrWithNullPtrDefault_NullPtrIsStored) + { + using namespace AZ::JsonSerializationResult; + + int* ptrValue = nullptr; + int* defaultPtrValue = nullptr; + + ResultCode result = + ContinueStoring(*m_jsonDocument, &ptrValue, &defaultPtrValue, azrtti_typeid(), *m_jsonSerializationContext, Flags::ResolvePointer); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + Expect_DocStrEq("null"); + } + TEST_F(BaseJsonSerializerTests, ContinueStoring_ReplaceDefault_ValueStoredCorrectly) { using namespace AZ::JsonSerializationResult; diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/SmartPointerSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/SmartPointerSerializerTests.cpp index df340a2e61..2fc131aae2 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/SmartPointerSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/SmartPointerSerializerTests.cpp @@ -32,11 +32,6 @@ namespace JsonSerializationTests return AZStd::make_shared(); } - AZStd::shared_ptr CreateDefaultInstance() override - { - return AZStd::make_shared(); - } - void Reflect(AZStd::unique_ptr& context) override { context->RegisterGenericType(); @@ -51,6 +46,13 @@ namespace JsonSerializationTests using SmartPointer = T; using Base = SmartPointerBaseTestDescription; + AZStd::shared_ptr CreateDefaultInstance() override + { + auto result = AZStd::make_shared(); + *result = SmartPointer(aznew SimpleClass()); + return result; + } + AZStd::shared_ptr CreateFullySetInstance() override { auto result = AZStd::make_shared(); @@ -106,21 +108,6 @@ namespace JsonSerializationTests } }; - template class T> - class SmartPointerSimpleClassWithInstanceTestDescription : - public SmartPointerSimpleClassTestDescription - { - public: - using SmartPointer = typename SmartPointerSimpleClassTestDescription::SmartPointer; - - AZStd::shared_ptr CreateDefaultInstance() override - { - auto result = AZStd::make_shared(); - *result = SmartPointer(aznew SimpleClass()); - return result; - } - }; - template class T> class SmartPointerSimpleDerivedClassTestDescription : public SmartPointerBaseTestDescription> @@ -129,6 +116,13 @@ namespace JsonSerializationTests using SmartPointer = T; using Base = SmartPointerBaseTestDescription; + AZStd::shared_ptr CreateDefaultInstance() override + { + auto result = AZStd::make_shared(); + *result = SmartPointer(aznew BaseClass()); + return result; + } + AZStd::shared_ptr CreateFullySetInstance() override { auto* instance = aznew SimpleInheritence(); @@ -272,6 +266,13 @@ namespace JsonSerializationTests using SmartPointer = T; using Base = SmartPointerBaseTestDescription; + AZStd::shared_ptr CreateDefaultInstance() override + { + auto result = AZStd::make_shared(); + *result = SmartPointer(aznew BaseClass2()); + return result; + } + AZStd::shared_ptr CreateFullySetInstance() override { auto* instance = aznew MultipleInheritence(); @@ -424,9 +425,6 @@ namespace JsonSerializationTests SmartPointerSimpleClassTestDescription, SmartPointerSimpleClassTestDescription, SmartPointerSimpleClassTestDescription, - SmartPointerSimpleClassWithInstanceTestDescription, - SmartPointerSimpleClassWithInstanceTestDescription, - SmartPointerSimpleClassWithInstanceTestDescription, // Simple derived class, include single inheritance. SmartPointerSimpleDerivedClassTestDescription, SmartPointerSimpleDerivedClassTestDescription, @@ -551,6 +549,37 @@ namespace JsonSerializationTests EXPECT_EQ(nullptr, *instance); } + TEST_F(JsonSmartPointerSerializerTests, Load_DefaultInstanceToNullptr_ReturnsSuccess) + { + namespace JSR = AZ::JsonSerializationResult; + + SmartPointer instance; + AZStd::shared_ptr compare = m_description.CreateDefaultInstance(); + m_jsonDocument->SetObject(); + + JSR::ResultCode result = + m_serializer.Load(&instance, azrtti_typeid(), *m_jsonDocument, *m_jsonDeserializationContext); + + EXPECT_EQ(JSR::Processing::Completed, result.GetProcessing()); + EXPECT_NE(nullptr, instance); + EXPECT_TRUE(m_description.AreEqual(instance, *compare)); + } + + TEST_F(JsonSmartPointerSerializerTests, Load_DefaultObjectDoesNotUpdateInstance_ReturnsSuccess) + { + namespace JSR = AZ::JsonSerializationResult; + + AZStd::shared_ptr instance = m_description.CreateFullySetInstance(); + AZStd::shared_ptr compare = m_description.CreateFullySetInstance(); + m_jsonDocument->SetObject(); + + JSR::ResultCode result = + m_serializer.Load(instance.get(), azrtti_typeid(), *m_jsonDocument, *m_jsonDeserializationContext); + + EXPECT_EQ(JSR::Processing::Completed, result.GetProcessing()); + EXPECT_TRUE(m_description.AreEqual(*instance, *compare)); + } + TEST_F(JsonSmartPointerSerializerTests, Load_InstanceBeingReplacedWithDifferentType_ReturnsSuccess) { namespace JSR = AZ::JsonSerializationResult; @@ -720,13 +749,66 @@ namespace JsonSerializationTests namespace JSR = AZ::JsonSerializationResult; AZStd::shared_ptr instance = m_description.CreateFullySetInstance(); - SmartPointer nullPtr; - JSR::ResultCode result = m_serializer.Store(*m_jsonDocument, instance.get(), &nullPtr, + SmartPointer defaultInstance; + JSR::ResultCode result = m_serializer.Store( + *m_jsonDocument, instance.get(), &defaultInstance, azrtti_typeid(), *m_jsonSerializationContext); EXPECT_EQ(JSR::Outcomes::Success, result.GetOutcome()); } + TEST_F(JsonSmartPointerSerializerTests, Store_ValuePointerIsNullPtr_ReturnsSuccessAndStoresNull) + { + namespace JSR = AZ::JsonSerializationResult; + + SmartPointer instance; + AZStd::shared_ptr defaultInstance = m_description.CreateFullySetInstance(); + JSR::ResultCode result = m_serializer.Store( + *m_jsonDocument, &instance, defaultInstance.get(), azrtti_typeid(), *m_jsonSerializationContext); + + EXPECT_EQ(JSR::Outcomes::Success, result.GetOutcome()); + EXPECT_TRUE(m_jsonDocument->IsNull()); + } + + TEST_F(JsonSmartPointerSerializerTests, Store_ValueAndDefaultPointersAreNullPtr_ReturnsSuccessAndStoresNull) + { + namespace JSR = AZ::JsonSerializationResult; + + SmartPointer instance; + SmartPointer defaultInstance; + JSR::ResultCode result = + m_serializer.Store(*m_jsonDocument, &instance, &defaultInstance, azrtti_typeid(), *m_jsonSerializationContext); + + EXPECT_EQ(JSR::Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_TRUE(m_jsonDocument->IsNull()); + } + + TEST_F(JsonSmartPointerSerializerTests, Store_ValueAndDefaultPointersAreBothDefault_ReturnsSuccess) + { + namespace JSR = AZ::JsonSerializationResult; + + AZStd::shared_ptr instance = m_description.CreateDefaultInstance(); + AZStd::shared_ptr defaultInstance = m_description.CreateDefaultInstance(); + JSR::ResultCode result = + m_serializer.Store(*m_jsonDocument, instance.get(), defaultInstance.get(), azrtti_typeid(), *m_jsonSerializationContext); + + EXPECT_EQ(JSR::Outcomes::DefaultsUsed, result.GetOutcome()); + Expect_ExplicitDefault(*m_jsonDocument); + } + + TEST_F(JsonSmartPointerSerializerTests, Store_ValueHasDefaultValuesAndDefaultHasNullPointer_ReturnsSuccess) + { + namespace JSR = AZ::JsonSerializationResult; + + AZStd::shared_ptr instance = m_description.CreateDefaultInstance(); + SmartPointer defaultInstance; + JSR::ResultCode result = m_serializer.Store( + *m_jsonDocument, instance.get(), &defaultInstance, azrtti_typeid(), *m_jsonSerializationContext); + + EXPECT_EQ(JSR::Outcomes::DefaultsUsed, result.GetOutcome()); + Expect_ExplicitDefault(*m_jsonDocument); + } + TEST_F(JsonSmartPointerSerializerTests, Store_DefaultPointerIsOtherClass_CompletesButDoesNotReturnDefaults) { namespace JSR = AZ::JsonSerializationResult; @@ -749,7 +831,7 @@ namespace JsonSerializationTests EXPECT_EQ(JSR::Processing::Completed, result.GetProcessing()); } - TEST_F(JsonSmartPointerSerializerTests, Store_SaveAnClassThatIsNotReflected_ReturnsUnknown) + TEST_F(JsonSmartPointerSerializerTests, Store_ClassThatIsNotReflected_ReturnsUnknown) { namespace JSR = AZ::JsonSerializationResult;