Juniper-NETCONF provider: Extend UTs; fix warnings
New UTs based on NETCONF responses lab devices.
No functional changes here; it is refactoring only.
Change-Id: I6a011fce9496285539c7a11deb8fbac930b12483
diff --git a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/ControllerConfigJuniperImpl.java b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/ControllerConfigJuniperImpl.java
index 536c1bc..fde4b1d 100644
--- a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/ControllerConfigJuniperImpl.java
+++ b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/ControllerConfigJuniperImpl.java
@@ -79,7 +79,7 @@
}
}
- private String buildRpcGetOpenFlowController() {
+ private static String buildRpcGetOpenFlowController() {
StringBuilder rpc = new StringBuilder(RPC_TAG_NETCONF_BASE);
rpc.append("<get-configuration>");
@@ -102,7 +102,7 @@
return rpc.toString();
}
- private String buildRpcSetOpenFlowController(List<ControllerInfo> controllers) {
+ private static String buildRpcSetOpenFlowController(List<ControllerInfo> controllers) {
StringBuilder request = new StringBuilder();
request.append("<protocols>");
@@ -115,15 +115,15 @@
request.append("<ofagent-mode>");
request.append("<controller>");
- for (int i = 0; i < controllers.size(); i++) {
+ for (ControllerInfo controller : controllers) {
request.append("<ip>");
request.append("<name>");
- request.append(controllers.get(i).ip().toString());
+ request.append(controller.ip().toString());
request.append("</name>");
request.append("<protocol>");
request.append("<tcp>");
request.append("<port>");
- request.append(Integer.toString(controllers.get(i).port()));
+ request.append(controller.port());
request.append("</port>");
request.append("</tcp>");
request.append("</protocol>");
@@ -138,7 +138,7 @@
return cliSetRequestBuilder(request);
}
- private String buildRpcRemoveOpenFlowController() {
+ private static String buildRpcRemoveOpenFlowController() {
StringBuilder request = new StringBuilder();
request.append("<protocols>");
@@ -154,7 +154,7 @@
return cliDeleteRequestBuilder(request);
}
- private String buildCommit() {
+ private static String buildCommit() {
StringBuilder rpc = new StringBuilder(RPC_TAG_NETCONF_BASE);
rpc.append("<commit/>");
@@ -164,7 +164,7 @@
return rpc.toString();
}
- private String buildDiscardChanges() {
+ private static String buildDiscardChanges() {
StringBuilder rpc = new StringBuilder(RPC_TAG_NETCONF_BASE);
rpc.append("<discard-changes/>");
@@ -226,7 +226,7 @@
}
reply = session.requestSync(buildCommit()).trim();
- log.debug(reply);
+ log.debug("reply : {}", reply);
} catch (NetconfException e) {
log.debug(e.getMessage());
return false;
@@ -235,10 +235,7 @@
return true;
}
- private boolean isOK(String reply) {
- if (reply != null && reply.indexOf("<ok/>") >= 0) {
- return true;
- }
- return false;
+ private static boolean isOK(String reply) {
+ return reply != null && reply.contains("<ok/>");
}
}
\ No newline at end of file
diff --git a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/FlowRuleJuniperImpl.java b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/FlowRuleJuniperImpl.java
index 3f4adb6..075d3dc 100644
--- a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/FlowRuleJuniperImpl.java
+++ b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/FlowRuleJuniperImpl.java
@@ -81,7 +81,6 @@
implements FlowRuleProgrammable {
private static final String OK = "<ok/>";
- public static final String IP_STRING = "ip";
private final org.slf4j.Logger log = getLogger(getClass());
@Override
@@ -92,7 +91,7 @@
NetconfSession session = controller.getDevicesMap().get(devId).getSession();
if (session == null) {
log.warn("Device {} is not registered in netconf", devId);
- return Collections.EMPTY_LIST;
+ return Collections.emptyList();
}
//Installed static routes
@@ -273,7 +272,7 @@
//Find if the route refers to a local interface.
Optional<Port> local = deviceService.getPorts(devId).stream().filter(this::isIp)
.filter(p -> criteria.ip().getIp4Prefix().contains(
- Ip4Address.valueOf(p.annotations().value(IP_STRING)))).findAny();
+ Ip4Address.valueOf(p.annotations().value(JuniperUtils.AK_IP)))).findAny();
if (local.isPresent()) {
return Optional.of(new StaticRoute(criteria.ip().getIp4Prefix(),
@@ -310,12 +309,11 @@
* @return the output instruction
*/
private Optional<OutputInstruction> getOutput(FlowRule flowRule) {
- Optional<OutputInstruction> output = flowRule
+ return flowRule
.treatment().allInstructions().stream()
.filter(instruction -> instruction
.type() == Instruction.Type.OUTPUT)
- .map(x -> (OutputInstruction) x).findFirst();
- return output;
+ .map(OutputInstruction.class::cast).findFirst();
}
private String routingTableBuilder() {
@@ -339,10 +337,7 @@
e));
}
- if (replay != null && replay.indexOf(OK) >= 0) {
- return true;
- }
- return false;
+ return replay != null && replay.contains(OK);
}
private boolean rollback() {
@@ -359,10 +354,7 @@
e));
}
- if (replay != null && replay.indexOf(OK) >= 0) {
- return true;
- }
- return false;
+ return replay != null && replay.contains(OK);
}
/**
@@ -381,7 +373,7 @@
DeviceService deviceService = this.handler().get(DeviceService.class);
//Using only links with adjacency discovered by the LLDP protocol (see LinkDiscoveryJuniperImpl)
Map<DeviceId, Port> dstPorts = links.stream().filter(l ->
- IP_STRING.toUpperCase().equals(l.annotations().value(AnnotationKeys.LAYER)))
+ JuniperUtils.AK_IP.toUpperCase().equals(l.annotations().value(AnnotationKeys.LAYER)))
.collect(Collectors.toMap(
l -> l.dst().deviceId(),
l -> deviceService.getPort(l.dst().deviceId(), l.dst().port())));
@@ -391,10 +383,10 @@
Optional<Port> childPort = deviceService.getPorts(entry.getKey()).stream()
.filter(p -> Strings.nullToEmpty(
p.annotations().value(AnnotationKeys.PORT_NAME)).contains(portName.trim()))
- .filter(p -> isIp(p))
+ .filter(this::isIp)
.findAny();
if (childPort.isPresent()) {
- return Optional.ofNullable(Ip4Address.valueOf(childPort.get().annotations().value("ip")));
+ return Optional.ofNullable(Ip4Address.valueOf(childPort.get().annotations().value(JuniperUtils.AK_IP)));
}
}
@@ -409,13 +401,12 @@
* @return true if the IP address is present. Otherwise false.
*/
private boolean isIp(Port port) {
- String ip4 = port.annotations().value(IP_STRING);
- if (StringUtils.isEmpty(ip4)) {
+ final String ipv4 = port.annotations().value(JuniperUtils.AK_IP);
+ if (StringUtils.isEmpty(ipv4)) {
return false;
}
try {
-
- Ip4Address.valueOf(port.annotations().value(IP_STRING));
+ Ip4Address.valueOf(ipv4);
} catch (IllegalArgumentException e) {
return false;
}
diff --git a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/JuniperUtils.java b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/JuniperUtils.java
index 1fc4f6c..77612cb 100644
--- a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/JuniperUtils.java
+++ b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/JuniperUtils.java
@@ -19,6 +19,8 @@
import com.google.common.base.MoreObjects;
import org.apache.commons.configuration.HierarchicalConfiguration;
import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
import org.onlab.packet.ChassisId;
import org.onlab.packet.Ip4Address;
import org.onlab.packet.Ip4Prefix;
@@ -120,7 +122,7 @@
Pattern.compile(".*Private base address\\s*([:,0-9,a-f,A-F]*).*", Pattern.DOTALL);
- public static final String PROTOCOL_NAME = "protocol-name";
+ private static final String PROTOCOL_NAME = "protocol-name";
private static final String JUNIPER = "JUNIPER";
private static final String UNKNOWN = "UNKNOWN";
@@ -136,6 +138,12 @@
static final String AK_ENCAPSULATION = "encapsulation";
/**
+ * Annotation key for IP.
+ */
+ static final String AK_IP = "ip";
+
+
+ /**
* Annotation key for interface description.
*/
static final String AK_DESCRIPTION = "description";
@@ -168,7 +176,7 @@
/**
* Default port speed {@value} Mbps.
*/
- private static final long DEFAULT_PORT_SPEED = 1000;
+ static final long DEFAULT_PORT_SPEED = 1000;
private JuniperUtils() {
@@ -226,11 +234,11 @@
rpc.append("<routing-options>\n");
rpc.append("<static>\n");
rpc.append("<route>\n");
- rpc.append("<destination>" + staticRoute.ipv4Dst().toString() + "</destination>\n");
- rpc.append("<next-hop>" + staticRoute.nextHop() + "</next-hop>\n");
+ rpc.append("<destination>").append(staticRoute.ipv4Dst().toString()).append("</destination>\n");
+ rpc.append("<next-hop>").append(staticRoute.nextHop()).append("</next-hop>\n");
if (staticRoute.getMetric() != DEFAULT_METRIC_STATIC_ROUTE) {
- rpc.append("<metric>" + staticRoute.getMetric() + "</metric>");
+ rpc.append("<metric>").append(staticRoute.getMetric()).append("</metric>");
}
rpc.append("</route>\n");
@@ -419,7 +427,7 @@
// preserving former behavior
setIfNonNull(lannotations,
- "ip",
+ AK_IP,
logIntf.getString("address-family.interface-address.ifa-local"));
setIfNonNull(lannotations,
@@ -445,7 +453,7 @@
* @param portStatus port status
* @return "up" if {@code portStats} is {@literal true}, "down" otherwise
*/
- static String toUpDown(boolean portStatus) {
+ private static String toUpDown(boolean portStatus) {
return portStatus ? "up" : "down";
}
@@ -458,7 +466,7 @@
* @param speed in String
* @return Mbps
*/
- static long toMbps(String speed) {
+ private static long toMbps(String speed) {
String s = Strings.nullToEmpty(speed).trim().toLowerCase();
Matcher matcher = SPEED_PATTERN.matcher(s);
if (matcher.matches()) {
@@ -485,7 +493,7 @@
* @param key Annotation key
* @param value Annotation value (can be {@literal null})
*/
- static void setIfNonNull(Builder builder, String key, String value) {
+ private static void setIfNonNull(Builder builder, String key, String value) {
if (value != null) {
builder.set(key, value.trim());
}
@@ -499,7 +507,7 @@
* @param s port number as string
* @return PortNumber instance or null on error
*/
- static PortNumber safePortNumber(String s) {
+ private static PortNumber safePortNumber(String s) {
try {
return portNumber(s);
} catch (RuntimeException e) {
@@ -602,14 +610,14 @@
}
/**
- * Device representation of the adjacency at the IP Layer.
+ * Device representation of the adjacency at the IP Layer. It is immutable.
*/
static final class LinkAbstraction {
- protected String localPortName;
- protected ChassisId remoteChassisId;
- protected long remotePortIndex;
- protected String remotePortId;
- protected String remotePortDescription;
+ protected final String localPortName;
+ protected final ChassisId remoteChassisId;
+ protected final long remotePortIndex;
+ protected final String remotePortId;
+ protected final String remotePortDescription;
protected LinkAbstraction(String pName, long chassisId, long pIndex, String pPortId, String pDescription) {
this.localPortName = pName;
@@ -628,6 +636,38 @@
.add("remotePortDescription", remotePortDescription)
.toString();
}
+
+ @Override
+ public boolean equals(final Object o) {
+ if (this == o) {
+ return true;
+ }
+
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+
+ final LinkAbstraction that = (LinkAbstraction) o;
+
+ return new EqualsBuilder()
+ .append(remotePortIndex, that.remotePortIndex)
+ .append(localPortName, that.localPortName)
+ .append(remoteChassisId, that.remoteChassisId)
+ .append(remotePortId, that.remotePortId)
+ .append(remotePortDescription, that.remotePortDescription)
+ .isEquals();
+ }
+
+ @Override
+ public int hashCode() {
+ return new HashCodeBuilder(17, 37)
+ .append(localPortName)
+ .append(remoteChassisId)
+ .append(remotePortIndex)
+ .append(remotePortId)
+ .append(remotePortDescription)
+ .toHashCode();
+ }
}
enum OperationType {
@@ -737,7 +777,7 @@
}
public static List<ControllerInfo> getOpenFlowControllersFromConfig(HierarchicalConfiguration cfg) {
- List<ControllerInfo> controllers = new ArrayList<ControllerInfo>();
+ List<ControllerInfo> controllers = new ArrayList<>();
String ipKey = "configuration.protocols.openflow.mode.ofagent-mode.controller.ip";
if (!cfg.configurationsAt(ipKey).isEmpty()) {
diff --git a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/LinkDiscoveryJuniperImpl.java b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/LinkDiscoveryJuniperImpl.java
index bdb8883..95c02a1 100644
--- a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/LinkDiscoveryJuniperImpl.java
+++ b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/LinkDiscoveryJuniperImpl.java
@@ -83,13 +83,8 @@
//find source port by local port name
Optional<Port> localPort = deviceService.getPorts(localDeviceId).stream()
- .filter(port -> {
- if (linkAbs.localPortName.equals(
- port.annotations().value(PORT_NAME))) {
- return true;
- }
- return false;
- }).findAny();
+ .filter(port -> linkAbs.localPortName.equals(
+ port.annotations().value(PORT_NAME))).findAny();
if (!localPort.isPresent()) {
log.warn("Port name {} does not exist in device {}",
linkAbs.localPortName, localDeviceId);
diff --git a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/package-info.java b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/package-info.java
index 0831659..63f98cf 100644
--- a/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/package-info.java
+++ b/drivers/juniper/src/main/java/org/onosproject/drivers/juniper/package-info.java
@@ -15,6 +15,6 @@
*/
/**
- * Package for juniper device drivers.
+ * Package for Juniper Networks device drivers.
*/
package org.onosproject.drivers.juniper;