Skip to content

Commit 705be71

Browse files
committed
Server configuration consistency updates.
- Fix #3078 - new {servertype}Configurion() should always return an Object which is initialized with the same values as {servertype}Configuration#loadFromJSON(DEFAULT_CONFIG_FFILE) does. As a result of this change new GridNodeConfiguration() will always return the default node 'capabilities' (firefox, chrome, ie) -- which might require a user to clear() and/or adjust as they see fit. - Partially fix #2973 - Node SeleniumServer needs to start with the updated configuration that SelfRegisteringRemote operates on. - Fix for boolean command line switches -- as a result of this change all boolean command line switches now have arity=1. Meaning you must provide true/false values. For example '-debug true' or '-debug false'. - Update logic for processing RegistrationRequests - Reduced the visibility of GridConfiguration class - Update and Add Tests.
1 parent 438485c commit 705be71

23 files changed

+714
-159
lines changed

java/server/src/org/openqa/grid/common/RegistrationRequest.java

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ public JsonObject toJson() {
115115
builder.registerTypeAdapter(new TypeToken<List<DesiredCapabilities>>(){}.getType(),
116116
new CollectionOfDesiredCapabilitiesSerializer());
117117

118-
return builder.excludeFieldsWithoutExposeAnnotation().create()
118+
// note: it's very important that nulls are serialized for this type.
119+
return builder.serializeNulls().excludeFieldsWithoutExposeAnnotation().create()
119120
.toJsonTree(this, RegistrationRequest.class).getAsJsonObject();
120121
}
121122

@@ -153,11 +154,20 @@ public static RegistrationRequest fromJson(String json) throws JsonSyntaxExcepti
153154
return request;
154155
}
155156

