Skip to content

fix: handle setting protected/type-enforced attributes correctly #433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 26 additions & 24 deletions libs/common/include/launchdarkly/attributes_builder.hpp
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#pragma once

Check notice on line 1 in libs/common/include/launchdarkly/attributes_builder.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

Run clang-format on libs/common/include/launchdarkly/attributes_builder.hpp

File libs/common/include/launchdarkly/attributes_builder.hpp does not conform to Custom style guidelines. (lines 23, 34, 154, 218, 234, 245)

#include <string>

#include <launchdarkly/attribute_reference.hpp>
#include <launchdarkly/attributes.hpp>
#include <launchdarkly/value.hpp>

namespace launchdarkly {
#include <map>
#include <string>

namespace launchdarkly {
class ContextBuilder;

/**
Expand All @@ -22,7 +22,7 @@
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
Expand All @@ -31,7 +31,8 @@
* @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
Expand All @@ -44,13 +45,13 @@
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()) {
for (auto& pair : attributes.CustomAttributes().AsObject()) {

Check warning on line 54 in libs/common/include/launchdarkly/attributes_builder.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/common/include/launchdarkly/attributes_builder.hpp:54:14 [readability-qualified-auto]

'auto &pair' can be declared as 'const auto &pair'
values_[pair.first] = pair.second;
}
}
Expand All @@ -65,7 +66,7 @@

// This cannot be noexcept because of:
// https://developercommunity.visualstudio.com/t/bug-in-stdmapstdpair-implementation-with-move-only/840554
AttributesBuilder(AttributesBuilder&& builder) = default;

Check warning on line 69 in libs/common/include/launchdarkly/attributes_builder.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/common/include/launchdarkly/attributes_builder.hpp:69:5 [performance-noexcept-move-constructor]

move constructors should be marked noexcept
~AttributesBuilder() = default;

/**
Expand All @@ -90,36 +91,36 @@
/**
* Add or update an attribute in the context.
*
* This method cannot be used to set the key, kind, name, anonymous, or
* _meta property of a context. The specific methods on the context builder,
* or attributes builder, should be used.
* This method cannot be used to set the key, kind, or _meta property
* of a context. The anonymous and name properties can be set, but must
* be of the correct types (boolean and string respectively). The specific
* methods on the context builder or attributes builder can also be used
* for these properties.
*
* @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.
*
* This method cannot be used to set the key, kind, name, or anonymous
* property of a context. The specific methods on the context builder, or
* attributes builder, should be used.
* This method cannot be used to set the key, kind, or _meta property
* of a context. The anonymous and name properties can be set, but must
* be of the correct types (boolean and string respectively). The specific
* methods on the context builder or attributes builder can also be used
* for these properties.
*
* Once you have set an attribute private it will remain in the private
* list even if you call `set` afterward. This method is just a convenience
* which also adds the attribute to the `PrivateAttributes`.
*
* @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:
Expand Down Expand Up @@ -150,7 +151,7 @@
* launchdarkly::config::shared::builders::EventsBuilder<SDK>::PrivateAttribute.
*
* The attributes "kind" and "key", and the "_meta" attributes cannot be
* made private.
* made private. The "anonymous" attribute can be marked private, but will not be redacted.
*
* In this example, firstName is marked as private, but lastName is not:
*
Expand Down Expand Up @@ -216,7 +217,7 @@
*/
[[nodiscard]] BuildType Build() const { return builder_.Build(); }

private:
private:
BuilderReturn& builder_;

/**
Expand All @@ -226,18 +227,19 @@
*/
void Key(std::string key) { key_ = std::move(key); }

Attributes BuildAttributes() const;

Check warning on line 230 in libs/common/include/launchdarkly/attributes_builder.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/common/include/launchdarkly/attributes_builder.hpp:230:5 [modernize-use-nodiscard]

function 'BuildAttributes' should be marked [[nodiscard]]

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<std::string, launchdarkly::Value> values_;
std::map<std::string, Value> values_;
AttributeReference::SetType private_attributes_;
};
} // namespace launchdarkly
} // namespace launchdarkly
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @file */

