diff --git a/OloEngine/src/OloEngine/Serialization/StreamReader.h b/OloEngine/src/OloEngine/Serialization/StreamReader.h index 5dec8efd7..96cc296d0 100644 --- a/OloEngine/src/OloEngine/Serialization/StreamReader.h +++ b/OloEngine/src/OloEngine/Serialization/StreamReader.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace OloEngine { @@ -53,94 +54,54 @@ namespace OloEngine T::Deserialize(this, obj); } - template - void ReadMap(std::map& map, u32 size = 0) + /// @brief Reads any associative container (std::map / std::unordered_map). + /// @note Each key and value is read via ReadElement, mirroring StreamWriter::WriteMap: + /// std::string keys/values come from ReadString, trivially-copyable elements are + /// raw, the rest use their static Deserialize. A size of 0 means "read the u32 + /// count first"; pass a non-zero size to skip the prefix. + template + void ReadMap(Map& map, u32 size = 0) { if (size == 0) ReadRaw(size); + using KeyType = typename Map::key_type; for (u32 i = 0; i < size; ++i) { - Key key; - if constexpr (std::is_trivially_copyable_v) - ReadRaw(key); - else - ReadObject(key); - - if constexpr (std::is_trivially_copyable_v) - ReadRaw(map[key]); - else - ReadObject(map[key]); + KeyType key; + ReadElement(key); + ReadElement(map[key]); } } - template - void ReadMap(std::unordered_map& map, u32 size = 0) + /// @brief Reads a vector. Each element is read via ReadElement (see ReadMap). + /// @note A size of 0 means "read the u32 count first"; pass a non-zero size to skip it. + template + void ReadArray(std::vector& array, u32 size = 0) { if (size == 0) ReadRaw(size); - for (u32 i = 0; i < size; ++i) - { - Key key; - if constexpr (std::is_trivially_copyable_v) - ReadRaw(key); - else - ReadObject(key); - - if constexpr (std::is_trivially_copyable_v) - ReadRaw(map[key]); - else - ReadObject(map[key]); - } - } - - template - void ReadMap(std::unordered_map& map, u32 size = 0) - { - if (size == 0) - ReadRaw(size); + array.resize(size); for (u32 i = 0; i < size; ++i) - { - std::string key; - ReadString(key); - - if constexpr (std::is_trivially_copyable_v) - ReadRaw(map[key]); - else - ReadObject(map[key]); - } + ReadElement(array[i]); } + private: + /// @brief Reads a single container element with the right primitive: std::string → + /// ReadString (length-prefixed), trivially-copyable → ReadRaw (raw bytes), + /// otherwise ReadObject (static Deserialize). template - void ReadArray(std::vector& array, u32 size = 0) + void ReadElement(T& element) { - if (size == 0) - ReadRaw(size); - - array.resize(size); - - for (u32 i = 0; i < size; ++i) - { - if constexpr (std::is_trivially_copyable_v) - ReadRaw(array[i]); - else - ReadObject(array[i]); - } + if constexpr (std::is_same_v) + ReadString(element); + else if constexpr (std::is_trivially_copyable_v) + ReadRaw(element); + else + ReadObject(element); } }; - template<> - inline void StreamReader::ReadArray(std::vector& array, u32 size) - { - if (size == 0) - ReadRaw(size); - - array.resize(size); - - for (u32 i = 0; i < size; ++i) - ReadString(array[i]); - } - } // namespace OloEngine diff --git a/OloEngine/src/OloEngine/Serialization/StreamWriter.h b/OloEngine/src/OloEngine/Serialization/StreamWriter.h index 6023d7a18..fba31d19d 100644 --- a/OloEngine/src/OloEngine/Serialization/StreamWriter.h +++ b/OloEngine/src/OloEngine/Serialization/StreamWriter.h @@ -6,6 +6,7 @@ #include #include #include +#include namespace OloEngine { @@ -47,87 +48,50 @@ namespace OloEngine T::Serialize(this, obj); } - template - void WriteMap(const std::map& map, bool writeSize = true) + /// @brief Writes any associative container (std::map / std::unordered_map). + /// @note Each key and value is written via WriteElement, so std::string keys/values go + /// through WriteString (length-prefixed) while trivially-copyable elements are raw + /// and the rest use their static Serialize. Wire format: optional u32 count, then + /// count × { key, value }. + template + void WriteMap(const Map& map, bool writeSize = true) { if (writeSize) - WriteRaw((u32)map.size()); + WriteRaw(static_cast(map.size())); for (const auto& [key, value] : map) { - if constexpr (std::is_trivially_copyable_v) - WriteRaw(key); - else - WriteObject(key); - - if constexpr (std::is_trivially_copyable_v) - WriteRaw(value); - else - WriteObject(value); + WriteElement(key); + WriteElement(value); } } - template - void WriteMap(const std::unordered_map& map, bool writeSize = true) - { - if (writeSize) - WriteRaw((u32)map.size()); - - for (const auto& [key, value] : map) - { - if constexpr (std::is_trivially_copyable_v) - WriteRaw(key); - else - WriteObject(key); - - if constexpr (std::is_trivially_copyable_v) - WriteRaw(value); - else - WriteObject(value); - } - } - - template - void WriteMap(const std::unordered_map& map, bool writeSize = true) + /// @brief Writes a vector. Each element is written via WriteElement (see WriteMap). + /// @note Wire format: optional u32 count, then count × element. + template + void WriteArray(const std::vector& array, bool writeSize = true) { if (writeSize) - WriteRaw((u32)map.size()); - - for (const auto& [key, value] : map) - { - WriteString(key); + WriteRaw(static_cast(array.size())); - if constexpr (std::is_trivially_copyable_v) - WriteRaw(value); - else - WriteObject(value); - } + for (const auto& element : array) + WriteElement(element); } + private: + /// @brief Writes a single container element with the right primitive: std::string → + /// WriteString (length-prefixed), trivially-copyable → WriteRaw (raw bytes), + /// otherwise WriteObject (static Serialize). template - void WriteArray(const std::vector& array, bool writeSize = true) + void WriteElement(const T& element) { - if (writeSize) - WriteRaw((u32)array.size()); - - for (const auto& element : array) - { - if constexpr (std::is_trivially_copyable_v) - WriteRaw(element); - else - WriteObject(element); - } + if constexpr (std::is_same_v) + WriteString(element); + else if constexpr (std::is_trivially_copyable_v) + WriteRaw(element); + else + WriteObject(element); } }; - template<> - inline void StreamWriter::WriteArray(const std::vector& array, bool writeSize) - { - if (writeSize) - WriteRaw((u32)array.size()); - - for (const auto& element : array) - WriteString(element); - } - } // namespace OloEngine diff --git a/OloEngine/tests/CMakeLists.txt b/OloEngine/tests/CMakeLists.txt index 72c5d9a08..7bd352d90 100644 --- a/OloEngine/tests/CMakeLists.txt +++ b/OloEngine/tests/CMakeLists.txt @@ -211,6 +211,7 @@ add_executable(OloEngine-Tests ShaderGraph/ShaderGraphCommandTest.cpp # Serialization Tests Serialization/ArchiveExtensionsTest.cpp + Serialization/StreamHelpersTest.cpp Serialization/MeshBinarySerializerTest.cpp Serialization/MeshAssetSerializerTest.cpp Serialization/SoundGraphSerializerTest.cpp diff --git a/OloEngine/tests/Serialization/StreamHelpersTest.cpp b/OloEngine/tests/Serialization/StreamHelpersTest.cpp new file mode 100644 index 000000000..41e2b48b3 --- /dev/null +++ b/OloEngine/tests/Serialization/StreamHelpersTest.cpp @@ -0,0 +1,363 @@ +// OLO_TEST_LAYER: unit +// +// Round-trip + wire-format coverage for the StreamWriter / StreamReader container +// helpers (WriteMap/ReadMap, WriteArray/ReadArray) after they were collapsed onto a +// single string-aware WriteElement/ReadElement helper. The byte-level assertions pin +// the on-disk format so the dedup refactor cannot silently change it: trivially-copyable +// elements stay raw, std::string keys/values stay length-prefixed, and class elements +// route through the static Serialize/Deserialize contract. + +#include "OloEnginePCH.h" +#include + +#include "OloEngine/Serialization/StreamWriter.h" +#include "OloEngine/Serialization/StreamReader.h" + +#include +#include +#include +#include +#include + +using namespace OloEngine; + +namespace +{ + // Minimal in-memory StreamWriter/StreamReader pair backed by a byte vector, so the + // container helpers can be round-tripped without touching the filesystem. + class MemoryStreamWriter final : public StreamWriter + { + public: + explicit MemoryStreamWriter(std::vector& buffer) : m_Buffer(buffer) {} + + bool IsStreamGood() const override + { + return true; + } + u64 GetStreamPosition() override + { + return static_cast(m_Position); + } + void SetStreamPosition(u64 position) override + { + m_Position = static_cast(position); + } + + bool WriteData(const char* data, sizet size) override + { + if (m_Position + size > m_Buffer.size()) + m_Buffer.resize(m_Position + size); + if (size > 0) + std::memcpy(m_Buffer.data() + m_Position, data, size); + m_Position += size; + return true; + } + + private: + std::vector& m_Buffer; + sizet m_Position = 0; + }; + + class MemoryStreamReader final : public StreamReader + { + public: + explicit MemoryStreamReader(const std::vector& buffer) : m_Buffer(buffer) {} + + bool IsStreamGood() const override + { + return m_Position <= m_Buffer.size(); + } + u64 GetStreamPosition() override + { + return static_cast(m_Position); + } + void SetStreamPosition(u64 position) override + { + m_Position = static_cast(position); + } + + bool ReadData(char* destination, sizet size) override + { + if (m_Position + size > m_Buffer.size()) + return false; + if (size > 0) + std::memcpy(destination, m_Buffer.data() + m_Position, size); + m_Position += size; + return true; + } + + private: + const std::vector& m_Buffer; + sizet m_Position = 0; + }; + + // Non-trivially-copyable element type exercising the WriteObject/ReadObject branch via the + // static Serialize/Deserialize contract the helpers expect. + struct Widget + { + std::string Name; + u32 Id = 0; + + bool operator==(const Widget& other) const + { + return Name == other.Name && Id == other.Id; + } + + static void Serialize(StreamWriter* writer, const Widget& w) + { + writer->WriteString(w.Name); + writer->WriteRaw(w.Id); + } + + static void Deserialize(StreamReader* reader, Widget& w) + { + reader->ReadString(w.Name); + reader->ReadRaw(w.Id); + } + }; + + // Helpers that build an expected byte layout independently of the stream helpers, so the + // wire-format assertions encode the format spec rather than mirroring the implementation. + template + void AppendRaw(std::vector& buffer, const T& value) + { + const char* bytes = reinterpret_cast(&value); + buffer.insert(buffer.end(), bytes, bytes + sizeof(T)); + } + + void AppendString(std::vector& buffer, const std::string& s) + { + AppendRaw(buffer, static_cast(s.size())); + buffer.insert(buffer.end(), s.begin(), s.end()); + } +} // namespace + +// ============================================================================ +// Array round-trips — trivially-copyable, std::string (folded specialization), +// and the class/WriteObject branch. +// ============================================================================ + +TEST(StreamHelpers, ArrayOfTrivialRoundtrips) +{ + std::vector original{ 7, 8, 9, 0, 4242 }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original); + + std::vector loaded; + MemoryStreamReader reader(buffer); + reader.ReadArray(loaded); + + EXPECT_EQ(loaded, original); +} + +TEST(StreamHelpers, ArrayOfStringsRoundtrips) +{ + std::vector original{ "hello", "world", "", "a longer string with spaces" }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original); + + std::vector loaded; + MemoryStreamReader reader(buffer); + reader.ReadArray(loaded); + + EXPECT_EQ(loaded, original); +} + +TEST(StreamHelpers, ArrayOfObjectsRoundtrips) +{ + std::vector original{ { "alpha", 1 }, { "", 0 }, { "gamma", 4242 } }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original); + + std::vector loaded; + MemoryStreamReader reader(buffer); + reader.ReadArray(loaded); + + EXPECT_EQ(loaded, original); +} + +TEST(StreamHelpers, EmptyArrayRoundtrips) +{ + std::vector original; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original); + + std::vector loaded{ 1, 2, 3 }; // pre-filled to verify it is cleared + MemoryStreamReader reader(buffer); + reader.ReadArray(loaded); + + EXPECT_TRUE(loaded.empty()); +} + +// ============================================================================ +// Map round-trips — every key/value combination the helpers must support. +// ============================================================================ + +TEST(StreamHelpers, MapTrivialKeyTrivialValueRoundtrips) +{ + std::map original{ { 1, 1.5f }, { 2, -2.75f }, { 99, 0.0f } }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteMap(original); + + std::map loaded; + MemoryStreamReader reader(buffer); + reader.ReadMap(loaded); + + ASSERT_EQ(loaded.size(), original.size()); + for (const auto& [key, value] : original) + { + ASSERT_TRUE(loaded.contains(key)); + EXPECT_FLOAT_EQ(loaded[key], value); + } +} + +TEST(StreamHelpers, UnorderedMapStringKeyRoundtrips) +{ + std::unordered_map original{ { "speed", 10 }, { "health", 100 }, { "", 7 } }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteMap(original); + + std::unordered_map loaded; + MemoryStreamReader reader(buffer); + reader.ReadMap(loaded); + + EXPECT_EQ(loaded, original); +} + +TEST(StreamHelpers, MapStringKeyObjectValueRoundtrips) +{ + std::map original{ + { "first", { "alpha", 1 } }, + { "second", { "beta", 2 } }, + }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteMap(original); + + std::map loaded; + MemoryStreamReader reader(buffer); + reader.ReadMap(loaded); + + EXPECT_EQ(loaded, original); +} + +TEST(StreamHelpers, MapTrivialKeyStringValueRoundtrips) +{ + std::unordered_map original{ { 1, "one" }, { 2, "two" }, { 3, "" } }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteMap(original); + + std::unordered_map loaded; + MemoryStreamReader reader(buffer); + reader.ReadMap(loaded); + + EXPECT_EQ(loaded, original); +} + +TEST(StreamHelpers, EmptyMapRoundtrips) +{ + std::unordered_map original; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteMap(original); + + // Unlike ReadArray (which resize()s and so clears), ReadMap inserts into the destination + // and never clears it — a count of 0 therefore performs no inserts. Read into a fresh map. + std::unordered_map loaded; + MemoryStreamReader reader(buffer); + reader.ReadMap(loaded); + + EXPECT_TRUE(loaded.empty()); +} + +// ============================================================================ +// writeSize=false / explicit-size path — mirrors the real callers (SoundGraph +// prototype, font ranges) that write/read the count out of band. +// ============================================================================ + +TEST(StreamHelpers, ArrayWithoutSizePrefixRoundtrips) +{ + std::vector original{ 11, 22, 33 }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original, /*writeSize=*/false); // no count prefix + + std::vector loaded; + MemoryStreamReader reader(buffer); + reader.ReadArray(loaded, static_cast(original.size())); // caller supplies the count + + EXPECT_EQ(loaded, original); +} + +// ============================================================================ +// Wire-format pins — the dedup must not change the bytes on disk. Expected +// buffers are built independently via AppendRaw/AppendString. +// ============================================================================ + +TEST(StreamHelpers, TrivialArrayWireFormatIsRawCountThenElements) +{ + std::vector original{ 7, 8 }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original); + + std::vector expected; + AppendRaw(expected, static_cast(original.size())); + for (auto v : original) + AppendRaw(expected, v); + + EXPECT_EQ(buffer, expected); +} + +TEST(StreamHelpers, StringArrayWireFormatIsLengthPrefixed) +{ + std::vector original{ "ab", "" }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteArray(original); + + // Strings must stay length-prefixed (u64 length + bytes), never raw — this is the + // behaviour the folded std::vector specialization used to provide. + std::vector expected; + AppendRaw(expected, static_cast(original.size())); + for (const auto& s : original) + AppendString(expected, s); + + EXPECT_EQ(buffer, expected); +} + +TEST(StreamHelpers, StringKeyMapWireFormatMatchesManualStringThenValue) +{ + // A single-entry map writes: u32 count, then (u64 keyLen + key bytes), then raw value. + std::unordered_map original{ { "key", 42 } }; + + std::vector buffer; + MemoryStreamWriter writer(buffer); + writer.WriteMap(original); + + std::vector expected; + AppendRaw(expected, static_cast(original.size())); + AppendString(expected, "key"); + AppendRaw(expected, static_cast(42)); + + EXPECT_EQ(buffer, expected); +}