Smart and raw pointer support in Json Serialization now behave similarly.

The behavior of smart pointers was not updated after the changes to raw pointers, in particular with how default JSON Objects were handled. They both now treat an empty JSON Object (a.k.a. the id for default instances) as meaning to create a default instance on the pointer even if the default is a nullptr. To have a nullptr a JSON Null has to be explicitly written.
2021-05-04 11:10:46 -07:00
parent ea5dc5eb24
commit 11b79f567a
5 changed files with 242 additions and 61 deletions
@@ -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<AZStd::string>().c_str(), typeId.ToString<AZStd::string>().c_str());

        void** objectPtr = reinterpret_cast<void**>(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<AZStd::string>().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<AZStd::string>().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);
@@ -38,6 +38,17 @@ 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.
    class JsonSerialization final
    {
    public:
@@ -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.");
    }
@@ -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<int>(), 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), AZStd::alignment_of<int>::value);
+    }
+
+    TEST_F(BaseJsonSerializerTests, ContinueLoading_DefaultToNullPointer_ValueLoadedCorrectly)
+    {
+        using namespace AZ::JsonSerializationResult;
+
+        rapidjson::Value json(rapidjson::kObjectType);
+        int* ptrValue = nullptr;
+
+        ResultCode result = ContinueLoading(&ptrValue, azrtti_typeid<int>(), json, *m_jsonDeserializationContext, Flags::ResolvePointer);
+
+        EXPECT_EQ(Processing::Completed, result.GetProcessing());
+        ASSERT_NE(nullptr, ptrValue);
+        
+        azfree(ptrValue, AZ::SystemAllocator, sizeof(int), AZStd::alignment_of<int>::value);
+    }
+
+    TEST_F(BaseJsonSerializerTests, ContinueLoading_NullDeletesObject_ValueLoadedCorrectly)
+    {
+        using namespace AZ::JsonSerializationResult;
+
+        rapidjson::Value json(rapidjson::kNullType);
+        int* ptrValue = reinterpret_cast<int*>(azmalloc(sizeof(int), AZStd::alignment_of<int>::value, AZ::SystemAllocator));
+
+        ResultCode result = ContinueLoading(&ptrValue, azrtti_typeid<int>(), 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<int>(), *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<int>(), *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<int>(), *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<int>(), *m_jsonSerializationContext, Flags::ResolvePointer);
+
+        EXPECT_EQ(Processing::Completed, result.GetProcessing());
+        Expect_DocStrEq("null");
+    }
+
    TEST_F(BaseJsonSerializerTests, ContinueStoring_ReplaceDefault_ValueStoredCorrectly)
    {
        using namespace AZ::JsonSerializationResult;
@@ -32,11 +32,6 @@ namespace JsonSerializationTests
            return AZStd::make_shared<AZ::JsonSmartPointerSerializer>();
        }

-        AZStd::shared_ptr<SmartPointer> CreateDefaultInstance() override
-        {
-            return AZStd::make_shared<SmartPointer>();
-        }
-
        void Reflect(AZStd::unique_ptr<AZ::SerializeContext>& context) override
        {
            context->RegisterGenericType<SmartPointer>();
@@ -51,6 +46,13 @@ namespace JsonSerializationTests
        using SmartPointer = T<SimpleClass>;
        using Base = SmartPointerBaseTestDescription<SmartPointer>;

+        AZStd::shared_ptr<SmartPointer> CreateDefaultInstance() override
+        {
+            auto result = AZStd::make_shared<SmartPointer>();
+            *result = SmartPointer(aznew SimpleClass());
+            return result;
+        }
+
        AZStd::shared_ptr<SmartPointer> CreateFullySetInstance() override
        {
            auto result = AZStd::make_shared<SmartPointer>();
@@ -106,21 +108,6 @@ namespace JsonSerializationTests
        }
    };

-    template<template<typename...> class T>
-    class SmartPointerSimpleClassWithInstanceTestDescription :
-        public SmartPointerSimpleClassTestDescription<T>
-    {
-    public:
-        using SmartPointer = typename SmartPointerSimpleClassTestDescription<T>::SmartPointer;
-
-        AZStd::shared_ptr<SmartPointer> CreateDefaultInstance() override
-        {
-            auto result = AZStd::make_shared<SmartPointer>();
-            *result = SmartPointer(aznew SimpleClass());
-            return result;
-        }
-    };
-
    template<template<typename...> class T>
    class SmartPointerSimpleDerivedClassTestDescription :
        public SmartPointerBaseTestDescription<T<BaseClass>>