157+
/**
158+
* Build a RegistrationRequest. This is different than {@code new RegistrationRequest()} because
159+
* it will "fixup" the resulting RegistrationRequest before returning the result
160+
* @return
161+
*/
162+
public static RegistrationRequest build() {
163+
return RegistrationRequest.build(null, null, null);
164+
}
165+
156166
/**
157167
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}. This is different
158-
* than {@code new RegistrationRequest(GridNodeConfiguration)} because it will merge the provided
159-
* configuration onto the {@link GridNodeConfiguration#DEFAULT_NODE_CONFIG_FILE} and then "fixup"
160-
* the resulting RegistrationRequest before returning the result
168+
* than {@code new RegistrationRequest(GridNodeConfiguration)} because it will merge any
169+
* specified {@link GridNodeConfiguration#nodeConfigFile} onto the provided configuration and it
170+
* will "fixup" the resulting RegistrationRequest before returning the result
161171
* @param configuration the {@link GridNodeConfiguration} to use
162172
* @return
163173
*/
@@ -168,8 +178,8 @@ public static RegistrationRequest build(GridNodeConfiguration configuration) {
168178
/**
169179
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}, use the provided name.
170180
* This is different than {@code new RegistrationRequest(GridNodeConfiguration, String)} because it
171-
* will merge the provided configuration onto the {@link GridNodeConfiguration#DEFAULT_NODE_CONFIG_FILE}
172-
* and then "fixup" the resulting RegistrationRequest before returning the result
181+
* will merge any specified {@link GridNodeConfiguration#nodeConfigFile} onto the provided
182+
* configuration and it will "fixup" the resulting RegistrationRequest before returning the result
173183
* @param configuration the {@link GridNodeConfiguration} to use
174184
* @param name the name for the remote
175185
* @return
@@ -181,18 +191,18 @@ public static RegistrationRequest build(GridNodeConfiguration configuration, Str
181191
/**
182192
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}, use the provided name
183193
* and description. This is different than
184-
* {@code new RegistrationRequest(GridNodeConfiguration, String, String)} because it will merge the
185-
* provided configuration onto the {@link GridNodeConfiguration#DEFAULT_NODE_CONFIG_FILE}
186-
* and then "fixup" the resulting RegistrationRequest before returning the result
194+
* {@code new RegistrationRequest(GridNodeConfiguration, String, String)} because it will merge any
195+
* specified {@link GridNodeConfiguration#nodeConfigFile} onto the provided configuration and it
196+
* will "fixup" the resulting RegistrationRequest before returning the result
187197
* @param configuration the {@link GridNodeConfiguration} to use
188198
* @param name the name for the remote
189199
* @param description the description for the remote host
190200
* @return
191201
*/
192202
public static RegistrationRequest build(GridNodeConfiguration configuration, String name, String description) {
193-
RegistrationRequest pendingRequest =
194-
new RegistrationRequest(GridNodeConfiguration
195-
.loadFromJSON(GridNodeConfiguration.DEFAULT_NODE_CONFIG_FILE));
203+
RegistrationRequest pendingRequest = (configuration == null) ?
204+
new RegistrationRequest(new GridNodeConfiguration(), name, description) :
205+
new RegistrationRequest(configuration, name, description);
196206

197207
if (configuration.nodeConfigFile != null) {
198208
pendingRequest.configuration = GridNodeConfiguration.loadFromJSON(configuration.nodeConfigFile);
@@ -207,9 +217,6 @@ public static RegistrationRequest build(GridNodeConfiguration configuration, Str
207217
pendingRequest.configuration.port = configuration.port;
208218
}
209219

210-
pendingRequest.name = name;
211-
pendingRequest.description = description;
212-
213220
// make sure we have a valid host
214221
pendingRequest.fixUpHost();
215222
// make sure the capabilities are updated with required fields
Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
{
2-
"host": null,
32
"port": 4444,
43
"newSessionWaitTimeout": -1,
54
"servlets" : [],
6-
"prioritizer": null,
5+
"withoutServlets": [],
6+
"custom": {},
77
"capabilityMatcher": "org.openqa.grid.internal.utils.DefaultCapabilityMatcher",
88
"throwOnCapabilityNotPresent": true,
9-
"nodePolling": 5000,
109
"cleanUpCycle": 5000,
10+
"role": "hub",
11+
"debug": false,
1112
"browserTimeout": 0,
12-
"jettyMaxThreads":-1
13+
"timeout": 1800
1314
}

java/server/src/org/openqa/grid/common/defaults/DefaultNodeWebDriver.json

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,14 @@
2222
"port": 5555,
2323
"register": true,
2424
"registerCycle": 5000,
25-
"hub": "http://localhost:4444"
25+
"hub": "http://localhost:4444",
26+
"nodeStatusCheckTimeout": 5000,
27+
"nodePolling": 5000,
28+
"role": "node",
29+
"unregisterIfStillDownAfter": 60000,
30+
"downPollingLimit": 2,
31+
"debug": false,
32+
"servlets" : [],
33+
"withoutServlets": [],
34+
"custom": {}
2635
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"port": 4444,
3+
"role": "standalone",
4+
"debug": false,
5+
"browserTimeout": 0,
6+
"timeout": 1800
7+
}

java/server/src/org/openqa/grid/internal/utils/SelfRegisteringRemote.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ public SelfRegisteringRemote(RegistrationRequest request) {
7575
try {
7676
GridHubConfiguration hubConfiguration = getHubConfiguration();
7777
// the node can not set these values. They must come from the hub
78-
if (hubConfiguration.timeout != null) {
78+
if (hubConfiguration.timeout != null && hubConfiguration.timeout >= 0) {
7979
registrationRequest.getConfiguration().timeout = hubConfiguration.timeout;
8080
}
81-
if (hubConfiguration.browserTimeout != null) {
81+
if (hubConfiguration.browserTimeout != null && hubConfiguration.browserTimeout >= 0) {
8282
registrationRequest.getConfiguration().browserTimeout = hubConfiguration.browserTimeout;
8383
}
8484
} catch (Exception e) {
@@ -184,7 +184,8 @@ public void startRegistrationProcess() {
184184
if (!register) {
185185
LOG.info("No registration sent ( register = false )");
186186
} else {
187-
final int registerCycleInterval = registrationRequest.getConfiguration().registerCycle;
187+
final int registerCycleInterval = registrationRequest.getConfiguration().registerCycle != null ?
188+
registrationRequest.getConfiguration().registerCycle : 0;
188189
if (registerCycleInterval > 0) {
189190
new Thread(new Runnable() { // Thread safety reviewed
190191

java/server/src/org/openqa/grid/internal/utils/configuration/GridConfiguration.java

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,72 +23,107 @@
2323

2424
import org.openqa.grid.internal.utils.configuration.converters.CustomConverter;
2525

26+
import java.util.ArrayList;
2627
import java.util.HashMap;
2728
import java.util.List;
2829
import java.util.Map;
2930

3031
import javax.servlet.Servlet;
3132

32-
public class GridConfiguration extends StandaloneConfiguration {
33-
33+
class GridConfiguration extends StandaloneConfiguration {
3434
/*
3535
* config parameters which serialize and deserialize to/from json
3636
*/
3737

38+
/**
39+
* Clean up cycle for remote proxies. Default determined by configuration type.
40+
*/
3841
@Expose
3942
@Parameter(
4043
names = "-cleanUpCycle",
4144
description = "<Integer> in ms : specifies how often the hub will poll running proxies for timed-out (i.e. hung) threads. Must also specify \"timeout\" option"
4245
)
46+
// initially defaults to null from type
4347
public Integer cleanUpCycle;
4448

49+
/**
50+
* Custom key/value pairs for the hub registry. Default empty.
51+
*/
4552
@Expose
4653
@Parameter(
4754
names = "-custom",
48-
description = "<String> : NOT RECOMMENDED--may be deprecated to encourage proper servlet customization. Comma separated key=value pairs for custom grid extensions. example: -custom myParamA=Value1,myParamB=Value2",
55+
description = "<String> : comma separated key=value pairs for custom grid extensions. NOT RECOMMENDED -- may be deprecated in a future revision. Example: -custom myParamA=Value1,myParamB=Value2",
4956
converter = CustomConverter.class
5057
)
5158
public Map<String, String> custom = new HashMap<>();
5259

60+
/**
61+
* Hostname or IP to use. Defaults to {@code null}. Automatically determined when {@code null}.
62+
*/
5363
@Expose
5464
@Parameter(
5565
names = "-host",
5666
description = "<String> IP or hostname : usually determined automatically. Most commonly useful in exotic network configurations (e.g. network with VPN)"
5767
)
68+
// initially defaults to null from type
5869
public String host;
5970

71+
/**
72+
* Max "browser" sessions a node can handle. Default determined by configuration type.
73+
*/
6074
@Expose
6175
@Parameter(
6276
names = "-maxSession",
6377
description = "<Integer> max number of tests that can run at the same time on the node, irrespective of the browser used"
6478
)
65-
public Integer maxSession = 1;
79+
// initially defaults to null from type
80+
public Integer maxSession;
6681

82+
/**
83+
* Extra servlets to initialize/use on the hub or node. Default empty.
84+
*/
6785
@Expose
6886
@Parameter(
6987
names = {"-servlet", "-servlets"},
7088
description = "<String> : list of extra servlets the grid (hub or node) will make available. Specify multiple on the command line: -servlet tld.company.ServletA -servlet tld.company.ServletB. The servlet must exist in the path: /grid/admin/ServletA /grid/admin/ServletB"
7189
)
72-
public List<String> servlets;
90+
public List<String> servlets = new ArrayList<>();
7391

92+
/**
93+
* Default servlets to exclude on the hub or node. Default empty.
94+
*/
7495
@Expose
7596
@Parameter(
7697
names = {"-withoutServlet", "-withoutServlets"},
7798
description = "<String> : list of default (hub or node) servlets to disable. Advanced use cases only. Not all default servlets can be disabled. Specify multiple on the command line: -withoutServlet tld.company.ServletA -withoutServlet tld.company.ServletB"
7899
)
79-
public List<String> withoutServlets;
100+
public List<String> withoutServlets = new ArrayList<>();
101+
102+
/**
103+
* Creates a new configuration with default values
104+
*/
105+
GridConfiguration() {
106+
// defeats instantiation outside of this package
107+
}
80108

81109
/**
82110
* replaces this instance of configuration value with the 'other' value if it's set.
83111
* @param other
84112
*/
85113
public void merge(GridConfiguration other) {
114+
if (other == null) {
115+
return;
116+
}
86117
super.merge(other);
118+
87119
// don't merge 'host'
88120
if (isMergeAble(other.cleanUpCycle, cleanUpCycle)) {
89121
cleanUpCycle = other.cleanUpCycle;
90122
}
91123
if (isMergeAble(other.custom, custom)) {
124+
if (custom == null) {
125+
custom = new HashMap<>();
126+
}
92127
custom.putAll(other.custom);
93128
}
94129
if (isMergeAble(other.maxSession, maxSession) &&

0 commit comments

Comments
 (0)