Skip to content

Commit

Permalink
chore: organize the internal APIs of the C++ binding to ensure they a…
Browse files Browse the repository at this point in the history
…re all private (#368)

* chore: organize the internal APIs of the C++ binding to ensure they are all private

* chore: organize the internal APIs of the C++ binding to ensure they are all private

* chore: organize the internal APIs of the C++ binding to ensure they are all private
  • Loading branch information
halajohn authored Dec 5, 2024
1 parent 83b8d96 commit 5a99b18
Show file tree
Hide file tree
Showing 84 changed files with 250 additions and 278 deletions.
4 changes: 2 additions & 2 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -440,9 +440,9 @@
"name": "app (C/C++) (lldb, launch)",
"type": "lldb",
"request": "launch",
"program": "${workspaceFolder}/out/linux/x64/tests/ten_runtime/integration/cpp/hello_world/hello_world_app/bin/hello_world_app_source",
"program": "${workspaceFolder}/out/linux/x64/tests/ten_runtime/integration/cpp/check_start_graph/check_start_graph_app/bin/check_start_graph_source",
"args": [],
"cwd": "${workspaceFolder}/out/linux/x64/tests/ten_runtime/integration/cpp/hello_world/hello_world_app",
"cwd": "${workspaceFolder}/out/linux/x64/tests/ten_runtime/integration/cpp/check_start_graph/check_start_graph_app",
"env": {
"ASAN_OPTIONS": "use_sigaltstack=0",
},
Expand Down
16 changes: 11 additions & 5 deletions core/include/ten_runtime/binding/cpp/detail/addon.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ class addon_t {
addon_t &operator=(const addon_t &&) = delete;
// @}

// @{
// Internal use only.
::ten_addon_t *get_c_addon() const { return c_addon; }
// @}

protected:
virtual void on_init(ten_env_t &ten_env) { ten_env.on_init_done(); }

Expand All @@ -62,6 +57,10 @@ class addon_t {
ten_addon_t *c_addon;
ten_env_t *cpp_ten_env{};

friend class addon_internal_accessor_t;

::ten_addon_t *get_c_addon() const { return c_addon; }

virtual void on_create_instance_impl(ten_env_t &ten_env, const char *name,
void *context) = 0;

Expand Down Expand Up @@ -202,6 +201,13 @@ struct addon_context_t {

} // namespace

class addon_internal_accessor_t {
public:
static ::ten_addon_t *get_c_addon(const addon_t *addon) {
return addon->get_c_addon();
}
};

class extension_addon_t : public addon_t {
private:
void on_create_instance_impl(ten_env_t &ten_env, const char *name,
Expand Down
6 changes: 4 additions & 2 deletions core/include/ten_runtime/binding/cpp/detail/addon_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@
ten_path_get_module_path(/* NOLINTNEXTLINE */ \
(void *) \
____ten_addon_##NAME##_register_handler__); \
ten_addon_register_extension(#NAME, ten_string_get_raw_str(base_dir), \
addon_instance->get_c_addon(), register_ctx); \
ten_addon_register_extension( \
#NAME, ten_string_get_raw_str(base_dir), \
ten::addon_internal_accessor_t::get_c_addon(addon_instance), \
register_ctx); \
ten_string_destroy(base_dir); \
} \
TEN_CONSTRUCTOR(____ten_addon_##NAME##_registrar____) { \
Expand Down
15 changes: 10 additions & 5 deletions core/include/ten_runtime/binding/cpp/detail/extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ class extension_t {
extension_t &operator=(const extension_t &&) = delete;
// @}

// @{
// Internal use only.
::ten_extension_t *get_c_extension() const { return c_extension; }
// @}

protected:
explicit extension_t(const std::string &name)
: // In order to keep type safety in C++, the type of the 'ten'
Expand Down Expand Up @@ -125,6 +120,9 @@ class extension_t {
private:
friend class ten_env_t;
friend class extension_group_t;
friend class extension_internal_accessor_t;

::ten_extension_t *get_c_extension() const { return c_extension; }

using cpp_extension_on_cmd_func_t =
void (extension_t:: *)(ten_env_t &, std::unique_ptr<cmd_t>);
Expand Down Expand Up @@ -532,4 +530,11 @@ class extension_t {
ten_env_t *cpp_ten_env;
};

class extension_internal_accessor_t {
public:
static ::ten_extension_t *get_c_extension(const extension_t *extension) {
return extension->get_c_extension();
}
};

} // namespace ten
54 changes: 27 additions & 27 deletions core/include/ten_runtime/binding/cpp/detail/ten_env.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,25 +201,6 @@ class ten_env_t {
return rc;
}

static void proxy_handle_return_error(::ten_env_t *ten_env, void *user_data,
::ten_error_t *err) {
TEN_ASSERT(ten_env, "Should not happen.");

auto *error_handler = static_cast<error_handler_func_t *>(user_data);
auto *cpp_ten_env =
static_cast<ten_env_t *>(ten_binding_handle_get_me_in_target_lang(
reinterpret_cast<ten_binding_handle_t *>(ten_env)));

if (err != nullptr) {
error_t cpp_err(err, false);
(*error_handler)(*cpp_ten_env, &cpp_err);
} else {
(*error_handler)(*cpp_ten_env, nullptr);
}

delete error_handler;
}

// If the 'cmd' has already been a command in the backward path, a extension
// could use this API to return the 'cmd' further.
bool return_result_directly(std::unique_ptr<cmd_result_t> &&cmd,
Expand Down Expand Up @@ -736,14 +717,6 @@ class ten_env_t {
return rc;
}

void *get_attached_target(error_t *err = nullptr) {
TEN_ASSERT(c_ten_env, "Should not happen.");

return ten_binding_handle_get_me_in_target_lang(
reinterpret_cast<ten_binding_handle_t *>(
ten_env_get_attached_target(c_ten_env)));
}

#define TEN_ENV_LOG_VERBOSE(ten_env, msg) \
do { \
(ten_env).log(TEN_LOG_LEVEL_VERBOSE, __func__, __FILE__, __LINE__, (msg)); \
Expand Down Expand Up @@ -804,8 +777,35 @@ class ten_env_t {

::ten_env_t *get_c_ten_env() { return c_ten_env; }

void *get_attached_target(error_t *err = nullptr) {
TEN_ASSERT(c_ten_env, "Should not happen.");

return ten_binding_handle_get_me_in_target_lang(
reinterpret_cast<ten_binding_handle_t *>(
ten_env_get_attached_target(c_ten_env)));
}

bool init_manifest_from_json(const char *json_str, error_t *err);

static void proxy_handle_return_error(::ten_env_t *ten_env, void *user_data,
::ten_error_t *err) {
TEN_ASSERT(ten_env, "Should not happen.");

auto *error_handler = static_cast<error_handler_func_t *>(user_data);
auto *cpp_ten_env =
static_cast<ten_env_t *>(ten_binding_handle_get_me_in_target_lang(
reinterpret_cast<ten_binding_handle_t *>(ten_env)));

if (err != nullptr) {
error_t cpp_err(err, false);
(*error_handler)(*cpp_ten_env, &cpp_err);
} else {
(*error_handler)(*cpp_ten_env, nullptr);
}

delete error_handler;
}

bool send_cmd_internal(std::unique_ptr<cmd_t> &&cmd,
result_handler_func_t &&result_handler = nullptr,
bool is_ex = false, error_t *err = nullptr) {
Expand Down
5 changes: 2 additions & 3 deletions core/include/ten_runtime/binding/cpp/detail/ten_env_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
//
#pragma once

#include <vector>

#include "ten_runtime/binding/cpp/detail/addon.h"
#include "ten_runtime/binding/cpp/detail/extension.h"
#include "ten_runtime/binding/cpp/detail/ten_env.h"
Expand All @@ -27,7 +25,8 @@ inline bool ten_env_t::on_create_instance_done(void *instance, void *context,

switch (cpp_context->task) {
case ADDON_TASK_CREATE_EXTENSION:
c_instance = static_cast<extension_t *>(instance)->get_c_extension();
c_instance = extension_internal_accessor_t::get_c_extension(
static_cast<extension_t *>(instance));
break;
default:
TEN_ASSERT(0, "Should not happen.");
Expand Down
39 changes: 5 additions & 34 deletions core/include/ten_runtime/binding/cpp/detail/ten_env_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class ten_env_proxy_t {
ten_env_proxy_t &operator=(ten_env_proxy_t &&other) = delete;
// @}

bool acquire_lock_mode(error_t *err) {
bool acquire_lock_mode(error_t *err = nullptr) {
if (c_ten_env_proxy == nullptr) {
TEN_ASSERT(0, "Invalid argument.");
return false;
Expand All @@ -112,9 +112,7 @@ class ten_env_proxy_t {
c_ten_env_proxy, err != nullptr ? err->get_c_error() : nullptr);
}

bool acquire_lock_mode() { return acquire_lock_mode(nullptr); }

bool release_lock_mode(error_t *err) {
bool release_lock_mode(error_t *err = nullptr) {
if (c_ten_env_proxy == nullptr) {
TEN_ASSERT(0, "Invalid argument.");
return false;
Expand All @@ -124,9 +122,8 @@ class ten_env_proxy_t {
c_ten_env_proxy, err != nullptr ? err->get_c_error() : nullptr);
}

bool release_lock_mode() { return release_lock_mode(nullptr); }

bool notify(notify_std_func_t &&notify_func, bool sync, error_t *err) {
bool notify(notify_std_func_t &&notify_func, bool sync = false,
error_t *err = nullptr) {
auto *info = new proxy_notify_info_t(std::move(notify_func));

auto rc =
Expand All @@ -139,20 +136,8 @@ class ten_env_proxy_t {
return rc;
}

bool notify(notify_std_func_t &&notify_func, bool sync) {
return notify(std::move(notify_func), sync, nullptr);
}

bool notify(notify_std_func_t &&notify_func, error_t *err) {
return notify(std::move(notify_func), false, err);
}

bool notify(notify_std_func_t &&notify_func) {
return notify(std::move(notify_func), false, nullptr);
}

bool notify(notify_std_with_user_data_func_t &&notify_func, void *user_data,
bool sync, error_t *err) {
bool sync = false, error_t *err = nullptr) {
auto *info = new proxy_notify_info_t(std::move(notify_func), user_data);

auto rc =
Expand All @@ -165,20 +150,6 @@ class ten_env_proxy_t {
return rc;
}

bool notify(notify_std_with_user_data_func_t &&notify_func, void *user_data,
bool sync) {
return notify(std::move(notify_func), user_data, sync, nullptr);
}

bool notify(notify_std_with_user_data_func_t &&notify_func, void *user_data,
error_t *err) {
return notify(std::move(notify_func), user_data, false, err);
}

bool notify(notify_std_with_user_data_func_t &&notify_func, void *user_data) {
return notify(std::move(notify_func), user_data, false, nullptr);
}

private:
::ten_env_proxy_t *c_ten_env_proxy;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,30 +12,18 @@ namespace ten {

class ten_env_internal_accessor_t {
public:
explicit ten_env_internal_accessor_t(ten_env_t *ten_env) : ten_env(ten_env) {}
~ten_env_internal_accessor_t() = default;

// @{
ten_env_internal_accessor_t(const ten_env_internal_accessor_t &) = delete;
ten_env_internal_accessor_t(ten_env_internal_accessor_t &&) = delete;
ten_env_internal_accessor_t &operator=(const ten_env_internal_accessor_t &) =
delete;
ten_env_internal_accessor_t &operator=(ten_env_internal_accessor_t &&) =
delete;
// @}

bool init_manifest_from_json(const char *json_str, error_t *err) {
return ten_env->init_manifest_from_json(json_str, err);
static bool init_manifest_from_json(ten_env_t &ten_env, const char *json_str,
error_t *err = nullptr) {
return ten_env.init_manifest_from_json(json_str, err);
}

bool init_manifest_from_json(const char *json_str) {
return ten_env->init_manifest_from_json(json_str, nullptr);
static ::ten_env_t *get_c_ten_env(ten_env_t &ten_env) {
return ten_env.get_c_ten_env();
}

::ten_env_t *get_c_ten_env() const { return ten_env->get_c_ten_env(); }

private:
ten_env_t *ten_env;
static void *get_attached_target(ten_env_t &ten_env, error_t *err = nullptr) {
return ten_env.get_attached_target(err);
}
};

} // namespace ten
12 changes: 7 additions & 5 deletions packages/core_extensions/py_init_extension_cpp/src/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "include_internal/ten_runtime/metadata/manifest.h"
#include "include_internal/ten_runtime/ten_env/ten_env.h"
#include "ten_runtime/addon/addon_manager.h"
#include "ten_runtime/binding/cpp/detail/addon.h"
#include "ten_runtime/binding/cpp/detail/ten_env.h"
#include "ten_runtime/binding/cpp/ten.h"
#include "ten_utils/container/list_str.h"
Expand Down Expand Up @@ -262,8 +263,8 @@ class py_init_addon_t : public ten::addon_t {
// can perform. Through a private API, it accesses the C `ten_env_t`,
// enabling special operations that only TEN framework developers are
// allowed to execute.
ten::ten_env_internal_accessor_t ten_env_internal_accessor(&ten_env);
ten_env_t *c_ten_env = ten_env_internal_accessor.get_c_ten_env();
ten_env_t *c_ten_env =
ten::ten_env_internal_accessor_t::get_c_ten_env(ten_env);
auto *c_app = static_cast<ten_app_t *>(
c_ten_env->attached_target.addon_host->user_data);
TEN_ASSERT(c_app, "Should not happen.");
Expand Down Expand Up @@ -435,9 +436,10 @@ static void ____ten_addon_py_init_extension_cpp_register_handler____(
ten_path_get_module_path(/* NOLINTNEXTLINE */
(void *)
____ten_addon_py_init_extension_cpp_register_handler____);
ten_addon_register_extension("py_init_extension_cpp",
ten_string_get_raw_str(base_dir),
addon_instance->get_c_addon(), register_ctx);
ten_addon_register_extension(
"py_init_extension_cpp", ten_string_get_raw_str(base_dir),
ten::addon_internal_accessor_t::get_c_addon(addon_instance),
register_ctx);
ten_string_destroy(base_dir);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include <nlohmann/json.hpp>
#include <thread>

#include "include_internal/ten_runtime/binding/cpp/ten.h"
#include "include_internal/ten_utils/lib/buf.h"
#include "ten_runtime/binding/cpp/ten.h"
#include "ten_utils/lib/buf.h"

#define DEFAULT_BUF_CAPACITY 512
Expand Down Expand Up @@ -648,7 +648,8 @@ void send_ten_msg_with_req_body(
}

auto *ext = static_cast<http_server_extension_t *>(
ten_env.get_attached_target());
ten::ten_env_internal_accessor_t::get_attached_target(
ten_env));
assert(ext && "Failed to get the attached extension.");

if (!ext->is_stopping) {
Expand Down Expand Up @@ -687,7 +688,8 @@ void send_ten_msg_without_req_body(
}

auto *ext = static_cast<http_server_extension_t *>(
ten_env.get_attached_target());
ten::ten_env_internal_accessor_t::get_attached_target(
ten_env));
assert(ext && "Failed to get the attached extension.");

if (!ext->is_stopping) {
Expand Down
4 changes: 2 additions & 2 deletions tests/ten_runtime/smoke/cmd_conversion/cmd_conversion_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ class test_extension_2 : public ten::extension_t {
class test_app : public ten::app_t {
public:
void on_configure(ten::ten_env_t &ten_env) override {
ten::ten_env_internal_accessor_t ten_env_internal_accessor(&ten_env);
bool rc = ten_env_internal_accessor.init_manifest_from_json(
bool rc = ten::ten_env_internal_accessor_t::init_manifest_from_json(
ten_env,
// clang-format off
R"###({
"type": "app",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class test_extension_2 : public ten::extension_t {
class test_app : public ten::app_t {
public:
void on_configure(ten::ten_env_t &ten_env) override {
ten::ten_env_internal_accessor_t ten_env_internal_accessor(&ten_env);
bool rc = ten_env_internal_accessor.init_manifest_from_json(
bool rc = ten::ten_env_internal_accessor_t::init_manifest_from_json(
ten_env,
// clang-format off
R"###({
"type": "app",
Expand Down
Loading

0 comments on commit 5a99b18

Please sign in to comment.