This discussion is archived
1 2 Previous Next 20 Replies Latest reply: Mar 19, 2010 12:58 AM by DarrylBurke RSS

Introspector finds wrong PropertyDescriptors on overriden methods

843798 Newbie
Currently Being Moderated
Hello all, I've noticed some odd behaviour from Introspector.getBeanInfo(Class).
public interface TestB {
}

public class TestBImpl implements TestB {
}

public interface TestA {
     public TestB getTestB();
}

public class TestAImpl implements TestA {

     private TestBImpl testB;
     
     public TestBImpl getTestB() {
          return testB;
     }

     public void setTestB(TestBImpl testB) {
          this.testB = testB;
     }
}
Notice TestAImpl overrides "public TestB getTestB(); " with the more specific "public TestBImpl getTestB()". TestAImpl also includes a setter for testB.

Now let's try and get the PropertyDescriptors of TestAImpl
BeanInfo bi = Introspector.getBeanInfo(TestAImpl.class);

for (PropertyDescriptor pd: bi.getPropertyDescriptors()) {
     if (pd.getName().equals("testB")) {
          System.out.println("Read Method: " + pd.getReadMethod().toGenericString());
          
          if (pd.getWriteMethod() == null) {
               System.out.println("Test Failed");
          } else {
               System.out.println("Write Method: " + pd.getWriteMethod().toGenericString());
               System.out.println("Test Passed");
          }
          
          break;
     }
}
The test fails with the following output
Read Method: public volatile TestB TestAImpl.getTestB()
Test Failed
The getter found is from the TestA interface, not from TestAImpl. It's no surprise then that it's unable to locate the setter which is not defined in the interface.

This this a bug or am I missing something? To me it should work similar to Class.getMethod(String, Class[]) which starts at the bottom and recurses up the super chain.

