From e56fb34b4da28e4fef2788150c9d59d7b2375ba6 Mon Sep 17 00:00:00 2001 From: "guillaume.lambert" Date: Thu, 15 Oct 2020 16:19:56 +0200 Subject: [PATCH] Service Handler optimizations and technical debt - optimize some pieces of code - add some protection on RpcStatus return Code treatments especially when a Pending or an unknown status is returned JIRA: TRNSPRTPCE-342 Signed-off-by: guillaume.lambert Change-Id: Ifd28578767a7c2c155e0f90ed04c736e5f104b3f (cherry picked from commit 8c5929030801f571131b1aed25cfbc54f4f5269d) --- .../listeners/PceListenerImpl.java | 200 +++++++++--------- .../listeners/RendererListenerImpl.java | 132 ++++++------ .../checks/ServicehandlerTxRxCheck.java | 52 +++-- 3 files changed, 202 insertions(+), 182 deletions(-) diff --git a/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/PceListenerImpl.java b/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/PceListenerImpl.java index 65e199b4f..9362390e6 100644 --- a/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/PceListenerImpl.java +++ b/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/PceListenerImpl.java @@ -82,95 +82,107 @@ public class PceListenerImpl implements TransportpcePceListener { private void onPathComputationResult(ServicePathRpcResult notification) { LOG.info("PCE '{}' Notification received : {}",servicePathRpcResult.getNotificationType().getName(), notification); - if (servicePathRpcResult.getStatus() == RpcStatusEx.Successful) { - LOG.info("PCE calculation done OK !"); - if (servicePathRpcResult.getPathDescription() != null) { - PathDescription pathDescription = new PathDescriptionBuilder() - .setAToZDirection(servicePathRpcResult.getPathDescription().getAToZDirection()) - .setZToADirection(servicePathRpcResult.getPathDescription().getZToADirection()).build(); - LOG.info("PathDescription gets : {}", pathDescription); - if (!serviceFeasiblity) { - if (input == null) { - LOG.error("Input is null !"); - } else { - OperationResult operationResult = null; - if (tempService) { - operationResult = this.serviceDataStoreOperations - .createTempService(input.getTempServiceCreateInput()); - if (!operationResult.isSuccess()) { - LOG.error("Temp Service not created in datastore !"); - } - } else { - operationResult = this.serviceDataStoreOperations - .createService(input.getServiceCreateInput()); - if (!operationResult.isSuccess()) { - LOG.error("Service not created in datastore !"); - } - } - ResponseParameters responseParameters = new ResponseParametersBuilder() - .setPathDescription(new org.opendaylight.yang.gen.v1.http.org - .transportpce.b.c._interface.service.types.rev200128 - .response.parameters.sp.response.parameters - .PathDescriptionBuilder(pathDescription).build()) - .build(); - PathComputationRequestOutput pceResponse = new PathComputationRequestOutputBuilder() - .setResponseParameters(responseParameters).build(); - OperationResult operationServicePathSaveResult = this.serviceDataStoreOperations - .createServicePath(input, pceResponse); - if (!operationServicePathSaveResult.isSuccess()) { - LOG.error("Service Path not created in datastore !"); - } - ServiceImplementationRequestInput serviceImplementationRequest = ModelMappingUtils - .createServiceImplementationRequest(input, pathDescription); - LOG.info("Sending serviceImplementation request : {}", serviceImplementationRequest); - this.rendererServiceOperations.serviceImplementation(serviceImplementationRequest); - } - } else { - LOG.warn("service-feasibility-check RPC "); - } - } else { - LOG.error("'PathDescription' parameter is null "); - } - } else if (servicePathRpcResult.getStatus() == RpcStatusEx.Failed) { + if (servicePathRpcResult.getStatus() == RpcStatusEx.Failed) { LOG.error("PCE path computation failed !"); + return; + } else if (servicePathRpcResult.getStatus() == RpcStatusEx.Pending) { + LOG.warn("PCE path computation returned a Penging RpcStatusEx code!"); + return; + } else if (servicePathRpcResult.getStatus() != RpcStatusEx.Successful) { + LOG.error("PCE path computation returned an unknown RpcStatusEx code!"); + return; + } + + LOG.info("PCE calculation done OK !"); + if (servicePathRpcResult.getPathDescription() == null) { + LOG.error("'PathDescription' parameter is null "); + return; + } + PathDescription pathDescription = new PathDescriptionBuilder() + .setAToZDirection(servicePathRpcResult.getPathDescription().getAToZDirection()) + .setZToADirection(servicePathRpcResult.getPathDescription().getZToADirection()) + .build(); + LOG.info("PathDescription gets : {}", pathDescription); + if (serviceFeasiblity) { + LOG.warn("service-feasibility-check RPC "); + return; } + if (input == null) { + LOG.error("Input is null !"); + return; + } + OperationResult operationResult = null; + if (tempService) { + operationResult = this.serviceDataStoreOperations.createTempService(input.getTempServiceCreateInput()); + if (!operationResult.isSuccess()) { + LOG.error("Temp Service not created in datastore !"); + } + } else { + operationResult = this.serviceDataStoreOperations.createService(input.getServiceCreateInput()); + if (!operationResult.isSuccess()) { + LOG.error("Service not created in datastore !"); + } + } + ResponseParameters responseParameters = new ResponseParametersBuilder() + .setPathDescription(new org.opendaylight.yang.gen.v1.http.org + .transportpce.b.c._interface.service.types.rev200128 + .response.parameters.sp.response.parameters + .PathDescriptionBuilder(pathDescription).build()) + .build(); + PathComputationRequestOutput pceResponse = new PathComputationRequestOutputBuilder() + .setResponseParameters(responseParameters).build(); + OperationResult operationServicePathSaveResult = this.serviceDataStoreOperations + .createServicePath(input, pceResponse); + if (!operationServicePathSaveResult.isSuccess()) { + LOG.error("Service Path not created in datastore !"); + } + ServiceImplementationRequestInput serviceImplementationRequest = ModelMappingUtils + .createServiceImplementationRequest(input, pathDescription); + LOG.info("Sending serviceImplementation request : {}", serviceImplementationRequest); + this.rendererServiceOperations.serviceImplementation(serviceImplementationRequest); } /** * Process cancel resource result. */ private void onCancelResourceResult() { - if (servicePathRpcResult.getStatus() == RpcStatusEx.Successful) { - LOG.info("PCE cancel resource done OK !"); - OperationResult deleteServicePathOperationResult = - this.serviceDataStoreOperations.deleteServicePath(input.getServiceName()); - if (!deleteServicePathOperationResult.isSuccess()) { - LOG.warn("Service path was not removed from datastore!"); - } - OperationResult deleteServiceOperationResult = null; - if (tempService) { - deleteServiceOperationResult = - this.serviceDataStoreOperations.deleteTempService(input.getServiceName()); - if (!deleteServiceOperationResult.isSuccess()) { - LOG.warn("Service was not removed from datastore!"); - } - } else { - deleteServiceOperationResult = - this.serviceDataStoreOperations.deleteService(input.getServiceName()); - if (!deleteServiceOperationResult.isSuccess()) { - LOG.warn("Service was not removed from datastore!"); - } + if (servicePathRpcResult.getStatus() == RpcStatusEx.Failed) { + LOG.info("PCE cancel resource failed !"); + return; + } else if (servicePathRpcResult.getStatus() == RpcStatusEx.Pending) { + LOG.warn("PCE cancel returned a Penging RpcStatusEx code!"); + return; + } else if (servicePathRpcResult.getStatus() != RpcStatusEx.Successful) { + LOG.error("PCE cancel returned an unknown RpcStatusEx code!"); + return; + } + LOG.info("PCE cancel resource done OK !"); + OperationResult deleteServicePathOperationResult = + this.serviceDataStoreOperations.deleteServicePath(input.getServiceName()); + if (!deleteServicePathOperationResult.isSuccess()) { + LOG.warn("Service path was not removed from datastore!"); + } + OperationResult deleteServiceOperationResult = null; + if (tempService) { + deleteServiceOperationResult = + this.serviceDataStoreOperations.deleteTempService(input.getServiceName()); + if (!deleteServiceOperationResult.isSuccess()) { + LOG.warn("Temp Service was not removed from datastore!"); } - /** - * if it was an RPC serviceReconfigure, re-launch PCR. - */ - if (this.serviceReconfigure) { - LOG.info("cancel resource reserve done, relaunching PCE path computation ..."); - this.pceServiceWrapper.performPCE(input.getServiceCreateInput(), true); - this.serviceReconfigure = false; + } else { + deleteServiceOperationResult = + this.serviceDataStoreOperations.deleteService(input.getServiceName()); + if (!deleteServiceOperationResult.isSuccess()) { + LOG.warn("Service was not removed from datastore!"); } - } else if (servicePathRpcResult.getStatus() == RpcStatusEx.Failed) { - LOG.info("PCE cancel resource failed !"); + } + /** + * if it was an RPC serviceReconfigure, re-launch PCR. + */ + if (this.serviceReconfigure) { + LOG.info("cancel resource reserve done, relaunching PCE path computation ..."); + this.pceServiceWrapper.performPCE(input.getServiceCreateInput(), true); + this.serviceReconfigure = false; } } @@ -178,24 +190,22 @@ public class PceListenerImpl implements TransportpcePceListener { value = "ES_COMPARING_STRINGS_WITH_EQ", justification = "false positives, not strings but real object references comparisons") private Boolean compareServicePathRpcResult(ServicePathRpcResult notification) { - Boolean result = true; if (servicePathRpcResult == null) { - result = false; - } else { - if (servicePathRpcResult.getNotificationType() != notification.getNotificationType()) { - result = false; - } - if (servicePathRpcResult.getServiceName() != notification.getServiceName()) { - result = false; - } - if (servicePathRpcResult.getStatus() != notification.getStatus()) { - result = false; - } - if (servicePathRpcResult.getStatusMessage() != notification.getStatusMessage()) { - result = false; - } + return false; + } + if (servicePathRpcResult.getNotificationType() != notification.getNotificationType()) { + return false; + } + if (servicePathRpcResult.getServiceName() != notification.getServiceName()) { + return false; + } + if (servicePathRpcResult.getStatus() != notification.getStatus()) { + return false; + } + if (servicePathRpcResult.getStatusMessage() != notification.getStatusMessage()) { + return false; } - return result; + return true; } public void setInput(ServiceInput serviceInput) { diff --git a/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/RendererListenerImpl.java b/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/RendererListenerImpl.java index 0669f842c..30f8d7a17 100644 --- a/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/RendererListenerImpl.java +++ b/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/listeners/RendererListenerImpl.java @@ -47,26 +47,26 @@ public class RendererListenerImpl implements TransportpceRendererListener { @Override public void onServiceRpcResultSp(ServiceRpcResultSp notification) { - if (!compareServiceRpcResultSp(notification)) { - serviceRpcResultSp = notification; - String serviceName = serviceRpcResultSp.getServiceName(); - int notifType = serviceRpcResultSp.getNotificationType().getIntValue(); - LOG.info("Renderer '{}' Notification received : {}", serviceRpcResultSp.getNotificationType().getName(), - notification); - switch (notifType) { - /* service-implementation-request. */ - case 3 : - onServiceImplementationResult(serviceName); - break; - /* service-delete. */ - case 4 : - onServiceDeleteResult(serviceName); - break; - default: - break; - } - } else { + if (compareServiceRpcResultSp(notification)) { LOG.warn("ServiceRpcResultSp already wired !"); + return; + } + serviceRpcResultSp = notification; + String serviceName = serviceRpcResultSp.getServiceName(); + int notifType = serviceRpcResultSp.getNotificationType().getIntValue(); + LOG.info("Renderer '{}' Notification received : {}", serviceRpcResultSp.getNotificationType().getName(), + notification); + switch (notifType) { + /* service-implementation-request. */ + case 3 : + onServiceImplementationResult(serviceName); + break; + /* service-delete. */ + case 4 : + onServiceDeleteResult(serviceName); + break; + default: + break; } } @@ -75,18 +75,24 @@ public class RendererListenerImpl implements TransportpceRendererListener { * @param serviceName String */ private void onServiceDeleteResult(String serviceName) { - if (serviceRpcResultSp.getStatus() == RpcStatusEx.Successful) { - LOG.info("Service '{}' deleted !", serviceName); - if (this.input != null) { - LOG.info("sending PCE cancel resource reserve for '{}'", this.input.getServiceName()); - this.pceServiceWrapper.cancelPCEResource(this.input.getServiceName(), - ServiceNotificationTypes.ServiceDeleteResult); - } else { - LOG.error("ServiceInput parameter is null !"); - } - } else if (serviceRpcResultSp.getStatus() == RpcStatusEx.Failed) { + if (serviceRpcResultSp.getStatus() == RpcStatusEx.Failed) { LOG.error("Renderer service delete failed !"); + return; + } else if (serviceRpcResultSp.getStatus() == RpcStatusEx.Pending) { + LOG.warn("Renderer service delete returned a Penging RpcStatusEx code!"); + return; + } else if (serviceRpcResultSp.getStatus() != RpcStatusEx.Successful) { + LOG.error("Renderer service delete returned an unknown RpcStatusEx code!"); + return; } + LOG.info("Service '{}' deleted !", serviceName); + if (this.input == null) { + LOG.error("ServiceInput parameter is null !"); + return; + } + LOG.info("sending PCE cancel resource reserve for '{}'", this.input.getServiceName()); + this.pceServiceWrapper.cancelPCEResource(this.input.getServiceName(), + ServiceNotificationTypes.ServiceDeleteResult); } /** @@ -99,7 +105,12 @@ public class RendererListenerImpl implements TransportpceRendererListener { onSuccededServiceImplementation(); } else if (serviceRpcResultSp.getStatus() == RpcStatusEx.Failed) { onFailedServiceImplementation(serviceName); + } else if (serviceRpcResultSp.getStatus() == RpcStatusEx.Pending) { + LOG.warn("Service Implementation still pending according to RpcStatusEx"); + } else { + LOG.warn("Service Implementation has an unknown RpcStatusEx code"); } + } /** @@ -107,21 +118,24 @@ public class RendererListenerImpl implements TransportpceRendererListener { */ private void onSuccededServiceImplementation() { LOG.info("Service implemented !"); - if (serviceDataStoreOperations != null) { - OperationResult operationResult = null; - if (tempService) { - operationResult = this.serviceDataStoreOperations.modifyTempService( - serviceRpcResultSp.getServiceName(), State.InService, AdminStates.InService); - if (!operationResult.isSuccess()) { - LOG.warn("Temp Service status not updated in datastore !"); - } - } else { - operationResult = this.serviceDataStoreOperations - .modifyService(serviceRpcResultSp.getServiceName(), - State.InService, AdminStates.InService); - if (!operationResult.isSuccess()) { - LOG.warn("Service status not updated in datastore !"); - } + if (serviceDataStoreOperations == null) { + LOG.debug("serviceDataStoreOperations is null"); + return; + } + OperationResult operationResult = null; + if (tempService) { + operationResult = this.serviceDataStoreOperations.modifyTempService( + serviceRpcResultSp.getServiceName(), State.InService, AdminStates.InService); + if (!operationResult.isSuccess()) { + LOG.warn("Temp Service status not updated in datastore !"); + } + } else { + operationResult = this.serviceDataStoreOperations.modifyService( + serviceRpcResultSp.getServiceName(), + State.InService, + AdminStates.InService); + if (!operationResult.isSuccess()) { + LOG.warn("Service status not updated in datastore !"); } } } @@ -156,24 +170,22 @@ public class RendererListenerImpl implements TransportpceRendererListener { value = "ES_COMPARING_STRINGS_WITH_EQ", justification = "false positives, not strings but real object references comparisons") private Boolean compareServiceRpcResultSp(ServiceRpcResultSp notification) { - Boolean result = true; if (serviceRpcResultSp == null) { - result = false; - } else { - if (serviceRpcResultSp.getNotificationType() != notification.getNotificationType()) { - result = false; - } - if (serviceRpcResultSp.getServiceName() != notification.getServiceName()) { - result = false; - } - if (serviceRpcResultSp.getStatus() != notification.getStatus()) { - result = false; - } - if (serviceRpcResultSp.getStatusMessage() != notification.getStatusMessage()) { - result = false; - } + return false; + } + if (serviceRpcResultSp.getNotificationType() != notification.getNotificationType()) { + return false; + } + if (serviceRpcResultSp.getServiceName() != notification.getServiceName()) { + return false; + } + if (serviceRpcResultSp.getStatus() != notification.getStatus()) { + return false; + } + if (serviceRpcResultSp.getStatusMessage() != notification.getStatusMessage()) { + return false; } - return result; + return true; } public void setServiceInput(ServiceInput serviceInput) { diff --git a/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/validation/checks/ServicehandlerTxRxCheck.java b/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/validation/checks/ServicehandlerTxRxCheck.java index cdcedfeda..fbca473af 100644 --- a/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/validation/checks/ServicehandlerTxRxCheck.java +++ b/servicehandler/src/main/java/org/opendaylight/transportpce/servicehandler/validation/checks/ServicehandlerTxRxCheck.java @@ -86,21 +86,20 @@ public final class ServicehandlerTxRxCheck { //sonar issue Reduce the number of conditional operators (4) used in the expression (maximum allowed 3) //won't be fixed because of functional checks needed public static boolean checkPort(Port port) { - boolean result = false; - if (port != null) { - String portDeviceName = port.getPortDeviceName(); - String portType = port.getPortType(); - String portName = port.getPortName(); - String portRack = port.getPortRack(); - String portShelf = port.getPortShelf(); - - return checkString(portDeviceName) - && checkString(portType) - && checkString(portName) - && checkString(portRack) - && checkString(portShelf); - } - return result; + if (port == null) { + return false; + } + String portDeviceName = port.getPortDeviceName(); + String portType = port.getPortType(); + String portName = port.getPortName(); + String portRack = port.getPortRack(); + String portShelf = port.getPortShelf(); + + return checkString(portDeviceName) + && checkString(portType) + && checkString(portName) + && checkString(portRack) + && checkString(portShelf); } /** @@ -111,18 +110,17 @@ public final class ServicehandlerTxRxCheck { * @return true if String ok false if not */ public static boolean checkLgx(Lgx lgx) { - boolean result = false; - if (lgx != null) { - String lgxDeviceName = lgx.getLgxDeviceName(); - String lgxPortName = lgx.getLgxPortName(); - String lgxPortRack = lgx.getLgxPortRack(); - String lgxPortShelf = lgx.getLgxPortShelf(); - if (checkString(lgxDeviceName) && checkString(lgxPortName) && checkString(lgxPortRack) - && checkString(lgxPortShelf)) { - result = true; - } - } - return result; + if (lgx == null) { + return false; + } + String lgxDeviceName = lgx.getLgxDeviceName(); + String lgxPortName = lgx.getLgxPortName(); + String lgxPortRack = lgx.getLgxPortRack(); + String lgxPortShelf = lgx.getLgxPortShelf(); + return checkString(lgxDeviceName) + && checkString(lgxPortName) + && checkString(lgxPortRack) + && checkString(lgxPortShelf); } /** -- 2.36.6