Merge branch '839_itemprivacy_cleanup_mgrauer'

* 839_itemprivacy_cleanup_mgrauer: BUG: refs #0839. Removed incorrect call to setprivacystatus. BUG: refs #0839. Subtle bug, created folder did not have current privacy status. BUG: refs #0839. Fixed method for setting privacy status.
midasplatform · Oct 24, 2012 · a8253ea · a8253ea
2 parents 41d696f + b28bc12
commit a8253ea
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 31 deletions.
diff --git a/core/models/base/ItemModelBase.php b/core/models/base/ItemModelBase.php
@@ -212,7 +212,6 @@ function duplicateItem($itemDao, $userDao, $folderDao)
     $newItem = $this->createItem($name, $description, $folderDao);
     $newItem->setType($itemDao->getType());
     $newItem->setSizebytes($itemDao->getSizebytes());
-    $newItem->setPrivacyStatus($itemDao->getPrivacyStatus());
     $newItem->setDateCreation(date('c'));
     $newItem->setDateUpdate(date('c'));
 

diff --git a/modules/api/controllers/components/ApiComponent.php b/modules/api/controllers/components/ApiComponent.php
@@ -315,27 +315,6 @@ function uploadGeneratetoken($args)
         throw new Exception('Invalid policy or folderid', MIDAS_INVALID_POLICY);
         }
       // create a new item in this folder
-      if(isset($args['itemprivacy']))
-        {
-        $privacy = $args['itemprivacy'];
-        if($privacy !== 'Public' && $privacy !== 'Private')
-          {
-          throw new Exception('itemprivacy should be one of [Public|Private]');
-          }
-        if($privacy === 'Public')
-          {
-          $privacy_status = MIDAS_PRIVACY_PUBLIC;
-          }
-        else
-          {
-          $privacy_status = MIDAS_PRIVACY_PRIVATE;
-          }
-        }
-      else
-        {
-        // Public by default
-        $privacy_status = MIDAS_PRIVACY_PUBLIC;
-        }
       $itemname = isset($args['itemname']) ? $args['itemname'] : $args['filename'];
       $description = isset($args['itemdescription']) ? $args['itemdescription'] : '';
       $item = $itemModel->createItem($itemname, $description, $folder);
@@ -345,8 +324,17 @@ function uploadGeneratetoken($args)
         }
       $itempolicyuserModel = MidasLoader::loadModel('Itempolicyuser');
       $itempolicyuserModel->createPolicy($userDao, $item, MIDAS_POLICY_ADMIN);
-      $item->setPrivacyStatus($privacy_status);
-      $itemModel->save($item);
+
+      if(isset($args['itemprivacy']))
+        {
+        $privacyCode = $this->_getValidPrivacyCode($args['itemprivacy']);
+        }
+      else
+        {
+        // Public by default
+        $privacyCode = MIDAS_PRIVACY_PUBLIC;
+        }
+      $this->_setItemPrivacy($item, $privacyCode);
       }
 
     if(array_key_exists('checksum', $args))
@@ -832,6 +820,7 @@ protected function _setFolderPrivacy($folder, $privacyCode)
     $groupModel = MidasLoader::loadModel('Group');
     $anonymousGroup = $groupModel->load(MIDAS_GROUP_ANONYMOUS_KEY);
     $folderpolicygroupDao = $folderpolicygroupModel->getPolicy($anonymousGroup, $folder);
