[DS-4087] First pass at better error reporting.

dspace-api/src/main/java/org/dspace/administer/StructBuilder.java

@@ -21,8 +21,10 @@ import javax.xml.transform.TransformerException;
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.CommandLineParser;
+import org.apache.commons.cli.DefaultParser;
+import org.apache.commons.cli.HelpFormatter;
 import org.apache.commons.cli.Options;
-import org.apache.commons.cli.PosixParser;
+import org.apache.commons.cli.ParseException;
 import org.apache.xpath.XPathAPI;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
@@ -45,6 +47,7 @@ import org.xml.sax.SAXException;
  * an XML file.
  *
  * The XML file structure needs to be:
+ * <p>
  * {@code
  * <import_structure>
  * <community>
@@ -56,29 +59,31 @@ import org.xml.sax.SAXException;
  * </community>
  * </import_structure>
  * }
- * it can be arbitrarily deep, and supports all the metadata elements
+ * <p>
+ * It can be arbitrarily deep, and supports all the metadata elements
  * that make up the community and collection metadata.  See the system
- * documentation for more details
+ * documentation for more details.
  *
  * @author Richard Jones
  */
 
 public class StructBuilder {
     /**
-     * the output xml document which will contain updated information about the
+     * The output XML document which will contain updated information about the
-     * imported structure
+     * imported structure.
      */
-    private static org.jdom.Document xmlOutput = new org.jdom.Document(new Element("imported_structure"));
+    private static final org.jdom.Document xmlOutput
+            = new org.jdom.Document(new Element("imported_structure"));
 
     /**
-     * a hashtable to hold metadata for the collection being worked on
+     * A hash table to hold metadata for the collection being worked on.
      */
-    private static Map<String, String> collectionMap = new HashMap<String, String>();
+    private static final Map<String, String> collectionMap = new HashMap<>();
 
     /**
-     * a hashtable to hold metadata for the community being worked on
+     * A hash table to hold metadata for the community being worked on.
      */
-    private static Map<String, String> communityMap = new HashMap<String, String>();
+    private static final Map<String, String> communityMap = new HashMap<>();
 
     protected static CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService();
     protected static CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService();
@@ -101,19 +106,34 @@ public class StructBuilder {
      * with the handle for each imported item added as an attribute.
      *
      * @param argv the command line arguments given
-     * @throws Exception if an error occurs
+     * @throws ParserConfigurationException passed through.
+     * @throws SQLException passed through.
      */
     public static void main(String[] argv)
-        throws Exception {
+            throws ParserConfigurationException, SQLException {
-        CommandLineParser parser = new PosixParser();
+        CommandLineParser parser = new DefaultParser();
 
         Options options = new Options();
 
-        options.addOption("f", "file", true, "file");
+        options.addOption("h", "help", false, "help");
+        options.addOption("?", "help");
+        options.addOption("f", "file", true, "input structure document");
         options.addOption("e", "eperson", true, "eperson");
-        options.addOption("o", "output", true, "output");
+        options.addOption("o", "output", true, "output structure document");
 
-        CommandLine line = parser.parse(options, argv);
+        CommandLine line = null;
+        try {
+            line = parser.parse(options, argv);
+        } catch (ParseException ex) {
+            System.err.println(ex.getMessage());
+            usage(options);
+            System.exit(1);
+        }
+
+        if (line.hasOption('h') || line.hasOption('?')) {
+            usage(options);
+            System.exit(0);
+        }
 
         String file = null;
         String eperson = null;
@@ -132,22 +152,41 @@ public class StructBuilder {
         }
 
         if (output == null || eperson == null || file == null) {
-            usage();
+            usage(options);
-            System.exit(0);
+            System.exit(1);
         }
 
         // create a context
         Context context = new Context();
 
         // set the context
+        try {
             context.setCurrentUser(ePersonService.findByEmail(context, eperson));
+        } catch (SQLException ex) {
+            System.err.format("That user could not be found:  %s%n", ex.getMessage());
+            System.exit(1);
+        }
 
         // load the XML
-        Document document = loadXML(file);
+        Document document = null;
+        try {
+            document = loadXML(file);
+        } catch (IOException ex) {
+            System.err.format("The input document could not be read:  %s%n", ex.getMessage());
+            System.exit(1);
+        } catch (SAXException ex) {
+            System.err.format("The input document could not be parsed:  %s%n", ex.getMessage());
+            System.exit(1);
+        }
 
         // run the preliminary validation, to be sure that the the XML document
         // is properly structured
+        try {
             validate(document);
+        } catch (TransformerException ex) {
+            System.err.format("The input document is invalid:  %s%n", ex.getMessage());
+            System.exit(1);
+        }
 
         // load the mappings into the member variable hashmaps
         communityMap.put("name", "name");
@@ -164,52 +203,61 @@ public class StructBuilder {
         collectionMap.put("license", "license");
         collectionMap.put("provenance", "provenance_description");
 
+        Element[] elements = new Element[]{};
+        try {
             // get the top level community list
             NodeList first = XPathAPI.selectNodeList(document, "/import_structure/community");
 
             // run the import starting with the top level communities
-        Element[] elements = handleCommunities(context, first, null);
+            elements = handleCommunities(context, first, null);
+        } catch (TransformerException ex) {
+            System.err.format("Input content not understood:  %s%n", ex.getMessage());
+            System.exit(1);
+        } catch (AuthorizeException ex) {
+            System.err.format("Not authorized:  %s%n", ex.getMessage());
+            System.exit(1);
+        }
 
         // generate the output
         Element root = xmlOutput.getRootElement();
-        for (int i = 0; i < elements.length; i++) {
+        for (Element element : elements) {
-            root.addContent(elements[i]);
+            root.addContent(element);
         }
 
         // finally write the string into the output file
-        try {
+        try (BufferedWriter out = new BufferedWriter(new FileWriter(output));) {
-            BufferedWriter out = new BufferedWriter(new FileWriter(output));
             out.write(new XMLOutputter().outputString(xmlOutput));
-            out.close();
         } catch (IOException e) {
             System.out.println("Unable to write to output file " + output);
-            System.exit(0);
+            System.exit(1);
         }
 
         context.complete();
     }
 
     /**
-     * Output the usage information
+     * Output the usage information.
      */
-    private static void usage() {
+    private static void usage(Options options) {
-        System.out.println("Usage: java StructBuilder -f <source XML file> -o <output file> -e <eperson email>");
+        HelpFormatter helper = new HelpFormatter();
-        System.out.println(
+        helper.printHelp("Usage: java StructBuilder -f <source XML file> -o <output file> -e <eperson email>",
-            "Communities will be created from the top level, and a map of communities to handles will be returned in " +
+                "Load community/collection structure from a file.",
-                "the output file");
+                options,
-        return;
+                "Communities will be created from the top level,"
+                        + " and a map of communities to handles will be returned"
+                        + " in the output file");
     }
 
     /**
-     * Validate the XML document.  This method does not return, but if validation
+     * Validate the XML document.  This method returns if the document is valid.
-     * fails it generates an error and ceases execution
+     * If validation fails it generates an error and ceases execution.
      *
      * @param document the XML document object
      * @throws TransformerException if transformer error
      */
     private static void validate(org.w3c.dom.Document document)
         throws TransformerException {
-        StringBuffer err = new StringBuffer();
+        StringBuilder err = new StringBuilder();
         boolean trip = false;
 
         err.append("The following errors were encountered parsing the source XML\n");
@@ -236,7 +284,7 @@ public class StructBuilder {
 
     /**
      * Validate the communities section of the XML document.  This returns a string
-     * containing any errors encountered, or null if there were no errors
+     * containing any errors encountered, or null if there were no errors.
      *
      * @param communities the NodeList of communities to validate
      * @param level       the level in the XML document that we are at, for the purposes
@@ -246,7 +294,7 @@ public class StructBuilder {
      */
     private static String validateCommunities(NodeList communities, int level)
         throws TransformerException {
-        StringBuffer err = new StringBuffer();
+        StringBuilder err = new StringBuilder();
         boolean trip = false;
         String errs = null;
 
@@ -255,8 +303,9 @@ public class StructBuilder {
             NodeList name = XPathAPI.selectNodeList(n, "name");
             if (name.getLength() != 1) {
                 String pos = Integer.toString(i + 1);
-                err.append("-The level " + level + " community in position " + pos);
+                err.append("-The level ").append(level)
-                err.append(" does not contain exactly one name field\n");
+                        .append(" community in position ").append(pos)
+                        .append(" does not contain exactly one name field\n");
                 trip = true;
             }
 
@@ -286,7 +335,7 @@ public class StructBuilder {
 
     /**
      * validate the collection section of the XML document.  This generates a
-     * string containing any errors encountered, or returns null if no errors
+     * string containing any errors encountered, or returns null if no errors.
      *
      * @param collections a NodeList of collections to validate
      * @param level       the level in the XML document for the purposes of error reporting
@@ -294,7 +343,7 @@ public class StructBuilder {
      */
     private static String validateCollections(NodeList collections, int level)
         throws TransformerException {
-        StringBuffer err = new StringBuffer();
+        StringBuilder err = new StringBuilder();
         boolean trip = false;
         String errs = null;
 
@@ -303,8 +352,9 @@ public class StructBuilder {
             NodeList name = XPathAPI.selectNodeList(n, "name");
             if (name.getLength() != 1) {
                 String pos = Integer.toString(i + 1);
-                err.append("-The level " + level + " collection in position " + pos);
+                err.append("-The level ").append(level)
-                err.append(" does not contain exactly one name field\n");
+                        .append(" collection in position ").append(pos)
+                        .append(" does not contain exactly one name field\n");
                 trip = true;
             }
         }
@@ -363,7 +413,7 @@ public class StructBuilder {
      * created communities (e.g. the handles they have been assigned)
      */
     private static Element[] handleCommunities(Context context, NodeList communities, Community parent)
-        throws TransformerException, SQLException, Exception {
+        throws TransformerException, SQLException, AuthorizeException {
         Element[] elements = new Element[communities.getLength()];
 
         for (int i = 0; i < communities.getLength(); i++) {
@@ -390,12 +440,10 @@ public class StructBuilder {
             }
 
             // FIXME: at the moment, if the community already exists by name
-            // then this will throw a PSQLException on a duplicate key
+            // then this will throw an SQLException on a duplicate key
-            // violation
+            // violation.
-            // Ideally we'd skip this row and continue to create sub
+            // Ideally we'd skip this row and continue to create sub communities
-            // communities
+            // and so forth where they don't exist, but it's proving difficult
-            // and so forth where they don't exist, but it's proving
-            // difficult
             // to isolate the community that already exists without hitting
             // the database directly.
             communityService.update(context, community);
@@ -470,7 +518,7 @@ public class StructBuilder {
      * created collections (e.g. the handle)
      */
     private static Element[] handleCollections(Context context, NodeList collections, Community parent)
-        throws TransformerException, SQLException, AuthorizeException, IOException, Exception {
+        throws TransformerException, SQLException, AuthorizeException {
         Element[] elements = new Element[collections.getLength()];
 
         for (int i = 0; i < collections.getLength(); i++) {