Skip to content
This repository was archived by the owner on Feb 12, 2022. It is now read-only.

Commit 919d175

Browse files
AAlonAAlon
authored
Parameter Namespacing: Refactoring using the ParameterPath object. (#10)
* Refactoring using the ParameterPath object. * Tweaking documentation * Removing implicit conversion of namespace separators and adjusting ClientConfigurationProvider accordingly. * Adding explicit guarantee in doc of parameter_reader.h that the arguments remain unchanged if the query was unsuccessful. * Adjusted client_configuration_provider tests for the new ParameterReader API accepting ParameterPath. * Changing ParameterPath's name_ member to be const.
1 parent f68b7fa commit 919d175

File tree

6 files changed

+164
-76
lines changed

6 files changed

+164
-76
lines changed

‎aws_common/include/aws_common/sdk_utils/aws_error.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ enum AwsError {
4949
action/event */
5050
AWS_ERR_NOT_ENOUGH_SPACE,
5151
/** An error indicating that a data structure is empty. */
52-
AWS_ERR_EMPTY
52+
AWS_ERR_EMPTY,
53+
/** The feature is not supported. */
54+
AWS_ERR_NOT_SUPPORTED,
5355
};
5456
} // namespace Aws

‎aws_common/include/aws_common/sdk_utils/parameter_reader.h‎

