Skip to content
This repository has been archived by the owner on Apr 16, 2022. It is now read-only.

Briefcase is not respecting the daylight savings time offset when exporting (date-) time fields. #19

Closed
getodk-bot opened this issue Nov 23, 2016 · 4 comments
Labels
bug housekeeping Issue tagged for housekeeping revision needs reproduction

Comments

@getodk-bot
Copy link
Member

Issue by mitchellsundt
Thursday Jul 09, 2015 at 19:44 GMT
Originally opened as getodk/getodk#1075 (1 comment(s))


Originally reported on Google Code with ID 1074

What steps will reproduce the problem?
1. Fill up a form that will include "07:00:00.000Z" in some time field.
2. Export the data from the Briefcase using a computer that is (for example) in EET
time zone (+2) and the current date is such that it also has one hour offset for daylight
saving.   

What is the expected output? What do you see instead?
The expected output is "10:00 AM" but the actual output is "09:00". 

What version of the product are you using? On what operating system?
Any Briefcase under JRE 6. 

Please provide any additional information below.
The problem is lying under the getDate() function inside DateUtils.java (JavaRosa):


    public static Date getDate (DateFields f, String timezone) {
        Calendar cd = Calendar.getInstance();
        if(timezone != null) {
            cd.setTimeZone(TimeZone.getTimeZone(timezone));
        }
        cd.set(Calendar.YEAR, f.year);
        cd.set(Calendar.MONTH, f.month - MONTH_OFFSET);
        cd.set(Calendar.DAY_OF_MONTH, f.day);
        cd.set(Calendar.HOUR_OF_DAY, f.hour);
        cd.set(Calendar.MINUTE, f.minute);
        cd.set(Calendar.SECOND, f.second);
        cd.set(Calendar.MILLISECOND, f.secTicks);

        return cd.getTime();
    }

According to the following JDK bug, the "set()" method is clearing the DST information.
The workaround is to use "add()" instead:

https://bugs.openjdk.java.net/browse/JDK-6615045?focusedCommentId=13327706&page=com.atlassian.jira.plugin.system.issuetabpanels%3acomment-tabpanel#comment-13327706

Reported by meletis@surveycto.com on 2014-10-17 11:12:23

@getodk-bot
Copy link
Member Author

Comment by meletis
Wednesday Jan 27, 2016 at 20:28 GMT


Okay, the solution seems to be somewhere else, please ignore the "additional information" in the issue description above.

This is the line 412 in DateUtils.java

c.setTime(new Date(DateUtils.getDate(f, "UTC").getTime() + (((60 * timeOffset.hour) + timeOffset.minute) * 60 * 1000) + dstOffset));

...and this is how we think it can be solved...

long dstOffset = TimeZone.getDefault().inDaylightTime(new Date()) ? TimeZone.getDefault().getDSTSavings() : 0L;
c.setTime(new Date(DateUtils.getDate(f, "UTC").getTime() + (((60 * timeOffset.hour) + timeOffset.minute) * 60 * 1000) + dstOffset));

This is a test case that fails without the above change:

    public void testParseTime_with_DST() throws Exception {
        Locale.setDefault(Locale.US);

        TimeZone backupZone = TimeZone.getDefault();

        // this is a timezone that operates DST every day of the year!
        SimpleTimeZone dstTimezone = new SimpleTimeZone(
                7200000,
                "Europe/Athens",
                Calendar.JANUARY, 1, -Calendar.SUNDAY,
                0, SimpleTimeZone.UTC_TIME,
                Calendar.DECEMBER, 31, -Calendar.SUNDAY,
                3600 * 24, SimpleTimeZone.UTC_TIME,
                3600000);
        TimeZone.setDefault(dstTimezone);

        String time = "12:03:05.000Z";

        Date date = DateUtils.parseTime(time);

        DateFormat formatter = DateFormat.getTimeInstance();
        String formatted = formatter.format(date);

        // It should shift 3 hours, 2 for the zone and 1 for DST.
        assertEquals("3:03:05 PM", formatted);

        TimeZone.setDefault(backupZone);
    }

Please note that if that change is adopted, then the first call of "testCycle()" in the "testParity" test in DateUtilsTests.java will start failing when DST will become effective for the building workstation.

@yanokwa
Copy link
Member

yanokwa commented Sep 11, 2017

@dcbriccetti You've been poking around JavaRosa. Can you please try to reproduce this issue?

@dcbriccetti
Copy link
Contributor

Sure, when my cortisol levels are very low. 😀

@ggalmazor
Copy link
Contributor

I can reproduce this error with this test:

CsvFieldMappersTest.java:

  @Test
  public void time_parsing_on_plus_2_daylight_saving_time_zone() {
    TimeZone.setDefault(TimeZone.getTimeZone("Europe/Madrid"));
    scenario = nonGroup(DataType.TIME);
    List<Pair<String, String>> output = scenario.mapSimpleValue("07:00:00.000Z");
    assertThat(output.get(0).getRight(), is("10:00 AM"));
  }

I think this would be a JR bug, not a Briefcase bug.

DateUtils.java#351:

    public static Date parseTime (String str) {
        DateFields fields = getFields(new Date());
        fields.second = 0;
        fields.secTicks = 0;
        if (!parseTime(str, fields)) {
            return null;
        }
        // time zone may wrap time across midnight. Clear that.
        fields.year = 1970;
        fields.month = 1;
        fields.day = 1;
        return getDate(fields);
    }

Maybe using the current date for the date fields would make it work.

@ggalmazor ggalmazor added the housekeeping Issue tagged for housekeeping revision label Aug 30, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug housekeeping Issue tagged for housekeeping revision needs reproduction
Projects
None yet
Development

No branches or pull requests

5 participants