Check notice on line 1 in libs/common/include/launchdarkly/bindings/c/context_builder.h

View workflow job for this annotation

GitHub Actions / cpp-linter

Run clang-format on libs/common/include/launchdarkly/bindings/c/context_builder.h

File libs/common/include/launchdarkly/bindings/c/context_builder.h does not conform to Custom style guidelines. (lines 98, 182)
// NOLINTBEGIN modernize-use-using

#pragma once
Expand Down Expand Up @@ -94,8 +94,8 @@
* A subsequent call to @ref LDContextBuilder_Attributes_Set, for the same
* attribute, will not remove the private status.
*
* This method cannot be used to set the `key`, `kind`, `name`, or `anonymous`
* property of a context.
* This method cannot be used to set the `key`, `kind`, or `_meta`
* property of a context. The `anonymous` and `name` properties can be set, but must be of the correct types.
*
* Adding a @ref LDValue to the builder will consume that value.
*
Expand Down Expand Up @@ -179,7 +179,7 @@
* - @ref LDClientConfigBuilder_Events_PrivateAttribute
*
* The attributes "kind" and "key", and the "_meta" attributes cannot be
* made private.
* made private. The "anonymous" attribute can be marked private, but will not be redacted.
*
* @param builder The builder. Must not be NULL.
* @param kind The kind to set the attribute as private for. Must not be NULL.
Expand Down
94 changes: 84 additions & 10 deletions libs/common/src/attributes_builder.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#include <launchdarkly/attributes_builder.hpp>

Check notice on line 1 in libs/common/src/attributes_builder.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

Run clang-format on libs/common/src/attributes_builder.cpp

File libs/common/src/attributes_builder.cpp does not conform to Custom style guidelines. (lines 4, 27, 28, 29, 30, 40, 42, 43, 44, 46, 50, 51, 52, 53, 54, 57, 58, 59, 60, 61, 62, 63, 64, 65, 67, 68, 69, 70, 71, 72, 73, 76, 92, 111, 112, 119, 133, 137)
#include <launchdarkly/context_builder.hpp>

namespace launchdarkly {
#include <set>
#include <unordered_map>
#include <functional>

namespace launchdarkly {
template <>
AttributesBuilder<ContextBuilder, Context>&
AttributesBuilder<ContextBuilder, Context>::Name(std::string name) {
Expand All @@ -17,33 +20,104 @@
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<std::string> const& ProtectedAttributes() {
static std::set<std::string> 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<void(AttributesBuilder<ContextBuilder, Context>& builder,
Value)> setter;

TypedAttribute(enum Value::Type type,
std::function<void(
AttributesBuilder<ContextBuilder, Context>& 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<std::string, TypedAttribute> const&
TypedAttributes() {
static std::unordered_map<std::string, TypedAttribute> 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<ContextBuilder, Context>&
AttributesBuilder<ContextBuilder, Context>::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<ContextBuilder, Context>&
AttributesBuilder<ContextBuilder, Context>::Set(std::string name, Value value) {
AttributesBuilder<ContextBuilder, Context>::Set(
std::string name,
Value value) {
return Set(std::move(name), std::move(value), false);
}

template <>
AttributesBuilder<ContextBuilder, Context>&
AttributesBuilder<ContextBuilder, Context>::SetPrivate(std::string name,
Value value) {
Value value) {
return Set(std::move(name), std::move(value), true);
}

Expand All @@ -56,8 +130,8 @@
}

template <>
Attributes AttributesBuilder<ContextBuilder, Context>::BuildAttributes() const {
Attributes AttributesBuilder<
ContextBuilder, Context>::BuildAttributes() const {
return {key_, name_, anonymous_, values_, private_attributes_};
}

} // namespace launchdarkly
} // namespace launchdarkly
Loading
Loading