format() function with timezone

[jcberquist]

First of all, thanks for this cfc, I love moment.js and I am excited to see that functionality brought to cfml.

As it stands now, moment.cfc in the format() method will format for the current timezone of the server no matter what zone the moment has. (Easily seen by using a mask of 'long' or including the zone offset in the mask: 'zzzz').

Even if you are not interested in displaying the zone info in the formatted string, it could still be an issue, because of the times that are invalid due to the timezone (e.g. 2015-03-08 02:30AM in the America/New_York zone). If, for example, the server is not running on UTC but America/New_York, and maybe you don't care about this since the server ought to be on UTC :), then trying to convert 2015-03-07 09:30PM EST to UTC and outputting it will result in 2015-03-08 03:30AM, when it should be 2015-03-07 02:30AM.

[atuttle]

Hmm, as I live in America/New_York, maybe using it in a lot of my tests was not the best choice (because my system time will match)... I'll switch it up some and see what happens. I'll also try setting my system TZ to different zones and see what happens. Thanks for the report.

[atuttle]

I think the only way around this is going to be handling all of the formatting internally, only passing through to dateTimeFormat what we can rely on to get correct. If you could help define the unit tests that ensure every use-case is covered, it would be greatly appreciated. I'll get us started:

it("formats standard masks correctly across time zones", function(){
    var testZones = [
        {
            zone: 'Asia/Hong_Kong'
            ,short: 'CTT'
        }
        ,{
            zone: 'America/Los_Angeles'
            ,short: 'PST'
        }
    ];

    //these dates + times were chosen to have leading zeros in all applicable places, as well as being in and out of DST
    var timeNoDST = '2009-03-05 09:03:07';
    var timeDST = '2009-03-09 09:03:07';

    for(var z in testZones){
        var test = moment( timeNoDST, z.zone );
        debug( test );

        //basic date and time fields; these could easily pass through to dateTimeFormat
        expect( test.format('yyyy') ).toBe( '2009' );
        expect( test.format('yy') ).toBe( '09' );
        expect( test.format('mmmm') ).toBe( 'March' );
        expect( test.format('mmm') ).toBe( 'Mar' );
        expect( test.format('mm') ).toBe( '03' );
        expect( test.format('m') ).toBe( '3' );
        expect( test.format('EEEE') ).toBe( 'Thursday' );
        expect( test.format('EEE') ).toBe( 'Thu' );
        expect( test.format('dd') ).toBe( '05' );
        expect( test.format('d') ).toBe( '5' );
        //more to come here...

        //now check formatting with time zones
        expect( test.format('long') ).toBe( 'March 5, 2009 9:03:07 AM #z.short#' );
    }
});

I've committed the above (build fails :cry: but this correctly shows the bug you've reported), and I'll continue to work on expanding the things that the tests cover as time permits. Feel free to send a pull request with additional expectations added here.

[jcberquist]

Thanks for following up. You have basic issue covered in your test, I can look into adding some tests surrounding DST changes. I wanted to give some of the background of the DST issues I have had to see what you thought, but it got really long, so I won't mind if you don't read it :smile:. The bottom line is I am not sure that this.time and this.utcTime should be different "moments" in time at all. I think that at least for the formatting issue, they shouldn't be, but there are definitely other issues to take into consideration (for instance, what does serializeJSON() do with a datetime value, or what will CF send to a database for a given datetime).

