juneau-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Bognar <jamesbog...@apache.org>
Subject Re: JsonParser fails when getters/setters of bean have inconsistent types
Date Tue, 22 Aug 2017 13:55:24 GMT
Perhaps we don't need a restriction that the getter and setter be the same
type.  From our perspective, I can't think of a good reason why you can't
have...

   public boolean isP();
   public void setP(String p);


Getters/setters take precedence over fields, so in the following case, the
getter would be used by JsonSerializer, and the field would be used by
JsonParser....

   public boolean isP();
   public String p;


One question remains about how to resolve the setter when multiple setters
are found but none match the getter type:

   public boolean isP();
   public void setP(String);
   public void setP(Foo);

Ignore the setter in this case and assume the property is read-only?


On Mon, Aug 21, 2017 at 9:53 PM, James Bognar <jamesbognar@gmail.com> wrote:

> Thanks David!  That's some in-depth debugging!
>
> I'll look at this carefully tomorrow.  It's not immediately obvious to me
> what the correct behavior should be in this case.
>
> On Sat, Aug 19, 2017 at 3:52 PM David Goddard <goddard@acm.org> wrote:
>
>> Hi,
>>
>> I think this is probably a bug, and I will log a JIRA, but I thought I'd
>> throw it out here first.  I'm finding (after a lot of time narrowing
>> down the cause) that I am unable to parse JSON into an object that
>> contains a method with a certain name.
>>
>> However, this is quite a long message, so the tldr summary is:
>>    If the JsonParser attempts to parse an object into a class that
>> abuses the setter/getter naming conventions by having what looks like a
>> boolean getter for a non-boolean property, it fails, complaining that
>> the property is unknown.
>>
>> Here is my test code, using 6.3.1-INCUBATING:
>>
>>    package test;
>>    import org.apache.juneau.json.*;
>>
>>    public class ClassParseTest {
>>
>>      public static void main(String[] args) {
>>
>>        String json = "{\"p1\":\"p1val\",\"p2\":\"p2val\"}";
>>
>>        try {
>>          JsonParser p = new JsonParserBuilder().build();
>>          p.parse(json, GoodClass.class);
>>          p.parse(json, DodgyClass.class);
>>        } catch (Exception e) {
>>          e.printStackTrace();
>>        }
>>
>>      }
>>
>>    }
>>
>> The code is able to successfully parse GoodClass, but fails on
>> DodgyClass with this error:
>>
>>    org.apache.juneau.parser.ParseException: Parse exception occurred at
>>    {currentClass:'DodgyClass',line:1,column:7}.  Unknown property 'p1'
>>    encountered while trying to parse into class 'test.DodgyClass'
>>
>> Here is the offending class:
>>
>>    public class DodgyClass {
>>
>>      public boolean isP1() {
>>        return false;
>>      }
>>
>>      public String p1;
>>      public String p2;
>>
>>    }
>>
>> And here is the working class:
>>
>>    public class GoodClass {
>>
>>      public boolean somethingP1() {
>>        return false;
>>      }
>>
>>      public String p1;
>>      public String p2;
>>
>>    }
>>
>> Note that the only difference here is that the former contains a method
>> called "isP1", while the latter has had this method renamed.  Any other
>> name at all works fine - it is just "isP1" that fails.
>>
>> In the debugger, I see that in BeanMap.java, for the broken class
>> getPropertyMeta is using incorrect data:
>>
>>    public BeanPropertyMeta getPropertyMeta(String propertyName) {
>>      BeanPropertyMeta bpMeta = meta.properties.get(propertyName);
>>      if (bpMeta == null)
>>        bpMeta = meta.dynaProperty;
>>      return bpMeta;
>>    }
>>
>> The meta variable in the BeanMap only contains p2:
>>    test.DodgyClass {
>>      p2: java.lang.String, field=[public java.lang.String
>>          test.DodgyClass.p2], getter=[null], setter=[null],
>>    }
>>
>> Whereas for the working class, it is complete:
>>
>>    test.GoodClass {
>>      p1: java.lang.String, field=[public java.lang.String
>>          test.GoodClass.p1], getter=[null], setter=[null],
>>      p2: java.lang.String, field=[public java.lang.String
>>          test.GoodClass.p2], getter=[null], setter=[null],
>>    }
>>
>> Hence getPropertyMeta returns null and the parse fails, as the parser
>> thinks that p1 does not exist in the class.
>>
>> During the construction of the bean meta in BeanMeta line 286 we see:
>>
>>    for (BeanMethod bm : bms) {
>>      String pn = bm.propertyName;
>>      Method m = bm.method;
>>      if (! normalProps.containsKey(pn))
>>          normalProps.put(pn, new BeanPropertyMeta.Builder(beanMeta, pn));
>>      BeanPropertyMeta.Builder bpm = normalProps.get(pn);
>>      if (! bm.isSetter)
>>        bpm.setGetter(m);
>>    }
>>
>> For the method DodgyClass.isP1(), bpm.setGetter(m) is invoked, which
>> sets isP1() as a getter for the p1 field.
>>
>> In line 315 of BeanMeta, some setter/getter based validation occurs:
>>
>>    if (p.validate(ctx, beanRegistry, typeVarImpls)) {
>>      if (p.getter != null)
>>        getterProps.put(p.getter, p.name);
>>      if (p.setter != null)
>>        setterProps.put(p.setter, p.name);
>>    } else {
>>        i.remove();
>>    }
>>
>> For the affected class, p.validate() returns false for p1.  It does so
>> on this part of BeanPropertyMeta, line 171:
>>
>>    // Do some annotation validation.
>>    Class<?> c = rawTypeMeta.getInnerClass();
>>    if (getter != null) {
>>      if (isDyna) {
>>        if (! isParentClass(Map.class, c))
>>          return false;
>>      } else {
>>        if (! isParentClass(getter.getReturnType(), c))
>>          return false;
>>      }
>>    }
>>
>> Because isParentClass(getter.getReturnType(), c) returns false for p1 in
>> the affected class the call to validate() returns false and hence p1 is
>> removed from the meta.properties for the bean and this ultimately causes
>> the problem.
>>
>> I've not yet dug into whether isParentClass() is behaving as expected; I
>> guess it probably is.
>>
>> We should bear in mind that p1 is a String in this test, and isP1() is a
>> boolean, and that the problem does not occur if both are boolean.  My
>> question here is whether this is a valid bug, or simply a case of "don't
>> do that then".  The problem originally arose for me because I added a
>> boolean method to a class that did some logic based on a string property
>> P, and happened to name that method "isP", causing the parser to
>> identify it as a getter for P when it is not really.  This is probably
>> something of an edge case because it is quite a specific circumstance
>> that causes it, and it is also easy to resolve by renaming the method to
>> not sound like a getter.  Given that Java naming conventions state that
>> a getter for a boolean should begin with "is", it's a pretty good
>> argument that abusing this in this way is asking for trouble.
>>
>> If we view this as actually a limitation in the parser rather than user
>> error, then I guess one solution would be to enhance the logic for
>> determining setters/getters to only consider a method to be a
>> getter/setter if its return/input type matches the type of the variable.
>>   This is something I'd be happy to look further at.  Either way, it's
>> probably helpful to have this error and a potential cause documented in
>> the mailing list.
>>
>> Regards,
>>
>> David
>>
>>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message