Skip to content

Commit

Permalink
Fix new Sonar smells
Browse files Browse the repository at this point in the history
  • Loading branch information
artembilan committed Mar 3, 2020
1 parent e6e9f45 commit fc4907d
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 34 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2001-2019 the original author or authors.
* Copyright 2001-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -36,18 +36,20 @@
* for each new connection.
*
* @author Gary Russell
* @author Artem Bilan
*
* @since 2.0
*/
public abstract class AbstractServerConnectionFactory extends AbstractConnectionFactory
implements TcpServerConnectionFactory, SchedulingAwareRunnable, OrderlyShutdownCapable {

private static final int DEFAULT_BACKLOG = 5;

private volatile boolean listening;
private int backlog = DEFAULT_BACKLOG;

private volatile String localAddress;
private String localAddress;

private volatile int backlog = DEFAULT_BACKLOG;
private volatile boolean listening;

private volatile boolean shuttingDown;

Expand Down Expand Up @@ -161,7 +163,7 @@ public String getLocalAddress() {

/**
* Used on multi-homed systems to enforce the server to listen
* on a specfic network address instead of all network adapters.
* on a specific network address instead of all network adapters.
* @param localAddress the ip address of the required adapter.
*/
public void setLocalAddress(String localAddress) {
Expand Down Expand Up @@ -200,19 +202,20 @@ public int afterShutdown() {
}

protected void publishServerExceptionEvent(Exception e) {
if (getApplicationEventPublisher() != null) {
getApplicationEventPublisher().publishEvent(new TcpConnectionServerExceptionEvent(this, e));
ApplicationEventPublisher applicationEventPublisher = getApplicationEventPublisher();
if (applicationEventPublisher != null) {
applicationEventPublisher.publishEvent(new TcpConnectionServerExceptionEvent(this, e));
}
}

protected void publishServerListeningEvent(int port) {
final ApplicationEventPublisher eventPublisher = getApplicationEventPublisher();
if (eventPublisher != null) {
final TcpConnectionServerListeningEvent event = new TcpConnectionServerListeningEvent(this, port);
TaskScheduler taskScheduler = this.getTaskScheduler();
TaskScheduler taskScheduler = getTaskScheduler();
if (taskScheduler != null) {
try {
taskScheduler.schedule((Runnable) () -> eventPublisher.publishEvent(event), new Date());
taskScheduler.schedule(() -> eventPublisher.publishEvent(event), new Date());
}
catch (@SuppressWarnings("unused") TaskRejectedException e) {
eventPublisher.publishEvent(event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.springframework.core.serializer.Serializer;
import org.springframework.integration.ip.IpHeaders;
import org.springframework.integration.support.AbstractIntegrationMessageBuilder;
import org.springframework.lang.Nullable;
import org.springframework.messaging.Message;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -147,7 +148,7 @@ public void registerSender(TcpSender sender) {
}
}

@Override // NOSONAR
@Override
protected TcpConnectionSupport obtainConnection() throws InterruptedException {
FailoverTcpConnection sharedConnection = (FailoverTcpConnection) getTheConnection();
boolean shared = !isSingleUse() && !this.cachingDelegates;
Expand All @@ -170,16 +171,18 @@ protected TcpConnectionSupport obtainConnection() throws InterruptedException {
return failoverTcpConnection;
}

private void closeRefreshedIfNecessary(FailoverTcpConnection sharedConnection, boolean refreshShared,
private void closeRefreshedIfNecessary(@Nullable FailoverTcpConnection sharedConnection, boolean refreshShared,
FailoverTcpConnection failoverTcpConnection) {

this.creationTime = System.currentTimeMillis();
/*
* We may have simply wrapped the same connection in a new wrapper; don't close.
*/
if (refreshShared && this.closeOnRefresh
&& sharedConnection != null

This comment has been minimized.

Copy link
@garyrussell

garyrussell Mar 3, 2020

Contributor

@artembilan This is unnecessary; refreshShared will always be false if this is false. Shortcoming of sonar logic (not unreasonable though).

This should be a // NOSONAR instead.

This comment has been minimized.

Copy link
@artembilan

artembilan Mar 3, 2020

Author Member

OK. Reverting...
Honestly that was my own decision: IDEA shows as a possible NPE from calling this closeRefreshedIfNecessary().
No Sonar report on this. Yet 😄

This comment has been minimized.

Copy link
@garyrussell

garyrussell Mar 3, 2020

Contributor

Also, I think this will trigger another Sonar smell (too many boolean parts > 4).

This comment has been minimized.

Copy link
@garyrussell

garyrussell Mar 3, 2020

Contributor

Ha! Perhaps Sonar is smarter than IDEA 😄

&& !sharedConnection.delegate.equals(failoverTcpConnection.delegate)
&& sharedConnection.isOpen()) {

sharedConnection.close();
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,7 +25,10 @@

/**
* A client connection factory that creates {@link TcpNetConnection}s.
*
* @author Gary Russell
* @author Artem Bilan
*
* @since 2.0
*
*/
Expand All @@ -48,9 +51,10 @@ public TcpNetClientConnectionFactory(String host, int port) {
@Override
protected TcpConnectionSupport buildNewConnection() {
try {
Socket socket = createSocket(this.getHost(), this.getPort());
Socket socket = createSocket(getHost(), getPort());
setSocketAttributes(socket);
TcpConnectionSupport connection = this.tcpNetConnectionSupport.createNewConnection(socket, false, isLookupHost(),
TcpConnectionSupport connection =
this.tcpNetConnectionSupport.createNewConnection(socket, false, isLookupHost(),
getApplicationEventPublisher(), getComponentName());
connection = wrapConnection(connection);
initializeConnection(connection, socket);
Expand All @@ -73,16 +77,24 @@ public void setTcpNetConnectionSupport(TcpNetConnectionSupport connectionSupport
this.tcpNetConnectionSupport = connectionSupport;
}

public void setTcpSocketFactorySupport(TcpSocketFactorySupport tcpSocketFactorySupport) {
Assert.notNull(tcpSocketFactorySupport, "TcpSocketFactorySupport may not be null");
this.tcpSocketFactorySupport = tcpSocketFactorySupport;
}

protected TcpSocketFactorySupport getTcpSocketFactorySupport() {
return this.tcpSocketFactorySupport;
}

@Override
public void start() {
this.setActive(true);
setActive(true);
super.start();
}

/**
* Create a new {@link Socket}. This default implementation uses the default
* {@link javax.net.SocketFactory}. Override to use some other mechanism
*
* @param host The host.
* @param port The port.
* @return The Socket
Expand All @@ -94,14 +106,4 @@ protected Socket createSocket(String host, int port) throws IOException {
return socket;
}

protected TcpSocketFactorySupport getTcpSocketFactorySupport() {
return this.tcpSocketFactorySupport;
}

public void setTcpSocketFactorySupport(
TcpSocketFactorySupport tcpSocketFactorySupport) {
Assert.notNull(tcpSocketFactorySupport, "TcpSocketFactorySupport may not be null");
this.tcpSocketFactorySupport = tcpSocketFactorySupport;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2019 the original author or authors.
* Copyright 2017-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,11 +19,15 @@
import java.net.Socket;

import org.springframework.context.ApplicationEventPublisher;
import org.springframework.lang.Nullable;


/**
* Used by NET connection factories to instantiate a {@link TcpNetConnection} object.
*
* @author Gary Russell
* @author Artem Bilan
*
* @since 5.0
*
*/
Expand All @@ -44,7 +48,7 @@ public interface TcpNetConnectionSupport {
*/
TcpNetConnection createNewConnection(Socket socket,
boolean server, boolean lookupHost,
ApplicationEventPublisher applicationEventPublisher,
@Nullable ApplicationEventPublisher applicationEventPublisher,
String connectionFactoryName);

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -201,7 +201,7 @@ private void processSelectorWhileActive() throws IOException {
int soTimeout = getSoTimeout();
int selectionCount = 0;
try {
long timeout = soTimeout < 0 ? 0 : soTimeout;
long timeout = Math.max(soTimeout, 0);
if (getDelayedReads().size() > 0 && (timeout == 0 || getReadDelay() < timeout)) {
timeout = getReadDelay();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,12 +19,15 @@
import java.nio.channels.SocketChannel;

import org.springframework.context.ApplicationEventPublisher;
import org.springframework.lang.Nullable;


/**
* Used by NIO connection factories to instantiate a {@link TcpNioConnection} object.
* Implementations for SSL and non-SSL {@link TcpNioConnection}s are provided.
*
* @author Gary Russell
*
* @since 2.2
*
*/
Expand All @@ -45,7 +48,7 @@ public interface TcpNioConnectionSupport {
*/
TcpNioConnection createNewConnection(SocketChannel socketChannel,
boolean server, boolean lookupHost,
ApplicationEventPublisher applicationEventPublisher,
@Nullable ApplicationEventPublisher applicationEventPublisher,
String connectionFactoryName);

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -187,7 +187,7 @@ private void doSelect(ServerSocketChannel server, final Selector selectorToSelec
int soTimeout = getSoTimeout();
int selectionCount = 0;
try {
long timeout = soTimeout < 0 ? 0 : soTimeout;
long timeout = Math.max(soTimeout, 0);
if (getDelayedReads().size() > 0 && (timeout == 0 || getReadDelay() < timeout)) {
timeout = getReadDelay();
}
Expand Down

0 comments on commit fc4907d

Please sign in to comment.