Fixes #2227 spatial structure merge

src/OSPSuite.Core/Domain/Mappers/NeighborhoodCollectionToContainerMapper.cs

@@ -1,8 +1,11 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using OSPSuite.Core.Domain.Builder;
+using OSPSuite.Core.Domain.Services;
 using OSPSuite.Utility;
 using OSPSuite.Utility.Collections;
+using OSPSuite.Utility.Extensions;
 
 namespace OSPSuite.Core.Domain.Mappers
 {
@@ -21,40 +24,72 @@ internal class NeighborhoodCollectionToContainerMapper : INeighborhoodCollection
    {
       private readonly IObjectBaseFactory _objectBaseFactory;
       private readonly INeighborhoodBuilderToNeighborhoodMapper _neighborhoodMapper;
+      private readonly IContainerMergeTask _containerMergeTask;
 
       public NeighborhoodCollectionToContainerMapper(
          IObjectBaseFactory objectBaseFactory,
-         INeighborhoodBuilderToNeighborhoodMapper neighborhoodMapper)
+         INeighborhoodBuilderToNeighborhoodMapper neighborhoodMapper,
+         IContainerMergeTask containerMergeTask)
       {
          _objectBaseFactory = objectBaseFactory;
          _neighborhoodMapper = neighborhoodMapper;
+         _containerMergeTask = containerMergeTask;
       }
 
       public IContainer MapFrom(ModelConfiguration modelConfiguration)
       {
          var (_, simulationBuilder) = modelConfiguration;
-         var moleculeNames = simulationBuilder.AllPresentFloatingMoleculeNames();
 
          var neighborhoodsParentContainer = _objectBaseFactory.Create<IContainer>()
             .WithMode(ContainerMode.Logical)
             .WithName(Constants.NEIGHBORHOODS);
 
-         var startValuesForFloatingMolecules = presentMoleculesCachedByContainerPath(moleculeNames, simulationBuilder);
+         var allSpatialStructureAndMergeBehaviors = simulationBuilder.SpatialStructureAndMergeBehaviors;
+         if (!allSpatialStructureAndMergeBehaviors.Any())
+            return neighborhoodsParentContainer;
 
-         var moleculeNamesCopyProperties = simulationBuilder.AllPresentXenobioticFloatingMoleculeNames();
+         var mapToNeighborhood = mapToNeighborhoodDef(modelConfiguration);
+
+         IReadOnlyList<Neighborhood> mapNeighborhoods(SpatialStructure spatialStructure) =>
+            spatialStructure.Neighborhoods.Select(mapToNeighborhood).Where(x => x != null).ToList();
 
          //we use a cache to ensure that we are replacing neighborhoods defined in multiple structures
-         var neighborhoodCache = new ObjectBaseCache<Neighborhood>();
-         var allNeighborhoodBuilder = simulationBuilder.SpatialStructureAndMergeBehaviors.SelectMany(x => x.spatialStructure.Neighborhoods);
-         neighborhoodCache.AddRange(allNeighborhoodBuilder.Select(nb =>
-               _neighborhoodMapper.MapFrom(nb, moleculeNamesFor(nb, startValuesForFloatingMolecules), moleculeNamesCopyProperties, modelConfiguration))
-            //can be null if the neighbors are not defined in the merged spatial structure
-            .Where(x => x != null));
-
-         neighborhoodsParentContainer.AddChildren(neighborhoodCache);
+         var firstSpatialStructure = allSpatialStructureAndMergeBehaviors[0].spatialStructure;
+         var allOtherSpatialStructuresWithMergeBehavior = allSpatialStructureAndMergeBehaviors.Skip(1).ToList();
+
+
+         //first step: Add the neighborhoods from the first structure
+         neighborhoodsParentContainer.AddChildren(mapNeighborhoods(firstSpatialStructure));
+
+         //now merge all other neighborhoods
+         allOtherSpatialStructuresWithMergeBehavior
+            .Select(x => new {x.mergeBehavior, neighborhoods = mapNeighborhoods(x.spatialStructure)})
+            .Each(x => mergeNeighborhoodsInStructure(neighborhoodsParentContainer, x.neighborhoods, x.mergeBehavior));
+
          return neighborhoodsParentContainer;
       }
 
+      private void mergeNeighborhoodsInStructure(IContainer neighborhoods, IReadOnlyList<Neighborhood> neighborhoodsToMerge, MergeBehavior mergeBehavior)
+      {
+         neighborhoodsToMerge.Each(neighborhoodToMerge =>
+         {
+            if (mergeBehavior == MergeBehavior.Extend)
+               _containerMergeTask.AddOrMergeContainer(neighborhoods, neighborhoodToMerge);
+            else
+               _containerMergeTask.AddOrReplaceInContainer(neighborhoods, neighborhoodToMerge);
+         });
+      }
+
+      private Func<NeighborhoodBuilder, Neighborhood> mapToNeighborhoodDef(ModelConfiguration modelConfiguration)
+      {
+         var (_, simulationBuilder) = modelConfiguration;
+         var moleculeNames = simulationBuilder.AllPresentFloatingMoleculeNames();
+         var startValuesForFloatingMolecules = presentMoleculesCachedByContainerPath(moleculeNames, simulationBuilder);
+         var moleculeNamesCopyProperties = simulationBuilder.AllPresentXenobioticFloatingMoleculeNames();
+
+         return neighborhoodBuilder => _neighborhoodMapper.MapFrom(neighborhoodBuilder, moleculeNamesFor(neighborhoodBuilder, startValuesForFloatingMolecules), moleculeNamesCopyProperties, modelConfiguration);
+      }
+
       private ICache<string, List<string>> presentMoleculesCachedByContainerPath(IEnumerable<string> namesOfFloatingMolecules, SimulationBuilder simulationBuilder)
       {
          var initialConditions = simulationBuilder.AllPresentMoleculeValuesFor(namesOfFloatingMolecules).ToList();

src/OSPSuite.Core/Domain/Services/ContainerMergeTask.cs

@@ -0,0 +1,68 @@
+using System.Linq;
+using OSPSuite.Utility.Extensions;
+
+namespace OSPSuite.Core.Domain.Services
+{
+   public interface IContainerMergeTask
+   {
+      void MergeContainers(IContainer targetContainer, IContainer containerToMerge);
+      void AddOrMergeContainer(IContainer parentContainer, IContainer containerToMerge);
+
+      /// <summary>
+      ///    If the entity exists, it will be removed and replace by the new one, otherwise simply added
+      /// </summary>
+      void AddOrReplaceInContainer(IContainer container, IEntity entityToAddOrReplace);
+   }
+
+   public class ContainerMergeTask : IContainerMergeTask
+   {
+      public void AddOrMergeContainer(IContainer parentContainer, IContainer containerToMerge)
+      {
+         var existingContainer = parentContainer.Container(containerToMerge.Name);
+         if (existingContainer == null)
+            parentContainer.Add(containerToMerge);
+         else
+            MergeContainers(existingContainer, containerToMerge);
+      }
+
+      /// <summary>
+      ///    If the entity exists, it will be removed and replace by the new one, otherwise simply added
+      /// </summary>
+      public void AddOrReplaceInContainer(IContainer container, IEntity entityToAddOrReplace)
+      {
+         var existingChild = container.GetSingleChildByName(entityToAddOrReplace.Name);
+         if (existingChild != null)
+            container.RemoveChild(existingChild);
+
+         container.Add(entityToAddOrReplace);
+      }
+
+      public void MergeContainers(IContainer targetContainer, IContainer containerToMerge)
+      {
+         updateContainerProperties(targetContainer, containerToMerge);
+         var allChildrenContainerToMerge = containerToMerge.GetChildren<IContainer>(x => !x.IsAnImplementationOf<IDistributedParameter>()).ToList();
+         var allChildrenEntitiesToMerge = containerToMerge.GetChildren<IEntity>().Except(allChildrenContainerToMerge).ToList();
+
+         //First we do all containers (except distributed parameters which are to be seen as entities) recursively
+         allChildrenContainerToMerge.Each(x =>
+         {
+            var targetChildrenContainer = targetContainer.Container(x.Name);
+            //does not exist, we add it
+            if (targetChildrenContainer == null)
+               AddOrReplaceInContainer(targetContainer, x);
+            //it exists, we need to merge
+            else
+               MergeContainers(targetChildrenContainer, x);
+         });
+
+         //then we add or replace all non containers entities
+         allChildrenEntitiesToMerge.Each(x => AddOrReplaceInContainer(targetContainer, x));
+      }
+
+      private void updateContainerProperties(IContainer targetContainer, IContainer containerToMerge)
+      {
+         targetContainer.Mode = containerToMerge.Mode;
+         containerToMerge.Tags.Each(targetContainer.AddTag);
+      }
+   }
+}

src/OSPSuite.Core/Domain/Services/SpatialStructureMerger.cs

@@ -20,15 +20,18 @@ internal class SpatialStructureMerger : ISpatialStructureMerger
       private readonly IObjectBaseFactory _objectBaseFactory;
       private readonly IContainerBuilderToContainerMapper _containerMapper;
       private readonly INeighborhoodCollectionToContainerMapper _neighborhoodsMapper;
+      private readonly IContainerMergeTask _containerMergeTask;
 
       public SpatialStructureMerger(
          IObjectBaseFactory objectBaseFactory,
          IContainerBuilderToContainerMapper containerMapper,
-         INeighborhoodCollectionToContainerMapper neighborhoodsMapper)
+         INeighborhoodCollectionToContainerMapper neighborhoodsMapper,
+         IContainerMergeTask containerMergeTask)
       {
          _objectBaseFactory = objectBaseFactory;
          _containerMapper = containerMapper;
          _neighborhoodsMapper = neighborhoodsMapper;
+         _containerMergeTask = containerMergeTask;
       }
 
       public IContainer MergeContainerStructure(ModelConfiguration modelConfiguration)
@@ -57,7 +60,6 @@ private void createMergedContainerStructureInRoot(IContainer root, SimulationBui
          var allOtherSpatialStructuresWithMergeBehavior = allSpatialStructureAndMergeBehaviors.Skip(1).ToList();
          var mapToModelContainer = mapContainerDef(simulationBuilder);
 
-
          // First step: We create the container structure.
          // This is done by adding all top containers defined in the FIRST spatial structure
          // Then we merge all other top containers defined in the other spatial structures based on the merge behavior
@@ -84,7 +86,7 @@ private void createMergedContainerStructureInRoot(IContainer root, SimulationBui
 
          if (firstGlobalMoleculeContainer != null)
          {
-            otherGlobalMoleculeContainer.Each(x => mergeContainers(firstGlobalMoleculeContainer, x));
+            otherGlobalMoleculeContainer.Each(x => _containerMergeTask.MergeContainers(firstGlobalMoleculeContainer, x));
             root.Add(firstGlobalMoleculeContainer);
          }
       }
@@ -129,60 +131,15 @@ private void replaceOrMergeContainerIntoParent(IContainer parentContainer, ICont
       {
          //Merge behavior is extend or we have a special case to deal with when dealing with MoleculeProperties container that we merge instead of replacing
          if (mergeBehavior == MergeBehavior.Extend || containerToMerge.IsNamed(Constants.MOLECULE_PROPERTIES))
-            addOrMergeContainer(parentContainer, containerToMerge);
-         else
-            addOrReplaceInContainer(parentContainer, containerToMerge);
-      }
-
-      private void mergeContainers(IContainer targetContainer, IContainer containerToMerge)
-      {
-         var allChildrenContainerToMerge = containerToMerge.GetChildren<IContainer>(x => !x.IsAnImplementationOf<IDistributedParameter>()).ToList();
-         var allChildrenEntitiesToMerge = containerToMerge.GetChildren<IEntity>().Except(allChildrenContainerToMerge).ToList();
-
-         //First we do all containers (except distributed parameters which are to be seen as entities) recursively
-         allChildrenContainerToMerge.Each(x =>
-         {
-            var targetChildrenContainer = targetContainer.Container(x.Name);
-            //does not exist, we add it
-            if (targetChildrenContainer == null)
-               addOrReplaceInContainer(targetContainer, x);
-            //it exists, we need to merge
-            else
-               mergeContainers(targetChildrenContainer, x);
-         });
-
-         //then we add or replace all non containers entities
-         allChildrenEntitiesToMerge.Each(x => addOrReplaceInContainer(targetContainer, x));
-      }
-
-      private void addOrMergeContainer(IContainer parentContainer, IContainer containerToMerge)
-      {
-         var existingContainer = parentContainer.Container(containerToMerge.Name);
-         if (existingContainer == null)
-            parentContainer.Add(containerToMerge);
+            _containerMergeTask.AddOrMergeContainer(parentContainer, containerToMerge);
          else
-            mergeContainers(existingContainer, containerToMerge);
-      }
-
-      /// <summary>
-      ///    If the entity exists, it will be removed and replace by the new one, otherwise simply added
-      /// </summary>
-      private void addOrReplaceInContainer(IContainer container, IEntity entityToAddOrReplace)
-      {
-         var existingChild = container.GetSingleChildByName(entityToAddOrReplace.Name);
-         if (existingChild != null)
-            container.RemoveChild(existingChild);
-
-         container.Add(entityToAddOrReplace);
+            _containerMergeTask.AddOrReplaceInContainer(parentContainer, containerToMerge);
       }
 
       public IContainer MergeNeighborhoods(ModelConfiguration modelConfiguration) => _neighborhoodsMapper.MapFrom(modelConfiguration);
    }
 
    internal class ContainerNotFoundException : OSPSuiteException
    {
-      public ContainerNotFoundException() : base()
-      {
-      }
    }
 }

tests/OSPSuite.Core.IntegrationTests/ModuleIntegrationTests.cs

@@ -104,6 +104,13 @@ public void should_have_merged_the_tag_at_the_container_level()
          lung.Tags.Contains("Tag2").ShouldBeTrue();
       }
 
