Skip to content

Commit

Permalink
[FLINK-2437] handle the case of a non-public default ctor in TypeExtr…
Browse files Browse the repository at this point in the history
…actor.analyzePojo

Also changed some prints which printed the word "class" twice,
because class.toString also prepends it to the class name.

This closes apache#960.
  • Loading branch information
ggevay authored and mxm committed Aug 12, 2015
1 parent 54311aa commit a41bc8c
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 10 deletions.
2 changes: 1 addition & 1 deletion docs/apis/programming_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ Java and Scala classes are treated by Flink as a special POJO data type if they

- The type of a field must be supported by Flink. At the moment, Flink uses [Avro](http://avro.apache.org) to serialize arbitrary objects (such as `Date`).

Flink analyzes the structure of POJO types, i.e., it learns about the fields of a POJO. As a result POJO types are easier to use than general types. Moreover, they Flink can process POJOs more efficiently than general types.
Flink analyzes the structure of POJO types, i.e., it learns about the fields of a POJO. As a result POJO types are easier to use than general types. Moreover, Flink can process POJOs more efficiently than general types.

The following example shows a simple POJO with two public fields.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package org.apache.flink.api.java.typeutils;

import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.GenericArrayType;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -1298,10 +1299,10 @@ private boolean isValidPojoField(Field f, Class<?> clazz, ArrayList<Type> typeHi
return true;
} else {
if(!hasGetter) {
LOG.debug("Class "+clazz+" does not contain a getter for field "+f.getName() );
LOG.debug(clazz+" does not contain a getter for field "+f.getName() );
}
if(!hasSetter) {
LOG.debug("Class "+clazz+" does not contain a setter for field "+f.getName() );
LOG.debug(clazz+" does not contain a setter for field "+f.getName() );
}
return false;
}
Expand All @@ -1323,15 +1324,15 @@ else if(typeHierarchy.size() <= 1) {

List<Field> fields = getAllDeclaredFields(clazz);
if(fields.size() == 0) {
LOG.info("No fields detected for class " + clazz + ". Cannot be used as a PojoType. Will be handled as GenericType");
LOG.info("No fields detected for " + clazz + ". Cannot be used as a PojoType. Will be handled as GenericType");
return new GenericTypeInfo<OUT>(clazz);
}

List<PojoField> pojoFields = new ArrayList<PojoField>();
for (Field field : fields) {
Type fieldType = field.getGenericType();
if(!isValidPojoField(field, clazz, typeHierarchy)) {
LOG.info("Class " + clazz + " is not a valid POJO type");
LOG.info(clazz + " is not a valid POJO type");
return null;
}
try {
Expand All @@ -1357,24 +1358,29 @@ else if(typeHierarchy.size() <= 1) {
List<Method> methods = getAllDeclaredMethods(clazz);
for (Method method : methods) {
if (method.getName().equals("readObject") || method.getName().equals("writeObject")) {
LOG.info("Class "+clazz+" contains custom serialization methods we do not call.");
LOG.info(clazz+" contains custom serialization methods we do not call.");
return null;
}
}

// Try retrieving the default constructor, if it does not have one
// we cannot use this because the serializer uses it.
Constructor defaultConstructor = null;
try {
clazz.getDeclaredConstructor();
defaultConstructor = clazz.getDeclaredConstructor();
} catch (NoSuchMethodException e) {
if (clazz.isInterface() || Modifier.isAbstract(clazz.getModifiers())) {
LOG.info("Class " + clazz + " is abstract or an interface, having a concrete " +
LOG.info(clazz + " is abstract or an interface, having a concrete " +
"type can increase performance.");
} else {
LOG.info("Class " + clazz + " must have a default constructor to be used as a POJO.");
LOG.info(clazz + " must have a default constructor to be used as a POJO.");
return null;
}
}
if(defaultConstructor != null && !Modifier.isPublic(defaultConstructor.getModifiers())) {
LOG.info("The default constructor of " + clazz + " should be Public to be used as a POJO.");
return null;
}

// everything is checked, we return the pojo
return pojoType;
Expand All @@ -1394,7 +1400,7 @@ public static List<Field> getAllDeclaredFields(Class<?> clazz) {
continue; // we have no use for transient or static fields
}
if(hasFieldWithSameName(field.getName(), result)) {
throw new RuntimeException("The field "+field+" is already contained in the hierarchy of the class "+clazz+"."
throw new RuntimeException("The field "+field+" is already contained in the hierarchy of the "+clazz+"."
+ "Please use unique field names through your classes hierarchy");
}
result.add(field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ public int getMyField2() {
}
}

public static class PojoWithNonPublicDefaultCtor {
public int foo, bar;
PojoWithNonPublicDefaultCtor() {}
}

@SuppressWarnings({ "unchecked", "rawtypes" })
@Test
public void testPojo() {
Expand Down Expand Up @@ -345,6 +350,8 @@ public CustomType cross(CustomType first, Integer second) throws Exception {
Assert.assertFalse(ti2.isTupleType());
Assert.assertTrue(ti2 instanceof PojoTypeInfo);
Assert.assertEquals(ti2.getTypeClass(), CustomType.class);

Assert.assertFalse(TypeExtractor.getForClass(PojoWithNonPublicDefaultCtor.class) instanceof PojoTypeInfo);
}


Expand Down

0 comments on commit a41bc8c

Please sign in to comment.