This looks great for the getter half, but doesn't the setter half need to be fixed too? Otherwise, a getter may be called with too many parameters, right?
- Cassie On Thu, Jul 30, 2009 at 2:58 PM, Paul Lindner <[email protected]> wrote: > I think this should do it: > diff --git > a/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java > b/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java > index 9e392a8..b650178 100644 > --- a/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java > +++ b/java/common/src/main/java/org/apache/shindig/common/JsonUtil.java > @@ -84,9 +84,11 @@ public class JsonUtil { > methods = Maps.newHashMap(); > > for (Method method : clazz.getMethods()) { > - String name = getPropertyName(method); > - if (name != null) { > - methods.put(name, method); > + if (method.getParameterTypes().length == 0) { > + String name = getPropertyName(method); > + if (name != null) { > + methods.put(name, method); > + } > } > } > > diff --git > > a/java/common/src/test/java/org/apache/shindig/common/JsonSerializerTest.java > > b/java/common/src/test/java/org/apache/shindig/common/JsonSerializerTest.java > index 57145a5..7715d52 100644 > --- > > a/java/common/src/test/java/org/apache/shindig/common/JsonSerializerTest.java > +++ > > b/java/common/src/test/java/org/apache/shindig/common/JsonSerializerTest.java > @@ -136,6 +136,10 @@ public class JsonSerializerTest { > public Object getNullValue() { > return null; > } > + @JsonProperty("simple!") > + public void setSimpleName(int foo) { > + > + } > } > > @Test > > > On Thu, Jul 30, 2009 at 2:44 PM, Kevin Brown <[email protected]> wrote: > > > On Thu, Jul 30, 2009 at 2:29 PM, Paul Lindner <[email protected]> wrote: > > > > > How important is it to support non-bean style getters/setter method > > names? > > > If it's not a problem then we can just check for get/set before > checking > > > for JsonProperty annotations. > > > > > > Another thing we could do is check the results of > > Method#getParameterTypes > > > or Method#getReturnType calls to insure we have something that looks > like > > a > > > getter/setter > > > > > > I think it's pretty trivial: > > > > - if it takes no arguments, it's a getter. > > - if it does take arguments, it's a setter. > > > > looking at return types doesn't work because it can be ambiguous. > Consider > > this class: > > > > class Thingamjig { > > > > @JsonProperty("Foo") > > Thingamajig setFoo(Foo foo) { > > this.foo = foo; > > return this; > > } > > } > > > > > > > > > > > > > > > > > > > On Wed, Jul 29, 2009 at 3:47 PM, Cassie <[email protected]> wrote: > > > > > > > That makes sense, but unfortunately, the code is already using it in > > > > deserialization... in BeanJsonConverter, line 89-119ish, the > getSetter > > > > method calls getPropertyName which uses the annotation. > > > > > > > > So, can this be safely changed to check for a different annotation > > name, > > > > like JsonSetterProperty, while leaving the serialization code the > same? > > > > Because as it stands, its sorta broken. If no one is supposed to be > > using > > > > it, then I suppose it shouldn't affect anyone. > > > > > > > > - Cassie > > > > > > > > > > > > On Wed, Jul 29, 2009 at 3:39 PM, Kevin Brown <[email protected]> > wrote: > > > > > > > > > JsonProperty was originally only added for serialization (as > > indicated > > > by > > > > > the documentation). It could certainly be used for deserialization, > > but > > > > the > > > > > BeanJsonConverter and JsonSerializer both need to be updated > > > accordingly. > > > > > > > > > > On Wed, Jul 29, 2009 at 3:28 PM, Cassie <[email protected]> wrote: > > > > > > > > > > > Hey everyone - > > > > > > > > > > > > I have some legacy objects in my code base that I wanted to > convert > > > to > > > > > and > > > > > > from json with the BeanJsonConverter and JsonProperty. Problem > is, > > it > > > > > seems > > > > > > that the same logic/annotation is being used to find getters and > > > > setters > > > > > on > > > > > > my pojos. So when I try to convert my pojo to a string, it ends > up > > > > trying > > > > > > to > > > > > > call setters, and not getters (or vice versa). > > > > > > > > > > > > For example, I have a class Post.java with two fields "title" and > > > > "body". > > > > > > Add setters and getters for these fields and things work dandy. > > > > However, > > > > > I > > > > > > need my json to use capital case: "Title" and "Body". To do this > I > > > > figure > > > > > I > > > > > > need JsonProperty. So I go and add the JsonProperty("Title") and > > the > > > > > other > > > > > > annotation to the setters and who hoo I can take my json into a > > pojo > > > > with > > > > > > capital case. But then - on the way from pojo to json the code > > grabs > > > > all > > > > > > methods, and if they have a jsonProperty - it puts them in the > > getter > > > > > map! > > > > > > Unfortunately, this causes an IllegalArgException because of > course > > > my > > > > > > setters take params, and the code is expecting them to be > > getters... > > > > > > > > > > > > So, I'm thinking we either need a JsonGetProperty and > > JsonSetProperty > > > > > > annotation. Or, the code needs to check that a property exists > > -and- > > > > that > > > > > > the method starts with "get" or "set" or maybe there is something > > > > > better... > > > > > > > > > > > > Anyway, did I completely screw something up here? Is someone > > getting > > > > this > > > > > > to > > > > > > work correctly? > > > > > > If not, how should we fix it? > > > > > > Thanks! > > > > > > > > > > > > - Cassie > > > > > > > > > > > > > > > > > > > > >
