From e19472aecff4f5ca9351feadf21037e729ad2c9c Mon Sep 17 00:00:00 2001 From: Jan Blom Date: Tue, 21 Nov 2023 16:47:21 +0100 Subject: [PATCH] Restructuring database configuration. Work in process, but unit and integration tests all OK --- README.md | 14 +- .../org/ohdsi/databases/DBConfiguration.java | 194 ++++++++++++++++ .../ohdsi/databases/DBConnectorInterface.java | 7 +- .../ohdsi/databases/SnowflakeConnector.java | 141 ++++++++++-- .../org/ohdsi/utilities/files/IniFile.java | 9 +- .../ohdsi/databases/DBConfigurationTest.java | 59 +++++ .../databases/TestSnowflakeConnector.java | 215 ++++++++++-------- rabbit-core/src/test/resources/snowflake.ini | 16 ++ .../scan/SourceDataScanSnowflakeIT.java | 1 - .../scan/TestSourceDataScanCsvIniFile.java | 1 - 10 files changed, 539 insertions(+), 118 deletions(-) create mode 100644 rabbit-core/src/main/java/org/ohdsi/databases/DBConfiguration.java create mode 100644 rabbit-core/src/test/java/org/ohdsi/databases/DBConfigurationTest.java create mode 100644 rabbit-core/src/test/resources/snowflake.ini diff --git a/README.md b/README.md index 9d126a49..853e66f4 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ Requires Java 1.8 or higher, and read access to the database to be scanned. Java Dependencies ============ -For the distributable packages, the only requirement is Java 8. For building the package, also Maven is needed. +For the distributable packages, the only requirement is Java 8. For building the package, Java 17 and Maven are needed. Getting Started =============== @@ -91,15 +91,17 @@ Development White Rabbit and Rabbit in a Hat are structured as a Maven package and can be developed in Eclipse. Contributions are welcome. While the software in the project can be executed with Java 1.8, for development Java 17 is needed. -This has to do with test and verification dependencies that are not available in -a version compatible with Java 1.8 . +This has to do with test and verification dependencies that are not available in a version compatible with Java 1.8 . + +Please note that when using an IDE for development, source and target release must still be Java 1.8 . This is enforced +in the maven build file (pom.xml), To generate the files ready for distribution, run `mvn install`. ### Testing -A limited number of unit and integration tests exist. The integration tests run only in the `maven verify` phase, -and depend on docker being available to the user running the verification. If docker is not available, the +A limited number of unit and integration tests exist. The integration tests run only in the maven verification phase, +(`mn verify`) and depend on docker being available to the user running the verification. If docker is not available, the integration tests will fail. Also, GitHub actions have been configured to run the test suite automatically. @@ -119,7 +121,7 @@ is provided through environment variables: It is recommended that user, password, database and schema are created for these tests only, and do not relate in any way to any production environment. -The schema should not contain any tables. +The schema should not contain any tables when the test is started. It is possible to skip the Snowflake tests without failing the build by passing `-Dohdsi.org.whiterabbit.skip_snowflake_tests=1` to maven. diff --git a/rabbit-core/src/main/java/org/ohdsi/databases/DBConfiguration.java b/rabbit-core/src/main/java/org/ohdsi/databases/DBConfiguration.java new file mode 100644 index 00000000..f9f25247 --- /dev/null +++ b/rabbit-core/src/main/java/org/ohdsi/databases/DBConfiguration.java @@ -0,0 +1,194 @@ +package org.ohdsi.databases; + +import org.apache.commons.lang.StringUtils; +import org.ohdsi.utilities.files.IniFile; + +import java.io.PrintStream; +import java.util.*; + +public abstract class DBConfiguration { + public static final String ERROR_DUPLICATE_DEFINITIONS_FOR_FIELD = "Multiple definitions for field "; + private IniFile iniFile; + private ConfigurationFields configurationFields; + + private DBConfiguration() {} + public DBConfiguration(IniFile inifile) { + this.iniFile = inifile; + } + + protected DBConfiguration(ConfigurationField... fields) { + checkForDuplicates(fields); + this.configurationFields = new ConfigurationFields(fields); + } + + private void checkForDuplicates(ConfigurationField... fields) { + Set names = new HashSet<>(); + for (ConfigurationField field: fields) { + if (names.contains(field.name)) { + throw new DBConfigurationException(ERROR_DUPLICATE_DEFINITIONS_FOR_FIELD + field.name); + } + names.add(field.name); + } + } + public ValidationFeedback validate() { + ValidationFeedback configurationFeedback = new ValidationFeedback(); + for (ConfigurationField field: this.getFields()) { + for (FieldValidator validator: field.validators) { + ValidationFeedback feedback = validator.validate(field); + configurationFeedback.addWarnings(feedback.getWarnings()); + configurationFeedback.addErrors(feedback.getErrors()); + } + } + + return configurationFeedback; + } + + public List getFields() { + return configurationFields.getFields(); + } + public void printIniFileTemplate(PrintStream stream) { + for (ConfigurationField field: this.configurationFields.getFields()) { + stream.printf("%s: %s\t%s%n", + field.name, + StringUtils.isEmpty(field.getDefaultValue()) ? "_" : field.getDefaultValue(), + field.toolTip); + } + } + public interface FieldSet { + List getFields(); + + default void generateIniFileFormat() { + for (ConfigurationField field: getFields()) { + System.out.println(String.format("%s:\t___\t# %s", field.name, field.toolTip)); + } + } + } + + public static class DBConfigurationException extends RuntimeException { + public DBConfigurationException(String s) { + super(s); + } + }; + + @FunctionalInterface + public interface FieldValidator { + ValidationFeedback validate(ConfigurationField field); + } + + @FunctionalInterface + public interface ConfigurationValidator { + ValidationFeedback validate(ConfigurationFields fields); + } + + public static class ValidationFeedback { + private List warnings = new ArrayList<>(); + private List errors = new ArrayList<>(); + + public boolean isFullyValid() { + return warnings.isEmpty() && errors.isEmpty(); + } + + public boolean hasWarnings() { + return !warnings.isEmpty(); + } + + public boolean hasErrors() { + return !errors.isEmpty(); + } + + public List getWarnings() { + return this.warnings; + } + + public List getErrors() { + return this.errors; + } + + public void addWarning(String warning) { + this.warnings.add(warning); + } + public void addWarnings(List warnings) { + this.warnings.addAll(warnings); + } + public void addError(String error) { + this.errors.add(error); + } + public void addErrors(List errors) { + this.errors.addAll(errors); + } + } + + public static class ConfigurationFields implements FieldSet { + List fields; + + public ConfigurationFields(ConfigurationField... fields) { + this.fields = new ArrayList<>(Arrays.asList(fields)); + } + + public List getFields() { + return this.fields; + } + } + + public static class ConfigurationField { + public final String name; + public final String label; + public final String toolTip; + private String value; + private String defaultValue; + + public static final String VALUE_REQUIRED_FORMAT_STRING = "A non-empty value is required for field %s (name %s)"; + private List validators = new ArrayList<>(); + + private static FieldValidator fieldRequiredValidator = new FieldRequiredValidator(); + + private ConfigurationField(String name, String label, String toolTip) { + this.name = name; + this.label = label; + this.toolTip = toolTip; + this.defaultValue = ""; + } + + public static ConfigurationField create(String name, String label, String toolTip) { + return new ConfigurationField(name, label, toolTip); + } + + public ConfigurationField required() { + this.addValidator(fieldRequiredValidator); + return this; + } + + public ConfigurationField addDefaultValue(String value) { + this.defaultValue = value; + return this; + } + + public ConfigurationField addValidator(FieldValidator validator) { + this.validators.add(validator); + return this; + } + + public void setValue(String value) { + this.value = value; + } + + public String getValue() { + return this.value; + } + + public String getDefaultValue() { + return this.defaultValue; + } + + private static class FieldRequiredValidator implements FieldValidator { + public ValidationFeedback validate(ConfigurationField field) { + ValidationFeedback feedback = new ValidationFeedback(); + if (StringUtils.isEmpty(field.getValue())) { + feedback.addError(String.format(VALUE_REQUIRED_FORMAT_STRING, field.label, field.name)); + } + + return feedback; + } + } + } +} diff --git a/rabbit-core/src/main/java/org/ohdsi/databases/DBConnectorInterface.java b/rabbit-core/src/main/java/org/ohdsi/databases/DBConnectorInterface.java index 69a39711..35b17345 100644 --- a/rabbit-core/src/main/java/org/ohdsi/databases/DBConnectorInterface.java +++ b/rabbit-core/src/main/java/org/ohdsi/databases/DBConnectorInterface.java @@ -17,9 +17,9 @@ public interface DBConnectorInterface { void checkInitialised(); DBConnectorInterface getInstance(); - default public DBConnectorInterface getInstance(DbSettings dbSettings) { - return getInstance(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); - } +// default DBConnectorInterface getInstance(DbSettings dbSettings) { +// return getInstance(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); +// } DBConnectorInterface getInstance(String server, String database, String user, String password); /** * Returns the row count of the specified table. @@ -76,4 +76,5 @@ static DBConnectorInterface getDBConnectorInstance(IniFile iniFile) { DbSettings getDbSettings(); + public List getFields(); } diff --git a/rabbit-core/src/main/java/org/ohdsi/databases/SnowflakeConnector.java b/rabbit-core/src/main/java/org/ohdsi/databases/SnowflakeConnector.java index 3ee8a1f9..680d6425 100644 --- a/rabbit-core/src/main/java/org/ohdsi/databases/SnowflakeConnector.java +++ b/rabbit-core/src/main/java/org/ohdsi/databases/SnowflakeConnector.java @@ -10,9 +10,10 @@ import java.util.List; import org.apache.http.client.utils.URLEncodedUtils; import org.ohdsi.utilities.files.IniFile; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; - -import static org.ohdsi.databases.SnowflakeConnector.ServerConfig.buildUrl; +import static org.ohdsi.databases.SnowflakeConnector.ServerConfig.*; /* * SnowflakeDB implements all Snowflake specific logic required to connect to, and query, a Snowflake instance. @@ -22,6 +23,12 @@ public enum SnowflakeConnector implements DBConnectorInterface { INSTANCE(); + static Logger logger = LoggerFactory.getLogger(SnowflakeConnector.class); + + public static final String ERROR_NO_PASSWORD_OR_AUTHENTICATOR = "No authentication method (password or authenticator) specified for Snowflake."; + public static final String WARNING_PASSWORD_AND_AUTHENTICATOR_SPECIFIED = + "Both password and an authenticator method have been specified for Snowflake. The password will be ignored"; + private DbSettings dbSettings = null; private ServerConfig serverConfig = null; private Connection snowflakeConnection = null; @@ -55,6 +62,12 @@ public void resetConnection() throws SQLException { } public DBConnectorInterface getInstance(IniFile iniFile) { + readAndValidate(iniFile); + + return getInstance(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); + } + + public boolean readAndValidate(IniFile iniFile) { warehouse = iniFile.getOrFail("SNOWFLAKE_WAREHOUSE"); database = iniFile.getOrFail("SNOWFLAKE_DATABASE"); schema = iniFile.getOrFail("SNOWFLAKE_SCHEMA"); @@ -69,10 +82,32 @@ public DBConnectorInterface getInstance(IniFile iniFile) { authenticator = iniFile.get("SNOWFLAKE_AUTHENTICATOR"); if (StringUtils.isEmpty(dbSettings.password) && StringUtils.isEmpty(authenticator)) { - throw new RuntimeException("No authentication method (password or authenticator) specified for Snowflake."); + throw new RuntimeException(ERROR_NO_PASSWORD_OR_AUTHENTICATOR); + } else if (!StringUtils.isEmpty(dbSettings.password) && !StringUtils.isEmpty(authenticator)) { + logger.warn(WARNING_PASSWORD_AND_AUTHENTICATOR_SPECIFIED); + } else if (!StringUtils.isEmpty(authenticator) && !authenticator.equals("externalbrowser")) { + switch (authenticator.toLowerCase()) { + case "externalbrowser": + // this is a supported/tested authentication method, no need to inform the user + break; + case "snowflake": + case "oauth": + case "snowflake_jwt": + case "username_password_mfa": + // These methods are supported by Snowflake, but have not been tested. Warn the user about this. + // See https://docs.snowflake.com/en/developer-guide/jdbc/jdbc-parameters + logger.warn(String.format("Authentication method '%s' is untested. It will might not work.", authenticator)); + break; + default: + if (authenticator.matches("^https://.*\\.okta\\.com")) { + logger.warn(String.format("Authentication method for okta.com is untested. It will might not work.", authenticator)); + } else { + throw new RuntimeException(String.format("Unsupported authentication method for Snowflake: %s", authenticator)); + } + } } - return getInstance(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); + return true; } @Override @@ -122,18 +157,18 @@ public void checkInitialised() { } } - public Connection getConnection(DbSettings dbSettings) { - return getConnection(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); - } +// public Connection getConnection(DbSettings dbSettings) { +// return getConnection(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); +// } - public Connection getConnection(String server, String fullSchemaPath, String user, String password) { - SnowflakeConnector.INSTANCE.getInstance(server, fullSchemaPath, user, password); - return snowflakeConnection; - } - public Connection connect(DbSettings dbSettings) throws RuntimeException { - SnowflakeConnector.INSTANCE.getInstance(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); - return this.snowflakeConnection; - } +// public Connection getConnection(String server, String fullSchemaPath, String user, String password) { +// SnowflakeConnector.INSTANCE.getInstance(server, fullSchemaPath, user, password); +// return snowflakeConnection; +// } +// public Connection connect(DbSettings dbSettings) throws RuntimeException { +// SnowflakeConnector.INSTANCE.getInstance(dbSettings.server, dbSettings.database, dbSettings.user, dbSettings.password); +// return this.snowflakeConnection; +// } public DbSettings getDbSettings() { if (dbSettings == null) { @@ -143,6 +178,12 @@ public DbSettings getDbSettings() { return dbSettings; } + @Override + public List getFields() { + //SnowFlakeConfiguration[] iets = SnowFlakeConfiguration.values(); + return new ArrayList<>(); + } + private static Connection connectToSnowflake(String server, String schema, String user, String password) { try { Class.forName("net.snowflake.client.jdbc.SnowflakeDriver"); @@ -166,14 +207,61 @@ public ResultSet getFieldNames(String table) { } } - public static class ServerConfig { + public static class SnowFlakeConfiguration extends DBConfiguration { + public SnowFlakeConfiguration() { + super( + DBConfiguration.ConfigurationField.create( + SNOWFLAKE_ACCOUNT, + "Account", + "Account for the Snowflake instance"), + DBConfiguration.ConfigurationField.create( + SNOWFLAKE_USER, + "User", + "User for the Snowflake instance"), + DBConfiguration.ConfigurationField.create( + ServerConfig.SNOWFLAKE_PASSWORD, + "Password", + "Password for the Snowflake instance"), + DBConfiguration.ConfigurationField.create( + ServerConfig.SNOWFLAKE_WAREHOUSE, + "Warehouse", + "Warehouse for the Snowflake instance"), + DBConfiguration.ConfigurationField.create( + ServerConfig.SNOWFLAKE_DATABASE, + "Database", + "Database for the Snowflake instance"), + DBConfiguration.ConfigurationField.create( + ServerConfig.SNOWFLAKE_SCHEMA, + "Schema", + "Schema for the Snowflake instance"), + DBConfiguration.ConfigurationField.create( + ServerConfig.SNOWFLAKE_AUTHENTICATOR, + "Authenticator method", + "Snowflake JDBC authenticator method (only 'externalbrowser' is currently supported)") + ); + } + + private final List fields = new ArrayList<>(); + + } + public static class ServerConfig extends DBConfiguration { + public static final String SNOWFLAKE_ACCOUNT = "SNOWFLAKE_ACCOUNT"; + public static final String SNOWFLAKE_USER = "SNOWFLAKE_USER"; + public static final String SNOWFLAKE_PASSWORD = "SNOWFLAKE_PASSWORD"; + public static final String SNOWFLAKE_AUTHENTICATOR = "SNOWFLAKE_AUTHENTICATOR"; + public static final String SNOWFLAKE_WAREHOUSE = "SNOWFLAKE_WAREHOUSE"; + public static final String SNOWFLAKE_DATABASE = "SNOWFLAKE_DATABASE"; + public static final String SNOWFLAKE_SCHEMA = "SNOWFLAKE_SCHEMA"; public final String server; + public final String account; public final String user; public final String password; public final String fullSchemaPath; public final String warehouse; public final String database; public final String schema; + public final String authenticator; + public final DbSettings dbSettings; public static List checkSnowflakeConfig(String server, String user, String password, String schemaName) throws RuntimeException { @@ -243,6 +331,8 @@ private static String appendParameterIfSet(String url, String name, String value } } public ServerConfig(String server, String fullSchemaPath, String user, String password) { + super(new IniFile()); + this.account = ""; this.server = server; this.fullSchemaPath = fullSchemaPath; this.user = user; @@ -251,6 +341,25 @@ public ServerConfig(String server, String fullSchemaPath, String user, String pa this.warehouse = parts[0]; this.database = parts[1]; this.schema = parts[2]; + this.authenticator = ""; + + this.dbSettings = new DbSettings(); + } + + public ServerConfig(IniFile iniFile) { + super(iniFile); + + this.server = String.format("https://%s.snowflakecomputing.com", iniFile.getOrFail("SNOWFLAKE_ACCOUNT")); + this.account = iniFile.getOrFail(SNOWFLAKE_ACCOUNT); + this.user = iniFile.getOrFail(SNOWFLAKE_USER); + this.password = iniFile.get(SNOWFLAKE_PASSWORD); + this.authenticator = iniFile.get(SNOWFLAKE_AUTHENTICATOR); + this.warehouse = iniFile.getOrFail(SNOWFLAKE_WAREHOUSE); + this.database = iniFile.getOrFail(SNOWFLAKE_DATABASE); + this.schema = iniFile.getOrFail(SNOWFLAKE_SCHEMA); + this.fullSchemaPath = String.format("%s.%s.%s", this.warehouse, this.database, this.schema); + + this.dbSettings = new DbSettings(); } } } diff --git a/rabbit-core/src/main/java/org/ohdsi/utilities/files/IniFile.java b/rabbit-core/src/main/java/org/ohdsi/utilities/files/IniFile.java index 2113d4ef..0c1cc0e1 100644 --- a/rabbit-core/src/main/java/org/ohdsi/utilities/files/IniFile.java +++ b/rabbit-core/src/main/java/org/ohdsi/utilities/files/IniFile.java @@ -25,6 +25,9 @@ public class IniFile { private Map settings = new HashMap(); + public IniFile() { + + } public IniFile(String filename){ for (String line : new ReadTextFile(filename)){ int indexOfHash = line.lastIndexOf('#'); @@ -46,8 +49,12 @@ public String get(String fieldName){ return value; } + public void set(String fieldName, String value) { + settings.put(fieldName.trim().toLowerCase(), value); + } + public String getOrFail(String fieldName){ - String value = settings.get(fieldName.toLowerCase()); + String value = this.get(fieldName); if (StringUtils.isEmpty(value)) { throw new RuntimeException("Ini file should contain a value for '" + fieldName + "'"); } diff --git a/rabbit-core/src/test/java/org/ohdsi/databases/DBConfigurationTest.java b/rabbit-core/src/test/java/org/ohdsi/databases/DBConfigurationTest.java new file mode 100644 index 00000000..51f8e68c --- /dev/null +++ b/rabbit-core/src/test/java/org/ohdsi/databases/DBConfigurationTest.java @@ -0,0 +1,59 @@ +package org.ohdsi.databases; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; +import static org.ohdsi.databases.DBConfiguration.ConfigurationField.VALUE_REQUIRED_FORMAT_STRING; +import static org.ohdsi.databases.DBConfiguration.ERROR_DUPLICATE_DEFINITIONS_FOR_FIELD; + +class DBConfigurationTest { + + private final String NAME_FIELD1 = "FIELD_1"; + private final String LABEL_FIELD1 = "Field one"; + private final String TOOLTIP_FIELD1 = "Tooltip for field one"; + private final String NAME_FIELD2 = "FIELD_2"; + private final String LABEL_FIELD2 = "Field two"; + private final String TOOLTIP_FIELD2 = "Tooltip for field two"; + private class TestConfiguration extends DBConfiguration { + public TestConfiguration(ConfigurationField... fields) { + super(fields); + } + } + + @BeforeEach + void setUp() { + } + + @Test + void validateRequiredField() { + TestConfiguration testConfiguration = new TestConfiguration( + DBConfiguration.ConfigurationField.create(NAME_FIELD1, LABEL_FIELD1, TOOLTIP_FIELD1).required(), + DBConfiguration.ConfigurationField.create(NAME_FIELD2, LABEL_FIELD2, TOOLTIP_FIELD2)); + + DBConfiguration.ValidationFeedback feedback = testConfiguration.validate(); + assertEquals(0, feedback.getWarnings().size(), "There should be no warnings"); + assertEquals(1, feedback.getErrors().size(), "There should be 1 error"); + assertEquals(feedback.getErrors().get(0), String.format(VALUE_REQUIRED_FORMAT_STRING, LABEL_FIELD1, NAME_FIELD1), + String.format("Error should indicate that field %s has no value", NAME_FIELD1)); + } + + @Test + void doNotAcceptDuplicateDefinitionsForField() { + Exception exception = assertThrows(DBConfiguration.DBConfigurationException.class, () -> { + TestConfiguration testConfiguration = new TestConfiguration( + DBConfiguration.ConfigurationField.create(NAME_FIELD1, LABEL_FIELD1, TOOLTIP_FIELD1).required(), + DBConfiguration.ConfigurationField.create(NAME_FIELD1, LABEL_FIELD2, TOOLTIP_FIELD2)); + }); + assertTrue(exception.getMessage().startsWith(ERROR_DUPLICATE_DEFINITIONS_FOR_FIELD)); + } + + @Test + void getFields() { + } + + @Test + void printIniFileTemplate() { + } + +} \ No newline at end of file diff --git a/rabbit-core/src/test/java/org/ohdsi/databases/TestSnowflakeConnector.java b/rabbit-core/src/test/java/org/ohdsi/databases/TestSnowflakeConnector.java index 529ed352..c926a7bd 100644 --- a/rabbit-core/src/test/java/org/ohdsi/databases/TestSnowflakeConnector.java +++ b/rabbit-core/src/test/java/org/ohdsi/databases/TestSnowflakeConnector.java @@ -1,32 +1,32 @@ package org.ohdsi.databases; +import org.apache.commons.lang.StringUtils; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; +import org.ohdsi.utilities.files.IniFile; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.sql.Connection; -import java.sql.ResultSet; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; import java.sql.SQLException; -import java.util.List; import static org.junit.jupiter.api.Assertions.*; import static org.ohdsi.databases.SnowflakeConnector.*; -import static org.ohdsi.databases.SnowflakeConnector.ServerConfig.checkSnowflakeConfig; class TestSnowflakeConnector { Logger logger = LoggerFactory.getLogger(TestSnowflakeConnector.class); - @Test - @EnabledIfEnvironmentVariable(named = "SNOWFLAKE_WR_TEST_ACCOUNT", matches = ".+") - public void testConnectToSnowflake() throws SQLException { - DbSettings dbSettings = SnowflakeTestUtils.getTestDbSettingsSnowflake(); - Connection connection = INSTANCE.getInstance(dbSettings).getConnection(); - - assertNotNull(connection); - connection.close(); - logger.info("Connecting to Snowflake OK"); - } +// @Test +// @EnabledIfEnvironmentVariable(named = "SNOWFLAKE_WR_TEST_ACCOUNT", matches = ".+") +// public void testConnectToSnowflake() throws SQLException { +// DbSettings dbSettings = SnowflakeTestUtils.getTestDbSettingsSnowflake(); +// Connection connection = INSTANCE.getInstance(dbSettings).getConnection(); +// +// assertNotNull(connection); +// connection.close(); +// logger.info("Connecting to Snowflake OK"); +// } @Test public void testUninitializedSnowflakeConnection() throws SQLException { @@ -38,90 +38,125 @@ public void testUninitializedSnowflakeConnection() throws SQLException { assertEquals(ERROR_CONNECTION_NOT_INITIALIZED, exception.getMessage()); } - @Test - @EnabledIfEnvironmentVariable(named = "SNOWFLAKE_WR_TEST_ACCOUNT", matches = ".+") - public void testGetFieldNames() { - DbSettings dbSettings = SnowflakeTestUtils.getTestDbSettingsSnowflake(); - Connection connection = INSTANCE.getInstance(dbSettings).getConnection(); - ResultSet resultSet = INSTANCE.getFieldNames("person"); - try { - while (resultSet.next()) { - assertEquals(resultSet.getString("COLUMN_NAME"), "iets"); - assertEquals(resultSet.getString("TYPE_NAME"), "iets"); - } - } catch (SQLException e) { - throw new RuntimeException(e.getMessage()); - } - logger.info("Testing field names for Snowflake OK"); - } +// @Test +// @EnabledIfEnvironmentVariable(named = "SNOWFLAKE_WR_TEST_ACCOUNT", matches = ".+") +// void testGetFieldNames() { +// DbSettings dbSettings = SnowflakeTestUtils.getTestDbSettingsSnowflake(); +// Connection connection = INSTANCE.getInstance(dbSettings).getConnection(); +// ResultSet resultSet = INSTANCE.getFieldNames("person"); +// try { +// while (resultSet.next()) { +// assertEquals(resultSet.getString("COLUMN_NAME"), "iets"); +// assertEquals(resultSet.getString("TYPE_NAME"), "iets"); +// } +// } catch (SQLException e) { +// throw new RuntimeException(e.getMessage()); +// } +// logger.info("Testing field names for Snowflake OK"); +// } @Test - void testCheckSnowflakeConfig() { - List errors; - - // correct case, all required values in the server string - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "someuser", "somepassword", "wh.db.schema"); - assertEquals(0, errors.size()); + void testIniFileAuthenticatorMethod() { + IniFile iniFile = new IniFile(TestSnowflakeConnector.class.getClassLoader().getResource("snowflake.ini").getFile()); - // incorrect case, schema not complete - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "someuser", "somepassword", "first.second"); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_INCORRECT_SCHEMA_SPECIFICATION)); + assertTrue(SnowflakeConnector.INSTANCE.readAndValidate(iniFile)); - // incorrect case, schema not complete - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "someuser", "somepassword", "first"); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_INCORRECT_SCHEMA_SPECIFICATION)); - - // incorrect case, schema not correct (has 4 parts) - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "someuser", "somepassword", "first.second.third.fourth"); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_INCORRECT_SCHEMA_SPECIFICATION)); - - // no password specified at all should result in a warning - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "someuser", "", "first.second.third"); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " password")); - - // incorrect case, user must be specified - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "", "somepassword", "first.second.third"); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " user")); - - // incorrect case, schema must be specified - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "someuser", "somepassword", ""); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " database")); + iniFile.set("SNOWFLAKE_PASSWORD", ""); + Exception exception = assertThrows(RuntimeException.class, () -> { + assertTrue(SnowflakeConnector.INSTANCE.readAndValidate(iniFile)); + }); + assertTrue(exception.getMessage().equals(ERROR_NO_PASSWORD_OR_AUTHENTICATOR)); - // incorrect case, nothing specified, results in multiple errors - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", - "", "", ""); - assertEquals(3, errors.size()); - assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " database")); - assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " user")); - assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " password")); + iniFile.set("SNOWFLAKE_AUTHENTICATOR", "externalbrowser"); + assertTrue(SnowflakeConnector.INSTANCE.readAndValidate(iniFile)); - // incorrect case: invalid server string (space before '/') - errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com /", - "someuser", "somepassword", "wh.db.sch"); - assertEquals(1, errors.size()); - assertTrue(errorsContain(errors, ERROR_INVALID_SERVER_STRING)); + iniFile.set("SNOWFLAKE_PASSWORD", "some-password"); + assertTrue(SnowflakeConnector.INSTANCE.readAndValidate(iniFile)); } - private static boolean errorsContain(List errors, String messageStart) { - for (String error: errors) { - if (error.startsWith(messageStart)) { - return true; + @Test + void testPrintIniFileTemplate() throws IOException { + String output; + try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); PrintStream printStream = new PrintStream(outputStream)) { + DBConfiguration configuration = new SnowflakeConnector.SnowFlakeConfiguration(); + configuration.printIniFileTemplate(printStream); + output = outputStream.toString(); + for (DBConfiguration.ConfigurationField field: configuration.getFields()) { + assertTrue(output.contains(field.name), String.format("ini file template should contain field name (%s)", field.name)); + assertTrue(output.contains(field.toolTip), String.format("ini file template should contain tool tip (%s)", field.toolTip)); + if (!StringUtils.isEmpty(field.getDefaultValue())) { + assertTrue(output.contains(field.getDefaultValue()), String.format("ini file template should contain default value (%s)", field.getDefaultValue())); + } } } - - return false; } +// @Test +// void testCheckSnowflakeConfig() { +// List errors; +// +// // correct case, all required values in the server string +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "someuser", "somepassword", "wh.db.schema"); +// assertEquals(0, errors.size()); +// +// // incorrect case, schema not complete +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "someuser", "somepassword", "first.second"); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_INCORRECT_SCHEMA_SPECIFICATION)); +// +// // incorrect case, schema not complete +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "someuser", "somepassword", "first"); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_INCORRECT_SCHEMA_SPECIFICATION)); +// +// // incorrect case, schema not correct (has 4 parts) +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "someuser", "somepassword", "first.second.third.fourth"); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_INCORRECT_SCHEMA_SPECIFICATION)); +// +// // no password specified at all should result in a warning +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "someuser", "", "first.second.third"); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " password")); +// +// // incorrect case, user must be specified +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "", "somepassword", "first.second.third"); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " user")); +// +// // incorrect case, schema must be specified +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "someuser", "somepassword", ""); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " database")); +// +// // incorrect case, nothing specified, results in multiple errors +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com/", +// "", "", ""); +// assertEquals(3, errors.size()); +// assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " database")); +// assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " user")); +// assertTrue(errorsContain(errors, ERROR_NO_FIELD_OF_TYPE + " password")); +// +// // incorrect case: invalid server string (space before '/') +// errors = checkSnowflakeConfig("someaccount-id.snowflakecomputing.com /", +// "someuser", "somepassword", "wh.db.sch"); +// assertEquals(1, errors.size()); +// assertTrue(errorsContain(errors, ERROR_INVALID_SERVER_STRING)); +// } + +// private static boolean errorsContain(List errors, String messageStart) { +// for (String error: errors) { +// if (error.startsWith(messageStart)) { +// return true; +// } +// } +// +// return false; +// } } \ No newline at end of file diff --git a/rabbit-core/src/test/resources/snowflake.ini b/rabbit-core/src/test/resources/snowflake.ini new file mode 100644 index 00000000..6299dcea --- /dev/null +++ b/rabbit-core/src/test/resources/snowflake.ini @@ -0,0 +1,16 @@ +# Usage: dist/bin/whiteRabbit -ini +WORKING_FOLDER = . +DATA_TYPE = Snowflake +SNOWFLAKE_ACCOUNT = some-account +SNOWFLAKE_USER = some-user +SNOWFLAKE_PASSWORD = some-password +SNOWFLAKE_WAREHOUSE = some-warehouse +SNOWFLAKE_DATABASE = some-database +SNOWFLAKE_SCHEMA = some-schema +TABLES_TO_SCAN = * # Comma-delimited list of table names to scan. Use "*" (asterix) to include all tables in the database +SCAN_FIELD_VALUES = yes # Include the frequency of field values in the scan report? "yes" or "no" +MIN_CELL_COUNT = 5 # Minimum frequency for a field value to be included in the report +MAX_DISTINCT_VALUES = 1000 # Maximum number of distinct values per field to be reported +ROWS_PER_TABLE = 100000 # Maximum number of rows per table to be scanned for field values +CALCULATE_NUMERIC_STATS = no # Include average, standard deviation and quartiles in the scan report? "yes" or "no" +NUMERIC_STATS_SAMPLER_SIZE = 500 # Maximum number of rows used to calculate numeric statistics diff --git a/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/SourceDataScanSnowflakeIT.java b/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/SourceDataScanSnowflakeIT.java index ca821cb0..25c88445 100644 --- a/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/SourceDataScanSnowflakeIT.java +++ b/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/SourceDataScanSnowflakeIT.java @@ -92,7 +92,6 @@ void testProcessSnowflakeFromIni(@TempDir Path tempDir) throws URISyntaxExceptio .replaceAll("%SNOWFLAKE_SCHEMA%", SnowflakeTestUtils.getenvOrFail("SNOWFLAKE_WR_TEST_SCHEMA")); Files.write(iniFile, content.getBytes(charset)); WhiteRabbitMain wrMain = new WhiteRabbitMain(true, new String[]{"-ini", iniFile.toAbsolutePath().toString()}); - System.out.println("Hold it!"); assert referenceScanReport != null; assertTrue(ScanTestUtils.scanResultsSheetMatchesReference(tempDir.resolve("ScanReport.xlsx"), Paths.get(referenceScanReport.toURI()), DbType.SNOWFLAKE)); } diff --git a/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/TestSourceDataScanCsvIniFile.java b/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/TestSourceDataScanCsvIniFile.java index f77b9484..297b87ba 100644 --- a/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/TestSourceDataScanCsvIniFile.java +++ b/whiterabbit/src/test/java/org/ohdsi/whiterabbit/scan/TestSourceDataScanCsvIniFile.java @@ -38,7 +38,6 @@ void testSourceDataScanFromIniFile(@TempDir Path tempDir) throws URISyntaxExcept Files.copy(personCsv, tempDir.resolve("person.csv")); Files.copy(costCsv, tempDir.resolve("cost.csv")); WhiteRabbitMain wrMain = new WhiteRabbitMain(false, new String[]{"-ini", iniFile.toAbsolutePath().toString()}); - System.out.println("Hold it!"); assertNotNull(referenceScanReport); assertTrue(ScanTestUtils.scanResultsSheetMatchesReference(tempDir.resolve("ScanReport.xlsx"), Paths.get(referenceScanReport.toURI()), DbType.DELIMITED_TEXT_FILES)); }