From 87b9417205cc67bb3169fa43192bfc2eed0c4bfb Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Wed, 23 Apr 2025 08:56:25 +0200 Subject: [PATCH] Refactor: Split out some code from output_flex_t class The class output_flex_t is huge, output-flex.cpp with over 1000 lines is the largest source code file we have. This change moves some code out of there. This change introduces a new templated wrapper base class (lua_wrapper_base) for wrapping C++ objects in Lua. It is then used for the "Table" and "ExpireOutput" Lua objects. Functions that are called from Lua on those objects are called through the wrapper on the underlying C++ objects using little wrapper functions in the wrapper classes. A new macro TRAMPOLINE_WRAPPED_OBJECT() is used to call these functions where we used the TRAMPOLINE() macro before. One problem is though that more complex functions need more of the machinery in the output_flex_t class to work, specifically this is the "insert" function for tables which can not be implemented without full access to the output_flex_t class. So this function keeps using the old mechanism. This also changes the get_from_idx_param() helper function and the functions using it (output_flex_t::[get_table|expire_output]_from_param()) to return a reference instead of a const reference so that the wrapper will be initialized with a mutable pointer to the underlying C++ class. Currently all functions that can be called through the wrapper are const functions, but this will not necessarily always be the case in the future. --- src/flex-lua-expire-output.cpp | 83 ++++++++++++- src/flex-lua-expire-output.hpp | 24 +++- src/flex-lua-table.cpp | 86 +++++++++++++ src/flex-lua-table.hpp | 20 +++ src/flex-lua-wrapper.hpp | 60 +++++++++ src/output-flex.cpp | 218 +++------------------------------ src/output-flex.hpp | 27 ++-- 7 files changed, 295 insertions(+), 223 deletions(-) create mode 100644 src/flex-lua-wrapper.hpp diff --git a/src/flex-lua-expire-output.cpp b/src/flex-lua-expire-output.cpp index e86c73d78..1ec5f23d3 100644 --- a/src/flex-lua-expire-output.cpp +++ b/src/flex-lua-expire-output.cpp @@ -12,8 +12,6 @@ #include "expire-output.hpp" #include "format.hpp" #include "lua-utils.hpp" -#include "pgsql.hpp" -#include "util.hpp" #include @@ -67,6 +65,13 @@ create_expire_output(lua_State *lua_state, std::string const &default_schema, return new_expire_output; } +TRAMPOLINE_WRAPPED_OBJECT(expire_output, __tostring) +TRAMPOLINE_WRAPPED_OBJECT(expire_output, filename) +TRAMPOLINE_WRAPPED_OBJECT(expire_output, maxzoom) +TRAMPOLINE_WRAPPED_OBJECT(expire_output, minzoom) +TRAMPOLINE_WRAPPED_OBJECT(expire_output, schema) +TRAMPOLINE_WRAPPED_OBJECT(expire_output, table) + } // anonymous namespace int setup_flex_expire_output(lua_State *lua_state, @@ -88,3 +93,77 @@ int setup_flex_expire_output(lua_State *lua_state, return 1; } + +/** + * Define the osm2pgsql.ExpireOutput class/metatable. + */ +void lua_wrapper_expire_output::init(lua_State *lua_state) +{ + lua_getglobal(lua_state, "osm2pgsql"); + if (luaL_newmetatable(lua_state, osm2pgsql_expire_output_name) != 1) { + throw std::runtime_error{"Internal error: Lua newmetatable failed."}; + } + lua_pushvalue(lua_state, -1); // Copy of new metatable + + // Add metatable as osm2pgsql.ExpireOutput so we can access it from Lua + lua_setfield(lua_state, -3, "ExpireOutput"); + + // Now add functions to metatable + lua_pushvalue(lua_state, -1); + lua_setfield(lua_state, -2, "__index"); + luaX_add_table_func(lua_state, "__tostring", + lua_trampoline_expire_output___tostring); + luaX_add_table_func(lua_state, "filename", + lua_trampoline_expire_output_filename); + luaX_add_table_func(lua_state, "maxzoom", + lua_trampoline_expire_output_maxzoom); + luaX_add_table_func(lua_state, "minzoom", + lua_trampoline_expire_output_minzoom); + luaX_add_table_func(lua_state, "schema", + lua_trampoline_expire_output_schema); + luaX_add_table_func(lua_state, "table", lua_trampoline_expire_output_table); + + lua_pop(lua_state, 2); +} + +int lua_wrapper_expire_output::__tostring() const +{ + std::string const str = + fmt::format("osm2pgsql.ExpireOutput[minzoom={},maxzoom={},filename={}," + "schema={},table={}]", + self().minzoom(), self().maxzoom(), self().filename(), + self().schema(), self().table()); + luaX_pushstring(lua_state(), str); + + return 1; +} + +int lua_wrapper_expire_output::filename() const +{ + luaX_pushstring(lua_state(), self().filename()); + return 1; +} + +int lua_wrapper_expire_output::maxzoom() const +{ + lua_pushinteger(lua_state(), self().maxzoom()); + return 1; +} + +int lua_wrapper_expire_output::minzoom() const +{ + lua_pushinteger(lua_state(), self().minzoom()); + return 1; +} + +int lua_wrapper_expire_output::schema() const +{ + luaX_pushstring(lua_state(), self().schema()); + return 1; +} + +int lua_wrapper_expire_output::table() const +{ + luaX_pushstring(lua_state(), self().table()); + return 1; +} diff --git a/src/flex-lua-expire-output.hpp b/src/flex-lua-expire-output.hpp index 5c13f1a69..aa3bfac76 100644 --- a/src/flex-lua-expire-output.hpp +++ b/src/flex-lua-expire-output.hpp @@ -10,10 +10,12 @@ * For a full list of authors see the git log. */ -#include "expire-output.hpp" +#include "flex-lua-wrapper.hpp" +#include #include +class expire_output_t; struct lua_State; static char const *const osm2pgsql_expire_output_name = @@ -23,4 +25,24 @@ int setup_flex_expire_output(lua_State *lua_state, std::string const &default_schema, std::vector *expire_outputs); +class lua_wrapper_expire_output : public lua_wrapper_base +{ +public: + static void init(lua_State *lua_state); + + lua_wrapper_expire_output(lua_State *lua_state, + expire_output_t *expire_output) + : lua_wrapper_base(lua_state, expire_output) + { + } + + int __tostring() const; + int filename() const; + int maxzoom() const; + int minzoom() const; + int schema() const; + int table() const; + +}; // class lua_wrapper_expire_output + #endif // OSM2PGSQL_FLEX_LUA_EXPIRE_OUTPUT_HPP diff --git a/src/flex-lua-table.cpp b/src/flex-lua-table.cpp index 8dd8f647b..3fca95e6f 100644 --- a/src/flex-lua-table.cpp +++ b/src/flex-lua-table.cpp @@ -14,7 +14,9 @@ #include "flex-lua-index.hpp" #include "flex-table.hpp" #include "lua-utils.hpp" +#include "output-flex.hpp" #include "pgsql-capabilities.hpp" +#include "util.hpp" #include @@ -416,6 +418,12 @@ void setup_flex_table_indexes(lua_State *lua_state, flex_table_t *table, lua_pop(lua_state, 1); // "indexes" } +TRAMPOLINE_WRAPPED_OBJECT(table, __tostring) +TRAMPOLINE_WRAPPED_OBJECT(table, cluster) +TRAMPOLINE_WRAPPED_OBJECT(table, columns) +TRAMPOLINE_WRAPPED_OBJECT(table, name) +TRAMPOLINE_WRAPPED_OBJECT(table, schema) + } // anonymous namespace int setup_flex_table(lua_State *lua_state, std::vector *tables, @@ -442,3 +450,81 @@ int setup_flex_table(lua_State *lua_state, std::vector *tables, return 1; } + +/** + * Define the osm2pgsql.Table class/metatable. + */ +void lua_wrapper_table::init(lua_State *lua_state) +{ + lua_getglobal(lua_state, "osm2pgsql"); + if (luaL_newmetatable(lua_state, osm2pgsql_table_name) != 1) { + throw std::runtime_error{"Internal error: Lua newmetatable failed."}; + } + lua_pushvalue(lua_state, -1); // Copy of new metatable + + // Add metatable as osm2pgsql.Table so we can access it from Lua + lua_setfield(lua_state, -3, "Table"); + + // Now add functions to metatable + lua_pushvalue(lua_state, -1); + lua_setfield(lua_state, -2, "__index"); + luaX_add_table_func(lua_state, "__tostring", + lua_trampoline_table___tostring); + luaX_add_table_func(lua_state, "insert", lua_trampoline_table_insert); + luaX_add_table_func(lua_state, "name", lua_trampoline_table_name); + luaX_add_table_func(lua_state, "schema", lua_trampoline_table_schema); + luaX_add_table_func(lua_state, "cluster", lua_trampoline_table_cluster); + luaX_add_table_func(lua_state, "columns", lua_trampoline_table_columns); + + lua_pop(lua_state, 2); +} + +int lua_wrapper_table::__tostring() const +{ + std::string const str{fmt::format("osm2pgsql.Table[{}]", self().name())}; + luaX_pushstring(lua_state(), str); + + return 1; +} + +int lua_wrapper_table::cluster() const +{ + lua_pushboolean(lua_state(), self().cluster_by_geom()); + return 1; +} + +int lua_wrapper_table::columns() const +{ + lua_createtable(lua_state(), (int)self().num_columns(), 0); + + int n = 0; + for (auto const &column : self().columns()) { + lua_pushinteger(lua_state(), ++n); + lua_newtable(lua_state()); + + luaX_add_table_str(lua_state(), "name", column.name().c_str()); + luaX_add_table_str(lua_state(), "type", column.type_name().c_str()); + luaX_add_table_str(lua_state(), "sql_type", + column.sql_type_name().c_str()); + luaX_add_table_str(lua_state(), "sql_modifiers", + column.sql_modifiers().c_str()); + luaX_add_table_bool(lua_state(), "not_null", column.not_null()); + luaX_add_table_bool(lua_state(), "create_only", column.create_only()); + + lua_rawset(lua_state(), -3); + } + + return 1; +} + +int lua_wrapper_table::name() const +{ + luaX_pushstring(lua_state(), self().name()); + return 1; +} + +int lua_wrapper_table::schema() const +{ + luaX_pushstring(lua_state(), self().schema()); + return 1; +} diff --git a/src/flex-lua-table.hpp b/src/flex-lua-table.hpp index 6f82ca00e..6e83634ea 100644 --- a/src/flex-lua-table.hpp +++ b/src/flex-lua-table.hpp @@ -10,6 +10,8 @@ * For a full list of authors see the git log. */ +#include "flex-lua-wrapper.hpp" + #include #include @@ -24,4 +26,22 @@ int setup_flex_table(lua_State *lua_state, std::vector *tables, std::string const &default_schema, bool updatable, bool append_mode); +class lua_wrapper_table : public lua_wrapper_base +{ +public: + static void init(lua_State *lua_state); + + lua_wrapper_table(lua_State *lua_state, flex_table_t *table) + : lua_wrapper_base(lua_state, table) + { + } + + int __tostring() const; + int cluster() const; + int columns() const; + int name() const; + int schema() const; + +}; // class lua_wrapper_table + #endif // OSM2PGSQL_FLEX_LUA_TABLE_HPP diff --git a/src/flex-lua-wrapper.hpp b/src/flex-lua-wrapper.hpp new file mode 100644 index 000000000..7577e7c71 --- /dev/null +++ b/src/flex-lua-wrapper.hpp @@ -0,0 +1,60 @@ +#ifndef OSM2PGSQL_FLEX_LUA_WRAPPER_HPP +#define OSM2PGSQL_FLEX_LUA_WRAPPER_HPP + +/** + * SPDX-License-Identifier: GPL-2.0-or-later + * + * This file is part of osm2pgsql (https://osm2pgsql.org/). + * + * Copyright (C) 2006-2025 by the osm2pgsql developer community. + * For a full list of authors see the git log. + */ + +#include "output-flex.hpp" + +#include + +#define TRAMPOLINE_WRAPPED_OBJECT(obj_name, func_name) \ + int lua_trampoline_##obj_name##_##func_name(lua_State *lua_state) \ + { \ + try { \ + auto *flex = \ + static_cast(luaX_get_context(lua_state)); \ + auto &obj = flex->get_##obj_name##_from_param(); \ + return lua_wrapper_##obj_name{lua_state, &obj}.func_name(); \ + } catch (std::exception const &e) { \ + return luaL_error(lua_state, "Error in '" #func_name "': %s\n", \ + e.what()); \ + } catch (...) { \ + return luaL_error(lua_state, \ + "Unknown error in '" #func_name "'.\n"); \ + } \ + } + +struct lua_State; + +/** + * Helper class for wrapping C++ classes in Lua "classes". + */ +template +class lua_wrapper_base +{ +public: + lua_wrapper_base(lua_State *lua_state, WRAPPED *wrapped) + : m_lua_state(lua_state), m_self(wrapped) + { + } + +protected: + lua_State *lua_state() const noexcept { return m_lua_state; } + + WRAPPED const &self() const noexcept { return *m_self; } + WRAPPED &self() noexcept { return *m_self; } + +private: + lua_State *m_lua_state; + WRAPPED *m_self; + +}; // class lua_wrapper_base; + +#endif // OSM2PGSQL_FLEX_LUA_WRAPPER_HPP diff --git a/src/output-flex.cpp b/src/output-flex.cpp index e1f92215e..164d81b5e 100644 --- a/src/output-flex.cpp +++ b/src/output-flex.cpp @@ -7,15 +7,18 @@ * For a full list of authors see the git log. */ +#include "output-flex.hpp" + #include "db-copy.hpp" #include "debug-output.hpp" #include "expire-output.hpp" #include "expire-tiles.hpp" #include "flex-index.hpp" +#include "flex-lua-expire-output.hpp" #include "flex-lua-geom.hpp" #include "flex-lua-index.hpp" #include "flex-lua-table.hpp" -#include "flex-lua-expire-output.hpp" +#include "flex-lua-wrapper.hpp" #include "flex-write.hpp" #include "format.hpp" #include "geom-from-osm.hpp" @@ -27,9 +30,8 @@ #include "middle.hpp" #include "options.hpp" #include "osmtypes.hpp" -#include "output-flex.hpp" -#include "pgsql.hpp" #include "pgsql-capabilities.hpp" +#include "pgsql.hpp" #include "properties.hpp" #include "reprojection.hpp" #include "thread-pool.hpp" @@ -87,22 +89,10 @@ TRAMPOLINE(app_as_multilinestring, as_multilinestring) TRAMPOLINE(app_as_multipolygon, as_multipolygon) TRAMPOLINE(app_as_geometrycollection, as_geometrycollection) -TRAMPOLINE(table_name, name) -TRAMPOLINE(table_schema, schema) -TRAMPOLINE(table_cluster, cluster) -TRAMPOLINE(table_insert, insert) -TRAMPOLINE(table_columns, columns) -TRAMPOLINE(table_tostring, __tostring) - -TRAMPOLINE(expire_output_minzoom, minzoom) -TRAMPOLINE(expire_output_maxzoom, maxzoom) -TRAMPOLINE(expire_output_filename, filename) -TRAMPOLINE(expire_output_schema, schema) -TRAMPOLINE(expire_output_table, table) -TRAMPOLINE(expire_output_tostring, __tostring) - } // anonymous namespace +TRAMPOLINE(table_insert, insert) + prepared_lua_function_t::prepared_lua_function_t(lua_State *lua_state, calling_context context, char const *name, int nresults) @@ -233,15 +223,15 @@ std::size_t idx_from_param(lua_State *lua_state, char const *type_name) } template -typename CONTAINER::value_type const &get_from_idx_param(lua_State *lua_state, - CONTAINER *container, - char const *type_name) +typename CONTAINER::value_type &get_from_idx_param(lua_State *lua_state, + CONTAINER *container, + char const *type_name) { - if (lua_gettop(lua_state) != 1) { - throw fmt_error("Need exactly one parameter of type {}.", type_name); + if (lua_gettop(lua_state) < 1) { + throw fmt_error("Argument #1 has to be of type {}.", type_name); } - auto const &item = container->at(idx_from_param(lua_state, type_name)); + auto &item = container->at(idx_from_param(lua_state, type_name)); lua_remove(lua_state, 1); return item; } @@ -602,28 +592,18 @@ int output_flex_t::app_define_expire_output() m_expire_outputs.get()); } -flex_table_t const &output_flex_t::get_table_from_param() +flex_table_t &output_flex_t::get_table_from_param() { return get_from_idx_param(lua_state(), m_tables.get(), osm2pgsql_table_name); } -expire_output_t const &output_flex_t::get_expire_output_from_param() +expire_output_t &output_flex_t::get_expire_output_from_param() { return get_from_idx_param(lua_state(), m_expire_outputs.get(), osm2pgsql_expire_output_name); } -int output_flex_t::table_tostring() -{ - auto const &table = get_table_from_param(); - - std::string const str{fmt::format("osm2pgsql.Table[{}]", table.name())}; - luaX_pushstring(lua_state(), str); - - return 1; -} - bool output_flex_t::way_cache_t::init(middle_query_t const &middle, osmid_t id) { m_buffer.clear(); @@ -791,107 +771,6 @@ int output_flex_t::table_insert() return 1; } -int output_flex_t::table_columns() -{ - auto const &table = get_table_from_param(); - - lua_createtable(lua_state(), (int)table.num_columns(), 0); - - int n = 0; - for (auto const &column : table.columns()) { - lua_pushinteger(lua_state(), ++n); - lua_newtable(lua_state()); - - luaX_add_table_str(lua_state(), "name", column.name().c_str()); - luaX_add_table_str(lua_state(), "type", column.type_name().c_str()); - luaX_add_table_str(lua_state(), "sql_type", - column.sql_type_name().c_str()); - luaX_add_table_str(lua_state(), "sql_modifiers", - column.sql_modifiers().c_str()); - luaX_add_table_bool(lua_state(), "not_null", column.not_null()); - luaX_add_table_bool(lua_state(), "create_only", column.create_only()); - - lua_rawset(lua_state(), -3); - } - return 1; -} - -int output_flex_t::table_name() -{ - auto const &table = get_table_from_param(); - luaX_pushstring(lua_state(), table.name()); - return 1; -} - -int output_flex_t::table_schema() -{ - auto const &table = get_table_from_param(); - luaX_pushstring(lua_state(), table.schema()); - return 1; -} - -int output_flex_t::table_cluster() -{ - auto const &table = get_table_from_param(); - lua_pushboolean(lua_state(), table.cluster_by_geom()); - return 1; -} - -int output_flex_t::expire_output_tostring() -{ - auto const &expire_output = get_expire_output_from_param(); - - std::string const str = - fmt::format("osm2pgsql.ExpireOutput[minzoom={},maxzoom={},filename={}," - "schema={},table={}]", - expire_output.minzoom(), expire_output.maxzoom(), - expire_output.filename(), expire_output.schema(), - expire_output.table()); - luaX_pushstring(lua_state(), str); - - return 1; -} - -int output_flex_t::expire_output_minzoom() -{ - auto const &expire_output = get_expire_output_from_param(); - - lua_pushinteger(lua_state(), expire_output.minzoom()); - return 1; -} - -int output_flex_t::expire_output_maxzoom() -{ - auto const &expire_output = get_expire_output_from_param(); - - lua_pushinteger(lua_state(), expire_output.maxzoom()); - return 1; -} - -int output_flex_t::expire_output_filename() -{ - auto const &expire_output = get_expire_output_from_param(); - - luaX_pushstring(lua_state(), expire_output.filename()); - return 1; -} - -int output_flex_t::expire_output_schema() -{ - auto const &expire_output = get_expire_output_from_param(); - - luaX_pushstring(lua_state(), expire_output.schema()); - return 1; -} - -int output_flex_t::expire_output_table() -{ - auto const &expire_output = get_expire_output_from_param(); - - luaX_pushstring(lua_state(), expire_output.table()); - return 1; -} - void output_flex_t::call_lua_function(prepared_lua_function_t func) { lua_pushvalue(lua_state(), func.index()); @@ -1325,69 +1204,6 @@ output_flex_t::output_flex_t(std::shared_ptr const &mid, create_expire_tables(*m_expire_outputs, get_options()->connection_params); } -namespace { - -/** - * Define the osm2pgsql.Table class/metatable. - */ -void init_table_class(lua_State *lua_state) -{ - lua_getglobal(lua_state, "osm2pgsql"); - if (luaL_newmetatable(lua_state, osm2pgsql_table_name) != 1) { - throw std::runtime_error{"Internal error: Lua newmetatable failed."}; - } - lua_pushvalue(lua_state, -1); // Copy of new metatable - - // Add metatable as osm2pgsql.Table so we can access it from Lua - lua_setfield(lua_state, -3, "Table"); - - // Now add functions to metatable - lua_pushvalue(lua_state, -1); - lua_setfield(lua_state, -2, "__index"); - luaX_add_table_func(lua_state, "__tostring", lua_trampoline_table_tostring); - luaX_add_table_func(lua_state, "insert", lua_trampoline_table_insert); - luaX_add_table_func(lua_state, "name", lua_trampoline_table_name); - luaX_add_table_func(lua_state, "schema", lua_trampoline_table_schema); - luaX_add_table_func(lua_state, "cluster", lua_trampoline_table_cluster); - luaX_add_table_func(lua_state, "columns", lua_trampoline_table_columns); - - lua_pop(lua_state, 2); -} - -/** - * Define the osm2pgsql.ExpireOutput class/metatable. - */ -void init_expire_output_class(lua_State *lua_state) -{ - lua_getglobal(lua_state, "osm2pgsql"); - if (luaL_newmetatable(lua_state, osm2pgsql_expire_output_name) != 1) { - throw std::runtime_error{"Internal error: Lua newmetatable failed."}; - } - lua_pushvalue(lua_state, -1); // Copy of new metatable - - // Add metatable as osm2pgsql.ExpireOutput so we can access it from Lua - lua_setfield(lua_state, -3, "ExpireOutput"); - - // Now add functions to metatable - lua_pushvalue(lua_state, -1); - lua_setfield(lua_state, -2, "__index"); - luaX_add_table_func(lua_state, "__tostring", - lua_trampoline_expire_output_tostring); - luaX_add_table_func(lua_state, "minzoom", - lua_trampoline_expire_output_minzoom); - luaX_add_table_func(lua_state, "maxzoom", - lua_trampoline_expire_output_maxzoom); - luaX_add_table_func(lua_state, "filename", - lua_trampoline_expire_output_filename); - luaX_add_table_func(lua_state, "schema", - lua_trampoline_expire_output_schema); - luaX_add_table_func(lua_state, "table", lua_trampoline_expire_output_table); - - lua_pop(lua_state, 2); -} - -} // anonymous namespace - void output_flex_t::init_lua(std::string const &filename, properties_t const &properties) { @@ -1412,8 +1228,8 @@ void output_flex_t::init_lua(std::string const &filename, luaX_add_table_func(lua_state(), "define_expire_output", lua_trampoline_app_define_expire_output); - init_table_class(lua_state()); - init_expire_output_class(lua_state()); + lua_wrapper_expire_output::init(lua_state()); + lua_wrapper_table::init(lua_state()); // Clean up stack lua_settop(lua_state(), 0); diff --git a/src/output-flex.hpp b/src/output-flex.hpp index cc7bd1095..39cd1da70 100644 --- a/src/output-flex.hpp +++ b/src/output-flex.hpp @@ -164,20 +164,13 @@ class output_flex_t : public output_t int app_define_expire_output(); int app_get_bbox(); - int table_tostring(); int table_insert(); - int table_name(); - int table_schema(); - int table_cluster(); - int table_columns(); - - int expire_output_tostring(); - int expire_output_name(); - int expire_output_minzoom(); - int expire_output_maxzoom(); - int expire_output_filename(); - int expire_output_schema(); - int expire_output_table(); + + // Get the flex table that is as first parameter on the Lua stack. + flex_table_t &get_table_from_param(); + + // Get the expire output that is as first parameter on the Lua stack. + expire_output_t &get_expire_output_from_param(); private: void select_relation_members(); @@ -200,12 +193,6 @@ class output_flex_t : public output_t void init_lua(std::string const &filename, properties_t const &properties); - // Get the flex table that is as first parameter on the Lua stack. - flex_table_t const &get_table_from_param(); - - // Get the expire output that is as first parameter on the Lua stack. - expire_output_t const &get_expire_output_from_param(); - void check_context_and_state(char const *name, char const *context, bool condition); @@ -324,4 +311,6 @@ class output_flex_t : public output_t bool m_disable_insert = false; }; +int lua_trampoline_table_insert(lua_State *lua_state); + #endif // OSM2PGSQL_OUTPUT_FLEX_HPP