JsonOutput.toJson(object) is not returning expected results

Yes. I think JsonOutput.toJson(Object) should be generally in sync with object.dump(), which it is not currently.

@James... if you use a visibility modifier in a declaration for a property, then you actually do not declare a property, but a field only. So if you do "public String bar", then you don't have a bar property, but a bar field. If you do "String bar", then you get a bar property.

And a private getter makes a property?

Wouldn't it make sense if JsonOutput.toJson(Object) included public fields too?

the visibility of the getter is not included in determining if something is a property or not.

When this was initially implemented, the idea was to use properties, not public fields or anything else.
Also, I think only public properties should be used for that JSON serialization.
So according to that logic and concern, I think with your example that foo should be serialized as {} and bar as {"bar":"foo"} – ie. only public getters used for the serialization.

>> the visibility of the getter is not included in determining if something is a property or not.

Didn't mean in the usual "property" sense. I meant outputting a private getter in a JSON looks odd (and public fields are being ignored). As Guillaume seems to agree, only public getters should be used for JSON serialization.

From http://groovy.codehaus.org/Groovy+Beans – "In Groovy, fields and properties have been merged so that they act and look the same."

Given that statement, I still think these objects should serialize as I outlined.

probably(i am not sure) we need to do changes in MetaClassImpl.java.

I have made some changes and its working with me(its giving output as Mr. James said).

what am i supposed to do now? to whom do i need to submit the change for review ?

how do i run the test cases ? to whom i need to send the patch ?

you can attach the patch here. It would help to check if the test can be applied to the code base without any test failing.

@umang bhatt:
Could you attach your patch?

I am a beginner so please review my code. Also i have not run any tests yet.

I have changed public List<MetaProperty> getProperties() method.

I have tried fixing the issue. The order is a little different than what is desired.
I can change the behavior as you say(eg include the value that we get from the getter/accesor method).

I have attached the file that contains changes. I have also attached a screenshot.

Please always create a minimal patch file, just attaching the new file does not help much in judging if the change should be done or not. I created the patch for you, plus made some formatting changes to minimize the patch

patch

Part of the patch is puzzling me before we had (!setter && !getter) to filter out properties that are in fact fields and thus have neither getter or setter. What is kept are read- or write-only properties. With your (!setter || !getter) those are removed as well. I can understand from the JSON perspective, but for getProperties this looks wrong.

Next is the element.getModifiers() != 1, which seems to be there to filter out all non public fields. Since we filtered out all fields before already, thus is surely ok in principle, but you don't do something like that for MetaBeanProperties. They can be protected or package private as well imho. Why should getProperties show a protected MetaBeanProperty, but not a protected CachedField?

My Conclusion:
Changing getProperties to return many more is a potentially breaking change, so it surely wouldn't go into 1.8.x or 2.0.x. I think for 2.1.x it is too late. We have to investigate though how that method is used before we can say that it goes into 2.2, otherwise the next possible version would be 3.0 only. I would suggest finding another way to tackle the problem and keep the changes to the json classes itself if possible.

ok.
1. I would try to make changes in json related class, should it be fine ?
2. Do i need to change the behavior(output) in any way ? Or this is fine ?

First of all, before doing any further change to your patch, let's agree on what should be serialized or not.

• public properties: Yes (ie. both final and non final)
• public fields: Yes (both final and non final)
• static properties, static fields, whether public, protected or private: No

here is a new patch that does the same thing by adding a method in JsonOutput class

hmm... CachedField is only an internal representation for fields. That means the patch should include only fields that are public. But the usual property is one with getter and setter and private field. In the json_test.groovy attached above this would imho mean that Bar#bar is excluded since it is a property backed by a private field.

Also there is the question to answer if a property that consists only of a getter or setter and no field should be included. And dynamically added properties are up to discussion as well I think.

"Also there is the question to answer if a property that consists only of a getter or setter and no field should be included. And dynamically added properties are up to discussion as well I think."

When I originally filed the bug, and this still hold true for me, I assumed that the instance of the object would be serialized in such a fashion that it could be recreated from the JSON. That would imply dynamically added properties should be included in the serialization. Otherwise, what are you serializing?

but if you want this to recreate the object afterwards, then read-only-properties are going to be a problem for you - plus dynamically added properties will only be available if they have been added on the deserialization side as well.

Well, since Groovy doesn't deserialize JSON to an object anyway (that I can tell) that's a moot point. It would be left up to the deserializer as to how to handle that.

By the way, back when I filed this bug I ended up resorting to some ridiculousness to get around it – http://stackoverflow.com/questions/7853809/iterating-an-objects-own-properties-in-groovy