Skip to content

Commit

Permalink
storage: Fail if annotator changes with partial annotation update. #830
Browse files Browse the repository at this point in the history
  • Loading branch information
@j-coll
j-coll committed May 8, 2018
1 parent f600b96 commit 68fd27e
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 54 deletions.
23 changes: 19 additions & 4 deletions ...a/org/opencb/opencga/storage/core/variant/annotation/DefaultVariantAnnotationManager.java
View file
Expand Up @@ -107,11 +107,15 @@ public void annotate(Query query, ObjectMap params) throws VariantAnnotatorExcep
doCreate = true;
doLoad = true;
}
boolean overwrite = params.getBoolean(OVERWRITE_ANNOTATIONS, false);
if (!overwrite) {
query.put(VariantQueryParam.ANNOTATION_EXISTS.key(), false);
}

URI annotationFile;
if (doCreate) {
dbAdaptor.getStudyConfigurationManager().lockAndUpdateProject(projectMetadata -> {
checkCurrentAnnotation(variantAnnotator, projectMetadata);
checkCurrentAnnotation(variantAnnotator, projectMetadata, overwrite);
return projectMetadata;
});

Expand All @@ -136,6 +140,13 @@ public void annotate(Query query, ObjectMap params) throws VariantAnnotatorExcep
logger.info("Starting annotation load");
loadAnnotation(annotationFile, params);
logger.info("Finished annotation load {}ms", System.currentTimeMillis() - start);

if (doCreate) {
dbAdaptor.getStudyConfigurationManager().lockAndUpdateProject(projectMetadata -> {
updateCurrentAnnotation(variantAnnotator, projectMetadata, overwrite);
return projectMetadata;
});
}
}
}

