This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.12 by this push:
new 1985b51 GEODE-8489: "Select *" query by DistributedSystemMBean should
not hon… (#5507)
1985b51 is described below
commit 1985b510e077cef9d1c3c5677c45dcd505b211cc
Author: Jinmei Liao <[email protected]>
AuthorDate: Mon Sep 21 07:53:38 2020 -0700
GEODE-8489: "Select *" query by DistributedSystemMBean should not hon…
(#5507)
* null value fields are serialized as well
* move QueryResultFormatterTest to core
---
.../DistributedSystemMBeanIntegrationTest.java | 117 ++++++++++-----------
.../internal/json/AbstractJSONFormatter.java | 1 +
.../internal/json/QueryResultFormatter.java | 4 +
.../internal}/json/QueryResultFormatterTest.java | 29 ++++-
.../apache/geode/management/model/Employee.java | 94 +++++++++++++++++
5 files changed, 181 insertions(+), 64 deletions(-)
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/beans/DistributedSystemMBeanIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/beans/DistributedSystemMBeanIntegrationTest.java
index 8b47b0a..2c662f0 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/beans/DistributedSystemMBeanIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/beans/DistributedSystemMBeanIntegrationTest.java
@@ -21,6 +21,7 @@ import java.text.SimpleDateFormat;
import java.time.LocalDate;
import java.util.Date;
+import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
@@ -30,6 +31,7 @@ import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionShortcut;
import org.apache.geode.management.DistributedSystemMXBean;
import org.apache.geode.management.internal.json.QueryResultFormatter;
+import org.apache.geode.management.model.Employee;
import org.apache.geode.test.junit.assertions.TabularResultModelAssert;
import org.apache.geode.test.junit.rules.GfshCommandRule;
import org.apache.geode.test.junit.rules.MBeanServerConnectionRule;
@@ -37,7 +39,8 @@ import org.apache.geode.test.junit.rules.ServerStarterRule;
public class DistributedSystemMBeanIntegrationTest {
- public static final String SELECT = "select * from /testRegion r where
r.id=1";
+ public static final String SELECT_ALL = "select * from /testRegion r where
r.id=1";
+ public static final String SELECT_FIELDS = "select id, title from
/testRegion r where r.id=1";
@ClassRule
public static ServerStarterRule server = new ServerStarterRule()
@@ -53,6 +56,8 @@ public class DistributedSystemMBeanIntegrationTest {
@Rule
public GfshCommandRule gfsh = new GfshCommandRule();
+ private DistributedSystemMXBean bean;
+
private static Date date;
private static java.sql.Date sqlDate;
private static LocalDate localDate;
@@ -63,87 +68,81 @@ public class DistributedSystemMBeanIntegrationTest {
localDate = LocalDate.of(2020, 1, 1);
sqlDate = java.sql.Date.valueOf(localDate);
date = new Date(sqlDate.getTime());
- Data data = new Data(1, date, sqlDate, localDate);
- testRegion.put(1, data);
+ Employee employee = new Employee();
+ employee.setId(1);
+ employee.setName("John");
+ employee.setTitle("Manager");
+ employee.setStartDate(date);
+ employee.setEndDate(sqlDate);
+ employee.setBirthday(localDate);
+ testRegion.put(1, employee);
}
- // this is to make sure dates are formatted correctly
- @Test
- public void queryUsingMBean() throws Exception {
+ @Before
+ public void setup() throws Exception {
connectionRule.connect(server.getJmxPort());
+ bean = connectionRule.getProxyMXBean(DistributedSystemMXBean.class);
+ }
+
+ // this is to make sure dates are formatted correctly and it does not honor
the json annotations
+ @Test
+ public void queryAllUsingMBean() throws Exception {
SimpleDateFormat formater =
new SimpleDateFormat(QueryResultFormatter.DATE_FORMAT_PATTERN);
String dateString = formater.format(date);
- DistributedSystemMXBean bean =
connectionRule.getProxyMXBean(DistributedSystemMXBean.class);
- String result = bean.queryData(SELECT, "server", 100);
+ String result = bean.queryData(SELECT_ALL, "server", 100);
System.out.println(result);
assertThat(result)
+ .contains("id")
+ .contains("title")
+ .contains("address")
+ .doesNotContain("Job Title")
.contains("\"java.util.Date\",\"" + dateString + "\"")
.contains("\"java.sql.Date\",\"" + dateString + "\"")
.contains("\"java.time.LocalDate\",\"2020-01-01\"");
}
+ @Test
+ public void queryFieldsUsingMbeanDoesNotHonorAnnotations() throws Exception {
+ String result = bean.queryData(SELECT_FIELDS, "server", 100);
+ System.out.println(result);
+ assertThat(result).contains("id").contains("title");
+ }
+
// this is simply to document the current behavior of gfsh
- // gfsh doesn't attempt tp format the date objects as of now
+ // gfsh doesn't attempt to format the date objects as of now, and it respect
the json annotations
+ // when listing out the headers
@Test
- public void queryUsingGfsh() throws Exception {
+ public void queryAllUsingGfshDoesNotFormatDate() throws Exception {
gfsh.connectAndVerify(server.getJmxPort(),
GfshCommandRule.PortType.jmxManager);
TabularResultModelAssert tabularAssert =
- gfsh.executeAndAssertThat("query --query='" + SELECT + "'")
+ gfsh.executeAndAssertThat("query --query='" + SELECT_ALL + "'")
.statusIsSuccess()
.hasTableSection();
- tabularAssert.hasColumn("id").containsExactly("1");
- tabularAssert.hasColumn("date").containsExactly(date.getTime() + "");
- tabularAssert.hasColumn("sqlDate").containsExactly(sqlDate.getTime() + "");
- tabularAssert.hasColumn("localDate")
+ // note gfsh doesn't show id field and shows "title" field as "Job Title"
when doing select *
+ tabularAssert.hasColumns().asList().containsExactlyInAnyOrder("name",
"address", "startDate",
+ "endDate", "birthday", "Job Title");
+ tabularAssert.hasColumn("startDate").containsExactly(date.getTime() + "");
+ tabularAssert.hasColumn("endDate").containsExactly(sqlDate.getTime() + "");
+ tabularAssert.hasColumn("birthday")
.asList().asString().contains("\"year\":2020,\"month\":\"JANUARY\"");
}
- public static class Data {
- private int id;
- private Date date;
- private java.sql.Date sqlDate;
- private LocalDate localDate;
-
- public Data() {}
-
- public Data(int id, Date date, java.sql.Date sqlDate, LocalDate localDate)
{
- this.id = id;
- this.date = date;
- this.sqlDate = sqlDate;
- this.localDate = localDate;
- }
-
- public int getId() {
- return id;
- }
-
- public void setId(int id) {
- this.id = id;
- }
-
- public Date getDate() {
- return date;
- }
-
- public void setDate(Date date) {
- this.date = date;
- }
-
- public java.sql.Date getSqlDate() {
- return sqlDate;
- }
-
- public void setSqlDate(java.sql.Date sqlDate) {
- this.sqlDate = sqlDate;
- }
+ @Test
+ public void queryFieldsUsingGfshDoesNotHonorAnnotations() throws Exception {
+ gfsh.connectAndVerify(server.getJmxPort(),
GfshCommandRule.PortType.jmxManager);
+ gfsh.executeAndAssertThat("query --query='" + SELECT_FIELDS + "'")
+ .statusIsSuccess()
+ .hasTableSection().hasColumns().asList()
+ .containsExactlyInAnyOrder("id", "title");
+ }
- public LocalDate getLocalDate() {
- return localDate;
- }
+ @Test
+ public void documentZeroResultsBehavior() throws Exception {
+ String result = bean.queryData("select * from /testRegion r where r.id=0",
"server", 100);
+ assertThat(result).isEqualTo("{\"result\":[{\"message\":\"No Data
Found\"}]}");
- public void setLocalDate(LocalDate localDate) {
- this.localDate = localDate;
- }
+ result = bean.queryData("select id, title from /testRegion r where
r.id=0", "server", 100);
+ assertThat(result).isEqualTo("{\"result\":[{\"message\":\"No Data
Found\"}]}");
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/json/AbstractJSONFormatter.java
b/geode-core/src/main/java/org/apache/geode/management/internal/json/AbstractJSONFormatter.java
index f91f062..cdc5a40 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/json/AbstractJSONFormatter.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/json/AbstractJSONFormatter.java
@@ -97,6 +97,7 @@ public abstract class AbstractJSONFormatter {
// to support jdk8 java.time if jackson-datatype-jsr310 is included in the
classpath
mapper.findAndRegisterModules();
+ mapper.disable(MapperFeature.USE_ANNOTATIONS);
// allow objects with no content
mapper.disable(SerializationFeature.FAIL_ON_EMPTY_BEANS);
// use toString on Enums
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/json/QueryResultFormatter.java
b/geode-core/src/main/java/org/apache/geode/management/internal/json/QueryResultFormatter.java
index 3cc63ac..bbd5523 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/json/QueryResultFormatter.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/json/QueryResultFormatter.java
@@ -38,6 +38,7 @@ import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.ser.BeanSerializerModifier;
+import com.fasterxml.jackson.databind.ser.ResolvableSerializer;
import com.fasterxml.jackson.databind.type.ArrayType;
public class QueryResultFormatter extends AbstractJSONFormatter {
@@ -216,6 +217,9 @@ public class QueryResultFormatter extends
AbstractJSONFormatter {
} else {
gen.writeStartArray();
gen.writeString(value.getClass().getCanonicalName());
+ if (defaultSerializer instanceof ResolvableSerializer) {
+ ((ResolvableSerializer) defaultSerializer).resolve(serializers);
+ }
defaultSerializer.serialize(value, gen, serializers);
gen.writeEndArray();
}
diff --git
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/json/QueryResultFormatterTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/json/QueryResultFormatterTest.java
similarity index 92%
rename from
geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/json/QueryResultFormatterTest.java
rename to
geode-core/src/test/java/org/apache/geode/management/internal/json/QueryResultFormatterTest.java
index 256e4d9..1defccb 100644
---
a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/json/QueryResultFormatterTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/json/QueryResultFormatterTest.java
@@ -12,9 +12,10 @@
* or implied. See the License for the specific language governing permissions
and limitations under
* the License.
*/
-package org.apache.geode.management.internal.cli.json;
+package org.apache.geode.management.internal.json;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.InstanceOfAssertFactories.map;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -33,7 +34,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.Test;
import org.apache.geode.cache.query.data.CollectionHolder;
-import org.apache.geode.management.internal.json.QueryResultFormatter;
+import org.apache.geode.management.model.Employee;
import org.apache.geode.management.model.Item;
import org.apache.geode.management.model.Order;
import org.apache.geode.management.model.SubOrder;
@@ -257,7 +258,7 @@ public class QueryResultFormatterTest {
QueryResultFormatter queryResultFormatter =
new QueryResultFormatter(100).add(RESULT, Currency.DIME);
checkResult(queryResultFormatter,
-
"{\"result\":[[\"org.apache.geode.management.internal.cli.json.QueryResultFormatterTest.Currency\",\"DIME\"]]}");
+
"{\"result\":[[\"org.apache.geode.management.internal.json.QueryResultFormatterTest.Currency\",\"DIME\"]]}");
}
@Test
@@ -270,7 +271,7 @@ public class QueryResultFormatterTest {
QueryResultFormatter queryResultFormatter = new
QueryResultFormatter(100).add(RESULT, list);
checkResult(queryResultFormatter,
-
"{\"result\":[[\"java.util.ArrayList\",{\"0\":[\"org.apache.geode.management.internal.cli.json.QueryResultFormatterTest.Currency\",\"DIME\"],\"1\":[\"org.apache.geode.management.internal.cli.json.QueryResultFormatterTest.Currency\",\"NICKLE\"],\"2\":[\"org.apache.geode.management.internal.cli.json.QueryResultFormatterTest.Currency\",\"QUARTER\"],\"3\":[\"org.apache.geode.management.internal.cli.json.QueryResultFormatterTest.Currency\",\"NICKLE\"]}]]}");
+
"{\"result\":[[\"java.util.ArrayList\",{\"0\":[\"org.apache.geode.management.internal.json.QueryResultFormatterTest.Currency\",\"DIME\"],\"1\":[\"org.apache.geode.management.internal.json.QueryResultFormatterTest.Currency\",\"NICKLE\"],\"2\":[\"org.apache.geode.management.internal.json.QueryResultFormatterTest.Currency\",\"QUARTER\"],\"3\":[\"org.apache.geode.management.internal.json.QueryResultFormatterTest.Currency\",\"NICKLE\"]}]]}");
}
@Test
@@ -279,7 +280,25 @@ public class QueryResultFormatterTest {
QueryResultFormatter queryResultFormatter =
new QueryResultFormatter(100).add(RESULT, enumContainer);
checkResult(queryResultFormatter,
-
"{\"result\":[[\"org.apache.geode.management.internal.cli.json.QueryResultFormatterTest.EnumContainer\",{}]]}");
+
"{\"result\":[[\"org.apache.geode.management.internal.json.QueryResultFormatterTest.EnumContainer\",{}]]}");
+ }
+
+ @Test
+ public void testObjectWithJsonAnnotation() throws Exception {
+ Employee employee = new Employee();
+ employee.setId(10);
+ employee.setName("John");
+ employee.setTitle("Manager");
+ QueryResultFormatter queryResultFormatter = new QueryResultFormatter(100);
+ queryResultFormatter.add("result", employee);
+ // these to make sure we are keeping the pre 1.8 behavior
+ assertThat(queryResultFormatter.toString())
+ // make sure null values are serialized as well
+ .contains("\"address\":null")
+ // make sure we don't honor @JsonIgnore annotation
+ .contains("id")
+ // make sure we don't honor @JsonProperty annotation
+ .doesNotContain("Job Title");
}
private enum Currency {
diff --git
a/geode-junit/src/main/java/org/apache/geode/management/model/Employee.java
b/geode-junit/src/main/java/org/apache/geode/management/model/Employee.java
new file mode 100644
index 0000000..e7cf063
--- /dev/null
+++ b/geode-junit/src/main/java/org/apache/geode/management/model/Employee.java
@@ -0,0 +1,94 @@
+/*
+ * 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.model;
+
+import java.time.LocalDate;
+import java.util.Date;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+/**
+ * a domain object that has various date objects and json annotations.
+ * Used to test QueryResultsFormatter
+ */
+public class Employee {
+ private int id;
+ private String name;
+ private String title;
+ private String address;
+ private Date startDate;
+ private java.sql.Date endDate;
+ private LocalDate birthday;
+
+ @JsonIgnore
+ public int getId() {
+ return id;
+ }
+
+ public void setId(int id) {
+ this.id = id;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ @JsonProperty("Job Title")
+ public String getTitle() {
+ return title;
+ }
+
+ public void setTitle(String title) {
+ this.title = title;
+ }
+
+ public String getAddress() {
+ return address;
+ }
+
+ public void setAddress(String address) {
+ this.address = address;
+ }
+
+ public Date getStartDate() {
+ return startDate;
+ }
+
+ public void setStartDate(Date startDate) {
+ this.startDate = startDate;
+ }
+
+ public java.sql.Date getEndDate() {
+ return endDate;
+ }
+
+ public void setEndDate(java.sql.Date endDate) {
+ this.endDate = endDate;
+ }
+
+ public LocalDate getBirthday() {
+ return birthday;
+ }
+
+ public void setBirthday(LocalDate birthday) {
+ this.birthday = birthday;
+ }
+}
