feat: implement Identify method (#89)

This commit adds support for `IdentifyAsync`, which causes the client to
shut down its data source, switch contexts, and restart the data source.

Since it returns a future, users can call `wait()` to make it
synchronous. An alternative API would be to accept a completion handler.
That would imply executing user callbacks on a shared or separate
executor, and probably isn't as convenient for the common case of
blocking.

To enable safe shutdown of the Eventsource/Datasources, I had to
refactor them to use shared ownership.

This is because the eventsource client (or polling requester) has
completion handlers queued within asio that have pointers back to the
objects.

If we've shutdown and destroyed those objects, then the handlers will
cause undefined behavior when they execute and access the destroyed
object.

Now, we pass in `weak_ptr` of the Datasource into the Eventsource
client's callbacks, and lock the weak pointer before accessing any
members (such as the logger). So if a completion handler finally
executes long after the object is destroyed, it should be harmless.

Author: cwaldren-ld

.clang-tidy

@@ -1,5 +1,5 @@
 ---
 CheckOptions:
   - { key: readability-identifier-length.IgnoredParameterNames,   value: 'i|j|k|c|os|it' }
-  - { key: readability-identifier-length.IgnoredVariableNames, value: 'ec' }
+  - { key: readability-identifier-length.IgnoredVariableNames, value: 'ec|id' }
   - { key: readability-identifier-length.IgnoredLoopCounterNames,   value: 'i|j|k|c|os|it' }

apps/sdk-contract-tests/include/client_entity.hpp

@@ -13,24 +13,24 @@ class ClientEntity {
 
    private:
     tl::expected<nlohmann::json, std::string> Evaluate(
-        EvaluateFlagParams params);
+        EvaluateFlagParams const&);
 
     tl::expected<nlohmann::json, std::string> EvaluateDetail(
-        EvaluateFlagParams);
+        EvaluateFlagParams const&);
 
     tl::expected<nlohmann::json, std::string> EvaluateAll(
-        EvaluateAllFlagParams params);
+        EvaluateAllFlagParams const&);
 
     tl::expected<nlohmann::json, std::string> Identify(
-        IdentifyEventParams params);
+        IdentifyEventParams const&);
 
-    tl::expected<nlohmann::json, std::string> Custom(CustomEventParams params);
-
-    tl::expected<nlohmann::json, std::string> ContextBuild(
-        ContextBuildParams params);
-
-    tl::expected<nlohmann::json, std::string> ContextConvert(
-        ContextConvertParams params);
+    tl::expected<nlohmann::json, std::string> Custom(CustomEventParams const&);
 
     std::unique_ptr<launchdarkly::client_side::Client> client_;
 };
+
+static tl::expected<nlohmann::json, std::string> ContextConvert(
+    ContextConvertParams const&);
+
+static tl::expected<nlohmann::json, std::string> ContextBuild(
+    ContextBuildParams const&);

apps/sdk-contract-tests/include/entity_manager.hpp

@@ -39,14 +39,15 @@ class EntityManager {
      * @return An ID representing the entity, or none if the entity couldn't
      * be created.
      */
-    std::optional<std::string> create(ConfigParams params);
+    std::optional<std::string> create(ConfigParams const& params);
     /**
      * Destroy an entity with the given ID.
      * @param id ID of the entity.
      * @return True if the entity was found and destroyed.
      */
     bool destroy(std::string const& id);
 
-    tl::expected<nlohmann::json, std::string> command(std::string const& id,
-                                                      CommandParams params);
+    tl::expected<nlohmann::json, std::string> command(
+        std::string const& id,
+        CommandParams const& params);
 };

apps/sdk-contract-tests/src/client_entity.cpp

@@ -7,13 +7,36 @@
 #include <launchdarkly/serialization/json_value.hpp>
 #include <launchdarkly/value.hpp>
 
