-
Notifications
You must be signed in to change notification settings - Fork 3
feat: incomplete C bindings for client configuration #45
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
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| #include <functional> | ||
| #include <tl/expected.hpp> | ||
| #include "c_bindings/status.h" | ||
| #include "error.hpp" | ||
|
|
||
| template <typename T, typename = void> | ||
| struct has_result_type : std::false_type {}; | ||
|
|
||
| template <typename T> | ||
| struct has_result_type<T, std::void_t<typename T::Result>> : std::true_type {}; | ||
|
|
||
| template <typename T, typename ReturnType, typename = void> | ||
| struct has_build_method : std::false_type {}; | ||
|
|
||
| template <typename T, typename ReturnType> | ||
| struct has_build_method<T, | ||
| ReturnType, | ||
| std::void_t<decltype(std::declval<T>().Build())>> | ||
| : std::integral_constant< | ||
| bool, | ||
| std::is_same_v<decltype(std::declval<T>().Build()), ReturnType>> {}; | ||
|
|
||
| // NOLINTBEGIN cppcoreguidelines-pro-type-reinterpret-cast | ||
|
|
||
| /* | ||
| * Given a Builder, calls the Build() method and converts it into an | ||
| * OpaqueResult if successful, or an LDError if unsuccessful. | ||
| * | ||
| * In the case of an error, out_result is set to nullptr. | ||
| * | ||
| * In all cases, the given builder is freed. | ||
| */ | ||
| template <typename Builder, typename OpaqueBuilder, typename OpaqueResult> | ||
| LDStatus ConsumeBuilder(OpaqueBuilder opaque_builder, | ||
| OpaqueResult* out_result) { | ||
| using ReturnType = | ||
| tl::expected<typename Builder::Result, launchdarkly::Error>; | ||
|
|
||
| static_assert(has_result_type<Builder>::value, | ||
| "Builder must have an associated type named Result"); | ||
|
|
||
| static_assert( | ||
| has_build_method<Builder, ReturnType>::value, | ||
| "Builder must have a Build method that returns " | ||
| "tl::expected<typename Builder::Result, launchdarkly::Error>"); | ||
|
|
||
| auto builder = reinterpret_cast<Builder*>(opaque_builder); | ||
|
|
||
| tl::expected<typename Builder::Result, launchdarkly::Error> res = | ||
| builder->Build(); | ||
|
|
||
| delete builder; | ||
|
|
||
| if (!res) { | ||
| *out_result = nullptr; | ||
| return reinterpret_cast<LDStatus>(new launchdarkly::Error(res.error())); | ||
| } | ||
|
|
||
| *out_result = reinterpret_cast<OpaqueResult>( | ||
| new typename Builder::Result(std::move(res.value()))); | ||
|
|
||
| return LDStatus_Success(); | ||
| } | ||
|
|
||
| // NOLINTEND cppcoreguidelines-pro-type-reinterpret-cast | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // NOLINTBEGIN modernize-use-using | ||
|
|
||
| #pragma once | ||
|
|
||
| #include "../export.h" | ||
| #include "../status.h" | ||
| #include "./config.h" | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" { // only need to export C interface if | ||
| // used by C++ source code | ||
| #endif | ||
|
|
||
| typedef struct _LDClientConfigBuilder* LDClientConfigBuilder; | ||
|
|
||
| LD_EXPORT(LDClientConfigBuilder) LDClientConfigBuilder_New(char const* sdk_key); | ||
|
|
||
| /** | ||
| * Creates an LDClientConfig. The LDClientConfigBuilder is consumed. | ||
| * 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. | ||
| * @param builder Builder to consume. | ||
| * @param out_config Pointer to where the built config will be | ||
| * stored. | ||
| * @return Error status on failure. | ||
| */ | ||
| LD_EXPORT(LDStatus) | ||
| LDClientConfigBuilder_Build(LDClientConfigBuilder builder, | ||
| LDClientConfig* out_config); | ||
|
|
||
| /** | ||
| * Frees the builder; only necessary if not calling Build. | ||
| * @param builder Builder to free. | ||
| */ | ||
| LD_EXPORT(void) | ||
| LDClientConfigBuilder_Free(LDClientConfigBuilder builder); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| // NOLINTEND modernize-use-using |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // NOLINTBEGIN modernize-use-using | ||
|
|
||
| #pragma once | ||
|
|
||
| #include "../export.h" | ||
| #include "../status.h" | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" { // only need to export C interface if | ||
| // used by C++ source code | ||
| #endif | ||
|
|
||
| typedef struct _LDClientConfig* LDClientConfig; | ||
|
|
||
| /** | ||
| * Free the configuration. Configurations passed into an LDClient do not need to | ||
| * be freed. | ||
| * @param config Config to free. | ||
| */ | ||
| LD_EXPORT(void) LDClientConfig_Free(LDClientConfig config); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| // NOLINTEND modernize-use-using |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // NOLINTBEGIN modernize-use-using | ||
|
|
||
| #pragma once | ||
|
|
||
| #include "./export.h" | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" { // only need to export C interface if | ||
| // used by C++ source code | ||
| #endif | ||
|
|
||
| typedef struct _LDStatus* LDStatus; | ||
|
|
||
| /** | ||
| * Returns a string representing the error stored in an LDStatus, or | ||
| * NULL if the result indicates success. | ||
| * @param error Result to inspect. | ||
| * @return String or NULL. The returned string is valid only while the LDStatus | ||
| * is alive. | ||
| */ | ||
| LD_EXPORT(char const*) LDStatus_Error(LDStatus res); | ||
|
|
||
| /** | ||
| * Checks if a result indicates success. | ||
| * @param result Result to inspect. | ||
| * @return True if the result indicates success. | ||
| */ | ||
| LD_EXPORT(bool) LDStatus_Ok(LDStatus res); | ||
|
|
||
| /** | ||
| * Frees an LDStatus. It is only necessary to call LDStatus_Free if LDStatus_Ok | ||
| * returns false. | ||
| * @param res Result to free. | ||
| */ | ||
| LD_EXPORT(void) LDStatus_Free(LDStatus res); | ||
|
|
||
| /** | ||
| * Returns a status representing success. | ||
| * @return Successful status. | ||
| */ | ||
| LD_EXPORT(LDStatus) LDStatus_Success(void); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| // NOLINTEND modernize-use-using |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // NOLINTBEGIN cppcoreguidelines-pro-type-reinterpret-cast | ||
| // NOLINTBEGIN OCInconsistentNamingInspection | ||
|
|
||
| #include "c_bindings/config/builder.h" | ||
|
|
||
| #include "c_binding_helpers.hpp" | ||
| #include "config/client.hpp" | ||
|
|
||
| using namespace launchdarkly::client_side; | ||
|
|
||
| #define TO_BUILDER(ptr) (reinterpret_cast<ConfigBuilder*>(ptr)) | ||
| #define FROM_BUILDER(ptr) (reinterpret_cast<LDClientConfigBuilder>(ptr)) | ||
|
|
||
| LD_EXPORT(LDClientConfigBuilder) | ||
| LDClientConfigBuilder_New(char const* sdk_key) { | ||
| return FROM_BUILDER(new ConfigBuilder(sdk_key ? sdk_key : "")); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe as of c++2023 it will be illegal to even pass nullptr to an |
||
| } | ||
|
|
||
| LD_EXPORT(void) | ||
| LDClientConfigBuilder_Free(LDClientConfigBuilder builder) { | ||
| if (ConfigBuilder* b = TO_BUILDER(builder)) { | ||
| delete b; | ||
| } | ||
| } | ||
|
|
||
| LD_EXPORT(LDStatus) | ||
| LDClientConfigBuilder_Build(LDClientConfigBuilder builder, | ||
| LDClientConfig* out_config) { | ||
| return ConsumeBuilder<ConfigBuilder>(builder, out_config); | ||
| } | ||
|
|
||
| // NOLINTEND cppcoreguidelines-pro-type-reinterpret-cast | ||
| // NOLINTEND OCInconsistentNamingInspection | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // NOLINTBEGIN cppcoreguidelines-pro-type-reinterpret-cast | ||
| // NOLINTBEGIN OCInconsistentNamingInspection | ||
|
|
||
| #include "c_bindings/config/config.h" | ||
| #include "config/client.hpp" | ||
|
|
||
| #define TO_CONFIG(ptr) (reinterpret_cast<Config*>(ptr)) | ||
| #define FROM_CONFIG(ptr) (reinterpret_cast<LDClientConfig>(ptr)) | ||
|
|
||
| using namespace launchdarkly::client_side; | ||
|
|
||
| LD_EXPORT(void) LDClientConfig_Free(LDClientConfig config) { | ||
| if (Config* c = TO_CONFIG(config)) { | ||
| delete c; | ||
| } | ||
| } | ||
|
|
||
| // NOLINTEND cppcoreguidelines-pro-type-reinterpret-cast | ||
| // NOLINTEND OCInconsistentNamingInspection |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // NOLINTBEGIN cppcoreguidelines-pro-type-reinterpret-cast | ||
| // NOLINTBEGIN OCInconsistentNamingInspection | ||
|
|
||
| #include "c_bindings/status.h" | ||
|
|
||
| #include "error.hpp" | ||
|
|
||
| using namespace launchdarkly; | ||
|
|
||
| #define TO_ERROR(ptr) (reinterpret_cast<Error*>(ptr)) | ||
|
|
||
| LD_EXPORT(char const*) LDStatus_Error(LDStatus res) { | ||
| if (Error* e = TO_ERROR(res)) { | ||
| return ErrorToString(*e); | ||
| } | ||
| return nullptr; | ||
| } | ||
|
|
||
| LD_EXPORT(bool) LDStatus_Ok(LDStatus res) { | ||
| if (TO_ERROR(res) == nullptr) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| LD_EXPORT(void) LDStatus_Free(LDStatus error) { | ||
| if (Error* e = TO_ERROR(error)) { | ||
| delete e; | ||
| } | ||
| } | ||
|
|
||
| LD_EXPORT(LDStatus) LDStatus_Success(void) { | ||
| return nullptr; | ||
| } | ||
|
|
||
| // NOLINTEND cppcoreguidelines-pro-type-reinterpret-cast | ||
| // NOLINTEND OCInconsistentNamingInspection |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #include <gtest/gtest.h> | ||
| #include "c_bindings/config/builder.h" | ||
|
|
||
| TEST(ClientConfigBindings, ConfigBuilderNewFree) { | ||
| LDClientConfigBuilder builder = LDClientConfigBuilder_New("sdk-123"); | ||
| ASSERT_TRUE(builder); | ||
| LDClientConfigBuilder_Free(builder); | ||
| } | ||
|
|
||
| TEST(ClientConfigBindings, ConfigBuilderEmptyResultsInError) { | ||
| LDClientConfigBuilder builder = LDClientConfigBuilder_New(nullptr); | ||
|
|
||
| LDClientConfig config = nullptr; | ||
| LDStatus status = LDClientConfigBuilder_Build(builder, &config); | ||
|
|
||
| ASSERT_FALSE(config); | ||
| ASSERT_FALSE(LDStatus_Ok(status)); | ||
| ASSERT_TRUE(LDStatus_Error(status)); | ||
|
|
||
| LDStatus_Free(status); | ||
| // LDClientConfigBuilder is consumed by Build; no need to free it. | ||
| } | ||
|
|
||
| TEST(ClientConfigBindings, MinimalValidConfig) { | ||
| LDClientConfigBuilder builder = LDClientConfigBuilder_New("sdk-123"); | ||
|
|
||
| LDClientConfig config = nullptr; | ||
| LDStatus status = LDClientConfigBuilder_Build(builder, &config); | ||
| ASSERT_TRUE(LDStatus_Ok(status)); | ||
| ASSERT_TRUE(config); | ||
|
|
||
| LDClientConfig_Free(config); | ||
| // LDClientConfigBuilder is consumed by Build; no need to free it. | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #include <gtest/gtest.h> | ||
|
|
||
| #include "error.hpp" | ||
|
|
||
| #include "c_bindings/status.h" | ||
|
|
||
| // NOLINTBEGIN cppcoreguidelines-pro-type-reinterpret-cast | ||
|
|
||
| TEST(StatusBindingTests, StatusOk) { | ||
| LDStatus status = LDStatus_Success(); | ||
| ASSERT_TRUE(LDStatus_Ok(status)); | ||
| ASSERT_FALSE(LDStatus_Error(status)); | ||
| LDStatus_Free(status); | ||
| } | ||
|
|
||
| TEST(StatusBindingTests, StatusError) { | ||
| using namespace launchdarkly; | ||
|
|
||
| Error err = Error::kConfig_SDKKey_Empty; | ||
|
|
||
| auto status = reinterpret_cast<LDStatus>(new Error(err)); | ||
| ASSERT_FALSE(LDStatus_Ok(status)); | ||
| ASSERT_TRUE(LDStatus_Error(status)); | ||
| ASSERT_STREQ(LDStatus_Error(status), ErrorToString(err)); | ||
| LDStatus_Free(status); | ||
| } | ||
|
|
||
| // NOLINTEND cppcoreguidelines-pro-type-reinterpret-cast |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this a helper so we can ensure builders are handled consistently across the C binding (like - always assigning nullptr to out result on failure, or whatever we choose.)
I do wonder this method should consume the builder, though. It's not consuming in C++, so you could theoretically inspect the error and allow for rebuilding until the problem is fixed - but I'm not sure that would be a common use-case.