Refactor the interface after some chatting with @amazon-employee-dm

Signed-off-by: Nicholas Van Sickle <nvsickle@amazon.com>

Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonBackend.h

@@ -8,31 +8,34 @@
 
 #pragma once
 
-#include <AzCore/DOM/DomBackend.h>
 #include <AzCore/DOM/Backends/JSON/JsonSerializationUtils.h>
+#include <AzCore/DOM/DomBackend.h>
 
 namespace AZ::Dom
 {
     //! A DOM backend for serializing and deserializing JSON <=> UTF-8 text
     //! \param ParseFlags Controls how deserialized JSON is parsed.
     //! \param WriteFormat Controls how serialized JSON is formatted.
-    template<Json::ParseFlags ParseFlags = Json::ParseFlags::ParseComments, Json::OutputFormatting WriteFormat = Json::OutputFormatting::PrettyPrintedJson>
+    template<
+        Json::ParseFlags ParseFlags = Json::ParseFlags::ParseComments,
+        Json::OutputFormatting WriteFormat = Json::OutputFormatting::PrettyPrintedJson>
     class JsonBackend final : public Backend
     {
     public:
-        Visitor::Result ReadFromStringInPlace(AZStd::string& buffer, Visitor& visitor) override
+        Visitor::Result ReadFromBuffer(const char* buffer, size_t size, AZ::Dom::Lifetime lifetime, Visitor& visitor) override
         {
-            return Json::VisitSerializedJsonInPlace<ParseFlags>(buffer, visitor);
+            return Json::VisitSerializedJson<ParseFlags>({ buffer, size }, lifetime, visitor);
         }
 
-        Visitor::Result ReadFromString(AZStd::string_view buffer, Lifetime lifetime, Visitor& visitor) override
+        Visitor::Result ReadFromBufferInPlace(char* buffer, Visitor& visitor) override
         {
-            return Json::VisitSerializedJson<ParseFlags>(buffer, lifetime, visitor);
+            return Json::VisitSerializedJsonInPlace<ParseFlags>(buffer, visitor);
         }
 
-        AZStd::unique_ptr<Visitor> CreateStreamWriter(AZ::IO::GenericStream& stream) override
+        Visitor::Result WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback) override
         {
-            return Json::CreateJsonStreamWriter(stream, WriteFormat);
+            AZStd::unique_ptr<Visitor> visitor = Json::CreateJsonStreamWriter(stream, WriteFormat);
+            return callback(*visitor.get());
         }
     };
 } // namespace AZ::Dom

Code/Framework/AzCore/AzCore/DOM/Backends/JSON/JsonSerializationUtils.h

@@ -115,17 +115,17 @@ namespace AZ::Dom::Json
     //! rapidjson stream wrapper for AZStd::string suitable for in-situ parsing
     //! Faster than rapidjson::MemoryStream for reading from AZStd::string / AZStd::string_view (because it requires a null terminator)
     //! \note This needs to be inlined for performance reasons.
