GenotypesContext no longer have immutability in constructor

-- additional bug fixes throughout VariantContext and GenotypesContext objects

public/java/src/org/broadinstitute/sting/utils/variantcontext/GenotypesContext.java

@@ -38,7 +38,7 @@ public class GenotypesContext implements List<Genotype> {
      * static constant value for an empty GenotypesContext.  Useful since so many VariantContexts have no genotypes
      */
     public final static GenotypesContext NO_GENOTYPES =
-            new GenotypesContext(new ArrayList<Genotype>(0), new HashMap<String, Integer>(0), Collections.<String>emptyList(), true);
+            new GenotypesContext(new ArrayList<Genotype>(0), new HashMap<String, Integer>(0), Collections.<String>emptyList()).immutable();
 
     /**
      *sampleNamesInOrder a list of sample names, one for each genotype in genotypes, sorted in alphabetical order
@@ -77,24 +77,23 @@ public class GenotypesContext implements List<Genotype> {
      * Create an empty GenotypeContext
      */
     protected GenotypesContext() {
-        this(10, false);
+        this(10);
     }
 
     /**
      * Create an empty GenotypeContext, with initial capacity for n elements
      */
     @Requires("n >= 0")
-    protected GenotypesContext(final int n, final boolean immutable) {
+    protected GenotypesContext(final int n) {
-        this(new ArrayList<Genotype>(n), immutable);
+        this(new ArrayList<Genotype>(n));
     }
 
     /**
      * Create an GenotypeContext containing genotypes
      */
-    @Requires("genotypes != null")
+    @Requires({"genotypes != null", "noDups(genotypes)"})
-    protected GenotypesContext(final ArrayList<Genotype> genotypes, final boolean immutable) {
+    protected GenotypesContext(final ArrayList<Genotype> genotypes) {
         this.notToBeDirectlyAccessedGenotypes = genotypes;
-        this.immutable = immutable;
         this.sampleNameToOffset = null;
         this.cacheIsInvalid = true;
     }
@@ -109,19 +108,16 @@ public class GenotypesContext implements List<Genotype> {
      * genotype in the vector of genotypes
      * @param sampleNamesInOrder a list of sample names, one for each genotype in genotypes, sorted in alphabetical
      * order.
-     * @param immutable
      */
-    @Requires({"genotypes != null",
+    @Requires({"genotypes != null", "noDups(genotypes)",
             "sampleNameToOffset != null",
             "sampleNamesInOrder != null",
             "genotypes.size() == sampleNameToOffset.size()",
             "genotypes.size() == sampleNamesInOrder.size()"})
     protected GenotypesContext(final ArrayList<Genotype> genotypes,
                              final Map<String, Integer> sampleNameToOffset,
-                             final List<String> sampleNamesInOrder,
+                             final List<String> sampleNamesInOrder) {
-                             final boolean immutable) {
         this.notToBeDirectlyAccessedGenotypes = genotypes;
-        this.immutable = immutable;
         this.sampleNameToOffset = sampleNameToOffset;
         this.sampleNamesInOrder = sampleNamesInOrder;
         this.cacheIsInvalid = false;
@@ -149,7 +145,7 @@ public class GenotypesContext implements List<Genotype> {
     @Requires("nGenotypes >= 0")
     @Ensures({"result != null"})
     public static final GenotypesContext create(final int nGenotypes) {
-        return new GenotypesContext(nGenotypes, false);
+        return new GenotypesContext(nGenotypes);
     }
 
     /**
@@ -173,7 +169,7 @@ public class GenotypesContext implements List<Genotype> {
     public static final GenotypesContext create(final ArrayList<Genotype> genotypes,
                                                 final Map<String, Integer> sampleNameToOffset,
                                                 final List<String> sampleNamesInOrder) {
-        return new GenotypesContext(genotypes, sampleNameToOffset, sampleNamesInOrder, false);
+        return new GenotypesContext(genotypes, sampleNameToOffset, sampleNamesInOrder);
     }
 
     /**
@@ -185,7 +181,7 @@ public class GenotypesContext implements List<Genotype> {
     @Requires({"genotypes != null"})
     @Ensures({"result != null"})
     public static final GenotypesContext create(final ArrayList<Genotype> genotypes) {
-        return genotypes == null ? NO_GENOTYPES : new GenotypesContext(genotypes, false);
+        return genotypes == null ? NO_GENOTYPES : new GenotypesContext(genotypes);
     }
 
     /**
@@ -197,7 +193,7 @@ public class GenotypesContext implements List<Genotype> {
     @Requires({"genotypes != null"})
     @Ensures({"result != null"})
     public static final GenotypesContext create(final Genotype... genotypes) {
-        return new GenotypesContext(new ArrayList<Genotype>(Arrays.asList(genotypes)), false);
+        return create(new ArrayList<Genotype>(Arrays.asList(genotypes)));
     }
 
     /**
@@ -306,14 +302,16 @@ public class GenotypesContext implements List<Genotype> {
     }
 
     @Override
-    @Requires("genotype != null")
+    @Requires({"genotype != null", "get(genotype.getSampleName()) == null"})
+    @Ensures("noDups(getGenotypes())")
     public boolean add(final Genotype genotype) {
         checkImmutability();
         invalidateCaches();
         return getGenotypes().add(genotype);
     }
 
-    @Requires("genotype != null")
+    @Requires({"genotype != null", "! containsAny(Arrays.asList(genotype))"})
+    @Ensures("noDups(getGenotypes())")
     public boolean add(final Genotype ... genotype) {
         checkImmutability();
         invalidateCaches();
@@ -321,11 +319,15 @@ public class GenotypesContext implements List<Genotype> {
     }
 
     @Override
+    @Requires("! contains(genotype)")
+    @Ensures("noDups(getGenotypes())")
     public void add(final int i, final Genotype genotype) {
         throw new UnsupportedOperationException();
     }
 
     @Override
+    @Requires("! containsAny(genotypes)")
+    @Ensures("noDups(getGenotypes())")
     public boolean addAll(final Collection<? extends Genotype> genotypes) {
         checkImmutability();
         invalidateCaches();
@@ -347,6 +349,13 @@ public class GenotypesContext implements List<Genotype> {
         return getGenotypes().containsAll(objects);
     }
 
+    private boolean containsAny(final Collection<? extends Genotype> genotypes) {
+        for ( final Genotype g : genotypes ) {
+            if ( contains(g) ) return true;
+        }
+        return false;
+    }
+
     @Override
     public Genotype get(final int i) {
         return getGenotypes().get(i);
@@ -421,6 +430,7 @@ public class GenotypesContext implements List<Genotype> {
     }
 
     @Override
+    @Ensures("noDups(getGenotypes())")
     public Genotype set(final int i, final Genotype genotype) {
         checkImmutability();
         invalidateCaches();
@@ -604,6 +614,17 @@ public class GenotypesContext implements List<Genotype> {
         }
     }
 
+    protected final static boolean noDups(Collection<Genotype> genotypes) {
+        Set<String> names = new HashSet<String>(genotypes.size());
+        for ( final Genotype g : genotypes ) {
+            if ( names.contains(g.getSampleName()) )
+                return false;
+            names.add(g.getSampleName());
+        }
+
+        return true;
+     }
+
     protected final static boolean sameSamples(List<Genotype> genotypes, Collection<String> sampleNamesInOrder) {
         Set<String> names = new HashSet<String>(sampleNamesInOrder);
         if ( names.size() != sampleNamesInOrder.size() )

public/java/src/org/broadinstitute/sting/utils/variantcontext/LazyGenotypesContext.java

@@ -73,7 +73,7 @@ public class LazyGenotypesContext extends GenotypesContext {
     /**
      * Returns the data used in the full GenotypesContext constructor
      *
-     * {@link GenotypesContext#GenotypesContext(java.util.ArrayList, java.util.Map, java.util.List, boolean)}
+     * {@link GenotypesContext#GenotypesContext(java.util.ArrayList, java.util.Map, java.util.List)}
      */
     public static class LazyData {
         final ArrayList<Genotype> genotypes;
@@ -102,7 +102,7 @@ public class LazyGenotypesContext extends GenotypesContext {
      */
     @Requires({"parser != null", "unparsedGenotypeData != null", "nUnparsedGenotypes >= 0"})
     public LazyGenotypesContext(final LazyParser parser, final Object unparsedGenotypeData, final int nUnparsedGenotypes) {
-        super(EMPTY, false);
+        super(EMPTY);
         this.parser = parser;
         this.unparsedGenotypeData = unparsedGenotypeData;
         this.nUnparsedGenotypes = nUnparsedGenotypes;

public/java/test/org/broadinstitute/sting/gatk/walkers/annotator/VariantAnnotatorIntegrationTest.java

@@ -58,7 +58,7 @@ public class VariantAnnotatorIntegrationTest extends WalkerTest {
         // they don't get reordered.  It's a good test of the genotype ordering system.
         WalkerTestSpec spec = new WalkerTestSpec(
                 baseTestString() + " --variant:VCF3 " + validationDataLocation + "vcfexample3empty.vcf -I " + validationDataLocation + "NA12878.1kg.p2.chr1_10mb_11_mb.SLX.bam -L 1:10,000,000-10,050,000", 1,
-                Arrays.asList("0cc0ec59f0328792e6413b6ff3f71780"));
+                Arrays.asList("f2ddfa8105c290b1f34b7a261a02a1ac"));
         executeTest("test file doesn't have annotations, not asking for annotations, #2", spec);
     }
 

public/java/test/org/broadinstitute/sting/utils/variantcontext/GenotypesContextUnitTest.java

@@ -77,6 +77,11 @@ public class GenotypesContextUnitTest extends BaseTest {
         public GenotypesContext make(final List<Genotype> initialSamples) {
             return GenotypesContext.copy(initialSamples);
         }
+
+        @Override
+        public String toString() {
+            return "GenotypesContext";
+        }
     };
 
     private final class lazyMaker implements LazyGenotypesContext.LazyParser, ContextMaker {
@@ -91,6 +96,11 @@ public class GenotypesContextUnitTest extends BaseTest {
         public GenotypesContext make(final List<Genotype> initialSamples) {
             return new LazyGenotypesContext(this, initialSamples, initialSamples.size());
         }
+
+        @Override
+        public String toString() {
+            return "LazyGenotypesContext";
+        }
     }
 
     private Collection<ContextMaker> allMakers = Arrays.asList(baseMaker, new lazyMaker());
@@ -100,7 +110,7 @@ public class GenotypesContextUnitTest extends BaseTest {
         final List<Genotype> initialSamples;
 
         private GenotypesContextProvider(ContextMaker maker, List<Genotype> initialSamples) {
-            super(GenotypesContextProvider.class);
+            super(GenotypesContextProvider.class, String.format("%s with %d samples", maker.toString(), initialSamples.size()));
             this.maker = maker;
             this.initialSamples = initialSamples;
         }

public/java/test/org/broadinstitute/sting/utils/variantcontext/VariantContextUnitTest.java

@@ -834,7 +834,8 @@ public class VariantContextUnitTest extends BaseTest {
             for ( int j = 0; j < i; j++ ) {
                 nSamples++;
                 Genotype g = allGenotypes.get(j % allGenotypes.size());
-                gc.add(g);
+                final String name = String.format("%s_%d%d", g.getSampleName(), i, j);
+                gc.add(new Genotype(name, g.getAlleles()));
                 switch ( g.getType() ) {
                     case NO_CALL: nNoCall++; nNoCallAlleles++; break;
                     case HOM_REF: nA += 2; nHomRef++; break;