From 8b6c1619e79e273316cd46d4cf4eac9e3342f2ee Mon Sep 17 00:00:00 2001 From: Alex Shabalin Date: Tue, 7 May 2024 10:54:29 +0200 Subject: [PATCH] Support converting hash-based containers even when transformation changes the hash --- immer/extra/archive/champ/champ.hpp | 31 ++- immer/extra/archive/champ/traits.hpp | 28 +++ .../archive/test_special_archive_auto.cpp | 181 ++++++++++++++++++ 3 files changed, 238 insertions(+), 2 deletions(-) diff --git a/immer/extra/archive/champ/champ.hpp b/immer/extra/archive/champ/champ.hpp index 4f00e619..cc7831f3 100644 --- a/immer/extra/archive/champ/champ.hpp +++ b/immer/extra/archive/champ/champ.hpp @@ -45,9 +45,19 @@ class hash_validation_failed_exception : public archive_exception } }; +/** + * incompatible_hash_mode: + * When values are transformed in a way that changes how they are hashed, the + * structure of the champ can't be preserved. The only solution is to recreate + * the container from the values that it should contain. + * + * The mode can be enabled by returning incompatible_hash_wrapper from the + * function that handles the target_container_type_request. + */ template , - typename TransformF = boost::hana::id_t> + typename TransformF = boost::hana::id_t, + bool enable_incompatible_hash_mode = false> class container_loader { using champ_t = std::decay_t().impl())>; @@ -84,7 +94,23 @@ class container_loader throw invalid_node_id{root_id}; } - auto [root, values] = nodes_.load_inner(root_id); + auto [root, values] = nodes_.load_inner(root_id); + + if constexpr (enable_incompatible_hash_mode) { + if (auto* p = loaded_.find(root_id)) { + return *p; + } + + auto result = Container{}; + for (const auto& items : values) { + for (const auto& item : items) { + result = std::move(result).insert(item); + } + } + loaded_ = std::move(loaded_).set(root_id, result); + return result; + } + const auto items_count = [&values = values] { auto count = std::size_t{}; for (const auto& items : values) { @@ -129,6 +155,7 @@ class container_loader nodes_load, TransformF> nodes_; + immer::map loaded_; }; template diff --git a/immer/extra/archive/champ/traits.hpp b/immer/extra/archive/champ/traits.hpp index 10ff7fb2..9fb08537 100644 --- a/immer/extra/archive/champ/traits.hpp +++ b/immer/extra/archive/champ/traits.hpp @@ -82,6 +82,34 @@ auto transform_archive(const container_archive_load& ar, F&& func) .nodes = transform(ar.nodes, func), }; } + +/** + * The wrapper is used to enable the incompatible_hash_mode, which is required + * when the key of a hash-based container transformed in a way that changes its + * hash. + */ +template +struct incompatible_hash_wrapper +{}; } // namespace champ +template +struct container_traits> + : champ_traits +{ + using base_t = champ_traits; + + // Everything stays the same as for normal container, except that we tell + // the loader to do something special. + static constexpr bool enable_incompatible_hash_mode = true; + + template + using loader_t = + immer::archive::champ::container_loader; +}; + } // namespace immer::archive diff --git a/test/extra/archive/test_special_archive_auto.cpp b/test/extra/archive/test_special_archive_auto.cpp index a25606dc..a1c4e6b3 100644 --- a/test/extra/archive/test_special_archive_auto.cpp +++ b/test/extra/archive/test_special_archive_auto.cpp @@ -819,3 +819,184 @@ TEST_CASE("Test table with a funny value no auto") json_str); REQUIRE(loaded == value); } + +namespace { + +struct int_key +{ + BOOST_HANA_DEFINE_STRUCT(int_key, (int, id)); +}; +DEFINE_OPERATIONS(int_key); + +struct string_key +{ + BOOST_HANA_DEFINE_STRUCT(string_key, (std::string, id)); +}; +DEFINE_OPERATIONS(string_key); + +struct test_champs +{ + BOOST_HANA_DEFINE_STRUCT(test_champs, + (immer::map, map), + (immer::table, table), + (immer::set, set) + + ); +}; +DEFINE_OPERATIONS(test_champs); + +} // namespace + +TEST_CASE("Structure breaks when hash is changed") +{ + const auto value = test_champs{ + .map = {{123, "123"}, {456, "456"}}, + }; + + const auto names = immer::archive::get_archives_for_types( + hana::tuple_t, hana::make_map()); + + const auto [json_str, ar] = + immer::archive::to_json_with_auto_archive(value, names); + // REQUIRE(json_str == ""); + + constexpr auto convert_pair = [](const std::pair& old) { + return std::make_pair(fmt::format("_{}_", old.first), old.second); + }; + + const auto map = hana::make_map(hana::make_pair( + hana::type_c>, + hana::overload(convert_pair, + [](immer::archive::target_container_type_request) { + // We just return the desired new type, but the hash + // of int is not compatible with the hash of string. + return immer::map{}; + })) + + ); + + auto load_ar = immer::archive::transform_save_archive(ar, map); + + REQUIRE_THROWS_AS(immer::archive::convert_container(ar, load_ar, value.map), + immer::archive::champ::hash_validation_failed_exception); +} + +TEST_CASE("Converting between incompatible keys") +{ + const auto value = test_champs{ + .map = {{123, "123"}, {456, "456"}}, + .table = {{901}, {902}}, + }; + + const auto names = immer::archive::get_archives_for_types( + hana::tuple_t, hana::make_map()); + + const auto [json_str, ar] = + immer::archive::to_json_with_auto_archive(value, names); + // REQUIRE(json_str == ""); + + constexpr auto convert_pair = [](const std::pair& old) { + return std::make_pair(fmt::format("_{}_", old.first), old.second); + }; + + constexpr auto convert_int_key = [](const int_key& old) { + return string_key{fmt::format("x{}x", old.id)}; + }; + + /** + * The problem is that the new key of the map has a completely different + * hash from the old key, which makes the whole map structure unusable. We + * need to have some special mode that essentially rebuilds the map. We will + * lose all internal structural sharing but at least the same container_id + * must return the same container (root node sharing). + */ + const auto map = hana::make_map( + hana::make_pair( + hana::type_c>, + hana::overload( + convert_pair, + [](immer::archive::target_container_type_request) { + return immer::archive::champ::incompatible_hash_wrapper< + immer::map>{}; + })), + hana::make_pair( + hana::type_c>, + hana::overload( + convert_int_key, + [](immer::archive::target_container_type_request) { + return immer::archive::champ::incompatible_hash_wrapper< + immer::table>{}; + })), + hana::make_pair( + hana::type_c>, + hana::overload( + [convert_int_key](int old) { + return convert_int_key(int_key{old}).id; + }, + [](immer::archive::target_container_type_request) { + return immer::archive::champ::incompatible_hash_wrapper< + immer::set>{}; + })) + + ); + + auto load_ar = immer::archive::transform_save_archive(ar, map); + SECTION("maps") + { + constexpr auto convert_map = [convert_pair](const auto& map) { + auto result = immer::map{}; + for (const auto& item : map) { + result = std::move(result).insert(convert_pair(item)); + } + return result; + }; + + const auto converted = + immer::archive::convert_container(ar, load_ar, value.map); + REQUIRE(converted == convert_map(value.map)); + + // Converting the same thing should return the same data + const auto converted_2 = + immer::archive::convert_container(ar, load_ar, value.map); + REQUIRE(converted.identity() == converted_2.identity()); + } + SECTION("tables") + { + constexpr auto convert_table = [convert_int_key](const auto& table) { + auto result = immer::table{}; + for (const auto& item : table) { + result = std::move(result).insert(convert_int_key(item)); + } + return result; + }; + + const auto converted = + immer::archive::convert_container(ar, load_ar, value.table); + REQUIRE(converted == convert_table(value.table)); + + // Converting the same thing should return the same data + const auto converted_2 = + immer::archive::convert_container(ar, load_ar, value.table); + REQUIRE(converted.impl().root == converted_2.impl().root); + } + SECTION("sets") + { + constexpr auto convert_set = [convert_int_key](const auto& set) { + auto result = immer::set{}; + for (const auto& item : set) { + result = + std::move(result).insert(convert_int_key(int_key{item}).id); + } + return result; + }; + + const auto converted = + immer::archive::convert_container(ar, load_ar, value.set); + REQUIRE(converted == convert_set(value.set)); + + // Converting the same thing should return the same data + const auto converted_2 = + immer::archive::convert_container(ar, load_ar, value.set); + REQUIRE(converted.impl().root == converted_2.impl().root); + } +}