Fix sonar issues in PAP 71/85371/3
authorJim Hahn <jrh3@att.com>
Mon, 15 Apr 2019 18:37:46 +0000 (14:37 -0400)
committerJim Hahn <jrh3@att.com>
Mon, 15 Apr 2019 21:29:44 +0000 (17:29 -0400)
Moved methods into nested class of PdpModifyRequestMap.  Note: they
will have to be moved back out when the broadcast capability is
added.
Removed extra "continue" from Publisher.
Rather than remove unused parameters and cause significant clutter
to the code, chose to use the parameters in a logger call for RequestImpl.
Removed unnecessary cast from UpdateReq.
Use version constant in PapRestControllerV1.
Log exception in PdpGroupQueryControllerV1.
Log exception in PdpGroupStateChangeControllerV1.
Shortened functional in PdpGroupDeleteProvider.
Removed unnecessary type parameters from PdpGroupDeployProvider.
Removed unneeded parameter and exception from ProviderBase.
Extracted common string from Main.
Shorted functionals in PapActivator.
Changed uses of getCanonicalName() to getName(); the former only has
limited use, while the latter is typically what is wanted and is
actually required whenever forClass(name) is used.
Fix junit tests broken by rename of ToscaPolicy version from
"typeVersion" to "type_version".

Change-Id: Ia00e1b3541c9e25b196a951e45681f67aa7f1bfe
Issue-ID: POLICY-1542
Signed-off-by: Jim Hahn <jrh3@att.com>
22 files changed:
main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
main/src/main/java/org/onap/policy/pap/main/comm/Publisher.java
main/src/main/java/org/onap/policy/pap/main/comm/msgdata/RequestImpl.java
main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
main/src/main/java/org/onap/policy/pap/main/rest/PapRestControllerV1.java
main/src/main/java/org/onap/policy/pap/main/rest/PapRestServer.java
main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupQueryControllerV1.java
main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupStateChangeControllerV1.java
main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeleteProvider.java
main/src/main/java/org/onap/policy/pap/main/rest/depundep/PdpGroupDeployProvider.java
main/src/main/java/org/onap/policy/pap/main/rest/depundep/ProviderBase.java
main/src/main/java/org/onap/policy/pap/main/startstop/Main.java
main/src/main/java/org/onap/policy/pap/main/startstop/PapActivator.java
main/src/main/java/org/onap/policy/pap/main/startstop/PapCommandLineArguments.java
main/src/test/java/org/onap/policy/pap/main/parameters/CommonTestData.java
main/src/test/resources/simpleDeploy/createGroupNewPolicy.json
main/src/test/resources/simpleDeploy/daoPolicyList.json
main/src/test/resources/simpleDeploy/policy.json
main/src/test/resources/simpleDeploy/upgradeGroupDao.json
main/src/test/resources/simpleDeploy/upgradeGroupPolicy2.json
main/src/test/resources/simpleDeploy/upgradeGroupPolicy3.json
main/src/test/resources/simpleDeploy/upgradeGroupPolicy4.json

index c65ebb3..8dcb979 100644 (file)
@@ -205,62 +205,6 @@ public class PdpModifyRequestMap {
         }
     }
 
