From 6c3a8e56290753ea19ff9313469e55ff9f93e215 Mon Sep 17 00:00:00 2001 From: Marco Clemencic Date: Tue, 26 Mar 2024 14:16:32 +0100 Subject: [PATCH] Add check against missing 0 value for conditions If the requested time point is before the first condition start, GitCondDB returns an invalid IOV. We usually protect against it by adding a default condition value for time point 0, but if that is missing we should stop right away as the code downstream does not expect missing condition values. This change introduces the check and adds a specific test. --- Core/src/ConditionsGitReader.cpp | 5 + Core/tests/CMakeLists.txt | 1 + Core/tests/src/test_condition_git_reader.cpp | 113 +++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 Core/tests/src/test_condition_git_reader.cpp diff --git a/Core/src/ConditionsGitReader.cpp b/Core/src/ConditionsGitReader.cpp index 29bd95c83b..fca1c9916a 100644 --- a/Core/src/ConditionsGitReader.cpp +++ b/Core/src/ConditionsGitReader.cpp @@ -101,6 +101,11 @@ int LHCb::Detector::ConditionsGitReader::getObject( const std::string& system_id try { std::tie( data, iov ) = m_condDB->get( {m_dbtag, path, time_point} ); } catch ( std::runtime_error const& e ) { return 0; } + if ( !iov.valid() ) { + throw std::runtime_error( + fmt::format( "invalid IOV for {}:{}, cannot find data for point {} (probably missing 0 value)", m_dbtag, path, + time_point ) ); + } if ( !m_limitedIovPaths.empty() && m_limitedIovPaths.find( path ) != m_limitedIovPaths.end() ) { if ( iov.since != time_point ) { diff --git a/Core/tests/CMakeLists.txt b/Core/tests/CMakeLists.txt index da56121a0e..d116c353a1 100644 --- a/Core/tests/CMakeLists.txt +++ b/Core/tests/CMakeLists.txt @@ -29,6 +29,7 @@ add_executable(test_DDS src/test_DDS_tell40links.cpp src/test_DDS_interactionregion.cpp src/test_conddb_schema_handling.cpp + src/test_condition_git_reader.cpp ) target_link_libraries(test_DDS Catch2::Catch2WithMain diff --git a/Core/tests/src/test_condition_git_reader.cpp b/Core/tests/src/test_condition_git_reader.cpp new file mode 100644 index 0000000000..b0cb391a85 --- /dev/null +++ b/Core/tests/src/test_condition_git_reader.cpp @@ -0,0 +1,113 @@ +/*****************************************************************************\ +* (c) Copyright 2024 CERN for the benefit of the LHCb Collaboration * +* * +* This software is distributed under the terms of the GNU General Public * +* Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING". * +* * +* In applying this licence, CERN does not waive the privileges and immunities * +* granted to it by virtue of its status as an Intergovernmental Organization * +* or submit itself to any jurisdiction. * +\*****************************************************************************/ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace { + struct DummyDeIOVObject final : LHCb::Detector::detail::DeIOVObject { + using DeIOVObject::DeIOVObject; + void applyToAllChildren( const std::function )>& ) const override {} + }; + + struct DummyConditionCall : LHCb::Detector::DeConditionCall { + using DeConditionCall::DeConditionCall; + virtual dd4hep::Condition operator()( const dd4hep::ConditionKey&, + dd4hep::cond::ConditionUpdateContext& context ) override final { + auto det = dd4hep::Detector::getInstance().detector( "test" ); + return LHCb::Detector::DeIOV{new DummyDeIOVObject( det, context, 9000001, false )}; + } + }; + long create_conditions_recipes( dd4hep::Detector& description, xml_h e ) { + // Use the helper to load the XML, setup the callback according + LHCb::Detector::ConditionConfigHelper config_helper{description, "test", e}; + config_helper.configure(); + return 1; + } +} // namespace +DECLARE_XML_DOC_READER( Test_Dummy_cond, create_conditions_recipes ) + +static constexpr const char* main_xml = R"( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +)"; + +TEST_CASE( "ConditionGitReader.missing_0" ) { + Detector::Test::Fixture f; + + auto& description = f.description(); + dd4hep::DetectorLoad{description}.processXMLString( main_xml ); + + auto det = description.detector( "test" ); + // the `!!` is needed because handles have `operator!` but not `operator bool` + REQUIRE( !!det ); + + std::vector detector_list{"/world", "test"}; + LHCb::Detector::DetectorDataService dds( description, detector_list ); + // this DB is invalid as it misses the special condition for time point 0 + // and hitting it must throw an exception + dds.initialize( nlohmann::json{{"repository", + R"(json:{ + "Conditions": { "test": { "no0cond.yml": {".condition": "", "1000": "cond: {value: 42}"} } }, + ".schema.json": "{\"Conditions/test/no0cond.yml\":[\"cond\"]}" + })"}} ); + + // get a valid condition slice + auto slice = dds.get_slice( 2000 ); + REQUIRE( slice ); + + // this is a condition actually in the DB + auto cond = slice->get( det, LHCb::Detector::item_key( "cond" ) ).get(); + REQUIRE( cond.is_object() ); + CHECK( cond.contains( "value" ) ); + + // hit the problematic IOV + using Catch::Matchers::Contains; + CHECK_THROWS_WITH( dds.get_slice( 100 ), Contains( "Conditions/test/no0cond.yml" ) && Contains( "missing 0 value" ) ); +} + +#include "DD4hep/detail/Handle.inl" +DD4HEP_INSTANTIATE_HANDLE_UNNAMED( DummyDeIOVObject, LHCb::Detector::detail::DeIOVObject, ConditionObject ); -- GitLab