#49 FieldMap source descriptor caching broken

closed
None
5
2006-09-02
2006-08-09
David Baker
No

FieldMap.getSourcePropertyDescriptor(Class) improperly
caches the source property descriptor from its first
call. It assumes that the concrete class of the object
returned from all invocations of the field map will
match the concrete class of the object returned from
the first invocation. That is not necessarily true, and
when it is not it can lead to an
IllegalArgumentException thrown from line 102 of
BruteForcePropertyDescriptor.

The exception case is this:

class Foo { public Bar getBar() {...} }

class Bar { public Long getId() {...} }

class SubBar extends Bar { public Long getId() {...} }

The first time the fieldmap for Foo that includes Bar
as a source field is called, getBar() returns an object
of class SubBar.

A BruteForcePropertyDescriptor is created whose clazz
property is SubBar. When getting the id property, the
readMethod property is generated to be SubBar.getId()
and cached. The propertydescriptor is cached too. So
far so good.

The next time the Foo fieldmap is called, getBar()
returns an object of class Bar. Because the
propertydescriptor for the "bar" property is cached, we
use that to get the Bar's "id" property. But the read
method is declared on SubBar and cannot be invoked on
Bar -- hence the exception.

It looks like there was some attempt to address this
issue (line 317 of
BruteForcePropertyDescriptor.getReadMethod(Class)). But
this fails for several reasons:

1. getReadMethod(Class) is called (from line 102 of
BruteForcePropertyDescriptor.getPropertyValue(Object))
with the cached class, not the actual class of the bean
being accessed.
2. Even if (1) were fixed, when resetting the read
method (line 319) the cached class is again used
instead of the argument.

I've attached a patch to FieldMap that represents a
different solution to this problem: one that trades off
the speed hit of repeatedly using reflection to change
the read method for a small space hit of caching the
source property descriptors in a Map with the
sourceClass as a key, in getSourcePropertyDescriptor.
This way each concrete class represented by objects
returned by a get method will have its own source
property descriptor, so there will never be confusion
as to what class declares the read method. Please take
a look.

Discussion

  • David Baker

    David Baker - 2006-08-09

    Patch file for FieldMap.java (using CVS diff)

     
  • Franz Garsombke

    Franz Garsombke - 2006-08-10

    Logged In: YES
    user_id=550744

    Thanks for the patch David. When (If?) we have another
    release I will be sure to get this in.

    Thanks for looking.

    Franz

     
  • Matt Tierney

    Matt Tierney - 2006-08-13

    Logged In: YES
    user_id=1236069

    David,
    The patch worked great. After applying the patch code,
    the unit tests ran 100% green. I also tested performance
    impacts against our baseline and I did not notice any
    significant differences. I will check in your patch
    code. I am also going to investigate whether we can apply
    the same concept to other code that caches property
    descriptors. Thanks!

     
  • Matt Tierney

    Matt Tierney - 2006-08-13
    • status: open --> pending
     
  • Matt Tierney

    Matt Tierney - 2006-08-13

    Logged In: YES
    user_id=1236069

    David,
    Forgot to ask if you have a unit test that you could
    provide that reproduces the original issue. If so, please
    attach that to the bug and I can test it prior to
    releasing the code.

     
  • Matt Tierney

    Matt Tierney - 2006-09-01
    • assigned_to: nobody --> mhtierney
     
  • Matt Tierney

    Matt Tierney - 2006-09-02
    • status: pending --> closed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks