BUG: refs #0373. Cleaned signature of createNewRevision method.

Cleaned up the signature of the createNewItemRevision method, and made some
changes in related areas for improved readability.  I found that the number
of parameters in the new signature violated a style check, which is probably
why the signature is as it was, but that seems like an arbitrary limit, and
I don't think it should get in the way of clarity of a method signature.

Author: Michael Grauer

core/controllers/UploadController.php

@@ -345,11 +345,13 @@ public function saveuploadedAction()
     $license = $this->_getParam('license');
     if(!empty($path) && file_exists($path))
       {
-      $tmp = explode('-', $parent);
-      if(count($tmp) == 2) //means we upload a new revision
+      $itemId_itemRevisionNumber = explode('-', $parent);
+      if(count($itemId_itemRevisionNumber) == 2) //means we upload a new revision
         {
         $changes = $this->_getParam('changes');
-        $this->Component->Upload->createNewRevision($this->userSession->Dao, $filename, $path, $tmp, $changes, $license);
+        $itemId = $itemId_itemRevisionNumber[0];
+        $itemRevisionNumber = $itemId_itemRevisionNumber[1];
+        $this->Component->Upload->createNewRevision($this->userSession->Dao, $filename, $path, $changes, $itemId, $itemRevisionNumber, $license);
         }
       else
         {

core/controllers/components/UploadComponent.php

@@ -262,7 +262,7 @@ public function createUploadedItem($userDao, $name, $path, $parent = null, $lice
     }//end createUploadedItem
 
   /** save upload item in the DB */
-  public function createNewRevision($userDao, $name, $path, $item_revision, $changes, $license = null, $filemd5 = '')
+  public function createNewRevision($userDao, $name, $path, $changes, $itemId, $itemRevisionNumber = null, $license = null, $filemd5 = '')
     {
     if($userDao == null)
       {
@@ -278,21 +278,24 @@ public function createNewRevision($userDao, $name, $path, $item_revision, $chang
     $itemRevisionModel = $modelLoad->loadModel('ItemRevision');
     $feedpolicyuserModel = $modelLoad->loadModel('Feedpolicyuser');
 
-    $item = $itemModel->load($item_revision[0]);
+    $item = $itemModel->load($itemId);
 
     if($item == false)
       {
       throw new Zend_Exception('Unable to find item');
       }
 
-    $revisions = $item->getRevisions();
     $itemRevisionDao = null;
-    foreach($revisions as $revision)
+    if(isset($itemRevisionNumber))
       {
-      if($item_revision[1] == $revision->getRevision())
+      $revisions = $item->getRevisions();
+      foreach($revisions as $revision)
         {
-        $itemRevisionDao = $revision;
-        break;
+        if($itemRevisionNumber == $revision->getRevision())
+          {
+          $itemRevisionDao = $revision;
+          break;
+          }
         }
       }
 
@@ -360,7 +363,7 @@ public function createNewRevision($userDao, $name, $path, $item_revision, $chang
       }
     $itemRevisionModel->addBitstream($itemRevisionDao, $bitstreamDao);
     // now that we have updated the itemRevision, the item may be stale
-    $item = $itemModel->load($item_revision[0]);
+    $item = $itemModel->load($itemId);
 
     $this->getLogger()->info(__METHOD__." Upload ok :".$path);
     Zend_Registry::get('notifier')->notifyEvent("EVENT_CORE_UPLOAD_FILE", array($itemRevisionDao->getItem()->toArray(), $itemRevisionDao->toArray()));

modules/api/controllers/components/ApiComponent.php

@@ -427,19 +427,22 @@ function uploadPerform($args)
       }
 
     $uploadComponent = $componentLoader->loadComponent('Upload');
+    $license = null;
     if(isset($folder))
       {
-      $item = $uploadComponent->createUploadedItem($userDao, $filename, $filepath, $folder, '', $filemd5);
+      $item = $uploadComponent->createUploadedItem($userDao, $filename, $filepath, $folder, $license, $filemd5);
       }
     else if(isset($revision))
       {
-      $tmp = array($item->getKey(), $revision->getRevision()); //existing revision
-      $item = $uploadComponent->createNewRevision($userDao, $filename, $filepath, $tmp, '', null, $filemd5);
+      //an existing revision
+      $changes = '';
+      $item = $uploadComponent->createNewRevision($userDao, $filename, $filepath, $changes, $item->getKey(), $revision->getRevision(), $license, $filemd5);
       }
     else
       {
-      $tmp = array($item->getKey(), 99999); //new revision
-      $item = $uploadComponent->createNewRevision($userDao, $filename, $filepath, $tmp, '', null, $filemd5);
+      // a new revision
+      $changes = '';
+      $item = $uploadComponent->createNewRevision($userDao, $filename, $filepath, $changes, $item->getKey(), $revision, $license, $filemd5);
       }
 
     if(!$item)

tests/library/PhpCheckstyle/config/default.cfg.xml

@@ -150,7 +150,7 @@
 
     <!--  Checks for excessive parameters in a function declaration -->
     <test name="functionMaxParameters">
-        <property name="maxParameters" value="7"/>
+        <property name="maxParameters" value="8"/>
     </test>
 
     <!-- Check Cyclomatic Complexity -->