The problem I have encountered is that I need ISO8601 UTC timestamps for interacting with an API (AWS), but I have a server set to the America/New_York timezone (as said in my first comment, my first problem). So my original way of handling this was to use dateConvert( 'local2utc', now() ). However, the original way of implementing dateConvert() (still the case in Railo/Lucee but changed in CF after CF9 - I don't currently have access to CF9 but I am pretty confident about this), is it would get the offset for the given datetime and then add/subtract that offset from the given datetime to get UTC time. But no zone change was actually made. This meant that for a datetime such as 2015-03-07 21:30:00 EST it would determine a 5 hour offset from UTC and add that to the datetime. But the timeFormat()/dateTimeFormat() functions would still use America/New_York for rendering and +5 hours after the above date in this zone is 2015-03-08 03:30:00 EDT. 3:30 is the output obviously even if the zone info is left off. Consider the following code (on a server set to America/New_York):

testDateTime = createDateTime( 2015, 3, 7, 21, 30, 0 );
utcTime = dateConvert( "local2utc", testDateTime );
writeDump( dateTimeFormat( utcTime, 'long' ) );
writeDump( dateTimeFormat( utcTime, 'yyyy-mm-dd' ) & 'T' & dateTimeFormat( utcTime, 'HH:nn:ss' ) & 'Z' );

On Railo/Lucee this will still output March 8, 2015 3:30:00 AM EDT and 2015-03-08T03:30:00Z, and it will do that on CF9 and earlier as well (using dateFormat() and timeFormat()). But in CF10+ this will output March 8, 2015 2:30:00 AM UTC (notice the zone change) and 2015-03-08T02:30:00Z. I am using Railo/Lucee so this wasn't available to me, though.

I realized that, at least for my purposes, I didn't need to add or subtract from my original date, what was at stake was a question of formatting the date using a given zone. so I could simply do this:

writeDump( dateTimeFormat( testDateTime, 'yyyy-mm-dd', 'UTC' ) & 'T' & dateTimeFormat( testDateTime, 'HH:nn:ss', 'UTC' ) & 'Z' );

That will output 2015-03-08T02:30:00Z, no dateConvert() function needed.

This makes sense to me, because 2015-03-07 21:30:00 EST and 2015-03-08 02:30:00 UTC are the same "moment" in time, so to speak, just formatted in different zones. CF10+ does this properly because if you call testDateTime.getTime() and utcTime.getTime(), the same integer is returned: 1425781800000. On Railo/Lucee and CF9 you will get 1425781800000 and 1425799800000. It seems that all the dateConvert( 'local2utc', datetime ) function is doing in CF10+ is specifying that the formatting functions should output in UTC rather than the local server timezone. Note this causes other issues because the dateConvert() function is no longer adding or subtracting units of time. You might recall this: http://blogs.coldfusion.com/post.cfm/coldfusion-10-release-notes, some of those comments are relatively (in Adobe time) recent.

This shows, I think, that the zones matter when a datetime is parsed and created (string or createDateTime()) and it matters when the datetime is output, with the default in both cases being to use the local zone if no zone information is supplied. But an internal date/time object in ColdFusion or Railo/Lucee is independent of zones. See https://wikidocs.adobe.com/wiki/display/coldfusionen/GetTimeZoneInfo

ColdFusion stores date and time values as date-time objects: real numbers on a universal time line. It converts an object to a time zone when it formats an object; it converts a date/time value from a time zone to a real number when it parses a value.

and http://www.luceedocs.org/function/parsedatetime

A datetime object is independent of a specific timezone, it is only a offset in milliseconds from 1970-1-1 00.00:00 UTC (Coordinated Universal Time).

This makes me think that this.time and this.utcTime should not be different, what should be done is figuring out how to get the datestring/datetime passed in the init which will be interpreted using the system timezone to be interpreted using the zone passed in the init, if it is different from the system timezone. On the other hand, based on the above link, this approach could lead to other issues when using the datetimes in other contexts. If you get this far, I would love to hear your thoughts on this.

[atuttle]

If you get this far, I would love to hear your thoughts on this.

It took me a few days to come up with enough time, but I read it all. :)

My impression of the problem and solution is that if we detect the time zone masks and do their rendering separately, allowing normal masking to fall through to dateTimeFormat, that should work.

this.utcTime is only used in a few places, some of which could (should?) be easily replaced with a call to this.clone().utc()... But I'm not convinced it should be removed yet. If TZ formatting is the only remaining problem, and removing this.utcTime isn't a direct part of the solution, what's the gain of removing it?

[jcberquist]

Sorry if I am just repeating myself, but the problem I have had is that the formatting issues are not limited to outputting zones, how dateTimeFormat() renders the mask HH:nn:ss is dependent on the zone as well. So for example on my America/New_York server:

testMoment = new moment( '2015-03-07 21:30:00', 'America/New_York' );
writeOutput( testMoment.utc().format( 'iso8061' ) ); // incidentally there is a typo here it should be 'iso8601'
// outputs '2015-03-08T03:30:00Z'

That should output 2015-03-08T02:30:00Z but there is no way without specifying a different zone for rendering that the timeFormat() and dateTimeFormat() functions will output a 2:30AM time on 2015-03-08. Does that make sense?

[atuttle]

It does make sense, and thank you for continuing to try to beat this point into my head. :)

Basically the solution I have in mind is that anything time zone related, including iso8601 (typo will be fixed in a separate bug...) will be an exception to the normal, pass-thru, formatting. We'll need to make a list of every possible whole-mask (as in iso8601) and every partial-mask (as in z or Z, because dateTimeFormat allows the use of Java masks) that is zone-specific or shows zone info, and handle those separately.

