#6467: Add unit test checking serialising/deserialising entity key va…

…lues with double quotes. Deduplicate file loading code.

test/Brush.cpp

@@ -5,6 +5,7 @@
 #include "iselection.h"
 #include "itransformable.h"
 #include "scenelib.h"
+#include "algorithm/FileUtils.h"
 #include "math/Quaternion.h"
 #include "algorithm/Scene.h"
 #include "algorithm/Primitives.h"
@@ -289,11 +290,7 @@ TEST_F(Quake3BrushTest, SaveAxisAlignedBrushWithTransform)
 
     EXPECT_TRUE(fs::exists(tempPath)) << "File still doesn't exist: " << tempPath;
 
-    std::ifstream tempFile(tempPath);
-    std::stringstream content;
-    content << tempFile.rdbuf();
-
-    auto savedContent = content.str();
+    auto savedContent = algorithm::loadFileToString(tempPath);
 
     // This is checking the actual face string as written by the LegacyBrushDefExporter
     // including whitespace and all the syntax details

test/MapExport.cpp

@@ -97,11 +97,7 @@ std::string exportDefaultBrushUsingFormat(const std::string& gameType, const std
 
     EXPECT_TRUE(fs::exists(exportPath)) << "File still doesn't exist";
 
-    std::ifstream tempFile(exportPath);
-    std::stringstream content;
-    content << tempFile.rdbuf();
-
-    return content.str();
+    return algorithm::loadFileToString(exportPath);
 }
 
 TEST_F(MapExportTest, exportDoom3Brush)
@@ -320,11 +316,9 @@ void runExportWithEmptyFileExtension(const std::string& temporaryDataPath,const
 
     EXPECT_TRUE(fs::exists(tempPath)) << "File still doesn't exist";
 
-    std::ifstream tempFile(tempPath);
-    std::stringstream content;
-    content << tempFile.rdbuf();
+    auto content = algorithm::loadFileToString(tempPath);
 
-    EXPECT_NE(content.str().find("brushDef3"), std::string::npos) << "Couldn't find the brush keyword in the export";
+    EXPECT_NE(content.find("brushDef3"), std::string::npos) << "Couldn't find the brush keyword in the export";
 }
 
 }

test/MapSavingLoading.cpp

@@ -25,6 +25,7 @@
 #include "testutil/FileSelectionHelper.h"
 #include "testutil/FileSaveConfirmationHelper.h"
 #include "registry/registry.h"
+#include "testutil/TemporaryFile.h"
 
 using namespace std::chrono_literals;
 
@@ -1185,11 +1186,9 @@ TEST_F(MapSavingTest, saveWarnsAboutOverwriteAndUserCancels)
     EXPECT_TRUE(overwriteHelper.messageReceived());
 
     // File should not have been replaced
-    std::ifstream writtenFile(tempPath);
-    std::stringstream contents;
-    contents << writtenFile.rdbuf();
+    auto contents = algorithm::loadFileToString(tempPath);
 
-    EXPECT_EQ(contents.str(), tempContents);
+    EXPECT_EQ(contents, tempContents);
 }
 
 // #5729: Autosaver overwrites the stored filename that has been previously used for "Save Copy As"
@@ -1623,4 +1622,42 @@ TEST_F(MapLoadingTest, WarningAboutMissingInfoFile)
         << "Didn't receive a warning about the missing .darkradiant file";
 }
 
+TEST_F(MapSavingTest, EscapeCharactersInEntityKeyValues)
+{
+    auto mapContent = R"(Version 2
+// entity 0
+{
+"classname" "light"
+"name" "light"
+"origin" "0.160161 72.1389 24.7429"
+"test" "Test \"quoted\" test"
+"with\"quote" "--"
+}
+)";
+    string::replace_all_copy(mapContent, "\r\n", "\n"); // don't be fooled by platform-specific line breaks
+
+    fs::path mapPath = _context.getTemporaryDataPath();
+    mapPath /= "keyvalue_escaping.map";
+    TemporaryFile tempFile(mapPath.string(), mapContent);
+
+    GlobalCommandSystem().executeCommand("OpenMap", mapPath.string());
+
+    auto entity = algorithm::findFirstEntity(GlobalMapModule().getRoot(), [](auto&) { return true; });
+
+    EXPECT_EQ(Node_getEntity(entity)->getKeyValue("name"), "light") << "Wrong entity found after loading";
+
+    // Check the quoted keyvalue pairs
+    EXPECT_EQ(Node_getEntity(entity)->getKeyValue("with\"quote"), "--") << "Failed to deserialise quoted entity key";
+    EXPECT_EQ(Node_getEntity(entity)->getKeyValue("test"), "Test \"quoted\" test") << "Failed to deserialise quoted entity key value";
+
+    // Save the map
+    GlobalCommandSystem().executeCommand("SaveMapCopyAs", mapPath.string());
+
+    auto savedContent = algorithm::loadFileToString(mapPath);
+
+    // Exact same
+    string::replace_all_copy(savedContent, "\r\n", "\n"); // normalise line breaks
+    EXPECT_EQ(savedContent, mapContent) << "Failed to serialise quoted entity key values";
+}
+
 }

test/Settings.cpp

@@ -1,6 +1,8 @@
 #include "gtest/gtest.h"
 
 #include <fstream>
+
+#include "algorithm/FileUtils.h"
 #include "settings/MajorMinorVersion.h"
 #include "settings/SettingsManager.h"
 #include "module/ApplicationContextBase.h"
@@ -218,11 +220,7 @@ inline std::string loadSettingsFile(const IApplicationContext& context, const st
 
     if (existingFile.empty()) return {};
 
-    std::ifstream file(existingFile);
-    std::stringstream content;
-    content << file.rdbuf();
-
-    return content.str();
+    return algorithm::loadFileToString(existingFile);
 }
 
 // Checks that the version/base settings folders are respecting the version sort order

test/algorithm/FileUtils.h

@@ -87,12 +87,8 @@ inline std::string loadFileToString(const fs::path& path)
 // True if the file (full physical path) contains the given text (ignoring CRLF vs. LF line break differences)
 inline bool fileContainsText(const fs::path& path, const std::string& textToFind)
 {
-    std::stringstream contentStream;
-    std::ifstream input(path);
-
-    contentStream << input.rdbuf();
-
-    std::string contents = string::replace_all_copy(contentStream.str(), "\r\n", "\n");
+    auto rawContents = loadFileToString(path);
+    auto contents = string::replace_all_copy(rawContents, "\r\n", "\n");
 
     return contents.find(textToFind) != std::string::npos;
 }

test/algorithm/XmlUtils.h

@@ -3,6 +3,8 @@
 #include "gtest/gtest.h"
 #include <fstream>
 #include <sstream>
+
+#include "FileUtils.h"
 #include "string/predicate.h"
 
 namespace test
@@ -20,11 +22,7 @@ inline void assertStringIsMapxFile(const std::string& content)
 // Very rough check to see if path points to a file that looks like an XML document
 inline void assertFileIsMapxFile(const std::string& path)
 {
-    std::ifstream tempFile(path);
-    std::stringstream content;
-    content << tempFile.rdbuf();
-
-    assertStringIsMapxFile(content.str());
+    assertStringIsMapxFile(loadFileToString(path));
 }
 
 }