+      [Observation]
+      public void should_have_updated_the_container_mode_to_the_one_of_the_merged_container()
+      {
+         var lung = _model.Root.EntityAt<Container>(Constants.ORGANISM, Lung);
+         lung.Mode.ShouldBeEqualTo(ContainerMode.Physical);
+      }
+
       [Observation]
       public void should_have_not_merged_the_tag_at_the_parameter_level()
       {

tests/OSPSuite.HelpersForTests/ModuleHelperForSpecs.cs

@@ -177,8 +177,9 @@ private SpatialStructure getSpatialStructureModule1()
          //      - pH
          //   - Q
          //   - P
-
-         var lung = createContainerWithName(Lung);
+
+         //set the container to being physical. It will become logical after update
+         var lung = createContainerWithName(Lung, ContainerMode.Logical);
          lung.AddTag("Tag1");
          var lngPlasma = createContainerWithName(Plasma, ContainerMode.Physical);
          lngPlasma.Add(newConstantParameter(Volume, 2));
@@ -293,7 +294,7 @@ private Module createModule2()
          //      - Volume
          //      - pH
 
-         var lung = createContainerWithName(Lung);
+         var lung = createContainerWithName(Lung, ContainerMode.Physical);
 
          var lngPlasma = createContainerWithName(Plasma, ContainerMode.Physical);
          lngPlasma.Add(newConstantParameter(Volume, 20));
@@ -343,7 +344,7 @@ private Module createModule2()
          spatialStructure.AddNeighborhood(neighborhood2);
 
          //existing neighborhood between lngPlasma and lngCell
-         var neighborhood3 = _neighborhoodFactory.CreateBetween(lngPlasma, lngCell).WithName("lng_pls_to_lng_cell");
+         var neighborhood3 = _neighborhoodFactory.CreateBetween(lngPlasma, lngCell, lung.ParentPath).WithName("lng_pls_to_lng_cell");
          neighborhood3.AddTag("NeighborhoodTag2");
          spatialStructure.AddNeighborhood(neighborhood3);