Merge pull request DSpace#8991 from mwoodiupui/mediafilter-logging

On media filter failure log the name of the assetstore file and trace causes of exception

dspace-api/src/main/java/org/dspace/app/mediafilter/MediaFilterServiceImpl.java

@@ -10,6 +10,7 @@
 import java.io.InputStream;
 import java.sql.SQLException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -40,6 +41,7 @@
 import org.dspace.eperson.service.GroupService;
 import org.dspace.scripts.handler.DSpaceRunnableHandler;
 import org.dspace.services.ConfigurationService;
+import org.dspace.util.ThrowableUtils;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 
@@ -225,23 +227,9 @@ public boolean filterBitstream(Context context, Item myItem,
                         filtered = true;
                     }
                 } catch (Exception e) {
-                    String handle = myItem.getHandle();
-                    List<Bundle> bundles = myBitstream.getBundles();
-                    long size = myBitstream.getSizeBytes();
-                    String checksum = myBitstream.getChecksum() + " (" + myBitstream.getChecksumAlgorithm() + ")";
-                    int assetstore = myBitstream.getStoreNumber();
-
                     // Printout helpful information to find the errored bitstream.
-                    StringBuilder sb = new StringBuilder("ERROR filtering, skipping bitstream:\n");
-                    sb.append("\tItem Handle: ").append(handle);
-                    for (Bundle bundle : bundles) {
-                        sb.append("\tBundle Name: ").append(bundle.getName());
-                    }
-                    sb.append("\tFile Size: ").append(size);
-                    sb.append("\tChecksum: ").append(checksum);
-                    sb.append("\tAsset Store: ").append(assetstore);
-                    logError(sb.toString());
-                    logError(e.getMessage(), e);
+                    logError(formatBitstreamDetails(myItem.getHandle(), myBitstream));
+                    logError(ThrowableUtils.formatCauseChain(e));
                 }
             } else if (filterClass instanceof SelfRegisterInputFormats) {
                 // Filter implements self registration, so check to see if it should be applied
@@ -319,10 +307,10 @@ public boolean processBitstream(Context context, Item item, Bitstream source, Fo
 
         // check if destination bitstream exists
         Bundle existingBundle = null;
-        List<Bitstream> existingBitstreams = new ArrayList<Bitstream>();
+        List<Bitstream> existingBitstreams = new ArrayList<>();
         List<Bundle> bundles = itemService.getBundles(item, formatFilter.getBundleName());
 
-        if (bundles.size() > 0) {
+        if (!bundles.isEmpty()) {
             // only finds the last matching bundle and all matching bitstreams in the proper bundle(s)
             for (Bundle bundle : bundles) {
                 List<Bitstream> bitstreams = bundle.getBitstreams();
@@ -337,7 +325,7 @@ public boolean processBitstream(Context context, Item item, Bitstream source, Fo
         }
 
         // if exists and overwrite = false, exit
-        if (!overWrite && (existingBitstreams.size() > 0)) {
+        if (!overWrite && (!existingBitstreams.isEmpty())) {
             if (!isQuiet) {
                 logInfo("SKIPPED: bitstream " + source.getID()
                                        + " (item: " + item.getHandle() + ") because '" + newName + "' already exists");
@@ -370,7 +358,7 @@ public boolean processBitstream(Context context, Item item, Bitstream source, Fo
             }
 
             Bundle targetBundle; // bundle we're modifying
-            if (bundles.size() < 1) {
+            if (bundles.isEmpty()) {
                 // create new bundle if needed
                 targetBundle = bundleService.create(context, item, formatFilter.getBundleName());
             } else {
@@ -399,6 +387,7 @@ public boolean processBitstream(Context context, Item item, Bitstream source, Fo
 
         } catch (OutOfMemoryError oome) {
             logError("!!! OutOfMemoryError !!!");
+            logError(formatBitstreamDetails(item.getHandle(), source));
         }
 
         // we are overwriting, so remove old bitstream
@@ -496,6 +485,37 @@ public boolean inSkipList(String identifier) {
         }
     }
 
+    /**
+     * Describe a Bitstream in detail.  Format a single line of text with
+     * information such as Bitstore index, backing file ID, size, checksum,
+     * enclosing Item and Bundles.
+     *
+     * @param itemHandle Handle of the Item by which we found the Bitstream.
+     * @param bitstream the Bitstream to be described.
+     * @return Bitstream details.
+     */
+    private String formatBitstreamDetails(String itemHandle,
+            Bitstream bitstream) {
+        List<Bundle> bundles;
+        try {
+            bundles = bitstream.getBundles();
+        } catch (SQLException ex) {
+            logError("Unexpected error fetching Bundles", ex);
+            bundles = Collections.EMPTY_LIST;
+        }
+        StringBuilder sb = new StringBuilder("ERROR filtering, skipping bitstream:\n");
+        sb.append("\tItem Handle: ").append(itemHandle);
+        for (Bundle bundle : bundles) {
+            sb.append("\tBundle Name: ").append(bundle.getName());
+        }
+        sb.append("\tFile Size: ").append(bitstream.getSizeBytes());
+        sb.append("\tChecksum: ").append(bitstream.getChecksum())
+                .append(" (").append(bitstream.getChecksumAlgorithm()).append(')');
+        sb.append("\tAsset Store: ").append(bitstream.getStoreNumber());
+        sb.append("\tInternal ID: ").append(bitstream.getInternalId());
+        return sb.toString();
+    }
+
     private void logInfo(String message) {
         if (handler != null) {
             handler.logInfo(message);

dspace-api/src/main/java/org/dspace/util/ThrowableUtils.java

@@ -0,0 +1,43 @@
+/**
+ * The contents of this file are subject to the license and copyright
+ * detailed in the LICENSE and NOTICE files at the root of the source
+ * tree and available online at
+ *
+ * http://www.dspace.org/license/
+ */
+package org.dspace.util;
+
+/**
+ * Things you wish {@link Throwable} or some logging package would do for you.
+ *
+ * @author mwood
+ */
+public class ThrowableUtils {
+    /**
+     * Utility class:  do not instantiate.
+     */
+    private ThrowableUtils() { }
+
+    /**
+     * Trace a chain of {@code Throwable}s showing only causes.
+     * Less voluminous than a stack trace.  Useful if you just want to know
+     * what caused third-party code to return an uninformative exception
+     * message.
+     *
+     * @param throwable the exception or whatever.
+     * @return list of messages from each {@code Throwable} in the chain,
+     *          separated by '\n'.
+     */
+    static public String formatCauseChain(Throwable throwable) {
+        StringBuilder trace = new StringBuilder();
+        trace.append(throwable.getMessage());
+        Throwable cause = throwable.getCause();
+        while (null != cause) {
+            trace.append("\nCaused by:  ")
+                    .append(cause.getClass().getCanonicalName()).append(' ')
+                    .append(cause.getMessage());
+            cause = cause.getCause();
+        }
+        return trace.toString();
+    }
+}