This is an automated email from the ASF dual-hosted git repository. jark pushed a commit to branch release-0.8 in repository https://gitbox.apache.org/repos/asf/fluss.git
commit 6f1715fc86133d32152961d431d623a1ea5b63ee Author: Sergey Nuyanzin <[email protected]> AuthorDate: Fri Oct 31 07:23:49 2025 +0100 [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 7af5c0cc94db6afae3bbc21bf635c7c501426815) --- .../apache/fluss/client/converter/PojoType.java | 110 ++++++++++---- .../fluss/client/converter/PojoTypeTest.java | 163 +++++++++++++++++++++ 2 files changed, 244 insertions(+), 29 deletions(-) diff --git a/fluss-client/src/main/java/org/apache/fluss/client/converter/PojoType.java b/fluss-client/src/main/java/org/apache/fluss/client/converter/PojoType.java index 78af49700..7790b9143 100644 --- a/fluss-client/src/main/java/org/apache/fluss/client/converter/PojoType.java +++ b/fluss-client/src/main/java/org/apache/fluss/client/converter/PojoType.java @@ -66,8 +66,8 @@ final class PojoType<T> { Constructor<T> ctor = requirePublicDefaultConstructor(pojoClass); Map<String, Field> allFields = discoverAllInstanceFields(pojoClass); - Map<String, Method> getters = discoverGetters(pojoClass); - Map<String, Method> setters = discoverSetters(pojoClass); + Map<String, Method> getters = discoverGetters(pojoClass, allFields); + Map<String, Method> setters = discoverSetters(pojoClass, allFields); Map<String, Property> props = new LinkedHashMap<>(); for (Map.Entry<String, Field> e : allFields.entrySet()) { @@ -85,10 +85,11 @@ final class PojoType<T> { if (!publicField) { // When not a public field, require both getter and setter if (getter == null || setter == null) { + final String capitalizedName = capitalize(name); throw new IllegalArgumentException( String.format( "POJO class %s field '%s' must be public or have both getter and setter (get%s/set%s).", - pojoClass.getName(), name, capitalize(name), capitalize(name))); + pojoClass.getName(), name, capitalizedName, capitalizedName)); } } props.put( @@ -108,22 +109,30 @@ final class PojoType<T> { } private static <T> Constructor<T> requirePublicDefaultConstructor(Class<T> pojoClass) { - try { - Constructor<T> ctor = pojoClass.getDeclaredConstructor(); - if (!Modifier.isPublic(ctor.getModifiers())) { - throw new IllegalArgumentException( - String.format( - "POJO class %s must have a public default constructor.", - pojoClass.getName())); + Constructor<T>[] ctors = (Constructor<T>[]) pojoClass.getConstructors(); + for (Constructor<T> c : ctors) { + if (!Modifier.isPublic(c.getModifiers())) { + continue; + } + if (c.getParameterCount() == 0) { + + return c; + } + if (c.getParameterCount() == 1 + && pojoClass + .getName() + .equals( + c.getParameterTypes()[0].getName() + + "$" + + pojoClass.getSimpleName())) { + return c; } - return ctor; - } catch (NoSuchMethodException e) { - throw new IllegalArgumentException( - String.format( - "POJO class %s must have a public default constructor.", - pojoClass.getName()), - e); } + + throw new IllegalArgumentException( + String.format( + "POJO class %s must have a public default constructor.", + pojoClass.getName())); } private static Map<String, Field> discoverAllInstanceFields(Class<?> clazz) { @@ -135,6 +144,13 @@ final class PojoType<T> { if (Modifier.isStatic(mod) || Modifier.isTransient(mod)) { continue; } + // Skip references to enclosing class + if (f.getName().startsWith("this$")) { + final Class type = f.getType(); + if ((type.getName() + "$" + clazz.getSimpleName()).equals(clazz.getName())) { + continue; + } + } f.setAccessible(true); fields.putIfAbsent(f.getName(), f); } @@ -143,32 +159,68 @@ final class PojoType<T> { return fields; } - private static Map<String, Method> discoverGetters(Class<?> clazz) { - Map<String, Method> getters = new HashMap<>(); + private static Map<String, Method> discoverGetters( + Class<?> clazz, Map<String, Field> fieldMap) { + final Map<String, Method> getters = new HashMap<>(); for (Method m : clazz.getMethods()) { // public methods incl. inherited - if (m.getParameterCount() == 0 - && m.getName().startsWith("get") - && !m.getReturnType().equals(void.class)) { - String prop = decapitalize(m.getName().substring(3)); + final String prop = getGetterProp(m); + if (fieldMap.containsKey(prop)) { getters.put(prop, m); } } return getters; } - private static Map<String, Method> discoverSetters(Class<?> clazz) { - Map<String, Method> setters = new HashMap<>(); + private static String getGetterProp(Method m) { + if (m.getParameterCount() != 0) { + return null; + } + final Class<?> returnType = m.getReturnType(); + if (void.class.equals(returnType) || Void.class.equals(returnType)) { + return null; + } + final String name = m.getName(); + if (name.startsWith("get")) { + return decapitalize(name.substring(3)); + } + if (returnType.equals(boolean.class) || returnType.equals(Boolean.class)) { + if (name.startsWith("is")) { + return decapitalize(name.substring(2)); + } + if (name.startsWith("has")) { + return decapitalize(name.substring(3)); + } + } + return null; + } + + private static Map<String, Method> discoverSetters( + Class<?> clazz, Map<String, Field> fieldMap) { + final Map<String, Method> setters = new HashMap<>(); for (Method m : clazz.getMethods()) { // public methods incl. inherited - if (m.getParameterCount() == 1 - && m.getName().startsWith("set") - && m.getReturnType().equals(void.class)) { - String prop = decapitalize(m.getName().substring(3)); + final String prop = getSetterProp(m); + if (fieldMap.containsKey(prop)) { setters.put(prop, m); } } return setters; } + private static String getSetterProp(Method m) { + if (m.getParameterCount() != 1) { + return null; + } + final Class<?> returnType = m.getReturnType(); + if (!void.class.equals(returnType) && !Void.class.equals(returnType)) { + return null; + } + final String name = m.getName(); + if (name.startsWith("set")) { + return decapitalize(name.substring(3)); + } + return null; + } + private static String capitalize(String s) { if (s == null || s.isEmpty()) { return s; diff --git a/fluss-client/src/test/java/org/apache/fluss/client/converter/PojoTypeTest.java b/fluss-client/src/test/java/org/apache/fluss/client/converter/PojoTypeTest.java new file mode 100644 index 000000000..e2d44f581 --- /dev/null +++ b/fluss-client/src/test/java/org/apache/fluss/client/converter/PojoTypeTest.java @@ -0,0 +1,163 @@ +/* + * 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.fluss.client.converter; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Basic tests for {@link PojoType}. */ +class PojoTypeTest { + @Test + void test() { + assertThatThrownBy(() -> PojoType.of(ClassWithNoPublicConstructor.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must have a public default constructor."); + + assertThatThrownBy(() -> PojoType.of(ClassWithNonWithNonPublicField.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(" must be public."); + + assertThatThrownBy(() -> PojoType.of(PublicClass.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Primitive types are not allowed; all fields must be nullable (use wrapper types)."); + + assertThatThrownBy(() -> PojoType.of(PublicClass.InnerClass.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Primitive types are not allowed; all fields must be nullable (use wrapper types)."); + + assertThatThrownBy(() -> PojoType.of(PublicWithNonPrimitive.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be public or have both getter and setter"); + + assertThatThrownBy(() -> PojoType.of(PublicWithPublicWithBoolean.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be public or have both getter and setter"); + + assertThatThrownBy(() -> PojoType.of(PublicWithPublicWithBooleanWithGetterOnly.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must be public or have both getter and setter"); + + PojoType.of(PublicWithPublicWithBooleanWithGetterAndSetter.class); + PojoType.of(PublicWithPublicWithBooleanWithIsAndSetter.class); + PojoType.of(PublicWithPublicWithBooleanWithHasAndSetter.class); + PojoType.of(PublicWithPublicNonPrimitive.class); + } + + public class ClassWithNoPublicConstructor { + int f; + int j; + + private ClassWithNoPublicConstructor() {} + } + + class ClassWithNonWithNonPublicField { + int f; + int j; + } + + public class PublicClass { + public class InnerClass { + final int e; + + public InnerClass() { + e = 2; + } + } + + final int f; + final int j; + + public PublicClass() { + f = 1; + j = 1; + } + } + + public class PublicWithNonPrimitive { + String s; + + public PublicWithNonPrimitive() {} + } + + public class PublicWithPublicNonPrimitive { + public String s; + + public PublicWithPublicNonPrimitive() {} + } + + public class PublicWithPublicWithBoolean { + private Boolean b; + + public PublicWithPublicWithBoolean() {} + } + + public class PublicWithPublicWithBooleanWithGetterOnly { + private Boolean b; + + public PublicWithPublicWithBooleanWithGetterOnly() {} + + public Boolean getB() { + return b; + } + } + + public class PublicWithPublicWithBooleanWithGetterAndSetter { + private Boolean b; + + public PublicWithPublicWithBooleanWithGetterAndSetter() {} + + public Boolean getB() { + return b; + } + + public void setB(boolean b) { + this.b = b; + } + } + + public class PublicWithPublicWithBooleanWithIsAndSetter { + private Boolean b; + + public PublicWithPublicWithBooleanWithIsAndSetter() {} + + public Boolean isB() { + return b; + } + + public void setB(boolean b) { + this.b = b; + } + } + + public class PublicWithPublicWithBooleanWithHasAndSetter { + private Boolean b; + + public PublicWithPublicWithBooleanWithHasAndSetter() {} + + public Boolean hasB() { + return b; + } + + public void setB(boolean b) { + this.b = b; + } + } +}
