ilmoeuro / snakeyaml

Automatically exported from code.google.com/p/snakeyaml
0 stars 0 forks source link

Representer.java:208 should make sure property.getWriteMethod() != null as well as getReadMethod #47

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Dump a JavaBean with a getValue but no setValue
2. Try to load resulting Yaml. 

If there's no method to write the value at load time, it probably shouldn't 
dump that value at dump 
time either. 

Original issue reported on code.google.com by obast...@gmail.com on 11 Feb 2010 at 9:51

GoogleCodeExporter commented 9 years ago
This requirement is application specific.
Java->YAML->Java transformation with the same context indeed does not need to 
dump 
these properties.
There are other cases like data exchange with other languages and different 
application with another JavaBean.

YAML is not only used as a serialization format. There is no requirement to 
re-create 
exactly the same instance.

The classes you are talking about are most probably not JavaBeans. 
Unfortunately 
complex structures require more configuration.

Feel free to propose a general solution which can help to improve the situation.

Original comment by py4fun@gmail.com on 12 Feb 2010 at 8:18

GoogleCodeExporter commented 9 years ago
Should we combine this issue with #48 ?
(exclude some JavaBean properties at runtime)

Original comment by py4fun@gmail.com on 12 Feb 2010 at 9:14

GoogleCodeExporter commented 9 years ago
No, it seems like it should be in that method. You're already checking to make 
sure that there's a read method, it 
only makes sense to check that there's a write method as well. 

Original comment by obast...@gmail.com on 12 Feb 2010 at 3:29

GoogleCodeExporter commented 9 years ago
Perhaps an acceptable solution is that to support Java->YAML->Java, that the 
YAML loader would simply ignore 
those properties instead of throwing an exception? Perhaps as a loader option?

Original comment by obast...@gmail.com on 12 Feb 2010 at 3:31

GoogleCodeExporter commented 9 years ago
I am not in favor to introduce a setting to ignore things. Once we open the 
door to 
ignore nonsense how do we know where to stop ?
Then someone would propose ignore invalid anchors, invalid tags etc.

I still think that the solution must be in the application. Additional code is 
required. Especially taking into account that this is NOT a JavaBean.

Original comment by py4fun@gmail.com on 12 Feb 2010 at 5:21

GoogleCodeExporter commented 9 years ago
You keep saying that, but to define a read-only property all you need do is NOT 
have a setter. See last line 
below:

Section 8.3.1 of the JavaBean spec:

By default, we use design patterns to locate properties by looking for methods 
of the form:

public <PropertyType> get<PropertyName>(); 
public void set<PropertyName>(<PropertyType> a);

If we discover a matching pair of “get<PropertyName>” and 
“set<PropertyName>” methods that take and 
return the same type, then we regard these methods as defining a read-write 
property whose name will be 
“<propertyName>”. We will use the “get<PropertyName>” method to get the 
property value and the 
“set<PropertyName>” method to set the property value. The pair of methods 
may be located either in the 
same class or one may be in a base class and the other may be in a derived 
class.

If we find only one of these methods, then we regard it as defining either a 
read-only or a write- only 
property called “<propertyName>”

Original comment by obast...@gmail.com on 12 Feb 2010 at 6:49

GoogleCodeExporter commented 9 years ago
Let me restate my request:

Technically, attempting to "set" a read-only property on a JavaBean is a defect 
in the loader because having a 
read-only property is perfectly legal for JavaBeans. 

Two possible solutions:

1. Dump read-only properties, but don't call the setter on them when loading.

2. Don't dump read-only properties. 

#2 is easiest, because all you have to do is add:

&& property.getWriteMethod() != null

after

property.getReadMethod() != null. 

But I'm ok either way. 

Original comment by obast...@gmail.com on 12 Feb 2010 at 10:41

GoogleCodeExporter commented 9 years ago
I tried to get any feedback from the mailing list but without any success :)

I think I will go to your second proposal since it is the simplest. Feel free to
contribute code if you need it quicker.

Original comment by aso...@gmail.com on 18 Feb 2010 at 8:52

GoogleCodeExporter commented 9 years ago
I've always thought #2 was the right thing to do. 

Original comment by obast...@gmail.com on 18 Feb 2010 at 5:58

GoogleCodeExporter commented 9 years ago
I have pushed the change to a clone repository:
http://code.google.com/r/py4fun-calendar/source/detail?
r=cbf0784b24cb56e7ba38968807dbcf1040e141ab

This is the new test:
http://code.google.com/r/py4fun-
calendar/source/browse/src/test/java/org/yaml/snakeyaml/issues/issue47/ReadOnlyP
roper
tiesTest.java?
spec=svncbf0784b24cb56e7ba38968807dbcf1040e141ab&r=cbf0784b24cb56e7ba38968807dbc
f1040
e141ab

By default read-only properties are not emitted. DumperOptions gets a setting 
to 
include read-only JavaBean properties. This is no backwards compatible change.

Can you please test it ?

Original comment by py4fun@gmail.com on 19 Feb 2010 at 4:00

GoogleCodeExporter commented 9 years ago
it will be included in the 1.6 release

Original comment by aso...@gmail.com on 24 Feb 2010 at 8:38

GoogleCodeExporter commented 9 years ago
Awesome. You rock. Not doing read-only properties is definitely working. 

Original comment by obast...@gmail.com on 24 Feb 2010 at 10:22