Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored Design and Implementation Smells #147

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Refactored Design and Implementation Smells #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
54ad32c
refactored implementation smell - complex conditional
Kushaljp Mar 31, 2024
4cd0a6e
refactored implementation smell - complex conditional
Kushaljp Mar 31, 2024
d1438ca
refactored implementation smell - complex method
Kushaljp Mar 31, 2024
e1236c6
refactored implementation smell - magic number
Kushaljp Mar 31, 2024
1c2458b
Merge pull request #1 from Kushaljp/refactor-implementation-smells
Kushaljp Mar 31, 2024
1177f51
refactored design smell - feature envy
Kushaljp Mar 31, 2024
bce7352
refactored design smell - insufficient modularization
Kushaljp Apr 1, 2024
970fc60
Merge pull request #2 from Kushaljp/refactor-design-smells
Kushaljp Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 8 additions & 136 deletions src/main/java/org/openqa/selenium/htmlunit/HtmlUnitDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -465,132 +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:
final List<String> noProxyHosts = new ArrayList<>();
final String noProxy = proxy.getNoProxy();
if (noProxy != null && !noProxy.isEmpty()) {
final String[] hosts = noProxy.split(",");
for (final String host : hosts) {
if (host.trim().length() > 0) {
noProxyHosts.add(host.trim());
}
}
}

final String httpProxy = proxy.getHttpProxy();
if (httpProxy != null && !httpProxy.isEmpty()) {
String host = httpProxy;
int port = 0;

final int index = httpProxy.indexOf(":");
if (index != -1) {
host = httpProxy.substring(0, index);
port = Integer.parseInt(httpProxy.substring(index + 1));
}

setHTTPProxy(host, port, noProxyHosts);
}

final String socksProxy = proxy.getSocksProxy();
if (socksProxy != null && !socksProxy.isEmpty()) {
String host = socksProxy;
int port = 0;

final int index = socksProxy.indexOf(":");
if (index != -1) {
host = socksProxy.substring(0, index);
port = Integer.parseInt(socksProxy.substring(index + 1));
}

setSocksProxy(host, port, noProxyHosts);
}

// sslProxy is not supported/implemented
// ftpProxy is not supported/implemented

break;

case PAC:
final String pac = proxy.getProxyAutoconfigUrl();
if (pac != null && !pac.isEmpty()) {
setAutoProxy(pac);
}
break;

default:
break;
}
}

/**
* 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<String> 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<String> 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
Expand All @@ -605,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() {
Expand Down
57 changes: 24 additions & 33 deletions src/main/java/org/openqa/selenium/htmlunit/HtmlUnitKeyboard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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();
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/org/openqa/selenium/htmlunit/HtmlUnitTargetLocator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
* @author Ronald Brill
*/
public class HtmlUnitTargetLocator implements WebDriver.TargetLocator {
private static final int ALERT_LOCK_RETRY_COUNT = 5;
private static final int ALERT_LOCK_RETRY_DELAY_MS = 50;
private final HtmlUnitDriver driver_;

public HtmlUnitTargetLocator(final HtmlUnitDriver driver) {
Expand Down Expand Up @@ -189,12 +191,11 @@ public Alert alert() {
final HtmlUnitAlert alert = driver_.getAlert();

if (!alert.isLocked()) {
for (int i = 0; i < 5; i++) {
for (int i = 0; i < ALERT_LOCK_RETRY_COUNT; i++) {
if (!alert.isLocked()) {
try {
Thread.sleep(50);
}
catch (final InterruptedException e) {
Thread.sleep(ALERT_LOCK_RETRY_DELAY_MS);
} catch (final InterruptedException e) {
throw new RuntimeException(e);
}
}
Expand Down
27 changes: 15 additions & 12 deletions src/main/java/org/openqa/selenium/htmlunit/HtmlUnitWebElement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,20 +249,20 @@ public Boolean call() throws Exception {
void switchFocusToThisIfNeeded() {
final HtmlUnitWebElement oldActiveElement = (HtmlUnitWebElement) driver_.switchTo().activeElement();

final boolean jsEnabled = driver_.isJavascriptEnabled();
final boolean oldActiveEqualsCurrent = oldActiveElement.equals(this);
try {
final boolean isBody = oldActiveElement.getTagName().toLowerCase().equals("body");
if (jsEnabled && !oldActiveEqualsCurrent && !isBody) {
oldActiveElement.element_.blur();
}
}
catch (final StaleElementReferenceException ex) {
// old element has gone, do nothing
if (shouldSwitchFocus(oldActiveElement)) {
oldActiveElement.element_.blur();
}
element_.focus();
}

private boolean shouldSwitchFocus(HtmlUnitWebElement oldActiveElement) {
final boolean jsEnabled = driver_.isJavascriptEnabled();
final boolean oldActiveEqualsCurrent = oldActiveElement.equals(this);
final boolean isOldElementBody = "body".equalsIgnoreCase(oldActiveElement.getTagName());

return jsEnabled && !oldActiveEqualsCurrent && !isOldElementBody;
}

@Override
public void sendKeys(final CharSequence... value) {
if (value == null) {
Expand All @@ -282,9 +282,12 @@ public String getAttribute(final String name) {
assertElementNotStale();

final String lowerName = name.toLowerCase();
final boolean isHtmlInput = element_ instanceof HtmlInput;
final boolean isSelectableAttribute = "selected".equals(lowerName) || "checked".equals(lowerName);

if (element_ instanceof HtmlInput && ("selected".equals(lowerName) || "checked".equals(lowerName))) {
return trueOrNull(((HtmlInput) element_).isChecked());
if (isHtmlInput && isSelectableAttribute) {
HtmlInput htmlInput = (HtmlInput) element_;
return trueOrNull(htmlInput.isChecked());
}

if ("href".equals(lowerName)) {
Expand Down
17 changes: 16 additions & 1 deletion src/main/java/org/openqa/selenium/htmlunit/KeyboardModifiersState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import java.util.HashSet;
import java.util.Set;

import org.htmlunit.html.Keyboard;
import org.openqa.selenium.Keys;

/**
Expand Down Expand Up @@ -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;
Expand Down
Loading