Java Beans and aliases don't play nicely

[GoogleCodeExporter]

If an entry with a tag that specifies using a custom Java Bean also has an 
alias, the custom tag is lost when dumping the file.  Steps to reproduce:

1. Create a YAML document referring to a custom Java Bean and with an alias, 
e.g.

myBean: !!com.my.bean.MyBean &id001 {}
anotherBean: !!com.my.bean.AnotherBean
  myBeanRef: *id001

2. Load and dump the YAML.

3. Notice that the type tag for MyBean has been lost, i.e.:

myBean: &id001 {}
anotherBean: !!com.my.bean.AnotherBean
  myBeanRef: *id001

This happens when running SnakeYAML 1.8 on Java 1.6.

It appears that the culprit is Representer.checkGlobalTag() line 188, where it 
overwrites the JavaBean type tag with a MAP tag.

Original issue reported on code.google.com by gilea...@gmail.com on 15 Apr 2011 at 6:37

[GoogleCodeExporter]

Whoops, after some looking this issue is a bit more subtle than represented 
above: it requires typed collections to be in the mix.  So the following is 
what should trigger the bug:

myBean: !!com.my.bean.MyBean &id001 {}
anotherBean: !!com.my.bean.AnotherBean
  myBeanList:
  - *id001

And this is what would be outputted:

myBean: &id001 {}
anotherBean: !!com.my.bean.AnotherBean
  myBeanList:
  - *id001

I think the fix in Representer.checkGlobalTag() might be to only set the tag to 
MAP if the given object is not already in the representedObjects map, i.e. if 
the object isn't aliased.

Original comment by gilea...@gmail.com on 15 Apr 2011 at 6:57

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Hi,
the patch you have provided causes a few test failures.
I think it is better to continue as following:
1) take the latest source and run the tests [mvn clean test] to be sure they 
succeed
2) write a failing test for your usecase
3) change the source to make ALL the tests happy 
(http://code.google.com/p/snakeyaml/wiki/Developing)
4) provide the result

Then we can see the problem (exposed in your test) and the proposed solution
-
Andrey

Original comment by py4fun@gmail.com on 17 Apr 2011 at 12:45

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Here's a patch that shows the failing use case.  As you can see from the test, 
type information is lost after dumping and loading.  You might argue that I 
should use a JavaBeanLoader instead of using yaml.load(), but as it happens my 
real-world use case requires a map with a few strongly typed Java Beans in it, 
but is mostly just a map of maps.

My fix does break some existing tests, but in each case it's a "prettiness" 
test, i.e. it fails to remove some type information in a way that preserves 
safety over the beauty of the generated YAML.  I argue that safety trumps 
prettiness.

One possible change is to add a new dumper option to strip bean tags from 
aliased beans, or in other words an option to remove the behavior of my fix.  
However if such an option exists it should not be the default.  I believe that 
creating such an option goes beyond a bug fix into the realm of library design, 
so for now I plan to just submit the two patches and let you decide what's best 
to do.

Original comment by gilea...@gmail.com on 18 Apr 2011 at 4:53

• Added labels: ****
• Removed labels: ****

Attachments:

• Issue114Test.patch

[GoogleCodeExporter]

thx. I think your test case revealed interesting bug in the library.
I would expect the same behavior as you, but...

As Andrey mentioned, your patch breaks some test (to be exact 6).
But after some close look I would say it breaks 2 (testTag & testTag2), but 
exposes bug in others.

But I need to discuss it with Andrey to be sure (I might miss something).

Original comment by alexande...@gmail.com on 19 Apr 2011 at 6:44

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

It turns out that the correct fix was on line 74 of 
BaseRepresenter.representData() to add a withCheckedTag entry if the node 
already exists in the representedObjects map.  This keeps aliased objects from 
getting their type tag be replaced with a plain MAP tag.  Patch attached.

Original comment by gilea...@gmail.com on 15 Apr 2011 at 10:19

• Added labels: ****
• Removed labels: ****

Attachments:

• JavaBeanSequenceAlias.patch

[GoogleCodeExporter]

I have put the test into the source repository 
(http://code.google.com/p/snakeyaml/source/browse/src/test/java/org/yaml/snakeya
ml/issues/issue114/PreserveTypeTest.java)

As you can see it depends on the order. (when the collection is first there is 
no problem)

Original comment by py4fun@gmail.com on 19 Apr 2011 at 7:42

• Changed state: Accepted
• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Confirmed that your fix works for my real-world example.  Thanks!

Original comment by gilea...@gmail.com on 20 Apr 2011 at 3:36

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

The fix will be delivered in version 1.9

Original comment by py4fun@gmail.com on 20 Apr 2011 at 5:05

• Changed state: Fixed
• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

The fix is available in source or in 1.9-SNAPSHOT (Sonatype repository: 
http://code.google.com/p/snakeyaml/#Download_and_Installation)

Please test it.

Original comment by py4fun@gmail.com on 20 Apr 2011 at 8:55

• Changed state: Started
• Added labels: ****
• Removed labels: ****