+
     if($privacyCode == MIDAS_PRIVACY_PRIVATE && $folderpolicygroupDao !== false)
       {
       $folderpolicygroupModel->delete($folderpolicygroupDao);
@@ -840,6 +829,11 @@ protected function _setFolderPrivacy($folder, $privacyCode)
       {
       $policyDao = $folderpolicygroupModel->createPolicy($anonymousGroup, $folder, MIDAS_POLICY_READ);
       }
+    else
+      {
+      // ensure the cached privacy status value is up to date
+      $folderpolicygroupModel->computePolicyStatus($folder);
+      }
     }
 
   /**
@@ -950,7 +944,8 @@ function folderCreate($args)
         // explicitly set to Public
         $this->_setFolderPrivacy($new_folder, MIDAS_PRIVACY_PUBLIC);
         }
-
+      // reload folder to get up to date privacy status
+      $new_folder = $folderModel->load($new_folder->getFolderId());
       return $new_folder->toArray();
       }
     }
@@ -1475,6 +1470,11 @@ protected function _setItemPrivacy($item, $privacyCode)
       {
       $itempolicygroupDao = $itempolicygroupModel->createPolicy($anonymousGroup, $item, MIDAS_POLICY_READ);
       }
+    else
+      {
+      // ensure the cached privacy status value is up to date
+      $itempolicygroupModel->computePolicyStatus($item);
+      }
     }
 
 

diff --git a/modules/api/tests/controllers/ApiCallFolderMethodsTest.php b/modules/api/tests/controllers/ApiCallFolderMethodsTest.php
@@ -48,6 +48,8 @@ public function testFolderCreate()
     $this->assertEquals($userDao->getPublicfolderId(), $resp->data->parent_id);
     $this->assertEquals('testFolderCreate', $resp->data->name);
     $this->assertEquals('', $resp->data->description);
+    // ensure privacy status is correct on returned array
+    $this->assertEquals($resp->data->privacy_status, MIDAS_PRIVACY_PUBLIC, 'created folder has wrong privacy status');
     // ensure default privacy is Public
     $this->assertPrivacyStatus(array($folderModel->load($resp->data->folder_id)), array(), MIDAS_PRIVACY_PUBLIC);
 
@@ -60,6 +62,8 @@ public function testFolderCreate()
     $this->params['privacy'] = 'Public';
     $resp = $this->_callJsonApi();
     $this->_assertStatusOk($resp);
+    // ensure privacy status is correct on returned array
+    $this->assertEquals($resp->data->privacy_status, MIDAS_PRIVACY_PUBLIC, 'created folder has wrong privacy status');
     $this->assertPrivacyStatus(array($folderModel->load($resp->data->folder_id)), array(), MIDAS_PRIVACY_PUBLIC);
 
     $this->resetAll();
@@ -70,6 +74,9 @@ public function testFolderCreate()
     $this->params['privacy'] = 'Private';
     $resp = $this->_callJsonApi();
     $this->_assertStatusOk($resp);
+    // ensure privacy status is correct on returned array
+    $this->assertEquals($resp->data->privacy_status, MIDAS_PRIVACY_PRIVATE, 'created folder has wrong privacy status');
+    // ensure privacy status is correct on loaded resource
     $this->assertPrivacyStatus(array($folderModel->load($resp->data->folder_id)), array(), MIDAS_PRIVACY_PRIVATE);
 
     // try to create a folder where have read access on the parent folder

diff --git a/modules/api/tests/controllers/ApiCallItemMethodsTest.php b/modules/api/tests/controllers/ApiCallItemMethodsTest.php
@@ -213,16 +213,17 @@ public function testBitstreamUpload()
      *
      *14 generatetoken with folderid and itemid, ERROR
      *
-     *15 generatetoken passing in folderid
+     *15 generatetoken passing in folderid, default of Public
      *
-     *16 generatetoken passing in folderid and setting optional values
+     *16 generatetoken passing in folderid and setting optional values with Private itemprivacy
      *17 upload without passing a revision to 16's item, this should create a revision 1
      *18 generate a token for 16's item
      *19 upload to an item with an existing revision, without the revision parameter, which should create a new revision 2
      *20 generate a token for 16's item, after creating 2 new revisions, 3 and 4
      *21 upload to an item with 4 revisions, with revision parameter of 3, check that revision 3 has the bitstream and revision 4 has 0 bitstreams
      *
-     *22 generatetoken passing in folderid and a checksum of an existing bitstream
+     *22 generatetoken passing in folderid and a checksum of an existing bitstream,
+     *   and passing Public explicitly
      *
      */
 
@@ -527,7 +528,7 @@ public function testBitstreamUpload()
     $this->_assertStatusFail($resp, MIDAS_INVALID_PARAMETER);
 
     // 15
-    // test upload.generatetoken passing in folderid
+    // test upload.generatetoken passing in folderid, default of Public
     $this->resetAll();
     $this->params['token'] = $this->_loginAsNormalUser();
     $this->params['method'] = 'midas.upload.generatetoken';
@@ -556,7 +557,8 @@ public function testBitstreamUpload()
     $this->Item->delete($itemDao);
 
     // 16
-    // test upload.generatetoken passing in folderid and setting optional values
+    // test upload.generatetoken passing in folderid and setting optional values,
+    // with Private itemprivacy
     $filename = 'test.txt';
     $description = 'generated item description';
     $itemname = 'generated item name';
@@ -723,13 +725,15 @@ public function testBitstreamUpload()
     // with a previously uploaded bitstream, and we are passing the checksum,
     // we expect that the item will create a new revision for the bitstream,
     // but pass back an empty upload token, since we have the bitstream content
-    // already and do not need to actually upload it
+    // already and do not need to actually upload it; also test passing in Public
+    // explictly
     $this->resetAll();
     $this->params['token'] = $this->_loginAsNormalUser();
     $this->params['method'] = 'midas.upload.generatetoken';
     $this->params['filename'] = $filename;
     $this->params['checksum'] = $md5;
     $this->params['folderid'] = '1000';
+    $this->params['itemprivacy'] = 'Public';
     $resp = $this->_callJsonApi();
     $this->_assertStatusOk($resp);
     $token = $resp->data->token;
@@ -740,6 +744,8 @@ public function testBitstreamUpload()
     // this at least allows us to test it
     $generatedItemId = $generatedItemId + 1;
     $itemDao = $this->Item->load($generatedItemId);
+    // ensure privacy status when passing Public explicitly
+    $this->assertEquals($itemDao->getPrivacyStatus(), MIDAS_PRIVACY_PUBLIC, 'Expected a different privacy_status for generated item');
     $revisions = $itemDao->getRevisions();
     $this->assertEquals(count($revisions), 1, 'Wrong number of revisions in the item');
     $bitstreams = $revisions[0]->getBitstreams();