From c8b33a4b00b6821bf54f3204f81c3115597dbb4f Mon Sep 17 00:00:00 2001 From: Milos Fabian Date: Mon, 7 Jul 2014 12:59:23 +0200 Subject: [PATCH] Bug-731: fixed segment-routing sonar issues. Change-Id: Ie6e120f38ebf036fc09cb601d9c46a572cdffe7e Signed-off-by: Milos Fabian --- .../Sr02PCEPSessionProposalFactoryModule.java | 19 +++++--- ...CInitiated00SrpObjectWithPstTlvParser.java | 8 ++-- .../type01/PcepRpObjectWithPstTlvParser.java | 8 ++-- .../routing02/SrEroSubobjectParser.java | 5 +- .../pcep/segment/routing02/SrEroUtil.java | 6 +-- .../routing02/SrPcInitiateMessageParser.java | 8 ++-- .../routing02/SrPcRepMessageParser.java | 48 +++++++++---------- 7 files changed, 48 insertions(+), 54 deletions(-) diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/controller/config/yang/pcep/sr02/cfg/Sr02PCEPSessionProposalFactoryModule.java b/pcep/segment-routing/src/main/java/org/opendaylight/controller/config/yang/pcep/sr02/cfg/Sr02PCEPSessionProposalFactoryModule.java index 7f3e74d49e..0fe43331bc 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/controller/config/yang/pcep/sr02/cfg/Sr02PCEPSessionProposalFactoryModule.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/controller/config/yang/pcep/sr02/cfg/Sr02PCEPSessionProposalFactoryModule.java @@ -19,6 +19,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class Sr02PCEPSessionProposalFactoryModule extends org.opendaylight.controller.config.yang.pcep.sr02.cfg.AbstractSr02PCEPSessionProposalFactoryModule { + + private static final String VALUE_IS_NOT_SET = "value is not set."; + + private static final int DEADTIMER_KEEPALIVE_RATIO = 4; + private static final Logger LOG = LoggerFactory.getLogger(Sr02PCEPSessionProposalFactoryModule.class); public Sr02PCEPSessionProposalFactoryModule(org.opendaylight.controller.config.api.ModuleIdentifier identifier, org.opendaylight.controller.config.api.DependencyResolver dependencyResolver) { @@ -31,22 +36,22 @@ public class Sr02PCEPSessionProposalFactoryModule extends org.opendaylight.contr @Override public void customValidation() { - JmxAttributeValidationException.checkNotNull(getActive(), "value is not set.", activeJmxAttribute); - JmxAttributeValidationException.checkNotNull(getInitiated(), "value is not set.", initiatedJmxAttribute); - JmxAttributeValidationException.checkNotNull(getDeadTimerValue(), "value is not set.", deadTimerValueJmxAttribute); - JmxAttributeValidationException.checkNotNull(getKeepAliveTimerValue(), "value is not set.", keepAliveTimerValueJmxAttribute); + JmxAttributeValidationException.checkNotNull(getActive(), VALUE_IS_NOT_SET, activeJmxAttribute); + JmxAttributeValidationException.checkNotNull(getInitiated(), VALUE_IS_NOT_SET, initiatedJmxAttribute); + JmxAttributeValidationException.checkNotNull(getDeadTimerValue(), VALUE_IS_NOT_SET, deadTimerValueJmxAttribute); + JmxAttributeValidationException.checkNotNull(getKeepAliveTimerValue(), VALUE_IS_NOT_SET, keepAliveTimerValueJmxAttribute); if (getKeepAliveTimerValue() != 0) { JmxAttributeValidationException.checkCondition(getKeepAliveTimerValue() >= 1, "minimum value is 1.", keepAliveTimerValueJmxAttribute); - if (getDeadTimerValue() != 0 && (getDeadTimerValue() / getKeepAliveTimerValue() != 4)) { + if (getDeadTimerValue() != 0 && (getDeadTimerValue() / getKeepAliveTimerValue() != DEADTIMER_KEEPALIVE_RATIO)) { LOG.warn("DeadTimerValue should be 4 times greater than KeepAliveTimerValue"); } } if (getActive() && !getStateful()) { setStateful(true); } - JmxAttributeValidationException.checkNotNull(getStateful(), "value is not set.", statefulJmxAttribute); - JmxAttributeValidationException.checkNotNull(getSrCapable(), "value is not set.", srCapableJmxAttribute); + JmxAttributeValidationException.checkNotNull(getStateful(), VALUE_IS_NOT_SET, statefulJmxAttribute); + JmxAttributeValidationException.checkNotNull(getSrCapable(), VALUE_IS_NOT_SET, srCapableJmxAttribute); } @Override diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/CInitiated00SrpObjectWithPstTlvParser.java b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/CInitiated00SrpObjectWithPstTlvParser.java index ef7136bb3c..f89bb49956 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/CInitiated00SrpObjectWithPstTlvParser.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/CInitiated00SrpObjectWithPstTlvParser.java @@ -33,11 +33,9 @@ public class CInitiated00SrpObjectWithPstTlvParser extends CInitiated00SrpObject super.addTlv(builder, tlv); final Tlvs7Builder tlvBuilder = new Tlvs7Builder(); if (builder.getTlvs() != null) { - if (builder.getTlvs().getAugmentation(Tlvs7.class) != null) { - final Tlvs7 t = builder.getTlvs().getAugmentation(Tlvs7.class); - if (t.getPathSetupType() != null) { - tlvBuilder.setPathSetupType(t.getPathSetupType()); - } + final Tlvs7 tlvs = builder.getTlvs().getAugmentation(Tlvs7.class); + if (tlvs != null && tlvs.getPathSetupType() != null) { + tlvBuilder.setPathSetupType(tlvs.getPathSetupType()); } } if (tlv instanceof PathSetupType) { diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/PcepRpObjectWithPstTlvParser.java b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/PcepRpObjectWithPstTlvParser.java index 34e1e2fe9c..43980fd557 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/PcepRpObjectWithPstTlvParser.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/lsp/setup/type01/PcepRpObjectWithPstTlvParser.java @@ -33,11 +33,9 @@ public class PcepRpObjectWithPstTlvParser extends PCEPRequestParameterObjectPars super.addTlv(builder, tlv); final Tlvs1Builder tlvBuilder = new Tlvs1Builder(); if (builder.getTlvs() != null) { - if (builder.getTlvs().getAugmentation(Tlvs1.class) != null) { - final Tlvs1 t = builder.getTlvs().getAugmentation(Tlvs1.class); - if (t.getPathSetupType() != null) { - tlvBuilder.setPathSetupType(t.getPathSetupType()); - } + final Tlvs1 tlvs = builder.getTlvs().getAugmentation(Tlvs1.class); + if (tlvs != null && tlvs.getPathSetupType() != null) { + tlvBuilder.setPathSetupType(tlvs.getPathSetupType()); } } if (tlv instanceof PathSetupType) { diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroSubobjectParser.java b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroSubobjectParser.java index c16a5a1b6b..0ea0335a8f 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroSubobjectParser.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroSubobjectParser.java @@ -50,6 +50,7 @@ public class SrEroSubobjectParser implements EROSubobjectParser, EROSubobjectSer private static final int FLAGS_OFFSET = 1; private static final int HEADER_LENGTH = FLAGS_OFFSET + 1; private static final int MINIMAL_LENGTH = SID_LENGTH + HEADER_LENGTH; + private static final int SID_TYPE_BITS_OFFSET = 4; private static final int M_FLAG_POSITION = 7; private static final int C_FLAG_POSITION = 6; @@ -64,7 +65,7 @@ public class SrEroSubobjectParser implements EROSubobjectParser, EROSubobjectSer final SrEroSubobject srEroSubobject = (SrEroSubobject) subobject.getSubobjectType(); final ByteBuf body = Unpooled.buffer(MINIMAL_LENGTH); - writeUnsignedByte((short)(srEroSubobject.getSidType().getIntValue() << 4), body); + writeUnsignedByte((short)(srEroSubobject.getSidType().getIntValue() << SID_TYPE_BITS_OFFSET), body); final Flags flags = srEroSubobject.getFlags(); final BitSet bits = new BitSet(); @@ -116,7 +117,7 @@ public class SrEroSubobjectParser implements EROSubobjectParser, EROSubobjectSer + ";"); } final SrEroTypeBuilder srEroSubobjectBuilder = new SrEroTypeBuilder(); - final int sidTypeByte = buffer.readByte() >> 4; + final int sidTypeByte = buffer.readByte() >> SID_TYPE_BITS_OFFSET; final SidType sidType = SidType.forValue(sidTypeByte); srEroSubobjectBuilder.setSidType(sidType); diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroUtil.java b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroUtil.java index 7991c414e7..30a23613b0 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroUtil.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrEroUtil.java @@ -38,10 +38,8 @@ public final class SrEroUtil { } protected static boolean isPst(final PathSetupTypeTlv tlv) { - if (tlv != null && tlv.getPathSetupType() != null) { - if (tlv.getPathSetupType().isPst()) { - return true; - } + if (tlv != null && tlv.getPathSetupType() != null && tlv.getPathSetupType().isPst()) { + return true; } return false; } diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcInitiateMessageParser.java b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcInitiateMessageParser.java index 919fb5f2cf..7866b0d0e5 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcInitiateMessageParser.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcInitiateMessageParser.java @@ -54,11 +54,9 @@ public class SrPcInitiateMessageParser extends CInitiated00PCInitiateMessagePars objects.remove(0); final Object obj = objects.get(0); - if (obj != null) { - if (obj instanceof Ero) { - builder.setEro((Ero) obj); - objects.remove(0); - } + if (obj instanceof Ero) { + builder.setEro((Ero) obj); + objects.remove(0); } return builder.build(); } diff --git a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcRepMessageParser.java b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcRepMessageParser.java index 60c67bea05..864e838c2b 100644 --- a/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcRepMessageParser.java +++ b/pcep/segment-routing/src/main/java/org/opendaylight/protocol/pcep/segment/routing02/SrPcRepMessageParser.java @@ -40,13 +40,11 @@ public class SrPcRepMessageParser extends PCEPReplyMessageParser { final Rp rp = reply.getRp(); if (isSegmentRoutingPath(rp)) { serializeObject(rp, buffer); - if (reply.getResult() != null) { - if (reply.getResult() instanceof SuccessCase) { - final SuccessCase s = (SuccessCase) reply.getResult(); - for (final Paths p : s.getSuccess().getPaths()) { - final Ero ero = p.getEro(); - serializeObject(ero, buffer); - } + if (reply.getResult() instanceof SuccessCase) { + final SuccessCase s = (SuccessCase) reply.getResult(); + for (final Paths p : s.getSuccess().getPaths()) { + final Ero ero = p.getEro(); + serializeObject(ero, buffer); } } } else { @@ -64,28 +62,26 @@ public class SrPcRepMessageParser extends PCEPReplyMessageParser { if (isSegmentRoutingPath(rp)) { objects.remove(0); Result res = null; - if (!objects.isEmpty()) { - if (objects.get(0) instanceof Ero) { - final SuccessBuilder builder = new SuccessBuilder(); - final List paths = Lists.newArrayList(); - final PathsBuilder pBuilder = new PathsBuilder(); - while (!objects.isEmpty()) { - final Object object = objects.get(0); - if (object instanceof Ero) { - final Ero ero = (Ero) object; - final PCEPErrors error = SrEroUtil.validateSrEroSubobjects(ero); - if (error != null) { - errors.add(createErrorMsg(error)); - return null; - } else { - paths.add(pBuilder.setEro(ero).build()); - } + if (objects.get(0) instanceof Ero) { + final SuccessBuilder builder = new SuccessBuilder(); + final List paths = Lists.newArrayList(); + final PathsBuilder pBuilder = new PathsBuilder(); + while (!objects.isEmpty()) { + final Object object = objects.get(0); + if (object instanceof Ero) { + final Ero ero = (Ero) object; + final PCEPErrors error = SrEroUtil.validateSrEroSubobjects(ero); + if (error != null) { + errors.add(createErrorMsg(error)); + return null; + } else { + paths.add(pBuilder.setEro(ero).build()); } - objects.remove(0); } - builder.setPaths(paths); - res = new SuccessCaseBuilder().setSuccess(builder.build()).build(); + objects.remove(0); } + builder.setPaths(paths); + res = new SuccessCaseBuilder().setSuccess(builder.build()).build(); } return new RepliesBuilder().setRp(rp).setResult(res).build(); } -- 2.36.6