Skip to content

Commit 1505de7

Browse files
grahambrown11grahambrown11
committed
addressing some issues identified on SonarQube
1 parent 2c78a40 commit 1505de7

File tree

5 files changed

+71
-46
lines changed

5 files changed

+71
-46
lines changed

‎src/main/java/ch/loway/oss/ari4java/ARI.java‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*/
2424
public class ARI {
2525

26-
private final static String ALLOWED_IN_UID = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
26+
private static final String ALLOWED_IN_UID = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
2727

2828
private String appName = "";
2929
private AriVersion version;
@@ -65,11 +65,11 @@ public void setVersion(AriVersion version) {
6565
* Returns the current ARI version
6666
*
6767
* @return the version
68-
* @throws RuntimeException when version null
68+
* @throws ARIRuntimeException when version null
6969
*/
7070
public AriVersion getVersion() {
7171
if (version == null) {
72-
throw new RuntimeException("AriVersion not set");
72+
throw new ARIRuntimeException("AriVersion not set");
7373
}
7474
return version;
7575
}
@@ -433,7 +433,7 @@ public ActionSounds sounds() {
433433
* @return an Action object on which we'll set the default clients.
434434
* @throws IllegalArgumentException when error
435435
*/
436-
private Object setupAction(Object a) throws IllegalArgumentException {
436+
private Object setupAction(Object a) {
437437
if (a instanceof BaseAriAction) {
438438
BaseAriAction action = (BaseAriAction) a;
439439
if (httpClient == null || wsClient == null) {
@@ -456,8 +456,8 @@ private Object setupAction(Object a) throws IllegalArgumentException {
456456
public static void sleep(long ms) {
457457
try {
458458
Thread.sleep(ms);
459-
} catch (InterruptedException e) {
460-
logger.warn("Interrupted: " + e.getMessage(), e); //NOSONAR
459+
} catch (InterruptedException e) {//NOSONAR
460+
logger.warn("Interrupted: {}", e.getMessage(), e);
461461
}
462462
}
463463

@@ -474,7 +474,7 @@ public static String getUID() {
474474
if ((n % 5) == 0) {
475475
sb.append(".");
476476
}
477-
int pos = (int) (random.nextDouble() * ALLOWED_IN_UID.length());
477+
int pos = random.nextInt(ALLOWED_IN_UID.length());
478478
sb.append(ALLOWED_IN_UID.charAt(pos));
479479
}
480480
return sb.toString();
@@ -516,10 +516,10 @@ public void unsubscribeAll() throws RestException {
516516
* @return String
517517
*/
518518
public String getBuildVersion() {
519-
String version = "x";
519+
String buildVersion = "x";
520520
try {
521521
if (getClass().getPackage().getImplementationVersion() != null) {
522-
version = getClass().getPackage().getImplementationVersion();
522+
buildVersion = getClass().getPackage().getImplementationVersion();
523523
}
524524
} catch (Exception e) {
525525
// oh well
@@ -530,9 +530,10 @@ public String getBuildVersion() {
530530
if (stream != null) {
531531
Properties p = new Properties();
532532
p.load(stream);
533-
if (p.containsKey("BUILD_NUMBER") && p.getProperty("BUILD_NUMBER") != null &&
534-
!"x".equalsIgnoreCase(p.getProperty("BUILD_NUMBER"))) {
535-
version += " (Build: " + p.getProperty("BUILD_NUMBER") + ")";
533+
final String prop = "BUILD_NUMBER";
534+
if (p.containsKey(prop) && p.getProperty(prop) != null &&
535+
!"x".equalsIgnoreCase(p.getProperty(prop))) {
536+
buildVersion += " (Build: " + p.getProperty(prop) + ")";
536537
}
537538
}
538539
} catch (IOException e) {
@@ -546,7 +547,7 @@ public String getBuildVersion() {
546547
}
547548
}
548549
}
549-
return version;
550+
return buildVersion;
550551
}
551552

552553
/**

‎src/main/java/ch/loway/oss/ari4java/ARIRuntimeException.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package ch.loway.oss.ari4java;
2+
3+
public class ARIRuntimeException extends RuntimeException {
4+
5+
private static final long serialVersionUID = 1L;
6+
7+
public ARIRuntimeException(String message) {
8+
super(message);
9+
}
10+
11+
public ARIRuntimeException(Throwable t) {
12+
super(t.getMessage(), t);
13+
}
14+
15+
public ARIRuntimeException(String message, Throwable t) {
16+
super(message, t);
17+
}
18+
19+
}

‎src/main/java/ch/loway/oss/ari4java/tools/http/NettyHttpClient.java‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,15 @@ public class NettyHttpClient implements HttpClient, WsClient, WsClientAutoReconn
4444
public static final int MAX_HTTP_REQUEST = 16 * 1024 * 1024; // 16MB
4545
public static final int MAX_HTTP_BIN_REQUEST = 150 * 1024 * 1024; // 150MB
4646

47+
private static final String HTTP = "http";
48+
private static final String HTTPS = "https";
49+
private static final String HTTP_CODEC = "http-codec";
50+
private static final String HTTP_AGGREGATOR = "http-aggregator";
51+
private static final String HTTP_HANDLER = "http-handler";
52+
4753
private Logger logger = LoggerFactory.getLogger(NettyHttpClient.class);
4854

49-
protected Bootstrap bootStrap;
55+
protected Bootstrap httpBootstrap;
5056
protected URI baseUri;
5157
private EventLoopGroup group;
5258
private EventLoopGroup shutDownGroup;
@@ -82,16 +88,16 @@ public void initialize(String baseUrl, String username, String password) throws
8288
logger.debug("initialize url: {}, user: {}", baseUrl, username);
8389
baseUri = new URI(baseUrl);
8490
String protocol = baseUri.getScheme();
85-
if (!"http".equalsIgnoreCase(protocol) && !"https".equalsIgnoreCase(protocol)) {
91+
if (!HTTP.equalsIgnoreCase(protocol) && !HTTPS.equalsIgnoreCase(protocol)) {
8692
logger.warn("Not http(s), protocol: {}", protocol);
8793
throw new IllegalArgumentException("Unsupported protocol: " + protocol);
8894
}
8995
this.auth = "Basic " + Base64.encode(Unpooled.copiedBuffer((username + ":" + password), ARIEncoder.ENCODING)).toString(ARIEncoder.ENCODING);
90-
bootstrap();
96+
initHttpBootstrap();
9197
}
9298

93-
protected void bootstrap() {
94-
if (bootStrap == null) {
99+
protected void initHttpBootstrap() {
100+
if (httpBootstrap == null) {
95101
// Bootstrap is the factory for HTTP connections
96102
logger.debug("Bootstrap with\n" +
97103
" connection timeout: {},\n" +
@@ -100,17 +106,17 @@ protected void bootstrap() {
100106
CONNECTION_TIMEOUT_SEC,
101107
READ_TIMEOUT_SEC,
102108
MAX_HTTP_REQUEST);
103-
bootStrap = new Bootstrap();
104-
bootstrapOptions(bootStrap);
105-
bootStrap.handler(new ChannelInitializer<SocketChannel>() {
109+
httpBootstrap = new Bootstrap();
110+
bootstrapOptions(httpBootstrap);
111+
httpBootstrap.handler(new ChannelInitializer<SocketChannel>() {
106112
@Override
107113
public void initChannel(SocketChannel ch) throws Exception {
108114
ChannelPipeline pipeline = ch.pipeline();
109-
addSSLIfRequired(pipeline);
115+
addSSLIfRequired(pipeline, baseUri);
110116
pipeline.addLast("read-timeout", new ReadTimeoutHandler(READ_TIMEOUT_SEC));
111-
pipeline.addLast("http-codec", new HttpClientCodec());
112-
pipeline.addLast("http-aggregator", new HttpObjectAggregator(MAX_HTTP_REQUEST));
113-
pipeline.addLast("http-handler", new NettyHttpClientHandler());
117+
pipeline.addLast(HTTP_CODEC, new HttpClientCodec());
118+
pipeline.addLast(HTTP_AGGREGATOR, new HttpObjectAggregator(MAX_HTTP_REQUEST));
119+
pipeline.addLast(HTTP_HANDLER, new NettyHttpClientHandler());
114120
}
115121
});
116122
}
@@ -125,8 +131,8 @@ private void bootstrapOptions(Bootstrap bootStrap) {
125131
bootStrap.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECTION_TIMEOUT_SEC * 1000);
126132
}
127133

128-
private void addSSLIfRequired(ChannelPipeline pipeline) throws SSLException {
129-
if ("https".equalsIgnoreCase(baseUri.getScheme())) {
134+
private synchronized static void addSSLIfRequired(ChannelPipeline pipeline, URI baseUri) throws SSLException {
135+
if (HTTPS.equalsIgnoreCase(baseUri.getScheme())) {
130136
if (sslContext == null) {
131137
sslContext = SslContextBuilder.forClient().trustManager(InsecureTrustManagerFactory.INSTANCE).build();
132138
}
@@ -137,9 +143,9 @@ private void addSSLIfRequired(ChannelPipeline pipeline) throws SSLException {
137143
private int getPort() {
138144
int port = baseUri.getPort();
139145
if (port == -1) {
140-
if ("http".equalsIgnoreCase(baseUri.getScheme())) {
146+
if (HTTP.equalsIgnoreCase(baseUri.getScheme())) {
141147
port = 80;
142-
} else if ("https".equalsIgnoreCase(baseUri.getScheme())) {
148+
} else if (HTTPS.equalsIgnoreCase(baseUri.getScheme())) {
143149
port = 443;
144150
}
145151
}
@@ -148,7 +154,7 @@ private int getPort() {
148154

149155
protected ChannelFuture httpConnect() {
150156
logger.debug("HTTP Connect uri: {}", baseUri.toString());
151-
return bootStrap.connect(baseUri.getHost(), getPort());
157+
return httpBootstrap.connect(baseUri.getHost(), getPort());
152158
}
153159

154160
public void destroy() {
@@ -221,7 +227,7 @@ protected String buildURL(String path, List<HttpParam> parametersQuery, boolean
221227
// Factory for WS handshakes
222228
protected WebSocketClientHandshaker getWsHandshake(String path, List<HttpParam> parametersQuery) throws URISyntaxException {
223229
String url = buildURL(path, parametersQuery, true);
224-
if (url.regionMatches(true, 0, "http", 0, 4)) {
230+
if (url.regionMatches(true, 0, HTTP, 0, 4)) {
225231
// http(s):// -> ws(s)://
226232
url = "ws" + url.substring(4);
227233
}
@@ -306,7 +312,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
306312
}
307313
}
308314
}).syncUninterruptibly().channel();
309-
NettyHttpClientHandler handler = (NettyHttpClientHandler) ch.pipeline().get("http-handler");
315+
NettyHttpClientHandler handler = (NettyHttpClientHandler) ch.pipeline().get(HTTP_HANDLER);
310316
ch.writeAndFlush(request);
311317
ch.closeFuture().syncUninterruptibly();
312318
if (handler.getException() != null) {
@@ -322,7 +328,7 @@ private void replaceAggregator(boolean binary, Channel ch) {
322328
if (binary) {
323329
logger.debug("Is Binary, replace http-aggregator ...");
324330
ch.pipeline().replace(
325-
"http-aggregator", "http-aggregator", new HttpObjectAggregator(MAX_HTTP_BIN_REQUEST));
331+
HTTP_AGGREGATOR, HTTP_AGGREGATOR, new HttpObjectAggregator(MAX_HTTP_BIN_REQUEST));
326332
}
327333
}
328334

@@ -351,7 +357,7 @@ public void operationComplete(ChannelFuture future) throws Exception {
351357
public void operationComplete(ChannelFuture future) throws Exception {
352358
responseHandler.onResponseReceived();
353359
if (future.isSuccess()) {
354-
NettyHttpClientHandler handler = (NettyHttpClientHandler) future.channel().pipeline().get("http-handler");
360+
NettyHttpClientHandler handler = (NettyHttpClientHandler) future.channel().pipeline().get(HTTP_HANDLER);
355361
if (handler.getException() != null) {
356362
responseHandler.onFailure(new RestException(handler.getException()));
357363
} else if (httpResponseOkay(handler.getResponseStatus())) {
@@ -397,9 +403,9 @@ protected WsClientConnection connect(Bootstrap wsBootStrap, final HttpResponseHa
397403
@Override
398404
public void initChannel(SocketChannel ch) throws Exception {
399405
ChannelPipeline pipeline = ch.pipeline();
400-
addSSLIfRequired(pipeline);
401-
pipeline.addLast("http-codec", new HttpClientCodec());
402-
pipeline.addLast("http-aggregator", new HttpObjectAggregator(MAX_HTTP_REQUEST));
406+
addSSLIfRequired(pipeline, baseUri);
407+
pipeline.addLast(HTTP_CODEC, new HttpClientCodec());
408+
pipeline.addLast(HTTP_AGGREGATOR, new HttpObjectAggregator(MAX_HTTP_REQUEST));
403409
pipeline.addLast("ws-handler", wsHandler);
404410
}
405411
});
@@ -471,9 +477,9 @@ public void run() {
471477
for (int i = 0; i < 10; i++) {
472478
try {
473479
Thread.sleep(1000);
474-
} catch (InterruptedException e) {
480+
} catch (InterruptedException e) {//NOSONAR
475481
// probably from the reconnect, so stop running...
476-
return; //NOSONAR
482+
return;
477483
}
478484
if ((System.currentTimeMillis() - lastPong) < 10000) {
479485
logger.debug("Pong at {}", lastPong);

‎src/test/java/ch/loway/oss/ari4java/ARITest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ public void testBuildAction() throws Exception {
114114
public void testCreateUid() {
115115
String v = ARI.getUID();
116116
assertTrue("UID created", v.length() > 0);
117+
assertNotSame("new UID the same as previous", v, ARI.getUID());
117118
}
118119

119120
@Test

‎src/test/java/ch/loway/oss/ari4java/tools/http/NettyHttpClientTest.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,10 @@
99
import io.netty.buffer.Unpooled;
1010
import io.netty.channel.*;
1111
import io.netty.channel.embedded.EmbeddedChannel;
12-
import io.netty.channel.socket.SocketChannel;
1312
import io.netty.handler.codec.http.*;
1413
import io.netty.handler.codec.http.websocketx.WebSocketVersion;
15-
import io.netty.util.concurrent.EventExecutor;
1614
import org.junit.After;
1715
import org.junit.Test;
18-
import org.mockito.ArgumentCaptor;
1916

2017
import java.net.URI;
2118
import java.net.URISyntaxException;
@@ -35,7 +32,7 @@ public class NettyHttpClientTest {
3532

3633
private void initTestClient() throws URISyntaxException {
3734
client = new NettyHttpClient() {
38-
protected void bootstrap() {
35+
protected void initHttpBootstrap() {
3936
// for testing skip the bootstrapping
4037
}
4138

@@ -80,18 +77,19 @@ public void testInitialize() throws Exception {
8077
NettyHttpClient client = new NettyHttpClient();
8178
client.initialize("http://localhost:8088/", "user", "p@ss");
8279
client.destroy();
80+
assertNotNull(client.baseUri);
8381
}
8482

8583
@Test
8684
public void testHttpConnect() {
8785
Bootstrap bootstrap = mock(Bootstrap.class);
8886
NettyHttpClient client = new NettyHttpClient() {
8987
{
90-
bootstrap();
88+
initHttpBootstrap();
9189
}
9290
@Override
93-
protected void bootstrap() {
94-
bootStrap = bootstrap;
91+
protected void initHttpBootstrap() {
92+
httpBootstrap = bootstrap;
9593
try {
9694
baseUri = new URI("http://localhost:8088/");
9795
} catch (URISyntaxException e) {

0 commit comments

Comments
 (0)