From 9cb9e48fe1e2edbabebe1e33dd53b6428e3e2582 Mon Sep 17 00:00:00 2001
From: Kasper Marstal <kaspermarstal@gmail.com>
Date: Wed, 28 Oct 2015 11:32:24 +0100
Subject: [PATCH] ENH: Tests for blueprint interface for managing connections
 between components

---
 .../Core/Blueprints/include/elxBlueprint.h    | 11 ++-
 Modules/Core/Blueprints/src/elxBlueprint.cxx  | 15 ++--
 Testing/Unit/elxBluePrintTest.cxx             | 88 +++++++++++++++++++
 3 files changed, 101 insertions(+), 13 deletions(-)

diff --git a/Modules/Core/Blueprints/include/elxBlueprint.h b/Modules/Core/Blueprints/include/elxBlueprint.h
index ce185437..c0ff7ac7 100644
--- a/Modules/Core/Blueprints/include/elxBlueprint.h
+++ b/Modules/Core/Blueprints/include/elxBlueprint.h
@@ -59,12 +59,11 @@ public:
   ParameterMapType GetComponent( ComponentIndexType index );
   void SetComponent( ComponentIndexType, ParameterMapType parameterMap );
 
-  // TODO: Let user delete component. To do this, we need a proper way of 
+  // TODO: Let user delete component. Before we do this, we need a proper way of 
   // checking that a vertex exist. Otherwise a call to  GetComponent() on 
-  // a deleted vertex will result in segfault. It is not really a problem 
-  // that there is no delete vertex feature at this point since typically 
-  // the blueprint interface is used procedurally to generate a specific
-  // blueprint.
+  // a deleted vertex will result in segfault. It is not really a in issue
+  // _before_ realease since typically we use (the developers) use blueprint 
+  // interface procedurally.
   // void DeleteComponent( ComponentIndexType );
 
   ComponentIteratorPairType GetComponentIterator( void ) {
@@ -93,7 +92,7 @@ public:
   }
 
 private:
-
+  
   ConnectionIndexType GetConnectionIndex( ComponentIndexType upsteam, ComponentIndexType downstream );
 
   GraphType m_Graph;
