From 3c46a72672aa24f3d34227396e2d44e641a764af Mon Sep 17 00:00:00 2001 From: Yuriy Toporovskyy Date: Wed, 26 May 2021 16:48:39 -0400 Subject: [PATCH 01/15] Bug fix: handle the case where a container has only default elements --- .../Serialization/Json/BasicContainerSerializer.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp index 400a3b7949..525e42a1dd 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp @@ -102,6 +102,17 @@ namespace AZ return context.Report(retVal, "Processing of basic container was halted."); } + // If each container element was 'DefaultsUsed', then the result code will be 'DefaultsUsed' + // But this is wrong if the container has at least one element, because a container with + // at least one element is certainly not the default container value. + // Basically, the following are different objects: + // [ {} ] // The container which has only default elements, but is not the empty container + // {} // The default container, which is empty + if (index > 0) + { + retVal.Combine(JSR::ResultCode(JSR::Tasks::WriteValue, JSR::Outcomes::Success)); + } + if (context.ShouldKeepDefaults()) { outputValue = AZStd::move(array); From 6f50207b06083766875c0ae8a1a43d963ff963c6 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Thu, 10 Jun 2021 09:23:05 -0700 Subject: [PATCH 02/15] Updated the unit tests for the Json Serialization array fix --- .../Json/BasicContainerSerializer.cpp | 15 ++++----------- .../Json/BasicContainerSerializerTests.cpp | 5 ++--- .../Serialization/Json/MapSerializerTests.cpp | 4 ++-- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp index 525e42a1dd..8fe1f471c1 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp @@ -102,17 +102,6 @@ namespace AZ return context.Report(retVal, "Processing of basic container was halted."); } - // If each container element was 'DefaultsUsed', then the result code will be 'DefaultsUsed' - // But this is wrong if the container has at least one element, because a container with - // at least one element is certainly not the default container value. - // Basically, the following are different objects: - // [ {} ] // The container which has only default elements, but is not the empty container - // {} // The default container, which is empty - if (index > 0) - { - retVal.Combine(JSR::ResultCode(JSR::Tasks::WriteValue, JSR::Outcomes::Success)); - } - if (context.ShouldKeepDefaults()) { outputValue = AZStd::move(array); @@ -134,6 +123,10 @@ namespace AZ { if (retVal.HasDoneWork()) { + // If at least one value was written, even if it has all defaults, then the array has + // a value written to it and is therefore not in a default state anymore. + retVal.Combine(JSR::ResultCode(JSR::Tasks::WriteValue, JSR::Outcomes::Success)); + outputValue = AZStd::move(array); return context.Report(retVal, "Content written to basic container."); } diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp index 88e888743e..b3c900c710 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp @@ -301,7 +301,7 @@ namespace JsonSerializationTests ResultCode result = m_serializer->Store(*m_jsonDocument, &instance, &instance, azrtti_typeid(&instance), *m_jsonSerializationContext); EXPECT_EQ(Processing::Completed, result.GetProcessing()); - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); Expect_DocStrEq("[{}]"); } @@ -315,7 +315,7 @@ namespace JsonSerializationTests ResultCode result = m_serializer->Store(*m_jsonDocument, &instance, nullptr, azrtti_typeid(&instance), *m_jsonSerializationContext); EXPECT_EQ(Processing::Completed, result.GetProcessing()); - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); Expect_DocStrEq("[{},{}]"); } @@ -330,7 +330,6 @@ namespace JsonSerializationTests ResultCode result = m_serializer->Store(*m_jsonDocument, &instance, nullptr, azrtti_typeid(&instance), *m_jsonSerializationContext); EXPECT_EQ(Processing::Completed, result.GetProcessing()); EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); - EXPECT_NE(Outcomes::DefaultsUsed, result.GetOutcome()); Expect_DocStrEq(R"([{"$type": "SimpleInheritence"},{"$type": "SimpleInheritence"}])"); } diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp index 5dec394351..363cd7a7b7 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp @@ -636,7 +636,7 @@ namespace JsonSerializationTests azrtti_typeid(&values), *m_jsonSerializationContext); EXPECT_EQ(Processing::Completed, result.GetProcessing()); - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); Expect_DocStrEq(R"( { "{}": {} @@ -654,7 +654,7 @@ namespace JsonSerializationTests azrtti_typeid(&values), *m_jsonSerializationContext); EXPECT_EQ(Processing::Completed, result.GetProcessing()); - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); Expect_DocStrEq(R"( { "{}": {} From a98355e000095542aec130f7c8c6b2f5bbb5e1cc Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Fri, 11 Jun 2021 13:41:54 -0700 Subject: [PATCH 03/15] Fix PODs not initializing in Json Serialization When pointers are used new instances are created for pod types, which will have random values at that point. The Json Serialization did not set a value for these if they were explicitly set to defaults. This change adds initialization for explicit defaults in the bool, integer and double serializer plus unit tests to verify. --- .../Serialization/Json/BoolSerializer.cpp | 21 +++++-- .../Serialization/Json/BoolSerializer.h | 1 + .../Serialization/Json/DoubleSerializer.cpp | 31 ++++++++-- .../Serialization/Json/DoubleSerializer.h | 2 + .../Serialization/Json/IntSerializer.cpp | 46 ++++++++++----- .../AzCore/Serialization/Json/IntSerializer.h | 58 +++++++++---------- .../Json/BoolSerializerTests.cpp | 38 ++++++++++++ .../Json/DoubleSerializerTests.cpp | 49 ++++++++++++++++ .../Serialization/Json/IntSerializerTests.cpp | 38 ++++++++++++ 9 files changed, 229 insertions(+), 55 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp index d37e512ac2..7ac16b214d 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp @@ -30,8 +30,8 @@ namespace AZ if (text && textLength > 0) { - static constexpr const char trueString[] = "true"; - static constexpr const char falseString[] = "false"; + static constexpr const char* trueString = "true"; + static constexpr const char* falseString = "false"; // remove null terminator for string length counts // rapidjson stringlength doesn't include it in length calculations, but sizeof() will static constexpr size_t trueStringLength = sizeof(trueString) - 1; @@ -82,12 +82,18 @@ namespace AZ bool* valAsBool = reinterpret_cast(outputValue); + if (IsExplicitDefault(inputValue)) + { + *valAsBool = false; + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Boolean value set to default of 'false'."); + } + switch (inputValue.GetType()) { case rapidjson::kArrayType: - // fallthrough + [[fallthrough]]; case rapidjson::kObjectType: - // fallthrough + [[fallthrough]]; case rapidjson::kNullType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. Booleans can't be read from arrays, objects or null."); @@ -96,7 +102,7 @@ namespace AZ return SerializerInternal::TextToValue(valAsBool, inputValue.GetString(), inputValue.GetStringLength(), context); case rapidjson::kFalseType: - // fallthrough + [[fallthrough]]; case rapidjson::kTrueType: *valAsBool = inputValue.GetBool(); return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Success, "Successfully read boolean."); @@ -145,4 +151,9 @@ namespace AZ return context.Report(JSR::Tasks::WriteValue, JSR::Outcomes::DefaultsUsed, "Default boolean used."); } + + auto JsonBoolSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.h index ac288c73ed..f000a03317 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.h @@ -27,5 +27,6 @@ namespace AZ JsonDeserializerContext& context) override; JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; }; } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp index 9140be7fcd..647ff67eb6 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp @@ -62,19 +62,26 @@ namespace AZ } template - static JsonSerializationResult::Result Load(T* outputValue, const rapidjson::Value& inputValue, JsonDeserializerContext& context) + static JsonSerializationResult::Result Load( + T* outputValue, const rapidjson::Value& inputValue, JsonDeserializerContext& context, bool isExplicitDefault) { namespace JSR = JsonSerializationResult; // Used remove name conflicts in AzCore in uber builds. static_assert(AZStd::is_floating_point::value, "Expected T to be a floating point type"); AZ_Assert(outputValue, "Expected a valid pointer to load from json value."); + if (isExplicitDefault) + { + *outputValue = 0.0f; + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Double value set to default of 0.0."); + } + switch (inputValue.GetType()) { case rapidjson::kArrayType: - // fallthrough + [[fallthrough]]; case rapidjson::kObjectType: - // fallthrough + [[fallthrough]]; case rapidjson::kNullType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. Floating point values can't be read from arrays, objects or null."); @@ -83,7 +90,7 @@ namespace AZ return TextToValue(outputValue, inputValue.GetString(), context); case rapidjson::kFalseType: - // fallthrough + [[fallthrough]]; case rapidjson::kTrueType: *outputValue = inputValue.GetBool() ? 1.0f : 0.0f; return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Success, @@ -144,7 +151,8 @@ namespace AZ "Unable to deserialize double to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerFloatingPointInternal::Load(reinterpret_cast(outputValue), inputValue, context); + return SerializerFloatingPointInternal::Load( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonDoubleSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -156,6 +164,11 @@ namespace AZ return SerializerFloatingPointInternal::Store(outputValue, inputValue, defaultValue, context); } + auto JsonDoubleSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + JsonSerializationResult::Result JsonFloatSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { @@ -163,7 +176,8 @@ namespace AZ "Unable to deserialize float to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerFloatingPointInternal::Load(reinterpret_cast(outputValue), inputValue, context); + return SerializerFloatingPointInternal::Load( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonFloatSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -174,4 +188,9 @@ namespace AZ AZ_UNUSED(valueTypeId); return SerializerFloatingPointInternal::Store(outputValue, inputValue, defaultValue, context); } + + auto JsonFloatSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.h index 81e631d9ab..a8c37316f0 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.h @@ -28,6 +28,7 @@ namespace AZ JsonDeserializerContext& context) override; JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; }; class JsonFloatSerializer @@ -40,5 +41,6 @@ namespace AZ JsonDeserializerContext& context) override; JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; }; } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp index cc365da4f7..28da852da3 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp @@ -25,6 +25,8 @@ namespace AZ { + AZ_CLASS_ALLOCATOR_IMPL(BaseJsonIntegerSerializer, SystemAllocator, 0); + AZ_CLASS_ALLOCATOR_IMPL(JsonCharSerializer, SystemAllocator, 0); AZ_CLASS_ALLOCATOR_IMPL(JsonShortSerializer, SystemAllocator, 0); AZ_CLASS_ALLOCATOR_IMPL(JsonIntSerializer, SystemAllocator, 0); @@ -56,19 +58,25 @@ namespace AZ template static JsonSerializationResult::Result LoadInt(T* outputValue, const rapidjson::Value& inputValue, - JsonDeserializerContext& context) + JsonDeserializerContext& context, bool isDefaultValue) { namespace JSR = JsonSerializationResult; // Used remove name conflicts in AzCore in uber builds. static_assert(AZStd::is_integral(), "Expected T to be a signed or unsigned type"); AZ_Assert(outputValue, "Expected a valid pointer to load from json value."); + if (isDefaultValue) + { + *outputValue = 0; + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Integer value set to default of zero."); + } + switch (inputValue.GetType()) { case rapidjson::kArrayType: - // fallthrough + [[fallthrough]]; case rapidjson::kObjectType: - // fallthrough + [[fallthrough]]; case rapidjson::kNullType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. Integers can't be read from arrays, objects or null."); @@ -77,7 +85,7 @@ namespace AZ return TextToValue(outputValue, inputValue.GetString(), context); case rapidjson::kFalseType: - // fallthrough + [[fallthrough]]; case rapidjson::kTrueType: *outputValue = inputValue.GetBool() ? 1 : 0; return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Success, @@ -125,6 +133,11 @@ namespace AZ } } // namespace SerializerInternal + auto BaseJsonIntegerSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + JsonSerializationResult::Result JsonCharSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { @@ -132,7 +145,7 @@ namespace AZ "Unable to deserialize char to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonCharSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, @@ -151,7 +164,7 @@ namespace AZ "Unable to deserialize short to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonShortSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, @@ -170,7 +183,7 @@ namespace AZ "Unable to deserialize int to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonIntSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, @@ -189,7 +202,7 @@ namespace AZ "Unable to deserialize long to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonLongSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, @@ -208,7 +221,7 @@ namespace AZ "Unable to deserialize long long to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonLongLongSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, @@ -227,7 +240,8 @@ namespace AZ "Unable to deserialize unsigned char to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonUnsignedCharSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -246,7 +260,8 @@ namespace AZ "Unable to deserialize unsigned short to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonUnsignedShortSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -265,7 +280,8 @@ namespace AZ "Unable to deserialize unsigned int to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonUnsignedIntSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -284,7 +300,8 @@ namespace AZ "Unable to deserialize unsigned long to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonUnsignedLongSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -303,7 +320,8 @@ namespace AZ "Unable to deserialize unsigned long long to json because the provided type is %s", outputValueTypeId.ToString().c_str()); AZ_UNUSED(outputValueTypeId); - return SerializerInternal::LoadInt(reinterpret_cast(outputValue), inputValue, context); + return SerializerInternal::LoadInt( + reinterpret_cast(outputValue), inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonUnsignedLongLongSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.h index 9f522619ae..31d5a4164a 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.h @@ -18,11 +18,18 @@ namespace AZ { - class JsonCharSerializer - : public BaseJsonSerializer + class BaseJsonIntegerSerializer : public BaseJsonSerializer { public: - AZ_RTTI(JsonCharSerializer, "{CA2A4AAC-3068-40B2-94F8-A537FBA8236E}", BaseJsonSerializer); + AZ_RTTI(BaseJsonIntegerSerializer, "{FD060F54-D3B5-4D5B-B64A-AFE371CD6F20}", BaseJsonSerializer); + AZ_CLASS_ALLOCATOR_DECL; + OperationFlags GetOperationsFlags() const override; + }; + + class JsonCharSerializer : public BaseJsonIntegerSerializer + { + public: + AZ_RTTI(JsonCharSerializer, "{CA2A4AAC-3068-40B2-94F8-A537FBA8236E}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -30,11 +37,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonShortSerializer - : public BaseJsonSerializer + class JsonShortSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonShortSerializer, "{3D6789BD-231B-4E5D-B81D-609E71A2BCB5}", BaseJsonSerializer); + AZ_RTTI(JsonShortSerializer, "{3D6789BD-231B-4E5D-B81D-609E71A2BCB5}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -42,11 +48,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonIntSerializer - : public BaseJsonSerializer + class JsonIntSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonIntSerializer, "{29E26946-0F1F-44B0-A098-1171B7B0C8FA}", BaseJsonSerializer); + AZ_RTTI(JsonIntSerializer, "{29E26946-0F1F-44B0-A098-1171B7B0C8FA}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -54,11 +59,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonLongSerializer - : public BaseJsonSerializer + class JsonLongSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonLongSerializer, "{0EB432D0-A0C8-43B2-9D65-A73A4D6DFE3E}", BaseJsonSerializer); + AZ_RTTI(JsonLongSerializer, "{0EB432D0-A0C8-43B2-9D65-A73A4D6DFE3E}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -66,11 +70,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonLongLongSerializer - : public BaseJsonSerializer + class JsonLongLongSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonLongLongSerializer, "{5E7967DE-A4DC-40E1-81A1-2896A054BB8A}", BaseJsonSerializer); + AZ_RTTI(JsonLongLongSerializer, "{5E7967DE-A4DC-40E1-81A1-2896A054BB8A}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -78,11 +81,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonUnsignedCharSerializer - : public BaseJsonSerializer + class JsonUnsignedCharSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonUnsignedCharSerializer, "{1E6D606F-8490-4736-AAFF-91046FDEA2BB}", BaseJsonSerializer); + AZ_RTTI(JsonUnsignedCharSerializer, "{1E6D606F-8490-4736-AAFF-91046FDEA2BB}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -90,11 +92,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonUnsignedShortSerializer - : public BaseJsonSerializer + class JsonUnsignedShortSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonUnsignedShortSerializer, "{3C92D2CC-CB13-4A40-B779-47562EE36451}", BaseJsonSerializer); + AZ_RTTI(JsonUnsignedShortSerializer, "{3C92D2CC-CB13-4A40-B779-47562EE36451}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -102,11 +103,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonUnsignedIntSerializer - : public BaseJsonSerializer + class JsonUnsignedIntSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonUnsignedIntSerializer, "{70C0714A-690D-4F30-8986-ABC9DEFE9D62}", BaseJsonSerializer); + AZ_RTTI(JsonUnsignedIntSerializer, "{70C0714A-690D-4F30-8986-ABC9DEFE9D62}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -114,11 +114,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonUnsignedLongSerializer - : public BaseJsonSerializer + class JsonUnsignedLongSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonUnsignedLongSerializer, "{28E5499F-6AF4-4778-AE14-66BA40B56247}", BaseJsonSerializer); + AZ_RTTI(JsonUnsignedLongSerializer, "{28E5499F-6AF4-4778-AE14-66BA40B56247}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -126,11 +125,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonUnsignedLongLongSerializer - : public BaseJsonSerializer + class JsonUnsignedLongLongSerializer : public BaseJsonIntegerSerializer { public: - AZ_RTTI(JsonUnsignedLongLongSerializer, "{AB048BB3-C280-4166-9E2E-54CE2C3413CA}", BaseJsonSerializer); + AZ_RTTI(JsonUnsignedLongLongSerializer, "{AB048BB3-C280-4166-9E2E-54CE2C3413CA}", BaseJsonIntegerSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp index cf9d76e79d..6396e4ccdf 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp @@ -63,6 +63,18 @@ namespace JsonSerializationTests : public BaseJsonSerializerFixture { public: + struct BoolPointerWrapper + { + AZ_TYPE_INFO(BoolPointerWrapper, "{2E67C069-BB0F-4F00-A704-E964F5FE5ED2}"); + + bool* m_value{ nullptr }; + + ~BoolPointerWrapper() + { + azfree(m_value); + } + }; + void SetUp() override { BaseJsonSerializerFixture::SetUp(); @@ -75,6 +87,12 @@ namespace JsonSerializationTests BaseJsonSerializerFixture::TearDown(); } + void RegisterAdditional(AZStd::unique_ptr& serializeContext) override + { + serializeContext->Class() + ->Field("Value", &BoolPointerWrapper::m_value); + } + void Load(rapidjson::Value& testVal, bool expectedBool, AZ::JsonSerializationResult::Outcomes expectedOutcome) { using namespace AZ::JsonSerializationResult; @@ -242,4 +260,24 @@ namespace JsonSerializationTests Load(m_jsonValue.SetDouble(-1.0f), true, AZ::JsonSerializationResult::Outcomes::Success); Load(m_jsonValue.SetDouble(2.0), true, AZ::JsonSerializationResult::Outcomes::Success); } + + TEST_F(JsonBoolSerializerTests, Load_LoadDefaultToPointer_ValueIsIsInitialized) + { + using namespace AZ::JsonSerializationResult; + + BoolPointerWrapper instance; + + this->m_jsonDocument->Parse(R"({ "Value": {}})"); + ASSERT_FALSE(this->m_jsonDocument->HasParseError()); + + AZ::JsonDeserializerSettings settings; + settings.m_serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); + settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); + ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); + + EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_NE(nullptr, instance.m_value); + EXPECT_FALSE(*instance.m_value); + } } // namespace JsonSerializationTests diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp index d19c38fcdc..4279017276 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp @@ -71,6 +71,20 @@ namespace JsonSerializationTests : public BaseJsonSerializerFixture { public: + struct DoublePointerWrapper + { + AZ_TYPE_INFO(DoublePointerWrapper, "{C2FD9E0B-2641-4D24-A3D9-A29FD1A21A81}"); + + double* m_double{ nullptr }; + float* m_float{ nullptr }; + + ~DoublePointerWrapper() + { + azfree(m_float); + azfree(m_double); + } + }; + void SetUp() override { BaseJsonSerializerFixture::SetUp(); @@ -85,6 +99,13 @@ namespace JsonSerializationTests BaseJsonSerializerFixture::TearDown(); } + void RegisterAdditional(AZStd::unique_ptr& serializeContext) override + { + serializeContext->Class() + ->Field("Double", &DoublePointerWrapper::m_double) + ->Field("Float", &DoublePointerWrapper::m_float); + } + void TestSerializers(rapidjson::Value& testVal, double expectedValue, AZ::JsonSerializationResult::Outcomes expectedOutcome) { using namespace AZ::JsonSerializationResult; @@ -275,4 +296,32 @@ namespace JsonSerializationTests EXPECT_EQ(Outcomes::Unsupported, result.GetOutcome()); EXPECT_EQ(42.0f, value); } + + // Pointers + + TEST_F(JsonDoubleSerializerTests, Load_LoadDefaultToPointer_ValuesArIsInitialized) + { + using namespace AZ::JsonSerializationResult; + + DoublePointerWrapper instance; + + this->m_jsonDocument->Parse(R"( + { + "Double": {}, + "Float": {} + })"); + ASSERT_FALSE(this->m_jsonDocument->HasParseError()); + + AZ::JsonDeserializerSettings settings; + settings.m_serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); + settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); + ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); + + EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_NE(nullptr, instance.m_double); + ASSERT_NE(nullptr, instance.m_float); + EXPECT_DOUBLE_EQ(0.0, *instance.m_double); + EXPECT_FLOAT_EQ(0.0f, *instance.m_float); + } } // namespace JsonSerializationTests diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp index db764badcb..c4132e983c 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp @@ -147,6 +147,18 @@ namespace JsonSerializationTests : public BaseJsonSerializerFixture { public: + struct IntegerPointerWrapper + { + AZ_TYPE_INFO(IntegerPointerWrapper, "{F6B3BEF1-59A4-4E45-BF02-DDA868C38A28}"); + + typename SerializerInfo::DataType* m_value{ nullptr }; + + ~IntegerPointerWrapper() + { + azfree(m_value); + } + }; + AZStd::unique_ptr m_serializer; void SetUp() override @@ -161,6 +173,12 @@ namespace JsonSerializationTests BaseJsonSerializerFixture::TearDown(); } + void RegisterAdditional(AZStd::unique_ptr& serializeContext) override + { + serializeContext->Class() + ->Field("Value", &IntegerPointerWrapper::m_value); + } + template::value, int> = 0> void SetValue(rapidjson::Value& out, T in) { @@ -487,6 +505,26 @@ namespace JsonSerializationTests EXPECT_EQ(typename SerializerInfo::DataType(), convertedValue); } + TYPED_TEST(TypedJsonIntSerializerTests, Load_LoadDefaultToPointer_ValueIsIsInitialized) + { + using namespace AZ::JsonSerializationResult; + + IntegerPointerWrapper instance; + + this->m_jsonDocument->Parse(R"({ "Value": {}})"); + ASSERT_FALSE(this->m_jsonDocument->HasParseError()); + + AZ::JsonDeserializerSettings settings; + settings.m_serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); + settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); + ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); + + EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_NE(nullptr, instance.m_value); + EXPECT_EQ(0, *instance.m_value); + } + TYPED_TEST(TypedJsonIntSerializerTests, Load_MaxInt8Value_ConvertIfFitsOrUnsupported) { this->template TestMaxValue(); } TYPED_TEST(TypedJsonIntSerializerTests, Load_MaxShortValue_ConvertIfFitsOrUnsupported) { this->template TestMaxValue(); } TYPED_TEST(TypedJsonIntSerializerTests, Load_MaxIntValue_ConvertIfFitsOrUnsupported) { this->template TestMaxValue(); } From 716e99a8a74cf0c1952c96041896653191a31785 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Fri, 11 Jun 2021 15:28:56 -0700 Subject: [PATCH 04/15] Reverted string change because it cause incorrect string sizes. --- .../AzCore/AzCore/Serialization/Json/BoolSerializer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp index 7ac16b214d..4d0a29df71 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp @@ -30,8 +30,8 @@ namespace AZ if (text && textLength > 0) { - static constexpr const char* trueString = "true"; - static constexpr const char* falseString = "false"; + static constexpr const char trueString[] = "true"; + static constexpr const char falseString[] = "false"; // remove null terminator for string length counts // rapidjson stringlength doesn't include it in length calculations, but sizeof() will static constexpr size_t trueStringLength = sizeof(trueString) - 1; From bc5fc9a1914d8f94a6503479c9caa376d79b92ee Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Fri, 11 Jun 2021 19:07:41 -0700 Subject: [PATCH 05/15] Added missing reflection to NameJsonSerializerTests --- Code/Framework/AzCore/Tests/Name/NameJsonSerializerTests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Code/Framework/AzCore/Tests/Name/NameJsonSerializerTests.cpp b/Code/Framework/AzCore/Tests/Name/NameJsonSerializerTests.cpp index 4fa816cd4d..cb26937824 100644 --- a/Code/Framework/AzCore/Tests/Name/NameJsonSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Name/NameJsonSerializerTests.cpp @@ -32,6 +32,11 @@ namespace JsonSerializationTests AZ::NameDictionary::Destroy(); } + void Reflect(AZStd::unique_ptr& context) + { + AZ::Name::Reflect(context.get()); + } + void Reflect(AZStd::unique_ptr& context) { AZ::Name::Reflect(context.get()); From 08abc497f39a746c2812eff7a8173a4c0ce8ab63 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Mon, 14 Jun 2021 09:55:24 -0700 Subject: [PATCH 06/15] Fixed initialization of math types for Json Serialization Several math types in AzCore deliberately don't initialize through a constructor. This set of changes make sure that they still get properly initialized in the Json Serialization instead having random values. --- .../AzCore/AzCore/Math/ColorSerializer.cpp | 23 ++++-- .../AzCore/AzCore/Math/ColorSerializer.h | 2 + .../AzCore/Math/MathMatrixSerializer.cpp | 71 ++++++++----------- .../AzCore/AzCore/Math/MathMatrixSerializer.h | 23 +++--- .../AzCore/Math/MathVectorSerializer.cpp | 43 ++++++++--- .../AzCore/AzCore/Math/MathVectorSerializer.h | 28 ++++---- .../AzCore/Math/TransformSerializer.cpp | 13 +++- .../AzCore/AzCore/Math/TransformSerializer.h | 2 + .../AzCore/AzCore/Math/UuidSerializer.cpp | 26 +++++-- .../AzCore/AzCore/Math/UuidSerializer.h | 2 + 10 files changed, 150 insertions(+), 83 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp index 3f7a3c3a5d..ffdd73ffd8 100644 --- a/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp @@ -36,6 +36,12 @@ namespace AZ Color* color = reinterpret_cast(outputValue); AZ_Assert(color, "Output value for JsonColorSerializer can't be null."); + if (IsExplicitDefault(inputValue)) + { + *color = Color::CreateZero(); + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Color value set to default of zero."); + } + switch (inputValue.GetType()) { case rapidjson::kArrayType: @@ -43,10 +49,14 @@ namespace AZ case rapidjson::kObjectType: return LoadObject(*color, inputValue, context); - case rapidjson::kStringType: // fall through - case rapidjson::kNumberType: // fall through - case rapidjson::kNullType: // fall through - case rapidjson::kFalseType: // fall through + case rapidjson::kStringType: + [[fallthrough]]; + case rapidjson::kNumberType: + [[fallthrough]]; + case rapidjson::kNullType: + [[fallthrough]]; + case rapidjson::kFalseType: + [[fallthrough]]; case rapidjson::kTrueType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. Colors can only be read from arrays or objects."); @@ -91,6 +101,11 @@ namespace AZ } } + auto JsonColorSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + JsonSerializationResult::Result JsonColorSerializer::LoadObject(Color& output, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { diff --git a/Code/Framework/AzCore/AzCore/Math/ColorSerializer.h b/Code/Framework/AzCore/AzCore/Math/ColorSerializer.h index b7b948a25a..cd16506b28 100644 --- a/Code/Framework/AzCore/AzCore/Math/ColorSerializer.h +++ b/Code/Framework/AzCore/AzCore/Math/ColorSerializer.h @@ -28,6 +28,8 @@ namespace AZ JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; + private: enum class LoadAlpha { diff --git a/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp index 0b7e3300cf..3d6a378b13 100644 --- a/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp @@ -263,7 +263,7 @@ namespace AZ::JsonMathMatrixSerializerInternal template JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, - const rapidjson::Value& inputValue, JsonDeserializerContext& context) + const rapidjson::Value& inputValue, JsonDeserializerContext& context, bool isExplicitDefault) { namespace JSR = JsonSerializationResult; // Used remove name conflicts in AzCore in uber builds. @@ -279,6 +279,12 @@ namespace AZ::JsonMathMatrixSerializerInternal MatrixType* matrix = reinterpret_cast(outputValue); AZ_Assert(matrix, "Output value for JsonMatrix%zux%zuSerializer can't be null.", RowCount, ColumnCount); + if (isExplicitDefault) + { + *matrix = MatrixType::CreateIdentity(); + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Matrix value set to identity matrix."); + } + switch (inputValue.GetType()) { case rapidjson::kArrayType: @@ -381,6 +387,16 @@ namespace AZ::JsonMathMatrixSerializerInternal namespace AZ { + // BaseJsonMatrixSerializer + + AZ_CLASS_ALLOCATOR_IMPL(BaseJsonMatrixSerializer, SystemAllocator, 0); + + auto BaseJsonMatrixSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + + // Matrix3x3 AZ_CLASS_ALLOCATOR_IMPL(JsonMatrix3x3Serializer, SystemAllocator, 0); @@ -389,10 +405,7 @@ namespace AZ const rapidjson::Value& inputValue, JsonDeserializerContext& context) { return JsonMathMatrixSerializerInternal::Load( - outputValue, - outputValueTypeId, - inputValue, - context); + outputValue, outputValueTypeId, inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonMatrix3x3Serializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -401,11 +414,7 @@ namespace AZ outputValue.SetObject(); return JsonMathMatrixSerializerInternal::StoreRotationAndScale( - outputValue, - inputValue, - defaultValue, - valueTypeId, - context); + outputValue, inputValue, defaultValue, valueTypeId, context); } @@ -417,10 +426,7 @@ namespace AZ const rapidjson::Value& inputValue, JsonDeserializerContext& context) { return JsonMathMatrixSerializerInternal::Load( - outputValue, - outputValueTypeId, - inputValue, - context); + outputValue, outputValueTypeId, inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonMatrix3x4Serializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -428,19 +434,11 @@ namespace AZ { outputValue.SetObject(); - auto result = JsonMathMatrixSerializerInternal::StoreRotationAndScale( - outputValue, - inputValue, - defaultValue, - valueTypeId, - context); + auto result = + JsonMathMatrixSerializerInternal::StoreRotationAndScale(outputValue, inputValue, defaultValue, valueTypeId, context); - auto resultTranslation = JsonMathMatrixSerializerInternal::StoreTranslation( - outputValue, - inputValue, - defaultValue, - valueTypeId, - context); + auto resultTranslation = + JsonMathMatrixSerializerInternal::StoreTranslation(outputValue, inputValue, defaultValue, valueTypeId, context); result.GetResultCode().Combine(resultTranslation); return result; @@ -454,10 +452,7 @@ namespace AZ const rapidjson::Value& inputValue, JsonDeserializerContext& context) { return JsonMathMatrixSerializerInternal::Load( - outputValue, - outputValueTypeId, - inputValue, - context); + outputValue, outputValueTypeId, inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonMatrix4x4Serializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -465,19 +460,11 @@ namespace AZ { outputValue.SetObject(); - auto result = JsonMathMatrixSerializerInternal::StoreRotationAndScale( - outputValue, - inputValue, - defaultValue, - valueTypeId, - context); + auto result = + JsonMathMatrixSerializerInternal::StoreRotationAndScale(outputValue, inputValue, defaultValue, valueTypeId, context); - auto resultTranslation = JsonMathMatrixSerializerInternal::StoreTranslation( - outputValue, - inputValue, - defaultValue, - valueTypeId, - context); + auto resultTranslation = + JsonMathMatrixSerializerInternal::StoreTranslation(outputValue, inputValue, defaultValue, valueTypeId, context); result.GetResultCode().Combine(resultTranslation); return result; diff --git a/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.h b/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.h index 81c9635a79..773859a2c8 100644 --- a/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.h +++ b/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.h @@ -16,11 +16,18 @@ namespace AZ { - class JsonMatrix3x3Serializer - : public BaseJsonSerializer + class BaseJsonMatrixSerializer : public BaseJsonSerializer { public: - AZ_RTTI(JsonMatrix3x3Serializer, "{8C76CD6A-8576-4604-A746-CF7A7F20F366}", BaseJsonSerializer); + AZ_RTTI(BaseJsonMatrixSerializer, "{18CA4637-C9B7-454B-9126-107E18A8C096}", BaseJsonSerializer); + AZ_CLASS_ALLOCATOR_DECL; + OperationFlags GetOperationsFlags() const override; + }; + + class JsonMatrix3x3Serializer : public BaseJsonMatrixSerializer + { + public: + AZ_RTTI(JsonMatrix3x3Serializer, "{8C76CD6A-8576-4604-A746-CF7A7F20F366}", BaseJsonMatrixSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -28,11 +35,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonMatrix3x4Serializer - : public BaseJsonSerializer + class JsonMatrix3x4Serializer : public BaseJsonMatrixSerializer { public: - AZ_RTTI(JsonMatrix3x4Serializer, "{E801333B-4AF1-4F43-976C-579670B02DC5}", BaseJsonSerializer); + AZ_RTTI(JsonMatrix3x4Serializer, "{E801333B-4AF1-4F43-976C-579670B02DC5}", BaseJsonMatrixSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -40,11 +46,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonMatrix4x4Serializer - : public BaseJsonSerializer + class JsonMatrix4x4Serializer : public BaseJsonMatrixSerializer { public: - AZ_RTTI(JsonMatrix4x4Serializer, "{46E888FC-248A-4910-9221-4E101A10AEA1}", BaseJsonSerializer); + AZ_RTTI(JsonMatrix4x4Serializer, "{46E888FC-248A-4910-9221-4E101A10AEA1}", BaseJsonMatrixSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; diff --git a/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp index aa9686f709..0c0bc04338 100644 --- a/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp @@ -124,7 +124,7 @@ namespace AZ template JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, - JsonDeserializerContext& context) + JsonDeserializerContext& context, bool isExplicitDefault) { namespace JSR = JsonSerializationResult; // Used remove name conflicts in AzCore in uber builds. @@ -138,6 +138,12 @@ namespace AZ VectorType* vector = reinterpret_cast(outputValue); AZ_Assert(vector, "Output value for JsonVector%iSerializer can't be null.", ElementCount); + if (isExplicitDefault) + { + *vector = VectorType::CreateZero(); + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Math vector value set to default of zero."); + } + switch (inputValue.GetType()) { case rapidjson::kArrayType: @@ -145,10 +151,14 @@ namespace AZ case rapidjson::kObjectType: return LoadObject(*vector, inputValue, context); - case rapidjson::kStringType: // fall through - case rapidjson::kNumberType: // fall through - case rapidjson::kNullType: // fall through - case rapidjson::kFalseType: // fall through + case rapidjson::kStringType: + [[fallthrough]]; + case rapidjson::kNumberType: + [[fallthrough]]; + case rapidjson::kNullType: + [[fallthrough]]; + case rapidjson::kFalseType: + [[fallthrough]]; case rapidjson::kTrueType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. Math vectors can only be read from arrays or objects."); @@ -189,6 +199,16 @@ namespace AZ } } + + // BaseJsonVectorSerializer + + AZ_CLASS_ALLOCATOR_IMPL(BaseJsonVectorSerializer, SystemAllocator, 0); + + auto BaseJsonVectorSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + // Vector2 @@ -197,7 +217,8 @@ namespace AZ JsonSerializationResult::Result JsonVector2Serializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { - return JsonMathVectorSerializerInternal::Load(outputValue, outputValueTypeId, inputValue, context); + return JsonMathVectorSerializerInternal::Load( + outputValue, outputValueTypeId, inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonVector2Serializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -214,7 +235,8 @@ namespace AZ JsonSerializationResult::Result JsonVector3Serializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { - return JsonMathVectorSerializerInternal::Load(outputValue, outputValueTypeId, inputValue, context); + return JsonMathVectorSerializerInternal::Load( + outputValue, outputValueTypeId, inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonVector3Serializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -231,7 +253,8 @@ namespace AZ JsonSerializationResult::Result JsonVector4Serializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { - return JsonMathVectorSerializerInternal::Load(outputValue, outputValueTypeId, inputValue, context); + return JsonMathVectorSerializerInternal::Load( + outputValue, outputValueTypeId, inputValue, context, IsExplicitDefault(inputValue)); } JsonSerializationResult::Result JsonVector4Serializer::Store(rapidjson::Value& outputValue, const void* inputValue, @@ -252,7 +275,7 @@ namespace AZ // check for "yaw, pitch, roll" object if (inputValue.IsObject()) { - if (inputValue.GetObject().ObjectEmpty()) + if (IsExplicitDefault(inputValue)) { Quaternion* outQuaternion = reinterpret_cast(outputValue); *outQuaternion = Quaternion::CreateIdentity(); @@ -283,7 +306,7 @@ namespace AZ return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Success, "Successfully read quaternion."); } - return JsonMathVectorSerializerInternal::Load(outputValue, outputValueTypeId, inputValue, context); + return JsonMathVectorSerializerInternal::Load(outputValue, outputValueTypeId, inputValue, context, false); } JsonSerializationResult::Result JsonQuaternionSerializer::Store(rapidjson::Value& outputValue, const void* inputValue, diff --git a/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.h b/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.h index 9c2606b932..0f81709c17 100644 --- a/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.h +++ b/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.h @@ -16,11 +16,18 @@ namespace AZ { - class JsonVector2Serializer - : public BaseJsonSerializer + class BaseJsonVectorSerializer : public BaseJsonSerializer { public: - AZ_RTTI(JsonVector2Serializer, "{E1EAA209-9682-4120-B26B-3EDD9AD56D6F}", BaseJsonSerializer); + AZ_RTTI(BaseJsonVectorSerializer, "{C188D355-E6DF-4590-8B31-F40591F48A8E}", BaseJsonSerializer); + AZ_CLASS_ALLOCATOR_DECL; + OperationFlags GetOperationsFlags() const override; + }; + + class JsonVector2Serializer : public BaseJsonVectorSerializer + { + public: + AZ_RTTI(JsonVector2Serializer, "{E1EAA209-9682-4120-B26B-3EDD9AD56D6F}", BaseJsonVectorSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -28,11 +35,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonVector3Serializer - : public BaseJsonSerializer + class JsonVector3Serializer : public BaseJsonVectorSerializer { public: - AZ_RTTI(JsonVector3Serializer, "{BF82BBF3-3CD9-48DA-97CC-E4DF2EF01552}", BaseJsonSerializer); + AZ_RTTI(JsonVector3Serializer, "{BF82BBF3-3CD9-48DA-97CC-E4DF2EF01552}", BaseJsonVectorSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -40,11 +46,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonVector4Serializer - : public BaseJsonSerializer + class JsonVector4Serializer : public BaseJsonVectorSerializer { public: - AZ_RTTI(JsonVector4Serializer, "{05B45EA7-7102-4281-8AA0-2AC72D74AAFD}", BaseJsonSerializer); + AZ_RTTI(JsonVector4Serializer, "{05B45EA7-7102-4281-8AA0-2AC72D74AAFD}", BaseJsonVectorSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; @@ -52,11 +57,10 @@ namespace AZ const Uuid& valueTypeId, JsonSerializerContext& context) override; }; - class JsonQuaternionSerializer - : public BaseJsonSerializer + class JsonQuaternionSerializer : public BaseJsonVectorSerializer { public: - AZ_RTTI(JsonQuaternionSerializer, "{18604375-3606-49AC-B366-0F6DF9149FF3}", BaseJsonSerializer); + AZ_RTTI(JsonQuaternionSerializer, "{18604375-3606-49AC-B366-0F6DF9149FF3}", BaseJsonVectorSerializer); AZ_CLASS_ALLOCATOR_DECL; JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; diff --git a/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp index 36c40265af..49ba082618 100644 --- a/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp @@ -33,6 +33,12 @@ namespace AZ AZ::Transform* transformInstance = reinterpret_cast(outputValue); AZ_Assert(transformInstance, "Output value for JsonTransformSerializer can't be null."); + if (IsExplicitDefault(inputValue)) + { + *transformInstance = AZ::Transform::CreateIdentity(); + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Transform value set to identity."); + } + JSR::ResultCode result(JSR::Tasks::ReadField); { @@ -72,7 +78,7 @@ namespace AZ return context.Report( result, - result.GetProcessing() != JSR::Processing::Halted ? "Succesfully loaded Transform information." + result.GetProcessing() != JSR::Processing::Halted ? "Successfully loaded Transform information." : "Failed to load Transform information."); } @@ -140,4 +146,9 @@ namespace AZ : "Failed to store Transform information."); } + auto JsonTransformSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Math/TransformSerializer.h b/Code/Framework/AzCore/AzCore/Math/TransformSerializer.h index e03c41e31d..ef277f831a 100644 --- a/Code/Framework/AzCore/AzCore/Math/TransformSerializer.h +++ b/Code/Framework/AzCore/AzCore/Math/TransformSerializer.h @@ -30,6 +30,8 @@ namespace AZ rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; + private: // Note: These need to be defined as "const char[]" instead of "const char*" so that they can be implicitly converted // to a rapidjson::GenericStringRef<>. (This also lets rapidjson get the string length at compile time) diff --git a/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp index 9da7b11805..2c751e8a1c 100644 --- a/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp @@ -33,6 +33,11 @@ namespace AZ AZStd::regex_constants::icase | AZStd::regex_constants::optimize); } + auto JsonUuidSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::ManualDefault; + } + JsonSerializationResult::Result JsonUuidSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) { @@ -53,13 +58,24 @@ namespace AZ Uuid* valAsUuid = reinterpret_cast(outputValue); + if (IsExplicitDefault(inputValue)) + { + *valAsUuid = AZ::Uuid::CreateNull(); + return MessageResult("Uuid value set to default of null.", JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed)); + } + switch (inputValue.GetType()) { - case rapidjson::kArrayType: // fallthrough - case rapidjson::kObjectType:// fallthrough - case rapidjson::kFalseType: // fallthrough - case rapidjson::kTrueType: // fallthrough - case rapidjson::kNumberType:// fallthrough + case rapidjson::kArrayType: + [[fallthrough]]; + case rapidjson::kObjectType: + [[fallthrough]]; + case rapidjson::kFalseType: + [[fallthrough]]; + case rapidjson::kTrueType: + [[fallthrough]]; + case rapidjson::kNumberType: + [[fallthrough]]; case rapidjson::kNullType: return MessageResult("Unsupported type. Uuids can only be read from strings.", JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported)); diff --git a/Code/Framework/AzCore/AzCore/Math/UuidSerializer.h b/Code/Framework/AzCore/AzCore/Math/UuidSerializer.h index 1c350a2fcd..43487dcc10 100644 --- a/Code/Framework/AzCore/AzCore/Math/UuidSerializer.h +++ b/Code/Framework/AzCore/AzCore/Math/UuidSerializer.h @@ -41,6 +41,8 @@ namespace AZ JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; + //! Does the same as load, but doesn't report through the provided callback in the settings. Instead the final //! ResultCode and message are returned and it's up to the caller to report if need needed. MessageResult UnreportedLoad(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue); From fcd989c295ab2191a87aac6a073c32a50a4fb696 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Mon, 14 Jun 2021 10:00:01 -0700 Subject: [PATCH 07/15] Removed unit tests from bool, int and double Json Serialization A future commit will include a generic test conformity test suite to replace these. --- .../Json/BoolSerializerTests.cpp | 20 ------------- .../Json/DoubleSerializerTests.cpp | 30 +------------------ .../Serialization/Json/IntSerializerTests.cpp | 20 ------------- 3 files changed, 1 insertion(+), 69 deletions(-) diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp index 6396e4ccdf..cd6fcef5c5 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/BoolSerializerTests.cpp @@ -260,24 +260,4 @@ namespace JsonSerializationTests Load(m_jsonValue.SetDouble(-1.0f), true, AZ::JsonSerializationResult::Outcomes::Success); Load(m_jsonValue.SetDouble(2.0), true, AZ::JsonSerializationResult::Outcomes::Success); } - - TEST_F(JsonBoolSerializerTests, Load_LoadDefaultToPointer_ValueIsIsInitialized) - { - using namespace AZ::JsonSerializationResult; - - BoolPointerWrapper instance; - - this->m_jsonDocument->Parse(R"({ "Value": {}})"); - ASSERT_FALSE(this->m_jsonDocument->HasParseError()); - - AZ::JsonDeserializerSettings settings; - settings.m_serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); - settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); - ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); - - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); - EXPECT_EQ(Processing::Completed, result.GetProcessing()); - ASSERT_NE(nullptr, instance.m_value); - EXPECT_FALSE(*instance.m_value); - } } // namespace JsonSerializationTests diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp index 4279017276..1556318199 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/DoubleSerializerTests.cpp @@ -32,7 +32,7 @@ namespace JsonSerializationTests AZStd::shared_ptr CreateDefaultInstance() override { - return AZStd::make_shared(-2.0f); + return AZStd::make_shared(0.0f); } AZStd::shared_ptr CreateFullySetInstance() override @@ -296,32 +296,4 @@ namespace JsonSerializationTests EXPECT_EQ(Outcomes::Unsupported, result.GetOutcome()); EXPECT_EQ(42.0f, value); } - - // Pointers - - TEST_F(JsonDoubleSerializerTests, Load_LoadDefaultToPointer_ValuesArIsInitialized) - { - using namespace AZ::JsonSerializationResult; - - DoublePointerWrapper instance; - - this->m_jsonDocument->Parse(R"( - { - "Double": {}, - "Float": {} - })"); - ASSERT_FALSE(this->m_jsonDocument->HasParseError()); - - AZ::JsonDeserializerSettings settings; - settings.m_serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); - settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); - ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); - - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); - EXPECT_EQ(Processing::Completed, result.GetProcessing()); - ASSERT_NE(nullptr, instance.m_double); - ASSERT_NE(nullptr, instance.m_float); - EXPECT_DOUBLE_EQ(0.0, *instance.m_double); - EXPECT_FLOAT_EQ(0.0f, *instance.m_float); - } } // namespace JsonSerializationTests diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp index c4132e983c..5f81dd1b9c 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp @@ -505,26 +505,6 @@ namespace JsonSerializationTests EXPECT_EQ(typename SerializerInfo::DataType(), convertedValue); } - TYPED_TEST(TypedJsonIntSerializerTests, Load_LoadDefaultToPointer_ValueIsIsInitialized) - { - using namespace AZ::JsonSerializationResult; - - IntegerPointerWrapper instance; - - this->m_jsonDocument->Parse(R"({ "Value": {}})"); - ASSERT_FALSE(this->m_jsonDocument->HasParseError()); - - AZ::JsonDeserializerSettings settings; - settings.m_serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); - settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); - ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); - - EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); - EXPECT_EQ(Processing::Completed, result.GetProcessing()); - ASSERT_NE(nullptr, instance.m_value); - EXPECT_EQ(0, *instance.m_value); - } - TYPED_TEST(TypedJsonIntSerializerTests, Load_MaxInt8Value_ConvertIfFitsOrUnsupported) { this->template TestMaxValue(); } TYPED_TEST(TypedJsonIntSerializerTests, Load_MaxShortValue_ConvertIfFitsOrUnsupported) { this->template TestMaxValue(); } TYPED_TEST(TypedJsonIntSerializerTests, Load_MaxIntValue_ConvertIfFitsOrUnsupported) { this->template TestMaxValue(); } From f00fa26e1246eb2ac7e3ebcb7dec8b3657f4600c Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Tue, 15 Jun 2021 13:44:32 -0700 Subject: [PATCH 08/15] Separated initializing new and all objects in the Json Serialization Introduced OperationFlags::InitializeNewInstance to the Json Serialization which allows custom json serializers to indicate that they need to set defaults only to new instances. Objects created to fill in a pointer are considered new objects and serializer can use the new ContinuationFlags::LoadAsNewInstance to also inform that the load is happening on a new object. Serializer that use the InitializeNewInstance flag know that a new object is begin initialized if they're called with an explicit default object. --- .../AzCore/AzCore/Math/ColorSerializer.cpp | 2 +- .../AzCore/Math/MathMatrixSerializer.cpp | 2 +- .../AzCore/Math/MathVectorSerializer.cpp | 2 +- .../AzCore/Math/TransformSerializer.cpp | 2 +- .../AzCore/AzCore/Math/UuidSerializer.cpp | 2 +- .../Serialization/Json/BaseJsonSerializer.cpp | 3 ++- .../Serialization/Json/BaseJsonSerializer.h | 15 +++++++++----- .../Serialization/Json/BoolSerializer.cpp | 2 +- .../Serialization/Json/DoubleSerializer.cpp | 4 ++-- .../Serialization/Json/IntSerializer.cpp | 2 +- .../Serialization/Json/JsonDeserializer.cpp | 20 +++++++++++-------- .../Serialization/Json/JsonDeserializer.h | 5 +++-- .../Serialization/Json/JsonSerialization.cpp | 2 +- 13 files changed, 37 insertions(+), 26 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp index ffdd73ffd8..0d45a3fb6c 100644 --- a/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/ColorSerializer.cpp @@ -103,7 +103,7 @@ namespace AZ auto JsonColorSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } JsonSerializationResult::Result JsonColorSerializer::LoadObject(Color& output, const rapidjson::Value& inputValue, diff --git a/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp index 3d6a378b13..7f412f68e6 100644 --- a/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/MathMatrixSerializer.cpp @@ -393,7 +393,7 @@ namespace AZ auto BaseJsonMatrixSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } diff --git a/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp index 0c0bc04338..7225f1713c 100644 --- a/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/MathVectorSerializer.cpp @@ -206,7 +206,7 @@ namespace AZ auto BaseJsonVectorSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } diff --git a/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp index 49ba082618..b3c9ea3b7c 100644 --- a/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/TransformSerializer.cpp @@ -148,7 +148,7 @@ namespace AZ auto JsonTransformSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp b/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp index 2c751e8a1c..458744d5b2 100644 --- a/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Math/UuidSerializer.cpp @@ -35,7 +35,7 @@ namespace AZ auto JsonUuidSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } JsonSerializationResult::Result JsonUuidSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.cpp index 9a426a1e59..666fa0f96a 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.cpp @@ -216,9 +216,10 @@ namespace AZ JsonSerializationResult::ResultCode BaseJsonSerializer::ContinueLoading( void* object, const Uuid& typeId, const rapidjson::Value& value, JsonDeserializerContext& context, ContinuationFlags flags) { + bool loadAsNewInstance = (flags & ContinuationFlags::LoadAsNewInstance) == ContinuationFlags::LoadAsNewInstance; return (flags & ContinuationFlags::ResolvePointer) == ContinuationFlags::ResolvePointer ? JsonDeserializer::LoadToPointer(object, typeId, value, context) - : JsonDeserializer::Load(object, typeId, value, context); + : JsonDeserializer::Load(object, typeId, value, loadAsNewInstance, context); } JsonSerializationResult::ResultCode BaseJsonSerializer::ContinueStoring( diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.h index 06c5eda6de..99b63bf4de 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BaseJsonSerializer.h @@ -163,15 +163,20 @@ namespace AZ enum class ContinuationFlags { - None = 0, //! No extra flags. - ResolvePointer = 1 << 0, //! The pointer passed in contains a pointer. The (de)serializer will attempt to resolve to an instance. - ReplaceDefault = 1 << 1 //! The default value provided for storing will be replaced with a newly created one. + None = 0, //! No extra flags. + ResolvePointer = 1 << 0, //! The pointer passed in contains a pointer. The (de)serializer will attempt to resolve to an instance. + ReplaceDefault = 1 << 1, //! The default value provided for storing will be replaced with a newly created one. + LoadAsNewInstance = 1 << 2 //! Treats the value as if it's a newly created instance. This may trigger serializers marked with + //! OperationFlags::InitializeNewInstance. Used for instance by pointers or new instances added to + //! an array. }; enum class OperationFlags { - None = 0, //! No flags that control how the custom json serializer is used. - ManualDefault = 1 << 0 //! Even if an (explicit) default is found the custom json serializer will still be called. + None = 0, //! No flags that control how the custom json serializer is used. + ManualDefault = 1 << 0, //! Even if an (explicit) default is found the custom json serializer will still be called. + InitializeNewInstance = 1 << 1 //! If set, the custom json serializer will be called with an explicit default if a new + //! instance of its target type is created. }; virtual ~BaseJsonSerializer() = default; diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp index 4d0a29df71..defe470bcf 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp @@ -154,6 +154,6 @@ namespace AZ auto JsonBoolSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp index 647ff67eb6..14f2eaae6f 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp @@ -166,7 +166,7 @@ namespace AZ auto JsonDoubleSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } JsonSerializationResult::Result JsonFloatSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, @@ -191,6 +191,6 @@ namespace AZ auto JsonFloatSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp index 28da852da3..ebc99fd8c4 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp @@ -135,7 +135,7 @@ namespace AZ auto BaseJsonIntegerSerializer::GetOperationsFlags() const -> OperationFlags { - return OperationFlags::ManualDefault; + return OperationFlags::InitializeNewInstance; } JsonSerializationResult::Result JsonCharSerializer::Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp index 9c4641741e..d948966b4b 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.cpp @@ -24,20 +24,24 @@ namespace AZ { JsonSerializationResult::ResultCode JsonDeserializer::DeserializerDefaultCheck(BaseJsonSerializer* serializer, void* object, - const Uuid& typeId, const rapidjson::Value& value, JsonDeserializerContext& context) + const Uuid& typeId,const rapidjson::Value& value, bool isNewInstance, JsonDeserializerContext& context) { using namespace AZ::JsonSerializationResult; bool isExplicitDefault = IsExplicitDefault(value); bool manuallyDefaults = (serializer->GetOperationsFlags() & BaseJsonSerializer::OperationFlags::ManualDefault) == BaseJsonSerializer::OperationFlags::ManualDefault; - return !isExplicitDefault || (isExplicitDefault && manuallyDefaults) + bool initializeNewInstance = (serializer->GetOperationsFlags() & BaseJsonSerializer::OperationFlags::InitializeNewInstance) == + BaseJsonSerializer::OperationFlags::InitializeNewInstance; + + return + !isExplicitDefault || (isExplicitDefault && manuallyDefaults) || (isExplicitDefault && isNewInstance && initializeNewInstance) ? serializer->Load(object, typeId, value, context) : context.Report(Tasks::ReadField, Outcomes::DefaultsUsed, "Value has an explicit default."); } - JsonSerializationResult::ResultCode JsonDeserializer::Load(void* object, const Uuid& typeId, const rapidjson::Value& value, - JsonDeserializerContext& context) + JsonSerializationResult::ResultCode JsonDeserializer::Load( + void* object, const Uuid& typeId, const rapidjson::Value& value, bool isNewInstance, JsonDeserializerContext& context) { using namespace AZ::JsonSerializationResult; @@ -50,7 +54,7 @@ namespace AZ BaseJsonSerializer* serializer = context.GetRegistrationContext()->GetSerializerForType(typeId); if (serializer) { - return DeserializerDefaultCheck(serializer, object, typeId, value, context); + return DeserializerDefaultCheck(serializer, object, typeId, value, isNewInstance, context); } const SerializeContext::ClassData* classData = context.GetSerializeContext()->FindClassData(typeId); @@ -72,7 +76,7 @@ namespace AZ serializer = context.GetRegistrationContext()->GetSerializerForType(classData->m_azRtti->GetGenericTypeId()); if (serializer) { - return DeserializerDefaultCheck(serializer, object, typeId, value, context); + return DeserializerDefaultCheck(serializer, object, typeId, value, isNewInstance, context); } } @@ -133,7 +137,7 @@ namespace AZ const SerializeContext::ClassData* resolvedClassData = context.GetSerializeContext()->FindClassData(resolvedTypeId); if (resolvedClassData) { - status = JsonDeserializer::Load(*objectPtr, resolvedTypeId, value, context); + status = JsonDeserializer::Load(*objectPtr, resolvedTypeId, value, true, context); *objectPtr = resolvedClassData->m_azRtti->Cast(*objectPtr, typeId); @@ -174,7 +178,7 @@ namespace AZ } else { - return Load(object, classElement.m_typeId, value, context); + return Load(object, classElement.m_typeId, value, false, context); } } diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.h index 5954082ee0..1d16b49e39 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonDeserializer.h @@ -58,8 +58,8 @@ namespace AZ JsonDeserializer(const JsonDeserializer& rhs) = delete; JsonDeserializer(JsonDeserializer&& rhs) = delete; - static JsonSerializationResult::ResultCode Load(void* object, const Uuid& typeId, const rapidjson::Value& value, - JsonDeserializerContext& context); + static JsonSerializationResult::ResultCode Load( + void* object, const Uuid& typeId, const rapidjson::Value& value, bool isNewInstance, JsonDeserializerContext& context); static JsonSerializationResult::ResultCode LoadToPointer(void* object, const Uuid& typeId, const rapidjson::Value& value, JsonDeserializerContext& context); @@ -120,6 +120,7 @@ namespace AZ void* object, const Uuid& typeId, const rapidjson::Value& value, + bool isNewInstance, JsonDeserializerContext& context); }; } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.cpp index 0629f1c32e..6fabf51237 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSerialization.cpp @@ -249,7 +249,7 @@ namespace AZ { StackedString path(StackedString::Format::JsonPointer); JsonDeserializerContext context(settings); - result = JsonDeserializer::Load(object, objectType, root, context); + result = JsonDeserializer::Load(object, objectType, root, false, context); } return result; } From 780dd0df9fe70dbf9c5f3a29a68480fa708b4c5b Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Tue, 15 Jun 2021 13:52:16 -0700 Subject: [PATCH 09/15] Container fixes for the Json Serialization These changes fix the following: - Containers treat new values as new objects and make sure they're initialized. - Fixed sized containers behave slightly different and will initialize all values when a new fixed sized container is created. - Loading any values to a container will now return PartialDefaults instead of defaults used as adding any value to a container no longer makes the container a default as the default is always an empty container. - The previous doesn't apply to fixed sized containers as those containers are always considered to have the exact number of values they can hold. --- .../Serialization/Json/ArraySerializer.cpp | 65 +++++++--- .../Serialization/Json/ArraySerializer.h | 8 +- .../Json/BasicContainerSerializer.cpp | 21 +++- .../Serialization/Json/MapSerializer.cpp | 14 ++- .../Serialization/Json/TupleSerializer.cpp | 113 ++++++++++++------ .../Serialization/Json/TupleSerializer.h | 9 +- 6 files changed, 165 insertions(+), 65 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.cpp index d5a1730364..2e53eb5697 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.cpp @@ -32,13 +32,24 @@ namespace AZ switch (inputValue.GetType()) { case rapidjson::kArrayType: - return LoadContainer(outputValue, outputValueTypeId, inputValue, context); + return LoadContainer(outputValue, outputValueTypeId, inputValue, false, context); - case rapidjson::kObjectType: // fall through - case rapidjson::kNullType: // fall through - case rapidjson::kStringType: // fall through - case rapidjson::kFalseType: // fall through - case rapidjson::kTrueType: // fall through + case rapidjson::kObjectType: + if (IsExplicitDefault(inputValue)) + { + // Because this serializer has only the operation flag "InitializeNewInstance" set, the only time this will be called with + // an explicit default is when a new instance has been created. + return LoadContainer(outputValue, outputValueTypeId, inputValue, true, context); + } + [[fallthrough]]; + case rapidjson::kNullType: + [[fallthrough]]; + case rapidjson::kStringType: + [[fallthrough]]; + case rapidjson::kFalseType: + [[fallthrough]]; + case rapidjson::kTrueType: + [[fallthrough]]; case rapidjson::kNumberType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. AZStd::array entries can only be read from an array."); @@ -129,7 +140,16 @@ namespace AZ } } - JsonSerializationResult::Result JsonArraySerializer::LoadContainer(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, + auto JsonArraySerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::InitializeNewInstance; + } + + JsonSerializationResult::Result JsonArraySerializer::LoadContainer( + void* outputValue, + const Uuid& outputValueTypeId, + const rapidjson::Value& inputValue, + bool isNewInstance, JsonDeserializerContext& context) { namespace JSR = JsonSerializationResult; // Used to remove name conflicts in AzCore in uber builds. @@ -154,14 +174,7 @@ namespace AZ "Unable to retrieve the correct container information for AZStd::array instance."); } - const size_t size = container->Size(outputValue); - if (inputValue.Size() < size) - { - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, - "Not enough entries in JSON array to load an AZStd::array from."); - } - - ContinuationFlags flags = ContinuationFlags::None; + ContinuationFlags flags = isNewInstance ? ContinuationFlags::LoadAsNewInstance : ContinuationFlags::None; Uuid elementTypeId = Uuid::CreateNull(); auto typeEnumCallback = [&elementTypeId, &flags](const Uuid&, const SerializeContext::ClassElement* genericClassElement) { @@ -175,13 +188,23 @@ namespace AZ }; container->EnumTypes(typeEnumCallback); + const size_t size = container->Size(outputValue); + if (!isNewInstance && inputValue.Size() < size) + { + return context.Report( + JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Not enough entries in JSON array to load an AZStd::array from."); + } + + rapidjson::Value explicitDefaultValue = GetExplicitDefault(); + JSR::ResultCode retVal(JSR::Tasks::ReadField); for (size_t i = 0; i < size; ++i) { ScopedContextPath subPath(context, i); void* element = container->GetElementByIndex(outputValue, nullptr, i); - JSR::ResultCode result = ContinueLoading(element, elementTypeId, inputValue[aznumeric_caster(i)], context, flags); + JSR::ResultCode result = ContinueLoading( + element, elementTypeId, isNewInstance ? explicitDefaultValue : inputValue[aznumeric_caster(i)], context, flags); if (result.GetProcessing() == JSR::Processing::Halted) { return context.Report(result, "Failed to load data to element in AZStd::array."); @@ -189,15 +212,19 @@ namespace AZ retVal.Combine(result); } - if (container->Size(outputValue) == inputValue.Size()) + if (isNewInstance) + { + return context.Report(retVal, "Filled new instance of AZStd::array with defaults."); + } + else if (container->Size(outputValue) == inputValue.Size()) { return context.Report(retVal, "Successfully read entries into AZStd::array."); } else { retVal.Combine(JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::Skipped)); - return context.Report(retVal, - "Successfully read available entries into AZStd::array, but there were still values left in the JSON array."); + return context.Report( + retVal, "Successfully read available entries into AZStd::array, but there were still values left in the JSON array."); } } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.h index 11edc87139..a6d8dd37e3 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/ArraySerializer.h @@ -30,8 +30,14 @@ namespace AZ JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; + protected: - JsonSerializationResult::Result LoadContainer(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, + JsonSerializationResult::Result LoadContainer( + void* outputValue, + const Uuid& outputValueTypeId, + const rapidjson::Value& inputValue, + bool isNewInstance, JsonDeserializerContext& context); }; } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp index 8fe1f471c1..4eb4748eb5 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp @@ -34,11 +34,16 @@ namespace AZ case rapidjson::kArrayType: return LoadContainer(outputValue, outputValueTypeId, inputValue, context); - case rapidjson::kObjectType: // fall through - case rapidjson::kNullType: // fall through - case rapidjson::kStringType: // fall through - case rapidjson::kFalseType: // fall through - case rapidjson::kTrueType: // fall through + case rapidjson::kObjectType: + [[fallthrough]]; + case rapidjson::kNullType: + [[fallthrough]]; + case rapidjson::kStringType: + [[fallthrough]]; + case rapidjson::kFalseType: + [[fallthrough]]; + case rapidjson::kTrueType: + [[fallthrough]]; case rapidjson::kNumberType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. Basic containers can only be read from an array."); @@ -169,6 +174,7 @@ namespace AZ ContinuationFlags flags = classElement->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER ? ContinuationFlags::ResolvePointer : ContinuationFlags::None; + flags |= ContinuationFlags::LoadAsNewInstance; const size_t capacity = container->IsFixedCapacity() ? container->Capacity(outputValue) : std::numeric_limits::max(); @@ -247,6 +253,11 @@ namespace AZ } size_t addedCount = container->Size(outputValue) - containerSize; + if (addedCount > 0) + { + // Values were added which means the container is no longer in its default state of being emtpy. + retVal.Combine(JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::Success)); + } AZStd::string_view message = addedCount >= arraySize ? "Successfully read basic container.": addedCount == 0 ? "Unable to read data for basic container." : diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp index 437a648e1e..4dc3c44834 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/MapSerializer.cpp @@ -190,6 +190,12 @@ namespace AZ } size_t addedCount = container->Size(outputValue) - containerSize; + if (addedCount > 0) + { + // If at least one entry was added then the map is no longer in it's default state so + // mark is with success so the result can at best be partial defaults. + retVal.Combine(JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::Success)); + } AZStd::string_view message = addedCount >= maximumSize ? "Successfully read associative container." : addedCount == 0 ? "Unable to read data for the associative container." : @@ -215,10 +221,10 @@ namespace AZ // Load key void* keyAddress = pairContainer->GetElementByIndex(address, pairElement, 0); AZ_Assert(keyAddress, "Element reserved for associative container, but unable to retrieve address of the key."); - ContinuationFlags keyLoadFlags = ContinuationFlags::None; + ContinuationFlags keyLoadFlags = ContinuationFlags::LoadAsNewInstance; if (keyElement->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER) { - keyLoadFlags = ContinuationFlags::ResolvePointer; + keyLoadFlags |= ContinuationFlags::ResolvePointer; *reinterpret_cast(keyAddress) = nullptr; } JSR::ResultCode keyResult = ContinueLoading(keyAddress, keyElement->m_typeId, key, context, keyLoadFlags); @@ -231,10 +237,10 @@ namespace AZ // 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::None; + ContinuationFlags valueLoadFlags = ContinuationFlags::LoadAsNewInstance; if (valueElement->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER) { - valueLoadFlags = ContinuationFlags::ResolvePointer; + valueLoadFlags |= ContinuationFlags::ResolvePointer; *reinterpret_cast(valueAddress) = nullptr; } JSR::ResultCode valueResult = ContinueLoading(valueAddress, valueElement->m_typeId, value, context, valueLoadFlags); diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp index 5b43cec817..fd8951517a 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp @@ -34,13 +34,24 @@ namespace AZ switch (inputValue.GetType()) { case rapidjson::kArrayType: - return LoadContainer(outputValue, outputValueTypeId, inputValue, context); + return LoadContainer(outputValue, outputValueTypeId, inputValue, false, context); - case rapidjson::kObjectType: // fall through - case rapidjson::kNullType: // fall through - case rapidjson::kStringType: // fall through - case rapidjson::kFalseType: // fall through - case rapidjson::kTrueType: // fall through + case rapidjson::kObjectType: + if (IsExplicitDefault(inputValue)) + { + // Because this serializer has only the operation flag "InitializeNewInstance" set, the only time this will be called with + // an explicit default is when a new instance has been created. + return LoadContainer(outputValue, outputValueTypeId, inputValue, true, context); + } + [[fallthrough]]; + case rapidjson::kNullType: + [[fallthrough]]; + case rapidjson::kStringType: + [[fallthrough]]; + case rapidjson::kFalseType: + [[fallthrough]]; + case rapidjson::kTrueType: + [[fallthrough]]; case rapidjson::kNumberType: return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, "Unsupported type. AZStd::pair or AZStd::tuple can only be read from an array."); @@ -127,8 +138,13 @@ namespace AZ } } + auto JsonTupleSerializer::GetOperationsFlags() const -> OperationFlags + { + return OperationFlags::InitializeNewInstance; + } + JsonSerializationResult::Result JsonTupleSerializer::LoadContainer(void* outputValue, const Uuid& outputValueTypeId, - const rapidjson::Value& inputValue, JsonDeserializerContext& context) + const rapidjson::Value& inputValue, bool isNewInstance, JsonDeserializerContext& context) { namespace JSR = JsonSerializationResult; // Used to remove name conflicts in AzCore in uber builds. @@ -154,7 +170,7 @@ namespace AZ }; container->EnumTypes(typeCountCallback); - rapidjson::SizeType arraySize = inputValue.Size(); + rapidjson::SizeType arraySize = isNewInstance ? typeCount : inputValue.Size(); if (arraySize < typeCount) { return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, @@ -171,46 +187,73 @@ namespace AZ container->EnumTypes(typeEnumCallback); JSR::ResultCode retVal(JSR::Tasks::ReadField); - rapidjson::SizeType arrayIndex = 0; - size_t numElementsWritten = 0; - for (size_t i = 0; i < typeCount; ++i) + if (isNewInstance) { - ScopedContextPath subPath(context, i); - - void* elementAddress = container->GetElementByIndex(outputValue, nullptr, i); - AZ_Assert(elementAddress, "Address of AZStd::pair or AZStd::tuple element %zu could not be retrieved.", i); + rapidjson::Value explicitDefaultValue = GetExplicitDefault(); - ContinuationFlags flags = classElements[i]->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER - ? ContinuationFlags::ResolvePointer - : ContinuationFlags::None; - - while (arrayIndex < inputValue.Size()) + for (size_t i = 0; i < typeCount; ++i) { - JSR::ResultCode result = ContinueLoading(elementAddress, classElements[i]->m_typeId, inputValue[arrayIndex], context, flags); + ScopedContextPath subPath(context, i); + + void* elementAddress = container->GetElementByIndex(outputValue, nullptr, i); + AZ_Assert(elementAddress, "Address of AZStd::pair or AZStd::tuple element %zu could not be retrieved.", i); + + ContinuationFlags flags = ContinuationFlags::LoadAsNewInstance; + flags |= + (classElements[i]->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER ? ContinuationFlags::ResolvePointer + : ContinuationFlags::None); + JSR::ResultCode result = ContinueLoading(elementAddress, classElements[i]->m_typeId, explicitDefaultValue, context, flags); retVal.Combine(result); - arrayIndex++; if (result.GetProcessing() == JSR::Processing::Halted) { return context.Report(retVal, "Failed to read element for AZStd::pair or AZStd::tuple."); } - else if (result.GetProcessing() != JSR::Processing::Altered) - { - numElementsWritten++; - break; - } } - } - if (numElementsWritten < typeCount) - { - AZStd::string_view message = numElementsWritten == 0 ? - "Unable to read data for AZStd::pair or AZStd::tuple." : - "Partially read data for AZStd::pair or AZStd::tuple."; - return context.Report(retVal, message); + return context.Report(retVal, "Initialized AZStd::pair or AZStd::tuple to defaults."); } else { - return context.Report(retVal, "Successfully read AZStd::pair or AZStd::tuple."); + rapidjson::SizeType arrayIndex = 0; + size_t numElementsWritten = 0; + for (size_t i = 0; i < typeCount; ++i) + { + ScopedContextPath subPath(context, i); + + void* elementAddress = container->GetElementByIndex(outputValue, nullptr, i); + AZ_Assert(elementAddress, "Address of AZStd::pair or AZStd::tuple element %zu could not be retrieved.", i); + + ContinuationFlags flags = + (classElements[i]->m_flags & SerializeContext::ClassElement::Flags::FLG_POINTER ? ContinuationFlags::ResolvePointer + : ContinuationFlags::None); + while (arrayIndex < inputValue.Size()) + { + JSR::ResultCode result = + ContinueLoading(elementAddress, classElements[i]->m_typeId, inputValue[arrayIndex], context, flags); + retVal.Combine(result); + arrayIndex++; + if (result.GetProcessing() == JSR::Processing::Halted) + { + return context.Report(retVal, "Failed to read element for AZStd::pair or AZStd::tuple."); + } + else if (result.GetProcessing() != JSR::Processing::Altered) + { + numElementsWritten++; + break; + } + } + } + + if (numElementsWritten < typeCount) + { + AZStd::string_view message = numElementsWritten == 0 ? "Unable to read data for AZStd::pair or AZStd::tuple." + : "Partially read data for AZStd::pair or AZStd::tuple."; + return context.Report(retVal, message); + } + else + { + return context.Report(retVal, "Successfully read AZStd::pair or AZStd::tuple."); + } } } } // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.h index a105272811..70a506eb81 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.h @@ -23,13 +23,20 @@ namespace AZ public: AZ_RTTI(JsonTupleSerializer, "{1AA0ADC1-395A-4223-8A73-304ACDEE7793}", BaseJsonSerializer); AZ_CLASS_ALLOCATOR_DECL; + JsonSerializationResult::Result Load(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, JsonDeserializerContext& context) override; JsonSerializationResult::Result Store(rapidjson::Value& outputValue, const void* inputValue, const void* defaultValue, const Uuid& valueTypeId, JsonSerializerContext& context) override; + OperationFlags GetOperationsFlags() const override; + private: - JsonSerializationResult::Result LoadContainer(void* outputValue, const Uuid& outputValueTypeId, const rapidjson::Value& inputValue, + JsonSerializationResult::Result LoadContainer( + void* outputValue, + const Uuid& outputValueTypeId, + const rapidjson::Value& inputValue, + bool isNewInstance, JsonDeserializerContext& context); }; } // namespace AZ From 8af45d28be8c817a2733b71db38c28cd01b87749 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Tue, 15 Jun 2021 14:02:54 -0700 Subject: [PATCH 10/15] Additional unit tests for the Json Serialization To cover the recent changes to the return code from containers and the initialization fixes additional unit tests were added. Almost all new tests are part of the conformity test suite so that they test any custom json serializers outside of AzCore that might need to be updated due to the fixes. --- .../AzCore/Tests/AssetJsonSerializerTests.cpp | 4 + .../Json/ArraySerializerTests.cpp | 63 ++--- .../Json/BasicContainerSerializerTests.cpp | 19 ++ .../Json/JsonSerializerConformityTests.h | 217 +++++++++++++++++- .../Serialization/Json/MapSerializerTests.cpp | 126 +++++++++- .../Json/TupleSerializerTests.cpp | 13 +- .../Json/UnorderedSetSerializerTests.cpp | 10 + 7 files changed, 384 insertions(+), 68 deletions(-) diff --git a/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp b/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp index 3e4ddac3af..ceff314c81 100644 --- a/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp @@ -168,6 +168,10 @@ namespace JsonSerializationTests { features.EnableJsonType(rapidjson::kObjectType); features.m_typeToInject = rapidjson::kNullType; + // The type information in the Serialize Context is incomplete so this test will fail. + // This is because assets have traditionally been treated as a special case, so there's + // information missing in the Json Serialization to deal with these. + features.m_enableNewInstanceTests = false; } bool AreEqual(const Asset& lhs, const Asset& rhs) override diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp index da66bdd0b5..1f39feccbd 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp @@ -126,9 +126,9 @@ namespace JsonSerializationTests { auto array = AZStd::shared_ptr(new Array(), Deleter); (*array)[0] = nullptr; - (*array)[1] = aznew MultipleInheritence(); + (*array)[1] = nullptr; (*array)[2] = nullptr; - (*array)[3] = aznew MultipleInheritence(); + (*array)[3] = nullptr; return array; } @@ -154,6 +154,7 @@ namespace JsonSerializationTests null, null, { + "$type": "MultipleInheritence", "base_var": 242.0, "var1" : 142 } @@ -246,56 +247,21 @@ namespace JsonSerializationTests ])"; } - AZStd::string_view GetJsonFor_Store_SerializeFullySetInstance() override - { - // This is a unique situation because the $type is determined separate from other values, so all - // member values can be changed, but since the default type matches the stored type the $type - // will only be written if default values are explicitly kept. - return R"( - [ - { - "$type": "MultipleInheritence", - "base_var": 1142.0, - "base2_var1": 1242.0, - "base2_var2": 1342.0, - "base2_var3": 1442.0, - "var1" : 1542, - "var2" : 1642.0 - }, - { - "base_var": 2142.0, - "base2_var1": 2242.0, - "base2_var2": 2342.0, - "base2_var3": 2442.0, - "var1" : 2542, - "var2" : 2642.0 - }, - { - "$type": "MultipleInheritence", - "base_var": 3142.0, - "base2_var1": 3242.0, - "base2_var2": 3342.0, - "base2_var3": 3442.0, - "var1" : 3542, - "var2" : 3642.0 - }, - { - "base_var": 4142.0, - "base2_var1": 4242.0, - "base2_var2": 4342.0, - "base2_var3": 4442.0, - "var1" : 4542, - "var2" : 4642.0 - } - ])"; - } - void Reflect(AZStd::unique_ptr& context) override { Base::Reflect(context); MultipleInheritence::Reflect(context, true); } + void ConfigureFeatures(JsonSerializerConformityTestDescriptorFeatures& features) override + { + Base::ConfigureFeatures(features); + // These tests don't work with pointers because there'll be a random value in the pointer + // which the Json Serialization try to delete. The POD version of these tests already cover + // these cases. + features.m_enableNewInstanceTests = false; + } + bool AreEqual(const Array& lhs, const Array& rhs) override { size_t size = lhs.size(); @@ -311,6 +277,11 @@ namespace JsonSerializationTests return rhs[i] == nullptr; } + if (rhs[i] == nullptr) + { + return false; + } + if (!static_cast(lhs[i])->Equals(*static_cast(rhs[i]), true)) { return false; diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp index b3c900c710..508658e2a5 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/BasicContainerSerializerTests.cpp @@ -54,6 +54,11 @@ namespace JsonSerializationTests return AZStd::make_shared(Container{ 188, 288, 388 }); } + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + return AZStd::make_shared(Container{ 0 }); + } + AZStd::string_view GetJsonForFullySetInstance() override { return "[188, 288, 388]"; @@ -120,6 +125,13 @@ namespace JsonSerializationTests &SimplePointerTestDescription::Delete); } + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + int* value = reinterpret_cast(azmalloc(sizeof(int), alignof(int))); + *value = 0; + return AZStd::shared_ptr(new Container{ value }, &SimplePointerTestDescription::Delete); + } + AZStd::string_view GetJsonForFullySetInstance() override { return "[188, 288, 388]"; @@ -180,6 +192,13 @@ namespace JsonSerializationTests return instance; } + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + auto instance = AZStd::make_shared(); + *instance = { SimpleClass{} }; + return instance; + } + AZStd::string_view GetJsonForFullySetInstance() override { return R"([ diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h b/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h index c0f470378c..f45e0a3658 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h +++ b/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h @@ -62,6 +62,14 @@ namespace JsonSerializationTests //! can be used to manually create these documents. If that is also not an option the tests can be //! disabled by setting this flag to false. bool m_supportsInjection{ true }; + //! Enables the check that tries to determine if variables are initialized and if not whether they have the + //! OperationFlags::ManualDefault set. This applies for instance to integers, which won't be initialized if + //! constructed a new instance is created for pointers. + bool m_enableInitializationTest{ true }; + //! Enable the test that creates a new instance of the provided test type through the factory that's found in + //! the Serialize Context. This test is automatically disabled for classes that don't have a factory or + //! have a null factory. + bool m_enableNewInstanceTests{ true }; private: // There's no way to retrieve the number of types from RapidJSON so they're hard-coded here. @@ -87,6 +95,7 @@ namespace JsonSerializationTests { public: using Type = T; + virtual ~JsonSerializerConformityTestDescriptor() = default; virtual AZStd::shared_ptr CreateSerializer() = 0; @@ -104,13 +113,21 @@ namespace JsonSerializationTests virtual AZStd::shared_ptr CreatePartialDefaultInstance() { return nullptr; } //! Create an instance where all values are set to non-default values. virtual AZStd::shared_ptr CreateFullySetInstance() = 0; + //! Create an instance of the target array type with a single value that has all defaults. + //! If the target type doesn't support arrays or requires more than one entry this can be ignored and + //! tests using this value will be skipped. + virtual AZStd::shared_ptr CreateSingleArrayDefaultInstance() { return nullptr; } //! Get the json that represents the default instance. //! If the target type doesn't support partial specialization this can be ignored and //! tests for partial support will be skipped. - virtual AZStd::string_view GetJsonForPartialDefaultInstance() { return ""; } + virtual AZStd::string_view GetJsonForPartialDefaultInstance() { return ""; } //! Get the json that represents the instance with all values set. virtual AZStd::string_view GetJsonForFullySetInstance() = 0; + //! Get the json that represents an array with a single value that has only defaults. + //! If the target type doesn't support arrays or requires more than one entry this can be ignored and + //! tests using this value will be skipped. + virtual AZStd::string_view GetJsonForSingleArrayDefaultInstance() { return "[{}]"; } //! Get the json where additional values are added to the json file. //! If this function is not overloaded, but features.m_supportsInjection is enabled then //! the Json Serializer Conformity Tests will inject extra values in the json for a fully. @@ -138,12 +155,15 @@ namespace JsonSerializationTests virtual AZStd::string_view GetJsonFor_Load_DeserializeUnreflectedType() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Load_DeserializeFullySetInstance() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Load_DeserializePartialInstance() { return this->GetJsonForPartialDefaultInstance(); } + virtual AZStd::string_view GetJsonFor_Load_DeserializeArrayWithDefaultValue() { return this->GetJsonForSingleArrayDefaultInstance(); } + virtual AZStd::string_view GetJsonFor_Load_DeserializeFullInstanceOnTopOfPartialDefaulted() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Load_HaltedThroughCallback() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Store_SerializeWithDefaultsKept() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Store_SerializeFullySetInstance() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Store_SerializeWithoutDefault() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Store_SerializeWithoutDefaultAndDefaultsKept() { return this->GetJsonForFullySetInstance(); } virtual AZStd::string_view GetJsonFor_Store_SerializePartialInstance() { return this->GetJsonForPartialDefaultInstance(); } + virtual AZStd::string_view GetJsonFor_Store_SerializeArrayWithSingleDefaultValue() { return this->GetJsonForSingleArrayDefaultInstance(); } }; template @@ -154,6 +174,21 @@ namespace JsonSerializationTests using Description = T; using Type = typename T::Type; + struct PointerWrapper + { + AZ_TYPE_INFO(PointerWrapper, "{32FA6645-074A-458A-B79C-B173D0BD4B42}"); + AZ_CLASS_ALLOCATOR(PointerWrapper, AZ::SystemAllocator, 0); + + Type* m_value{ nullptr }; + + ~PointerWrapper() + { + // Using free because not all types can safely use delete. Since this just to clear the memory to satisfy the memory + // leak test, this is fine. + azfree(m_value); + } + }; + void SetUp() override { using namespace AZ::JsonSerializationResult; @@ -165,6 +200,7 @@ namespace JsonSerializationTests descriptor->ConfigureFeatures(this->m_features); descriptor->Reflect(this->m_serializeContext); descriptor->Reflect(this->m_jsonRegistrationContext); + this->m_serializeContext->Class()->Field("Value", &PointerWrapper::m_value); this->m_deserializationSettings->m_reporting = &Internal::VerifyCallback; this->m_serializationSettings->m_reporting = &Internal::VerifyCallback; @@ -185,6 +221,7 @@ namespace JsonSerializationTests this->m_jsonRegistrationContext->DisableRemoveReflection(); this->m_serializeContext->EnableRemoveReflection(); + this->m_serializeContext->Class()->Field("Value", &PointerWrapper::m_value); descriptor->Reflect(this->m_serializeContext); this->m_serializeContext->DisableRemoveReflection(); @@ -487,6 +524,41 @@ namespace JsonSerializationTests } } + TYPED_TEST_P(JsonSerializerConformityTests, Load_DeserializeArrayWithDefaultValue_SucceedsAndReportPartialDefaults) + { + using namespace AZ::JsonSerializationResult; + + if (this->m_features.SupportsJsonType(rapidjson::kArrayType)) + { + this->m_jsonDocument->Parse(this->m_description.GetJsonFor_Load_DeserializeArrayWithDefaultValue().data()); + ASSERT_FALSE(this->m_jsonDocument->HasParseError()); + + auto serializer = this->m_description.CreateSerializer(); + auto instance = this->m_description.CreateDefaultInstance(); + + this->m_jsonDeserializationContext->PushPath(DefaultPath); + + ResultCode result = + serializer->Load(instance.get(), azrtti_typeid(*instance), *this->m_jsonDocument, *this->m_jsonDeserializationContext); + + if (this->m_features.m_fixedSizeArray) + { + EXPECT_EQ(Outcomes::Unsupported, result.GetOutcome()); + EXPECT_EQ(Processing::Altered, result.GetProcessing()); + } + else + { + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + + auto compare = this->m_description.CreateSingleArrayDefaultInstance(); + ASSERT_NE(nullptr, compare) + << "Conformity tests for variably sized arrays require an implementation of CreateSingleArrayDefaultInstance"; + EXPECT_TRUE(this->m_description.AreEqual(*compare, *instance)); + } + } + } + TYPED_TEST_P(JsonSerializerConformityTests, Load_InterruptClearingTarget_ContainerIsNotCleared) { using namespace AZ::JsonSerializationResult; @@ -548,7 +620,6 @@ namespace JsonSerializationTests this->m_jsonDocument->Parse(json.data()); ASSERT_FALSE(this->m_jsonDocument->HasParseError()); - auto serializer = this->m_description.CreateSerializer(); auto instance = this->m_description.CreateDefaultConstructedInstance(); auto compare = this->m_description.CreateFullySetInstance(); @@ -622,6 +693,94 @@ namespace JsonSerializationTests } } + TYPED_TEST_P(JsonSerializerConformityTests, Load_DeserializeFullInstanceOnTopOfPartialDefaulted_SucceedsAndObjectMatchesParialInstance) + { + using namespace AZ::JsonSerializationResult; + + if (this->m_features.m_supportsPartialInitialization) + { + AZStd::string_view json = this->m_description.GetJsonFor_Load_DeserializeFullInstanceOnTopOfPartialDefaulted(); + // If tests for partial initialization are enabled than json for the partial initialization is needed. + ASSERT_FALSE(json.empty()); + this->m_jsonDocument->Parse(json.data()); + ASSERT_FALSE(this->m_jsonDocument->HasParseError()); + + auto serializer = this->m_description.CreateSerializer(); + auto instance = this->m_description.CreatePartialDefaultInstance(); + auto compare = this->m_description.CreateFullySetInstance(); + ASSERT_NE(nullptr, compare); + + // Clear containers which should effectively turn them into default containers. + this->m_deserializationSettings->m_clearContainers = true; + this->ResetJsonContexts(); + this->m_jsonDeserializationContext->PushPath(DefaultPath); + + ResultCode result = + serializer->Load(instance.get(), azrtti_typeid(*instance), *this->m_jsonDocument, *this->m_jsonDeserializationContext); + + EXPECT_EQ(Outcomes::Success, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + EXPECT_TRUE(this->m_description.AreEqual(*instance, *compare)); + } + } + + TYPED_TEST_P(JsonSerializerConformityTests, Load_DefaultToPointer_SucceedsAndValueIsInitialized) + { + using namespace AZ::JsonSerializationResult; + + if (this->m_features.m_enableNewInstanceTests) + { + AZ::SerializeContext* serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); + const AZ::SerializeContext::ClassData* classData = serializeContext->FindClassData(azrtti_typeid()); + ASSERT_NE(nullptr, classData); + // Skip this test if the target type doesn't have a factor to create a new instance with or if the factor explicit + // prohibits construction. + if (classData->m_factory && classData->m_factory != AZ::Internal::NullFactory::GetInstance()) + { + PointerWrapper instance; + auto compare = this->m_description.CreateDefaultInstance(); + + this->m_jsonDocument->Parse(R"({ "Value": {}})"); + ASSERT_FALSE(this->m_jsonDocument->HasParseError()); + + AZ::JsonDeserializerSettings settings; + settings.m_serializeContext = serializeContext; + settings.m_registrationContext = this->m_jsonDeserializationContext->GetRegistrationContext(); + ResultCode result = AZ::JsonSerialization::Load(instance, *this->m_jsonDocument, settings); + + EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + ASSERT_NE(nullptr, instance.m_value); + EXPECT_TRUE(this->m_description.AreEqual(*instance.m_value, *compare)); + } + } + } + + TYPED_TEST_P(JsonSerializerConformityTests, Load_InitializeNewInstance_SucceedsAndValueIsInitialized) + { + using namespace AZ; + using namespace AZ::JsonSerializationResult; + + if (this->m_features.m_enableNewInstanceTests) + { + auto serializer = this->m_description.CreateSerializer(); + if ((serializer->GetOperationsFlags() & BaseJsonSerializer::OperationFlags::InitializeNewInstance) == + BaseJsonSerializer::OperationFlags::InitializeNewInstance) + { + Type instance; + auto compare = this->m_description.CreateDefaultInstance(); + this->m_jsonDocument->SetObject(); + + ResultCode result = + serializer->Load(&instance, azrtti_typeid(instance), *this->m_jsonDocument, *this->m_jsonDeserializationContext); + + EXPECT_EQ(Outcomes::DefaultsUsed, result.GetOutcome()); + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + EXPECT_TRUE(this->m_description.AreEqual(instance, *compare)); + } + } + } + TYPED_TEST_P(JsonSerializerConformityTests, Load_HaltedThroughCallback_LoadFailsAndHaltReported) { using namespace AZ::JsonSerializationResult; @@ -909,6 +1068,28 @@ namespace JsonSerializationTests } } + TYPED_TEST_P(JsonSerializerConformityTests, Store_SerializeArrayWithSingleDefaultValue_StoredSuccessfullyAndJsonMatches) + { + using namespace AZ::JsonSerializationResult; + + if (this->m_features.SupportsJsonType(rapidjson::kArrayType) && !this->m_features.m_fixedSizeArray) + { + this->m_jsonSerializationContext->PushPath(DefaultPath); + + auto serializer = this->m_description.CreateSerializer(); + auto instance = this->m_description.CreateSingleArrayDefaultInstance(); + ASSERT_NE(nullptr, instance) + << "Conformity tests for variably sized arrays require an implementation of CreateSingleArrayDefaultInstance"; + + ResultCode result = serializer->Store( + *this->m_jsonDocument, instance.get(), instance.get(), azrtti_typeid(*instance), *this->m_jsonSerializationContext); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); + this->Expect_DocStrEq(this->m_description.GetJsonFor_Store_SerializeArrayWithSingleDefaultValue()); + } + } + TYPED_TEST_P(JsonSerializerConformityTests, Store_HaltedThroughCallback_StoreFailsAndHaltReported) { using namespace AZ::JsonSerializationResult; @@ -1017,7 +1198,29 @@ namespace JsonSerializationTests } } - TYPED_TEST_P(JsonSerializerConformityTests, GetOperationsFlags_ManualDefaultSetIfNeeded_ManualDefaultOperationSetIfMandatoryFieldsAreDeclared) + TYPED_TEST_P(JsonSerializerConformityTests, GetOperationFlags_RequiresExplicitInit_ObjectsThatDoNotConstructHaveExplicitInitOption) + { + using namespace AZ; + using namespace AZ::JsonSerializationResult; + + if (this->m_features.m_enableInitializationTest) + { + auto instance = this->m_description.CreateDefaultInstance(); + Type compare; + if (!this->m_description.AreEqual(*instance, compare)) + { + auto serializer = this->m_description.CreateSerializer(); + BaseJsonSerializer::OperationFlags flags = serializer->GetOperationsFlags(); + bool hasManualDefaultSet = + (flags & BaseJsonSerializer::OperationFlags::ManualDefault) == BaseJsonSerializer::OperationFlags::ManualDefault || + (flags & BaseJsonSerializer::OperationFlags::InitializeNewInstance) == + BaseJsonSerializer::OperationFlags::InitializeNewInstance; + EXPECT_TRUE(hasManualDefaultSet); + } + } + } + + TYPED_TEST_P(JsonSerializerConformityTests, GetOperationFlags_ManualDefaultSetIfNeeded_ManualDefaultOperationSetIfMandatoryFieldsAreDeclared) { if (this->m_features.SupportsJsonType(rapidjson::kObjectType)) { @@ -1048,10 +1251,14 @@ namespace JsonSerializationTests Load_DeserializeEmptyArray_SucceedsAndObjectMatchesDefaults, Load_DeserializeEmptyArrayWithClearEnabled_SucceedsAndObjectMatchesDefaults, Load_DeserializeEmptyArrayWithClearedTarget_SucceedsAndObjectMatchesDefaults, + Load_DeserializeArrayWithDefaultValue_SucceedsAndReportPartialDefaults, Load_InterruptClearingTarget_ContainerIsNotCleared, Load_DeserializeFullySetInstance_SucceedsAndObjectMatchesFullySetInstance, Load_DeserializeFullySetInstanceThroughMainLoad_SucceedsAndObjectMatchesFullySetInstance, Load_DeserializePartialInstance_SucceedsAndObjectMatchesParialInstance, + Load_DeserializeFullInstanceOnTopOfPartialDefaulted_SucceedsAndObjectMatchesParialInstance, + Load_DefaultToPointer_SucceedsAndValueIsInitialized, + Load_InitializeNewInstance_SucceedsAndValueIsInitialized, Load_DeserializeWithMissingMandatoryField_LoadFailedAndUnsupportedReported, Load_InsertAdditionalData_SucceedsAndObjectMatchesFullySetInstance, Load_HaltedThroughCallback_LoadFailsAndHaltReported, @@ -1066,13 +1273,15 @@ namespace JsonSerializationTests Store_SerializeWithoutDefaultAndDefaultsKept_StoredSuccessfullyAndJsonMatches, Store_SerializePartialInstance_StoredSuccessfullyAndJsonMatches, Store_SerializeEmptyArray_StoredSuccessfullyAndJsonMatches, + Store_SerializeArrayWithSingleDefaultValue_StoredSuccessfullyAndJsonMatches, Store_HaltedThroughCallback_StoreFailsAndHaltReported, StoreLoad_RoundTripWithPartialDefault_IdenticalInstances, StoreLoad_RoundTripWithFullSet_IdenticalInstances, StoreLoad_RoundTripWithDefaultsKept_IdenticalInstances, - GetOperationsFlags_ManualDefaultSetIfNeeded_ManualDefaultOperationSetIfMandatoryFieldsAreDeclared); + GetOperationFlags_RequiresExplicitInit_ObjectsThatDoNotConstructHaveExplicitInitOption, + GetOperationFlags_ManualDefaultSetIfNeeded_ManualDefaultOperationSetIfMandatoryFieldsAreDeclared); } // namespace JsonSerializationTests namespace AZ diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp index 363cd7a7b7..7ec5ca3b9a 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/MapSerializerTests.cpp @@ -35,6 +35,11 @@ namespace JsonSerializationTests return AZStd::make_shared(); } + AZStd::string_view GetJsonForSingleArrayDefaultInstance() override + { + return R"({ "{}": {} })"; + } + void ConfigureFeatures(JsonSerializerConformityTestDescriptorFeatures& features) override { features.EnableJsonType(rapidjson::kArrayType); @@ -61,6 +66,13 @@ namespace JsonSerializationTests public: using Map = T; + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + auto instance = AZStd::make_shared(); + instance->emplace(AZStd::make_pair(0, 0.0)); + return instance; + } + AZStd::shared_ptr CreateFullySetInstance() override { auto instance = AZStd::make_shared(); @@ -100,6 +112,13 @@ namespace JsonSerializationTests public: using Map = T; + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + auto instance = AZStd::make_shared(); + instance->emplace(AZStd::make_pair(AZStd::string(), 0.0)); + return instance; + } + AZStd::shared_ptr CreateFullySetInstance() override { auto instance = AZStd::make_shared(); @@ -163,6 +182,14 @@ namespace JsonSerializationTests return instance; } + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + auto instance = AZStd::shared_ptr(new Map{}, &Delete); + instance->emplace(AZStd::make_pair(aznew SimpleClass(), aznew SimpleClass())); + return instance; + } + + AZStd::string_view GetJsonForPartialDefaultInstance() override { if constexpr (IsMultiMap) @@ -237,13 +264,23 @@ namespace JsonSerializationTests return false; } - auto compare = [](typename Map::const_reference lhs, typename Map::const_reference rhs) -> bool + // Naive compare to avoid having to split up the test because comparing for ordered and unordered maps would need to be + // different. + for (auto&& [key, value] : lhs) { - return - lhs.first->Equals(*rhs.first, true) && - lhs.second->Equals(*rhs.second, true); - }; - return AZStd::equal(lhs.begin(), lhs.end(), rhs.begin(), compare); + for (auto&& [keyCompare, valueCompare] : rhs) + { + if (key->Equals(*keyCompare, true)) + { + if (!value->Equals(*valueCompare, true)) + { + return false; + } + break; + } + } + } + return true; } }; @@ -393,6 +430,29 @@ namespace JsonSerializationTests { using namespace AZ::JsonSerializationResult; + m_jsonDocument->Parse(R"( + { + "{}": {} + })"); + ASSERT_FALSE(m_jsonDocument->HasParseError()); + + TestStringMap values; + ResultCode result = m_unorderedMapSerializer.Load(&values, azrtti_typeid(&values), *m_jsonDocument, *m_jsonDeserializationContext); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); + + EXPECT_EQ(1, values.size()); + + auto defaultKey = values.find(TestString()); + EXPECT_NE(values.end(), defaultKey); + EXPECT_STRCASEEQ(TestString().m_value.c_str(), defaultKey->second.m_value.c_str()); + } + + TEST_F(JsonMapSerializerTests, Load_DefaultForStringKeyAndAdditionalValue_LoadedBackWithDefaults) + { + using namespace AZ::JsonSerializationResult; + m_jsonDocument->Parse(R"( { "{}": {}, @@ -563,13 +623,13 @@ namespace JsonSerializationTests EXPECT_EQ(Outcomes::Catastrophic, result.GetOutcome()); } - TEST_F(JsonMapSerializerTests, Load_DefaultValueInMultiMap_DefaultUsed) + TEST_F(JsonMapSerializerTests, Load_DefaultObjectInMultiMap_DefaultUsed) { using namespace AZ::JsonSerializationResult; m_jsonDocument->Parse(R"( { - "Hello": {} + "World": {} })"); ASSERT_FALSE(m_jsonDocument->HasParseError()); @@ -581,17 +641,40 @@ namespace JsonSerializationTests EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); ASSERT_FALSE(values.empty()); - EXPECT_STREQ("Hello", values.begin()->first.m_value.c_str()); + EXPECT_STREQ("World", values.begin()->first.m_value.c_str()); EXPECT_STREQ(TestString().m_value.c_str(), values.begin()->second.m_value.c_str()); } - TEST_F(JsonMapSerializerTests, Load_DefaultArrayValueInMultiMap_DefaultUsed) + TEST_F(JsonMapSerializerTests, Load_FullDefaultObjectInMultiMap_DefaultUsed) { using namespace AZ::JsonSerializationResult; m_jsonDocument->Parse(R"( { - "Hello": [{}] + "{}": {} + })"); + ASSERT_FALSE(m_jsonDocument->HasParseError()); + + TestStringMultiMap values; + ResultCode result = + m_unorderedMultiMapSerializer.Load(&values, azrtti_typeid(&values), *m_jsonDocument, *m_jsonDeserializationContext); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); + + ASSERT_FALSE(values.empty()); + EXPECT_STREQ("Hello", values.begin()->first.m_value.c_str()); + EXPECT_STREQ("Hello", values.begin()->second.m_value.c_str()); + EXPECT_STREQ(TestString().m_value.c_str(), values.begin()->second.m_value.c_str()); + } + + TEST_F(JsonMapSerializerTests, Load_DefaultObjectValueInMultiMap_DefaultUsed) + { + using namespace AZ::JsonSerializationResult; + + m_jsonDocument->Parse(R"( + { + "World": [{}] })"); ASSERT_FALSE(m_jsonDocument->HasParseError()); @@ -603,7 +686,8 @@ namespace JsonSerializationTests EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); ASSERT_FALSE(values.empty()); - EXPECT_STREQ("Hello", values.begin()->first.m_value.c_str()); + EXPECT_STREQ("World", values.begin()->first.m_value.c_str()); + EXPECT_STREQ("Hello", values.begin()->second.m_value.c_str()); EXPECT_STREQ(TestString().m_value.c_str(), values.begin()->second.m_value.c_str()); } @@ -661,6 +745,24 @@ namespace JsonSerializationTests })"); } + TEST_F(JsonMapSerializerTests, Store_SingleAllDefaulValue_InitializedWithDefaults) + { + using namespace AZ::JsonSerializationResult; + + SimpleClassMap values; + values.emplace(SimpleClass(), SimpleClass()); + + ResultCode result = + m_unorderedMapSerializer.Store(*m_jsonDocument, &values, nullptr, azrtti_typeid(&values), *m_jsonSerializationContext); + + EXPECT_EQ(Processing::Completed, result.GetProcessing()); + EXPECT_EQ(Outcomes::PartialDefaults, result.GetOutcome()); + Expect_DocStrEq(R"( + { + "{}": {} + })"); + } + TEST_F(JsonMapSerializerTests, Store_DefaultsWithObjectKey_InitializedWithDefaults) { using namespace AZ::JsonSerializationResult; diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/TupleSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/TupleSerializerTests.cpp index eceaaae5f1..77a88fda6c 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/TupleSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/TupleSerializerTests.cpp @@ -48,12 +48,12 @@ namespace JsonSerializationTests AZStd::shared_ptr CreateDefaultInstance() override { - return AZStd::make_shared(142, 242.0); + return AZStd::make_shared(0, 0.0); } AZStd::shared_ptr CreatePartialDefaultInstance() override { - return AZStd::make_shared(142, 288.0); + return AZStd::make_shared(0, 288.0); } AZStd::shared_ptr CreateFullySetInstance() override @@ -102,12 +102,12 @@ namespace JsonSerializationTests AZStd::shared_ptr CreateDefaultInstance() override { - return AZStd::make_shared(142, 242.0, 342.0f); + return AZStd::make_shared(0, 0.0, 0.0f); } AZStd::shared_ptr CreatePartialDefaultInstance() override { - return AZStd::make_shared(142, 288.0, 342.0f); + return AZStd::make_shared(0, 288.0, 0.0f); } AZStd::shared_ptr CreateFullySetInstance() override @@ -345,6 +345,7 @@ namespace JsonSerializationTests { TupleSerializerTestsInternal::ConfigureFeatures(features); features.m_supportsPartialInitialization = true; + features.m_enableNewInstanceTests = false; } void Reflect(AZStd::unique_ptr& context) override @@ -447,14 +448,14 @@ namespace JsonSerializationTests { return AZStd::make_shared( AZStd::vector(), - AZStd::make_pair(442, "")); + AZStd::make_pair(0, "")); } AZStd::shared_ptr CreatePartialDefaultInstance() override { return AZStd::make_shared( AZStd::vector(), - AZStd::make_pair(442, "hello")); + AZStd::make_pair(0, "hello")); } AZStd::shared_ptr CreateFullySetInstance() override diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/UnorderedSetSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/UnorderedSetSerializerTests.cpp index 6e9cd0f3d6..fb7b683d2e 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/UnorderedSetSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/UnorderedSetSerializerTests.cpp @@ -36,6 +36,11 @@ namespace JsonSerializationTests return AZStd::make_shared(); } + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + return AZStd::make_shared(Set{ 0 }); + } + AZStd::shared_ptr CreateFullySetInstance() override { return AZStd::make_shared(Set{42, -88, 342}); @@ -80,6 +85,11 @@ namespace JsonSerializationTests return AZStd::make_shared(); } + AZStd::shared_ptr CreateSingleArrayDefaultInstance() override + { + return AZStd::make_shared(MultiSet{ 0 }); + } + AZStd::shared_ptr CreateFullySetInstance() override { return AZStd::make_shared(MultiSet{ 42, -88, 42, 342 }); From 98ff91d854e48aedefd52fcf0162ce9bd173249b Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Tue, 15 Jun 2021 16:12:36 -0700 Subject: [PATCH 11/15] Removed unused test structure. --- .../Serialization/Json/IntSerializerTests.cpp | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp index 5f81dd1b9c..db764badcb 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/IntSerializerTests.cpp @@ -147,18 +147,6 @@ namespace JsonSerializationTests : public BaseJsonSerializerFixture { public: - struct IntegerPointerWrapper - { - AZ_TYPE_INFO(IntegerPointerWrapper, "{F6B3BEF1-59A4-4E45-BF02-DDA868C38A28}"); - - typename SerializerInfo::DataType* m_value{ nullptr }; - - ~IntegerPointerWrapper() - { - azfree(m_value); - } - }; - AZStd::unique_ptr m_serializer; void SetUp() override @@ -173,12 +161,6 @@ namespace JsonSerializationTests BaseJsonSerializerFixture::TearDown(); } - void RegisterAdditional(AZStd::unique_ptr& serializeContext) override - { - serializeContext->Class() - ->Field("Value", &IntegerPointerWrapper::m_value); - } - template::value, int> = 0> void SetValue(rapidjson::Value& out, T in) { From c360e29fbf32e738b4e549a7444b12edc5ef86d8 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 16 Jun 2021 13:57:17 -0700 Subject: [PATCH 12/15] Fixed compile errors from Clang. --- .../Json/JsonSerializerConformityTests.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h b/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h index f45e0a3658..1dcf8e29c6 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h +++ b/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h @@ -200,7 +200,7 @@ namespace JsonSerializationTests descriptor->ConfigureFeatures(this->m_features); descriptor->Reflect(this->m_serializeContext); descriptor->Reflect(this->m_jsonRegistrationContext); - this->m_serializeContext->Class()->Field("Value", &PointerWrapper::m_value); + this->m_serializeContext->template Class()->Field("Value", &PointerWrapper::m_value); this->m_deserializationSettings->m_reporting = &Internal::VerifyCallback; this->m_serializationSettings->m_reporting = &Internal::VerifyCallback; @@ -221,7 +221,7 @@ namespace JsonSerializationTests this->m_jsonRegistrationContext->DisableRemoveReflection(); this->m_serializeContext->EnableRemoveReflection(); - this->m_serializeContext->Class()->Field("Value", &PointerWrapper::m_value); + this->m_serializeContext->template Class()->Field("Value", &PointerWrapper::m_value); descriptor->Reflect(this->m_serializeContext); this->m_serializeContext->DisableRemoveReflection(); @@ -731,13 +731,13 @@ namespace JsonSerializationTests if (this->m_features.m_enableNewInstanceTests) { AZ::SerializeContext* serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); - const AZ::SerializeContext::ClassData* classData = serializeContext->FindClassData(azrtti_typeid()); + const AZ::SerializeContext::ClassData* classData = serializeContext->FindClassData(azrtti_typeid()); ASSERT_NE(nullptr, classData); // Skip this test if the target type doesn't have a factor to create a new instance with or if the factor explicit // prohibits construction. if (classData->m_factory && classData->m_factory != AZ::Internal::NullFactory::GetInstance()) { - PointerWrapper instance; + typename JsonSerializerConformityTests::PointerWrapper instance; auto compare = this->m_description.CreateDefaultInstance(); this->m_jsonDocument->Parse(R"({ "Value": {}})"); @@ -767,7 +767,7 @@ namespace JsonSerializationTests if ((serializer->GetOperationsFlags() & BaseJsonSerializer::OperationFlags::InitializeNewInstance) == BaseJsonSerializer::OperationFlags::InitializeNewInstance) { - Type instance; + typename TypeParam::Type instance; auto compare = this->m_description.CreateDefaultInstance(); this->m_jsonDocument->SetObject(); @@ -1206,7 +1206,7 @@ namespace JsonSerializationTests if (this->m_features.m_enableInitializationTest) { auto instance = this->m_description.CreateDefaultInstance(); - Type compare; + typename TypeParam::Type compare; if (!this->m_description.AreEqual(*instance, compare)) { auto serializer = this->m_description.CreateSerializer(); From ddc60041d345e97ebd2a62683cd99166ed7295db Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 16 Jun 2021 16:40:46 -0700 Subject: [PATCH 13/15] Addressing PR feedback --- .../Json/BasicContainerSerializer.cpp | 2 +- .../AzCore/Serialization/Json/DoubleSerializer.cpp | 2 +- .../AzCore/Serialization/Json/TupleSerializer.cpp | 14 +++++++------- .../AzCore/Tests/AssetJsonSerializerTests.cpp | 6 +++--- .../Serialization/Json/ArraySerializerTests.cpp | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp index 4eb4748eb5..d61366413f 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp @@ -255,7 +255,7 @@ namespace AZ size_t addedCount = container->Size(outputValue) - containerSize; if (addedCount > 0) { - // Values were added which means the container is no longer in its default state of being emtpy. + // Values were added which means the container is no longer in its default state of being empty. retVal.Combine(JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::Success)); } AZStd::string_view message = diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp index 14f2eaae6f..be885dfaa9 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp @@ -73,7 +73,7 @@ namespace AZ if (isExplicitDefault) { *outputValue = 0.0f; - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Double value set to default of 0.0."); + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Floating point value set to default of 0.0."); } switch (inputValue.GetType()) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp index fd8951517a..e8a6f549a7 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp @@ -170,13 +170,6 @@ namespace AZ }; container->EnumTypes(typeCountCallback); - rapidjson::SizeType arraySize = isNewInstance ? typeCount : inputValue.Size(); - if (arraySize < typeCount) - { - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, - "Not enough entries in array to load an AZStd::pair or AZStd::tuple from."); - } - AZStd::vector classElements; classElements.reserve(typeCount); auto typeEnumCallback = [&classElements](const Uuid&, const SerializeContext::ClassElement* genericClassElement) @@ -214,6 +207,13 @@ namespace AZ } else { + if (inputValue.Size() < typeCount) + { + return context.Report( + JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, + "Not enough entries in array to load an AZStd::pair or AZStd::tuple from."); + } + rapidjson::SizeType arrayIndex = 0; size_t numElementsWritten = 0; for (size_t i = 0; i < typeCount; ++i) diff --git a/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp b/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp index ceff314c81..47dba62ec8 100644 --- a/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp @@ -168,9 +168,9 @@ namespace JsonSerializationTests { features.EnableJsonType(rapidjson::kObjectType); features.m_typeToInject = rapidjson::kNullType; - // The type information in the Serialize Context is incomplete so this test will fail. - // This is because assets have traditionally been treated as a special case, so there's - // information missing in the Json Serialization to deal with these. + // Assets are not fully registered with the Serialize Context for historical reasons. Due to the missing + // information the Json Serializer Conformity Tests can't run the subsection of tests that explicitly + // require the missing information. features.m_enableNewInstanceTests = false; } diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp index 1f39feccbd..f696a9d270 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp @@ -257,7 +257,7 @@ namespace JsonSerializationTests { Base::ConfigureFeatures(features); // These tests don't work with pointers because there'll be a random value in the pointer - // which the Json Serialization try to delete. The POD version of these tests already cover + // which the Json Serialization will try to delete. The POD version of these tests already cover // these cases. features.m_enableNewInstanceTests = false; } From c482c17c9ec97e2855c945e7ace7dde9a24afdab Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 16 Jun 2021 17:14:44 -0700 Subject: [PATCH 14/15] Fixed an issue with the new Json Serializer Conformity tests and Atom's materials --- .../Serialization/Json/JsonSerializerConformityTests.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h b/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h index 1dcf8e29c6..4931c203cf 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h +++ b/Code/Framework/AzCore/Tests/Serialization/Json/JsonSerializerConformityTests.h @@ -68,7 +68,7 @@ namespace JsonSerializationTests bool m_enableInitializationTest{ true }; //! Enable the test that creates a new instance of the provided test type through the factory that's found in //! the Serialize Context. This test is automatically disabled for classes that don't have a factory or - //! have a null factory. + //! have a null factory as well as for classes that have mandatory fields. bool m_enableNewInstanceTests{ true }; private: @@ -728,7 +728,7 @@ namespace JsonSerializationTests { using namespace AZ::JsonSerializationResult; - if (this->m_features.m_enableNewInstanceTests) + if (this->m_features.m_enableNewInstanceTests && this->m_features.m_mandatoryFields.empty()) { AZ::SerializeContext* serializeContext = this->m_jsonDeserializationContext->GetSerializeContext(); const AZ::SerializeContext::ClassData* classData = serializeContext->FindClassData(azrtti_typeid()); @@ -761,7 +761,7 @@ namespace JsonSerializationTests using namespace AZ; using namespace AZ::JsonSerializationResult; - if (this->m_features.m_enableNewInstanceTests) + if (this->m_features.m_enableNewInstanceTests && this->m_features.m_mandatoryFields.empty()) { auto serializer = this->m_description.CreateSerializer(); if ((serializer->GetOperationsFlags() & BaseJsonSerializer::OperationFlags::InitializeNewInstance) == From 1b1a5a28f46689552c7d09b327f5665bb06f878f Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 16 Jun 2021 17:26:14 -0700 Subject: [PATCH 15/15] Using zero-initializer for defaults in the Json Serializer instead of explicit values --- .../AzCore/AzCore/Serialization/Json/BoolSerializer.cpp | 2 +- .../AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp | 2 +- .../AzCore/AzCore/Serialization/Json/IntSerializer.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp index defe470bcf..3a9b5323b0 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BoolSerializer.cpp @@ -84,7 +84,7 @@ namespace AZ if (IsExplicitDefault(inputValue)) { - *valAsBool = false; + *valAsBool = {}; return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Boolean value set to default of 'false'."); } diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp index be885dfaa9..0921587041 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp @@ -72,7 +72,7 @@ namespace AZ if (isExplicitDefault) { - *outputValue = 0.0f; + *outputValue = {}; return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Floating point value set to default of 0.0."); } diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp index ebc99fd8c4..a6b1118ac1 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/IntSerializer.cpp @@ -67,7 +67,7 @@ namespace AZ if (isDefaultValue) { - *outputValue = 0; + *outputValue = {}; return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Integer value set to default of zero."); }