Merge pull request #294 from AdenKoperczak/location_markers_ids

Location markers ids

scwx-qt/source/scwx/qt/main/application.cpp

@@ -44,6 +44,14 @@ void WaitForInitialization()
    }
 }
 
+// Only use for test cases
+void ResetInitilization()
+{
+   logger_->debug("Application initialization reset");
+   std::unique_lock lock(initializationMutex_);
+   initialized_ = false;
+}
+
 } // namespace Application
 } // namespace main
 } // namespace qt

scwx-qt/source/scwx/qt/main/application.hpp

@@ -11,6 +11,8 @@ namespace Application
 
 void FinishInitialization();
 void WaitForInitialization();
+// Only use for test cases
+void ResetInitilization();
 
 } // namespace Application
 } // namespace main

scwx-qt/source/scwx/qt/manager/marker_manager.cpp

@@ -36,8 +36,10 @@ public:
    explicit Impl(MarkerManager* self) : self_ {self} {}
    ~Impl() { threadPool_.join(); }
 
-   std::string                                markerSettingsPath_ {};
+   std::string                                markerSettingsPath_ {""};
    std::vector<std::shared_ptr<MarkerRecord>> markerRecords_ {};
+   std::unordered_map<types::MarkerId, size_t> idToIndex_ {};
+
 
    MarkerManager* self_;
 
@@ -48,6 +50,10 @@ public:
    void                          ReadMarkerSettings();
    void                          WriteMarkerSettings();
    std::shared_ptr<MarkerRecord> GetMarkerByName(const std::string& name);
+
+   void InitalizeIds();
+   types::MarkerId NewId();
+   types::MarkerId lastId_;
 };
 
 class MarkerManager::Impl::MarkerRecord
@@ -88,6 +94,16 @@ public:
    }
 };
 
+void MarkerManager::Impl::InitalizeIds()
+{
+   lastId_ = 0;
+}
+
+types::MarkerId MarkerManager::Impl::NewId()
+{
+   return ++lastId_;
+}
+
 void MarkerManager::Impl::InitializeMarkerSettings()
 {
    std::string appDataPath {
@@ -109,6 +125,7 @@ void MarkerManager::Impl::InitializeMarkerSettings()
 void MarkerManager::Impl::ReadMarkerSettings()
 {
    logger_->info("Reading location marker settings");
+   InitalizeIds();
 
    boost::json::value markerJson = nullptr;
    {
@@ -125,6 +142,7 @@ void MarkerManager::Impl::ReadMarkerSettings()
          // For each marker entry
          auto& markerArray = markerJson.as_array();
          markerRecords_.reserve(markerArray.size());
+         idToIndex_.reserve(markerArray.size());
          for (auto& markerEntry : markerArray)
          {
             try
@@ -134,8 +152,12 @@ void MarkerManager::Impl::ReadMarkerSettings()
 
                if (!record.markerInfo_.name.empty())
                {
+                  types::MarkerId id    = NewId();
+                  size_t          index = markerRecords_.size();
+                  record.markerInfo_.id = id;
                   markerRecords_.emplace_back(
                      std::make_shared<MarkerRecord>(record.markerInfo_));
+                  idToIndex_.emplace(id, index);
                }
             }
             catch (const std::exception& ex)
@@ -176,14 +198,13 @@ MarkerManager::Impl::GetMarkerByName(const std::string& name)
 
 MarkerManager::MarkerManager() : p(std::make_unique<Impl>(this))
 {
+   p->InitializeMarkerSettings();
 
    boost::asio::post(p->threadPool_,
                      [this]()
                      {
                         try
                         {
-                           p->InitializeMarkerSettings();
-
                            // Read Marker settings on startup
                            main::Application::WaitForInitialization();
                            p->ReadMarkerSettings();
@@ -207,11 +228,17 @@ size_t MarkerManager::marker_count()
    return p->markerRecords_.size();
 }
 
-std::optional<types::MarkerInfo> MarkerManager::get_marker(size_t index)
+std::optional<types::MarkerInfo> MarkerManager::get_marker(types::MarkerId id)
 {
    std::shared_lock lock(p->markerRecordLock_);
+   if (!p->idToIndex_.contains(id))
+   {
+      return {};
+   }
+   size_t index = p->idToIndex_[id];
    if (index >= p->markerRecords_.size())
    {
+      logger_->warn("id in idToIndex_ but out of range!");
       return {};
    }
    std::shared_ptr<MarkerManager::Impl::MarkerRecord>& markerRecord =
@@ -219,45 +246,81 @@ std::optional<types::MarkerInfo> MarkerManager::get_marker(size_t index)
    return markerRecord->toMarkerInfo();
 }
 
-void MarkerManager::set_marker(size_t index, const types::MarkerInfo& marker)
+std::optional<size_t> MarkerManager::get_index(types::MarkerId id)
+{
+   std::shared_lock lock(p->markerRecordLock_);
+   if (!p->idToIndex_.contains(id))
+   {
+      return {};
+   }
+   return p->idToIndex_[id];
+}
+
+void MarkerManager::set_marker(types::MarkerId id, const types::MarkerInfo& marker)
 {
    {
       std::unique_lock lock(p->markerRecordLock_);
+      if (!p->idToIndex_.contains(id))
+      {
+         return;
+      }
+      size_t index = p->idToIndex_[id];
       if (index >= p->markerRecords_.size())
       {
+         logger_->warn("id in idToIndex_ but out of range!");
          return;
       }
       std::shared_ptr<MarkerManager::Impl::MarkerRecord>& markerRecord =
          p->markerRecords_[index];
       markerRecord->markerInfo_ = marker;
    }
-   Q_EMIT MarkerChanged(index);
+   Q_EMIT MarkerChanged(id);
    Q_EMIT MarkersUpdated();
 }
 
 void MarkerManager::add_marker(const types::MarkerInfo& marker)
 {
+   types::MarkerId id;
    {
       std::unique_lock lock(p->markerRecordLock_);
+      id = p->NewId();
+      size_t index = p->markerRecords_.size();
+      p->idToIndex_.emplace(id, index);
       p->markerRecords_.emplace_back(std::make_shared<Impl::MarkerRecord>(marker));
+      p->markerRecords_[index]->markerInfo_.id = id;
    }
-   Q_EMIT MarkerAdded();
+   Q_EMIT MarkerAdded(id);
    Q_EMIT MarkersUpdated();
 }
 
-void MarkerManager::remove_marker(size_t index)
+void MarkerManager::remove_marker(types::MarkerId id)
 {
    {
       std::unique_lock lock(p->markerRecordLock_);
+      if (!p->idToIndex_.contains(id))
+      {
+         return;
+      }
+      size_t index = p->idToIndex_[id];
       if (index >= p->markerRecords_.size())
       {
+         logger_->warn("id in idToIndex_ but out of range!");
          return;
       }
 
       p->markerRecords_.erase(std::next(p->markerRecords_.begin(), index));
+      p->idToIndex_.erase(id);
+
+      for (auto& pair : p->idToIndex_)
+      {
+         if (pair.second > index)
+         {
+            pair.second -= 1;
+         }
+      }
    }
 
-   Q_EMIT MarkerRemoved(index);
+   Q_EMIT MarkerRemoved(id);
    Q_EMIT MarkersUpdated();
 }
 
@@ -293,6 +356,22 @@ void MarkerManager::move_marker(size_t from, size_t to)
    Q_EMIT MarkersUpdated();
 }
 
+void MarkerManager::for_each(std::function<MarkerForEachFunc> func)
+{
+   std::shared_lock lock(p->markerRecordLock_);
+   for (auto marker : p->markerRecords_)
+   {
+      func(marker->markerInfo_);
+   }
+}
+
+// Only use for testing
+void MarkerManager::set_marker_settings_path(const std::string& path)
+{
+   p->markerSettingsPath_ = path;
+}
+
+
 std::shared_ptr<MarkerManager> MarkerManager::Instance()
 {
    static std::weak_ptr<MarkerManager> markerManagerReference_ {};

scwx-qt/source/scwx/qt/manager/marker_manager.hpp

@@ -12,6 +12,7 @@ namespace qt
 namespace manager
 {
 
+typedef void MarkerForEachFunc(const types::MarkerInfo&);
 class MarkerManager : public QObject
 {
    Q_OBJECT
@@ -20,21 +21,27 @@ public:
    explicit MarkerManager();
    ~MarkerManager();
 
-   size_t                   marker_count();
+   size_t                           marker_count();
-   std::optional<types::MarkerInfo> get_marker(size_t index);
+   std::optional<types::MarkerInfo> get_marker(types::MarkerId id);
-   void set_marker(size_t index, const types::MarkerInfo& marker);
+   std::optional<size_t> get_index(types::MarkerId id);
+   void set_marker(types::MarkerId id, const types::MarkerInfo& marker);
    void add_marker(const types::MarkerInfo& marker);
-   void remove_marker(size_t index);
+   void remove_marker(types::MarkerId id);
    void move_marker(size_t from, size_t to);
 
+   void for_each(std::function<MarkerForEachFunc> func);
+
+   // Only use for testing
+   void set_marker_settings_path(const std::string& path);
+
    static std::shared_ptr<MarkerManager> Instance();
 
 signals:
    void MarkersInitialized(size_t count);
    void MarkersUpdated();
-   void MarkerChanged(size_t index);
+   void MarkerChanged(types::MarkerId id);
-   void MarkerAdded();
+   void MarkerAdded(types::MarkerId id);
-   void MarkerRemoved(size_t index);
+   void MarkerRemoved(types::MarkerId id);
 
 private:
    class Impl;

scwx-qt/source/scwx/qt/map/marker_layer.cpp

@@ -55,17 +55,13 @@ void MarkerLayer::Impl::ReloadMarkers()
 
    geoIcons_->StartIcons();
 
-   for (size_t i = 0; i < markerManager->marker_count(); i++)
+   markerManager->for_each(
-   {
+      [this](const types::MarkerInfo& marker)
-      std::optional<types::MarkerInfo> marker = markerManager->get_marker(i);
-      if (!marker)
       {
-         break;
+         std::shared_ptr<gl::draw::GeoIconDrawItem> icon = geoIcons_->AddIcon();
-      }
+         geoIcons_->SetIconTexture(icon, markerIconName_, 0);
-      std::shared_ptr<gl::draw::GeoIconDrawItem> icon = geoIcons_->AddIcon();
+         geoIcons_->SetIconLocation(icon, marker.latitude, marker.longitude);
-      geoIcons_->SetIconTexture(icon, markerIconName_, 0);
+      });
-      geoIcons_->SetIconLocation(icon, marker->latitude, marker->longitude);
-   }
 
    geoIcons_->FinishIcons();
    Q_EMIT self_->NeedsRendering();

scwx-qt/source/scwx/qt/model/marker_model.cpp

@@ -5,6 +5,8 @@
 #include <scwx/qt/types/qt_types.hpp>
 #include <scwx/util/logger.hpp>
 
+#include <vector>
+
 #include <QApplication>
 
 namespace scwx
@@ -30,6 +32,7 @@ public:
    ~Impl() = default;
    std::shared_ptr<manager::MarkerManager> markerManager_ {
       manager::MarkerManager::Instance()};
+   std::vector<types::MarkerId> markerIds_;
 };
 
 MarkerModel::MarkerModel(QObject* parent) :
@@ -63,7 +66,7 @@ int MarkerModel::rowCount(const QModelIndex& parent) const
 {
    return parent.isValid() ?
              0 :
-             static_cast<int>(p->markerManager_->marker_count());
+             static_cast<int>(p->markerIds_.size());
 }
 
 int MarkerModel::columnCount(const QModelIndex& parent) const
@@ -95,15 +98,19 @@ QVariant MarkerModel::data(const QModelIndex& index, int role) const
    static const char COORDINATE_FORMAT    = 'g';
    static const int  COORDINATE_PRECISION = 10;
 
-   if (!index.isValid() || index.row() < 0)
+   if (!index.isValid() || index.row() < 0 ||
+       static_cast<size_t>(index.row()) >= p->markerIds_.size())
    {
+      logger_->debug("Failed to get data index {}", index.row());
       return QVariant();
    }
 
+   types::MarkerId id = p->markerIds_[index.row()];
    std::optional<types::MarkerInfo> markerInfo =
-      p->markerManager_->get_marker(index.row());
+      p->markerManager_->get_marker(id);
    if (!markerInfo)
    {
+      logger_->debug("Failed to get data index {} id {}", index.row(), id);
       return QVariant();
    }
 
@@ -154,6 +161,16 @@ QVariant MarkerModel::data(const QModelIndex& index, int role) const
    return QVariant();
 }
 
+std::optional<types::MarkerId> MarkerModel::getId(int index)
+{
+   if (index < 0 || static_cast<size_t>(index) >= p->markerIds_.size())
+   {
+      return {};
+   }
+
+   return p->markerIds_[index];
+}
+
 QVariant MarkerModel::headerData(int             section,
                                  Qt::Orientation orientation,
                                  int             role) const
@@ -186,12 +203,16 @@ bool MarkerModel::setData(const QModelIndex& index,
                           const QVariant&    value,
                           int                role)
 {
-   if (!index.isValid() || index.row() < 0)
+
+   if (!index.isValid() || index.row() < 0 ||
+       static_cast<size_t>(index.row()) >= p->markerIds_.size())
    {
       return false;
    }
+
+   types::MarkerId id = p->markerIds_[index.row()];
    std::optional<types::MarkerInfo> markerInfo =
-      p->markerManager_->get_marker(index.row());
+      p->markerManager_->get_marker(id);
    if (!markerInfo)
    {
       return false;
@@ -205,7 +226,7 @@ bool MarkerModel::setData(const QModelIndex& index,
       {
          QString str = value.toString();
          markerInfo->name = str.toStdString();
-         p->markerManager_->set_marker(index.row(), *markerInfo);
+         p->markerManager_->set_marker(id, *markerInfo);
          result = true;
       }
       break;
@@ -219,7 +240,7 @@ bool MarkerModel::setData(const QModelIndex& index,
          if (!str.isEmpty() && ok && -90 <= latitude && latitude <= 90)
          {
             markerInfo->latitude = latitude;
-            p->markerManager_->set_marker(index.row(), *markerInfo);
+            p->markerManager_->set_marker(id, *markerInfo);
             result = true;
          }
       }
@@ -234,7 +255,7 @@ bool MarkerModel::setData(const QModelIndex& index,
          if (!str.isEmpty() && ok && -180 <= longitude && longitude <= 180)
          {
             markerInfo->longitude = longitude;
-            p->markerManager_->set_marker(index.row(), *markerInfo);
+            p->markerManager_->set_marker(id, *markerInfo);
             result = true;
          }
       }
@@ -254,34 +275,64 @@ bool MarkerModel::setData(const QModelIndex& index,
 
 void MarkerModel::HandleMarkersInitialized(size_t count)
 {
+   if (count == 0)
+   {
+      return;
+   }
    const int index = static_cast<int>(count - 1);
 
+   p->markerIds_.reserve(count);
    beginInsertRows(QModelIndex(), 0, index);
+   p->markerManager_->for_each(
+      [this](const types::MarkerInfo& info)
+      {
+         p->markerIds_.push_back(info.id);
+      });
    endInsertRows();
 }
 
-void MarkerModel::HandleMarkerAdded()
+void MarkerModel::HandleMarkerAdded(types::MarkerId id)
 {
-   const int newIndex = static_cast<int>(p->markerManager_->marker_count() - 1);
+   std::optional<size_t> index = p->markerManager_->get_index(id);
+   if (!index)
+   {
+      return;
+   }
+   const int newIndex = static_cast<int>(*index);
 
    beginInsertRows(QModelIndex(), newIndex, newIndex);
+   auto it = std::next(p->markerIds_.begin(), newIndex);
+   p->markerIds_.emplace(it, id);
    endInsertRows();
 }
 
-void MarkerModel::HandleMarkerChanged(size_t index)
+void MarkerModel::HandleMarkerChanged(types::MarkerId id)
 {
-   const int changedIndex = static_cast<int>(index);
+   auto it = std::find(p->markerIds_.begin(), p->markerIds_.end(), id);
+   if (it == p->markerIds_.end())
+   {
+      return;
+   }
+   const int changedIndex = std::distance(p->markerIds_.begin(), it);
+
    QModelIndex topLeft = createIndex(changedIndex, kFirstColumn);
    QModelIndex bottomRight = createIndex(changedIndex, kLastColumn);
 
    Q_EMIT dataChanged(topLeft, bottomRight);
 }
 
-void MarkerModel::HandleMarkerRemoved(size_t index)
+void MarkerModel::HandleMarkerRemoved(types::MarkerId id)
 {
-   const int removedIndex = static_cast<int>(index);
+   auto it = std::find(p->markerIds_.begin(), p->markerIds_.end(), id);
+   if (it == p->markerIds_.end())
+   {
+      return;
+   }
+
+   const int removedIndex = std::distance(p->markerIds_.begin(), it);
 
    beginRemoveRows(QModelIndex(), removedIndex, removedIndex);
+   p->markerIds_.erase(it);
    endRemoveRows();
 }
 

scwx-qt/source/scwx/qt/model/marker_model.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <QAbstractTableModel>
+#include <scwx/qt/types/marker_types.hpp>
 
 namespace scwx
 {
@@ -37,12 +38,13 @@ public:
                 const QVariant&    value,
                 int                role = Qt::EditRole) override;
 
+   std::optional<types::MarkerId> getId(int index);
 
 public slots:
    void HandleMarkersInitialized(size_t count);
-   void HandleMarkerAdded();
+   void HandleMarkerAdded(types::MarkerId id);
-   void HandleMarkerChanged(size_t index);
+   void HandleMarkerChanged(types::MarkerId id);
-   void HandleMarkerRemoved(size_t index);
+   void HandleMarkerRemoved(types::MarkerId id);
 
 private:
    class Impl;

scwx-qt/source/scwx/qt/types/marker_types.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <string>
+#include <cstdint>
 
 namespace scwx
 {
@@ -8,6 +9,7 @@ namespace qt
 {
 namespace types
 {
+typedef std::uint64_t MarkerId;
 
 struct MarkerInfo
 {
@@ -16,6 +18,7 @@ struct MarkerInfo
    {
    }
 
+   MarkerId    id;
    std::string name;
    double      latitude;
    double      longitude;

scwx-qt/source/scwx/qt/ui/marker_settings_widget.cpp

@@ -65,21 +65,25 @@ void MarkerSettingsWidgetImpl::ConnectSignals()
                     {
                        markerManager_->add_marker(types::MarkerInfo("", 0, 0));
                     });
-   QObject::connect(self_->ui->removeButton,
+   QObject::connect(
-                    &QPushButton::clicked,
+      self_->ui->removeButton,
-                    self_,
+      &QPushButton::clicked,
-                    [this]()
+      self_,
-                    {
+      [this]()
-                       auto selectionModel =
+      {
-                          self_->ui->markerView->selectionModel();
+         auto        selectionModel = self_->ui->markerView->selectionModel();
-                       QModelIndex selected =
+         QModelIndex selected       = selectionModel
-                          selectionModel
+                                   ->selectedRows(static_cast<int>(
-                             ->selectedRows(static_cast<int>(
+                                      model::MarkerModel::Column::Name))
-                                model::MarkerModel::Column::Name))
+                                   .first();
-                             .first();
+         std::optional<types::MarkerId> id = markerModel_->getId(selected.row());
+         if (!id)
+         {
+            return;
+         }
 
-                       markerManager_->remove_marker(selected.row());
+         markerManager_->remove_marker(*id);
-                    });
+      });
    QObject::connect(
       self_->ui->markerView->selectionModel(),
       &QItemSelectionModel::selectionChanged,

test/source/scwx/qt/model/marker_model.test.cpp

@@ -0,0 +1,240 @@
+#include <scwx/qt/model/marker_model.hpp>
+#include <scwx/qt/manager/marker_manager.hpp>
+#include <scwx/qt/main/application.hpp>
+
+#include <filesystem>
+#include <fstream>
+#include <QObject>
+
+#include <condition_variable>
+#include <gtest/gtest.h>
+
+
+namespace scwx
+{
+namespace qt
+{
+namespace model
+{
+
+static const std::string EMPTY_MARKERS_FILE =
+   std::string(SCWX_TEST_DATA_DIR) + "/json/markers/markers-empty.json";
+static const std::string TEMP_MARKERS_FILE =
+   std::string(SCWX_TEST_DATA_DIR) + "/json/markers/markers-temp.json";
+static const std::string ONE_MARKERS_FILE =
+   std::string(SCWX_TEST_DATA_DIR) + "/json/markers/markers-one.json";
+static const std::string FIVE_MARKERS_FILE =
+   std::string(SCWX_TEST_DATA_DIR) + "/json/markers/markers-five.json";
+
+static std::mutex              initializedMutex {};
+static std::condition_variable initializedCond {};
+static bool                    initialized;
+
+void CompareFiles(const std::string& file1, const std::string& file2)
+{
+   std::ifstream     ifs1 {file1};
+   std::stringstream buffer1;
+   buffer1 << ifs1.rdbuf();
+
+   std::ifstream     ifs2 {file2};
+   std::stringstream buffer2;
+   buffer2 << ifs2.rdbuf();
+
+   EXPECT_EQ(buffer1.str(), buffer2.str());
+}
+
+void CopyFile(const std::string& from, const std::string& to)
+{
+   std::filesystem::copy_file(from, to);
+   CompareFiles(from, to);
+}
+
+typedef void TestFunction(std::shared_ptr<manager::MarkerManager> manager,
+                          MarkerModel&                            model);
+
+void RunTest(const std::string& filename, TestFunction testFunction)
+{
+   {
+      main::Application::ResetInitilization();
+      MarkerModel                             model = MarkerModel();
+      std::shared_ptr<manager::MarkerManager> manager =
+         manager::MarkerManager::Instance();
+
+      manager->set_marker_settings_path(TEMP_MARKERS_FILE);
+
+      initialized = false;
+      QObject::connect(manager.get(),
+                       &manager::MarkerManager::MarkersInitialized,
+                       [](size_t count)
+                       {
+                          std::unique_lock lock(initializedMutex);
+                          initialized = true;
+                          initializedCond.notify_all();
+                       });
+
+      main::Application::FinishInitialization();
+
+      std::unique_lock lock(initializedMutex);
+      while (!initialized)
+      {
+         initializedCond.wait(lock);
+      }
+      // This is not jank
+      model.HandleMarkersInitialized(manager->marker_count());
+
+      testFunction(manager, model);
+   }
+
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), true);
+
+   CompareFiles(TEMP_MARKERS_FILE, filename);
+}
+
+TEST(MarkerModelTest, CreateJson)
+{
+   // Verify file doesn't exist prior to test start
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+
+   RunTest(EMPTY_MARKERS_FILE,
+           [](std::shared_ptr<manager::MarkerManager>, MarkerModel&) {});
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+TEST(MarkerModelTest, LoadEmpty)
+{
+   CopyFile(EMPTY_MARKERS_FILE, TEMP_MARKERS_FILE);
+
+   RunTest(EMPTY_MARKERS_FILE,
+           [](std::shared_ptr<manager::MarkerManager>, MarkerModel&) {});
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+TEST(MarkerModelTest, AddRemove)
+{
+   CopyFile(EMPTY_MARKERS_FILE, TEMP_MARKERS_FILE);
+
+   RunTest(ONE_MARKERS_FILE,
+           [](std::shared_ptr<manager::MarkerManager> manager, MarkerModel&)
+           { manager->add_marker(types::MarkerInfo("Null", 0, 0)); });
+   RunTest(
+      EMPTY_MARKERS_FILE,
+      [](std::shared_ptr<manager::MarkerManager> manager, MarkerModel& model)
+      {
+         std::optional<types::MarkerId> id = model.getId(0);
+         EXPECT_TRUE(id);
+         if (id)
+         {
+            manager->remove_marker(*id);
+         }
+      });
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+TEST(MarkerModelTest, AddFive)
+{
+   CopyFile(EMPTY_MARKERS_FILE, TEMP_MARKERS_FILE);
+
+   RunTest(FIVE_MARKERS_FILE,
+           [](std::shared_ptr<manager::MarkerManager> manager, MarkerModel&)
+           {
+              manager->add_marker(types::MarkerInfo("Null", 0, 0));
+              manager->add_marker(types::MarkerInfo("North", 90, 0));
+              manager->add_marker(types::MarkerInfo("South", -90, 0));
+              manager->add_marker(types::MarkerInfo("East", 0, 90));
+              manager->add_marker(types::MarkerInfo("West", 0, -90));
+           });
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+TEST(MarkerModelTest, AddFour)
+{
+   CopyFile(ONE_MARKERS_FILE, TEMP_MARKERS_FILE);
+
+   RunTest(FIVE_MARKERS_FILE,
+           [](std::shared_ptr<manager::MarkerManager> manager, MarkerModel&)
+           {
+              manager->add_marker(types::MarkerInfo("North", 90, 0));
+              manager->add_marker(types::MarkerInfo("South", -90, 0));
+              manager->add_marker(types::MarkerInfo("East", 0, 90));
+              manager->add_marker(types::MarkerInfo("West", 0, -90));
+           });
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+TEST(MarkerModelTest, RemoveFive)
+{
+   CopyFile(FIVE_MARKERS_FILE, TEMP_MARKERS_FILE);
+
+   RunTest(
+      EMPTY_MARKERS_FILE,
+      [](std::shared_ptr<manager::MarkerManager> manager, MarkerModel& model)
+      {
+         std::optional<types::MarkerId> id;
+         id = model.getId(4);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(3);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(2);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(1);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(0);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+      });
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+TEST(MarkerModelTest, RemoveFour)
+{
+   CopyFile(FIVE_MARKERS_FILE, TEMP_MARKERS_FILE);
+
+   RunTest(
+      ONE_MARKERS_FILE,
+      [](std::shared_ptr<manager::MarkerManager> manager, MarkerModel& model)
+      {
+         std::optional<types::MarkerId> id;
+         id = model.getId(4);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(3);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(2);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+
+         id = model.getId(1);
+         EXPECT_TRUE(id);
+         manager->remove_marker(*id);
+      });
+
+   std::filesystem::remove(TEMP_MARKERS_FILE);
+   EXPECT_EQ(std::filesystem::exists(TEMP_MARKERS_FILE), false);
+}
+
+} // namespace model
+} // namespace qt
+} // namespace scwx

test/test.cmake

@@ -25,7 +25,8 @@ set(SRC_QT_CONFIG_TESTS source/scwx/qt/config/county_database.test.cpp
 set(SRC_QT_MANAGER_TESTS source/scwx/qt/manager/settings_manager.test.cpp
                          source/scwx/qt/manager/update_manager.test.cpp)
 set(SRC_QT_MAP_TESTS source/scwx/qt/map/map_provider.test.cpp)
-set(SRC_QT_MODEL_TESTS source/scwx/qt/model/imgui_context_model.test.cpp)
+set(SRC_QT_MODEL_TESTS source/scwx/qt/model/imgui_context_model.test.cpp
+                       source/scwx/qt/model/marker_model.test.cpp)
 set(SRC_QT_SETTINGS_TESTS source/scwx/qt/settings/settings_container.test.cpp
                           source/scwx/qt/settings/settings_variable.test.cpp)
 set(SRC_QT_UTIL_TESTS source/scwx/qt/util/q_file_input_stream.test.cpp