From d7ca070c627ea2706541ced44b9639b29a4e9174 Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Thu, 29 Oct 2026 11:04:44 +0100 Subject: [PATCH 1/8] cleanup support (draft) Signed-off-by: Chao Jia --- .../include/flatdata/ArchiveBuilder.h | 1 + .../include/flatdata/FileResourceStorage.h | 77 +++++++++++++++++++ .../include/flatdata/ResourceStorage.h | 15 ++++ flatdata-cpp/src/ArchiveBuilder.cpp | 7 ++ .../generator/templates/cpp/archive.jinja2 | 67 +++++++++++++--- 5 files changed, 156 insertions(+), 11 deletions(-) diff --git a/flatdata-cpp/include/flatdata/ArchiveBuilder.h b/flatdata-cpp/include/flatdata/ArchiveBuilder.h index c84ba3df..b7fea5bc 100644 --- a/flatdata-cpp/include/flatdata/ArchiveBuilder.h +++ b/flatdata-cpp/include/flatdata/ArchiveBuilder.h @@ -52,6 +52,7 @@ class ArchiveBuilder const flatdata::ResourceStorage& storage( ) const; bool is_created( ) const; void check_created( ) const; + void clean_up_signature( flatdata::ResourceStorage& storage ); private: std::shared_ptr< flatdata::ResourceStorage > m_storage; diff --git a/flatdata-cpp/include/flatdata/FileResourceStorage.h b/flatdata-cpp/include/flatdata/FileResourceStorage.h index 699541d5..b08021dc 100644 --- a/flatdata-cpp/include/flatdata/FileResourceStorage.h +++ b/flatdata-cpp/include/flatdata/FileResourceStorage.h @@ -42,6 +42,11 @@ class FileResourceStorage : public ResourceStorage bool exists( const char* key ) override; + void remove_resource( const char* resource_name, const char* expected_schema ) override; + + void remove_multivector_index_resource( const char* resource_name, + const char* expected_schema ) override; + protected: std::shared_ptr< std::ostream > create_output_stream( const char* key ) override; MemoryDescriptor read_resource( const char* key ) override; @@ -150,4 +155,76 @@ FileResourceStorage::exists( const char* key ) return !ec; } +inline void +logger( const std::string& msg ) +{ + static std::mutex m; + std::lock_guard< std::mutex > lock( m ); + printf( "%s\n", msg.c_str( ) ); +} + +inline void +FileResourceStorage::remove_resource( const char* resource_name, const char* expected_schema ) +{ + const std::string schema_name = std::string( resource_name ) + ".schema"; + const bool resource_exists = exists( resource_name ); + const bool schema_exists = exists( schema_name.c_str( ) ); + + if ( !resource_exists && !schema_exists ) + { + logger( "[INFO] resource " + std::string( resource_name ) + ": does not exist" ); + return; + } + + if ( resource_exists ^ schema_exists ) + { + // [CJ-TODO] + // remove what's left or simply return? + // can happen when crashes during writting + logger( "[ERR] resource " + std::string( resource_name ) + ": no schema or resource" ); + } + + auto schema = read_resource( ( schema_name.c_str( ) ) ); + std::string stored_schema( reinterpret_cast< const char* >( schema.data( ) ), + schema.size_in_bytes( ) ); + + if ( stored_schema != expected_schema ) + { + // [CJ-TODO] + // What if the schema change is intended? + logger( "[ERR] resource " + std::string( resource_name ) + ": unexpected schema" ); + return; + } + + boost::system::error_code ec_resource, ec_schema; + boost::filesystem::remove( get_path( resource_name ), ec_resource ); + boost::filesystem::remove( get_path( schema_name.c_str( ) ), ec_schema ); + if ( ec_resource.failed( ) ) + { + logger( "[ERR] resource " + std::string( resource_name ) + ": failed to remove resource" ); + } + else + { + logger( "[INFO] resource " + std::string( resource_name ) + ": removed resource" ); + } + if ( ec_schema.failed( ) ) + { + logger( "[ERR] resource " + std::string( resource_name ) + ": failed to remove schema" ); + } + else + { + logger( "[INFO] resource " + std::string( resource_name ) + ": removed schema" ); + } +} + +inline void +FileResourceStorage::remove_multivector_index_resource( const char* resource_name, + const char* expected_schema ) +{ + std::string index_name = std::string( resource_name ) + internal::INDEX_SUFFIX; + std::string index_schema = internal::multivector_index_schema( expected_schema ); + + remove_resource( index_name.c_str( ), index_schema.c_str( ) ); +} + } // namespace flatdata diff --git a/flatdata-cpp/include/flatdata/ResourceStorage.h b/flatdata-cpp/include/flatdata/ResourceStorage.h index 2085167f..c47f8a52 100644 --- a/flatdata-cpp/include/flatdata/ResourceStorage.h +++ b/flatdata-cpp/include/flatdata/ResourceStorage.h @@ -96,6 +96,21 @@ class ResourceStorage : private boost::noncopyable */ virtual bool exists( const char* key ) = 0; + virtual void + remove_resource( const char* /* resource_name */, const char* /* expected_schema */ ) + { + // [CJ-TODO] make it pure virtual + return; + } + + virtual void + remove_multivector_index_resource( const char* /* resource_name */, + const char* /* expected_schema */ ) + { + // [CJ-TODO] make it pure virtual + return; + } + protected: /** * @brief Creates output stream for given key diff --git a/flatdata-cpp/src/ArchiveBuilder.cpp b/flatdata-cpp/src/ArchiveBuilder.cpp index 4333b5d9..cbd3f9d3 100644 --- a/flatdata-cpp/src/ArchiveBuilder.cpp +++ b/flatdata-cpp/src/ArchiveBuilder.cpp @@ -67,4 +67,11 @@ ArchiveBuilder::initialize( ) return static_cast< bool >( signature ); } +void +ArchiveBuilder::clean_up_signature( flatdata::ResourceStorage& storage ) +{ + const auto signature_name = internal::signature_name( name( ) ); + storage.remove_resource( signature_name.c_str( ), schema( ) ); +} + } // namespace flatdata diff --git a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 index 97b5e7de..cf7c6322 100644 --- a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 +++ b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 @@ -51,7 +51,7 @@ class {{ builder_name }} : public flatdata::ArchiveBuilder { public: /// Creates Archive builder - static {{ builder_name }} open( std::shared_ptr< flatdata::ResourceStorage > storage ); + static {{ builder_name }} open( std::shared_ptr< flatdata::ResourceStorage > storage, bool force_overwrite = false ); /// Archive schema static const char* schema_definition( ); @@ -65,19 +65,22 @@ public: /// Resources using {{ r|typedef_name }} = {{ resource.writer_type(r) }}; using {{ r|typedef_name("Reader") }} = {{ resource.reader_type(r) }}; {% if r|resource_provides_incremental_builder %} - {{ r|typedef_name }} start_{{ r.name }}( ); + {{ r|typedef_name }} start_{{ r.name }}( bool force_overwrite = false ); {% endif %} {% if r|provides_setter %} - bool set_{{ r.name }}( {{ r|typedef_name("Reader") }} data ); + bool set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite = false ); {% endif %} + void clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ); {% endfor %} {% for r in archive.resources|archive_resources %} using {{ r|typedef_name }} = {{ resource.writer_type(r) }}; - {{ r|typedef_name }}& {{ r.name }}( ); + {{ r|typedef_name }}& {{ r.name }}( bool force_overwrite = false ); {% endfor %} + void clean_up( flatdata::ResourceStorage& storage ); + private: {{ builder_name }}( std::shared_ptr< flatdata::ResourceStorage > storage ); @@ -205,9 +208,13 @@ inline inline {{ builder_name }} -{{ builder_name }}::open(std::shared_ptr< flatdata::ResourceStorage > storage ) +{{ builder_name }}::open(std::shared_ptr< flatdata::ResourceStorage > storage, bool force_overwrite ) { {{ builder_name }} result( storage ); + if ( force_overwrite ) + { + result.clean_up( *storage ); + } if ( !result.initialize( ) ) { return {{ builder_name }}( ); @@ -217,31 +224,69 @@ inline {{ builder_name }} {% for r in archive.resources|simple_resources %} {% if r|resource_provides_incremental_builder %} -inline auto {{ builder_name }}::start_{{ r.name }}( ) -> {{ r|typedef_name }} +inline auto {{ builder_name }}::start_{{ r.name }}( bool force_overwrite ) -> {{ r|typedef_name }} { - check_created( ); + if ( force_overwrite ) + { + clean_up_{{ r.name }}( storage( ) ); + } + else + { + check_created( ); + } return storage( ).{{ resource.incremental_write_call(r) }}( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); } {% endif %} {% if r|provides_setter %} inline bool -{{ builder_name }}::set_{{ r.name }}( {{ r|typedef_name("Reader") }} data ) +{{ builder_name }}::set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite ) { - check_created( ); + if ( force_overwrite ) + { + clean_up_{{ r.name }}( storage( ) ); + } + else + { + check_created( ); + } return storage( ).write< {{ r|typedef_name("Reader") }} >( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__, data ); } {% endif %} +inline void +{{ builder_name }}::clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ) +{ + storage.remove_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); + {% if r|is_multivector %} + storage.remove_multivector_index_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); + {% endif %} +} {% endfor %} {% for r in archive.resources|archive_resources %} inline auto -{{ builder_name }}::{{ r.name }}( ) -> {{ r|typedef_name }}& +{{ builder_name }}::{{ r.name }}( bool force_overwrite ) -> {{ r|typedef_name }}& { if ( !m_{{ r.name }}.is_open( ) ) { - m_{{ r.name }} = {{ r|typedef_name }}::open( this->storage( ).create_directory( "{{ r.name }}" ) ); + m_{{ r.name }} = {{ r|typedef_name }}::open( this->storage( ).create_directory( "{{ r.name }}" ), force_overwrite ); } return m_{{ r.name }}; } {% endfor %} + +inline void +{{ builder_name }}::clean_up( flatdata::ResourceStorage& storage ) +{ +{% for r in archive.resources|simple_resources %} + clean_up_{{ r.name }}( storage ); +{% endfor %} +{% for r in archive.resources|archive_resources %} + if ( auto subarchive_storage = storage.directory( "{{ r.name }}" ) ) + { + m_{{ r.name }}.clean_up( *subarchive_storage ); + } +{% endfor %} + clean_up_signature( storage ); +} + {% endmacro %} From 7bbc1779c664fccb52fc0ec728aac1236e318093 Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Fri, 30 Oct 2026 16:51:09 +0100 Subject: [PATCH 2/8] cleanup support for MemoryResourceStorage Signed-off-by: Chao Jia --- .../include/flatdata/ArchiveBuilder.h | 2 +- .../include/flatdata/FileResourceStorage.h | 80 ++----------------- .../include/flatdata/MemoryResourceStorage.h | 22 +++++ .../include/flatdata/ResourceStorage.h | 28 +++---- .../flatdata/TarArchiveResourceStorage.h | 8 ++ .../flatdata/internal/ResourceStorage.inl | 38 +++++++++ flatdata-cpp/src/ArchiveBuilder.cpp | 4 +- .../generator/templates/cpp/archive.jinja2 | 34 ++++---- 8 files changed, 110 insertions(+), 106 deletions(-) diff --git a/flatdata-cpp/include/flatdata/ArchiveBuilder.h b/flatdata-cpp/include/flatdata/ArchiveBuilder.h index b7fea5bc..1cbbe875 100644 --- a/flatdata-cpp/include/flatdata/ArchiveBuilder.h +++ b/flatdata-cpp/include/flatdata/ArchiveBuilder.h @@ -52,7 +52,7 @@ class ArchiveBuilder const flatdata::ResourceStorage& storage( ) const; bool is_created( ) const; void check_created( ) const; - void clean_up_signature( flatdata::ResourceStorage& storage ); + bool clean_up_signature( flatdata::ResourceStorage& storage ); private: std::shared_ptr< flatdata::ResourceStorage > m_storage; diff --git a/flatdata-cpp/include/flatdata/FileResourceStorage.h b/flatdata-cpp/include/flatdata/FileResourceStorage.h index b08021dc..02ab5341 100644 --- a/flatdata-cpp/include/flatdata/FileResourceStorage.h +++ b/flatdata-cpp/include/flatdata/FileResourceStorage.h @@ -41,11 +41,7 @@ class FileResourceStorage : public ResourceStorage std::unique_ptr< ResourceStorage > directory( const char* key ) override; bool exists( const char* key ) override; - - void remove_resource( const char* resource_name, const char* expected_schema ) override; - - void remove_multivector_index_resource( const char* resource_name, - const char* expected_schema ) override; + bool remove( const char* key ) override; protected: std::shared_ptr< std::ostream > create_output_stream( const char* key ) override; @@ -155,76 +151,12 @@ FileResourceStorage::exists( const char* key ) return !ec; } -inline void -logger( const std::string& msg ) -{ - static std::mutex m; - std::lock_guard< std::mutex > lock( m ); - printf( "%s\n", msg.c_str( ) ); -} - -inline void -FileResourceStorage::remove_resource( const char* resource_name, const char* expected_schema ) -{ - const std::string schema_name = std::string( resource_name ) + ".schema"; - const bool resource_exists = exists( resource_name ); - const bool schema_exists = exists( schema_name.c_str( ) ); - - if ( !resource_exists && !schema_exists ) - { - logger( "[INFO] resource " + std::string( resource_name ) + ": does not exist" ); - return; - } - - if ( resource_exists ^ schema_exists ) - { - // [CJ-TODO] - // remove what's left or simply return? - // can happen when crashes during writting - logger( "[ERR] resource " + std::string( resource_name ) + ": no schema or resource" ); - } - - auto schema = read_resource( ( schema_name.c_str( ) ) ); - std::string stored_schema( reinterpret_cast< const char* >( schema.data( ) ), - schema.size_in_bytes( ) ); - - if ( stored_schema != expected_schema ) - { - // [CJ-TODO] - // What if the schema change is intended? - logger( "[ERR] resource " + std::string( resource_name ) + ": unexpected schema" ); - return; - } - - boost::system::error_code ec_resource, ec_schema; - boost::filesystem::remove( get_path( resource_name ), ec_resource ); - boost::filesystem::remove( get_path( schema_name.c_str( ) ), ec_schema ); - if ( ec_resource.failed( ) ) - { - logger( "[ERR] resource " + std::string( resource_name ) + ": failed to remove resource" ); - } - else - { - logger( "[INFO] resource " + std::string( resource_name ) + ": removed resource" ); - } - if ( ec_schema.failed( ) ) - { - logger( "[ERR] resource " + std::string( resource_name ) + ": failed to remove schema" ); - } - else - { - logger( "[INFO] resource " + std::string( resource_name ) + ": removed schema" ); - } -} - -inline void -FileResourceStorage::remove_multivector_index_resource( const char* resource_name, - const char* expected_schema ) +inline bool +FileResourceStorage::remove( const char* key ) { - std::string index_name = std::string( resource_name ) + internal::INDEX_SUFFIX; - std::string index_schema = internal::multivector_index_schema( expected_schema ); - - remove_resource( index_name.c_str( ), index_schema.c_str( ) ); + boost::system::error_code ec; + boost::filesystem::remove( get_path( key ).c_str( ), ec ); + return !ec; } } // namespace flatdata diff --git a/flatdata-cpp/include/flatdata/MemoryResourceStorage.h b/flatdata-cpp/include/flatdata/MemoryResourceStorage.h index f8dbf72b..ca9d6f55 100644 --- a/flatdata-cpp/include/flatdata/MemoryResourceStorage.h +++ b/flatdata-cpp/include/flatdata/MemoryResourceStorage.h @@ -32,6 +32,8 @@ class MemoryResourceStorage : public ResourceStorage public: static std::unique_ptr< MemoryResourceStorage > create( ); bool exists( const char* key ) override; + bool remove( const char* key ) override; + std::unique_ptr< ResourceStorage > create_directory( const char* key ) override; std::unique_ptr< ResourceStorage > directory( const char* key ) override; @@ -259,6 +261,26 @@ MemoryResourceStorage::exists( const char* key ) || helpers::prefix_exists( m_storage->streams, path + "/" ); } +inline bool +MemoryResourceStorage::remove( const char* key ) +{ + const auto path = get_path( key ); + std::lock_guard< std::mutex > lock( m_storage_mutex ); + + m_storage->streams.erase( path ); + auto found_resource = m_storage->resources.find( path ); + if ( found_resource != m_storage->resources.end( ) ) + { +#ifdef DEBUG_DATA_ACCESS_STATISTICS + auto& string = found_resource->second; + DebugDataAccessStatistics::deregister_mapping( MemoryDescriptor( + reinterpret_cast< const unsigned char* >( string->c_str( ) ), string->size( ) ) ); +#endif + m_storage->resources.erase( found_resource ); + } + return true; +} + template < typename ResourceSerializer > std::string MemoryResourceStorage::dump_resources( bool dump_schemas, ResourceSerializer&& f ) const diff --git a/flatdata-cpp/include/flatdata/ResourceStorage.h b/flatdata-cpp/include/flatdata/ResourceStorage.h index c47f8a52..40f827e3 100644 --- a/flatdata-cpp/include/flatdata/ResourceStorage.h +++ b/flatdata-cpp/include/flatdata/ResourceStorage.h @@ -17,9 +17,19 @@ #include #include +#include // [CJ-TODO] remove namespace flatdata { +inline void +logger( const std::string& msg ) +{ + // [CJ-TODO] remove + static std::mutex m; + std::lock_guard< std::mutex > lock( m ); + printf( "%s\n", msg.c_str( ) ); +} + class MemoryMappedFileStorage; /** @@ -95,21 +105,11 @@ class ResourceStorage : private boost::noncopyable * @brief Check if resource exists */ virtual bool exists( const char* key ) = 0; + virtual bool remove( const char* key ) = 0; - virtual void - remove_resource( const char* /* resource_name */, const char* /* expected_schema */ ) - { - // [CJ-TODO] make it pure virtual - return; - } - - virtual void - remove_multivector_index_resource( const char* /* resource_name */, - const char* /* expected_schema */ ) - { - // [CJ-TODO] make it pure virtual - return; - } + bool remove_resource( const char* resource_name, const char* expected_schema ); + bool remove_multivector_index_resource( const char* resource_name, + const char* expected_schema ); protected: /** diff --git a/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h b/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h index 870fc0a5..6263d65c 100644 --- a/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h +++ b/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h @@ -32,6 +32,7 @@ class TarArchiveResourceStorage : public ResourceStorage std::unique_ptr< ResourceStorage > create_directory( const char* key ) override; std::unique_ptr< ResourceStorage > directory( const char* key ) override; bool exists( const char* key ) override; + bool remove( const char* key ) override; protected: std::shared_ptr< std::ostream > create_output_stream( const char* key ) override; @@ -123,4 +124,11 @@ TarArchiveResourceStorage::exists( const char* key ) return m_storage->read( get_path( key ).c_str( ) ).data( ) != nullptr; } +inline bool +TarArchiveResourceStorage::remove( const char* /* key */ ) +{ + // not supported since to TAR files is not supported + return false; +} + } // namespace flatdata diff --git a/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl b/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl index 0520da23..5f6d6b97 100644 --- a/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl +++ b/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl @@ -207,4 +207,42 @@ ResourceStorage::create_multi_vector( const char* resource_name, const char* sch internal::ResourceReader( this, resource_name, schema ) ) ); } +inline bool +ResourceStorage::remove_resource( const char* resource_name, const char* expected_schema ) +{ + const std::string schema_name = std::string( resource_name ) + ".schema"; + const bool resource_exists = exists( resource_name ); + const bool schema_exists = exists( schema_name.c_str( ) ); + + if ( !resource_exists && !schema_exists ) + { + return true; + } + + if ( resource_exists ^ schema_exists ) + { + return false; + } + + auto schema = read_resource( ( schema_name.c_str( ) ) ); + std::string stored_schema( reinterpret_cast< const char* >( schema.data( ) ), + schema.size_in_bytes( ) ); + + if ( stored_schema != expected_schema ) + { + return false; + } + return remove( resource_name ) && remove( schema_name.c_str( ) ); +} + +inline bool +ResourceStorage::remove_multivector_index_resource( const char* resource_name, + const char* expected_schema ) +{ + std::string index_name = std::string( resource_name ) + internal::INDEX_SUFFIX; + std::string index_schema = internal::multivector_index_schema( expected_schema ); + + return remove_resource( index_name.c_str( ), index_schema.c_str( ) ); +} + } // namespace flatdata diff --git a/flatdata-cpp/src/ArchiveBuilder.cpp b/flatdata-cpp/src/ArchiveBuilder.cpp index cbd3f9d3..bf3de9d6 100644 --- a/flatdata-cpp/src/ArchiveBuilder.cpp +++ b/flatdata-cpp/src/ArchiveBuilder.cpp @@ -67,11 +67,11 @@ ArchiveBuilder::initialize( ) return static_cast< bool >( signature ); } -void +bool ArchiveBuilder::clean_up_signature( flatdata::ResourceStorage& storage ) { const auto signature_name = internal::signature_name( name( ) ); - storage.remove_resource( signature_name.c_str( ), schema( ) ); + return storage.remove_resource( signature_name.c_str( ), schema( ) ); } } // namespace flatdata diff --git a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 index cf7c6322..8c785286 100644 --- a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 +++ b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 @@ -70,7 +70,7 @@ public: /// Resources {% if r|provides_setter %} bool set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite = false ); {% endif %} - void clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ); + bool clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ); {% endfor %} @@ -79,7 +79,7 @@ public: /// Resources {{ r|typedef_name }}& {{ r.name }}( bool force_overwrite = false ); {% endfor %} - void clean_up( flatdata::ResourceStorage& storage ); + bool clean_up( flatdata::ResourceStorage& storage ); private: {{ builder_name }}( std::shared_ptr< flatdata::ResourceStorage > storage ); @@ -211,9 +211,9 @@ inline {{ builder_name }} {{ builder_name }}::open(std::shared_ptr< flatdata::ResourceStorage > storage, bool force_overwrite ) { {{ builder_name }} result( storage ); - if ( force_overwrite ) + if ( force_overwrite && !result.clean_up( *storage ) ) { - result.clean_up( *storage ); + return {{ builder_name }}( ); } if ( !result.initialize( ) ) { @@ -226,9 +226,9 @@ inline {{ builder_name }} {% if r|resource_provides_incremental_builder %} inline auto {{ builder_name }}::start_{{ r.name }}( bool force_overwrite ) -> {{ r|typedef_name }} { - if ( force_overwrite ) + if ( force_overwrite && !clean_up_{{ r.name }}( storage( ) ) ) { - clean_up_{{ r.name }}( storage( ) ); + throw std::runtime_error( "Failed to clean up existing resource for force overwrite" ); } else { @@ -241,9 +241,9 @@ inline auto {{ builder_name }}::start_{{ r.name }}( bool force_overwrite ) -> {{ inline bool {{ builder_name }}::set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite ) { - if ( force_overwrite ) + if ( force_overwrite && !clean_up_{{ r.name }}( storage( ) ) ) { - clean_up_{{ r.name }}( storage( ) ); + throw std::runtime_error( "Failed to clean up existing resource for force overwrite" ); } else { @@ -252,13 +252,14 @@ inline bool return storage( ).write< {{ r|typedef_name("Reader") }} >( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__, data ); } {% endif %} -inline void +inline bool {{ builder_name }}::clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ) { - storage.remove_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); + bool res = storage.remove_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); {% if r|is_multivector %} - storage.remove_multivector_index_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); + res &= storage.remove_multivector_index_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); {% endif %} + return res; } {% endfor %} @@ -274,19 +275,22 @@ inline auto } {% endfor %} -inline void +inline bool {{ builder_name }}::clean_up( flatdata::ResourceStorage& storage ) { + bool res = true; {% for r in archive.resources|simple_resources %} - clean_up_{{ r.name }}( storage ); + res &= clean_up_{{ r.name }}( storage ); {% endfor %} {% for r in archive.resources|archive_resources %} if ( auto subarchive_storage = storage.directory( "{{ r.name }}" ) ) { - m_{{ r.name }}.clean_up( *subarchive_storage ); + res &= m_{{ r.name }}.clean_up( *subarchive_storage ); } {% endfor %} - clean_up_signature( storage ); + res &= clean_up_signature( storage ); + + return res; } {% endmacro %} From d99c322461ca555779570f2f2541aadd722b0eb1 Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Tue, 10 Nov 2026 02:55:33 +0100 Subject: [PATCH 3/8] clean up support for rust implementation Signed-off-by: Chao Jia --- .../generator/templates/rust/archive.jinja2 | 24 +++++++++++ flatdata-rs/lib/src/filestorage.rs | 21 ++++++++++ flatdata-rs/lib/src/memstorage.rs | 11 +++++ flatdata-rs/lib/src/storage.rs | 41 +++++++++++++++++++ flatdata-rs/lib/src/tarstorage.rs | 6 +++ flatdata-rs/lib/src/test/mod.rs | 2 + flatdata-rs/tests/coappearances/src/lib.rs | 6 +-- 7 files changed, 108 insertions(+), 3 deletions(-) diff --git a/flatdata-generator/flatdata/generator/templates/rust/archive.jinja2 b/flatdata-generator/flatdata/generator/templates/rust/archive.jinja2 index a3867cbb..00789e2c 100644 --- a/flatdata-generator/flatdata/generator/templates/rust/archive.jinja2 +++ b/flatdata-generator/flatdata/generator/templates/rust/archive.jinja2 @@ -256,7 +256,31 @@ impl {{archive.name}}Builder { } {% endif %} + #[inline] + pub fn clean_up_{{ r.name }}_storage(storage: flatdata::StorageHandle) -> Result<(), ::std::io::Error> { + {% if [r] | subarchive_resources %} + {% set t = fully_qualified_name(archive, r.target.node) %} + {{t}}Builder::clean_up(storage.subdir("{{r.name}}")) + {% else %} + {% if [r] | multivector_resources %} + storage.remove_multivector_index_resource("{{r.name}}", schema::{{ archive_ns }}::resources::{{ r.name | upper }})?; + {% endif %} + storage.remove_resource("{{r.name}}", schema::{{ archive_ns }}::resources::{{ r.name | upper }}) + {% endif %} + } + #[inline] + pub fn clean_up_{{ r.name }}(&self) -> Result<(), ::std::io::Error> { + Self::clean_up_{{ r.name }}_storage(self.storage.clone()) + } {% endfor %} + + #[inline] + pub fn clean_up(storage: flatdata::StorageHandle) -> Result<(), ::std::io::Error> { + {% for r in archive.resources | supported_resources %} + Self::clean_up_{{ r.name }}_storage(storage.clone())?; + {% endfor %} + storage.remove_resource("{{archive.name}}.archive", schema::{{ archive_ns }}::{{ archive.name | camel_to_snake_case | upper }}) + } } {% endif %} diff --git a/flatdata-rs/lib/src/filestorage.rs b/flatdata-rs/lib/src/filestorage.rs index 8e1e14d9..489525bb 100644 --- a/flatdata-rs/lib/src/filestorage.rs +++ b/flatdata-rs/lib/src/filestorage.rs @@ -77,6 +77,27 @@ impl ResourceStorage for FileResourceStorage { self.path.join(resource_name).exists() } + fn remove(&self, resource_name: &str) -> Result<(), io::Error> { + let resource_path = self.path.join(resource_name); + if let Err(e) = fs::remove_file(&resource_path) { + // cj-TODO: remove + println!( + "Failed to remove {}", + resource_path.to_str().unwrap_or(resource_name) + ); + return Err(io::Error::new( + e.kind(), + String::from(resource_path.to_str().unwrap_or(resource_name)), + )); + } + // cj-TODO: remove + println!( + "removed {}", + resource_path.to_str().unwrap_or(resource_name) + ); + Ok(()) + } + fn read_resource(&self, resource_name: &str) -> Result<&[u8], io::Error> { let resource_path = self.path.join(resource_name); if !resource_path.exists() { diff --git a/flatdata-rs/lib/src/memstorage.rs b/flatdata-rs/lib/src/memstorage.rs index 04fdee21..64eeb797 100644 --- a/flatdata-rs/lib/src/memstorage.rs +++ b/flatdata-rs/lib/src/memstorage.rs @@ -104,6 +104,17 @@ impl ResourceStorage for MemoryResourceStorage { .contains_key(&resource_path) } + fn remove(&self, resource_name: &str) -> Result<(), io::Error> { + let resource_path = self.path.join(resource_name); + self.storage.streams.lock().unwrap().remove(&resource_path); + self.storage + .resources + .lock() + .unwrap() + .remove(&resource_path); + Ok(()) + } + fn read_resource(&self, resource_name: &str) -> Result<&[u8], io::Error> { let resource_path = self.path.join(resource_name); if !self diff --git a/flatdata-rs/lib/src/storage.rs b/flatdata-rs/lib/src/storage.rs index d3c4eabf..e8d00cdf 100644 --- a/flatdata-rs/lib/src/storage.rs +++ b/flatdata-rs/lib/src/storage.rs @@ -50,6 +50,44 @@ pub trait ResourceStorage: std::fmt::Debug { write_schema(schema, &mut stream) } + /// remove resource and the corresponding schema + /// fail if resource or schema is missing or the existing schema doesn't match the expected + /// schema + #[doc(hidden)] + fn remove_resource(&self, resource_name: &str, expected_schema: &str) -> Result<(), io::Error> { + let schema_name = format!("{}.schema", resource_name); + let resource_exists = self.exists(resource_name); + let schema_exists = self.exists(&schema_name); + if !resource_exists && !schema_exists { + return Ok(()); + } else if resource_exists != schema_exists { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("resource or schema missing for {}", resource_name), + )); + } + let stored_schema = self.read_resource(&schema_name)?; + if stored_schema != expected_schema.as_bytes() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("unexpedted schema for {}", resource_name), + )); + } + self.remove(resource_name)?; + self.remove(&schema_name) + } + + /// remove multivector index resource + #[doc(hidden)] + fn remove_multivector_index_resource( + &self, + resource_name: &str, + expected_schema: &str, + ) -> Result<(), io::Error> { + let index_name = format!("{}_index", resource_name); + let index_schema = format!("index({})", expected_schema); + self.remove_resource(&index_name, &index_schema) + } // // Virtual // @@ -60,6 +98,9 @@ pub trait ResourceStorage: std::fmt::Debug { /// Returns `true` if resource exists in the storage. fn exists(&self, resource_name: &str) -> bool; + /// remove the resource with key `resouce_name` + fn remove(&self, resource_name: &str) -> Result<(), io::Error>; + /// Reads a resource in storage and returns a pointer to its raw data. /// /// This is a low level facility for opening and reading resources. Cf. diff --git a/flatdata-rs/lib/src/tarstorage.rs b/flatdata-rs/lib/src/tarstorage.rs index 6809fc81..d43b25af 100644 --- a/flatdata-rs/lib/src/tarstorage.rs +++ b/flatdata-rs/lib/src/tarstorage.rs @@ -112,6 +112,12 @@ impl ResourceStorage for TarArchiveResourceStorage { .is_some() } + fn remove(&self, _resource_name: &str) -> Result<(), io::Error> { + Err(io::Error::other( + "Removing from tar archives is not supported", + )) + } + fn read_resource(&self, resource_name: &str) -> Result<&[u8], io::Error> { let resource_path = self.sub_path.join(resource_name); if let Some(data) = self.storage.read(&resource_path) { diff --git a/flatdata-rs/lib/src/test/mod.rs b/flatdata-rs/lib/src/test/mod.rs index 01869db3..fdb46825 100644 --- a/flatdata-rs/lib/src/test/mod.rs +++ b/flatdata-rs/lib/src/test/mod.rs @@ -18,6 +18,8 @@ // TODO: Implement missing docs // TODO: Implement missing debugs #![allow(dead_code, missing_debug_implementations, missing_docs)] +// The generated file defines `mod test`, which is included inside `mod test`. +#![allow(clippy::module_inception)] include!("test_generated.rs"); diff --git a/flatdata-rs/tests/coappearances/src/lib.rs b/flatdata-rs/tests/coappearances/src/lib.rs index 20f4aa44..28ef99b7 100644 --- a/flatdata-rs/tests/coappearances/src/lib.rs +++ b/flatdata-rs/tests/coappearances/src/lib.rs @@ -177,11 +177,11 @@ fn copy_coappearances_archive( // open for writing let archive_path = env::temp_dir().join(to_path); println!("Will create archive in: {}", archive_path.to_str().unwrap()); - if archive_path.exists() { - fs::remove_dir_all(&archive_path).expect("could not remove already existing archive"); - } fs::create_dir_all(&archive_path).expect("could not create archive dir"); let storage = flatdata::FileResourceStorage::new(archive_path.clone()); + if let Err(e) = coappearances::GraphBuilder::clean_up(storage.clone()) { + panic!("Cleanup failed: {}", e); + } let gb = coappearances::GraphBuilder::new(storage).expect("could not create archive"); // copy data From 0f8b916fdeba02c120fc022651d6bbe86f36bc86 Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Tue, 10 Nov 2026 08:54:54 +0100 Subject: [PATCH 4/8] update cleanup support for c++ implementation Signed-off-by: Chao Jia --- .../include/flatdata/ArchiveBuilder.h | 2 +- .../include/flatdata/FileResourceStorage.h | 12 ++++--- .../include/flatdata/MemoryResourceStorage.h | 5 ++- .../include/flatdata/ResourceStorage.h | 15 ++------ .../flatdata/TarArchiveResourceStorage.h | 6 ++-- .../flatdata/internal/ResourceStorage.inl | 18 +++++----- flatdata-cpp/src/ArchiveBuilder.cpp | 4 +-- flatdata-cpp/test/ResourceStorageTest.cpp | 23 +++++++++++++ .../generator/templates/cpp/archive.jinja2 | 34 ++++++++----------- 9 files changed, 67 insertions(+), 52 deletions(-) diff --git a/flatdata-cpp/include/flatdata/ArchiveBuilder.h b/flatdata-cpp/include/flatdata/ArchiveBuilder.h index 1cbbe875..b7fea5bc 100644 --- a/flatdata-cpp/include/flatdata/ArchiveBuilder.h +++ b/flatdata-cpp/include/flatdata/ArchiveBuilder.h @@ -52,7 +52,7 @@ class ArchiveBuilder const flatdata::ResourceStorage& storage( ) const; bool is_created( ) const; void check_created( ) const; - bool clean_up_signature( flatdata::ResourceStorage& storage ); + void clean_up_signature( flatdata::ResourceStorage& storage ); private: std::shared_ptr< flatdata::ResourceStorage > m_storage; diff --git a/flatdata-cpp/include/flatdata/FileResourceStorage.h b/flatdata-cpp/include/flatdata/FileResourceStorage.h index 02ab5341..64501d7b 100644 --- a/flatdata-cpp/include/flatdata/FileResourceStorage.h +++ b/flatdata-cpp/include/flatdata/FileResourceStorage.h @@ -41,7 +41,7 @@ class FileResourceStorage : public ResourceStorage std::unique_ptr< ResourceStorage > directory( const char* key ) override; bool exists( const char* key ) override; - bool remove( const char* key ) override; + void remove( const char* key ) override; protected: std::shared_ptr< std::ostream > create_output_stream( const char* key ) override; @@ -151,12 +151,16 @@ FileResourceStorage::exists( const char* key ) return !ec; } -inline bool +inline void FileResourceStorage::remove( const char* key ) { boost::system::error_code ec; - boost::filesystem::remove( get_path( key ).c_str( ), ec ); - return !ec; + std::string file_path = get_path( key ); + boost::filesystem::remove( file_path.c_str( ), ec ); + if ( ec ) + { + throw std::runtime_error( std::string( "Failed to remove " ) + file_path ); + } } } // namespace flatdata diff --git a/flatdata-cpp/include/flatdata/MemoryResourceStorage.h b/flatdata-cpp/include/flatdata/MemoryResourceStorage.h index ca9d6f55..306368ae 100644 --- a/flatdata-cpp/include/flatdata/MemoryResourceStorage.h +++ b/flatdata-cpp/include/flatdata/MemoryResourceStorage.h @@ -32,7 +32,7 @@ class MemoryResourceStorage : public ResourceStorage public: static std::unique_ptr< MemoryResourceStorage > create( ); bool exists( const char* key ) override; - bool remove( const char* key ) override; + void remove( const char* key ) override; std::unique_ptr< ResourceStorage > create_directory( const char* key ) override; std::unique_ptr< ResourceStorage > directory( const char* key ) override; @@ -261,7 +261,7 @@ MemoryResourceStorage::exists( const char* key ) || helpers::prefix_exists( m_storage->streams, path + "/" ); } -inline bool +inline void MemoryResourceStorage::remove( const char* key ) { const auto path = get_path( key ); @@ -278,7 +278,6 @@ MemoryResourceStorage::remove( const char* key ) #endif m_storage->resources.erase( found_resource ); } - return true; } template < typename ResourceSerializer > diff --git a/flatdata-cpp/include/flatdata/ResourceStorage.h b/flatdata-cpp/include/flatdata/ResourceStorage.h index 40f827e3..c16378b1 100644 --- a/flatdata-cpp/include/flatdata/ResourceStorage.h +++ b/flatdata-cpp/include/flatdata/ResourceStorage.h @@ -21,15 +21,6 @@ namespace flatdata { -inline void -logger( const std::string& msg ) -{ - // [CJ-TODO] remove - static std::mutex m; - std::lock_guard< std::mutex > lock( m ); - printf( "%s\n", msg.c_str( ) ); -} - class MemoryMappedFileStorage; /** @@ -105,10 +96,10 @@ class ResourceStorage : private boost::noncopyable * @brief Check if resource exists */ virtual bool exists( const char* key ) = 0; - virtual bool remove( const char* key ) = 0; + virtual void remove( const char* key ) = 0; - bool remove_resource( const char* resource_name, const char* expected_schema ); - bool remove_multivector_index_resource( const char* resource_name, + void remove_resource( const char* resource_name, const char* expected_schema ); + void remove_multivector_index_resource( const char* resource_name, const char* expected_schema ); protected: diff --git a/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h b/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h index 6263d65c..c9a4ba79 100644 --- a/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h +++ b/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h @@ -32,7 +32,7 @@ class TarArchiveResourceStorage : public ResourceStorage std::unique_ptr< ResourceStorage > create_directory( const char* key ) override; std::unique_ptr< ResourceStorage > directory( const char* key ) override; bool exists( const char* key ) override; - bool remove( const char* key ) override; + void remove( const char* key ) override; protected: std::shared_ptr< std::ostream > create_output_stream( const char* key ) override; @@ -124,11 +124,11 @@ TarArchiveResourceStorage::exists( const char* key ) return m_storage->read( get_path( key ).c_str( ) ).data( ) != nullptr; } -inline bool +inline void TarArchiveResourceStorage::remove( const char* /* key */ ) { // not supported since to TAR files is not supported - return false; + throw std::runtime_error( "Removing from TAR files is not supported" ); } } // namespace flatdata diff --git a/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl b/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl index 5f6d6b97..a8be4cdd 100644 --- a/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl +++ b/flatdata-cpp/include/flatdata/internal/ResourceStorage.inl @@ -207,7 +207,7 @@ ResourceStorage::create_multi_vector( const char* resource_name, const char* sch internal::ResourceReader( this, resource_name, schema ) ) ); } -inline bool +inline void ResourceStorage::remove_resource( const char* resource_name, const char* expected_schema ) { const std::string schema_name = std::string( resource_name ) + ".schema"; @@ -216,12 +216,13 @@ ResourceStorage::remove_resource( const char* resource_name, const char* expecte if ( !resource_exists && !schema_exists ) { - return true; + return; } - if ( resource_exists ^ schema_exists ) + if ( resource_exists != schema_exists ) { - return false; + throw std::runtime_error( std::string( "Resource or schema missing for " ) + + resource_name ); } auto schema = read_resource( ( schema_name.c_str( ) ) ); @@ -230,19 +231,20 @@ ResourceStorage::remove_resource( const char* resource_name, const char* expecte if ( stored_schema != expected_schema ) { - return false; + throw std::runtime_error( std::string( "unexpected schema for " ) + resource_name ); } - return remove( resource_name ) && remove( schema_name.c_str( ) ); + remove( resource_name ); + remove( schema_name.c_str( ) ); } -inline bool +inline void ResourceStorage::remove_multivector_index_resource( const char* resource_name, const char* expected_schema ) { std::string index_name = std::string( resource_name ) + internal::INDEX_SUFFIX; std::string index_schema = internal::multivector_index_schema( expected_schema ); - return remove_resource( index_name.c_str( ), index_schema.c_str( ) ); + remove_resource( index_name.c_str( ), index_schema.c_str( ) ); } } // namespace flatdata diff --git a/flatdata-cpp/src/ArchiveBuilder.cpp b/flatdata-cpp/src/ArchiveBuilder.cpp index bf3de9d6..cbd3f9d3 100644 --- a/flatdata-cpp/src/ArchiveBuilder.cpp +++ b/flatdata-cpp/src/ArchiveBuilder.cpp @@ -67,11 +67,11 @@ ArchiveBuilder::initialize( ) return static_cast< bool >( signature ); } -bool +void ArchiveBuilder::clean_up_signature( flatdata::ResourceStorage& storage ) { const auto signature_name = internal::signature_name( name( ) ); - return storage.remove_resource( signature_name.c_str( ), schema( ) ); + storage.remove_resource( signature_name.c_str( ), schema( ) ); } } // namespace flatdata diff --git a/flatdata-cpp/test/ResourceStorageTest.cpp b/flatdata-cpp/test/ResourceStorageTest.cpp index 0b5c148d..6be503af 100644 --- a/flatdata-cpp/test/ResourceStorageTest.cpp +++ b/flatdata-cpp/test/ResourceStorageTest.cpp @@ -81,5 +81,28 @@ TEST_CASE( "schema is checked for written vector", "[ResourceStorage]" ) ( a->read< ArrayView< AStruct > >( resource_name, correct_schema ).is_initialized( ) ) ); } +TEST_CASE( "resource doesn't exist after removing it", "[ResourceStorage]" ) +{ + std::unique_ptr< ResourceStorage > a = MemoryResourceStorage::create( ); + auto create = [ & ] { + auto v = a->create_external_vector< AStruct >( resource_name, correct_schema ); + v.close( ); + }; + REQUIRE_NOTHROW( create( ) ); + + REQUIRE_FALSE( + ( a->read< ArrayView< AStruct > >( resource_name, incorrect_schema ).is_initialized( ) ) ); + REQUIRE( + ( a->read< ArrayView< AStruct > >( resource_name, correct_schema ).is_initialized( ) ) ); + + std::string schema_name = std::string( resource_name ) + ".schema"; + REQUIRE( a->exists( resource_name ) ); + REQUIRE( a->exists( schema_name.c_str( ) ) ); + + a->remove_resource( resource_name, correct_schema ); + REQUIRE_FALSE( a->exists( resource_name ) ); + REQUIRE_FALSE( a->exists( schema_name.c_str( ) ) ); +} + } // namespace test } // namespace flatdata diff --git a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 index 8c785286..c704d270 100644 --- a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 +++ b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 @@ -70,7 +70,7 @@ public: /// Resources {% if r|provides_setter %} bool set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite = false ); {% endif %} - bool clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ); + static void clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ); {% endfor %} @@ -79,7 +79,7 @@ public: /// Resources {{ r|typedef_name }}& {{ r.name }}( bool force_overwrite = false ); {% endfor %} - bool clean_up( flatdata::ResourceStorage& storage ); + void clean_up( flatdata::ResourceStorage& storage ); private: {{ builder_name }}( std::shared_ptr< flatdata::ResourceStorage > storage ); @@ -211,9 +211,9 @@ inline {{ builder_name }} {{ builder_name }}::open(std::shared_ptr< flatdata::ResourceStorage > storage, bool force_overwrite ) { {{ builder_name }} result( storage ); - if ( force_overwrite && !result.clean_up( *storage ) ) + if ( force_overwrite ) { - return {{ builder_name }}( ); + result.clean_up( *storage ); } if ( !result.initialize( ) ) { @@ -226,9 +226,9 @@ inline {{ builder_name }} {% if r|resource_provides_incremental_builder %} inline auto {{ builder_name }}::start_{{ r.name }}( bool force_overwrite ) -> {{ r|typedef_name }} { - if ( force_overwrite && !clean_up_{{ r.name }}( storage( ) ) ) + if ( force_overwrite ) { - throw std::runtime_error( "Failed to clean up existing resource for force overwrite" ); + clean_up_{{ r.name }}( storage( ) ); } else { @@ -241,9 +241,9 @@ inline auto {{ builder_name }}::start_{{ r.name }}( bool force_overwrite ) -> {{ inline bool {{ builder_name }}::set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite ) { - if ( force_overwrite && !clean_up_{{ r.name }}( storage( ) ) ) + if ( force_overwrite) { - throw std::runtime_error( "Failed to clean up existing resource for force overwrite" ); + clean_up_{{ r.name }}( storage( ) ); } else { @@ -252,14 +252,13 @@ inline bool return storage( ).write< {{ r|typedef_name("Reader") }} >( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__, data ); } {% endif %} -inline bool +inline void {{ builder_name }}::clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ) { - bool res = storage.remove_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); + storage.remove_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); {% if r|is_multivector %} - res &= storage.remove_multivector_index_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); + storage.remove_multivector_index_resource( "{{ r.name }}", internal::{{ archive.name }}__{{ r.name }}__schema__ ); {% endif %} - return res; } {% endfor %} @@ -275,22 +274,19 @@ inline auto } {% endfor %} -inline bool +inline void {{ builder_name }}::clean_up( flatdata::ResourceStorage& storage ) { - bool res = true; {% for r in archive.resources|simple_resources %} - res &= clean_up_{{ r.name }}( storage ); + {{ builder_name }}::clean_up_{{ r.name }}( storage ); {% endfor %} {% for r in archive.resources|archive_resources %} if ( auto subarchive_storage = storage.directory( "{{ r.name }}" ) ) { - res &= m_{{ r.name }}.clean_up( *subarchive_storage ); + m_{{ r.name }}.clean_up( *subarchive_storage ); } {% endfor %} - res &= clean_up_signature( storage ); - - return res; + clean_up_signature( storage ); } {% endmacro %} From 016f8fd8072ec2c31474776f53f9b429a99a1078 Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Tue, 10 Nov 2026 09:42:24 +0100 Subject: [PATCH 5/8] remove debugging code Signed-off-by: Chao Jia --- flatdata-cpp/include/flatdata/ResourceStorage.h | 1 - flatdata-rs/lib/src/filestorage.rs | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/flatdata-cpp/include/flatdata/ResourceStorage.h b/flatdata-cpp/include/flatdata/ResourceStorage.h index c16378b1..5aefa2c9 100644 --- a/flatdata-cpp/include/flatdata/ResourceStorage.h +++ b/flatdata-cpp/include/flatdata/ResourceStorage.h @@ -17,7 +17,6 @@ #include #include -#include // [CJ-TODO] remove namespace flatdata { diff --git a/flatdata-rs/lib/src/filestorage.rs b/flatdata-rs/lib/src/filestorage.rs index 489525bb..fb1cc79c 100644 --- a/flatdata-rs/lib/src/filestorage.rs +++ b/flatdata-rs/lib/src/filestorage.rs @@ -80,21 +80,11 @@ impl ResourceStorage for FileResourceStorage { fn remove(&self, resource_name: &str) -> Result<(), io::Error> { let resource_path = self.path.join(resource_name); if let Err(e) = fs::remove_file(&resource_path) { - // cj-TODO: remove - println!( - "Failed to remove {}", - resource_path.to_str().unwrap_or(resource_name) - ); return Err(io::Error::new( e.kind(), String::from(resource_path.to_str().unwrap_or(resource_name)), )); } - // cj-TODO: remove - println!( - "removed {}", - resource_path.to_str().unwrap_or(resource_name) - ); Ok(()) } From 1114cbf6c090cc3b761cc8a13be8c7201c9dc5bb Mon Sep 17 00:00:00 2001 From: Chao <6785880+ChaoJia@users.noreply.github.com> Date: Wed, 11 Nov 2026 13:31:49 +0100 Subject: [PATCH 6/8] Apply suggestion from @epavese Co-authored-by: epavese Signed-off-by: Chao Jia --- flatdata-rs/lib/src/storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flatdata-rs/lib/src/storage.rs b/flatdata-rs/lib/src/storage.rs index e8d00cdf..0c3bb00f 100644 --- a/flatdata-rs/lib/src/storage.rs +++ b/flatdata-rs/lib/src/storage.rs @@ -70,7 +70,7 @@ pub trait ResourceStorage: std::fmt::Debug { if stored_schema != expected_schema.as_bytes() { return Err(io::Error::new( io::ErrorKind::InvalidData, - format!("unexpedted schema for {}", resource_name), + format!("unexpected schema for {}", resource_name), )); } self.remove(resource_name)?; From 74d4d13cd2c68ac279a76af7d455c18f2bcac353 Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Wed, 11 Nov 2026 14:53:35 +0100 Subject: [PATCH 7/8] address some review comments Signed-off-by: Chao Jia --- .../include/flatdata/TarArchiveResourceStorage.h | 2 +- .../flatdata/generator/templates/cpp/archive.jinja2 | 1 - flatdata-rs/lib/src/filestorage.rs | 9 ++++----- flatdata-rs/lib/src/tarstorage.rs | 2 +- flatdata-rs/tests/coappearances/src/lib.rs | 5 ++--- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h b/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h index c9a4ba79..9e31b031 100644 --- a/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h +++ b/flatdata-cpp/include/flatdata/TarArchiveResourceStorage.h @@ -127,7 +127,7 @@ TarArchiveResourceStorage::exists( const char* key ) inline void TarArchiveResourceStorage::remove( const char* /* key */ ) { - // not supported since to TAR files is not supported + // Removing parts of TAR files is not supported throw std::runtime_error( "Removing from TAR files is not supported" ); } diff --git a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 index c704d270..913cdc02 100644 --- a/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 +++ b/flatdata-generator/flatdata/generator/templates/cpp/archive.jinja2 @@ -71,7 +71,6 @@ public: /// Resources bool set_{{ r.name }}( {{ r|typedef_name("Reader") }} data, bool force_overwrite = false ); {% endif %} static void clean_up_{{ r.name }}( flatdata::ResourceStorage& storage ); - {% endfor %} {% for r in archive.resources|archive_resources %} diff --git a/flatdata-rs/lib/src/filestorage.rs b/flatdata-rs/lib/src/filestorage.rs index fb1cc79c..523f81a6 100644 --- a/flatdata-rs/lib/src/filestorage.rs +++ b/flatdata-rs/lib/src/filestorage.rs @@ -79,13 +79,12 @@ impl ResourceStorage for FileResourceStorage { fn remove(&self, resource_name: &str) -> Result<(), io::Error> { let resource_path = self.path.join(resource_name); - if let Err(e) = fs::remove_file(&resource_path) { - return Err(io::Error::new( + fs::remove_file(&resource_path).map_err(|e| { + io::Error::new( e.kind(), String::from(resource_path.to_str().unwrap_or(resource_name)), - )); - } - Ok(()) + ) + }) } fn read_resource(&self, resource_name: &str) -> Result<&[u8], io::Error> { diff --git a/flatdata-rs/lib/src/tarstorage.rs b/flatdata-rs/lib/src/tarstorage.rs index d43b25af..2b4be66b 100644 --- a/flatdata-rs/lib/src/tarstorage.rs +++ b/flatdata-rs/lib/src/tarstorage.rs @@ -114,7 +114,7 @@ impl ResourceStorage for TarArchiveResourceStorage { fn remove(&self, _resource_name: &str) -> Result<(), io::Error> { Err(io::Error::other( - "Removing from tar archives is not supported", + "Removing parts of tar archives is not supported", )) } diff --git a/flatdata-rs/tests/coappearances/src/lib.rs b/flatdata-rs/tests/coappearances/src/lib.rs index 28ef99b7..794a6e3a 100644 --- a/flatdata-rs/tests/coappearances/src/lib.rs +++ b/flatdata-rs/tests/coappearances/src/lib.rs @@ -179,9 +179,8 @@ fn copy_coappearances_archive( println!("Will create archive in: {}", archive_path.to_str().unwrap()); fs::create_dir_all(&archive_path).expect("could not create archive dir"); let storage = flatdata::FileResourceStorage::new(archive_path.clone()); - if let Err(e) = coappearances::GraphBuilder::clean_up(storage.clone()) { - panic!("Cleanup failed: {}", e); - } + coappearances::GraphBuilder::clean_up(storage.clone()) + .expect("Failed to clean up {}", archive_path.to_str()); let gb = coappearances::GraphBuilder::new(storage).expect("could not create archive"); // copy data From d86775b79483975f0f08caa80a30def6d452beca Mon Sep 17 00:00:00 2001 From: Chao Jia Date: Wed, 11 Nov 2026 15:10:45 +0100 Subject: [PATCH 8/8] small fix Signed-off-by: Chao Jia --- flatdata-rs/tests/coappearances/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flatdata-rs/tests/coappearances/src/lib.rs b/flatdata-rs/tests/coappearances/src/lib.rs index 794a6e3a..27db72a4 100644 --- a/flatdata-rs/tests/coappearances/src/lib.rs +++ b/flatdata-rs/tests/coappearances/src/lib.rs @@ -179,8 +179,7 @@ fn copy_coappearances_archive( println!("Will create archive in: {}", archive_path.to_str().unwrap()); fs::create_dir_all(&archive_path).expect("could not create archive dir"); let storage = flatdata::FileResourceStorage::new(archive_path.clone()); - coappearances::GraphBuilder::clean_up(storage.clone()) - .expect("Failed to clean up {}", archive_path.to_str()); + coappearances::GraphBuilder::clean_up(storage.clone()).expect("Cleanup failed"); let gb = coappearances::GraphBuilder::new(storage).expect("could not create archive"); // copy data