[dotnet] Gracefully handle clashing device names in Actions (#14713)

Author: RenderMichael

Diff for: dotnet/src/webdriver/Interactions/ActionSequence.cs

@@ -29,7 +29,6 @@ namespace OpenQA.Selenium.Interactions
     public class ActionSequence
     {
         private List<Interaction> interactions = new List<Interaction>();
-        private InputDevice device;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ActionSequence"/> class.
@@ -52,7 +51,7 @@ public ActionSequence(InputDevice device, int initialSize)
                 throw new ArgumentNullException(nameof(device), "Input device cannot be null.");
             }
 
-            this.device = device;
+            this.InputDevice = device;
 
             for (int i = 0; i < initialSize; i++)
             {
@@ -71,10 +70,13 @@ public int Count
         /// <summary>
         /// Gets the input device for this Action sequence.
         /// </summary>
-        public InputDevice inputDevice
-        {
-            get { return this.inputDevice; }
-        }
+        [Obsolete("This property has been renamed to InputDevice and will be removed in a future version")]
+        public InputDevice inputDevice => InputDevice;
+
+        /// <summary>
+        /// Gets the input device for this Action sequence.
+        /// </summary>
+        public InputDevice InputDevice { get; }
 
         /// <summary>
         /// Adds an action to the sequence.
@@ -88,9 +90,9 @@ public ActionSequence AddAction(Interaction interactionToAdd)
                 throw new ArgumentNullException(nameof(interactionToAdd), "Interaction to add to sequence must not be null");
             }
 
-            if (!interactionToAdd.IsValidFor(this.device.DeviceKind))
+            if (!interactionToAdd.IsValidFor(this.InputDevice.DeviceKind))
             {
-                throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Interaction {0} is invalid for device type {1}.", interactionToAdd.GetType(), this.device.DeviceKind), nameof(interactionToAdd));
+                throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "Interaction {0} is invalid for device type {1}.", interactionToAdd.GetType(), this.InputDevice.DeviceKind), nameof(interactionToAdd));
             }
 
             this.interactions.Add(interactionToAdd);
@@ -103,7 +105,7 @@ public ActionSequence AddAction(Interaction interactionToAdd)
         /// <returns>A <see cref="Dictionary{TKey, TValue}"/> containing the actions in this sequence.</returns>
         public Dictionary<string, object> ToDictionary()
         {
-            Dictionary<string, object> toReturn = this.device.ToDictionary();
+            Dictionary<string, object> toReturn = this.InputDevice.ToDictionary();
 
             List<object> encodedActions = new List<object>();
             foreach (Interaction action in this.interactions)
@@ -122,7 +124,7 @@ public Dictionary<string, object> ToDictionary()
         /// <returns>A string that represents the current <see cref="ActionSequence"/>.</returns>
         public override string ToString()
         {
-            StringBuilder builder = new StringBuilder("Action sequence - ").Append(this.device.ToString());
+            StringBuilder builder = new StringBuilder("Action sequence - ").Append(this.InputDevice.ToString());
             foreach (Interaction action in this.interactions)
             {
                 builder.AppendLine();

Diff for: dotnet/src/webdriver/Interactions/Actions.cs

@@ -31,23 +31,23 @@ public class Actions : IAction
         private PointerInputDevice activePointer;
         private KeyInputDevice activeKeyboard;
         private WheelInputDevice activeWheel;
-        private IActionExecutor actionExecutor;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Actions"/> class.
         /// </summary>
         /// <param name="driver">The <see cref="IWebDriver"/> object on which the actions built will be performed.</param>
+        /// <exception cref="ArgumentException">If <paramref name="driver"/> does not implement <see cref="IActionExecutor"/>.</exception>
         public Actions(IWebDriver driver)
             : this(driver, TimeSpan.FromMilliseconds(250))
         {
-
         }
 
         /// <summary>
         /// Initializes a new instance of the <see cref="Actions"/> class.
         /// </summary>
         /// <param name="driver">The <see cref="IWebDriver"/> object on which the actions built will be performed.</param>
         /// <param name="duration">How long durable action is expected to take.</param>
+        /// <exception cref="ArgumentException">If <paramref name="driver"/> does not implement <see cref="IActionExecutor"/>.</exception>
         public Actions(IWebDriver driver, TimeSpan duration)
         {
             IActionExecutor actionExecutor = GetDriverAs<IActionExecutor>(driver);
@@ -56,52 +56,33 @@ public Actions(IWebDriver driver, TimeSpan duration)
                 throw new ArgumentException("The IWebDriver object must implement or wrap a driver that implements IActionExecutor.", nameof(driver));
             }
 
-            this.actionExecutor = actionExecutor;
+            this.ActionExecutor = actionExecutor;
 
             this.duration = duration;
         }
 
         /// <summary>
         /// Returns the <see cref="IActionExecutor"/> for the driver.
         /// </summary>
-        protected IActionExecutor ActionExecutor
-        {
-            get { return this.actionExecutor; }
-        }
+        protected IActionExecutor ActionExecutor { get; }
 
         /// <summary>
         /// Sets the active pointer device for this Actions class.
         /// </summary>
         /// <param name="kind">The kind of pointer device to set as active.</param>
         /// <param name="name">The name of the pointer device to set as active.</param>
         /// <returns>A self-reference to this Actions class.</returns>
+        /// <exception cref="InvalidOperationException">If a device with this name exists but is not a pointer.</exception>
         public Actions SetActivePointer(PointerKind kind, string name)
         {
-            IList<ActionSequence> sequences = this.actionBuilder.ToActionSequenceList();
+            InputDevice device = FindDeviceById(name);
 
-            InputDevice device = null;
-
-            foreach (var sequence in sequences)
+            this.activePointer = device switch
             {
-                Dictionary<string, object> actions = sequence.ToDictionary();
-
-                string id = (string)actions["id"];
-
-                if (id == name)
-                {
-                    device = sequence.inputDevice;
-                    break;
-                }
-            }
-
-            if (device == null)
-            {
-                this.activePointer = new PointerInputDevice(kind, name);
-            }
-            else
-            {
-                this.activePointer = (PointerInputDevice)device;
-            }
+                null => new PointerInputDevice(kind, name),
+                PointerInputDevice pointerDevice => pointerDevice,
+                _ => throw new InvalidOperationException($"Device under the name \"{name}\" is not a pointer. Actual input type: {device.DeviceKind}"),
+            };
 
             return this;
         }
@@ -111,33 +92,17 @@ public Actions SetActivePointer(PointerKind kind, string name)
         /// </summary>
         /// <param name="name">The name of the keyboard device to set as active.</param>
         /// <returns>A self-reference to this Actions class.</returns>
+        /// <exception cref="InvalidOperationException">If a device with this name exists but is not a keyboard.</exception>
         public Actions SetActiveKeyboard(string name)
         {
-            IList<ActionSequence> sequences = this.actionBuilder.ToActionSequenceList();
+            InputDevice device = FindDeviceById(name);
 
-            InputDevice device = null;
-
-            foreach (var sequence in sequences)
+            this.activeKeyboard = device switch
             {
-                Dictionary<string, object> actions = sequence.ToDictionary();
-
-                string id = (string)actions["id"];
-
-                if (id == name)
-                {
-                    device = sequence.inputDevice;
-                    break;
-                }
-            }
-
-            if (device == null)
-            {
-                this.activeKeyboard = new KeyInputDevice(name);
-            }
-            else
-            {
-                this.activeKeyboard = (KeyInputDevice)device;
-            }
+                null => new KeyInputDevice(name),
+                KeyInputDevice keyDevice => keyDevice,
+                _ => throw new InvalidOperationException($"Device under the name \"{name}\" is not a keyboard. Actual input type: {device.DeviceKind}"),
+            };
 
             return this;
         }
