geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject [geode] branch feature/GEODE-5951 updated: wip: the pool-properties are now fully implemented added validation to ensure that the pool related properties are only allowed if --pooled.
Date Wed, 31 Oct 2018 00:32:36 GMT
This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch feature/GEODE-5951
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/feature/GEODE-5951 by this push:
     new b7a405b  wip: the pool-properties are now fully implemented added validation to ensure
that the pool related properties are only allowed if --pooled.
b7a405b is described below

commit b7a405bc3a9c64a1daafa83ce5716bbf6f08561c
Author: Darrel Schneider <dschneider@pivotal.io>
AuthorDate: Tue Oct 30 17:31:00 2018 -0700

    wip: the pool-properties are now fully implemented
    added validation to ensure that the pool related properties are only allowed if --pooled.
---
 .../commands/CreateDataSourceCommandDUnitTest.java |  2 +-
 .../cli/commands/CreateDataSourceCommand.java      | 32 ++++++++++++++--------
 .../cli/commands/UsernamePasswordInterceptor.java  | 18 ++++++++++++
 .../management/internal/cli/i18n/CliStrings.java   |  6 ++++
 4 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommandDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommandDUnitTest.java
index 0dcc960..9aa8018 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommandDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommandDUnitTest.java
@@ -63,7 +63,7 @@ public class CreateDataSourceCommandDUnitTest {
 
     // create the data-source
     gfsh.executeAndAssertThat(
-        "create data-source --name=jndi1 --username=myuser --password=mypass --pooled=false
--url=\"jdbc:derby:newDB;create=true\" --properties={'name':'name1','value':'value1'},{'name':'name2','value':'value2'}")
+        "create data-source --name=jndi1 --username=myuser --password=mypass --pooled=false
--url=\"jdbc:derby:newDB;create=true\" --pool-properties={'name':'name1','value':'value1'},{'name':'name2','value':'value2'}")
         .statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1", "server-2");
 
     // verify cluster config is updated
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommand.java
index afe48ef..351778a 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateDataSourceCommand.java
@@ -24,6 +24,7 @@ import org.springframework.shell.core.annotation.CliOption;
 import org.apache.geode.cache.configuration.CacheConfig;
 import org.apache.geode.cache.configuration.CacheElement;
 import org.apache.geode.cache.configuration.JndiBindingsType;
+import org.apache.geode.cache.configuration.JndiBindingsType.JndiBinding.ConfigProperty;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
 import org.apache.geode.internal.logging.LogService;
