Improve time localization code

Summary:
  - We throw on a missing date right now, in the DateTime constructor. This can
happen in reasonable cases and this is display code, so handle it more
gracefully (see T520).
  - This stuff is a little slow and we sometimes render many hundreds of dates
per page. I've been seeing it in profiles on and off. Memoize timezones to
improve performance.
  - Some minor code duplication that would have become less-minor with the
constructor change, consolidate the logic.
  - Add some unit tests and a little documentation.

Test Plan:
  - Ran unit tests.
  - Profiled 1,000 calls to phabricator_datetime(), cost dropped from ~49ms to
~19ms with addition of memoization. This is still slower than I'd like but I
don't think there's an easy way to squeeze it down further.

Reviewers: ajtrichards, jungejason, nh, tuomaspelkonen, aran

Reviewed By: ajtrichards

CC: aran, ajtrichards, epriestley

Differential Revision: 966

Author: epriestley

src/__phutil_library_map__.php

@@ -457,6 +457,7 @@
     'PhabricatorLintEngine' => 'infrastructure/lint/engine',
     'PhabricatorLiskDAO' => 'applications/base/storage/lisk',
     'PhabricatorLocalDiskFileStorageEngine' => 'applications/files/engine/localdisk',
+    'PhabricatorLocalTimeTestCase' => 'view/utils/__tests__',
     'PhabricatorLoginController' => 'applications/auth/controller/login',
     'PhabricatorLogoutController' => 'applications/auth/controller/logout',
     'PhabricatorMailImplementationAdapter' => 'applications/metamta/adapter/base',
@@ -700,6 +701,7 @@
   ),
   'function' =>
   array(
+    '__phabricator_format_local_time' => 'view/utils',
     '_qsprintf_check_scalar_type' => 'storage/qsprintf',
     '_qsprintf_check_type' => 'storage/qsprintf',
     'celerity_generate_unique_node_id' => 'infrastructure/celerity/api',
@@ -1073,6 +1075,7 @@
     'PhabricatorLintEngine' => 'PhutilLintEngine',
     'PhabricatorLiskDAO' => 'LiskDAO',
     'PhabricatorLocalDiskFileStorageEngine' => 'PhabricatorFileStorageEngine',
+    'PhabricatorLocalTimeTestCase' => 'PhabricatorTestCase',
     'PhabricatorLoginController' => 'PhabricatorAuthController',
     'PhabricatorLogoutController' => 'PhabricatorAuthController',
     'PhabricatorMailImplementationAmazonSESAdapter' => 'PhabricatorMailImplementationPHPMailerLiteAdapter',

src/view/utils/__tests__/PhabricatorLocalTimeTestCase.php

@@ -0,0 +1,52 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class PhabricatorLocalTimeTestCase extends PhabricatorTestCase {
+
+  public function testLocalTimeFormatting() {
+    $user = new PhabricatorUser();
+    $user->setTimezoneIdentifier('America/Los_Angeles');
+
+    $utc = new PhabricatorUser();
+    $utc->setTimezoneIdentifier('UTC');
+
+    $this->assertEqual(
+      'Jan 1 2000, 12:00 AM',
+      phabricator_datetime(946684800, $utc),
+      'Datetime formatting');
+    $this->assertEqual(
+      'Jan 1 2000',
+      phabricator_date(946684800, $utc),
+      'Date formatting');
+    $this->assertEqual(
+      '12:00 AM',
+      phabricator_time(946684800, $utc),
+      'Time formatting');
+
+    $this->assertEqual(
+      'Dec 31 1999, 4:00 PM',
+      phabricator_datetime(946684800, $user),
+      'Localization');
+
+    $this->assertEqual(
+      '',
+      phabricator_datetime(0, $user),
+      'Missing epoch should fail gracefully');
+  }
+
+}

src/view/utils/__tests__/__init__.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/people/storage/user');
+phutil_require_module('phabricator', 'infrastructure/testing/testcase');
+phutil_require_module('phabricator', 'view/utils');
+
+
+phutil_require_source('PhabricatorLocalTimeTestCase.php');

src/view/utils/viewutils.php

@@ -17,24 +17,60 @@
  */
 
 function phabricator_date($epoch, $user) {
-  $zone = new DateTimeZone($user->getTimezoneIdentifier());
-  $date = new DateTime('@'.$epoch);
-  $date->setTimeZone($zone);
-  return $date->format('M j Y');
+  return __phabricator_format_local_time(
+    $epoch,
+    $user,
+    'M j Y');
 }
 
 function phabricator_time($epoch, $user) {
-  $zone = new DateTimeZone($user->getTimezoneIdentifier());
-  $date = new DateTime('@'.$epoch);
-  $date->setTimeZone($zone);
-  return $date->format('g:i A');
+  return __phabricator_format_local_time(
+    $epoch,
+    $user,
+    'g:i A');
 }
 
 function phabricator_datetime($epoch, $user) {
-  $zone = new DateTimeZone($user->getTimezoneIdentifier());
+  return __phabricator_format_local_time(
+    $epoch,
+    $user,
+    'M j Y, g:i A');
+}
+
+/**
+ * Internal date rendering method. Do not call this directly; instead, call
+ * @{function:phabricator_date}, @{function:phabricator_time}, or
+ * @{function:phabricator_datetime}.
+ *
+ * @param int Unix epoch timestamp.
+ * @param PhabricatorUser User viewing the timestamp.
+ * @param string Date format, as per DateTime class.
+ * @return string Formatted, local date/time.
+ */
+function __phabricator_format_local_time($epoch, $user, $format) {
+  if (!$epoch) {
+    // If we're missing date information for something, the DateTime class will
+    // throw an exception when we try to construct an object. Since this is a
+    // display function, just return an empty string.
+    return '';
+  }
+
+  $user_zone = $user->getTimezoneIdentifier();
+
+  static $zones = array();
+  if (empty($zones[$user_zone])) {
+    $zones[$user_zone] = new DateTimeZone($user_zone);
+  }
+  $zone = $zones[$user_zone];
+
+  // NOTE: Although DateTime takes a second DateTimeZone parameter to its
+  // constructor, it ignores it if the date string includes timezone
+  // information. Further, it treats epoch timestamps ("@946684800") as having
+  // a UTC timezone. Set the timezone explicitly after constructing the object.
   $date = new DateTime('@'.$epoch);
   $date->setTimeZone($zone);
-  return $date->format('M j Y, g:i A');
+
+  return $date->format($format);
 }
 
 function phabricator_format_relative_time($duration) {