[Tickets #12801] Re: Unable to parse recurring events during iCalendar import

[noreply at bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/12801
------------------------------------------------------------------------------
  Ticket             | 12801
  Updated By         | wrcdrijfhout at imperamus.eu
  Summary            | Unable to parse recurring events during iCalendar
                     | import
  Queue              | Kronolith
  Version            | 4.1.3
  Type               | Bug
  State              | Resolved
  Priority           | 2. Medium
  Milestone          |
  Patch              |
  Owners             | Jan Schneider
------------------------------------------------------------------------------


wrcdrijfhout at imperamus.eu (2013-11-03 14:55) wrote:

>> * The ICAL-URL is completely empty, supposedly because PHP's request
>> handling died prematurely.
>>
>> I will contact Jan via e-mail about a larger ICS-file with which the
>> problem can be reproduced.
>
> Works perfectly fine for me with a current Horde checkout even with  
> the other ICS file.
While I do not run the latest Horde checkout, I backported the fix you  
committed. The problem persists; I traced it to an OutOfMemory-problem:

In kronolith/lib/Api.php, line 896 onward, I wrote:
`$i=-1;
             foreach ($events as $dayevents) {
print(++$i.'<br/>');
print((memory_get_usage()/1024/1024).'MB / '. ini_get('memory_limit')  
. '<br/>');
                 foreach ($dayevents as $event) {
                     $iCal->addComponent($event->toiCalendar($iCal));
                 }
             }
die();
`

 From this I deduced I exceeded the memory limit. I find it dubious  
that experting a mere 100 calendar entries would require 184MB to  
yield a 2.9MB file; can we optimize this?

In Icalendar.php, we can (at least) do this:
`
     public function addComponent(&$components)
     {
         if (is_array($components)) {
             foreach ($components as &$component) {
                 $this->addComponent($component);
             }
         } else if ($components instanceof Horde_Icalendar) {
             $components->_container = &$this;
             $this->_components[] = &$components;
         }
     }

`
Notice the ampersands; i.e., by-reference/address-of operator. The  
line "$this->_components[] = &$component;" is still the most expensive  
one. Anyhow, by replacing addComponent as such, we cut down memory  
usage from 184MB to 53MB (and gain more performance as well)!

I recommend exploring further applications of passing values  
by-reference (by using the &-operator) instead of by-value throughout  
Horde (especially oft-used functions, like domain-entity functions).




-----------


Also, the previous fix for kronolith/lib/Event.php you added checks  
for the Timezone-object being at index 0 or 1, but array_pop always  
retrieves the object at the last index (rather than the non-Timezone  
index). I suggest the following generalization:
`
                     foreach ($vEventException_r as $key => &$vEE){
                         if ($vEE instanceof Horde_Icalendar_Vtimezone){
                             unset($vEventException_r[$key]);
                         }
                     }
                     // This should never happen, but protect against  
it anyway.
                     if (count($vEventException_r) > 1) {
                         throw new Kronolith_Exception(_("Unable to  
parse event."));
                     }
                     $vEventException = array_pop($vEventException_r);
`