fix Dyn-6060 - duplicate python engine names (#14731)

mjkkirschner · web-flow · commit 469642fe15a6 · 2023-12-12T15:12:29.000-05:00
merging this and making a cherry pick.
diff --git a/src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs b/src/DynamoCoreWpf/ViewModels/Menu/PreferencesViewModel.cs
@@ -1256,7 +1256,7 @@ internal void HideUnhideNamespace(bool hide, string library, string namespc)
         private void AddPythonEnginesOptions()
         {
             var options = new ObservableCollection<string> { Res.DefaultPythonEngineNone };
-            foreach (var item in PythonEngineManager.Instance.AvailableEngines)
+            foreach (var item in PythonEngineManager.Instance.AvailableEngines.GroupBy(x=>x.Name).Select(g=>g.FirstOrDefault()).ToList())
             {
                 options.Add(item.Name);
             }
diff --git a/src/Libraries/PythonNodeModelsWpf/PythonNode.cs b/src/Libraries/PythonNodeModelsWpf/PythonNode.cs
@@ -1,4 +1,6 @@
 using System;
+using System.Collections;
+using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Linq;
 using System.Windows;
@@ -20,6 +22,36 @@
 
 namespace PythonNodeModelsWpf
 {
+    internal static class PythonNodeUtils
+    {
+        /// <summary>
+        /// Returns unique set of names from loaded and deserialized engines.
+        /// </summary>
+        /// <param name="nodeModel"></param>
+        /// <returns></returns>
+        internal static List<string> GetEngineNames(NodeModel nodeModel)
+        {
+            var engineName = string.Empty;
+            switch (nodeModel)
+            {
+                case PythonNode pyNode:
+                    engineName = pyNode.EngineName;
+                    break;
+                case PythonStringNode pyNode:
+                    engineName = pyNode.EngineName;
+                    break;
+            }
+
+            var availableEngineNames = PythonEngineManager.Instance.AvailableEngines.Select(x => x.Name).ToList();
+            // Add the serialized Python Engine even if it is missing (so that the user does not see an empty slot)
+            if (!availableEngineNames.Contains(engineName))
+            {
+                availableEngineNames.Add(engineName);
+            }
+            return availableEngineNames.Distinct().ToList();
+        }
+    }
+
     public class PythonNodeViewCustomization : VariableInputNodeViewCustomization, INodeViewCustomization<PythonNode>
     {       
         private DynamoViewModel dynamoViewModel;
@@ -61,13 +93,8 @@ public void CustomizeView(PythonNode nodeModel, NodeView nodeView)
 
             learnMoreItem.Click += OpenPythonLearningMaterial;
 
-            var availableEngineNames = PythonEngineManager.Instance.AvailableEngines.Select(x => x.Name).ToList();
-            // Add the serialized Python Engine even if it is missing (so that the user does not see an empty slot)
-            if (!availableEngineNames.Contains(nodeModel.EngineName))
-            {
-                availableEngineNames.Add(nodeModel.EngineName);
-            }
-            availableEngineNames.ForEach(x => AddPythonEngineToMenuItems(x));
+            PythonNodeUtils.GetEngineNames(nodeModel).ForEach(engineName => AddPythonEngineToMenuItems(
+                pythonNodeModel,pythonEngineVersionMenu,UpdateEngine,engineName));
 
             PythonEngineManager.Instance.AvailableEngines.CollectionChanged += PythonEnginesChanged;
 
@@ -89,6 +116,8 @@ public void CustomizeView(PythonNode nodeModel, NodeView nodeView)
             Grid.SetRow(engineLabel, 3);
         }
 
+       
+
         /// <summary>
         /// Check if the script editor is saved.
         /// </summary>
@@ -286,18 +315,29 @@ private void PythonEnginesChanged(object sender, NotifyCollectionChangedEventArg
             {
                 foreach (var item in e.NewItems)
                 {
-                    AddPythonEngineToMenuItems((item as PythonEngine).Name);
+                    if(item is PythonEngine newEngine)
+                    {
+                        AddPythonEngineToMenuItems(pythonNodeModel,pythonEngineVersionMenu,UpdateEngine,newEngine.Name);
+                    }
                 }   
             }
         }
 
         /// <summary>
-        /// Adds python engine to MenuItems
+        /// Adds python engine to MenuItems, if that engine name is not already present.
         /// </summary>
-        private void AddPythonEngineToMenuItems(string engineName)
+        internal static void AddPythonEngineToMenuItems(PythonNodeBase pythonNodeModel,
+            MenuItem pythonEngineVersionMenu,
+            RoutedEventHandler updateEngineDelegate,
+            string engineName)
         {
+         
+            if (pythonEngineVersionMenu.Items.Cast<MenuItem>().Any(x => x.Header as string == engineName))
+            {
+                return;
+            }
             var pythonEngineItem = new MenuItem { Header = engineName, IsCheckable = false };
-            pythonEngineItem.Click += UpdateEngine;
+            pythonEngineItem.Click += updateEngineDelegate;
             pythonEngineItem.SetBinding(MenuItem.IsCheckedProperty, new Binding(nameof(pythonNodeModel.EngineName))
             {
                 Source = pythonNodeModel,
diff --git a/src/Libraries/PythonNodeModelsWpf/PythonStringNode.cs b/src/Libraries/PythonNodeModelsWpf/PythonStringNode.cs
@@ -33,13 +33,8 @@ public void CustomizeView(PythonStringNode nodeModel, NodeView nodeView)
             pythonEngineVersionMenu = new MenuItem { Header = PythonNodeModels.Properties.Resources.PythonNodeContextMenuEngineSwitcher, IsCheckable = false };
             nodeView.MainContextMenu.Items.Add(pythonEngineVersionMenu);
 
-            var availableEngineNames = PythonEngineManager.Instance.AvailableEngines.Select(x => x.Name).ToList();
-            // Add the serialized Python Engine even if it is missing (so that the user does not see an empty slot)
-            if (!availableEngineNames.Contains(nodeModel.EngineName))
-            {
-                availableEngineNames.Add(nodeModel.EngineName);
-            }
-            availableEngineNames.ForEach(x => AddPythonEngineToMenuItems(x));
+            PythonNodeUtils.GetEngineNames(nodeModel).ForEach(engineName => PythonNodeViewCustomization.AddPythonEngineToMenuItems(
+                pythonStringNodeModel, pythonEngineVersionMenu, UpdateEngine, engineName));
 
             PythonEngineManager.Instance.AvailableEngines.CollectionChanged += PythonEnginesChanged;
 
@@ -107,28 +102,15 @@ private void UpdateEngine(object sender, EventArgs e)
         private void PythonEnginesChanged(object sender, NotifyCollectionChangedEventArgs e)
         {
             if (e.Action == NotifyCollectionChangedAction.Add)
-            {
                 foreach (var item in e.NewItems)
                 {
-                    AddPythonEngineToMenuItems((item as PythonEngine).Name);
+                    if (item is PythonEngine newEngine)
+                    {
+                        PythonNodeViewCustomization.AddPythonEngineToMenuItems(
+                            pythonStringNodeModel,pythonEngineVersionMenu,UpdateEngine,newEngine.Name);
+                    }
                 }
-            }
-        }
-
-        /// <summary>
-        /// Adds python engine to MenuItems
-        /// </summary>
-        private void AddPythonEngineToMenuItems(string engineName)
-        {
-            var pythonEngineItem = new MenuItem { Header = engineName, IsCheckable = false };
-            pythonEngineItem.Click += UpdateEngine;
-            pythonEngineItem.SetBinding(MenuItem.IsCheckedProperty, new Binding(nameof(pythonStringNodeModel.EngineName))
-            {
-                Source = pythonStringNodeModel,
-                Converter = new CompareToParameterConverter(),
-                ConverterParameter = engineName
-            });
-            pythonEngineVersionMenu.Items.Add(pythonEngineItem);
         }
     }
 }
+
diff --git a/src/NodeServices/PythonServices.cs b/src/NodeServices/PythonServices.cs
@@ -111,9 +111,11 @@ internal static readonly Lazy<PythonEngineManager>
         public static PythonEngineManager Instance { get { return lazy.Value; } }
         #endregion
 
+        //TODO see DYN-6550 when hiding/replacing this obsolete field.
         /// <summary>
         /// An observable collection of all the loaded Python engines
         /// </summary>
+        [Obsolete("AvailableEngines field will be replaced in a future Dynamo release.")]
         public ObservableCollection<PythonEngine> AvailableEngines;
 
         #region Constant strings
@@ -182,9 +184,9 @@ private void LoadDefaultPythonEngine(Assembly a)
             }
         }
 
-        private PythonEngine GetEngine(string version)
+        private PythonEngine GetEngine(string name)
         {
-            return AvailableEngines.FirstOrDefault(x => x.Name == version);
+            return AvailableEngines.FirstOrDefault(x => x.Name == name);
         }
 
         // This method can throw exceptions.
diff --git a/test/DynamoCoreWpfTests/PythonNodeCustomizationTests.cs b/test/DynamoCoreWpfTests/PythonNodeCustomizationTests.cs
@@ -1,15 +1,18 @@
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 using System.Windows;
 using System.Windows.Controls;
 using System.Windows.Controls.Primitives;
 using System.Windows.Input;
+using Autodesk.DesignScript.Runtime;
 using CoreNodeModels;
 using Dynamo.Controls;
 using Dynamo.DocumentationBrowser;
 using Dynamo.Graph.Workspaces;
 using Dynamo.PythonServices;
+using Dynamo.PythonServices.EventHandlers;
 using Dynamo.Tests;
 using Dynamo.Utilities;
 using Dynamo.ViewModels;
@@ -794,5 +797,50 @@ class NoTabs():
 
             Assert.IsTrue(foldings == 4);
         }
+
+        private class TestEngine : PythonEngine
+        {
+            public override object InputDataMarshaler => throw new NotImplementedException();
+
+            public override object OutputDataMarshaler => throw new NotImplementedException();
+
+            public override string Name => nameof(TestEngine);
+
+            public override event EvaluationStartedEventHandler EvaluationStarted;
+            public override event EvaluationFinishedEventHandler EvaluationFinished;
+
+            public override object Evaluate(string code, IList bindingNames, [ArbitraryDimensionArrayImport] IList bindingValues)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        [Test]
+        public void OnlyUniqueEnginesAddedToUI()
+        {
+            var node = new PythonNode();
+            Model.CurrentWorkspace.AddAndRegisterNode(node);
+            PythonEngineManager.Instance.AvailableEngines.Add(new TestEngine());
+            PythonEngineManager.Instance.AvailableEngines.Add(new TestEngine());
+            PythonEngineManager.Instance.AvailableEngines.Add(new TestEngine());
+            DispatcherUtil.DoEvents();
+            var nodeView = View.NodeViewsInFirstWorkspace().FirstOrDefault();
+            var menu = nodeView.MainContextMenu.Items.Cast<MenuItem>().First(x => (x.Header as string) == PythonNodeModels.Properties.Resources.PythonNodeContextMenuEngineSwitcher);
+            Assert.AreEqual(2, menu.Items.Count);
+        }
+        [Test]
+        public void OnlyUniqueEnginesAddedToUI2()
+        {
+            var node = new PythonNode();
+            Model.CurrentWorkspace.AddAndRegisterNode(node);
+            DispatcherUtil.DoEvents();
+            PythonEngineManager.Instance.AvailableEngines.Add(new TestEngine());
+            PythonEngineManager.Instance.AvailableEngines.Add(new TestEngine());
+            PythonEngineManager.Instance.AvailableEngines.Add(new TestEngine());
+       
+            var nodeView = View.NodeViewsInFirstWorkspace().FirstOrDefault();
+            var menu = nodeView.MainContextMenu.Items.Cast<MenuItem>().First(x => (x.Header as string) == PythonNodeModels.Properties.Resources.PythonNodeContextMenuEngineSwitcher);
+            Assert.AreEqual(2, menu.Items.Count);
+        }
     }
 }
