RFR (JAXP): 8033980 : Xerces Update: datatype XMLGregorianCalendarImpl and DurationImpl

[huizhe wang]

On 2/18/2014 3:39 AM, Daniel Fuchs wrote:
> On 2/17/14 11:17 PM, huizhe wang wrote:
>> Thanks Daniel.
>>
>> I updated the test to call class.getResourceAsStream (see fromFile
>> method) instead at the runtime. The filePath now is only used for the
>> helper method to generate test file. Also added readObject() to
>> XMLGregorianCalendarImpl (at the bottom). The method calls the save()
>> method to initialize the orig_* fields as done in the constructors.
>>
>> http://cr.openjdk.java.net/~joehw/jdk9/8033980/webrev/
>
> Hi Joe,
>
> The changes look good.
>
> In the serialization test I wonder if it would be better
> to simply replace:
>
>   53     {
>   54         filePath = 
> SerializationTest.class.getResource("SerializationTest.java").getPath();
>   55         int pos = filePath.lastIndexOf('/');
>   56         filePath = filePath.substring(0, pos + 1);
>   57     }
>
> with:
>
>         {
>              filePath = System.getProperty("test.src", ".");
>         }

Ok, I changed that to:

   54         filePath = System.getProperty("test.src");
   55         if (filePath == null) {
   56             //current directory
   57             filePath = System.getProperty("user.dir");
   58         }
   59         filePath += File.separator;


Thanks,
Joe

>
> best regards,
>
> -- daniel
>
>>
>> Thanks,
>> Joe
>>
>> On 2/17/2014 8:23 AM, Daniel Fuchs wrote:
>>> Hi Joe,
>>>
>>> Sorry for the late reply...
>>>
>>> On 2/14/14 6:35 PM, huizhe wang wrote:
>>>> Hi All,
>>>>
>>>> I added a SerializationTest. The test contains a helper that can
>>>> generate serialization files for XMLGregorianCalendar and Duration. 
>>>> I've
>>>> created such files for jdk6,7,8 and 9, and manually run the test, that
>>>> is, read them back with JDK6, 7, 8 and 9. The test worked fine. In the
>>>> JDK9(or 10 in the future) repo and for an auto-run, it would use the
>>>> current JDK9/10 build and test against JDK6, 7, 8 and 9. Past 
>>>> JDK10, we
>>>> could consider add serialization files for JDK10.
>>>>
>>>> The new fields did not affect serialization compatibility. The above
>>>> tests passed with/without the new fields being transient. But I added
>>>> transient since it's the right thing to do.
>>>>
>>>> Adding fields is a compatible change in accordance with Java Object
>>>> Serialization Spec
>>>> <http://docs.oracle.com/javase/7/docs/platform/serialization/spec/version.html#6678>. 
>>>>
>>>>
>>>
>>> Concerning the new test I think it would be much more
>>> reliable to open the .ser files as a resource, using
>>> something like:
>>>
>>>    InputStream streamIn = SerializationTest.class.getResourceAsStream(
>>>            javaVersion + FILENAME_CAL);
>>>    ObjectInputStream objectinputstream = new 
>>> ObjectInputStream(streamIn);
>>>
>>> The reason is that URL.getPath() is an URL path - not a file path, 
>>> so it
>>> can contain encoded characters (such as %20) that FileInputStream will
>>> not understand. I suspect that if there's a white space or some other
>>> special character in the path somewhere your current mechanism
>>> will not work.
>>>
>>> Also it would be good to verify what is the value of the transient
>>> fields after deserialization: I see that XMLGregorianCalendarImpl
>>> initializes them with non-default values - and you may need to
>>> write a readObject() to reinitialize those fields to these non
>>> default values: the deserialization framework may simply leave
>>> them with the global default for fields of that type (null
>>> for objects, 0 for numerical values,  false for boolean etc...)
>>> [I am not 100% sure but it will not hurt to check]
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>>
>>>>
>>>> Thanks,
>>>> Joe
>>>>
>>>> On 2/13/2014 6:23 AM, Alan Bateman wrote:
>>>>> On 13/02/2014 08:18, huizhe wang wrote:
>>>>>> Hi Alan, Lance, and Daniel,
>>>>>>
>>>>>> The Xerces serialization revision meant to create a serialization
>>>>>> form that would help maintain future serialization compatibility. 
>>>>>> But
>>>>>> in reality it itself is causing significant incompatibility as Alan
>>>>>> pointed out below and we discussed previously. I've removed the
>>>>>> revision from the patch as a result.
>>>>>>
>>>>>> Please see the new webrev here:
>>>>>> http://cr.openjdk.java.net/~joehw/jdk9/8033980/webrev/
>>>>> Thanks for dropping the serialization change as it was just not going
>>>>> to work the way you had intended.
>>>>>
>>>>> I agree with Daniel's comment about all the new fields added to
>>>>> XMLGregorianCalendarImpl as it's not clear why they aren't transient.
>>>>>
>>>>> I have not studied the rest of the changes but I think Daniel and
>>>>> Lance are.
>>>>>
>>>>> -Alan
>>>>
>>>
>>
>