refs #720, adjusted model loader

* callback function to data consolidation (so I can refer to consolidation in BlackCore)
* use status message in parser
* avoid duplicate keys (WOAI has ambiguous model strings which are to be excluded)

src/blackmisc/simulation/aircraftmodelloader.cpp

@@ -7,11 +7,12 @@
  * contained in the LICENSE file.
  */
 
-#include "blackmisc/compare.h"
 #include "blackmisc/simulation/aircraftmodelloader.h"
 #include "blackmisc/simulation/fscommon/aircraftcfgparser.h"
 #include "blackmisc/simulation/xplane/aircraftmodelloaderxplane.h"
 #include "blackmisc/simulation/xplane/xplaneutil.h"
+#include "blackmisc/compare.h"
+#include "blackmisc/logmessage.h"
 
 #include <QDir>
 #include <Qt>
@@ -72,8 +73,15 @@ namespace BlackMisc
 
         void IAircraftModelLoader::ps_loadFinished(bool success)
         {
-            Q_UNUSED(success);
             this->m_loadingInProgress = false;
+            if (this->m_loadingMessages.hasWarningOrErrorMessages())
+            {
+                CLogMessage::preformatted(this->m_loadingMessages);
+            }
+            else
+            {
+                CLogMessage(this).info("Loading finished, success %1") << boolToYesNo(success);
+            }
         }
 
         QStringList IAircraftModelLoader::getModelDirectoriesOrDefault() const
@@ -113,7 +121,7 @@ namespace BlackMisc
             return this->setCachedModels(CAircraftModelList());
         }
 
-        void IAircraftModelLoader::startLoading(LoadMode mode, const CAircraftModelList &dbModels)
+        void IAircraftModelLoader::startLoading(LoadMode mode, const ModelConsolidation &modelConsolidation)
         {
             if (this->m_loadingInProgress) { return; }
             this->m_loadingInProgress = true;
@@ -138,7 +146,7 @@ namespace BlackMisc
                 emit loadingFinished(false, this->getSimulator());
                 return;
             }
-            this->startLoadingFromDisk(mode, dbModels);
+            this->startLoadingFromDisk(mode, modelConsolidation);
         }
 
         const CSimulatorInfo IAircraftModelLoader::getSimulator() const

src/blackmisc/simulation/aircraftmodelloader.h

@@ -28,6 +28,7 @@
 #include <QStringList>
 #include <atomic>
 #include <memory>
