SAK-31743 Restoring files from trash can get quota exceeded (sakaiproject#3318)

Author: juanjmerono

kernel/kernel-impl/src/main/java/org/sakaiproject/content/impl/BaseContentService.java

@@ -4658,6 +4658,24 @@ public void restoreResource(String id) throws PermissionException, IdUsedExcepti
 			newResource.setResourceType(deleResource.getResourceType());
 			newResource.setAvailability(deleResource.isHidden(), deleResource.getReleaseDate(),deleResource.getRetractDate());
 			newResource.setContent(m_storage.streamDeletedResourceBody(deleResource));
+			try {
+				// If you're storing the file in DB this breaks as it removes the restored file.
+				removeDeletedResource(deleResource);
+				// close the edit object
+				((BaseResourceEdit) deleResource).closeEdit();
+			} catch (PermissionException pe) {
+				M_log.error("restoreResource: access to resource not permitted" + id, pe);
+				try
+				{
+					removeResource(newResource.getId());
+				}
+				catch(Exception e1)
+				{
+					// ignore -- no need to remove the resource if it doesn't exist
+					M_log.debug("Unable to remove partially completed resource: " + deleResource.getId() + "\n" + e1);
+				}
+				throw pe;
+			}
 			try {
 				addProperties(newResource.getPropertiesEdit(), deleResource.getProperties());
 				commitResource(newResource, NotificationService.NOTI_NONE);
@@ -4687,24 +4705,6 @@ public void restoreResource(String id) throws PermissionException, IdUsedExcepti
 				}
 				throw e;
 			}
-			try {
-				// If you're storing the file in DB this breaks as it removes the restored file.
-				removeDeletedResource(deleResource);
-				// close the edit object
-				((BaseResourceEdit) deleResource).closeEdit();
-			} catch (PermissionException pe) {
-				M_log.error("restoreResource: access to resource not permitted" + id, pe);
-				try
-				{
-					removeResource(newResource.getId());
-				}
-				catch(Exception e1)
-				{
-					// ignore -- no need to remove the resource if it doesn't exist
-					M_log.debug("Unable to remove partially completed resource: " + deleResource.getId() + "\n" + e1);
-				}
-				throw pe;
-			}
 		} catch (IdUnusedException iue) {
 			M_log.error("restoreResource: cannot locate deleted resource " + id, iue);
 			throw iue;

kernel/kernel-impl/src/test/java/org/sakaiproject/content/impl/test/ContentHostingServiceRecycleTest.java

@@ -12,9 +12,14 @@
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.sakaiproject.content.api.ContentCollection;
+import org.sakaiproject.content.api.ContentCollectionEdit;
 import org.sakaiproject.content.api.ContentHostingService;
 import org.sakaiproject.content.api.ContentResource;
 import org.sakaiproject.content.api.ContentResourceEdit;
+import org.sakaiproject.entity.api.ResourceProperties;
+import org.sakaiproject.entity.api.ResourcePropertiesEdit;
+import org.sakaiproject.exception.OverQuotaException;
 import org.sakaiproject.exception.IdUnusedException;
 import org.sakaiproject.exception.IdUsedException;
 import org.sakaiproject.test.SakaiKernelTestBase;
@@ -27,7 +32,8 @@
  */
 public class ContentHostingServiceRecycleTest  extends SakaiKernelTestBase {
 	private static Logger log = LoggerFactory.getLogger(ContentHostingServiceRecycleTest.class);
-
+	private static final String SAMPLE_FOLDER = "/user/admin/";
+	
 	@BeforeClass
 	public static void beforeClass() {
 		try {
@@ -146,6 +152,66 @@ public void testDeleteResourceRestoreOnTop() throws Exception {
         ch.removeResource(filename);
     }
 
+    /**
+     * This is to check that when a restore is attempted and the file exceed quota
+     * the file is not restored and we correctly unlock it.
+     * @throws Exception
+     */
+	@Test
+    public void testRestoreOnOverquota() throws Exception {
+        ContentHostingService ch = getService(ContentHostingService.class);
+        SessionManager sm = getService(SessionManager.class);
+        ThreadLocalManager tl = getService(ThreadLocalManager.class);
+        reset(tl, sm);
+
+        // Set quota to 1kb
+		ResourcePropertiesEdit props = ch.newResourceProperties();
+		props.addProperty (ResourceProperties.PROP_COLLECTION_BODY_QUOTA,"1");
+		ContentCollection c = ch.addCollection(SAMPLE_FOLDER,props);
+		ContentCollectionEdit ce = ch.editCollection(SAMPLE_FOLDER);
+		ch.commitCollection(ce);
+
+		long quota = ch.getQuota(ch.getCollection(SAMPLE_FOLDER));
+		Assert.assertEquals("The quota is set to 1",1,quota);
+		
+        // Create a file
+        String filename = SAMPLE_FOLDER + UUID.randomUUID().toString();
+
+        try {
+        	ContentResourceEdit resource = ch.addResource(filename);
+        	resource.setContent(new byte[1048]);
+        	ch.commitResource(resource);
+        	Assert.fail("We should have exceed the quota.");
+        } catch (OverQuotaException oqe) {
+        	// OverQuota Resource Goes to Trash
+        }
+        
+        try {
+            ch.getResource(filename);
+            Assert.fail("We shouldn't be able to find: "+ filename);
+        } catch (IdUnusedException e) {
+            // Expected
+        }
+        
+        try {
+            ch.restoreResource(filename);
+            Assert.fail("We shouldn't be able to restore: "+ filename);
+        } catch (OverQuotaException e) {
+            // Expected
+        }
+
+        try {
+            ch.getResource(filename);
+            Assert.fail("We shouldn't be able to find: "+ filename);
+        } catch (IdUnusedException e) {
+            // Expected
+        }
+
+        List<ContentResource> allDeleted = ch.getAllDeletedResources(SAMPLE_FOLDER);
+        Assert.assertEquals("There should only be one copy of the file in the recycle bin.", 1, allDeleted.size());
+
+    }
+
     /**
      * Clear out any threadlocals and reset the session to be admin.
      * @param tl ThreadLocalManager service.