On Mon, Jan 27, 2020 at 7:58 PM Gary Gregory <garydgreg...@gmail.com> wrote:
> On Mon, Jan 27, 2020 at 6:54 PM Alex Herbert <alex.d.herb...@gmail.com>
> wrote:
>
>> I’ve had a look at the Serialization of CSVRecord. Fields have been added
>> and removed from CSVRecord as:
>>
>> 1.0
>>
>> /** The accumulated comments (if any) */
>> private final String comment;
>>
>> /** The column name to index mapping. */
>> private final Map<String, Integer> mapping;
>>
>> /** The record number. */
>> private final long recordNumber;
>>
>> /** The values of the record */
>> private final String[] values;
>>
>> 1.1
>>
>> // Added
>>
>> private final long characterPosition;
>>
>> 1.2 to 1.6
>>
>> No changes
>>
>> 1.7
>>
>> // Removed
>>
>> /** The column name to index mapping. */
>> private final Map<String, Integer> mapping;
>>
>> // Added a non-serialisable field -> broken
>>
>> /** The parser that originates this record. */
>> private final CSVParser parser;
>>
>> 1.8
>>
>> // Fixed by marking as transient
>> private final transient CSVParser parser;
>>
>>
>> If you write and read records in 1.8 the parser is not serialised. This
>> contains the map between column names and indices. So the following methods
>> are affected:
>>
>> boolean CSVRecord.isMapped(String);
>> boolean CSVRecord.isSet(String);
>> Map<String, String> CSVRecord.toMap();
>> String CSVRecord.get(String);
>>
>> The original object would have valid returns for these. A deserialised
>> version returns false for all map keys, the map representation is an empty
>> map and get will throw as access by name is not supported.
>>
>>
>> You can read records from 1.0 in using 1.8. It ignores the map that was
>> serialised as part of the record. It also ignore the missing character
>> position field and assigns it a default value of 0.
>>
>>
>> Likewise you can read records from 1.8 in using 1.0. Here the map is
>> missing and so all functionality linked to the map fails. The failures are
>> exactly as for reading 1.0 in to 1.8.
>>
>>
>> So this indicates you can serialise and deserialise between different
>> versions back to 1.0 excluding 1.7. Deserialisation may not create records
>> entirely. The following lists what should work with no loss of
>> functionality in the destination version:
>>
>> Serialised from => deserialised to
>>
>> 1.0 => 1.0
>> 1.1 - 1.6 => 1.0 (the character position field is ignored)
>> 1.1 - 1.6 => 1.1 - 1.6
>>
>> The following will result in absent information:
>>
>> 1.0 => 1.1 - 1.6 (no character position)
>> 1.0 => 1.8 (no character position, no mapping functionality)
>> 1.1 - 1.6 => 1.8 (no mapping functionality)
>> 1.8 => 1.0 - 1.8 (no mapping functionality)
>>
>> 1.7 is ignored as it cannot be serialised
>>
>>
>> So even though you can deserialise between versions without an exception
>> the functionality is impaired in certain cases, i.e. when jumping between
>> 1.0-1.6 and 1.8. Since deserialisation can create an instance a change to
>> the serial version ID is not warranted. The compatibility should be noted
>> in the documentation, e.g. something like:
>>
>> * <p>
>> * Note: Support for {@link Serializable} is scheduled to be removed in
>> version 2.0.
>> * In version 1.8 the mapping between the column header and the column
>> index was
>> * removed from the serialised state. The class maintains serialization
>> compatibility
>> * with versions pre-1.8 for the record values; these must be accessed by
>> index
>> * following deserialization. There will be loss of any functionally
>> linked to the header
>> * map when transferring serialised forms pre-1.8 to 1.8 and vice versa.
>> * </p>
>>
>> Thoughts on this?
>>
>
> +1 to updating the docs.
>
Thanks for the update Alex. Do we want anything else for RC2?
Gary
>
> Gary
>
>
>>
>> Alex
>>
>> > On 25 Jan 2020, at 23:02, Alex Herbert <alex.d.herb...@gmail.com>
>> wrote:
>> >
>> >
>> >
>> >> On 25 Jan 2020, at 22:05, Gary Gregory <garydgreg...@gmail.com> wrote:
>> >>
>> >> Hi Alex,
>> >>
>> >> Is there more you'd like to do for 1.8?
>> >>
>> >
>> > Not functionality. The remaining things are the documentation of:
>> >
>> > - the intent to remove Serialisation support to the CSVRecord in 2.0
>> > - the intent to not support Serialisation of additional fields added
>> after from 1.6
>> >
>> > This is for the CSVRecord class header and to the release notes in
>> changes.xml.
>> >
>> > We have the test to show that you can deserialise from 1.6. I think
>> that I should change the .bin serialised file to the version from 1.0. Thus
>> the test demonstrates serialisation compatibility with the original
>> version. Adding 1.1 through 1.6 could be done although I do not see the
>> need. No fields were added until 1.7.
>> >
>> > WDYT?
>> >
>> >
>> >> Gary
>> >>
>> >>
>> >> On Fri, Jan 24, 2020 at 12:36 PM Gary Gregory <garydgreg...@gmail.com>
>> >> wrote:
>> >>
>> >>> On Fri, Jan 24, 2020 at 11:09 AM sebb <seb...@gmail.com> wrote:
>> >>>
>> >>>> On Fri, 24 Jan 2020 at 15:05, Alex Herbert <alex.d.herb...@gmail.com
>> >
>> >>>> wrote:
>> >>>>>
>> >>>>>
>> >>>>> On 24/01/2020 13:34, Gary Gregory wrote:
>> >>>>>> On Fri, Jan 24, 2020 at 6:14 AM sebb <seb...@gmail.com> wrote:
>> >>>>>>
>> >>>>>>> On Thu, 23 Jan 2020 at 18:10, Alex Herbert <
>> alex.d.herb...@gmail.com
>> >>>>>
>> >>>>>>> wrote:
>> >>>>>>>>
>> >>>>>>>> On 23/01/2020 13:55, sebb wrote:
>> >>>>>>>>> I think we don't want temporary serialisation fixes to encourage
>> >>>> the
>> >>>>>>>>> use of serialisation.
>> >>>>>>>>>
>> >>>>>>>>> So I suggest that the Release Notes and Javadoc should point out
>> >>>> that
>> >>>>>>>>> although serialisation is possible, it is not fully supported,
>> and
>> >>>>>>>>> that there are plans to drop all serialisation support.
>> >>>>>>>> The javadoc for the new field that is not serialized have been
>> >>>>>>>> documented. This current code is able to deserialize a record
>> >>>> created
>> >>>>>>>> using version 1.0 and 1.6. I did not test the in between
>> releases.
>> >>>>>>>> Serialisation broke in 1.7.
>> >>>>>>>>
>> >>>>>>>> Should a note be added to the header for CSVRecord stating that
>> the
>> >>>>>>>> class is serialization compatible with version 1.0 - 1.6. Fields
>> >>>> added
>> >>>>>>>> after version 1.6 are not serialized and the intension is to
>> remove
>> >>>>>>>> serialisation support in version 2.0.
>> >>>>>>>>
>> >>>>>>>> WDYT?
>> >>>>>>> LGTM (apart from some spelling issues!)
>> >>>>>>>
>> >>>>>>> However, I think it's worth noting in the Release Notes as well.
>> >>>>>>>
>> >>>>>> I'm OK with what Sebb said.
>> >>>>>>
>> >>>>>> Gary
>> >>>>>
>> >>>>> OK. I'll update the description in the changes.xml for this release
>> >>>>> (which IIUC become the release notes)
>> >>>>
>> >>>> This is not automatic, but yes, changes.xml can be used to generate
>> the RN
>> >>>>
>> >>>
>> >>> I use the changes.xml file to generate the RNs.
>> >>>
>> >>> Gary
>> >>>
>> >>>
>> >>>>
>> >>>>> and javadoc the CSVRecord in the
>> >>>>> class header.
>> >>>>
>> >>>> Thanks!
>> >>>>
>> >>>>> Alex
>> >>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>>>> Alex
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>> ---------------------------------------------------------------------
>> >>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> >>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
>> >>>>>>>>
>> >>>>>>>
>> ---------------------------------------------------------------------
>> >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> >>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
>> >>>>>>>
>> >>>>>>>
>> >>>>>
>> >>>>>
>> ---------------------------------------------------------------------
>> >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> >>>>> For additional commands, e-mail: dev-h...@commons.apache.org
>> >>>>>
>> >>>>
>> >>>> ---------------------------------------------------------------------
>> >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> >>>> For additional commands, e-mail: dev-h...@commons.apache.org
>> >>>>
>> >>>>
>> >
>>
>>
