From b827d6f522acada37e6a27648067a20c6f5c2e29 Mon Sep 17 00:00:00 2001 From: Anton Pryakhin Date: Mon, 14 Oct 2024 16:29:58 +0300 Subject: [PATCH] Address review pt1 Signed-off-by: Anton Pryakhin --- src/groups/mwc/group/mwc.t.dep | 1 + .../mwc/mwctsk/mwctsk_logcontroller.cpp | 163 ------------------ src/groups/mwc/mwctsk/mwctsk_logcontroller.h | 44 ++--- .../mwc/mwctsk/mwctsk_logcontroller.t.cpp | 98 +++++------ 4 files changed, 64 insertions(+), 242 deletions(-) diff --git a/src/groups/mwc/group/mwc.t.dep b/src/groups/mwc/group/mwc.t.dep index 2f1f34bb8..2a33d0d06 100644 --- a/src/groups/mwc/group/mwc.t.dep +++ b/src/groups/mwc/group/mwc.t.dep @@ -1,2 +1,3 @@ benchmark +mqb mwc diff --git a/src/groups/mwc/mwctsk/mwctsk_logcontroller.cpp b/src/groups/mwc/mwctsk/mwctsk_logcontroller.cpp index 3ca41b2c6..f9f7623c6 100644 --- a/src/groups/mwc/mwctsk/mwctsk_logcontroller.cpp +++ b/src/groups/mwc/mwctsk/mwctsk_logcontroller.cpp @@ -229,169 +229,6 @@ void LogControllerConfig::clearCategoriesProperties() d_categories.clear(); } -int LogControllerConfig::fromDatum(bsl::ostream& errorDescription, - const bdld::Datum& datum) -{ - enum RcEnum { - // Value for the various RC error categories - rc_SUCCESS = 0, - rc_INVALID_DATUM = -1, - rc_INVALID_KEY_TYPE = -2, - rc_INVALID_KEY_VALUE = -3, - rc_UNSET_VALUE = -4, - rc_UNKNOWN_KEY = -5 - }; - - if (!datum.isMap()) { - errorDescription << "The specified datum must be a map"; - return rc_INVALID_DATUM; // RETURN - } - -#define PARSE_ENTRY(ENTRY, FIELD, TYPE, KEY_STR, KEY_PATH) \ - if (bdlb::String::areEqualCaseless(ENTRY.key(), KEY_STR)) { \ - if (!ENTRY.value().is##TYPE()) { \ - errorDescription << "Key '" << #KEY_PATH << "' type must be a " \ - << #TYPE; \ - return rc_INVALID_KEY_TYPE; \ - } \ - FIELD = ENTRY.value().the##TYPE(); \ - continue; \ - } - -#define PARSE_CONF(FIELD, TYPE, KEY_STR) \ - PARSE_ENTRY(entry, FIELD, TYPE, KEY_STR, KEY_STR) - -#define PARSE_SYSLOG(FIELD, TYPE, KEY_STR) \ - PARSE_ENTRY(syslog, FIELD, TYPE, KEY_STR, "syslog/" + KEY_STR) - - double fileMaxAgeDays = -1; - double rotationBytes = -1; - bsl::string loggingVerbosityStr; - bsl::string bslsLogSeverityStr; - bsl::string consoleSeverityStr; - bsl::string syslogVerbosityStr; - - // Iterate over each keys of the datum map.. - for (bsl::size_t i = 0; i < datum.theMap().size(); ++i) { - const bdld::DatumMapEntry& entry = datum.theMap().data()[i]; - PARSE_CONF(d_fileName, String, "fileName"); - PARSE_CONF(fileMaxAgeDays, Double, "fileMaxAgeDays"); - PARSE_CONF(rotationBytes, Double, "rotationBytes"); - PARSE_CONF(d_logfileFormat, String, "logfileFormat"); - PARSE_CONF(d_consoleFormat, String, "consoleFormat"); - PARSE_CONF(loggingVerbosityStr, String, "loggingVerbosity"); - PARSE_CONF(bslsLogSeverityStr, String, "bslsLogSeverityThreshold"); - PARSE_CONF(consoleSeverityStr, String, "consoleSeverityThreshold"); - - if (bdlb::String::areEqualCaseless(entry.key(), "categories")) { - if (!entry.value().isArray()) { - errorDescription << "Key 'categories' must be an array"; - return rc_INVALID_KEY_TYPE; // RETURN - } - bdld::DatumArrayRef array = entry.value().theArray(); - for (bsls::Types::size_type idx = 0; idx < array.length(); ++idx) { - if (!array[idx].isString()) { - errorDescription << "Invalid type for categories[" << idx - << "]: must be a string"; - return rc_INVALID_KEY_TYPE; // RETURN - } - int rc = addCategoryProperties(array[idx].theString()); - if (rc != 0) { - errorDescription << "Invalid string format for categories" - << "[" << idx << "] [rc: " << rc << "]"; - return rc_INVALID_KEY_VALUE; // RETURN - } - } - continue; // CONTINUE - } - - if (bdlb::String::areEqualCaseless(entry.key(), "syslog")) { - if (!entry.value().isMap()) { - errorDescription << "Key 'syslog' must be a map"; - return rc_INVALID_KEY_TYPE; // RETURN - } - - bdld::DatumMapRef syslogConfig = entry.value().theMap(); - for (bsl::size_t j = 0; j < syslogConfig.size(); ++j) { - const bdld::DatumMapEntry& syslog = syslogConfig.data()[j]; - - PARSE_SYSLOG(d_syslogEnabled, Boolean, "enabled"); - PARSE_SYSLOG(d_syslogFormat, String, "logFormat"); - PARSE_SYSLOG(d_syslogAppName, String, "appName"); - PARSE_SYSLOG(syslogVerbosityStr, String, "verbosity"); - - // In a normal workflow should just 'continue' - errorDescription << "Unknown key 'syslog/" << syslog.key() - << "'"; - return rc_UNKNOWN_KEY; // RETURN - } - - continue; // CONTINUE - } - - // In a normal workflow should just 'continue' - errorDescription << "Unknown key '" << entry.key() << "'"; - return rc_UNKNOWN_KEY; // RETURN - } - -#undef PARSE_SYSLOG -#undef PARSE_CONF -#undef PARSE_ENTRY - - if (fileMaxAgeDays <= 0) { - errorDescription << "Unset key 'fileMaxAgeDays'"; - return rc_UNSET_VALUE; // RETURN - } - else { - d_fileMaxAgeDays = static_cast(fileMaxAgeDays); - } - - if (rotationBytes <= 0) { - errorDescription << "Unset key 'rotationBytes'"; - return rc_UNSET_VALUE; // RETURN - } - else { - d_rotationBytes = static_cast(rotationBytes); - } - - if (ball::SeverityUtil::fromAsciiCaseless(&d_loggingVerbosity, - loggingVerbosityStr.c_str()) != - 0) { - errorDescription << "Invalid value for 'loggingVerbosity' ('" - << loggingVerbosityStr << "')"; - return rc_INVALID_KEY_VALUE; // RETURN - } - - ball::Severity::Level bslsSeverityAsBal; - if (ball::SeverityUtil::fromAsciiCaseless(&bslsSeverityAsBal, - bslsLogSeverityStr.c_str()) != - 0) { - errorDescription << "Invalid value for 'bslsLogSeverityThreshold' ('" - << bslsLogSeverityStr << "')"; - return rc_INVALID_KEY_VALUE; // RETURN - } - d_bslsLogSeverityThreshold = LogControllerConfig::balToBslsLogLevel( - bslsSeverityAsBal); - - if (ball::SeverityUtil::fromAsciiCaseless(&d_consoleSeverityThreshold, - consoleSeverityStr.c_str()) != - 0) { - errorDescription << "Invalid value for 'consoleSeverityThreshold' ('" - << consoleSeverityStr << "')"; - return rc_INVALID_KEY_VALUE; // RETURN - } - - if (d_syslogEnabled && ball::SeverityUtil::fromAsciiCaseless( - &d_syslogVerbosity, - syslogVerbosityStr.c_str()) != 0) { - errorDescription << "Invalid value for 'syslog/verbosity' ('" - << syslogVerbosityStr << "')"; - return rc_INVALID_KEY_VALUE; // RETURN - } - - return rc_SUCCESS; -} - // ------------------- // class LogController // ------------------- diff --git a/src/groups/mwc/mwctsk/mwctsk_logcontroller.h b/src/groups/mwc/mwctsk/mwctsk_logcontroller.h index a3ad6aa52..10c4d5969 100644 --- a/src/groups/mwc/mwctsk/mwctsk_logcontroller.h +++ b/src/groups/mwc/mwctsk/mwctsk_logcontroller.h @@ -300,16 +300,6 @@ class LogControllerConfig { /// Clear the registered list of category properties. void clearCategoriesProperties(); - /// Populate members of this object from the corresponding fields in the - /// specified `datum`. Return 0 on success, or a non-zero return code - /// on error, populating the specified `errorDescription` with a - /// description of the error. Note that in case of error, some of the - /// values from `datum` may have already been applied and so this object - /// might be partially altered. Refer to the component level - /// documentation (section: "LogControllerConfig: Datum format") for the - /// expected format of the `datum`. - int fromDatum(bsl::ostream& errorDescription, const bdld::Datum& datum); - /// Populate members of this object from the corresponding fields in the /// specified `obj` (which should be of a type compatible with one /// generated from a schema as described at the top in the component @@ -540,7 +530,9 @@ int LogControllerConfig::fromObj(bsl::ostream& errorDescription, .setSyslogEnabled(obj.syslog().enabled()) .setSyslogAppName(obj.syslog().appName()) .setSyslogFormat(obj.syslog().logFormat()) - .setRecordBufferSize(obj.logDump().recordBufferSize()); + .setRecordBufferSize(32768); + // TODO: use obj.logDump() when the config is updated + // .setRecordBufferSize(obj.logDump().recordBufferSize()); if (ball::SeverityUtil::fromAsciiCaseless( &d_loggingVerbosity, @@ -550,21 +542,23 @@ int LogControllerConfig::fromObj(bsl::ostream& errorDescription, return -1; // RETURN } - if (ball::SeverityUtil::fromAsciiCaseless( - &d_recordingVerbosity, - obj.logDump().recordingLevel().c_str()) != 0) { - errorDescription << "Invalid value for 'recordingLevel' ('" - << obj.logDump().recordingLevel() << "')"; - return -1; // RETURN - } + d_recordingVerbosity = ball::Severity::e_TRACE; + d_triggerVerbosity = ball::Severity::e_FATAL; + // if (ball::SeverityUtil::fromAsciiCaseless( + // &d_recordingVerbosity, + // obj.logDump().recordingLevel().c_str()) != 0) { + // errorDescription << "Invalid value for 'recordingLevel' ('" + // << obj.logDump().recordingLevel() << "')"; + // return -1; // RETURN + // } - if (ball::SeverityUtil::fromAsciiCaseless( - &d_triggerVerbosity, - obj.logDump().triggerLevel().c_str()) != 0) { - errorDescription << "Invalid value for 'triggerLevel' ('" - << obj.logDump().triggerLevel() << "')"; - return -1; // RETURN - } + // if (ball::SeverityUtil::fromAsciiCaseless( + // &d_triggerVerbosity, + // obj.logDump().triggerLevel().c_str()) != 0) { + // errorDescription << "Invalid value for 'triggerLevel' ('" + // << obj.logDump().triggerLevel() << "')"; + // return -1; // RETURN + // } ball::Severity::Level bslsSeverityAsBal = ball::Severity::e_ERROR; // TODO: enforcing 'obj' to have 'bslsLogSeverityThreshold' accessor is a diff --git a/src/groups/mwc/mwctsk/mwctsk_logcontroller.t.cpp b/src/groups/mwc/mwctsk/mwctsk_logcontroller.t.cpp index 907f4f137..d4da6b938 100644 --- a/src/groups/mwc/mwctsk/mwctsk_logcontroller.t.cpp +++ b/src/groups/mwc/mwctsk/mwctsk_logcontroller.t.cpp @@ -16,6 +16,9 @@ // mwctsk_logcontroller.t.cpp -*-C++-*- #include +// MQB +#include + // MWC #include @@ -35,7 +38,7 @@ using namespace bsl; // TESTS // ---------------------------------------------------------------------------- -static void test1_logControllerConfigFromDatum() +static void test1_logControllerConfigFromObj() // ------------------------------------------------------------------------ // LOG CONTROLLER CONFIG FROM DATUM // @@ -44,76 +47,63 @@ static void test1_logControllerConfigFromDatum() // - Inner map with SyslogConfig should be processed correctly too. // // Plan: -// 1. Fill the bdld::Datum structure representing LogControllerConfig. -// 2. Initialize the LogControllerConfig with the given bdld::Datum. -// 3. Verify that fromDatum call succeeded. +// 1. Fill the MockObj structure representing LogControllerConfig. +// 2. Initialize the LogControllerConfig with the given MockObj. +// 3. Verify that fromObj call succeeded. // 4. Verify that syslog properties were correctly set. +// 5. Verify that logDump properties were correctly set. // // Testing: -// - LogControllerConfig::fromDatum +// - LogControllerConfig::fromObj // ------------------------------------------------------------------------ { - mwctst::TestHelper::printTestName("LogControllerConfig::fromDatum Test"); - - bdld::DatumMapBuilder syslogBuilder(s_allocator_p); - syslogBuilder.pushBack("enabled", bdld::Datum::createBoolean(true)); - syslogBuilder.pushBack("appName", - bdld::Datum::createStringRef("testapp", - s_allocator_p)); - syslogBuilder.pushBack( - "logFormat", - bdld::Datum::createStringRef("test %d (%t) %s %F:%l %m\n\n", - s_allocator_p)); - syslogBuilder.pushBack("verbosity", - bdld::Datum::createStringRef("info", - s_allocator_p)); - - bdld::DatumArrayBuilder categoriesBuilder(s_allocator_p); - categoriesBuilder.pushBack( - bdld::Datum::copyString("category:info:red", s_allocator_p)); - - bdld::DatumMapBuilder logControllerBuilder(s_allocator_p); - logControllerBuilder.pushBack("fileName", - bdld::Datum::copyString("fileName", - s_allocator_p)); - logControllerBuilder.pushBack("fileMaxAgeDays", - bdld::Datum::createDouble(8.2)); - logControllerBuilder.pushBack("rotationBytes", - bdld::Datum::createDouble(2048)); - logControllerBuilder.pushBack( - "logfileFormat", - bdld::Datum::copyString("%d (%t) %s %F:%l %m\n\n", s_allocator_p)); - logControllerBuilder.pushBack( - "consoleFormat", - bdld::Datum::copyString("%d (%t) %s %F:%l %m\n\n", s_allocator_p)); - logControllerBuilder.pushBack("loggingVerbosity", - bdld::Datum::copyString("debug", - s_allocator_p)); - logControllerBuilder.pushBack("bslsLogSeverityThreshold", - bdld::Datum::copyString("info", - s_allocator_p)); - logControllerBuilder.pushBack("consoleSeverityThreshold", - bdld::Datum::copyString("info", - s_allocator_p)); - logControllerBuilder.pushBack("categories", categoriesBuilder.commit()); - logControllerBuilder.pushBack("syslog", syslogBuilder.commit()); - - bdld::Datum datum = logControllerBuilder.commit(); + mqbcfg::LogController lc(s_allocator_p); + + lc.syslog().enabled() = true; + lc.syslog().appName() = "testapp"; + lc.syslog().logFormat() = "test %d (%t) %s %F:%l %m\n\n"; + lc.syslog().verbosity() = "INFO"; + + lc.categories().push_back("category:info:red"); + // TODO: uncomment when setup of this parameter is uncommented + // lc.logDump().recordBufferSize() = 12345; + // lc.logDump().recordingLevel() = "TRACE"; + // lc.logDump().triggerLevel() = "FATAL"; + + lc.fileName() = "fileName"; + lc.fileMaxAgeDays() = 8; + lc.rotationBytes() = 2048; + lc.logfileFormat() = "%d (%t) %s %F:%l %m\n\n"; + lc.consoleFormat() = "%d (%t) %s %F:%l %m\n\n"; + lc.loggingVerbosity() = "debug"; + // TODO: uncomment when setup of this parameter is uncommented + // lc.bslsLogSeverityThreshold() = "error"; + lc.consoleSeverityThreshold() = "info"; + mwctsk::LogControllerConfig config(s_allocator_p); mwcu::MemOutStream errorDesc(s_allocator_p); - config.fromDatum(errorDesc, datum); - bdld::Datum::destroy(datum, s_allocator_p); + + ASSERT_EQ(config.fromObj(errorDesc, lc), 0); ASSERT_D(errorDesc.str(), errorDesc.str().empty()); + ASSERT_EQ(config.fileName(), "fileName"); ASSERT_EQ(config.fileMaxAgeDays(), 8); ASSERT_EQ(config.rotationBytes(), 2048); + ASSERT_EQ(config.logfileFormat(), "%d (%t) %s %F:%l %m\n\n"); + ASSERT_EQ(config.consoleFormat(), "%d (%t) %s %F:%l %m\n\n"); ASSERT_EQ(config.loggingVerbosity(), ball::Severity::DEBUG); + ASSERT_EQ(config.bslsLogSeverityThreshold(), bsls::LogSeverity::e_ERROR); + ASSERT_EQ(config.consoleSeverityThreshold(), ball::Severity::INFO); ASSERT_EQ(config.syslogEnabled(), true); ASSERT_EQ(config.syslogFormat(), "test %d (%t) %s %F:%l %m\n\n"); ASSERT_EQ(config.syslogAppName(), "testapp"); ASSERT_EQ(config.syslogVerbosity(), ball::Severity::INFO); + + ASSERT_EQ(config.recordBufferSize(), 32768); + ASSERT_EQ(config.recordingVerbosity(), ball::Severity::TRACE); + ASSERT_EQ(config.triggerVerbosity(), ball::Severity::FATAL); } // ============================================================================ @@ -126,7 +116,7 @@ int main(int argc, char* argv[]) switch (_testCase) { case 0: - case 1: test1_logControllerConfigFromDatum(); break; + case 1: test1_logControllerConfigFromObj(); break; default: { cerr << "WARNING: CASE '" << _testCase << "' NOT FOUND." << endl; s_testStatus = -1;