Skip to content

Commit 6f1715f

Browse files
snuyanzinwuchong
snuyanzin
authored and
wuchong
committed
[client] Add support for inner class and is/has methods for boolean (#1908)
* [client] Add support for inner class and is/has methods for boolean * fix (cherry picked from commit 7af5c0c)
1 parent 699bc1d commit 6f1715f

File tree

2 files changed

+244
-29
lines changed

2 files changed

+244
-29
lines changed

‎fluss-client/src/main/java/org/apache/fluss/client/converter/PojoType.java‎

Lines changed: 81 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ static <T> PojoType<T> of(Class<T> pojoClass) {
6666
Constructor<T> ctor = requirePublicDefaultConstructor(pojoClass);
6767

6868
Map<String, Field> allFields = discoverAllInstanceFields(pojoClass);
69-
Map<String, Method> getters = discoverGetters(pojoClass);
70-
Map<String, Method> setters = discoverSetters(pojoClass);
69+
Map<String, Method> getters = discoverGetters(pojoClass, allFields);
70+
Map<String, Method> setters = discoverSetters(pojoClass, allFields);
7171

7272
Map<String, Property> props = new LinkedHashMap<>();
7373
for (Map.Entry<String, Field> e : allFields.entrySet()) {
@@ -85,10 +85,11 @@ static <T> PojoType<T> of(Class<T> pojoClass) {
8585
if (!publicField) {
8686
// When not a public field, require both getter and setter
8787
if (getter == null || setter == null) {
88+
final String capitalizedName = capitalize(name);
8889
throw new IllegalArgumentException(
8990
String.format(
9091
"POJO class %s field '%s' must be public or have both getter and setter (get%s/set%s).",
91-
pojoClass.getName(), name, capitalize(name), capitalize(name)));
92+
pojoClass.getName(), name, capitalizedName, capitalizedName));
9293
}
9394
}
9495
props.put(
@@ -108,22 +109,30 @@ private static <T> void validatePublicClass(Class<T> pojoClass) {
108109
}
109110

110111
private static <T> Constructor<T> requirePublicDefaultConstructor(Class<T> pojoClass) {
111-
try {
112-
Constructor<T> ctor = pojoClass.getDeclaredConstructor();
113-
if (!Modifier.isPublic(ctor.getModifiers())) {
114-
throw new IllegalArgumentException(
115-
String.format(
116-
"POJO class %s must have a public default constructor.",
117-
pojoClass.getName()));
112+
Constructor<T>[] ctors = (Constructor<T>[]) pojoClass.getConstructors();
113+
for (Constructor<T> c : ctors) {
114+
if (!Modifier.isPublic(c.getModifiers())) {
115+
continue;
116+
}
117+
if (c.getParameterCount() == 0) {
118+
119+
return c;
120+
}
121+
if (c.getParameterCount() == 1
122+
&& pojoClass
123+
.getName()
124+
.equals(
125+
c.getParameterTypes()[0].getName()
126+
+ "$"
127+
+ pojoClass.getSimpleName())) {
128+
return c;
118129
}
119-
return ctor;
120-
} catch (NoSuchMethodException e) {
121-
throw new IllegalArgumentException(
122-
String.format(
123-
"POJO class %s must have a public default constructor.",
124-
pojoClass.getName()),
125-
e);
126130
}
131+
132+
throw new IllegalArgumentException(
133+
String.format(
134+
"POJO class %s must have a public default constructor.",
135+
pojoClass.getName()));
127136
}
128137

129138
private static Map<String, Field> discoverAllInstanceFields(Class<?> clazz) {
@@ -135,6 +144,13 @@ private static Map<String, Field> discoverAllInstanceFields(Class<?> clazz) {
135144
if (Modifier.isStatic(mod) || Modifier.isTransient(mod)) {
136145
continue;
137146
}
147+
// Skip references to enclosing class
148+
if (f.getName().startsWith("this$")) {
149+
final Class type = f.getType();
150+
if ((type.getName() + "$" + clazz.getSimpleName()).equals(clazz.getName())) {
151+
continue;
152+
}
153+
}
138154
f.setAccessible(true);
139155
fields.putIfAbsent(f.getName(), f);
140156
}
@@ -143,32 +159,68 @@ private static Map<String, Field> discoverAllInstanceFields(Class<?> clazz) {
143159
return fields;
144160
}
145161

146-
private static Map<String, Method> discoverGetters(Class<?> clazz) {
147-
Map<String, Method> getters = new HashMap<>();
162+
private static Map<String, Method> discoverGetters(
163+
Class<?> clazz, Map<String, Field> fieldMap) {
164+
final Map<String, Method> getters = new HashMap<>();
148165
for (Method m : clazz.getMethods()) { // public methods incl. inherited
149-
if (m.getParameterCount() == 0
150-
&& m.getName().startsWith("get")
151-
&& !m.getReturnType().equals(void.class)) {
152-
String prop = decapitalize(m.getName().substring(3));
166+
final String prop = getGetterProp(m);
167+
if (fieldMap.containsKey(prop)) {
153168
getters.put(prop, m);
154169
}
155170
}
156171
return getters;
157172
}
158173

159-
private static Map<String, Method> discoverSetters(Class<?> clazz) {
160-
Map<String, Method> setters = new HashMap<>();
174+
private static String getGetterProp(Method m) {
175+
if (m.getParameterCount() != 0) {
176+
return null;
177+
}
178+
final Class<?> returnType = m.getReturnType();
179+
if (void.class.equals(returnType) || Void.class.equals(returnType)) {
180+
return null;
181+
}
182+
final String name = m.getName();
183+
if (name.startsWith("get")) {
184+
return decapitalize(name.substring(3));
185+
}
186+
if (returnType.equals(boolean.class) || returnType.equals(Boolean.class)) {
187+
if (name.startsWith("is")) {
188+
return decapitalize(name.substring(2));
189+
}
190+
if (name.startsWith("has")) {
191+
return decapitalize(name.substring(3));
192+
}
193+
}
194+
return null;
195+
}
196+
197+
private static Map<String, Method> discoverSetters(
198+
Class<?> clazz, Map<String, Field> fieldMap) {
199+
final Map<String, Method> setters = new HashMap<>();
161200
for (Method m : clazz.getMethods()) { // public methods incl. inherited
162-
if (m.getParameterCount() == 1
163-
&& m.getName().startsWith("set")
164-
&& m.getReturnType().equals(void.class)) {
165-
String prop = decapitalize(m.getName().substring(3));
201+
final String prop = getSetterProp(m);
202+
if (fieldMap.containsKey(prop)) {
166203
setters.put(prop, m);
167204
}
168205
}
169206
return setters;
170207
}
171208

209+
private static String getSetterProp(Method m) {
210+
if (m.getParameterCount() != 1) {
211+
return null;
212+
}
213+
final Class<?> returnType = m.getReturnType();
214+
if (!void.class.equals(returnType) && !Void.class.equals(returnType)) {
215+
return null;
216+
}
217+
final String name = m.getName();
218+
if (name.startsWith("set")) {
219+
return decapitalize(name.substring(3));
220+
}
221+
return null;
222+
}
223+
172224
private static String capitalize(String s) {
173225
if (s == null || s.isEmpty()) {
174226
return s;

‎fluss-client/src/test/java/org/apache/fluss/client/converter/PojoTypeTest.java‎

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.fluss.client.converter;
19+
20+
import org.junit.jupiter.api.Test;
21+
22+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
23+
24+
/** Basic tests for {@link PojoType}. */
25+
class PojoTypeTest {
26+
@Test
27+
void test() {
28+
assertThatThrownBy(() -> PojoType.of(ClassWithNoPublicConstructor.class))
29+
.isInstanceOf(IllegalArgumentException.class)
30+
.hasMessageContaining("must have a public default constructor.");
31+
32+
assertThatThrownBy(() -> PojoType.of(ClassWithNonWithNonPublicField.class))
33+
.isInstanceOf(IllegalArgumentException.class)
34+
.hasMessageContaining(" must be public.");
35+
36+
assertThatThrownBy(() -> PojoType.of(PublicClass.class))
37+
.isInstanceOf(IllegalArgumentException.class)
38+
.hasMessageContaining(
39+
"Primitive types are not allowed; all fields must be nullable (use wrapper types).");
40+
41+
assertThatThrownBy(() -> PojoType.of(PublicClass.InnerClass.class))
42+
.isInstanceOf(IllegalArgumentException.class)
43+
.hasMessageContaining(
44+
"Primitive types are not allowed; all fields must be nullable (use wrapper types).");
45+
46+
assertThatThrownBy(() -> PojoType.of(PublicWithNonPrimitive.class))
47+
.isInstanceOf(IllegalArgumentException.class)
48+
.hasMessageContaining("must be public or have both getter and setter");
49+
50+
assertThatThrownBy(() -> PojoType.of(PublicWithPublicWithBoolean.class))
51+
.isInstanceOf(IllegalArgumentException.class)
52+
.hasMessageContaining("must be public or have both getter and setter");
53+
54+
assertThatThrownBy(() -> PojoType.of(PublicWithPublicWithBooleanWithGetterOnly.class))
55+
.isInstanceOf(IllegalArgumentException.class)
56+
.hasMessageContaining("must be public or have both getter and setter");
57+
58+
PojoType.of(PublicWithPublicWithBooleanWithGetterAndSetter.class);
59+
PojoType.of(PublicWithPublicWithBooleanWithIsAndSetter.class);
60+
PojoType.of(PublicWithPublicWithBooleanWithHasAndSetter.class);
61+
PojoType.of(PublicWithPublicNonPrimitive.class);
62+
}
63+
64+
public class ClassWithNoPublicConstructor {
65+
int f;
66+
int j;
67+
68+
private ClassWithNoPublicConstructor() {}
69+
}
70+
71+
class ClassWithNonWithNonPublicField {
72+
int f;
73+
int j;
74+
}
75+
76+
public class PublicClass {
77+
public class InnerClass {
78+
final int e;
79+
80+
public InnerClass() {
81+
e = 2;
82+
}
83+
}
84+
85+
final int f;
86+
final int j;
87+
88+
public PublicClass() {
89+
f = 1;
90+
j = 1;
91+
}
92+
}
93+
94+
public class PublicWithNonPrimitive {
95+
String s;
96+
97+
public PublicWithNonPrimitive() {}
98+
}
99+
100+
public class PublicWithPublicNonPrimitive {
101+
public String s;
102+
103+
public PublicWithPublicNonPrimitive() {}
104+
}
105+
106+
public class PublicWithPublicWithBoolean {
107+
private Boolean b;
108+
109+
public PublicWithPublicWithBoolean() {}
110+
}
111+
112+
public class PublicWithPublicWithBooleanWithGetterOnly {
113+
private Boolean b;
114+
115+
public PublicWithPublicWithBooleanWithGetterOnly() {}
116+
117+
public Boolean getB() {
118+
return b;
119+
}
120+
}
121+
122+
public class PublicWithPublicWithBooleanWithGetterAndSetter {
123+
private Boolean b;
124+
125+
public PublicWithPublicWithBooleanWithGetterAndSetter() {}
126+
127+
public Boolean getB() {
128+
return b;
129+
}
130+
131+
public void setB(boolean b) {
132+
this.b = b;
133+
}
134+
}
135+
136+
public class PublicWithPublicWithBooleanWithIsAndSetter {
137+
private Boolean b;
138+
139+
public PublicWithPublicWithBooleanWithIsAndSetter() {}
140+
141+
public Boolean isB() {
142+
return b;
143+
}
144+
145+
public void setB(boolean b) {
146+
this.b = b;
147+
}
148+
}
149+
150+
public class PublicWithPublicWithBooleanWithHasAndSetter {
151+
private Boolean b;
152+
153+
public PublicWithPublicWithBooleanWithHasAndSetter() {}
154+
155+
public Boolean hasB() {
156+
return b;
157+
}
158+
159+
public void setB(boolean b) {
160+
this.b = b;
161+
}
162+
}
163+
}

0 commit comments

Comments
 (0)