You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by on...@apache.org on 2019/05/21 00:10:16 UTC
[geode] branch develop updated: GEODE-6182: convert string value into the correct object type based o… (#3608)
This is an automated email from the ASF dual-hosted git repository.
onichols pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 908890e GEODE-6182: convert string value into the correct object type based o… (#3608)
908890e is described below
commit 908890e9941c995855c8eb26867c8326d75c1d2c
Author: Jinmei Liao <ji...@pivotal.io>
AuthorDate: Mon May 20 17:10:04 2019 -0700
GEODE-6182: convert string value into the correct object type based o… (#3608)
* GEODE-6182: convert string value into the correct object type based on the signature
---
.../cli/converters/MemberIdNameConverterTest.java | 62 ++++++++++++++++
.../apache/geode/util/internal/GeodeConverter.java | 64 +++++++++++++++++
.../geode/util/internal/GeodeConverterTest.java | 84 ++++++++++++++++++++++
.../cli/converters/MemberIdNameConverter.java | 6 +-
.../internal/web/shell/HttpOperationInvoker.java | 6 +-
.../web/shell/support/HttpInvocationHandler.java | 4 ++
.../web/controllers/AbstractBaseController.java | 5 +-
.../geode/rest/internal/web/util/NumberUtils.java | 43 -----------
.../web/controllers/ShellCommandsController.java | 6 ++
9 files changed, 229 insertions(+), 51 deletions(-)
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverterTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverterTest.java
new file mode 100644
index 0000000..9d98a6c
--- /dev/null
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverterTest.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.converters;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+public class MemberIdNameConverterTest {
+ @ClassRule
+ public static LocatorStarterRule locator =
+ new LocatorStarterRule().withHttpService().withAutoStart();
+
+ @ClassRule
+ public static GfshCommandRule gfsh = new GfshCommandRule();
+
+ private MemberIdNameConverter converter;
+
+ @Before
+ public void name() throws Exception {
+ converter = spy(MemberIdNameConverter.class);
+ doReturn(gfsh.getGfsh()).when(converter).getGfsh();
+ }
+
+ @Test
+ public void completeMemberWhenConnectedWithJmx() throws Exception {
+ gfsh.connectAndVerify(locator.getJmxPort(), GfshCommandRule.PortType.jmxManager);
+ Set<String> values = converter.getCompletionValues();
+ assertThat(values).hasSize(0);
+ gfsh.disconnect();
+ }
+
+ @Test
+ public void completeMembersWhenConnectedWithHttp() throws Exception {
+ gfsh.connectAndVerify(locator.getHttpPort(), GfshCommandRule.PortType.http);
+ Set<String> values = converter.getCompletionValues();
+ assertThat(values).hasSize(0);
+ gfsh.disconnect();
+ }
+}
diff --git a/geode-common/src/main/java/org/apache/geode/util/internal/GeodeConverter.java b/geode-common/src/main/java/org/apache/geode/util/internal/GeodeConverter.java
new file mode 100644
index 0000000..2dbd724
--- /dev/null
+++ b/geode-common/src/main/java/org/apache/geode/util/internal/GeodeConverter.java
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.util.internal;
+
+public class GeodeConverter {
+ public static Object convertToActualType(String stringValue, String type)
+ throws IllegalArgumentException {
+ if (stringValue != null && type == null) {
+ return stringValue;
+ } else {
+ Object o = null;
+ if (type.startsWith("java.lang."))
+ type = type.substring(10);
+
+ if ("string".equalsIgnoreCase(type))
+ return stringValue;
+
+ if ("char".equalsIgnoreCase(type) || "Character".equals(type))
+ return stringValue.charAt(0);
+
+ try {
+ if ("byte".equalsIgnoreCase(type)) {
+ o = Byte.parseByte(stringValue);
+ return o;
+ } else if ("short".equalsIgnoreCase(type)) {
+ o = Short.parseShort(stringValue);
+ return o;
+ } else if ("int".equalsIgnoreCase(type) || "Integer".equals(type)) {
+ o = Integer.parseInt(stringValue);
+ return o;
+ } else if ("long".equalsIgnoreCase(type)) {
+ o = Long.parseLong(stringValue);
+ return o;
+ } else if ("double".equalsIgnoreCase(type)) {
+ o = Double.parseDouble(stringValue);
+ return o;
+ } else if ("boolean".equalsIgnoreCase(type)) {
+ o = Boolean.parseBoolean(stringValue);
+ return o;
+ } else if ("float".equalsIgnoreCase(type)) {
+ o = Float.parseFloat(stringValue);
+ return o;
+ }
+ return o;
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(
+ "Failed to convert input key to " + type + " Msg : " + e.getMessage());
+ }
+ }
+ }
+}
diff --git a/geode-common/src/test/java/org/apache/geode/util/internal/GeodeConverterTest.java b/geode-common/src/test/java/org/apache/geode/util/internal/GeodeConverterTest.java
new file mode 100644
index 0000000..2154be0
--- /dev/null
+++ b/geode-common/src/test/java/org/apache/geode/util/internal/GeodeConverterTest.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.util.internal;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Test;
+
+public class GeodeConverterTest {
+ @Test
+ public void coercesInt() {
+ assertThat(GeodeConverter.convertToActualType("8", "int")).isEqualTo(8);
+ assertThat(GeodeConverter.convertToActualType("8", Integer.class.getName())).isEqualTo(8);
+ assertThat(GeodeConverter.convertToActualType("-8", "int")).isEqualTo(-8);
+ assertThat(GeodeConverter.convertToActualType("-8", Integer.class.getName())).isEqualTo(-8);
+ }
+
+ @Test
+ public void coercesBoolean() {
+ assertThat(GeodeConverter.convertToActualType("true", "boolean")).isEqualTo(true);
+ assertThat(GeodeConverter.convertToActualType("true", Boolean.class.getName())).isEqualTo(true);
+ assertThat(GeodeConverter.convertToActualType("false", "boolean")).isEqualTo(false);
+ assertThat(GeodeConverter.convertToActualType("false", Boolean.class.getName()))
+ .isEqualTo(false);
+ }
+
+ @Test
+ public void coercesString() {
+ assertThat(GeodeConverter.convertToActualType("foo", "string")).isEqualTo("foo");
+ assertThat(GeodeConverter.convertToActualType("foo", String.class.getName())).isEqualTo("foo");
+ }
+
+ @Test
+ public void coercesChar() {
+ assertThat(GeodeConverter.convertToActualType("C", "char")).isEqualTo('C');
+ assertThat(GeodeConverter.convertToActualType("C", Character.class.getName())).isEqualTo('C');
+ }
+
+ @Test
+ public void coercesByte() {
+ assertThat(GeodeConverter.convertToActualType("5", "byte")).isEqualTo((byte) 5);
+ assertThat(GeodeConverter.convertToActualType("5", Byte.class.getName())).isEqualTo((byte) 5);
+ }
+
+ @Test
+ public void coercesShort() {
+ assertThat(GeodeConverter.convertToActualType("5", "short")).isEqualTo((short) 5);
+ assertThat(GeodeConverter.convertToActualType("5", Short.class.getName())).isEqualTo((short) 5);
+ }
+
+ @Test
+ public void coercesLong() {
+ assertThat(GeodeConverter.convertToActualType("555555555555555555", "long"))
+ .isEqualTo(555555555555555555L);
+ assertThat(GeodeConverter.convertToActualType("5", Long.class.getName())).isEqualTo((long) 5);
+ }
+
+ @Test
+ public void coercesFloat() {
+ assertThat(GeodeConverter.convertToActualType("5.0", "float")).isEqualTo((float) 5);
+ assertThat(GeodeConverter.convertToActualType("5.0", Float.class.getName()))
+ .isEqualTo((float) 5);
+ }
+
+ @Test
+ public void coercesDouble() {
+ assertThat(GeodeConverter.convertToActualType("5.0", "double")).isEqualTo((double) 5);
+ assertThat(GeodeConverter.convertToActualType("5.0", Double.class.getName()))
+ .isEqualTo((double) 5);
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverter.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverter.java
index b53c04a..8cad15d 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverter.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/MemberIdNameConverter.java
@@ -37,7 +37,7 @@ public class MemberIdNameConverter extends BaseStringConverter {
public Set<String> getCompletionValues() {
final Set<String> nonLocatorMembers = new TreeSet<>();
- final Gfsh gfsh = Gfsh.getCurrentInstance();
+ final Gfsh gfsh = getGfsh();
if (gfsh != null && gfsh.isConnectedAndReady()) {
nonLocatorMembers.addAll(
@@ -54,4 +54,8 @@ public class MemberIdNameConverter extends BaseStringConverter {
return nonLocatorMembers;
}
+ Gfsh getGfsh() {
+ return Gfsh.getCurrentInstance();
+ }
+
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
index a925e06..d1f508f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
@@ -52,8 +52,6 @@ import org.apache.geode.management.internal.web.shell.support.HttpMBeanProxyFact
* @see org.apache.geode.management.internal.cli.shell.Gfsh
* @see org.apache.geode.management.internal.cli.shell.OperationInvoker
* @see org.apache.geode.management.internal.web.shell.HttpOperationInvoker
- * @see org.springframework.http.client.SimpleClientHttpRequestFactory
- * @see org.springframework.web.client.RestTemplate
* @since GemFire 8.0
*/
@SuppressWarnings("unused")
@@ -293,8 +291,6 @@ public class HttpOperationInvoker implements OperationInvoker {
*
* @return a proxy instance of the GemFire Manager's DistributedSystem MXBean.
* @see #getMBeanProxy(javax.management.ObjectName, Class)
- * @see org.apache.geode.management.DistributedSystemMXBean
- * @see org.apache.geode.management.internal.MBeanJMXAdapter#getDistributedSystemName()
*/
@Override
public DistributedSystemMXBean getDistributedSystemMXBean() {
@@ -335,7 +331,7 @@ public class HttpOperationInvoker implements OperationInvoker {
final String[] signatures) {
final URI link = HttpRequester.createURI(baseUrl, "/mbean/operation");
- MultiValueMap<String, Object> content = new LinkedMultiValueMap<String, Object>();
+ MultiValueMap<String, Object> content = new LinkedMultiValueMap<>();
content.add("resourceName", resourceName);
content.add("operationName", operationName);
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/support/HttpInvocationHandler.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/support/HttpInvocationHandler.java
index 6500bfc..3129376 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/support/HttpInvocationHandler.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/support/HttpInvocationHandler.java
@@ -80,6 +80,10 @@ public class HttpInvocationHandler implements InvocationHandler {
signature.add(parameterType.getName());
}
+ if (signature.size() == 0) {
+ return null;
+ }
+
return signature.toArray(new String[signature.size()]);
}
diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
index 340d6b0..9d71542 100644
--- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
+++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/AbstractBaseController.java
@@ -82,6 +82,7 @@ import org.apache.geode.rest.internal.web.util.IdentifiableUtils;
import org.apache.geode.rest.internal.web.util.JSONUtils;
import org.apache.geode.rest.internal.web.util.NumberUtils;
import org.apache.geode.rest.internal.web.util.ValidationUtils;
+import org.apache.geode.util.internal.GeodeConverter;
/**
* AbstractBaseController class contains common functionalities required for other controllers.
@@ -546,7 +547,7 @@ public abstract class AbstractBaseController implements InitializingBean {
Object actualValue = value;
if (valueType != null) {
try {
- actualValue = NumberUtils.convertToActualType(value, valueType);
+ actualValue = GeodeConverter.convertToActualType(value, valueType);
} catch (IllegalArgumentException ie) {
throw new GemfireRestException(ie.getMessage(), ie);
}
@@ -705,7 +706,7 @@ public abstract class AbstractBaseController implements InitializingBean {
if (NumberUtils.isPrimitiveOrObject(typeValue)) {
final Object primitiveValue = rawDataBinding.get("@value");
try {
- return (T) NumberUtils.convertToActualType(primitiveValue.toString(), typeValue);
+ return (T) GeodeConverter.convertToActualType(primitiveValue.toString(), typeValue);
} catch (IllegalArgumentException e) {
throw new GemfireRestException(
"Server has encountered error (illegal or inappropriate arguments).", e);
diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/NumberUtils.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/NumberUtils.java
index 0750c36..72b4d41 100644
--- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/NumberUtils.java
+++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/NumberUtils.java
@@ -81,47 +81,4 @@ public abstract class NumberUtils {
|| Boolean.class.equals(ClassUtils.getClass(value))
|| String.class.equals(ClassUtils.getClass(value));
}
-
- @SuppressWarnings({"rawtypes", "unchecked"})
- public static Object convertToActualType(String stringValue, String type)
- throws IllegalArgumentException {
- if (stringValue != null && type == null) {
- return stringValue;
- } else {
- Object o = null;
-
- if ("string".equalsIgnoreCase(type))
- return stringValue;
-
- try {
- if ("byte".equalsIgnoreCase(type)) {
- o = Byte.parseByte(stringValue);
- return o;
- } else if ("short".equalsIgnoreCase(type)) {
- o = Short.parseShort(stringValue);
- return o;
- } else if ("int".equalsIgnoreCase(type)) {
- o = Integer.parseInt(stringValue);
- return o;
- } else if ("long".equalsIgnoreCase(type)) {
- o = Long.parseLong(stringValue);
- return o;
- } else if ("double".equalsIgnoreCase(type)) {
- o = Double.parseDouble(stringValue);
- return o;
- } else if ("boolean".equalsIgnoreCase(type)) {
- o = Boolean.parseBoolean(stringValue);
- return o;
- } else if ("float".equalsIgnoreCase(type)) {
- o = Float.parseFloat(stringValue);
- return o;
- }
- return o;
- } catch (NumberFormatException e) {
- throw new IllegalArgumentException(
- "Failed to convert input key to " + type + " Msg : " + e.getMessage());
- }
-
- }
- }
}
diff --git a/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java b/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
index 55dd04e..9d8f3e4 100644
--- a/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
+++ b/geode-web/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
@@ -55,6 +55,7 @@ import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.result.model.ResultModel;
import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
import org.apache.geode.management.internal.web.domain.QueryParameterSource;
+import org.apache.geode.util.internal.GeodeConverter;
/**
* The ShellCommandsController class implements GemFire REST API calls for Gfsh Shell Commands.
@@ -148,6 +149,11 @@ public class ShellCommandsController extends AbstractCommandsController {
parameters = (parameters != null ? parameters : ArrayUtils.EMPTY_OBJECT_ARRAY);
MBeanServer mBeanServer = getMBeanServer();
ObjectName objectName = ObjectName.getInstance(decode(resourceName));
+ // GEODE-6182
+ for (int i = 0; i < signature.length; i++) {
+ parameters[i] = GeodeConverter.convertToActualType(parameters[i].toString(), signature[i]);
+ }
+
final Object result =
mBeanServer.invoke(objectName, decode(operationName), parameters, signature);
byte[] serializedResult = IOUtils.serializeObject(result);