Lines changed: 125 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,110 @@
2020
namespace Aws {
2121
namespace Client {
2222

23+
class ParameterPath {
24+
public:
25+
/**
26+
* Legacy constructor for backwards compatibility.
27+
* @hint For ROS1, the node namespace separator is the same as the parameter namespace separator,
28+
* and so the path would simply be the given string (dictated by Ros1NodeParameterReader).
29+
* For ROS2 the separators differ; If only node namespace separators were found, we would treat them as parameter separators.
30+
* @param name Parameter name, which in practice would be the path of the parameter, e.g. config/timeoutMs.
31+
*/
32+
ParameterPath(const char * name) : name_(name) {}
33+
34+
/**
35+
* @example The parameter under node "lidar_node" in namespace "sensor_nodes",
36+
* with parameter namespace "settings" and parameter key "frequency", should be specified as follows:
37+
* node_namespace: ["sensor_nodes", "lidar_node"]
38+
* parameter_path_keys: ["settings", "frequency"]
39+
* @note Node namespaces should be empty if used by a node looking for a parameter of its own (a local parameter).
40+
* @param node_namespaces
41+
* @param parameter_path_keys
42+
*/
43+
ParameterPath(const std::vector<std::string> & node_namespaces, const std::vector<std::string> & parameter_path_keys) :
44+
node_namespaces_(node_namespaces), parameter_path_keys_(parameter_path_keys) {}
45+
46+
/**
47+
* @example The local parameter "timeout" in parameter namespace "config" should be specified as follows:
48+
* ParameterPath("config", "timeout")
49+
* @tparam Args
50+
* @param parameter_path_keys
51+
*/
52+
template<typename ...Args>
53+
ParameterPath(Args... parameter_path_keys) :
54+
parameter_path_keys_(std::vector<std::string>{(parameter_path_keys)...}) {}
55+
56+
/**
57+
* @note Only applies if node_namespaces was specified during construction; otherwise, an empty string is returned.
58+
* @param node_namespace_separator
59+
* @return string The parameter's whereabouts in the node namespace hierarchy.
60+
*/
61+
std::string get_node_path(char node_namespace_separator) const
62+
{
63+
std::string resolved_path;
64+
/* Construct the node's path by the provided lists of keys */
65+
for (auto key = node_namespaces_.begin(); key != node_namespaces_.end(); key++) {
66+
resolved_path += *key + node_namespace_separator;
67+
}
68+
if (!resolved_path.empty() && resolved_path.back() == node_namespace_separator) {
69+
resolved_path.pop_back();
70+
}
71+
return resolved_path;
72+
}
73+
/**
74+
* @note Only applies if parameter_path_keys was specified during construction; otherwise, an empty string is returned.
75+
* @param parameter_namespace_separator
76+
* @return string The parameter path including parameter namespaces but excluding node's namespaces.
77+
*/
78+
std::string get_local_path(char parameter_namespace_separator) const
79+
{
80+
std::string resolved_path;
81+
/* Construct the parameter's path by the provided lists of keys */
82+
for (auto key = parameter_path_keys_.begin(); key != parameter_path_keys_.end(); key++) {
83+
resolved_path += *key + parameter_namespace_separator;
84+
}
85+
if (!resolved_path.empty() && resolved_path.back() == parameter_namespace_separator) {
86+
resolved_path.pop_back();
87+
}
88+
return resolved_path;
89+
}
90+
/**
91+
* Resolves & returns the parameter's path. Supports two separate use cases:
92+
* 1. A single, 'flat' string was provided upon construction.
93+
* In this case, we return it as is.
94+
* 2. Detailed lists of strings were specified for the parameter & node namespaces.
95+
* In this case, we construct the resolved path using the provided separators.
96+
* @param node_namespace_separator
97+
* @param parameter_namespace_separator
98+
* @return string representing the full, resolved path of the parameter.
99+
* @note If node_namespaces and parameter_path_keys are empty, an empty string would be returned.
100+
*/
101+
std::string get_resolved_path(char node_namespace_separator,
102+
char parameter_namespace_separator) const
103+
{
104+
if (!name_.empty()) {
105+
return name_;
106+
} else {
107+
std::string resolved_path = get_node_path(node_namespace_separator);
108+
if (!resolved_path.empty()) {
109+
resolved_path += node_namespace_separator;
110+
}
111+
resolved_path += get_local_path(parameter_namespace_separator);
112+
return resolved_path;
113+
}
114+
}
115+
private:
116+
/**
117+
* Member variables to store the parameter's path in the namespace hierarchy.
118+
* parameter_path_keys_ Parameter namespaces list.
119+
* node_namespaces_ Node namespaces list.
120+
* name_ A single string. Used when simpler initialization is required, instead of the aforementioned namespace lists.
121+
*/
122+
const std::vector<std::string> parameter_path_keys_;
123+
const std::vector<std::string> node_namespaces_;
124+
const std::string name_;
125+
};
126+
23127
class ParameterReaderInterface
24128
{
25129
public:
@@ -28,58 +132,65 @@ class ParameterReaderInterface
28132
/**
29133
* read a list from the provided parameter name
30134
* @param name the name of the parameter to be read
31-
* @param out the output of 'double' type
135+
* @param out the output of 'double' type.
32136
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
137+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
33138
*/
34-
virtual AwsError ReadList(const char * name, std::vector<std::string> & out) const = 0;
139+
virtual AwsError ReadList(const ParameterPath & parameter_path, std::vector<std::string> & out) const = 0;
35140

36141
/**
37142
* read a double value from the provided parameter name
38-
* @param name the name of the parameter to be read
143+
* @param parameter_path an object representing the path of the parameter to be read
39144
* @param out the output of 'double' type
40145
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
146+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
41147
*/
42-
virtual AwsError ReadDouble(const char * name, double & out) const = 0;
148+
virtual AwsError ReadDouble(const ParameterPath & parameter_path, double & out) const = 0;
43149

44150
/**
45151
* read an integer value from the provided parameter name
46-
* @param name the name of the parameter to be read
152+
* @param parameter_path an object representing the path of the parameter to be read
47153
* @param out the output of 'int' type
48154
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
155+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
49156
*/
50-
virtual AwsError ReadInt(const char * name, int & out) const = 0;
157+
virtual AwsError ReadInt(const ParameterPath & parameter_path, int & out) const = 0;
51158

52159
/**
53160
* read a boolean value from the provided parameter name
54-
* @param name the name of the parameter to be read
161+
* @param parameter_path an object representing the path of the parameter to be read
55162
* @param out the output of 'bool' type
56163
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
164+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
57165
*/
58-
virtual AwsError ReadBool(const char * name, bool & out) const = 0;
166+
virtual AwsError ReadBool(const ParameterPath & parameter_path, bool & out) const = 0;
59167

60168
/**
61169
* read a string value from the provided parameter name
62-
* @param name the name of the parameter to be read
170+
* @param parameter_path an object representing the path of the parameter to be read
63171
* @param out the output of 'Aws::String' type
64172
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
173+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
65174
*/
66-
virtual AwsError ReadString(const char * name, Aws::String & out) const = 0;
175+
virtual AwsError ReadString(const ParameterPath & parameter_path, Aws::String & out) const = 0;
67176

68177
/**
69178
* read a string from the provided parameter name
70-
* @param name the name of the parameter to be read
179+
* @param parameter_path an object representing the path of the parameter to be read
71180
* @param out the output of 'std::string' type
72181
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
182+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
73183
*/
74-
virtual AwsError ReadStdString(const char * name, std::string & out) const = 0;
184+
virtual AwsError ReadStdString(const ParameterPath & parameter_path, std::string & out) const = 0;
75185

76186
/**
77187
* read a map from the provided parameter name
78-
* @param name the name of the parameter to be read
188+
* @param parameter_path an object representing the path of the parameter to be read
79189
* @param out the output of 'std::map' type
80190
* @return AWS_ERR_OK if read was successful, AWS_ERR_NOT_FOUND if the parameter was not found
191+
* @note if the return code is not AWS_ERR_OK, out remains unchanged.
81192
*/
82-
virtual AwsError ReadMap(const char * name, std::map<std::string, std::string> & out) const = 0;
193+
virtual AwsError ReadMap(const ParameterPath & parameter_path, std::map<std::string, std::string> & out) const = 0;
83194
};
84195

85196
} // namespace Client

‎aws_common/package.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?xml version="1.0"?>
22
<package format="2">
33
<name>aws_common</name>
4-
<version>1.0.0</version>
4+
<version>2.0.0</version>
55
<description>Common AWS SDK utilities, intended for use by ROS packages using the AWS SDK</description>
66
<url>http://wiki.ros.org/aws_common</url>
77

‎aws_common/src/sdk_utils/client_configuration_provider.cpp‎

Lines changed: 25 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -86,64 +86,39 @@ ClientConfiguration ClientConfigurationProvider::GetClientConfiguration(
8686
* defaults to us-east-1).
8787
*/
8888
config.region = profile_provider.GetProfile().GetRegion();
89-
90-
Aws::String temp_string;
91-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/region", temp_string)) {
92-
config.region = temp_string;
93-
}
89+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "region"), config.region);
9490
PopulateUserAgent(config.userAgent, ros_version_override);
95-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/user_agent", temp_string)) {
96-
config.userAgent = temp_string;
97-
}
98-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/endpoint_override", temp_string)) {
99-
config.endpointOverride = temp_string;
100-
}
101-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/proxy_host", temp_string)) {
102-
config.proxyHost = temp_string;
103-
}
104-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/proxy_user_name", temp_string)) {
105-
config.proxyUserName = temp_string;
106-
}
107-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/proxy_password", temp_string)) {
108-
config.proxyPassword = temp_string;
109-
}
110-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/ca_path", temp_string)) {
111-
config.caPath = temp_string;
112-
}
113-
if (AWS_ERR_OK == reader_->ReadString(CLIENT_CONFIG_PREFIX "/ca_file", temp_string)) {
114-
config.caFile = temp_string;
115-
}
91+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "user_agent"), config.userAgent);
92+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "endpoint_override"), config.endpointOverride);
93+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "proxy_host"), config.proxyHost);
94+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "proxy_user_name"), config.proxyUserName);
95+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "proxy_password"), config.proxyPassword);
96+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "ca_path"), config.caPath);
97+
reader_->ReadString(ParameterPath(CLIENT_CONFIG_PREFIX, "ca_file"), config.caFile);
11698

