Bug 1529291 - Make WebElement a unit struct. r=ato

Differential Revision: https://phabricator.services.mozilla.com/D21917

--HG--
extra : moz-landing-system : lando

testing/geckodriver/src/command.rs

@@ -87,7 +87,7 @@ impl WebDriverExtensionRoute for GeckoExtensionRoute {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().to_string());
+                let element = WebElement(element_id.as_str().to_string());
                 GeckoExtensionCommand::XblAnonymousChildren(element)
             }
             XblAnonymousByAttribute => {
@@ -97,7 +97,7 @@ impl WebDriverExtensionRoute for GeckoExtensionRoute {
                     "Missing elementId parameter"
                 );
                 GeckoExtensionCommand::XblAnonymousByAttribute(
-                    WebElement::new(element_id.as_str().into()),
+                    WebElement(element_id.as_str().into()),
                     serde_json::from_value(body_data.clone())?,
                 )
             }

testing/geckodriver/src/marionette.rs

@@ -407,7 +407,7 @@ impl MarionetteSession {
             ErrorStatus::UnknownError,
             "Failed to convert web element reference value to string"
         ).to_string();
-        Ok(WebElement::new(id))
+        Ok(WebElement(id))
     }
 
     pub fn next_command_id(&mut self) -> u64 {
@@ -828,7 +828,7 @@ impl MarionetteCommand {
             ElementClick(ref x) => (Some("WebDriver:ElementClick"), Some(x.to_marionette())),
             ElementSendKeys(ref e, ref x) => {
                 let mut data = Map::new();
-                data.insert("id".to_string(), Value::String(e.id.clone()));
+                data.insert("id".to_string(), Value::String(e.to_string()));
                 data.insert("text".to_string(), Value::String(x.text.clone()));
                 data.insert(
                     "value".to_string(),
@@ -849,13 +849,13 @@ impl MarionetteCommand {
             FindElement(ref x) => (Some("WebDriver:FindElement"), Some(x.to_marionette())),
             FindElementElement(ref e, ref x) => {
                 let mut data = x.to_marionette()?;
-                data.insert("element".to_string(), Value::String(e.id.clone()));
+                data.insert("element".to_string(), Value::String(e.to_string()));
                 (Some("WebDriver:FindElement"), Some(Ok(data)))
             }
             FindElements(ref x) => (Some("WebDriver:FindElements"), Some(x.to_marionette())),
             FindElementElements(ref e, ref x) => {
                 let mut data = x.to_marionette()?;
-                data.insert("element".to_string(), Value::String(e.id.clone()));
+                data.insert("element".to_string(), Value::String(e.to_string()));
                 (Some("WebDriver:FindElements"), Some(Ok(data)))
             }
             FullscreenWindow => (Some("WebDriver:FullscreenWindow"), None),
@@ -866,19 +866,19 @@ impl MarionetteCommand {
             GetCurrentUrl => (Some("WebDriver:GetCurrentURL"), None),
             GetCSSValue(ref e, ref x) => {
                 let mut data = Map::new();
-                data.insert("id".to_string(), Value::String(e.id.clone()));
+                data.insert("id".to_string(), Value::String(e.to_string()));
                 data.insert("propertyName".to_string(), Value::String(x.clone()));
                 (Some("WebDriver:GetElementCSSValue"), Some(Ok(data)))
             }
             GetElementAttribute(ref e, ref x) => {
                 let mut data = Map::new();
-                data.insert("id".to_string(), Value::String(e.id.clone()));
+                data.insert("id".to_string(), Value::String(e.to_string()));
                 data.insert("name".to_string(), Value::String(x.clone()));
                 (Some("WebDriver:GetElementAttribute"), Some(Ok(data)))
             }
             GetElementProperty(ref e, ref x) => {
                 let mut data = Map::new();
-                data.insert("id".to_string(), Value::String(e.id.clone()));
+                data.insert("id".to_string(), Value::String(e.to_string()));
                 data.insert("name".to_string(), Value::String(x.clone()));
                 (Some("WebDriver:GetElementProperty"), Some(Ok(data)))
             }
@@ -938,7 +938,7 @@ impl MarionetteCommand {
             SwitchToWindow(ref x) => (Some("WebDriver:SwitchToWindow"), Some(x.to_marionette())),
             TakeElementScreenshot(ref e) => {
                 let mut data = Map::new();
-                data.insert("id".to_string(), Value::String(e.id.clone()));
+                data.insert("id".to_string(), Value::String(e.to_string()));
                 data.insert("highlights".to_string(), Value::Array(vec![]));
                 data.insert("full".to_string(), Value::Bool(false));
                 (Some("WebDriver:TakeScreenshot"), Some(Ok(data)))
@@ -963,14 +963,14 @@ impl MarionetteCommand {
                 }
                 XblAnonymousByAttribute(e, x) => {
                     let mut data = x.to_marionette()?;
-                    data.insert("element".to_string(), Value::String(e.id.clone()));
+                    data.insert("element".to_string(), Value::String(e.to_string()));
                     (Some("WebDriver:FindElement"), Some(Ok(data)))
                 }
                 XblAnonymousChildren(e) => {
                     let mut data = Map::new();
                     data.insert("using".to_owned(), serde_json::to_value("anon")?);
                     data.insert("value".to_owned(), Value::Null);
-                    data.insert("element".to_string(), serde_json::to_value(e.id.clone())?);
+                    data.insert("element".to_string(), serde_json::to_value(e.to_string())?);
                     (Some("WebDriver:FindElements"), Some(Ok(data)))
                 }
                 TakeFullScreenshot => {
@@ -1458,14 +1458,13 @@ impl ToMarionette for NewWindowParameters {
 impl ToMarionette for SwitchToFrameParameters {
     fn to_marionette(&self) -> WebDriverResult<Map<String, Value>> {
         let mut data = Map::new();
-        let key = match self.id {
+        match self.id {
             None => None,
-            Some(FrameId::Short(_)) => Some("id"),
-            Some(FrameId::Element(_)) => Some("element"),
+            Some(FrameId::Short(_)) => data.insert("id".to_string(),
+                                            serde_json::to_value(&self.id)?),
+            Some(FrameId::Element(ref web_element)) => data.insert("element".to_string(),
+                                            serde_json::to_value(web_element.to_string())?),
         };
-        if let Some(x) = key {
-            data.insert(x.to_string(), serde_json::to_value(&self.id)?);
-        }
         Ok(data)
     }
 }
@@ -1494,7 +1493,7 @@ impl ToMarionette for TimeoutsParameters {
 impl ToMarionette for WebElement {
     fn to_marionette(&self) -> WebDriverResult<Map<String, Value>> {
         let mut data = Map::new();
-        data.insert("id".to_string(), serde_json::to_value(&self.id)?);
+        data.insert("id".to_string(), serde_json::to_value(self.to_string())?);
         Ok(data)
     }
 }

testing/webdriver/src/actions.rs

@@ -199,7 +199,7 @@ impl<'de> Deserialize<'de> for PointerOrigin {
         let value = Value::deserialize(deserializer)?;
         if let Some(web_element) = value.get(ELEMENT_KEY) {
             String::deserialize(web_element)
-                .map(|id| PointerOrigin::Element(WebElement { id }))
+                .map(|id| PointerOrigin::Element(WebElement(id)))
                 .map_err(de::Error::custom)
         } else if value == "pointer" {
             Ok(PointerOrigin::Pointer)
@@ -218,7 +218,7 @@ fn serialize_webelement_id<S>(element: &WebElement, serializer: S) -> Result<S::
 where
     S: Serializer,
 {
-    element.id.serialize(serializer)
+    element.to_string().serialize(serializer)
 }
 
 fn deserialize_to_option_i64<'de, D>(deserializer: D) -> Result<Option<i64>, D::Error>
@@ -915,7 +915,7 @@ mod test {
         }"#;
         let data = PointerAction::Move(PointerMoveAction {
             duration: Some(100),
-            origin: PointerOrigin::Element(WebElement { id: "elem".into() }),
+            origin: PointerOrigin::Element(WebElement("elem".into())),
             x: Some(5),
             y: Some(10),
         });
@@ -936,7 +936,7 @@ mod test {
         }"#;
         let data = PointerAction::Move(PointerMoveAction {
             duration: Some(100),
-            origin: PointerOrigin::Element(WebElement { id: "elem".into() }),
+            origin: PointerOrigin::Element(WebElement("elem".into())),
             x: Some(5),
             y: Some(10),
         });
@@ -1116,7 +1116,7 @@ mod test {
     #[test]
     fn test_json_pointer_origin_web_element() {
         let json = r#"{"element-6066-11e4-a52e-4f735466cecf":"elem"}"#;
-        let data = PointerOrigin::Element(WebElement { id: "elem".into() });
+        let data = PointerOrigin::Element(WebElement("elem".into()));
 
         check_serialize_deserialize(&json, &data);
     }

testing/webdriver/src/command.rs

@@ -149,7 +149,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::FindElementElement(element, serde_json::from_str(raw_body)?)
             }
             Route::FindElementElements => {
@@ -158,7 +158,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::FindElementElements(element, serde_json::from_str(raw_body)?)
             }
             Route::GetActiveElement => WebDriverCommand::GetActiveElement,
@@ -168,7 +168,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::IsDisplayed(element)
             }
             Route::IsSelected => {
@@ -177,7 +177,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::IsSelected(element)
             }
             Route::GetElementAttribute => {
@@ -186,7 +186,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 let attr = try_opt!(
                     params.name("name"),
                     ErrorStatus::InvalidArgument,
@@ -200,7 +200,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 let property = try_opt!(
                     params.name("name"),
                     ErrorStatus::InvalidArgument,
@@ -214,7 +214,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 let property = try_opt!(
                     params.name("propertyName"),
                     ErrorStatus::InvalidArgument,
@@ -228,7 +228,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::GetElementText(element)
             }
             Route::GetElementTagName => {
@@ -237,7 +237,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::GetElementTagName(element)
             }
             Route::GetElementRect => {
@@ -246,7 +246,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::GetElementRect(element)
             }
             Route::IsEnabled => {
@@ -255,7 +255,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::IsEnabled(element)
             }
             Route::ElementClick => {
@@ -264,7 +264,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::ElementClick(element)
             }
             Route::ElementClear => {
@@ -273,7 +273,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::ElementClear(element)
             }
             Route::ElementSendKeys => {
@@ -282,7 +282,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::ElementSendKeys(element, serde_json::from_str(raw_body)?)
             }
             Route::ExecuteScript => {
@@ -329,7 +329,7 @@ impl<U: WebDriverExtensionRoute> WebDriverMessage<U> {
                     ErrorStatus::InvalidArgument,
                     "Missing elementId parameter"
                 );
-                let element = WebElement::new(element_id.as_str().into());
+                let element = WebElement(element_id.as_str().into());
                 WebDriverCommand::TakeElementScreenshot(element)
             }
             Route::Status => WebDriverCommand::Status,
@@ -1197,7 +1197,7 @@ mod tests {
     fn test_json_take_screenshot_parameters_with_element() {
         let json = r#"{"element":{"element-6066-11e4-a52e-4f735466cecf":"elem"}}"#;
         let data = TakeScreenshotParameters {
-            element: Some(WebElement::new("elem".into())),
+            element: Some(WebElement("elem".into())),
         };
 
         check_deserialize(&json, &data);
@@ -1229,7 +1229,7 @@ mod tests {
     fn test_json_take_screenshot_parameters_with_unknown_field() {
         let json = r#"{"element":{"element-6066-11e4-a52e-4f735466cecf":"elem"},"foo":"bar"}"#;
         let data = TakeScreenshotParameters {
-            element: Some(WebElement::new("elem".into())),
+            element: Some(WebElement("elem".into())),
         };
 
         check_deserialize(&json, &data);

testing/webdriver/src/common.rs

@@ -1,4 +1,5 @@
 use serde::ser::{Serialize, Serializer};
+use serde::{Deserialize, Deserializer};
 
 pub static ELEMENT_KEY: &'static str = "element-6066-11e4-a52e-4f735466cecf";
 pub static FRAME_KEY: &'static str = "frame-075b-4da1-b6ba-e579c2d3230a";
@@ -38,7 +39,7 @@ fn serialize_webelement_id<S>(element: &WebElement, serializer: S) -> Result<S::
 where
     S: Serializer,
 {
-    element.id.serialize(serializer)
+    element.serialize(serializer)
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)]
@@ -55,15 +56,37 @@ pub enum LocatorStrategy {
     XPath,
 }
 
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
-pub struct WebElement {
+#[derive(Clone, Debug, PartialEq)]
+pub struct WebElement(pub String);
+
+// private
+#[derive(Serialize, Deserialize)]
+struct WebElementObject {
     #[serde(rename = "element-6066-11e4-a52e-4f735466cecf")]
-    pub id: String,
+    id: String,
+}
+
+impl Serialize for WebElement {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        WebElementObject { id: self.0.clone() }.serialize(serializer)
+    }
+}
+
+impl<'de> Deserialize<'de> for WebElement {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        Deserialize::deserialize(deserializer).map(|WebElementObject { id }| WebElement(id))
+    }
 }
 
 impl WebElement {
-    pub fn new(id: String) -> WebElement {
-        WebElement { id }
+    pub fn to_string(&self) -> String {
+        self.0.clone()
     }
 }
 
@@ -97,8 +120,8 @@ mod tests {
 
     #[test]
     fn test_json_frame_id_webelement() {
-        let json = r#""elem""#;
-        let data = FrameId::Element(WebElement::new("elem".into()));
+        let json = r#"{"element-6066-11e4-a52e-4f735466cecf":"elem"}"#;
+        let data = FrameId::Element(WebElement("elem".into()));
 
         check_serialize(&json, &data);
     }
@@ -158,7 +181,7 @@ mod tests {
     #[test]
     fn test_json_webelement() {
         let json = r#"{"element-6066-11e4-a52e-4f735466cecf":"elem"}"#;
-        let data = WebElement::new("elem".into());
+        let data = WebElement("elem".into());
 
         check_serialize_deserialize(&json, &data);
     }