From dba1832821adac79d9fecc55b77e3cfff1cad5d9 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Mon, 7 Jun 2021 16:58:08 -0700 Subject: [PATCH 1/4] Improved reporting on unsupported types by the Json Serialization The Json Serialization currently doesn't support AZStd::any, AZStd::variant and AZStd::optional due to various JSON formatting concerns. This wasn't properly reported resulting in confusion about whether the missing functionality is intentional or a bug. This change makes this explicit with an error message. As this solution uses a custom json serializer to report the issue, an option was added so it's possible to overwrite a serializer for a specific type. This doesn't mean that the three listed types will never be supported. If/when a suitable format is found an implementation will be added. --- .../Json/JsonSystemComponent.cpp | 15 +- .../Json/RegistrationContext.cpp | 38 ++--- .../Serialization/Json/RegistrationContext.h | 32 ++-- .../Json/UnsupportedTypesSerializer.cpp | 56 +++++++ .../Json/UnsupportedTypesSerializer.h | 72 +++++++++ .../AzCore/AzCore/azcore_files.cmake | 2 + .../Json/JsonRegistrationContextTests.cpp | 30 ++++ .../Json/UnsupportedTypesSerializerTests.cpp | 143 ++++++++++++++++++ .../AzCore/Tests/azcoretests_files.cmake | 1 + 9 files changed, 353 insertions(+), 36 deletions(-) create mode 100644 Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.cpp create mode 100644 Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.h create mode 100644 Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSystemComponent.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSystemComponent.cpp index 6e3c8cec60..8fe1bf50e2 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSystemComponent.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonSystemComponent.cpp @@ -24,7 +24,12 @@ #include #include #include +#include #include +#include +#include +#include +#include #include #include #include @@ -33,12 +38,11 @@ #include #include #include +#include #include #include #include #include -#include -#include namespace AZ { @@ -98,6 +102,13 @@ namespace AZ jsonContext->Serializer() ->HandlesType(); + jsonContext->Serializer() + ->HandlesType(); + jsonContext->Serializer() + ->HandlesType(); + jsonContext->Serializer() + ->HandlesType(); + MathReflect(jsonContext); } else if (SerializeContext* serializeContext = azrtti_cast(reflectContext)) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp index 552b16a731..50ea08e0a0 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp @@ -33,34 +33,38 @@ namespace AZ , m_serializerIter(serializerMapIter) {} - JsonRegistrationContext::SerializerBuilder* JsonRegistrationContext::SerializerBuilder::HandlesTypeId(const Uuid& uuid) + JsonRegistrationContext::SerializerBuilder* JsonRegistrationContext::SerializerBuilder::HandlesTypeId( + const Uuid& uuid, bool overwriteExisting) { if (!m_context->IsRemovingReflection()) { auto serializer = m_serializerIter->second.get(); if (uuid.IsNull()) { - AZ_Error("Serialization", false, - "Could not register Json serializer %s. Its Uuid is null.", - serializer->RTTI_GetTypeName() - ); + AZ_Assert(false, "Could not register Json serializer %s. Its Uuid is null.", serializer->RTTI_GetTypeName()); return this; } - auto serializerIter = m_context->m_handledTypesMap.find(uuid); - - if (serializerIter == m_context->m_handledTypesMap.end()) + if (!overwriteExisting) { - m_context->m_handledTypesMap.emplace(uuid, serializer); - return this; + auto serializerIter = m_context->m_handledTypesMap.find(uuid); + if (serializerIter == m_context->m_handledTypesMap.end()) + { + m_context->m_handledTypesMap.emplace(uuid, serializer); + } + else + { + AZ_Assert( + false, + "Couldn't register Json serializer %s. Another serializer (%s) has already been registered for the same Uuid (%s).", + serializer->RTTI_GetTypeName(), serializerIter->second->RTTI_GetTypeName(), + serializerIter->first.ToString().c_str()); + } + } + else + { + m_context->m_handledTypesMap.insert_or_assign(uuid, serializer); } - - AZ_Error("Serialization", false, - "Couldn't register Json serializer %s. Another serializer (%s) has already been registered for the same Uuid (%s).", - serializer->RTTI_GetTypeName(), - serializerIter->second->RTTI_GetTypeName(), - serializerIter->first.ToString().c_str() - ); } else { diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.h b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.h index 89d69e223f..fa1575a916 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.h @@ -63,52 +63,50 @@ namespace AZ SerializerBuilder* operator->(); template - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } template class T> - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } template class T> - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } template class T> - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } template class T> - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } template class T> - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } template class T> - SerializerBuilder* HandlesType() + SerializerBuilder* HandlesType(bool overwriteExisting = false) { - return HandlesTypeId(azrtti_typeid()); + return HandlesTypeId(azrtti_typeid(), overwriteExisting); } protected: - struct Placeholder { AZ_TYPE_INFO(PlaceHolder, "{4425191C-F497-411A-A7C3-52928E720B0A}"); }; - SerializerBuilder(JsonRegistrationContext* context, SerializerMap::const_iterator serializerMapIter); - SerializerBuilder* HandlesTypeId(const AZ::Uuid& uuid); + SerializerBuilder* HandlesTypeId(const AZ::Uuid& uuid, bool overwriteExisting); JsonRegistrationContext* m_context = nullptr; SerializerMap::const_iterator m_serializerIter; diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.cpp new file mode 100644 index 0000000000..b03215dd75 --- /dev/null +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.cpp @@ -0,0 +1,56 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#include +#include + +namespace AZ +{ + AZ_CLASS_ALLOCATOR_IMPL(JsonUnsupportedTypesSerializer, SystemAllocator, 0); + AZ_CLASS_ALLOCATOR_IMPL(JsonAnySerializer, SystemAllocator, 0); + AZ_CLASS_ALLOCATOR_IMPL(JsonVariantSerializer, SystemAllocator, 0); + AZ_CLASS_ALLOCATOR_IMPL(JsonOptionalSerializer, SystemAllocator, 0); + + JsonSerializationResult::Result JsonUnsupportedTypesSerializer::Load(void*, const Uuid&, const rapidjson::Value&, + JsonDeserializerContext& context) + { + namespace JSR = JsonSerializationResult; + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Invalid, GetMessage()); + } + + JsonSerializationResult::Result JsonUnsupportedTypesSerializer::Store(rapidjson::Value&, const void*, const void*, const Uuid&, + JsonSerializerContext& context) + { + namespace JSR = JsonSerializationResult; + return context.Report(JSR::Tasks::WriteValue, JSR::Outcomes::Invalid, GetMessage()); + } + + AZStd::string_view JsonAnySerializer::GetMessage() const + { + return "The Json Serialization doesn't support AZStd::any by design. The Json Serialization attempts to minimize the use of $type, " + "in particular the guid version, but no way has yet been found to use AZStd::any without explicitly and always requiring " + "one."; + } + + AZStd::string_view JsonVariantSerializer::GetMessage() const + { + return "The Json Serialization doesn't support AZStd::variant by design. The Json Serialization attempts to minimize the use of " + "$type, in particular the guid version. While combinations of AZStd::variant can be constructed that don't require a $type, " + "this cannot be guaranteed in general."; + } + + AZStd::string_view JsonOptionalSerializer::GetMessage() const + { + return "The Json Serialization doesn't support AZStd::optional by design. No JSON format has yet been found that wasn't deemed too " + "complex or overly verbose."; + } +} // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.h b/Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.h new file mode 100644 index 0000000000..882b479e5d --- /dev/null +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/UnsupportedTypesSerializer.h @@ -0,0 +1,72 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#pragma once + +#include +#include +#include + +namespace AZ +{ + class JsonUnsupportedTypesSerializer : public BaseJsonSerializer + { + public: + AZ_RTTI(JsonUnsupportedTypesSerializer, "{AFCC76B9-1F28-429D-8B4E-020BFD95ADAC}", 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; + + protected: + virtual AZStd::string_view GetMessage() const = 0; + }; + + class JsonAnySerializer : public JsonUnsupportedTypesSerializer + { + public: + AZ_RTTI(JsonAnySerializer, "{699A0864-C4E2-4266-8141-99793C76870F}", JsonUnsupportedTypesSerializer); + AZ_CLASS_ALLOCATOR_DECL; + + protected: + AZStd::string_view GetMessage() const override; + }; + + class JsonVariantSerializer : public JsonUnsupportedTypesSerializer + { + public: + AZ_RTTI(JsonVariantSerializer, "{08F8E746-F8A4-4E83-8902-713E90F3F498}", JsonUnsupportedTypesSerializer); + AZ_CLASS_ALLOCATOR_DECL; + + protected: + AZStd::string_view GetMessage() const override; + }; + + class JsonOptionalSerializer : public JsonUnsupportedTypesSerializer + { + public: + AZ_RTTI(JsonOptionalSerializer, "{F8AF1C95-BD1B-44D2-9B4A-F5726133A104}", JsonUnsupportedTypesSerializer); + AZ_CLASS_ALLOCATOR_DECL; + + protected: + AZStd::string_view GetMessage() const override; + }; +} // namespace AZ diff --git a/Code/Framework/AzCore/AzCore/azcore_files.cmake b/Code/Framework/AzCore/AzCore/azcore_files.cmake index dc0fb13f00..ca5b8cb7f2 100644 --- a/Code/Framework/AzCore/AzCore/azcore_files.cmake +++ b/Code/Framework/AzCore/AzCore/azcore_files.cmake @@ -544,6 +544,8 @@ set(FILES Serialization/Json/TupleSerializer.cpp Serialization/Json/UnorderedSetSerializer.h Serialization/Json/UnorderedSetSerializer.cpp + Serialization/Json/UnsupportedTypesSerializer.h + Serialization/Json/UnsupportedTypesSerializer.cpp Serialization/std/VariantReflection.inl Settings/CommandLine.cpp Settings/CommandLine.h diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp index b10b569a42..6012a824c7 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp @@ -95,6 +95,20 @@ namespace JsonSerializationTests } }; + class SerializerWithOneDuplicatedTypeWithOverride + : public JsonSerializerTemplatedMock + { + public: + AZ_RTTI(SerializerWithOneDuplicatedTypeWithOverride, "{4218D591-E578-499B-B578-ACA70C9944AB}", BaseJsonSerializer); + ~SerializerWithOneDuplicatedTypeWithOverride() override = default; + + static void Reflect(AZ::JsonRegistrationContext* context) + { + context->Serializer() + ->HandlesType(true); + } + }; + // Attempts to register the same type twice class SerializerWithTwoSameTypes : public JsonSerializerTemplatedMock @@ -278,6 +292,22 @@ namespace JsonSerializationTests SerializerWithOneDuplicatedType::Unreflect(m_jsonRegistrationContext.get()); } + TEST_F(JsonRegistrationContextTests, OverwriteRegisterSameUuidWithMultipleSerializers_Succeeds) + { + EXPECT_NE(AZ::AzTypeInfo::Uuid(), AZ::AzTypeInfo::Uuid()); + + SerializerWithOneType::Reflect(m_jsonRegistrationContext.get()); + SerializerWithOneDuplicatedTypeWithOverride::Reflect(m_jsonRegistrationContext.get()); + + EXPECT_EQ(1, m_jsonRegistrationContext->GetRegisteredSerializers().size()); + AZ::BaseJsonSerializer* mockSerializer = m_jsonRegistrationContext->GetSerializerForType(azrtti_typeid()); + EXPECT_NE(mockSerializer, nullptr); + EXPECT_EQ(AZ::AzTypeInfo::Uuid(), mockSerializer->RTTI_GetType()); + + SerializerWithOneType::Unreflect(m_jsonRegistrationContext.get()); + SerializerWithOneDuplicatedTypeWithOverride::Unreflect(m_jsonRegistrationContext.get()); + } + TEST_F(JsonRegistrationContextTests, RegisterSameUuidWithSameSerializers_Fails) { AZ_TEST_START_ASSERTTEST; diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp new file mode 100644 index 0000000000..8c9f5d0d3d --- /dev/null +++ b/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp @@ -0,0 +1,143 @@ +/* + * All or portions of this file Copyright (c) Amazon.com, Inc. or its affiliates or + * its licensors. + * + * For complete copyright and license terms please see the LICENSE at the root of this + * distribution (the "License"). All use of this software is governed by the License, + * or, if provided, by the license below or the license accompanying this file. Do not + * remove or modify any license notices. This file is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * + */ + +#include +#include +#include +#include +#include + +namespace JsonSerializationTests +{ + struct AnyInfo + { + using Type = AZStd::any; + using Serializer = AZ::JsonAnySerializer; + }; + + struct VariantInfo + { + using Type = AZStd::variant; + using Serializer = AZ::JsonVariantSerializer; + }; + + struct OptionalInfo + { + using Type = AZStd::optional; + using Serializer = AZ::JsonVariantSerializer; + }; + + template + class JsonUnsupportedTypesSerializerTests : public BaseJsonSerializerFixture + { + public: + using Type = typename Info::Type; + using Serializer = typename Info::Serializer; + + void SetUp() override + { + BaseJsonSerializerFixture::SetUp(); + m_serializer = AZStd::make_unique(); + } + + void TearDown() override + { + m_serializer.reset(); + BaseJsonSerializerFixture::TearDown(); + } + + protected: + AZStd::unique_ptr m_serializer; + }; + + using UnsupportedTypesTestTypes = ::testing::Types; + TYPED_TEST_CASE(JsonUnsupportedTypesSerializerTests, UnsupportedTypesTestTypes); + + TYPED_TEST(JsonUnsupportedTypesSerializerTests, Load_CallDirectly_ReportsIssueAndHalts) + { + using namespace AZ::JsonSerializationResult; + + bool hasMessage = false; + auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + { + hasMessage = !message.empty(); + return result; + }; + m_jsonDeserializationContext->PushReporter(AZStd::move(callback)); + + Type instance{}; + Result result = m_serializer->Load(&instance, azrtti_typeid(), *m_jsonDocument, *m_jsonDeserializationContext); + m_jsonDeserializationContext->PopReporter(); + + EXPECT_EQ(Processing::Halted, result.GetResultCode().GetProcessing()); + EXPECT_TRUE(hasMessage); + } + + TYPED_TEST(JsonUnsupportedTypesSerializerTests, Load_CallThroughFrontEnd_ReportsIssueAndHalts) + { + using namespace AZ::JsonSerializationResult; + + bool hasMessage = false; + auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + { + hasMessage = !message.empty(); + return result; + }; + m_deserializationSettings->m_reporting = AZStd::move(callback); + + Type instance{}; + ResultCode result = AZ::JsonSerialization::Load(instance, *m_jsonDocument, *m_deserializationSettings); + + EXPECT_EQ(Processing::Halted, result.GetProcessing()); + EXPECT_TRUE(hasMessage); + } + + TYPED_TEST(JsonUnsupportedTypesSerializerTests, Save_CallDirectly_ReportsIssueAndHalts) + { + using namespace AZ::JsonSerializationResult; + + bool hasMessage = false; + auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + { + hasMessage = !message.empty(); + return result; + }; + m_jsonSerializationContext->PushReporter(AZStd::move(callback)); + + Type instance{}; + Result result = m_serializer->Store(*m_jsonDocument, &instance, nullptr, azrtti_typeid(), *m_jsonSerializationContext); + m_jsonSerializationContext->PopReporter(); + + EXPECT_EQ(Processing::Halted, result.GetResultCode().GetProcessing()); + EXPECT_TRUE(hasMessage); + } + + TYPED_TEST(JsonUnsupportedTypesSerializerTests, Save_CallThroughFrontEnd_ReportsIssueAndHalts) + { + using namespace AZ::JsonSerializationResult; + + bool hasMessage = false; + auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + { + hasMessage = !message.empty(); + return result; + }; + m_serializationSettings->m_reporting = AZStd::move(callback); + + Type instance{}; + ResultCode result = + AZ::JsonSerialization::Store(*m_jsonDocument, m_jsonDocument->GetAllocator(), instance, *m_serializationSettings); + + EXPECT_EQ(Processing::Halted, result.GetProcessing()); + EXPECT_TRUE(hasMessage); + } +} // namespace JsonSerializationTests diff --git a/Code/Framework/AzCore/Tests/azcoretests_files.cmake b/Code/Framework/AzCore/Tests/azcoretests_files.cmake index 78b2701d92..ffbfb75f6b 100644 --- a/Code/Framework/AzCore/Tests/azcoretests_files.cmake +++ b/Code/Framework/AzCore/Tests/azcoretests_files.cmake @@ -128,6 +128,7 @@ set(FILES Serialization/Json/TransformSerializerTests.cpp Serialization/Json/TupleSerializerTests.cpp Serialization/Json/UnorderedSetSerializerTests.cpp + Serialization/Json/UnsupportedTypesSerializerTests.cpp Serialization/Json/UuidSerializerTests.cpp Math/AabbTests.cpp Math/ColorTests.cpp From 6063e3a391651bb8744123ed567cd4e48d7ad5c1 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 9 Jun 2021 11:07:28 -0700 Subject: [PATCH 2/4] Simplified Json Serializer registration code Updated the Json Serializer registeration code in the RegistrationContext.cpp to use try_emplace instead of find + end check + insert. --- .../Serialization/Json/RegistrationContext.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp index 50ea08e0a0..9aa80a2ef2 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp @@ -47,19 +47,11 @@ namespace AZ if (!overwriteExisting) { - auto serializerIter = m_context->m_handledTypesMap.find(uuid); - if (serializerIter == m_context->m_handledTypesMap.end()) - { - m_context->m_handledTypesMap.emplace(uuid, serializer); - } - else - { - AZ_Assert( - false, - "Couldn't register Json serializer %s. Another serializer (%s) has already been registered for the same Uuid (%s).", - serializer->RTTI_GetTypeName(), serializerIter->second->RTTI_GetTypeName(), - serializerIter->first.ToString().c_str()); - } + auto emplaceResult = m_context->m_handledTypesMap.try_emplace(uuid, serializer); + AZ_Assert( + emplaceResult.second, + "Couldn't register Json serializer %s. Another serializer (%s) has already been registered for the same Uuid (%s).", + serializer->RTTI_GetTypeName(), emplaceResult.first->second->RTTI_GetTypeName(), uuid); } else { From e3fe4705f65d035b5e9a62ff5f9f4cdf8152740b Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 9 Jun 2021 13:44:18 -0700 Subject: [PATCH 3/4] Post merge and Linux fixes. --- .../Json/UnsupportedTypesSerializerTests.cpp | 57 +++++++++---------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp index 8c9f5d0d3d..b7ad6bceae 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/UnsupportedTypesSerializerTests.cpp @@ -46,17 +46,18 @@ namespace JsonSerializationTests void SetUp() override { BaseJsonSerializerFixture::SetUp(); - m_serializer = AZStd::make_unique(); + this->m_serializer = AZStd::make_unique(); } void TearDown() override { - m_serializer.reset(); + this->m_serializer.reset(); BaseJsonSerializerFixture::TearDown(); } protected: AZStd::unique_ptr m_serializer; + Type m_instance{}; }; using UnsupportedTypesTestTypes = ::testing::Types; @@ -64,80 +65,78 @@ namespace JsonSerializationTests TYPED_TEST(JsonUnsupportedTypesSerializerTests, Load_CallDirectly_ReportsIssueAndHalts) { - using namespace AZ::JsonSerializationResult; + namespace JSR = AZ::JsonSerializationResult; bool hasMessage = false; - auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + auto callback = [&hasMessage](AZStd::string_view message, JSR::ResultCode result, AZStd::string_view) -> JSR::ResultCode { hasMessage = !message.empty(); return result; }; - m_jsonDeserializationContext->PushReporter(AZStd::move(callback)); + this->m_jsonDeserializationContext->PushReporter(AZStd::move(callback)); - Type instance{}; - Result result = m_serializer->Load(&instance, azrtti_typeid(), *m_jsonDocument, *m_jsonDeserializationContext); - m_jsonDeserializationContext->PopReporter(); + JSR::Result result = this->m_serializer->Load( + &this->m_instance, azrtti_typeid(this->m_instance), *this->m_jsonDocument, *this->m_jsonDeserializationContext); + this->m_jsonDeserializationContext->PopReporter(); - EXPECT_EQ(Processing::Halted, result.GetResultCode().GetProcessing()); + EXPECT_EQ(JSR::Processing::Halted, result.GetResultCode().GetProcessing()); EXPECT_TRUE(hasMessage); } TYPED_TEST(JsonUnsupportedTypesSerializerTests, Load_CallThroughFrontEnd_ReportsIssueAndHalts) { - using namespace AZ::JsonSerializationResult; + namespace JSR = AZ::JsonSerializationResult; bool hasMessage = false; - auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + auto callback = [&hasMessage](AZStd::string_view message, JSR::ResultCode result, AZStd::string_view) -> JSR::ResultCode { hasMessage = !message.empty(); return result; }; - m_deserializationSettings->m_reporting = AZStd::move(callback); + this->m_deserializationSettings->m_reporting = AZStd::move(callback); - Type instance{}; - ResultCode result = AZ::JsonSerialization::Load(instance, *m_jsonDocument, *m_deserializationSettings); + JSR::ResultCode result = AZ::JsonSerialization::Load(this->m_instance, *this->m_jsonDocument, *this->m_deserializationSettings); - EXPECT_EQ(Processing::Halted, result.GetProcessing()); + EXPECT_EQ(JSR::Processing::Halted, result.GetProcessing()); EXPECT_TRUE(hasMessage); } TYPED_TEST(JsonUnsupportedTypesSerializerTests, Save_CallDirectly_ReportsIssueAndHalts) { - using namespace AZ::JsonSerializationResult; + namespace JSR = AZ::JsonSerializationResult; bool hasMessage = false; - auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + auto callback = [&hasMessage](AZStd::string_view message, JSR::ResultCode result, AZStd::string_view) -> JSR::ResultCode { hasMessage = !message.empty(); return result; }; - m_jsonSerializationContext->PushReporter(AZStd::move(callback)); + this->m_jsonSerializationContext->PushReporter(AZStd::move(callback)); - Type instance{}; - Result result = m_serializer->Store(*m_jsonDocument, &instance, nullptr, azrtti_typeid(), *m_jsonSerializationContext); - m_jsonSerializationContext->PopReporter(); + JSR::Result result = this->m_serializer->Store( + *this->m_jsonDocument, &this->m_instance, nullptr, azrtti_typeid(this->m_instance), *this->m_jsonSerializationContext); + this->m_jsonSerializationContext->PopReporter(); - EXPECT_EQ(Processing::Halted, result.GetResultCode().GetProcessing()); + EXPECT_EQ(JSR::Processing::Halted, result.GetResultCode().GetProcessing()); EXPECT_TRUE(hasMessage); } TYPED_TEST(JsonUnsupportedTypesSerializerTests, Save_CallThroughFrontEnd_ReportsIssueAndHalts) { - using namespace AZ::JsonSerializationResult; + namespace JSR = AZ::JsonSerializationResult; bool hasMessage = false; - auto callback = [&hasMessage](AZStd::string_view message, ResultCode result, AZStd::string_view) -> ResultCode + auto callback = [&hasMessage](AZStd::string_view message, JSR::ResultCode result, AZStd::string_view) -> JSR::ResultCode { hasMessage = !message.empty(); return result; }; - m_serializationSettings->m_reporting = AZStd::move(callback); + this->m_serializationSettings->m_reporting = AZStd::move(callback); - Type instance{}; - ResultCode result = - AZ::JsonSerialization::Store(*m_jsonDocument, m_jsonDocument->GetAllocator(), instance, *m_serializationSettings); + JSR::ResultCode result = AZ::JsonSerialization::Store( + *this->m_jsonDocument, this->m_jsonDocument->GetAllocator(), this->m_instance, *this->m_serializationSettings); - EXPECT_EQ(Processing::Halted, result.GetProcessing()); + EXPECT_EQ(JSR::Processing::Halted, result.GetProcessing()); EXPECT_TRUE(hasMessage); } } // namespace JsonSerializationTests From c46c82079c293477d68233508fda52f711ea3868 Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 9 Jun 2021 18:35:04 -0700 Subject: [PATCH 4/4] Fixed string format bug in JsonRegistrationContext --- .../AzCore/AzCore/Serialization/Json/RegistrationContext.cpp | 3 ++- .../Tests/Serialization/Json/JsonRegistrationContextTests.cpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp index 9aa80a2ef2..82db4df000 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/RegistrationContext.cpp @@ -51,7 +51,8 @@ namespace AZ AZ_Assert( emplaceResult.second, "Couldn't register Json serializer %s. Another serializer (%s) has already been registered for the same Uuid (%s).", - serializer->RTTI_GetTypeName(), emplaceResult.first->second->RTTI_GetTypeName(), uuid); + serializer->RTTI_GetTypeName(), emplaceResult.first->second->RTTI_GetTypeName(), + uuid.ToString().c_str()); } else { diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp index 6012a824c7..7367fd47ff 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp @@ -285,7 +285,7 @@ namespace JsonSerializationTests EXPECT_EQ(1, m_jsonRegistrationContext->GetRegisteredSerializers().size()); AZ::BaseJsonSerializer* mockSerializer = m_jsonRegistrationContext->GetSerializerForType(azrtti_typeid()); - EXPECT_NE(mockSerializer, nullptr); + ASSERT_NE(mockSerializer, nullptr); EXPECT_EQ(AZ::AzTypeInfo::Uuid(), mockSerializer->RTTI_GetType()); SerializerWithOneType::Unreflect(m_jsonRegistrationContext.get()); @@ -301,7 +301,7 @@ namespace JsonSerializationTests EXPECT_EQ(1, m_jsonRegistrationContext->GetRegisteredSerializers().size()); AZ::BaseJsonSerializer* mockSerializer = m_jsonRegistrationContext->GetSerializerForType(azrtti_typeid()); - EXPECT_NE(mockSerializer, nullptr); + ASSERT_NE(mockSerializer, nullptr); EXPECT_EQ(AZ::AzTypeInfo::Uuid(), mockSerializer->RTTI_GetType()); SerializerWithOneType::Unreflect(m_jsonRegistrationContext.get());