Merge pull request #1575 from mwoodiupui/DS-2707

[DS-2707] Poor messaging when batch upload directory cannot be created

dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java

@@ -273,23 +273,25 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
             // mode
 
             System.out.println("Adding items from directory: " + sourceDir);
+            log.debug("Adding items from directory: " + sourceDir);
             System.out.println("Generating mapfile: " + mapFile);
+            log.debug("Generating mapfile: " + mapFile);
 
-        boolean directoryFileCollections = false;
+            boolean directoryFileCollections = false;
-        if (mycollections == null)
+            if (mycollections == null)
-        {
-            directoryFileCollections = true;
-        }
-
-        if (!isTest)
-        {
-            // get the directory names of items to skip (will be in keys of
-            // hash)
-            if (isResume)
             {
-                skipItems = readMapFile(mapFile);
+                directoryFileCollections = true;
             }
 
+            if (!isTest)
+            {
+                // get the directory names of items to skip (will be in keys of
+                // hash)
+                if (isResume)
+                {
+                    skipItems = readMapFile(mapFile);
+                }
+
                 // sneaky isResume == true means open file in append mode
                 outFile = new File(mapFile);
                 mapOut = new PrintWriter(new FileWriter(outFile, isResume));
@@ -312,39 +314,39 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
 
             Arrays.sort(dircontents, ComparatorUtils.naturalComparator());
 
-        for (int i = 0; i < dircontents.length; i++)
+            for (int i = 0; i < dircontents.length; i++)
-        {
-            if (skipItems.containsKey(dircontents[i]))
             {
-                System.out.println("Skipping import of " + dircontents[i]);
+                if (skipItems.containsKey(dircontents[i]))
-            }
+                {
-            else
+                    System.out.println("Skipping import of " + dircontents[i]);
-            {
-                List<Collection> clist;
-                if (directoryFileCollections) {
-                    String path = sourceDir + File.separatorChar + dircontents[i];
-                    try {
-                        List<Collection> cols = processCollectionFile(c, path, "collections");
-                        if (cols == null) {
-                            System.out.println("No collections specified for item " + dircontents[i] + ". Skipping.");
-                            continue;
-                        }
-                        clist = cols;
-                    }
-                    catch (IllegalArgumentException e)
-                    {
-                        System.out.println(e.getMessage() + " Skipping." );
-                        continue;
-                    }
                 }
                 else
                 {
-                    clist = mycollections;
+                    List<Collection> clist;
+                    if (directoryFileCollections) {
+                        String path = sourceDir + File.separatorChar + dircontents[i];
+                        try {
+                            List<Collection> cols = processCollectionFile(c, path, "collections");
+                            if (cols == null) {
+                                System.out.println("No collections specified for item " + dircontents[i] + ". Skipping.");
+                                continue;
+                            }
+                            clist = cols;
+                        }
+                        catch (IllegalArgumentException e)
+                        {
+                            System.out.println(e.getMessage() + " Skipping." );
+                            continue;
+                        }
+                    }
+                    else
+                    {
+                        clist = mycollections;
+                    }
+                    addItem(c, clist, sourceDir, dircontents[i], mapOut, template);
+                    System.out.println(i + " " + dircontents[i]);
                 }
-                addItem(c, clist, sourceDir, dircontents[i], mapOut, template);
-                System.out.println(i + " " + dircontents[i]);
             }
-        }
 
         } finally {
             if(mapOut!=null) {
@@ -466,6 +468,7 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
         String mapOutputString = null;
 
         System.out.println("Adding item from directory " + itemname);
+        log.debug("adding item from directory " + itemname);
 
         // create workspace item
         Item myitem = null;
@@ -1652,7 +1655,7 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
             entry = entries.nextElement();
             if (entry.isDirectory())
             {
-                if (!new File(zipDir + entry.getName()).mkdir())
+                if (!new File(zipDir + entry.getName()).mkdirs())
                 {
                     log.error("Unable to create contents directory: " + zipDir + entry.getName());
                 }
@@ -2040,17 +2043,28 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea
     }
 
     @Override
-    public File getTempWorkDirFile() {
+    public File getTempWorkDirFile()
+            throws IOException
+    {
         File tempDirFile = new File(getTempWorkDir());
         if(!tempDirFile.exists()) {
-            tempDirFile.mkdirs();
+            boolean success = tempDirFile.mkdirs();
+            if (!success)
+            {
+                throw new IOException("Work directory "
+                         + tempDirFile.getAbsolutePath()
+                         + " could not be created.");
+            }
+            else log.debug("Created directory " + tempDirFile.getAbsolutePath());
         }
+        else log.debug("Work directory exists:  " + tempDirFile.getAbsolutePath());
         return tempDirFile;
     }
 
     @Override
     public void cleanupZipTemp() {
         System.out.println("Deleting temporary zip directory: " + tempWorkDir);
+        log.debug("Deleting temporary zip directory: " + tempWorkDir);
         deleteDirectory(new File(tempWorkDir));
     }
 

dspace-api/src/main/java/org/dspace/app/itemimport/service/ItemImportService.java

@@ -199,8 +199,9 @@ public interface ItemImportService {
     /**
      * Get temporary work directory (as File)
      * @return directory as File
+     * @throws java.io.IOException if the directory cannot be created.
      */
-    public File getTempWorkDirFile();
+    public File getTempWorkDirFile() throws IOException;
 
     /**
      * Cleanup

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/administrative/FlowBatchImportUtils.java

@@ -13,7 +13,6 @@ import org.apache.cocoon.servlet.multipart.PartOnDisk;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
-import org.dspace.app.itemimport.ItemImportServiceImpl;
 import org.dspace.app.itemimport.factory.ItemImportServiceFactory;
 import org.dspace.app.itemimport.service.ItemImportService;
 import org.dspace.app.xmlui.wing.Message;
@@ -21,7 +20,6 @@ import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
 import org.dspace.core.Context;
 import org.dspace.core.LogManager;
-import org.dspace.handle.HandleServiceImpl;
 import org.dspace.handle.factory.HandleServiceFactory;
 import org.dspace.handle.service.HandleService;
 
@@ -51,7 +49,7 @@ public class FlowBatchImportUtils {
     private static final Message T_failed_no_collection = new Message("default", "xmlui.administrative.batchimport.flow.failed_no_collection");
 
     // Other variables
-    private static Logger log = Logger.getLogger(FlowBatchImportUtils.class);
+    private static final Logger log = Logger.getLogger(FlowBatchImportUtils.class);
 
     protected static final HandleService handleService = HandleServiceFactory.getInstance().getHandleService();
     protected static final ItemImportService itemImportService = ItemImportServiceFactory.getInstance().getItemImportService();
@@ -138,17 +136,19 @@ public class FlowBatchImportUtils {
                 File mapFile = null;
                 try {
                     mapFile = File.createTempFile(file.getName(), ".map", itemImportService.getTempWorkDirFile());
+                    log.debug("Temporary map file " + mapFile.getAbsolutePath());
                 } catch (IOException e) {
                     log.error("BatchImportUI Unable to create mapfile", e);
                     result.setContinue(false);
                     result.setOutcome(false);
                     result.setMessage(T_import_failed);
+                    result.setCharacters("On server:  " + e.getMessage());
                     return result;
                 }
 
 
                 log.info("Attempt UIBatchImport to collection: " + collections.get(0).getName()
-                        + ", zip: " + file.getName()
+                        + ", zip: " + file.getAbsolutePath()
                         + ", map: " + mapFile.getAbsolutePath());
 
             /*
@@ -176,6 +176,7 @@ public class FlowBatchImportUtils {
                 String sourceBatchDir = null;
                 try {
                     sourceBatchDir = itemImportService.unzip(file);
+                    log.debug("Unzipped to " + sourceBatchDir);
                 } catch (IOException e) {
                     log.error("BatchImportUI Unable to unzip", e);
                     result.setContinue(false);