From bdf9c5499fea45f0073fbc9f246e5c811626c7dd Mon Sep 17 00:00:00 2001 From: bergmann Date: Thu, 28 Feb 2019 17:11:53 +0100 Subject: [PATCH] * fixed table cleanup after a table that contains a sub-table was deleted --- .../driver/mariadb/schema/table.h | 4 +- .../driver/mariadb/schema/table.cpp | 28 ++++-- test/cpphibernate_destroy.cpp | 10 +-- test/cpphibernate_init.cpp | 18 ++++ test/cpphibernate_update.cpp | 89 +++++++++++++++++++ test/test_schema.h | 13 +++ 6 files changed, 145 insertions(+), 17 deletions(-) diff --git a/include/cpphibernate/driver/mariadb/schema/table.h b/include/cpphibernate/driver/mariadb/schema/table.h index 28605e4..198077c 100644 --- a/include/cpphibernate/driver/mariadb/schema/table.h +++ b/include/cpphibernate/driver/mariadb/schema/table.h @@ -130,6 +130,8 @@ beg_namespace_cpphibernate_driver_mariadb virtual std::string create_update_base(const create_update_context& context) const; protected: + using table_set = std::set; + void init_stage1_exec (const init_context& context) const; void init_stage2_exec (const init_context& context) const; @@ -140,7 +142,7 @@ beg_namespace_cpphibernate_driver_mariadb virtual void destroy_intern (const destroy_context& context) const; void destroy_exec (const destroy_context& context) const; - void destroy_cleanup (const base_context& context, bool check_derived, bool check_base) const; + void destroy_cleanup (const base_context& context, table_set& processed, bool check_derived, bool check_base) const; }; /* table_simple_t */ diff --git a/src/cpphibernate/driver/mariadb/schema/table.cpp b/src/cpphibernate/driver/mariadb/schema/table.cpp index 195807c..c19cfe3 100644 --- a/src/cpphibernate/driver/mariadb/schema/table.cpp +++ b/src/cpphibernate/driver/mariadb/schema/table.cpp @@ -1253,6 +1253,9 @@ std::string table_t::execute_create_update( delete_statement.set(1, *key); cpphibernate_debug_log("execute DELETE old foreign one query: " << delete_statement.query(connection)); connection.execute(delete_statement); + + table_set processed; + field.referenced_table->destroy_cleanup(context, processed, true, true); } } @@ -1397,7 +1400,8 @@ std::string table_t::execute_create_update( /* delete non referenced elements */ if (context.is_update) { - ref_table.destroy_cleanup(context, true, true); + table_set processed; + ref_table.destroy_cleanup(context, processed, true, true); } } @@ -1778,30 +1782,38 @@ void table_t::destroy_exec(const destroy_context& context) const cpphibernate_debug_log("execute DELETE query: " << statement.query(connection)); connection.execute(statement); - for (auto& ptr : foreign_table_many_fields) + table_set processed; + for (auto& ptr : foreign_table_fields) { assert(ptr); assert(ptr->referenced_table); auto& ref_table = *ptr->referenced_table; - ref_table.destroy_cleanup(context, true, true); + if (ref_table.is_used_in_container) + ref_table.destroy_cleanup(context, processed, true, true); } } -void table_t::destroy_cleanup(const base_context& context, bool check_derived, bool check_base) const +void table_t::destroy_cleanup(const base_context& context, table_set& processed, bool check_derived, bool check_base) const { + if (processed.count(this)) + return; + + processed.emplace(this); + execute_foreign_many_delete(context); - for (auto ptr : foreign_table_many_fields) + for (auto ptr : foreign_table_fields) { assert(ptr); assert(ptr->referenced_table); auto& ref_table = *ptr->referenced_table; - ref_table.destroy_cleanup(context, true, true); + if (ref_table.is_used_in_container) + ref_table.destroy_cleanup(context, processed, true, true); } if (check_base && base_table) { - base_table->destroy_cleanup(context, false, true); + base_table->destroy_cleanup(context, processed, false, true); } if (check_derived) @@ -1809,7 +1821,7 @@ void table_t::destroy_cleanup(const base_context& context, bool check_derived, b for (auto ptr : derived_tables) { assert(ptr); - ptr->destroy_cleanup(context, true, false); + ptr->destroy_cleanup(context, processed, true, false); } } } \ No newline at end of file diff --git a/test/cpphibernate_destroy.cpp b/test/cpphibernate_destroy.cpp index 6834cb8..013c9c1 100644 --- a/test/cpphibernate_destroy.cpp +++ b/test/cpphibernate_destroy.cpp @@ -221,13 +221,6 @@ TEST(CppHibernateTests, destroy_derived3) "WHERE " "(`tbl_derived3_id_test3_list` IS NULL) AND " "(`tbl_derived3_id_test3_vector` IS NULL)"); - expect_query(mock, "DELETE " - "`tbl_test3` " - "FROM " - "`tbl_test3` " - "WHERE " - "(`tbl_derived3_id_test3_list` IS NULL) AND " - "(`tbl_derived3_id_test3_vector` IS NULL)"); expect_query(mock, "COMMIT"); EXPECT_CALL( @@ -289,4 +282,5 @@ TEST(CppHibernateTests, destroy_double_usage) ::cppmariadb::connection connection(reinterpret_cast(0x1111)); auto context = make_context(test_schema, connection); context.destroy(d); -} \ No newline at end of file +} + diff --git a/test/cpphibernate_init.cpp b/test/cpphibernate_init.cpp index f86e94a..f81a4b5 100644 --- a/test/cpphibernate_init.cpp +++ b/test/cpphibernate_init.cpp @@ -197,6 +197,17 @@ TEST(CppHibernateTests, init) "ENGINE = InnoDB\n" "DEFAULT CHARACTER SET = utf8"); + expect_query(mock, "CREATE TABLE IF NOT EXISTS `tbl_double_usage_wrapper`\n" + "(\n" + " `tbl_double_usage_wrapper_id` INT NOT NULL,\n" + " `tbl_double_usage_id_double_usage` INT NULL DEFAULT NULL,\n" + " PRIMARY KEY ( `tbl_double_usage_wrapper_id` ),\n" + " UNIQUE INDEX `index_tbl_double_usage_wrapper_id` ( `tbl_double_usage_wrapper_id` ASC ),\n" + " INDEX `index_tbl_double_usage_id_double_usage` ( `tbl_double_usage_id_double_usage` ASC )\n" + ")\n" + "ENGINE = InnoDB\n" + "DEFAULT CHARACTER SET = utf8"); + expect_query(mock, "ALTER TABLE `tbl_test3`\n" " ADD CONSTRAINT `fk_tbl_test3_to_tbl_derived3_id_test3_list`\n" " FOREIGN KEY IF NOT EXISTS (`tbl_derived3_id_test3_list`)\n" @@ -269,6 +280,13 @@ TEST(CppHibernateTests, init) " ON DELETE SET NULL\n" " ON UPDATE NO ACTION"); + expect_query(mock, "ALTER TABLE `tbl_double_usage_wrapper`\n" + " ADD CONSTRAINT `fk_tbl_double_usage_wrapper_to_tbl_double_usage_id_double_usage`\n" + " FOREIGN KEY IF NOT EXISTS (`tbl_double_usage_id_double_usage`)\n" + " REFERENCES `test`.`tbl_double_usage` (`tbl_double_usage_id`)\n" + " ON DELETE SET NULL\n" + " ON UPDATE NO ACTION"); + expect_query(mock, "COMMIT"); EXPECT_CALL( diff --git a/test/cpphibernate_update.cpp b/test/cpphibernate_update.cpp index e4c859c..3307c12 100644 --- a/test/cpphibernate_update.cpp +++ b/test/cpphibernate_update.cpp @@ -795,6 +795,95 @@ TEST(CppHibernateTests, update_double_usage) d.multiple_items.back().id = 1003; d.multiple_items.back().data = 789; + ::cppmariadb::connection connection(reinterpret_cast(0x1111)); + auto context = make_context(test_schema, connection); + context.update(d); +} + +TEST(CppHibernateTests, update_double_usage_wrapper) +{ + StrictMock mock; + + expect_query(mock, "START TRANSACTION"); + expect_query(mock, "INSERT INTO `tbl_double_usage`", + result_id(101)); + expect_query(mock, "INSERT INTO " + "`tbl_double_usage_item` " + "SET " + "`tbl_double_usage_id_single_item`='X101X', " + "`tbl_double_usage_id_multiple_items`=null, " + "`tbl_double_usage_index_multiple_items`='X0X', " + "`data`='X123X'", + result_id(1001)); + expect_query(mock, "INSERT INTO " + "`tbl_double_usage_item` " + "SET " + "`tbl_double_usage_id_single_item`=null, " + "`tbl_double_usage_id_multiple_items`='X101X', " + "`tbl_double_usage_index_multiple_items`='X0X', " + "`data`='X456X'", + result_id(1002)); + expect_query(mock, "INSERT INTO " + "`tbl_double_usage_item` " + "SET " + "`tbl_double_usage_id_single_item`=null, " + "`tbl_double_usage_id_multiple_items`='X101X', " + "`tbl_double_usage_index_multiple_items`='X1X', " + "`data`='X789X'", + result_id(1003)); + expect_query(mock, "DELETE " + "`tbl_double_usage`, `T0` " + "FROM " + "`tbl_double_usage` " + "LEFT JOIN " + "`tbl_double_usage_item` AS `T0` ON `tbl_double_usage`.`tbl_double_usage_id`=`T0`.`tbl_double_usage_id_single_item` " + "WHERE " + "`tbl_double_usage_id` IN (" + "SELECT " + "`tbl_double_usage_id_double_usage` " + "FROM " + "`tbl_double_usage_wrapper` " + "WHERE " + "`tbl_double_usage_wrapper_id`='X1X' " + "AND `tbl_double_usage_id_double_usage`!='X101X'" + ")"); + expect_query(mock, "DELETE " + "`tbl_double_usage_item` " + "FROM " + "`tbl_double_usage_item` " + "WHERE " + "(`tbl_double_usage_id_single_item` IS NULL) " + "AND (`tbl_double_usage_id_multiple_items` IS NULL)"); + expect_query(mock, "UPDATE " + "`tbl_double_usage_wrapper` " + "SET " + "`tbl_double_usage_id_double_usage`='X101X' " + "WHERE " + "`tbl_double_usage_wrapper_id`='X1X'", + result_affected_rows(1)); + expect_query(mock, "COMMIT"); + + EXPECT_CALL( + mock, + mysql_real_escape_string(reinterpret_cast(0x1111), _, _, _)) + .Times(AnyNumber()) + .WillRepeatedly(WithArgs<1, 2, 3>(EscapeString())); + + EXPECT_CALL( + mock, + mysql_close( + reinterpret_cast(0x1111))); + + double_usage_wrapper d; + d.id = 1; + d.double_usage.reset(new double_usage()); + d.double_usage->single_item.reset(new double_usage_item()); + d.double_usage->single_item->data = 123; + d.double_usage->multiple_items.emplace_back(); + d.double_usage->multiple_items.back().data = 456; + d.double_usage->multiple_items.emplace_back(); + d.double_usage->multiple_items.back().data = 789; + ::cppmariadb::connection connection(reinterpret_cast(0x1111)); auto context = make_context(test_schema, connection); context.update(d); diff --git a/test/test_schema.h b/test/test_schema.h index bc77cb7..0437940 100644 --- a/test/test_schema.h +++ b/test/test_schema.h @@ -125,6 +125,12 @@ struct double_usage std::vector multiple_items; }; +struct double_usage_wrapper +{ + int id { 0 }; + std::unique_ptr double_usage; +}; + constexpr decltype(auto) test_schema = cpphibernate_make_schema( test, cpphibernate_make_table_name( @@ -219,5 +225,12 @@ constexpr decltype(auto) test_schema = cpphibernate_make_schema( cpphibernate_make_id (&double_usage::id), cpphibernate_make_field (double_usage, single_item), cpphibernate_make_field (double_usage, multiple_items) + ), + cpphibernate_make_table_name( + tbl_double_usage_wrapper, + double_usage_wrapper, + 18, + cpphibernate_make_id (&double_usage_wrapper::id), + cpphibernate_make_field (double_usage_wrapper, double_usage) ) ); \ No newline at end of file