-    struct AzStringStream
+    struct NullDelimitedStringStream
     {
         using Ch = char; //<! Denotes the string character storage type for rapidjson
 
-        AZ_FORCE_INLINE AzStringStream(AZStd::string& buffer)
+        AZ_FORCE_INLINE NullDelimitedStringStream(char* buffer)
         {
-            m_cursor = buffer.data();
+            m_cursor = buffer;
             m_begin = m_cursor;
         }
 
-        AZ_FORCE_INLINE AzStringStream(AZStd::string_view buffer)
+        AZ_FORCE_INLINE NullDelimitedStringStream(AZStd::string_view buffer)
         {
             // rapidjson won't actually call PutBegin or Put unless kParseInSituFlag is set, so this is safe
             m_cursor = const_cast<char*>(buffer.data());
@@ -193,6 +193,7 @@ namespace AZ::Dom::Json
     //! \return The aggregate result specifying whether the visitor operations were successful.
     template<ParseFlags parseFlags = ParseFlags::ParseComments>
     Visitor::Result VisitSerializedJson(AZStd::string_view buffer, Lifetime lifetime, Visitor& visitor);
+
     //! Reads serialized JSON from a string in-place and applies it to a visitor.
     //! \param buffer The UTF-8 serialized JSON to read. This buffer will be modified as part of the deserialization process to
     //! apply null terminators.
@@ -201,7 +202,7 @@ namespace AZ::Dom::Json
     //! \param parseFlags (template) Settings for adjusting parser behavior.
     //! \return The aggregate result specifying whether the visitor operations were successful.
     template<ParseFlags parseFlags = ParseFlags::ParseComments>
-    Visitor::Result VisitSerializedJsonInPlace(AZStd::string& buffer, Visitor& visitor);
+    Visitor::Result VisitSerializedJsonInPlace(char* buffer, Visitor& visitor);
 
     //! Takes a visitor specified by a callback and produces a rapidjson::Document.
     //! \param writeCallback A callback specifying a visitor to accept to build the resulting document.
@@ -231,7 +232,7 @@ namespace AZ::Dom::Json
         // If the string is null terminated, we can use the faster AzStringStream path - otherwise we fall back on rapidjson::MemoryStream
         if (buffer.data()[buffer.size()] == '\0')
         {
-            AzStringStream stream(buffer);
+            NullDelimitedStringStream stream(buffer);
             reader.Parse<static_cast<int>(parseFlags)>(stream, handler);
         }
         else
@@ -243,10 +244,10 @@ namespace AZ::Dom::Json
     }
 
     template<ParseFlags parseFlags>
-    Visitor::Result VisitSerializedJsonInPlace(AZStd::string& buffer, Visitor& visitor)
+    Visitor::Result VisitSerializedJsonInPlace(char* buffer, Visitor& visitor)
     {
         rapidjson::Reader reader;
-        AzStringStream stream(buffer);
+        NullDelimitedStringStream stream(buffer);
         RapidJsonReadHandler handler(&visitor, Lifetime::Persistent);
 
         reader.Parse<static_cast<int>(parseFlags) | rapidjson::kParseInsituFlag>(stream, handler);

Code/Framework/AzCore/AzCore/DOM/DomBackend.cpp

@@ -8,40 +8,10 @@
 
 #include <AzCore/DOM/DomBackend.h>
 
-#include <AzCore/Utils/Utils.h>
-#include <AzCore/IO/ByteContainerStream.h>
-#include <AzCore/DOM/DomBackend.h>
-
 namespace AZ::Dom
 {
-    Visitor::Result Backend::ReadFromStream(AZ::IO::GenericStream* stream, Visitor& visitor, size_t maxSize)
-    {
-        size_t length = stream->GetLength();
-        if (length > maxSize)
-        {
-            return AZ::Failure(VisitorError(VisitorErrorCode::InternalError, "Stream is too large."));
-        }
-        AZStd::string buffer;
-        buffer.resize(length);
-        stream->Read(length, buffer.data());
-        return ReadFromString(buffer, Lifetime::Temporary, visitor);
-    }
-
-    Visitor::Result Backend::ReadFromStringInPlace(AZStd::string& buffer, Visitor& visitor)
-    {
-        return ReadFromString(buffer, Lifetime::Persistent, visitor);
-    }
-
-    Visitor::Result Backend::WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback)
-    {
-        AZStd::unique_ptr<Visitor> writer = CreateStreamWriter(stream);
-        return callback(*writer.get());
-    }
-
-    Visitor::Result Backend::WriteToString(AZStd::string& buffer, WriteCallback callback)
+    Visitor::Result Backend::ReadFromBufferInPlace(char* buffer, Visitor& visitor)
     {
-        AZ::IO::ByteContainerStream<AZStd::string> stream{&buffer};
-        AZStd::unique_ptr<Visitor> writer = CreateStreamWriter(stream);
-        return callback(*writer.get());
+        return ReadFromBuffer(buffer, strlen(buffer), AZ::Dom::Lifetime::Persistent, visitor);
     }
-}
+} // namespace AZ::Dom

Code/Framework/AzCore/AzCore/DOM/DomBackend.h

@@ -22,28 +22,22 @@ namespace AZ::Dom
     public:
         virtual ~Backend() = default;
 
-        //! Attempt to read this format from the given stream into the target Visitor.
-        //! The base implementation reads the stream into memory and calls ReadFromString.
-        virtual Visitor::Result ReadFromStream(
-            AZ::IO::GenericStream* stream, Visitor& visitor, size_t maxSize = AZStd::numeric_limits<size_t>::max());
+        //! Attempt to read this format from the given buffer into the target Visitor.
+        virtual Visitor::Result ReadFromBuffer(
+            const char* buffer, size_t size, AZ::Dom::Lifetime lifetime, Visitor& visitor) = 0;
         //! Attempt to read this format from a mutable string into the target Visitor. This enables some backends to
         //! parse without making additional string allocations.
+        //! This string must be null terminated.
         //! This string may be modified and read in place without being copied, so when calling this please ensure that:
         //! - The string won't be deallocated until the visitor no longer needs the values and
         //! - The string is safe to modify in place.
-        //! The base implementation simply calls ReadFromString.
-        virtual Visitor::Result ReadFromStringInPlace(AZStd::string& buffer, Visitor& visitor);
-        //! Attempt to read this format from an immutable buffer in memory into the target Visitor.
-        virtual Visitor::Result ReadFromString(AZStd::string_view buffer, Lifetime lifetime, Visitor& visitor) = 0;
+        //! The base implementation simply calls ReadFromBuffer.
+        virtual Visitor::Result ReadFromBufferInPlace(char* buffer, Visitor& visitor);
 
-        //! Acquire a visitor interface for writing to the target output file.
-        virtual AZStd::unique_ptr<Visitor> CreateStreamWriter(AZ::IO::GenericStream& stream) = 0;
         //! A callback that accepts a Visitor, making DOM calls to inform the serializer, and returns an
         //! aggregate error code to indicate whether or not the operation succeeded.
         using WriteCallback = AZStd::function<Visitor::Result(Visitor&)>;
         //! Attempt to write a value to a stream using a write callback.
