Fixed Scariest level bugs from findbugs. 05/19805/2
authorMartin Bobak <mbobak@cisco.com>
Thu, 7 May 2015 14:07:55 +0000 (16:07 +0200)
committerMartin Bobak <mbobak@cisco.com>
Thu, 7 May 2015 14:57:15 +0000 (16:57 +0200)
Change-Id: I22b51dd31887c9c61a049b7c06835f082fd7fd4f
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
Signed-off-by: Martin Bobak <mbobak@cisco.com>
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceContextImpl.java
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/device/DeviceManagerImpl.java
openflowplugin-impl/src/main/java/org/opendaylight/openflowplugin/impl/statistics/StatisticsContextImpl.java

index 28751c699756108fc081dadb7a00609be37bf2b5..afe95d7660f2606345954647222aec478e1b5415 100644 (file)
@@ -90,7 +90,7 @@ public class DeviceContextImpl implements DeviceContext {
     private final DataBroker dataBroker;
     private final XidGenerator xidGenerator;
     private final HashedWheelTimer hashedWheelTimer;
-    private Map<Long, RequestContext> requests = new TreeMap();
+    private final Map<Long, RequestContext> requests = new TreeMap();
 
     private final Map<SwitchConnectionDistinguisher, ConnectionContext> auxiliaryConnectionContexts;
     private final TransactionChainManager txChainManager;
@@ -103,7 +103,7 @@ public class DeviceContextImpl implements DeviceContext {
     private NotificationService notificationService;
     private final MessageSpy<Class> messageSpy;
     private DeviceDisconnectedHandler deviceDisconnectedHandler;
-    private List<DeviceContextClosedHandler> closeHandlers = new ArrayList<>();
+    private final List<DeviceContextClosedHandler> closeHandlers = new ArrayList<>();
     private NotificationPublishService notificationPublishService;
 
 
@@ -143,10 +143,14 @@ public class DeviceContextImpl implements DeviceContext {
 
     @Override
     public void addAuxiliaryConenctionContext(final ConnectionContext connectionContext) {
-        final SwitchConnectionDistinguisher connectionDistinguisher = new SwitchConnectionCookieOFImpl(connectionContext.getFeatures().getAuxiliaryId());
+        final SwitchConnectionDistinguisher connectionDistinguisher = createConnectionDistinguisher(connectionContext);
         auxiliaryConnectionContexts.put(connectionDistinguisher, connectionContext);
     }
 
+    private SwitchConnectionDistinguisher createConnectionDistinguisher(final ConnectionContext connectionContext) {
+        return new SwitchConnectionCookieOFImpl(connectionContext.getFeatures().getAuxiliaryId());
+    }
+
     @Override
     public void removeAuxiliaryConenctionContext(final ConnectionContext connectionContext) {
         // TODO Auto-generated method stub
@@ -189,7 +193,7 @@ public class DeviceContextImpl implements DeviceContext {
     }
 
     @Override
-    public RequestContext lookupRequest(Xid xid) {
+    public RequestContext lookupRequest(final Xid xid) {
         return requests.get(xid.getValue());
     }
 
@@ -204,7 +208,7 @@ public class DeviceContextImpl implements DeviceContext {
     }
 
     @Override
-    public RequestContext unhookRequestCtx(Xid xid) {
+    public RequestContext unhookRequestCtx(final Xid xid) {
         return requests.remove(xid.getValue());
     }
 
@@ -281,7 +285,7 @@ public class DeviceContextImpl implements DeviceContext {
                     .withResult(ofHeaderList)
                     .build();
             replyFuture.set(rpcResult);
-            for (MultipartReply multipartReply : ofHeaderList) {
+            for (final MultipartReply multipartReply : ofHeaderList) {
                 messageSpy.spyMessage(multipartReply.getImplementedInterface(), MessageSpy.STATISTIC_GROUP.FROM_SWITCH_PUBLISHED_FAILURE);
             }
 
@@ -397,15 +401,15 @@ public class DeviceContextImpl implements DeviceContext {
             primaryConnectionContext.setConnectionState(ConnectionContext.CONNECTION_STATE.RIP);
             primaryConnectionContext.getConnectionAdapter().disconnect();
         }
-        for (Map.Entry<Long, RequestContext> entry : requests.entrySet()) {
+        for (final Map.Entry<Long, RequestContext> entry : requests.entrySet()) {
             RequestContextUtil.closeRequestContextWithRpcError(entry.getValue(), DEVICE_DISCONNECTED);
         }
-        for (ConnectionContext connectionContext : auxiliaryConnectionContexts.values()) {
+        for (final ConnectionContext connectionContext : auxiliaryConnectionContexts.values()) {
             if (connectionContext.getConnectionAdapter().isAlive()) {
                 connectionContext.getConnectionAdapter().disconnect();
             }
         }
-        for (DeviceContextClosedHandler deviceContextClosedHandler : closeHandlers) {
+        for (final DeviceContextClosedHandler deviceContextClosedHandler : closeHandlers) {
             deviceContextClosedHandler.onDeviceContextClosed(this);
         }
 
@@ -416,14 +420,15 @@ public class DeviceContextImpl implements DeviceContext {
         if (this.getPrimaryConnectionContext().equals(connectionContext)) {
             try {
                 close();
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 LOG.trace("Error closing device context.");
             }
             if (null != deviceDisconnectedHandler) {
                 deviceDisconnectedHandler.onDeviceDisconnected(connectionContext);
             }
         } else {
-            auxiliaryConnectionContexts.remove(connectionContext);
+            final SwitchConnectionDistinguisher connectionDistinguisher = createConnectionDistinguisher(connectionContext);
+            auxiliaryConnectionContexts.remove(connectionDistinguisher);
         }
     }
 
index 36769ce0c207ef0f07fe009b8eaca623c310e9bb..ad40a7cbebf3be7cf07a7a2dad8e219e727d5a66 100644 (file)
@@ -30,6 +30,7 @@ import org.opendaylight.controller.md.sal.binding.api.NotificationPublishService
 import org.opendaylight.controller.md.sal.binding.api.NotificationService;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.openflowplugin.api.ConnectionException;
 import org.opendaylight.openflowplugin.api.OFConstants;
 import org.opendaylight.openflowplugin.api.openflow.connection.ConnectionContext;
 import org.opendaylight.openflowplugin.api.openflow.device.DeviceContext;
@@ -102,7 +103,7 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
 
     private static final long TICK_DURATION = 500; // 0.5 sec.
     private ScheduledThreadPoolExecutor spyPool;
-    private int spyRate = 10;
+    private final int spyRate = 10;
 
     private final DataBroker dataBroker;
     private final HashedWheelTimer hashedWheelTimer;
@@ -177,9 +178,10 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
 
         deviceContext.attachOpenflowMessageListener(messageListener);
 
-        ListenableFuture<List<RpcResult<List<MultipartReply>>>> deviceFeaturesFuture = null;
+        final ListenableFuture<List<RpcResult<List<MultipartReply>>>> deviceFeaturesFuture;
 
-        if (connectionContext.getFeatures().getVersion() == OFConstants.OFP_VERSION_1_0) {
+        final Short version = connectionContext.getFeatures().getVersion();
+        if (OFConstants.OFP_VERSION_1_0 == version) {
             final CapabilitiesV10 capabilitiesV10 = connectionContext.getFeatures().getCapabilitiesV10();
 
             DeviceStateUtil.setDeviceStateBasedOnV10Capabilities(deviceState, capabilitiesV10);
@@ -203,10 +205,12 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
                 final InstanceIdentifier<NodeConnector> connectorII = deviceState.getNodeInstanceIdentifier().child(NodeConnector.class, connector.getKey());
                 deviceContext.writeToTransaction(LogicalDatastoreType.OPERATIONAL, connectorII, connector);
             }
-        } else if (connectionContext.getFeatures().getVersion() == OFConstants.OFP_VERSION_1_3) {
+        } else if (OFConstants.OFP_VERSION_1_3 == version) {
             final Capabilities capabilities = connectionContext.getFeatures().getCapabilities();
             DeviceStateUtil.setDeviceStateBasedOnV13Capabilities(deviceState, capabilities);
             deviceFeaturesFuture = createDeviceFeaturesForOF13(messageListener, deviceContext, deviceState);
+        } else {
+            deviceFeaturesFuture = Futures.immediateFailedFuture(new ConnectionException("Unsupported version " + version));
         }
 
         Futures.addCallback(deviceFeaturesFuture, new FutureCallback<List<RpcResult<List<MultipartReply>>>>() {
@@ -225,12 +229,12 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
         });
     }
 
-    private void chainTableTrunkWriteOF10(final DeviceContext deviceContext, ListenableFuture<List<RpcResult<List<MultipartReply>>>> deviceFeaturesFuture) {
+    private void chainTableTrunkWriteOF10(final DeviceContext deviceContext, final ListenableFuture<List<RpcResult<List<MultipartReply>>>> deviceFeaturesFuture) {
         Futures.addCallback(deviceFeaturesFuture, new FutureCallback<List<RpcResult<List<MultipartReply>>>>() {
             @Override
-            public void onSuccess(List<RpcResult<List<MultipartReply>>> results) {
+            public void onSuccess(final List<RpcResult<List<MultipartReply>>> results) {
                 boolean allSucceeded = true;
-                for (RpcResult<List<MultipartReply>> rpcResult : results) {
+                for (final RpcResult<List<MultipartReply>> rpcResult : results) {
                     allSucceeded &= rpcResult.isSuccessful();
                 }
                 if (allSucceeded) {
@@ -243,28 +247,28 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
             }
 
             @Override
-            public void onFailure(Throwable t) {
+            public void onFailure(final Throwable t) {
                 //NOOP
             }
         });
     }
 
 
-    private ListenableFuture<RpcResult<List<MultipartReply>>> processReplyDesc(OpenflowProtocolListenerFullImpl messageListener,
-                                                                               DeviceContext deviceContext,
-                                                                               DeviceState deviceState) {
+    private ListenableFuture<RpcResult<List<MultipartReply>>> processReplyDesc(final OpenflowProtocolListenerFullImpl messageListener,
+                                                                               final DeviceContext deviceContext,
+                                                                               final DeviceState deviceState) {
         final ListenableFuture<RpcResult<List<MultipartReply>>> replyDesc = getNodeStaticInfo(messageListener,
                 MultipartType.OFPMPDESC, deviceContext, deviceState.getNodeInstanceIdentifier(), deviceState.getVersion());
         return replyDesc;
     }
 
-    private ListenableFuture<List<RpcResult<List<MultipartReply>>>> createDeviceFeaturesForOF10(OpenflowProtocolListenerFullImpl messageListener,
-                                                                                                DeviceContext deviceContext,
-                                                                                                DeviceState deviceState) {
+    private ListenableFuture<List<RpcResult<List<MultipartReply>>>> createDeviceFeaturesForOF10(final OpenflowProtocolListenerFullImpl messageListener,
+                                                                                                final DeviceContext deviceContext,
+                                                                                                final DeviceState deviceState) {
         return Futures.allAsList(Arrays.asList(processReplyDesc(messageListener, deviceContext, deviceState)));
     }
 
-    private ListenableFuture<List<RpcResult<List<MultipartReply>>>> createDeviceFeaturesForOF13(OpenflowProtocolListenerFullImpl messageListener,
+    private ListenableFuture<List<RpcResult<List<MultipartReply>>>> createDeviceFeaturesForOF13(final OpenflowProtocolListenerFullImpl messageListener,
                                                                                                 final DeviceContext deviceContext,
                                                                                                 final DeviceState deviceState) {
         final ListenableFuture<RpcResult<List<MultipartReply>>> replyDesc = processReplyDesc(messageListener, deviceContext, deviceState);
@@ -442,13 +446,13 @@ public class DeviceManagerImpl implements DeviceManager, AutoCloseable {
 
     @Override
     public void close() throws Exception {
-        for (DeviceContext deviceContext : synchronizedDeviceContextsList) {
+        for (final DeviceContext deviceContext : synchronizedDeviceContextsList) {
             deviceContext.close();
         }
     }
 
     private static void createEmptyFlowCapableNodeInDs(final DeviceContext deviceContext) {
-        FlowCapableNodeBuilder flowCapableNodeBuilder = new FlowCapableNodeBuilder();
+        final FlowCapableNodeBuilder flowCapableNodeBuilder = new FlowCapableNodeBuilder();
         final InstanceIdentifier<FlowCapableNode> fNodeII = deviceContext.getDeviceState().getNodeInstanceIdentifier().augmentation(FlowCapableNode.class);
         deviceContext.writeToTransaction(LogicalDatastoreType.OPERATIONAL, fNodeII, flowCapableNodeBuilder.build());
     }
index eaf1aa320ab379fce00d2c8ebc943d6fe150654f..4a9d382d93934b2dca76a9e53fd4e09e7ccb28f1 100644 (file)
@@ -56,7 +56,7 @@ public class StatisticsContextImpl implements StatisticsContext {
 
         if (ConnectionContext.CONNECTION_STATE.WORKING.equals(deviceContext.getPrimaryConnectionContext().getConnectionState())) {
             final DeviceState devState = deviceContext.getDeviceState();
-            ListenableFuture<Boolean> emptyFuture = Futures.immediateFuture(new Boolean(false));
+            final ListenableFuture<Boolean> emptyFuture = Futures.immediateFuture(new Boolean(false));
             final ListenableFuture<Boolean> flowStatistics = devState.isFlowStatisticsAvailable() ? wrapLoggingOnStatisticsRequestCall(MultipartType.OFPMPFLOW) : emptyFuture;
 
             final ListenableFuture<Boolean> tableStatistics = devState.isTableStatisticsAvailable() ? wrapLoggingOnStatisticsRequestCall(MultipartType.OFPMPTABLE) : emptyFuture;
@@ -77,7 +77,7 @@ public class StatisticsContextImpl implements StatisticsContext {
                 @Override
                 public void onSuccess(final List<Boolean> booleans) {
                     boolean atLeastOneSuccess = false;
-                    for (Boolean bool : booleans) {
+                    for (final Boolean bool : booleans) {
                         atLeastOneSuccess |= bool.booleanValue();
                     }
                     settableResultingFuture.set(new Boolean(atLeastOneSuccess));
@@ -121,7 +121,7 @@ public class StatisticsContextImpl implements StatisticsContext {
 
     @Override
     public <T> void forgetRequestContext(final RequestContext<T> requestContext) {
-        requestContexts.remove(requestContexts);
+        requestContexts.remove(requestContext);
     }
 
     @Override
@@ -137,7 +137,7 @@ public class StatisticsContextImpl implements StatisticsContext {
 
     @Override
     public void close() throws Exception {
-        for (RequestContext requestContext : requestContexts) {
+        for (final RequestContext requestContext : requestContexts) {
             RequestContextUtil.closeRequestContextWithRpcError(requestContext, CONNECTION_CLOSED);
         }
     }