Changed SAMRecord.toString() to emit the SAM format string with all fields

src/main/java/htsjdk/samtools/DuplicateSetIterator.java

@@ -148,7 +148,7 @@ public DuplicateSet next() {
 
                     if (0 < cmp) {
                         throw new SAMException("The input records were not sorted in duplicate order:\n" +
-                                representative.getSAMString() + record.getSAMString());
+                                representative.getSAMString() + "\n" + record.getSAMString());
                     } else if (cmp < 0) {
                         duplicateSet = this.duplicateSet;
                         this.duplicateSet = new DuplicateSet(this.comparator);

src/main/java/htsjdk/samtools/SAMRecord.java

@@ -1840,82 +1840,6 @@ public int getAttributesBinarySize() {
         return -1;
     }
 
-    /**
-     *
-     * @return String representation of this.
-     * @deprecated This method is not guaranteed to return a valid SAM text representation of the SAMRecord.
-     * To get standard SAM text representation, {@link SAMRecord#getSAMString}.
-     */
-    @Deprecated
-    public String format() {
-        final StringBuilder buffer = new StringBuilder();
-        addField(buffer, getReadName(), null, null);
-        addField(buffer, getFlags(), null, null);
-        addField(buffer, getReferenceName(), null, "*");
-        addField(buffer, getAlignmentStart(), 0, "*");
-        addField(buffer, getMappingQuality(), 0, "0");
-        addField(buffer, getCigarString(), null, "*");
-        addField(buffer, getMateReferenceName(), null, "*");
-        addField(buffer, getMateAlignmentStart(), 0, "*");
-        addField(buffer, getInferredInsertSize(), 0, "*");
-        addField(buffer, getReadString(), null, "*");
-        addField(buffer, getBaseQualityString(), null, "*");
-        if (mAttributes != null) {
-            SAMBinaryTagAndValue entry = getBinaryAttributes();
-            while (entry != null) {
-                addField(buffer, formatTagValue(entry.tag, entry.value));
-                entry = entry.getNext();
-            }
-        }
-        return buffer.toString();
-    }
-
-    private void addField(final StringBuilder buffer, final Object value, final Object defaultValue, final String defaultString) {
-        if (safeEquals(value, defaultValue)) {
-            addField(buffer, defaultString);
-        } else if (value == null) {
-            addField(buffer, "");
-        } else {
-            addField(buffer, value.toString());
-        }
-    }
-
-    private void addField(final StringBuilder buffer, final String field) {
-        if (buffer.length() > 0) {
-            buffer.append('\t');
-        }
-        buffer.append(field);
-    }
-
-    private static String formatTagValue(final short tag, final Object value) {
-        final String tagString = SAMTag.makeStringTag(tag);
-        if (value == null || value instanceof String) {
-            return tagString + ":Z:" + value;
-        } else if (value instanceof Integer || value instanceof Long ||
-                value instanceof Short || value instanceof Byte) {
-            return tagString + ":i:" + value;
-        } else if (value instanceof Character) {
-            return tagString + ":A:" + value;
-        } else if (value instanceof Float) {
-            return tagString + ":f:" + value;
-        } else if (value instanceof byte[]) {
-            return tagString + ":H:" + StringUtil.bytesToHexString((byte[]) value);
-        } else {
-            throw new RuntimeException("Unexpected value type for tag " + tagString +
-                    ": " + value + " of class " + value.getClass().getName());
-        }
-    }
-
-    private boolean safeEquals(final Object o1, final Object o2) {
-        if (o1 == o2) {
-            return true;
-        } else if (o1 == null || o2 == null) {
-            return false;
-        } else {
-            return o1.equals(o2);
-        }
-    }
-
     /**
      * Force all lazily-initialized data members to be initialized.  If a subclass overrides this method,
      * typically it should also call  super method.
@@ -2386,37 +2310,15 @@ public SAMRecord deepCopy() {
         return newSAM;
     }
 
-    /** Simple toString() that gives a little bit of useful info about the read. */
+    /** Returns this record formatted as it would appear in a SAM file. */
     @Override
     public String toString() {
-        final StringBuilder builder = new StringBuilder(64);
-        builder.append(getReadName());
-        if (getReadPairedFlag()) {
-            if (getFirstOfPairFlag()) {
-                builder.append(" 1/2");
-            }
-            else {
-                builder.append(" 2/2");
-            }
-        }
-
-        builder.append(' ')
-                .append(String.valueOf(getReadLength()))
-                .append('b');
-
-        if (getReadUnmappedFlag()) {
-            builder.append(" unmapped read.");
-        }
-        else {
-            builder.append(String.format(" aligned to %s:%d-%d.", getContig(), getAlignmentStart(), getAlignmentEnd()));
-        }
-
-        return builder.toString();
+        return getSAMString();
     }
 
     /**
-     Returns the record in the SAM line-based text format.  Fields are
-     separated by '\t' characters, and the String is terminated by '\n'.
+     Returns the record in the SAM line-based text format.  Fields are separated by
+     '\t' characters. The returned String is NOT terminated by a newline.
      */
     public String getSAMString() {
         return SAMTextWriter.getSAMString(this);

src/main/java/htsjdk/samtools/SAMSortOrderChecker.java

@@ -72,7 +72,7 @@ public String getSortKey(final SAMRecord rec) {
             case queryname:
                 return rec.getReadName();
             default:
-                return rec.getSAMString().trim();
+                return rec.getSAMString();
         }
     }
 }

src/main/java/htsjdk/samtools/SAMTextWriter.java

@@ -123,6 +123,16 @@ public SAMTextWriter(final OutputStream stream, final SamFlagField samFlagFieldO
      */
     @Override
     public void writeAlignment(final SAMRecord alignment) {
+        writeAlignmentNoNewline(alignment);
+        try {
+            out.write("\n");
+        } catch (final IOException e) {
+            throw new RuntimeIOException(e);
+        }
+    }
+
+    /** Writes the alignment fields without a trailing newline. Used by {@link #getSAMString}. */
+    private void writeAlignmentNoNewline(final SAMRecord alignment) {
         try {
             out.write(alignment.getReadName());
             out.write(FIELD_SEPARATOR);
@@ -164,21 +174,15 @@ public void writeAlignment(final SAMRecord alignment) {
                 out.write(encodedTag);
                 attribute = attribute.getNext();
             }
-            out.write("\n");
-
         } catch (final IOException e) {
             throw new RuntimeIOException(e);
         }
     }
 
     /* This method is called by SAMRecord.getSAMString(). */
-    private static SAMTextWriter textWriter = null;
-    private static StringWriter stringWriter = null;
-    static synchronized String getSAMString(final SAMRecord alignment) {
-        if (stringWriter == null) stringWriter = new StringWriter();
-        if (textWriter == null) textWriter = new SAMTextWriter(stringWriter);
-        stringWriter.getBuffer().setLength(0);
-        textWriter.writeAlignment(alignment);
+    static String getSAMString(final SAMRecord alignment) {
+        final StringWriter stringWriter = new StringWriter();
+        new SAMTextWriter(stringWriter).writeAlignmentNoNewline(alignment);
         return stringWriter.toString();
     }
 

src/main/java/htsjdk/samtools/SamReader.java

@@ -594,8 +594,8 @@ public SAMRecord next() {
                 if (!checker.isSorted(result)) {
                     throw new IllegalStateException(String.format(
                             "Record %s should come after %s when sorting with %s ordering.",
-                            previous.getSAMString().trim(),
-                            result.getSAMString().trim(), checker.getSortOrder()));
+                            previous.getSAMString(),
+                            result.getSAMString(), checker.getSortOrder()));
                 }
             }
             return result;

src/main/java/htsjdk/samtools/cram/CramComparison.java

@@ -131,8 +131,8 @@ private static int compareFiles(final SamReaderFactory factory, final String fil
                     diffCount++;
                     if (diffCount <= maxDiffs) {
                         emit(out, "Record %d: %s", recordCount, diff);
-                        emit(out, "  file1: %s", rec1.getSAMString().trim());
-                        emit(out, "  file2: %s", rec2.getSAMString().trim());
+                        emit(out, "  file1: %s", rec1.getSAMString());
+                        emit(out, "  file2: %s", rec2.getSAMString());
                     }
                 }
             }

src/main/java/htsjdk/samtools/sra/SRALazyRecord.java

@@ -725,14 +725,6 @@ public Object clone() throws CloneNotSupportedException {
         return newObject;
     }
 
-    @Override
-    public String format() {
-        if (!initializedAttributes.contains(LazyAttribute.RG)) {
-            getAttribute("RG");
-        }
-        return super.format();
-    }
-
     @Override
     public List<SAMValidationError> isValid(final boolean firstOnly) {
         loadFields();

src/test/java/htsjdk/samtools/BAMFileIndexTest.java

@@ -418,9 +418,6 @@ private int runQueryTest(final File bamFile, final String sequence, final int st
                 record2 = iter2.next();
                 count2++;
             }
-            // System.out.println("Iteration:");
-            // System.out.println(" Record1 = " + ((record1 == null) ? "null" : record1.format()));
-            // System.out.println(" Record2 = " + ((record2 == null) ? "null" : record2.format()));
             if (record1 == null && record2 == null) {
                 break;
             }

src/test/java/htsjdk/samtools/BAMRemoteFileTest.java

@@ -169,7 +169,6 @@ private void runLocalRemoteTest(final URL bamURL, final File bamFile, final Stri
         assertTrue(records1.size() > 0);
         assertEquals(records1.size(), records2.size());
         for (int i = 0; i < records1.size(); i++) {
-            //System.out.println(records1.get(i).format());
             assertEquals(records1.get(i).getSAMString(), records2.get(i).getSAMString());
         }
     }

src/test/java/htsjdk/samtools/SAMRecordUnitTest.java

@@ -1205,4 +1205,48 @@ public void testAlignmentBlockCacheInvalidation() {
         rec.setAlignmentStart(100);
         Assert.assertEquals(100, rec.getAlignmentBlocks().get(0).getReferenceStart());
     }
+
+    @Test
+    public void testToStringReturnsSamLine() {
+        final SAMRecord rec = createTestRecordHelper();
+        final String[] fields = rec.toString().split("\t", -1);
+        Assert.assertTrue(fields.length >= 11, "Expected at least 11 mandatory SAM fields, got " + fields.length);
+        Assert.assertEquals(fields[0], rec.getReadName());
+        Assert.assertEquals(fields[1], Integer.toString(rec.getFlags()));
+        Assert.assertEquals(fields[2], rec.getReferenceName());
+        Assert.assertEquals(fields[3], Integer.toString(rec.getAlignmentStart()));
+        Assert.assertEquals(fields[5], rec.getCigarString());
+    }
+
+    @Test
+    public void testToStringHasNoTrailingNewline() {
+        final SAMRecord rec = createTestRecordHelper();
+        final String s = rec.toString();
+        Assert.assertFalse(s.isEmpty());
+        Assert.assertNotEquals(s.charAt(s.length() - 1), '\n');
+    }
+
+    @Test
+    public void testGetSAMStringHasNoTrailingNewline() {
+        final SAMRecord rec = createTestRecordHelper();
+        final String s = rec.getSAMString();
+        Assert.assertFalse(s.isEmpty());
+        Assert.assertNotEquals(s.charAt(s.length() - 1), '\n');
+    }
+
+    @Test
+    public void testToStringEqualsGetSAMString() {
+        final SAMRecord rec = createTestRecordHelper();
+        Assert.assertEquals(rec.toString(), rec.getSAMString());
+    }
+
+    @Test
+    public void testToStringIncludesAttributes() {
+        final SAMRecord rec = createTestRecordHelper();
+        rec.setAttribute("XX", 42);
+        rec.setAttribute("YY", "hello");
+        final String s = rec.toString();
+        Assert.assertTrue(s.contains("XX:i:42"), "Expected XX:i:42 in: " + s);
+        Assert.assertTrue(s.contains("YY:Z:hello"), "Expected YY:Z:hello in: " + s);
+    }
 }

src/test/java/htsjdk/samtools/SAMTextReaderTest.java

@@ -69,9 +69,9 @@ public void testBasic() throws Exception {
 
         final String[] samResults =
                 {"read_28833_29006_6945\t99\tchr20\t28833\t20\t10M1D25M\t=\t28993\t195\t" + seq1 + "\t" + qual1 +
-                        "\tH0:i:0\tH1:i:0\tMF:i:130\tRG:Z:L1\tNm:i:1\n",
+                        "\tH0:i:0\tH1:i:0\tMF:i:130\tRG:Z:L1\tNm:i:1",
                         "read_28701_28881_323b\t147\tchr20\t28834\t30\t35M\t=\t28701\t-168\t" + seq2 + "\t" + qual2 +
-                                "\tH0:i:1\tH1:i:0\tMF:i:18\tRG:Z:L2\tNm:i:0\n"
+                                "\tH0:i:1\tH1:i:0\tMF:i:18\tRG:Z:L2\tNm:i:0"
                 };
 
         final SamReader samReader = createSamFileReader(samExample);

src/test/java/htsjdk/samtools/SAMTextWriterTest.java

@@ -131,6 +131,6 @@ public void testEmptyArrayAttributeHasNoCommaWhenWrittenToSAM(){
         final SAMFileHeader header = new SAMFileHeader();
         final SAMRecord record = new SAMRecord(header);
         record.setAttribute("xa", new int[0]);
-        Assert.assertTrue(record.getSAMString().endsWith("xa:B:i\n"));
+        Assert.assertTrue(record.getSAMString().endsWith("xa:B:i"));
     }
 }

src/test/java/htsjdk/samtools/cram/cram31/CRAM31FidelityTestBase.java

@@ -118,8 +118,8 @@ static void compareCRAMFiles(final Path path1, final Path path2, final Path refe
             for (final SAMRecord rec1 : reader1) {
                 final SAMRecord rec2 = iterator2.next();
                 if (!rec1.equals(rec2)) {
-                    System.err.printf("%s%s", rec1.getReadUnmappedFlag() ? "unmapped: " : "", rec1.getSAMString());
-                    System.err.printf("%s%s", rec2.getReadUnmappedFlag() ? "unmapped: " : "", rec2.getSAMString());
+                    System.err.printf("%s%s%n", rec1.getReadUnmappedFlag() ? "unmapped: " : "", rec1.getSAMString());
+                    System.err.printf("%s%s%n", rec2.getReadUnmappedFlag() ? "unmapped: " : "", rec2.getSAMString());
                     diffCount++;
                 }
             }