a few import / xmlrpc improvements;

- put back debug code I had inadvertently removed that didn't belong to
  me
- throw an exception i think is missing (someone please review this)
- better oo & namespacing
- better validation of incoming 'data' per format
- paths to zip and zip args (relies on a patch that i am about to put on
  master)

htdocs/api/xmlrpc/client.php

@@ -91,6 +91,7 @@ class Client {
             $xml = new SimpleXMLElement($this->rawresponse);
         }
         catch (Exception $e) {
+            log_debug($this->rawresponse);
             throw new XmlrpcClientException('Payload is not a valid XML document (payload is above)', 6001);
         }
 
@@ -155,6 +156,7 @@ class Client {
                         throw new XmlrpcClientException('Remote site claims to have sent a public key, but they LIE');
                     }
                 }
+                throw new XmlrpcClientException('Unknown error occured: ' . $this->response['faultCode'] . ': ' . $this->response['faultString']);
             }
 
             // Clean up so object can be re-used.

htdocs/api/xmlrpc/lib.php

@@ -268,17 +268,17 @@ function user_authorise($token, $useragent) {
 
 function send_content_intent($username) {
     global $REMOTEWWWROOT;
+    require_once('import.php');
 
     if (!$user = find_remote_user($username, $REMOTEWWWROOT)) {
-        // @todo return an error message we can understand
-        return false;
+        throw new ImportException("Could not find user $username for $REMOTEWWWROOT");
     }
 
     // @todo penny check for zip libraries here
 
     // check whatever config values we have to check
     // generate a token, insert it into the queue table
-    $usequeue = 1; // @todo change this (check whatever)
+    $usequeue = (int)!(Importer::import_immediately_allowed());
 
     $queue = new StdClass;
     $queue->token = generate_token();
@@ -296,8 +296,9 @@ function send_content_intent($username) {
     );
 }
 
-function send_content_ready($token, $username, $format, $filesmanifest, $fetchnow=false) {
+function send_content_ready($token, $username, $format, $importdata, $fetchnow=false) {
     global $REMOTEWWWROOT;
+    require_once('import.php');
 
     if (!$user = find_remote_user($username, $REMOTEWWWROOT)) {
         throw new ImportException("Could not find user $username for $REMOTEWWWROOT");
@@ -312,18 +313,25 @@ function send_content_ready($token, $username, $format, $filesmanifest, $fetchno
         throw new ImportException("Queue record has expired");
     }
 
-    // @todo penny verify format and filesmanifest
-
     $queue->format = $format;
-    $queue->data = serialize(array('filesmanifest' => $filesmanifest));
+    $class = null;
+    try {
+        $class = Importer::class_from_format($format);
+    } catch (Exception $e) {
+        throw new ImportException('Invalid format $format');
+    }
+    try {
+        call_static_method($class, 'validate_import_data', $importdata);
+    } catch (Exception $e) {
+        throw new ImportException('Invalid importdata: ' . $e->getMessage());
+    }
+
+    $queue->data = serialize($importdata);
 
     update_record('import_queue', $queue);
 
     $result = new StdClass;
-    // @todo penny change whatever we need here to match
-    // send_content_intent
-    if ($fetchnow && true) {
-        require_once('import.php');
+    if ($fetchnow && Importer::import_immediately_allowed()) {
         // either immediately spawn a curl request to go fetch the file
         $importer = Importer::create_importer($queue->id, $queue);
         $importer->prepare();
@@ -335,7 +343,6 @@ function send_content_ready($token, $username, $format, $filesmanifest, $fetchno
         $result->status = set_field('import_queue', 'ready', 1, 'id', $queue->id);
         $result->type = 'queued';
     }
-    log_debug($result);
     return $result;
 }
 

htdocs/lib/import.php

@@ -49,6 +49,7 @@ function import_process_queue() {
             $processed[] = $item->id;
         }
         catch (Exception $e) {
+            log_debug('an error occured on import: ' . $e->getMessage());
             $importer->get('importertransport')->cleanup();
         }
     }
@@ -153,21 +154,35 @@ abstract class Importer {
         );
     }
 
+    public static function class_from_format($format) {
+        switch (trim($format)) {
+            case 'file':
+            case 'files':
+                return 'FilesImporter';
+            default:
+                // @todo more laterz (like mahara native and/or leap)
+                throw new ParamException("unknown import format $format");
+
+        }
+    }
+
     public static function create_importer($id, $record=null) {
         if (empty($record)) {
             if (!$record = get_record('import_queue', 'id', $id)) {
                 throw new NotFoundException("Failed to find import queue record with id $id");
             }
         }
-        switch (trim($record->format)) {
-            case 'file':
-            case 'files':
-                return new FilesImporter($id, $record);
-            default:
-                // @todo more laterz (like mahara native and/or leap)
-                throw new ImportException("unknown import format $record->format");
-        }
+        $class = self::class_from_format($record->format);
+        return new $class($id,$record);
+    }
+
+    public static abstract function validate_import_data($importdata);
+
+    public function import_immediately_allowed() {
+    // @todo change this (check whatever)
+        return false;
     }
+
 }
 
 abstract class ImporterTransport {
@@ -198,14 +213,19 @@ class FilesImporter extends Importer {
     public function __construct($id, $record=null) {
         parent::__construct($id, $record);
         $data = $this->get('data');
-        if (empty($data) ||
-            !is_array($data) ||
-            !array_key_exists('filesmanifest', $data) ||
-            !is_array($data['filesmanifest']) ||
-            count($data['filesmanifest']) == 0) {
+        self::validate_import_data($data);
+        $this->manifest = $data['filesmanifest'];
+    }
+
+    public static function validate_import_data($importdata) {
+        if (empty($importdata) ||
+            !is_array($importdata) ||
+            !array_key_exists('filesmanifest', $importdata) ||
+            !is_array($importdata['filesmanifest']) ||
+            count($importdata['filesmanifest']) == 0) {
             throw new ImportException('Missing files manifest in import data');
         }
-        $this->manifest = $data['filesmanifest'];
+        return true;
     }
 
     public function process() {
@@ -225,8 +245,12 @@ class FilesImporter extends Importer {
             throw new ImportException('Failed to create the temporary directories to work in');
         }
 
-        // @todo penny maybe later replace this with a zip library
-        $command = "unzip " . escapeshellarg(get_config('dataroot') . '/' . $this->relativepath . '/' . $this->zipfile) . ' -d ' . escapeshellarg($this->unzipdir);
+        $command = sprintf('%s %s %s %s',
+            get_config('pathtounzip'),
+            escapeshellarg(get_config('dataroot') . '/' . $this->relativepath . '/' . $this->zipfile),
+            get_config('unzipdirarg'),
+            escapeshellarg($this->unzipdir)
+        );
         $output = array();
         exec($command, $output, $returnvar);
         if ($returnvar != 0) {
@@ -342,8 +366,7 @@ class MnetImporterTransport extends ImporterTransport {
                     ->add_param($this->token)
                     ->send($this->host->wwwroot);
         } catch (XmlrpcClientException $e) {
-            log_debug($e);
-            throw new ImportException('Failed to retrieve zipfile from remote server');
+            throw new ImportException('Failed to retrieve zipfile from remote server: ' . $e->getMessage());
         }
         if (!$filecontents = base64_decode($client->response)) {
             throw new ImportException('Failed to retrieve zipfile from remote server');