REF: Modernize to use filesystem paths

Author: threecgreen

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## 0.37.0 - TBD
+
+### Breaking changes
+- Changed the type of the `output_dir` parameter in ` HistoricalClient::BatchDownload()`
+  to a `std::filesystem::path` and the return type to `std::filesystem::path`
+- Changed the type of the `file_path` parameter in
+  `HistoricalClient::TimeseriesGetRangeToFile()` to a `std::filesystem::path`
+- Changed the type of the `file_path` parameter in the `DbnFileStore`, `InFileStream`,
+  and `OutFileStream` constructors to a `std::filesystem::path`
+
 ## 0.36.0 - 2025-05-27
 
 This version marks the release of DBN version 3 (DBNv3), which is the new default.

include/databento/dbn_file_store.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <string>
+#include <filesystem>  // path
 
 #include "databento/dbn.hpp"          // DecodeMetadata
 #include "databento/dbn_decoder.hpp"  // DbnDecoder
@@ -16,8 +16,9 @@ namespace databento {
 // used on a given instance.
 class DbnFileStore {
  public:
-  explicit DbnFileStore(const std::string& file_path);
-  DbnFileStore(ILogReceiver* log_receiver, const std::string& file_path,
+  explicit DbnFileStore(const std::filesystem::path& file_path);
+  DbnFileStore(ILogReceiver* log_receiver,
+               const std::filesystem::path& file_path,
                VersionUpgradePolicy upgrade_policy);
 
   // Callback API: calling Replay consumes the input.

include/databento/file_stream.hpp

@@ -1,16 +1,16 @@
 #pragma once
 
-#include <cstddef>  // byte, size_t
-#include <fstream>  // ifstream, ofstream
-#include <string>
+#include <cstddef>     // byte, size_t
+#include <filesystem>  // path
+#include <fstream>     // ifstream, ofstream
 
 #include "databento/ireadable.hpp"
 #include "databento/iwritable.hpp"
 
 namespace databento {
 class InFileStream : public IReadable {
  public:
-  explicit InFileStream(const std::string& file_path);
+  explicit InFileStream(const std::filesystem::path& file_path);
 
   // Read exactly `length` bytes into `buffer`.
   void ReadExact(std::byte* buffer, std::size_t length) override;
@@ -24,7 +24,7 @@ class InFileStream : public IReadable {
 
 class OutFileStream : public IWritable {
  public:
-  explicit OutFileStream(const std::string& file_path);
+  explicit OutFileStream(const std::filesystem::path& file_path);
 
   void WriteAll(const std::byte* buffer, std::size_t length) override;
 

include/databento/historical.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <cstdint>
+#include <filesystem>
 #include <map>  // multimap
 #include <string>
 #include <vector>
@@ -75,12 +76,12 @@ class Historical {
   // Lists all files associated with a batch job.
   std::vector<BatchFileDesc> BatchListFiles(const std::string& job_id);
   // Returns the paths of the downloaded files.
-  std::vector<std::string> BatchDownload(const std::string& output_dir,
-                                         const std::string& job_id);
+  std::vector<std::filesystem::path> BatchDownload(
+      const std::filesystem::path& output_dir, const std::string& job_id);
   // Returns the path of the downloaded file.
-  std::string BatchDownload(const std::string& output_dir,
-                            const std::string& job_id,
-                            const std::string& filename_to_download);
+  std::filesystem::path BatchDownload(const std::filesystem::path& output_dir,
+                                      const std::string& job_id,
+                                      const std::string& filename_to_download);
 
   /*
    * Metadata API
@@ -206,30 +207,33 @@ class Historical {
       const std::string& dataset,
       const DateTimeRange<UnixNanos>& datetime_range,
       const std::vector<std::string>& symbols, Schema schema,
-      const std::string& file_path);
+      const std::filesystem::path& file_path);
   DbnFileStore TimeseriesGetRangeToFile(
       const std::string& dataset,
       const DateTimeRange<std::string>& datetime_range,
       const std::vector<std::string>& symbols, Schema schema,
-      const std::string& file_path);
+      const std::filesystem::path& file_path);
   DbnFileStore TimeseriesGetRangeToFile(
       const std::string& dataset,
       const DateTimeRange<UnixNanos>& datetime_range,
       const std::vector<std::string>& symbols, Schema schema, SType stype_in,
-      SType stype_out, std::uint64_t limit, const std::string& file_path);
+      SType stype_out, std::uint64_t limit,
+      const std::filesystem::path& file_path);
   DbnFileStore TimeseriesGetRangeToFile(
       const std::string& dataset,
       const DateTimeRange<std::string>& datetime_range,
       const std::vector<std::string>& symbols, Schema schema, SType stype_in,
-      SType stype_out, std::uint64_t limit, const std::string& file_path);
+      SType stype_out, std::uint64_t limit,
+      const std::filesystem::path& file_path);
 
  private:
   using HttplibParams = std::multimap<std::string, std::string>;
 
   BatchJob BatchSubmitJob(const HttplibParams& params);
   void StreamToFile(const std::string& url_path, const HttplibParams& params,
-                    const std::string& file_path);
-  void DownloadFile(const std::string& url, const std::string& output_path);
+                    const std::filesystem::path& file_path);
+  void DownloadFile(const std::string& url,
+                    const std::filesystem::path& output_path);
   std::vector<BatchJob> BatchListJobs(const HttplibParams& params);
   std::vector<DatasetConditionDetail> MetadataGetDatasetCondition(
       const HttplibParams& params);
@@ -240,7 +244,7 @@ class Historical {
                           const MetadataCallback& metadata_callback,
                           const RecordCallback& record_callback);
   DbnFileStore TimeseriesGetRangeToFile(const HttplibParams& params,
-                                        const std::string& file_path);
+                                        const std::filesystem::path& file_path);
 
   ILogReceiver* log_receiver_;
   const std::string key_;

src/dbn_file_store.cpp

@@ -8,11 +8,11 @@
 
 using databento::DbnFileStore;
 
-DbnFileStore::DbnFileStore(const std::string& file_path)
+DbnFileStore::DbnFileStore(const std::filesystem::path& file_path)
     : decoder_{ILogReceiver::Default(), InFileStream{file_path}} {}
 
 DbnFileStore::DbnFileStore(ILogReceiver* log_receiver,
-                           const std::string& file_path,
+                           const std::filesystem::path& file_path,
                            VersionUpgradePolicy upgrade_policy)
     : decoder_{log_receiver, std::make_unique<InFileStream>(file_path),
                upgrade_policy} {}

src/file_stream.cpp

@@ -7,11 +7,12 @@
 
 using databento::InFileStream;
 
-InFileStream::InFileStream(const std::string& file_path)
+InFileStream::InFileStream(const std::filesystem::path& file_path)
     : stream_{file_path, std::ios::binary} {
   if (stream_.fail()) {
-    throw InvalidArgumentError{"InFileStream", "file_path",
-                               "Non-existent or invalid file at " + file_path};
+    throw InvalidArgumentError{
+        "InFileStream", "file_path",
+        "Non-existent or invalid file at " + file_path.string()};
   }
 }
 
@@ -33,7 +34,7 @@ std::size_t InFileStream::ReadSome(std::byte* buffer, std::size_t max_length) {
 
 using databento::OutFileStream;
 
-OutFileStream::OutFileStream(const std::string& file_path)
+OutFileStream::OutFileStream(const std::filesystem::path& file_path)
     : stream_{file_path, std::ios::binary} {
   if (stream_.fail()) {
     throw InvalidArgumentError{"OutFileStream", "file_path",

src/historical.cpp

@@ -117,16 +117,6 @@ void TryCreateDir(const std::filesystem::path& dir_name) {
   throw databento::Exception{"Unable to create directory "s +
                              dir_name.generic_string() + ": " + ec.message()};
 }
-
-std::string PathJoin(const std::string& dir, const std::string& path) {
-  if (dir.empty()) {
-    return path;
-  }
-  if (dir[dir.length() - 1] == '/') {
-    return dir + path;
-  }
-  return dir + '/' + path;
-}
 }  // namespace
 
 Historical::Historical(ILogReceiver* log_receiver, std::string key,
@@ -285,25 +275,25 @@ std::vector<databento::BatchFileDesc> Historical::BatchListFiles(
   return files;
 }
 
-std::vector<std::string> Historical::BatchDownload(
-    const std::string& output_dir, const std::string& job_id) {
+std::vector<std::filesystem::path> Historical::BatchDownload(
+    const std::filesystem::path& output_dir, const std::string& job_id) {
   TryCreateDir(output_dir);
-  const std::string job_dir = PathJoin(output_dir, job_id);
+  const std::filesystem::path job_dir = output_dir / job_id;
   TryCreateDir(job_dir);
   const auto file_descs = BatchListFiles(job_id);
-  std::vector<std::string> paths;
+  std::vector<std::filesystem::path> paths;
   for (const auto& file_desc : file_descs) {
-    std::string output_path = PathJoin(job_dir, file_desc.filename);
+    std::filesystem::path output_path = job_dir / file_desc.filename;
     DownloadFile(file_desc.https_url, output_path);
     paths.emplace_back(std::move(output_path));
   }
   return paths;
 }
-std::string Historical::BatchDownload(const std::string& output_dir,
-                                      const std::string& job_id,
-                                      const std::string& filename_to_download) {
+std::filesystem::path Historical::BatchDownload(
+    const std::filesystem::path& output_dir, const std::string& job_id,
+    const std::string& filename_to_download) {
   TryCreateDir(output_dir);
-  const std::string job_dir = PathJoin(output_dir, job_id);
+  const std::filesystem::path job_dir = output_dir / job_id;
   TryCreateDir(job_dir);
   const auto file_descs = BatchListFiles(job_id);
   const auto file_desc_it =
@@ -316,14 +306,14 @@ std::string Historical::BatchDownload(const std::string& output_dir,
                                "filename_to_download",
                                "Filename not found for batch job " + job_id};
   }
-  std::string output_path = PathJoin(job_dir, file_desc_it->filename);
+  std::filesystem::path output_path = job_dir / file_desc_it->filename;
   DownloadFile(file_desc_it->https_url, output_path);
   return output_path;
 }
 
 void Historical::StreamToFile(const std::string& url_path,
                               const HttplibParams& params,
-                              const std::string& file_path) {
+                              const std::filesystem::path& file_path) {
   OutFileStream out_file{file_path};
   this->client_.GetRawStream(
       url_path, params, [&out_file](const char* data, std::size_t length) {
@@ -333,7 +323,7 @@ void Historical::StreamToFile(const std::string& url_path,
 }
 
 void Historical::DownloadFile(const std::string& url,
-                              const std::string& output_path) {
+                              const std::filesystem::path& output_path) {
   static const std::string kEndpoint = "Historical::DownloadFile";
   // extract path from URL
   const auto protocol_divider = url.find("://");
@@ -870,7 +860,7 @@ static const std::string kTimeseriesGetRangeToFileEndpoint =
 databento::DbnFileStore Historical::TimeseriesGetRangeToFile(
     const std::string& dataset, const DateTimeRange<UnixNanos>& datetime_range,
     const std::vector<std::string>& symbols, Schema schema,
-    const std::string& file_path) {
+    const std::filesystem::path& file_path) {
   return this->TimeseriesGetRangeToFile(dataset, datetime_range, symbols,
                                         schema, kDefaultSTypeIn,
                                         kDefaultSTypeOut, {}, file_path);
@@ -879,15 +869,16 @@ databento::DbnFileStore Historical::TimeseriesGetRangeToFile(
     const std::string& dataset,
     const DateTimeRange<std::string>& datetime_range,
     const std::vector<std::string>& symbols, Schema schema,
-    const std::string& file_path) {
+    const std::filesystem::path& file_path) {
   return this->TimeseriesGetRangeToFile(dataset, datetime_range, symbols,
                                         schema, kDefaultSTypeIn,
                                         kDefaultSTypeOut, {}, file_path);
 }
 databento::DbnFileStore Historical::TimeseriesGetRangeToFile(
     const std::string& dataset, const DateTimeRange<UnixNanos>& datetime_range,
     const std::vector<std::string>& symbols, Schema schema, SType stype_in,
-    SType stype_out, std::uint64_t limit, const std::string& file_path) {
+    SType stype_out, std::uint64_t limit,
+    const std::filesystem::path& file_path) {
   httplib::Params params{
       {"dataset", dataset},
       {"encoding", "dbn"},
@@ -906,7 +897,8 @@ databento::DbnFileStore Historical::TimeseriesGetRangeToFile(
     const std::string& dataset,
     const DateTimeRange<std::string>& datetime_range,
     const std::vector<std::string>& symbols, Schema schema, SType stype_in,
-    SType stype_out, std::uint64_t limit, const std::string& file_path) {
+    SType stype_out, std::uint64_t limit,
+    const std::filesystem::path& file_path) {
   httplib::Params params{
       {"dataset", dataset},
       {"encoding", "dbn"},
@@ -922,7 +914,7 @@ databento::DbnFileStore Historical::TimeseriesGetRangeToFile(
   return this->TimeseriesGetRangeToFile(params, file_path);
 }
 databento::DbnFileStore Historical::TimeseriesGetRangeToFile(
-    const HttplibParams& params, const std::string& file_path) {
+    const HttplibParams& params, const std::filesystem::path& file_path) {
   StreamToFile(kTimeseriesGetRangePath, params, file_path);
   return DbnFileStore{log_receiver_, file_path,
                       VersionUpgradePolicy::UpgradeToV3};

tests/include/temp_file.hpp

@@ -16,14 +16,12 @@ namespace databento {
 // goes out of scope.
 class TempFile {
  public:
-  explicit TempFile(const std::filesystem::path& path)
-      : TempFile{path.string()} {}
-  explicit TempFile(std::string path) : path_{std::move(path)} {
+  explicit TempFile(std::filesystem::path path) : path_{std::move(path)} {
     std::ifstream f{path_};
     if (Exists()) {
       throw InvalidArgumentError{
           "TempFile::TempFile", "path",
-          "File at path " + path_ + " shouldn't already exist"};
+          "File at path " + path_.string() + " shouldn't already exist"};
     }
   }
   TempFile(const TempFile&) = delete;
@@ -36,14 +34,14 @@ class TempFile {
                       << ::strerror(errno);
   }
 
-  const std::string& Path() const { return path_; }
+  const std::filesystem::path& Path() const { return path_; }
 
   bool Exists() const {
     std::ifstream f{path_};
     return f.good();
   }
 
  private:
-  std::string path_;
+  std::filesystem::path path_;
 };
 }  // namespace databento

tests/src/historical_tests.cpp

@@ -244,19 +244,17 @@ TEST_F(HistoricalTests, TestBatchDownloadAll) {
                                static_cast<std::uint16_t>(port)};
   ASSERT_FALSE(temp_metadata_file.Exists());
   ASSERT_FALSE(temp_dbn_file.Exists());
-  const std::vector<std::string> paths =
-      target.BatchDownload(tmp_path_.string(), kJobId);
+  const std::vector<std::filesystem::path> paths =
+      target.BatchDownload(tmp_path_, kJobId);
   EXPECT_TRUE(temp_metadata_file.Exists());
   EXPECT_TRUE(temp_dbn_file.Exists());
   ASSERT_EQ(paths.size(), 2);
-  EXPECT_NE(
-      std::find_if(paths.begin(), paths.end(),
-                   [&temp_metadata_file](const auto& path) {
-                     return std::filesystem::path{path}.lexically_normal() ==
-                            std::filesystem::path{temp_metadata_file.Path()}
-                                .lexically_normal();
-                   }),
-      paths.end());
+  EXPECT_NE(std::find_if(paths.begin(), paths.end(),
+                         [&temp_metadata_file](const auto& path) {
+                           return path.lexically_normal() ==
+                                  temp_metadata_file.Path().lexically_normal();
+                         }),
+            paths.end());
   EXPECT_NE(
       std::find_if(paths.begin(), paths.end(),
                    [&temp_dbn_file](const auto& path) {
@@ -278,12 +276,11 @@ TEST_F(HistoricalTests, TestBatchDownloadSingle) {
   databento::Historical target{logger_.get(), kApiKey, "localhost",
                                static_cast<std::uint16_t>(port)};
   ASSERT_FALSE(temp_metadata_file.Exists());
-  const std::string path =
-      target.BatchDownload(tmp_path_.string(), kJobId, "test_metadata.json");
+  const std::filesystem::path path =
+      target.BatchDownload(tmp_path_, kJobId, "test_metadata.json");
   EXPECT_TRUE(temp_metadata_file.Exists());
-  EXPECT_EQ(
-      std::filesystem::path{path}.lexically_normal(),
-      std::filesystem::path{temp_metadata_file.Path()}.lexically_normal());
+  EXPECT_EQ(path.lexically_normal(),
+            temp_metadata_file.Path().lexically_normal());
 }
 
 TEST_F(HistoricalTests, TestBatchDownloadSingleInvalidFile) {
@@ -294,9 +291,8 @@ TEST_F(HistoricalTests, TestBatchDownloadSingleInvalidFile) {
 
   databento::Historical target{logger_.get(), kApiKey, "localhost",
                                static_cast<std::uint16_t>(port)};
-  ASSERT_THROW(
-      target.BatchDownload(tmp_path_.string(), kJobId, "test_metadata.js"),
-      InvalidArgumentError);
+  ASSERT_THROW(target.BatchDownload(tmp_path_, kJobId, "test_metadata.js"),
+               InvalidArgumentError);
 }
 
 TEST_F(HistoricalTests, TestMetadataListPublishers) {