Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Milliseconds on iso8601TimestampToDateValueTransformer #4

Open
chadpod opened this issue Nov 13, 2013 · 2 comments
Open

Milliseconds on iso8601TimestampToDateValueTransformer #4

chadpod opened this issue Nov 13, 2013 · 2 comments

Comments

@chadpod
Copy link
Contributor

chadpod commented Nov 13, 2013

The dates I get back from our server (Django/Piston) are of this format:

"2013-11-13T05:33:02.965Z"

I wanted to not lose the milliseconds portion because my modified date attribute check was failing because I was seeing some sub-second date changes. I changed the iso8601TimestampToDateValueTransformer as follows:

static unsigned int const ISO_8601_MAX_LENGTH = 29;
...
/* Check for milliseconds */
double milliseconds = 0.f;
if ([inputValue length] == 24 && [inputValue hasSuffix:@"Z"] && [inputValue characterAtIndex:19] == '.') {
    NSString *millisecondsString = [inputValue substringWithRange:NSMakeRange(20, 3)];
    milliseconds = [millisecondsString doubleValue]/1000.f;
}

if (length == 24 && source[length - 1] == 'Z') {
    memcpy(destination, source, length - 1);
    strncpy(destination + length - 1, "+0000\0", 6);
} else if (length == 29 && source[26] == ':') {
    memcpy(destination, source, 26);
    memcpy(destination + 26, source + 27, 2);
} else if (length == 20 && source[length - 1] == 'Z') {
    memcpy(destination, source, length - 1);
    strncpy(destination + length - 1, "+0000\0", 6);
} else if (length == 25 && source[22] == ':') {
    memcpy(destination, source, 22);
    memcpy(destination + 22, source + 23, 2);
} else {
    memcpy(destination, source, MIN(length, ISO_8601_MAX_LENGTH - 1));
}

destination[sizeof(destination) - 1] = 0;

struct tm time = {
    .tm_isdst = -1,
};

strptime_l(destination, "%FT%T%z", &time, NULL);

time_t timeIntervalSince1970 = timegm(&time);
RKValueTransformerTestTransformation(timeIntervalSince1970 != -1, error, @"Failed transformation to date representation: time range is beyond the bounds supported by mktime");
*outputValue = [NSDate dateWithTimeIntervalSince1970:((double)timeIntervalSince1970 + milliseconds)];

One thing that jumped out at me was the original version was using mktime, and I was getting dates that were shifted into the future by the number of hours my local timezone was offset from UTC. Since all dates I get back from the server are UTC, using 'timegm' got me the correct date.

As all this C date parsing stuff is new to me, I was hoping to get some feedback on the above changes and maybe they are of some use to you.

@blakewatters
Copy link
Member

@chadpod Thanks for the changes here. We definitely just need to blow out the test coverage with additional cases. IIRC the parser should be handling timezone offset formats and sending either ‘Z’ or ‘+0000’ would indicate a UTC absolute time. I am guessing that the addition of the milliseconds is throwing off the positional checks.

I think we need tests for:

  • Timestamp string without offset (should be assumed UTC)
  • Timestamp string with +/- XXXX
  • Timestamp string with Z
  • Timestamp string with milliseconds without offset (should be assumed UTC)
  • Timestamp string with milliseconds with +/- XXXX
  • Timestamp string with milliseconds and Z

When we have all of these in place and passing we’re probably in good shape.

@chadpod
Copy link
Contributor Author

chadpod commented Nov 13, 2013

Going to work on this a bit and see what I can pull together for you. On the timezone stuff, my understanding is mktime will use the local timezone, regardless of what timezone is in the tm time struct. Seems to be what the docs say and my limited testing seems to agree.

Related SO: http://stackoverflow.com/questions/530519/stdmktime-and-timezone-info

Similarly, timegm will use UTC regardless of what is in the tm time struct.

Just got the repo forked and workspace going via Cocoapods. Hopefully have a pull request for you soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants