[ANT-1890] Properly handle playlist_reset in generaldata.ini (#879)

Fix #864 Properly handle data and avoid exception thrown when reading not numerical values with stoi when reading "playlist_reset" entry in general_data.ini --------- Co-authored-by: Florian Omnès <[email protected]>
AntaresSimulatorTeam · Jul 23, 2024 · caf2805 · caf2805
1 parent 6619f9f
commit caf2805
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 24 deletions.
diff --git a/src/cpp/lpnamer/input_reader/GeneralDataReader.cpp b/src/cpp/lpnamer/input_reader/GeneralDataReader.cpp
@@ -1,8 +1,38 @@
 #include "GeneralDataReader.h"
 
+#include <algorithm>
+#include <filesystem>
+#include <fstream>
 #include <iterator>
+#include <numeric>
+#include <sstream>
+#include <stdexcept>
+#include <vector>
 
+#include "INIReader.h"
 #include "LogUtils.h"
+#include "ProblemGenerationLogger.h"
+#include "StringManip.h"
+class IniReaderUtils {
+ public:
+  static bool LineIsNotASectionHeader(const std::string& line) {
+    return StringManip::split(StringManip::trim(line), '=').size() == 2;
+  }
+
+  static std::string ReadSectionHeader(const std::string& line) {
+    auto str = StringManip::trim(line);
+    str.erase(std::remove(str.begin(), str.end(), '['), str.end());
+    str.erase(std::remove(str.begin(), str.end(), ']'), str.end());
+    return str;
+  }
+
+  static std::pair<std::string, int> GetKeyValFromLine(std::string_view line) {
+    const auto splitLine = StringManip::split(line, '=');
+    auto key = StringManip::trim(splitLine[0]);
+    auto val = std::stoi(StringManip::trim(splitLine[1]));
+    return {key, val};
+  }
+};
 GeneralDataIniReader::GeneralDataIniReader(
     const std::filesystem::path& file_path,
     ProblemGenerationLog::ProblemGenerationLoggerSharedPointer logger)
@@ -52,7 +82,7 @@ std::vector<int> GeneralDataIniReader::GetActiveYears_() {
 std::vector<int> GeneralDataIniReader::ActiveYearsFromActiveList() {
   std::vector<int> active_years;
   for (auto year = 0; year < mc_years_; year++) {
-    if (std::find(active_year_list_.begin(), active_year_list_.end(), year) !=
+    if (std::ranges::find(active_year_list_, year) !=
         active_year_list_.end()) {
       active_years.push_back(year + 1);
     }
@@ -63,7 +93,7 @@ std::vector<int> GeneralDataIniReader::ActiveYearsFromActiveList() {
 std::vector<int> GeneralDataIniReader::ActiveYearsFromInactiveList() {
   std::vector<int> active_years;
   for (auto year = 0; year < mc_years_; year++) {
-    if (std::find(inactive_year_list_.begin(), inactive_year_list_.end(),
+    if (std::ranges::find(inactive_year_list_,
                   year) == inactive_year_list_.end()) {
       active_years.push_back(year + 1);
     }
@@ -79,6 +109,9 @@ GeneralDataIniReader::GetRawPlaylist() {
   for (const auto& line : file_lines_) {
     if (IniReaderUtils::LineIsNotASectionHeader(line)) {
       if (current_section == "playlist") {
+        if (line.find("playlist_reset") != std::string::npos) {
+          continue;
+        }
         auto [key, val] = IniReaderUtils::GetKeyValFromLine(line);
         ReadPlaylistVal(key, val);
       }
@@ -104,14 +137,19 @@ std::string GeneralDataIniReader::ReadPlaylist(
     const std::string& current_section, const std::string& line) {
   if (IniReaderUtils::LineIsNotASectionHeader(line)) {
     if (current_section == "playlist") {
-      auto [key, val] = IniReaderUtils::GetKeyValFromLine(line);
-      ReadPlaylistVal(key, val);
+      ReadPlaylist(line);
     }
   } else {
     return IniReaderUtils::ReadSectionHeader(line);
   }
   return current_section;
 }
+void GeneralDataIniReader::ReadPlaylist(const std::string& line) {
+  if (line.find("playlist_reset") == std::string::npos) {
+    auto [key, val] = IniReaderUtils::GetKeyValFromLine(line);
+    ReadPlaylistVal(key, val);
+  }
+}
 
 void GeneralDataIniReader::ReadPlaylistVal(const std::string& key, int val) {
   if (key == "playlist_year +") {

diff --git a/src/cpp/lpnamer/input_reader/GeneralDataReader.h b/src/cpp/lpnamer/input_reader/GeneralDataReader.h
@@ -18,26 +18,6 @@ class IniFileNotFound : public std::runtime_error {
   explicit IniFileNotFound(const std::string& msg) : std::runtime_error(msg) {}
 };
 
-class IniReaderUtils {
- public:
-  static bool LineIsNotASectionHeader(const std::string& line) {
-    return StringManip::split(StringManip::trim(line), '=').size() == 2;
-  }
-
-  static std::string ReadSectionHeader(const std::string& line) {
-    auto str = StringManip::trim(line);
-    str.erase(std::remove(str.begin(), str.end(), '['), str.end());
-    str.erase(std::remove(str.begin(), str.end(), ']'), str.end());
-    return str;
-  }
-
-  static std::pair<std::string, int> GetKeyValFromLine(std::string_view line) {
-    auto key = StringManip::trim(StringManip::split(line, '=')[0]);
-    auto val = std::stoi(StringManip::trim(StringManip::split(line, '=')[1]));
-    return {key, val};
-  }
-};
-
 class GeneralDataIniReader {
  private:
   ProblemGenerationLog::ProblemGenerationLoggerSharedPointer logger_;
@@ -57,6 +37,7 @@ class GeneralDataIniReader {
   std::string ReadPlaylist(const std::string& current_section,
                            const std::string& line);
   void ReadPlaylistVal(const std::string& key, int val);
+  void ReadPlaylist(const std::string& line);
 
  public:
   explicit GeneralDataIniReader(

diff --git a/tests/cpp/lp_namer/CMakeLists.txt b/tests/cpp/lp_namer/CMakeLists.txt
@@ -36,6 +36,7 @@ add_executable (lp_namer_tests
         WeightsFileReaderTest.cpp
         LpFilesExtractorTest.cpp
         MpsTxtWriterTest.cpp
+        GeneralDataReadetTests.cpp
 )
 
 target_link_libraries (lp_namer_tests PRIVATE

diff --git a/tests/cpp/lp_namer/GeneralDataReadetTests.cpp b/tests/cpp/lp_namer/GeneralDataReadetTests.cpp
@@ -0,0 +1,42 @@
+#include <gtest/gtest.h>
+
+#include "GeneralDataReader.h"
+
+//Test for the GeneralDataIniReader class
+class GeneralDataIniReaderTests : public testing::TestWithParam<bool> {
+ public:
+  GeneralDataIniReaderTests() = default;
+  ~GeneralDataIniReaderTests() override = default;
+};
+
+INSTANTIATE_TEST_SUITE_P(reset_value, GeneralDataIniReaderTests,
+                         testing::Bool());
+TEST_P(GeneralDataIniReaderTests, ReadPlaylistReset_option) {
+  auto test_dir = std::filesystem::temp_directory_path() / std::tmpnam(nullptr);
+  std::filesystem::create_directory(test_dir);
+  auto ini_file = test_dir / "test.ini";
+  std::ofstream file(ini_file);
+  file << "[general]\n"
+          "nbyears=10\n"
+          "user-playlist=true\n"
+          "[playlist]\n"
+          "playlist_reset = ";
+  file << (GetParam() ? "true\n" : "false\n");
+  file << "playlist_year += 0\n"
+          "playlist_year += 5\n"
+          "playlist_year += 8\n"
+          "playlist_year -= 1\n"
+          "playlist_year -= 2\n"
+          "playlist_year -= 3\n";
+  file.close();
+
+  GeneralDataIniReader reader(ini_file, nullptr);
+  //GetActiveYears() returns the active years whereas file contains index. year=index+1
+  //reset = false => Active playlist (+=)
+  //rest = true => !inactive playlist (-=)
+  if (GetParam()) {
+    EXPECT_EQ(reader.GetActiveYears(), std::vector<int>({1, 5, 6, 7, 8, 9, 10}));
+  } else {
+    EXPECT_EQ(reader.GetActiveYears(), std::vector<int>({1, 6, 9}));
+  }
+}