-        Visitor::Result WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback);
-        //! Attempt to write a value to a string using a write callback.
-        Visitor::Result WriteToString(AZStd::string& buffer, WriteCallback callback);
+        virtual Visitor::Result WriteToStream(AZ::IO::GenericStream& stream, WriteCallback callback) = 0;
     };
 } // namespace AZ::Dom

Code/Framework/AzCore/AzCore/azcore_files.cmake

@@ -127,6 +127,8 @@ set(FILES
     Debug/TraceReflection.h
     DOM/DomBackend.cpp
     DOM/DomBackend.h
+    DOM/DomUtils.cpp
+    DOM/DomUtils.h
     DOM/DomVisitor.cpp
     DOM/DomVisitor.h
     DOM/Backends/JSON/JsonBackend.h

Code/Framework/AzCore/Tests/DOM/DomJsonBenchmarks.cpp

@@ -8,6 +8,7 @@
 
 #if defined(HAVE_BENCHMARK)
 
+#include <AzCore/DOM/DomUtils.h>
 #include <AzCore/DOM/Backends/JSON/JsonBackend.h>
 #include <AzCore/DOM/Backends/JSON/JsonSerializationUtils.h>
 #include <AzCore/JSON/document.h>
@@ -132,7 +133,7 @@ namespace Benchmark
             auto result = AZ::Dom::Json::WriteToRapidJsonDocument(
                 [&](AZ::Dom::Visitor& visitor)
                 {
-                    return backend.ReadFromStringInPlace(payloadCopy, visitor);
+                    return AZ::Dom::Utils::ReadFromStringInPlace(backend, payloadCopy, visitor);
                 });
 
             benchmark::DoNotOptimize(result.GetValue());
@@ -152,7 +153,7 @@ namespace Benchmark
             auto result = AZ::Dom::Json::WriteToRapidJsonDocument(
                 [&](AZ::Dom::Visitor& visitor)
                 {
-                    return backend.ReadFromString(serializedPayload, AZ::Dom::Lifetime::Temporary, visitor);
+                    return AZ::Dom::Utils::ReadFromString(backend, serializedPayload, AZ::Dom::Lifetime::Temporary, visitor);
                 });
 
             benchmark::DoNotOptimize(result.GetValue());

Code/Framework/AzCore/Tests/DOM/DomJsonTests.cpp

@@ -8,6 +8,7 @@
 
 #include <AzCore/DOM/Backends/JSON/JsonBackend.h>
 #include <AzCore/DOM/Backends/JSON/JsonSerializationUtils.h>
+#include <AzCore/DOM/DomUtils.h>
 #include <AzCore/Name/NameDictionary.h>
 #include <AzCore/Serialization/Json/JsonSerialization.h>
 #include <AzCore/Serialization/Json/JsonUtils.h>
@@ -70,7 +71,7 @@ namespace AZ::Dom::Tests
             {
                 AZStd::string serializedDocument;
                 JsonBackend backend;
-                auto result = backend.WriteToString(serializedDocument, visitDocumentFn);
+                auto result = Dom::Utils::WriteToString(backend, serializedDocument, visitDocumentFn);
                 EXPECT_TRUE(result.IsSuccess());
                 EXPECT_EQ(canonicalSerializedDocument, serializedDocument);
             }
@@ -81,7 +82,7 @@ namespace AZ::Dom::Tests
                     [&canonicalSerializedDocument](AZ::Dom::Visitor& visitor)
                     {
                         JsonBackend backend;
-                        return backend.ReadFromString(canonicalSerializedDocument, AZ::Dom::Lifetime::Temporary, visitor);
+                        return Dom::Utils::ReadFromString(backend, canonicalSerializedDocument, Dom::Lifetime::Persistent, visitor);
                     });
                 EXPECT_TRUE(result.IsSuccess());
                 EXPECT_EQ(AZ::JsonSerialization::Compare(*m_document, result.GetValue()), JsonSerializerCompareResult::Equal);
@@ -91,11 +92,11 @@ namespace AZ::Dom::Tests
             {
                 AZStd::string serializedDocument;
                 JsonBackend backend;
-                auto result = backend.WriteToString(
-                    serializedDocument,
+                auto result = Dom::Utils::WriteToString(
+                    backend, serializedDocument,
                     [&backend, &canonicalSerializedDocument](AZ::Dom::Visitor& visitor)
                     {
-                        return backend.ReadFromString(canonicalSerializedDocument, AZ::Dom::Lifetime::Temporary, visitor);
+                        return Dom::Utils::ReadFromString(backend, canonicalSerializedDocument, Dom::Lifetime::Persistent, visitor);
                     });
                 EXPECT_TRUE(result.IsSuccess());
                 EXPECT_EQ(canonicalSerializedDocument, serializedDocument);