From a90c8acb3cf4fc4eb219799aa297a672709532a6 Mon Sep 17 00:00:00 2001 From: "Michael J. Rubinsky" Date: Fri, 28 May 2010 14:58:54 -0400 Subject: [PATCH] Improvments in listing events - do not query tag storage when listing events. No need to query tag storage 99% of the time we call listEvents. Do not retrieve tags unless explicitly requested, and when we do, do it in a single batch query for all events returned in listEvents. Also, lazy load the tags on events when accessing the tags property. Still need to do something similar with the geoLocation stuff. --- .../ActiveSync/Driver/Horde/Connector/Registry.php | 5 +-- kronolith/lib/Api.php | 7 +++-- kronolith/lib/Driver/Kolab.php | 8 ++--- kronolith/lib/Driver/Sql.php | 20 ++++++++---- kronolith/lib/Event.php | 36 +++++++++++++++++----- kronolith/lib/Event/Kolab.php | 1 + kronolith/lib/Kronolith.php | 7 +++-- kronolith/lib/Resource/Single.php | 2 +- kronolith/lib/Tagger.php | 17 +++++----- 9 files changed, 68 insertions(+), 35 deletions(-) diff --git a/framework/ActiveSync/lib/Horde/ActiveSync/Driver/Horde/Connector/Registry.php b/framework/ActiveSync/lib/Horde/ActiveSync/Driver/Horde/Connector/Registry.php index a190e56e6..5ad961a7f 100644 --- a/framework/ActiveSync/lib/Horde/ActiveSync/Driver/Horde/Connector/Registry.php +++ b/framework/ActiveSync/lib/Horde/ActiveSync/Driver/Horde/Connector/Registry.php @@ -58,7 +58,8 @@ class Horde_ActiveSync_Driver_Horde_Connector_Registry false, // Alarms only false, // Show remote true, // Hide exception events - false); // Don't return multi-day events on *each* day + false, // Don't return multi-day events on *each* day + true); // Return Tags } catch (Exception $e) { return array(); } @@ -108,7 +109,7 @@ class Horde_ActiveSync_Driver_Horde_Connector_Registry * * @param string $uid The calendar id * - * @return The iCalendar representation of the calendar + * @return Horde_ActiveSync_Message_Appointment */ public function calendar_export($uid) { diff --git a/kronolith/lib/Api.php b/kronolith/lib/Api.php index 77b401830..12b3d01be 100644 --- a/kronolith/lib/Api.php +++ b/kronolith/lib/Api.php @@ -790,7 +790,7 @@ class Kronolith_Api extends Horde_Registry_Api } $kronolith_driver = Kronolith::getDriver(null, $calendar); - $events = $kronolith_driver->listEvents(); + $events = $kronolith_driver->listEvents(null, null, null, true, false, true, false, true, true); $version = '2.0'; switch ($contentType) { @@ -1096,7 +1096,7 @@ class Kronolith_Api extends Horde_Registry_Api public function listEvents($startstamp = null, $endstamp = null, $calendars = null, $showRecurrence = true, $alarmsOnly = false, $showRemote = true, $hideExceptions = false, - $coverDates = true) + $coverDates = true, $fetchTags = false) { if (!isset($calendars)) { $calendars = array($GLOBALS['prefs']->getValue('default_share')); @@ -1118,7 +1118,8 @@ class Kronolith_Api extends Horde_Registry_Api $alarmsOnly, $showRemote, $hideExceptions, - $coverDates); + $coverDates, + $fetchTags); } /** diff --git a/kronolith/lib/Driver/Kolab.php b/kronolith/lib/Driver/Kolab.php index 2855c311f..a24c7c5ac 100644 --- a/kronolith/lib/Driver/Kolab.php +++ b/kronolith/lib/Driver/Kolab.php @@ -215,7 +215,8 @@ class Kronolith_Driver_Kolab extends Kronolith_Driver */ public function listEvents($startDate = null, $endDate = null, $showRecurrence = false, $hasAlarm = false, - $json = false, $coverDates = true) + $json = false, $coverDates = true, + $fetchTags = false) { $result = $this->synchronize(); @@ -363,11 +364,10 @@ class Kronolith_Driver_Kolab extends Kronolith_Driver $result = $this->_store->save($attributes, $stored_uid); /* Deal with tags */ - $tagger = Kronolith::getTagger(); if (!empty($edit)) { - $tagger->replaceTags($event->uid, $event->tags, $event->creator, 'event'); + Kronolith::getTagger()->replaceTags($event->uid, $event->tags, $event->creator, 'event'); } else { - $tagger->tag($event->uid, $event->tags, $event->creator, 'event'); + Kronolith::getTagger()->tag($event->uid, $event->tags, $event->creator, 'event'); } $cal = $GLOBALS['kronolith_shares']->getShare($event->calendar); diff --git a/kronolith/lib/Driver/Sql.php b/kronolith/lib/Driver/Sql.php index 0490f102b..2b547ec2f 100644 --- a/kronolith/lib/Driver/Sql.php +++ b/kronolith/lib/Driver/Sql.php @@ -244,13 +244,15 @@ class Kronolith_Driver_Sql extends Kronolith_Driver * toJson() method? * @param boolean $coverDates Whether to add the events to all days * that they cover. + * @param boolean $fetchTags Whether to fetch tags for all events * * @return array Events in the given time range. * @throws Kronolith_Exception */ public function listEvents($startDate = null, $endDate = null, $showRecurrence = false, $hasAlarm = false, - $json = false, $coverDates = true, $hideExceptions = false) + $json = false, $coverDates = true, $hideExceptions = false, + $fetchTags = false) { if (!is_null($startDate)) { $startDate = clone $startDate; @@ -273,9 +275,16 @@ class Kronolith_Driver_Sql extends Kronolith_Driver $events = $this->_listEventsConditional($startDate, $endDate, $conditions, $values); $results = array(); + if ($fetchTags) { + $tags = Kronolith::getTagger()->getTags(array_keys($events)); + } foreach ($events as $id) { - Kronolith::addEvents($results, $this->getEvent($id), $startDate, - $endDate, $showRecurrence, $json, $coverDates); + $event = $this->getEvent($id); + if (isset($tags) && !empty($tags[$event->uid])) { + $event->setTags($tags[$event->uid]); + } + Kronolith::addEvents($results, $event, $startDate, $endDate, + $showRecurrence, $json, $coverDates); } return $results; @@ -584,8 +593,7 @@ class Kronolith_Driver_Sql extends Kronolith_Driver } /* Update tags */ - $tagger = Kronolith::getTagger(); - $tagger->replaceTags($event->uid, $event->tags, $event->creator, 'event'); + Kronolith::getTagger()->replaceTags($event->uid, $event->tags, $event->creator, 'event'); /* Add tags again, but as the share owner (replaceTags removes ALL tags). */ try { @@ -593,7 +601,7 @@ class Kronolith_Driver_Sql extends Kronolith_Driver } catch (Horde_Share_Exception $e) { throw new Kronolith_Exception($e); } - $tagger->tag($event->uid, $event->tags, $cal->get('owner'), 'event'); + Kronolith::getTagger()->tag($event->uid, $event->tags, $cal->get('owner'), 'event'); /* Update Geolocation */ if ($gDriver = Kronolith::getGeoDriver()) { diff --git a/kronolith/lib/Event.php b/kronolith/lib/Event.php index c5c641f74..a979ec12a 100644 --- a/kronolith/lib/Event.php +++ b/kronolith/lib/Event.php @@ -112,7 +112,7 @@ abstract class Kronolith_Event * * @var array|string */ - public $tags = array(); + protected $_tags = null; /** * Geolocation @@ -300,10 +300,6 @@ abstract class Kronolith_Event if (!is_null($eventObject)) { $this->fromDriver($eventObject); - /* Get tags */ - $tagger = Kronolith::getTagger(); - $this->tags = $tagger->getTags($this->uid, 'event'); - /* Get geolocation data */ if ($gDriver = Kronolith::getGeoDriver()) { try { @@ -368,6 +364,8 @@ abstract class Kronolith_Event case 'span': case 'rowspan': return $this->{'_' . $name}; + case 'tags': + return $this->getTags(); } $trace = debug_backtrace(); trigger_error('Undefined property via __set(): ' . $name @@ -781,7 +779,7 @@ abstract class Kronolith_Event // Tags $categories = $vEvent->getAttributeValues('CATEGORIES'); if (!($categories instanceof PEAR_Error)) { - $this->tags = $categories; + $this->_tags = $categories; } // Description @@ -2244,7 +2242,7 @@ abstract class Kronolith_Event } // Tags. - $this->tags = Horde_Util::getFormData('tags', $this->tags); + $this->_tags = Horde_Util::getFormData('tags', $this->tags); // Geolocation $this->geoLocation = array('lat' => Horde_Util::getFormData('lat'), @@ -2706,6 +2704,30 @@ abstract class Kronolith_Event return 'kronolithEvent'; } + /** + * Setter for tags + * + * @param array $tags An array of tag_names + */ + public function setTags($tags) + { + $this->_tags = $tags; + } + + /** + * Getter for tags + * + * @return mixed An array of tag_names, or false if tags were never set. + */ + public function getTags() + { + if (!isset($this->_tags)) { + $this->_tags = Kronolith::getTagger()->getTags($this->uid, 'event'); + } + + return $this->_tags; + } + private function _formIDEncode($id) { return str_replace(array('[', ']'), diff --git a/kronolith/lib/Event/Kolab.php b/kronolith/lib/Event/Kolab.php index e05f1b5a1..e60024f4b 100644 --- a/kronolith/lib/Event/Kolab.php +++ b/kronolith/lib/Event/Kolab.php @@ -39,6 +39,7 @@ class Kronolith_Event_Kolab extends Kronolith_Event $this->alarm = $alarm; parent::__construct($driver, $eventObject); + $this->_tags = Kronolith::getTagger()->getTags($this->uid, 'event'); } public function fromDriver($event) diff --git a/kronolith/lib/Kronolith.php b/kronolith/lib/Kronolith.php index 99bca3216..343c41251 100644 --- a/kronolith/lib/Kronolith.php +++ b/kronolith/lib/Kronolith.php @@ -466,6 +466,8 @@ class Kronolith * listTimeObjects as well? * @param boolean $hideExceptions Hide events that represent exceptions to * a recurring event? + * @param boolean $fetchTags Should we fetch each event's tags from + * storage? * * @return array The events happening in this time period. * @throws Kronolith_Exception @@ -473,7 +475,8 @@ class Kronolith public static function listEvents($startDate, $endDate, $calendars = null, $showRecurrence = true, $alarmsOnly = false, $showRemote = true, - $hideExceptions = false, $coverDates = true) + $hideExceptions = false, $coverDates = true, + $fetchTags = false) { $results = array(); @@ -486,7 +489,7 @@ class Kronolith $driver->open($calendar); $events = $driver->listEvents($startDate, $endDate, $showRecurrence, $alarmsOnly, false, $coverDates, - $hideExceptions); + $hideExceptions, $fetchTags); self::mergeEvents($results, $events); } diff --git a/kronolith/lib/Resource/Single.php b/kronolith/lib/Resource/Single.php index 2ac8f0402..cc2991e32 100644 --- a/kronolith/lib/Resource/Single.php +++ b/kronolith/lib/Resource/Single.php @@ -156,7 +156,7 @@ class Kronolith_Resource_Single extends Kronolith_Resource_Base $to->status = $from->status; $to->description = $from->description; $to->url = $from->url; - $to->tags = $from->tags; + $to->setTags($from->tags); $to->geoLocation = $from->geoLocation; $to->first = $from ->first; $to->last = $from->last; diff --git a/kronolith/lib/Tagger.php b/kronolith/lib/Tagger.php index 8091d263f..b70c687f1 100644 --- a/kronolith/lib/Tagger.php +++ b/kronolith/lib/Tagger.php @@ -36,7 +36,7 @@ class Kronolith_Tagger $this->_type_ids = array('calendar' => (int)$types[0], 'event' => (int)$types[1]); - // Cache the tagger statically + // Cache the tagger $this->_tagger = $GLOBALS['injector']->getInstance('Content_Tagger'); } @@ -69,22 +69,19 @@ class Kronolith_Tagger /** * Retrieves the tags on given object(s). * - * @param string $localId The identifier of the kronolith object. + * @param mixed $localId Either the identifier of the kronolith object or + * an array of identifiers. * @param string $type The type of object $localId represents. * - * @return a tag_id => tag_name hash. + * @return array A tag_id => tag_name hash, possibly wrapped in a localid hash. */ public function getTags($localId, $type = 'event') { - if (!is_array($localId)) { - $localId = array($localId); - } - $tags = array(); - foreach ($localId as $id) { - $tags = $tags + $this->_tagger->getTags(array('objectId' => array('object' => $id, 'type' => $this->_type_ids[$type]))); + if (is_array($localId)) { + return $this->_tagger->getTagsByObjects($localId, $type); } - return $tags; + return $this->_tagger->getTags(array('objectId' => array('object' => $localId, 'type' => $this->_type_ids[$type]))); } /** -- 2.11.0