From e8ca616001558216f81b72c7f20a162449e8c6b9 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 1 Oct 2024 11:50:31 -0700 Subject: [PATCH 1/2] feat: add client-side C binding for fetching data source state (#442) We were missing a convenient way to convert a data source status's state variable to a string. This is useful in logging and debugging applications. While adding this, I also applied the "ensure C enum is at least 32 bits wide" treatment that has been done in the past on the LogLevel enum. --- .../launchdarkly/client_side/bindings/c/sdk.h | 16 +++++++++++- .../client_side/data_source_status.hpp | 19 ++++++++------ libs/client-sdk/src/bindings/c/sdk.cpp | 8 ++++++ .../src/data_sources/data_source_status.cpp | 25 ++++++++++--------- .../tests/client_c_bindings_test.cpp | 21 ++++++++++++++++ 5 files changed, 68 insertions(+), 21 deletions(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/sdk.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/sdk.h index e45ba26ad..7ed7a4830 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/sdk.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/sdk.h @@ -501,9 +501,23 @@ enum LDDataSourceStatus_State { * SDK key will never become valid), or because the SDK client was * explicitly shut down. */ - LD_DATASOURCESTATUS_STATE_SHUTDOWN = 4 + LD_DATASOURCESTATUS_STATE_SHUTDOWN = 4, + + LD_DATASOURCESTATUS_STATE_UNUSED_MAXVALUE = + INT32_MAX /* Used to ensure the underlying type is + * at least 32 bits. */ }; +/** + * @param state The state to convert to a string. + * @param default_if_unknown The default string to return if the state is not + * recognized. + * @return Returns the name of the given LDDataSourceStatus_State. + */ +LD_EXPORT(char const*) +LDDataSourceStatus_State_Name(enum LDDataSourceStatus_State state, + char const* default_if_unknown); + /** * Get an enumerated value representing the overall current state of the data * source. diff --git a/libs/client-sdk/include/launchdarkly/client_side/data_source_status.hpp b/libs/client-sdk/include/launchdarkly/client_side/data_source_status.hpp index 4c14572c5..6909ecdd1 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/data_source_status.hpp +++ b/libs/client-sdk/include/launchdarkly/client_side/data_source_status.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -15,7 +16,7 @@ namespace launchdarkly::client_side::data_sources { /** * Enumeration of possible data source states. */ -enum class DataSourceState { +enum class DataSourceState : std::int32_t { /** * The initial state of the data source when the SDK is being * initialized. @@ -62,15 +63,17 @@ enum class DataSourceState { * SDK key will never become valid), or because the SDK client was * explicitly shut down. */ - kShutdown = 4, - - // BackgroundDisabled, - - // TODO: A plugin of sorts would likely be required to implement - // network availability. - // kNetworkUnavailable, + kShutdown = 4 }; +/** + * + * @return Returns the name of the given DataSourceState as a string. If + * the enum value is not recognized, the default string value is returned. + */ +char const* GetDataSourceStateName(DataSourceState state, + char const* default_if_unknown); + using DataSourceStatus = common::data_sources::DataSourceStatusBase; diff --git a/libs/client-sdk/src/bindings/c/sdk.cpp b/libs/client-sdk/src/bindings/c/sdk.cpp index da070b98e..078a4ba55 100644 --- a/libs/client-sdk/src/bindings/c/sdk.cpp +++ b/libs/client-sdk/src/bindings/c/sdk.cpp @@ -352,6 +352,14 @@ LDDataSourceStatus_GetState(LDDataSourceStatus status) { TO_DATASOURCESTATUS(status)->State()); } +LD_EXPORT(char const*) +LDDataSourceStatus_State_Name(enum LDDataSourceStatus_State state, + char const* default_if_unknown) { + return GetDataSourceStateName( + static_cast(state), + default_if_unknown); +} + LD_EXPORT(LDDataSourceStatus_ErrorInfo) LDDataSourceStatus_GetLastError(LDDataSourceStatus status) { LD_ASSERT_NOT_NULL(status); diff --git a/libs/client-sdk/src/data_sources/data_source_status.cpp b/libs/client-sdk/src/data_sources/data_source_status.cpp index 437b5b357..98446622d 100644 --- a/libs/client-sdk/src/data_sources/data_source_status.cpp +++ b/libs/client-sdk/src/data_sources/data_source_status.cpp @@ -4,26 +4,27 @@ namespace launchdarkly::client_side::data_sources { -std::ostream& operator<<(std::ostream& out, - DataSourceStatus::DataSourceState const& state) { +char const* GetDataSourceStateName(DataSourceState state, + char const* default_if_unknown) { switch (state) { case DataSourceStatus::DataSourceState::kInitializing: - out << "INITIALIZING"; - break; + return "INITIALIZING"; case DataSourceStatus::DataSourceState::kValid: - out << "VALID"; - break; + return "VALID"; case DataSourceStatus::DataSourceState::kInterrupted: - out << "INTERRUPTED"; - break; + return "INTERRUPTED"; case DataSourceStatus::DataSourceState::kSetOffline: - out << "OFFLINE"; - break; + return "OFFLINE"; case DataSourceStatus::DataSourceState::kShutdown: - out << "SHUTDOWN"; - break; + return "SHUTDOWN"; + default: + return default_if_unknown; } +} +std::ostream& operator<<(std::ostream& out, + DataSourceStatus::DataSourceState const& state) { + out << GetDataSourceStateName(state, "UNKNOWN"); return out; } diff --git a/libs/client-sdk/tests/client_c_bindings_test.cpp b/libs/client-sdk/tests/client_c_bindings_test.cpp index 42f5dac0c..f0b91cc35 100644 --- a/libs/client-sdk/tests/client_c_bindings_test.cpp +++ b/libs/client-sdk/tests/client_c_bindings_test.cpp @@ -194,3 +194,24 @@ TEST(ClientBindings, ComplexDataSourceStatus) { LDDataSourceStatus_ErrorInfo_Free(info); } + +TEST(ClientBindings, TestDataSourceStatusStateName) { + ASSERT_STREQ(LDDataSourceStatus_State_Name(LD_DATASOURCESTATUS_STATE_VALID, + "unknown"), + "VALID"); + ASSERT_STREQ(LDDataSourceStatus_State_Name( + LD_DATASOURCESTATUS_STATE_OFFLINE, "unknown"), + "OFFLINE"); + ASSERT_STREQ(LDDataSourceStatus_State_Name( + LD_DATASOURCESTATUS_STATE_INITIALIZING, "unknown"), + "INITIALIZING"); + ASSERT_STREQ(LDDataSourceStatus_State_Name( + LD_DATASOURCESTATUS_STATE_SHUTDOWN, "unknown"), + "SHUTDOWN"); + ASSERT_STREQ(LDDataSourceStatus_State_Name( + LD_DATASOURCESTATUS_STATE_INTERRUPTED, "unknown"), + "INTERRUPTED"); + ASSERT_STREQ(LDDataSourceStatus_State_Name( + LD_DATASOURCESTATUS_STATE_UNUSED_MAXVALUE, "unknown"), + "unknown"); +} From 23625ca8f2e5080b1b22bbb7e2e43614a081fe43 Mon Sep 17 00:00:00 2001 From: Casey Waldren Date: Tue, 1 Oct 2024 11:50:48 -0700 Subject: [PATCH 2/2] docs: add warning for accessing builders after consumption (#443) We've had multiple issues filed for accessing builders that have already been consumed. Our docs need to be more prescriptive on this issue. --- .../client_side/bindings/c/config/builder.h | 59 ++++++++++++++++--- .../launchdarkly/bindings/c/context_builder.h | 8 ++- .../server_side/bindings/c/config/builder.h | 50 +++++++++++++--- 3 files changed, 97 insertions(+), 20 deletions(-) diff --git a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h index 7e9b4fe1a..e4fd2c34a 100644 --- a/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h +++ b/libs/client-sdk/include/launchdarkly/client_side/bindings/c/config/builder.h @@ -264,7 +264,12 @@ LD_EXPORT(void) LDClientConfigBuilder_DataSource_UseReport(LDClientConfigBuilder b, bool use_report); /** - * Set the streaming configuration for the builder. + * Set the streaming configuration for the builder. The builder is automatically + * freed. + * + * WARNING: Do not call any other + * LDDataSourceStreamBuilder function on the provided LDDataSourceStreamBuilder + * after calling this function. It is undefined behavior. * * A data source may either be streaming or polling. Setting a streaming * builder indicates the data source will use streaming. Setting a polling @@ -272,7 +277,7 @@ LDClientConfigBuilder_DataSource_UseReport(LDClientConfigBuilder b, * * @param b Client config builder. Must not be NULL. * @param stream_builder The streaming builder. The builder is consumed; do not - * free it. + * free it. Must not be NULL. */ LD_EXPORT(void) LDClientConfigBuilder_DataSource_MethodStream( @@ -280,7 +285,12 @@ LDClientConfigBuilder_DataSource_MethodStream( LDDataSourceStreamBuilder stream_builder); /** - * Set the polling configuration for the builder. + * Set the polling configuration for the builder. The builder is automatically + * freed + * + * WARNING: Do not call any other + * LDDataSourcePollBuilder function on the provided LDDataSourcePollBuilder + * after calling this function. It is undefined behavior. * * A data source may either be streaming or polling. Setting a stream * builder indicates the data source will use streaming. Setting a polling @@ -288,7 +298,7 @@ LDClientConfigBuilder_DataSource_MethodStream( * * @param b Client config builder. Must not be NULL. * @param poll_builder The polling builder. The builder is consumed; do not free - * it. + * it. Must not be NULL. */ LD_EXPORT(void) LDClientConfigBuilder_DataSource_MethodPoll( @@ -396,7 +406,13 @@ LDClientConfigBuilder_HttpProperties_Header(LDClientConfigBuilder b, char const* value); /** - * Sets the TLS options builder. The builder is consumed; do not free it. + * Sets the TLS options builder. The builder is automatically freed. + * + * WARNING: Do not call any other + * LDClientHttpPropertiesTlsBuilder function on the provided + * LDClientHttpPropertiesTlsBuilder after calling this function. + * It is undefined behavior. + * * @param b Client config builder. Must not be NULL. * @param tls_builder The TLS options builder. Must not be NULL. */ @@ -468,7 +484,14 @@ LD_EXPORT(void) LDClientConfigBuilder_Logging_Disable(LDClientConfigBuilder b); /** - * Configures the SDK with basic logging. + * Configures the SDK with basic logging. The logging builder is + * automatically freed. + * + * WARNING: Do not call any other + * LDLoggingBasicBuilder function on the provided + * LDLoggingBasicBuilder after calling this function. + * It is undefined behavior. + * * @param b Client config builder. Must not be NULL. * @param basic_builder The basic logging builder. Must not be NULL. */ @@ -477,7 +500,13 @@ LDClientConfigBuilder_Logging_Basic(LDClientConfigBuilder b, LDLoggingBasicBuilder basic_builder); /** - * Configures the SDK with custom logging. + * Configures the SDK with custom logging. The custom builder is automatically + * freed. + * + * WARNING: Do not call any other + * LDLoggingCustomBuilder function on the provided LDLoggingCustomBuilder after + * calling this function. It is undefined behavior. + * * @param b Client config builder. Must not be NULL. * @param custom_builder The custom logging builder. Must not be NULL. */ @@ -514,7 +543,14 @@ LDPersistenceCustomBuilder_Implementation(LDPersistenceCustomBuilder b, struct LDPersistence impl); /** - * Configures the SDK with custom persistence. + * Configures the SDK with custom persistence. The persistence builder + * is automatically freed. + * + * WARNING: Do not call any other + * LDPersistenceCustomBuilder function on the provided + * LDPersistenceCustomBuilder after calling this function. It is undefined + * behavior. + * * @param b Client config builder. Must not be NULL. * @param custom_builder The custom persistence builder. Must not be NULL. * @return @@ -532,7 +568,12 @@ LD_EXPORT(void) LDClientConfigBuilder_Persistence_None(LDClientConfigBuilder b); /** - * Creates an LDClientConfig. The LDClientConfigBuilder is consumed. + * Creates an LDClientConfig. The builder is automatically freed. + * + * WARNING: Do not call any other + * LDClientConfigBuilder function on the provided LDClientConfigBuilder after + * calling this function. It is undefined behavior. + * * On success, the config will be stored in out_config; otherwise, * out_config will be set to NULL and the returned LDStatus will indicate * the error. diff --git a/libs/common/include/launchdarkly/bindings/c/context_builder.h b/libs/common/include/launchdarkly/bindings/c/context_builder.h index 9c29dc3e2..166c98ba3 100644 --- a/libs/common/include/launchdarkly/bindings/c/context_builder.h +++ b/libs/common/include/launchdarkly/bindings/c/context_builder.h @@ -35,10 +35,12 @@ LD_EXPORT(void) LDContextBuilder_Free(LDContextBuilder builder); /** - * Construct a context from a context builder. + * Construct a context from a context builder. The builder is automatically + * freed. * - * When building a context using @ref LDContextBuilder_Build the builder will be - * consumed and you do not need to call @ref LDContextBuilder_Free. + * WARNING: Do not call any other + * LDContextBuilder function on the provided LDContextBuilder after + * calling this function. It is undefined behavior. * * @param builder The builder to build a context from. Must not be NULL. * @return The built context. diff --git a/libs/server-sdk/include/launchdarkly/server_side/bindings/c/config/builder.h b/libs/server-sdk/include/launchdarkly/server_side/bindings/c/config/builder.h index 3129a9f6f..1e4cbba53 100644 --- a/libs/server-sdk/include/launchdarkly/server_side/bindings/c/config/builder.h +++ b/libs/server-sdk/include/launchdarkly/server_side/bindings/c/config/builder.h @@ -185,7 +185,13 @@ LDServerConfigBuilder_Events_PrivateAttribute(LDServerConfigBuilder b, char const* attribute_reference); /** - * Configures the Background Sync data system with a Streaming synchronizer. + * Configures the Background Sync data system with a Streaming synchronizer. The + * builder is automatically freed. + * + * WARNING: Do not call any other + * LDServerDataSourceStreamBuilder function on the provided + * LDServerDataSourceStreamBuilder after calling this function. It is undefined + * behavior. * * This is the default data system configuration for the SDK. * @@ -196,7 +202,7 @@ LDServerConfigBuilder_Events_PrivateAttribute(LDServerConfigBuilder b, * * @param b Server config builder. Must not be NULL. * @param stream_builder The streaming builder. The builder is consumed; do not - * free it. + * free it. Must not be NULL. */ LD_EXPORT(void) LDServerConfigBuilder_DataSystem_BackgroundSync_Streaming( @@ -204,7 +210,13 @@ LDServerConfigBuilder_DataSystem_BackgroundSync_Streaming( LDServerDataSourceStreamBuilder stream_builder); /** - * Configures the Background Sync data system with a Polling synchronizer. + * Configures the Background Sync data system with a Polling synchronizer. The + * builder is automatically freed. + * + * WARNING: Do not call any other + * LDServerDataSourcePollBuilder function on the provided + * LDServerDataSourcePollBuilder after calling this function. It is undefined + * behavior. * * This synchronizer may be chosen to override the default Streaming mode. * @@ -214,7 +226,7 @@ LDServerConfigBuilder_DataSystem_BackgroundSync_Streaming( * * @param b Server config builder. Must not be NULL. * @param poll_builder The polling builder. The builder is consumed; do not free - * it. + * it. Must not be NULL. */ LD_EXPORT(void) LDServerConfigBuilder_DataSystem_BackgroundSync_Polling( @@ -222,9 +234,15 @@ LDServerConfigBuilder_DataSystem_BackgroundSync_Polling( LDServerDataSourcePollBuilder poll_builder); /** - * Configures the Lazy Load data system. This method is mutually exclusive with + * Configures the Lazy Load data system. The builder is automatically consumed. + * + * This method is mutually exclusive with * the BackgroundSync_Polling and BackgroundSync_Streaming builders. * + * WARNING: Do not call any other + * LDServerLazyLoadBuilder function on the provided LDServerLazyLoadBuilder + * after calling this function. It is undefined behavior. + * * In this mode the SDK will query a data source on-demand as required, with an * in-memory cache to reduce the number of queries. * @@ -396,7 +414,13 @@ LDServerConfigBuilder_HttpProperties_Header(LDServerConfigBuilder b, char const* value); /** - * Sets the TLS options builder. The builder is consumed; do not free it. + * Sets the TLS options builder. The builder is automatically freed. + * + * WARNING: Do not call any other + * LDServerHttpPropertiesTlsBuilder function on the provided + * LDServerHttpPropertiesTlsBuilder after calling this function. It is undefined + * behavior. + * * @param b Server config builder. Must not be NULL. * @param tls_builder The TLS options builder. Must not be NULL. */ @@ -468,7 +492,12 @@ LD_EXPORT(void) LDServerConfigBuilder_Logging_Disable(LDServerConfigBuilder b); /** - * Configures the SDK with basic logging. + * Configures the SDK with basic logging. The builder is automatically freed. + * + * WARNING: Do not call any other + * LDLoggingBasicBuilder function on the provided LDLoggingBasicBuilder after + * calling this function. It is undefined behavior. + * * @param b Server config builder. Must not be NULL. * @param basic_builder The basic logging builder. Must not be NULL. */ @@ -486,7 +515,12 @@ LDServerConfigBuilder_Logging_Custom(LDServerConfigBuilder b, LDLoggingCustomBuilder custom_builder); /** - * Creates an LDClientConfig. The LDServerConfigBuilder is consumed. + * Creates an LDClientConfig. The builder is automatically freed. + * + * WARNING: Do not call any other + * LDServerConfigBuilder function on the provided LDServerConfigBuilder after + * calling this function. It is undefined behavior. + * * On success, the config will be stored in out_config; otherwise, * out_config will be set to NULL and the returned LDStatus will indicate * the error.