Skip to content
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

fix: initialization of LDFlagListener #218

Merged
merged 3 commits into from
Aug 28, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <launchdarkly/bindings/c/context.h>
#include <launchdarkly/bindings/c/data/evaluation_detail.h>
#include <launchdarkly/bindings/c/export.h>
#include <launchdarkly/bindings/c/flag_listener.h>
#include <launchdarkly/bindings/c/listener_connection.h>
#include <launchdarkly/bindings/c/memory_routines.h>
#include <launchdarkly/bindings/c/status.h>
Expand Down Expand Up @@ -412,52 +413,6 @@ LDClientSDK_AllFlags(LDClientSDK sdk);
*/
LD_EXPORT(void) LDClientSDK_Free(LDClientSDK sdk);

typedef void (*FlagChangedCallbackFn)(char const* flag_key,
LDValue new_value,
LDValue old_value,
bool deleted,
void* user_data);

/**
* Defines a feature flag listener which may be used to listen for flag changes.
* The struct should be initialized using LDFlagListener_Init before use.
*/
struct LDFlagListener {
/**
* Callback function which is invoked for flag changes.
*
* The provided pointers are only valid for the duration of the function
* call (excluding UserData, whose lifetime is controlled by the caller).
*
* @param flag_key The name of the flag that changed.
* @param new_value The new value of the flag. If there was not an new
* value, because the flag was deleted, then the LDValue will be of a null
* type. Check the deleted parameter to see if a flag was deleted.
* @param old_value The old value of the flag. If there was not an old
* value, for instance a newly created flag, then the Value will be of a
* null type.
* @param deleted True if the flag has been deleted.
*/
FlagChangedCallbackFn FlagChanged;

/**
* UserData is forwarded into callback functions.
*/
void* UserData;
};

/**
* Initializes a flag listener. Must be called before passing the listener
* to LDClientSDK_FlagNotifier_OnFlagChange.
*
* Create the struct, initialize the struct, set the FlagChanged handler
* and optionally UserData, and then pass the struct to
* LDClientSDK_FlagNotifier_OnFlagChange.
*
* @param listener Listener to initialize.
*/
LD_EXPORT(void) LDFlagListener_Init(struct LDFlagListener listener);

/**
* Listen for changes for the specific flag.
*
Expand Down
5 changes: 0 additions & 5 deletions libs/client-sdk/src/bindings/c/sdk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,6 @@ LD_EXPORT(time_t) LDDataSourceStatus_StateSince(LDDataSourceStatus status) {
.count();
}

LD_EXPORT(void) LDFlagListener_Init(struct LDFlagListener listener) {
listener.FlagChanged = nullptr;
listener.UserData = nullptr;
}

LD_EXPORT(LDDataSourceStatus_ErrorKind)
LDDataSourceStatus_ErrorInfo_GetKind(LDDataSourceStatus_ErrorInfo info) {
LD_ASSERT_NOT_NULL(info);
Expand Down
11 changes: 9 additions & 2 deletions libs/client-sdk/tests/client_c_bindings_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
TEST(ClientBindings, MinimalInstantiation) {
LDClientConfigBuilder cfg_builder = LDClientConfigBuilder_New("sdk-123");

LDClientConfig config;

Check warning on line 17 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:17:20 [cppcoreguidelines-init-variables]

variable 'config' is not initialized
LDStatus status = LDClientConfigBuilder_Build(cfg_builder, &config);
ASSERT_TRUE(LDStatus_Ok(status));

Expand All @@ -33,7 +33,7 @@
}

void FlagListenerFunction(char const* flag_key,
LDValue new_value,

Check warning on line 36 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:36:27 [bugprone-easily-swappable-parameters]

2 adjacent parameters of 'FlagListenerFunction' of similar type ('LDValue') are easily swapped by mistake
LDValue old_value,
bool deleted,
void* user_data) {}
Expand All @@ -44,7 +44,7 @@
LDClientConfigBuilder cfg_builder = LDClientConfigBuilder_New("sdk-123");
LDClientConfigBuilder_Offline(cfg_builder, true);

LDClientConfig config;

Check warning on line 47 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:47:20 [cppcoreguidelines-init-variables]

variable 'config' is not initialized
LDStatus status = LDClientConfigBuilder_Build(cfg_builder, &config);
ASSERT_TRUE(LDStatus_Ok(status));

Expand All @@ -56,12 +56,19 @@
LDClientSDK sdk = LDClientSDK_New(config, context);

bool success = false;
LDClientSDK_Start(sdk, 3000, &success);

Check warning on line 59 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:59:28 [cppcoreguidelines-avoid-magic-numbers

3000 is a magic number; consider replacing it with a named constant
EXPECT_TRUE(success);

struct LDFlagListener listener {};
LDFlagListener_Init(listener);
struct LDFlagListener listener {
reinterpret_cast<FlagChangedCallbackFn>(0x123),

Check warning on line 63 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:63:9 [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

Check warning on line 63 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:63:49 [cppcoreguidelines-avoid-magic-numbers

0x123 is a magic number; consider replacing it with a named constant
reinterpret_cast<void*>(0x456)

Check warning on line 64 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:64:13 [cppcoreguidelines-pro-type-reinterpret-cast]

do not use reinterpret_cast

Check warning on line 64 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:64:37 [cppcoreguidelines-avoid-magic-numbers

0x456 is a magic number; consider replacing it with a named constant
};

LDFlagListener_Init(&listener);
ASSERT_EQ(listener.FlagChanged, nullptr);
ASSERT_EQ(listener.UserData, nullptr);

listener.UserData = const_cast<char*>("Potato");

Check warning on line 71 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:71:25 [cppcoreguidelines-pro-type-const-cast]

do not use const_cast
listener.FlagChanged = FlagListenerFunction;

LDListenerConnection connection =
Expand All @@ -83,7 +90,7 @@
LDClientConfigBuilder cfg_builder = LDClientConfigBuilder_New("sdk-123");
LDClientConfigBuilder_Offline(cfg_builder, true);

LDClientConfig config;

Check warning on line 93 in libs/client-sdk/tests/client_c_bindings_test.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/client-sdk/tests/client_c_bindings_test.cpp:93:20 [cppcoreguidelines-init-variables]

variable 'config' is not initialized
LDStatus status = LDClientConfigBuilder_Build(cfg_builder, &config);
ASSERT_TRUE(LDStatus_Ok(status));

Expand Down
64 changes: 64 additions & 0 deletions libs/common/include/launchdarkly/bindings/c/flag_listener.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/** @file */
// NOLINTBEGIN modernize-use-using

