From 1177f513050021e65a559c7d05b3b5775bcdc54c Mon Sep 17 00:00:00 2001 From: Kushaljp Date: Sun, 31 Mar 2024 12:53:17 -0300 Subject: [PATCH 1/2] refactored design smell - feature envy --- .../selenium/htmlunit/HtmlUnitKeyboard.java | 57 ++++++++----------- .../htmlunit/KeyboardModifiersState.java | 17 +++++- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitKeyboard.java b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitKeyboard.java index 87e91ba4..6cc252d7 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitKeyboard.java +++ b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitKeyboard.java @@ -68,12 +68,11 @@ void sendKeys(final HtmlUnitWebElement htmlElem, final boolean releaseAllAtEnd, } private void sendKeys(final HtmlElement element, - final InputKeysContainer keysToSend, final boolean releaseAllAtEnd) { + final InputKeysContainer keysToSend, final boolean releaseAllAtEnd) { keysToSend.setCapitalization(modifiersState_.isShiftPressed()); final String keysSequence = keysToSend.toString(); - // HtmlElement.type doesn't modify the value of a file input element. Special - // case. + // Special case for HtmlFileInput if (element instanceof HtmlFileInput) { final HtmlFileInput fileInput = (HtmlFileInput) element; fileInput.setValue(keysSequence); @@ -82,21 +81,13 @@ private void sendKeys(final HtmlElement element, try { final boolean startAtEnd = lastElement_ != element && !(element instanceof HtmlNumberInput); - final Keyboard keyboard = asHtmlUnitKeyboard(startAtEnd, keysSequence, true); - if (releaseAllAtEnd) { - if (isShiftPressed()) { - addToKeyboard(keyboard, Keys.SHIFT.charAt(0), false); - } - if (isAltPressed()) { - addToKeyboard(keyboard, Keys.ALT.charAt(0), false); - } - if (isCtrlPressed()) { - addToKeyboard(keyboard, Keys.CONTROL.charAt(0), false); - } + final Keyboard keyboard = new Keyboard(startAtEnd); + for (int i = 0; i < keysSequence.length(); i++) { + final char ch = keysSequence.charAt(i); + modifiersState_.addToKeyboard(keyboard, ch, releaseAllAtEnd); // This line is modified } element.type(keyboard); - } - catch (final IOException e) { + } catch (final IOException e) { throw new WebDriverException(e); } lastElement_ = element; @@ -107,27 +98,27 @@ private Keyboard asHtmlUnitKeyboard(final boolean startAtEnd, final CharSequence final Keyboard keyboard = new Keyboard(startAtEnd); for (int i = 0; i < keysSequence.length(); i++) { final char ch = keysSequence.charAt(i); - addToKeyboard(keyboard, ch, isPress); + modifiersState_.addToKeyboard(keyboard, ch, isPress); } return keyboard; } - private void addToKeyboard(final Keyboard keyboard, final char ch, final boolean isPress) { - if (HtmlUnitKeyboardMapping.isSpecialKey(ch)) { - final int keyCode = HtmlUnitKeyboardMapping.getKeysMapping(ch); - if (isPress) { - keyboard.press(keyCode); - modifiersState_.storeKeyDown(ch); - } - else { - keyboard.release(keyCode); - modifiersState_.storeKeyUp(ch); - } - } - else { - keyboard.type(ch); - } - } +// private void addToKeyboard(final Keyboard keyboard, final char ch, final boolean isPress) { +// if (HtmlUnitKeyboardMapping.isSpecialKey(ch)) { +// final int keyCode = HtmlUnitKeyboardMapping.getKeysMapping(ch); +// if (isPress) { +// keyboard.press(keyCode); +// modifiersState_.storeKeyDown(ch); +// } +// else { +// keyboard.release(keyCode); +// modifiersState_.storeKeyUp(ch); +// } +// } +// else { +// keyboard.type(ch); +// } +// } public void pressKey(final CharSequence keyToPress) { final HtmlUnitWebElement htmlElement = (HtmlUnitWebElement) parent_.switchTo().activeElement(); diff --git a/src/main/java/org/openqa/selenium/htmlunit/KeyboardModifiersState.java b/src/main/java/org/openqa/selenium/htmlunit/KeyboardModifiersState.java index a77532e2..dc49ae31 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/KeyboardModifiersState.java +++ b/src/main/java/org/openqa/selenium/htmlunit/KeyboardModifiersState.java @@ -19,7 +19,7 @@ import java.util.HashSet; import java.util.Set; - +import org.htmlunit.html.Keyboard; import org.openqa.selenium.Keys; /** @@ -61,6 +61,21 @@ public void storeKeyUp(final char key) { set_.remove(key); } + public void addToKeyboard(final Keyboard keyboard, final char ch, final boolean isPress) { + if (HtmlUnitKeyboardMapping.isSpecialKey(ch)) { + final int keyCode = HtmlUnitKeyboardMapping.getKeysMapping(ch); + if (isPress) { + keyboard.press(keyCode); + storeKeyDown(ch); + } else { + keyboard.release(keyCode); + storeKeyUp(ch); + } + } else { + keyboard.type(ch); + } + } + private void storeIfEqualsShift(final char key, final boolean keyState) { if (key == Keys.SHIFT.charAt(0)) { shiftPressed_ = keyState; From bce735221d8f2690265cf508b72deb0f9704c16f Mon Sep 17 00:00:00 2001 From: Kushaljp Date: Sun, 31 Mar 2024 22:29:28 -0300 Subject: [PATCH 2/2] refactored design smell - insufficient modularization --- .../selenium/htmlunit/HtmlUnitDriver.java | 147 +----------------- .../htmlunit/ProxyConfigurationManager.java | 117 ++++++++++++++ .../selenium/htmlunit/HtmlUnitProxyTest.java | 41 +++-- 3 files changed, 156 insertions(+), 149 deletions(-) create mode 100644 src/main/java/org/openqa/selenium/htmlunit/ProxyConfigurationManager.java diff --git a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitDriver.java b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitDriver.java index 7b780fff..4e9601e0 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitDriver.java +++ b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitDriver.java @@ -151,6 +151,10 @@ public class HtmlUnitDriver implements WebDriver, JavascriptExecutor, HasCapabil /** JAVASCRIPT_ENABLED = "javascriptEnabled". */ public static final String JAVASCRIPT_ENABLED = "javascriptEnabled"; + private WebClient webClient; + + + /** * The Lock for the {@link #mainCondition_}, which waits at the end of * {@link #runAsync(Runnable)} till either and alert is triggered, or @@ -163,6 +167,8 @@ public class HtmlUnitDriver implements WebDriver, JavascriptExecutor, HasCapabil private final ExecutorService defaultExecutor_; private Executor executor_; + + private ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); /** * Constructs a new instance with JavaScript disabled, and the * {@link BrowserVersion#getDefault() default} BrowserVersion. @@ -255,7 +261,7 @@ private HtmlUnitDriver(final BrowserVersion version, final boolean enableJavascr clientOptions.setUseInsecureSSL(true); setJavascriptEnabled(enableJavascript); - setProxySettings(proxy); + proxyConfigurationManager.setProxySettings(proxy); webClient_.setRefreshHandler(new WaitingRefreshHandler()); webClient_.setClipboardHandler(new AwtClipboardHandler()); @@ -465,135 +471,7 @@ public void setCurrentWindow(final WebWindow window) { } } - /** - * Set proxy for WebClient using Proxy. - * - * @param proxy The proxy preferences. - */ - public void setProxySettings(final Proxy proxy) { - if (proxy == null || proxy.getProxyType() == Proxy.ProxyType.UNSPECIFIED) { - return; - } - - switch (proxy.getProxyType()) { - case MANUAL: - configureManualProxy(proxy); - break; - case PAC: - configurePacProxy(proxy); - break; - - default: - // Do nothing for other proxy types or log a warning that it's not supported - break; - } - } - - private void configureManualProxy(Proxy proxy) { - final List noProxyHosts = extractNoProxyHosts(proxy); - final String httpProxy = proxy.getHttpProxy(); - if (httpProxy != null && !httpProxy.isEmpty()) { - configureHttpProxy(httpProxy, noProxyHosts); - } - final String socksProxy = proxy.getSocksProxy(); - if (socksProxy != null && !socksProxy.isEmpty()) { - configureSocksProxy(socksProxy, noProxyHosts); - } - // Add any other manual proxy configuration if needed - } - - private List extractNoProxyHosts(Proxy proxy) { - final List noProxyHosts = new ArrayList<>(); - final String noProxy = proxy.getNoProxy(); - if (noProxy != null && !noProxy.isEmpty()) { - for (String host : noProxy.split(",")) { - if (!host.trim().isEmpty()) { - noProxyHosts.add(host.trim()); - } - } - } - return noProxyHosts; - } - - private void configureHttpProxy(String httpProxy, List noProxyHosts) { - final String[] proxyParts = httpProxy.split(":", 2); - final String host = proxyParts[0]; - final int port = proxyParts.length > 1 ? Integer.parseInt(proxyParts[1]) : 0; - setHTTPProxy(host, port, noProxyHosts); - } - - private void configureSocksProxy(String socksProxy, List noProxyHosts) { - final String[] proxyParts = socksProxy.split(":", 2); - final String host = proxyParts[0]; - final int port = proxyParts.length > 1 ? Integer.parseInt(proxyParts[1]) : 0; - setSocksProxy(host, port, noProxyHosts); - } - - private void configurePacProxy(Proxy proxy) { - final String pac = proxy.getProxyAutoconfigUrl(); - if (pac != null && !pac.isEmpty()) { - setAutoProxy(pac); - } - } - /** - * Sets HTTP proxy for WebClient. - * - * @param host The hostname of HTTP proxy - * @param port The port of HTTP proxy, 0 means HTTP proxy w/o port - */ - public void setProxy(final String host, final int port) { - setHTTPProxy(host, port, null); - } - - /** - * Sets HTTP proxy for WebClient with bypass proxy hosts. - * - * @param host The hostname of HTTP proxy - * @param port The port of HTTP proxy, 0 means HTTP proxy w/o port - * @param noProxyHosts The list of hosts which need to bypass HTTP proxy - */ - public void setHTTPProxy(final String host, final int port, final List noProxyHosts) { - final ProxyConfig proxyConfig = new ProxyConfig(); - proxyConfig.setProxyHost(host); - proxyConfig.setProxyPort(port); - if (noProxyHosts != null && noProxyHosts.size() > 0) { - for (final String noProxyHost : noProxyHosts) { - proxyConfig.addHostsToProxyBypass(noProxyHost); - } - } - getWebClient().getOptions().setProxyConfig(proxyConfig); - } - - /** - * Sets SOCKS proxy for WebClient. - * - * @param host The hostname of SOCKS proxy - * @param port The port of SOCKS proxy, 0 means HTTP proxy w/o port - */ - public void setSocksProxy(final String host, final int port) { - setSocksProxy(host, port, null); - } - - /** - * Sets SOCKS proxy for WebClient with bypass proxy hosts. - * - * @param host The hostname of SOCKS proxy - * @param port The port of SOCKS proxy, 0 means HTTP proxy w/o port - * @param noProxyHosts The list of hosts which need to bypass SOCKS proxy - */ - public void setSocksProxy(final String host, final int port, final List noProxyHosts) { - final ProxyConfig proxyConfig = new ProxyConfig(); - proxyConfig.setProxyHost(host); - proxyConfig.setProxyPort(port); - proxyConfig.setSocksProxy(true); - if (noProxyHosts != null && noProxyHosts.size() > 0) { - for (final String noProxyHost : noProxyHosts) { - proxyConfig.addHostsToProxyBypass(noProxyHost); - } - } - getWebClient().getOptions().setProxyConfig(proxyConfig); - } /** * Sets the {@link Executor} to be used for submitting async tasks to. You have @@ -608,16 +486,7 @@ public void setExecutor(final Executor executor) { this.executor_ = executor; } - /** - * Sets Proxy Autoconfiguration URL for WebClient. - * - * @param autoProxyUrl The Proxy Autoconfiguration URL - */ - public void setAutoProxy(final String autoProxyUrl) { - final ProxyConfig proxyConfig = new ProxyConfig(); - proxyConfig.setProxyAutoConfigUrl(autoProxyUrl); - getWebClient().getOptions().setProxyConfig(proxyConfig); - } + @Override public Capabilities getCapabilities() { diff --git a/src/main/java/org/openqa/selenium/htmlunit/ProxyConfigurationManager.java b/src/main/java/org/openqa/selenium/htmlunit/ProxyConfigurationManager.java new file mode 100644 index 00000000..c82f4c79 --- /dev/null +++ b/src/main/java/org/openqa/selenium/htmlunit/ProxyConfigurationManager.java @@ -0,0 +1,117 @@ +package org.openqa.selenium.htmlunit; + +import org.htmlunit.ProxyConfig; +import org.htmlunit.WebClient; +import org.openqa.selenium.NoSuchSessionException; +import org.openqa.selenium.Proxy; + +import java.util.ArrayList; +import java.util.List; + +public class ProxyConfigurationManager { + private final ProxyConfig proxyConfig; + + private WebClient webClient_; + + + public ProxyConfigurationManager() { + this.proxyConfig = new ProxyConfig(); + } + + public WebClient getWebClient() { + if (webClient_ == null) { + throw new NoSuchSessionException("Session is closed"); + } + return webClient_; + } + public void setProxySettings(final Proxy proxy) { + if (proxy == null) { + return; + } + switch (proxy.getProxyType()) { + case MANUAL: + configureManualProxy(proxy); + break; + case PAC: + configurePacProxy(proxy); + break; + default: + // Unsupported proxy types or no proxy configuration + break; + } + } + + private void configureManualProxy(Proxy proxy) { + final List noProxyHosts = extractNoProxyHosts(proxy); + final String httpProxy = proxy.getHttpProxy(); + if (httpProxy != null && !httpProxy.isEmpty()) { + configureHttpProxy(httpProxy, noProxyHosts); + } + final String socksProxy = proxy.getSocksProxy(); + if (socksProxy != null && !socksProxy.isEmpty()) { + configureSocksProxy(socksProxy, noProxyHosts); + } + } + + private void configurePacProxy(Proxy proxy) { + final String pac = proxy.getProxyAutoconfigUrl(); + if (pac != null && !pac.isEmpty()) { + this.proxyConfig.setProxyAutoConfigUrl(pac); + } + } + + private List extractNoProxyHosts(Proxy proxy) { + final List noProxyHosts = new ArrayList<>(); + final String noProxy = proxy.getNoProxy(); + if (noProxy != null && !noProxy.isEmpty()) { + for (String host : noProxy.split(",")) { + if (!host.trim().isEmpty()) { + noProxyHosts.add(host.trim()); + } + } + } + return noProxyHosts; + } + + private void configureHttpProxy(String httpProxy, List noProxyHosts) { + final String[] proxyParts = httpProxy.split(":", 2); + final String host = proxyParts[0]; + final int port = proxyParts.length > 1 ? Integer.parseInt(proxyParts[1]) : 0; + setHTTPProxy(host, port, noProxyHosts); + } + + private void configureSocksProxy(String socksProxy, List noProxyHosts) { + final String[] proxyParts = socksProxy.split(":", 2); + final String host = proxyParts[0]; + final int port = proxyParts.length > 1 ? Integer.parseInt(proxyParts[1]) : 0; + setSocksProxy(host, port, noProxyHosts); + } + + public void setHTTPProxy(final String host, final int port, final List noProxyHosts) { + this.proxyConfig.setProxyHost(host); + this.proxyConfig.setProxyPort(port); + noProxyHosts.forEach(this.proxyConfig::addHostsToProxyBypass); + } + + public void setSocksProxy(final String host, final int port, final List noProxyHosts) { + this.proxyConfig.setSocksProxy(true); + setHTTPProxy(host, port, noProxyHosts); + } + + public void setProxy(final String host, final int port) { + setHTTPProxy(host, port, null); + } + + public void setSocksProxy(final String host, final int port) { + setSocksProxy(host, port, null); + } + + + public void setAutoProxy(final String autoProxyUrl) { + this.proxyConfig.setProxyAutoConfigUrl(autoProxyUrl); + } + + public void applyProxySettings(WebClient webClient) { + webClient.getOptions().setProxyConfig(this.proxyConfig); + } +} diff --git a/src/test/java/org/openqa/selenium/htmlunit/HtmlUnitProxyTest.java b/src/test/java/org/openqa/selenium/htmlunit/HtmlUnitProxyTest.java index 9cdfb137..c006b095 100644 --- a/src/test/java/org/openqa/selenium/htmlunit/HtmlUnitProxyTest.java +++ b/src/test/java/org/openqa/selenium/htmlunit/HtmlUnitProxyTest.java @@ -32,6 +32,8 @@ import org.openqa.selenium.remote.Browser; import org.openqa.selenium.remote.DesiredCapabilities; + + /** * Test the proxy setting. * @@ -62,7 +64,9 @@ public void testManualHttpProxy() { final Proxy proxy = new Proxy().setHttpProxy("http.proxy:1234"); final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setProxySettings(proxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setProxySettings(proxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -77,7 +81,9 @@ public void testManualHttpProxy() { public void testManualHttpProxyDirectly() { final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setProxy("http.proxy", 1234); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setProxy("http.proxy", 1234); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -93,7 +99,9 @@ public void testManualHttpProxyWithNoProxy() { final Proxy proxy = new Proxy().setHttpProxy("http.proxy").setNoProxy("localhost, 127.0.0.1"); final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setProxySettings(proxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setProxySettings(proxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -111,7 +119,9 @@ public void testManualHttpProxyWithNoProxyDirectly() { final List noProxy = new ArrayList<>(); noProxy.add("localhost"); noProxy.add("127.0.0.1"); - driver.setHTTPProxy("http.proxy", 0, noProxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setHTTPProxy("http.proxy", 0, noProxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -127,7 +137,9 @@ public void testManualSocksProxy() { final Proxy proxy = new Proxy().setSocksProxy("socks.proxy:1234"); final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setProxySettings(proxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setProxySettings(proxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -142,7 +154,9 @@ public void testManualSocksProxy() { public void testManualSocksProxyDirectly() { final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setSocksProxy("socks.proxy", 1234); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setSocksProxy("socks.proxy", 1234); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -158,7 +172,9 @@ public void testManualSocksProxyWithNoProxy() { final Proxy proxy = new Proxy().setSocksProxy("socks.proxy").setNoProxy("localhost"); final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setProxySettings(proxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setProxySettings(proxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -174,7 +190,9 @@ public void testManualSocksProxyWithNoProxyDirectly() { final HtmlUnitDriver driver = new HtmlUnitDriver(); final List noProxy = new ArrayList<>(); noProxy.add("localhost"); - driver.setSocksProxy("socks.proxy", 0, noProxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setSocksProxy("socks.proxy", 0, noProxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -190,7 +208,9 @@ public void testPACProxy() { final Proxy proxy = new Proxy().setProxyAutoconfigUrl("http://aaa/bb.pac"); final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setProxySettings(proxy); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + + proxyConfigurationManager.setProxySettings(proxy); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig(); @@ -202,7 +222,8 @@ public void testPACProxy() { @Test public void testPACProxyDirectly() { final HtmlUnitDriver driver = new HtmlUnitDriver(); - driver.setAutoProxy("http://aaa/bb.pac"); + final ProxyConfigurationManager proxyConfigurationManager=new ProxyConfigurationManager(); + proxyConfigurationManager.setAutoProxy("http://aaa/bb.pac"); final ProxyConfig config = driver.getWebClient().getOptions().getProxyConfig();