diff options
author | Jim Hahn <jrh3@att.com> | 2020-08-25 17:05:37 -0400 |
---|---|---|
committer | Jim Hahn <jrh3@att.com> | 2020-08-25 18:47:23 -0400 |
commit | 72cf248dced4e272e69a8c48f5ee24c9d345f41d (patch) | |
tree | b09ed0f9b7ee7aac6493119d950d78d50ae08da1 | |
parent | 5c71b5a62f4208030b3c3cd18b3f42398a0a86e5 (diff) |
Address more sonars in drools-pdp
Addressed the following sonars:
- either log or rethrow
- call "remove()" for thread-local-storage
- use assertEquals
- only one method call in exception test
- swap arguments in assertEquals
- add assertion to assertThatThrownBy()
- explain @Ignore
Also addressed eclipse warnings:
- unused fields and methods
Issue-ID: POLICY-2616
Change-Id: I6590c0d2b103885bc933014d48bf5fd92401cd80
Signed-off-by: Jim Hahn <jrh3@att.com>
17 files changed, 81 insertions, 71 deletions
diff --git a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateUnsupportedTest.java b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateUnsupportedTest.java index 1b7c62a3..400aa169 100644 --- a/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateUnsupportedTest.java +++ b/feature-lifecycle/src/test/java/org/onap/policy/drools/lifecycle/LifecycleStateUnsupportedTest.java @@ -90,19 +90,22 @@ public abstract class LifecycleStateUnsupportedTest { @Test public void update() { - assertThatThrownBy(() -> state.update(new PdpUpdate())) + PdpUpdate update = new PdpUpdate(); + assertThatThrownBy(() -> state.update(update)) .isInstanceOf(UnsupportedOperationException.class); } @Test public void stateChange() { - assertThatThrownBy(() -> state.stateChange(new PdpStateChange())) + PdpStateChange change = new PdpStateChange(); + assertThatThrownBy(() -> state.stateChange(change)) .isInstanceOf(UnsupportedOperationException.class); } @Test public void changeState() { - assertThatThrownBy(() -> state.transitionToState(new LifecycleStateActive(new LifecycleFsm()))) + LifecycleStateActive active = new LifecycleStateActive(new LifecycleFsm()); + assertThatThrownBy(() -> state.transitionToState(active)) .isInstanceOf(UnsupportedOperationException.class); } } diff --git a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/DmaapManagerTest.java b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/DmaapManagerTest.java index d5b397a6..7f73a702 100644 --- a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/DmaapManagerTest.java +++ b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/DmaapManagerTest.java @@ -290,7 +290,7 @@ public class DmaapManagerTest { @Test public void testPublish() throws PoolingFeatureException { // cannot publish before starting - assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,pre"); + assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,pre").isInstanceOf(PoolingFeatureException.class); mgr.startPublisher(); @@ -306,7 +306,7 @@ public class DmaapManagerTest { // stop and verify we can no longer publish mgr.stopPublisher(0); - assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,stopped"); + assertThatThrownBy(() -> mgr.publish(MSG)).as("publish,stopped").isInstanceOf(PoolingFeatureException.class); } @Test(expected = PoolingFeatureException.class) diff --git a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/EndToEndFeatureTest.java b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/EndToEndFeatureTest.java index bee25ff3..08c7ebe4 100644 --- a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/EndToEndFeatureTest.java +++ b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/EndToEndFeatureTest.java @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * ONAP * ================================================================================ - * Copyright (C) 2018-2019 AT&T Intellectual Property. All rights reserved. + * Copyright (C) 2018-2020 AT&T Intellectual Property. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -178,21 +178,33 @@ public class EndToEndFeatureTest { } } + /* + * This test should only be run manually, after configuring all of the fields, + * thus it is ignored. + */ @Ignore @Test - public void test_SingleHost() throws Exception { + public void test_SingleHost() throws Exception { // NOSONAR run(70, 1); } + /* + * This test should only be run manually, after configuring all of the fields, + * thus it is ignored. + */ @Ignore @Test - public void test_TwoHosts() throws Exception { + public void test_TwoHosts() throws Exception { // NOSONAR run(200, 2); } + /* + * This test should only be run manually, after configuring all of the fields, + * thus it is ignored. + */ @Ignore @Test - public void test_ThreeHosts() throws Exception { + public void test_ThreeHosts() throws Exception { // NOSONAR run(200, 3); } diff --git a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/SerializerTest.java b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/SerializerTest.java index 09e83852..662e0a7c 100644 --- a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/SerializerTest.java +++ b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/SerializerTest.java @@ -97,7 +97,8 @@ public class SerializerTest { assertEquals(msg.getChannel(), decoded.getChannel()); // invalid subclass when encoding - assertThatThrownBy(() -> ser.encodeMsg(new Message() {})).isInstanceOf(JsonParseException.class) + Message msg2 = new Message() {}; + assertThatThrownBy(() -> ser.encodeMsg(msg2)).isInstanceOf(JsonParseException.class) .hasMessageContaining("cannot serialize"); // missing type when decoding diff --git a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/state/ProcessingStateTest.java b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/state/ProcessingStateTest.java index 82346f5c..7dc7b2fd 100644 --- a/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/state/ProcessingStateTest.java +++ b/feature-pooling-dmaap/src/test/java/org/onap/policy/drools/pooling/state/ProcessingStateTest.java @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * ONAP * ================================================================================ - * Copyright (C) 2018 AT&T Intellectual Property. All rights reserved. + * Copyright (C) 2018, 2020 AT&T Intellectual Property. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -269,7 +269,7 @@ public class ProcessingStateTest extends SupportBasicStateTester { String[] arr = captureHostArray(); - assertNotSame(arr, HOST_ARR3); + assertNotSame(HOST_ARR3, arr); assertEquals(Arrays.asList(HOST_ARR3), Arrays.asList(arr)); } diff --git a/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java b/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java index ed427255..efe2a294 100644 --- a/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java +++ b/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/PersistenceFeature.java @@ -767,6 +767,7 @@ public class PersistenceFeature implements PolicySessionFeatureApi, PolicyEngine } } + session.removePolicySession(); logger.info("PersistentThreadModel completed"); } } diff --git a/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/PersistenceFeatureTest.java b/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/PersistenceFeatureTest.java index 25f805d7..ec261982 100644 --- a/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/PersistenceFeatureTest.java +++ b/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/PersistenceFeatureTest.java @@ -1098,15 +1098,9 @@ public class PersistenceFeatureTest { // return adjunct on next call when(polcont.getAdjunct(any())).thenReturn(adjcap.getValue()); - try { - doThrow(new IllegalArgumentException(EXPECTED)).when(emf).close(); - - feat.destroyKieSession(polsess); - fail(MISSING_EXCEPTION); - - } catch (IllegalArgumentException ex) { - logger.trace(EXPECTED, ex); - } + IllegalArgumentException exception = new IllegalArgumentException(EXPECTED); + doThrow(exception).when(emf).close(); + assertThatCode(() -> feat.destroyKieSession(polsess)).isEqualTo(exception); verify(bds, times(2)).close(); } @@ -1126,15 +1120,10 @@ public class PersistenceFeatureTest { // return adjunct on next call when(polcont.getAdjunct(any())).thenReturn(adjcap.getValue()); - try { - doThrow(new SQLException(EXPECTED)).when(bds).close(); - - feat.destroyKieSession(polsess); - fail(MISSING_EXCEPTION); - - } catch (PersistenceFeatureException ex) { - logger.trace(EXPECTED, ex); - } + SQLException cause = new SQLException(EXPECTED); + doThrow(cause).when(bds).close(); + assertThatCode(() -> feat.destroyKieSession(polsess)).isInstanceOf(PersistenceFeatureException.class) + .hasCause(cause); } /** diff --git a/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java b/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java index 347e8c90..33bfaedc 100644 --- a/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java +++ b/feature-state-management/src/test/java/org/onap/policy/drools/statemanagement/test/StateManagementTest.java @@ -20,6 +20,7 @@ package org.onap.policy.drools.statemanagement.test; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import java.io.File; @@ -143,8 +144,8 @@ public class StateManagementTest { logger.debug("avail = {}", avail); logger.debug("standby = {}", standby); - assertTrue("Admin state not unlocked after initialization", admin.equals(StateManagement.UNLOCKED)); - assertTrue("Operational state not enabled after initialization", oper.equals(StateManagement.ENABLED)); + assertEquals("Admin state not unlocked after initialization", StateManagement.UNLOCKED, admin); + assertEquals("Operational state not enabled after initialization", StateManagement.ENABLED, oper); try { stateManagementFeature.disableFailed(); @@ -164,8 +165,8 @@ public class StateManagementTest { logger.debug("avail = {}", avail); logger.debug("standby = {}", standby); - assertTrue("Operational state not disabled after disableFailed()", oper.equals(StateManagement.DISABLED)); - assertTrue("Availability status not failed after disableFailed()", avail.equals(StateManagement.FAILED)); + assertEquals("Operational state not disabled after disableFailed()", StateManagement.DISABLED, oper); + assertEquals("Availability status not failed after disableFailed()", StateManagement.FAILED, avail); try { @@ -185,7 +186,7 @@ public class StateManagementTest { logger.debug("avail = {}", avail); logger.debug("standby = {}", standby); - assertTrue("Standby status not coldstandby after promote()", standby.equals(StateManagement.COLD_STANDBY)); + assertEquals("Standby status not coldstandby after promote()", StateManagement.COLD_STANDBY, standby); /* *************Repository Audit Test. ************* */ logger.debug("\n\ntestStateManagementOperation: Repository Audit\n\n"); diff --git a/policy-core/src/main/java/org/onap/policy/drools/core/PolicySession.java b/policy-core/src/main/java/org/onap/policy/drools/core/PolicySession.java index 49ff7bb8..389d5b37 100644 --- a/policy-core/src/main/java/org/onap/policy/drools/core/PolicySession.java +++ b/policy-core/src/main/java/org/onap/policy/drools/core/PolicySession.java @@ -190,6 +190,16 @@ public class PolicySession } /** + * Unset this 'PolicySession' instance as the one associated with the + * currently-running thread. + */ + public void removePolicySession() { + if (policySess.get() == this) { + policySess.remove(); + } + } + + /** * Get current session. * * @return the 'PolicySession' instance associated with the current thread @@ -498,6 +508,8 @@ public class PolicySession logger.error("startThread error in kieSession1.fireUntilHalt", e); } } + + session.removePolicySession(); logger.info("fireUntilHalt() returned"); } } diff --git a/policy-core/src/test/java/org/onap/policy/drools/core/PolicySessionTest.java b/policy-core/src/test/java/org/onap/policy/drools/core/PolicySessionTest.java index 63c71608..3b882195 100644 --- a/policy-core/src/test/java/org/onap/policy/drools/core/PolicySessionTest.java +++ b/policy-core/src/test/java/org/onap/policy/drools/core/PolicySessionTest.java @@ -102,7 +102,7 @@ public class PolicySessionTest { } @Test - public void testSetPolicySession_testGetCurrentSession() { + public void testSetPolicySession_testGetCurrentSession_testRemovePolicySession() { PolicySession sess2 = new PolicySession(MY_NAME + "-b", container, kie); session.setPolicySession(); @@ -110,6 +110,14 @@ public class PolicySessionTest { sess2.setPolicySession(); assertEquals(sess2, PolicySession.getCurrentSession()); + + // remove a different session - should be unchanged + session.removePolicySession(); + assertEquals(sess2, PolicySession.getCurrentSession()); + + // remove the session + sess2.removePolicySession(); + assertNull(PolicySession.getCurrentSession()); } @Test diff --git a/policy-domains/src/test/java/org/onap/policy/drools/domain/models/DomainPolicyTypesTest.java b/policy-domains/src/test/java/org/onap/policy/drools/domain/models/DomainPolicyTypesTest.java index 77dfed8d..f737766f 100644 --- a/policy-domains/src/test/java/org/onap/policy/drools/domain/models/DomainPolicyTypesTest.java +++ b/policy-domains/src/test/java/org/onap/policy/drools/domain/models/DomainPolicyTypesTest.java @@ -47,14 +47,8 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaServiceTemplate; public class DomainPolicyTypesTest { // Policy Types - private static final String OPERATIONAL_DROOLS_POLICY_TYPE = "onap.policies.controlloop.operational.common.Drools"; private static final String NATIVE_DROOLS_POLICY_TYPE = "onap.policies.native.drools.Artifact"; - // Operational vCPE Policy - private static final String OP_POLICY_NAME_VCPE = "operational.restart"; - private static final String VCPE_OPERATIONAL_DROOLS_POLICY_JSON = - "policies/vCPE.policy.operational.input.tosca.json"; - // Native Drools Policy private static final String EXAMPLE_NATIVE_DROOLS_POLICY_NAME = "example"; private static final String EXAMPLE_NATIVE_DROOLS_POLICY_JSON = @@ -184,8 +178,4 @@ public class DomainPolicyTypesTest { ToscaServiceTemplate serviceTemplate = new StandardCoder().decode(policyJson, ToscaServiceTemplate.class); return serviceTemplate.getToscaTopologyTemplate().getPolicies().get(0).get(policyName); } - - private String getExamplesPolicyString(String resourcePath, String policyName) throws CoderException { - return nonValCoder.encode(getExamplesPolicy(resourcePath, policyName)); - } -}
\ No newline at end of file +} diff --git a/policy-management/src/main/java/org/onap/policy/drools/system/Main.java b/policy-management/src/main/java/org/onap/policy/drools/system/Main.java index 4f0dc39c..9e6d0432 100644 --- a/policy-management/src/main/java/org/onap/policy/drools/system/Main.java +++ b/policy-management/src/main/java/org/onap/policy/drools/system/Main.java @@ -157,10 +157,8 @@ public class Main { .flush(); logger.warn( LoggerUtil.TRANSACTION_LOG_MARKER, - "Main: cannot start {} (bad state) because of {}", - PolicyEngineConstants.getManager(), - e.getMessage(), - e); + "Main: cannot start {} (bad state)", + PolicyEngineConstants.getManager()); throw new PolicyDroolsPdpRuntimeException( String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e); } catch (final Exception e) { @@ -171,10 +169,8 @@ public class Main { .flush(); logger.warn( LoggerUtil.TRANSACTION_LOG_MARKER, - "Main: cannot start {} because of {}", - PolicyEngineConstants.getManager(), - e.getMessage(), - e); + "Main: cannot start {}", + PolicyEngineConstants.getManager()); throw new PolicyDroolsPdpRuntimeException( String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e); } @@ -209,10 +205,8 @@ public class Main { .flush(); logger.error( LoggerUtil.TRANSACTION_LOG_MARKER, - "Main: cannot instantiate policy-controller {} because of {}", - controllerName, - e.getMessage(), - e); + "Main: cannot instantiate policy-controller {}", + controllerName); throw new PolicyDroolsPdpRuntimeException( String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e); } catch (final LinkageError e) { @@ -223,10 +217,8 @@ public class Main { .flush(); logger.warn( LoggerUtil.TRANSACTION_LOG_MARKER, - "Main: cannot instantiate policy-controller {} (linkage) because of {}", - controllerName, - e.getMessage(), - e); + "Main: cannot instantiate policy-controller {} (linkage)", + controllerName); throw new PolicyDroolsPdpRuntimeException( String.format(MessageConstants.START_FAILURE_MSG, MessageConstants.POLICY_DROOLS_PDP), e); } diff --git a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java index 3741e71a..3560671a 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/ControllerConfigurationTest.java @@ -66,7 +66,7 @@ public class ControllerConfigurationTest { ControllerConfiguration controllerConfig = new ControllerConfiguration(NAME, OPERATION, DROOLS_CONFIG); - assertEquals(controllerConfig, controllerConfig); + assertEquals(controllerConfig, (Object) controllerConfig); assertNotEquals(controllerConfig, new Object()); ControllerConfiguration controllerConfig2 = new ControllerConfiguration(); diff --git a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java index 92726f89..a18a20b4 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/DroolsConfigurationTest.java @@ -56,7 +56,7 @@ public class DroolsConfigurationTest { additionalProperties.put(ADDITIONAL_PROPERTY_KEY, ADDITIONAL_PROPERTY_VALUE); final DroolsConfiguration droolsConfig = new DroolsConfiguration(ARTIFACT, GROUPID, VERSION); - assertEquals(droolsConfig, droolsConfig); + assertEquals(droolsConfig, (Object) droolsConfig); droolsConfig.set(ARTIFACT_ID_STRING, "foobar"); assertEquals("foobar", droolsConfig.get(ARTIFACT_ID_STRING)); diff --git a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java index 352d8ce1..b02090b4 100644 --- a/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java +++ b/policy-management/src/test/java/org/onap/policy/drools/protocol/configuration/PdpdConfigurationTest.java @@ -74,7 +74,7 @@ public class PdpdConfigurationTest { drools.set("version", VERSION); drools.set(PROPERTY1, VALUE1); - assertEquals(drools, drools); + assertEquals(drools, (Object) drools); assertNotEquals(drools, new Object()); logger.info("Drools HashCode {}", drools.hashCode()); @@ -131,7 +131,7 @@ public class PdpdConfigurationTest { controller.set("drools", drools); controller.set(PROPERTY1, VALUE1); - assertEquals(controller, controller); + assertEquals(controller, (Object) controller); assertNotEquals(controller, new Object()); logger.info("Controller HashCode {}", controller.hashCode()); @@ -194,7 +194,7 @@ public class PdpdConfigurationTest { config.set("controllers", controllers); config.set(PROPERTY1, VALUE1); - assertEquals(config, config); + assertEquals(config, (Object) config); assertNotEquals(config, new Object()); logger.info("Config HashCode {}", config.hashCode()); diff --git a/policy-utils/src/test/java/org/onap/policy/drools/policies/DomainMakerTest.java b/policy-utils/src/test/java/org/onap/policy/drools/policies/DomainMakerTest.java index 47d6c2d8..5bc767cd 100644 --- a/policy-utils/src/test/java/org/onap/policy/drools/policies/DomainMakerTest.java +++ b/policy-utils/src/test/java/org/onap/policy/drools/policies/DomainMakerTest.java @@ -108,7 +108,8 @@ public class DomainMakerTest { assertDomainPolicy(domainAPolicy); domainAPolicy.getProperties().getNested().setNested1(""); - assertThatThrownBy(() -> domainMaker.conformance(policy1.getTypeIdentifier(), domainAPolicy)) + ToscaPolicyTypeIdentifier ident1 = policy1.getTypeIdentifier(); + assertThatThrownBy(() -> domainMaker.conformance(ident1, domainAPolicy)) .isInstanceOf(ValidationFailedException.class) .hasMessageContaining("Pattern ^(.+)$ is not contained in text"); } @@ -142,8 +143,9 @@ public class DomainMakerTest { @Test public void testConvertToSchema() { + ToscaPolicyType type = new ToscaPolicyType(); assertThatThrownBy(() -> domainMaker - .convertToSchema(new ToscaPolicyType())) + .convertToSchema(type)) .isInstanceOf(UnsupportedOperationException.class); } diff --git a/policy-utils/src/test/java/org/onap/policy/drools/utils/logging/MdcTransactionTest.java b/policy-utils/src/test/java/org/onap/policy/drools/utils/logging/MdcTransactionTest.java index dbf6a67c..82a2b2e6 100644 --- a/policy-utils/src/test/java/org/onap/policy/drools/utils/logging/MdcTransactionTest.java +++ b/policy-utils/src/test/java/org/onap/policy/drools/utils/logging/MdcTransactionTest.java @@ -164,7 +164,6 @@ public class MdcTransactionTest { assertEquals(trans.getInvocationId(), MDC.get(MdcTransactionConstants.INVOCATION_ID)); assertEquals(trans.timestamp(trans.getStartTime()), MDC.get(MdcTransactionConstants.BEGIN_TIMESTAMP)); assertEquals(trans.timestamp(trans.getEndTime()), MDC.get(MdcTransactionConstants.END_TIMESTAMP)); - assertNotEquals(trans.getElapsedTime(), MDC.get(MdcTransactionConstants.ELAPSED_TIME)); assertEquals(String.valueOf(Duration.between(trans.getStartTime(), trans.getEndTime()).toMillis()), MDC.get(MdcTransactionConstants.ELAPSED_TIME)); assertEquals(trans.getServiceInstanceId(), MDC.get(MdcTransactionConstants.SERVICE_INSTANCE_ID)); |