diff --git a/Modules/Core/Blueprints/src/elxBlueprint.cxx b/Modules/Core/Blueprints/src/elxBlueprint.cxx
index 278725aa..b6e37cff 100644
--- a/Modules/Core/Blueprints/src/elxBlueprint.cxx
+++ b/Modules/Core/Blueprints/src/elxBlueprint.cxx
@@ -80,15 +80,16 @@ Blueprint
 {
   this->Modified();
 
-  // If the connection does not exist, add the parameter map, otherwise don't do anything
-  // because previous settings will be overwritten. If the user do want to overwrite 
-  // current settings, she should use SetConnection() instead where the intent is explicit.  
-  if( this->ConnectionExists( upstream, downstream) ) {
-    return false;
+  if( !this->ConnectionExists( upstream, downstream ) ) {
+    ConnectionIndexType index = boost::add_edge( upstream, downstream, this->m_Graph ).first;
+    this->m_Graph[ index ].parameterMap = parameterMap;
+    return true;
   }
 
-  this->m_Graph[ this->GetConnectionIndex( upstream, downstream ) ].parameterMap = parameterMap;
-  return true;
+  // If the connection does not exist don't do anything because previous settings 
+  // will be overwritten.  If the user do want to overwrite current settings, 
+  // she should use SetConnection() instead where the intent is explicit.  
+  return false;
 }
 
 Blueprint::ParameterMapType
diff --git a/Testing/Unit/elxBluePrintTest.cxx b/Testing/Unit/elxBluePrintTest.cxx
index f2c39fdf..68a951ca 100644
--- a/Testing/Unit/elxBluePrintTest.cxx
+++ b/Testing/Unit/elxBluePrintTest.cxx
@@ -67,4 +67,92 @@ TEST_F( BlueprintTest, SetComponent )
 //   EXPECT_ANY_THROW( parameterMapTest = blueprint->GetComponent( index ) );
 // }
 
+TEST_F( BlueprintTest, AddConnection )
+{
+  BlueprintPointerType blueprint = Blueprint::New();
+
+  ComponentIndexType index0 = blueprint->AddComponent();
+  ComponentIndexType index1 = blueprint->AddComponent();
+  ComponentIndexType index2 = blueprint->AddComponent();
+
+  // Connection should not exist
+  EXPECT_FALSE( blueprint->ConnectionExists( index0, index1 ) );
+
+  // Connection should be added
+  EXPECT_TRUE( blueprint->AddConnection( index0, index1 ) );
+
+  // Connection should exist
+  EXPECT_TRUE( blueprint->ConnectionExists( index0, index1 ) );
+
+  // Another connection between same components should not be added
+  // (user should use SetComponent() instead)
+  EXPECT_FALSE( blueprint->AddConnection( index0, index1 ) );
+
+  // Connection should be empty
+  ParameterMapType parameterMapTest0;
+  EXPECT_NO_THROW( parameterMapTest0 = blueprint->GetConnection( index0, index1 ) );
+  EXPECT_EQ( 0u, parameterMapTest0.size() );
+
+  // Connection with properties should be added. Testing if properties was 
+  // properly set requires a call to GetConnection() which is the responsibility
+  // of the next test.
+  ParameterMapType parameterMapTest1;
+  EXPECT_TRUE( blueprint->AddConnection( index1, index2, parameterMap ) );
+
+  // It is  not be possible to add connection between components that do not exist
+  // because you do not have necessary indexes
+}
+
+TEST_F( BlueprintTest, GetConnection )
+{
+  BlueprintPointerType blueprint = Blueprint::New();
+
+  ComponentIndexType index0 = blueprint->AddComponent();
+  ComponentIndexType index1 = blueprint->AddComponent();
+
+  ParameterMapType parameterMapTest0;
+  EXPECT_TRUE( blueprint->AddConnection( index0, index1, parameterMap ) );
+  EXPECT_NO_THROW( parameterMapTest0 = blueprint->GetConnection( index0, index1 ) );
+  EXPECT_EQ( parameterMap["ComponentName"], parameterMapTest0["ComponentName"] );
+}
+
+TEST_F( BlueprintTest, SetConnection )
+{
+  BlueprintPointerType blueprint = Blueprint::New();
+
+  ComponentIndexType index0 = blueprint->AddComponent();
+  ComponentIndexType index1 = blueprint->AddComponent();
+  blueprint->AddConnection( index0, index1, parameterMap );
+
+  ParameterMapType parameterMapTest0;
+  parameterMapTest0 = blueprint->GetConnection( index0, index1 );
+  EXPECT_EQ( parameterMap["ComponentName"], parameterMapTest0["ComponentName"] );
+
+  ParameterMapType parameterMapTest1;
+  parameterMapTest1["ComponentName"] = ParameterValueType(1, "OtherName");
+  EXPECT_TRUE( blueprint->SetConnection( index0, index1, parameterMapTest1 ) );
+
+  ParameterMapType parameterMapTest2;
+  EXPECT_NO_THROW( parameterMapTest2 = blueprint->GetConnection( index0, index1 ) );
+  EXPECT_EQ( parameterMapTest1["ComponentName"], parameterMapTest2["ComponentName"] );
+}
+
+TEST_F( BlueprintTest, DeleteConnection )
+{
+  BlueprintPointerType blueprint = Blueprint::New();
+
+  ComponentIndexType index0 = blueprint->AddComponent();
+  ComponentIndexType index1 = blueprint->AddComponent();
+  blueprint->AddConnection( index0, index1 );
+
+  // Connection should exist
+  EXPECT_TRUE( blueprint->ConnectionExists( index0, index1 ) );
+
+  // Connection be deleted
+  EXPECT_TRUE( blueprint->DeleteConnection( index0, index1 ) );
+
+  // Connection should not exist 
+  EXPECT_FALSE( blueprint->ConnectionExists( index0, index1 ) );
+}
+
 } // namespace elx
-- 
GitLab