refactor(csv): scan for csv in-tree, dropping vincentlaucsb-csv-parser

CMakeLists.txt

@@ -56,7 +56,6 @@ find_package(pugixml REQUIRED)
 find_package(miniz REQUIRED)
 find_package(cryptopp REQUIRED)
 find_package(nlohmann_json REQUIRED)
-find_package(vincentlaucsb-csv-parser REQUIRED)
 find_package(uchardet REQUIRED)
 find_package(utf8cpp REQUIRED)
 
@@ -266,7 +265,6 @@ target_link_libraries(odr
         miniz::miniz
         cryptopp::cryptopp
         nlohmann_json::nlohmann_json
-        vincentlaucsb-csv-parser::vincentlaucsb-csv-parser
         uchardet::uchardet
         utf8::cpp
 )

conan.lock

@@ -4,7 +4,6 @@
         "zstd/1.5.7#b68ca8e3de04ba5957761751d1d661f4%1760955092.069",
         "zlib/1.3.2#1cb806da49011867778ffb6ac7190fcb%1777558780.503",
         "xz_utils/5.8.3#a8432fead347c69d8b2737c35f936132%1775752656.4",
-        "vincentlaucsb-csv-parser/2.3.0#ac67e368e82c9e3da4a663c35e3a1b2f%1718528275.177",
         "utfcpp/4.0.9#b5eb56fe6b829b6f3eb8da167c4f41b2%1773905531.586",
         "uchardet/0.0.8#6ab25e452021fcdb560f4e37f4a27bc1%1759735438.978",
         "pybind11/2.13.6#42746850cd4c68d1b1ea42de456c2182%1755673714.548",

conanfile.py

@@ -50,7 +50,6 @@ def requirements(self):
         self.requires("cryptopp/8.9.0")
         self.requires("miniz/3.0.2")
         self.requires("nlohmann_json/3.12.0")
-        self.requires("vincentlaucsb-csv-parser/2.3.0")
         self.requires("uchardet/0.0.8")
         self.requires("utfcpp/4.0.9")
         if self.options.get_safe("with_http_server", False):

src/odr/internal/csv/csv_util.cpp

@@ -1,27 +1,105 @@
 #include <odr/internal/csv/csv_util.hpp>
 
