Skip to content

Commit 76bde21

Browse files
kai-ionkai-ion
committed
added 3 more tests verifying duplication urls, modify the services object with more encapsulation
1 parent de8e6cb commit 76bde21

File tree

3 files changed

+107
-17
lines changed

3 files changed

+107
-17
lines changed

‎src/aws-cpp-sdk-core/include/aws/core/config/AWSProfileConfig.h‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,14 @@ namespace Aws
2626
class Services
2727
{
2828
public:
29-
inline const Aws::Map<Aws::String, Aws::String>& GetEndpoints() const { return m_endpoints; }
30-
inline const Aws::String& GetServiceBlockName() const { return m_name; }
29+
Services() = default;
30+
Services(Aws::Map<Aws::String, Aws::String>&& endpoints, Aws::String name)
31+
: m_endpoints(std::move(endpoints)), m_name(std::move(name)) {}
32+
33+
inline Aws::Map<Aws::String, Aws::String> GetEndpoints() const { return m_endpoints; }
34+
inline Aws::String GetServiceBlockName() const { return m_name; }
3135
inline bool IsSet() const { return !m_name.empty(); }
3236
private:
33-
friend class ConfigFileProfileFSM;
34-
void SetEndpoints(Aws::Map<Aws::String, Aws::String>&& endpoints, Aws::String name) {
35-
m_endpoints = std::move(endpoints);
36-
m_name = std::move(name);
37-
}
3837
Aws::Map<Aws::String, Aws::String> m_endpoints;
3938
Aws::String m_name;
4039
};
@@ -114,8 +113,9 @@ namespace Aws
114113
}
115114

116115
inline const Services& GetServices() const { return m_services; }
116+
inline void SetServices(Services&& services) { m_services = std::move(services); }
117117

118-
inline Aws::Crt::Optional<Aws::String> GetEndpointUrl() const {
118+
inline Aws::Crt::Optional<Aws::String> GetGlobalEndpointUrl() const {
119119
const Aws::String& endpoint = GetValue("endpoint_url");
120120
return endpoint.empty() ? Aws::Crt::Optional<Aws::String>() : Aws::Crt::Optional<Aws::String>(endpoint);
121121
}
@@ -126,7 +126,6 @@ namespace Aws
126126
inline void SetSsoSession(SsoSession&& value) { m_ssoSessionSet = true; m_ssoSession = std::move(value); }
127127

128128
private:
129-
friend class ConfigFileProfileFSM;
130129
Aws::String m_name;
131130
Aws::String m_region;
132131
Aws::Auth::AWSCredentials m_credentials;

‎src/aws-cpp-sdk-core/source/config/AWSConfigFileProfileConfigLoader.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ namespace Aws
218218
if (servicesBlk != m_services.end()) {
219219
endpoints = std::move(servicesBlk->second);
220220
}
221-
profile.m_services.SetEndpoints(std::move(endpoints), servicesRef);
221+
profile.SetServices(Profile::Services(std::move(endpoints), servicesRef));
222222
}
223223
}
224224

‎tests/aws-cpp-sdk-core-tests/aws/config/ServiceEndpointsConfigFileLoaderTest.cpp‎

Lines changed: 98 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificEndpoints)
4242
auto profileIt = profiles.find("default");
4343
ASSERT_NE(profiles.end(), profileIt);
4444
const auto& profile = profileIt->second;
45-
auto globalEndpoint = profile.GetEndpointUrl();
45+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
4646
ASSERT_TRUE(globalEndpoint.has_value());
4747
ASSERT_STREQ("https://global.example.com", globalEndpoint->c_str());
4848

@@ -75,7 +75,7 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificEndpointsOnly)
7575
const auto& profile = profileIt->second;
7676

7777
// Test that global endpoint is null when not set
78-
auto globalEndpoint = profile.GetEndpointUrl();
78+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
7979
ASSERT_FALSE(globalEndpoint.has_value());
8080

8181
// Test services endpoints are parsed correctly
@@ -103,7 +103,7 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestGlobalEndpointOnly)
103103
const auto& profile = profileIt->second;
104104

105105
// Test global endpoint
106-
auto globalEndpoint = profile.GetEndpointUrl();
106+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
107107
ASSERT_TRUE(globalEndpoint.has_value());
108108
ASSERT_STREQ("https://play.min.io:9000", globalEndpoint->c_str());
109109

@@ -140,7 +140,7 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestServiceSpecificAndGlobalEndpoin
140140
ASSERT_EQ("https://play.min.io:9000", endpoints.at("S3"));
141141

142142
// Test global endpoint
143-
auto globalEndpoint = profile.GetEndpointUrl();
143+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
144144
ASSERT_TRUE(globalEndpoint.has_value());
145145
ASSERT_STREQ("http://localhost:1234", globalEndpoint->c_str());
146146
}
@@ -194,7 +194,7 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestIgnoreGlobalEndpointInServicesS
194194
const auto& profile = profileIt->second;
195195

196196
// Test that global endpoint in services section is ignored
197-
auto globalEndpoint = profile.GetEndpointUrl();
197+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
198198
ASSERT_FALSE(globalEndpoint.has_value());
199199

200200
// Test that services endpoints are empty (global endpoint_url ignored)
@@ -239,11 +239,11 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestSourceProfileEndpointIsolation)
239239
ASSERT_EQ("https://profile-b-ec2-endpoint.aws", endpointsB.at("EC2"));
240240