+#include <chrono>
+#include <future>
+
 ClientEntity::ClientEntity(
     std::unique_ptr<launchdarkly::client_side::Client> client)
     : client_(std::move(client)) {}
 
 tl::expected<nlohmann::json, std::string> ClientEntity::Identify(
-    IdentifyEventParams params) {
-    return tl::make_unexpected("identify not yet supported");
+    IdentifyEventParams const& params) {
+    boost::system::error_code ec;
+    auto json_value = boost::json::parse(params.context.dump(), ec);
+    if (ec) {
+        return tl::make_unexpected(ec.what());
+    }
+
+    auto maybe_ctx = boost::json::value_to<
+        tl::expected<launchdarkly::Context, launchdarkly::JsonError>>(
+        json_value);
+
+    if (!maybe_ctx) {
+        return tl::make_unexpected(
+            launchdarkly::ErrorToString(maybe_ctx.error()));
+    }
+
+    if (!maybe_ctx->valid()) {
+        return tl::make_unexpected(maybe_ctx->errors());
+    }
+
+    client_->IdentifyAsync(*maybe_ctx).wait();
+    return nlohmann::json{};
 }
 
 static void BuildContextFromParams(launchdarkly::ContextBuilder& builder,
@@ -38,8 +61,8 @@ static void BuildContextFromParams(launchdarkly::ContextBuilder& builder,
     }
 }
 
