Clean up Section in BasicConfig.h:

The following changes were made:
- Removed dependency on template defined in beast detail namespace.
- Removed Section::find() method which had an obsolete interface.
- Made Section::get<>() easier to use for the common case of
  retrieving a std::string.  The revised get() method replaces old
  calls to Section::find().
- Provided a default template parameter to free function
  get<>(Section config, std::string name) so it stays similar to
  Section::get<>().

Then the rest of the code was adapted to these changes.

- Calls to Section::find() were replaced with calls to Section::get.
- Unnecessary get<std::string>() arguments were reduced to get().

These changes dug up an interesting artifact in the SHAMap unit
tests.  I'm not sure why the tests were working before, but there
was a problem with the case of a Section key.  The unit test is
fixed.
This commit is contained in:
Scott Schurr
2021-05-06 18:08:12 -07:00
committed by manojsdoshi
parent b9943d3746
commit a1fd579756
19 changed files with 172 additions and 127 deletions

View File

@@ -32,13 +32,13 @@ public:
CollectorManagerImp(Section const& params, beast::Journal journal)
: m_journal(journal)
{
std::string const& server = get<std::string>(params, "server");
std::string const& server = get(params, "server");
if (server == "statsd")
{
beast::IP::Endpoint const address(beast::IP::Endpoint::from_string(
get<std::string>(params, "address")));
std::string const& prefix(get<std::string>(params, "prefix"));
beast::IP::Endpoint const address(
beast::IP::Endpoint::from_string(get(params, "address")));
std::string const& prefix(get(params, "prefix"));
m_collector =
beast::insight::StatsDCollector::New(address, prefix, journal);

View File

@@ -431,18 +431,17 @@ GRPCServerImpl::GRPCServerImpl(Application& app)
{
Section section = app_.config().section("port_grpc");
std::pair<std::string, bool> ipPair = section.find("ip");
if (!ipPair.second)
auto const optIp = section.get("ip");
if (!optIp)
return;
std::pair<std::string, bool> portPair = section.find("port");
if (!portPair.second)
auto const optPort = section.get("port");
if (!optPort)
return;
try
{
boost::asio::ip::tcp::endpoint endpoint(
boost::asio::ip::make_address(ipPair.first),
std::stoi(portPair.first));
boost::asio::ip::make_address(*optIp), std::stoi(*optPort));
std::stringstream ss;
ss << endpoint;
@@ -451,16 +450,15 @@ GRPCServerImpl::GRPCServerImpl(Application& app)
catch (std::exception const&)
{
JLOG(journal_.error()) << "Error setting grpc server address";
Throw<std::exception>();
Throw<std::runtime_error>("Error setting grpc server address");
}
std::pair<std::string, bool> secureGateway =
section.find("secure_gateway");
if (secureGateway.second)
auto const optSecureGateway = section.get("secure_gateway");
if (optSecureGateway)
{
try
{
std::stringstream ss{secureGateway.first};
std::stringstream ss{*optSecureGateway};
std::string ip;
while (std::getline(ss, ip, ','))
{
@@ -472,7 +470,8 @@ GRPCServerImpl::GRPCServerImpl(Application& app)
JLOG(journal_.error())
<< "Can't pass unspecified IP in "
<< "secure_gateway section of port_grpc";
Throw<std::exception>();
Throw<std::runtime_error>(
"Unspecified IP in secure_gateway section");
}
secureGatewayIPs_.emplace_back(addr);
@@ -482,7 +481,8 @@ GRPCServerImpl::GRPCServerImpl(Application& app)
{
JLOG(journal_.error())
<< "Error parsing secure gateway IPs for grpc server";
Throw<std::exception>();
Throw<std::runtime_error>(
"Error parsing secure_gateway section");
}
}
}

View File

@@ -100,7 +100,7 @@ SHAMapStoreImp::SHAMapStoreImp(
}
// RocksDB only. Use sensible defaults if no values specified.
if (boost::iequals(get<std::string>(section, "type"), "RocksDB"))
if (boost::iequals(get(section, "type"), "RocksDB"))
{
if (!section.exists("cache_mb"))
{
@@ -419,7 +419,7 @@ void
SHAMapStoreImp::dbPaths()
{
Section section{app_.config().section(ConfigSection::nodeDatabase())};
boost::filesystem::path dbPath = get<std::string>(section, "path");
boost::filesystem::path dbPath = get(section, "path");
if (boost::filesystem::exists(dbPath))
{
@@ -493,7 +493,7 @@ SHAMapStoreImp::dbPaths()
<< "The existing data is in a corrupted state.\n"
<< "To resume operation, remove the files matching "
<< stateDbPathName.string() << " and contents of the directory "
<< get<std::string>(section, "path") << '\n'
<< get(section, "path") << '\n'
<< "Optionally, you can move those files to another\n"
<< "location if you wish to analyze or back up the data.\n"
<< "However, there is no guarantee that the data in its\n"
@@ -515,7 +515,7 @@ SHAMapStoreImp::makeBackendRotating(std::string path)
}
else
{
boost::filesystem::path p = get<std::string>(section, "path");
boost::filesystem::path p = get(section, "path");
p /= dbPrefix_;
p += ".%%%%";
newPath = boost::filesystem::unique_path(p);

View File

@@ -54,8 +54,7 @@ RelationalDBInterface::init(
const Section& rdb_section{config.section(SECTION_RELATIONAL_DB)};
if (!rdb_section.empty())
{
if (boost::iequals(
get<std::string>(rdb_section, "backend"), "sqlite"))
if (boost::iequals(get(rdb_section, "backend"), "sqlite"))
{
use_sqlite = true;
}
@@ -63,7 +62,7 @@ RelationalDBInterface::init(
{
Throw<std::runtime_error>(
"Invalid rdb_section backend value: " +
get<std::string>(rdb_section, "backend"));
get(rdb_section, "backend"));
}
}
else

View File

@@ -27,6 +27,8 @@
#include <boost/asio/ip/tcp.hpp>
#include <boost/beast/core.hpp>
#include <boost/beast/websocket.hpp>
#include <cctype>
#include <charconv>
#include <cstdlib>
#include <iostream>
#include <string>
@@ -855,29 +857,26 @@ ReportingETL::ReportingETL(Application& app)
JLOG(journal_.debug()) << "val is " << v;
Section source = app_.config().section(v);
std::pair<std::string, bool> ipPair = source.find("source_ip");
if (!ipPair.second)
auto optIp = source.get("source_ip");
if (!optIp)
continue;
std::pair<std::string, bool> wsPortPair =
source.find("source_ws_port");
if (!wsPortPair.second)
auto optWsPort = source.get("source_ws_port");
if (!optWsPort)
continue;
std::pair<std::string, bool> grpcPortPair =
source.find("source_grpc_port");
if (!grpcPortPair.second)
auto optGrpcPort = source.get("source_grpc_port");
if (!optGrpcPort)
{
// add source without grpc port
// used in read-only mode to detect when new ledgers have
// been validated. Used for publishing
if (app_.config().reportingReadOnly())
loadBalancer_.add(ipPair.first, wsPortPair.first);
loadBalancer_.add(*optIp, *optWsPort);
continue;
}
loadBalancer_.add(
ipPair.first, wsPortPair.first, grpcPortPair.first);
loadBalancer_.add(*optIp, *optWsPort, *optGrpcPort);
}
// this is true iff --reportingReadOnly was passed via command line
@@ -887,37 +886,65 @@ ReportingETL::ReportingETL(Application& app)
// file. Command line takes precedence
if (!readOnly_)
{
std::pair<std::string, bool> ro = section.find("read_only");
if (ro.second)
auto const optRO = section.get("read_only");
if (optRO)
{
readOnly_ = (ro.first == "true" || ro.first == "1");
readOnly_ = (*optRO == "true" || *optRO == "1");
app_.config().setReportingReadOnly(readOnly_);
}
}
// lambda throws a useful message if string to integer conversion fails
auto asciiToIntThrows =
[](auto& dest, std::string const& src, char const* onError) {
char const* const srcEnd = src.data() + src.size();
auto [ptr, err] = std::from_chars(src.data(), srcEnd, dest);
if (err == std::errc())
// skip whitespace at end of string
while (ptr != srcEnd &&
std::isspace(static_cast<unsigned char>(*ptr)))
++ptr;
// throw if
// o conversion error or
// o entire string is not consumed
if (err != std::errc() || ptr != srcEnd)
Throw<std::runtime_error>(onError + src);
};
// handle command line arguments
if (app_.config().START_UP == Config::StartUpType::FRESH && !readOnly_)
{
startSequence_ = std::stol(app_.config().START_LEDGER);
asciiToIntThrows(
*startSequence_,
app_.config().START_LEDGER,
"Expected integral START_LEDGER command line argument. Got: ");
}
// if not passed via command line, check config for start sequence
if (!startSequence_)
{
std::pair<std::string, bool> start = section.find("start_sequence");
if (start.second)
{
startSequence_ = std::stoi(start.first);
}
auto const optStartSeq = section.get("start_sequence");
if (optStartSeq)
asciiToIntThrows(
*startSequence_,
*optStartSeq,
"Expected integral start_sequence config entry. Got: ");
}
std::pair<std::string, bool> flushInterval =
section.find("flush_interval");
if (flushInterval.second)
flushInterval_ = std::stoi(flushInterval.first);
auto const optFlushInterval = section.get("flush_interval");
if (optFlushInterval)
asciiToIntThrows(
flushInterval_,
*optFlushInterval,
"Expected integral flush_interval config entry. Got: ");
std::pair<std::string, bool> numMarkers = section.find("num_markers");
if (numMarkers.second)
numMarkers_ = std::stoi(numMarkers.first);
auto const optNumMarkers = section.get("num_markers");
if (optNumMarkers)
asciiToIntThrows(
numMarkers_,
*optNumMarkers,
"Expected integral num_markers config entry. Got: ");
}
}

View File

@@ -24,7 +24,6 @@
#include <boost/beast/core/string.hpp>
#include <boost/lexical_cast.hpp>
#include <algorithm>
#include <beast/unit_test/detail/const_container.hpp>
#include <map>
#include <optional>
#include <ostream>
@@ -40,15 +39,17 @@ using IniFileSections = std::map<std::string, std::vector<std::string>>;
/** Holds a collection of configuration values.
A configuration file contains zero or more sections.
*/
class Section : public beast::unit_test::detail::const_container<
std::map<std::string, std::string, boost::beast::iless>>
class Section
{
private:
std::string name_;
std::map<std::string, std::string> lookup_;
std::vector<std::string> lines_;
std::vector<std::string> values_;
bool had_trailing_comments_ = false;
using const_iterator = decltype(lookup_)::const_iterator;
public:
/** Create an empty section. */
explicit Section(std::string const& name = "");
@@ -132,18 +133,12 @@ public:
bool
exists(std::string const& name) const;
/** Retrieve a key/value pair.
@return A pair with bool `true` if the string was found.
*/
std::pair<std::string, bool>
find(std::string const& name) const;
template <class T>
template <class T = std::string>
std::optional<T>
get(std::string const& name) const
{
auto const iter = cont().find(name);
if (iter == cont().end())
auto const iter = lookup_.find(name);
if (iter == lookup_.end())
return std::nullopt;
return boost::lexical_cast<T>(iter->second);
}
@@ -167,6 +162,48 @@ public:
friend std::ostream&
operator<<(std::ostream&, Section const& section);
// Returns `true` if there are no key/value pairs.
bool
empty() const
{
return lookup_.empty();
}
// Returns the number of key/value pairs.
std::size_t
size() const
{
return lookup_.size();
}
// For iteration of key/value pairs.
const_iterator
begin() const
{
return lookup_.cbegin();
}
// For iteration of key/value pairs.
const_iterator
cbegin() const
{
return lookup_.cbegin();
}
// For iteration of key/value pairs.
const_iterator
end() const
{
return lookup_.cend();
}
// For iteration of key/value pairs.
const_iterator
cend() const
{
return lookup_.cend();
}
};
//------------------------------------------------------------------------------
@@ -311,7 +348,7 @@ set(T& target,
and can be parsed, or else defaultValue.
*/
// NOTE This routine might be more clumsy than the previous two
template <class T>
template <class T = std::string>
T
get(Section const& section,
std::string const& name,
@@ -332,7 +369,7 @@ get(Section const& section, std::string const& name, const char* defaultValue)
{
try
{
auto const val = section.get<std::string>(name);
auto const val = section.get(name);
if (val.has_value())
return *val;
}

View File

@@ -31,9 +31,7 @@ Section::Section(std::string const& name) : name_(name)
void
Section::set(std::string const& key, std::string const& value)
{
auto const result = cont().emplace(key, value);
if (!result.second)
result.first->second = value;
lookup_.insert_or_assign(key, value);
}
void
@@ -106,22 +104,13 @@ Section::append(std::vector<std::string> const& lines)
bool
Section::exists(std::string const& name) const
{
return cont().find(name) != cont().end();
}
std::pair<std::string, bool>
Section::find(std::string const& name) const
{
auto const iter = cont().find(name);
if (iter == cont().end())
return {{}, false};
return {iter->second, true};
return lookup_.find(name) != lookup_.end();
}
std::ostream&
operator<<(std::ostream& os, Section const& section)
{
for (auto const& [k, v] : section.cont())
for (auto const& [k, v] : section.lookup_)
os << k << "=" << v << "\n";
return os;
}

View File

@@ -347,7 +347,7 @@ PgPool::PgPool(Section const& pgConfig, beast::Journal j) : j_(j)
// The connection object must be freed using the libpq API PQfinish() call.
pg_connection_type conn(
PQconnectdb(get<std::string>(pgConfig, "conninfo").c_str()),
PQconnectdb(get(pgConfig, "conninfo").c_str()),
[](PGconn* conn) { PQfinish(conn); });
if (!conn)
Throw<std::runtime_error>("Can't create DB connection.");

View File

@@ -671,7 +671,7 @@ Config::loadFromString(std::string const& fileContents)
try
{
if (auto val = sec.get<std::string>("max_unknown_time"))
if (auto val = sec.get("max_unknown_time"))
MAX_UNKNOWN_TIME =
seconds{beast::lexicalCastThrow<std::uint32_t>(*val)};
}
@@ -689,7 +689,7 @@ Config::loadFromString(std::string const& fileContents)
try
{
if (auto val = sec.get<std::string>("max_diverged_time"))
if (auto val = sec.get("max_diverged_time"))
MAX_DIVERGED_TIME =
seconds{beast::lexicalCastThrow<std::uint32_t>(*val)};
}

View File

@@ -175,8 +175,7 @@ public:
Throw<std::runtime_error>(
"nodestore:: Failed to create CassCluster");
std::string secureConnectBundle =
get<std::string>(config_, "secure_connect_bundle");
std::string secureConnectBundle = get(config_, "secure_connect_bundle");
if (!secureConnectBundle.empty())
{
@@ -196,8 +195,7 @@ public:
}
else
{
std::string contact_points =
get<std::string>(config_, "contact_points");
std::string contact_points = get(config_, "contact_points");
if (contact_points.empty())
{
Throw<std::runtime_error>(
@@ -241,16 +239,14 @@ public:
Throw<std::runtime_error>(ss.str());
}
std::string username = get<std::string>(config_, "username");
std::string username = get(config_, "username");
if (username.size())
{
std::cout << "user = " << username.c_str() << " password = "
<< get<std::string>(config_, "password").c_str()
std::cout << "user = " << username
<< " password = " << get(config_, "password")
<< std::endl;
cass_cluster_set_credentials(
cluster,
username.c_str(),
get<std::string>(config_, "password").c_str());
cluster, username.c_str(), get(config_, "password").c_str());
}
unsigned int const workers = std::thread::hardware_concurrency();
@@ -280,7 +276,7 @@ public:
;
}
std::string certfile = get<std::string>(config_, "certfile");
std::string certfile = get(config_, "certfile");
if (certfile.size())
{
std::ifstream fileStream(
@@ -318,14 +314,14 @@ public:
cass_ssl_free(context);
}
std::string keyspace = get<std::string>(config_, "keyspace");
std::string keyspace = get(config_, "keyspace");
if (keyspace.empty())
{
Throw<std::runtime_error>(
"nodestore: Missing keyspace in Cassandra config");
}
std::string tableName = get<std::string>(config_, "table_name");
std::string tableName = get(config_, "table_name");
if (tableName.empty())
{
Throw<std::runtime_error>(

View File

@@ -90,7 +90,7 @@ public:
size_t keyBytes,
Section const& keyValues,
beast::Journal journal)
: name_(get<std::string>(keyValues, "path")), journal_(journal)
: name_(get(keyValues, "path")), journal_(journal)
{
boost::ignore_unused(journal_); // Keep unused journal_ just in case.
if (name_.empty())

View File

@@ -61,7 +61,7 @@ public:
: j_(journal)
, keyBytes_(keyBytes)
, burstSize_(burstSize)
, name_(get<std::string>(keyValues, "path"))
, name_(get(keyValues, "path"))
, deletePath_(false)
, scheduler_(scheduler)
{
@@ -80,7 +80,7 @@ public:
: j_(journal)
, keyBytes_(keyBytes)
, burstSize_(burstSize)
, name_(get<std::string>(keyValues, "path"))
, name_(get(keyValues, "path"))
, db_(context)
, deletePath_(false)
, scheduler_(scheduler)

View File

@@ -172,9 +172,7 @@ public:
if (keyValues.exists("bbt_options"))
{
auto const s = rocksdb::GetBlockBasedTableOptionsFromString(
table_options,
get<std::string>(keyValues, "bbt_options"),
&table_options);
table_options, get(keyValues, "bbt_options"), &table_options);
if (!s.ok())
Throw<std::runtime_error>(
std::string("Unable to set RocksDB bbt_options: ") +
@@ -186,7 +184,7 @@ public:
if (keyValues.exists("options"))
{
auto const s = rocksdb::GetOptionsFromString(
m_options, get<std::string>(keyValues, "options"), &m_options);
m_options, get(keyValues, "options"), &m_options);
if (!s.ok())
Throw<std::runtime_error>(
std::string("Unable to set RocksDB options: ") +

View File

@@ -1300,7 +1300,7 @@ DatabaseShardImp::initConfig(std::lock_guard<std::mutex> const&)
}
// NuDB is the default and only supported permanent storage backend
backendName_ = get<std::string>(section, "type", "nudb");
backendName_ = get(section, "type", "nudb");
if (!boost::iequals(backendName_, "NuDB"))
return fail("'type' value unsupported");

View File

@@ -84,7 +84,7 @@ DeterministicShard::init(Serializer const& finalKey)
Config const& config{app_.config()};
Section section{config.section(ConfigSection::shardDatabase())};
auto const type{get<std::string>(section, "type", "nudb")};
auto const type{get(section, "type", "nudb")};
auto const factory{Manager::instance().find(type)};
if (!factory)
return fail("failed to find factory for " + type);

View File

@@ -48,7 +48,7 @@ ManagerImp::make_Backend(
Scheduler& scheduler,
beast::Journal journal)
{
std::string const type{get<std::string>(parameters, "type")};
std::string const type{get(parameters, "type")};
if (type.empty())
missing_backend();

View File

@@ -99,7 +99,7 @@ bool
Shard::init(Scheduler& scheduler, nudb::context& context)
{
Section section{app_.config().section(ConfigSection::shardDatabase())};
std::string const type{get<std::string>(section, "type", "nudb")};
std::string const type{get(section, "type", "nudb")};
auto const factory{Manager::instance().find(type)};
if (!factory)
{

View File

@@ -76,10 +76,10 @@ populate(
bool allowAllIps,
std::vector<beast::IP::Address> const& admin_ip)
{
auto const result = section.find(field);
if (result.second)
auto const optResult = section.get(field);
if (optResult)
{
std::stringstream ss(result.first);
std::stringstream ss(*optResult);
std::string ip;
bool has_any(false);
@@ -139,30 +139,29 @@ void
parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
{
{
auto result = section.find("ip");
if (result.second)
auto const optResult = section.get("ip");
if (optResult)
{
try
{
port.ip = boost::asio::ip::address::from_string(result.first);
port.ip = boost::asio::ip::address::from_string(*optResult);
}
catch (std::exception const&)
{
log << "Invalid value '" << result.first
<< "' for key 'ip' in [" << section.name() << "]";
log << "Invalid value '" << *optResult << "' for key 'ip' in ["
<< section.name() << "]";
Rethrow();
}
}
}
{
auto const result = section.find("port");
if (result.second)
auto const optResult = section.get("port");
if (optResult)
{
try
{
port.port =
beast::lexicalCastThrow<std::uint16_t>(result.first);
port.port = beast::lexicalCastThrow<std::uint16_t>(*optResult);
// Port 0 is not supported
if (*port.port == 0)
@@ -170,7 +169,7 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
}
catch (std::exception const&)
{
log << "Invalid value '" << result.first << "' for key "
log << "Invalid value '" << *optResult << "' for key "
<< "'port' in [" << section.name() << "]";
Rethrow();
}
@@ -178,11 +177,11 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
}
{
auto const result = section.find("protocol");
if (result.second)
auto const optResult = section.get("protocol");
if (optResult)
{
for (auto const& s : beast::rfc2616::split_commas(
result.first.begin(), result.first.end()))
optResult->begin(), optResult->end()))
port.protocol.insert(s);
}
}
@@ -207,13 +206,13 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
}
{
auto const result = section.find("send_queue_limit");
if (result.second)
auto const optResult = section.get("send_queue_limit");
if (optResult)
{
try
{
port.ws_queue_limit =
beast::lexicalCastThrow<std::uint16_t>(result.first);
beast::lexicalCastThrow<std::uint16_t>(*optResult);
// Queue must be greater than 0
if (port.ws_queue_limit == 0)
@@ -221,7 +220,7 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
}
catch (std::exception const&)
{
log << "Invalid value '" << result.first << "' for key "
log << "Invalid value '" << *optResult << "' for key "
<< "'send_queue_limit' in [" << section.name() << "]";
Rethrow();
}

View File

@@ -57,7 +57,7 @@ public:
{
Section testSection;
testSection.set("type", "memory");
testSection.set("Path", "SHAMap_test");
testSection.set("path", "SHAMap_test");
db_ = NodeStore::Manager::instance().make_Database(
megabytes(4), scheduler_, 1, testSection, j);
}