diff --git a/test/Libraries/DynamoPythonTests/PythonEngineSelectorTests.cs b/test/Libraries/DynamoPythonTests/PythonEngineSelectorTests.cs
@@ -1,4 +1,4 @@
-﻿using System;
+using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Linq;

Original file line number	Diff line number	Diff line change
`@@ -1256,7 +1256,7 @@ internal void HideUnhideNamespace(bool hide, string library, string namespc)`
`1256`	`1256`	`private void AddPythonEnginesOptions()`
`1257`	`1257`	`{`
`1258`	`1258`	`var options = new ObservableCollection<string> { Res.DefaultPythonEngineNone };`
`1259`		`- foreach (var item in PythonEngineManager.Instance.AvailableEngines)`
	`1259`	`+ foreach (var item in PythonEngineManager.Instance.AvailableEngines.GroupBy(x=>x.Name).Select(g=>g.FirstOrDefault()).ToList())`
`1260`	`1260`	`{`
`1261`	`1261`	`options.Add(item.Name);`
`1262`	`1262`	`}`
Original file line number	Diff line number	Diff line change
`@@ -111,9 +111,11 @@ internal static readonly Lazy<PythonEngineManager>`
`111`	`111`	`public static PythonEngineManager Instance { get { return lazy.Value; } }`
`112`	`112`	`#endregion`
`113`	`113`
	`114`	`+ //TODO see DYN-6550 when hiding/replacing this obsolete field.`
`114`	`115`	`/// <summary>`
`115`	`116`	`/// An observable collection of all the loaded Python engines`
`116`	`117`	`/// </summary>`
	`118`	`+ [Obsolete("AvailableEngines field will be replaced in a future Dynamo release.")]`
`117`	`119`	`public ObservableCollection<PythonEngine> AvailableEngines;`
`118`	`120`
`119`	`121`	`#region Constant strings`
`@@ -182,9 +184,9 @@ private void LoadDefaultPythonEngine(Assembly a)`
`182`	`184`	`}`
`183`	`185`	`}`
`184`	`186`
`185`		`- private PythonEngine GetEngine(string version)`
	`187`	`+ private PythonEngine GetEngine(string name)`
`186`	`188`	`{`
`187`		`- return AvailableEngines.FirstOrDefault(x => x.Name == version);`
	`189`	`+ return AvailableEngines.FirstOrDefault(x => x.Name == name);`
`188`	`190`	`}`
`189`	`191`
`190`	`192`	`// This method can throw exceptions.`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-using System;`
	`1`	`+using System;`
`2`	`2`	`using System.Collections;`
`3`	`3`	`using System.Collections.Generic;`
`4`	`4`	`using System.Linq;`