#pragma once

#include <launchdarkly/bindings/c/export.h>
#include <launchdarkly/bindings/c/value.h>

#ifdef __cplusplus
extern "C" { // only need to export C interface if
// used by C++ source code
#endif

typedef void (*FlagChangedCallbackFn)(char const* flag_key,
LDValue new_value,
LDValue old_value,
bool deleted,
void* user_data);

/**
* Defines a feature flag listener which may be used to listen for flag changes.
* The struct should be initialized using LDFlagListener_Init before use.
*/
struct LDFlagListener {
/**
* Callback function which is invoked for flag changes.
*
* The provided pointers are only valid for the duration of the function
* call (excluding UserData, whose lifetime is controlled by the caller).
*
* @param flag_key The name of the flag that changed.
* @param new_value The new value of the flag. If there was not an new
* value, because the flag was deleted, then the LDValue will be of a null
* type. Check the deleted parameter to see if a flag was deleted.
* @param old_value The old value of the flag. If there was not an old
* value, for instance a newly created flag, then the Value will be of a
* null type.
* @param deleted True if the flag has been deleted.
*/
FlagChangedCallbackFn FlagChanged;

/**
* UserData is forwarded into callback functions.
*/
void* UserData;
};

/**
* Initializes a flag listener. Must be called before passing the listener
* to LDClientSDK_FlagNotifier_OnFlagChange.
*
* Create the struct, initialize the struct, set the FlagChanged handler
* and optionally UserData, and then pass the struct to
* LDClientSDK_FlagNotifier_OnFlagChange.
*
* @param listener Listener to initialize.
*/
LD_EXPORT(void) LDFlagListener_Init(struct LDFlagListener* listener);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signature has changed from (struct LDFlagListener listener)


#ifdef __cplusplus
}
#endif

// NOLINTEND modernize-use-using
1 change: 1 addition & 0 deletions libs/common/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ add_library(${LIBNAME} OBJECT
bindings/c/config/logging_builder.cpp
bindings/c/data/evaluation_detail.cpp
bindings/c/listener_connection.cpp
bindings/c/flag_listener.cpp
bindings/c/memory_routines.cpp
log_level.cpp
config/persistence_builder.cpp
Expand Down
6 changes: 6 additions & 0 deletions libs/common/src/bindings/c/flag_listener.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <launchdarkly/bindings/c/flag_listener.h>

LD_EXPORT(void) LDFlagListener_Init(struct LDFlagListener* listener) {
listener->FlagChanged = nullptr;
listener->UserData = nullptr;
}