NorwegianDateUtil.isHoliday does not handle non-Europe/Oslo timezones (e.g. jvm default timezone)

[torgeir]

A test similar to the following fails on my end:

    @Test
    public void midnight_may17_is_a_norwegian_holiday_even_if_the_jvm_timezone_is_gmt() {
        System.setProperty("user.timezone", ZoneId.of("GMT").getId());

        ZonedDateTime may17 = 
            ZonedDateTime.parse("2018-05-17T00:00:00+02:00")
                .withZoneSameInstant(ZoneId.of("Europe/Oslo"));

        assertTrue(NorwegianDateUtil.isHoliday(Date.from(may17.toInstant())));
    }

I would argue that the 17th of may is a norwegian holiday even if the jvm timezone is GMT 😉

[eivinhb]

But you ask isHoliday with an date at 16.05.2018 22:00. How would you argue that its the date utils fault when you give it a date for the previous day?

[eivinhb]

I would be great to rework the api in NoCommons to use java.time.

[torgeir]

Because its the first hour of the 17th of may in norwegian time! as shown in the created ZonedDateTime in the test (i.e. when the clock is 16.05.2018 22:00 in London, norwegians would be celebrating the 17 of May)

[eivinhb]

But you enter what is equal to new Date(1526508000000L) and that is may 16th. So if you sendt that to the Calendar du get, it will say that its May 16.

If you can spot a code change in NoCommons, please do not hesitate to create a PR. :)

[eivinhb]

ZonedDateTime may17 = ZonedDateTime.parse("2018-05-17T00:00:00+02:00").withZoneSameInstant(ZoneId.of("Europe/Oslo")); 
// 2018-05-17T00:00+02:00[Europe/Oslo]

Instant instant = may17.toInstant(); // 2018-05-16T22:00:00Z
Date from = Date.from(instant); // Wed May 16 22:00:00 GMT 2018
assertTrue(NorwegianDateUtil.isHoliday(from)); // false

[runeflobakk]

new Date(1526508000000L) is not May 16 in Norway. The problem is that Date is only a container for millis since epoch, similar to Instant, and when converting it to some "human readable form", one must be very careful to provide the TimeZone.

To resolve anything involving the years, days, and so on from milliseconds from epoch, one must provide a TimeZone. If the API permits you to omit the TimeZone, some assumption must be made. The toString() you see in the comments in #30 (comment) must assume a time zone, and probably reaches for the JVM time zone user.timezone.

Here is a little more involved code example:

		ZoneId norwayZone = ZoneId.of("Europe/Oslo");
		long epochMillis = 1526508000000L;

		ZonedDateTime java8DateTime = Instant.ofEpochMilli(epochMillis).atZone(norwayZone);
		System.out.println(java8DateTime);  // 2018-05-17T00:00+02:00[Europe/Oslo]

		Date crapDate = new Date(epochMillis);
		Calendar crapCalendar = Calendar.getInstance(TimeZone.getTimeZone(norwayZone));
		crapCalendar.setTime(crapDate);
		SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm");
		format.setTimeZone(TimeZone.getTimeZone(norwayZone));
		System.out.println(format.format(crapCalendar.getTime()));  // 2018-05-17 00:00

Damn, I hate that old Date/Calendar API with a passion!

[torgeir]

My expectations here seem to be off with how this is actually implemented 😄

If this was a lib you queried for if a date was the 17th of may I would agree!

As this is a lib you query for whether a given date is a norwegian holiday or not, I expected the test I provided to pass, with the argument being that even if I bring my program over to the UK where the default jvm timezone would be GMT, the same point in linear time is still a norwegian holiday. But I see now that the code does not really take time zones into consideration. This is a pain with the old Date apis, and would probably be a lot easier to handle with the new java.time apis.

[runeflobakk]

I would argue that if I am in UK, it is 10PM on the 16th of May, and I query "is now a holiday in Norway", I would expect the answer to be "yes". So I agree with @torgeir's expectations.

