From 2c7bc763a51d95d8d147fb93eed3eb6cb02c455f Mon Sep 17 00:00:00 2001
From: ado <adnan.abd0124@gmail.com>
Date: Sun, 6 Aug 2023 16:55:52 +0200
Subject: [PATCH] Update ssp.hpp, add more parser unit tests

---
 ssp.hpp              | 51 ++++++++++++++++++++++++++++++++--------
 test/test_parser.cpp | 55 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 90 insertions(+), 16 deletions(-)

diff --git a/ssp.hpp b/ssp.hpp
index 2f01507..101ef9d 100644
--- a/ssp.hpp
+++ b/ssp.hpp
@@ -2399,9 +2399,9 @@ public:
             if constexpr (throw_on_error) {
                 try {
                     reader_.parse();
-                } catch (...) {
+                } catch (const ss::exception& e) {
                     read_line();
-                    throw;
+                    decorate_rethrow(e);
                 }
             } else {
                 reader_.parse();
@@ -2427,9 +2427,9 @@ public:
                 auto value = reader_.converter_.template convert<T, Ts...>();
                 read_line();
                 return value;
-            } catch (...) {
+            } catch (const ss::exception& e) {
                 read_line();
-                throw;
+                decorate_rethrow(e);
             }
         }
 
@@ -2510,12 +2510,15 @@ public:
             using value = std::conditional_t<get_object, T,
                                              no_void_validator_tup_t<T, Ts...>>;
 
-            iterator() : parser_{nullptr} {
+            iterator() : parser_{nullptr}, value_{} {
             }
 
-            iterator(parser<Options...>* parser) : parser_{parser} {
+            iterator(parser<Options...>* parser) : parser_{parser}, value_{} {
             }
 
+            iterator(const iterator& other) = default;
+            iterator(iterator&& other) = default;
+
             value& operator*() {
                 return value_;
             }
@@ -2525,7 +2528,7 @@ public:
             }
 
             iterator& operator++() {
-                if (parser_->eof()) {
+                if (!parser_ || parser_->eof()) {
                     parser_ = nullptr;
                 } else {
                     if constexpr (get_object) {
@@ -2554,8 +2557,8 @@ public:
             }
 
         private:
-            value value_;
             parser<Options...>* parser_;
+            value value_;
         };
 
         iterable(parser<Options...>* parser) : parser_{parser} {
@@ -2766,7 +2769,14 @@ private:
         std::string raw_header_copy = raw_header_;
         splitter.split(raw_header_copy.data(), reader_.delim_);
         for (const auto& [begin, end] : splitter.split_data_) {
-            header_.emplace_back(begin, end);
+            std::string field{begin, end};
+            if (std::find(header_.begin(), header_.end(), field) !=
+                header_.end()) {
+                handle_error_invalid_header(field);
+                header_.clear();
+                return;
+            }
+            header_.push_back(std::move(field));
         }
     }
 
@@ -2898,6 +2908,29 @@ private:
         }
     }
 
+    void handle_error_invalid_header(const std::string& field) {
+        constexpr static auto error_msg = "header contains duplicates: ";
+
+        if constexpr (string_error) {
+            error_.clear();
+            error_.append(error_msg).append(error_msg);
+        } else if constexpr (throw_on_error) {
+            throw ss::exception{error_msg + field};
+        } else {
+            error_ = true;
+        }
+    }
+
+    void decorate_rethrow(const ss::exception& e) const {
+        static_assert(throw_on_error,
+                      "throw_on_error needs to be enabled to use this method");
+        throw ss::exception{std::string{file_name_}
+                                .append(" ")
+                                .append(std::to_string(reader_.line_number_))
+                                .append(": ")
+                                .append(e.what())};
+    }
+
     ////////////////
     // line reading
     ////////////////
diff --git a/test/test_parser.cpp b/test/test_parser.cpp
index 40b5969..9e29a90 100644
--- a/test/test_parser.cpp
+++ b/test/test_parser.cpp
@@ -1259,18 +1259,44 @@ void test_invalid_fields_impl(const std::vector<std::string>& lines,
     {
         // Field used multiple times
         ss::parser<Ts...> p{f.name, ","};
-        auto command = [&] { p.use_fields(fields[0], fields[0]); };
-        expect_error_on_command(p, command);
+        auto command = [&] { p.use_fields(fields.at(0), fields.at(0)); };
+        if (!fields.empty()) {
+            expect_error_on_command(p, command);
+        }
     }
 
     {
         // Mapping out of range
         ss::parser<Ts...> p{f.name, ","};
         auto command = [&] {
-            p.use_fields(fields[0]);
+            p.use_fields(fields.at(0));
             p.template get_next<std::string, std::string>();
         };
-        expect_error_on_command(p, command);
+        if (!fields.empty()) {
+            expect_error_on_command(p, command);
+        }
+    }
+
+    {
+        // Invalid header
+        ss::parser<Ts...> p{f.name, ","};
+        auto command = [&] { p.use_fields(fields); };
+
+        if (!fields.empty()) {
+            // Pass if there are no duplicates, fail otherwise
+            if (std::unordered_set<std::string>{fields.begin(), fields.end()}
+                    .size() != fields.size()) {
+                expect_error_on_command(p, command);
+            } else {
+                command();
+                CHECK(p.valid());
+                if (!p.valid()) {
+                    if constexpr (ss::setup<Ts...>::string_error) {
+                        std::cout << p.error_msg() << std::endl;
+                    }
+                }
+            }
+        }
     }
 }
 
@@ -1282,13 +1308,28 @@ void test_invalid_fields(const std::vector<std::string>& lines,
     test_invalid_fields_impl<ss::throw_on_error>(lines, fields);
 }
 
-// TODO add more test cases
 TEST_CASE("parser test invalid header fields usage") {
+    test_invalid_fields({}, {});
+
+    test_invalid_fields({"Int"}, {"Int"});
+    test_invalid_fields({"Int", "1"}, {"Int"});
+    test_invalid_fields({"Int", "1", "2"}, {"Int"});
+
+    test_invalid_fields({"Int,String"}, {"Int", "String"});
+    test_invalid_fields({"Int,String", "1,hi"}, {"Int", "String"});
+    test_invalid_fields({"Int,String", "2,hello"}, {"Int", "String"});
+
+    test_invalid_fields({"Int,String,Double"}, {"Int", "String", "Double"});
     test_invalid_fields({"Int,String,Double", "1,hi,2.34"},
                         {"Int", "String", "Double"});
+    test_invalid_fields({"Int,String,Double", "1,hi,2.34", "2,hello,3.45"},
+                        {"Int", "String", "Double"});
 
-    test_invalid_fields({"Int,Int,Int", "1,2,3"},
-                        {"Int", "Int", "Int"});
+    test_invalid_fields({"Int,Int,Int"}, {"Int", "Int", "Int"});
+    test_invalid_fields({"Int,Int,Int", "1,2,3"}, {"Int", "Int", "Int"});
+
+    test_invalid_fields({"Int,String,Int"}, {"Int", "String", "Int"});
+    test_invalid_fields({"Int,String,Int", "1,hi,3"}, {"Int", "String", "Int"});
 }
 
 template <typename... Ts>