Representer seems to generate bogus references for null values

[GoogleCodeExporter]

I found my representer generating strange output in some situations.
I have condensed the problem down to the minimum required to make it
happen and the code is included below.

The output this generates is:

&id001 !<tag:example.com,2011:bean>
a: a
b: *id001

I'm not sure where this &id001 comes from or why "b" has been set to
point to that reference instead of just being "null".

Example code follows.

public class SnakeTest {
   private static final Tag MY_TAG = new Tag("tag:example.com,
2011:bean");

   public static void main(String[] args) {
       Bean bean = new Bean();

       bean.setA("a"); // leave b null

       Yaml yaml = new Yaml(new Representer() {
           @Override
           public Node representData(Object data) {
               if (data instanceof Bean) {
                   Bean bean = (Bean) data;
                   Map<String, Object> fields = new
LinkedHashMap<String, Object>(2);
                   fields.put("a", bean.getA());
                   fields.put("b", bean.getB());
                   return representMapping(MY_TAG, fields, false);
               } else {
                   return super.representData(data);
               }
           }
       });

       System.out.println(yaml.dump(bean));
   }

   public static class Bean {
       private String a;
       private String b;
       public String getA() { return a; }
       public void setA(String a) { this.a = a; }
       public String getB() { return b; }
       public void setB(String b) { this.b = b; }
   }
}

Original issue reported on code.google.com by trejkaz on 13 Jul 2011 at 2:14

[GoogleCodeExporter]

Scalars with the value 'null' shall not be represented with anchors

Original comment by py4fun@gmail.com on 13 Jul 2011 at 8:26

• Changed state: Accepted

[GoogleCodeExporter]

When you override method 'representData(Object data)' you do not keep its full 
functionality. We have added some example where you can see how you fix your 
code and what is the preferred way to work. See the code here:
http://code.google.com/p/snakeyaml/source/browse/src/test/java/org/yaml/snakeyam
l/issues/issue127/NullAliasTest.java

Can we close the issue ?

Original comment by py4fun@gmail.com on 13 Jul 2011 at 9:39

• Changed state: Started

[GoogleCodeExporter]

That is one hell of a gotcha.  An overriding method shouldn't need to know that 
it has to manipulate some protected field just to work correctly. :)

Perhaps representData should be final and another protected method should be 
introduced?  Or, setting that field could be done before calling 
representData...

Original comment by trejkaz on 13 Jul 2011 at 12:43

[GoogleCodeExporter]

We were thinking to introduce such a method but we do not see the need. Your 
use case can be solved in a standard way. Check how it is done in the 
test/example. No need to override anything, you just need to implement a method.
If your use case is wider then let us know. 

Original comment by py4fun@gmail.com on 13 Jul 2011 at 3:51

[GoogleCodeExporter]

In my case, the things I am representing are interfaces.  When I try to use the 
representers.put approach, this appears to pose a problem.  I gett:

Can't construct a java object for 
tag:yaml.org,2002:org.trypticon.hex.gui.notebook.DefaultNotebook; 
exception=java.lang.NoSuchMethodException: 
org.trypticon.hex.gui.notebook.DefaultNotebook.<init>()
 in "<reader>", line 1, column 1:
    !!org.trypticon.hex.gui.notebook ... 
    ^

    at org.yaml.snakeyaml.constructor.Constructor$ConstructYamlObject.construct(Constructor.java:325)
    at org.yaml.snakeyaml.constructor.BaseConstructor.constructObject(BaseConstructor.java:181)
    at org.yaml.snakeyaml.constructor.BaseConstructor.constructDocument(BaseConstructor.java:140)
    at org.yaml.snakeyaml.constructor.BaseConstructor.getSingleData(BaseConstructor.java:126)
    at org.yaml.snakeyaml.Yaml.loadFromReader(Yaml.java:296)
    at org.yaml.snakeyaml.Yaml.load(Yaml.java:290)

DefaultNotebook is the implementation class, Notebook is the interface.  From 
the module responsible for doing the storage, there is no access to the 
DefaultNotebook class as that is a private implementation - only the interfaces 
are public.

