[rust] Refactor logic to discover driver version and download browser if necessary

bonigarcia · bonigarcia · commit 1d7d3aaea984 · 2023-11-13T17:46:08.000+01:00
diff --git a/rust/src/chrome.rs b/rust/src/chrome.rs
@@ -55,6 +55,7 @@ pub struct ChromeManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
     pub driver_url: Option<String>,
     pub browser_url: Option<String>,
 }
@@ -72,6 +73,7 @@ impl ChromeManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
             driver_url: None,
             browser_url: None,
         }))
@@ -556,6 +558,14 @@ impl SeleniumManager for ChromeManager {
     ) -> Result<Option<&str>, Error> {
         Ok(None)
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }
 
 #[derive(Serialize, Deserialize)]
diff --git a/rust/src/edge.rs b/rust/src/edge.rs
@@ -61,6 +61,7 @@ pub struct EdgeManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
     pub browser_url: Option<String>,
 }
 
@@ -81,6 +82,7 @@ impl EdgeManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
             browser_url: None,
         }))
     }
@@ -519,6 +521,14 @@ impl SeleniumManager for EdgeManager {
         };
         Ok(Some(browser_label))
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }
 
 #[derive(Serialize, Deserialize, Debug)]
diff --git a/rust/src/firefox.rs b/rust/src/firefox.rs
@@ -73,6 +73,7 @@ pub struct FirefoxManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
 }
 
 impl FirefoxManager {
@@ -88,6 +89,7 @@ impl FirefoxManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
         }))
     }
 
@@ -592,6 +594,14 @@ impl SeleniumManager for FirefoxManager {
         };
         Ok(Some(browser_label))
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }
 
 #[derive(Serialize, Deserialize, Debug)]
diff --git a/rust/src/grid.rs b/rust/src/grid.rs
@@ -45,6 +45,7 @@ pub struct GridManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
     pub driver_url: Option<String>,
 }
 
@@ -62,6 +63,7 @@ impl GridManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
             driver_url: None,
         }))
     }
@@ -261,4 +263,12 @@ impl SeleniumManager for GridManager {
     ) -> Result<Option<&str>, Error> {
         self.unavailable_download()
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }
diff --git a/rust/src/iexplorer.rs b/rust/src/iexplorer.rs
@@ -51,6 +51,7 @@ pub struct IExplorerManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
     pub driver_url: Option<String>,
 }
 
@@ -68,6 +69,7 @@ impl IExplorerManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
             driver_url: None,
         }))
     }
@@ -267,4 +269,12 @@ impl SeleniumManager for IExplorerManager {
     ) -> Result<Option<&str>, Error> {
         self.unavailable_download()
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }
diff --git a/rust/src/lib.rs b/rust/src/lib.rs
@@ -170,6 +170,10 @@ pub trait SeleniumManager {
     fn get_browser_label_for_download(&self, _browser_version: &str)
         -> Result<Option<&str>, Error>;
 
+    fn is_download_browser(&self) -> bool;
+
+    fn set_download_browser(&mut self, download_browser: bool);
+
     // ----------------------------------------------------------
     // Shared functions
     // ----------------------------------------------------------
@@ -422,15 +426,9 @@ pub trait SeleniumManager {
         browser_version
     }
 
-    fn discover_driver_version_and_download_browser_if_necessary(
-        &mut self,
-    ) -> Result<String, Error> {
-        let original_browser_version = self.get_config().browser_version.clone();
-        let mut download_browser = self.is_force_browser_download();
-        let major_browser_version = self.get_major_browser_version();
-
-        // First, we try to discover the browser version
-        if !download_browser {
+    fn discover_local_browser(&mut self) -> Result<(), Error> {
+        if !self.is_force_browser_download() {
+            let major_browser_version = self.get_major_browser_version();
             match self.discover_browser_version()? {
                 Some(discovered_version) => {
                     if !self.is_safari() {
@@ -465,7 +463,7 @@ pub trait SeleniumManager {
                                     self.get_browser_name(),
                                     discovered_major_browser_version,
                                 ));
-                                download_browser = true;
+                                self.set_download_browser(true);
                             }
                         } else {
                             self.set_browser_version(discovered_version);
@@ -480,7 +478,7 @@ pub trait SeleniumManager {
                             discovered_major_browser_version,
                             major_browser_version,
                         ));
-                        download_browser = true;
+                        self.set_download_browser(true);
                     } else {
                         self.set_browser_version(discovered_version);
                     }
@@ -500,12 +498,19 @@ pub trait SeleniumManager {
                         self.get_browser_name(),
                         self.get_browser_version_label()
                     ));
-                    download_browser = true;
+                    self.set_download_browser(true);
                 }
             }
         }
 
