Guard DateConverter against null

[pingpingy1]

When null is passed to the DateConverter constructor, NullPointerException will eventually be thrown. However, since the constructor is lazily evaluated, this is not thrown until much later, leading to difficult debugging. This commit implements a guard against null input, throwing an IllegalArgumentException.

[jungm]

Generally looks good to me, but fails checkstyle execution in mvn checkstyle:check:

[ERROR] /home/markus/tmp/johnzon-pingpingy1/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/converter/DateConverterTest.java:3:1: Redundant import from the same package - org.apache.johnzon.mapper.converter.DateConverter. [RedundantImport]
[ERROR] /home/markus/tmp/johnzon-pingpingy1/johnzon-mapper/src/test/java/org/apache/johnzon/mapper/converter/DateConverterTest.java:9:1: File contains tab characters (this is the first instance). [FileTabCharacter]

[rmannibucau]

now we moved the minimum java version we can probably move to DateTimeFormatter and drop the threadlocal (just need to maintain some pattern compatibility but it is almost the same format IIRC) so then we can just leverage the already existing validations and get rid of this dirty threadlocal no? wdyt?

[pingpingy1]

Fixed the checkstyle issue for the moment, in case it might be of help. I don't consider myself wholly qualified for conducting the transition you mentioned, but it sounds good IMHO.

[rmannibucau]

In the spirit (date parsing can be enhanced) - this is what we do for JSON-B already for ex:

public class DateConverter implements Converter<Date> {
    private static final ZoneId UTC = ZoneId.of("UTC");

    private final DateTimeFormatter formatter;

    public DateConverter(final String pattern) {
        this.formatter = ofPattern(pattern, ROOT);
    }

    @Override
    public String toString(final Date instance) {
        return formatter.format(ZonedDateTime.ofInstant(instance.toInstant(), UTC));
    }

    @Override
    public Date fromString(final String text) {
        try {
            return Date.from(parseZonedDateTime(text, formatter).toInstant());
        } catch (final DateTimeParseException dpe) {
            return Date.from(LocalDateTime.parse(text).toInstant(ZoneOffset.UTC));
        }
    }

    private static ZonedDateTime parseZonedDateTime(final String text, final DateTimeFormatter formatter) {
        final TemporalAccessor parse = formatter.parse(text);
        ZoneId zone = parse.query(TemporalQueries.zone());
        if (Objects.isNull(zone)) {
            zone = UTC;
        }
        final int year = parse.isSupported(YEAR) ? parse.get(YEAR) : 0;
        final int month = parse.isSupported(MONTH_OF_YEAR) ? parse.get(MONTH_OF_YEAR) : 0;
        final int day = parse.isSupported(DAY_OF_MONTH) ? parse.get(DAY_OF_MONTH) : 0;
        final int hour = parse.isSupported(HOUR_OF_DAY) ? parse.get(HOUR_OF_DAY) : 0;
        final int minute = parse.isSupported(MINUTE_OF_HOUR) ? parse.get(MINUTE_OF_HOUR) : 0;
        final int second = parse.isSupported(SECOND_OF_MINUTE) ? parse.get(SECOND_OF_MINUTE) : 0;
        final int millisecond = parse.isSupported(MILLI_OF_SECOND) ? parse.get(MILLI_OF_SECOND) : 0;
        return ZonedDateTime.of(year, month, day, hour, minute, second, millisecond, zone);
    }
}

Checked the pattern table and formatter seems to be a superset of simple dateformat so we shouldn't be too bad. Once applied there are a few failures but mainly around the error messages so test can be "fixed".

Do you want to try that path?

[jungm]

@pingpingy1, want to take care of this PR as well? :slightly_smiling_face:

[jungm]

superseded by 841ebb3a8627f6ae617eb1deec013f46678969b8