Skip to content

Commit

Permalink
Address review pt1
Browse files Browse the repository at this point in the history 
Signed-off-by: Anton Pryakhin <[email protected]>
  • Loading branch information
@waldgange
waldgange committed Oct 16, 2024
1 parent 97ce8e6 commit b827d6f
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 242 deletions.
1 change: 1 addition & 0 deletions src/groups/mwc/group/mwc.t.dep
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
benchmark
mqb
mwc
163 changes: 0 additions & 163 deletions src/groups/mwc/mwctsk/mwctsk_logcontroller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<int>(fileMaxAgeDays);
}

if (rotationBytes <= 0) {
errorDescription << "Unset key 'rotationBytes'";
return rc_UNSET_VALUE; // RETURN
}
else {
d_rotationBytes = static_cast<int>(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
// -------------------
Expand Down
44 changes: 19 additions & 25 deletions src/groups/mwc/mwctsk/mwctsk_logcontroller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
98 changes: 44 additions & 54 deletions src/groups/mwc/mwctsk/mwctsk_logcontroller.t.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
// mwctsk_logcontroller.t.cpp -*-C++-*-
#include <mwctsk_logcontroller.h>

// MQB
#include <mqbcfg_messages.h>

// MWC
#include <mwcu_memoutstream.h>

Expand All @@ -35,7 +38,7 @@ using namespace bsl;
// TESTS
// ----------------------------------------------------------------------------

static void test1_logControllerConfigFromDatum()
static void test1_logControllerConfigFromObj()
// ------------------------------------------------------------------------
// LOG CONTROLLER CONFIG FROM DATUM
//
Expand All @@ -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<mqbcfg::LogController>(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);
}

// ============================================================================
Expand All @@ -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;
Expand Down

0 comments on commit b827d6f

Please sign in to comment.