juneau-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Goddard <godd...@acm.org>
Subject JsonParser fails when getters/setters of bean have inconsistent types
Date Sat, 19 Aug 2017 19:52:20 GMT
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
View raw message