@@ -147,35 +112,36 @@ public Actions SetActiveKeyboard(string name)
         /// </summary>
         /// <param name="name">The name of the wheel device to set as active.</param>
         /// <returns>A self-reference to this Actions class.</returns>
+        /// <exception cref="InvalidOperationException">If a device with this name exists but is not a wheel.</exception>
         public Actions SetActiveWheel(string name)
         {
-            IList<ActionSequence> sequences = this.actionBuilder.ToActionSequenceList();
+            InputDevice device = FindDeviceById(name);
+
+            this.activeWheel = device switch
+            {
+                null => new WheelInputDevice(name),
+                WheelInputDevice wheelDevice => wheelDevice,
+                _ => throw new InvalidOperationException($"Device under the name \"{name}\" is not a wheel. Actual input type: {device.DeviceKind}"),
+            };
 
-            InputDevice device = null;
+            return this;
+        }
 
-            foreach (var sequence in sequences)
+        private InputDevice FindDeviceById(string name)
+        {
+            foreach (var sequence in this.actionBuilder.ToActionSequenceList())
             {
                 Dictionary<string, object> actions = sequence.ToDictionary();
 
                 string id = (string)actions["id"];
 
                 if (id == name)
                 {
-                    device = sequence.inputDevice;
-                    break;
+                    return sequence.inputDevice;
                 }
             }
 
-            if (device == null)
-            {
-                this.activeWheel = new WheelInputDevice(name);
-            }
-            else
-            {
-                this.activeWheel = (WheelInputDevice)device;
-            }
-
-            return this;
+            return null;
         }
 
         /// <summary>
@@ -619,7 +585,7 @@ public IAction Build()
         /// </summary>
         public void Perform()
         {
-            this.actionExecutor.PerformActions(this.actionBuilder.ToActionSequenceList());
+            this.ActionExecutor.PerformActions(this.actionBuilder.ToActionSequenceList());
             this.actionBuilder.ClearSequences();
         }
 

Diff for: dotnet/test/common/Interactions/CombinedInputActionsTest.cs

@@ -397,6 +397,37 @@ public void PerformsPause()
             Assert.IsTrue(DateTime.Now - start > TimeSpan.FromMilliseconds(1200));
         }
 
+        [Test]
+        public void ShouldHandleClashingDeviceNamesGracefully()
+        {
+            var actionsWithPointer = new Actions(driver)
+                .SetActivePointer(PointerKind.Mouse, "test")
+                .Click();
+
+            Assert.That(() =>
+            {
+                actionsWithPointer.SetActiveWheel("test");
+            }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a wheel. Actual input type: Pointer"));
+
+            var actionsWithKeyboard = new Actions(driver)
+                .SetActiveKeyboard("test")
+                .KeyDown(Keys.Shift).KeyUp(Keys.Shift);
+
+            Assert.That(() =>
+            {
+                actionsWithKeyboard.SetActivePointer(PointerKind.Pen, "test");
+            }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a pointer. Actual input type: Key"));
+
+            var actionsWithWheel = new Actions(driver)
+               .SetActiveWheel("test")
+               .ScrollByAmount(0, 0);
+
+            Assert.That(() =>
+            {
+                actionsWithWheel.SetActiveKeyboard("test");
+            }, Throws.InvalidOperationException.With.Message.EqualTo("Device under the name \"test\" is not a keyboard. Actual input type: Wheel"));
+        }
+
         private bool FuzzyPositionMatching(int expectedX, int expectedY, string locationTuple)
         {
             string[] splitString = locationTuple.Split(',');