241241
// Test that profile B has no global endpoint (doesn't inherit from profile A)
242-
auto globalEndpointB = profileB.GetEndpointUrl();
242+
auto globalEndpointB = profileB.GetGlobalEndpointUrl();
243243
ASSERT_FALSE(globalEndpointB.has_value());
244244

245245
// Test that profile A still has its own global endpoint
246-
auto globalEndpointA = profileA.GetEndpointUrl();
246+
auto globalEndpointA = profileA.GetGlobalEndpointUrl();
247247
ASSERT_TRUE(globalEndpointA.has_value());
248248
ASSERT_STREQ("https://profile-a-endpoint.aws/", globalEndpointA->c_str());
249249

@@ -304,4 +304,95 @@ TEST_F(ServiceEndpointsConfigFileLoaderTest, TestMultipleServicesDefinitions)
304304
const auto& endpoints = services.GetEndpoints();
305305
ASSERT_EQ(1u, endpoints.size());
306306
ASSERT_EQ("http://foo.com", endpoints.at("S3"));
307+
}
308+
309+
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestDuplicateGlobalEndpointUrl)
310+
{
311+
TempFile configFile(std::ios_base::out | std::ios_base::trunc);
312+
ASSERT_TRUE(configFile.good());
313+
314+
configFile << "[profile dev-global]\n";
315+
configFile << "endpoint_url = https://play.min.io:9000\n";
316+
configFile << "endpoint_url = https://play2.min.io:9000\n";
317+
configFile.flush();
318+
319+
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
320+
ASSERT_TRUE(loader.Load());
321+
auto profiles = loader.GetProfiles();
322+
auto profileIt = profiles.find("dev-global");
323+
ASSERT_NE(profiles.end(), profileIt);
324+
const auto& profile = profileIt->second;
325+
326+
// Test that last value wins for duplicate global endpoint_url
327+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
328+
ASSERT_TRUE(globalEndpoint.has_value());
329+
ASSERT_STREQ("https://play2.min.io:9000", globalEndpoint->c_str());
330+
}
331+
332+
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestDuplicateServiceEndpointUrl)
333+
{
334+
TempFile configFile(std::ios_base::out | std::ios_base::trunc);
335+
ASSERT_TRUE(configFile.good());
336+
337+
configFile << "[services s3test]\n";
338+
configFile << "s3 =\n";
339+
configFile << " endpoint_url = https://play.min.io:9000\n";
340+
configFile << "s3 =\n";
341+
configFile << " endpoint_url = https://play2.min.io:9000\n";
342+
configFile << "\n[profile dev]\n";
343+
configFile << "services = s3test\n";
344+
configFile.flush();
345+
346+
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
347+
ASSERT_TRUE(loader.Load());
348+
auto profiles = loader.GetProfiles();
349+
auto profileIt = profiles.find("dev");
350+
ASSERT_NE(profiles.end(), profileIt);
351+
const auto& profile = profileIt->second;
352+
353+
// Test that last value wins for duplicate service endpoint_url
354+
const auto& services = profile.GetServices();
355+
ASSERT_TRUE(services.IsSet());
356+
const auto& endpoints = services.GetEndpoints();
357+
ASSERT_EQ(1u, endpoints.size());
358+
ASSERT_EQ("https://play2.min.io:9000", endpoints.at("S3"));
359+
}
360+
361+
TEST_F(ServiceEndpointsConfigFileLoaderTest, TestMixedDuplicateEndpoints)
362+
{
363+
TempFile configFile(std::ios_base::out | std::ios_base::trunc);
364+
ASSERT_TRUE(configFile.good());
365+
366+
configFile << "[profile dev-mixed]\n";
367+
configFile << "endpoint_url = https://global1.example.com\n";
368+
configFile << "services = mixed-services\n";
369+
configFile << "endpoint_url = https://global2.example.com\n";
370+
configFile << "\n[services mixed-services]\n";
371+
configFile << "s3 =\n";
372+
configFile << " endpoint_url = https://s3-first.example.com\n";
373+
configFile << "dynamodb =\n";
374+
configFile << " endpoint_url = https://dynamo.example.com\n";
375+
configFile << "s3 =\n";
376+
configFile << " endpoint_url = https://s3-last.example.com\n";
377+
configFile.flush();
378+
379+
AWSConfigFileProfileConfigLoader loader(configFile.GetFileName(), true);
380+
ASSERT_TRUE(loader.Load());
381+
auto profiles = loader.GetProfiles();
382+
auto profileIt = profiles.find("dev-mixed");
383+
ASSERT_NE(profiles.end(), profileIt);
384+
const auto& profile = profileIt->second;
385+
386+
// Test that last global endpoint_url wins
387+
auto globalEndpoint = profile.GetGlobalEndpointUrl();
388+
ASSERT_TRUE(globalEndpoint.has_value());
389+
ASSERT_STREQ("https://global2.example.com", globalEndpoint->c_str());
390+
391+
// Test that last service endpoint_url wins, but other services remain
392+
const auto& services = profile.GetServices();
393+
ASSERT_TRUE(services.IsSet());
394+
const auto& endpoints = services.GetEndpoints();
395+
ASSERT_EQ(2u, endpoints.size());
396+
ASSERT_EQ("https://s3-last.example.com", endpoints.at("S3"));
397+
ASSERT_EQ("https://dynamo.example.com", endpoints.at("DYNAMODB"));
307398
}

0 commit comments

Comments
 (0)