+#include <functional>
 
 namespace BlackMisc
 {
@@ -55,7 +56,7 @@ namespace BlackMisc
             {
                 NotSet                = 0,
                 LoadDirectly          = 1 << 0,   //!< load syncronously (blocking), normally for testing
-                LoadInBackground      = 1 << 1,   //!< load in background, asnycronously
+                LoadInBackground      = 1 << 1,   //!< load in background, asyncronously
                 CacheUntilNewer       = 1 << 2,   //!< use cache until newer data re available
                 CacheFirst            = 1 << 3,   //!< always use cache (if it has data)
                 CacheSkipped          = 1 << 4,   //!< ignore cache
@@ -68,9 +69,13 @@ namespace BlackMisc
             //! Destructor
             virtual ~IAircraftModelLoader();
 
+            //! Callback to consolidate data, normally with DB data
+            //! \remark this has to be a abstarct, as DB handling is subject of BlackCore
+            using ModelConsolidation = std::function<int (BlackMisc::Simulation::CAircraftModelList &, bool)>;
+
             //! Start the loading process from disk.
             //! Optional DB models can be passed and used for data consolidation.
-            void startLoading(LoadMode mode = InBackgroundWithCache, const CAircraftModelList &dbModels = {});
+            void startLoading(LoadMode mode = InBackgroundWithCache, const ModelConsolidation &modelConsolidation = {});
 
             //! Loading finished?
             virtual bool isLoadingFinished() const = 0;
@@ -123,10 +128,12 @@ namespace BlackMisc
         public slots:
             //! Set cache from outside, this should only be used in special cases.
             //! But it allows to modify data elsewhere and update the cache with manipulated data.
+            //! Normally used to consoidate data with DB data and write them back
             BlackMisc::CStatusMessage setCachedModels(const CAircraftModelList &models, const CSimulatorInfo &simulator = CSimulatorInfo());
 
             //! Set cache from outside, this should only be used in special cases.
             //! But it allows to modify data elsewhere and update the cache with manipulated data.
+            //! Normally used to consoidate data with DB data and write them back
             BlackMisc::CStatusMessage replaceOrAddCachedModels(const CAircraftModelList &models, const CSimulatorInfo &simulator = CSimulatorInfo());
 
         signals:
@@ -150,20 +157,20 @@ namespace BlackMisc
             bool existsDir(const QString &directory) const;
 
             //! Start the loading process from disk
-            virtual void startLoadingFromDisk(LoadMode mode, const BlackMisc::Simulation::CAircraftModelList &dbModels) = 0;
+            virtual void startLoadingFromDisk(LoadMode mode, const ModelConsolidation &modelConsolidation) = 0;
 
-            std::atomic<bool> m_cancelLoading { false };                                           //!< flag
+            std::atomic<bool> m_cancelLoading { false };                        //!< flag
-            std::atomic<bool> m_loadingInProgress { false };                                       //!< Loading in progress
+            std::atomic<bool> m_loadingInProgress { false };                    //!< Loading in progress
-            BlackMisc::Simulation::Data::CModelCaches m_caches { this };                           //!< caches
+            BlackMisc::Simulation::Data::CModelCaches m_caches { this };        //!< caches
             BlackMisc::Simulation::CMultiSimulatorSettings m_settings { this }; //!< settings
+            BlackMisc::CStatusMessageList m_loadingMessages;                    //!< loading messages
 
         protected slots:
-            //! Loading finished
+            //! Loading finished, also logs messages
             void ps_loadFinished(bool success);
         };
-
+    } // ns
-    } // namespace
+} // ns
-} // namespace
 
 Q_DECLARE_METATYPE(BlackMisc::Simulation::IAircraftModelLoader::LoadMode)
 Q_DECLARE_METATYPE(BlackMisc::Simulation::IAircraftModelLoader::LoadModeFlag)

src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.cpp

@@ -11,6 +11,7 @@
 #include "blackmisc/range.h"
 #include "blackmisc/simulation/aircraftmodel.h"
 #include "blackmisc/simulation/fscommon/aircraftcfgentrieslist.h"
+#include "blackmisc/statusmessagelist.h"
 
 using namespace BlackMisc;
 using namespace BlackMisc::Simulation;
@@ -26,7 +27,7 @@ namespace BlackMisc
             {
                 if (title.isEmpty()) { return false; }
                 return this->containsBy(
-                           [ = ](const CAircraftCfgEntries & entries) { return title.compare(entries.getTitle(), caseSensitivity) == 0; }
+                [ = ](const CAircraftCfgEntries & entries) { return title.compare(entries.getTitle(), caseSensitivity) == 0; }
                        );
             }
 
@@ -57,25 +58,35 @@ namespace BlackMisc
                 return titles;
             }
 
-            CAircraftModelList CAircraftCfgEntriesList::toAircraftModelList() const
+            CAircraftModelList CAircraftCfgEntriesList::toAircraftModelList(bool ignoreDuplicatesAndEmptyModelStrings, CStatusMessageList &msgs) const
             {
                 CAircraftModelList ml;
+                QSet<QString> keys;
                 for (const CAircraftCfgEntries &entries : (*this))
                 {
+                    if (ignoreDuplicatesAndEmptyModelStrings)
+                    {
+                        const QString key = entries.getTitle().toUpper();
+                        if (key.isEmpty()) { continue; }
+                        if (keys.contains(key))
+                        {
+                            CStatusMessage msg(this);
+                            msg.warning("Duplicate model string %1 in %2 %3")
+                                    << entries.getTitle() << entries.getFileDirectory() << entries.getFileName();
+                            msgs.push_back(msg);
+                            continue;
+                        }
+                        keys.insert(key);
+                    }
                     ml.push_back(entries.toAircraftModel());
                 }
                 return ml;
             }
 