-        if download_browser
+        Ok(())
+    }
+
+    fn download_browser_if_necessary(
+        &mut self,
+        original_browser_version: String,
+    ) -> Result<(), Error> {
+        if self.is_download_browser()
             && !self.is_avoid_browser_download()
             && !self.is_iexplorer()
             && !self.is_grid()
@@ -528,8 +533,11 @@ pub trait SeleniumManager {
                 )));
             }
         }
+        Ok(())
+    }
 
-        // Second, we request the driver version using online endpoints
+    fn discover_driver_version(&mut self) -> Result<String, Error> {
+        // We request the driver version using online endpoints
         let driver_version = self.request_driver_version()?;
         if driver_version.is_empty() {
             Err(anyhow!(format!(
@@ -722,6 +730,7 @@ pub trait SeleniumManager {
     fn setup(&mut self) -> Result<PathBuf, Error> {
         let mut driver_in_path = None;
         let mut driver_in_path_version = None;
+        let original_browser_version = self.get_config().browser_version.clone();
 
         // Try to find driver in PATH
         if !self.is_safari() && !self.is_grid() {
@@ -741,24 +750,33 @@ pub trait SeleniumManager {
             }
         }
 
-        // Discover browser version (or download it, if not available and possible).
-        // With the found browser version, discover the proper driver version using online endpoints
+        // Discover browser version (or the need to download it, if not available and possible)
+        match self.discover_local_browser() {
+            Ok(_) => {}
+            Err(err) => {
+                self.check_error_with_driver_in_path(&driver_in_path, &driver_in_path_version, err)?
+            }
+        }
+
+        // Download browser if necessary
+        match self.download_browser_if_necessary(original_browser_version) {
+            Ok(_) => {}
+            Err(err) => {
+                self.check_error_with_driver_in_path(&driver_in_path, &driver_in_path_version, err)?
+            }
+        }
+
+        // With the discovered browser version, discover the proper driver version using online endpoints
         if self.get_driver_version().is_empty() {
-            match self.discover_driver_version_and_download_browser_if_necessary() {
+            match self.discover_driver_version() {
                 Ok(driver_version) => {
                     self.set_driver_version(driver_version);
                 }
-                Err(err) => {
-                    if driver_in_path_version.is_some() && driver_in_path.is_some() {
-                        self.get_logger().warn(format!(
-                            "Exception managing {}: {}",
-                            self.get_browser_name(),
-                            err
-                        ));
-                    } else {
-                        return Err(err);
-                    }
-                }
+                Err(err) => self.check_error_with_driver_in_path(
+                    &driver_in_path,
+                    &driver_in_path_version,
+                    err,
+                )?,
             }
         }
 
@@ -807,6 +825,24 @@ pub trait SeleniumManager {
         Ok(driver_path)
     }
 
+    fn check_error_with_driver_in_path(
+        &mut self,
+        driver_in_path: &Option<String>,
+        driver_in_path_version: &Option<String>,
+        err: Error,
+    ) -> Result<(), Error> {
+        if driver_in_path_version.is_some() && driver_in_path.is_some() {
+            self.get_logger().warn(format!(
+                "Exception managing {}: {}",
+                self.get_browser_name(),
+                err
+            ));
+            Ok(())
+        } else {
+            Err(err)
+        }
+    }
+
     fn is_driver(&self, entry: &DirEntry) -> bool {
         let is_file = entry.path().is_file();
 
diff --git a/rust/src/safari.rs b/rust/src/safari.rs
@@ -37,6 +37,7 @@ pub struct SafariManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
 }
 
 impl SafariManager {
@@ -52,6 +53,7 @@ impl SafariManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
         }))
     }
 }
@@ -160,4 +162,12 @@ impl SeleniumManager for SafariManager {
     ) -> Result<Option<&str>, Error> {
         self.unavailable_download()
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }
diff --git a/rust/src/safaritp.rs b/rust/src/safaritp.rs
@@ -43,6 +43,7 @@ pub struct SafariTPManager {
     pub config: ManagerConfig,
     pub http_client: Client,
     pub log: Logger,
+    pub download_browser: bool,
 }
 
 impl SafariTPManager {
@@ -58,6 +59,7 @@ impl SafariTPManager {
             http_client: create_http_client(default_timeout, default_proxy)?,
             config,
             log: Logger::new(),
+            download_browser: false,
         }))
     }
 }
@@ -168,4 +170,12 @@ impl SeleniumManager for SafariTPManager {
     ) -> Result<Option<&str>, Error> {
         self.unavailable_download()
     }
+
+    fn is_download_browser(&self) -> bool {
+        self.download_browser
+    }
+
+    fn set_download_browser(&mut self, download_browser: bool) {
+        self.download_browser = download_browser;
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ pub struct SafariManager {`
`37`	`37`	`pub config: ManagerConfig,`
`38`	`38`	`pub http_client: Client,`
`39`	`39`	`pub log: Logger,`
	`40`	`+ pub download_browser: bool,`
`40`	`41`	`}`
`41`	`42`
`42`	`43`	`impl SafariManager {`
`@@ -52,6 +53,7 @@ impl SafariManager {`
`52`	`53`	`http_client: create_http_client(default_timeout, default_proxy)?,`
`53`	`54`	`config,`
`54`	`55`	`log: Logger::new(),`
	`56`	`+ download_browser: false,`
`55`	`57`	`}))`
`56`	`58`	`}`
`57`	`59`	`}`
`@@ -160,4 +162,12 @@ impl SeleniumManager for SafariManager {`
`160`	`162`	`) -> Result<Option<&str>, Error> {`
`161`	`163`	`self.unavailable_download()`
`162`	`164`	`}`
	`165`	`+`
	`166`	`+ fn is_download_browser(&self) -> bool {`
	`167`	`+ self.download_browser`
	`168`	`+ }`
	`169`	`+`
	`170`	`+ fn set_download_browser(&mut self, download_browser: bool) {`
	`171`	`+ self.download_browser = download_browser;`
	`172`	`+ }`
`163`	`173`	`}`
Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,7 @@ pub struct SafariTPManager {`
`43`	`43`	`pub config: ManagerConfig,`
`44`	`44`	`pub http_client: Client,`
`45`	`45`	`pub log: Logger,`
	`46`	`+ pub download_browser: bool,`
`46`	`47`	`}`
`47`	`48`
`48`	`49`	`impl SafariTPManager {`
`@@ -58,6 +59,7 @@ impl SafariTPManager {`
`58`	`59`	`http_client: create_http_client(default_timeout, default_proxy)?,`
`59`	`60`	`config,`
`60`	`61`	`log: Logger::new(),`
	`62`	`+ download_browser: false,`
`61`	`63`	`}))`
`62`	`64`	`}`
`63`	`65`	`}`
`@@ -168,4 +170,12 @@ impl SeleniumManager for SafariTPManager {`
`168`	`170`	`) -> Result<Option<&str>, Error> {`
`169`	`171`	`self.unavailable_download()`
`170`	`172`	`}`
	`173`	`+`
	`174`	`+ fn is_download_browser(&self) -> bool {`
	`175`	`+ self.download_browser`
	`176`	`+ }`
	`177`	`+`
	`178`	`+ fn set_download_browser(&mut self, download_browser: bool) {`
	`179`	`+ self.download_browser = download_browser;`
	`180`	`+ }`
`171`	`181`	`}`