Thanks
Mark
  • 1. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    EJP Guru
    Currently Being Moderated
    The real question is why are you doing it this way?
    public class TestAImpl implements TestA {

         private TestBImpl testB;
         
         public TestBImpl getTestB() {
              return testB;
         }
    Why would you specify the implementation type here when you have a nice interface already declared? That's contrary to good OOP practice.
         public void setTestB(TestBImpl testB) {
    Ditto, in fact this is worse. If you ever put a setter into the interface this would fail to satisfy it.

    I agree that it's curious that it gave you the interface's getter not the class's, but given that it did that its failure to find the non-existent corresponding setter is reasonable.

    As far as I know it does indeed recurse up the super chain but perhaps it does a sideways look into the interfaces first?.
  • 2. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    I agree this is an unusual case and not generally considered good practice but let assume that there's a good case for this structure. Just because it's not good practice doesn't mean it shouldn't work.

    A sideways look at the interfaces first also seams broken to me. IMHO, it should always start with the class you've asked for.

    Thanks
    Mark
  • 3. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    EJP Guru
    Currently Being Moderated
    It's not a sideways look at the interfaces actually. javap -p TestAImpl shows that the compiler has inserted a thunk method public TestB getTestB(), to satisfy the interface, that calls public TestBImpl getTestB().

    I suppose the Introspector is using the thunk and ignoring the other one, on some grounds that wouldn't be hard to imagine, e.g. choosing the most general form.
  • 4. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    Hrm yah, that does make some sense. I can think of more instances where I'd want the most specific form rather then the most generic though.
  • 5. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    Found an interesting workaround. Introspector looks for an explicit BeanInfo class in the same package as the target class. This allows you to create a TestAImplBeanInfo which implements BeanInfo (or extends SimpleBeanInfo) and short-cuts the discovery process. Introspector will also check if the target class implements BeanInfo itself (meaning it's, its own BeanInfo).

    I still think the default logic is backwards but this at least gives you the ability to override.
  • 6. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    I now see exactly how this happens.

    Introspector constructs a map of fields to List<PropertyDescriptors> (one PD for each get/set found for that property). The list of PD's is constructed using Class.getDeclaredMethods() on TestAImpl.
    This list will be in the order:

    public TestBImpl TestAImpl.getTestB()
    public volatile TestB TestAImpl.getTestB()
    public void TestAImpl.setTestB(TestBImpl)

    Introspector now needs to compress the PropertyDescriptors into a single PropertyDescriptor which represents both the getter and setter. It does this by itterating over the PD list for the "testB" field looking for the "latest" getter.

    processPropertyDescriptors() in Introspector.java:666 (jdk 1.6)
    processPropertyDescriptors() in Introspector.java:651 (jdk 1.5)
    // First pass. Find the latest getter method. Merge properties
    // of previous getter methods.
    for (int i = 0; i < list.size(); i++) {
         pd = (PropertyDescriptor)list.get(i);
         if (pd instanceof IndexedPropertyDescriptor) {
    
              // our pd is not an instance of IndexedPropertyDescriptor
              ...
    
         } else {
              if (pd.getReadMethod() != null) {
                   if (gpd != null) {
                        // Don't replace the existing read
                        // method if it starts with "is"
                        Method method = gpd.getReadMethod();
                        if (!method.getName().startsWith(IS_PREFIX)) {
                             gpd = new PropertyDescriptor(gpd, pd);
                        }
                   } else {
                        gpd = pd;
                   }
              }
         }
    }
    The first pass correctly find the "public TestBImpl TestAImpl.getTestB()" PropertyDescriptor and sets gpd to it, however it is replaced by the "public volatile TestB TestAImpl.getTestB()" in the second pass. This appears to be the intended functionality from the comment at the beginning "Find the latest getter method".

    The method then goes on to try and find the latest setter using the return type of the getter. This will fail however since it's now looking for a setTestB(TestB).

    My question now is why is it looking for the latest getter rather then the first?

    This is backwards from how Class.getMethod() works (From the getMethod(String, Class[]) java docs)

    To find a matching method in a class C:  If C declares exactly one public method with the specified name and exactly the same formal parameter types, that is the method reflected. If more than one such method is found in C, and one of these methods has a return type that is more specific than any of the others, that method is reflected; otherwise one of the methods is chosen arbitrarily.

    Does anyone else agree that this is a bug? I'm contemplating entering a bug report for it unless someone can come up with a good reason why it works this way.

    Thanks
    Mark
  • 7. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    EJP Guru
    Currently Being Moderated
    I agree that it's a bug, but I think the correct resolution would be for it to do exactly what it does now in this particular case, i.e. find the most general form of the getter, i.e. the compiler thunk, so as to satisfy interfaces. Rather than the latest or first.
  • 8. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    Hmm...
    I'm not sure if that is a bug or not having never run across an instance where it caused a problem (likely due to coding patterns).

    I'm not sure this is a bug, because TestAImpl doesn't properly/actually implement TestA interface (wrong return type). it knows that it IS implementing the interface however, and it knows that that TestA.getTestB() returns a TestB so its actually working as I would expect it.

    getTestB() and setTestB(TestBImpl o) are not actually a paired accessor because getTestB() returns a TestB not a TestBImpl. They actually have different signatures and the only real issue is that even though you have monkeyed with the signature of getTestB() it still knows what it should be (IMO you should have got a compiler error).

    I think the VM will actually see setTestB(TestBImple o) and setTestB(TestB 0) as two different overloaded setters, which would follow based on your test.
    You can't overload the getters in the same way, so it uses the "correct" signature from the interface because it knows testAImple.getTestB() is an implementation and it knows that TestBImpl implements TestB and does not violate the TestA contract.


    So... without further investigation, my opinion is that it's working as expected and is not a bug.
  • 9. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    EJP Guru
    Currently Being Moderated
    I'm not sure this is a bug, because TestAImpl doesn't properly/actually implement TestA interface (wrong return type).
    (a) TestAImpl does indeed implement TestA, because of the compiler-generated thunk method. (b) Your conclusion doesn't follow from the premiss whether true or false.
    it knows that it IS implementing the interface
    So which is it?
    (IMO you should have got a compiler error).
    Wrong. It is legal Java. See the [JLS #8.4.5|http://java.sun.com/docs/books/jls/third_edition/html/classes.html#8.4.5].
    I think the VM will actually see setTestB(TestBImple o) and setTestB(TestB 0) as two different overloaded setters
    Except that one of them doesn't exist.
    You can't overload the getters in the same way, so it uses the "correct" signature from the interface because ...
    No it doesn't. Read the thread. The OP has already established why it uses the getter it does: because it is last. He even quoted the source code. No reason for all this guesswork.
    So... without further investigation, my opinion is
    ... not of any great interest, given that (apart from just repeating a lot of stuff that had already been stated): your inferences are invalid; you clearly haven't read the thread; you've never heard of covariant return types; and you're guessing against the hard evidence of what the source code actually does.
  • 10. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    @ejp Wow, you are the rudest poster I've run across in a long time. Very poor form from someone i expect professionalism from. Anyway, yes I'm aware of most of your comments.

    I don't even think it's simply a case of most/least generic. After talking with the OP we realized that although it's odd code and is likely not run across very often because people tend not to write their code that way, the "bug" is in the fact that the introspecter and getDefinedMethods doesn't work the same, which I think is a valid bug to submit and as was pointed out to me, it's simply missing a break in the underlying code... IMO it's likely an unintentional bug.

    It remains to bee seen what else it would break however if it was fixed... it may be that we are stuck with it for the duration because I could see it causing havoc with millions of critical systems out there that use the buggy feature.

    That said, I'm for fixing it now, before the problem get worse... maybe the OP can let us know if he submits a bug report so we can track it.
  • 11. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    ejp wrote:
    I agree that it's a bug, but I think the correct resolution would be for it to do exactly what it does now in this particular case, i.e. find the most general form of the getter, i.e. the compiler thunk, so as to satisfy interfaces. Rather than the latest or first.
    Could I get you to clarify this for me? If the correct resolution is the current functionality, what part do you believe is a bug?

    To me this comes down to one fact, all of the reflecting methods on Class are parent-last. Introspector is parent-first. I believe they should work the same. I do see your point about finding the most general to satisfy interfaces though. Perhaps the bug here is actually just some missing documentation.
  • 12. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    bpappin wrote:
    It remains to bee seen what else it would break however if it was fixed... it may be that we are stuck with it for the duration because I could see it causing havoc with millions of critical systems out there that use the buggy feature.
    I'm concerned about this as well. Introspector is used heavily by many commonly used libraries (commons-beanutils, hibernate, etc..). Changing the resolution order may have undesirable effects in many applications.
  • 13. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    I find this odd too. We use DWR at work and all properties were being set correctly until I had one of my beans implement an interface and then all the sudden the Instospector was unable to see the property setter. I posted about it here with a running test case [http://raulraja.com/2009/09/12/java-beans-introspector-odd-behavio/|http://raulraja.com/2009/09/12/java-beans-introspector-odd-behavio/]

    Edited by: raulraja on Sep 12, 2009 3:26 PM
  • 14. Re: Introspector finds wrong PropertyDescriptors on overriden methods
    843798 Newbie
    Currently Being Moderated
    I submitted a bug about this issue. The problem is not only with interfaces but with basic subclassing too.
    See this code and output:
    import java.beans.BeanInfo;
    import java.beans.IntrospectionException;
    import java.beans.Introspector;
    import java.beans.PropertyDescriptor;
    import java.lang.reflect.Method;
    import java.lang.reflect.InvocationTargetException;
    
    /**
     * Shows odd behavior when using an instrospector from the java.beans api
     */
    public class InstrospectorPossibleBugTest {
    
           /**
            * super class for people
            */
           public class Person {
    
                   public Object getName() {
                           return null;
                   }
    
           }
    
           /**
            * A worker is a Person
            */
           public class Worker extends Person {
    
                   private String name;
    
                   public String getName() {
                           return name;
                   }
    
                   public void setName(String name) {
                           this.name = name;
                   }
           }
    
    
           /**
            * This small test cases shows the odd behavior.
            * Even though Worker conforms with the getters and setters for the javabean spec the
            * Instrospector seems to not be able to find any setter for the property name.
            * Seems like instead of following a bottom up lookup for property accessors it starts and stops once it
            * finds a compatible property in one of the implemented interfaces or parent classes.
            */
           public static void main(String... args) throws IntrospectionException, InvocationTargetException, IllegalAccessException {
                   BeanInfo info = Introspector.getBeanInfo(Worker.class);
                   PropertyDescriptor[] descriptors = info.getPropertyDescriptors();
                   for (PropertyDescriptor descriptor : descriptors) {
                           if (!descriptor.getName().equals("class")) {
                                   System.out.println("property: \t\t\t\t\t\t" + descriptor.getName());
                                   System.out.println("getter: \t\t\t\t\t\t" + descriptor.getReadMethod());
                                   System.out.println("getter returns: \t\t\t\t" + descriptor.getReadMethod().getReturnType());
                                   System.out.println("setter: \t\t\t\t\t\t" + descriptor.getWriteMethod());
                                   String propertyName = descriptor.getName();
                                   String setterName = "set" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1);
                                   Method setter = findMethodImplFirst(Worker.class, setterName, String.class);
                                   System.out.println("setter with reflection: \t\t" + setter);
                           }
                   }
    
           }
    
    
    }
    Output:

    property: name
    getter: public java.lang.Object Worker.getName()
    getter returns: class java.lang.Object
    setter: null
1 2 Previous Next