Expand Down Expand Up @@ -234,13 +245,17 @@ protected QueryOptions getIteratorQueryOptions(Query query, ObjectMap params) {
public void loadAnnotation(URI uri, ObjectMap params) throws IOException, StorageEngineException {
Path path = Paths.get(uri);
String fileName = path.getFileName().toString().toLowerCase();
if (VariantReaderUtils.isAvro(fileName) || VariantReaderUtils.isJson(fileName)) {
loadVariantAnnotation(uri, params);
} else {
if (isCustomAnnotation(fileName)) {
loadCustomAnnotation(uri, params);
} else {
loadVariantAnnotation(uri, params);
}
}

protected boolean isCustomAnnotation(String fileName) {
return !VariantReaderUtils.isAvro(fileName) && !VariantReaderUtils.isJson(fileName);
}

/**
* Loads variant annotations from an specified file into the selected Variant DataBase.
*
Expand Down
114 changes: 79 additions & 35 deletions ...ain/java/org/opencb/opencga/storage/core/variant/annotation/VariantAnnotationManager.java
View file
Expand Up @@ -21,6 +21,8 @@
import org.opencb.commons.datastore.core.Query;
import org.opencb.opencga.storage.core.exceptions.StorageEngineException;
import org.opencb.opencga.storage.core.metadata.ProjectMetadata;
import org.opencb.opencga.storage.core.metadata.ProjectMetadata.VariantAnnotationMetadata;
import org.opencb.opencga.storage.core.metadata.ProjectMetadata.VariantAnnotatorProgram;
import org.opencb.opencga.storage.core.variant.annotation.annotators.VariantAnnotator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -61,90 +63,132 @@ public abstract class VariantAnnotationManager {

public abstract void deleteAnnotationSnapshot(String name, ObjectMap options) throws StorageEngineException, VariantAnnotatorException;

protected final ProjectMetadata.VariantAnnotationMetadata checkCurrentAnnotation(VariantAnnotator annotator,
ProjectMetadata projectMetadata)
throws IOException {
ProjectMetadata.VariantAnnotatorProgram newAnnotator = annotator.getVariantAnnotatorProgram();
List<ObjectMap> newSourceVersion = annotator.getVariantAnnotatorSourceVersion();
protected final VariantAnnotationMetadata checkCurrentAnnotation(VariantAnnotator annotator, ProjectMetadata projectMetadata,
boolean overwrite)
throws VariantAnnotatorException {
VariantAnnotatorProgram newAnnotator;
List<ObjectMap> newSourceVersion;
try {
newAnnotator = annotator.getVariantAnnotatorProgram();
newSourceVersion = annotator.getVariantAnnotatorSourceVersion();
} catch (IOException e) {
throw new VariantAnnotatorException("Error reading current annotation metadata!", e);
}
if (newSourceVersion == null) {
newSourceVersion = Collections.emptyList();
}
if (newAnnotator == null) {
throw new IllegalArgumentException("Missing annotator information for VariantAnnotator: " + annotator.getClass());
}
if (newSourceVersion.isEmpty()) {
throw new IllegalArgumentException("Missing annotator source version for VariantAnnotator: " + annotator.getClass());
}
return checkCurrentAnnotation(projectMetadata, overwrite, newAnnotator, newSourceVersion);
}

ProjectMetadata.VariantAnnotationMetadata current = projectMetadata.getAnnotation().getCurrent();
protected final VariantAnnotationMetadata checkCurrentAnnotation(ProjectMetadata projectMetadata, boolean overwrite,
VariantAnnotatorProgram newAnnotator, List<ObjectMap> newSourceVersion)
throws VariantAnnotatorException {
VariantAnnotationMetadata current = projectMetadata.getAnnotation().getCurrent();
if (current == null) {
current = new ProjectMetadata.VariantAnnotationMetadata();
current = new VariantAnnotationMetadata();
projectMetadata.getAnnotation().setCurrent(current);
current.setId(-1);
current.setName(LATEST);
}

// Check using same annotator and same source version
ProjectMetadata.VariantAnnotatorProgram currentAnnotator = current.getAnnotator();
VariantAnnotatorProgram currentAnnotator = current.getAnnotator();
if (currentAnnotator != null && !currentAnnotator.equals(newAnnotator)) {
if (newAnnotator == null) {
throw new IllegalArgumentException("Missing annotator information for VariantAnnotator: " + annotator.getClass());
}
if (!currentAnnotator.getName().equals(newAnnotator.getName())
|| !currentAnnotator.getVersion().equals(newAnnotator.getVersion())) {
String msg = "Using a different annotator! "
+ "Existing annotation calculated with " + currentAnnotator.toString()
+ ", attempting to annotate with " + newAnnotator.toString();
logger.error(msg);
// throw new VariantAnnotatorException(msg);
if (overwrite) {
logger.info(msg);
} else {
throw new VariantAnnotatorException(msg);
}
} else if (!currentAnnotator.getCommit().equals(newAnnotator.getCommit())) {
logger.warn("Using a different commit for annotating variants. "
String msg = "Using a different commit for annotating variants. "
+ "Existing annotation calculated with " + currentAnnotator.toString()
+ ", attempting to annotate with " + newAnnotator.toString());
+ ", attempting to annotate with " + newAnnotator.toString();
if (overwrite) {
logger.info(msg);
} else {
logger.warn(msg);
}
}
}
current.setAnnotator(newAnnotator);

List<ObjectMap> currentSourceVersion = current.getSourceVersion();
if (CollectionUtils.isNotEmpty(currentSourceVersion) && !currentSourceVersion.equals(newSourceVersion)) {
if (newSourceVersion.isEmpty()) {
throw new IllegalArgumentException("Missing annotator source version for VariantAnnotator: " + annotator.getClass());
}
String msg = "Source version of the annotator has changed. "
+ "Existing annotation calculated with "
+ currentSourceVersion.stream().map(ObjectMap::toJson).collect(Collectors.joining(" , ", "[ ", " ]"))
+ ", attempting to annotate with "
+ newSourceVersion.stream().map(ObjectMap::toJson).collect(Collectors.joining(" , ", "[ ", " ]"));
logger.error(msg);
// throw new VariantAnnotatorException(msg);
if (overwrite) {
logger.info(msg);
} else {
throw new VariantAnnotatorException(msg);
}
}
current.setSourceVersion(newSourceVersion);

return current;
}

protected final ProjectMetadata.VariantAnnotationMetadata registerNewAnnotationSnapshot(String name, VariantAnnotator annotator,
protected final void updateCurrentAnnotation(VariantAnnotator annotator, ProjectMetadata projectMetadata,
boolean overwrite)
throws VariantAnnotatorException {
VariantAnnotatorProgram newAnnotator;
List<ObjectMap> newSourceVersion;
try {
newAnnotator = annotator.getVariantAnnotatorProgram();
newSourceVersion = annotator.getVariantAnnotatorSourceVersion();
} catch (IOException e) {
throw new VariantAnnotatorException("Error reading current annotation metadata!", e);
}
if (newSourceVersion == null) {
newSourceVersion = Collections.emptyList();
}
if (newAnnotator == null) {
throw new IllegalArgumentException("Missing annotator information for VariantAnnotator: " + annotator.getClass());
}
if (newSourceVersion.isEmpty()) {
throw new IllegalArgumentException("Missing annotator source version for VariantAnnotator: " + annotator.getClass());
}
checkCurrentAnnotation(projectMetadata, overwrite, newAnnotator, newSourceVersion);

projectMetadata.getAnnotation().getCurrent().setAnnotator(newAnnotator);
projectMetadata.getAnnotation().getCurrent().setSourceVersion(newSourceVersion);
}

protected final VariantAnnotationMetadata registerNewAnnotationSnapshot(String name, VariantAnnotator annotator,
ProjectMetadata projectMetadata)
throws VariantAnnotatorException {
ProjectMetadata.VariantAnnotationMetadata current = projectMetadata.getAnnotation().getCurrent();
VariantAnnotationMetadata current = projectMetadata.getAnnotation().getCurrent();
if (current == null) {
// Should never enter here
try {
current = checkCurrentAnnotation(annotator, projectMetadata);
} catch (IOException e) {
throw new VariantAnnotatorException("Missing current annotation metadata!", e);
}
current = checkCurrentAnnotation(annotator, projectMetadata, true);
}

boolean nameDuplicated = projectMetadata.getAnnotation().getSaved()
.stream()
.map(ProjectMetadata.VariantAnnotationMetadata::getName)
.map(VariantAnnotationMetadata::getName)
.anyMatch(s -> s.equalsIgnoreCase(name));

if (nameDuplicated) {
throw new VariantAnnotatorException("Annotation snapshot name already exists!");
}
Integer maxId = projectMetadata.getAnnotation().getSaved()
.stream()
.map(ProjectMetadata.VariantAnnotationMetadata::getId)
.map(VariantAnnotationMetadata::getId)
.max(Integer::compareTo)
.orElse(0);

ProjectMetadata.VariantAnnotationMetadata newSnapshot = new ProjectMetadata.VariantAnnotationMetadata(
VariantAnnotationMetadata newSnapshot = new VariantAnnotationMetadata(
maxId + 1,
name,
Date.from(Instant.now()),
Expand All @@ -155,10 +199,10 @@ protected final ProjectMetadata.VariantAnnotationMetadata registerNewAnnotationS
return newSnapshot;
}

protected final ProjectMetadata.VariantAnnotationMetadata removeAnnotationSnapshot(String name, ProjectMetadata projectMetadata)
protected final VariantAnnotationMetadata removeAnnotationSnapshot(String name, ProjectMetadata projectMetadata)
throws VariantAnnotatorException {
Iterator<ProjectMetadata.VariantAnnotationMetadata> iterator = projectMetadata.getAnnotation().getSaved().iterator();
ProjectMetadata.VariantAnnotationMetadata annotation = null;
Iterator<VariantAnnotationMetadata> iterator = projectMetadata.getAnnotation().getSaved().iterator();
VariantAnnotationMetadata annotation = null;
boolean found = false;
while (iterator.hasNext()) {
annotation = iterator.next();
Expand Down
77 changes: 69 additions & 8 deletions ...tation/VariantAnnotationSnapshotTest.java → ...otation/VariantAnnotationManagerTest.java
View file
Expand Up @@ -22,17 +22,71 @@
import java.util.List;
import java.util.stream.Collectors;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.opencb.opencga.storage.core.variant.annotation.VariantAnnotationManager.ANNOTATOR;
import static org.opencb.opencga.storage.core.variant.annotation.VariantAnnotationManager.VARIANT_ANNOTATOR_CLASSNAME;
import static org.junit.Assert.*;
import static org.opencb.opencga.storage.core.variant.annotation.VariantAnnotationManager.*;

/**
* Created on 24/04/18.
*
* @author Jacobo Coll &lt;jacobo167@gmail.com&gt;
*/
public abstract class VariantAnnotationSnapshotTest extends VariantStorageBaseTest {
public abstract class VariantAnnotationManagerTest extends VariantStorageBaseTest {

@Test
public void testChangeAnnotator() throws Exception {
VariantStorageEngine variantStorageEngine = getVariantStorageEngine();
runDefaultETL(smallInputUri, variantStorageEngine, newStudyConfiguration(),
new ObjectMap(VariantStorageEngine.Options.ANNOTATE.key(), false));

variantStorageEngine.getOptions()
.append(VARIANT_ANNOTATOR_CLASSNAME, TestAnnotator.class.getName())
.append(ANNOTATOR, VariantAnnotatorFactory.AnnotationSource.OTHER);

// First annotation. Should run ok.
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v1"));
assertEquals("v1", variantStorageEngine.getStudyConfigurationManager().getProjectMetadata().first().getAnnotation().getCurrent().getAnnotator().getVersion());

// Second annotation. New annotator. Overwrite. Should run ok.
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v2").append(OVERWRITE_ANNOTATIONS, true));
assertEquals("v2", variantStorageEngine.getStudyConfigurationManager().getProjectMetadata().first().getAnnotation().getCurrent().getAnnotator().getVersion());

// Third annotation. New annotator. Do not overwrite. Should fail.
thrown.expect(VariantAnnotatorException.class);
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v3").append(OVERWRITE_ANNOTATIONS, false));
}

@Test
public void testChangeAnnotatorFail() throws Exception {
VariantStorageEngine variantStorageEngine = getVariantStorageEngine();
runDefaultETL(smallInputUri, variantStorageEngine, newStudyConfiguration(),
new ObjectMap(VariantStorageEngine.Options.ANNOTATE.key(), false));

variantStorageEngine.getOptions()
.append(VARIANT_ANNOTATOR_CLASSNAME, TestAnnotator.class.getName())
.append(ANNOTATOR, VariantAnnotatorFactory.AnnotationSource.OTHER);

// First annotation. Should run ok.
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v1"));

try {
// Second annotation. New annotator. Overwrite. Fail annotation
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v2")
.append(TestAnnotator.FAIL, true)
.append(OVERWRITE_ANNOTATIONS, true));
fail("Expected to fail!");
} catch (VariantAnnotatorException e) {
e.printStackTrace();
// Annotator information does not change
assertEquals("v1", variantStorageEngine.getStudyConfigurationManager().getProjectMetadata().first().getAnnotation().getCurrent().getAnnotator().getVersion());
}


// Second annotation bis. New annotator. Overwrite.
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v2")
.append(TestAnnotator.FAIL, false)
.append(OVERWRITE_ANNOTATIONS, true));
assertEquals("v2", variantStorageEngine.getStudyConfigurationManager().getProjectMetadata().first().getAnnotation().getCurrent().getAnnotator().getVersion());
}

@Test
public void testMultiAnnotations() throws Exception {
Expand All @@ -46,11 +100,11 @@ public void testMultiAnnotations() throws Exception {
.append(ANNOTATOR, VariantAnnotatorFactory.AnnotationSource.OTHER);

variantStorageEngine.createAnnotationSnapshot("v0", new ObjectMap());
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v1"));
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v1").append(OVERWRITE_ANNOTATIONS, true));
variantStorageEngine.createAnnotationSnapshot("v1", new ObjectMap());
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v2"));
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v2").append(OVERWRITE_ANNOTATIONS, true));
variantStorageEngine.createAnnotationSnapshot("v2", new ObjectMap());
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v3"));
variantStorageEngine.annotate(new Query(), new ObjectMap(TestAnnotator.ANNOT_KEY, "v3").append(OVERWRITE_ANNOTATIONS, true));

assertEquals(0, variantStorageEngine.getAnnotation("v0", null, null).getResult().size());
checkAnnotationSnapshot(variantStorageEngine, "v1", "v1");
Expand Down Expand Up @@ -98,6 +152,7 @@ public void checkAnnotationSnapshot(VariantStorageEngine variantStorageEngine, S
int count = 0;
for (VariantAnnotation annotation: variantStorageEngine.getAnnotation(name, null, null).getResult()) {
assertEquals(expectedId, annotation.getId());
assertEquals("1", annotation.getAdditionalAttributes().get("opencga").getAttribute().get("release"));
count++;
}
assertEquals(count, variantStorageEngine.count(new Query()).first().intValue());
Expand All @@ -106,15 +161,21 @@ public void checkAnnotationSnapshot(VariantStorageEngine variantStorageEngine, S
public static class TestAnnotator extends VariantAnnotator {

public static final String ANNOT_KEY = "ANNOT_KEY";
public static final String FAIL = "ANNOT_FAIL";
private final boolean fail;
private String key;

public TestAnnotator(StorageConfiguration configuration, ProjectMetadata projectMetadata, ObjectMap options) throws VariantAnnotatorException {
super(configuration, projectMetadata, options);
key = options.getString(ANNOT_KEY);
fail = options.getBoolean(FAIL, false);
}

@Override
public List<VariantAnnotation> annotate(List<Variant> variants) throws VariantAnnotatorException {
if (fail) {
throw new VariantAnnotatorException("Fail because reasons");
}
return variants.stream().map(v -> {
VariantAnnotation a = new VariantAnnotation();
a.setChromosome(v.getChromosome());
Expand Down

0 comments on commit 68fd27e

Please sign in to comment.