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