Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix distinct causes assigned properties to be lost #355

Merged
62 changes: 61 additions & 1 deletion .ci/unit-tests/BHoM_Adapter_Tests/PushTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
using System.Diagnostics.Contracts;
using AutoBogus;
using Shouldly;

using BH.oM.Geometry;

namespace BH.Tests.Adapter.Structure
{
Expand Down Expand Up @@ -516,5 +516,65 @@ public void Preprocess_PanelLoadsDuplicateIds()


}

[Test]
[Description("Tests that objects being pushed are correctly 'merged' by calls to CopyProperties modules. \n" +
"Two nodes pushed in the same location, one with a support and one without, the adapter should make sure the final node being sent for creation should contain the support.")]
public void CopyProperties_NodesReplaced()
{
//Create bar from line. Nodes will have null-constraints on the Bar
Line line = new Line { Start = new Point { X = 0 }, End = new Point { X = 10 } };
Bar bar = BH.Engine.Structure.Create.Bar(line, null, 0);

//Create nodes in the same position
Node node1 = new Node { Position = line.Start, Support = Create.RandomObject<Constraint6DOF>() };
Node node2 = new Node { Position = line.End, Support = Create.RandomObject<Constraint6DOF>() };

//Push with bar before the nodes
List<IBHoMObject> inputObjs = new List<IBHoMObject> { bar, node1, node2 };
sa.Push(inputObjs);

//Make sure the nodes in the model contain the supports
var supports = sa.Created.Where(x => x.Item1 == typeof(Node)).SelectMany(x => x.Item2).Cast<Node>().Select(x => x.Support).Where(x => x != null);
supports.ShouldContain(x => x.Name == node1.Support.Name);
supports.ShouldContain(x => x.Name == node2.Support.Name);
}

