Wednesday, May 20, 2009

Breaking change in Malevich

This will read as a daily WTF, unfortunately... but after 6 months of development, I found a really, really nasty bug in Malevich, and if you're maintaining an installation, you should read this.

Interestingly, the bug was in my face all this time, I just wasn't paying attention.

Malevich stores a bunch of timestamps in its database - one for change list itself, one for each review iteration, and one for each file version. Normally these are stored in UTC - after all, how else would you store times in a web application where users are scattered around the globe?

Well, as it turned out, all but not all. Change list time stamp was stored as local time. Of course the bug was on display on the dashboard all this time, literally in my face. But I think this was because all the rest of the dates were in UTC and therefore not very useful - I just taught myself to ignore all time stamps. So I never noticed that unlike all the rest, these ones looked correct - and they should not have!

Anyway, for quite a while Malevich fielded complaints that the time stamps are silly, and I was thinking on and off on how I might fix that. You see, .NET runtime which Malevich is built upon is great at manipulating the time, except for one thing - the web site has no idea what user's time zone is - this is by design, a privacy issue.

The client side does know what the local time offset is, so the only way to transform time correctly must lay with the client.

So this is what Malevich does - each time stamp is wrapped in a <span id='timestamp' name='timestamp> element. When the page loads, I get all the elements by the name 'timestamp', and convert them to local time.

The conversion is pretty crappy from the globalization perspective (heck, but this is *free* software :-)) - because default JavaScript time conversion functions are terrible - they produce very long strings (for example, days of the week are spelled out by some browsers).

So I compose the date/time string myself, in US format. The only thing where I try to be smart is determine if the format is 12- or 24-hours upfront and then generate the date that would use the same format. Which does not work on Chrome because it uses 24-hour format no matter what.


//-----------------------------------------------------------------------
//
// Copyright (C) Sergey Solyanik for The Malevich Project.
//
// This file is subject to the terms and conditions of the Microsoft
// Public License (MS-PL).
// See http://www.microsoft.com/opensource/licenses.mspx#Ms-PL for
// more details.
//

//-----------------------------------------------------------------------
//
// Javascript module to recalculate times to browser local.
//

//
// Is time a 24-hour format or AM/PM?
//
var TIME_FORMAT_24HR = ComputeDateFormat();

//
// Recomputes all date times from UTC to local.
//
function RebuildDates()
{
var dates = document.getElementsByName('timestamp');
for (var i = 0; i < dates.length; ++i)
{
var date = new Date(dates[i].firstChild.nodeValue);

var month = date.getMonth() + 1
var year = date.getYear()
var day = date.getDate()

if (day < 10) day = '0' + day
if (month < 10) month = '0' + month
if (year < 1000) year += 1900

var hour = date.getHours();
var minute = date.getMinutes();
var seconds = date.getSeconds();

var suffix = '';
if (TIME_FORMAT_24HR)
{
if (hour < 10) hour = '0' + hour;
}
else
{
if (hour < 12) suffix = ' AM'; else suffix = ' PM';
if (hour == 0) hour = 12;
if (hour > 12) hour -= 12;
if (hour < 10) hour = ' ' + hour;
}

if (minute < 10) minute = '0' + minute;
if (seconds < 10) seconds = '0' + seconds;

dates[i].firstChild.nodeValue =
[month, '/', day, '/', year, ' ', hour, ':', minute,
':', seconds, suffix].join('');
}
}

//
// Checks if the date in the current locale is AM/PM or 24 hours.
//
function ComputeDateFormat()
{
var dateString = (new Date()).toLocaleTimeString();
if (dateString.indexOf(' AM') == -1 && dateString.indexOf(' PM') == -1)
return true;
return false;
}

//
// Makes RebuildDates be called on page load.
//
if (window.attachEvent)
{
window.attachEvent('onload', RebuildDates);
}
else
{
if (window.onload)
{
var curronload = window.onload;
window.onload = function()
{
curronload();
RebuildDates();
};
}
else
{
window.onload = RebuildDates;
}
}


Anyway, so I make this change, and now I start looking at dates very closely, and - crrrrap! - all the time stamps now look great, except the most important one - the change list time stamp!

And the worst part of course is that it's in the database! Moreover, it's data! And it's now on a few dozen servers at Microsoft alone, and I have no idea how to even find them.

Anyway, I put notices on the web site (http://malevich.codeplex.com/Wiki/View.aspx?title=special%20note%20on%20Malevich%20upgrades, and on this blog (you're reading it), and in the change list description, and on Microsoft's internal share point site for Malevich.

I modify the installer so that it detects this condition (or, rather, the right version transition), and tries to fix up the problem:

USE [CodeReview]

DECLARE @UTCDate datetime
DECLARE @LocalDate datetime
DECLARE @TimeDiff int

SET @UTCDate = GETUTCDATE()
SET @LocalDate = GETDATE()
SET @TimeDiff = DATEDIFF(hh, @LocalDate, @UTCDate)

UPDATE [dbo].[ChangeList] SET TimeStamp = DATEADD(hh, @TimeDiff, TimeStamp)


What else can I do?

Of course, the script above does not work completely, either, because it does not account for the daylight savings time (which is really, really hard to compute in T-SQL - I love human time system!), and because it assumes that local time is the same as server time, which is of course incorrect, because a bunch of teams use the tool between here and China... But it does make the time somewhat more right than it was before.

So this is the story of the worst bug in Malevich in the 6 months since it was born.

And the moral of the story... what is the moral of this story? I wrote it primarily to attract the attention of Malevich admins to the fact that they need to be thoughtful about their next upgrade. But if you have a suggestion for a witty ending, offer it in the comments section! :-)

2 comments:

NS said...

Provide an utility to convert timestamps in the DB - configurable with time offset and user name? The user (usually) knows what TZ is he(she, it) in.

Илья Казначеев said...

Do you really have a handsome of spans in your html, all with id="timestamp"?
If so, I would advice you to stop blaming javascript and stop producing html.