Skip to content

Commit

Permalink
Replace IndexFactory reflection. (#1431)
Browse files Browse the repository at this point in the history 
* Replacing use of reflection in IndexFactory with method references

* IndexFactory used old reflection code that was unnecessarily brittle and may cause problems in java 11.
  Replaced it by using direct references to Index constructor methods instead of finding them with reflection.
* Breaking changes: Removed public methods IndexType.getIndexCreator() and IndexType.getIndexType(). The first of these was broken and always resulted in an exception and the second is mostly useless and also unlikely to be missed.
* Removed  a public method which was broken and always resulted in an exception.
* This doesn't attempt to fix any design issues in this class other than the bad reflection code.
  • Loading branch information
@lbergelson
lbergelson authored Oct 29, 2019
1 parent 3f8ba35 commit f70befd
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 48 deletions.
85 changes: 43 additions & 42 deletions src/main/java/htsjdk/tribble/index/IndexFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,32 @@
import htsjdk.samtools.seekablestream.ISeekableStreamFactory;
import htsjdk.samtools.seekablestream.SeekableStream;
import htsjdk.samtools.seekablestream.SeekableStreamFactory;
import htsjdk.samtools.util.*;
import htsjdk.tribble.*;
import htsjdk.samtools.util.BlockCompressedInputStream;
import htsjdk.samtools.util.FileExtensions;
import htsjdk.samtools.util.IOUtil;
import htsjdk.samtools.util.LocationAware;
import htsjdk.tribble.CloseableTribbleIterator;
import htsjdk.tribble.Feature;
import htsjdk.tribble.FeatureCodec;
import htsjdk.tribble.TribbleException;
import htsjdk.tribble.index.interval.IntervalIndexCreator;
import htsjdk.tribble.index.interval.IntervalTreeIndex;
import htsjdk.tribble.index.linear.LinearIndex;
import htsjdk.tribble.index.linear.LinearIndexCreator;
import htsjdk.tribble.index.tabix.TabixFormat;
import htsjdk.tribble.index.tabix.TabixIndex;
import htsjdk.tribble.index.tabix.TabixIndexCreator;
import htsjdk.tribble.readers.*;
import htsjdk.tribble.readers.PositionalBufferedStream;
import htsjdk.tribble.util.LittleEndianInputStream;
import htsjdk.tribble.util.ParsingUtils;

import java.io.BufferedInputStream;
import java.io.EOFException;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.EOFException;
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.nio.channels.SeekableByteChannel;
import java.util.HashMap;
import java.util.Iterator;
Expand All @@ -73,47 +77,47 @@ public enum IndexBalanceApproach {
* an enum that contains all of the information about the index types, and how to create them
*/
public enum IndexType {
LINEAR(LinearIndex.MAGIC_NUMBER, LinearIndex.INDEX_TYPE, LinearIndexCreator.class, LinearIndex.class, LinearIndexCreator.DEFAULT_BIN_WIDTH),
INTERVAL_TREE(IntervalTreeIndex.MAGIC_NUMBER, IntervalTreeIndex.INDEX_TYPE, IntervalIndexCreator.class, IntervalTreeIndex.class, IntervalIndexCreator.DEFAULT_FEATURE_COUNT),
LINEAR(AbstractIndex.MAGIC_NUMBER, LinearIndex.INDEX_TYPE, true, LinearIndex::new, LinearIndexCreator.DEFAULT_BIN_WIDTH),
INTERVAL_TREE(AbstractIndex.MAGIC_NUMBER, IntervalTreeIndex.INDEX_TYPE, true, IntervalTreeIndex::new, IntervalIndexCreator.DEFAULT_FEATURE_COUNT),
// Tabix index initialization requires additional information, so generic construction won't work, thus indexCreatorClass is null.
TABIX(TabixIndex.MAGIC_NUMBER, null, null, TabixIndex.class, -1);
TABIX(TabixIndex.MAGIC_NUMBER, null, false, TabixIndex::new, -1) ;

private final int magicNumber;
private final Integer tribbleIndexType;
private final Class<IndexCreator> indexCreatorClass;
private final int defaultBinSize;
private final Class<Index> indexType;
private final boolean canCreate;
private final IndexFromStreamFunction createFromInputStream;

public int getDefaultBinSize() {
return defaultBinSize;
}

public IndexCreator getIndexCreator() {
try {
return indexCreatorClass.newInstance();
} catch ( final InstantiationException | IllegalAccessException e ) {
throw new TribbleException("Couldn't make index creator in " + this, e);
}
public boolean canCreate() {
return canCreate;
}

public boolean canCreate() {
return indexCreatorClass != null;
private interface IndexFromStreamFunction {
Index apply(InputStream t) throws IOException;
}

IndexType(final int magicNumber, final Integer tribbleIndexType, final Class creator, final Class indexClass, final int defaultBinSize) {
IndexType(final int magicNumber, final Integer tribbleIndexType, final boolean canCreate, final IndexFromStreamFunction createFromInputStream, final int defaultBinSize) {
this.magicNumber = magicNumber;
this.tribbleIndexType = tribbleIndexType;
indexCreatorClass = creator;
indexType = indexClass;
this.canCreate = canCreate;
this.createFromInputStream = createFromInputStream;
this.defaultBinSize = defaultBinSize;
}

public Integer getTribbleIndexType() {
return tribbleIndexType;
}

public Class<Index> getIndexType() {
return indexType;
public Index createIndex(final InputStream in) {
try {
return createFromInputStream.apply(in);
} catch (IOException e) {
throw new TribbleException("Failed to create index from stream.", e);
}
}

public int getMagicNumber() { return magicNumber; }
Expand Down Expand Up @@ -189,21 +193,18 @@ public static Index loadIndex(final String indexFile, Function<SeekableByteChann
public static Index loadIndex(final String source, final InputStream inputStream) {
// Must be buffered, because getIndexType uses mark and reset
try (BufferedInputStream bufferedInputStream = new BufferedInputStream(inputStream, Defaults.NON_ZERO_BUFFER_SIZE)) {
final Class<Index> indexClass = IndexType.getIndexType(bufferedInputStream).getIndexType();
final Constructor<Index> ctor = indexClass.getConstructor(InputStream.class);
return ctor.newInstance(bufferedInputStream);
} catch (final TribbleException ex) {
throw ex;
} catch (final InvocationTargetException ex) {
if (ex.getCause() instanceof EOFException) {
return createIndex(bufferedInputStream);
} catch (final EOFException ex) {
throw new TribbleException.CorruptedIndexFile("Index file is corrupted", source, ex);
}
throw new RuntimeException(ex);
} catch (final Exception ex) {
throw new RuntimeException(ex);
} catch (final IOException ex) {
throw new TribbleException.UnableToReadIndexFile("Failed to read index file", source, ex);
}
}

private static Index createIndex(BufferedInputStream bufferedInputStream) throws IOException {
return IndexType.getIndexType(bufferedInputStream).createIndex(bufferedInputStream);
}

private static InputStream indexFileInputStream(final String indexFile, Function<SeekableByteChannel, SeekableByteChannel> indexWrapper) throws IOException {
final InputStream inputStreamInitial = ParsingUtils.openInputStream(indexFile, indexWrapper);
if (indexFile.endsWith(".gz")) {
Expand All @@ -223,7 +224,7 @@ else if (indexFile.endsWith(FileExtensions.TABIX_INDEX)) {
* @param inputFile the input file to load features from
* @param codec the codec to use for decoding records
*/
public static LinearIndex createLinearIndex(final File inputFile, final FeatureCodec codec) {
public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> LinearIndex createLinearIndex(final File inputFile, final FeatureCodec<FEATURE_TYPE, SOURCE_TYPE> codec) {
return createLinearIndex(inputFile, codec, LinearIndexCreator.DEFAULT_BIN_WIDTH);
}

Expand All @@ -238,7 +239,7 @@ public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> LinearIndex createLine
final FeatureCodec<FEATURE_TYPE, SOURCE_TYPE> codec,
final int binSize) {
final LinearIndexCreator indexCreator = new LinearIndexCreator(inputFile, binSize);
return (LinearIndex)createIndex(inputFile, new FeatureIterator<FEATURE_TYPE, SOURCE_TYPE>(inputFile, codec), indexCreator);
return (LinearIndex)createIndex(inputFile, new FeatureIterator<>(inputFile, codec), indexCreator);
}

/**
Expand All @@ -264,7 +265,7 @@ public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> IntervalTreeIndex crea
final FeatureCodec<FEATURE_TYPE, SOURCE_TYPE> codec,
final int featuresPerInterval) {
final IntervalIndexCreator indexCreator = new IntervalIndexCreator(inputFile, featuresPerInterval);
return (IntervalTreeIndex)createIndex(inputFile, new FeatureIterator<FEATURE_TYPE, SOURCE_TYPE>(inputFile, codec), indexCreator);
return (IntervalTreeIndex)createIndex(inputFile, new FeatureIterator<>(inputFile, codec), indexCreator);
}

/**
Expand Down Expand Up @@ -306,8 +307,8 @@ public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> Index createIndex(fina
case INTERVAL_TREE: return createIntervalIndex(inputFile, codec);
case LINEAR: return createLinearIndex(inputFile, codec);
case TABIX: return createTabixIndex(inputFile, codec, sequenceDictionary);
default: throw new IllegalArgumentException("Unrecognized IndexType " + type);
}
throw new IllegalArgumentException("Unrecognized IndexType " + type);
}

/**
Expand All @@ -334,7 +335,7 @@ public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> Index createDynamicInd
final IndexBalanceApproach iba) {
// get a list of index creators
final DynamicIndexCreator indexCreator = new DynamicIndexCreator(inputFile, iba);
return createIndex(inputFile, new FeatureIterator<FEATURE_TYPE, SOURCE_TYPE>(inputFile, codec), indexCreator);
return createIndex(inputFile, new FeatureIterator<>(inputFile, codec), indexCreator);
}

/**
Expand All @@ -349,7 +350,7 @@ public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> TabixIndex createTabix
final TabixFormat tabixFormat,
final SAMSequenceDictionary sequenceDictionary) {
final TabixIndexCreator indexCreator = new TabixIndexCreator(sequenceDictionary, tabixFormat);
return (TabixIndex)createIndex(inputFile, new FeatureIterator<FEATURE_TYPE, SOURCE_TYPE>(inputFile, codec), indexCreator);
return (TabixIndex)createIndex(inputFile, new FeatureIterator<>(inputFile, codec), indexCreator);
}

/**
Expand All @@ -368,7 +369,7 @@ public static <FEATURE_TYPE extends Feature, SOURCE_TYPE> TabixIndex createTabix
private static Index createIndex(final File inputFile, final FeatureIterator iterator, final IndexCreator creator) {
Feature lastFeature = null;
Feature currentFeature;
final Map<String, Feature> visitedChromos = new HashMap<String, Feature>(40);
final Map<String, Feature> visitedChromos = new HashMap<>(40);
while (iterator.hasNext()) {
final long position = iterator.getPosition();
currentFeature = iterator.next();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/htsjdk/tribble/index/TribbleIndexCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public abstract class TribbleIndexCreator implements IndexCreator {
// a constant we use for marking sequence dictionary entries in the Tribble index property list
private static final String SEQUENCE_DICTIONARY_PROPERTY_PREDICATE = "DICT:";

protected LinkedHashMap<String, String> properties = new LinkedHashMap<String, String>();
protected LinkedHashMap<String, String> properties = new LinkedHashMap<>();

public void addProperty(final String key, final String value) {
properties.put(key, value);
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/htsjdk/tribble/index/linear/LinearIndex.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
public class LinearIndex extends AbstractIndex {

// NOTE: To debug uncomment the System.getProperty and recompile.
public static final double MAX_FEATURES_PER_BIN = Double.valueOf(System.getProperty("MAX_FEATURES_PER_BIN", "100"));
public static final double MAX_FEATURES_PER_BIN = Double.parseDouble(System.getProperty("MAX_FEATURES_PER_BIN", "100"));
public static final int INDEX_TYPE = IndexType.LINEAR.fileHeaderTypeIdentifier;

private final static int MAX_BIN_WIDTH = 1 * 1000 * 1000 * 1000; // widths must be less than 1 billion
Expand Down Expand Up @@ -135,8 +135,8 @@ protected int getType() {

@Override
public List<String> getSequenceNames() {
return (chrIndices == null ? Collections.EMPTY_LIST :
Collections.unmodifiableList(new ArrayList<String>(chrIndices.keySet())));
return (chrIndices == null ? Collections.emptyList() :
Collections.unmodifiableList(new ArrayList<>(chrIndices.keySet())));
}

@Override
Expand Down Expand Up @@ -229,10 +229,10 @@ public List<Block> getBlocks(final int start, final int end) {
final long endPos = blocks.get(endBinNumber).getStartPosition() + blocks.get(endBinNumber).getSize();
final long size = endPos - startPos;
if (size == 0) {
return Collections.EMPTY_LIST;
return Collections.emptyList();
} else {
final Block mergedBlock = new Block(startPos, size);
return Arrays.asList(mergedBlock);
return Collections.singletonList(mergedBlock);
}
}
}
Expand Down

0 comments on commit f70befd

Please sign in to comment.