@@ -129,6 +116,13 @@ namespace JsonSerializationTests
        using SmartPointer = T<BaseClass>;
        using Base = SmartPointerBaseTestDescription<SmartPointer>;

+        AZStd::shared_ptr<SmartPointer> CreateDefaultInstance() override
+        {
+            auto result = AZStd::make_shared<SmartPointer>();
+            *result = SmartPointer(aznew BaseClass());
+            return result;
+        }
+
        AZStd::shared_ptr<SmartPointer> CreateFullySetInstance() override
        {
            auto* instance = aznew SimpleInheritence();
@@ -272,6 +266,13 @@ namespace JsonSerializationTests
        using SmartPointer = T<BaseClass2>;
        using Base = SmartPointerBaseTestDescription<SmartPointer>;

+        AZStd::shared_ptr<SmartPointer> CreateDefaultInstance() override
+        {
+            auto result = AZStd::make_shared<SmartPointer>();
+            *result = SmartPointer(aznew BaseClass2());
+            return result;
+        }
+
        AZStd::shared_ptr<SmartPointer> CreateFullySetInstance() override
        {
            auto* instance = aznew MultipleInheritence();
@@ -424,9 +425,6 @@ namespace JsonSerializationTests
        SmartPointerSimpleClassTestDescription<AZStd::unique_ptr>,
        SmartPointerSimpleClassTestDescription<AZStd::shared_ptr>,
        SmartPointerSimpleClassTestDescription<AZStd::intrusive_ptr>,
-        SmartPointerSimpleClassWithInstanceTestDescription<AZStd::unique_ptr>,
-        SmartPointerSimpleClassWithInstanceTestDescription<AZStd::shared_ptr>,
-        SmartPointerSimpleClassWithInstanceTestDescription<AZStd::intrusive_ptr>,
        // Simple derived class, include single inheritance.
        SmartPointerSimpleDerivedClassTestDescription<AZStd::unique_ptr>,
        SmartPointerSimpleDerivedClassTestDescription<AZStd::shared_ptr>,
@@ -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<SmartPointer> compare = m_description.CreateDefaultInstance();
+        m_jsonDocument->SetObject();
+
+        JSR::ResultCode result =
+            m_serializer.Load(&instance, azrtti_typeid<SmartPointer>(), *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<SmartPointer> instance = m_description.CreateFullySetInstance();
+        AZStd::shared_ptr<SmartPointer> compare = m_description.CreateFullySetInstance();
+        m_jsonDocument->SetObject();
+
+        JSR::ResultCode result =
+            m_serializer.Load(instance.get(), azrtti_typeid<SmartPointer>(), *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<SmartPointer> 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<SmartPointer>(), *m_jsonSerializationContext);

        EXPECT_EQ(JSR::Outcomes::Success, result.GetOutcome());
    }

+    TEST_F(JsonSmartPointerSerializerTests, Store_ValuePointerIsNullPtr_ReturnsSuccessAndStoresNull)
+    {
+        namespace JSR = AZ::JsonSerializationResult;
+
+        SmartPointer instance;
+        AZStd::shared_ptr<SmartPointer> defaultInstance = m_description.CreateFullySetInstance();
+        JSR::ResultCode result = m_serializer.Store(
+            *m_jsonDocument, &instance, defaultInstance.get(), azrtti_typeid<SmartPointer>(), *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<SmartPointer>(), *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<SmartPointer> instance = m_description.CreateDefaultInstance();
+        AZStd::shared_ptr<SmartPointer> defaultInstance = m_description.CreateDefaultInstance();
+        JSR::ResultCode result =
+            m_serializer.Store(*m_jsonDocument, instance.get(), defaultInstance.get(), azrtti_typeid<SmartPointer>(), *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<SmartPointer> instance = m_description.CreateDefaultInstance();
+        SmartPointer defaultInstance;
+        JSR::ResultCode result = m_serializer.Store(
+            *m_jsonDocument, instance.get(), &defaultInstance, azrtti_typeid<SmartPointer>(), *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;