Use transactions when importing events in Calendar, and update existing events

Summary:
Ref T10747.

  - Apply what changes we can with transactions, so you can see how an event has changed and import actions are more explicit.
    - I'll hide these from email/feed soon: I want them to appear on the event, but not generate notifications, since that could be especially annoying for automated events.
  - When importing, try to update existing events if we can.

Test Plan:
Imported a ".ics" file several times with minor changes, saw them reflected in the UI with transactions.

{F1870027}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16701

src/applications/calendar/import/PhabricatorCalendarImportEngine.php

@@ -36,7 +36,7 @@ abstract class PhabricatorCalendarImportEngine
 
     $event_type = PhutilCalendarEventNode::NODETYPE;
 
-    $events = array();
+    $nodes = array();
     foreach ($root->getChildren() as $document) {
       foreach ($document->getChildren() as $node) {
         if ($node->getNodeType() != $event_type) {
@@ -44,25 +44,212 @@ abstract class PhabricatorCalendarImportEngine
           continue;
         }
 
-        $event = PhabricatorCalendarEvent::newFromDocumentNode($viewer, $node);
-
-        $event
-          ->setImportAuthorPHID($viewer->getPHID())
-          ->setImportSourcePHID($import->getPHID())
-          ->attachImportSource($import);
-
-        $events[] = $event;
+        $nodes[] = $node;
       }
     }
 
-    // TODO: Use transactions.
-    // TODO: Update existing events instead of fataling.
-    foreach ($events as $event) {
-      $event->save();
+    $node_map = array();
+    $parent_uids = array();
+    foreach ($nodes as $node) {
+      $full_uid = $this->getFullNodeUID($node);
+      if (isset($node_map[$full_uid])) {
+        // TODO: Warn that we got a duplicate.
+        continue;
+      }
+      $node_map[$full_uid] = $node;
     }
 
+    if ($node_map) {
+      $events = id(new PhabricatorCalendarEventQuery())
+        ->setViewer($viewer)
+        ->withImportAuthorPHIDs(array($viewer->getPHID()))
+        ->withImportUIDs(array_keys($node_map))
+        ->execute();
+      $events = mpull($events, null, 'getImportUID');
+    } else {
+      $events = null;
+    }
+
+    $xactions = array();
+    $update_map = array();
+    foreach ($node_map as $full_uid => $node) {
+      $event = idx($events, $full_uid);
+      if (!$event) {
+        $event = PhabricatorCalendarEvent::initializeNewCalendarEvent($viewer);
+      }
+
+      $event
+        ->setImportAuthorPHID($viewer->getPHID())
+        ->setImportSourcePHID($import->getPHID())
+        ->setImportUID($full_uid)
+        ->attachImportSource($import);
+
+      $this->updateEventFromNode($viewer, $event, $node);
+      $xactions[$full_uid] = $this->newUpdateTransactions($event, $node);
+      $update_map[$full_uid] = $event;
+    }
+
+    // Reorder events so we create parents first. This allows us to populate
+    // "instanceOfEventPHID" correctly.
+    $insert_order = array();
+    foreach ($update_map as $full_uid => $event) {
+      $parent_uid = $this->getParentNodeUID($node_map[$full_uid]);
+      if ($parent_uid === null) {
+        $insert_order[$full_uid] = $full_uid;
+        continue;
+      }
+
+      if (empty($update_map[$parent_uid])) {
+        // The parent was not present in this import, which means it either
+        // does not exist or we're going to delete it anyway. We just drop
+        // this node.
+
+        // TODO: Warn that we got rid of an event with no parent.
+
+        continue;
+      }
+
+      // Otherwise, we're going to insert the parent first, then insert
+      // the child.
+      $insert_order[$parent_uid] = $parent_uid;
+      $insert_order[$full_uid] = $full_uid;
+    }
+
+    // TODO: Define per-engine content sources so this can say "via Upload" or
+    // whatever.
+    $content_source = PhabricatorContentSource::newForSource(
+      PhabricatorWebContentSource::SOURCECONST);
+
+    $update_map = array_select_keys($update_map, $insert_order);
+    foreach ($update_map as $full_uid => $event) {
+      $parent_uid = $this->getParentNodeUID($node_map[$full_uid]);
+      if ($parent_uid) {
+        $parent_phid = $update_map[$full_uid]->getPHID();
+      } else {
+        $parent_phid = null;
+      }
+
+      $event->setInstanceOfEventPHID($parent_phid);
+
+      $event_xactions = $xactions[$full_uid];
+
+      $editor = id(new PhabricatorCalendarEventEditor())
+        ->setActor($viewer)
+        ->setActingAsPHID($import->getPHID())
+        ->setContentSource($content_source)
+        ->setContinueOnNoEffect(true)
+        ->setContinueOnMissingFields(true);
+
+      $editor->applyTransactions($event, $event_xactions);
+    }
+
+    // TODO: When the source is a subscription-based ICS file or some other
+    // similar source, we should load all events from the source here and
+    // destroy the ones we didn't update. These are events that have been
+    // deleted.
   }
 
