diff --git a/bindings/python/tests/model/test-py-brep.py b/bindings/python/tests/model/test-py-brep.py index 51460cc0c..63cdf0dd7 100644 --- a/bindings/python/tests/model/test-py-brep.py +++ b/bindings/python/tests/model/test-py-brep.py @@ -831,6 +831,20 @@ def test_registry(brep): raise ValueError("[Test] Wrong collection registry entry") +def test_unknown_component_access(): + brep = model.BRep() + unknown = basic.uuid() + try: + brep.surface(unknown) + except Exception as exception: + if f"Surface with id {unknown}" not in str(exception): + raise ValueError( + "[Test] Accessing an unknown Surface should name its type and id" + ) + else: + raise ValueError("[Test] Accessing an unknown Surface should raise") + + def test_component_id_hash(brep, surface_uuids): surface = brep.surface(surface_uuids[0]) same = model.ComponentID( @@ -915,6 +929,7 @@ def test_component_id_hash(brep, surface_uuids): test_physical_properties(brep, physical_attribute_id) test_clone(brep) test_component_id_hash(brep, surface_uuids) + test_unknown_component_access() brep_copy = model.BRep() model.BRepBuilder(brep_copy).copy(brep) test_physical_properties(brep_copy, physical_attribute_id) diff --git a/include/geode/model/mixin/core/detail/components_storage.hpp b/include/geode/model/mixin/core/detail/components_storage.hpp index 338468c14..78ac4af53 100644 --- a/include/geode/model/mixin/core/detail/components_storage.hpp +++ b/include/geode/model/mixin/core/detail/components_storage.hpp @@ -162,7 +162,9 @@ namespace geode::detail void delete_component( const uuid& component_id ) { - components_.erase( components_.find( component_id ) ); + const auto it = components_.find( component_id ); + check_component_exists( it, component_id ); + components_.erase( it ); } /*! @@ -287,14 +289,22 @@ namespace geode::detail components_ = std::move( repaired ); } - [[nodiscard]] Component& find_component( + void check_component_exists( + typename ComponentsStore::const_iterator it, const uuid& component_id ) const { - const auto it = components_.find( component_id ); OpenGeodeModelException::check_exception( it != components_.end(), component_id, OpenGeodeException::TYPE::data, - "[ComponentsStorage::component] Component with id ", + "[ComponentsStorage] ", + Component::component_type_static().get(), " with id ", component_id.string(), " does not exist" ); + } + + [[nodiscard]] Component& find_component( + const uuid& component_id ) const + { + const auto it = components_.find( component_id ); + check_component_exists( it, component_id ); auto& component = *it->second; OpenGeodeModelException::check_exception( component.id() == component_id, component_id, diff --git a/tests/model/test-brep.cpp b/tests/model/test-brep.cpp index 12cda8dae..392504ab1 100644 --- a/tests/model/test-brep.cpp +++ b/tests/model/test-brep.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -1873,6 +1874,29 @@ void test_unknown_component_access( Accessor&& accessor ) false, "Accessing an unknown component should throw" ); } +void test_remove_unknown_component() +{ + geode::BRep brep; + geode::BRepBuilder builder{ brep }; + builder.add_surface(); + const geode::uuid unknown_id; + bool thrown{ false }; + try + { + builder.remove_surfaces( { unknown_id } ); + } + catch( const geode::OpenGeodeException& exception ) + { + thrown = absl::StrContains( exception.what(), + absl::StrCat( "Surface with id ", unknown_id.string() ) ); + } + geode::OpenGeodeModelException::test( thrown, + "Removing an unknown Surface should throw an exception naming the " + "component type and id" ); + geode::OpenGeodeModelException::test( brep.nb_surfaces() == 1, + "Removing an unknown Surface should not modify the model" ); +} + void test_unknown_components( const geode::BRep& model ) { test_unknown_component_access( [&model]( const geode::uuid& id ) { @@ -2177,6 +2201,7 @@ void test() model, surface_uuids, surface_collection_uuids ); test_block_collection_ranges( model, block_uuid, block_collection_uuid ); test_unknown_components( model ); + test_remove_unknown_component(); test_is_closed(); test_bounding_box(); const geode::uuid physical_attribute_id;