/htsjdk

Fix JexlMap and GenotypeJEXLContext NullPtrException #668

Merged
merged 6 commits into from Sep 13, 2016
Commits
Jump to file or symbol
Failed to load files and symbols.
+305 −240
Split
View
25 src/main/java/htsjdk/variant/variantcontext/GenotypeJEXLContext.java
@@ -26,16 +26,16 @@
attributes.put("g", (Genotype g) -> g);
attributes.put(VCFConstants.GENOTYPE_KEY, Genotype::getGenotypeString);
- attributes.put("isHom", (Genotype g) -> g.isHom() ? "1" : "0");
- attributes.put("isHomRef", (Genotype g) -> g.isHomRef() ? "1" : "0");
- attributes.put("isHet", (Genotype g) -> g.isHet() ? "1" : "0");
- attributes.put("isHomVar", (Genotype g) -> g.isHomVar() ? "1" : "0");
- attributes.put("isCalled", (Genotype g) -> g.isCalled() ? "1" : "0");
- attributes.put("isNoCall", (Genotype g) -> g.isNoCall() ? "1" : "0");
- attributes.put("isMixed", (Genotype g) -> g.isMixed() ? "1" : "0");
- attributes.put("isAvailable", (Genotype g) -> g.isAvailable() ? "1" : "0");
- attributes.put("isPassFT", (Genotype g) -> g.isFiltered() ? "0" : "1");
- attributes.put(VCFConstants.GENOTYPE_FILTER_KEY, (Genotype g) -> g.isFiltered()? g.getFilters() : "PASS");
+ attributes.put("isHom", (Genotype g) -> g.isHom() ? true_string : false_string);
+ attributes.put("isHomRef", (Genotype g) -> g.isHomRef() ? true_string : false_string);
+ attributes.put("isHet", (Genotype g) -> g.isHet() ? true_string : false_string);
+ attributes.put("isHomVar", (Genotype g) -> g.isHomVar() ? true_string : false_string);
+ attributes.put("isCalled", (Genotype g) -> g.isCalled() ? true_string : false_string);
+ attributes.put("isNoCall", (Genotype g) -> g.isNoCall() ? true_string : false_string);
+ attributes.put("isMixed", (Genotype g) -> g.isMixed() ? true_string : false_string);
+ attributes.put("isAvailable", (Genotype g) -> g.isAvailable() ? true_string : false_string);
+ attributes.put("isPassFT", (Genotype g) -> g.isFiltered() ? false_string : true_string);
+ attributes.put(VCFConstants.GENOTYPE_FILTER_KEY, (Genotype g) -> g.isFiltered()? g.getFilters() : VCFConstants.PASSES_FILTERS_v4);
attributes.put(VCFConstants.GENOTYPE_QUALITY_KEY, Genotype::getGQ);
}
@@ -44,14 +44,15 @@ public GenotypeJEXLContext(VariantContext vc, Genotype g) {
this.g = g;
}
+ @Override
public Object get(String name) {
//should matching genotype attributes always supersede vc?
if ( attributes.containsKey(name) ) { // dynamic resolution of name -> value via map
return attributes.get(name).get(g);
} else if ( g.hasAnyAttribute(name) ) {
return g.getAnyAttribute(name);
- } else if ( g.getFilters().contains(name) ) {
- return "1";
+ } else if ( g.getFilters() != null && g.getFilters().contains(name) ) {
+ return true_string;
} else
return super.get(name);
}
View
174 src/main/java/htsjdk/variant/variantcontext/JEXLMap.java
@@ -2,6 +2,7 @@
import htsjdk.variant.variantcontext.VariantContextUtils.JexlVCMatchExp;
import org.apache.commons.jexl2.JexlContext;
+import org.apache.commons.jexl2.JexlException;
import org.apache.commons.jexl2.MapContext;
import java.util.Collection;
@@ -11,12 +12,8 @@
import java.util.Set;
/**
- * this is an implementation of a Map of JexlVCMatchExp to true or false values. It lazy initializes each value
- * as requested to save as much processing time as possible.
- *
- * Compatible with JEXL 1.1 (this code will be easier if we move to 2.0, all of the functionality can go into the
- * JexlContext's get()
- *
+ * This is an implementation of a Map of {@link JexlVCMatchExp} to true or false values.
+ * It lazily initializes each value as requested to save as much processing time as possible.
*/
class JEXLMap implements Map<JexlVCMatchExp, Boolean> {
@@ -27,137 +24,148 @@
// our context
private JexlContext jContext = null;
- // our mapping from JEXLVCMatchExp to Booleans, which will be set to NULL for previously uncached JexlVCMatchExp
+ /**
+ * our mapping from {@link JexlVCMatchExp} to {@link Boolean}s, which will be set to {@code NULL}
+ * for previously un-cached {@link JexlVCMatchExp}.
+ */
private Map<JexlVCMatchExp,Boolean> jexl;
-
- public JEXLMap(Collection<JexlVCMatchExp> jexlCollection, VariantContext vc, Genotype g) {
+ public JEXLMap(final Collection<JexlVCMatchExp> jexlCollection, final VariantContext vc, final Genotype g) {
+ initialize(jexlCollection);
this.vc = vc;
this.g = g;
- initialize(jexlCollection);
}
- public JEXLMap(Collection<JexlVCMatchExp> jexlCollection, VariantContext vc) {
+ public JEXLMap(final Collection<JexlVCMatchExp> jexlCollection, final VariantContext vc) {
this(jexlCollection, vc, null);
}
- private void initialize(Collection<JexlVCMatchExp> jexlCollection) {
- jexl = new HashMap<JexlVCMatchExp,Boolean>();
- for (JexlVCMatchExp exp: jexlCollection) {
- jexl.put(exp, null);
- }
- }
-
/**
- * create the internal JexlContext, only when required. This code is where new JEXL context variables
- * should get added.
+ * Note: due to laziness, this accessor actually modifies the instance by possibly forcing evaluation of an Jexl expression.
*
+ * @throws IllegalArgumentException when {@code o} is {@code null} or
+ * when any of the JexlVCMatchExp (i.e. keys) contains invalid Jexl expressions.
*/
- private void createContext() {
- if ( vc == null ) {
- jContext = new MapContext(Collections.emptyMap());
- }
- else if (g == null) {
- jContext = new VariantJEXLContext(vc);
+ public Boolean get(Object o) {
+ if (o == null) {
+ throw new IllegalArgumentException("Query key is null");
}
- else {
- jContext = new GenotypeJEXLContext(vc, g);
+
+ // if we've already determined the value, return it
+ if (jexl.containsKey(o) && jexl.get(o) != null) {
+ return jexl.get(o);
}
- }
- /**
- * @return the size of the internal data structure
- */
- public int size() {
- return jexl.size();
+ // otherwise cast the expression and try again
+ final JexlVCMatchExp e = (JexlVCMatchExp) o;
+ evaluateExpression(e);
+ return jexl.get(e);
}
/**
- * @return true if we're empty
- */
- public boolean isEmpty() { return this.jexl.isEmpty(); }
-
- /**
* do we contain the specified key
* @param o the key
* @return true if we have a value for that key
*/
public boolean containsKey(Object o) { return jexl.containsKey(o); }
- public Boolean get(Object o) {
- // if we've already determined the value, return it
- if (jexl.containsKey(o) && jexl.get(o) != null) return jexl.get(o);
-
- // try and cast the expression
- JexlVCMatchExp e = (JexlVCMatchExp) o;
- evaluateExpression(e);
- return jexl.get(e);
- }
-
- /**
- * get the keyset of map
- * @return a set of keys of type JexlVCMatchExp
- */
public Set<JexlVCMatchExp> keySet() {
return jexl.keySet();
}
/**
- * get all the values of the map. This is an expensive call, since it evaluates all keys that haven't
- * been evaluated yet. This is fine if you truely want all the keys, but if you only want a portion, or know
+ * Get all the values of the map, i.e. the {@link Boolean} values.
+ * This is an expensive call, since it evaluates all keys that haven't been evaluated yet.
+ * This is fine if you truly want all the keys, but if you only want a portion, or know
* the keys you want, you would be better off using get() to get them by name.
+ *
+ * Note: due to laziness, this accessor actually modifies the instance by possibly forcing evaluation of an Jexl expression.
+ *
* @return a collection of boolean values, representing the results of all the variants evaluated
+ *
+ * @throws IllegalArgumentException when any of the JexlVCMatchExp (i.e. keys) contains invalid Jexl expressions.
*/
public Collection<Boolean> values() {
- // this is an expensive call
- for (JexlVCMatchExp exp : jexl.keySet())
- if (jexl.get(exp) == null)
+ for (final JexlVCMatchExp exp : jexl.keySet()) {
+ if (jexl.get(exp) == null) {
evaluateExpression(exp);
+ }
+ }
return jexl.values();
}
/**
- * evaulate a JexlVCMatchExp's expression, given the current context (and setup the context if it's null)
- * @param exp the JexlVCMatchExp to evaluate
+ * @return the number of keys, i.e. {@link JexlVCMatchExp}'s held by this mapping.
+ */
+ public int size() {
+ return jexl.size();
+ }
+
+ public boolean isEmpty() { return this.jexl.isEmpty(); }
+
+ public Boolean put(JexlVCMatchExp jexlVCMatchExp, Boolean aBoolean) {
+ return jexl.put(jexlVCMatchExp, aBoolean);
+ }
+
+ public void putAll(Map<? extends JexlVCMatchExp, ? extends Boolean> map) {
+ jexl.putAll(map);
+ }
+
+ /**
+ * Initializes all keys with null values indicating that they have not yet been evaluated.
+ * The actual value will be computed only when the key is requested via {@link #get(Object)} or {@link #values()}.
*/
- private void evaluateExpression(JexlVCMatchExp exp) {
+ private void initialize(Collection<JexlVCMatchExp> jexlCollection) {
+ jexl = new HashMap<>();
+ for (final JexlVCMatchExp exp: jexlCollection) {
+ jexl.put(exp, null);
+ }
+ }
+
+ /**
+ * Evaluates a {@link JexlVCMatchExp}'s expression, given the current context (and setup the context if it's {@code null}).
+ *
+ * @param exp the {@link JexlVCMatchExp} to evaluate
+ *
+ * @throws IllegalArgumentException when {@code exp} is {@code null}, or
+ * when the Jexl expression in {@code exp} fails to evaluate the JexlContext
+ * constructed with the input VC or genotype.
+ */
+ private void evaluateExpression(final JexlVCMatchExp exp) {
// if the context is null, we need to create it to evaluate the JEXL expression
- if (this.jContext == null) createContext();
+ if (this.jContext == null) {
+ createContext();
+ }
+
try {
final Boolean value = (Boolean) exp.exp.evaluate(jContext);
// treat errors as no match
jexl.put(exp, value == null ? false : value);
- } catch (Exception e) {
+ } catch (final JexlException.Variable e) {
// if exception happens because variable is undefined (i.e. field in expression is not present), evaluate to FALSE
- // todo - might be safer if we explicitly checked for an exception type, but Apache's API doesn't seem to have that ability
- if (e.getMessage() != null && e.getMessage().contains("undefined variable"))
- jexl.put(exp,false);
- else
- throw new IllegalArgumentException(String.format("Invalid JEXL expression detected for %s with message %s", exp.name, (e.getMessage() == null ? "no message" : e.getMessage())));
+ jexl.put(exp,false);
+ } catch (final JexlException e) {
+ // todo - might be better if no exception is caught here but let's user decide how to deal with them; note this will propagate to get() and values()
+ throw new IllegalArgumentException(String.format("Invalid JEXL expression detected for %s", exp.name), e);
}
}
/**
- * helper function: adds the list of attributes to the information map we're building
- * @param infoMap the map
- * @param attributes the attributes
+ * Create the internal JexlContext, only when required.
+ * This code is where new JEXL context variables should get added.
*/
- private static void addAttributesToMap(Map<String, Object> infoMap, Map<String, ?> attributes ) {
- for (Entry<String, ?> e : attributes.entrySet()) {
- infoMap.put(e.getKey(), String.valueOf(e.getValue()));
+ private void createContext() {
+ if (vc == null) {
+ jContext = new MapContext(Collections.emptyMap());
+ } else if (g == null) {
+ jContext = new VariantJEXLContext(vc);
+ } else {
+ jContext = new GenotypeJEXLContext(vc, g);
}
}
- public Boolean put(JexlVCMatchExp jexlVCMatchExp, Boolean aBoolean) {
- return jexl.put(jexlVCMatchExp,aBoolean);
- }
-
- public void putAll(Map<? extends JexlVCMatchExp, ? extends Boolean> map) {
- jexl.putAll(map);
- }
-
// //////////////////////////////////////////////////////////////////////////////////////
- // The Following are unsupported at the moment
+ // The Following are unsupported at the moment (date: 2016/08/18)
// //////////////////////////////////////////////////////////////////////////////////////
// this doesn't make much sense to implement, boolean doesn't offer too much variety to deal
Oops, something went wrong.