117-
int temp_int;
118-
if (AWS_ERR_OK == reader_->ReadInt(CLIENT_CONFIG_PREFIX "/request_timeout_ms", temp_int)) {
119-
config.requestTimeoutMs = temp_int;
99+
int temp;
100+
if (AWS_ERR_OK == reader_->ReadInt(ParameterPath(CLIENT_CONFIG_PREFIX, "request_timeout_ms"), temp)) {
101+
config.requestTimeoutMs = temp;
120102
}
121-
if (AWS_ERR_OK == reader_->ReadInt(CLIENT_CONFIG_PREFIX "/connect_timeout_ms", temp_int)) {
122-
config.connectTimeoutMs = temp_int;
103+
if (AWS_ERR_OK == reader_->ReadInt(ParameterPath(CLIENT_CONFIG_PREFIX, "connect_timeout_ms"), temp)) {
104+
config.connectTimeoutMs = temp;
123105
}
124-
if (AWS_ERR_OK == reader_->ReadInt(CLIENT_CONFIG_PREFIX "/max_connections", temp_int)) {
125-
config.maxConnections = temp_int;
106+
if (AWS_ERR_OK == reader_->ReadInt(ParameterPath(CLIENT_CONFIG_PREFIX, "max_connections"), temp)) {
107+
config.maxConnections = temp;
126108
}
127-
if (AWS_ERR_OK == reader_->ReadInt(CLIENT_CONFIG_PREFIX "/proxy_port", temp_int)) {
128-
config.proxyPort = temp_int;
109+
if (AWS_ERR_OK == reader_->ReadInt(ParameterPath(CLIENT_CONFIG_PREFIX, "proxy_port"), temp)) {
110+
config.proxyPort = temp;
129111
}
130112

131-
bool temp_bool;
132-
if (AWS_ERR_OK == reader_->ReadBool(CLIENT_CONFIG_PREFIX "/use_dual_stack", temp_bool)) {
133-
config.useDualStack = temp_bool;
134-
}
135-
if (AWS_ERR_OK == reader_->ReadBool(CLIENT_CONFIG_PREFIX "/enable_clock_skew_adjustment", temp_bool)) {
136-
config.enableClockSkewAdjustment = temp_bool;
137-
}
138-
if (AWS_ERR_OK == reader_->ReadBool(CLIENT_CONFIG_PREFIX "/follow_redirects", temp_bool)) {
139-
config.followRedirects = temp_bool;
140-
}
141-
if (AWS_ERR_OK == reader_->ReadBool(CLIENT_CONFIG_PREFIX "/verify_SSL", temp_bool)) {
142-
config.verifySSL = temp_bool;
143-
}
113+
reader_->ReadBool(ParameterPath(CLIENT_CONFIG_PREFIX, "use_dual_stack"), config.useDualStack);
114+
reader_->ReadBool(ParameterPath(CLIENT_CONFIG_PREFIX, "enable_clock_skew_adjustment"),
115+
config.enableClockSkewAdjustment);
116+
reader_->ReadBool(ParameterPath(CLIENT_CONFIG_PREFIX, "follow_redirects"), config.followRedirects);
117+
reader_->ReadBool(ParameterPath(CLIENT_CONFIG_PREFIX, "verify_SSL"), config.verifySSL);
144118

145-
if (AWS_ERR_OK == reader_->ReadInt(CLIENT_CONFIG_PREFIX "/max_retries", temp_int)) {
146-
config.retryStrategy = std::make_shared<Aws::Client::DefaultRetryStrategy>(temp_int);
119+
int max_retries;
120+
if (AWS_ERR_OK == reader_->ReadInt(ParameterPath(CLIENT_CONFIG_PREFIX, "max_retries"), max_retries)) {
121+
config.retryStrategy = std::make_shared<Aws::Client::DefaultRetryStrategy>(max_retries);
147122
}
148123

149124
return config;

‎aws_common/test/include/aws_common/sdk_utils/parameter_reader_mock.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ namespace Client {
2323
class ParameterReaderMock : public ParameterReaderInterface
2424
{
2525
public:
26-
MOCK_CONST_METHOD2(ReadList, Aws::AwsError(const char *, std::vector<std::string> &));
27-
MOCK_CONST_METHOD2(ReadDouble, Aws::AwsError(const char *, double &));
28-
MOCK_CONST_METHOD2(ReadInt, Aws::AwsError(const char *, int &));
29-
MOCK_CONST_METHOD2(ReadBool, Aws::AwsError(const char *, bool &));
30-
MOCK_CONST_METHOD2(ReadString, Aws::AwsError(const char *, Aws::String &));
31-
MOCK_CONST_METHOD2(ReadStdString, Aws::AwsError(const char *, std::string &));
32-
MOCK_CONST_METHOD2(ReadMap, Aws::AwsError(const char *, std::map<std::string, std::string> &));
26+
MOCK_CONST_METHOD2(ReadList, Aws::AwsError(const ParameterPath &, std::vector<std::string> &));
27+
MOCK_CONST_METHOD2(ReadDouble, Aws::AwsError(const ParameterPath &, double &));
28+
MOCK_CONST_METHOD2(ReadInt, Aws::AwsError(const ParameterPath &, int &));
29+
MOCK_CONST_METHOD2(ReadBool, Aws::AwsError(const ParameterPath &, bool &));
30+
MOCK_CONST_METHOD2(ReadString, Aws::AwsError(const ParameterPath &, Aws::String &));
31+
MOCK_CONST_METHOD2(ReadStdString, Aws::AwsError(const ParameterPath &, std::string &));
32+
MOCK_CONST_METHOD2(ReadMap, Aws::AwsError(const ParameterPath &, std::map<std::string, std::string> &));
3333
};
3434

3535
} // namespace Client

‎aws_common/test/sdk_utils/client_configuration_provider_test.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,11 @@ TEST_F(ClientConfigurationProviderFixture, TestAllReaderErrorsIgnored)
7575

7676
ClientConfiguration config = test_subject_->GetClientConfiguration();
7777

78-
EXPECT_NE(unexpected_str, config.region);
78+
EXPECT_EQ(unexpected_str, config.region);
7979
EXPECT_NE(unkExpectedInt, config.maxConnections);
80-
EXPECT_NE(unkExpectedBool, config.useDualStack);
80+
EXPECT_EQ(unkExpectedBool, config.useDualStack);
8181
EXPECT_EQ(default_config.maxConnections, config.maxConnections);
82-
EXPECT_EQ(default_config.useDualStack, config.useDualStack);
82+
EXPECT_NE(default_config.useDualStack, config.useDualStack);
8383
}
8484

8585
TEST_F(ClientConfigurationProviderFixture, TestRosVersionOverride) {

0 commit comments

Comments
 (0)