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

[Daniel Fuchs]

Thanks Joe,

+1

There's a small alignment issue in SerializationTest.java line 152.

Also the

    if (condition) {
    }
    else {
    }

formatting in the source files is bizarre, but if it can help with
future merges I have no issues.

No need to regenerate a webrev.

-- daniel

On 2/18/14 6:42 PM, huizhe wang wrote:
>
> 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
>>>>>
>>>>
>>>
>>
>