-    /**
-     * Starts the next request associated with a PDP.
-     *
-     * @param requests current set of requests
-     * @param request the request that just completed
-     */
-    private void startNextRequest(PdpRequests requests, Request request) {
-        if (!requests.startNextRequest(request)) {
-            pdp2requests.remove(requests.getPdpName(), requests);
-        }
-    }
-
-    /**
-     * Disables a PDP by removing it from its subgroup and then sending it a PASSIVE
-     * request.
-     *
-     * @param requests the requests associated with the PDP to be disabled
-     */
-    private void disablePdp(PdpRequests requests) {
-
-        // remove the requests from the map
-        if (!pdp2requests.remove(requests.getPdpName(), requests)) {
-            // don't have the info we need to disable it
-            logger.warn("no requests with which to disable {}", requests.getPdpName());
-            return;
-        }
-
-        logger.warn("disabling {}", requests.getPdpName());
-
-        requests.stopPublishing();
-
-        // remove the PDP from all groups
-        boolean removed = false;
-        try {
-            removed = removeFromGroups(requests.getPdpName());
-        } catch (PfModelException e) {
-            logger.info("unable to remove PDP {} from subgroup", requests.getPdpName(), e);
-        }
-
-        // send the state change
-        PdpStateChange change = new PdpStateChange();
-        change.setName(requests.getPdpName());
-        change.setState(PdpState.PASSIVE);
-
-        if (removed) {
-            // send an update, too
-            PdpUpdate update = new PdpUpdate();
-            update.setName(requests.getPdpName());
-
-            addRequest(update, change);
-
-        } else {
-            addRequest(change);
-        }
-    }
-
     /**
      * Removes a PDP from all active groups.
      *
@@ -384,5 +328,61 @@ public class PdpModifyRequestMap {
         public void retryCountExhausted() {
             disablePdp(requests);
         }
+
+        /**
+         * Starts the next request associated with a PDP.
+         *
+         * @param requests current set of requests
+         * @param request the request that just completed
+         */
+        private void startNextRequest(PdpRequests requests, Request request) {
+            if (!requests.startNextRequest(request)) {
+                pdp2requests.remove(requests.getPdpName(), requests);
+            }
+        }
+
+        /**
+         * Disables a PDP by removing it from its subgroup and then sending it a PASSIVE
+         * request.
+         *
+         * @param requests the requests associated with the PDP to be disabled
+         */
+        private void disablePdp(PdpRequests requests) {
+
+            // remove the requests from the map
+            if (!pdp2requests.remove(requests.getPdpName(), requests)) {
+                // don't have the info we need to disable it
+                logger.warn("no requests with which to disable {}", requests.getPdpName());
+                return;
+            }
+
+            logger.warn("disabling {}", requests.getPdpName());
+
+            requests.stopPublishing();
+
+            // remove the PDP from all groups
+            boolean removed = false;
+            try {
+                removed = removeFromGroups(requests.getPdpName());
+            } catch (PfModelException e) {
+                logger.info("unable to remove PDP {} from subgroup", requests.getPdpName(), e);
+            }
+
+            // send the state change
+            PdpStateChange change = new PdpStateChange();
+            change.setName(requests.getPdpName());
+            change.setState(PdpState.PASSIVE);
+
+            if (removed) {
+                // send an update, too
+                PdpUpdate update = new PdpUpdate();
+                update.setName(requests.getPdpName());
+
+                addRequest(update, change);
+
+            } else {
+                addRequest(change);
+            }
+        }
     }
 }
index 6032d17..ec54dd5 100644 (file)
@@ -107,11 +107,9 @@ public class Publisher implements Runnable {
             }
 
             PdpMessage data = token.replaceItem(null);
-            if (data == null) {
-                continue;
+            if (data != null) {
+                client.send(data);
             }
-
-            client.send(data);
         }
     }
 
index 45ca2db..b9a0a6d 100644 (file)
@@ -112,7 +112,7 @@ public abstract class RequestImpl implements Request {
                             () -> timer = params.getTimers().register(this.message.getRequestId(), this::handleTimeout),
                             () -> timer.cancel())
                         .addAction("enqueue",
-                            () -> enqueue(),
+                            this::enqueue,
                             () -> {
                                 // do not remove from the queue - token may be re-used
                             });