[atuttle]

Entered #10 to address the misspelled mask

[jcberquist]

Ha ha, sorry, I guess I have spent enough time on this that I needed to vent somewhere.

So my argument was rather than figuring out every mask that will need correction, moment.cfc should just take advantage of the fact that the zone can be specified in dateTimeFormat(). Doing this would mean that this.time and this.utcTime would need to be the same time, and not be offset from one another, or in other words, this.utcTime would be superfluous.

I will stop browbeating you now :smile:.

[atuttle]

Ah yes, I always forget that argument... I'm willing to give it a try, but we still need a comprehensive set of tests. :+1:

[atuttle]

@jcberquist I've been making an attempt at using this 3rd argument to dateTimeFormat() to set the desired output time zone, but just like localToUTC() (etc) it's calculated from the executing machine's (e.g. server's) current time zone offset.

So, with the code (format() method) changed to:

return dateTimeFormat( this.time, mask, this.zone );

And the test:

var test = moment( '2009-03-05 09:03:07', 'Asia/Hong_Kong' );
expect( test.format('long') ).toBe( 'March 5, 2009 9:03:07 AM HKT' );

I'm currently in UTC-5 (US/Central), and Hong Kong is UTC+8 (total difference of 13), and this is the test result:

Expected [March 5, 2009 9:03:07 AM HKT] but received [March 5, 2009 10:03:07 PM HKT]

The actual output is 13 hours ahead of expected; so I take this to mean that the dateTimeFormat() line is ASSUMING that the source date is "local."

Maybe instead of using this.utcTime we should have this.localTime which is calculated at creation and can be used when we are using internal conversions that assume the source is local. Thoughts?

[atuttle]

Initial testing seems to indicate that this (this.localTime) is a big step forward (solves the unit tests that I had that were already failing), but I'm seeing an issue with PST/PDT -- not sure if it's related to the dateTimeFormat() 3rd argument yet or not. But it's lunch time, so I'll have to come back to it later.

[jcberquist]

My understanding of the situation is as follows. When moment.cfc runs the init() method, it uses parseDateTime() to convert the supplied date/time string to a CFML date object (at least it will in the above example). As discussed, a date object in CFML under the hood is an offset from 1970-1-1 00:00:00 UTC. So parseDateTime() is effectively converting the supplied string to a UTC integer offset. But to do this it needs a timezone; absent any timezone information in the supplied string, parseDateTime() assumes the string is in the local timezone of the server. Unfortunately, CF doesn't let you specify a timezone as an argument to parseDateTime() (Railo/Lucee does let you do this). That is why the dateTimeFormat() output in the test is not what it should be, since the date it is being asked to format is effectively 2009-03-05 09:03:07-0500. Am I making sense?

[atuttle]

Sure, that makes sense. Do you have any ideas on how we can deal with this problem?

[atuttle]

I think I've figured out a bug in my tests, and the latest changes (to be pushed) resolve this as far as I know. I'm not going to close this until you confirm that everything looks good to you!

[jcberquist]

I took a look at this tonight; I cloned the repo and ran the tests. I have 8 fails here out of the 80 tests. I ran them on Lucee 4.5.1 with a system timezone of US/Eastern. The utc() tests fail, as well as the one you mention above, outputting in the HKT zone. I see that the CI tests pass, so I wonder what the difference is?

[jbgithub]

Hello, I was testing moment.cfc (great cfc!) and noticed format("iso8601") was not giving correct results.

Two issues: 1) Appending "Z" to all dates regardless of timezone; should have offset for non-GMT timezone (see iso8601 spec). 2) Using "this.time" only works if the server is in the same timezone of the date/time set. Using "this.localTime" provides correct result;.

The updated code below fixes both issues; works for my "America/Los_Angeles" to "America/New_York" dates.

Updated code (line 207):

//return dateTimeFormat(this.time, 'yyyy-mm-dd') & 'T' & dateTimeFormat(this.time, 'HH:nn:ss') & 'Z';
return replace(dateTimeFormat( this.localTime, "yyyy-mm-dd'T'HH:nn:ssZ", this.zone ), "+0000", "Z");

FYI: DateTimeFormat() allows you to embed a single-quoted string in the mask just like Java.

I don't know if this helps with the issues discussed here but it works for me.

Again, great cfc!

Thanks