+  private function getFullNodeUID(PhutilCalendarEventNode $node) {
+    $uid = $node->getUID();
+    $instance_epoch = $this->getNodeInstanceEpoch($node);
+    $full_uid = $uid.'/'.$instance_epoch;
 
+    return $full_uid;
+  }
+
+  private function getParentNodeUID(PhutilCalendarEventNode $node) {
+    $recurrence_id = $node->getRecurrenceID();
+
+    if (!strlen($recurrence_id)) {
+      return null;
+    }
+
+    return $node->getUID().'/';
+  }
+
+  private function getNodeInstanceEpoch(PhutilCalendarEventNode $node) {
+    $instance_iso = $node->getRecurrenceID();
+    if (strlen($instance_iso)) {
+      $instance_datetime = PhutilCalendarAbsoluteDateTime::newFromISO8601(
+        $instance_iso);
+      $instance_epoch = $instance_datetime->getEpoch();
+    } else {
+      $instance_epoch = null;
+    }
+
+    return $instance_epoch;
+  }
+
+  private function newUpdateTransactions(
+    PhabricatorCalendarEvent $event,
+    PhutilCalendarEventNode $node) {
+
+    $xactions = array();
+    $uid = $node->getUID();
+
+    $name = $node->getName();
+    if (!strlen($name)) {
+      if (strlen($uid)) {
+        $name = pht('Unnamed Event "%s"', $uid);
+      } else {
+        $name = pht('Unnamed Imported Event');
+      }
+    }
+    $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      ->setTransactionType(
+        PhabricatorCalendarEventNameTransaction::TRANSACTIONTYPE)
+      ->setNewValue($name);
+
+    $description = $node->getDescription();
+    $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      ->setTransactionType(
+        PhabricatorCalendarEventDescriptionTransaction::TRANSACTIONTYPE)
+      ->setNewValue((string)$description);
+
+    $is_recurring = (bool)$node->getRecurrenceRule();
+    $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      ->setTransactionType(
+        PhabricatorCalendarEventRecurringTransaction::TRANSACTIONTYPE)
+      ->setNewValue($is_recurring);
+
+    return $xactions;
+  }
+
+  private function updateEventFromNode(
+    PhabricatorUser $actor,
+    PhabricatorCalendarEvent $event,
+    PhutilCalendarEventNode $node) {
+
+    $instance_epoch = $this->getNodeInstanceEpoch($node);
+    $event->setUTCInstanceEpoch($instance_epoch);
+
+    $timezone = $actor->getTimezoneIdentifier();
+
+    // TODO: These should be transactional, but the transaction only accepts
+    // epoch timestamps right now.
+    $start_datetime = $node->getStartDateTime()
+      ->setViewerTimezone($timezone);
+    $end_datetime = $node->getEndDateTime()
+      ->setViewerTimezone($timezone);
+
+    $event
+      ->setStartDateTime($start_datetime)
+      ->setEndDateTime($end_datetime);
+
+    // TODO: This should be transactional, but the transaction only accepts
+    // simple frequency rules right now.
+    $rrule = $node->getRecurrenceRule();
+    if ($rrule) {
+      $event->setRecurrenceRule($rrule);
+
+      $until_datetime = $rrule->getUntil()
+        ->setViewerTimezone($timezone);
+      if ($until_datetime) {
+        $event->setUntilDateTime($until_datetime);
+      }
+    }
+
+    return $event;
+  }
 
 }