-            CAircraftModelList CAircraftCfgEntriesList::toAircraftModelList(const CSimulatorInfo &simInfo) const
+            CAircraftModelList CAircraftCfgEntriesList::toAircraftModelList(const CSimulatorInfo &simInfo, bool ignoreDuplicatesAndEmptyModelStrings, CStatusMessageList &msgs) const
             {
-                CAircraftModelList ml;
+                CAircraftModelList ml = this->toAircraftModelList(ignoreDuplicatesAndEmptyModelStrings, msgs);
-                for (const CAircraftCfgEntries &entries : (*this))
+                ml.setSimulatorInfo(simInfo);
-                {
-                    CAircraftModel m(entries.toAircraftModel());
-                    m.setSimulator(simInfo);
-                    ml.push_back(m);
-                }
                 return ml;
             }
 

src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.h

@@ -53,10 +53,10 @@ namespace BlackMisc
                 QStringList getTitles(bool sorted = false) const;
 
                 //! As aircraft models
-                BlackMisc::Simulation::CAircraftModelList toAircraftModelList() const;
+                BlackMisc::Simulation::CAircraftModelList toAircraftModelList(bool ignoreDuplicatesAndEmptyModelStrings, CStatusMessageList &msgs) const;
 
                 //! As aircraft models for simulator
-                BlackMisc::Simulation::CAircraftModelList toAircraftModelList(const BlackMisc::Simulation::CSimulatorInfo &simInfo) const;
+                BlackMisc::Simulation::CAircraftModelList toAircraftModelList(const BlackMisc::Simulation::CSimulatorInfo &simInfo, bool ignoreDuplicatesAndEmptyModelStrings, CStatusMessageList &msgs) const;
 
                 //! Ambiguous titles
                 QStringList detectAmbiguousTitles() const;

src/blackmisc/simulation/fscommon/aircraftcfgparser.cpp

@@ -7,13 +7,14 @@
  * contained in the LICENSE file.
  */
 
-#include "blackmisc/fileutils.h"
+#include "blackcore/db/databaseutils.h"
-#include "blackmisc/logmessage.h"
 #include "blackmisc/simulation/aircraftmodelutils.h"
 #include "blackmisc/simulation/fscommon/aircraftcfgentries.h"
 #include "blackmisc/simulation/fscommon/aircraftcfgparser.h"
 #include "blackmisc/simulation/fscommon/fscommonutil.h"
-#include "blackmisc/statusmessage.h"
+#include "blackmisc/fileutils.h"
+#include "blackmisc/logmessage.h"
+#include "blackmisc/statusmessagelist.h"
 #include "blackmisc/worker.h"
 
 #include <QDateTime>
@@ -36,6 +37,7 @@ using namespace BlackMisc;
 using namespace BlackMisc::Simulation;
 using namespace BlackMisc::Simulation::FsCommon;
 using namespace BlackMisc::Network;
