From eb74efa9bab58e2bccc77e371c634af6e1fffa19 Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Fri, 15 Jan 2016 02:20:39 +0100 Subject: [PATCH] refs #568, improved error handling / parameter names in readers * better categories * parameter name as static functions * error message in structure CDatabaseReader::JsonDatastoreResponse * prefixed members of structure with _m * consolidate severity --- src/blackcore/databasereader.cpp | 25 ++++++++++------- src/blackcore/databasereader.h | 39 ++++++++++++++++++++------- src/blackcore/icaodatareader.cpp | 21 ++++++++------- src/blackcore/modeldatareader.cpp | 27 ++++++++++++++----- src/blackcore/modeldatareader.h | 6 +++++ src/blackcore/webdataservices.cpp | 13 +++++++-- src/blackmisc/network/entityflags.cpp | 2 +- 7 files changed, 96 insertions(+), 37 deletions(-) diff --git a/src/blackcore/databasereader.cpp b/src/blackcore/databasereader.cpp index 3cc226bd6..50dffd9d5 100644 --- a/src/blackcore/databasereader.cpp +++ b/src/blackcore/databasereader.cpp @@ -39,11 +39,13 @@ namespace BlackCore CDatabaseReader::JsonDatastoreResponse CDatabaseReader::transformReplyIntoDatastoreResponse(QNetworkReply *nwReply) const { this->threadAssertCheck(); + static const CLogCategoryList cats(CLogCategoryList(this).join({ CLogCategory::webservice()})); + JsonDatastoreResponse datastoreResponse; if (this->isAbandoned()) { - CLogMessage(this).info("Terminated data parsing process"); // for users nwReply->abort(); + datastoreResponse.setMessage(CStatusMessage(cats, CStatusMessage::SeverityError, "Terminated data parsing process")); return datastoreResponse; // stop, terminate straight away, ending thread } @@ -51,22 +53,27 @@ namespace BlackCore { const QString dataFileData = nwReply->readAll().trimmed(); nwReply->close(); // close asap - if (dataFileData.isEmpty()) { datastoreResponse.updated = QDateTime::currentDateTimeUtc(); return datastoreResponse; } + if (dataFileData.isEmpty()) + { + datastoreResponse.setMessage(CStatusMessage(cats, CStatusMessage::SeverityError, "Empty response, no data")); + datastoreResponse.m_updated = QDateTime::currentDateTimeUtc(); + return datastoreResponse; + } QJsonDocument jsonResponse = QJsonDocument::fromJson(dataFileData.toUtf8()); if (jsonResponse.isArray()) { // directly an array, no further info - datastoreResponse.jsonArray = jsonResponse.array(); - datastoreResponse.updated = QDateTime::currentDateTimeUtc(); + datastoreResponse.m_jsonArray = jsonResponse.array(); + datastoreResponse.m_updated = QDateTime::currentDateTimeUtc(); } else { QJsonObject responseObject(jsonResponse.object()); - datastoreResponse.jsonArray = responseObject["data"].toArray(); + datastoreResponse.m_jsonArray = responseObject["data"].toArray(); QString ts(responseObject["latest"].toString()); - datastoreResponse.updated = ts.isEmpty() ? QDateTime::currentDateTimeUtc() : CDatastoreUtility::parseTimestamp(ts); - datastoreResponse.restricted = responseObject["restricted"].toBool(); + datastoreResponse.m_updated = ts.isEmpty() ? QDateTime::currentDateTimeUtc() : CDatastoreUtility::parseTimestamp(ts); + datastoreResponse.m_restricted = responseObject["restricted"].toBool(); } return datastoreResponse; } @@ -74,8 +81,9 @@ namespace BlackCore // no valid response QString error(nwReply->errorString()); QString url(nwReply->url().toString()); - CLogMessage(this).warning("Reading data failed %1 %2") << error << url; nwReply->abort(); + datastoreResponse.setMessage(CStatusMessage(cats, CStatusMessage::SeverityError, + QString("Reading data failed: " + error + " " + url))); return datastoreResponse; } @@ -159,5 +167,4 @@ namespace BlackCore bool ok = CNetworkUtils::canConnect(url, m); this->setConnectionStatus(ok, m); } - } // namespace diff --git a/src/blackcore/databasereader.h b/src/blackcore/databasereader.h index f33ad189f..90fc186c4 100644 --- a/src/blackcore/databasereader.h +++ b/src/blackcore/databasereader.h @@ -31,30 +31,49 @@ namespace BlackCore //! Response from our database struct JsonDatastoreResponse { - QJsonArray jsonArray; //!< JSON array data - QDateTime updated; //!< when was the latest updated? - bool restricted = false; //!< restricted reponse, only data changed + QJsonArray m_jsonArray; //!< JSON array data + QDateTime m_updated; //!< when was the latest updated? + bool m_restricted = false; //!< restricted reponse, only data changed + BlackMisc::CStatusMessage m_message; //!< last error or warning //! Any data? - bool isEmpty() const { return jsonArray.isEmpty(); } + bool isEmpty() const { return m_jsonArray.isEmpty(); } //! Number of elements - int size() const { return jsonArray.size(); } + int size() const { return m_jsonArray.size(); } //! Any timestamp? - bool hasTimestamp() const { return updated.isValid(); } + bool hasTimestamp() const { return m_updated.isValid(); } //! Is response newer? - bool isNewer(const QDateTime &ts) const { return updated.toMSecsSinceEpoch() > ts.toMSecsSinceEpoch(); } + bool isNewer(const QDateTime &ts) const { return m_updated.toMSecsSinceEpoch() > ts.toMSecsSinceEpoch(); } //! Is response newer? - bool isNewer(qint64 mSecsSinceEpoch) const { return updated.toMSecsSinceEpoch() > mSecsSinceEpoch; } + bool isNewer(qint64 mSecsSinceEpoch) const { return m_updated.toMSecsSinceEpoch() > mSecsSinceEpoch; } //! Incremental data - bool isRestricted() const { return restricted; } + bool isRestricted() const { return m_restricted; } + + //! Error message? + bool hasErrorMessage() const { return m_message.getSeverity() == BlackMisc::CStatusMessage::SeverityError; } + + //! Warning or error message? + bool hasWarningOrAboveMessage() const { return m_message.isWarningOrAbove(); } + + //! Last error or warning + const BlackMisc::CStatusMessage &lastWarningOrAbove() const { return m_message; } + + //! Set the error/warning message + void setMessage(const BlackMisc::CStatusMessage &lastErrorOrWarning) { m_message = lastErrorOrWarning; } + + //! Get the JSON array + QJsonArray getJsonArray() const { return m_jsonArray; } + + //! Set the JSON array + void setJsonArray(const QJsonArray &value) { m_jsonArray = value; } //! Implicit conversion - operator QJsonArray() const { return jsonArray; } + operator QJsonArray() const { return m_jsonArray; } }; //! Start reading in own thread diff --git a/src/blackcore/icaodatareader.cpp b/src/blackcore/icaodatareader.cpp index 64dd29071..b712ee278 100644 --- a/src/blackcore/icaodatareader.cpp +++ b/src/blackcore/icaodatareader.cpp @@ -203,13 +203,14 @@ namespace BlackCore // required to use delete later as object is created in a different thread QScopedPointer nwReply(nwReplyPtr); QString urlString(nwReply->url().toString()); - QJsonArray array = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); - if (array.isEmpty()) + CDatabaseReader::JsonDatastoreResponse res = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); + if (res.hasErrorMessage()) { + CLogMessage(this).preformatted(res.lastWarningOrAbove()); emit dataRead(CEntityFlags::AircraftIcaoEntity, CEntityFlags::ReadFailed, 0); return; } - CAircraftIcaoCodeList codes = CAircraftIcaoCodeList::fromDatabaseJson(array); + CAircraftIcaoCodeList codes = CAircraftIcaoCodeList::fromDatabaseJson(res); // this part needs to be synchronized int n = codes.size(); @@ -225,13 +226,14 @@ namespace BlackCore { QScopedPointer nwReply(nwReplyPtr); QString urlString(nwReply->url().toString()); - QJsonArray array = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); - if (array.isEmpty()) + CDatabaseReader::JsonDatastoreResponse res = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); + if (res.hasErrorMessage()) { + CLogMessage(this).preformatted(res.lastWarningOrAbove()); emit dataRead(CEntityFlags::AirlineIcaoEntity, CEntityFlags::ReadFailed, 0); return; } - CAirlineIcaoCodeList codes = CAirlineIcaoCodeList::fromDatabaseJson(array); + CAirlineIcaoCodeList codes = CAirlineIcaoCodeList::fromDatabaseJson(res); // this part needs to be synchronized int n = codes.size(); @@ -247,13 +249,14 @@ namespace BlackCore { QScopedPointer nwReply(nwReplyPtr); QString urlString(nwReply->url().toString()); - QJsonArray array = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); - if (array.isEmpty()) + CDatabaseReader::JsonDatastoreResponse res = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); + if (res.hasErrorMessage()) { + CLogMessage(this).preformatted(res.lastWarningOrAbove()); emit dataRead(CEntityFlags::CountryEntity, CEntityFlags::ReadFailed, 0); return; } - CCountryList countries = CCountryList::fromDatabaseJson(array); + CCountryList countries = CCountryList::fromDatabaseJson(res); // this part needs to be synchronized int n = m_countries.size(); diff --git a/src/blackcore/modeldatareader.cpp b/src/blackcore/modeldatareader.cpp index 62e538236..93ec0a763 100644 --- a/src/blackcore/modeldatareader.cpp +++ b/src/blackcore/modeldatareader.cpp @@ -162,7 +162,7 @@ namespace BlackCore if (!newerThan.isNull()) { const QString tss(newerThan.toString(Qt::ISODate)); - url.appendQuery("newer=" + tss); + url.appendQuery(QString(parameterLatestTimestamp() + "=" + tss)); } QNetworkRequest requestLivery(url); CNetworkUtils::ignoreSslVerification(requestLivery); @@ -183,7 +183,7 @@ namespace BlackCore if (!newerThan.isNull()) { const QString tss(newerThan.toString(Qt::ISODate)); - url.appendQuery("newer=" + tss); + url.appendQuery(QString(parameterLatestTimestamp() + "=" + tss)); } QNetworkRequest requestDistributor(url); CNetworkUtils::ignoreSslVerification(requestDistributor); @@ -204,7 +204,7 @@ namespace BlackCore if (!newerThan.isNull()) { const QString tss(newerThan.toString(Qt::ISODate)); - url.appendQuery("newer=" + tss); + url.appendQuery(QString(parameterLatestTimestamp() + "=" + tss)); } QNetworkRequest requestModel(url); CNetworkUtils::ignoreSslVerification(requestModel); @@ -230,8 +230,9 @@ namespace BlackCore QScopedPointer nwReply(nwReplyPtr); QString urlString(nwReply->url().toString()); CDatabaseReader::JsonDatastoreResponse res = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); - if (res.isEmpty()) + if (res.hasErrorMessage()) { + CLogMessage(this).preformatted(res.lastWarningOrAbove()); emit dataRead(CEntityFlags::LiveryEntity, CEntityFlags::ReadFailed, 0); return; } @@ -267,8 +268,9 @@ namespace BlackCore QScopedPointer nwReply(nwReplyPtr); QString urlString(nwReply->url().toString()); CDatabaseReader::JsonDatastoreResponse res = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); - if (res.isEmpty()) + if (res.hasErrorMessage()) { + CLogMessage(this).preformatted(res.lastWarningOrAbove()); emit dataRead(CEntityFlags::DistributorEntity, CEntityFlags::ReadFailed, 0); return; } @@ -302,8 +304,9 @@ namespace BlackCore QScopedPointer nwReply(nwReplyPtr); QString urlString(nwReply->url().toString()); CDatabaseReader::JsonDatastoreResponse res = this->setStatusAndTransformReplyIntoDatastoreResponse(nwReply.data()); - if (res.isEmpty()) + if (res.hasErrorMessage()) { + CLogMessage(this).preformatted(res.lastWarningOrAbove()); emit dataRead(CEntityFlags::ModelEntity, CEntityFlags::ReadFailed, 0); return; } @@ -453,4 +456,16 @@ namespace BlackCore return getBaseUrl().withAppendedPath("service/jsonaircraftmodel.php"); } + const QString &CModelDataReader::parameterLatestTimestamp() + { + static const QString p("latestTimestamp"); + return p; + } + + const QString &CModelDataReader::parameterLatestId() + { + static const QString p("latestId"); + return p; + } + } // namespace diff --git a/src/blackcore/modeldatareader.h b/src/blackcore/modeldatareader.h index e500c22e5..a86e6f09c 100644 --- a/src/blackcore/modeldatareader.h +++ b/src/blackcore/modeldatareader.h @@ -109,6 +109,12 @@ namespace BlackCore //! Write to JSON file bool writeToJsonFiles(const QString &dir) const; + //! Name of latest timestamp + static const QString ¶meterLatestTimestamp(); + + //! Name of latest timestamp + static const QString ¶meterLatestId(); + signals: //! Combined read signal void dataRead(BlackMisc::Network::CEntityFlags::Entity entity, BlackMisc::Network::CEntityFlags::ReadState state, int number); diff --git a/src/blackcore/webdataservices.cpp b/src/blackcore/webdataservices.cpp index bedead8d7..6d37efdd9 100644 --- a/src/blackcore/webdataservices.cpp +++ b/src/blackcore/webdataservices.cpp @@ -514,13 +514,22 @@ namespace BlackCore void CWebDataServices::ps_readFromSwiftDb(CEntityFlags::Entity entity, CEntityFlags::ReadState state, int number) { + static const CLogCategoryList cats(CLogCategoryList(this).join({ CLogCategory::webservice()})); if (CEntityFlags::isWarningOrAbove(state)) { - CLogMessage(this).warning("Read data %1 entries: %2 state: %3") << CEntityFlags::flagToString(entity) << number << CEntityFlags::flagToString(state); + CStatusMessage::StatusSeverity severity = CEntityFlags::flagToSeverity(state); + if (severity == CStatusMessage::SeverityWarning) + { + CLogMessage(cats).warning("Read data %1 entries: %2 state: %3") << CEntityFlags::flagToString(entity) << number << CEntityFlags::flagToString(state); + } + else + { + CLogMessage(cats).error("Read data %1 entries: %2 state: %3") << CEntityFlags::flagToString(entity) << number << CEntityFlags::flagToString(state); + } } else { - CLogMessage(this).info("Read data %1 entries: %2 state: %3") << CEntityFlags::flagToString(entity) << number << CEntityFlags::flagToString(state); + CLogMessage(cats).info("Read data %1 entries: %2 state: %3") << CEntityFlags::flagToString(entity) << number << CEntityFlags::flagToString(state); } } diff --git a/src/blackmisc/network/entityflags.cpp b/src/blackmisc/network/entityflags.cpp index 1bf7a40fd..481fb9205 100644 --- a/src/blackmisc/network/entityflags.cpp +++ b/src/blackmisc/network/entityflags.cpp @@ -77,7 +77,7 @@ namespace BlackMisc case StartRead: return CStatusMessage::SeverityInfo; case ReadFailed: - return CStatusMessage::SeverityWarning; + return CStatusMessage::SeverityError; default: Q_ASSERT_X(false, Q_FUNC_INFO, "Missing state"); return CStatusMessage::SeverityInfo;