src/applications/calendar/query/PhabricatorCalendarEventQuery.php

@@ -15,6 +15,8 @@ final class PhabricatorCalendarEventQuery
   private $isStub;
   private $parentEventPHIDs;
   private $importSourcePHIDs;
+  private $importAuthorPHIDs;
+  private $importUIDs;
 
   private $generateGhosts = false;
 
@@ -83,6 +85,16 @@ final class PhabricatorCalendarEventQuery
     return $this;
   }
 
+  public function withImportAuthorPHIDs(array $author_phids) {
+    $this->importAuthorPHIDs = $author_phids;
+    return $this;
+  }
+
+  public function withImportUIDs(array $uids) {
+    $this->importUIDs = $uids;
+    return $this;
+  }
+
   protected function getDefaultOrderVector() {
     return array('start', 'id');
   }
@@ -424,6 +436,20 @@ final class PhabricatorCalendarEventQuery
         $this->importSourcePHIDs);
     }
 
+    if ($this->importAuthorPHIDs !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'event.importAuthorPHID IN (%Ls)',
+        $this->importAuthorPHIDs);
+    }
+
+    if ($this->importUIDs !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'event.importUID IN (%Ls)',
+        $this->importUIDs);
+    }
+
     return $where;
   }
 

src/applications/calendar/storage/PhabricatorCalendarEvent.php

@@ -102,66 +102,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
       ->applyViewerTimezone($actor);
   }
 
-  public static function newFromDocumentNode(
-    PhabricatorUser $actor,
-    PhutilCalendarEventNode $node) {
-    $timezone = $actor->getTimezoneIdentifier();
-
-    $uid = $node->getUID();
-
-    $name = $node->getName();
-    if (!strlen($name)) {
-      if (strlen($uid)) {
-        $name = pht('Unnamed Event "%s"', $node->getUID());
-      } else {
-        $name = pht('Unnamed Imported Event');
-      }
-    }
-
-    $description = $node->getDescription();
-
-    $instance_iso = $node->getRecurrenceID();
-    if (strlen($instance_iso)) {
-      $instance_datetime = PhutilCalendarAbsoluteDateTime::newFromISO8601(
-        $instance_iso);
-      $instance_epoch = $instance_datetime->getEpoch();
-    } else {
-      $instance_epoch = null;
-    }
-    $full_uid = $uid.'/'.$instance_epoch;
-
-    $start_datetime = $node->getStartDateTime()
-      ->setViewerTimezone($timezone);
-    $end_datetime = $node->getEndDateTime()
-      ->setViewerTimezone($timezone);
-
-    $rrule = $node->getRecurrenceRule();
-
-    $event = self::initializeNewCalendarEvent($actor)
-      ->setName($name)
-      ->setStartDateTime($start_datetime)
-      ->setEndDateTime($end_datetime)
-      ->setImportUID($full_uid)
-      ->setUTCInstanceEpoch($instance_epoch);
-
-    if (strlen($description)) {
-      $event->setDescription($description);
-    }
-
-    if ($rrule) {
-      $event->setRecurrenceRule($rrule);
-      $event->setIsRecurring(1);
-
-      $until_datetime = $rrule->getUntil()
-        ->setViewerTimezone($timezone);
-      if ($until_datetime) {
-        $event->setUntilDateTime($until_datetime);
-      }
-    }
-
-    return $event;
-  }
-
   private function newChild(
     PhabricatorUser $actor,
     $sequence,