+using namespace BlackCore::Db;
 
 namespace BlackMisc
 {
@@ -44,7 +46,7 @@ namespace BlackMisc
         namespace FsCommon
         {
             // response for async. loading
-            using LoaderResponse = std::tuple<CAircraftCfgEntriesList, CAircraftModelList, bool>;
+            using LoaderResponse = std::tuple<CAircraftCfgEntriesList, CAircraftModelList, BlackMisc::CStatusMessageList, bool>;
 
             CAircraftCfgParser::CAircraftCfgParser(const CSimulatorInfo &simInfo) :
                 IAircraftModelLoader(simInfo)
@@ -61,7 +63,7 @@ namespace BlackMisc
                 if (this->m_parserWorker) { this->m_parserWorker->waitForFinished(); }
             }
 
-            void CAircraftCfgParser::startLoadingFromDisk(LoadMode mode, const CAircraftModelList &dbModels)
+            void CAircraftCfgParser::startLoadingFromDisk(LoadMode mode, const ModelConsolidation &modelConsolidation)
             {
                 const CSimulatorInfo simulator = this->getSimulator();
                 const QString modelDirectory(this->m_settings.getFirstModelDirectoryOrDefault(simulator)); // expect only one directory
@@ -71,21 +73,23 @@ namespace BlackMisc
                 {
                     if (m_parserWorker && !m_parserWorker->isFinished()) { return; }
                     m_parserWorker = BlackMisc::CWorker::fromTask(this, "CAircraftCfgParser::changeDirectory",
-                                     [this, modelDirectory, excludedDirectoryPatterns, simulator, dbModels]()
+                                     [this, modelDirectory, excludedDirectoryPatterns, simulator, modelConsolidation]()
                     {
                         bool ok = false;
-                        const auto aircraftCfgEntriesList = this->performParsing(modelDirectory, excludedDirectoryPatterns, &ok);
+                        CStatusMessageList msgs;
+                        const auto aircraftCfgEntriesList = this->performParsing(modelDirectory, excludedDirectoryPatterns, msgs, &ok);
                         CAircraftModelList models;
                         if (ok)
                         {
-                            models = aircraftCfgEntriesList.toAircraftModelList(simulator);
+                            models = aircraftCfgEntriesList.toAircraftModelList(simulator, true, msgs);
-                            CAircraftModelUtilities::mergeWithDbData(models, dbModels);
+                            if (modelConsolidation) { modelConsolidation(models, true); }
                         }
-                        return std::make_tuple(aircraftCfgEntriesList, models, ok);
+                        return std::make_tuple(aircraftCfgEntriesList, models, msgs, ok);
                     });
                     m_parserWorker->thenWithResult<LoaderResponse>(this, [this, simulator](const LoaderResponse & tuple)
                     {
-                        const bool ok = std::get<2>(tuple);
+                        const bool ok = std::get<3>(tuple);
+                        this->m_loadingMessages = std::get<2>(tuple);
                         if (ok)
                         {
                             this->m_parsedCfgEntriesList = std::get<0>(tuple);
@@ -93,7 +97,7 @@ namespace BlackMisc
                             const bool hasData = !models.isEmpty();
                             if (hasData)
                             {
-                                this->setCachedModels(models); // not thread safe
+                                this->setCachedModels(models, simulator); // not thread safe
                             }
                             // currently I treat no data as error
                             emit this->loadingFinished(hasData, simulator);
@@ -107,9 +111,10 @@ namespace BlackMisc
                 else if (mode == LoadDirectly)
                 {
                     bool ok;
-                    this->m_parsedCfgEntriesList = performParsing(modelDirectory, excludedDirectoryPatterns, &ok);
+                    CStatusMessageList msgs;
-                    CAircraftModelList models(this->m_parsedCfgEntriesList.toAircraftModelList(simulator));
+                    this->m_parsedCfgEntriesList = performParsing(modelDirectory, excludedDirectoryPatterns, msgs, &ok);
-                    CAircraftModelUtilities::mergeWithDbData(models, dbModels);
+                    CAircraftModelList models(this->m_parsedCfgEntriesList.toAircraftModelList(simulator, true, msgs));
+                    this->m_loadingMessages = msgs;
                     const bool hasData = !models.isEmpty();
                     if (hasData)
                     {
@@ -143,7 +148,7 @@ namespace BlackMisc
                            true, { fileFilter() }, this->getModelExcludeDirectoryPatterns());
             }
 
-            CAircraftCfgEntriesList CAircraftCfgParser::performParsing(const QString &directory, const QStringList &excludeDirectories, bool *ok)
+            CAircraftCfgEntriesList CAircraftCfgParser::performParsing(const QString &directory, const QStringList &excludeDirectories, CStatusMessageList &messages, bool *ok)
             {
                 //
                 // function has to be thread safe
@@ -183,7 +188,7 @@ namespace BlackMisc
                         if (dir == currentDir) { continue; } // do not recursively call same directory
 
                         bool dirOk;
-                        const CAircraftCfgEntriesList subList(performParsing(nextDir, excludeDirectories, &dirOk));
+                        const CAircraftCfgEntriesList subList(performParsing(nextDir, excludeDirectories, messages, &dirOk));
                         if (dirOk)
                         {
                             result.push_back(subList);
@@ -378,9 +383,8 @@ namespace BlackMisc
                 static const QString f("aircraft.cfg");
                 return f;
             }
-
+        } // ns
-        } // namespace
+    } // ns
-    } // namespace
+} // ns
-} // namespace
 
 Q_DECLARE_METATYPE(BlackMisc::Simulation::FsCommon::LoaderResponse)

src/blackmisc/simulation/fscommon/aircraftcfgparser.h

@@ -30,7 +30,6 @@ class QSettings;
 namespace BlackMisc
 {
     class CWorker;
-
     namespace Simulation
     {
         namespace FsCommon
@@ -62,7 +61,7 @@ namespace BlackMisc
             protected:
                 //! \name Interface functions
                 //! @{
-                virtual void startLoadingFromDisk(LoadMode mode, const BlackMisc::Simulation::CAircraftModelList &dbModels) override;
+                virtual void startLoadingFromDisk(LoadMode mode, const ModelConsolidation &modelConsolidation) override;
                 //! @}
 
             private slots:
@@ -80,7 +79,9 @@ namespace BlackMisc
 
                 //! Perform the parsing
                 //! \threadsafe
-                CAircraftCfgEntriesList performParsing(const QString &directory, const QStringList &excludeDirectories, bool *ok);
+                CAircraftCfgEntriesList performParsing(
+                    const QString &directory, const QStringList &excludeDirectories,
+                    BlackMisc::CStatusMessageList &messages, bool *ok);
 
                 //! Fix the content read
                 static QString fixedStringContent(const QVariant &qv);
@@ -94,11 +95,11 @@ namespace BlackMisc
                 //! Files to be used
                 static const QString &fileFilter();
 
-                CAircraftCfgEntriesList      m_parsedCfgEntriesList; //!< parsed entries
+                CAircraftCfgEntriesList       m_parsedCfgEntriesList; //!< parsed entries
-                QPointer<BlackMisc::CWorker> m_parserWorker;         //!< worker will destroy itself, so weak pointer
+                QPointer<BlackMisc::CWorker>  m_parserWorker;         //!< worker will destroy itself, so weak pointer
             };
-        } // namespace
+        } // ns
-    } // namespace
+    } // ns
-} // namespace
+} // ns
 
 #endif // guard

