Fix database properties

Issue-ID: POLICY-1095
Change-Id: I3edd70898836d3bd978643857d1ba29599b1cf6c
Signed-off-by: liamfallon <liam.fallon@est.tech>

10 files changed, 46 insertions, 90 deletions

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 @@
         <class>org.onap.policy.models.pdp.persistence.concepts.JpaPdp</class>
 
         <properties>
-            <property name="javax.persistence.jdbc.driver" value="org.h2.Driver" />
-            <property name="javax.persistence.jdbc.url" value="jdbc:h2:mem:testdb" />
-            <property name="javax.persistence.jdbc.user" value="policy" />
-            <property name="javax.persistence.jdbc.password" value="P01icY" />
             <property name="eclipselink.ddl-generation" value="drop-and-create-tables" />
             <property name="eclipselink.ddl-generation.output-mode" value="database" />
             <property name="eclipselink.logging.level" value="INFO" />
-        </properties>
-    </persistence-unit>
-
-    <persistence-unit name="ToscaConceptMariaDBTest" transaction-type="RESOURCE_LOCAL">
-        <provider>org.eclipse.persistence.jpa.PersistenceProvider</provider>
-
-        <class>org.onap.policy.models.dao.converters.CDataConditioner</class>
-        <class>org.onap.policy.models.dao.converters.Uuid2String</class>
-        <class>org.onap.policy.models.base.PfConceptKey</class>
-        <class>org.onap.policy.models.tosca.simple.concepts.JpaToscaPolicyType</class>
-        <class>org.onap.policy.models.tosca.simple.concepts.JpaToscaPolicy</class>
-        <class>org.onap.policy.models.pdp.persistence.concepts.JpaPdpGroup</class>
-        <class>org.onap.policy.models.pdp.persistence.concepts.JpaPdpSubGroup</class>
-        <class>org.onap.policy.models.pdp.persistence.concepts.JpaPdp</class>
-
-        <properties>
-            <property name="javax.persistence.jdbc.driver" value="org.mariadb.jdbc.Driver" />
-            <property name="javax.persistence.jdbc.url" value="jdbc:mariadb://localhost:3306/policy" />
-            <property name="javax.persistence.jdbc.user" value="policy" />
-            <property name="javax.persistence.jdbc.password" value="P01icY" />
-            <property name="javax.persistence.schema-generation.database.action" value="create" />
 
             <!-- property name="eclipselink.logging.level" value="ALL" />
             <property name="eclipselink.logging.level.jpa" value="ALL" />
@@ -72,10 +47,6 @@
             <property name="eclipselink.logging.level.server" value="ALL" />
             <property name="eclipselink.logging.level.query" value="ALL" />
             <property name="eclipselink.logging.level.properties" value="ALL" /-->
-
-            <property name="eclipselink.ddl-generation" value="drop-and-create-tables" />
-            <property name="eclipselink.ddl-generation.output-mode" value="database" />
-            <property name="eclipselink.logging.level" value="INFO" />
         </properties>
     </persistence-unit>
 </persistence>