[ 
https://issues.apache.org/jira/browse/GEODE-4160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16313698#comment-16313698
 ] 

ASF GitHub Bot commented on GEODE-4160:
---------------------------------------

dschneider-pivotal closed pull request #1223: GEODE-4160: fix gfsh describe 
jdbc-connection
URL: https://github.com/apache/geode/pull/1223
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilder.java
 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilder.java
index 308f61e7dc..4ab2b7f479 100644
--- 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilder.java
+++ 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilder.java
@@ -28,7 +28,7 @@
   private String url;
   private String user;
   private String password;
-  private Map<String, String> parameters = new HashMap<>();
+  private Map<String, String> parameters;
 
   public ConnectionConfigBuilder withName(String name) {
     this.name = name;
@@ -51,25 +51,26 @@ public ConnectionConfigBuilder withPassword(String 
password) {
   }
 
   public ConnectionConfigBuilder withParameters(String[] params) {
-    if (params == null) {
-      parameters = null;
-    } else {
+    if (params != null) {
+      parameters = new HashMap<>();
       for (String param : params) {
         if (param.isEmpty()) {
           continue;
         }
         String[] keyValuePair = param.split(PARAMS_DELIMITER);
         validateParam(keyValuePair, param);
-        if (keyValuePair.length == 2) {
-          parameters.put(keyValuePair[0], keyValuePair[1]);
-        }
+        parameters.put(keyValuePair[0], keyValuePair[1]);
       }
+    } else {
+      parameters = null;
     }
     return this;
   }
 
   private void validateParam(String[] paramKeyValue, String param) {
-    if ((paramKeyValue.length <= 1) || paramKeyValue[0].isEmpty() || 
paramKeyValue[1].isEmpty()) {
+    // paramKeyValue is produced by split which will never give us
+    // an empty second element
+    if ((paramKeyValue.length != 2) || paramKeyValue[0].isEmpty()) {
       throw new IllegalArgumentException("Parameter '" + param
           + "' is not of the form 'parameterName" + PARAMS_DELIMITER + 
"value'");
     }
diff --git 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfiguration.java
 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfiguration.java
index 6a69079c4f..c80e146096 100644
--- 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfiguration.java
+++ 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfiguration.java
@@ -23,9 +23,6 @@
 
 @Experimental
 public class ConnectionConfiguration implements Serializable {
-  private static final Object USER = "user";
-  private static final Object PASSWORD = "password";
-
   private final String name;
   private final String url;
   private final String user;
diff --git 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilder.java
 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilder.java
index 302f6cdf0f..0d989a48c1 100644
--- 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilder.java
+++ 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilder.java
@@ -50,14 +50,15 @@ public RegionMappingBuilder withConnectionConfigName(String 
connectionConfigName
     return this;
   }
 
-  // TODO: delete withPrimaryKeyInValue(String)
-  public RegionMappingBuilder withPrimaryKeyInValue(String primaryKeyInValue) {
-    this.primaryKeyInValue = Boolean.parseBoolean(primaryKeyInValue);
+  public RegionMappingBuilder withPrimaryKeyInValue(String v) {
+    if (v != null) {
+      withPrimaryKeyInValue(Boolean.parseBoolean(v));
+    }
     return this;
   }
 
-  public RegionMappingBuilder withPrimaryKeyInValue(Boolean primaryKeyInValue) 
{
-    this.primaryKeyInValue = primaryKeyInValue;
+  public RegionMappingBuilder withPrimaryKeyInValue(Boolean v) {
+    this.primaryKeyInValue = v;
     return this;
   }
 
@@ -76,16 +77,16 @@ public RegionMappingBuilder 
withFieldToColumnMappings(String[] mappings) {
         }
         String[] keyValuePair = mapping.split(MAPPINGS_DELIMITER);
         validateParam(keyValuePair, mapping);
-        if (keyValuePair.length == 2) {
-          fieldToColumnMap.put(keyValuePair[0], keyValuePair[1]);
-        }
+        fieldToColumnMap.put(keyValuePair[0], keyValuePair[1]);
       }
     }
     return this;
   }
 
   private void validateParam(String[] paramKeyValue, String mapping) {
-    if (paramKeyValue.length <= 1 || paramKeyValue[0].isEmpty() || 
paramKeyValue[1].isEmpty()) {
+    // paramKeyValue is produced by split which will never give us
+    // an empty second element
+    if (paramKeyValue.length != 2 || paramKeyValue[0].isEmpty()) {
       throw new IllegalArgumentException("Field to column mapping '" + mapping
           + "' is not of the form 'Field" + MAPPINGS_DELIMITER + "Column'");
     }
diff --git 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommand.java
 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommand.java
index 9ecf900dcc..3295340cad 100644
--- 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommand.java
+++ 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommand.java
@@ -103,9 +103,11 @@ private void fillResultData(ConnectionConfiguration 
config, CompositeResultData
     }
     TabularResultData tabularResultData = 
sectionResult.addTable(CREATE_CONNECTION__PARAMS);
     tabularResultData.setHeader("Additional connection parameters:");
-    config.getParameters().entrySet().forEach((entry) -> {
-      tabularResultData.accumulate("Param Name", entry.getKey());
-      tabularResultData.accumulate("Value", entry.getValue());
-    });
+    if (config.getParameters() != null) {
+      config.getParameters().entrySet().forEach((entry) -> {
+        tabularResultData.accumulate("Param Name", entry.getKey());
+        tabularResultData.accumulate("Value", entry.getValue());
+      });
+    }
   }
 }
diff --git 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/ElementType.java
 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/ElementType.java
index eac053abd9..716e15f32a 100644
--- 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/ElementType.java
+++ 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/ElementType.java
@@ -51,21 +51,22 @@ void startElement(Stack<Object> stack, Attributes 
attributes) {
         throw new CacheXmlException(
             "jdbc <connection> elements must occur within <connector-service> 
elements");
       }
-      ConnectionConfigBuilder connectionConfig = new ConnectionConfigBuilder()
+      ConnectionConfigBuilder connectionConfigBuilder = new 
ConnectionConfigBuilder()
           .withName(attributes.getValue(JdbcConnectorServiceXmlParser.NAME))
           .withUrl(attributes.getValue(JdbcConnectorServiceXmlParser.URL))
           .withUser(attributes.getValue(JdbcConnectorServiceXmlParser.USER))
-          
.withPassword(attributes.getValue(JdbcConnectorServiceXmlParser.PASSWORD));
-      addParameters(connectionConfig,
-          attributes.getValue(JdbcConnectorServiceXmlParser.PARAMETERS));
-      stack.push(connectionConfig);
+          
.withPassword(attributes.getValue(JdbcConnectorServiceXmlParser.PASSWORD))
+          .withParameters(parseParameters(attributes));
+      stack.push(connectionConfigBuilder);
     }
 
-    private void addParameters(ConnectionConfigBuilder connectionConfig, 
String value) {
-      if (value == null) {
-        return;
+    private String[] parseParameters(Attributes attributes) {
+      String[] result = null;
+      String value = 
attributes.getValue(JdbcConnectorServiceXmlParser.PARAMETERS);
+      if (value != null) {
+        result = value.split(",");
       }
-      connectionConfig.withParameters(value.split(","));
+      return result;
     }
 
     @Override
diff --git 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGenerator.java
 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGenerator.java
index 8fa59c86a4..70f9540ea4 100644
--- 
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGenerator.java
+++ 
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGenerator.java
@@ -102,9 +102,15 @@ private void outputConnectionConfiguration(ContentHandler 
handler, ConnectionCon
     AttributesImpl attributes = new AttributesImpl();
     XmlGeneratorUtils.addAttribute(attributes, NAME, config.getName());
     XmlGeneratorUtils.addAttribute(attributes, URL, config.getUrl());
-    XmlGeneratorUtils.addAttribute(attributes, USER, config.getUser());
-    XmlGeneratorUtils.addAttribute(attributes, PASSWORD, config.getPassword());
-    XmlGeneratorUtils.addAttribute(attributes, PARAMETERS, 
createParametersString(config));
+    if (config.getUser() != null) {
+      XmlGeneratorUtils.addAttribute(attributes, USER, config.getUser());
+    }
+    if (config.getPassword() != null) {
+      XmlGeneratorUtils.addAttribute(attributes, PASSWORD, 
config.getPassword());
+    }
+    if (config.getParameters() != null) {
+      XmlGeneratorUtils.addAttribute(attributes, PARAMETERS, 
createParametersString(config));
+    }
     XmlGeneratorUtils.emptyElement(handler, PREFIX, 
ElementType.CONNECTION.getTypeName(),
         attributes);
   }
@@ -114,17 +120,23 @@ private void outputRegionMapping(ContentHandler handler, 
RegionMapping mapping)
     AttributesImpl attributes = new AttributesImpl();
     XmlGeneratorUtils.addAttribute(attributes, CONNECTION_NAME, 
mapping.getConnectionConfigName());
     XmlGeneratorUtils.addAttribute(attributes, REGION, 
mapping.getRegionName());
-    XmlGeneratorUtils.addAttribute(attributes, TABLE, mapping.getTableName());
-    XmlGeneratorUtils.addAttribute(attributes, PDX_CLASS, 
mapping.getPdxClassName());
-    XmlGeneratorUtils.addAttribute(attributes, PRIMARY_KEY_IN_VALUE,
-        Boolean.toString(mapping.isPrimaryKeyInValue()));
+    if (mapping.getTableName() != null) {
+      XmlGeneratorUtils.addAttribute(attributes, TABLE, 
mapping.getTableName());
+    }
+    if (mapping.getPdxClassName() != null) {
+      XmlGeneratorUtils.addAttribute(attributes, PDX_CLASS, 
mapping.getPdxClassName());
+    }
+    if (mapping.isPrimaryKeyInValue() != null) {
+      XmlGeneratorUtils.addAttribute(attributes, PRIMARY_KEY_IN_VALUE,
+          Boolean.toString(mapping.isPrimaryKeyInValue()));
+    }
 
-    XmlGeneratorUtils.startElement(handler, PREFIX, 
ElementType.REGION_MAPPING.getTypeName(),
-        attributes);
     if (mapping.getFieldToColumnMap() != null) {
+      XmlGeneratorUtils.startElement(handler, PREFIX, 
ElementType.REGION_MAPPING.getTypeName(),
+          attributes);
       addFieldMappings(handler, mapping.getFieldToColumnMap());
+      XmlGeneratorUtils.endElement(handler, PREFIX, 
ElementType.REGION_MAPPING.getTypeName());
     }
-    XmlGeneratorUtils.endElement(handler, PREFIX, 
ElementType.REGION_MAPPING.getTypeName());
   }
 
   private void addFieldMappings(ContentHandler handler, Map<String, String> 
fieldMappings)
@@ -139,16 +151,13 @@ private void addFieldMappings(ContentHandler handler, 
Map<String, String> fieldM
   }
 
   private String createParametersString(ConnectionConfiguration config) {
-    Properties properties = config.getConnectionProperties();
+    assert config.getParameters() != null;
     StringBuilder stringBuilder = new StringBuilder();
-    for (Map.Entry<Object, Object> entry : properties.entrySet()) {
-      Object key = entry.getKey();
-      if (!key.equals(USER) && !key.equals(PASSWORD)) {
-        if (stringBuilder.length() > 0) {
-          stringBuilder.append(",");
-        }
-        
stringBuilder.append(key).append(PARAMS_DELIMITER).append(entry.getValue());
+    for (Map.Entry<String, String> entry : config.getParameters().entrySet()) {
+      if (stringBuilder.length() > 0) {
+        stringBuilder.append(",");
       }
+      
stringBuilder.append(entry.getKey()).append(PARAMS_DELIMITER).append(entry.getValue());
     }
     return stringBuilder.toString();
   }
diff --git 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java
 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilderTest.java
similarity index 71%
rename from 
geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java
rename to 
geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilderTest.java
index 7afcf833f8..4e4b18ca68 100644
--- 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ConnectionConfigBuilderTest.java
+++ 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/ConnectionConfigBuilderTest.java
@@ -12,7 +12,7 @@
  * or implied. See the License for the specific language governing permissions 
and limitations under
  * the License.
  */
-package org.apache.geode.connectors.jdbc.internal.xml;
+package org.apache.geode.connectors.jdbc.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -51,6 +51,30 @@ public void createsObjectWithCorrectValues() {
         .containsEntry("param2", "value2");
   }
 
+  @Test
+  public void createsObjectWithEmptyParameterElement() {
+    ConnectionConfiguration config = new 
ConnectionConfigBuilder().withName("name").withUrl("url")
+        .withUser("user").withPassword("password")
+        .withParameters(new String[] {"param1:value1", "", 
"param2:value2"}).build();
+
+    assertThat(config.getName()).isEqualTo("name");
+    assertThat(config.getUrl()).isEqualTo("url");
+    assertThat(config.getUser()).isEqualTo("user");
+    assertThat(config.getPassword()).isEqualTo("password");
+    assertThat(config.getConnectionProperties()).containsEntry("param1", 
"value1")
+        .containsEntry("param2", "value2");
+  }
+
+  @Test
+  public void createsObjectWithNullParameters() {
+    ConnectionConfiguration config =
+        new 
ConnectionConfigBuilder().withName("name").withUrl("url").withParameters(null).build();
+
+    assertThat(config.getName()).isEqualTo("name");
+    assertThat(config.getUrl()).isEqualTo("url");
+    assertThat(config.getParameters()).isNull();
+  }
+
   @Test
   public void throwsExceptionWithInvalidParams() {
     ConnectionConfigBuilder config = new ConnectionConfigBuilder();
@@ -60,6 +84,8 @@ public void throwsExceptionWithInvalidParams() {
         .isInstanceOf(IllegalArgumentException.class);
     assertThatThrownBy(() -> config.withParameters(new String[] 
{"param1value1:"}))
         .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> config.withParameters(new String[] {"1:2:3"}))
+        .isInstanceOf(IllegalArgumentException.class);
     assertThatThrownBy(() -> config.withParameters(new String[] {":"}))
         .isInstanceOf(IllegalArgumentException.class);
   }
diff --git 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java
 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilderTest.java
similarity index 69%
rename from 
geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java
rename to 
geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilderTest.java
index d7f31a596a..5abfd74796 100644
--- 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/RegionMappingBuilderTest.java
+++ 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/RegionMappingBuilderTest.java
@@ -12,7 +12,7 @@
  * or implied. See the License for the specific language governing permissions 
and limitations under
  * the License.
  */
-package org.apache.geode.connectors.jdbc.internal.xml;
+package org.apache.geode.connectors.jdbc.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -53,6 +53,26 @@ public void createsMappingWithSpecifiedValues() {
     
assertThat(regionMapping.getColumnNameForField("fieldName")).isEqualTo("columnName");
   }
 
+  @Test
+  public void createsMappingWithNullAsPrimaryKeyInValue() {
+    RegionMapping regionMapping = new 
RegionMappingBuilder().withRegionName("regionName")
+        .withConnectionConfigName("configName").withPrimaryKeyInValue((String) 
null).build();
+
+    assertThat(regionMapping.getRegionName()).isEqualTo("regionName");
+    
assertThat(regionMapping.getConnectionConfigName()).isEqualTo("configName");
+    assertThat(regionMapping.isPrimaryKeyInValue()).isNull();
+  }
+
+  @Test
+  public void createsMappingWithNullFieldToColumnMappings() {
+    RegionMapping regionMapping = new 
RegionMappingBuilder().withRegionName("regionName")
+        
.withConnectionConfigName("configName").withFieldToColumnMappings(null).build();
+
+    assertThat(regionMapping.getRegionName()).isEqualTo("regionName");
+    
assertThat(regionMapping.getConnectionConfigName()).isEqualTo("configName");
+    assertThat(regionMapping.getFieldToColumnMap()).isNull();
+  }
+
   @Test
   public void createsFieldMappingsFromArray() {
     String[] fieldMappings = new String[] {"field1:column1", "field2:column2"};
@@ -63,6 +83,16 @@ public void createsFieldMappingsFromArray() {
     
assertThat(regionMapping.getColumnNameForField("field2")).isEqualTo("column2");
   }
 
+  @Test
+  public void createsFieldMappingsFromArrayWithEmptyElement() {
+    String[] fieldMappings = new String[] {"field1:column1", "", 
"field2:column2"};
+    RegionMapping regionMapping =
+        new 
RegionMappingBuilder().withFieldToColumnMappings(fieldMappings).build();
+
+    
assertThat(regionMapping.getColumnNameForField("field1")).isEqualTo("column1");
+    
assertThat(regionMapping.getColumnNameForField("field2")).isEqualTo("column2");
+  }
+
   @Test
   public void throwsExceptionForInvalidFieldMappings() {
     RegionMappingBuilder regionMappingbuilder = new RegionMappingBuilder();
@@ -73,6 +103,9 @@ public void throwsExceptionForInvalidFieldMappings() {
     assertThatThrownBy(
         () -> regionMappingbuilder.withFieldToColumnMappings(new String[] 
{":field1column1"}))
             .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(
+        () -> regionMappingbuilder.withFieldToColumnMappings(new String[] 
{"field1:column1:extra"}))
+            .isInstanceOf(IllegalArgumentException.class);
     assertThatThrownBy(
         () -> regionMappingbuilder.withFieldToColumnMappings(new String[] 
{"field1column1:"}))
             .isInstanceOf(IllegalArgumentException.class);
diff --git 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommandIntegrationTest.java
 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommandIntegrationTest.java
index 05b5051fec..84e93c371b 100644
--- 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommandIntegrationTest.java
+++ 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeConnectionCommandIntegrationTest.java
@@ -106,6 +106,30 @@ public void 
displaysConnectionInformationWhenConfigurationExists() throws Except
 
   }
 
+  @Test
+  public void 
displaysConnectionInformationForConfigurationWithNullParameters() throws 
Exception {
+    connectionConfig = new 
ConnectionConfigBuilder().withName(CONNECTION).withUrl("myUrl")
+        .withParameters(null).build();
+    service.createConnectionConfig(connectionConfig);
+    Result result = command.describeConnection(CONNECTION);
+
+    assertThat(result.getStatus()).isSameAs(Result.Status.OK);
+    CommandResult commandResult = (CommandResult) result;
+    GfJsonObject sectionContent = commandResult.getTableContent()
+        .getJSONObject(SECTION_DATA_ACCESSOR + "-" + RESULT_SECTION_NAME);
+
+    
assertThat(sectionContent.get(CREATE_CONNECTION__NAME)).isEqualTo(connectionConfig.getName());
+    
assertThat(sectionContent.get(CREATE_CONNECTION__URL)).isEqualTo(connectionConfig.getUrl());
+    
assertThat(sectionContent.get(CREATE_CONNECTION__USER)).isEqualTo(connectionConfig.getUser());
+    
assertThat(sectionContent.get(CREATE_CONNECTION__PASSWORD)).isEqualTo(null);
+
+    GfJsonObject tableContent =
+        sectionContent.getJSONObject(TABLE_DATA_ACCESSOR + "-" + 
CREATE_CONNECTION__PARAMS)
+            .getJSONObject("content");
+    assertThat(tableContent.get("Param Name")).isNull();
+    assertThat(tableContent.get("Value")).isNull();
+  }
+
   @Test
   public void doesNotDisplayParametersWithNoValue() throws Exception {
     connectionConfig = new 
ConnectionConfigBuilder().withName(CONNECTION).withUrl("myUrl").build();
diff --git 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
index 144d1fa1d3..7612527747 100644
--- 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
+++ 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/ElementTypeTest.java
@@ -34,6 +34,8 @@
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
+import java.util.HashMap;
+import java.util.Map;
 import java.util.Stack;
 
 import org.junit.Before;
@@ -136,6 +138,29 @@ public void startElementConnection() {
     assertThat(config.getUrl()).isEqualTo("url");
     assertThat(config.getUser()).isEqualTo("username");
     assertThat(config.getPassword()).isEqualTo("secret");
+    assertThat(config.getParameters()).isNull();
+  }
+
+  @Test
+  public void startElementConnectionWithParameters() {
+    JdbcServiceConfiguration serviceConfiguration = 
mock(JdbcServiceConfiguration.class);
+    stack.push(serviceConfiguration);
+    
when(attributes.getValue(JdbcConnectorServiceXmlParser.NAME)).thenReturn("connectionName");
+    
when(attributes.getValue(JdbcConnectorServiceXmlParser.URL)).thenReturn("url");
+    when(attributes.getValue(JdbcConnectorServiceXmlParser.PARAMETERS))
+        .thenReturn("key1:value1,key2:value2");
+
+    CONNECTION.startElement(stack, attributes);
+
+    ConnectionConfiguration config = ((ConnectionConfigBuilder) 
stack.pop()).build();
+    assertThat(config.getName()).isEqualTo("connectionName");
+    assertThat(config.getUrl()).isEqualTo("url");
+    assertThat(config.getUser()).isNull();
+    assertThat(config.getPassword()).isNull();
+    Map<String, String> expectedParams = new HashMap<>();
+    expectedParams.put("key1", "value1");
+    expectedParams.put("key2", "value2");
+    assertThat(config.getParameters()).isEqualTo(expectedParams);
   }
 
   @Test
diff --git 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
index d59aaafd4b..1ba728f441 100644
--- 
a/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
+++ 
b/geode-connectors/src/test/java/org/apache/geode/connectors/jdbc/internal/xml/JdbcConnectorServiceXmlGeneratorIntegrationTest.java
@@ -174,6 +174,35 @@ public void 
generatedXmlWithConnectionConfigurationCanBeParsed() throws Exceptio
     assertThat(service.getConnectionConfig("name")).isEqualTo(config);
   }
 
+  @Test
+  public void 
generatedXmlWithConnectionConfigurationWithNoUserNameAndPasswordCanBeParsed()
+      throws Exception {
+    JdbcConnectorService service = 
cache.getService(JdbcConnectorService.class);
+    ConnectionConfiguration config =
+        new ConnectionConfigBuilder().withName("name").withUrl("url").build();
+    service.createConnectionConfig(config);
+    generateXml();
+    cache.close();
+
+    createCacheUsingXml();
+    service = cache.getService(JdbcConnectorService.class);
+    assertThat(service.getConnectionConfig("name")).isEqualTo(config);
+  }
+
+  @Test
+  public void 
generatedXmlWithConnectionConfigurationWithParametersCanBeParsed() throws 
Exception {
+    JdbcConnectorService service = 
cache.getService(JdbcConnectorService.class);
+    ConnectionConfiguration config = new 
ConnectionConfigBuilder().withName("name").withUrl("url")
+        .withParameters(new String[] {"key1:value1", "key2:value2"}).build();
+    service.createConnectionConfig(config);
+    generateXml();
+    cache.close();
+
+    createCacheUsingXml();
+    service = cache.getService(JdbcConnectorService.class);
+    assertThat(service.getConnectionConfig("name")).isEqualTo(config);
+  }
+
   @Test
   public void generatedXmlWithRegionMappingCanBeParsed() throws Exception {
     JdbcConnectorService service = 
cache.getService(JdbcConnectorService.class);
@@ -190,6 +219,20 @@ public void generatedXmlWithRegionMappingCanBeParsed() 
throws Exception {
     assertThat(service.getMappingForRegion("region")).isEqualTo(mapping);
   }
 
+  @Test
+  public void 
generatedXmlWithRegionMappingWithNoOptionalParametersCanBeParsed() throws 
Exception {
+    JdbcConnectorService service = 
cache.getService(JdbcConnectorService.class);
+    RegionMapping mapping = new RegionMappingBuilder().withRegionName("region")
+        .withConnectionConfigName("connection").build();
+    service.createRegionMapping(mapping);
+    generateXml();
+    cache.close();
+
+    createCacheUsingXml();
+    service = cache.getService(JdbcConnectorService.class);
+    assertThat(service.getMappingForRegion("region")).isEqualTo(mapping);
+  }
+
   @Test
   public void generatedXmlWithEverythingCanBeParsed() throws Exception {
     JdbcConnectorService service = 
cache.getService(JdbcConnectorService.class);


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> gfsh describe jdbc-connections errors describing a valid connection
> -------------------------------------------------------------------
>
>                 Key: GEODE-4160
>                 URL: https://issues.apache.org/jira/browse/GEODE-4160
>             Project: Geode
>          Issue Type: Bug
>          Components: regions
>    Affects Versions: 1.4.0
>            Reporter: Fred Krone
>            Assignee: Darrel Schneider
>
> Steps to reproduce issue:
> 1) create a jdbc-connection
> 2) describe that jdbc-connection
> 3) you get an error even though the mapping exists (via list mapping, etc)
> gfsh>describe jdbc-mapping --region=employee
> Could not process command due to error. Error while processing command 
> <describe jdbc-mapping --region=employee> Reason : null
> gfsh>create jdbc-connection --name=connection1 
> --url="jdbc:derby://localhost:1527/MyDbTest"
> (Experimental)
> Member | Status
> ------ | -----------------------------------------
> s1 | Created JDBC connection connection1 on s1
> gfsh>describe jdbc-connection --name=connection1
> Could not process command due to error. Error while processing command 
> <describe jdbc-connection --name=connection1> Reason : null
> gfsh>



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to