src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp

@@ -68,7 +68,7 @@ namespace BlackMisc
                 if (this->m_parserWorker) { this->m_parserWorker->waitForFinished(); }
             }
 
-            void CAircraftModelLoaderXPlane::startLoadingFromDisk(LoadMode mode, const CAircraftModelList &dbModels)
+            void CAircraftModelLoaderXPlane::startLoadingFromDisk(LoadMode mode, const ModelConsolidation &modelConsolidation)
             {
                 //! \todo according to meeting XP needs to support multiple directories
                 const CSimulatorInfo simulator = this->getSimulator();
@@ -86,10 +86,10 @@ namespace BlackMisc
                 {
                     if (m_parserWorker && !m_parserWorker->isFinished()) { return; }
                     m_parserWorker = BlackMisc::CWorker::fromTask(this, "CAircraftModelLoaderXPlane::performParsing",
-                                     [this, modelDirectory, excludedDirectoryPatterns, dbModels]()
+                                     [this, modelDirectory, excludedDirectoryPatterns, modelConsolidation]()
                     {
                         auto models = performParsing(modelDirectory, excludedDirectoryPatterns);
-                        CAircraftModelUtilities::mergeWithDbData(models, dbModels);
+                        if (modelConsolidation) { modelConsolidation(models, true); }
                         return models;
                     });
                     m_parserWorker->thenWithResult<CAircraftModelList>(this, [this](const auto & models)
@@ -100,7 +100,6 @@ namespace BlackMisc
                 else if (mode.testFlag(LoadDirectly))
                 {
                     CAircraftModelList models(performParsing(this->getFirstModelDirectoryOrDefault(), excludedDirectoryPatterns));
-                    CAircraftModelUtilities::mergeWithDbData(models, dbModels);
                     updateInstalledModels(models);
                 }
             }

src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.h

@@ -60,7 +60,7 @@ namespace BlackMisc
             protected:
                 //! \name Interface functions
                 //! @{
-                virtual void startLoadingFromDisk(LoadMode mode, const BlackMisc::Simulation::CAircraftModelList &dbModels) override;
+                virtual void startLoadingFromDisk(LoadMode mode, const ModelConsolidation &modelConsolidation) override;
                 //! @}
 
             private: