From 144ba62572d2748bd49f8c027b0dd2fa38451202 Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Thu, 25 Feb 2016 22:26:28 +0000 Subject: [PATCH] refs #601, #600, #485 Status message handling. * follow up of debug session, added failure/success to status message * return status messages instead of directly logging in functions returning CStatusMessage. * Ignore empty preformatted messages. * new log category --- src/blackmisc/logcategory.h | 8 ++++ src/blackmisc/logmessage.cpp | 1 + src/blackmisc/logpattern.cpp | 1 + src/blackmisc/statusmessage.cpp | 10 +++++ src/blackmisc/statusmessage.h | 6 +++ src/blackmisc/valuecache.cpp | 68 +++++++++++++++++------------- src/blackmisc/valuecache.h | 3 ++ src/blackmisc/valuecacheprivate.h | 2 +- tests/blackmisc/testvaluecache.cpp | 4 +- 9 files changed, 70 insertions(+), 33 deletions(-) diff --git a/src/blackmisc/logcategory.h b/src/blackmisc/logcategory.h index d4c9bf172..bebc03fbf 100644 --- a/src/blackmisc/logcategory.h +++ b/src/blackmisc/logcategory.h @@ -50,6 +50,13 @@ namespace BlackMisc return cat; } + //! Core/base services such as caching etc. + static const CLogCategory &services() + { + static const CLogCategory cat { "swift.services" }; + return cat; + } + //! Contexts static const CLogCategory &context() { @@ -106,6 +113,7 @@ namespace BlackMisc static const QList cats { uncategorized(), + services(), validation(), context(), contextSlot(), diff --git a/src/blackmisc/logmessage.cpp b/src/blackmisc/logmessage.cpp index 2e76ad092..ffea4fffa 100644 --- a/src/blackmisc/logmessage.cpp +++ b/src/blackmisc/logmessage.cpp @@ -179,6 +179,7 @@ namespace BlackMisc void CLogMessage::preformatted(const CStatusMessage &statusMessage) { + if (statusMessage.isEmpty()) { return; } // just skip empty messages CLogMessage msg(statusMessage.getCategories()); msg.m_severity = statusMessage.getSeverity(); msg.m_message = statusMessage.getMessage(); diff --git a/src/blackmisc/logpattern.cpp b/src/blackmisc/logpattern.cpp index 1a41b3fd4..150a8b1c5 100644 --- a/src/blackmisc/logpattern.cpp +++ b/src/blackmisc/logpattern.cpp @@ -19,6 +19,7 @@ namespace BlackMisc { "uncategorized (swift)", exactMatch(CLogCategory::uncategorized()) }, { "validation", exactMatch(CLogCategory::validation()) }, { "verification", exactMatch(CLogCategory::verification()) }, + { "services", exactMatch(CLogCategory::services()) }, { "model mapping", exactMatch(CLogCategory::mapping()) }, { "swift contexts", exactMatch(CLogCategory::context()) }, { "swift context slots", exactMatch(CLogCategory::contextSlot()) }, diff --git a/src/blackmisc/statusmessage.cpp b/src/blackmisc/statusmessage.cpp index d5095f9b0..0c874a011 100644 --- a/src/blackmisc/statusmessage.cpp +++ b/src/blackmisc/statusmessage.cpp @@ -142,6 +142,16 @@ namespace BlackMisc return c.isEmpty() ? this->getCategoriesAsString() : c; } + bool CStatusMessage::isSuccess() const + { + return !isFailure(); + } + + bool CStatusMessage::isFailure() const + { + return getSeverity() == SeverityError; + } + void CStatusMessage::prependMessage(const QString &msg) { if (msg.isEmpty()) { return; } diff --git a/src/blackmisc/statusmessage.h b/src/blackmisc/statusmessage.h index e787605ba..b41e54476 100644 --- a/src/blackmisc/statusmessage.h +++ b/src/blackmisc/statusmessage.h @@ -107,6 +107,12 @@ namespace BlackMisc //! Warning or above bool isWarningOrAbove() const { return this->m_severity == SeverityWarning || this->m_severity == SeverityError; } + //! Operation considered successful + bool isSuccess() const; + + //! Operation considered unsuccessful + bool isFailure() const; + //! Message QString getMessage() const { return this->m_message; } diff --git a/src/blackmisc/valuecache.cpp b/src/blackmisc/valuecache.cpp index efc0b776a..18d009c83 100644 --- a/src/blackmisc/valuecache.cpp +++ b/src/blackmisc/valuecache.cpp @@ -85,13 +85,19 @@ namespace BlackMisc // CValueCache //////////////////////////////// + const CLogCategoryList &CValueCache::getLogCategories() + { + static const CLogCategoryList cats({ CLogCategory("swift.valuecache") , CLogCategory::services()} ); + return cats; + } + CValueCache::CValueCache(CValueCache::DistributionMode mode, QObject *parent) : QObject(parent) { if (mode == LocalOnly) { // loopback signal to own slot for local operation - connect(this, &CValueCache::valuesChangedByLocal, this, [ = ](const CValueCachePacket &values) + connect(this, &CValueCache::valuesChangedByLocal, this, [ = ](const CValueCachePacket & values) { changeValuesFromRemote(values, CIdentifier()); }); @@ -227,7 +233,7 @@ namespace BlackMisc QMutexLocker lock(&m_mutex); auto values = getAllValues(keyPrefix); auto status = saveToFiles(dir, values); - if (! status.isEmpty()) { markAllAsSaved(keyPrefix); } + if (status.isSuccess()) { markAllAsSaved(keyPrefix); } return status; } @@ -240,31 +246,33 @@ namespace BlackMisc } if (! QDir::root().mkpath(dir)) { - return CLogMessage(this).error("Failed to create directory %1") << dir; + return CStatusMessage(this, CStatusMessage::SeverityError, "Failed to create directory " + dir); } for (auto it = namespaces.cbegin(); it != namespaces.cend(); ++it) { CAtomicFile file(dir + "/" + it.key() + ".json"); if (! file.open(QFile::ReadWrite | QFile::Text)) { - return CLogMessage(this).error("Failed to open %1: %2") << file.fileName() << file.errorString(); + return CStatusMessage(this, CStatusMessage::SeverityError, QString("Failed to open %1: %2").arg(file.fileName()).arg(file.errorString())); } auto json = QJsonDocument::fromJson(file.readAll()); if (json.isArray() || (json.isNull() && ! json.isEmpty())) { - return CLogMessage(this).error("Invalid JSON format in %1") << file.fileName(); + return CStatusMessage(this, CStatusMessage::SeverityError, "Invalid JSON format in " + file.fileName()); } CVariantMap storedValues; storedValues.convertFromJson(json.object()); storedValues.insert(*it); json.setObject(storedValues.toJson()); - if (! (file.seek(0) && file.resize(0) && file.write(json.toJson()) > 0 && file.checkedClose())) + if (!(file.seek(0) && file.resize(0) && file.write(json.toJson()) > 0 && file.checkedClose())) { - return CLogMessage(this).error("Failed to write to %1: %2") << file.fileName() << file.errorString(); + return CStatusMessage(this, CStatusMessage::SeverityError, + QString("Failed to write to %1: %2").arg(file.fileName()).arg(file.errorString())); } } - return {}; + return CStatusMessage(this, CStatusMessage::SeverityInfo, + QString("Written %1 files for value cache in %2").arg(namespaces.size()).arg(dir)); } CStatusMessage CValueCache::loadFromFiles(const QString &dir) @@ -281,19 +289,21 @@ namespace BlackMisc { if (! QDir(dir).isReadable()) { - return CLogMessage(this).error("Failed to read directory %1") << dir; + return CStatusMessage(this, CStatusMessage::SeverityError, "Failed to create directory " + dir); } - for (const auto &filename : QDir(dir).entryList({ "*.json" }, QDir::Files)) + + const QStringList entries(QDir(dir).entryList({ "*.json" }, QDir::Files)); + for (const auto &filename : entries) { QFile file(dir + "/" + filename); if (! file.open(QFile::ReadOnly | QFile::Text)) { - return CLogMessage(this).error("Failed to open %1 : %2") << file.fileName() << file.errorString(); + return CStatusMessage(this, CStatusMessage::SeverityError, QString("Failed to open %1: %2").arg(file.fileName()).arg(file.errorString())); } auto json = QJsonDocument::fromJson(file.readAll()); if (json.isArray() || (json.isNull() && ! json.isEmpty())) { - return CLogMessage(this).error("Invalid JSON format in %1") << file.fileName(); + return CStatusMessage(this, CStatusMessage::SeverityError, "Invalid JSON format in " + file.fileName()); } CVariantMap temp; temp.convertFromJson(json.object()); @@ -304,7 +314,8 @@ namespace BlackMisc temp.removeDuplicates(currentValues); o_values.insert(temp, QFileInfo(file).lastModified().toMSecsSinceEpoch()); } - return {}; + return CStatusMessage(this, CStatusMessage::SeverityInfo, + QString("Loaded value cache from %1 files in %2").arg(entries.size()).arg(dir)); } void CValueCache::markAllAsSaved(const QString &keyPrefix) @@ -371,7 +382,7 @@ namespace BlackMisc CValuePage &CValuePage::getPageFor(QObject *parent, CValueCache *cache) { auto pages = parent->findChildren(); - auto it = std::find_if(pages.cbegin(), pages.cend(), [cache](CValuePage *page) { return page->m_cache == cache; }); + auto it = std::find_if(pages.cbegin(), pages.cend(), [cache](CValuePage * page) { return page->m_cache == cache; }); if (it == pages.cend()) { return *new CValuePage(parent, cache); } else { return **it; } } @@ -401,12 +412,13 @@ namespace BlackMisc auto &element = *(m_elements[key] = ElementPtr(new Element(key, metaType, validator, defaultValue, slot))); std::forward_as_tuple(element.m_value.uniqueWrite(), element.m_timestamp, element.m_saved) = m_cache->getValue(key); - auto error = validate(element, element.m_value.read()); - if (! error.isEmpty()) + auto status = validate(element, element.m_value.read(), CStatusMessage::SeverityDebug); + if (!status.isEmpty()) // intentionally kept !empty here, debug message supposed to write default value { - CLogMessage::preformatted(error); element.m_value.uniqueWrite() = defaultValue; + CLogMessage::preformatted(status); } + return element; } @@ -422,8 +434,8 @@ namespace BlackMisc if (timestamp == 0) { timestamp = QDateTime::currentMSecsSinceEpoch(); } if (element.m_value.read() == value && element.m_timestamp == timestamp) { return {}; } - auto error = validate(element, value); - if (error.isEmpty()) + auto status = validate(element, value, CStatusMessage::SeverityError); + if (status.isSuccess()) { if (m_batchMode > 0) { @@ -439,11 +451,7 @@ namespace BlackMisc emit valuesWantToCache({ { { element.m_key, value } }, timestamp, save }); } } - else - { - CLogMessage::preformatted(error); - } - return error; + return status; } const QString &CValuePage::getKey(const Element &element) const @@ -472,7 +480,7 @@ namespace BlackMisc QList notifySlots; - forEachIntersection(m_elements, values, [changedBy, this, ¬ifySlots, &values](const QString &key, const ElementPtr &element, CValueCachePacket::const_iterator it) + forEachIntersection(m_elements, values, [changedBy, this, ¬ifySlots, &values](const QString & key, const ElementPtr & element, CValueCachePacket::const_iterator it) { if (changedBy == this) // round trip { @@ -481,8 +489,8 @@ namespace BlackMisc } else if (element->m_pendingChanges == 0) // ratify a change only if own change is not pending, to ensure consistency { - auto error = validate(*element, it.value()); - if (error.isEmpty()) + auto error = validate(*element, it.value(), CStatusMessage::SeverityError); + if (error.isSuccess()) { element->m_value.uniqueWrite() = it.value(); element->m_timestamp = it.timestamp(); @@ -529,7 +537,7 @@ namespace BlackMisc if (m_batchMode <= 0 && ! m_batchedValues.empty()) { qint64 timestamp = QDateTime::currentMSecsSinceEpoch(); - forEachIntersection(m_elements, m_batchedValues, [timestamp](const QString &, const ElementPtr &element, CVariantMap::const_iterator it) + forEachIntersection(m_elements, m_batchedValues, [timestamp](const QString &, const ElementPtr & element, CVariantMap::const_iterator it) { Q_ASSERT(isSafeToIncrement(element->m_pendingChanges)); element->m_pendingChanges++; @@ -541,11 +549,11 @@ namespace BlackMisc } } - CStatusMessage CValuePage::validate(const Element &element, const CVariant &value) const + CStatusMessage CValuePage::validate(const Element &element, const CVariant &value, CStatusMessage::StatusSeverity invalidSeverity) const { if (! value.isValid()) { - return CStatusMessage(this, CStatusMessage::SeverityDebug, "Uninitialized value for " + element.m_key); + return CStatusMessage(this, invalidSeverity, "Uninitialized value for " + element.m_key); } else if (value.userType() != element.m_metaType) { diff --git a/src/blackmisc/valuecache.h b/src/blackmisc/valuecache.h index afae38bd6..3baedbec5 100644 --- a/src/blackmisc/valuecache.h +++ b/src/blackmisc/valuecache.h @@ -113,6 +113,9 @@ namespace BlackMisc Distributed //!< Distributed among multiple processes. }; + //! Log categories + static const CLogCategoryList &getLogCategories(); + //! Constructor. //! \param mode Whether or not the cache can be distributed among multiple processes. //! \param parent The parent of the QObject. diff --git a/src/blackmisc/valuecacheprivate.h b/src/blackmisc/valuecacheprivate.h index 533aa3ac9..3856b7bcb 100644 --- a/src/blackmisc/valuecacheprivate.h +++ b/src/blackmisc/valuecacheprivate.h @@ -105,7 +105,7 @@ namespace BlackMisc CVariantMap m_batchedValues; CValuePage(QObject *parent, CValueCache *cache); - CStatusMessage validate(const Element &element, const CVariant &value) const; + CStatusMessage validate(const Element &element, const CVariant &value, CStatusMessage::StatusSeverity invalidSeverity) const; }; } // namespace diff --git a/tests/blackmisc/testvaluecache.cpp b/tests/blackmisc/testvaluecache.cpp index 104cfc8c2..0ae09adf8 100644 --- a/tests/blackmisc/testvaluecache.cpp +++ b/tests/blackmisc/testvaluecache.cpp @@ -227,7 +227,7 @@ namespace BlackMiscTest if (dir.exists()) { dir.removeRecursively(); } auto status = cache.saveToFiles(dir.absolutePath()); - QVERIFY(status.isEmpty()); + QVERIFY(status.isSuccess()); auto files = dir.entryInfoList(QDir::AllEntries | QDir::NoDotAndDotDot, QDir::Name); QCOMPARE(files.size(), 2); @@ -236,7 +236,7 @@ namespace BlackMiscTest CValueCache cache2(CValueCache::LocalOnly); status = cache2.loadFromFiles(dir.absolutePath()); - QVERIFY(status.isEmpty()); + QVERIFY(status.isSuccess()); QCOMPARE(cache2.getAllValues(), testData); }