If I had some method I could override to return a Represent for a given object, 
I would have quite an elegant solution.

Original comment by trejkaz on 14 Jul 2011 at 8:14

[GoogleCodeExporter]

Just in case my comment was a bit ambiguous... the "objectToRepresent = data" 
fix works nicely.  It's the "preferred" approach which doesn't.

Original comment by trejkaz on 14 Jul 2011 at 8:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I am afraid I did not quite catch you. Your initial request is about 
Object->YAML but the last stack trace from the opposite direction YAML->Object.
If you can provide the complete test similar to what you have done before then 
we can see the problem.
It is not difficult to add the method you ask, but we would like to understand 
why.

Original comment by py4fun@gmail.com on 14 Jul 2011 at 3:39

[GoogleCodeExporter]

The stack trace fails reading the objects back in because the Constructor wants 
interfaces, not concrete classes.  It's unrelated to my initial request, but 
it's an issue with the "preferred" example you provided back in comment 2.  I 
am working with interfaces so if the representer is naively assuming that 
getClass() can be used as a lookup into a Map, it won't work.

Original comment by trejkaz on 15 Jul 2011 at 5:58

[GoogleCodeExporter]

Here's the actual source from github:

https://gist.github.com/trejkaz/hex/tree/master/main/src/main/org/trypticon/hex/
gui/notebook

The code in ExtendedRepresenter includes the one-line trick to work around this 
issue.

Original comment by trejkaz on 15 Jul 2011 at 6:04

[GoogleCodeExporter]

1) The comment in your code is very confusing. There is no bug in SnakeYAML. 
The line is required to fix _your_ code. When you override a method you must 
respect the business of the method you overwrite. 
By the way, the method is not supposed to be overridden. We may change the 
visibility (from protected to package) it in the future to avoid such a mistake.
2) Check the example I showed above. Replace all your 'represent*' methods in 
ExtendedRepresenter with the implementations on the Represent interface as it 
is done in the example.

Original comment by py4fun@gmail.com on 15 Jul 2011 at 3:05

[GoogleCodeExporter]

trejkaz: Re your comment #5, there is a proposed modification to SnakeYaml to 
let you do something very similar. The modification is in this clone: 
http://code.google.com/r/jordanangold-separate-config/source/browse

In summary, your change would be reduced to:

----
// Setting up your custom Representation handlers...
YamlConfig config = YamlConfig.createDefault();
config.getRepresenters.putExactClassRepresenter ( Bean.class, new 
MyBeanRepresent() );

// Usage, for example:
Yaml yaml = new Yaml ( config );
String output = yaml.dump ( myBean );

// ...definition of Represent handler...
class MyBeanRepresent implements Represent {
    @Override
    public Node represent ( Representer parent, Object data ) {
        Bean bean = (Bean) data;
        Map<String, Object> fields = new LinkedHashMap<String, Object>(2);
        fields.put("a", bean.getA());
        fields.put("b", bean.getB());
        return parent.representMapping(MY_TAG, fields, false);
    }
}
----

This eliminates the need to override anything in Representer, which can be 
difficult (as you found out). The Representer implementation knows how to 
select the most correct Represent instance for a given object, so all you need 
to do it give it your custom instance. You can even give it several different 
instances, each for a different kind of object.

If you are interested in this approach, I will write a Wiki article on how to 
preview this proposed modification to SnakeYaml.

Original comment by JordanAn...@gmail.com on 15 Jul 2011 at 4:50

[GoogleCodeExporter]

I agree that we may use a lot of ideas from 
'http://code.google.com/r/jordanangold-separate-config/source/browse'

But I would like that someone else would take a critical look. Anyway the 
change will not come very quick and we need to give recommendations now.
I think it would be better to disallow to override 'representData()' in the 
coming 1.9 release. Then when we migrate to another public API we do not need 
to care about too many explanations how to migrate.

Original comment by py4fun@gmail.com on 15 Jul 2011 at 5:28

[GoogleCodeExporter]

BaseRepresenter.representData(Object data) has been made 'final' to avoid 
confusion

Original comment by py4fun@gmail.com on 22 Jul 2011 at 4:59

• Changed state: Fixed