Ignoring any Einsteinian relativistic effects 😄

[eivinhb]

Exactly, if you run a user.timezone on jvm as GMT, then new Date(1526508000000L) IS may 16. And that is all that is checked. The api needs to have more than a long to work with.
NoCommons need pre adjusted dates.

[runeflobakk]

Yes, 1526508000000L is May 16 in GMT, but that is not relevant, because you are asking if 1526508000000L is a Norwegian holiday. I would expect the time zone to be implicitly "Europe/Oslo", or any equivalent, since the library is about Norwegian temporal events.

If I am calling someone in Norway from UK on May 16 10PM, and asks her if now is a holiday, I would get "yes" back.

If I have some kind of application with a rule that says "never bother Norwegians in their holidays", and I run that application on the other side of the planet, I would expect that application not to send any SMSs during when Norway celebrates their constitution day.

[torgeir]

Put another way;

As someone living abroad, you are of course entitled to celebrate the norwegian constitution day whenever your country reaches the 17th of the month, but that does not change when Norwegians at home celebrate their holiday.

[eivinhb]

I do see your point. But its not designed that way. Put simply: if the toString of the date you ask with is 16th of may, then it probably is read as that and not as any thing else. 🤷‍♂️

edit:
I think you both demand too much into what this is as it sits to day. I'm not sure how this is supposed to be used, but I guess that you can make a calendar and given a date can check if that is a holiday or not. It can for instance count working days between two dates. But I think that this is designed naive and use only one timesone.

Sidenote:
I have never actually worked at a shop that have full control over timezones and need to have. And java.util.Date, sql DATETIME, and javascript Date has to be a good reason why this is a problem.

Then lets write an api that is capable of doing that. But then we need to ditch Date from util and sql and use java.time.

[mlundela]

We had issues with timezones as well, and decided to implement one that use LocalDate:

import java.time.DayOfWeek;
import java.time.LocalDate;
import java.time.Month;
import java.util.HashSet;
import java.util.Set;

public class Calendar {

    static boolean isWorkingDay(LocalDate date) {
        DayOfWeek day = date.getDayOfWeek();
        if (day == DayOfWeek.SATURDAY || day == DayOfWeek.SUNDAY) return false;
        return !holidays(date.getYear()).contains(date);
    }

    private static Set<LocalDate> holidays(int year) {
        Set<LocalDate> out = new HashSet<>();

        // Fixed holidays
        out.add(LocalDate.of(year, Month.JANUARY, 1));
        out.add(LocalDate.of(year, Month.MAY, 1));
        out.add(LocalDate.of(year, Month.MAY, 17));
        out.add(LocalDate.of(year, Month.DECEMBER, 25));
        out.add(LocalDate.of(year, Month.DECEMBER, 26));

        // Variable holidays
        LocalDate easterDay = getEasterDay(year);
        out.add(easterDay.minusDays(7)); // Sunday before Easter
        out.add(easterDay.minusDays(3)); // Thursday before Easter
        out.add(easterDay.minusDays(2)); // Friday before Easter
        out.add(easterDay); // Easter day
        out.add(easterDay.plusDays(1)); // Second Easter day
        out.add(easterDay.plusDays(39)); // Kristi himmelfart
        out.add(easterDay.plusDays(49)); // Pinse
        out.add(easterDay.plusDays(50)); // Pinse

        return out;
    }

    private static LocalDate getEasterDay(int year) {
        int a = year % 19;
        int b = year / 100;
        int c = year % 100;
        int d = b / 4;
        int e = b % 4;
        int f = (b + 8) / 25;
        int g = (b - f + 1) / 3;
        int h = (19 * a + b - d - g + 15) % 30;
        int i = c / 4;
        int k = c % 4;
        int l = (32 + 2 * e + 2 * i - h - k) % 7;
        int m = (a + 11 * h + 22 * l) / 451;
        int v = h + l - 7 * m + 114;
        int n = v / 31;
        int p = v % 31;
        return LocalDate.of(year, n, p + 1);
    }
}