From d010fb918de5215fd5ff9219041ea11c77a4059a Mon Sep 17 00:00:00 2001 From: liamfallon Date: Thu, 11 Apr 2019 15:59:47 +0000 Subject: Fix database properties Issue-ID: POLICY-1095 Change-Id: I3edd70898836d3bd978643857d1ba29599b1cf6c Signed-off-by: liamfallon --- .../provider/PolicyModelsProviderParameters.java | 6 ++ .../impl/DatabasePolicyModelsProviderImpl.java | 69 +++++++++------------- .../PolicyModelsProviderParametersTest.java | 1 + .../impl/DatabasePolicyModelsProviderTest.java | 26 +++----- .../impl/PolicyLegacyGuardPersistenceTest.java | 1 + .../PolicyLegacyOperationalPersistenceTest.java | 1 + .../provider/impl/PolicyPersistenceTest.java | 1 + .../provider/impl/PolicyToscaPersistenceTest.java | 1 + .../provider/impl/PolicyTypePersistenceTest.java | 1 + .../src/test/resources/META-INF/persistence.xml | 29 --------- 10 files changed, 46 insertions(+), 90 deletions(-) (limited to 'models-provider/src') diff --git a/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProviderParameters.java b/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProviderParameters.java index 5abafed22..65aa72eb2 100644 --- a/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProviderParameters.java +++ b/models-provider/src/main/java/org/onap/policy/models/provider/PolicyModelsProviderParameters.java @@ -55,6 +55,7 @@ public class PolicyModelsProviderParameters implements ParameterGroup { private String name; private String implementation = DEFAULT_IMPLEMENTATION; + private String databaseDriver; private String databaseUrl; private String databaseUser; private String databasePassword; @@ -73,6 +74,11 @@ public class PolicyModelsProviderParameters implements ParameterGroup { "a PolicyModelsProvider implementation must be specified"); } + if (!ParameterValidationUtils.validateStringParameter(databaseDriver)) { + validationResult.setResult("databaseUrl", ValidationStatus.INVALID, + "a driver must be specified for the JDBC connection to the database"); + } + if (!ParameterValidationUtils.validateStringParameter(databaseUrl)) { validationResult.setResult("databaseUrl", ValidationStatus.INVALID, "a URL must be specified for the JDBC connection to the database"); diff --git a/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java b/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java index cc70df903..475e5769d 100644 --- a/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java +++ b/models-provider/src/main/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderImpl.java @@ -20,11 +20,10 @@ package org.onap.policy.models.provider.impl; -import java.sql.Connection; -import java.sql.DriverManager; import java.util.Base64; import java.util.List; import java.util.Map; +import java.util.Properties; import javax.ws.rs.core.Response; @@ -64,12 +63,20 @@ import org.slf4j.LoggerFactory; * @author Liam Fallon (liam.fallon@est.tech) */ public class DatabasePolicyModelsProviderImpl implements PolicyModelsProvider { + private static final Logger LOGGER = LoggerFactory.getLogger(DefaultPfDao.class); + // Constants for persistence properties + // @formatter:off + private static final String JAVAX_PERSISTENCE_JDBC_DRIVER = "javax.persistence.jdbc.driver"; + private static final String JAVAX_PERSISTENCE_JDBC_URL = "javax.persistence.jdbc.url"; + private static final String JAVAX_PERSISTENCE_JDBC_USER = "javax.persistence.jdbc.user"; + private static final String JAVAX_PERSISTENCE_JDBC_PWORD = "javax.persistence.jdbc.password"; + // @formatter:on + private final PolicyModelsProviderParameters parameters; // Database connection and the DAO for reading and writing Policy Framework concepts - private Connection connection; private PfDao pfDao; /** @@ -86,30 +93,31 @@ public class DatabasePolicyModelsProviderImpl implements PolicyModelsProvider { LOGGER.debug("opening the database connection to {} using persistence unit {}", parameters.getDatabaseUrl(), parameters.getPersistenceUnit()); - if (connection != null || pfDao != null) { + if (pfDao != null) { String errorMessage = "provider is already initialized"; LOGGER.warn(errorMessage); throw new PfModelException(Response.Status.NOT_ACCEPTABLE, errorMessage); } - // Decode the password using Base64 - String decodedPassword = new String(Base64.getDecoder().decode(parameters.getDatabasePassword())); - - // Connect to the database, call does not implement AutoCloseable for try-with-resources - try { - connection = DriverManager.getConnection(parameters.getDatabaseUrl(), parameters.getDatabaseUser(), - decodedPassword); - } catch (Exception exc) { - String errorMessage = "could not connect to database with URL \"" + parameters.getDatabaseUrl() + "\""; - LOGGER.warn(errorMessage, exc); - throw new PfModelException(Response.Status.NOT_ACCEPTABLE, errorMessage, exc); - } - // Parameters for the DAO final DaoParameters daoParameters = new DaoParameters(); daoParameters.setPluginClass(DefaultPfDao.class.getCanonicalName()); daoParameters.setPersistenceUnit(parameters.getPersistenceUnit()); + // Decode the password using Base64 + String decodedPassword = new String(Base64.getDecoder().decode(parameters.getDatabasePassword())); + + // @formatter:off + Properties jdbcProperties = new Properties(); + jdbcProperties.setProperty(JAVAX_PERSISTENCE_JDBC_DRIVER, parameters.getDatabaseDriver()); + jdbcProperties.setProperty(JAVAX_PERSISTENCE_JDBC_URL, parameters.getDatabaseUrl()); + jdbcProperties.setProperty(JAVAX_PERSISTENCE_JDBC_USER, parameters.getDatabaseUser()); + jdbcProperties.setProperty(JAVAX_PERSISTENCE_JDBC_PWORD, decodedPassword); + // @formatter:on + + daoParameters.setJdbcProperties(jdbcProperties); + + pfDao = new PfDaoFactory().createPfDao(daoParameters); try { pfDao = new PfDaoFactory().createPfDao(daoParameters); pfDao.init(daoParameters); @@ -133,20 +141,6 @@ public class DatabasePolicyModelsProviderImpl implements PolicyModelsProvider { pfDao = null; } - if (connection != null) { - try { - connection.close(); - } catch (Exception exc) { - - String errorMessage = - "could not close connection to database with URL \"" + parameters.getDatabaseUrl() + "\""; - LOGGER.warn(errorMessage, exc); - throw new PfModelException(Response.Status.INTERNAL_SERVER_ERROR, errorMessage, exc); - } finally { - connection = null; - } - } - LOGGER.debug("closed the database connection to {} using persistence unit {}", parameters.getDatabaseUrl(), parameters.getPersistenceUnit()); } @@ -328,8 +322,8 @@ public class DatabasePolicyModelsProviderImpl implements PolicyModelsProvider { } @Override - public void updatePdp(@NonNull String pdpGroupName, @NonNull String pdpGroupVersion, - @NonNull String pdpSubGroup, @NonNull Pdp pdp) throws PfModelException { + public void updatePdp(@NonNull String pdpGroupName, @NonNull String pdpGroupVersion, @NonNull String pdpSubGroup, + @NonNull Pdp pdp) throws PfModelException { new PdpProvider().updatePdp(pfDao, pdpGroupName, pdpGroupVersion, pdpSubGroup, pdp); } @@ -364,13 +358,4 @@ public class DatabasePolicyModelsProviderImpl implements PolicyModelsProvider { throw new PfModelRuntimeException(Response.Status.BAD_REQUEST, errorMessage); } } - - /** - * Hook for unit test mocking of database connection. - * - * @param client the mocked client - */ - protected void setConnection(final Connection connection) { - this.connection = connection; - } } diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/PolicyModelsProviderParametersTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/PolicyModelsProviderParametersTest.java index 626d2bf5d..2f3f89c0a 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/PolicyModelsProviderParametersTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/PolicyModelsProviderParametersTest.java @@ -36,6 +36,7 @@ public class PolicyModelsProviderParametersTest { @Test public void testParameters() { PolicyModelsProviderParameters pars = new PolicyModelsProviderParameters(); + pars.setDatabaseDriver("MichaelsShumacher"); pars.setDatabaseUrl("jdbc://www.acmecorp/roadrunner"); pars.setPersistenceUnit("WileECoyote"); diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java index 99358c4c6..ff8dfe322 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/DatabasePolicyModelsProviderTest.java @@ -66,11 +66,11 @@ public class DatabasePolicyModelsProviderTest { @Before public void setupParameters() { parameters = new PolicyModelsProviderParameters(); + parameters.setDatabaseDriver("org.h2.Driver"); parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setDatabaseUser("policy"); parameters.setDatabasePassword(Base64.getEncoder().encodeToString("P01icY".getBytes())); parameters.setPersistenceUnit("ToscaConceptTest"); - } @Test @@ -84,27 +84,21 @@ public class DatabasePolicyModelsProviderTest { parameters.setDatabaseUrl("jdbc://www.acmecorp.nonexist"); - assertThatThrownBy(() -> { - databaseProvider.close(); - databaseProvider.init(); - }).hasMessage("could not connect to database with URL \"jdbc://www.acmecorp.nonexist\""); - - parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); - try { - databaseProvider.init(); databaseProvider.close(); + databaseProvider.init(); } catch (Exception pfme) { fail("test shold not throw an exception here"); } + databaseProvider.close(); + + parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setPersistenceUnit("WileECoyote"); - String errorMessage = "could not create Data Access Object (DAO) using url " - + "\"jdbc:h2:mem:testdb\" and persistence unit \"WileECoyote\""; assertThatThrownBy(() -> { databaseProvider.init(); - }).hasMessage(errorMessage); + }).hasMessageContaining("could not create Data Access Object (DAO)"); parameters.setPersistenceUnit("ToscaConceptTest"); @@ -112,6 +106,7 @@ public class DatabasePolicyModelsProviderTest { databaseProvider.init(); databaseProvider.close(); } catch (Exception pfme) { + pfme.printStackTrace(); fail("test shold not throw an exception here"); } @@ -131,13 +126,6 @@ public class DatabasePolicyModelsProviderTest { } catch (Exception pfme) { fail("test shold not throw an exception here"); } - - assertThatThrownBy(() -> { - DatabasePolicyModelsProviderImpl databaseProviderImpl = (DatabasePolicyModelsProviderImpl) databaseProvider; - databaseProvider.init(); - databaseProviderImpl.setConnection(new DummyConnection()); - databaseProvider.close(); - }).hasMessage("could not close connection to database with URL \"jdbc:h2:mem:testdb\""); } @Test diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyGuardPersistenceTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyGuardPersistenceTest.java index 5ebb44814..741ae8998 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyGuardPersistenceTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyGuardPersistenceTest.java @@ -76,6 +76,7 @@ public class PolicyLegacyGuardPersistenceTest { @Before public void setupParameters() throws PfModelException { PolicyModelsProviderParameters parameters = new PolicyModelsProviderParameters(); + parameters.setDatabaseDriver("org.h2.Driver"); parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setDatabaseUser("policy"); parameters.setDatabasePassword(Base64.getEncoder().encodeToString("P01icY".getBytes())); diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyOperationalPersistenceTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyOperationalPersistenceTest.java index af6c4a4cc..2e33a11a0 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyOperationalPersistenceTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyLegacyOperationalPersistenceTest.java @@ -76,6 +76,7 @@ public class PolicyLegacyOperationalPersistenceTest { @Before public void setupParameters() throws PfModelException { PolicyModelsProviderParameters parameters = new PolicyModelsProviderParameters(); + parameters.setDatabaseDriver("org.h2.Driver"); parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setDatabaseUser("policy"); parameters.setDatabasePassword(Base64.getEncoder().encodeToString("P01icY".getBytes())); diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyPersistenceTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyPersistenceTest.java index 81795dbda..668f63497 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyPersistenceTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyPersistenceTest.java @@ -85,6 +85,7 @@ public class PolicyPersistenceTest { @Before public void setupParameters() throws PfModelException { PolicyModelsProviderParameters parameters = new PolicyModelsProviderParameters(); + parameters.setDatabaseDriver("org.h2.Driver"); parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setDatabaseUser("policy"); parameters.setDatabasePassword(Base64.getEncoder().encodeToString("P01icY".getBytes())); diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyToscaPersistenceTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyToscaPersistenceTest.java index 8f05244b9..613c5a2c6 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyToscaPersistenceTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyToscaPersistenceTest.java @@ -83,6 +83,7 @@ public class PolicyToscaPersistenceTest { @Before public void setupParameters() throws PfModelException { PolicyModelsProviderParameters parameters = new PolicyModelsProviderParameters(); + parameters.setDatabaseDriver("org.h2.Driver"); parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setDatabaseUser("policy"); parameters.setDatabasePassword(Base64.getEncoder().encodeToString("P01icY".getBytes())); diff --git a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyTypePersistenceTest.java b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyTypePersistenceTest.java index 528395efc..7b541e128 100644 --- a/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyTypePersistenceTest.java +++ b/models-provider/src/test/java/org/onap/policy/models/provider/impl/PolicyTypePersistenceTest.java @@ -84,6 +84,7 @@ public class PolicyTypePersistenceTest { @Before public void setupParameters() throws PfModelException { PolicyModelsProviderParameters parameters = new PolicyModelsProviderParameters(); + parameters.setDatabaseDriver("org.h2.Driver"); parameters.setDatabaseUrl("jdbc:h2:mem:testdb"); parameters.setDatabaseUser("policy"); parameters.setDatabasePassword(Base64.getEncoder().encodeToString("P01icY".getBytes())); diff --git a/models-provider/src/test/resources/META-INF/persistence.xml b/models-provider/src/test/resources/META-INF/persistence.xml index c7d6d1e4b..7b5bc14b9 100644 --- a/models-provider/src/test/resources/META-INF/persistence.xml +++ b/models-provider/src/test/resources/META-INF/persistence.xml @@ -33,34 +33,9 @@ org.onap.policy.models.pdp.persistence.concepts.JpaPdp - - - - - - - - - org.eclipse.persistence.jpa.PersistenceProvider - - org.onap.policy.models.dao.converters.CDataConditioner - org.onap.policy.models.dao.converters.Uuid2String - org.onap.policy.models.base.PfConceptKey - org.onap.policy.models.tosca.simple.concepts.JpaToscaPolicyType - org.onap.policy.models.tosca.simple.concepts.JpaToscaPolicy - org.onap.policy.models.pdp.persistence.concepts.JpaPdpGroup - org.onap.policy.models.pdp.persistence.concepts.JpaPdpSubGroup - org.onap.policy.models.pdp.persistence.concepts.JpaPdp - - - - - - - - - - - -- cgit 1.2.3-korg