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.

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
> >>>>
> >>>>
> >
>
>

Reply via email to