diff --git a/src/Libraries/CoreNodeModels/DefineData.cs b/src/Libraries/CoreNodeModels/DefineData.cs index b71c5b28885..9e9e6ae6d8e 100644 --- a/src/Libraries/CoreNodeModels/DefineData.cs +++ b/src/Libraries/CoreNodeModels/DefineData.cs @@ -30,7 +30,6 @@ public class DefineData : DSDropDownBase private List serializedItems; private bool isAutoMode = true; // default start with auto-detect 'on' private bool isList; - private string playerValue = ""; private string displayValue = Properties.Resources.DefineDataDisplayValueMessage; /// @@ -99,16 +98,23 @@ public override bool IsInputNode get { return true; } } - [JsonIgnore] - public string PlayerValue + + private string value = ""; + + [JsonProperty("InputValue")] + public virtual string Value { - get { return playerValue; } + get + { + return value; + } set { - if (Equals(this.playerValue, null) || !this.playerValue.Equals(value)) + if (Equals(this.value, null) || !this.value.Equals(value)) { - playerValue = value ?? ""; + this.value = value ?? ""; MarkNodeAsModified(); + RaisePropertyChanged(nameof(Value)); } } } @@ -120,8 +126,6 @@ public DefineData() : base(">") { PropertyChanged += OnPropertyChanged; - //Items.Add(new DynamoDropDownItem("Select a type", null)); - foreach (var dataType in Data.DataNodeDynamoTypeList) { var displayName = dataType.Name; @@ -158,11 +162,8 @@ public override void Dispose() DataBridge.Instance.UnregisterCallback(GUID.ToString()); } - private static readonly string BuiltinDictionaryTypeName = typeof(DesignScript.Builtin.Dictionary).FullName; - private static readonly string BuiltinDictionaryGet = nameof(DesignScript.Builtin.Dictionary.ValueAtKey); - public override IEnumerable BuildOutputAst(List inputAstNodes) - { + { var resultAst = new List(); // function call inputs - reference to the function, and the function arguments coming from the inputs @@ -170,28 +171,29 @@ public override IEnumerable BuildOutputAst(List>(DSCore.Data.IsSupportedDataNodeType); - var funtionInputs = new List { + var functionInputs = new List { inputAstNodes[0], AstFactory.BuildStringNode((Items[SelectedIndex].Item as Data.DataNodeDynamoType).Type.ToString()), AstFactory.BuildBooleanNode(IsList), AstFactory.BuildBooleanNode(IsAutoMode), - AstFactory.BuildStringNode(PlayerValue) + AstFactory.BuildStringNode(Value) }; - var functionCall = AstFactory.BuildFunctionCall(function, funtionInputs); + var functionCall = AstFactory.BuildFunctionCall(function, functionInputs); var functionCallIdentifier = AstFactory.BuildIdentifier(GUID + "_func"); resultAst.Add(AstFactory.BuildAssignment(functionCallIdentifier, functionCall)); - //Next add the first key value pair to the output port - var getFirstKey = AstFactory.BuildFunctionCall(BuiltinDictionaryTypeName, BuiltinDictionaryGet, - new List { functionCallIdentifier, AstFactory.BuildStringNode(">") }); + // Next add the first key value pair to the output port + var safeExtractDictionaryValue = new Func, string, object>(DSCore.Data.SafeExtractDictionaryValue); + var getFirstKey = AstFactory.BuildFunctionCall(safeExtractDictionaryValue, + [functionCallIdentifier, AstFactory.BuildStringNode(">")]); resultAst.Add(AstFactory.BuildAssignment(GetAstIdentifierForOutputIndex(0), getFirstKey)); - //Second get the key value pair to pass to the databridge callback - var getSecondKey = AstFactory.BuildFunctionCall(BuiltinDictionaryTypeName, BuiltinDictionaryGet, - new List { functionCallIdentifier, AstFactory.BuildStringNode("Validation") }); + // Second get the key value pair to pass to the databridge callback + var getSecondKey = AstFactory.BuildFunctionCall(safeExtractDictionaryValue, + [functionCallIdentifier, AstFactory.BuildStringNode("Validation")]); resultAst.Add(AstFactory.BuildAssignment( AstFactory.BuildIdentifier(GUID + "_db"), @@ -201,17 +203,13 @@ public override IEnumerable BuildOutputAst(List - /// Not sure at the moment how relevant is the databridge for this node type - /// - /// private void DataBridgeCallback(object data) { //Todo If the playerValue is not empty string then we can chanage the UI to reflect the value is coming from the player //Todo if the function call throws we don't get back to DatabridgeCallback. Not sure if we need to handle this case //Now we reset this value to empty string so that the next time a value is set from upstream nodes we can know that it is not coming from the player - playerValue = ""; + value = ""; // If data is null if (data == null) @@ -219,7 +217,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 { @@ -290,7 +287,7 @@ protected override bool UpdateValueCore(UpdateValueParams updateValueParams) switch (name) { case "Value": - PlayerValue = value; + Value = value; return true; // UpdateValueCore handled. } diff --git a/src/Libraries/CoreNodeModelsWpf/NodeViewCustomizations/DefineData.cs b/src/Libraries/CoreNodeModelsWpf/NodeViewCustomizations/DefineData.cs index 68a15726159..101a28e6d9b 100644 --- a/src/Libraries/CoreNodeModelsWpf/NodeViewCustomizations/DefineData.cs +++ b/src/Libraries/CoreNodeModelsWpf/NodeViewCustomizations/DefineData.cs @@ -161,7 +161,6 @@ public void CustomizeView(DefineData model, NodeView nodeView) }; selectedItemDisplay.SetBinding(TextBox.WidthProperty, widthBinding); - // Move the ComboBox to the placeholder var placeholderText = formControl.FindName("TextPlaceholder") as TextBox; if (placeholderText != null) @@ -189,6 +188,11 @@ public void CustomizeView(DefineData model, NodeView nodeView) dropdown.DropDownClosed -= dropDown_DropDownClosed; } + if (selectedItemDisplay != null) + { + selectedItemDisplay.IsEnabledChanged -= selectedItemDisplay_IsEnabledChanged; + } + if (listToggleButton != null) { listToggleButton.Click -= listToggle_IsClicked; @@ -217,6 +221,14 @@ private void dropDown_DropDownClosed(object sender, EventArgs e) } } + private void selectedItemDisplay_IsEnabledChanged(object sender, DependencyPropertyChangedEventArgs e) + { + if (sender is TextBox textBox) + { + textBox.MinWidth = textBox.IsEnabled ? 200 : 220; + } + } + private void listToggle_IsClicked(object sender, RoutedEventArgs e) { if (modeToggleButton != null && modeToggleButton.IsChecked == true) diff --git a/src/Libraries/CoreNodes/Data.cs b/src/Libraries/CoreNodes/Data.cs index 6d9758afc14..0064f77b1b1 100644 --- a/src/Libraries/CoreNodes/Data.cs +++ b/src/Libraries/CoreNodes/Data.cs @@ -648,6 +648,23 @@ static Data() DataNodeDynamoTypeList = new ReadOnlyCollection(typeList); } + /// + /// A helper function to safely extract a dictionary value + /// + /// The dictionary to extract the value from + /// The key of the key/value pair + /// + [IsVisibleInDynamoLibrary(false)] + public static object SafeExtractDictionaryValue(Dictionary dict, string key) + { + if (dict?.TryGetValue(key, out var value) == true) + { + return value; + } + + return null; + } + /// /// This is the function used by AST /// Handles some of the the node logic while performing the validation @@ -656,11 +673,13 @@ static Data() /// The Type as string (it would be better to pass an object of type 'Type' for direct type comparison) /// If the input is of type `ArrayList` /// If the node is in Auto mode + /// The value coming from Dynamo Player /// [IsVisibleInDynamoLibrary(false)] public static Dictionary 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 @@ -673,7 +692,8 @@ public static Dictionary 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. @@ -687,19 +707,18 @@ public static Dictionary 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 - // 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; @@ -716,28 +735,23 @@ public static Dictionary 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 - { - { ">", inputValue }, - { "Validation", result } - }; } else { @@ -745,17 +759,54 @@ public static Dictionary IsSupportedDataNodeType([ArbitraryDimen 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 + 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 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 + { + { ">", inputValue }, + { "Validation", result } + }; } private static string ConcatenateUniqueDataTypes(object inputValue) @@ -837,15 +888,12 @@ private static DataNodeDynamoType FindClosestCommonAncestor(List 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; } diff --git a/test/DynamoCoreTests/DSCoreDataTests.cs b/test/DynamoCoreTests/DSCoreDataTests.cs index 1023a290ebb..82f6cacddbf 100644 --- a/test/DynamoCoreTests/DSCoreDataTests.cs +++ b/test/DynamoCoreTests/DSCoreDataTests.cs @@ -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; @@ -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"; @@ -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 + { + "test", + 10, + false + }; + var ex = Assert.Throws(() => 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(() => 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(() => 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(() => 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(() => DSCore.Data.IsSupportedDataNodeType(detectTypeInput, booleanString, false, false, "")); + Assert.That(ex.Message.Split("{")[0].StartsWith(DSCore.Properties.Resources.DefineDataUnexpectedInputExceptionMessage.Split("{")[0])); + } } } diff --git a/test/DynamoCoreWpfTests/CustomNodeTests.cs b/test/DynamoCoreWpfTests/CustomNodeTests.cs index 53fa2a64e88..322970a1ea7 100644 --- a/test/DynamoCoreWpfTests/CustomNodeTests.cs +++ b/test/DynamoCoreWpfTests/CustomNodeTests.cs @@ -107,7 +107,7 @@ public void TestDataInputInitializationTest() // Assert - default node values Assert.AreEqual(node.SelectedString, supportedDynamoTypesList.First().Name); Assert.AreEqual(node.DisplayValue, CoreNodeModels.Properties.Resources.DefineDataDisplayValueMessage); - Assert.AreEqual(node.PlayerValue, string.Empty); + Assert.AreEqual(node.Value, string.Empty); Assert.IsTrue(node.IsAutoMode); // default value of automode is now true Assert.IsFalse(node.IsList); }