From ebf75c09a1cbebf175a953e6c7192fe52d1bcb73 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Fri, 7 Aug 2026 08:34:58 +0200 Subject: [PATCH] #14411 Enforce name uniqueness for surfaces, polygons and folders * #14411 Enforce name uniqueness for surfaces, polygons and folders Names must be unique among siblings sharing the same parent folder. Items in different folders may keep identical names, and comparison is case sensitive. Siblings are the objects held by the same caf::PdmChildArrayField. Items and folders live in two distinct child arrays in caf::PdmNestedCollection, so a folder and an item may share a name under the same parent. The Python API gets an OnNameConflict flag on AddFolder, CreatePolygon, ImportSurface and NewRegularSurface, supporting FAIL (default), AUTO_RENAME and OVERWRITE. NewSurface is left out, as grid case surfaces derive their tree label from the case and K index and carry no name of their own. --- ApplicationLibCode/Application/RiaDefines.cpp | 10 + ApplicationLibCode/Application/RiaDefines.h | 9 + .../Application/Tools/CMakeLists_files.cmake | 2 + .../Tools/RiaNameUniquenessTools.cpp | 262 ++++++++++++++++++ .../Tools/RiaNameUniquenessTools.h | 101 +++++++ .../RicNewNestedCollectionFeature.cpp | 4 + .../ProjectDataModel/Polygons/RimPolygon.cpp | 6 + .../Polygons/RimPolygonCollection.cpp | 5 +- .../Polygons/RimPolygonContainer.cpp | 26 ++ .../Polygons/RimPolygonContainer.h | 7 + .../Polygons/RimPolygonFile.cpp | 8 + .../ProjectDataModel/Surfaces/RimSurface.cpp | 4 + .../Surfaces/RimSurfaceCollection.cpp | 34 +++ .../Surfaces/RimSurfaceCollection.h | 6 + .../RimcNestedCollectionBase.cpp | 26 +- .../RimcNestedCollectionBase.h | 6 +- .../RimcPolygonCollection.cpp | 23 ++ .../RimcPolygonCollection.h | 8 +- .../RimcSurfaceCollection.cpp | 46 ++- .../RimcSurfaceCollection.h | 25 +- ApplicationLibCode/UnitTests/CMakeLists.txt | 1 + .../UnitTests/RiaNameUniquenessTools-Test.cpp | 169 +++++++++++ .../cafPdmNestedCollection.h | 7 +- .../cafPdmNestedCollection.inl | 9 + .../cafPdmNestedCollectionBase.h | 6 + .../Python/rips/tests/test_polygon.py | 66 +++++ .../rips/tests/test_polygon_appearance.py | 4 +- .../Python/rips/tests/test_surfaces.py | 35 +++ 28 files changed, 889 insertions(+), 26 deletions(-) create mode 100644 ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.cpp create mode 100644 ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.h create mode 100644 ApplicationLibCode/UnitTests/RiaNameUniquenessTools-Test.cpp diff --git a/ApplicationLibCode/Application/RiaDefines.cpp b/ApplicationLibCode/Application/RiaDefines.cpp index 1c920f4a21..cb6fde2646 100644 --- a/ApplicationLibCode/Application/RiaDefines.cpp +++ b/ApplicationLibCode/Application/RiaDefines.cpp @@ -179,6 +179,15 @@ void caf::AppEnum::setUp() setDefault( RiaDefines::RowCount::ROWS_2 ); } +template <> +void caf::AppEnum::setUp() +{ + addItem( RiaDefines::NameConflictPolicy::FAIL, "FAIL", "Fail" ); + addItem( RiaDefines::NameConflictPolicy::AUTO_RENAME, "AUTO_RENAME", "Auto Rename" ); + addItem( RiaDefines::NameConflictPolicy::OVERWRITE, "OVERWRITE", "Overwrite" ); + setDefault( RiaDefines::NameConflictPolicy::FAIL ); +} + } // namespace caf namespace @@ -192,6 +201,7 @@ struct RegisterScriptEnumNames { caf::PdmScriptEnumNameRegistry::registerName( "PropertyType" ); caf::PdmScriptEnumNameRegistry::registerName( "PorosityModelType" ); + caf::PdmScriptEnumNameRegistry::registerName( "NameConflictPolicy" ); } }; const RegisterScriptEnumNames s_registerScriptEnumNames; diff --git a/ApplicationLibCode/Application/RiaDefines.h b/ApplicationLibCode/Application/RiaDefines.h index 68025e5697..f02ec1827f 100644 --- a/ApplicationLibCode/Application/RiaDefines.h +++ b/ApplicationLibCode/Application/RiaDefines.h @@ -211,6 +211,15 @@ enum class WellProductionType : short bool isInjector( WellProductionType wellProductionType ); +// How to handle a new item colliding with the name of an existing item in the same folder. +// Used by the Python/gRPC API, where no user is present to resolve the conflict interactively. +enum class NameConflictPolicy +{ + FAIL, // Abort the operation and report an error + AUTO_RENAME, // Append a numeric suffix to make the name unique + OVERWRITE // Delete the existing item carrying the name +}; + QString stringListSeparator(); enum class ColumnCount diff --git a/ApplicationLibCode/Application/Tools/CMakeLists_files.cmake b/ApplicationLibCode/Application/Tools/CMakeLists_files.cmake index 2dcd098a07..22d3c698e3 100644 --- a/ApplicationLibCode/Application/Tools/CMakeLists_files.cmake +++ b/ApplicationLibCode/Application/Tools/CMakeLists_files.cmake @@ -36,6 +36,7 @@ set(SOURCE_GROUP_HEADER_FILES ${CMAKE_CURRENT_LIST_DIR}/RiaWeightedMeanCalculator.inl ${CMAKE_CURRENT_LIST_DIR}/RiaWeightedGeometricMeanCalculator.h ${CMAKE_CURRENT_LIST_DIR}/RiaWeightedHarmonicMeanCalculator.h + ${CMAKE_CURRENT_LIST_DIR}/RiaNameUniquenessTools.h ${CMAKE_CURRENT_LIST_DIR}/RiaOptionItemFactory.h ${CMAKE_CURRENT_LIST_DIR}/RiaGitDiff.h ${CMAKE_CURRENT_LIST_DIR}/RiaWslTools.h @@ -94,6 +95,7 @@ set(SOURCE_GROUP_SOURCE_FILES ${CMAKE_CURRENT_LIST_DIR}/RiaTimeHistoryCurveResampler.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaWeightedGeometricMeanCalculator.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaWeightedHarmonicMeanCalculator.cpp + ${CMAKE_CURRENT_LIST_DIR}/RiaNameUniquenessTools.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaOptionItemFactory.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaGitDiff.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaWslTools.cpp diff --git a/ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.cpp b/ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.cpp new file mode 100644 index 0000000000..e669356a49 --- /dev/null +++ b/ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.cpp @@ -0,0 +1,262 @@ +///////////////////////////////////////////////////////////////////////////////// +// +// Copyright (C) 2026 Equinor ASA +// +// ResInsight is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ResInsight is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. +// +// See the GNU General Public License at +// for more details. +// +///////////////////////////////////////////////////////////////////////////////// + +#include "RiaNameUniquenessTools.h" + +#include "RiaLogging.h" + +#include "RimNamedObject.h" +#include "RimSurface.h" + +#include "cafPdmChildArrayField.h" +#include "cafPdmNestedCollectionBase.h" +#include "cafPdmObjectHandle.h" +#include "cafPdmUiObjectHandle.h" +#include "cafPdmValueField.h" + +#include + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RiaNameUniquenessTools::objectName( const caf::PdmObjectHandle* object ) +{ + if ( !object ) return {}; + + auto* mutableObject = const_cast( object ); + + if ( auto* surface = dynamic_cast( mutableObject ) ) return surface->userDescription(); + if ( auto* namedObject = dynamic_cast( mutableObject ) ) return namedObject->name(); + if ( auto* collection = dynamic_cast( mutableObject ) ) return collection->collectionName(); + + if ( auto* uiObjectHandle = caf::uiObj( object ) ) + { + if ( auto* valueField = dynamic_cast( uiObjectHandle->userDescriptionField() ) ) + { + return valueField->toQVariant().toString(); + } + } + + return {}; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RiaNameUniquenessTools::setObjectName( caf::PdmObjectHandle* object, const QString& name ) +{ + if ( !object ) return; + + if ( auto* surface = dynamic_cast( object ) ) + { + surface->setUserDescription( name ); + return; + } + + if ( auto* namedObject = dynamic_cast( object ) ) + { + namedObject->setName( name ); + return; + } + + if ( auto* collection = dynamic_cast( object ) ) + { + collection->setCollectionName( name ); + return; + } + + if ( auto* uiObjectHandle = caf::uiObj( object ) ) + { + if ( auto* valueField = dynamic_cast( uiObjectHandle->userDescriptionField() ) ) + { + valueField->setFromQVariant( name ); + } + } +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +std::set RiaNameUniquenessTools::namesInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, + const caf::PdmObjectHandle* objectToExclude ) +{ + std::set names; + if ( !childArrayField ) return names; + + auto* mutableField = const_cast( childArrayField ); + for ( size_t i = 0; i < mutableField->size(); i++ ) + { + auto* child = mutableField->at( i ); + if ( !child || child == objectToExclude ) continue; + + names.insert( objectName( child ) ); + } + + return names; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +std::set RiaNameUniquenessTools::siblingNames( const caf::PdmObjectHandle* object ) +{ + if ( !object ) return {}; + + auto* childArrayField = dynamic_cast( object->parentField() ); + if ( !childArrayField ) return {}; + + return namesInCollection( childArrayField, object ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RiaNameUniquenessTools::makeUnique( const QString& candidateName, const std::set& takenNames ) +{ + // An empty name means the object has no name of its own, and derives its tree label from + // other properties (a grid case surface is labelled from its case and K index). Numbering + // those would only produce labels like "_1 - K:3". + if ( candidateName.isEmpty() ) return candidateName; + + if ( !takenNames.contains( candidateName ) ) return candidateName; + + int counter = 1; + QString uniqueName; + do + { + uniqueName = QString( "%1_%2" ).arg( candidateName ).arg( counter++ ); + } while ( takenNames.contains( uniqueName ) ); + + return uniqueName; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +bool RiaNameUniquenessTools::isUniqueAmongSiblings( const caf::PdmObjectHandle* object, const QString& candidateName ) +{ + if ( candidateName.isEmpty() ) return true; + + return !siblingNames( object ).contains( candidateName ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RiaNameUniquenessTools::makeUniqueAmongSiblings( const caf::PdmObjectHandle* object, const QString& candidateName ) +{ + return makeUnique( candidateName, siblingNames( object ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RiaNameUniquenessTools::ensureUniqueAmongSiblings( caf::PdmObjectHandle* object ) +{ + const QString currentName = objectName( object ); + const QString uniqueName = makeUniqueAmongSiblings( object, currentName ); + + if ( uniqueName != currentName ) setObjectName( object, uniqueName ); + + return uniqueName; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RiaNameUniquenessTools::resolveRenameConflict( const caf::PdmObjectHandle* object, const QString& desiredName ) +{ + const QString uniqueName = makeUniqueAmongSiblings( object, desiredName ); + + if ( uniqueName != desiredName ) + { + RiaLogging::info( + QString( "\"%1\" already exists in this folder, using \"%2\" instead." ).arg( desiredName ).arg( uniqueName ).toStdString() ); + } + + return uniqueName; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +bool RiaNameUniquenessTools::isUniqueInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, const QString& candidateName ) +{ + if ( candidateName.isEmpty() ) return true; + + return !namesInCollection( childArrayField ).contains( candidateName ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RiaNameUniquenessTools::makeUniqueInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, const QString& candidateName ) +{ + return makeUnique( candidateName, namesInCollection( childArrayField ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +caf::PdmObjectHandle* RiaNameUniquenessTools::findInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, const QString& name ) +{ + if ( !childArrayField ) return nullptr; + + auto* mutableField = const_cast( childArrayField ); + for ( size_t i = 0; i < mutableField->size(); i++ ) + { + auto* child = mutableField->at( i ); + if ( child && objectName( child ) == name ) return child; + } + + return nullptr; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +RiaNameUniquenessTools::ConflictResolution RiaNameUniquenessTools::applyConflictPolicy( const caf::PdmChildArrayFieldHandle* childArrayField, + const QString& candidateName, + RiaDefines::NameConflictPolicy policy ) +{ + ConflictResolution resolution; + resolution.nameToUse = candidateName; + + if ( candidateName.isEmpty() ) return resolution; + + auto* existingObject = findInCollection( childArrayField, candidateName ); + if ( !existingObject ) return resolution; + + switch ( policy ) + { + case RiaDefines::NameConflictPolicy::AUTO_RENAME: + resolution.nameToUse = makeUniqueInCollection( childArrayField, candidateName ); + break; + + case RiaDefines::NameConflictPolicy::OVERWRITE: + resolution.objectToReplace = existingObject; + break; + + case RiaDefines::NameConflictPolicy::FAIL: + default: + resolution.errorMessage = QString( "An item named '%1' already exists in this folder" ).arg( candidateName ); + break; + } + + return resolution; +} diff --git a/ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.h b/ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.h new file mode 100644 index 0000000000..d7e065e47e --- /dev/null +++ b/ApplicationLibCode/Application/Tools/RiaNameUniquenessTools.h @@ -0,0 +1,101 @@ +///////////////////////////////////////////////////////////////////////////////// +// +// Copyright (C) 2026 Equinor ASA +// +// ResInsight is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ResInsight is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. +// +// See the GNU General Public License at +// for more details. +// +///////////////////////////////////////////////////////////////////////////////// + +#pragma once + +#include "RiaDefines.h" + +#include + +#include + +namespace caf +{ +class PdmChildArrayFieldHandle; +class PdmObjectHandle; +} // namespace caf + +//================================================================================================== +/// +/// Helpers used to keep names unique among siblings in the project tree. +/// +/// Siblings are the objects held by the same caf::PdmChildArrayField. Items and folders live in +/// two distinct child arrays (m_items and m_subCollections in caf::PdmNestedCollection), so a +/// folder and an item may carry the same name under the same parent. Names in different folders +/// never conflict. +/// +/// All comparison is case sensitive. +/// +//================================================================================================== +namespace RiaNameUniquenessTools +{ +// Name of an object as shown in the project tree. Knows about the name field of surfaces, +// named objects and nested collections, and falls back to the user description field. +QString objectName( const caf::PdmObjectHandle* object ); +void setObjectName( caf::PdmObjectHandle* object, const QString& name ); + +// Names of the objects in a child array field, optionally excluding one object. +std::set namesInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, + const caf::PdmObjectHandle* objectToExclude = nullptr ); + +// Names used by the other objects in the child array field holding this object. +std::set siblingNames( const caf::PdmObjectHandle* object ); + +// Appends _1, _2, ... to candidateName until the result is not present in takenNames. +QString makeUnique( const QString& candidateName, const std::set& takenNames ); + +bool isUniqueAmongSiblings( const caf::PdmObjectHandle* object, const QString& candidateName ); +QString makeUniqueAmongSiblings( const caf::PdmObjectHandle* object, const QString& candidateName ); + +// Renames the object if one of its siblings already carries the same name. The object must +// already be inserted into its parent collection. Returns the name in effect after the call. +QString ensureUniqueAmongSiblings( caf::PdmObjectHandle* object ); + +// Returns the name to apply when the user renames an object. If desiredName is already unique +// among the siblings of the object, it is returned unchanged. Otherwise an auto-generated unique +// name is returned, and the substitution is written to the log. +QString resolveRenameConflict( const caf::PdmObjectHandle* object, const QString& desiredName ); + +// For objects not yet inserted into a parent collection. +bool isUniqueInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, const QString& candidateName ); +QString makeUniqueInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, const QString& candidateName ); + +// The object in the child array field carrying this exact name, or nullptr. +caf::PdmObjectHandle* findInCollection( const caf::PdmChildArrayFieldHandle* childArrayField, const QString& name ); + +//================================================================================================== +/// Outcome of applying a name conflict policy. Exactly one of the three states applies: +/// - errorMessage set : the policy is FAIL and the name is taken, abort the operation +/// - objectToReplace set : the policy is OVERWRITE, the caller must delete this object +/// - neither set : use nameToUse, which is unique +//================================================================================================== +struct ConflictResolution +{ + QString nameToUse; + caf::PdmObjectHandle* objectToReplace = nullptr; + QString errorMessage; +}; + +// Resolves a name conflict for a name about to be used in childArrayField. Deleting the object +// to replace is left to the caller, as the removal must go through the owning collection to keep +// views and referring objects in sync. +ConflictResolution applyConflictPolicy( const caf::PdmChildArrayFieldHandle* childArrayField, + const QString& candidateName, + RiaDefines::NameConflictPolicy policy ); + +}; // namespace RiaNameUniquenessTools diff --git a/ApplicationLibCode/Commands/RicNewNestedCollectionFeature.cpp b/ApplicationLibCode/Commands/RicNewNestedCollectionFeature.cpp index 9937168407..48d1e0cc17 100644 --- a/ApplicationLibCode/Commands/RicNewNestedCollectionFeature.cpp +++ b/ApplicationLibCode/Commands/RicNewNestedCollectionFeature.cpp @@ -18,6 +18,8 @@ #include "RicNewNestedCollectionFeature.h" +#include "RiaNameUniquenessTools.h" + #include "Riu3DMainWindowTools.h" #include "cafPdmNestedCollectionBase.h" @@ -64,6 +66,8 @@ void RicNewNestedCollectionFeature::onActionTriggered( bool isChecked ) caf::PdmObject* added = parent->addNewSubCollection(); if ( !added ) return; + RiaNameUniquenessTools::ensureUniqueAmongSiblings( added ); + Riu3DMainWindowTools::selectAsCurrentItem( added ); } diff --git a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygon.cpp b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygon.cpp index e323350fe5..99700b602d 100644 --- a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygon.cpp +++ b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygon.cpp @@ -20,6 +20,7 @@ #include "RiaApplication.h" #include "RiaColorTools.h" +#include "RiaNameUniquenessTools.h" #include "RiaVec3Tools.h" #include "RigPolyLinesData.h" @@ -223,6 +224,11 @@ void RimPolygon::fieldChangedByUi( const caf::PdmFieldHandle* changedField, cons { coordinatesChanged.send(); } + else if ( changedField == nameField() ) + { + // Keep the name unique among the polygons in the same folder + setName( RiaNameUniquenessTools::resolveRenameConflict( this, newValue.toString() ) ); + } objectChanged.send(); } diff --git a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonCollection.cpp b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonCollection.cpp index f8473d321c..e9d05813e3 100644 --- a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonCollection.cpp +++ b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonCollection.cpp @@ -19,6 +19,7 @@ #include "RimPolygonCollection.h" #include "RiaColorTables.h" +#include "RiaNameUniquenessTools.h" #include "Rim3dView.h" #include "RimPolygon.h" @@ -69,7 +70,7 @@ RimPolygonCollection* RimPolygonCollection::createTopmost() RimPolygon* RimPolygonCollection::createUserDefinedPolygon() { auto newPolygon = new RimPolygon(); - newPolygon->setName( "Polygon " + QString::number( allPolygons().size() + 1 ) ); + newPolygon->setName( "Polygon " + QString::number( items().size() + 1 ) ); auto colorCandidates = RiaColorTables::summaryCurveDefaultPaletteColors(); newPolygon->setColor( colorCandidates.cycledColor3f( allPolygons().size() ) ); @@ -94,6 +95,7 @@ RimPolygon* RimPolygonCollection::appendUserDefinedPolygon() void RimPolygonCollection::addUserDefinedPolygon( RimPolygon* polygon ) { m_items.push_back( polygon ); + ensureUniquePolygonName( polygon ); connectPolygonSignals( polygon ); @@ -140,6 +142,7 @@ void RimPolygonCollection::addPolygonFile( RimPolygonFile* polygonFile ) if ( !polygonFile ) return; addSubCollection( polygonFile ); + RiaNameUniquenessTools::ensureUniqueAmongSiblings( polygonFile ); connectPolygonFileSignals( polygonFile ); updateViewTreeItems(); diff --git a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.cpp b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.cpp index e314bb9347..ea58ec6eb2 100644 --- a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.cpp +++ b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.cpp @@ -18,6 +18,8 @@ #include "RimPolygonContainer.h" +#include "RiaNameUniquenessTools.h" + #include "RimPolygon.h" #include "RimPolygonCollection.h" @@ -44,6 +46,20 @@ RimPolygonContainer* RimPolygonContainer::addNewSubCollection() return sub; } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RimPolygonContainer::fieldChangedByUi( const caf::PdmFieldHandle* changedField, const QVariant& oldValue, const QVariant& newValue ) +{ + if ( changedField == &m_collectionName ) + { + // Keep the name unique among the folders in the same parent folder + m_collectionName = RiaNameUniquenessTools::resolveRenameConflict( this, newValue.toString() ); + } + + caf::PdmNestedCollection::fieldChangedByUi( changedField, oldValue, newValue ); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -54,3 +70,13 @@ void RimPolygonContainer::loadData() if ( sub ) sub->loadData(); } } + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RimPolygonContainer::ensureUniquePolygonName( RimPolygon* polygon ) +{ + if ( !polygon ) return; + + RiaNameUniquenessTools::ensureUniqueAmongSiblings( polygon ); +} diff --git a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.h b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.h index d5b03dfa1f..e7cc16f3de 100644 --- a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.h +++ b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonContainer.h @@ -48,4 +48,11 @@ public: // Default behavior recurses into sub-collections. Leaf containers (e.g., file-backed) // override to load their own data; folder containers inherit the recursion. virtual void loadData(); + + // Renames the polygon if another polygon in this container already carries the same name. + void ensureUniquePolygonName( RimPolygon* polygon ); + +protected: + // Enforces name uniqueness among sibling folders when the folder is renamed. + void fieldChangedByUi( const caf::PdmFieldHandle* changedField, const QVariant& oldValue, const QVariant& newValue ) override; }; diff --git a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonFile.cpp b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonFile.cpp index 808e5fba7c..52abfe0ae8 100644 --- a/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonFile.cpp +++ b/ApplicationLibCode/ProjectDataModel/Polygons/RimPolygonFile.cpp @@ -98,6 +98,12 @@ void RimPolygonFile::loadData() m_items.deleteChildren(); m_items.setValue( polygonsFromFile ); + + // A file may contain several polygons sharing the same id, and thus the same generated name + for ( auto* polygon : polygonsFromFile ) + { + ensureUniquePolygonName( polygon ); + } } if ( polygonsFromFile.empty() ) @@ -179,6 +185,8 @@ void RimPolygonFile::fieldChangedByUi( const caf::PdmFieldHandle* changedField, loadData(); } + RimPolygonContainer::fieldChangedByUi( changedField, oldValue, newValue ); + objectChanged.send(); } diff --git a/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurface.cpp b/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurface.cpp index 15736b61fb..0ffc349d24 100644 --- a/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurface.cpp +++ b/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurface.cpp @@ -19,6 +19,7 @@ #include "RimSurface.h" #include "RiaApplication.h" +#include "RiaNameUniquenessTools.h" #include "RimCase.h" #include "RimProject.h" @@ -252,6 +253,9 @@ void RimSurface::fieldChangedByUi( const caf::PdmFieldHandle* changedField, cons } else if ( changedField == &m_userDescription ) { + // Keep the name unique among the surfaces in the same folder + m_userDescription = RiaNameUniquenessTools::resolveRenameConflict( this, newValue.toString() ); + updateConnectedEditors(); } else if ( changedField == &m_depthOffset ) diff --git a/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.cpp b/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.cpp index 4bdf2c972d..1cab31ddff 100644 --- a/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.cpp +++ b/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.cpp @@ -20,6 +20,7 @@ #include "RiaColorTables.h" #include "RiaLogging.h" +#include "RiaNameUniquenessTools.h" #include "Rim2dIntersectionView.h" #include "Rim2dIntersectionViewCollection.h" @@ -92,6 +93,31 @@ RimSurfaceCollection* RimSurfaceCollection::createTopmost() void RimSurfaceCollection::addSurface( RimSurface* surface ) { m_items.push_back( surface ); + ensureUniqueSurfaceName( surface ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RimSurfaceCollection::ensureUniqueSurfaceName( RimSurface* surface ) +{ + if ( !surface ) return; + + RiaNameUniquenessTools::ensureUniqueAmongSiblings( surface ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RimSurfaceCollection::fieldChangedByUi( const caf::PdmFieldHandle* changedField, const QVariant& oldValue, const QVariant& newValue ) +{ + if ( changedField == &m_collectionName ) + { + // Keep the name unique among the folders in the same parent folder + m_collectionName = RiaNameUniquenessTools::resolveRenameConflict( this, newValue.toString() ); + } + + caf::PdmNestedCollection::fieldChangedByUi( changedField, oldValue, newValue ); } //-------------------------------------------------------------------------------------------------- @@ -234,6 +260,7 @@ RimSurface* RimSurfaceCollection::copySurfaces( std::vector surface for ( RimSurface* surface : newsurfaces ) { m_items.push_back( surface ); + ensureUniqueSurfaceName( surface ); retsurf = surface; } @@ -259,6 +286,7 @@ RimSurface* RimSurfaceCollection::addGridCaseSurface( RimCase* sourceCase, int o } m_items.push_back( s ); + ensureUniqueSurfaceName( s ); updateConnectedEditors(); @@ -481,6 +509,12 @@ RimSurface* RimSurfaceCollection::addSurfacesAtIndex( int position, std::vector< } } + // Surfaces moved in from another folder may collide with the names already present here + for ( auto surf : surfaces ) + { + ensureUniqueSurfaceName( surf ); + } + // make sure the views are in sync with the collection order updateViews(); diff --git a/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.h b/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.h index 7e4ab779d6..fa718460f6 100644 --- a/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.h +++ b/ApplicationLibCode/ProjectDataModel/Surfaces/RimSurfaceCollection.h @@ -64,9 +64,15 @@ public: std::vector surfaces() const; + // Renames the surface if another surface in this folder already carries the same name. + void ensureUniqueSurfaceName( RimSurface* surface ); + protected: void appendMenuItems( caf::CmdFeatureMenuBuilder& menuBuilder ) const override; + // Enforces name uniqueness among sibling folders when the folder is renamed. + void fieldChangedByUi( const caf::PdmFieldHandle* changedField, const QVariant& oldValue, const QVariant& newValue ) override; + private: void orderChanged( const caf::SignalEmitter* emitter ); }; diff --git a/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.cpp b/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.cpp index 71fea0adb5..1d5b27f3d6 100644 --- a/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.cpp +++ b/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.cpp @@ -18,6 +18,9 @@ #include "RimcNestedCollectionBase.h" +#include "RiaNameUniquenessTools.h" + +#include "cafPdmChildArrayField.h" #include "cafPdmFieldScriptingCapability.h" #include "cafPdmNestedCollectionBase.h" @@ -32,6 +35,13 @@ RimcNestedCollectionBase::RimcNestedCollectionBase( caf::PdmObjectHandle* self ) CAF_PDM_InitObject( "Add Folder", "", "", "Add a new folder" ); CAF_PDM_InitScriptableField( &m_folderName, "FolderName", QString( "Folder" ), "", "", "", "New folder name" ); + CAF_PDM_InitScriptableField( &m_onNameConflict, + "OnNameConflict", + caf::AppEnum( RiaDefines::NameConflictPolicy::FAIL ), + "", + "", + "", + "How to handle a folder name already used in this folder" ); } //-------------------------------------------------------------------------------------------------- @@ -45,6 +55,20 @@ std::expected RimcNestedCollectionBase::execute( return std::unexpected( "Cannot add subfolder" ); } + auto* subCollections = coll->subCollectionsField(); + auto resolution = RiaNameUniquenessTools::applyConflictPolicy( subCollections, m_folderName(), m_onNameConflict().value() ); + if ( !resolution.errorMessage.isEmpty() ) return std::unexpected( resolution.errorMessage ); + + if ( resolution.objectToReplace && subCollections ) + { + size_t index = subCollections->indexOf( resolution.objectToReplace ); + if ( index < subCollections->size() ) + { + subCollections->erase( index ); + delete resolution.objectToReplace; + } + } + caf::PdmObject* added = coll->addNewSubCollection(); if ( !added ) { @@ -53,7 +77,7 @@ std::expected RimcNestedCollectionBase::execute( if ( auto* asNested = dynamic_cast( added ) ) { - asNested->setCollectionName( m_folderName() ); + asNested->setCollectionName( resolution.nameToUse ); } coll->uiCapability()->updateConnectedEditors(); return added; diff --git a/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.h b/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.h index 5482e4cc38..b8ab6e992c 100644 --- a/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.h +++ b/ApplicationLibCode/ProjectDataModelCommands/RimcNestedCollectionBase.h @@ -18,6 +18,9 @@ #pragma once +#include "RiaDefines.h" + +#include "cafAppEnum.h" #include "cafPdmField.h" #include "cafPdmObjectHandle.h" #include "cafPdmObjectMethod.h" @@ -42,5 +45,6 @@ public: QString classKeywordReturnedType() const override; private: - caf::PdmField m_folderName; + caf::PdmField m_folderName; + caf::PdmField> m_onNameConflict; }; diff --git a/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.cpp b/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.cpp index 8bf406ec46..2241714665 100644 --- a/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.cpp +++ b/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.cpp @@ -18,6 +18,8 @@ #include "RimcPolygonCollection.h" +#include "RiaNameUniquenessTools.h" + #include "Polygons/RimPolygon.h" #include "Polygons/RimPolygonCollection.h" @@ -37,6 +39,13 @@ RimcPolygonCollection_createPolygon::RimcPolygonCollection_createPolygon( caf::P CAF_PDM_InitScriptableFieldNoDefault( &m_name, "Name", "Name" ); CAF_PDM_InitScriptableFieldNoDefault( &m_coordinates, "Coordinates", "Coordinates" ); + CAF_PDM_InitScriptableField( &m_onNameConflict, + "OnNameConflict", + caf::AppEnum( RiaDefines::NameConflictPolicy::FAIL ), + "", + "", + "", + "How to handle a polygon name already used in this folder" ); } //-------------------------------------------------------------------------------------------------- @@ -49,6 +58,20 @@ std::expected RimcPolygonCollection_createPolygo QString name = m_name(); std::vector coords = m_coordinates(); + if ( !name.isEmpty() ) + { + auto resolution = RiaNameUniquenessTools::applyConflictPolicy( &polygonCollection->itemsField(), name, m_onNameConflict().value() ); + if ( !resolution.errorMessage.isEmpty() ) return std::unexpected( resolution.errorMessage ); + + // The new polygon is added below, and that refreshes the views for both objects + if ( auto* existingPolygon = dynamic_cast( resolution.objectToReplace ) ) + { + polygonCollection->deleteItem( existingPolygon ); + } + + name = resolution.nameToUse; + } + RimPolygon* polygon = new RimPolygon(); if ( !name.isEmpty() ) { diff --git a/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.h b/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.h index 19b63178b8..5a0e999f67 100644 --- a/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.h +++ b/ApplicationLibCode/ProjectDataModelCommands/RimcPolygonCollection.h @@ -18,6 +18,9 @@ #pragma once +#include "RiaDefines.h" + +#include "cafAppEnum.h" #include "cafPdmField.h" #include "cafPdmObjectHandle.h" #include "cafPdmObjectMethod.h" @@ -40,6 +43,7 @@ public: QString classKeywordReturnedType() const override; private: - caf::PdmField m_name; - caf::PdmField> m_coordinates; + caf::PdmField m_name; + caf::PdmField> m_coordinates; + caf::PdmField> m_onNameConflict; }; diff --git a/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.cpp b/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.cpp index 6e8a3c5bba..e148be878a 100644 --- a/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.cpp +++ b/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.cpp @@ -19,6 +19,8 @@ #include "SurfaceCommands/RicImportSurfacesFeature.h" +#include "RiaNameUniquenessTools.h" + #include "RimCase.h" #include "RimFileSurface.h" #include "RimGridCaseSurface.h" @@ -29,6 +31,7 @@ #include "cafPdmFieldScriptingCapability.h" #include "cafPdmObjectScriptingCapability.h" +#include #include CAF_PDM_OBJECT_METHOD_SOURCE_INIT( RimSurfaceCollection, RimcSurfaceCollection_importSurface, "ImportSurface" ); @@ -45,6 +48,13 @@ RimcSurfaceCollection_importSurface::RimcSurfaceCollection_importSurface( caf::P CAF_PDM_InitObject( "Import Surface", "", "", "Import a new surface from file" ); CAF_PDM_InitScriptableFieldNoDefault( &m_fileName, "FileName", "", "", "", "Filename to import surface from" ); + CAF_PDM_InitScriptableField( &m_onNameConflict, + "OnNameConflict", + caf::AppEnum( RiaDefines::NameConflictPolicy::FAIL ), + "", + "", + "", + "How to handle a surface name already used in this folder" ); } //-------------------------------------------------------------------------------------------------- @@ -53,13 +63,22 @@ RimcSurfaceCollection_importSurface::RimcSurfaceCollection_importSurface( caf::P std::expected RimcSurfaceCollection_importSurface::execute() { RimSurfaceCollection* coll = self(); - if ( coll ) + if ( !coll ) return nullptr; + + // Imported surfaces are named after the file, see RimSurfaceCollection::importSurfacesFromFiles + const QString surfaceName = QFileInfo( m_fileName() ).fileName(); + + auto resolution = RiaNameUniquenessTools::applyConflictPolicy( &coll->itemsField(), surfaceName, m_onNameConflict().value() ); + if ( !resolution.errorMessage.isEmpty() ) return std::unexpected( resolution.errorMessage ); + + if ( auto* existingSurface = dynamic_cast( resolution.objectToReplace ) ) { - QStringList filelist; - filelist << m_fileName(); - return coll->importSurfacesFromFiles( filelist ); + coll->deleteItem( existingSurface ); } - return nullptr; + + QStringList filelist; + filelist << m_fileName(); + return coll->importSurfacesFromFiles( filelist ); } //-------------------------------------------------------------------------------------------------- @@ -113,6 +132,13 @@ RimcSurfaceCollection_newRegularSurface::RimcSurfaceCollection_newRegularSurface CAF_PDM_InitObject( "New Regular Surface", "", "", "Create a new regular surface" ); CAF_PDM_InitScriptableField( &m_name, "Name", QString( "" ), "Name" ); + CAF_PDM_InitScriptableField( &m_onNameConflict, + "OnNameConflict", + caf::AppEnum( RiaDefines::NameConflictPolicy::FAIL ), + "", + "", + "", + "How to handle a surface name already used in this folder" ); CAF_PDM_InitScriptableField( &m_originX, "OriginX", 0.0, "Origin X" ); CAF_PDM_InitScriptableField( &m_originY, "OriginY", 0.0, "Origin Y" ); @@ -140,8 +166,16 @@ std::expected RimcSurfaceCollection_newRegularSu if ( m_incrementY() <= 0.0 ) return std::unexpected( "Invalid increment Y. Must be positive." ); if ( m_rotation() < 0.0 || m_rotation() > 360.0 ) return std::unexpected( "Invalid rotation. Valid range: [0.0-360.0]" ); + auto resolution = RiaNameUniquenessTools::applyConflictPolicy( &coll->itemsField(), m_name(), m_onNameConflict().value() ); + if ( !resolution.errorMessage.isEmpty() ) return std::unexpected( resolution.errorMessage ); + + if ( auto* existingSurface = dynamic_cast( resolution.objectToReplace ) ) + { + coll->deleteItem( existingSurface ); + } + RimRegularSurface* surface = new RimRegularSurface; - surface->setUserDescription( m_name() ); + surface->setUserDescription( resolution.nameToUse ); surface->setNx( m_nx() ); surface->setNy( m_ny() ); diff --git a/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.h b/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.h index 510688d51c..2254f5c72e 100644 --- a/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.h +++ b/ApplicationLibCode/ProjectDataModelCommands/RimcSurfaceCollection.h @@ -20,6 +20,9 @@ #include "RimSurfaceCollection.h" +#include "RiaDefines.h" + +#include "cafAppEnum.h" #include "cafPdmField.h" #include "cafPdmObjectHandle.h" #include "cafPdmObjectMethod.h" @@ -47,7 +50,8 @@ public: QString classKeywordReturnedType() const override; private: - caf::PdmField m_fileName; + caf::PdmField m_fileName; + caf::PdmField> m_onNameConflict; }; //================================================================================================== @@ -82,13 +86,14 @@ public: QString classKeywordReturnedType() const override; private: - caf::PdmField m_name; - caf::PdmField m_nx; - caf::PdmField m_ny; - caf::PdmField m_originX; - caf::PdmField m_originY; - caf::PdmField m_depth; - caf::PdmField m_incrementX; - caf::PdmField m_incrementY; - caf::PdmField m_rotation; + caf::PdmField m_name; + caf::PdmField> m_onNameConflict; + caf::PdmField m_nx; + caf::PdmField m_ny; + caf::PdmField m_originX; + caf::PdmField m_originY; + caf::PdmField m_depth; + caf::PdmField m_incrementX; + caf::PdmField m_incrementY; + caf::PdmField m_rotation; }; diff --git a/ApplicationLibCode/UnitTests/CMakeLists.txt b/ApplicationLibCode/UnitTests/CMakeLists.txt index bf10a93ed6..da26f26318 100644 --- a/ApplicationLibCode/UnitTests/CMakeLists.txt +++ b/ApplicationLibCode/UnitTests/CMakeLists.txt @@ -135,6 +135,7 @@ set(SOURCE_UNITTEST_FILES ${CMAKE_CURRENT_LIST_DIR}/RigVfpTables-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RiaResultName-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RigPolygonTools-Test.cpp + ${CMAKE_CURRENT_LIST_DIR}/RiaNameUniquenessTools-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RifVtkSurfaceImporter-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RifVtkReader-Test.cpp ${CMAKE_CURRENT_LIST_DIR}/RifVtkImportUtil-Test.cpp diff --git a/ApplicationLibCode/UnitTests/RiaNameUniquenessTools-Test.cpp b/ApplicationLibCode/UnitTests/RiaNameUniquenessTools-Test.cpp new file mode 100644 index 0000000000..e8f1475bc3 --- /dev/null +++ b/ApplicationLibCode/UnitTests/RiaNameUniquenessTools-Test.cpp @@ -0,0 +1,169 @@ +///////////////////////////////////////////////////////////////////////////////// +// +// Copyright (C) 2026 Equinor ASA +// +// ResInsight is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// ResInsight is distributed in the hope that it will be useful, but WITHOUT ANY +// WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. +// +// See the GNU General Public License at +// for more details. +// +///////////////////////////////////////////////////////////////////////////////// + +#include "gtest/gtest.h" + +#include "RiaNameUniquenessTools.h" + +#include "Polygons/RimPolygon.h" +#include "Polygons/RimPolygonCollection.h" +#include "RimRegularSurface.h" +#include "RimSurfaceCollection.h" + +#include + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, MakeUniqueAppendsNumericSuffix ) +{ + EXPECT_EQ( QString( "Top" ), RiaNameUniquenessTools::makeUnique( "Top", {} ) ); + EXPECT_EQ( QString( "Top_1" ), RiaNameUniquenessTools::makeUnique( "Top", { "Top" } ) ); + EXPECT_EQ( QString( "Top_2" ), RiaNameUniquenessTools::makeUnique( "Top", { "Top", "Top_1" } ) ); + EXPECT_EQ( QString( "Top_3" ), RiaNameUniquenessTools::makeUnique( "Top", { "Top", "Top_1", "Top_2" } ) ); + + // A gap in the numbering is filled + EXPECT_EQ( QString( "Top_1" ), RiaNameUniquenessTools::makeUnique( "Top", { "Top", "Top_2" } ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, MakeUniqueIsCaseSensitive ) +{ + EXPECT_EQ( QString( "fault" ), RiaNameUniquenessTools::makeUnique( "fault", { "Fault" } ) ); + EXPECT_EQ( QString( "Fault_1" ), RiaNameUniquenessTools::makeUnique( "Fault", { "Fault", "fault" } ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// An empty name means the object derives its tree label from other properties, and must not be +/// numbered. +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, MakeUniqueLeavesEmptyNameUntouched ) +{ + EXPECT_EQ( QString( "" ), RiaNameUniquenessTools::makeUnique( "", { "", "Top" } ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, SurfacesAreUniquePerFolder ) +{ + auto rootCollection = std::make_unique(); + + auto* firstSurface = new RimRegularSurface(); + firstSurface->setUserDescription( "Top" ); + rootCollection->addSurface( firstSurface ); + + auto* secondSurface = new RimRegularSurface(); + secondSurface->setUserDescription( "Top" ); + rootCollection->addSurface( secondSurface ); + + EXPECT_EQ( QString( "Top" ), firstSurface->userDescription() ); + EXPECT_EQ( QString( "Top_1" ), secondSurface->userDescription() ); + + // The same name is free again in another folder + auto* subFolder = new RimSurfaceCollection(); + rootCollection->addSubCollection( subFolder ); + + auto* surfaceInSubFolder = new RimRegularSurface(); + surfaceInSubFolder->setUserDescription( "Top" ); + subFolder->addSurface( surfaceInSubFolder ); + + EXPECT_EQ( QString( "Top" ), surfaceInSubFolder->userDescription() ); +} + +//-------------------------------------------------------------------------------------------------- +/// Folders and items are two separate namespaces, a folder and a surface may share a name. +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, FoldersAndItemsAreSeparateNamespaces ) +{ + auto rootCollection = std::make_unique(); + + auto* surface = new RimRegularSurface(); + surface->setUserDescription( "Top" ); + rootCollection->addSurface( surface ); + + auto* subFolder = new RimSurfaceCollection(); + subFolder->setCollectionName( "Top" ); + rootCollection->addSubCollection( subFolder ); + RiaNameUniquenessTools::ensureUniqueAmongSiblings( subFolder ); + + EXPECT_EQ( QString( "Top" ), surface->userDescription() ); + EXPECT_EQ( QString( "Top" ), subFolder->collectionName() ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, PolygonsAreUniquePerFolder ) +{ + auto rootCollection = std::make_unique(); + + auto* firstPolygon = new RimPolygon(); + firstPolygon->setName( "Fence" ); + rootCollection->addUserDefinedPolygon( firstPolygon ); + + auto* secondPolygon = new RimPolygon(); + secondPolygon->setName( "Fence" ); + rootCollection->addUserDefinedPolygon( secondPolygon ); + + EXPECT_EQ( QString( "Fence" ), firstPolygon->name() ); + EXPECT_EQ( QString( "Fence_1" ), secondPolygon->name() ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +TEST( RiaNameUniquenessTools, ConflictPolicyForScripting ) +{ + auto rootCollection = std::make_unique(); + + auto* surface = new RimRegularSurface(); + surface->setUserDescription( "Top" ); + rootCollection->addSurface( surface ); + + { + auto resolution = + RiaNameUniquenessTools::applyConflictPolicy( &rootCollection->itemsField(), "Base", RiaDefines::NameConflictPolicy::FAIL ); + EXPECT_TRUE( resolution.errorMessage.isEmpty() ); + EXPECT_EQ( nullptr, resolution.objectToReplace ); + EXPECT_EQ( QString( "Base" ), resolution.nameToUse ); + } + + { + auto resolution = + RiaNameUniquenessTools::applyConflictPolicy( &rootCollection->itemsField(), "Top", RiaDefines::NameConflictPolicy::FAIL ); + EXPECT_FALSE( resolution.errorMessage.isEmpty() ); + } + + { + auto resolution = + RiaNameUniquenessTools::applyConflictPolicy( &rootCollection->itemsField(), "Top", RiaDefines::NameConflictPolicy::AUTO_RENAME ); + EXPECT_TRUE( resolution.errorMessage.isEmpty() ); + EXPECT_EQ( QString( "Top_1" ), resolution.nameToUse ); + } + + { + auto resolution = + RiaNameUniquenessTools::applyConflictPolicy( &rootCollection->itemsField(), "Top", RiaDefines::NameConflictPolicy::OVERWRITE ); + EXPECT_TRUE( resolution.errorMessage.isEmpty() ); + EXPECT_EQ( surface, resolution.objectToReplace ); + EXPECT_EQ( QString( "Top" ), resolution.nameToUse ); + } +} diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.h b/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.h index 2afc8aa587..89bf19ab7a 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.h +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.h @@ -72,9 +72,10 @@ public: std::vector subCollections() const; // Subcollection CRUD - void addSubCollection( SelfT* sub ); - PdmObject* addNewSubCollection() override; - SelfT* findSubCollectionByName( const QString& name ) const; + void addSubCollection( SelfT* sub ); + PdmObject* addNewSubCollection() override; + PdmChildArrayFieldHandle* subCollectionsField() override; + SelfT* findSubCollectionByName( const QString& name ) const; // Returns items held by this collection and recursively by all subcollections. // For items at this level only, use items(). diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.inl b/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.inl index f5d91288f0..8569f178df 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.inl +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollection.inl @@ -197,6 +197,15 @@ PdmObject* PdmNestedCollection::addNewSubCollection() return sub; } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +template +PdmChildArrayFieldHandle* PdmNestedCollection::subCollectionsField() +{ + return &m_subCollections; +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollectionBase.h b/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollectionBase.h index da6108e9c8..26ef90e5eb 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollectionBase.h +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmNestedCollectionBase.h @@ -25,6 +25,7 @@ namespace caf { +class PdmChildArrayFieldHandle; //================================================================================================== /// @@ -59,6 +60,11 @@ public: // nullptr; concrete derivations (typically PdmNestedCollection<>) override. virtual PdmObject* addNewSubCollection() { return nullptr; } + // Type-erased access to the field holding the sub-collections, for generic code that must + // inspect the folders without knowing the concrete collection type. Default returns nullptr; + // PdmNestedCollection<> overrides. + virtual PdmChildArrayFieldHandle* subCollectionsField() { return nullptr; } + // Marks this instance as the topmost folder: hides and disables IO on the collection name, // makes the object non-deletable. void setAsTopmostFolder(); diff --git a/GrpcInterface/Python/rips/tests/test_polygon.py b/GrpcInterface/Python/rips/tests/test_polygon.py index 9a644f1db6..8d4ef7d70f 100644 --- a/GrpcInterface/Python/rips/tests/test_polygon.py +++ b/GrpcInterface/Python/rips/tests/test_polygon.py @@ -2,6 +2,8 @@ import sys import os import math +import pytest + sys.path.insert(1, os.path.join(sys.path[0], "../../")) import rips @@ -65,3 +67,67 @@ def test_add_folders_and_polygons(rips_instance, initialize_test): assert [p.name for p in folder_a.polygons()] == ["A poly"] assert [p.name for p in folder_b.polygons()] == [] assert [p.name for p in nested.polygons()] == ["nested poly"] + + +def test_polygon_name_uniqueness(rips_instance, initialize_test): + rips_instance.project.open( + dataroot.PATH + "/TEST10K_FLT_LGR_NNC/10KWithWellLog.rsp" + ) + + root = rips_instance.project.descendants(rips.PolygonCollection)[0] + + coordinates = [ + [0.0, 0.0, -1000.0], + [100.0, 0.0, -1000.0], + [100.0, 100.0, -1000.0], + [0.0, 100.0, -1000.0], + ] + + root.create_polygon(name="Fence", coordinates=coordinates) + + # The default policy is to fail on a name already used in the same folder + with pytest.raises(rips.RipsError, match="already exists"): + root.create_polygon(name="Fence", coordinates=coordinates) + + renamed = root.create_polygon( + name="Fence", + coordinates=coordinates, + on_name_conflict=rips.NameConflictPolicy.AUTO_RENAME, + ) + assert renamed.name == "Fence_1" + + # Names are case sensitive, so this is not a conflict + other_case = root.create_polygon(name="fence", coordinates=coordinates) + assert other_case.name == "fence" + + overwritten = root.create_polygon( + name="Fence", + coordinates=coordinates, + on_name_conflict=rips.NameConflictPolicy.OVERWRITE, + ) + assert overwritten.name == "Fence" + assert [p.name for p in root.polygons()].count("Fence") == 1 + + # A different folder is a separate namespace + folder = root.add_folder(folder_name="Folder A") + in_folder = folder.create_polygon(name="Fence", coordinates=coordinates) + assert in_folder.name == "Fence" + + +def test_folder_name_uniqueness(rips_instance, initialize_test): + rips_instance.project.open( + dataroot.PATH + "/TEST10K_FLT_LGR_NNC/10KWithWellLog.rsp" + ) + + root = rips_instance.project.descendants(rips.PolygonCollection)[0] + + root.add_folder(folder_name="Folder A") + + with pytest.raises(rips.RipsError, match="already exists"): + root.add_folder(folder_name="Folder A") + + renamed = root.add_folder( + folder_name="Folder A", + on_name_conflict=rips.NameConflictPolicy.AUTO_RENAME, + ) + assert renamed.polygon_collection_name == "Folder A_1" diff --git a/GrpcInterface/Python/rips/tests/test_polygon_appearance.py b/GrpcInterface/Python/rips/tests/test_polygon_appearance.py index 1884275c06..0b5ee63a67 100644 --- a/GrpcInterface/Python/rips/tests/test_polygon_appearance.py +++ b/GrpcInterface/Python/rips/tests/test_polygon_appearance.py @@ -16,8 +16,8 @@ import rips @pytest.fixture -def project(rips_instance): - """Get the current project.""" +def project(rips_instance, initialize_test): + """Get the current project, reset between tests so each gets a clean polygon collection.""" return rips_instance.project diff --git a/GrpcInterface/Python/rips/tests/test_surfaces.py b/GrpcInterface/Python/rips/tests/test_surfaces.py index cc57dca69c..acd6752cc4 100644 --- a/GrpcInterface/Python/rips/tests/test_surfaces.py +++ b/GrpcInterface/Python/rips/tests/test_surfaces.py @@ -156,3 +156,38 @@ def test_create_regular_surface_invalid_values(rips_instance, initialize_test): with pytest.raises(rips.RipsError, match="Invalid rotation."): surface_collection.new_regular_surface(rotation=-1.0) + + +def test_surface_name_uniqueness(rips_instance, initialize_test): + case_path = dataroot.PATH + "/Case_with_10_timesteps/Real0/BRUGGE_0000.EGRID" + rips_instance.project.load_case(path=case_path) + + surface_collection = rips_instance.project.descendants(rips.SurfaceCollection)[0] + + surface_collection.new_regular_surface(name="Top") + + # The default policy is to fail on a name already used in the same folder + with pytest.raises(rips.RipsError, match="already exists"): + surface_collection.new_regular_surface(name="Top") + + renamed = surface_collection.new_regular_surface( + name="Top", on_name_conflict=rips.NameConflictPolicy.AUTO_RENAME + ) + assert renamed.surface_user_description == "Top_1" + + # Names are case sensitive, so this is not a conflict + other_case = surface_collection.new_regular_surface(name="top") + assert other_case.surface_user_description == "top" + + overwritten = surface_collection.new_regular_surface( + name="Top", on_name_conflict=rips.NameConflictPolicy.OVERWRITE + ) + assert overwritten.surface_user_description == "Top" + + names = [s.surface_user_description for s in surface_collection.surfaces_field()] + assert names.count("Top") == 1 + + # A different folder is a separate namespace + folder = surface_collection.add_folder(folder_name="Folder A") + in_folder = folder.new_regular_surface(name="Top") + assert in_folder.surface_user_description == "Top"