From b9760b4c600834257ecf6612a60dc2aafe040b1f Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Sun, 10 Sep 2017 23:35:15 +0200 Subject: [PATCH] Ref T149, use URL log to log network access in threaded reader classes * added getFromNetworkAndLog * adjusted DB reader * adjusted other readers --- src/blackcore/db/airportdatareader.cpp | 2 +- src/blackcore/db/databasereader.cpp | 1 + src/blackcore/db/icaodatareader.cpp | 6 ++--- src/blackcore/db/infodatareader.cpp | 2 +- src/blackcore/db/modeldatareader.cpp | 6 ++--- src/blackcore/threadedreader.cpp | 25 +++++++++++++++++-- src/blackcore/threadedreader.h | 15 +++++++++++ src/blackcore/vatsim/vatsimbookingreader.cpp | 4 +-- src/blackcore/vatsim/vatsimdatafilereader.cpp | 5 ++-- src/blackcore/vatsim/vatsimmetarreader.cpp | 5 ++-- .../vatsim/vatsimstatusfilereader.cpp | 3 ++- 11 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/blackcore/db/airportdatareader.cpp b/src/blackcore/db/airportdatareader.cpp index a7ba49a8e..545aedcf4 100644 --- a/src/blackcore/db/airportdatareader.cpp +++ b/src/blackcore/db/airportdatareader.cpp @@ -236,7 +236,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CAirportDataReader::ps_parseAirportData }); + this->getFromNetworkAndLog(url, { this, &CAirportDataReader::ps_parseAirportData }); emit dataRead(CEntityFlags::AirportEntity, CEntityFlags::StartRead, 0); } else diff --git a/src/blackcore/db/databasereader.cpp b/src/blackcore/db/databasereader.cpp index 5daaf59d2..f539a684a 100644 --- a/src/blackcore/db/databasereader.cpp +++ b/src/blackcore/db/databasereader.cpp @@ -592,6 +592,7 @@ namespace BlackCore Q_ASSERT_X(nwReply, Q_FUNC_INFO, "Missing network reply"); if (nwReply && nwReply->isFinished()) { + this->logNetworkReplyReceived(nwReply); this->setReplyStatus(nwReply->error(), nwReply->errorString()); } } diff --git a/src/blackcore/db/icaodatareader.cpp b/src/blackcore/db/icaodatareader.cpp index eaa5c0803..7c56f0801 100644 --- a/src/blackcore/db/icaodatareader.cpp +++ b/src/blackcore/db/icaodatareader.cpp @@ -147,7 +147,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CIcaoDataReader::ps_parseAircraftIcaoData }); + this->getFromNetworkAndLog(url, { this, &CIcaoDataReader::ps_parseAircraftIcaoData }); entitiesTriggered |= CEntityFlags::AircraftIcaoEntity; } else @@ -162,7 +162,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CIcaoDataReader::ps_parseAirlineIcaoData }); + this->getFromNetworkAndLog(url, { this, &CIcaoDataReader::ps_parseAirlineIcaoData }); entitiesTriggered |= CEntityFlags::AirlineIcaoEntity; } else @@ -177,7 +177,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CIcaoDataReader::ps_parseCountryData }); + this->getFromNetworkAndLog(url, { this, &CIcaoDataReader::ps_parseCountryData }); entitiesTriggered |= CEntityFlags::CountryEntity; } else diff --git a/src/blackcore/db/infodatareader.cpp b/src/blackcore/db/infodatareader.cpp index b053dae6c..cfdc013c4 100644 --- a/src/blackcore/db/infodatareader.cpp +++ b/src/blackcore/db/infodatareader.cpp @@ -116,7 +116,7 @@ namespace BlackCore const CUrl url(this->getInfoObjectsUrl()); if (!url.isEmpty()) { - sApp->getFromNetwork(url, { this, &CInfoDataReader::parseInfoObjectsData}); + this->getFromNetworkAndLog(url, { this, &CInfoDataReader::parseInfoObjectsData}); emit dataRead(this->getEntityForMode(), CEntityFlags::StartRead, 0); } else diff --git a/src/blackcore/db/modeldatareader.cpp b/src/blackcore/db/modeldatareader.cpp index fc94008e3..25c9999db 100644 --- a/src/blackcore/db/modeldatareader.cpp +++ b/src/blackcore/db/modeldatareader.cpp @@ -174,7 +174,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CModelDataReader::ps_parseLiveryData}); + this->getFromNetworkAndLog(url, { this, &CModelDataReader::ps_parseLiveryData}); triggeredRead |= CEntityFlags::LiveryEntity; } else @@ -189,7 +189,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CModelDataReader::ps_parseDistributorData}); + this->getFromNetworkAndLog(url, { this, &CModelDataReader::ps_parseDistributorData}); triggeredRead |= CEntityFlags::DistributorEntity; } else @@ -204,7 +204,7 @@ namespace BlackCore if (!url.isEmpty()) { url.appendQuery(queryLatestTimestamp(newerThan)); - sApp->getFromNetwork(url, { this, &CModelDataReader::ps_parseModelData}); + this->getFromNetworkAndLog(url, { this, &CModelDataReader::ps_parseModelData}); triggeredRead |= CEntityFlags::ModelEntity; } else diff --git a/src/blackcore/threadedreader.cpp b/src/blackcore/threadedreader.cpp index 50cfecf28..1399429dc 100644 --- a/src/blackcore/threadedreader.cpp +++ b/src/blackcore/threadedreader.cpp @@ -115,6 +115,12 @@ namespace BlackCore this->m_markedAsFailed = failed; } + CUrlLogList CThreadedReader::getReadLog() const + { + QReadLocker rl(&this->m_lock); + return m_urlReadLog; + } + void CThreadedReader::threadAssertCheck() const { Q_ASSERT_X(QCoreApplication::instance()->thread() != QThread::currentThread(), Q_FUNC_INFO, "Needs to run in own thread"); @@ -146,12 +152,27 @@ namespace BlackCore bool CThreadedReader::doWorkCheck() const { - if (!m_unitTest && (!sApp || !sApp->hasWebDataServices())) { return false; } - if (!isEnabled()) { return false; } + // sApp->hasWebDataServices() cannot be used, as some readers are already used during init phase + if (!m_unitTest && (!sApp || sApp->isShuttingDown())) { return false; } + if (!isEnabled()) { return false; } if (isAbandoned()) { return false; } return true; } + QNetworkReply *CThreadedReader::getFromNetworkAndLog(const CUrl &url, const BlackMisc::CSlot &callback) + { + // returned QNetworkReply normally nullptr since QAM is in different thread + const int id = m_urlReadLog.addPendingUrl(url); + return sApp->getFromNetwork(url, id, callback); + } + + void CThreadedReader::logNetworkReplyReceived(QNetworkReply *reply) + { + if (!reply) { return; } + QWriteLocker wl(&m_lock); + m_urlReadLog.markAsReceived(reply, reply->error() == QNetworkReply::NoError); + } + void CThreadedReader::logInconsistentData(const CStatusMessage &msg, const char *funcInfo) { if (msg.isEmpty()) { return; } diff --git a/src/blackcore/threadedreader.h b/src/blackcore/threadedreader.h index a1d698a38..dd507586d 100644 --- a/src/blackcore/threadedreader.h +++ b/src/blackcore/threadedreader.h @@ -14,6 +14,7 @@ #include "blackcore/vatsim/vatsimsettings.h" #include "blackcore/blackcoreexport.h" +#include "blackmisc/network/urlloglist.h" #include "blackmisc/logcategorylist.h" #include "blackmisc/worker.h" @@ -58,12 +59,18 @@ namespace BlackCore //! Is marked as read failed //! \threadsafe + //! \deprecated likely to be removed bool isMarkedAsFailed() const; //! Set marker for read failed //! \threadsafe + //! \deprecated likely to be removed void setMarkedAsFailed(bool failed); + //! Get the read log + //! \threadsafe + BlackMisc::Network::CUrlLogList getReadLog() const; + //! Starts the reader //! \threadsafe void startReader(); @@ -101,6 +108,13 @@ namespace BlackCore //! Still enabled etc.? bool doWorkCheck() const; + //! Get request from network, and log with m_urlReadLog + QNetworkReply *getFromNetworkAndLog(const BlackMisc::Network::CUrl &url, const BlackMisc::CSlot &callback); + + //! Network reply received, mark in m_urlReadLog + //! \threadsafe + void logNetworkReplyReceived(QNetworkReply *reply); + //! Use this to log inconsistent data //! \remark here in a single place severity / format can be adjusted static void logInconsistentData(const BlackMisc::CStatusMessage &msg, const char *funcInfo = nullptr); @@ -115,6 +129,7 @@ namespace BlackCore uint m_contentHash = 0; //!< has of the content given std::atomic m_markedAsFailed { false }; //!< marker if reading failed bool m_unitTest { false }; //!< mark as unit test + BlackMisc::Network::CUrlLogList m_urlReadLog; //!< URL based reading can be logged }; } // namespace diff --git a/src/blackcore/vatsim/vatsimbookingreader.cpp b/src/blackcore/vatsim/vatsimbookingreader.cpp index 06e959028..1722559e7 100644 --- a/src/blackcore/vatsim/vatsimbookingreader.cpp +++ b/src/blackcore/vatsim/vatsimbookingreader.cpp @@ -69,7 +69,7 @@ namespace BlackCore Q_ASSERT_X(sApp, Q_FUNC_INFO, "No application"); const QUrl url(sApp->getGlobalSetup().getVatsimBookingsUrl()); if (url.isEmpty()) { return; } - sApp->getFromNetwork(url, { this, &CVatsimBookingReader::ps_parseBookings}); + this->getFromNetworkAndLog(url, { this, &CVatsimBookingReader::ps_parseBookings}); } void CVatsimBookingReader::ps_parseBookings(QNetworkReply *nwReplyPtr) @@ -77,7 +77,6 @@ namespace BlackCore // wrap pointer, make sure any exit cleans up reply // required to use delete later as object is created in a different thread QScopedPointer nwReply(nwReplyPtr); - this->threadAssertCheck(); // Worker thread, make sure to write no members here od do it threadsafe @@ -87,6 +86,7 @@ namespace BlackCore return; // stop, terminate straight away, ending thread } + this->logNetworkReplyReceived(nwReplyPtr); if (nwReply->error() == QNetworkReply::NoError) { static const QString timestampFormat("yyyy-MM-dd HH:mm:ss"); diff --git a/src/blackcore/vatsim/vatsimdatafilereader.cpp b/src/blackcore/vatsim/vatsimdatafilereader.cpp index ccfe4207c..c51e9d21e 100644 --- a/src/blackcore/vatsim/vatsimdatafilereader.cpp +++ b/src/blackcore/vatsim/vatsimdatafilereader.cpp @@ -192,7 +192,7 @@ namespace BlackCore CFailoverUrlList urls(sApp->getVatsimDataFileUrls()); const QUrl url(urls.obtainNextWorkingUrl(true)); if (url.isEmpty()) { return; } - sApp->getFromNetwork(url, { this, &CVatsimDataFileReader::ps_parseVatsimFile}); + this->getFromNetworkAndLog(url, { this, &CVatsimDataFileReader::ps_parseVatsimFile}); } void CVatsimDataFileReader::ps_parseVatsimFile(QNetworkReply *nwReplyPtr) @@ -200,15 +200,16 @@ namespace BlackCore // wrap pointer, make sure any exit cleans up reply // required to use delete later as object is created in a different thread QScopedPointer nwReply(nwReplyPtr); + this->threadAssertCheck(); // Worker thread, make sure to write only synced here! - this->threadAssertCheck(); if (!this->doWorkCheck()) { CLogMessage(this).info("Terminated VATSIM file parsing process"); return; // stop, terminate straight away, ending thread } + this->logNetworkReplyReceived(nwReplyPtr); QStringList illegalIcaoCodes; if (nwReply->error() == QNetworkReply::NoError) { diff --git a/src/blackcore/vatsim/vatsimmetarreader.cpp b/src/blackcore/vatsim/vatsimmetarreader.cpp index db2e67220..3c0fd9227 100644 --- a/src/blackcore/vatsim/vatsimmetarreader.cpp +++ b/src/blackcore/vatsim/vatsimmetarreader.cpp @@ -82,7 +82,7 @@ namespace BlackCore const CUrl url(urls.obtainNextWorkingUrl(true)); if (url.isEmpty()) { return; } Q_ASSERT_X(sApp, Q_FUNC_INFO, "No Application"); - sApp->getFromNetwork(url.withAppendedQuery("id=all"), { this, &CVatsimMetarReader::decodeMetars}); + this->getFromNetworkAndLog(url.withAppendedQuery("id=all"), { this, &CVatsimMetarReader::decodeMetars}); } void CVatsimMetarReader::decodeMetars(QNetworkReply *nwReplyPtr) @@ -99,6 +99,7 @@ namespace BlackCore return; // stop, terminate straight away, ending thread } + this->logNetworkReplyReceived(nwReplyPtr); if (nwReply->error() == QNetworkReply::NoError) { QString metarData = nwReply->readAll(); @@ -135,7 +136,7 @@ namespace BlackCore else { // network error - CLogMessage(this).warning("Reading METARs failed %1 %2") << nwReply->errorString() << nwReply->url().toString(); + CLogMessage(this).warning("Reading METARs failed '%1' for '%2'") << nwReply->errorString() << nwReply->url().toString(); nwReply->abort(); emit dataRead(CEntityFlags::MetarEntity, CEntityFlags::ReadFailed, 0); } diff --git a/src/blackcore/vatsim/vatsimstatusfilereader.cpp b/src/blackcore/vatsim/vatsimstatusfilereader.cpp index 3374905fc..feac2ab9b 100644 --- a/src/blackcore/vatsim/vatsimstatusfilereader.cpp +++ b/src/blackcore/vatsim/vatsimstatusfilereader.cpp @@ -72,7 +72,7 @@ namespace BlackCore CFailoverUrlList urls(sApp->getGlobalSetup().getVatsimStatusFileUrls()); const CUrl url(urls.obtainNextWorkingUrl(true)); // random working URL if (url.isEmpty()) { return; } - sApp->getFromNetwork(url, { this, &CVatsimStatusFileReader::ps_parseVatsimFile}); + this->getFromNetworkAndLog(url, { this, &CVatsimStatusFileReader::ps_parseVatsimFile}); } void CVatsimStatusFileReader::ps_parseVatsimFile(QNetworkReply *nwReplyPtr) @@ -90,6 +90,7 @@ namespace BlackCore return; // stop, terminate straight away, ending thread } + this->logNetworkReplyReceived(nwReplyPtr); if (nwReply->error() == QNetworkReply::NoError) { const QString dataFileData = nwReply->readAll();