[Test]
[Description("Tests that all objects sent to the push have AdapterIds assigned, even though some have been identified as duplicates and hence culled out.")]
public void DuplicateObjects_EnsureAllOutputHaveIds()
{
//Create duplicate elements
Steel steel1 = Create.RandomObject<Steel>();
Steel steel2 = Create.RandomObject<Steel>();

steel1.Name = "MatName";
steel2.Name = steel1.Name;

SteelSection section1 = Create.RandomObject<SteelSection>();
SteelSection section2 = Create.RandomObject<SteelSection>();
section1.Material = steel1;
section2.Material = steel2;
section1.Name = "SecName";
section2.Name = section1.Name;

Line line = new Line { Start = new Point { X = 0 }, End = new Point { X = 10 } };
Bar bar1 = BH.Engine.Structure.Create.Bar(line, section1, 0);
Bar bar2 = BH.Engine.Structure.Create.Bar(line, section1, 0);

Node node1 = new Node { Position = line.Start };
Node node2 = new Node { Position = line.End };

//Push duplicates
List<IBHoMObject> inputObjs = new List<IBHoMObject> { bar1, bar2, node1, node2, section1, section2, steel1, steel2 };
List<IBHoMObject> pushed = sa.Push(inputObjs).OfType<IBHoMObject>().ToList();

//Make sure correct number of items has been created to ensure comparers work.
//If this does not work, the check of all objects having assigned Ids is pointless
sa.Created.Where(x => x.Item1 != typeof(Node)).ShouldAllBe(x => x.Item2.Count() == 1);
sa.Created.Where(x => x.Item1 == typeof(Node)).ShouldAllBe(x => x.Item2.Count() == 2);

pushed.ShouldAllBe(x => BH.Engine.Base.Query.FindFragment<StructuralAdapterId>(x, typeof(StructuralAdapterId)) != null, "At least one of the pushed objects did not contain an AdapterId Fragment.");
}
}
}
3 changes: 2 additions & 1 deletion Adapter_Engine/Query/GetCopyPropertiesModules.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ namespace BH.Engine.Adapter
{
public static partial class Query
{
public static List<ICopyPropertiesModule<T>> GetCopyPropertiesModules<T>(this IBHoMAdapter adapter) where T : class, IBHoMObject
[Description("Gets any adapter module on the adapter for copying properties of an object of type T to another object of type T.")]
public static List<ICopyPropertiesModule<T>> GetCopyPropertiesModules<T>(this IBHoMAdapter adapter) where T : IBHoMObject
{
return adapter.AdapterModules.OfType<ICopyPropertiesModule<T>>().ToList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,15 @@ protected virtual bool CreateOnly<T>(IEnumerable<T> objectsToPush, string tag =
{
bool callDistinct = objectLevel == 0 ? m_AdapterSettings.CreateOnly_DistinctObjects : m_AdapterSettings.CreateOnly_DistinctDependencies;

List<T> newObjects = !callDistinct ? objectsToPush.ToList() : objectsToPush.Distinct(Engine.Adapter.Query.GetComparerForType<T>(this, actionConfig)).ToList();
List<T> newObjects;
IEnumerable<IGrouping<T, T>> distinctGroups = null;
if (!callDistinct)
newObjects = objectsToPush.ToList();
else
{
distinctGroups = GroupAndCopyProperties(objectsToPush, actionConfig);
newObjects = distinctGroups.Select(x => x.Key).ToList();
}

// Tag the objects, if tag is given.
if (tag != "")
Expand All @@ -66,20 +74,21 @@ protected virtual bool CreateOnly<T>(IEnumerable<T> objectsToPush, string tag =
else if(!ICreate(newObjects, actionConfig))
return false;

if (callDistinct && m_AdapterSettings.UseAdapterId)
if (callDistinct && m_AdapterSettings.UseAdapterId && distinctGroups != null)
{
// Map Ids to the original set of objects (before we extracted the distincts elements from it).
// If some objects of the original set were not Created (because e.g. they were already existing in the external model and had already an id,
// therefore no new id was assigned to them) they will not get mapped, so the original set will be left with them intact.
IEqualityComparer<T> comparer = Engine.Adapter.Query.GetComparerForType<T>(this, actionConfig);
foreach (T item in objectsToPush)
foreach (var group in distinctGroups)
{
// Fetch any existing IAdapterId fragment and assign it to the item.
// This preserves any additional property other than `Id` that may be in the fragment.
IFragment fragment;
newObjects.First(x => comparer.Equals(x, item)).Fragments.TryGetValue(AdapterIdFragmentType, out fragment);

item.SetAdapterId(fragment as IAdapterId);
IFragment idFragment;
if (group.Key.Fragments.TryGetValue(AdapterIdFragmentType, out idFragment))
{
foreach (T item in group.Skip(1)) //Skip 1 as first instance is the key
{
item.SetAdapterId(idFragment as IAdapterId);
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,14 @@ public abstract partial class BHoMAdapter
// These methods dispatch calls to different CRUD methods as required by the Push.

[Description("Performs the full CRUD, calling the single CRUD methods as appropriate.")]
protected bool FullCRUD<T>(IEnumerable<T> objectsToPush, PushType pushType = PushType.AdapterDefault, string tag = "", ActionConfig actionConfig = null) where T : class, IBHoMObject
protected bool FullCRUD<T>(IEnumerable<T> objectsToPush, PushType pushType = PushType.AdapterDefault, string tag = "", ActionConfig actionConfig = null) where T : IBHoMObject
{
if (objectsToPush == null || !objectsToPush.Any())
return true;

// Make sure objects are distinct
List<T> newObjects = objectsToPush.Distinct(Engine.Adapter.Query.GetComparerForType<T>(this, actionConfig)).ToList();
// Make sure objects are distinct and that any copy-proeprty module for the type is run
IEnumerable<IGrouping<T,T>> distinctGroups = GroupAndCopyProperties(objectsToPush, actionConfig);
List<T> newObjects = distinctGroups.Select(x => x.Key).ToList();

// Add the tag if provided
if (!string.IsNullOrWhiteSpace(tag))
Expand Down Expand Up @@ -113,15 +114,16 @@ protected bool FullCRUD<T>(IEnumerable<T> objectsToPush, PushType pushType = Pus
// Map Ids to the original set of objects (before we extracted the distincts elements from it).
// If some objects of the original set were not Created (because e.g. they were already existing in the external model and had already an id,
// therefore no new id was assigned to them) they will not get mapped, so the original set will be left with them intact.
IEqualityComparer<T> comparer = Engine.Adapter.Query.GetComparerForType<T>(this, actionConfig);
foreach (T item in objectsToPush)
foreach (var group in distinctGroups)
{
// Fetch any existing IAdapterId fragment and assign it to the item.
// This preserves any additional property other than `Id` that may be in the fragment.
IFragment fragment;
newObjects.First(x => comparer.Equals(x, item)).Fragments.TryGetValue(AdapterIdFragmentType, out fragment);

item.SetAdapterId(fragment as IAdapterId);
IFragment idFragment;
if (group.Key.Fragments.TryGetValue(AdapterIdFragmentType, out idFragment))
{
foreach (T item in group.Skip(1)) //Skip 1 as first instance is the key
{
item.SetAdapterId(idFragment as IAdapterId);
}
}
}
}

Expand All @@ -130,7 +132,7 @@ protected bool FullCRUD<T>(IEnumerable<T> objectsToPush, PushType pushType = Pus

/***************************************************/

protected IEnumerable<T> ReplaceInMemory<T>(IEnumerable<T> newObjects, IEnumerable<T> existingOjects, string tag, ActionConfig actionConfig, bool mergeWithComparer = false) where T : class, IBHoMObject
protected IEnumerable<T> ReplaceInMemory<T>(IEnumerable<T> newObjects, IEnumerable<T> existingOjects, string tag, ActionConfig actionConfig, bool mergeWithComparer = false) where T : IBHoMObject
{
// Separate objects based on tags
List<T> multiTaggedObjects = existingOjects.Where(x => x.Tags.Contains(tag) && x.Tags.Count > 1).ToList();
Expand Down Expand Up @@ -164,7 +166,7 @@ protected IEnumerable<T> ReplaceInMemory<T>(IEnumerable<T> newObjects, IEnumerab

/***************************************************/

protected IEnumerable<T> ReplaceThroughAPI<T>(IEnumerable<T> objsToPush, IEnumerable<T> readObjs, string tag, ActionConfig actionConfig, PushType pushType) where T : class, IBHoMObject
protected IEnumerable<T> ReplaceThroughAPI<T>(IEnumerable<T> objsToPush, IEnumerable<T> readObjs, string tag, ActionConfig actionConfig, PushType pushType) where T : IBHoMObject
{
IEqualityComparer<T> comparer = Engine.Adapter.Query.GetComparerForType<T>(this, actionConfig);
VennDiagram<T> diagram = Engine.Data.Create.VennDiagram(objsToPush, readObjs, comparer);
Expand Down
2 changes: 1 addition & 1 deletion BHoM_Adapter/HelperMethods/CopyBHoMObjectProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace BH.Adapter
public abstract partial class BHoMAdapter
{
[Description("Gets called during the Push. Takes properties specified from the source IBHoMObject and assigns them to the target IBHoMObject.")]
public void CopyBHoMObjectProperties<T>(T target, T source) where T : class, IBHoMObject
public void CopyBHoMObjectProperties<T>(T target, T source) where T : IBHoMObject
{
// Port tags from source to target
foreach (string tag in source.Tags)
Expand Down
48 changes: 48 additions & 0 deletions BHoM_Adapter/HelperMethods/DistinctWithCopiedProperties.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* This file is part of the Buildings and Habitats object Model (BHoM)
* Copyright (c) 2015 - 2023, the respective contributors. All rights reserved.
*
* Each contributor holds copyright over their respective contributions.
* The project versioning (Git) records all such contribution source information.
*
*
* The BHoM is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3.0 of the License, or
* (at your option) any later version.
*
* The BHoM 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 Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this code. If not, see <https://www.gnu.org/licenses/lgpl-3.0.html>.
*/

using BH.Engine.Adapter;
using BH.Engine.Base;
using BH.oM.Adapter;
using BH.oM.Base;
using BH.oM.Data;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;

namespace BH.Adapter
{

public abstract partial class BHoMAdapter
{
[Description("Gets distinct objects based on implemented AdapterComparer of the particular type, with CopyPropertyModules available for the type run.")]
private List<T> DistinctWithCopiedProperties<T>(IEnumerable<T> objectsToPush, ActionConfig actionConfig = null) where T : IBHoMObject
{
return GroupAndCopyProperties(objectsToPush, actionConfig).Select(x => x.Key).ToList();
}
}
}




64 changes: 64 additions & 0 deletions BHoM_Adapter/HelperMethods/GroupAndCopyProperties.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* This file is part of the Buildings and Habitats object Model (BHoM)
* Copyright (c) 2015 - 2023, the respective contributors. All rights reserved.
*
* Each contributor holds copyright over their respective contributions.
* The project versioning (Git) records all such contribution source information.
*
*
* The BHoM is free software: you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published by
* the Free Software Foundation, either version 3.0 of the License, or
* (at your option) any later version.
*
* The BHoM 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 Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this code. If not, see <https://www.gnu.org/licenses/lgpl-3.0.html>.
*/

using BH.Engine.Adapter;
using BH.Engine.Base;
using BH.oM.Adapter;
using BH.oM.Base;
using BH.oM.Data;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;

namespace BH.Adapter
{
public abstract partial class BHoMAdapter
{
[Description("Groups the objects by the coparer for the particular type, and then runs any CopyPropertiesModules available for the type.")]
private IEnumerable<IGrouping<T, T>> GroupAndCopyProperties<T>(IEnumerable<T> objectsToPush, ActionConfig actionConfig = null) where T : IBHoMObject
{
IEnumerable<IGrouping<T, T>> grouped = objectsToPush.GroupBy(x => x, Engine.Adapter.Query.GetComparerForType<T>(this, actionConfig));

List<ICopyPropertiesModule<T>> copyPropertiesModules = this.GetCopyPropertiesModules<T>();

foreach (var group in grouped)
{
T keep = group.Key;
foreach (T item in group.Skip(1)) //Skip 1 as first instance is the key
{
CopyBHoMObjectProperties(keep, item);
foreach (var copyModule in copyPropertiesModules)
{
copyModule.CopyProperties(keep, item);
}
}
}

return grouped;
}
}
}




25 changes: 21 additions & 4 deletions Structure_AdapterModules/CopyNodeProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
using System.ComponentModel;
using BH.oM.Structure.Elements;
using BH.oM.Structure.SectionProperties;
using BH.Engine.Structure;
using BH.Engine.Geometry;

namespace BH.Adapter.Modules
{
Expand All @@ -38,12 +40,27 @@ public class CopyNodeProperties : ICopyPropertiesModule<Node>
public void CopyProperties(Node target, Node source)
{
// If source is constrained and target is not, add source constraint to target
if (source.Support != null && target.Support == null)
target.Support = source.Support;
if (source.Support != null)
{
if (target.Support == null)
target.Support = source.Support;
else
{
string desc1 = target.Support.Description();
string desc2 = source.Support.Description();
if(desc1 != desc2)
Engine.Base.Compute.RecordNote($"Node in position ({target.Position.X},{target.Position.Y},{target.Position.Z}) contains conflicting supports. Support {desc1} will be used on the node.");
}
}

// If source has a defined orientation and target does not, add local orientation from the source
if (source.Orientation != null && target.Orientation == null)
target.Orientation = source.Orientation;
if (source.Orientation != null)
{
if (target.Orientation == null)
target.Orientation = source.Orientation;
else if(!source.Orientation.IsEqual(target.Orientation))
BH.Engine.Base.Compute.RecordNote($"Node in position ({target.Position.X}, {target.Position.Y}, {target.Position.Z}) contains conflicting orientaions. Orientation with Normal vector ({target.Orientation.Z.X}, {target.Orientation.Z.Y}, {target.Orientation.Z.Z}) will be used on the node.");
}
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions Structure_AdapterModules/Structure_AdapterModules.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,21 @@
<Private>false</Private>
<SpecificVersion>false</SpecificVersion>
</Reference>
<Reference Include="Spatial_oM">
<HintPath>C:\ProgramData\BHoM\Assemblies\Spatial_oM.dll</HintPath>
<Private>false</Private>
<SpecificVersion>false</SpecificVersion>
</Reference>
<Reference Include="Structure_oM">
<HintPath>C:\ProgramData\BHoM\Assemblies\Structure_oM.dll</HintPath>
<Private>false</Private>
<SpecificVersion>false</SpecificVersion>
</Reference>
<Reference Include="Structure_Engine">
<HintPath>C:\ProgramData\BHoM\Assemblies\Structure_Engine.dll</HintPath>
<Private>false</Private>
<SpecificVersion>false</SpecificVersion>
</Reference>
</ItemGroup>

</Project>