AS_RMSMappingQuality parseRawData and ReducibleAnnotationData.getAlleles

[cmnbroad]

The latest code in htsjdk, which includes samtools/htsjdk#1454 (changes the Allele class into an interface, and uses SimpleAllele as the concrete implementation) causes the VariantAnnotatorEngineUnitTest.testCombineAnnotations test to fail because the order of the list returned by ReducibleAnnotationData.getAlleles is different with that change than it is without it (presumably due to the different hashCode/equals implementations).

AS_RMSMappingQuality.parseRawData seems to assume that the order of the Alleles in the list returned by
ReducibleAnnotationData.getAlleles exactly matches the order of the raw data in the String returned by ReducibleAnnotationData.getRawData, since it uses indexed access to the list, but I don't see anything that states or ensures/enforces this. Changing the Map maintained by ReducibleAnnotationData into a LinkedHashMap fixes the issue for this test, but that just changes the order to be input order - the real issue is that the contract around how the order of the list and the order of the raw data is maintained isn't clear.

This will need to be addressed before we can upgrade to the next release of htsjdk.

[cmnbroad]

@jamesemery @ldgauthier Any thoughts on this ?

[ldgauthier]

Since Valentin's PR recently got merged, I'll try to take a look at this soon-ish. I agree with what Louis said in the PR review: LinkedHashMap all the things. Beyond that, what do you recommend @cmnbroad ? Javadoc? I feel like there used to be a Google library we used to annotate methods with contracts. It's not immediately obvious to me how to enforce this, but I'm pretty rusty on the details.

[cmnbroad]

@ldgauthier I don't see any other way to enforce this - I think javadoc stating that getAlleles and getRawData will/must always be kept in sync is probably fine. I probably should have just added that myself in #7585. I wasn't sure what was going on at the time but looking back at it now it seems obvious that that was the intention. I'm not going to merge that today since at the moment I'm not sure if there is another release pending, but will do so soon.