diff --git a/libs/common/include/launchdarkly/attributes_builder.hpp b/libs/common/include/launchdarkly/attributes_builder.hpp index bc0dc82c9..3c7765507 100644 --- a/libs/common/include/launchdarkly/attributes_builder.hpp +++ b/libs/common/include/launchdarkly/attributes_builder.hpp @@ -1,13 +1,13 @@ #pragma once -#include - #include #include #include -namespace launchdarkly { +#include +#include +namespace launchdarkly { class ContextBuilder; /** @@ -22,7 +22,7 @@ template class AttributesBuilder final { friend class ContextBuilder; - public: +public: /** * Create an attributes builder with the given kind and key. * @param builder The context builder associated with this attributes @@ -31,7 +31,8 @@ class AttributesBuilder final { * @param key The key for the kind. */ AttributesBuilder(BuilderReturn& builder, std::string kind, std::string key) - : key_(std::move(key)), kind_(std::move(kind)), builder_(builder) {} + : builder_(builder), kind_(std::move(kind)), key_(std::move(key)) { + } /** * Crate an attributes builder with the specified kind, and pre-populated @@ -44,9 +45,9 @@ class AttributesBuilder final { AttributesBuilder(BuilderReturn& builder, std::string kind, Attributes const& attributes) - : key_(attributes.Key()), + : builder_(builder), kind_(std::move(kind)), - builder_(builder), + key_(attributes.Key()), name_(attributes.Name()), anonymous_(attributes.Anonymous()), private_attributes_(attributes.PrivateAttributes()) { @@ -96,11 +97,9 @@ class AttributesBuilder final { * * @param name The name of the attribute. * @param value The value for the attribute. - * @param private_attribute If the attribute should be considered private: - * that is, the value will not be sent to LaunchDarkly in analytics events. * @return A reference to the current builder. */ - AttributesBuilder& Set(std::string name, launchdarkly::Value value); + AttributesBuilder& Set(std::string name, Value value); /** * Add or update a private attribute in the context. @@ -115,11 +114,9 @@ class AttributesBuilder final { * * @param name The name of the attribute. * @param value The value for the attribute. - * @param private_attribute If the attribute should be considered private: - * that is, the value will not be sent to LaunchDarkly in analytics events. * @return A reference to the current builder. */ - AttributesBuilder& SetPrivate(std::string name, launchdarkly::Value value); + AttributesBuilder& SetPrivate(std::string name, Value value); /** * Designate a context attribute, or properties within them, as private: @@ -216,7 +213,7 @@ class AttributesBuilder final { */ [[nodiscard]] BuildType Build() const { return builder_.Build(); } - private: +private: BuilderReturn& builder_; /** @@ -229,15 +226,16 @@ class AttributesBuilder final { Attributes BuildAttributes() const; AttributesBuilder& Set(std::string name, - launchdarkly::Value value, + Value value, bool private_attribute); + std::string kind_; std::string key_; std::string name_; bool anonymous_ = false; - std::map values_; + std::map values_; AttributeReference::SetType private_attributes_; }; -} // namespace launchdarkly +} // namespace launchdarkly diff --git a/libs/common/src/attributes_builder.cpp b/libs/common/src/attributes_builder.cpp index 0f8d2a6d2..adb5aef73 100644 --- a/libs/common/src/attributes_builder.cpp +++ b/libs/common/src/attributes_builder.cpp @@ -1,8 +1,11 @@ #include #include -namespace launchdarkly { +#include +#include +#include +namespace launchdarkly { template <> AttributesBuilder& AttributesBuilder::Name(std::string name) { @@ -17,33 +20,104 @@ AttributesBuilder::Anonymous(bool anonymous) { return *this; } +// These attributes values cannot be set via the public Set/SetPrivate methods. +// 'key' and 'kind' are defined in the AttributesBuilder constructor, whereas +// '_meta' is set internally by the SDK. +std::set const& ProtectedAttributes() { + static std::set protectedAttrs = { + "key", + "kind", + "_meta" + }; + return protectedAttrs; +} + +// Utility struct to enforce the type-safe setting of a particular attribute +// via a method on AttributesBuilder. +struct TypedAttribute { + enum Value::Type type; + std::function& builder, + Value)> setter; + + TypedAttribute(enum Value::Type type, + std::function& builder, + Value const&)> setter) + : type(type), setter(std::move(setter)) { + } +}; + +// These attribute values, while able to be set via public Set/SetPrivate methods, +// are regarded as special and are type-enforced. Instead of being stored in the +// internal AttributesBuilder::values map, they are stored as individual fields. +// This is because they have special meaning/usage within the LaunchDarkly Platform. +std::unordered_map const& +TypedAttributes() { + static std::unordered_map typedAttrs = { + { + "anonymous", {Value::Type::kBool, + [](AttributesBuilder< + ContextBuilder, Context> + & builder, + Value const& value) { + builder.Anonymous( + value.AsBool()); + }} + }, + {"name", {Value::Type::kString, + [](AttributesBuilder< + ContextBuilder, Context>& + builder, + Value const& value) { + builder.Name(value.AsString()); + }}} + }; + return typedAttrs; +} + + template <> AttributesBuilder& AttributesBuilder::Set(std::string name, Value value, bool private_attribute) { - if (name == "key" || name == "kind" || name == "anonymous" || - name == "name" || name == "_meta") { + // Protected attributes cannot be set at all. + if (ProtectedAttributes().count(name) > 0) { + return *this; + } + + // Typed attributes can be set only if the value is of the correct type; + // the setting is forwarded to one of AttributeBuilder's dedicated methods. + auto const typed_attr = TypedAttributes().find(name); + const bool is_typed = typed_attr != TypedAttributes().end(); + if (is_typed && value.Type() != typed_attr->second.type) { return *this; } + if (is_typed) { + typed_attr->second.setter(*this, value); + } else { + values_[name] = std::move(value); + } + if (private_attribute) { - this->private_attributes_.insert(name); + this->private_attributes_.insert(std::move(name)); } - values_[std::move(name)] = std::move(value); return *this; } template <> AttributesBuilder& -AttributesBuilder::Set(std::string name, Value value) { +AttributesBuilder::Set( + std::string name, + Value value) { return Set(std::move(name), std::move(value), false); } template <> AttributesBuilder& AttributesBuilder::SetPrivate(std::string name, - Value value) { + Value value) { return Set(std::move(name), std::move(value), true); } @@ -56,8 +130,8 @@ AttributesBuilder::AddPrivateAttribute( } template <> -Attributes AttributesBuilder::BuildAttributes() const { +Attributes AttributesBuilder< + ContextBuilder, Context>::BuildAttributes() const { return {key_, name_, anonymous_, values_, private_attributes_}; } - -} // namespace launchdarkly +} // namespace launchdarkly diff --git a/libs/common/tests/context_builder_test.cpp b/libs/common/tests/context_builder_test.cpp index f5a46fa57..5f61ff6d6 100644 --- a/libs/common/tests/context_builder_test.cpp +++ b/libs/common/tests/context_builder_test.cpp @@ -29,14 +29,14 @@ TEST(ContextBuilderTests, CanMakeBasicContext) { TEST(ContextBuilderTests, CanMakeSingleContextWithCustomAttributes) { auto context = ContextBuilder() - .Kind("user", "bobby-bobberson") - .Name("Bob") - .Anonymous(true) - // Set a custom attribute. - .Set("likesCats", true) - // Set a private custom attribute. - .SetPrivate("email", "email@email.email") - .Build(); + .Kind("user", "bobby-bobberson") + .Name("Bob") + .Anonymous(true) + // Set a custom attribute. + .Set("likesCats", true) + // Set a private custom attribute. + .SetPrivate("email", "email@email.email") + .Build(); EXPECT_TRUE(context.Valid()); @@ -56,25 +56,25 @@ TEST(ContextBuilderTests, CanMakeSingleContextWithCustomAttributes) { TEST(ContextBuilderTests, CanBuildComplexMultiContext) { auto context = ContextBuilder() - .Kind("user", "user-key") - .Anonymous(true) - .Name("test") - .Set("string", "potato") - .Set("int", 42) - .Set("double", 3.14) - .Set("array", {false, true, 42}) - - .SetPrivate("private", "this is private") - .AddPrivateAttribute("double") - .AddPrivateAttributes(std::vector{"string", "int"}) - .AddPrivateAttributes( - std::vector{"explicitArray"}) - // Start the org kind. - .Kind("org", "org-key") - .Name("Macdonwalds") - .Set("explicitArray", Array{"egg", "ham"}) - .Set("object", Object{{"string", "bacon"}, {"boolean", false}}) - .Build(); + .Kind("user", "user-key") + .Anonymous(true) + .Name("test") + .Set("string", "potato") + .Set("int", 42) + .Set("double", 3.14) + .Set("array", {false, true, 42}) + + .SetPrivate("private", "this is private") + .AddPrivateAttribute("double") + .AddPrivateAttributes(std::vector{"string", "int"}) + .AddPrivateAttributes( + std::vector{"explicitArray"}) + // Start the org kind. + .Kind("org", "org-key") + .Name("Macdonwalds") + .Set("explicitArray", Array{"egg", "ham"}) + .Set("object", Object{{"string", "bacon"}, {"boolean", false}}) + .Build(); EXPECT_TRUE(context.Valid()); @@ -94,7 +94,7 @@ TEST(ContextBuilderTests, CanBuildComplexMultiContext) { EXPECT_EQ(1, context.Attributes("user").PrivateAttributes().count("double")); EXPECT_EQ(1, context.Attributes("user").PrivateAttributes().count( - "explicitArray")); + "explicitArray")); EXPECT_EQ(1, context.Attributes("user").PrivateAttributes().count("string")); EXPECT_EQ(1, @@ -177,11 +177,11 @@ TEST(ContextBuilderTests, AccessKindBuilderMultipleTimes) { TEST(ContextBuilderTests, AddAttributeToExistingContext) { auto context = ContextBuilder() - .Kind("user", "potato") - .Name("Bob") - .Set("city", "Reno") - .SetPrivate("private", "a") - .Build(); + .Kind("user", "potato") + .Name("Bob") + .Set("city", "Reno") + .SetPrivate("private", "a") + .Build(); auto builder = ContextBuilder(context); if (auto updater = builder.Kind("user")) { @@ -196,16 +196,20 @@ TEST(ContextBuilderTests, AddAttributeToExistingContext) { EXPECT_EQ("Nevada", updated_context.Get("user", "state").AsString()); EXPECT_EQ(2, updated_context.Attributes("user").PrivateAttributes().size()); - EXPECT_EQ(1, updated_context.Attributes("user").PrivateAttributes().count("private")); - EXPECT_EQ(1, updated_context.Attributes("user").PrivateAttributes().count("sneaky")); + EXPECT_EQ( + 1, updated_context.Attributes("user").PrivateAttributes().count( + "private")); + EXPECT_EQ( + 1, updated_context.Attributes("user").PrivateAttributes().count("sneaky" + )); } TEST(ContextBuilderTests, AddKindToExistingContext) { auto context = ContextBuilder() - .Kind("user", "potato") - .Name("Bob") - .Set("city", "Reno") - .Build(); + .Kind("user", "potato") + .Name("Bob") + .Set("city", "Reno") + .Build(); auto updated_context = ContextBuilder(context).Kind("org", "org-key").Build(); @@ -241,4 +245,62 @@ TEST(ContextBuilderTests, UseTheSameBuilderToBuildMultipleContexts) { EXPECT_EQ("tomato", context_c.Get("bob", "key").AsString()); } +TEST(ContextBuilderTests, CannotSetProtectedAttributes) { + auto builder = ContextBuilder(); + auto context = builder.Kind("user", "potato").Set("_meta", "a"). + Set("kind", "b").Set("key", "c").Build(); + ASSERT_TRUE(context.Valid()); + EXPECT_EQ(context.Get("user", "_meta"), Value::Null()); + EXPECT_EQ(context.Get("user", "kind"), Value::Null()); + EXPECT_EQ(context.Get("user", "key"), "potato"); +} + +TEST(ContextBuilderTests, CannotMarkProtectedAttributesAsPrivate) { + auto builder = ContextBuilder(); + auto context = builder.Kind("user", "potato").SetPrivate("_meta", "a"). + SetPrivate("kind", "b").SetPrivate("key", "c"). + Build(); + ASSERT_TRUE(context.Valid()); + EXPECT_EQ(context.Get("user", "_meta"), Value::Null()); + EXPECT_EQ(context.Get("user", "kind"), Value::Null()); + EXPECT_EQ(context.Get("user", "key"), "potato"); + EXPECT_EQ(context.Attributes("user").PrivateAttributes().size(), 0); +} + +TEST(ContextBuilderTests, CannotSetIncorrectTypedAttributes) { + auto builder = ContextBuilder(); + auto context = builder.Kind("user", "potato").Set("anonymous", "hello"). + Set("name", 42).Build(); + ASSERT_TRUE(context.Valid()); + // By default, anonymous is false. + EXPECT_EQ(context.Get("user", "anonymous"), Value(false)); + // By default, name is the empty string. + EXPECT_EQ(context.Get("user", "name"), Value("")); +} + +TEST(ContextBuilderTests, CanSetCorrectTypedAttributes) { + auto builder = ContextBuilder(); + auto context = builder.Kind("user", "potato").Set("anonymous", true). + Set("name", "Bob").Build(); + ASSERT_TRUE(context.Valid()); + EXPECT_EQ(context.Get("user", "anonymous"), Value(true)); + EXPECT_EQ(context.Get("user", "name"), Value("Bob")); +} + +TEST(ContextBuilderTests, CanMarkTypedAttributesAsPrivate) { + auto builder = ContextBuilder(); + auto context = builder.Kind("user", "potato").SetPrivate( + "anonymous", Value(true)).SetPrivate("name", Value("Bob")).Build(); + ASSERT_TRUE(context.Valid()); + // Even if these attributes cannot *actually* be redacted, the attributes builder + // still allows them to be marked private. Whether this should be allowed or not is + // up for debate (ideally we'd log it, but we don't have access to a logger. We could + // silently not allow it to be added to the private attributes list instead.) + EXPECT_EQ(context.Get("user", "anonymous"), Value(true)); + EXPECT_EQ(context.Attributes("user").PrivateAttributes().count("anonymous"), + 1); + EXPECT_EQ(context.Get("user", "name"), Value("Bob")); + EXPECT_EQ(context.Attributes("user").PrivateAttributes().count("name"), 1); +} + // NOLINTEND cppcoreguidelines-avoid-magic-numbers