-tl::expected<nlohmann::json, std::string> ClientEntity::ContextBuild(
-    ContextBuildParams params) {
+tl::expected<nlohmann::json, std::string> ContextBuild(
+    ContextBuildParams const& params) {
     ContextResponse resp{};
 
     auto builder = launchdarkly::ContextBuilder();
@@ -62,8 +85,8 @@ tl::expected<nlohmann::json, std::string> ClientEntity::ContextBuild(
     return resp;
 }
 
-tl::expected<nlohmann::json, std::string> ClientEntity::ContextConvert(
-    ContextConvertParams params) {
+tl::expected<nlohmann::json, std::string> ContextConvert(
+    ContextConvertParams const& params) {
     ContextResponse resp{};
 
     boost::system::error_code ec;
@@ -92,7 +115,7 @@ tl::expected<nlohmann::json, std::string> ClientEntity::ContextConvert(
 }
 
 tl::expected<nlohmann::json, std::string> ClientEntity::Custom(
-    CustomEventParams params) {
+    CustomEventParams const& params) {
     auto data = params.data ? boost::json::value_to<launchdarkly::Value>(
                                   boost::json::parse(params.data->dump()))
                             : launchdarkly::Value::Null();
@@ -113,7 +136,7 @@ tl::expected<nlohmann::json, std::string> ClientEntity::Custom(
 }
 
 tl::expected<nlohmann::json, std::string> ClientEntity::EvaluateAll(
-    EvaluateAllFlagParams params) {
+    EvaluateAllFlagParams const& params) {
     EvaluateAllFlagsResponse resp{};
 
     boost::ignore_unused(params);
@@ -127,7 +150,7 @@ tl::expected<nlohmann::json, std::string> ClientEntity::EvaluateAll(
 }
 
 tl::expected<nlohmann::json, std::string> ClientEntity::EvaluateDetail(
-    EvaluateFlagParams params) {
+    EvaluateFlagParams const& params) {
     auto const& key = params.flagKey;
 
     auto const& defaultVal = params.defaultValue;
@@ -202,7 +225,7 @@ tl::expected<nlohmann::json, std::string> ClientEntity::EvaluateDetail(
     return result;
 }
 tl::expected<nlohmann::json, std::string> ClientEntity::Evaluate(
-    EvaluateFlagParams params) {
+    EvaluateFlagParams const& params) {
     if (params.detail) {
         return EvaluateDetail(params);
     }
@@ -279,7 +302,7 @@ tl::expected<nlohmann::json, std::string> ClientEntity::Command(
             }
             return Custom(*params.customEvent);
         case Command::FlushEvents:
-            client_->AsyncFlush();
+            client_->FlushAsync();
             return nlohmann::json{};
         case Command::ContextBuild:
             if (!params.contextBuild) {

apps/sdk-contract-tests/src/entity_manager.cpp

@@ -23,7 +23,7 @@ ParseContext(nlohmann::json value) {
         boost_json_val);
 }
 
-std::optional<std::string> EntityManager::create(ConfigParams in) {
+std::optional<std::string> EntityManager::create(ConfigParams const& in) {
     std::string id = std::to_string(counter_++);
 
     auto config_builder = ConfigBuilder(in.credential);
@@ -159,7 +159,7 @@ bool EntityManager::destroy(std::string const& id) {
 
 tl::expected<nlohmann::json, std::string> EntityManager::command(
     std::string const& id,
-    CommandParams params) {
+    CommandParams const& params) {
     auto it = entities_.find(id);
     if (it == entities_.end()) {
         return tl::make_unexpected("entity not found");

apps/sdk-contract-tests/test-suppressions.txt

@@ -1,56 +1,10 @@
 events/summary events/basic counter behavior
-events/summary events/context kinds
 events/summary events/reset after each flush
-events/identify events/basic properties/single kind default
-events/identify events/basic properties/single kind non-default
-events/identify events/basic properties/multi-kind
-events/custom events/basic properties/single kind default
-events/custom events/basic properties/single kind non-default
-events/custom events/basic properties/multi-kind
-events/context properties/single-kind minimal/debug event
-events/context properties/single-kind minimal/identify event
-events/context properties/multi-kind minimal/debug event
-events/context properties/multi-kind minimal/identify event
-events/context properties/single-kind with attributes, nothing private/debug event
-events/context properties/single-kind with attributes, nothing private/identify event
-events/context properties/single-kind, allAttributesPrivate/debug event
-events/context properties/single-kind, allAttributesPrivate/identify event
-events/context properties/single-kind, specific private attributes/debug event
-events/context properties/single-kind, specific private attributes/identify event
-events/context properties/single-kind, private attribute nested property/debug event
-events/context properties/single-kind, private attribute nested property/identify event
-events/context properties/custom attribute with value false/debug event
-events/context properties/custom attribute with value false/identify event
-events/context properties/custom attribute with value true/debug event
-events/context properties/custom attribute with value true/identify event
-events/context properties/custom attribute with value -1000/debug event
-events/context properties/custom attribute with value -1000/identify event
-events/context properties/custom attribute with value 0/debug event
-events/context properties/custom attribute with value 0/identify event
-events/context properties/custom attribute with value 1000/debug event
-events/context properties/custom attribute with value 1000/identify event
-events/context properties/custom attribute with value -1000.5/debug event
-events/context properties/custom attribute with value -1000.5/identify event
-events/context properties/custom attribute with value 1000.5/debug event
-events/context properties/custom attribute with value 1000.5/identify event
-events/context properties/custom attribute with value ""/debug event
-events/context properties/custom attribute with value ""/identify event
-events/context properties/custom attribute with value "abc"/debug event
-events/context properties/custom attribute with value "abc"/identify event
-events/context properties/custom attribute with value "has \"escaped\" characters"/debug event
-events/context properties/custom attribute with value "has \"escaped\" characters"/identify event
-events/context properties/custom attribute with value []/debug event
-events/context properties/custom attribute with value []/identify event
-events/context properties/custom attribute with value ["a","b"]/debug event
-events/context properties/custom attribute with value ["a","b"]/identify event
-events/context properties/custom attribute with value {}/debug event
-events/context properties/custom attribute with value {}/identify event
-events/context properties/custom attribute with value {"a":1}/debug event
-events/context properties/custom attribute with value {"a":1}/identify event
-events/disabling/identify event
 streaming/requests/query parameters/evaluationReasons set to true/GET
 streaming/requests/query parameters/evaluationReasons set to true/REPORT
 tags/disallowed characters
+
+# The Client doesn't need to know how to deserialize users.
 context type/convert/old user to context/{"key": ""}
 context type/convert/old user to context/{"key": "a"}
 context type/convert/old user to context/{"key": "a"}

apps/sse-contract-tests/include/entity_manager.hpp

@@ -42,7 +42,7 @@ class EntityManager {
      * @return An ID representing the entity, or none if the entity couldn't
      * be created.
      */
-    std::optional<std::string> create(ConfigParams params);
+    std::optional<std::string> create(ConfigParams const& params);
     /**
      * Destroy an entity with the given ID.
      * @param id ID of the entity.

apps/sse-contract-tests/src/entity_manager.cpp

@@ -5,12 +5,9 @@ using launchdarkly::LogLevel;
 
 EntityManager::EntityManager(boost::asio::any_io_executor executor,
                              launchdarkly::Logger& logger)
-    : entities_(),
-      counter_{0},
-      executor_{std::move(executor)},
-      logger_{logger} {}
+    : counter_{0}, executor_{std::move(executor)}, logger_{logger} {}
 
-std::optional<std::string> EntityManager::create(ConfigParams params) {
+std::optional<std::string> EntityManager::create(ConfigParams const& params) {
     std::string id = std::to_string(counter_++);
 
     auto poster = std::make_shared<EventOutbox>(executor_, params.callbackUrl);
@@ -20,8 +17,8 @@ std::optional<std::string> EntityManager::create(ConfigParams params) {
         launchdarkly::sse::Builder(executor_, params.streamUrl);
 
     if (params.headers) {
-        for (auto const& h : *params.headers) {
-            client_builder.header(h.first, h.second);
+        for (auto const& header : *params.headers) {
+            client_builder.header(header.first, header.second);
         }
     }
 
@@ -65,7 +62,7 @@ bool EntityManager::destroy(std::string const& id) {
         return false;
     }
 
-    it->second.first->close();
+    it->second.first->async_shutdown(nullptr);
     it->second.second->stop();
 
     entities_.erase(it);

libs/client-sdk/include/launchdarkly/client_side/client.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <chrono>
+#include <future>
 #include <memory>
 #include <string>
 #include <unordered_map>
@@ -87,16 +88,23 @@ class IClient {
      * Tells the client that all pending analytics events (if any) should be
      * delivered as soon as possible.
      */
-    virtual void AsyncFlush() = 0;
+    virtual void FlushAsync() = 0;
 
     /**
      * Changes the current evaluation context, requests flags for that context
      * from LaunchDarkly if we are online, and generates an analytics event to
      * tell LaunchDarkly about the context.
      *
+     * Only one IdentifyAsync can be in progress at once; calling it
+     * concurrently is undefined behavior.
+     *
+     * To block until the identify operation is complete, call wait() on
+     * the returned future.
+     *
      * @param context The new evaluation context.
      */
-    virtual void AsyncIdentify(Context context) = 0;
+
+    virtual std::future<void> IdentifyAsync(Context context) = 0;
 
     /**
      * Returns the boolean value of a feature flag for a given flag key.
@@ -265,9 +273,9 @@ class Client : public IClient {
 
     void Track(std::string event_name) override;
 
-    void AsyncFlush() override;
+    void FlushAsync() override;
 
-    void AsyncIdentify(Context context) override;
+    std::future<void> IdentifyAsync(Context context) override;
 
     bool BoolVariation(FlagKey const& key, bool default_value) override;
 
@@ -307,4 +315,4 @@ class Client : public IClient {
     std::unique_ptr<IClient> client;
 };
 
-}  // namespace launchdarkly::client_side
+}  // namespace launchdarkly::client_side

libs/client-sdk/src/client.cpp

@@ -28,12 +28,12 @@ void Client::Track(std::string event_name) {
     client->Track(std::move(event_name));
 }
 
-void Client::AsyncFlush() {
-    client->AsyncFlush();
+void Client::FlushAsync() {
+    client->FlushAsync();
 }
 
-void Client::AsyncIdentify(Context context) {
-    client->AsyncIdentify(std::move(context));
+std::future<void> Client::IdentifyAsync(Context context) {
+    return client->IdentifyAsync(std::move(context));
 }
 
 bool Client::BoolVariation(FlagKey const& key, bool default_value) {