Attachment Migrater fixes

include/class.migrater.php

@@ -94,43 +94,51 @@ class DatabaseMigrater {
 include_once(INCLUDE_DIR.'class.file.php');
 
 class AttachmentMigrater {
+
+
     function AttachmentMigrater() {
-        $this->queue = array();
-        $this->current = 0;
-        $this->errors = 0;
+        $this->queue = &$_SESSION['ost_upgrader']['AttachmentMigrater']['queue'];
+        $this->skipList = &$_SESSION['ost_upgrader']['AttachmentMigrater']['skip'];
         $this->errorList = array();
     }
-    /**
-     * Identifies attachments in need of migration and queues them for
-     * migration to the new database schema.
-     * 
-     * @see ::next() for output along the way
-     *
-     * Returns:
-     * TRUE/FALSE to indicate if migration finished without any errors
-     */
-    function start_migration() {
-        $this->findAttachments();
-        $this->total = count($this->queue);
-    }
+
+
     /**
      * Process the migration for a unit of time. This will be used to
      * overcome the execution time restriction of PHP. This instance can be
      * stashed in a session and have this method called periodically to
      * process another batch of attachments
+     *
+     * Returns:
+     * Number of pending attachments to migrate.
      */
-    function do_batch($max, $time=20) {
-        $start = time();
-        $this->errors = 0;
-        while (count($this->queue) && $count++ < $max && time()-$start < $time)
-            $this->next();
-        # TODO: Log success/error indication of migration of attachments
-        return (!$this->errors);
+    function do_batch($time=30, $max=0) {
+
+        if(!$this->queueAttachments() || !$this->getQueueLength())
+            return 0;
+
+        $count = 0;
+        $start = Misc::micro_time();
+        while ($this->getQueueLength() && (Misc::micro_time()-$start) < $time)
+            if($this->next() && $max && ++$count>=$max)
+                break;
+
+        return $this->queueAttachments();
+
+    }
+
+    function getSkipList() {
+        return $this->skipList;
     }
 
     function queue($fileinfo) {
         $this->queue[] = $fileinfo;
     }
+
+    function getQueue() {
+        return $this->queue;
+    }
+
     function getQueueLength() { return count($this->queue); }
     /**
      * Processes the next item on the work queue. Emits a JSON messages to
@@ -146,14 +154,16 @@ class AttachmentMigrater {
         # Attach file to the ticket
         if (!($info['data'] = @file_get_contents($info['path']))) {
             # Continue with next file
-            return $this->error(
+            return $this->skip($info['attachId'],
                 sprintf('%s: Cannot read file contents', $info['path']));
         }
         # Get the mime/type of each file
         # XXX: Use finfo_buffer for PHP 5.3+
-        $info['type'] = mime_content_type($info['path']);
+        if(function_exists('mime_content_type')) //XXX: function depreciated in newer versions of PHP!!!!!
+            $info['type'] = mime_content_type($info['path']);
+
         if (!($fileId = AttachmentFile::save($info))) {
-            return $this->error(
+            return $this->skip($info['attachId'],
                 sprintf('%s: Unable to migrate attachment', $info['path']));
         }
         # Update the ATTACHMENT_TABLE record to set file_id
@@ -163,7 +173,7 @@ class AttachmentMigrater {
         # Remove disk image of the file. If this fails, the migration for
         # this file would not be retried, because the file_id in the
         # TICKET_ATTACHMENT_TABLE has a nonzero value now
-        if (!@unlink($info['path']))
+        if (!@unlink($info['path'])) //XXX: what should we do on failure?
             $this->error(
                 sprintf('%s: Unable to remove file from disk',
                 $info['path']));
@@ -174,19 +184,39 @@ class AttachmentMigrater {
      * From (class Ticket::fixAttachments), used to detect the locations of
      * attachment files
      */
-    /* static */ function findAttachments(){
-        global $cfg;
+    /* static */ function queueAttachments($limit=0){
+        global $cfg, $ost;
 
-        $res=db_query('SELECT attach_id, file_name, file_key, Ti.created'
+        # Since the queue is persistent - we want to make sure we get to empty 
+        # before we find more attachments.
+        if(($qc=$this->getQueueLength()))
+            return $qc;
+
+        $sql='SELECT attach_id, file_name, file_key, Ti.created'
             .' FROM '.TICKET_ATTACHMENT_TABLE.' TA'
             .' JOIN '.TICKET_TABLE.' Ti ON Ti.ticket_id=TA.ticket_id'
-            .' WHERE NOT file_id');
-        if (!$res) {
-            return $this->error('Unable to query for attached files');
-        } elseif (!db_num_rows($res)) {
-            return true;
-        }
+            .' WHERE NOT file_id '; //XXX: ignore orphaned attachments?
+
+        if(($skipList=$this->getSkipList()))
+            $sql.= ' AND attach_id NOT IN('.implode(',', db_input($skipList)).')';
+
+        if($limit && is_numeric($limit))
+            $sql.=' LIMIT '.$limit;
+
+        //XXX: Do a hard fail or error querying the database?
+        if(!($res=db_query($sql)))
+            return $this->error('Unable to query DB for attached files to migrate!');
+
+        $ost->logDebug('Found '.db_num_rows($res).' attachments to migrate');
+        if(!db_num_rows($res))
+            return 0;  //Nothing else to do!!
+
         $dir=$cfg->getUploadDir();
+        if(!$dir || !is_dir($dir)) //XXX: Abort the upgrade??? Attachments are obviously critical!
+            return $this->error("Attachment directory [$dir] is invalid - aborting attachment migration");
+
+        //Clear queue
+        $this->queue = array();
         while (list($id,$name,$key,$created)=db_fetch_row($res)) {
             $month=date('my',strtotime($created));
             $info=array(
@@ -201,8 +231,9 @@ class AttachmentMigrater {
                 $info['path'] = $filename16;
             } else {
                 # XXX Cannot find file for attachment
-                $this->error(sprintf('%s: Unable to locate attachment file',
-                    $name));
+                $this->skip($id,
+                        sprintf('%s: Unable to locate attachment file',
+                            $name));
                 # No need to further process this file
                 continue;
             }
@@ -224,11 +255,23 @@ class AttachmentMigrater {
             # Coroutines would be nice ..
             $this->queue($info);
         }
+
+        return $this->getQueueLength();
+    }
+
+    function skip($attachId, $error) {
+
+        $this->skipList[] = $attachId;
+
+        return $this->error($error." (ID #$attachId)");
     }
 
     function error($what) {
+        global $ost;
+
         $this->errors++;
         $this->errorList[] = $what;
+        $ost->logDebug('Upgrader: Attachment Migrater', $what);
         # Assist in returning FALSE for inline returns with this method
         return false;
     }
@@ -236,5 +279,4 @@ class AttachmentMigrater {
         return $this->errorList;
     }
 }
-
 ?>