Final cleanup and unit testing for GATKRunReport

-- Bringing code up to document, style, and code coverage specs
-- Move GATKRunReportUnitTest to private
-- Fully expand GATKRunReportUnitTests to coverage writing and reading GATKRunReport to local disk, to standard out, to AWS.
-- Move documentation URL from GATKRunReport to UserException
-- Delete a few unused files from s3GATKReport
-- Added capabilities to GATKRunReport to make testing easier
-- Added capabilities to deserialize GATKRunReports from an InputStream

public/java/src/org/broadinstitute/sting/gatk/CommandLineExecutable.java

@@ -130,7 +130,7 @@ public abstract class CommandLineExecutable extends CommandLineProgram {
              getArgumentCollection().phoneHomeType == GATKRunReport.PhoneHomeOption.STDOUT ) {
             if ( getArgumentCollection().gatkKeyFile == null ) {
                 throw new UserException("Running with the -et NO_ET or -et STDOUT option requires a GATK Key file. " +
-                                        "Please see " + GATKRunReport.PHONE_HOME_DOCS_URL +
+                                        "Please see " + UserException.PHONE_HOME_DOCS_URL +
                                         " for more information and instructions on how to obtain a key.");
             }
             else {

public/java/src/org/broadinstitute/sting/gatk/arguments/GATKArgumentCollection.java

@@ -34,6 +34,7 @@ import org.broadinstitute.sting.gatk.phonehome.GATKRunReport;
 import org.broadinstitute.sting.gatk.samples.PedigreeValidationType;
 import org.broadinstitute.sting.utils.QualityUtils;
 import org.broadinstitute.sting.utils.baq.BAQ;
+import org.broadinstitute.sting.utils.exceptions.UserException;
 
 import java.io.File;
 import java.util.ArrayList;
@@ -68,10 +69,10 @@ public class GATKArgumentCollection {
     //
     // --------------------------------------------------------------------------------------------------------------
 
-    @Argument(fullName = "phone_home", shortName = "et", doc="What kind of GATK run report should we generate? STANDARD is the default, can be NO_ET so nothing is posted to the run repository. Please see " + GATKRunReport.PHONE_HOME_DOCS_URL + " for details.", required = false)
+    @Argument(fullName = "phone_home", shortName = "et", doc="What kind of GATK run report should we generate? STANDARD is the default, can be NO_ET so nothing is posted to the run repository. Please see " + UserException.PHONE_HOME_DOCS_URL + " for details.", required = false)
     public GATKRunReport.PhoneHomeOption phoneHomeType = GATKRunReport.PhoneHomeOption.STANDARD;
 
-    @Argument(fullName = "gatk_key", shortName = "K", doc="GATK Key file. Required if running with -et NO_ET. Please see " + GATKRunReport.PHONE_HOME_DOCS_URL + " for details.", required = false)
+    @Argument(fullName = "gatk_key", shortName = "K", doc="GATK Key file. Required if running with -et NO_ET. Please see " + UserException.PHONE_HOME_DOCS_URL + " for details.", required = false)
     public File gatkKeyFile = null;
 
     /**

public/java/src/org/broadinstitute/sting/gatk/phonehome/GATKRunReport.java

@@ -25,6 +25,8 @@
 
 package org.broadinstitute.sting.gatk.phonehome;
 
+import com.google.java.contract.Ensures;
+import com.google.java.contract.Requires;
 import org.apache.log4j.Level;
 import org.apache.log4j.Logger;
 import org.broadinstitute.sting.gatk.CommandLineGATK;
@@ -33,7 +35,6 @@ import org.broadinstitute.sting.gatk.walkers.Walker;
 import org.broadinstitute.sting.utils.Utils;
 import org.broadinstitute.sting.utils.crypt.CryptUtils;
 import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
-import org.broadinstitute.sting.utils.exceptions.UserException;
 import org.broadinstitute.sting.utils.io.IOUtils;
 import org.broadinstitute.sting.utils.io.Resource;
 import org.broadinstitute.sting.utils.threading.ThreadEfficiencyMonitor;
@@ -43,99 +44,102 @@ import org.jets3t.service.impl.rest.httpclient.RestS3Service;
 import org.jets3t.service.model.S3Object;
 import org.jets3t.service.security.AWSCredentials;
 import org.simpleframework.xml.Element;
-import org.simpleframework.xml.ElementList;
 import org.simpleframework.xml.Serializer;
 import org.simpleframework.xml.core.Persister;
-import org.simpleframework.xml.stream.Format;
-import org.simpleframework.xml.stream.HyphenStyle;
 
 import java.io.*;
 import java.security.NoSuchAlgorithmException;
 import java.security.PublicKey;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
-import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 
 
 /**
- * @author depristo
- *
  * A detailed description of a GATK run, and error if applicable.  Simply create a GATKRunReport
  * with the constructor, providing the walker that was run and the fully instantiated GenomeAnalysisEngine
  * <b>after the run finishes</b> and the GATKRunReport will collect all of the report information
  * into this object.  Call postReport to write out the report, as an XML document, to either STDOUT,
  * a file (in which case the output is gzipped), or with no arguments the report will be posted to the
  * GATK run report database.
+ *
+ * @author depristo
+ * @since 2010
  */
 public class GATKRunReport {
+    protected static final String REPORT_BUCKET_NAME = "GATK_Run_Reports";
+    protected static final String TEST_REPORT_BUCKET_NAME = "GATK_Run_Reports_Test";
     protected final static String AWS_ACCESS_KEY_MD5 = "43433e5488d60788042ed5de3dcf9b0a";
     protected final static String AWS_SECRET_KEY_MD5 = "0aa28b227ecacbdc9d2d5e8d82b10d32";
 
+    private static final DateFormat DATE_FORMAT = new SimpleDateFormat("yyyy/MM/dd HH.mm.ss");
+
+    /**
+     * number of milliseconds before the S3 put operation is timed-out:
+     */
+    private static final long S3_PUT_TIME_OUT = 10 * 1000;
+
     /**
      * The root file system directory where we keep common report data
      */
-    private static File REPORT_DIR = new File("/humgen/gsa-hpprojects/GATK/reports");
+    private final static File REPORT_DIR = new File("/humgen/gsa-hpprojects/GATK/reports");
 
-    private static final String REPORT_BUCKET_NAME = "GATK_Run_Reports";
 
     /**
      * The full path to the direct where submitted (and uncharacterized) report files are written
      */
-    private static File REPORT_SUBMIT_DIR = new File(REPORT_DIR.getAbsolutePath() + "/submitted");
+    private final static File REPORT_SUBMIT_DIR = new File(REPORT_DIR.getAbsolutePath() + "/submitted");
 
     /**
      * Full path to the sentinel file that controls whether reports are written out.  If this file doesn't
      * exist, no long will be written
      */
-    private static File REPORT_SENTINEL = new File(REPORT_DIR.getAbsolutePath() + "/ENABLE");
+    private final static File REPORT_SENTINEL = new File(REPORT_DIR.getAbsolutePath() + "/ENABLE");
-
-    // number of milliseconds before the S3 put operation is timed-out:
-    private static final long S3PutTimeOut = 10 * 1000;
-
-    public static final String PHONE_HOME_DOCS_URL = "http://gatkforums.broadinstitute.org/discussion/1250/what-is-phone-home-and-how-does-it-affect-me#latest";
 
     /**
      * our log
      */
     protected static final Logger logger = Logger.getLogger(GATKRunReport.class);
 
+    // -----------------------------------------------------------------
+    // elements captured for the report
+    // -----------------------------------------------------------------
 
     @Element(required = false, name = "id")
-    private final String id;
+    private String id;
 
     @Element(required = false, name = "exception")
-    private final ExceptionToXML mException;
+    private GATKRunReportException mException;
 
-    @Element(required = true, name = "start_time")
+    @Element(required = true, name = "start-time")
     private String startTime = "ND";
 
-    @Element(required = true, name = "end_time")
+    @Element(required = true, name = "end-time")
     private String endTime;
 
-    @Element(required = true, name = "run_time")
+    @Element(required = true, name = "run-time")
     private long runTime = 0;
 
-    @Element(required = true, name = "walker_name")
+    @Element(required = true, name = "walker-name")
     private String walkerName;
 
-    @Element(required = true, name = "svn_version")
+    @Element(required = true, name = "svn-version")
     private String svnVersion;
 
-    @Element(required = true, name = "total_memory")
+    @Element(required = true, name = "total-memory")
     private long totalMemory;
 
-    @Element(required = true, name = "max_memory")
+    @Element(required = true, name = "max-memory")
     private long maxMemory;
 
-    @Element(required = true, name = "user_name")
+    @Element(required = true, name = "user-name")
     private String userName;
 
-    @Element(required = true, name = "host_name")
+    @Element(required = true, name = "host-name")
     private String hostName;
 
     @Element(required = true, name = "java")
@@ -150,31 +154,75 @@ public class GATKRunReport {
     @Element(required = true, name = "tag")
     private String tag;
 
-    // -----------------------------------------------------------------
+    @Element(required = true, name = "num-threads")
-    // elements related to multi-threading and efficiency
-    // -----------------------------------------------------------------
-
-    @Element(required = true, name = "numThreads")
     private int numThreads;
-    @Element(required = true, name = "percent_time_running")
+    @Element(required = true, name = "percent-time-running")
     private String percentTimeRunning;
-    @Element(required = true, name = "percent_time_waiting")
+    @Element(required = true, name = "percent-time-waiting")
     private String percentTimeWaiting;
-    @Element(required = true, name = "percent_time_blocking")
+    @Element(required = true, name = "percent-time-blocking")
     private String percentTimeBlocking;
-    @Element(required = true, name = "percent_time_waiting_for_io")
+    @Element(required = true, name = "percent-time-waiting-for-io")
     private String percentTimeWaitingForIO;
 
+    /**
+     * How should the GATK report its usage?
+     */
     public enum PhoneHomeOption {
         /** Disable phone home */
         NO_ET,
         /** Standard option.  Writes to local repository if it can be found, or S3 otherwise */
         STANDARD,
+        /** Forces the report to go to S3 */
+        AWS,
         /** Force output to STDOUT.  For debugging only */
         STDOUT
     }
 
-    private static final DateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH.mm.ss");
+    /**
+     * To allow us to deserial reports from XML
+     */
+    private GATKRunReport() { }
+
+    /**
+     * Read a GATKRunReport from the serialized XML representation in String reportAsXML
+     * @param stream an input stream containing a serialized XML report
+     * @return a reconstituted GATKRunReport from reportAsXML
+     * @throws Exception if parsing fails for any reason
+     */
+    @Ensures("result != null")
+    protected static GATKRunReport deserializeReport(final InputStream stream) throws Exception {
+        final Serializer serializer = new Persister();
+        return serializer.read(GATKRunReport.class, stream);
+    }
+
+    /**
+     * Create a new GATKRunReport from a report on S3
+     *
+     * Assumes that s3Object has already been written to S3, and this function merely
+     * fetches it from S3 and deserializes it.  The access keys must have permission to
+     * GetObject from S3.
+     *
+     * @param downloaderAccessKey AWS access key with permission to GetObject from bucketName
+     * @param downloaderSecretKey AWS secret key with permission to GetObject from bucketName
+     * @param bucketName the name of the bucket holding the report
+     * @param s3Object the s3Object we wrote to S3 in bucketName that we want to get back and decode
+     * @return a deserialized report derived from s3://bucketName/s3Object.getName()
+     * @throws Exception
+     */
+    @Ensures("result != null")
+    protected static GATKRunReport deserializeReport(final String downloaderAccessKey,
+                                                     final String downloaderSecretKey,
+                                                     final String bucketName,
+                                                     final S3Object s3Object) throws Exception {
+        final S3Service s3Service = initializeAWSService(downloaderAccessKey, downloaderSecretKey);
+
+        // Retrieve the whole data object we created previously
+        final S3Object objectComplete = s3Service.getObject(bucketName, s3Object.getName());
+
+        // Read the data from the object's DataInputStream using a loop, and print it out.
+        return deserializeReport(new GZIPInputStream(objectComplete.getDataInputStream()));
+    }
 
     /**
      * Create a new RunReport and population all of the fields with values from the walker and engine
@@ -196,9 +244,9 @@ public class GATKRunReport {
 
         // runtime performance metrics
         Date end = new java.util.Date();
-        endTime = dateFormat.format(end);
+        endTime = DATE_FORMAT.format(end);
         if ( engine.getStartTime() != null ) { // made it this far during initialization
-            startTime = dateFormat.format(engine.getStartTime());
+            startTime = DATE_FORMAT.format(engine.getStartTime());
             runTime = (end.getTime() - engine.getStartTime().getTime()) / 1000L; // difference in seconds
         }
 
@@ -224,7 +272,7 @@ public class GATKRunReport {
         machine = Utils.join("-", Arrays.asList(System.getProperty("os.name"), System.getProperty("os.arch")));
 
         // if there was an exception, capture it
-        this.mException = e == null ? null : new ExceptionToXML(e);
+        this.mException = e == null ? null : new GATKRunReportException(e);
 
         numThreads = engine.getTotalNumberOfThreads();
         percentTimeRunning = getThreadEfficiencyPercent(engine, ThreadEfficiencyMonitor.State.USER_CPU);
@@ -233,6 +281,11 @@ public class GATKRunReport {
         percentTimeWaitingForIO = getThreadEfficiencyPercent(engine, ThreadEfficiencyMonitor.State.WAITING_FOR_IO);
     }
 
+    /**
+     * Get the random alpha-numeric ID of this GATKRunReport
+     * @return a non-null string ID
+     */
+    @Ensures("result != null")
     public String getID() {
         return id;
     }
@@ -244,62 +297,113 @@ public class GATKRunReport {
      * @param state the state whose occupancy we wish to know
      * @return a string representation of the percent occupancy of state, or NA is not possible
      */
+    @Requires({"engine != null", "state != null"})
+    @Ensures("result != null")
     private String getThreadEfficiencyPercent(final GenomeAnalysisEngine engine, final ThreadEfficiencyMonitor.State state) {
         final ThreadEfficiencyMonitor tem = engine.getThreadEfficiencyMonitor();
         return tem == null ? "NA" : String.format("%.2f", tem.getStatePercent(state));
     }
 
+    /**
+     * Get a filename (no path) appropriate for this report
+     *
+     * @return a non-null string filename
+     */
+    @Ensures("result != null")
+    protected String getReportFileName() {
+        return getID() + ".report.xml.gz";
+    }
+
+    // ---------------------------------------------------------------------------
+    //
+    // Main public interface method for posting reports
+    //
+    // ---------------------------------------------------------------------------
+
+    /**
+     * Post this GATK report to the destination implied by the PhoneHomeOption type
+     *
+     * Guaranteed to never throw an exception (exception noted below) and to return
+     * with a reasonable (~10 seconds) time regardless of successful writing of the report.
+     *
+     * @throws IllegalArgumentException if type == null
+     * @param type the type of phoning home we want to do
+     * @return true if a report was successfully written, false otherwise
+     */
+    public boolean postReport(final PhoneHomeOption type) {
+        if ( type == null ) throw new IllegalArgumentException("type cannot be null");
 
-    public void postReport(PhoneHomeOption type) {
         logger.debug("Posting report of type " + type);
         switch (type) {
             case NO_ET: // don't do anything
-                break;
+                return false;
             case STANDARD:
-                if ( repositoryIsOnline() ) {
+            case AWS:
-                    postReportToLocalDisk(REPORT_SUBMIT_DIR);
+                if ( type == PhoneHomeOption.STANDARD && repositoryIsOnline() ) {
+                    return postReportToLocalDisk(getLocalReportFullPath()) != null;
                 } else {
-                    postReportToAWSS3();
+                    wentToAWS = true;
+                    return postReportToAWSS3() != null;
                 }
-                break;
             case STDOUT:
-                postReportToStream(System.out);
+                return postReportToStream(System.out);
-                break;
             default:
                 exceptDuringRunReport("BUG: unexpected PhoneHomeOption ");
-                break;
+                return false;
         }
     }
 
+    // ---------------------------------------------------------------------------
+    //
+    // Code for sending reports to local files
+    //
+    // ---------------------------------------------------------------------------
+
     /**
      * Write an XML representation of this report to the stream, throwing a StingException if the marshalling
      * fails for any reason.
      *
-     * @param stream
+     * @param stream an output stream to write the report to
      */
-    private void postReportToStream(OutputStream stream) {
+    @Requires("stream != null")
-        Serializer serializer = new Persister(new Format(new HyphenStyle()));
+    protected boolean postReportToStream(final OutputStream stream) {
+        final Serializer serializer = new Persister();
         try {
             serializer.write(this, stream);
-            //throw new StingException("test");
+            return true;
         } catch (Exception e) {
-            throw new ReviewedStingException("Failed to marshal the data to the file " + stream, e);
+            return false;
         }
     }
 
-    private final String getKey() {
+    /**
-        return getID() + ".report.xml.gz";
+     * Get the full path as a file where we'll write this report to local disl
+     * @return a non-null File
+     */
+    @Ensures("result != null")
+    protected File getLocalReportFullPath() {
+        return new File(REPORT_SUBMIT_DIR, getReportFileName());
     }
 
+    /**
+     * Is the local GATKRunReport repository available for writing reports?
+     *
+     * @return true if and only if the common run report repository is available and online to receive reports
+     */
+    private boolean repositoryIsOnline() {
+        return REPORT_SENTINEL.exists();
+    }
+
+
     /**
      * Main entry point to writing reports to disk.  Posts the XML report to the common GATK run report repository.
      * If this process fails for any reason, all exceptions are handled and this routine merely prints a warning.
      * That is, postReport() is guarenteed not to fail for any reason.
+     *
+     * @return the path where the file was written, or null if any failure occurred
      */
-    private File postReportToLocalDisk(File rootDir) {
+    @Requires("destination != null")
-        final String filename = getKey();
+    private File postReportToLocalDisk(final File destination) {
-        final File destination = new File(rootDir, filename);
-
         try {
             final BufferedOutputStream out = new BufferedOutputStream(
                     new GZIPOutputStream(
@@ -316,18 +420,38 @@ public class GATKRunReport {
         }
     }
 
+    // ---------------------------------------------------------------------------
+    //
+    // Code for sending reports to s3
+    //
+    // ---------------------------------------------------------------------------
+
+    /**
+     * Get the name of the S3 bucket where we should upload this report
+     *
+     * @return the string name of the s3 bucket
+     */
+    @Ensures("result != null")
+    protected String getS3ReportBucket() {
+        return s3ReportBucket;
+    }
+
     /**
      * Decrypts encrypted AWS key from encryptedKeySource
      * @param encryptedKeySource a file containing an encrypted AWS key
      * @return a decrypted AWS key as a String
      */
+    @Ensures("result != null")
     public static String decryptAWSKey(final File encryptedKeySource) throws FileNotFoundException {
+        if ( encryptedKeySource == null ) throw new IllegalArgumentException("encryptedKeySource cannot be null");
         return decryptAWSKey(new FileInputStream(encryptedKeySource));
     }
 
     /**
      * @see #decryptAWSKey(java.io.File) but with input from an inputstream
      */
+    @Requires("encryptedKeySource != null")
+    @Ensures("result != null")
     private static String decryptAWSKey(final InputStream encryptedKeySource) {
         final PublicKey key = CryptUtils.loadGATKDistributedPublicKey();
         final byte[] fromDisk = IOUtils.readStreamIntoByteArray(encryptedKeySource);
@@ -340,6 +464,8 @@ public class GATKRunReport {
      * @param name the name of the file containing the needed AWS key
      * @return a non-null GATK
      */
+    @Requires("name != null")
+    @Ensures("result != null")
     private static String getAWSKey(final String name) {
         final Resource resource = new Resource(name, GATKRunReport.class);
         return decryptAWSKey(resource.getResourceContentsAsStream());
@@ -349,7 +475,8 @@ public class GATKRunReport {
      * Get the AWS access key for the GATK user
      * @return a non-null AWS access key for the GATK user
      */
-    protected static String getAWSAccessKey() {
+    @Ensures("result != null")
+    protected static String getAWSUploadAccessKey() {
         return getAWSKey("resources/GATK_AWS_access.key");
     }
 
@@ -357,7 +484,8 @@ public class GATKRunReport {
      * Get the AWS secret key for the GATK user
      * @return a non-null AWS secret key for the GATK user
      */
-    protected static String getAWSSecretKey() {
+    @Ensures("result != null")
+    protected static String getAWSUploadSecretKey() {
         return getAWSKey("resources/GATK_AWS_secret.key");
     }
 
@@ -368,8 +496,8 @@ public class GATKRunReport {
      */
     public static void checkAWSAreValid() {
         try {
-            final String accessKeyMD5 = Utils.calcMD5(getAWSAccessKey());
+            final String accessKeyMD5 = Utils.calcMD5(getAWSUploadAccessKey());
-            final String secretKeyMD5 = Utils.calcMD5(getAWSSecretKey());
+            final String secretKeyMD5 = Utils.calcMD5(getAWSUploadSecretKey());
 
             if ( ! AWS_ACCESS_KEY_MD5.equals(accessKeyMD5) ) {
                 throw new ReviewedStingException("Invalid AWS access key found, expected MD5 " + AWS_ACCESS_KEY_MD5 + " but got " + accessKeyMD5);
@@ -383,43 +511,77 @@ public class GATKRunReport {
         }
     }
 
+    /**
+     * Get an initialized S3Service for use in communicating with AWS/s3
+     *
+     * @param awsAccessKey our AWS access key to use
+     * @param awsSecretKey our AWS secret key to use
+     * @return an initialized S3Service object that can be immediately used to interact with S3
+     * @throws S3ServiceException
+     */
+    @Requires({"awsAccessKey != null", "awsSecretKey != null"})
+    @Ensures("result != null")
+    protected static S3Service initializeAWSService(final String awsAccessKey, final String awsSecretKey) throws S3ServiceException {
+        // To communicate with S3, create a class that implements an S3Service. We will use the REST/HTTP
+        // implementation based on HttpClient, as this is the most robust implementation provided with JetS3t.
+        final AWSCredentials awsCredentials = new AWSCredentials(awsAccessKey, awsSecretKey);
+        return new RestS3Service(awsCredentials);
+    }
+
+    /**
+     * A runnable that pushes this GATKReport up to s3.
+     *
+     * Should be run in a separate thread so we can time it out if something is taking too long
+     */
     private class S3PutRunnable implements Runnable {
+        /** Was the upload operation successful? */
+        public final AtomicBoolean isSuccess;
+        /** The name of this report */
+        private final String filename;
+        /** The contents of this report */
+        private final byte[] contents;
 
-        public AtomicBoolean isSuccess;
+        /** The s3Object that we created to upload, or null if it failed */
-        private final String key;
-        private final byte[] report;
-
         public S3Object s3Object;
-        public String errorMsg;
+        /** The error message, if one occurred, or null if none did */
+        public String errorMsg = null;
+        /** The error that occurred, if one did, or null if none did */
         public Throwable errorThrow;
 
-        public S3PutRunnable(String key, byte[] report){
+        @Requires({"filename != null", "contents != null"})
-            isSuccess = new AtomicBoolean();
+        public S3PutRunnable(final String filename, final byte[] contents){
-            this.key = key;
+            this.isSuccess = new AtomicBoolean();
-            this.report = report;
+            this.filename = filename;
+            this.contents = contents;
         }
 
         public void run() {
             try {
-                // Your Amazon Web Services (AWS) login credentials are required to manage S3 accounts. These credentials
+                switch ( awsMode ) {
-                // are stored in an AWSCredentials object:
+                    case FAIL_WITH_EXCEPTION:
+                        throw new IllegalStateException("We are throwing an exception for testing purposes");
+                    case TIMEOUT:
+                        try {
+                            Thread.sleep(S3_PUT_TIME_OUT * 100);
+                        } catch ( InterruptedException e ) {
+                            // supposed to be empty
+                        }
+                        break;
+                    case NORMAL:
+                        // IAM GATK user credentials -- only right is to PutObject into GATK_Run_Report bucket
+                        final S3Service s3Service = initializeAWSService(getAWSUploadAccessKey(), getAWSUploadSecretKey());
 
-                // IAM GATK user credentials -- only right is to PutObject into GATK_Run_Report bucket
+                        // Create an S3Object based on a file, with Content-Length set automatically and
-                final String awsAccessKey = getAWSAccessKey(); // GATK AWS user
+                        // Content-Type set based on the file's extension (using the Mimetypes utility class)
-                final String awsSecretKey = getAWSSecretKey(); // GATK AWS user
+                        final S3Object fileObject = new S3Object(filename, contents);
-                final AWSCredentials awsCredentials = new AWSCredentials(awsAccessKey, awsSecretKey);
+                        //logger.info("Created S3Object" + fileObject);
-
+                        //logger.info("Uploading " + localFile + " to AWS bucket");
-                // To communicate with S3, create a class that implements an S3Service. We will use the REST/HTTP
+                        s3Object = s3Service.putObject(getS3ReportBucket(), fileObject);
-                // implementation based on HttpClient, as this is the most robust implementation provided with JetS3t.
+                        isSuccess.set(true);
-                final S3Service s3Service = new RestS3Service(awsCredentials);
+                        break;
-
+                    default:
-                // Create an S3Object based on a file, with Content-Length set automatically and
+                        throw new IllegalStateException("Unexpected AWS exception");
-                // Content-Type set based on the file's extension (using the Mimetypes utility class)
+                }
-                final S3Object fileObject = new S3Object(key, report);
-                //logger.info("Created S3Object" + fileObject);
-                //logger.info("Uploading " + localFile + " to AWS bucket");
-                s3Object = s3Service.putObject(REPORT_BUCKET_NAME, fileObject);
-                isSuccess.set(true);
             } catch ( S3ServiceException e ) {
                 setException("S3 exception occurred", e);
             } catch ( NoSuchAlgorithmException e ) {
@@ -429,17 +591,29 @@ public class GATKRunReport {
             }
         }
 
-        private void setException(String msg, Throwable e){
+        /**
+         * Set the error message and thrown exception, if one did occurred
+         *
+         * @param msg the error message
+         * @param e the exception that occurred
+         */
+        private void setException(final String msg, final Throwable e){
             errorMsg=msg;
             errorThrow=e;
         }
     }
 
-    private void postReportToAWSS3() {
+    /**
+     * Post this GATK report to the AWS s3 GATK_Run_Report log
+     *
+     * @return the s3Object pointing to our pushed report, or null if we failed to push
+     */
+    protected S3Object postReportToAWSS3() {
         // modifying example code from http://jets3t.s3.amazonaws.com/toolkit/code-samples.html
         this.hostName = Utils.resolveHostname(); // we want to fill in the host name
-        final String key = getKey();
+        final String key = getReportFileName();
         logger.debug("Generating GATK report to AWS S3 with key " + key);
+
         try {
             // create an byte output stream so we can capture the output as a byte[]
             final ByteArrayOutputStream byteStream = new ByteArrayOutputStream(8096);
@@ -449,17 +623,17 @@ public class GATKRunReport {
             final byte[] report = byteStream.toByteArray();
 
             // stop us from printing the annoying, and meaningless, mime types warning
-            Logger mimeTypeLogger = Logger.getLogger(org.jets3t.service.utils.Mimetypes.class);
+            final Logger mimeTypeLogger = Logger.getLogger(org.jets3t.service.utils.Mimetypes.class);
             mimeTypeLogger.setLevel(Level.FATAL);
 
             // Set the S3 upload on its own thread with timeout:
-            S3PutRunnable s3run = new S3PutRunnable(key,report);
+            final S3PutRunnable s3run = new S3PutRunnable(key,report);
-            Thread s3thread = new Thread(s3run);
+            final Thread s3thread = new Thread(s3run);
             s3thread.setDaemon(true);
             s3thread.setName("S3Put-Thread");
             s3thread.start();
 
-            s3thread.join(S3PutTimeOut);
+            s3thread.join(S3_PUT_TIME_OUT);
 
             if(s3thread.isAlive()){
                 s3thread.interrupt();
@@ -467,6 +641,7 @@ public class GATKRunReport {
             } else if(s3run.isSuccess.get()) {
                 logger.info("Uploaded run statistics report to AWS S3");
                 logger.debug("Uploaded to AWS: " + s3run.s3Object);
+                return s3run.s3Object;
             } else {
                 if((s3run.errorMsg != null) && (s3run.errorThrow != null)){
                     exceptDuringRunReport(s3run.errorMsg,s3run.errorThrow);
@@ -479,57 +654,138 @@ public class GATKRunReport {
         } catch ( InterruptedException e) {
             exceptDuringRunReport("Run statistics report upload interrupted", e);
         }
+
+        return null;
     }
 
+    /**
+     * Note that an exception occurred during creating or writing this report
+     * @param msg the message to print
+     * @param e the exception that occurred
+     */
     private void exceptDuringRunReport(String msg, Throwable e) {
         logger.debug("A problem occurred during GATK run reporting [*** everything is fine, but no report could be generated; please do not post this to the support forum ***].  Message is: " + msg + ".  Error message is: " + e.getMessage());
-        //e.printStackTrace();
     }
 
+    /**
+     * Note that an exception occurred during creating or writing this report
+     * @param msg the message to print
+     */
     private void exceptDuringRunReport(String msg) {
         logger.debug("A problem occurred during GATK run reporting [*** everything is fine, but no report could be generated; please do not post this to the support forum ***].  Message is " + msg);
     }
 
+    // ---------------------------------------------------------------------------
+    //
+    // Equals and hashcode -- purely for comparing reports for testing
+    //
+    // ---------------------------------------------------------------------------
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        GATKRunReport that = (GATKRunReport) o;
+
+        if (maxMemory != that.maxMemory) return false;
+        if (nIterations != that.nIterations) return false;
+        if (numThreads != that.numThreads) return false;
+        if (runTime != that.runTime) return false;
+        if (totalMemory != that.totalMemory) return false;
+        if (endTime != null ? !endTime.equals(that.endTime) : that.endTime != null) return false;
+        if (hostName != null ? !hostName.equals(that.hostName) : that.hostName != null) return false;
+        if (id != null ? !id.equals(that.id) : that.id != null) return false;
+        if (javaVersion != null ? !javaVersion.equals(that.javaVersion) : that.javaVersion != null) return false;
+        if (mException != null ? !mException.equals(that.mException) : that.mException != null) return false;
+        if (machine != null ? !machine.equals(that.machine) : that.machine != null) return false;
+        if (percentTimeBlocking != null ? !percentTimeBlocking.equals(that.percentTimeBlocking) : that.percentTimeBlocking != null)
+            return false;
+        if (percentTimeRunning != null ? !percentTimeRunning.equals(that.percentTimeRunning) : that.percentTimeRunning != null)
+            return false;
+        if (percentTimeWaiting != null ? !percentTimeWaiting.equals(that.percentTimeWaiting) : that.percentTimeWaiting != null)
+            return false;
+        if (percentTimeWaitingForIO != null ? !percentTimeWaitingForIO.equals(that.percentTimeWaitingForIO) : that.percentTimeWaitingForIO != null)
+            return false;
+        if (startTime != null ? !startTime.equals(that.startTime) : that.startTime != null) return false;
+        if (svnVersion != null ? !svnVersion.equals(that.svnVersion) : that.svnVersion != null) return false;
+        if (tag != null ? !tag.equals(that.tag) : that.tag != null) return false;
+        if (userName != null ? !userName.equals(that.userName) : that.userName != null) return false;
+        if (walkerName != null ? !walkerName.equals(that.walkerName) : that.walkerName != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = id != null ? id.hashCode() : 0;
+        result = 31 * result + (mException != null ? mException.hashCode() : 0);
+        result = 31 * result + (startTime != null ? startTime.hashCode() : 0);
+        result = 31 * result + (endTime != null ? endTime.hashCode() : 0);
+        result = 31 * result + (int) (runTime ^ (runTime >>> 32));
+        result = 31 * result + (walkerName != null ? walkerName.hashCode() : 0);
+        result = 31 * result + (svnVersion != null ? svnVersion.hashCode() : 0);
+        result = 31 * result + (int) (totalMemory ^ (totalMemory >>> 32));
+        result = 31 * result + (int) (maxMemory ^ (maxMemory >>> 32));
+        result = 31 * result + (userName != null ? userName.hashCode() : 0);
+        result = 31 * result + (hostName != null ? hostName.hashCode() : 0);
+        result = 31 * result + (javaVersion != null ? javaVersion.hashCode() : 0);
+        result = 31 * result + (machine != null ? machine.hashCode() : 0);
+        result = 31 * result + (int) (nIterations ^ (nIterations >>> 32));
+        result = 31 * result + (tag != null ? tag.hashCode() : 0);
+        result = 31 * result + numThreads;
+        result = 31 * result + (percentTimeRunning != null ? percentTimeRunning.hashCode() : 0);
+        result = 31 * result + (percentTimeWaiting != null ? percentTimeWaiting.hashCode() : 0);
+        result = 31 * result + (percentTimeBlocking != null ? percentTimeBlocking.hashCode() : 0);
+        result = 31 * result + (percentTimeWaitingForIO != null ? percentTimeWaitingForIO.hashCode() : 0);
+        return result;
+    }
+
+    // ---------------------------------------------------------------------------
+    //
+    // Code specifically for testing the GATKRunReport
+    //
+    // ---------------------------------------------------------------------------
 
     /**
-     * Returns true if and only if the common run report repository is available and online to receive reports
+     * Enum specifying how the S3 uploader should behave.  Must be normal by default.  Purely for testing purposes
-     *
-     * @return
      */
-    private boolean repositoryIsOnline() {
+    protected enum AWSMode {
-        return REPORT_SENTINEL.exists();
+        NORMAL, // write normally to AWS
+        FAIL_WITH_EXCEPTION, // artificially fail during writing
+        TIMEOUT // sleep, so we time out
+    }
+    /** Our AWS mode */
+    private AWSMode awsMode = AWSMode.NORMAL;
+    /** The bucket were we send the GATK report on AWS/s3 */
+    private String s3ReportBucket = REPORT_BUCKET_NAME;
+    /** Did we send the report to AWS? */
+    private boolean wentToAWS = false;
+
+    /**
+     * Send the report to the AWS test bucket -- for testing only
+     */
+    protected void sendAWSToTestBucket() {
+        s3ReportBucket = TEST_REPORT_BUCKET_NAME;
     }
 
     /**
-     * A helper class for formatting in XML the throwable chain starting at e.
+     * Has the report been written to AWS?
+     *
+     * Does not imply anything about the success of the send, just that it was attempted
+     *
+     * @return true if the report has been sent to AWS, false otherwise
      */
-    private class ExceptionToXML {
+    protected boolean wentToAWS() {
-        @Element(required = false, name = "message")
+        return wentToAWS;
-        String message = null;
+    }
 
-        @ElementList(required = false, name = "stacktrace")
+    /**
-        final List<String> stackTrace = new ArrayList<String>();
+     * Purely for testing purposes.  Tells the AWS uploader whether to actually upload or simulate errors
-
+     * @param mode what we want to do
-        @Element(required = false, name = "cause")
+     */
-        ExceptionToXML cause = null;
+    @Requires("mode != null")
-
+    protected void setAwsMode(final AWSMode mode) {
-        @Element(required = false, name = "is-user-exception")
+        this.awsMode = mode;
-        Boolean isUserException;
-
-        @Element(required = false, name = "exception-class")
-        Class exceptionClass;
-
-        public ExceptionToXML(Throwable e) {
-            message = e.getMessage();
-            exceptionClass = e.getClass();
-            isUserException = e instanceof UserException;
-            for (StackTraceElement element : e.getStackTrace()) {
-                stackTrace.add(element.toString());
-            }
-
-            if ( e.getCause() != null ) {
-                cause = new ExceptionToXML(e.getCause());
-            }
-        }
     }
 }

public/java/src/org/broadinstitute/sting/gatk/phonehome/GATKRunReportException.java

@@ -0,0 +1,99 @@
+/*
+* Copyright (c) 2012 The Broad Institute
+* 
+* Permission is hereby granted, free of charge, to any person
+* obtaining a copy of this software and associated documentation
+* files (the "Software"), to deal in the Software without
+* restriction, including without limitation the rights to use,
+* copy, modify, merge, publish, distribute, sublicense, and/or sell
+* copies of the Software, and to permit persons to whom the
+* Software is furnished to do so, subject to the following
+* conditions:
+* 
+* The above copyright notice and this permission notice shall be
+* included in all copies or substantial portions of the Software.
+* 
+* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
+* THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+*/
+
+package org.broadinstitute.sting.gatk.phonehome;
+
+import org.broadinstitute.sting.utils.exceptions.UserException;
+import org.simpleframework.xml.Element;
+import org.simpleframework.xml.ElementList;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * A helper class for formatting in XML the throwable chain starting at e.
+ */
+class GATKRunReportException {
+    @Element(required = false, name = "message")
+    String message = null;
+
+    @ElementList(required = false, name = "stacktrace")
+    final List<String> stackTrace = new ArrayList<String>();
+
+    @Element(required = false, name = "cause")
+    GATKRunReportException cause = null;
+
+    @Element(required = false, name = "is-user-exception")
+    Boolean isUserException;
+
+    @Element(required = false, name = "exception-class")
+    Class exceptionClass;
+
+    /**
+     * Allow us to deserialize from XML
+     */
+    public GATKRunReportException() { }
+
+    public GATKRunReportException(Throwable e) {
+        message = e.getMessage();
+        exceptionClass = e.getClass();
+        isUserException = e instanceof UserException;
+        for (StackTraceElement element : e.getStackTrace()) {
+            stackTrace.add(element.toString());
+        }
+
+        if ( e.getCause() != null ) {
+            cause = new GATKRunReportException(e.getCause());
+        }
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        GATKRunReportException that = (GATKRunReportException) o;
+
+        if (cause != null ? !cause.equals(that.cause) : that.cause != null) return false;
+        if (exceptionClass != null ? !exceptionClass.equals(that.exceptionClass) : that.exceptionClass != null)
+            return false;
+        if (isUserException != null ? !isUserException.equals(that.isUserException) : that.isUserException != null)
+            return false;
+        if (message != null ? !message.equals(that.message) : that.message != null) return false;
+        if (stackTrace != null ? !stackTrace.equals(that.stackTrace) : that.stackTrace != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = message != null ? message.hashCode() : 0;
+        result = 31 * result + (stackTrace != null ? stackTrace.hashCode() : 0);
+        result = 31 * result + (cause != null ? cause.hashCode() : 0);
+        result = 31 * result + (isUserException != null ? isUserException.hashCode() : 0);
+        result = 31 * result + (exceptionClass != null ? exceptionClass.hashCode() : 0);
+        return result;
+    }
+}

public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java

@@ -28,7 +28,6 @@ package org.broadinstitute.sting.utils.exceptions;
 import net.sf.samtools.SAMFileHeader;
 import net.sf.samtools.SAMRecord;
 import net.sf.samtools.SAMSequenceDictionary;
-import org.broadinstitute.sting.gatk.phonehome.GATKRunReport;
 import org.broadinstitute.sting.utils.GenomeLoc;
 import org.broadinstitute.sting.utils.help.DocumentedGATKFeature;
 import org.broadinstitute.sting.utils.help.HelpConstants;
@@ -50,6 +49,11 @@ import java.io.File;
         groupName = "User exceptions",
         summary = "Exceptions caused by incorrect user behavior, such as bad files, bad arguments, etc." )
 public class UserException extends ReviewedStingException {
+    /**
+     * The URL where people can get help messages.  Printed when an error occurs
+     */
+    public static final String PHONE_HOME_DOCS_URL = "http://gatkforums.broadinstitute.org/discussion/1250/what-is-phone-home-and-how-does-it-affect-me#latest";
+
     public UserException(String msg) { super(msg); }
     public UserException(String msg, Throwable e) { super(msg, e); }
     private UserException(Throwable e) { super("", e); } // cannot be called, private access
@@ -407,7 +411,7 @@ public class UserException extends ReviewedStingException {
         public UnreadableKeyException ( File f, Exception e ) {
             super(String.format("Key file %s cannot be read (possibly the key file is corrupt?). Error was: %s. " +
                                 "Please see %s for help.",
-                                f.getAbsolutePath(), getMessage(e), GATKRunReport.PHONE_HOME_DOCS_URL));
+                                f.getAbsolutePath(), getMessage(e), PHONE_HOME_DOCS_URL));
         }
 
         public UnreadableKeyException ( String message, Exception e ) {
@@ -417,7 +421,7 @@ public class UserException extends ReviewedStingException {
         public UnreadableKeyException ( String message ) {
             super(String.format("Key file cannot be read (possibly the key file is corrupt?): %s. " +
                                 "Please see %s for help.",
-                                message, GATKRunReport.PHONE_HOME_DOCS_URL));
+                                message, PHONE_HOME_DOCS_URL));
         }
     }
 
@@ -426,7 +430,7 @@ public class UserException extends ReviewedStingException {
             super(String.format("The signature in key file %s failed cryptographic verification. " +
                                 "If this key was valid in the past, it's likely been revoked. " +
                                 "Please see %s for help.",
-                                f.getAbsolutePath(), GATKRunReport.PHONE_HOME_DOCS_URL));
+                                f.getAbsolutePath(), PHONE_HOME_DOCS_URL));
         }
     }
 }

public/java/test/org/broadinstitute/sting/gatk/phonehome/GATKRunReportUnitTest.java

@@ -1,52 +0,0 @@
-/*
-* Copyright (c) 2012 The Broad Institute
-* 
-* Permission is hereby granted, free of charge, to any person
-* obtaining a copy of this software and associated documentation
-* files (the "Software"), to deal in the Software without
-* restriction, including without limitation the rights to use,
-* copy, modify, merge, publish, distribute, sublicense, and/or sell
-* copies of the Software, and to permit persons to whom the
-* Software is furnished to do so, subject to the following
-* conditions:
-* 
-* The above copyright notice and this permission notice shall be
-* included in all copies or substantial portions of the Software.
-* 
-* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
-* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
-* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
-* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
-* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
-* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
-* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
-* THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-*/
-
-package org.broadinstitute.sting.gatk.phonehome;
-
-import junit.framework.Assert;
-import org.broadinstitute.sting.BaseTest;
-import org.broadinstitute.sting.utils.Utils;
-import org.testng.annotations.Test;
-
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-
-public class GATKRunReportUnitTest extends BaseTest {
-    @Test
-    public void testAccessKey() throws Exception {
-        testAWSKey(GATKRunReport.getAWSAccessKey(), GATKRunReport.AWS_ACCESS_KEY_MD5);
-    }
-
-    @Test
-    public void testSecretKey() throws Exception {
-        testAWSKey(GATKRunReport.getAWSSecretKey(), GATKRunReport.AWS_SECRET_KEY_MD5);
-    }
-
-    private void testAWSKey(final String accessKey, final String expectedMD5) throws Exception {
-        Assert.assertNotNull(accessKey, "AccessKey should not be null");
-        final String actualmd5 = Utils.calcMD5(accessKey);
-        Assert.assertEquals(actualmd5, expectedMD5);
-    }
-}