@@ -46,7 +47,9 @@ public class CreateDataSourceCommand extends SingleGfshCommand {
   static final String POOLED_DATA_SOURCE_FACTORY_CLASS = "pooled-data-source-factory-class";
   static final String POOLED_DATA_SOURCE_FACTORY_CLASS__HELP =
       "This class will be created, by calling its no-arg constructor, and used as the pool
of this data source. "
-          + " The class must implement org.apache.geode.datasource.PooledDataSourceFactory.";
+          + " The class must implement org.apache.geode.datasource.PooledDataSourceFactory.
Only valid if --pooled.";
+  private static final String DEFAULT_POOLED_DATA_SOURCE_FACTORY_CLASS =
+      "org.apache.geode.connectors.jdbc.JdbcPooledDataSourceFactory";
   static final String URL = "url";
   static final String URL__HELP =
       "This is the JDBC data source URL string, for example, jdbc:hsqldb:hsql://localhost:1701.";
@@ -63,10 +66,10 @@ public class CreateDataSourceCommand extends SingleGfshCommand {
       "By default a pooled data source is created. If this option is false then a non-pooled
data source is created.";
   static final String IFNOTEXISTS__HELP =
       "Skip the create operation when a data source with the same name already exists.  Without
specifying this option, this command execution results into an error.";
-  static final String PROPERTIES = "properties";
-  static final String PROPERTIES_HELP =
-      "Properties for the data source. When used with a pooled data source, these properties
will be used to configure the database data source unless the name begins with \"pool.\"."
-          + " If that prefix is used it will be used to configure the pool data source. For
non-pooled data sources, these properties are just passed to the database data source. "
+  static final String POOL_PROPERTIES = "pool-properties";
+  static final String POOL_PROPERTIES_HELP =
+      "Properties for a pooled data source. Only valid if --pooled. These properties will
be used to configure the database data source unless the name begins with \"pool.\"."
+          + " If that prefix is used it will be used to configure the pool data source."
           + "The value is a comma separated list of json strings. Each json string contains
a name and value. "
           + "For example: --properties={'name':'name1','value':'value1'},{'name':'name2','value':'value2'}";
 
@@ -77,7 +80,8 @@ public class CreateDataSourceCommand extends SingleGfshCommand {
       operation = ResourcePermission.Operation.MANAGE)
   public ResultModel createJDNIBinding(
       @CliOption(key = POOLED_DATA_SOURCE_FACTORY_CLASS,
-          help = POOLED_DATA_SOURCE_FACTORY_CLASS__HELP) String connectionPooledDatasource,
+          help = POOLED_DATA_SOURCE_FACTORY_CLASS__HELP,
+          unspecifiedDefaultValue = DEFAULT_POOLED_DATA_SOURCE_FACTORY_CLASS) String pooledDataSourceFactoryClass,
       @CliOption(key = URL, mandatory = true,
           help = URL__HELP) String url,
       @CliOption(key = NAME, mandatory = true, help = NAME__HELP) String jndiName,
@@ -87,12 +91,12 @@ public class CreateDataSourceCommand extends SingleGfshCommand {
           specifiedDefaultValue = "true", unspecifiedDefaultValue = "false") boolean ifNotExists,
       @CliOption(key = POOLED, help = POOLED__HELP,
           specifiedDefaultValue = "true", unspecifiedDefaultValue = "true") boolean pooled,
-      @CliOption(key = PROPERTIES, optionContext = "splittingRegex=,(?![^{]*\\})",
-          help = PROPERTIES_HELP) DataSourceProperty[] properties) {
+      @CliOption(key = POOL_PROPERTIES, optionContext = "splittingRegex=,(?![^{]*\\})",
+          help = POOL_PROPERTIES_HELP) DataSourceProperty[] poolProperties) {
 
     JndiBindingsType.JndiBinding configuration = new JndiBindingsType.JndiBinding();
     configuration.setCreatedByDataSourceCommand(true);
-    configuration.setConnPooledDatasourceClass(connectionPooledDatasource);
+    configuration.setConnPooledDatasourceClass(pooledDataSourceFactoryClass);
     configuration.setConnectionUrl(url);
     configuration.setJndiName(jndiName);
     configuration.setPassword(password);
@@ -102,9 +106,13 @@ public class CreateDataSourceCommand extends SingleGfshCommand {
       configuration.setType(DATASOURCE_TYPE.SIMPLE.getType());
     }
     configuration.setUserName(username);
-    if (properties != null && properties.length > 0) {
-      // TODO convert DataSourceProperty to ConfigProperty
-      configuration.getConfigProperties().addAll(Arrays.asList(properties));
+    if (poolProperties != null && poolProperties.length > 0) {
+      List<ConfigProperty> configProperties = configuration.getConfigProperties();
+      for (DataSourceProperty dataSourceProperty : poolProperties) {
+        String name = dataSourceProperty.getName();
+        String value = dataSourceProperty.getValue();
+        configProperties.add(new ConfigProperty(name, "type", value));
+      }
     }
 
     InternalConfigurationPersistenceService service =
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UsernamePasswordInterceptor.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UsernamePasswordInterceptor.java
index 0aec277..9e59103 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UsernamePasswordInterceptor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/UsernamePasswordInterceptor.java
@@ -30,6 +30,24 @@ public class UsernamePasswordInterceptor extends AbstractCliAroundInterceptor
{
       return new ResultModel();
     }
 
+    if (CreateDataSourceCommand.CREATE_DATA_SOURCE.equals(parseResult.getCommandName()))
{
+      String pooled = parseResult.getParamValueAsString("pooled");
+      if (pooled != null && pooled.equalsIgnoreCase("false")) {
+        String poolProperties =
+            parseResult.getParamValueAsString(CreateDataSourceCommand.POOL_PROPERTIES);
+        if (poolProperties != null && poolProperties.length() > 0) {
+          return ResultModel
+              .createError(CliStrings.POOL_PROPERTIES_ONLY_VALID_ON_POOLED_DATA_SOURCE);
+        }
+        String pooledDataSourceFactoryClass = parseResult
+            .getParamValueAsString(CreateDataSourceCommand.POOLED_DATA_SOURCE_FACTORY_CLASS);
+        if (pooledDataSourceFactoryClass != null && pooledDataSourceFactoryClass.length()
> 0) {
+          return ResultModel.createError(
+              CliStrings.POOLED_DATA_SOURCE_FACTORY_CLASS_ONLY_VALID_ON_POOLED_DATA_SOURCE);
+        }
+      }
+    }
+
     String userInput = parseResult.getUserInput();
 
     String username = parseResult.getParamValueAsString("username");
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index 7c40805..f5e4726 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -3185,6 +3185,12 @@ public class CliStrings {
   public static final String START_SERVER__REDIRECT_OUTPUT__HELP =
       "Causes the member to redirect standard out and standard error to its own log file.";
 
+
+  public static final String POOL_PROPERTIES_ONLY_VALID_ON_POOLED_DATA_SOURCE =
+      "The --pool-properties option is only valid on --pooled data sources.";
+  public static final String POOLED_DATA_SOURCE_FACTORY_CLASS_ONLY_VALID_ON_POOLED_DATA_SOURCE
=
+      "The --pool-properties option is only valid on --pooled data sources.";
+
   /**
    * Creates a MessageFormat with the given pattern and uses it to format the given argument.
    *


Mime
View raw message