diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 21077d3e6..bd6250276 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -16,6 +16,7 @@ - `GetMinorVersion`. - Removed from `Azure::Core::Http::Policies` namespace: `HttpPolicyOrder`, `TransportPolicy`, `RetryPolicy`, `RequestIdPolicy`, `TelemetryPolicy`, `BearerTokenAuthenticationPolicy`, `LogPolicy`. - Renamed `Azure::Core::Http::RawResponse::GetBodyStream()` to `ExtractBodyStream()`. +- Introduced `Azure::Core::Context::Key` class which takes place of `std::string` used for `Azure::Core::Context` keys previously. ## 1.0.0-beta.7 (2021-03-11) diff --git a/sdk/core/azure-core/inc/azure/core/context.hpp b/sdk/core/azure-core/inc/azure/core/context.hpp index c1e583245..64d7d762b 100644 --- a/sdk/core/azure-core/inc/azure/core/context.hpp +++ b/sdk/core/azure-core/inc/azure/core/context.hpp @@ -39,6 +39,23 @@ namespace Azure { namespace Core { */ class Context { public: + /** + * @brief A context key. + */ + class Key final { + Key const* m_uniqueAddress; + + public: + Key() : m_uniqueAddress(this) {} + + bool operator==(Key const& other) const + { + return this->m_uniqueAddress == other.m_uniqueAddress; + } + + bool operator!=(Key const& other) const { return !(*this == other); } + }; + /** * @brief A type used to provide a point in time for context expiration. */ @@ -49,7 +66,7 @@ namespace Azure { namespace Core { { std::shared_ptr Parent; std::atomic_int64_t CancelAtMsecSinceEpoch; - std::string Key; + Context::Key Key; std::shared_ptr Value; const std::type_info& ValueType; @@ -82,7 +99,7 @@ namespace Azure { namespace Core { explicit ContextSharedState( const std::shared_ptr& parent, time_point cancelAt, - const std::string& key, + Context::Key const& key, T value) // NOTE, should this be T&& : Parent(parent), CancelAtMsecSinceEpoch(ToMsecSinceEpoch(cancelAt)), Key(key), Value(std::make_shared(std::move(value))), ValueType(typeid(T)) @@ -131,7 +148,7 @@ namespace Azure { namespace Core { * * @return A child context with no expiration and the \p key and \p value associated with it. */ - template Context WithValue(const std::string& key, T&& value) const + template Context WithValue(Key const& key, T&& value) const { return Context{std::make_shared( m_contextSharedState, (time_point::max)(), key, std::forward(value))}; @@ -146,21 +163,18 @@ namespace Azure { namespace Core { * @return A value associated with the context found; an empty value if a specific value can't * be found. */ - template const T& Get(const std::string& key) const + template const T& Get(Key const& key) const { - if (!key.empty()) + for (auto ptr = m_contextSharedState; ptr; ptr = ptr->Parent) { - for (auto ptr = m_contextSharedState; ptr; ptr = ptr->Parent) + if (ptr->Key == key) { - if (ptr->Key == key) + if (typeid(T) != ptr->ValueType) { - if (typeid(T) != ptr->ValueType) - { - // type mismatch - std::abort(); - } - return *reinterpret_cast(ptr->Value.get()); + // type mismatch + std::abort(); } + return *reinterpret_cast(ptr->Value.get()); } } std::abort(); @@ -177,16 +191,13 @@ namespace Azure { namespace Core { * @return `true` if this context, or the tree branch this context belongs to has a \p key * associated with it. `false` otherwise. */ - bool HasKey(const std::string& key) const + bool HasKey(Key const& key) const { - if (!key.empty()) + for (auto ptr = m_contextSharedState; ptr; ptr = ptr->Parent) { - for (auto ptr = m_contextSharedState; ptr; ptr = ptr->Parent) + if (ptr->Key == key) { - if (ptr->Key == key) - { - return true; - } + return true; } } return false; diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index 93a0b60bd..d378df796 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -144,7 +144,9 @@ bool ShouldRetryOnResponse( return true; } -static constexpr char RetryKey[] = "AzureSdkRetryPolicyCounter"; +namespace { + Context::Key const RetryKey; +} /** * @brief Creates a new #Context node from \p parent with the information about the retrying while diff --git a/sdk/core/azure-core/test/ut/context.cpp b/sdk/core/azure-core/test/ut/context.cpp index debb7a318..a4b93d179 100644 --- a/sdk/core/azure-core/test/ut/context.cpp +++ b/sdk/core/azure-core/test/ut/context.cpp @@ -16,25 +16,30 @@ using namespace Azure::Core; TEST(Context, Basic) { Context context; - EXPECT_FALSE(context.HasKey("")); - EXPECT_FALSE(context.HasKey("key")); + Context::Key const key; + + EXPECT_FALSE(context.HasKey(key)); } TEST(Context, BasicBool) { Context context; + Context::Key const key; + // New context from previous - auto c2 = context.WithValue("key", true); - auto& value = c2.Get("key"); + auto c2 = context.WithValue(key, true); + auto& value = c2.Get(key); EXPECT_TRUE(value == true); } TEST(Context, BasicInt) { Context context; + Context::Key const key; + // New context from previous - auto c2 = context.WithValue("key", 123); - auto& value = c2.Get("key"); + auto c2 = context.WithValue(key, 123); + auto& value = c2.Get(key); EXPECT_TRUE(value == 123); } @@ -43,9 +48,11 @@ TEST(Context, BasicStdString) std::string s("Test String"); Context context; + Context::Key const key; + // New context from previous - auto c2 = context.WithValue("key", s); - auto& value = c2.Get("key"); + auto c2 = context.WithValue(key, s); + auto& value = c2.Get(key); EXPECT_TRUE(value == s); } @@ -55,9 +62,11 @@ TEST(Context, BasicChar) std::string s(str); Context context; + Context::Key const key; + // New context from previous - auto c2 = context.WithValue("key", str); - auto& value = c2.Get("key"); + auto c2 = context.WithValue(key, str); + auto& value = c2.Get(key); EXPECT_TRUE(value == s); EXPECT_TRUE(value == str); } @@ -80,10 +89,12 @@ TEST(Context, NestedIsCancelled) auto deadline = std::chrono::system_clock::now() + duration; Context context; - auto c2 = context.WithValue("Key", "Value"); + Context::Key const key; + + auto c2 = context.WithValue(key, "Value"); EXPECT_FALSE(c2.IsCancelled()); - EXPECT_TRUE(c2.HasKey("Key")); - EXPECT_FALSE(context.HasKey("Key")); + EXPECT_TRUE(c2.HasKey(key)); + EXPECT_FALSE(context.HasKey(key)); auto c3 = context.WithDeadline(deadline); EXPECT_FALSE(context.IsCancelled()); @@ -95,26 +106,28 @@ TEST(Context, NestedIsCancelled) EXPECT_FALSE(c2.IsCancelled()); EXPECT_TRUE(c3.IsCancelled()); - EXPECT_TRUE(c2.HasKey("Key")); - EXPECT_FALSE(context.HasKey("Key")); - EXPECT_FALSE(c3.HasKey("Key")); + EXPECT_TRUE(c2.HasKey(key)); + EXPECT_FALSE(context.HasKey(key)); + EXPECT_FALSE(c3.HasKey(key)); } TEST(Context, CancelWithValue) { Context context; - auto c2 = context.WithValue("Key", "Value"); + Context::Key const key; + + auto c2 = context.WithValue(key, "Value"); EXPECT_FALSE(context.IsCancelled()); EXPECT_FALSE(c2.IsCancelled()); - EXPECT_TRUE(c2.HasKey("Key")); - EXPECT_FALSE(context.HasKey("Key")); + EXPECT_TRUE(c2.HasKey(key)); + EXPECT_FALSE(context.HasKey(key)); c2.Cancel(); EXPECT_TRUE(c2.IsCancelled()); EXPECT_FALSE(context.IsCancelled()); - EXPECT_TRUE(c2.HasKey("Key")); - EXPECT_FALSE(context.HasKey("Key")); + EXPECT_TRUE(c2.HasKey(key)); + EXPECT_FALSE(context.HasKey(key)); } TEST(Context, ThrowIfCancelled) @@ -132,22 +145,30 @@ TEST(Context, ThrowIfCancelled) TEST(Context, Chain) { Context context; - // New context from previous - auto c2 = context.WithValue("c2", 123); - auto c3 = c2.WithValue("c3", 456); - auto c4 = c3.WithValue("c4", 789); - auto c5 = c4.WithValue("c5", "5"); - auto c6 = c5.WithValue("c6", "6"); - auto c7 = c6.WithValue("c7", "7"); - auto finalContext = c7.WithValue("finalContext", "Final"); + Context::Key const key2; + Context::Key const key3; + Context::Key const key4; + Context::Key const key5; + Context::Key const key6; + Context::Key const key7; + Context::Key const keyFinal; - auto& valueT2 = finalContext.Get("c2"); - auto& valueT3 = finalContext.Get("c3"); - auto& valueT4 = finalContext.Get("c4"); - auto& valueT5 = finalContext.Get("c5"); - auto& valueT6 = finalContext.Get("c6"); - auto& valueT7 = finalContext.Get("c7"); - auto& valueT8 = finalContext.Get("finalContext"); + // New context from previous + auto c2 = context.WithValue(key2, 123); + auto c3 = c2.WithValue(key3, 456); + auto c4 = c3.WithValue(key4, 789); + auto c5 = c4.WithValue(key5, "5"); + auto c6 = c5.WithValue(key6, "6"); + auto c7 = c6.WithValue(key7, "7"); + auto finalContext = c7.WithValue(keyFinal, "Final"); + + auto& valueT2 = finalContext.Get(key2); + auto& valueT3 = finalContext.Get(key3); + auto& valueT4 = finalContext.Get(key4); + auto& valueT5 = finalContext.Get(key5); + auto& valueT6 = finalContext.Get(key6); + auto& valueT7 = finalContext.Get(key7); + auto& valueT8 = finalContext.Get(keyFinal); EXPECT_TRUE(valueT2 == 123); EXPECT_TRUE(valueT3 == 456); @@ -161,12 +182,14 @@ TEST(Context, Chain) TEST(Context, MatchingKeys) { Context context; - // New context from previous - auto c2 = context.WithValue("key", 123); - auto c3 = c2.WithValue("key", 456); + Context::Key const key; - auto& valueT2 = c2.Get("key"); - auto& valueT3 = c3.Get("key"); + // New context from previous + auto c2 = context.WithValue(key, 123); + auto c3 = c2.WithValue(key, 456); + + auto& valueT2 = c2.Get(key); + auto& valueT3 = c3.Get(key); EXPECT_TRUE(valueT2 == 123); EXPECT_TRUE(valueT3 == 456); @@ -179,68 +202,126 @@ struct SomeStructForContext TEST(Context, InstanceValue) { - auto contextP = Context::GetApplicationContext().WithValue("struct", SomeStructForContext()); - auto& contextValueRef = contextP.Get("struct"); + Context::Key const key; + auto contextP = Context::GetApplicationContext().WithValue(key, SomeStructForContext()); + auto& contextValueRef = contextP.Get(key); EXPECT_EQ(contextValueRef.someField, 12345); } TEST(Context, UniquePtr) { - auto contextP = Context::GetApplicationContext().WithValue( - "struct", std::make_unique()); - auto& contextValueRef = contextP.Get>("struct"); + Context::Key const key; + auto contextP + = Context::GetApplicationContext().WithValue(key, std::make_unique()); + auto& contextValueRef = contextP.Get>(key); EXPECT_EQ(contextValueRef->someField, 12345); } TEST(Context, HeapLinkIntegrity) { + Context::Key const a; + Context::Key const b; + Context::Key const c; + Context::Key const d; + Context::Key const e; Context thirdGeneration; // To be used at the end { Context root; - auto firstGeneration = root.WithValue("a", std::string("a")); - EXPECT_TRUE(firstGeneration.HasKey("a")); + auto firstGeneration = root.WithValue(a, std::string("a")); + EXPECT_TRUE(firstGeneration.HasKey(a)); - auto secondGeneration = firstGeneration.WithValue("b", std::string("b")); - EXPECT_TRUE(secondGeneration.HasKey("a")); - EXPECT_EQ("a", secondGeneration.Get("a")); - EXPECT_TRUE(secondGeneration.HasKey("b")); - EXPECT_EQ("b", secondGeneration.Get("b")); + auto secondGeneration = firstGeneration.WithValue(b, std::string("b")); + EXPECT_TRUE(secondGeneration.HasKey(a)); + EXPECT_EQ("a", secondGeneration.Get(a)); + EXPECT_TRUE(secondGeneration.HasKey(b)); + EXPECT_EQ("b", secondGeneration.Get(b)); // Now overide the generation - secondGeneration = secondGeneration.WithValue("c", std::string("c")); + secondGeneration = secondGeneration.WithValue(c, std::string("c")); EXPECT_TRUE( - secondGeneration.HasKey("a")); // Still know about first gen - The link is still in heap - EXPECT_EQ("a", secondGeneration.Get("a")); + secondGeneration.HasKey(a)); // Still know about first gen - The link is still in heap + EXPECT_EQ("a", secondGeneration.Get(a)); EXPECT_TRUE(secondGeneration.HasKey( - "b")); // Still knows about the initial second gen, as a shared_ptr, it is still on heap - EXPECT_EQ("b", secondGeneration.Get("b")); - EXPECT_TRUE(secondGeneration.HasKey("c")); // Check new value - EXPECT_EQ("c", secondGeneration.Get("c")); + b)); // Still knows about the initial second gen, as a shared_ptr, it is still on heap + EXPECT_EQ("b", secondGeneration.Get(b)); + EXPECT_TRUE(secondGeneration.HasKey(c)); // Check new value + EXPECT_EQ("c", secondGeneration.Get(c)); // One more override - secondGeneration = secondGeneration.WithValue("d", std::string("d")); - EXPECT_TRUE(secondGeneration.HasKey("a")); - EXPECT_EQ("a", secondGeneration.Get("a")); - EXPECT_TRUE(secondGeneration.HasKey("b")); - EXPECT_EQ("b", secondGeneration.Get("b")); - EXPECT_TRUE(secondGeneration.HasKey("c")); - EXPECT_EQ("c", secondGeneration.Get("c")); - EXPECT_TRUE(secondGeneration.HasKey("d")); - EXPECT_EQ("d", secondGeneration.Get("d")); + secondGeneration = secondGeneration.WithValue(d, std::string("d")); + EXPECT_TRUE(secondGeneration.HasKey(a)); + EXPECT_EQ("a", secondGeneration.Get(a)); + EXPECT_TRUE(secondGeneration.HasKey(b)); + EXPECT_EQ("b", secondGeneration.Get(b)); + EXPECT_TRUE(secondGeneration.HasKey(c)); + EXPECT_EQ("c", secondGeneration.Get(c)); + EXPECT_TRUE(secondGeneration.HasKey(d)); + EXPECT_EQ("d", secondGeneration.Get(d)); // New Gen - thirdGeneration = secondGeneration.WithValue("e", std::string("e")); + thirdGeneration = secondGeneration.WithValue(e, std::string("e")); } // Went out of scope, root and secondGeneration are destroyed. but should remain in heap for the // third-generation since the previous geneations are still alive inside his heart <3. - EXPECT_TRUE(thirdGeneration.HasKey("a")); - EXPECT_EQ("a", thirdGeneration.Get("a")); - EXPECT_TRUE(thirdGeneration.HasKey("b")); - EXPECT_EQ("b", thirdGeneration.Get("b")); - EXPECT_TRUE(thirdGeneration.HasKey("c")); - EXPECT_EQ("c", thirdGeneration.Get("c")); - EXPECT_TRUE(thirdGeneration.HasKey("d")); - EXPECT_EQ("d", thirdGeneration.Get("d")); - EXPECT_TRUE(thirdGeneration.HasKey("e")); - EXPECT_EQ("e", thirdGeneration.Get("e")); + EXPECT_TRUE(thirdGeneration.HasKey(a)); + EXPECT_EQ("a", thirdGeneration.Get(a)); + EXPECT_TRUE(thirdGeneration.HasKey(b)); + EXPECT_EQ("b", thirdGeneration.Get(b)); + EXPECT_TRUE(thirdGeneration.HasKey(c)); + EXPECT_EQ("c", thirdGeneration.Get(c)); + EXPECT_TRUE(thirdGeneration.HasKey(d)); + EXPECT_EQ("d", thirdGeneration.Get(d)); + EXPECT_TRUE(thirdGeneration.HasKey(e)); + EXPECT_EQ("e", thirdGeneration.Get(e)); +} + +Context::Key const GlobalKey1; +Context::Key const GlobalKey2; + +namespace { +Context::Key const UnnamedNamespaceKey1; +Context::Key const UnnamedNamespaceKey2; +} // namespace + +TEST(Context, KeyComparison) +{ + EXPECT_EQ(GlobalKey1, GlobalKey1); + EXPECT_EQ(GlobalKey2, GlobalKey2); + + EXPECT_NE(GlobalKey1, GlobalKey2); + EXPECT_NE(GlobalKey2, GlobalKey1); + + EXPECT_EQ(UnnamedNamespaceKey1, UnnamedNamespaceKey1); + EXPECT_EQ(UnnamedNamespaceKey2, UnnamedNamespaceKey2); + + EXPECT_NE(UnnamedNamespaceKey1, UnnamedNamespaceKey2); + EXPECT_NE(UnnamedNamespaceKey2, UnnamedNamespaceKey1); + + Context::Key const localKey1; + Context::Key const localKey2; + + EXPECT_EQ(localKey1, localKey1); + EXPECT_EQ(localKey2, localKey2); + + EXPECT_NE(localKey1, localKey2); + EXPECT_NE(localKey2, localKey1); + + Context::Key const localKey1Copy = localKey1; + Context::Key const localKey2Copy = localKey2; + + EXPECT_EQ(localKey1Copy, localKey1Copy); + EXPECT_EQ(localKey2Copy, localKey2Copy); + + EXPECT_NE(localKey1Copy, localKey2Copy); + EXPECT_NE(localKey2Copy, localKey1Copy); + + EXPECT_EQ(localKey1, localKey1Copy); + EXPECT_EQ(localKey2, localKey2Copy); + EXPECT_EQ(localKey1Copy, localKey1); + EXPECT_EQ(localKey2Copy, localKey2); + + EXPECT_NE(localKey1, localKey2Copy); + EXPECT_NE(localKey2, localKey1Copy); + EXPECT_NE(localKey1Copy, localKey2); + EXPECT_NE(localKey2Copy, localKey1); } diff --git a/sdk/core/azure-core/test/ut/global_context.cpp b/sdk/core/azure-core/test/ut/global_context.cpp index 71d69852d..59b27e709 100644 --- a/sdk/core/azure-core/test/ut/global_context.cpp +++ b/sdk/core/azure-core/test/ut/global_context.cpp @@ -24,12 +24,7 @@ TEST(Context, ApplicationContext) { Context appContext = Context::GetApplicationContext(); - EXPECT_FALSE(appContext.HasKey("Key")); - EXPECT_FALSE(appContext.HasKey("key")); - EXPECT_FALSE(appContext.HasKey("Value")); - EXPECT_FALSE(appContext.HasKey("value")); - EXPECT_FALSE(appContext.HasKey("1")); - EXPECT_FALSE(appContext.HasKey("")); + EXPECT_FALSE(appContext.HasKey(Context::Key())); auto duration = std::chrono::milliseconds(250); EXPECT_FALSE(appContext.IsCancelled()); diff --git a/sdk/core/azure-core/test/ut/policy.cpp b/sdk/core/azure-core/test/ut/policy.cpp index 045a6e5a4..351d4dd1e 100644 --- a/sdk/core/azure-core/test/ut/policy.cpp +++ b/sdk/core/azure-core/test/ut/policy.cpp @@ -47,6 +47,8 @@ struct TestRetryPolicySharedState : public Azure::Core::Http::Policies::HttpPoli } }; +Azure::Core::Context::Key const TheKey; + struct TestContextTreeIntegrity : public Azure::Core::Http::Policies::HttpPolicy { std::unique_ptr Clone() const override @@ -59,10 +61,10 @@ struct TestContextTreeIntegrity : public Azure::Core::Http::Policies::HttpPolicy Azure::Core::Http::Policies::NextHttpPolicy nextHttpPolicy, Azure::Core::Context const& ctx) const override { - EXPECT_TRUE(ctx.HasKey("TheKey")); - if (ctx.HasKey("TheKey")) + EXPECT_TRUE(ctx.HasKey(TheKey)); + if (ctx.HasKey(TheKey)) { - auto value = ctx.Get("TheKey"); + auto value = ctx.Get(TheKey); EXPECT_EQ("TheValue", value); } return nextHttpPolicy.Send(request, ctx); @@ -223,6 +225,6 @@ TEST(Policy, RetryPolicyKeepContext) HttpPipeline pipeline(policies); Request request(HttpMethod::Get, Url("url")); auto withValueContext - = Context::GetApplicationContext().WithValue("TheKey", std::string("TheValue")); + = Context::GetApplicationContext().WithValue(TheKey, std::string("TheValue")); pipeline.Send(request, withValueContext); } diff --git a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_switch_to_secondary_policy.hpp b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_switch_to_secondary_policy.hpp index ac5f919ef..b8e65a6ff 100644 --- a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_switch_to_secondary_policy.hpp +++ b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_switch_to_secondary_policy.hpp @@ -8,10 +8,11 @@ #include +#include "azure/storage/common/dll_import_export.hpp" + namespace Azure { namespace Storage { namespace _internal { - static constexpr const char* SecondaryHostReplicaStatusKey - = "AzureSdkStorageSecondaryHostReplicaStatusKey"; + AZ_STORAGE_COMMON_DLLEXPORT extern const Azure::Core::Context::Key SecondaryHostReplicaStatusKey; inline Azure::Core::Context WithReplicaStatus(const Azure::Core::Context& context) { diff --git a/sdk/storage/azure-storage-common/src/storage_switch_to_secondary_policy.cpp b/sdk/storage/azure-storage-common/src/storage_switch_to_secondary_policy.cpp index ceb99a16a..909338990 100644 --- a/sdk/storage/azure-storage-common/src/storage_switch_to_secondary_policy.cpp +++ b/sdk/storage/azure-storage-common/src/storage_switch_to_secondary_policy.cpp @@ -5,6 +5,8 @@ namespace Azure { namespace Storage { namespace _internal { + Azure::Core::Context::Key const SecondaryHostReplicaStatusKey; + std::unique_ptr StorageSwitchToSecondaryPolicy::Send( Azure::Core::Http::Request& request, Azure::Core::Http::Policies::NextHttpPolicy nextHttpPolicy,