[ 
https://issues.apache.org/jira/browse/FLINK-2437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14693133#comment-14693133
 ] 

ASF GitHub Bot commented on FLINK-2437:
---------------------------------------

Github user mxm commented on a diff in the pull request:

    https://github.com/apache/flink/pull/960#discussion_r36837638
  
    --- Diff: 
flink-java/src/main/java/org/apache/flink/api/java/typeutils/TypeExtractor.java 
---
    @@ -1328,24 +1329,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 && 
(defaultConstructor.getModifiers() & Modifier.PUBLIC) == 0) {
    --- End diff --
    
    `if(defaultConstructor != null && 
Modifier.isPublic(defaultConstructor.getModifiers())` seems to be more readable 
to me but your approach is fine too.


> TypeExtractor.analyzePojo has some problems around the default constructor 
> detection
> ------------------------------------------------------------------------------------
>
>                 Key: FLINK-2437
>                 URL: https://issues.apache.org/jira/browse/FLINK-2437
>             Project: Flink
>          Issue Type: Bug
>          Components: Type Serialization System
>            Reporter: Gabor Gevay
>            Assignee: Gabor Gevay
>            Priority: Minor
>
> If a class does have a default constructor, but the user forgot to make it 
> public, then TypeExtractor.analyzePojo still thinks everything is OK, so it 
> creates a PojoTypeInfo. Then PojoSerializer.createInstance blows up.
> Furthermore, a "return null" seems to be missing from the then case of the if 
> after catching the NoSuchMethodException which would also cause a headache 
> for PojoSerializer.
> An additional minor issue is that the word "class" is printed twice in 
> several places, because class.toString also prepends it to the class name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to