diff --git a/rwgame/GameConfig.cpp b/rwgame/GameConfig.cpp index de6f48ab..076691a2 100644 --- a/rwgame/GameConfig.cpp +++ b/rwgame/GameConfig.cpp @@ -13,7 +13,7 @@ GameConfig::GameConfig(const std::string& configName, const std::string& configPath) : m_configName(configName) , m_configPath(configPath) - , m_valid(false) + , m_parseResult() , m_inputInvertY(false) { if (m_configPath.empty()) { m_configPath = getDefaultConfigPath(); @@ -23,7 +23,7 @@ GameConfig::GameConfig(const std::string& configName, auto configFile = getConfigFile(); std::string dummy; - m_valid = parseConfig(ParseType::FILE, configFile, ParseType::CONFIG, dummy); + m_parseResult = parseConfig(ParseType::FILE, configFile, ParseType::CONFIG, dummy); } std::string GameConfig::getConfigFile() const { @@ -31,7 +31,11 @@ std::string GameConfig::getConfigFile() const { } bool GameConfig::isValid() const { - return m_valid; + return m_parseResult.isValid(); +} + +const GameConfig::ParseResult &GameConfig::getParseResult() const { + return m_parseResult; } std::string GameConfig::getDefaultConfigPath() { @@ -109,7 +113,7 @@ struct IntTranslator { } }; -bool GameConfig::saveConfig() { +GameConfig::ParseResult GameConfig::saveConfig() { auto filename = getConfigFile(); return parseConfig(ParseType::CONFIG, "", ParseType::FILE, filename); @@ -121,32 +125,32 @@ std::string GameConfig::getDefaultINIString() { return result; } -bool GameConfig::parseConfig( +GameConfig::ParseResult GameConfig::parseConfig( GameConfig::ParseType srcType, const std::string &source, ParseType destType, std::string &destination) { pt::ptree srcTree; + ParseResult parseResult; try { if (srcType == ParseType::STRING) { pt::read_ini(source, srcTree); } else if (srcType == ParseType::FILE) { - std::ifstream ifs(source); - pt::read_ini(ifs, srcTree); + pt::read_ini(source, srcTree); } } catch (pt::ini_parser_error &e) { // Catches illegal input files (nonsensical input, duplicate keys) + parseResult.failInputFile(e.filename(), e.line(), e.message()); RW_MESSAGE(e.what()); - return false; + return parseResult; } if (destType == ParseType::DEFAULT) { + parseResult.failArgument(); RW_ERROR("Target cannot be DEFAULT."); - return false; + return parseResult; } - - bool success = true; - + auto read_config = [&](const std::string &key, auto &target, const auto &defaultValue, auto &translator, bool optional=true) { @@ -168,15 +172,14 @@ bool GameConfig::parseConfig( } catch (pt::ptree_bad_path &e) { // Catches missing key-value pairs: fail when required if (!optional) { - success = false; - RW_MESSAGE(e.what()); - return; + parseResult.failRequiredMissing(key); + RW_MESSAGE(e.what()); + return; } sourceValue = defaultValue; } catch (pt::ptree_bad_data &e) { // Catches illegal value data: always fail - success = false; - RW_MESSAGE("invalid data"); + parseResult.failInvalidData(key); RW_MESSAGE(e.what()); return; } @@ -187,7 +190,7 @@ bool GameConfig::parseConfig( switch (destType) { case ParseType::DEFAULT: // Target cannot be DEFAULT (case already handled) - success = false; + parseResult.failArgument(); break; case ParseType::CONFIG: // Don't care if success == false @@ -211,8 +214,8 @@ bool GameConfig::parseConfig( read_config("input.invert_y", this->m_inputInvertY, false, boolt); - if (!success) - return success; + if (!parseResult.isValid()) + return parseResult; try { if (destType == ParseType::STRING) { @@ -223,9 +226,64 @@ bool GameConfig::parseConfig( pt::write_ini(destination, srcTree); } } catch (pt::ini_parser_error &e) { - success = false; + parseResult.failOutputFile(e.filename(), e.line(), e.message()); RW_MESSAGE(e.what()); } - return success; + return parseResult; } + +GameConfig::ParseResult::ParseResult() + : m_result(ErrorType::GOOD) + , m_filename() + , m_line(0) + , m_message() + , m_keys_requiredMissing() + , m_keys_invalidData() { +} + +GameConfig::ParseResult::ErrorType GameConfig::ParseResult::type() const { + return this->m_result; +} + +bool GameConfig::ParseResult::isValid() const { + return this->type() == ErrorType::GOOD; +} + +void GameConfig::ParseResult::failInputFile(const std::string &filename, size_t line, + const std::string &message) { + this->m_result = ParseResult::ErrorType::INVALIDINPUTFILE; + this->m_filename = filename; + this->m_line = line; + this->m_message = message; +} + +void GameConfig::ParseResult::failArgument() { + this->m_result = ParseResult::ErrorType::INVALIDARGUMENT; +} + +void GameConfig::ParseResult::failRequiredMissing(const std::string &key) { + this->m_result = ParseResult::ErrorType::INVALIDCONTENT; + this->m_keys_requiredMissing.push_back(key); +} + +void GameConfig::ParseResult::failInvalidData(const std::string &key) { + this->m_result = ParseResult::ErrorType::INVALIDCONTENT; + this->m_keys_invalidData.push_back(key); +} + +void GameConfig::ParseResult::failOutputFile(const std::string &filename, size_t line, const std::string &message) { + this->m_result = ParseResult::ErrorType::INVALIDOUTPUTFILE; + this->m_filename = filename; + this->m_line = line; + this->m_message = message; +} + +const std::vector &GameConfig::ParseResult::getKeysRequiredMissing() const { + return this->m_keys_requiredMissing; +} + +const std::vector &GameConfig::ParseResult::getKeysInvalidData() const { + return this->m_keys_invalidData; +} + diff --git a/rwgame/GameConfig.hpp b/rwgame/GameConfig.hpp index 91199228..7824a8c1 100644 --- a/rwgame/GameConfig.hpp +++ b/rwgame/GameConfig.hpp @@ -1,9 +1,104 @@ #ifndef RWGAME_GAMECONFIG_HPP #define RWGAME_GAMECONFIG_HPP #include +#include class GameConfig { public: + class ParseResult { + public: + enum ErrorType { + /// GOOD: Input file/string was good + GOOD, + /// INVALIDINPUTFILE: There was some error while reading from a file or string or the input was ambiguous (e.g. duplicate keys) + INVALIDINPUTFILE, + /// INVALIDARGUMENT: The parser received impossible arguments + INVALIDARGUMENT, + /// INVALIDCONTENT: Some required keys were missing or some values were of incorrect type + INVALIDCONTENT, + /// INVALIDOUTPUTFILE: There was some error while writing to a file or string + INVALIDOUTPUTFILE + }; + /** + * @brief ParseResult Holds the issues occurred while parsing of a config file. + */ + + ParseResult(); + /** + * @brief type Get the type of error + * @return Type of error or GOOD if there was no error + */ + ErrorType type() const; + + /** + * @brief getKeysRequiredMissing Get the keys that were missing + * @return A vector with all the keys + */ + const std::vector &getKeysRequiredMissing() const; + + /** + * @brief getKeysInvalidData Get the keys that contained invalid data + * @return A vector with all the keys + */ + const std::vector &getKeysInvalidData() const; + /** + * @brief failInputFile Fail because the input file was invalid + * @param filename Filename of the invalid file + * @param line Line number where the error is located + * @param message Description of the error + */ + void failInputFile(const std::string &filename, size_t line, const std::string &message); + + /** + * @brief failArgument Fail because an argument was invalid + */ + void failArgument(); + + /** + * @brief failRequiredMissing Fail because a required key is missing + * @param key The key that is missing + */ + void failRequiredMissing(const std::string &key); + + /** + * @brief failInvalidData Fail because a key contains invalid data + * @param key The key that contains invalid data + */ + void failInvalidData(const std::string &key); + + /** + * @brief failOutputFile Fail because an error occurred while while writing to the output + * @param filename Filename of the invalid file + * @param line Line number where the error is located + * @param message Description of the error + */ + void failOutputFile(const std::string &filename, size_t line, const std::string &message); + + /** + * @brief isValid + * @return True if the loaded configuration is valid + */ + bool isValid() const; + private: + /// Type of the failure + ErrorType m_result; + + /// Filename of the invalid input or output file + std::string m_filename; + + /// Line number where the failure occurred (on invalid input or output file) + size_t m_line; + + /// Description of the failure (on invalid input or output file) + std::string m_message; + + /// All required keys that are missing + std::vector m_keys_requiredMissing; + + /// All keys that contain invalid data + std::vector m_keys_invalidData; + }; + /** * @brief GameConfig Loads a game configuration * @param configName The configuration filename to load @@ -20,7 +115,7 @@ public: /** * @brief writeConfig Save the game configuration */ - bool saveConfig(); + ParseResult saveConfig(); /** * @brief isValid @@ -28,6 +123,12 @@ public: */ bool isValid() const; + /** + * @brief getParseResult Get more information on parsing failures + * @return A ParseResult object containing more information + */ + const ParseResult &getParseResult() const; + /** * @brief getConfigString Returns the content of the default INI configuration. * @return INI string @@ -66,13 +167,13 @@ private: * INI string if srcType == STRING * @return True if the parsing succeeded */ - bool parseConfig(ParseType srcType, const std::string &source, + ParseResult parseConfig(ParseType srcType, const std::string &source, ParseType destType, std::string &destination); /* Config State */ std::string m_configName; std::string m_configPath; - bool m_valid; + ParseResult m_parseResult; /* Actual Configuration */ diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 8f52cfd9..252c43ca 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -58,13 +58,27 @@ public: std::string dirname() { return this->m_path.parent_path().string(); } + void change_perms_readonly() { + fs::permissions(this->m_path, fs::perms::owner_read + | fs::perms::group_read | fs::perms::others_read); + } template - void write(T t) { + bool append(T t) { // Append argument at the end of the file. // File is open/closes repeatedly. Not optimal. std::ofstream ofs(this->path(), std::ios::out | std::ios::app); ofs << t; ofs.close(); + return ofs.good(); + } + template + bool write(T t) { + // Write the argument to the file, discarding all contents. + // File is open/closes repeatedly. Not optimal. + std::ofstream ofs(this->path(), std::ios::out | std::ios::trunc); + ofs << t; + ofs.close(); + return ofs.good(); } private: static fs::path getRandomFilePath() { @@ -88,14 +102,18 @@ BOOST_AUTO_TEST_CASE(test_TempFile) { BOOST_CHECK_EQUAL(tempFile.exists(), true); tempFile.remove(); - tempFile.write("abc"); - tempFile.write("def"); + BOOST_CHECK_EQUAL(tempFile.append("abc"), true); + BOOST_CHECK_EQUAL(tempFile.append("def"), true); BOOST_CHECK_EQUAL(tempFile.exists(), true); tempFile.touch(); std::ifstream ifs(tempFile.path()); std::string line; std::getline(ifs, line); BOOST_CHECK_EQUAL(line, "abcdef"); + + tempFile.change_perms_readonly(); + BOOST_CHECK_EQUAL(tempFile.write("abc"), false); + BOOST_CHECK_EQUAL(tempFile.append("def"), false); } BOOST_AUTO_TEST_CASE(test_config_valid) { @@ -103,11 +121,14 @@ BOOST_AUTO_TEST_CASE(test_config_valid) { auto cfg = getValidConfig(); TempFile tempFile; - tempFile.write(cfg); + tempFile.append(cfg); GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(config.isValid()); + BOOST_CHECK_EQUAL(config.getParseResult().type(), GameConfig::ParseResult::GOOD); + BOOST_CHECK_EQUAL(config.getParseResult().getKeysRequiredMissing().size(), 0); + BOOST_CHECK_EQUAL(config.getParseResult().getKeysInvalidData().size(), 0); BOOST_CHECK_EQUAL(config.getGameDataPath(), "/dev/test"); BOOST_CHECK_EQUAL(config.getGameLanguage(), "american"); @@ -121,11 +142,14 @@ BOOST_AUTO_TEST_CASE(test_config_valid_modified) { cfg["input"]["invert_y"] = "0"; TempFile tempFile; - tempFile.write(cfg); + tempFile.append(cfg); GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(config.isValid()); + BOOST_CHECK_EQUAL(config.getParseResult().type(), GameConfig::ParseResult::GOOD); + BOOST_CHECK_EQUAL(config.getParseResult().getKeysRequiredMissing().size(), 0); + BOOST_CHECK_EQUAL(config.getParseResult().getKeysInvalidData().size(), 0); BOOST_CHECK_EQUAL(config.getInputInvertY(), false); BOOST_CHECK_EQUAL(config.getGameDataPath(), "Liberty City"); @@ -137,33 +161,49 @@ BOOST_AUTO_TEST_CASE(test_config_save) { cfg["game"]["path"] = "Liberty City"; TempFile tempFile; - tempFile.write(cfg); + tempFile.append(cfg); GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(config.isValid()); - + tempFile.remove(); BOOST_CHECK(!tempFile.exists()); - BOOST_CHECK(config.saveConfig()); + auto writeResult = config.saveConfig(); + BOOST_CHECK(writeResult.isValid()); BOOST_CHECK(tempFile.exists()); GameConfig config2(tempFile.filename(), tempFile.dirname()); - BOOST_CHECK_EQUAL(config2.getGameDataPath(), "Liberty City"); } +BOOST_AUTO_TEST_CASE(test_config_save_readonly) { + // Test whether saving to a readonly INI file fails + auto cfg = getValidConfig(); + + TempFile tempFile; + tempFile.append(cfg); + tempFile.change_perms_readonly(); + + GameConfig config(tempFile.filename(), tempFile.dirname()); + BOOST_CHECK_EQUAL(config.isValid(), true); + + auto writeResult = config.saveConfig(); + BOOST_CHECK(!writeResult.isValid()); + BOOST_CHECK_EQUAL(writeResult.type(), GameConfig::ParseResult::INVALIDOUTPUTFILE); +} + BOOST_AUTO_TEST_CASE(test_config_valid_default) { // Test whether the default INI string is valid TempFile tempFile; BOOST_CHECK(!tempFile.exists()); - + GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(!config.isValid()); auto defaultINI = config.getDefaultINIString(); - tempFile.write(defaultINI); + tempFile.append(defaultINI); config = GameConfig(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(config.isValid()); @@ -175,11 +215,13 @@ BOOST_AUTO_TEST_CASE(test_config_invalid_duplicate) { cfg["input"]["invert_y "] = "0"; TempFile tempFile; - tempFile.write(cfg); + tempFile.append(cfg); GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(!config.isValid()); + const auto &parseResult = config.getParseResult(); + BOOST_CHECK_EQUAL(parseResult.type(), GameConfig::ParseResult::INVALIDINPUTFILE); } BOOST_AUTO_TEST_CASE(test_config_invalid_required_missing) { @@ -188,11 +230,19 @@ BOOST_AUTO_TEST_CASE(test_config_invalid_required_missing) { cfg["game"].erase("path"); TempFile tempFile; - tempFile.write(cfg); + tempFile.append(cfg); GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(!config.isValid()); + + const auto &parseResult = config.getParseResult(); + BOOST_CHECK_EQUAL(parseResult.type(), GameConfig::ParseResult::INVALIDCONTENT); + + BOOST_CHECK_EQUAL(parseResult.getKeysRequiredMissing().size(), 1); + BOOST_CHECK_EQUAL(parseResult.getKeysInvalidData().size(), 0); + + BOOST_CHECK_EQUAL(parseResult.getKeysRequiredMissing()[0], "game.path"); } BOOST_AUTO_TEST_CASE(test_config_invalid_wrong_type) { @@ -201,16 +251,24 @@ BOOST_AUTO_TEST_CASE(test_config_invalid_wrong_type) { cfg["input"]["invert_y"]="d"; TempFile tempFile; - tempFile.write(cfg); + tempFile.append(cfg); GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(!config.isValid()); + + const auto &parseResult = config.getParseResult(); + BOOST_CHECK_EQUAL(parseResult.type(), GameConfig::ParseResult::INVALIDCONTENT); + + BOOST_CHECK_EQUAL(parseResult.getKeysRequiredMissing().size(), 0); + BOOST_CHECK_EQUAL(parseResult.getKeysInvalidData().size(), 1); + + BOOST_CHECK_EQUAL(parseResult.getKeysInvalidData()[0], "input.invert_y"); } BOOST_AUTO_TEST_CASE(test_config_invalid_empty) { // Test reading empty configuration file - + // An empty file has a valid data structure, but has missing keys and is thus invalid. TempFile tempFile; tempFile.touch(); BOOST_CHECK(tempFile.exists()); @@ -218,6 +276,10 @@ BOOST_AUTO_TEST_CASE(test_config_invalid_empty) { GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(!config.isValid()); + + const auto &parseResult = config.getParseResult(); + BOOST_CHECK_EQUAL(parseResult.type(), GameConfig::ParseResult::INVALIDCONTENT); + BOOST_CHECK_GE(parseResult.getKeysRequiredMissing().size(), 1); } BOOST_AUTO_TEST_CASE(test_config_invalid_nonexisting) { @@ -228,6 +290,9 @@ BOOST_AUTO_TEST_CASE(test_config_invalid_nonexisting) { GameConfig config(tempFile.filename(), tempFile.dirname()); BOOST_CHECK(!config.isValid()); + + const auto &parseResult = config.getParseResult(); + BOOST_CHECK_EQUAL(parseResult.type(), GameConfig::ParseResult::INVALIDINPUTFILE); } BOOST_AUTO_TEST_SUITE_END()