diff --git a/include/ProxySQL_Plugin.h b/include/ProxySQL_Plugin.h index d7933c80b..22fe253bc 100644 --- a/include/ProxySQL_Plugin.h +++ b/include/ProxySQL_Plugin.h @@ -3,78 +3,77 @@ #include #include -#include -enum ProxySQL_PluginDBKind { - admin_db, - config_db, - stats_db, +class SQLite3DB; +class SQLite3_result; + +enum class ProxySQL_PluginDBKind : uint8_t { + admin_db = 0, + config_db = 1, + stats_db = 2 }; struct ProxySQL_PluginTableDef { - ProxySQL_PluginDBKind db_kind{admin_db}; - const char *schema_name{nullptr}; - const char *table_name{nullptr}; - const char *create_statement{nullptr}; + ProxySQL_PluginDBKind db_kind; + const char *table_name; + const char *table_def; }; struct ProxySQL_PluginCommandContext { - const char *command_name{nullptr}; - std::vector arguments; + SQLite3DB *admindb; + SQLite3DB *configdb; + SQLite3DB *statsdb; }; struct ProxySQL_PluginCommandResult { - bool success{true}; + int error_code; + uint64_t rows_affected; std::string message; - std::vector> rows; }; -class ProxySQL_PluginServices; - using proxysql_plugin_admin_command_cb = - bool (*)(const ProxySQL_PluginCommandContext &, ProxySQL_PluginCommandResult &, void *); + ProxySQL_PluginCommandResult (*)(const ProxySQL_PluginCommandContext &, const char *); using proxysql_plugin_register_table_cb = - bool (*)(void *, const ProxySQL_PluginTableDef &); + void (*)(const ProxySQL_PluginTableDef &); using proxysql_plugin_register_command_cb = - bool (*)(void *, const char *, proxysql_plugin_admin_command_cb, void *); + void (*)(const char *, proxysql_plugin_admin_command_cb); using proxysql_plugin_snapshot_cb = - bool (*)(void *, std::vector> &); + SQLite3_result *(*)(); using proxysql_plugin_log_message_cb = - void (*)(void *, int, const char *); + void (*)(int, const char *); struct ProxySQL_PluginServices { - proxysql_plugin_register_table_cb register_table{nullptr}; - proxysql_plugin_register_command_cb register_command{nullptr}; - proxysql_plugin_snapshot_cb get_mysql_users_snapshot{nullptr}; - proxysql_plugin_snapshot_cb get_mysql_servers_snapshot{nullptr}; - proxysql_plugin_snapshot_cb get_mysql_group_replication_hostgroups_snapshot{nullptr}; - proxysql_plugin_log_message_cb log_message{nullptr}; - void *context{nullptr}; + proxysql_plugin_register_table_cb register_table; + proxysql_plugin_register_command_cb register_command; + proxysql_plugin_snapshot_cb get_mysql_users_snapshot; + proxysql_plugin_snapshot_cb get_mysql_servers_snapshot; + proxysql_plugin_snapshot_cb get_mysql_group_replication_hostgroups_snapshot; + proxysql_plugin_log_message_cb log_message; }; using proxysql_plugin_init_cb = - bool (*)(ProxySQL_PluginServices *, std::string &); + bool (*)(ProxySQL_PluginServices *); using proxysql_plugin_start_cb = - bool (*)(std::string &); + bool (*)(); using proxysql_plugin_stop_cb = bool (*)(); -using proxysql_plugin_status_cb = - bool (*)(std::string &); +using proxysql_plugin_status_json_cb = + const char *(*)(); struct ProxySQL_PluginDescriptor { - uint32_t abi_version{0}; - const char *name{nullptr}; - proxysql_plugin_init_cb init{nullptr}; - proxysql_plugin_start_cb start{nullptr}; - proxysql_plugin_stop_cb stop{nullptr}; - proxysql_plugin_status_cb status{nullptr}; + const char *name; + uint32_t abi_version; + proxysql_plugin_init_cb init; + proxysql_plugin_start_cb start; + proxysql_plugin_stop_cb stop; + proxysql_plugin_status_json_cb status_json; }; using proxysql_plugin_descriptor_v1_t = const ProxySQL_PluginDescriptor *(*)(); diff --git a/lib/ProxySQL_PluginManager.cpp b/lib/ProxySQL_PluginManager.cpp index 943349443..61ea61012 100644 --- a/lib/ProxySQL_PluginManager.cpp +++ b/lib/ProxySQL_PluginManager.cpp @@ -1,8 +1,7 @@ #include "ProxySQL_PluginManager.h" -#include - #include +#include namespace { @@ -14,9 +13,18 @@ std::string format_dl_error(const char *prefix) { return std::string(prefix) + dl_err; } +std::string plugin_name(const ProxySQL_PluginDescriptor *descriptor) { + if (descriptor == nullptr || descriptor->name == nullptr) { + return "unknown"; + } + return descriptor->name; +} + } // namespace -ProxySQL_PluginManager::ProxySQL_PluginManager() : services_{} {} +ProxySQL_PluginManager::ProxySQL_PluginManager() { + std::memset(&services_, 0, sizeof(services_)); +} ProxySQL_PluginManager::~ProxySQL_PluginManager() { stop_all(); @@ -78,7 +86,8 @@ bool ProxySQL_PluginManager::init_all(std::string &err) { plugin.initialized = true; continue; } - if (!plugin.descriptor->init(&services_, err)) { + if (!plugin.descriptor->init(&services_)) { + err = "plugin init failed: " + plugin_name(plugin.descriptor); return false; } plugin.initialized = true; @@ -94,11 +103,16 @@ bool ProxySQL_PluginManager::start_all(std::string &err) { if (plugin.started || plugin.stopped) { continue; } + if (!plugin.initialized) { + err = "plugin not initialized: " + plugin_name(plugin.descriptor); + return false; + } if (plugin.descriptor == nullptr || plugin.descriptor->start == nullptr) { plugin.started = true; continue; } - if (!plugin.descriptor->start(err)) { + if (!plugin.descriptor->start()) { + err = "plugin start failed: " + plugin_name(plugin.descriptor); return false; } plugin.started = true; diff --git a/test/tap/test_helpers/fake_plugin.cpp b/test/tap/test_helpers/fake_plugin.cpp index 78b4a7d76..fc7b725c4 100644 --- a/test/tap/test_helpers/fake_plugin.cpp +++ b/test/tap/test_helpers/fake_plugin.cpp @@ -1,14 +1,12 @@ #include "ProxySQL_Plugin.h" -#include - namespace { -bool fake_init(ProxySQL_PluginServices *, std::string &) { +bool fake_init(ProxySQL_PluginServices *) { return true; } -bool fake_start(std::string &) { +bool fake_start() { return true; } @@ -16,17 +14,17 @@ bool fake_stop() { return true; } -bool fake_status(std::string &) { - return true; +const char *fake_status_json() { + return "{\"name\":\"fake_plugin\",\"state\":\"running\"}"; } const ProxySQL_PluginDescriptor fake_descriptor = { - 1, "fake_plugin", + 1, &fake_init, &fake_start, &fake_stop, - &fake_status, + &fake_status_json, }; } // namespace diff --git a/test/tap/tests/unit/Makefile b/test/tap/tests/unit/Makefile index f5c881ebd..f2983b7b2 100644 --- a/test/tap/tests/unit/Makefile +++ b/test/tap/tests/unit/Makefile @@ -242,12 +242,17 @@ $(ODIR)/test_globals.o: $(TEST_HELPERS_DIR)/test_globals.cpp | $(ODIR) $(ODIR)/test_init.o: $(TEST_HELPERS_DIR)/test_init.cpp | $(ODIR) $(CXX) -c -o $@ $< $(OPT) $(IDIRS) -Wall -$(FAKE_PLUGIN_SO): $(TEST_HELPERS_DIR)/fake_plugin.cpp | $(TEST_HELPERS_DIR) +$(FAKE_PLUGIN_SO): $(TEST_HELPERS_DIR)/fake_plugin.cpp $(PROXYSQL_PATH)/include/ProxySQL_Plugin.h | $(TEST_HELPERS_DIR) $(CXX) -shared -fPIC -o $@ $< $(STDCPP) $(IDIRS) -ldl # Keep on-demand unit-test library rebuilds aligned with the default top-level # feature set so incremental rebuilds do not mix incompatible objects. -$(LIBPROXYSQLAR): +# Always delegate libproxysql.a freshness checks to the lib submake so unit +# tests do not link against a stale archive after source/header changes. +.PHONY: FORCE +FORCE: + +$(LIBPROXYSQLAR): FORCE $(MAKE) -C $(PROXYSQL_PATH)/lib libproxysql.a \ PROXYSQLCLICKHOUSE=1 PROXYSQLGENAI=$(PROXYSQLGENAI) \ PROXYSQLFFTO=$(PROXYSQLFFTO) PROXYSQLTSDB=$(PROXYSQLTSDB) \ @@ -293,6 +298,7 @@ UNIT_TESTS := smoke_test-t query_cache_unit-t query_processor_unit-t \ log_utils_unit-t \ genai_thread_unit-t \ listen_validator_unit-t \ + plugin_config_unit-t \ c_tokenizer_unit-t \ pgsql_tokenizer_unit-t \ ezoption_parser_unit-t \ @@ -327,6 +333,7 @@ ezoption_parser_unit-t: ezoption_parser_unit-t.cpp $(ODIR)/tap.o $(ODIR)/tap_noi plugin_manager_unit-t: plugin_manager_unit-t.cpp $(FAKE_PLUGIN_SO) $(ODIR)/tap.o $(ODIR)/test_globals.o $(ODIR)/test_init.o $(LIBPROXYSQLAR) $(CXX) $< $(ODIR)/tap.o $(ODIR)/test_globals.o $(ODIR)/test_init.o \ + -DPROXYSQL_FAKE_PLUGIN_PATH=\"$(FAKE_PLUGIN_SO)\" \ $(IDIRS) $(LDIRS) $(OPT) $(LIBPROXYSQLAR_FULL) $(STATIC_LIBS) \ $(MYLIBS) -ldl $(ALLOW_MULTI_DEF) -o $@ @@ -346,4 +353,4 @@ plugin_manager_unit-t: plugin_manager_unit-t.cpp $(FAKE_PLUGIN_SO) $(ODIR)/tap.o .PHONY: clean .SILENT: clean clean: - rm -rf $(ODIR) $(UNIT_TESTS) + rm -rf $(ODIR) $(UNIT_TESTS) $(FAKE_PLUGIN_SO) diff --git a/test/tap/tests/unit/plugin_manager_unit-t.cpp b/test/tap/tests/unit/plugin_manager_unit-t.cpp index e2a9a4577..4c229edf0 100644 --- a/test/tap/tests/unit/plugin_manager_unit-t.cpp +++ b/test/tap/tests/unit/plugin_manager_unit-t.cpp @@ -3,20 +3,31 @@ #include +#ifndef PROXYSQL_FAKE_PLUGIN_PATH +#error "PROXYSQL_FAKE_PLUGIN_PATH must be defined" +#endif + static void test_loader_round_trip() { ProxySQL_PluginManager mgr; std::string err; - ok(mgr.load("../../test_helpers/libproxysql_fake_plugin.so", err), + const bool loaded = mgr.load(PROXYSQL_FAKE_PLUGIN_PATH, err); + ok(loaded, "load fake plugin succeeds"); + if (!loaded) { + diag("load error: %s", err.c_str()); + BAIL_OUT("fake plugin must load before lifecycle assertions"); + } ok(mgr.size() == 1, "exactly one plugin is loaded"); + ok(!mgr.start_all(err), "start_all rejects uninitialized plugins"); + ok(!err.empty(), "start_all without init reports an error"); ok(mgr.init_all(err), "init_all succeeds"); ok(mgr.start_all(err), "start_all succeeds"); ok(mgr.stop_all(), "stop_all succeeds"); } int main() { - plan(5); + plan(7); test_loader_round_trip();