From 135ed6936987476952a19bf90ce1b62b88180a6c Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Fri, 21 Aug 2026 15:31:15 +0200 Subject: [PATCH] #14559 Summary Calculator: Update dependent data when the last calculation is deleted deleteCalculation() removes the calculation before rebuildCaseMetaData() is called, and updateDataDependingOnCalculations() returned early for an empty collection. The addresses created by the last calculation were then left behind in the readers until the next refresh or a reload of the project. Perform one more update after the last calculation is deleted, and keep the early return for the case where no calculations have been present. --- .../RimSummaryCalculationCollection.cpp | 5 +- .../RimSummaryCalculationCollection.h | 4 + ApplicationLibCode/UnitTests/CMakeLists.txt | 1 + .../UnitTests/RimMockSummaryCase.h | 4 + .../RimSummaryCalculationCollection-Test.cpp | 94 +++++++++++++++++++ 5 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 ApplicationLibCode/UnitTests/RimSummaryCalculationCollection-Test.cpp diff --git a/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.cpp b/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.cpp index 50e76eb289..0cf30ba7f6 100644 --- a/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.cpp +++ b/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.cpp @@ -51,7 +51,10 @@ RimSummaryCalculation* RimSummaryCalculationCollection::createCalculation() cons //-------------------------------------------------------------------------------------------------- void RimSummaryCalculationCollection::updateDataDependingOnCalculations() { - if ( calculations().empty() ) return; + // One more update is required after the last calculation is deleted, to remove the data created by that calculation. + if ( calculations().empty() && !m_hasDataFromCalculations ) return; + + m_hasDataFromCalculations = !calculations().empty(); // Refresh data sources tree // Refresh meta data for all summary cases and rebuild AddressNodes in the summary tree diff --git a/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.h b/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.h index eea4c734c5..17fea26379 100644 --- a/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.h +++ b/ApplicationLibCode/ProjectDataModel/RimSummaryCalculationCollection.h @@ -43,4 +43,8 @@ public: private: void updateDataDependingOnCalculations(); + +private: + // True if the last update was performed with calculations present. Used to detect deletion of the last calculation. + bool m_hasDataFromCalculations = false; }; diff --git a/ApplicationLibCode/UnitTests/CMakeLists.txt b/ApplicationLibCode/UnitTests/CMakeLists.txt index b539c1887e..6201244576 100644 --- a/ApplicationLibCode/UnitTests/CMakeLists.txt +++ b/ApplicationLibCode/UnitTests/CMakeLists.txt @@ -167,6 +167,7 @@ set(SOURCE_UNITTEST_FILES ${CMAKE_CURRENT_LIST_DIR}/RimMockSummaryCase-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RimSummaryCalculation-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RifMultipleSummaryReaders-Test.cpp + ${CMAKE_CURRENT_LIST_DIR}/RimSummaryCalculationCollection-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaConnectorTools-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RigWellTargetMappingTools-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RicWellPathExportMswGeometryPath-Test.cpp diff --git a/ApplicationLibCode/UnitTests/RimMockSummaryCase.h b/ApplicationLibCode/UnitTests/RimMockSummaryCase.h index 6544395f7a..353c8c7472 100644 --- a/ApplicationLibCode/UnitTests/RimMockSummaryCase.h +++ b/ApplicationLibCode/UnitTests/RimMockSummaryCase.h @@ -54,6 +54,9 @@ public: size_t keywordCount() const override { return m_allResultAddresses.size(); } + void refreshCalculatedAddresses() override { m_refreshCalculatedAddressesCount++; } + int refreshCalculatedAddressesCount() const { return m_refreshCalculatedAddressesCount; } + private: struct VectorData { @@ -64,6 +67,7 @@ private: QString m_name = "MockCase"; std::map m_data; + int m_refreshCalculatedAddressesCount = 0; }; //-------------------------------------------------------------------------------------------------- diff --git a/ApplicationLibCode/UnitTests/RimSummaryCalculationCollection-Test.cpp b/ApplicationLibCode/UnitTests/RimSummaryCalculationCollection-Test.cpp new file mode 100644 index 0000000000..6aa2a70649 --- /dev/null +++ b/ApplicationLibCode/UnitTests/RimSummaryCalculationCollection-Test.cpp @@ -0,0 +1,94 @@ +#include "gtest/gtest.h" + +#include "Summary/RiaSummaryTools.h" + +#include "RifEclipseSummaryAddress.h" + +#include "RimMockSummaryCase.h" +#include "RimProject.h" +#include "RimSummaryAddress.h" +#include "RimSummaryCalculation.h" +#include "RimSummaryCalculationCollection.h" +#include "RimSummaryCalculationVariable.h" +#include "RimSummaryCaseMainCollection.h" +#include "RimSummaryEnsemble.h" + +#include + +namespace +{ +//-------------------------------------------------------------------------------------------------- +/// Create a calculation producing a single field address +//-------------------------------------------------------------------------------------------------- +RimSummaryCalculation* createFieldCalculation() +{ + auto* calculation = dynamic_cast( RimProject::current()->calculationCollection()->addCalculation() ); + + calculation->setExpression( "MY_CALCULATION := x + 1" ); + calculation->parseExpression(); + + auto* variable = dynamic_cast( calculation->variables()->at( 0 ) ); + + RimSummaryAddress address; + address.setAddress( RifEclipseSummaryAddress::fieldAddress( "FOPT" ) ); + variable->setSummaryAddress( address ); + + return calculation; +} +} // namespace + +//-------------------------------------------------------------------------------------------------- +/// The summary case main collection is a shared global object, so every test must leave it empty to +/// keep the tests order independent. +//-------------------------------------------------------------------------------------------------- +class RimSummaryCalculationCollectionTest : public ::testing::Test +{ +protected: + RimSummaryCaseMainCollection* mainCollection() const { return RiaSummaryTools::summaryCaseMainCollection(); } + + void TearDown() override + { + for ( auto* ensemble : mainCollection()->summaryEnsembles() ) + { + mainCollection()->removeEnsemble( ensemble ); + delete ensemble; + } + } +}; + +//-------------------------------------------------------------------------------------------------- +/// The addresses of the realizations of an ensemble are not created up front, and the calculated addresses of the realizations must be +/// refreshed both when a calculation is created and when the last calculation is deleted. +/// +/// https://github.com/OPM/ResInsight/issues/14559 +//-------------------------------------------------------------------------------------------------- +TEST_F( RimSummaryCalculationCollectionTest, RefreshCalculatedAddressesForAllRealizations ) +{ + std::vector mockCases; + std::vector summaryCases; + for ( int realizationNumber = 0; realizationNumber < 3; realizationNumber++ ) + { + auto* summaryCase = createMockCase( realizationNumber ); + mockCases.push_back( dynamic_cast( summaryCase ) ); + summaryCases.push_back( summaryCase ); + } + + mainCollection()->addEnsemble( summaryCases, "Ensemble", true ); + + auto* calculation = createFieldCalculation(); + + std::vector refreshCountAfterCreate; + for ( auto* mockCase : mockCases ) + { + EXPECT_GT( mockCase->refreshCalculatedAddressesCount(), 0 ); + refreshCountAfterCreate.push_back( mockCase->refreshCalculatedAddressesCount() ); + } + + RimProject::current()->calculationCollection()->deleteCalculation( calculation ); + + // Deleting the last calculation must refresh the addresses, to discard the addresses created by the calculation + for ( size_t i = 0; i < mockCases.size(); i++ ) + { + EXPECT_GT( mockCases[i]->refreshCalculatedAddressesCount(), refreshCountAfterCreate[i] ); + } +}