feat: Change builder to allow for better loops.

libs/common/include/attributes_builder.hpp

@@ -8,6 +8,8 @@
 
 namespace launchdarkly {
 
+class ContextBuilder;
+
 /**
  * This is used in the implementation of the context builder for setting
  * attributes for a single context. This is not intended to be directly
@@ -18,6 +20,8 @@ namespace launchdarkly {
  */
 template <class BuilderReturn, class BuildType>
 class AttributesBuilder {
+    friend class ContextBuilder;
+
    public:
     /**
      * Create an attributes builder with the given key.
@@ -26,6 +30,17 @@ class AttributesBuilder {
     AttributesBuilder(BuilderReturn& builder, std::string kind, std::string key)
         : key_(std::move(key)), kind_(std::move(kind)), builder_(builder) {}
 
+    /**
+     * The attributes builder should never be copied. We depend on a stable
+     * reference stored in the context builder.
+     */
+    AttributesBuilder(AttributesBuilder const& builder) = delete;
+    AttributesBuilder& operator=(AttributesBuilder const&) = delete;
+    AttributesBuilder& operator=(AttributesBuilder&&) = delete;
+
+    AttributesBuilder(AttributesBuilder&& builder) noexcept = default;
+    ~AttributesBuilder() = default;
+
     /**
      * The context's name.
      *
@@ -146,8 +161,7 @@ class AttributesBuilder {
         return *this;
     }
 
-    AttributesBuilder kind(std::string kind, std::string key) {
-        builder_.internal_add_kind(kind_, build_attributes());
+    AttributesBuilder& kind(std::string kind, std::string key) {
         return builder_.kind(kind, key);
     }
 
@@ -157,14 +171,18 @@ class AttributesBuilder {
      *
      * @return The built context.
      */
-    [[nodiscard]] BuildType build() {
-        builder_.internal_add_kind(kind_, build_attributes());
-        return builder_.build();
-    }
+    [[nodiscard]] BuildType build() { return builder_.build(); }
 
    private:
     BuilderReturn& builder_;
 
+    /**
+     * Used internally for updating the attributes key.
+     * @param key The key to replace the existing key.
+     * @return A reference to this builder.
+     */
+    void key(std::string key) { key_ = std::move(key); }
+
     Attributes build_attributes();
 
     AttributesBuilder& set(std::string name,

libs/common/include/context.hpp

@@ -17,6 +17,8 @@ struct ContextErrors {
     // generation and persistence is added.
     inline static const std::string kInvalidKey =
         "\"The key for a context may not be empty.\"";
+    inline static const std::string kMissingKinds =
+        "\"The context must contain at least 1 kind.\"";
 };
 
 class ContextBuilder;

libs/common/include/context_builder.hpp

@@ -40,30 +40,59 @@ namespace launchdarkly {
  *      .set("goal", "money")
  *      .build();
  * @endcode
+ *
+ * Using the builder with loops.
+ * @code
+ * auto builder = ContextBuilder();
+ * // The data in this sample is not realistic, but it is intended to show
+ * // how to use the builder with loops.
+ * for (auto const& kind : kinds) { // Some collection we are using to make
+ * kinds.
+ *     // The `kind` method returns a reference, always store it in a reference.
+ *     auto& kind_builder = builder.kind(kind, kind + "-key");
+ *     for (auto const& prop : props) { // A collection of props we want to add.
+ *         kind_builder.set(prop.first, prop.second);
+ *     }
+ * }
+ *
+ * auto context = builder.build();
+ * @endcode
  */
 class ContextBuilder {
     friend AttributesBuilder<ContextBuilder, Context>;
 
    public:
     /**
      * Start adding a kind to the context.
+     *
+     * If you call this function multiple times with the same kind, then
+     * the same builder will be returned each time. If you previously called
+     * the function with the same kind, but different key, then the key
+     * will be updated.
+     *
      * @param kind The kind being added.
      * @param key The key for the kind.
      * @return A builder which allows adding attributes for the kind.
      */
-    AttributesBuilder<ContextBuilder, Context> kind(std::string kind,
-                                                    std::string key);
+    AttributesBuilder<ContextBuilder, Context>& kind(std::string const& kind,
+                                                     std::string key);
 
-   private:
     /**
      * Build a context. This should only be called once, because
-     * the contents of the builder are moved.
+     * the contents of the builder are moved into the created context.
+     *
+     * After the context is build, then this builder, and any kind builders
+     * associated with it, should no longer be used.
+     *
+     * You MUST add at least one kind before building a context. Not doing
+     * so will result in an invalid context.
+     *
      * @return The built context.
      */
     Context build();
 
-    void internal_add_kind(std::string kind, Attributes attrs);
-
+   private:
+    std::map<std::string, AttributesBuilder<ContextBuilder, Context>> builders_;
     std::map<std::string, Attributes> kinds_;
     bool valid_ = true;
     std::string errors_;

libs/common/src/context_builder.cpp

@@ -22,42 +22,64 @@ static bool ValidKind(std::string_view kind) {
     });
 }
 
-AttributesBuilder<ContextBuilder, Context> ContextBuilder::kind(
-    std::string kind,
+AttributesBuilder<ContextBuilder, Context>& ContextBuilder::kind(
+    std::string const& kind,
     std::string key) {
-    bool kind_valid = ValidKind(kind);
-    bool key_valid = !key.empty();
-    if (!kind_valid || !key_valid) {
+    auto existing = builders_.find(kind);
+    if (existing != builders_.end()) {
+        auto& kind_builder = builders_.at(kind);
+        kind_builder.key(key);
+        return kind_builder;
+    }
+
+    builders_.emplace(kind, AttributesBuilder<ContextBuilder, Context>(
+                                *this, kind, std::move(key)));
+    return builders_.at(kind);
+}
+
+Context ContextBuilder::build() {
+    if (builders_.empty()) {
         valid_ = false;
-        auto append = errors_.length() != 0;
-        std::stringstream stream(errors_);
-        if (append) {
-            stream << ", ";
+        if (!errors_.empty()) {
+            errors_.append(", ");
         }
-        if (!kind_valid) {
-            stream << kind << ": " << ContextErrors::kInvalidKind;
-            if (!key_valid) {
+        errors_.append(ContextErrors::kMissingKinds);
+    }
+    // We need to validate all the kinds. Being as kinds could be updated
+    // we cannot do this validation in the `kind` method.
+    for (auto& kind_builder : builders_) {
+        auto const& kind = kind_builder.first;
+        bool kind_valid = ValidKind(kind);
+        bool key_valid = !kind_builder.second.key_.empty();
+        if (!kind_valid || !key_valid) {
+            valid_ = false;
+            auto append = errors_.length() != 0;
+            std::stringstream stream(errors_);
+            if (append) {
                 stream << ", ";
             }
+            if (!kind_valid) {
+                stream << kind << ": " << ContextErrors::kInvalidKind;
+                if (!key_valid) {
+                    stream << ", ";
+                }
+            }
+            if (!key_valid) {
+                stream << kind << ": " << ContextErrors::kInvalidKey;
+            }
+            stream.flush();
+            errors_ = stream.str();
         }
-        if (!key_valid) {
-            stream << kind << ": " << ContextErrors::kInvalidKey;
-        }
-        stream.flush();
-        errors_ = stream.str();
     }
-    return {*this, std::move(kind), std::move(key)};
-}
-
-Context ContextBuilder::build() {
     if (valid_) {
+        for (auto& kind_builder : builders_) {
+            kinds_.emplace(kind_builder.first,
+                           kind_builder.second.build_attributes());
+        }
+        builders_.clear();
         return {std::move(kinds_)};
     }
     return {std::move(errors_)};
 }
 
-void ContextBuilder::internal_add_kind(std::string kind, Attributes attrs) {
-    kinds_.insert_or_assign(std::move(kind), std::move(attrs));
-}
-
 }  // namespace launchdarkly

libs/common/tests/context_builder_test.cpp

@@ -1,5 +1,8 @@
 #include <gtest/gtest.h>
 
+#include <map>
+#include <vector>
+
 #include "context_builder.hpp"
 
 using launchdarkly::ContextBuilder;
@@ -9,6 +12,8 @@ using Object = launchdarkly::Value::Object;
 using Array = launchdarkly::Value::Array;
 using launchdarkly::AttributeReference;
 
+// NOLINTBEGIN cppcoreguidelines-avoid-magic-numbers
+
 TEST(ContextBuilderTests, CanMakeBasicContext) {
     auto context = ContextBuilder().kind("user", "user-key").build();
 
@@ -127,3 +132,48 @@ TEST(ContextBuilderTests, HandlesMultipleErrors) {
         "a context may not be empty.\"",
         context.errors());
 }
+
+TEST(ContextBuilderTests, HandlesEmptyContext) {
+    auto context = ContextBuilder().build();
+    EXPECT_FALSE(context.valid());
+    EXPECT_EQ("\"The context must contain at least 1 kind.\"",
+              context.errors());
+}
+
+TEST(ContextBuilderTests, UseWithLoops) {
+    auto kinds = std::vector<std::string>{"user", "org"};
+    auto props = std::map<std::string, Value>{{"a", "b"}, {"c", "d"}};
+
+    auto builder = ContextBuilder();
+
+    for (auto const& kind : kinds) {
+        auto& kind_builder = builder.kind(kind, kind + "-key");
+        for (auto const& prop : props) {
+            kind_builder.set(prop.first, prop.second);
+        }
+    }
+
+    auto context = builder.build();
+
+    EXPECT_EQ("b", context.get("user", "/a").as_string());
+    EXPECT_EQ("d", context.get("user", "/c").as_string());
+
+    EXPECT_EQ("b", context.get("org", "/a").as_string());
+    EXPECT_EQ("d", context.get("org", "/c").as_string());
+}
+
+TEST(ContextBuilderTests, AccessKindBuilderMultipleTimes) {
+    auto builder = ContextBuilder();
+
+    builder.kind("user", "potato").name("Bob").set("city", "Reno");
+    builder.kind("user", "ham").set("isCat", true);
+
+    auto context = builder.build();
+
+    EXPECT_EQ("ham", context.get("user", "key").as_string());
+    EXPECT_EQ("Bob", context.get("user", "name").as_string());
+    EXPECT_EQ("Reno", context.get("user", "city").as_string());
+    EXPECT_TRUE(context.get("user", "isCat").as_bool());
+}
+
+// NOLINTEND cppcoreguidelines-avoid-magic-numbers