From 019b623f43189972e943d7824bf8cb6eb0fb5550 Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Tue, 9 Aug 2016 01:33:32 +0200 Subject: [PATCH] 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) --- .../simulation/aircraftmodelloader.cpp | 16 +++++-- .../simulation/aircraftmodelloader.h | 27 +++++++---- .../fscommon/aircraftcfgentrieslist.cpp | 31 +++++++++---- .../fscommon/aircraftcfgentrieslist.h | 4 +- .../simulation/fscommon/aircraftcfgparser.cpp | 46 ++++++++++--------- .../simulation/fscommon/aircraftcfgparser.h | 17 +++---- .../xplane/aircraftmodelloaderxplane.cpp | 7 ++- .../xplane/aircraftmodelloaderxplane.h | 2 +- 8 files changed, 90 insertions(+), 60 deletions(-) diff --git a/src/blackmisc/simulation/aircraftmodelloader.cpp b/src/blackmisc/simulation/aircraftmodelloader.cpp index d870e4572..a602d402b 100644 --- a/src/blackmisc/simulation/aircraftmodelloader.cpp +++ b/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 #include @@ -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 diff --git a/src/blackmisc/simulation/aircraftmodelloader.h b/src/blackmisc/simulation/aircraftmodelloader.h index 3ac787115..acbf25522 100644 --- a/src/blackmisc/simulation/aircraftmodelloader.h +++ b/src/blackmisc/simulation/aircraftmodelloader.h @@ -28,6 +28,7 @@ #include #include #include +#include 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; + //! 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 m_cancelLoading { false }; //!< flag - std::atomic m_loadingInProgress { false }; //!< Loading in progress - BlackMisc::Simulation::Data::CModelCaches m_caches { this }; //!< caches + std::atomic m_cancelLoading { false }; //!< flag + std::atomic m_loadingInProgress { false }; //!< Loading in progress + 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); }; - - } // namespace -} // namespace + } // ns +} // ns Q_DECLARE_METATYPE(BlackMisc::Simulation::IAircraftModelLoader::LoadMode) Q_DECLARE_METATYPE(BlackMisc::Simulation::IAircraftModelLoader::LoadModeFlag) diff --git a/src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.cpp b/src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.cpp index 7e4186cdf..c59d6a44c 100644 --- a/src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.cpp +++ b/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 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; - for (const CAircraftCfgEntries &entries : (*this)) - { - CAircraftModel m(entries.toAircraftModel()); - m.setSimulator(simInfo); - ml.push_back(m); - } + CAircraftModelList ml = this->toAircraftModelList(ignoreDuplicatesAndEmptyModelStrings, msgs); + ml.setSimulatorInfo(simInfo); return ml; } diff --git a/src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.h b/src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.h index 084e3537f..877c38a0d 100644 --- a/src/blackmisc/simulation/fscommon/aircraftcfgentrieslist.h +++ b/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; diff --git a/src/blackmisc/simulation/fscommon/aircraftcfgparser.cpp b/src/blackmisc/simulation/fscommon/aircraftcfgparser.cpp index 0d643d02c..ad429b400 100644 --- a/src/blackmisc/simulation/fscommon/aircraftcfgparser.cpp +++ b/src/blackmisc/simulation/fscommon/aircraftcfgparser.cpp @@ -7,13 +7,14 @@ * contained in the LICENSE file. */ -#include "blackmisc/fileutils.h" -#include "blackmisc/logmessage.h" +#include "blackcore/db/databaseutils.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 @@ -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; + using LoaderResponse = std::tuple; 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); - CAircraftModelUtilities::mergeWithDbData(models, dbModels); + models = aircraftCfgEntriesList.toAircraftModelList(simulator, true, msgs); + if (modelConsolidation) { modelConsolidation(models, true); } } - return std::make_tuple(aircraftCfgEntriesList, models, ok); + return std::make_tuple(aircraftCfgEntriesList, models, msgs, ok); }); m_parserWorker->thenWithResult(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); - CAircraftModelList models(this->m_parsedCfgEntriesList.toAircraftModelList(simulator)); - CAircraftModelUtilities::mergeWithDbData(models, dbModels); + CStatusMessageList msgs; + this->m_parsedCfgEntriesList = performParsing(modelDirectory, excludedDirectoryPatterns, msgs, &ok); + 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; } - - } // namespace - } // namespace -} // namespace + } // ns + } // ns +} // ns Q_DECLARE_METATYPE(BlackMisc::Simulation::FsCommon::LoaderResponse) diff --git a/src/blackmisc/simulation/fscommon/aircraftcfgparser.h b/src/blackmisc/simulation/fscommon/aircraftcfgparser.h index 929176f1d..5591c506a 100644 --- a/src/blackmisc/simulation/fscommon/aircraftcfgparser.h +++ b/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 - QPointer m_parserWorker; //!< worker will destroy itself, so weak pointer + CAircraftCfgEntriesList m_parsedCfgEntriesList; //!< parsed entries + QPointer m_parserWorker; //!< worker will destroy itself, so weak pointer }; - } // namespace - } // namespace -} // namespace + } // ns + } // ns +} // ns #endif // guard diff --git a/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp b/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp index 2a21076ce..33562edba 100644 --- a/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.cpp +++ b/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(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); } } diff --git a/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.h b/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.h index 3bf45f805..6c11e9a66 100644 --- a/src/blackmisc/simulation/xplane/aircraftmodelloaderxplane.h +++ b/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: