SAMReaderID: fix bug with hash code and equals() method

-Two SAMReaderIDs that pointed at the same underlying bam file through
 a relative vs. an absolute path were not being treated as equal, and
 had different hash codes. This was causing problems in the engine, since
 SAMReaderIDs are often used as the keys of HashMaps.

-Fix: explicitly use the absolute path to the encapsulated bam file in
 hashCode() and equals()

-Added tests to ensure this doesn't break again
This commit is contained in:
David Roazen 2013-07-15 13:28:07 -04:00
parent 51b95589e5
commit c15751e41e
4 changed files with 69 additions and 6 deletions

View File

@ -530,8 +530,8 @@ public class GenomeAnalysisEngine {
}
if ( duplicateSamFiles.size() > 0 ) {
throw new ArgumentException("The following BAM files appear multiple times in the list of input files: " +
duplicateSamFiles + " BAM files may be specified at most once.");
throw new UserException("The following BAM files appear multiple times in the list of input files: " +
duplicateSamFiles + " BAM files may be specified at most once.");
}
}

View File

@ -98,7 +98,7 @@ public class SAMReaderID implements Comparable {
if(!(other instanceof SAMReaderID)) return false;
SAMReaderID otherID = (SAMReaderID)other;
return this.samFile.equals(otherID.samFile);
return this.getSamFilePath().equals(otherID.getSamFilePath());
}
/**
@ -107,7 +107,7 @@ public class SAMReaderID implements Comparable {
*/
@Override
public int hashCode() {
return samFile.hashCode();
return samFile.getAbsolutePath().hashCode();
}
/**

View File

@ -52,7 +52,7 @@ import java.util.List;
*/
public class GenomeAnalysisEngineUnitTest extends BaseTest {
@Test(expectedExceptions=ArgumentException.class)
@Test(expectedExceptions=UserException.class)
public void testDuplicateSamFileHandlingSingleDuplicate() throws Exception {
GenomeAnalysisEngine testEngine = new GenomeAnalysisEngine();
@ -64,7 +64,7 @@ public class GenomeAnalysisEngineUnitTest extends BaseTest {
testEngine.checkForDuplicateSamFiles();
}
@Test(expectedExceptions=ArgumentException.class)
@Test(expectedExceptions=UserException.class)
public void testDuplicateSamFileHandlingMultipleDuplicates() throws Exception {
GenomeAnalysisEngine testEngine = new GenomeAnalysisEngine();
@ -78,6 +78,20 @@ public class GenomeAnalysisEngineUnitTest extends BaseTest {
testEngine.checkForDuplicateSamFiles();
}
@Test(expectedExceptions=UserException.class)
public void testDuplicateSamFileHandlingAbsoluteVsRelativePath() {
GenomeAnalysisEngine testEngine = new GenomeAnalysisEngine();
final File relativePathToBAMFile = new File("public/testdata/exampleBAM.bam");
final File absolutePathToBAMFile = new File(relativePathToBAMFile.getAbsolutePath());
Collection<SAMReaderID> samFiles = new ArrayList<SAMReaderID>();
samFiles.add(new SAMReaderID(relativePathToBAMFile, new Tags()));
samFiles.add(new SAMReaderID(absolutePathToBAMFile, new Tags()));
testEngine.setSAMFileIDs(samFiles);
testEngine.checkForDuplicateSamFiles();
}
@Test
public void testEmptyIntervalSetHandling() throws Exception {
GenomeLocParser genomeLocParser = new GenomeLocParser(ArtificialSAMUtils.createArtificialSamHeader(1, 1, 1000).getSequenceDictionary());

View File

@ -0,0 +1,49 @@
/*
* 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.datasources.reads;
import org.broadinstitute.sting.BaseTest;
import org.broadinstitute.sting.commandline.Tags;
import org.testng.Assert;
import org.testng.annotations.Test;
import java.io.File;
public class SAMReaderIDUnitTest extends BaseTest {
@Test
public void testSAMReaderIDHashingAndEquality() {
// Test to make sure that two SAMReaderIDs that point at the same file via an absolute vs. relative
// path are equal according to equals() and have the same hash code
final File relativePathToBAMFile = new File("public/testdata/exampleBAM.bam");
final File absolutePathToBAMFile = new File(relativePathToBAMFile.getAbsolutePath());
final SAMReaderID relativePathSAMReaderID = new SAMReaderID(relativePathToBAMFile, new Tags());
final SAMReaderID absolutePathSAMReaderID = new SAMReaderID(absolutePathToBAMFile, new Tags());
Assert.assertEquals(relativePathSAMReaderID, absolutePathSAMReaderID, "Absolute-path and relative-path SAMReaderIDs not equal according to equals()");
Assert.assertEquals(relativePathSAMReaderID.hashCode(), absolutePathSAMReaderID.hashCode(), "Absolute-path and relative-path SAMReaderIDs have different hash codes");
}
}