@@ -260,7 +260,7 @@ public abstract class RequestImpl implements Request {
 
             String reason = checkResponse(response);
             if (reason != null) {
-                logger.info("{} PDP data mismatch: {}", getName(), reason);
+                logger.info("{} PDP data mismatch via {}: {}", getName(), infra, reason);
                 listener.failure(pdpName, reason);
                 return;
             }
@@ -286,7 +286,7 @@ public abstract class RequestImpl implements Request {
             stopPublishing();
 
             if (!bumpRetryCount()) {
-                logger.info("{} timeout - retry count {} exhausted", getName(), retryCount);
+                logger.info("{} timeout {} - retry count {} exhausted", getName(), timerName, retryCount);
                 listener.retryCountExhausted();
                 return;
             }
index 0f6d73f..9067131 100644 (file)
@@ -62,7 +62,7 @@ public class UpdateReq extends RequestImpl {
             return reason;
         }
 
-        PdpUpdate message = (PdpUpdate) getMessage();
+        PdpUpdate message = getMessage();
         if (!StringUtils.equals(message.getPdpGroup(), response.getPdpGroup())) {
             return "group does not match";
         }
index fb5d2d8..ccdebc3 100644 (file)
@@ -100,7 +100,7 @@ public class PapRestControllerV1 {
      */
     public ResponseBuilder addVersionControlHeaders(ResponseBuilder respBuilder) {
         return respBuilder.header(VERSION_MINOR_NAME, "0").header(VERSION_PATCH_NAME, "0").header(VERSION_LATEST_NAME,
-                        "1.0.0");
+                        API_VERSION);
     }
 
     /**
index 01c3361..87db6ad 100644 (file)
@@ -66,7 +66,7 @@ public class PapRestServer implements Startable {
             servers = HttpServletServer.factory.build(getServerProperties());
             for (final HttpServletServer server : servers) {
                 if (server.isAaf()) {
-                    server.addFilterClass(null, PapAafFilter.class.getCanonicalName());
+                    server.addFilterClass(null, PapAafFilter.class.getName());
                 }
                 server.start();
             }
@@ -93,12 +93,12 @@ public class PapRestServer implements Startable {
         props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_PORT_SUFFIX,
                         Integer.toString(restServerParameters.getPort()));
         props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_REST_CLASSES_SUFFIX,
-                        String.join(",", HealthCheckRestControllerV1.class.getCanonicalName(),
-                                        StatisticsRestControllerV1.class.getCanonicalName(),
-                                        PdpGroupDeployControllerV1.class.getCanonicalName(),
-                                        PdpGroupDeleteControllerV1.class.getCanonicalName(),
-                                        PdpGroupStateChangeControllerV1.class.getCanonicalName(),
-                                        PdpGroupQueryControllerV1.class.getCanonicalName()));
+                        String.join(",", HealthCheckRestControllerV1.class.getName(),
+                                        StatisticsRestControllerV1.class.getName(),
+                                        PdpGroupDeployControllerV1.class.getName(),
+                                        PdpGroupDeleteControllerV1.class.getName(),
+                                        PdpGroupStateChangeControllerV1.class.getName(),
+                                        PdpGroupQueryControllerV1.class.getName()));
         props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_MANAGED_SUFFIX, "false");
         props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_SWAGGER_SUFFIX, "true");
         props.setProperty(svcpfx + PolicyEndPointProperties.PROPERTY_HTTP_AUTH_USERNAME_SUFFIX,
index 74d6e61..51270f2 100644 (file)
@@ -1,6 +1,7 @@
 /*-
  * ============LICENSE_START=======================================================
  *  Copyright (C) 2019 Nordix Foundation.
+ *  Modifications Copyright (C) 2019 AT&T Intellectual Property.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,6 +41,8 @@ import javax.ws.rs.core.Response.Status;
 import org.apache.commons.lang3.tuple.Pair;
 import org.onap.policy.models.base.PfModelException;
 import org.onap.policy.models.pdp.concepts.PdpGroups;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Class to provide REST end points for PAP component to query details of all PDP groups.
@@ -48,6 +51,8 @@ import org.onap.policy.models.pdp.concepts.PdpGroups;
  */
 public class PdpGroupQueryControllerV1 extends PapRestControllerV1 {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(PdpGroupQueryControllerV1.class);
+
     private final PdpGroupQueryProvider provider = new PdpGroupQueryProvider();
 
     /**
@@ -89,6 +94,7 @@ public class PdpGroupQueryControllerV1 extends PapRestControllerV1 {
             return addLoggingHeaders(addVersionControlHeaders(Response.status(pair.getLeft())), requestId)
                     .entity(pair.getRight()).build();
         } catch (final PfModelException exp) {
+            LOGGER.info("group query failed", exp);
             return addLoggingHeaders(
                     addVersionControlHeaders(Response.status(exp.getErrorResponse().getResponseCode())), requestId)
                             .entity(exp.getErrorResponse()).build();
index c9369a2..e739b2f 100644 (file)
@@ -1,6 +1,7 @@
 /*-
  * ============LICENSE_START=======================================================
  *  Copyright (C) 2019 Nordix Foundation.
+ *  Modifications Copyright (C) 2019 AT&T Intellectual Property.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,6 +44,8 @@ import org.apache.commons.lang3.tuple.Pair;
 import org.onap.policy.models.base.PfModelException;
 import org.onap.policy.models.pap.concepts.PdpGroupStateChangeResponse;
 import org.onap.policy.models.pdp.enums.PdpState;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Class to provide REST end points for PAP component to change state of a PDP group.
@@ -51,6 +54,8 @@ import org.onap.policy.models.pdp.enums.PdpState;
  */
 public class PdpGroupStateChangeControllerV1 extends PapRestControllerV1 {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(PdpGroupQueryControllerV1.class);
+
     private final PdpGroupStateChangeProvider provider = new PdpGroupStateChangeProvider();
 
     /**
@@ -96,6 +101,7 @@ public class PdpGroupStateChangeControllerV1 extends PapRestControllerV1 {
             return addLoggingHeaders(addVersionControlHeaders(Response.status(pair.getLeft())), requestId)
                     .entity(pair.getRight()).build();
         } catch (final PfModelException exp) {
+            LOGGER.info("group state-change failed", exp);
             return addLoggingHeaders(
                     addVersionControlHeaders(Response.status(exp.getErrorResponse().getResponseCode())), requestId)
                             .entity(exp.getErrorResponse()).build();
index 7d47b3e..c64a0ef 100644 (file)
@@ -138,10 +138,7 @@ public class PdpGroupDeleteProvider extends ProviderBase<PdpGroupDeleteResponse>
     protected BiFunction<PdpGroup, PdpSubGroup, Boolean> makeUpdater(ToscaPolicy policy) {
         ToscaPolicyIdentifier desiredIdent = policy.getIdentifier();
 
-        return (group, subgroup) -> {
-
-            // remove the policy from the subgroup
-            return subgroup.getPolicies().remove(desiredIdent);
-        };
+        // remove the policy from the subgroup
+        return (group, subgroup) -> subgroup.getPolicies().remove(desiredIdent);
     }
 }
index 3d44a0c..5fd1213 100644 (file)
@@ -328,8 +328,8 @@ public class PdpGroupDeployProvider extends ProviderBase<PdpGroupDeployResponse>
      */
     private <T> boolean updateList(List<T> dblist, List<T> newList, Consumer<List<T>> setter) {
 
-        Set<T> dbTypes = new HashSet<T>(dblist);
-        Set<T> newTypes = new HashSet<T>(newList);
+        Set<T> dbTypes = new HashSet<>(dblist);
+        Set<T> newTypes = new HashSet<>(newList);
 
         if (dbTypes.equals(newTypes)) {
             return false;
index b7575df..999941f 100644 (file)
@@ -179,7 +179,7 @@ public abstract class ProviderBase<R extends SimpleResponse> {
         BiFunction<PdpGroup, PdpSubGroup, Boolean> updater = makeUpdater(policy);
 
         for (PdpGroup group : groups) {
-            upgradeGroup(data, policy, group, updater);
+            upgradeGroup(data, group, updater);
         }
     }
 
@@ -212,13 +212,11 @@ public abstract class ProviderBase<R extends SimpleResponse> {
      * Updates a group, assigning a new version number, if it actually changes.
      *
      * @param data session data
-     * @param policy policy to be added to or removed from the group
      * @param group the original group, to be updated
      * @param updater function to update a group
-     * @throws PfModelException if a DAO error occurred
      */
-    private void upgradeGroup(SessionData data, ToscaPolicy policy, PdpGroup group,
-                    BiFunction<PdpGroup, PdpSubGroup, Boolean> updater) throws PfModelException {
+    private void upgradeGroup(SessionData data, PdpGroup group,
+                    BiFunction<PdpGroup, PdpSubGroup, Boolean> updater) {
 
         boolean updated = false;
 
index f159816..e8b48e6 100644 (file)
@@ -40,6 +40,8 @@ import org.slf4j.LoggerFactory;
  */
 public class Main {
 
+    private static final String START_FAILED = "start of policy pap service failed";
+
     private static final Logger LOGGER = LoggerFactory.getLogger(Main.class);
 
     private PapActivator activator;
@@ -66,7 +68,7 @@ public class Main {
             // Validate that the arguments are sane
             arguments.validate();
         } catch (final PolicyPapException e) {
-            LOGGER.error("start of policy pap service failed", e);
+            LOGGER.error(START_FAILED, e);
             return;
         }
 
@@ -74,7 +76,7 @@ public class Main {
         try {
             parameterGroup = new PapParameterHandler().getParameters(arguments);
         } catch (final Exception e) {
-            LOGGER.error("start of policy pap service failed", e);
+            LOGGER.error(START_FAILED, e);
             return;
         }
 
@@ -86,7 +88,7 @@ public class Main {
                 props.load(stream);
             }
         } catch (final Exception e) {
-            LOGGER.error("start of policy pap service failed", e);
+            LOGGER.error(START_FAILED, e);
             return;
         }
 
@@ -94,7 +96,7 @@ public class Main {
         try {
             new PapDatabaseInitializer().initializePapDatabase(parameterGroup.getDatabaseProviderParameters());
         } catch (final PolicyPapException exp) {
-            LOGGER.error("start of policy pap service failed, used parameters are {}", Arrays.toString(args), exp);
+            LOGGER.error(START_FAILED + ", used parameters are {}", Arrays.toString(args), exp);
             return;
         }
 
index f34d13a..8f3583e 100644 (file)
@@ -135,12 +135,12 @@ public class PapActivator extends ServiceManagerContainer {
             () -> msgDispatcher.unregister(PdpMessageType.PDP_STATUS.name()));
 
         addAction("Message Dispatcher",
-            () -> registerMsgDispatcher(),
-            () -> unregisterMsgDispatcher());
+            this::registerMsgDispatcher,
+            this::unregisterMsgDispatcher);
 
         addAction("topics",
-            () -> TopicEndpoint.manager.start(),
-            () -> TopicEndpoint.manager.shutdown());
+            TopicEndpoint.manager::start,
+            TopicEndpoint.manager::shutdown);
 
         addAction("PAP statistics",
             () -> Registry.register(PapConstants.REG_STATISTICS_MANAGER, new PapStatisticsManager()),
@@ -183,12 +183,8 @@ public class PapActivator extends ServiceManagerContainer {
             () -> Registry.unregister(PapConstants.REG_PDP_MODIFY_MAP));
 
         addAction("Create REST server",
-            () -> {
-                restServer = new PapRestServer(papParameterGroup.getRestServerParameters());
-            },
-            () -> {
-                restServer = null;
-            });
+            () -> restServer = new PapRestServer(papParameterGroup.getRestServerParameters()),
+            () -> restServer = null);
 
         addAction("REST server",
             () -> restServer.start(),
index b0bbfd8..fd728d2 100644 (file)
@@ -138,7 +138,7 @@ public class PapCommandLineArguments {
         }
 
         if (commandLine.hasOption('h')) {
-            return help(Main.class.getCanonicalName());
+            return help(Main.class.getName());
         }
 
         if (commandLine.hasOption('v')) {
index e7ffce0..10e500f 100644 (file)
@@ -170,7 +170,7 @@ public class CommonTestData {
     public Map<String, Object> getPolicyModelsProviderParametersMap() {
         final Map<String, Object> map = new TreeMap<>();
         map.put("name", PolicyModelsProviderParameters.class.getSimpleName());
-        map.put("implementation", DatabasePolicyModelsProviderImpl.class.getCanonicalName());
+        map.put("implementation", DatabasePolicyModelsProviderImpl.class.getName());
         map.put("databaseDriver", "org.h2.Driver");
         map.put("databaseUrl", "jdbc:h2:mem:testdb");
         map.put("databaseUser", "policy");
index 285db31..9aa54cb 100644 (file)
@@ -4,7 +4,7 @@
             "name": "policyB",
             "version": "1.2.3",
             "type": "typeA",
-            "typeVersion": "100.2.3"
+            "type_version": "100.2.3"
         }
     ]
 }
index 93ee914..873cb71 100644 (file)
@@ -4,7 +4,7 @@
             "name": "policyA",
             "version": "1.2.3",
             "type": "typeA",
-            "typeVersion": "100.2.3"
+            "type_version": "100.2.3"
         }
     ]
 }
index 7786aab..084aafc 100644 (file)
@@ -2,5 +2,5 @@
     "name": "policyA",
     "version": "1.2.3",
     "type": "typeA",
-    "typeVersion": "100.2.3"
+    "type_version": "100.2.3"
 }
index 278b69a..bee81be 100644 (file)
@@ -47,7 +47,7 @@
                             "name": "policyA",
                             "version": "1.2.3",
                             "type": "typeA",
-                            "typeVersion": "100.2.3"
+                            "type_version": "100.2.3"
                         }
                     ]
                 },
index 3a13d0c..54201b9 100644 (file)
@@ -4,7 +4,7 @@
             "name": "policyB",
             "version": "1.2.3",
             "type": "typeB",
-            "typeVersion": "100.2.3"
+            "type_version": "100.2.3"
         }
     ]
 }
index bf5602e..57a8f84 100644 (file)
@@ -4,7 +4,7 @@
             "name": "policyC",
             "version": "1.2.3",
             "type": "typeC",
-            "typeVersion": "100.2.3"
+            "type_version": "100.2.3"
         }
     ]
 }
index 3c0fb72..8b7c96c 100644 (file)
@@ -4,7 +4,7 @@
             "name": "policyD",
             "version": "1.2.3",
             "type": "typeA",
-            "typeVersion": "100.2.3"
+            "type_version": "100.2.3"
         }
     ]
 }