Fix Java lint reported errors
Replaced some API deprecations with TODO comments
Added suppressions for rawtypes fromes in OF code
Removed superfluous casts in OF code
Turned on -Werror to make future warnings break the build
Change-Id: I63a1770e1e2d0d97089d49261ac17c83fdd9b5e8
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index eb78c97..b464dbc 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -512,13 +512,15 @@
" during handshake {exception}",
explanation = "Could not process the switch description string",
recommendation = LogMessageDoc.CHECK_SWITCH)
+ @SuppressWarnings("unchecked")
void processSwitchDescReply() {
try {
// Read description, if it has been updated
- @SuppressWarnings("unchecked")
- Future<List<OFStatistics>> desc_future =
- (Future<List<OFStatistics>>) sw.
+ Future<?> future =
+ (Future<?>) sw.
getAttribute(IOFSwitch.SWITCH_DESCRIPTION_FUTURE);
+ Future<List<OFStatistics>> desc_future =
+ (Future<List<OFStatistics>>) future;
List<OFStatistics> values =
desc_future.get(0, TimeUnit.MILLISECONDS);
if (values != null) {
@@ -601,11 +603,11 @@
synchronized (roleChanger) {
// We need to keep track of all of the switches that are connected
- // to the controller, in any role, so that we can later send the
+ // to the controller, in any role, so that we can later send the
// role request messages when the controller role changes.
- // We need to be synchronized while doing this: we must not
+ // We need to be synchronized while doing this: we must not
// send a another role request to the connectedSwitches until
- // we were able to add this new switch to connectedSwitches
+ // we were able to add this new switch to connectedSwitches
// *and* send the current role to the new switch.
connectedSwitches.add(sw);
@@ -631,11 +633,11 @@
// This is a probe that we'll use to determine if the switch
// actually supports the role request message. If it does we'll
// get back a role reply message. If it doesn't we'll get back an
- // OFError message.
+ // OFError message.
// If role is MASTER we will promote switch to active
// list when we receive the switch's role reply messages
/*
- log.debug("This controller's role is {}, " +
+ log.debug("This controller's role is {}, " +
"sending initial role request msg to {}",
role, sw);
Collection<OFSwitchImpl> swList = new ArrayList<OFSwitchImpl>(1);
@@ -644,7 +646,7 @@
*/
} else {
// Role supported not enabled on controller (for now)
- // automatically promote switch to active state.
+ // automatically promote switch to active state.
log.debug("This controller's role is {}, " +
"not sending role request msg to {}",
role, sw);
@@ -671,15 +673,15 @@
}
/* Handle a role reply message we received from the switch. Since
- * netty serializes message dispatch we don't need to synchronize
+ * netty serializes message dispatch we don't need to synchronize
* against other receive operations from the same switch, so no need
* to synchronize addSwitch(), removeSwitch() operations from the same
- * connection.
+ * connection.
* FIXME: However, when a switch with the same DPID connects we do
* need some synchronization. However, handling switches with same
* DPID needs to be revisited anyways (get rid of r/w-lock and synchronous
* removedSwitch notification):1
- *
+ *
*/
@LogMessageDoc(level = "ERROR",
message = "Invalid role value in role reply message",
@@ -722,22 +724,22 @@
getAlwaysClearFlowsOnSwAdd()) {
// This is the first role-reply message we receive from
// this switch or roles were disabled when the switch
- // connected:
- // Delete all pre-existing flows for new connections to
+ // connected:
+ // Delete all pre-existing flows for new connections to
// the master
//
- // FIXME: Need to think more about what the test should
- // be for when we flush the flow-table? For example,
- // if all the controllers are temporarily in the backup
- // role (e.g. right after a failure of the master
- // controller) at the point the switch connects, then
- // all of the controllers will initially connect as
- // backup controllers and not flush the flow-table.
+ // FIXME: Need to think more about what the test should
+ // be for when we flush the flow-table? For example,
+ // if all the controllers are temporarily in the backup
+ // role (e.g. right after a failure of the master
+ // controller) at the point the switch connects, then
+ // all of the controllers will initially connect as
+ // backup controllers and not flush the flow-table.
// Then when one of them is promoted to master following
// the master controller election the flow-table
- // will still not be flushed because that's treated as
- // a failover event where we don't want to flush the
- // flow-table. The end result would be that the flow
+ // will still not be flushed because that's treated as
+ // a failover event where we don't want to flush the
+ // flow-table. The end result would be that the flow
// table for a newly connected switch is never
// flushed. Not sure how to handle that case though...
sw.clearAllFlowMods();
@@ -748,27 +750,27 @@
// Some switches don't seem to update us with port
// status messages while in slave role.
- //readSwitchPortStateFromStorage(sw);
+ //readSwitchPortStateFromStorage(sw);
- // Only add the switch to the active switch list if
- // we're not in the slave role. Note that if the role
- // attribute is null, then that means that the switch
+ // Only add the switch to the active switch list if
+ // we're not in the slave role. Note that if the role
+ // attribute is null, then that means that the switch
// doesn't support the role request messages, so in that
- // case we're effectively in the EQUAL role and the
+ // case we're effectively in the EQUAL role and the
// switch should be included in the active switch list.
addSwitch(sw);
log.debug("Added master switch {} to active switch list",
HexString.toHexString(sw.getId()));
} else if (isActive && !sw.isActive()) {
- // Transition from MASTER to SLAVE: remove switch
- // from active switch list.
+ // Transition from MASTER to SLAVE: remove switch
+ // from active switch list.
log.debug("Removed slave switch {} from active switch" +
" list", HexString.toHexString(sw.getId()));
removeSwitch(sw);
}
- // Indicate that we have received a role reply message.
+ // Indicate that we have received a role reply message.
state.firstRoleReplyReceived = true;
}
@@ -894,9 +896,9 @@
break;
case ERROR:
log.debug("Recieved ERROR message from switch {}: {}", sw, m);
- // TODO: we need better error handling. Especially for
+ // TODO: we need better error handling. Especially for
// request/reply style message (stats, roles) we should have
- // a unified way to lookup the xid in the error message.
+ // a unified way to lookup the xid in the error message.
// This will probable involve rewriting the way we handle
// request/reply style messages.
OFError error = (OFError) m;
@@ -907,9 +909,9 @@
boolean isBadVendorError =
(error.getErrorType() == OFError.OFErrorType.
OFPET_BAD_REQUEST.getValue());
- // We expect to receive a bad vendor error when
- // we're connected to a switch that doesn't support
- // the Nicira vendor extensions (i.e. not OVS or
+ // We expect to receive a bad vendor error when
+ // we're connected to a switch that doesn't support
+ // the Nicira vendor extensions (i.e. not OVS or
// derived from OVS). By protocol, it should also be
// BAD_VENDOR, but too many switch implementations
// get it wrong and we can already check the xid()
@@ -933,7 +935,7 @@
//is now never SLAVE, always MASTER.
// the switch doesn't understand role request
// messages and the current controller role is
- // slave. We need to disconnect the switch.
+ // slave. We need to disconnect the switch.
// @see RoleChanger for rationale
log.warn("Closing {} channel because controller's role " +
"is SLAVE", sw);
@@ -942,36 +944,36 @@
log.debug("Adding switch {} because we got an error" +
" returned from a MASTER role request", sw);
// Controller's role is master: add to
- // active
+ // active
// TODO: check if clearing flow table is
// right choice here.
// Need to clear FlowMods before we add the switch
// and dispatch updates otherwise we have a race condition.
// TODO: switch update is async. Won't we still have a potential
- // race condition?
+ // race condition?
sw.clearAllFlowMods();
addSwitch(sw);
}
}
} else {
// TODO: Is this the right thing to do if we receive
- // some other error besides a bad vendor error?
+ // some other error besides a bad vendor error?
// Presumably that means the switch did actually
- // understand the role request message, but there
+ // understand the role request message, but there
// was some other error from processing the message.
// OF 1.2 specifies a OFPET_ROLE_REQUEST_FAILED
- // error code, but it doesn't look like the Nicira
- // role request has that. Should check OVS source
+ // error code, but it doesn't look like the Nicira
+ // role request has that. Should check OVS source
// code to see if it's possible for any other errors
// to be returned.
// If we received an error the switch is not
- // in the correct role, so we need to disconnect it.
+ // in the correct role, so we need to disconnect it.
// We could also resend the request but then we need to
// check if there are other pending request in which
// case we shouldn't resend. If we do resend we need
// to make sure that the switch eventually accepts one
// of our requests or disconnect the switch. This feels
- // cumbersome.
+ // cumbersome.
log.debug("Closing {} channel because we recieved an " +
"error other than BAD_VENDOR", sw);
sw.getChannel().close();
@@ -995,8 +997,8 @@
}
break;
case PORT_STATUS:
- // We want to update our port state info even if we're in
- // the slave role, but we only want to update storage if
+ // We want to update our port state info even if we're in
+ // the slave role, but we only want to update storage if
// we're the master (or equal).
boolean updateStorage = state.hsState.
equals(HandshakeState.READY) &&
@@ -1019,20 +1021,20 @@
"from switch {} before switch is " +
"fully configured.", m.getType(), sw);
}
- // Check if the controller is in the slave role for the
- // switch. If it is, then don't dispatch the message to
+ // Check if the controller is in the slave role for the
+ // switch. If it is, then don't dispatch the message to
// the listeners.
- // TODO: Should we dispatch messages that we expect to
- // receive when we're in the slave role, e.g. port
- // status messages? Since we're "hiding" switches from
- // the listeners when we're in the slave role, then it
+ // TODO: Should we dispatch messages that we expect to
+ // receive when we're in the slave role, e.g. port
+ // status messages? Since we're "hiding" switches from
+ // the listeners when we're in the slave role, then it
// seems a little weird to dispatch port status messages
- // to them. On the other hand there might be special
+ // to them. On the other hand there might be special
// modules that care about all of the connected switches
// and would like to receive port status notifications.
else if (sw.getRole() == Role.SLAVE) {
- // Don't log message if it's a port status message
- // since we expect to receive those from the switch
+ // Don't log message if it's a port status message
+ // since we expect to receive those from the switch
// and don't want to emit spurious messages.
if (m.getType() != OFType.PORT_STATUS) {
log.debug("Ignoring message type {} received " +
@@ -1178,6 +1180,7 @@
explanation = "The switch sent a message not handled by " +
"the controller")
})
+ @SuppressWarnings("fallthrough")
protected void handleMessage(IOFSwitch sw, OFMessage m,
FloodlightContext bContext)
throws IOException {
diff --git a/src/main/java/net/floodlightcontroller/core/web/SwitchResourceBase.java b/src/main/java/net/floodlightcontroller/core/web/SwitchResourceBase.java
index 835cb17..47f6175 100644
--- a/src/main/java/net/floodlightcontroller/core/web/SwitchResourceBase.java
+++ b/src/main/java/net/floodlightcontroller/core/web/SwitchResourceBase.java
@@ -47,6 +47,7 @@
*
* @author readams
*/
+
public class SwitchResourceBase extends ServerResource {
protected final static Logger log = LoggerFactory.getLogger(SwitchResourceBase.class);
@@ -100,12 +101,12 @@
requestLength += specificReq.getLength();
} else if (statType == OFStatisticsType.PORT) {
OFPortStatisticsRequest specificReq = new OFPortStatisticsRequest();
- specificReq.setPortNumber((short) OFPort.OFPP_NONE.getValue());
+ specificReq.setPortNumber(OFPort.OFPP_NONE.getValue());
req.setStatistics(Collections.singletonList((OFStatistics) specificReq));
requestLength += specificReq.getLength();
} else if (statType == OFStatisticsType.QUEUE) {
OFQueueStatisticsRequest specificReq = new OFQueueStatisticsRequest();
- specificReq.setPortNumber((short) OFPort.OFPP_ALL.getValue());
+ specificReq.setPortNumber(OFPort.OFPP_ALL.getValue());
// LOOK! openflowj does not define OFPQ_ALL! pulled this from openflow.h
// note that I haven't seen this work yet though...
specificReq.setQueueId(0xffffffff);
diff --git a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
index df70dd4..9fd8870 100644
--- a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
+++ b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
@@ -374,6 +374,7 @@
ShortestPathIntent intent = new ShortestPathIntent(intentId,
sw.getDpid(), inPort.getNumber(), srcMacAddress.toLong(),
destinationDpid, destinationPort, dstMacAddress.toLong());
+
intent.setIdleTimeout(idleTimeout + SRC_SWITCH_TIMEOUT_ADJUST_SECOND);
intent.setFirstSwitchIdleTimeout(idleTimeout);
IntentOperation.Operator operator = IntentOperation.Operator.ADD;
diff --git a/src/main/java/net/onrc/onos/core/datagrid/IEventChannel.java b/src/main/java/net/onrc/onos/core/datagrid/IEventChannel.java
index 0c1663a..23b7be9 100644
--- a/src/main/java/net/onrc/onos/core/datagrid/IEventChannel.java
+++ b/src/main/java/net/onrc/onos/core/datagrid/IEventChannel.java
@@ -79,7 +79,7 @@
* @param key the key of the entry to get.
* @return the entry if found, otherwise null.
*/
- @Deprecated
+ // TODO - this is intended to be refactored and removed
V getEntry(K key);
/**
@@ -87,12 +87,12 @@
*
* @return all entries that are currently in the channel.
*/
- @Deprecated
+ // TODO - this is intended to be refactored and removed
Collection<V> getAllEntries();
/**
* Remove all entries in the channel.
*/
- @Deprecated
+ // TODO - this is intended to be refactored and removed
void removeAllEntries();
}
diff --git a/src/main/java/net/onrc/onos/core/datastore/serializers/Device.java b/src/main/java/net/onrc/onos/core/datastore/serializers/Device.java
index 41b9991..0bbf5af 100644
--- a/src/main/java/net/onrc/onos/core/datastore/serializers/Device.java
+++ b/src/main/java/net/onrc/onos/core/datastore/serializers/Device.java
@@ -588,7 +588,7 @@
public Builder addAllPortIds(
java.lang.Iterable<? extends com.google.protobuf.ByteString> values) {
ensurePortIdsIsMutable();
- super.addAll(values, portIds_);
+ Builder.addAll(values, portIds_);
onChanged();
return this;
}
diff --git a/src/main/java/net/onrc/onos/core/intent/ShortestPathIntent.java b/src/main/java/net/onrc/onos/core/intent/ShortestPathIntent.java
index 660cb7c..3c8d345 100644
--- a/src/main/java/net/onrc/onos/core/intent/ShortestPathIntent.java
+++ b/src/main/java/net/onrc/onos/core/intent/ShortestPathIntent.java
@@ -110,42 +110,42 @@
return pathIntentId;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public int getIdleTimeout() {
return idleTimeout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public int getHardTimeout() {
return hardTimeout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public void setIdleTimeout(int idleTimeout) {
this.idleTimeout = idleTimeout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public void setHardTimeout(int hardTimeout) {
this.hardTimeout = hardTimeout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public int getFirstSwitchIdleTimeout() {
return firstSwitchIdleTimeout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public int getFirstSwitchHardTimetout() {
return firstSwitchHardTimetout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public void setFirstSwitchIdleTimeout(int firstSwitchIdleTimeout) {
this.firstSwitchIdleTimeout = firstSwitchIdleTimeout;
}
- @Deprecated
+ // TODO - this is intended to be refactored and removed
public void setFirstSwitchHardTimetout(int firstSwitchHardTimetout) {
this.firstSwitchHardTimetout = firstSwitchHardTimetout;
}
diff --git a/src/main/java/org/openflow/protocol/OFFlowMod.java b/src/main/java/org/openflow/protocol/OFFlowMod.java
index 51b5de3..0ec8be9 100644
--- a/src/main/java/org/openflow/protocol/OFFlowMod.java
+++ b/src/main/java/org/openflow/protocol/OFFlowMod.java
@@ -367,7 +367,7 @@
flowMod.setMatch(neoMatch);
List<OFAction> neoActions = new LinkedList<OFAction>();
for(OFAction action: this.actions)
- neoActions.add((OFAction) action.clone());
+ neoActions.add(action.clone());
flowMod.setActions(neoActions);
return flowMod;
}
diff --git a/src/main/java/org/openflow/protocol/OFMatchBeanInfo.java b/src/main/java/org/openflow/protocol/OFMatchBeanInfo.java
index 16a813f..5309ce0 100644
--- a/src/main/java/org/openflow/protocol/OFMatchBeanInfo.java
+++ b/src/main/java/org/openflow/protocol/OFMatchBeanInfo.java
@@ -37,7 +37,7 @@
* @author Rob Sherwood (rob.sherwood@stanford.edu)
*
*/
-
+@SuppressWarnings("rawtypes")
public class OFMatchBeanInfo extends SimpleBeanInfo {
@Override
diff --git a/src/main/java/org/openflow/protocol/OFType.java b/src/main/java/org/openflow/protocol/OFType.java
index c828f0a..7428c9c 100644
--- a/src/main/java/org/openflow/protocol/OFType.java
+++ b/src/main/java/org/openflow/protocol/OFType.java
@@ -27,6 +27,7 @@
* @author David Erickson (daviderickson@cs.stanford.edu)
*
*/
+@SuppressWarnings("rawtypes")
public enum OFType {
HELLO (0, OFHello.class, new Instantiable<OFMessage>() {
@Override
diff --git a/src/main/java/org/openflow/protocol/action/OFActionType.java b/src/main/java/org/openflow/protocol/action/OFActionType.java
index b0c2c47..84c0801 100644
--- a/src/main/java/org/openflow/protocol/action/OFActionType.java
+++ b/src/main/java/org/openflow/protocol/action/OFActionType.java
@@ -30,6 +30,7 @@
*
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
+@SuppressWarnings("rawtypes")
public enum OFActionType {
OUTPUT (0, OFActionOutput.class, new Instantiable<OFAction>() {
@Override
diff --git a/src/main/java/org/openflow/protocol/statistics/OFStatisticsType.java b/src/main/java/org/openflow/protocol/statistics/OFStatisticsType.java
index f2b9e30..4a84674 100644
--- a/src/main/java/org/openflow/protocol/statistics/OFStatisticsType.java
+++ b/src/main/java/org/openflow/protocol/statistics/OFStatisticsType.java
@@ -22,6 +22,7 @@
import org.openflow.protocol.Instantiable;
import org.openflow.protocol.OFType;
+@SuppressWarnings("rawtypes")
public enum OFStatisticsType {
DESC (0, OFDescriptionStatistics.class, OFDescriptionStatistics.class,
new Instantiable<OFStatistics>() {
diff --git a/src/test/java/net/onrc/onos/apps/proxyarp/ProxyArpManagerTest.java b/src/test/java/net/onrc/onos/apps/proxyarp/ProxyArpManagerTest.java
index e34bde9..f0ebc7a 100644
--- a/src/test/java/net/onrc/onos/apps/proxyarp/ProxyArpManagerTest.java
+++ b/src/test/java/net/onrc/onos/apps/proxyarp/ProxyArpManagerTest.java
@@ -44,6 +44,7 @@
@RunWith(PowerMockRunner.class)
@PrepareForTest({ ProxyArpManager.class, ArpCache.class })
+@SuppressWarnings({ "rawtypes", "unchecked" })
public class ProxyArpManagerTest {
String defaultStrAgingMsec = "60000";
String defaultStrCleanupMsec = "60000";
diff --git a/src/test/java/net/onrc/onos/core/devicemanager/OnosDeviceManagerTest.java b/src/test/java/net/onrc/onos/core/devicemanager/OnosDeviceManagerTest.java
index dd104e2..fc0a008 100644
--- a/src/test/java/net/onrc/onos/core/devicemanager/OnosDeviceManagerTest.java
+++ b/src/test/java/net/onrc/onos/core/devicemanager/OnosDeviceManagerTest.java
@@ -64,6 +64,7 @@
@Override
@Before
+ @SuppressWarnings("unchecked")
public void setUp() throws Exception {
super.setUp();
MockTopology topology = new MockTopology();
diff --git a/src/test/java/org/openflow/protocol/OFErrorTest.java b/src/test/java/org/openflow/protocol/OFErrorTest.java
index 45d5257..7acd727 100644
--- a/src/test/java/org/openflow/protocol/OFErrorTest.java
+++ b/src/test/java/org/openflow/protocol/OFErrorTest.java
@@ -34,14 +34,14 @@
public void testWriteRead() throws Exception {
OFError msg = (OFError) messageFactory.getMessage(OFType.ERROR);
msg.setMessageFactory(messageFactory);
- msg.setErrorType((short) OFErrorType.OFPET_HELLO_FAILED.getValue());
+ msg.setErrorType(OFErrorType.OFPET_HELLO_FAILED.getValue());
msg.setErrorCode((short) OFHelloFailedCode.OFPHFC_INCOMPATIBLE
.ordinal());
ChannelBuffer bb = ChannelBuffers.dynamicBuffer();
bb.clear();
msg.writeTo(bb);
msg.readFrom(bb);
- TestCase.assertEquals((short) OFErrorType.OFPET_HELLO_FAILED.getValue(),
+ TestCase.assertEquals(OFErrorType.OFPET_HELLO_FAILED.getValue(),
msg.getErrorType());
TestCase.assertEquals((short) OFHelloFailedCode.OFPHFC_INCOMPATIBLE
.ordinal(), msg.getErrorType());
@@ -51,7 +51,7 @@
bb.clear();
msg.writeTo(bb);
msg.readFrom(bb);
- TestCase.assertEquals((short) OFErrorType.OFPET_HELLO_FAILED.getValue(),
+ TestCase.assertEquals(OFErrorType.OFPET_HELLO_FAILED.getValue(),
msg.getErrorType());
TestCase.assertEquals((short) OFHelloFailedCode.OFPHFC_INCOMPATIBLE
.ordinal(), msg.getErrorType());