-#include <odr/internal/util/stream_util.hpp>
-
-#include <csv.hpp>
+#include <cstddef>
+#include <istream>
+#include <optional>
+#include <stdexcept>
+#include <string>
+#include <utility>
+#include <vector>
 
 namespace odr::internal {
 
-void csv::check_csv_file(std::istream &in) {
-  // TODO it might be better to read the file from disk to decide on the format
-  // https://github.com/vincentlaucsb/csv-parser#memory-mapped-files-vs-streams
+namespace {
+
+constexpr char csv_delimiter = ',';
+constexpr char csv_quote = '"';
+
+} // namespace
+
+bool csv::read_record(std::istream &in, std::vector<std::string> &fields) {
+  fields.clear();
+  std::string field;
+  bool quoted = false;
+  bool started = false;
+
+  const auto end_field = [&] {
+    fields.push_back(std::move(field));
+    field.clear();
+  };
+
+  char c = 0;
+  while (in.get(c)) {
+    if (quoted) {
+      if (c != csv_quote) {
+        field.push_back(c);
+        continue;
+      }
+      // a doubled quote is an escaped one and stays inside the field
+      if (in.peek() == csv_quote) {
+        in.get(c);
+        field.push_back(csv_quote);
+        continue;
+      }
+      quoted = false;
+      continue;
+    }
 
-  ::csv::CSVFormat format;
-  // TODO safe to say a CSV with variable columns is invalid?
-  format.variable_columns(::csv::VariableColumnPolicy::THROW);
+    switch (c) {
+    case csv_quote:
+      quoted = true;
+      started = true;
+      break;
+    case csv_delimiter:
+      end_field();
+      started = true;
+      break;
+    case '\r':
+      break; // CRLF, and a lone CR
+    case '\n':
+      if (!started) {
+        break; // empty line
+      }
+      end_field();
+      return true;
+    default:
+      field.push_back(c);
+      started = true;
+      break;
+    }
+  }
+
+  // reaching EOF inside a quoted field means the quote was never closed; the
+  // partial field is not a record
+  if (quoted) {
+    throw std::runtime_error("csv quoted field is not terminated");
+  }
 
-  // TODO feed in junks; limit check size
-  auto parser = ::csv::parse(util::stream::read(in), format);
+  if (!started) {
+    return false;
+  }
+  end_field();
+  return true;
+}
+
+void csv::check_csv_file(std::istream &in) {
+  std::optional<std::size_t> columns;
 
-  // this will actually check `variable_columns`
-  for ([[maybe_unused]] auto &&_ : parser) {
+  std::vector<std::string> fields;
+  while (read_record(in, fields)) {
+    if (!columns.has_value()) {
+      columns = fields.size();
+      continue;
+    }
+    if (fields.size() != *columns) {
+      throw std::runtime_error("csv row has " + std::to_string(fields.size()) +
+                               " fields, expected " + std::to_string(*columns));
+    }
   }
 
-  if (parser.get_col_names().size() <= 1) {
+  // one column is every line of prose ever written, so it is no evidence of a
+  // csv; the caller is probing an otherwise unrecognised text file
+  if (!columns.has_value() || *columns <= 1) {
     throw std::runtime_error("no csv file");
   }
 }

src/odr/internal/csv/csv_util.hpp

@@ -1,9 +1,22 @@
 #pragma once
 
 #include <iosfwd>
+#include <string>
+#include <vector>
 
 namespace odr::internal::csv {
 
+/// Reads the next record into @p fields; `false` once the input is exhausted.
+///
+/// RFC 4180: `,` separates, `"` quotes, `""` is a literal quote inside a quoted
+/// field, and a quoted field may span lines. An empty line yields no record,
+/// which is what keeps a trailing newline from reading as a one-field row.
+///
+/// @throws std::runtime_error if the input ends inside a quoted field.
+bool read_record(std::istream &in, std::vector<std::string> &fields);
+
+/// Throws unless @p in parses as a csv whose records all carry the same number
+/// of fields, and more than one of them.
 void check_csv_file(std::istream &in);
 
-}
+} // namespace odr::internal::csv

test/CMakeLists.txt

@@ -52,7 +52,6 @@ add_executable(odr_test
         "src/internal/crypto/crypto_util_test.cpp"
 
         "src/internal/csv/csv_file_test.cpp"
-        "src/internal/csv/csv_test.cpp"
 
         "src/internal/oldms/doc_test.cpp"
         "src/internal/oldms/ppt_test.cpp"
@@ -117,7 +116,6 @@ target_link_libraries(odr_test
         miniz::miniz
         cryptopp::cryptopp
         nlohmann_json::nlohmann_json
-        vincentlaucsb-csv-parser::vincentlaucsb-csv-parser
         uchardet::uchardet
         GTest::gtest
 

test/src/internal/csv/csv_file_test.cpp

@@ -3,10 +3,15 @@
 
 #include <test_util.hpp>
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 #include <odr/internal/csv/csv_file.hpp>
 
+#include <memory>
+#include <stdexcept>
+#include <string>
+
 using namespace odr;
 using namespace odr::test;
 
@@ -25,5 +30,46 @@ TEST(CsvFile, txt) {
 }
 
 TEST(CsvFile, csv) {
-  File(TestData::test_file_path("odr-public/csv/file_example_ODS_5000.csv"));
+  const File file(
+      TestData::test_file_path("odr-public/csv/file_example_ODS_5000.csv"));
+
+  EXPECT_NO_THROW(internal::csv::CsvFile(
+      std::make_shared<internal::text::TextFile>(file.impl())));
+
+  // and the probe in `open_strategy` reaches the same conclusion
+  EXPECT_THAT(DecodedFile::list_file_types(file),
+              testing::Contains(FileType::comma_separated_values));
+}
+
+TEST(CsvFile, records_must_have_a_consistent_field_count) {
+  const auto check = [](const std::string &content) {
+    internal::csv::CsvFile(std::make_shared<internal::text::TextFile>(
+        File::from_memory(content).impl()));
+  };
+
+  EXPECT_NO_THROW(check("a,b,c\n1,2,3\n"));
+  EXPECT_NO_THROW(check("a,b,c\n1,2,3"));        // no trailing newline
+  EXPECT_NO_THROW(check("a,b\r\n1,2\r\n"));      // crlf
+  EXPECT_NO_THROW(check("a,b\n\"x,y\",2\n"));    // a quoted delimiter
+  EXPECT_NO_THROW(check("a,b\n\"x\ny\",2\n"));   // a quoted newline
+  EXPECT_NO_THROW(check("a,b\n\"x\"\"y\",2\n")); // an escaped quote
+
+  EXPECT_THROW(check("a,b,c\n1,2\n"), std::runtime_error); // short row
+  EXPECT_THROW(check("a,b\n1,2,3\n"), std::runtime_error); // long row
+  EXPECT_THROW(check("one column\nand another\n"), std::runtime_error);
+  EXPECT_THROW(check(""), std::runtime_error);
+}
+
+/// An unterminated quote used to reach EOF and emit the partial field as a
+/// record, so text that happens to hold an odd number of quotes and a
+/// consistent comma count was classified as csv.
+TEST(CsvFile, a_quoted_field_must_be_terminated) {
+  const auto check = [](const std::string &content) {
+    internal::csv::CsvFile(std::make_shared<internal::text::TextFile>(
+        File::from_memory(content).impl()));
+  };
+
+  EXPECT_THROW(check("a,b\n1,\"2,3"), std::runtime_error);
+  EXPECT_THROW(check("a,b\n\"1,2\n"), std::runtime_error);
+  EXPECT_THROW(check("a,b\n1,\"2"), std::runtime_error);
 }

test/src/test_util.cpp

@@ -6,12 +6,13 @@
 #include <odr/odr.hpp>
 
 #include <odr/internal/common/path.hpp>
-
-#include <csv.hpp>
+#include <odr/internal/csv/csv_util.hpp>
 
 #include <algorithm>
 #include <filesystem>
+#include <fstream>
 #include <string>
+#include <unordered_map>
 #include <utility>
 
 using namespace odr;
@@ -22,6 +23,29 @@ namespace odr::test {
 
 namespace {
 
+/// The rows of a header-carrying csv, each keyed by column name. Absent columns
+/// read as empty, so a row that stops short of the header is not an error.
+std::vector<std::unordered_map<std::string, std::string>>
+read_indexed_csv(const std::string &path) {
+  std::ifstream in(path);
+
+  std::vector<std::string> header;
+  if (!csv::read_record(in, header)) {
+    return {};
+  }
+
+  std::vector<std::unordered_map<std::string, std::string>> rows;
+  std::vector<std::string> fields;
+  while (csv::read_record(in, fields)) {
+    std::unordered_map<std::string, std::string> row;
+    for (std::size_t i = 0; i < header.size() && i < fields.size(); ++i) {
+      row[header[i]] = fields[i];
+    }
+    rows.push_back(std::move(row));
+  }
+  return rows;
+}
+
 TestFile get_test_file(const std::string &root_path,
                        std::string absolute_path) {
   const FileType type =
@@ -54,13 +78,18 @@ std::vector<TestFile> get_test_files(const std::string &root_path,
 
   const std::string index_path = input_path + "/index.csv";
   if (fs::is_regular_file(index_path)) {
-    for (const auto &row : csv::CSVReader(index_path)) {
-      std::string absolute_path = input_path + "/" + row["path"].get<>();
+    for (const auto &row : read_indexed_csv(index_path)) {
+      const auto field = [&row](const std::string &name) {
+        const auto it = row.find(name);
+        return it == row.end() ? std::string() : it->second;
+      };
+
+      std::string absolute_path = input_path + "/" + field("path");
       std::string short_path = absolute_path.substr(root_path.size() + 1);
-      FileType type = file_type_by_file_extension(row["type"].get<>());
-      std::optional<std::string> password = row["encrypted"].get<>() == "yes"
-                                                ? row["password"].get<>()
-                                                : std::optional<std::string>();
+      FileType type = file_type_by_file_extension(field("type"));
+      std::optional<std::string> password =
+          field("encrypted") == "yes" ? std::optional(field("password"))
+                                      : std::optional<std::string>();
 
       if (type == FileType::unknown) {
         continue;