Skip to content

Commit

Permalink
bug fix, code refactor, new tests
Browse files Browse the repository at this point in the history 
- fixed a bug where incompatible lists would be allowed
- added test to check all error conditions
- code cleanup
  • Loading branch information
@dnenov
dnenov committed May 21, 2024
1 parent 695984b commit 979dcca
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 38 deletions.
5 changes: 2 additions & 3 deletions src/Libraries/CoreNodeModels/DefineData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,13 @@ public override IEnumerable<AssociativeNode> BuildOutputAst(List<AssociativeNode

resultAst.Add(AstFactory.BuildAssignment(functionCallIdentifier, functionCall));

//Next add the first key value pair to the output port
// Next add the first key value pair to the output port
var getFirstKey = AstFactory.BuildFunctionCall(BuiltinDictionaryTypeName, BuiltinDictionaryGet,
new List<AssociativeNode> { functionCallIdentifier, AstFactory.BuildStringNode(">") });

resultAst.Add(AstFactory.BuildAssignment(GetAstIdentifierForOutputIndex(0), getFirstKey));

//Second get the key value pair to pass to the databridge callback
// Second get the key value pair to pass to the databridge callback
var getSecondKey = AstFactory.BuildFunctionCall(BuiltinDictionaryTypeName, BuiltinDictionaryGet,
new List<AssociativeNode> { functionCallIdentifier, AstFactory.BuildStringNode("Validation") });

Expand Down Expand Up @@ -219,7 +219,6 @@ private void DataBridgeCallback(object data)
if(IsAutoMode)
{
DisplayValue = string.Empty; // show blank if we are in locked mode (as we cannot interact with the node)
//DisplayValue = Properties.Resources.DefineDataDisplayValueMessage;
}
else
{
Expand Down
97 changes: 64 additions & 33 deletions src/Libraries/CoreNodes/Data.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,13 @@ static Data()
/// <param name="typeString">The Type as string (it would be better to pass an object of type 'Type' for direct type comparison)</param>
/// <param name="isList">If the input is of type `ArrayList`</param>
/// <param name="isAutoMode">If the node is in Auto mode</param>
/// <param name="playerValue">The value coming from Dynamo Player</param>
/// <returns></returns>
[IsVisibleInDynamoLibrary(false)]
public static Dictionary<string, object> IsSupportedDataNodeType([ArbitraryDimensionArrayImport] object inputValue,
string typeString, bool isList, bool isAutoMode, string playerValue)
{

if (inputValue == null)
{
// Don't raise a warning if the node is unused
Expand All @@ -673,7 +675,8 @@ public static Dictionary<string, object> IsSupportedDataNodeType([ArbitraryDimen

if (!IsSingleValueOrSingleLevelArrayList(inputValue))
{
throw new NotSupportedException(Properties.Resources.DefineDataSupportedInputValueExceptionMessage);
var warning = Properties.Resources.DefineDataSupportedInputValueExceptionMessage;
return DefineDataResult(inputValue, false, false, DataNodeDynamoTypeList.First(), warning);
}

// If the playerValue is not empty, then we assume it was set by the player.
Expand All @@ -687,19 +690,18 @@ public static Dictionary<string, object> IsSupportedDataNodeType([ArbitraryDimen
catch (Exception ex)
{
dynamoLogger?.Log("A Player value failed to deserialize with this exception: " + ex.Message);
throw new NotSupportedException(Properties.Resources.Exception_Deserialize_Unsupported_Cache);

var warning = Properties.Resources.Exception_Deserialize_Unsupported_Cache;
return DefineDataResult(inputValue, false, false, DataNodeDynamoTypeList.First(), warning);
}
}

object result; // Tuple<IsValid: bool, UpdateList: bool, InputType: DataNodeDynamoType>

// Currently working around passing the type as a string from the node - can be developed further to pass directly the type value
var type = DataNodeDynamoTypeList.First(x => x.Type.ToString().Equals(typeString));

if (isAutoMode)
{
// If running in AutoMode, then we would propagate the actual Type and List value and validate against them
// List logic
bool updateList = false;

var assertList = inputValue is ArrayList;
Expand All @@ -716,46 +718,78 @@ public static Dictionary<string, object> IsSupportedDataNodeType([ArbitraryDimen
{
// We couldn't find a common ancestor - list containing unsupported or incompatible data types
var incompatibleDataTypes = ConcatenateUniqueDataTypes(inputValue);
throw new ArgumentException(string.Format(Properties.Resources.DefineDataUnsupportedCombinationOfDataTypesExceptionMessage,
incompatibleDataTypes));
var warning = string.Format(Properties.Resources.DefineDataUnsupportedCombinationOfDataTypesExceptionMessage, incompatibleDataTypes);
return DefineDataResult(inputValue, false, updateList, DataNodeDynamoTypeList.First(), warning);
}
var inputType = DataNodeDynamoTypeList.FirstOrDefault(x => x.Type == valueType, null);
if(inputType == null)
if (inputType == null)
{
// We couldn't find a Dynamo data type that fits, so we throw
throw new ArgumentException(string.Format(Properties.Resources.DefineDataUnsupportedDataTypeExceptionMessage,
valueType.Name));
var warning = string.Format(Properties.Resources.DefineDataUnsupportedDataTypeExceptionMessage, valueType.Name);
return DefineDataResult(inputValue, false, updateList, inputType, warning);

}
result = (IsValid: false, UpdateList: updateList, InputType: inputType);
return DefineDataResult(inputValue, false, updateList, inputType, string.Empty);
}
else
{
result = (IsValid: true, UpdateList: updateList, InputType: type);
return DefineDataResult(inputValue, true, updateList, type, string.Empty);
}

return new Dictionary<string, object>
{
{ ">", inputValue },
{ "Validation", result }
};
}
else
{
// If we are in 'Manual mode' then we just validate and throw as needed
var isSupportedType = IsSupportedDataNodeDynamoType(inputValue, type.Type, isList);
if (!isSupportedType)
{
throw new ArgumentException(string.Format(Properties.Resources.DefineDataUnexpectedInputExceptionMessage, type.Type.FullName,
inputValue.GetType().FullName));
var expectedType = GetStringTypeFromInput(type, isList);
var currentType = GetStringTypeFromInput(inputValue);
var warning = string.Format(Properties.Resources.DefineDataUnexpectedInputExceptionMessage, expectedType, currentType);
return DefineDataResult(inputValue, false, false, DataNodeDynamoTypeList.First(), warning);
}
result = (IsValid: isSupportedType, UpdateList: false, InputType: type);

return new Dictionary<string, object>
return DefineDataResult(inputValue, isSupportedType, false, type, string.Empty);

}
}

private static string GetStringTypeFromInput(object inputValue)
{
if (inputValue == null) return string.Empty;
try
{
if (inputValue is ArrayList || inputValue is IEnumerable)
{
{ ">", inputValue },
{ "Validation", result }
};
var values = ConcatenateUniqueDataTypes(inputValue);
return $"List of {values}";
}

return inputValue.GetType().FullName;
}
catch (Exception) { return string.Empty; }
}

private static string GetStringTypeFromInput(DataNodeDynamoType type, bool isList)
{
if (type == null) return string.Empty;

var typeFullName = type.Type.FullName;
return isList ? $"List of {typeFullName}" : typeFullName;
}

private static Dictionary<string, object> DefineDataResult(object inputValue, bool isSupportedType, bool updateList, DataNodeDynamoType type, string warning)
{
if(warning != string.Empty)
{
throw new Exception(warning);
}

var result = (IsValid: isSupportedType, UpdateList: updateList, InputType: type);
return new Dictionary<string, object>
{
{ ">", inputValue },
{ "Validation", result }
};
}

private static string ConcatenateUniqueDataTypes(object inputValue)
Expand Down Expand Up @@ -837,15 +871,12 @@ private static DataNodeDynamoType FindClosestCommonAncestor(List<DataNodeDynamoT
// Try to predict the likely ancestor as the single lowest-level node type
var likelyAncestor = LikelyAncestor(nodes);

var uniqueLevelNodes = nodes
.GroupBy(x => x.Level)
.Select(g => g.First())
.ToList();

foreach (var node in uniqueLevelNodes)
foreach (var node in nodes)
{
if (node.Type == likelyAncestor.Type) continue;
likelyAncestor = FindCommonAncestorBetweenTwoNodes(node, likelyAncestor);
if (node.Type == likelyAncestor.Type) continue; // skip self

// if at least one node type is not related to the likely ancestor, bail
likelyAncestor = FindCommonAncestorBetweenTwoNodes(node, likelyAncestor);

if (likelyAncestor == null) break;
}
Expand Down
52 changes: 50 additions & 2 deletions test/DynamoCoreTests/DSCoreDataTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,6 @@ public void ThrowsWhenPassedAnObjectThatCanNotSerialize()

[Test]
[Category("UnitTests")]
[Ignore("Temp ignore, fixed in the follow-up PR")]
public void IsNotSupportedNullInput()
{
object nullInput = null;
Expand All @@ -596,7 +595,6 @@ public void IsNotSupportedNullInput()

[Test]
[Category("UnitTests")]
[Ignore("Temp ignore, fixed in the follow-up PR")]
public void IsSupportedPrimitiveDataType()
{
var vString = "input string";
Expand Down Expand Up @@ -898,5 +896,55 @@ public void IsSupportedInheritanceDataType()
validate = DSCore.Data.IsSupportedDataNodeDynamoType(ivSurfaceInheritanceList, typeof(Surface), true);
Assert.AreEqual(false, validate, "Shouldn't validate DataTypes inheriting from Surface with Cylindar, Cuboid and Sphere in the list.");
}


[Test]
[Category("UnitTests")]
public void ThrowsCorrectErrorTypes()
{
// Arrange
var booleanString = typeof(bool).ToString();


// Unsupported IEnumerable input
var listInput = new List<dynamic>
{
"test",
10,
false
};
var ex = Assert.Throws<Exception>(() => DSCore.Data.IsSupportedDataNodeType(listInput, booleanString, true, true, ""));
Assert.That(ex.Message.Equals(DSCore.Properties.Resources.DefineDataSupportedInputValueExceptionMessage));


// Heterogenous list of mismatched types (no common ancestor)
var arrayListInput = new ArrayList()
{
"test",
10,
false
};
ex = Assert.Throws<Exception>(() => DSCore.Data.IsSupportedDataNodeType(arrayListInput, booleanString, true, true, ""));
Assert.That(ex.Message.Split("{")[0].StartsWith(DSCore.Properties.Resources.DefineDataUnsupportedCombinationOfDataTypesExceptionMessage.Split("{")[0]));


// Wrong DynamoPlayer input
var input = "Test";
var dynamoPlayerInput = Color.Red.ToString();
ex = Assert.Throws<Exception>(() => DSCore.Data.IsSupportedDataNodeType(input, booleanString, true, true, dynamoPlayerInput));
Assert.That(ex.Message.Equals(DSCore.Properties.Resources.Exception_Deserialize_Unsupported_Cache));


// Invalid input
var invalidTypeInput = Color.Red;
ex = Assert.Throws<Exception>(() => DSCore.Data.IsSupportedDataNodeType(invalidTypeInput, booleanString, true, true, ""));
Assert.That(ex.Message.Split("{")[0].StartsWith(DSCore.Properties.Resources.DefineDataUnsupportedDataTypeExceptionMessage.Split("{")[0]));


// Detect type - unexpected input
var detectTypeInput = 1;
ex = Assert.Throws<Exception>(() => DSCore.Data.IsSupportedDataNodeType(detectTypeInput, booleanString, false, false, ""));
Assert.That(ex.Message.Split("{")[0].StartsWith(DSCore.Properties.Resources.DefineDataUnexpectedInputExceptionMessage.Split("{")[0]));
}
}
}

0 comments on commit 979dcca

Please sign in to comment.