5 files changed, 174 insertions, 58 deletions

diff --git a/src/database/include/database.hpp b/src/database/include/database.hpp
index 6cdaaca6..29872e8d 100644
--- a/src/database/include/database.hpp
+++ b/src/database/include/database.hpp
@@ -9,6 +9,7 @@
 #include <utility>
 #include <vector>
 
+#include "file_gatherer.hpp"
 #include "leveldb/cache.h"
 #include "leveldb/db.h"
 #include "leveldb/iterator.h"
@@ -17,6 +18,7 @@
 #include "records.hpp"
 #include "result.hpp"
 #include "song.hpp"
+#include "tag_parser.hpp"
 
 namespace database {
 
@@ -61,12 +63,15 @@ class Database {
     ALREADY_OPEN,
     FAILED_TO_OPEN,
   };
+  static auto Open(IFileGatherer* file_gatherer, ITagParser* tag_parser)
+      -> cpp::result<Database*, DatabaseError>;
   static auto Open() -> cpp::result<Database*, DatabaseError>;
 
+  static auto Destroy() -> void;
+
   ~Database();
 
   auto Update() -> std::future<void>;
-  auto Destroy() -> std::future<void>;
 
   auto GetSongs(std::size_t page_size) -> std::future<Result<Song>>;
   auto GetMoreSongs(std::size_t page_size, Continuation c)
@@ -80,10 +85,19 @@ class Database {
   Database& operator=(const Database&) = delete;
 
  private:
-  std::unique_ptr<leveldb::DB> db_;
-  std::unique_ptr<leveldb::Cache> cache_;
-
-  Database(leveldb::DB* db, leveldb::Cache* cache);
+  // Owned. Dumb pointers because destruction needs to be done in an explicit
+  // order.
+  leveldb::DB* db_;
+  leveldb::Cache* cache_;
+
+  // Not owned.
+  IFileGatherer* file_gatherer_;
+  ITagParser* tag_parser_;
+
+  Database(leveldb::DB* db,
+           leveldb::Cache* cache,
+           IFileGatherer* file_gatherer,
+           ITagParser* tag_parser);
 
   auto dbMintNewSongId() -> SongId;
   auto dbEntomb(SongId song, uint64_t hash) -> void;
diff --git a/src/database/include/file_gatherer.hpp b/src/database/include/file_gatherer.hpp
index 5df5a61b..bba4d0df 100644
--- a/src/database/include/file_gatherer.hpp
+++ b/src/database/include/file_gatherer.hpp
@@ -9,51 +9,20 @@
 
 namespace database {
 
-static_assert(sizeof(TCHAR) == sizeof(char), "TCHAR must be CHAR");
-
-template <typename Callback>
-auto FindFiles(const std::string& root, Callback cb) -> void {
-  std::deque<std::string> to_explore;
-  to_explore.push_back(root);
-
-  while (!to_explore.empty()) {
-    std::string next_path_str = to_explore.front();
-    const TCHAR* next_path = static_cast<const TCHAR*>(next_path_str.c_str());
-
-    FF_DIR dir;
-    FRESULT res = f_opendir(&dir, next_path);
-    if (res != FR_OK) {
-      // TODO: log.
-      continue;
-    }
-
-    for (;;) {
-      FILINFO info;
-      res = f_readdir(&dir, &info);
-      if (res != FR_OK || info.fname[0] == 0) {
-        // No more files in the directory.
-        break;
-      } else if (info.fattrib & (AM_HID | AM_SYS) || info.fname[0] == '.') {
-        // System or hidden file. Ignore it and move on.
-        continue;
-      } else {
-        std::stringstream full_path;
-        full_path << next_path_str << "/" << info.fname;
-
-        if (info.fattrib & AM_DIR) {
-          // This is a directory. Add it to the explore queue.
-          to_explore.push_back(full_path.str());
-        } else {
-          // This is a file! Let the callback know about it.
-          // std::invoke(cb, full_path.str(), info);
-          std::invoke(cb, full_path.str());
-        }
-      }
-    }
-
-    f_closedir(&dir);
-    to_explore.pop_front();
-  }
-}
+class IFileGatherer {
+ public:
+  virtual ~IFileGatherer(){};
+
+  virtual auto FindFiles(const std::string& root,
+                         std::function<void(const std::string&)> cb)
+      -> void = 0;
+};
+
+class FileGathererImpl : public IFileGatherer {
+ public:
+  virtual auto FindFiles(const std::string& root,
+                         std::function<void(const std::string&)> cb)
+      -> void override;
+};
 
 }  // namespace database
diff --git a/src/database/include/records.hpp b/src/database/include/records.hpp
index 22d2ca5b..371d8d58 100644
--- a/src/database/include/records.hpp
+++ b/src/database/include/records.hpp
@@ -1,14 +1,21 @@
 #pragma once
 
-#include <leveldb/db.h>
 #include <stdint.h>
+
 #include <string>
 
+#include "leveldb/db.h"
 #include "leveldb/slice.h"
+
 #include "song.hpp"
 
 namespace database {
 
+/*
+ * Helper class for creating leveldb Slices bundled with the data they point to.
+ * Slices are otherwise non-owning, which can make handling them post-creation
+ * difficult.
+ */
 class OwningSlice {
  public:
   std::string data;
@@ -17,16 +24,50 @@ class OwningSlice {
   explicit OwningSlice(std::string d);
 };
 
+/*
+ * Returns the prefix added to every SongData key. This can be used to iterate
+ * over every data record in the database.
+ */
 auto CreateDataPrefix() -> OwningSlice;
+
+/* Creates a data key for a song with the specified id. */
 auto CreateDataKey(const SongId& id) -> OwningSlice;
+
+/*
+ * Encodes a SongData instance into bytes, in preparation for storing it within
+ * the database. This encoding is consistent, and will remain stable over time.
+ */
 auto CreateDataValue(const SongData& song) -> OwningSlice;
+
+/*
+ * Parses bytes previously encoded via CreateDataValue back into a SongData. May
+ * return nullopt if parsing fails.
+ */
 auto ParseDataValue(const leveldb::Slice& slice) -> std::optional<SongData>;
 
+/* Creates a hash key for the specified hash. */
 auto CreateHashKey(const uint64_t& hash) -> OwningSlice;
-auto ParseHashValue(const leveldb::Slice&) -> std::optional<SongId>;
+
+/*
+ * Encodes a hash value (at this point just a song id) into bytes, in
+ * preparation for storing within the database. This encoding is consistent, and
+ * will remain stable over time.
+ */
 auto CreateHashValue(SongId id) -> OwningSlice;
 
+/*
+ * Parses bytes previously encoded via CreateHashValue back into a song id. May
+ * return nullopt if parsing fails.
+ */
+auto ParseHashValue(const leveldb::Slice&) -> std::optional<SongId>;
+
+/* Encodes a SongId as bytes. */
 auto SongIdToBytes(SongId id) -> OwningSlice;
-auto BytesToSongId(const std::string& bytes) -> SongId;
+
+/*
+ * Converts a song id encoded via SongIdToBytes back into a SongId. May return
+ * nullopt if parsing fails.
+ */
+auto BytesToSongId(const std::string& bytes) -> std::optional<SongId>;
 
 }  // namespace database
diff --git a/src/database/include/song.hpp b/src/database/include/song.hpp
index 12a7ef0c..e51e5587 100644
--- a/src/database/include/song.hpp
+++ b/src/database/include/song.hpp
@@ -1,7 +1,7 @@
 #pragma once
 
 #include <stdint.h>
-#include <cstdint>
+
 #include <optional>
 #include <string>
 
@@ -10,20 +10,68 @@
 
 namespace database {
 
+/*
+ * Uniquely describes a single song within the database. This value will be
+ * consistent across database updates, and should ideally (but is not guaranteed
+ * to) endure even across a song being removed and re-added.
+ *
+ * Four billion songs should be enough for anybody.
+ */
 typedef uint32_t SongId;
 
-enum Encoding { ENC_UNSUPPORTED, ENC_MP3 };
+/*
+ * Audio file encodings that we are aware of. Used to select an appropriate
+ * decoder at play time.
+ *
+ * Values of this enum are persisted in this database, so it is probably never a
+ * good idea to change the int representation of an existing value.
+ */
+enum class Encoding {
+  kUnsupported = 0,
+  kMp3 = 1,
+};
 
+/*
+ * Owning container for tag-related song metadata that was extracted from a
+ * file.
+ */
 struct SongTags {
   Encoding encoding;
   std::optional<std::string> title;
+
+  // TODO(jacqueline): It would be nice to use shared_ptr's for the artist and
+  // album, since there's likely a fair number of duplicates for each
+  // (especially the former).
+
   std::optional<std::string> artist;
   std::optional<std::string> album;
+
+  /*
+   * Returns a hash of the 'identifying' tags of this song. That is, a hash that
+   * can be used to determine if one song is likely the same as another, across
+   * things like re-encoding, re-mastering, or moving the underlying file.
+   */
   auto Hash() const -> uint64_t;
-};
 
-auto ReadAndParseTags(const std::string& path, SongTags* out) -> bool;
+  bool operator==(const SongTags&) const = default;
+};
 
+/*
+ * Immutable owning container for all of the metadata we store for a particular
+ * song. This includes two main kinds of metadata:
+ *  1. static(ish) attributes, such as the id, path on disk, hash of the tags
+ *  2. dynamic attributes, such as the number of times this song has been
+ *  played.
+ *
+ * Because a SongData is immutable, it is thread safe but will not reflect any
+ * changes to the dynamic attributes that may happen after it was obtained.
+ *
+ * Songs may be 'tombstoned'; this indicates that the song is no longer present
+ * at its previous location on disk, and we do not have any existing files with
+ * a matching tags_hash. When this is the case, we ignore this SongData for most
+ * purposes. We keep the entry in our database so that we can properly restore
+ * dynamic attributes (such as play count) if the song later re-appears on disk.
+ */
 class SongData {
  private:
   const SongId id_;
@@ -33,12 +81,14 @@ class SongData {
   const bool is_tombstoned_;
 
  public:
+  /* Constructor used when adding new songs to the database. */
   SongData(SongId id, const std::string& path, uint64_t hash)
       : id_(id),
         filepath_(path),
         tags_hash_(hash),
         play_count_(0),
         is_tombstoned_(false) {}
+
   SongData(SongId id,
            const std::string& path,
            uint64_t hash,
@@ -57,12 +107,30 @@ class SongData {
   auto is_tombstoned() const -> bool { return is_tombstoned_; }
 
   auto UpdateHash(uint64_t new_hash) const -> SongData;
+
+  /*
+   * Marks this song data as a 'tombstone'. Tombstoned songs are not playable,
+   * and should not generally be shown to users.
+   */
   auto Entomb() const -> SongData;
+
+  /*
+   * Clears the tombstone bit of this song, and updates the path to reflect its
+   * new location.
+   */
   auto Exhume(const std::string& new_path) const -> SongData;
 
   bool operator==(const SongData&) const = default;
 };
 
+/*
+ * Immutable and owning combination of a song's tags and metadata.
+ *
+ * Note that instances of this class may have a fairly large memory impact, due
+ * to the large number of strings they own. Prefer to query the database again
+ * (which has its own caching layer), rather than retaining Song instances for a
+ * long time.
+ */
 class Song {
  public:
   Song(SongData data, SongTags tags) : data_(data), tags_(tags) {}
@@ -70,6 +138,8 @@ class Song {
   auto data() -> const SongData& { return data_; }
   auto tags() -> const SongTags& { return tags_; }
 
+  bool operator==(const Song&) const = default;
+
  private:
   const SongData data_;
   const SongTags tags_;
diff --git a/src/database/include/tag_parser.hpp b/src/database/include/tag_parser.hpp
new file mode 100644
index 00000000..1aee94c7
--- /dev/null
+++ b/src/database/include/tag_parser.hpp
@@ -0,0 +1,22 @@
+#pragma once
+
+#include <string>
+
+#include "song.hpp"
+
+namespace database {
+
+class ITagParser {
+ public:
+  virtual ~ITagParser() {}
+  virtual auto ReadAndParseTags(const std::string& path, SongTags* out)
+      -> bool = 0;
+};
+
+class TagParserImpl : public ITagParser {
+ public:
+  virtual auto ReadAndParseTags(const std::string& path, SongTags* out)
+      -> bool override;
+};
+
+}  // namespace database