Skip to content
This repository has been archived by the owner on Sep 15, 2021. It is now read-only.

Commit

Permalink
Merge pull request #23 from deflaux/master
Browse files Browse the repository at this point in the history 
Provide a better error for client-side shard filtering.
  • Loading branch information
@calbach
calbach committed Mar 21, 2015
2 parents ee2f74a + 00c633e commit 517f581
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/google/cloud/genomics/utils/Contig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class Contig implements Serializable {

private static final long serialVersionUID = -1730387112193404207L;

public static final long DEFAULT_NUMBER_OF_BASES_PER_SHARD = 100000;
public static final long DEFAULT_NUMBER_OF_BASES_PER_SHARD = 1000000;

public enum SexChromosomeFilter { INCLUDE_XY, EXCLUDE_XY }

Expand Down
106 changes: 92 additions & 14 deletions src/main/java/com/google/cloud/genomics/utils/Paginator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
*/
package com.google.cloud.genomics.utils;

import java.io.IOException;
import java.io.Serializable;
import java.util.Collections;
import java.util.Iterator;
import java.util.Map;

import com.google.api.services.genomics.Genomics;
import com.google.api.services.genomics.GenomicsRequest;
import com.google.api.services.genomics.model.Annotation;
Expand Down Expand Up @@ -51,17 +57,16 @@
import com.google.api.services.genomics.model.Variant;
import com.google.api.services.genomics.model.VariantSet;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
import com.google.common.collect.AbstractSequentialIterator;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;

import java.io.IOException;
import java.io.Serializable;
import java.util.Collections;
import java.util.Iterator;
import com.google.common.collect.Maps;

/**
* An abstraction that understands the {@code pageToken} / {@code nextPageToken} protocol for paging
Expand Down Expand Up @@ -319,6 +324,13 @@ public static class Reads extends Paginator<
SearchReadsResponse,
Read> {

// TODO: When this is supported server-side, these additional fields are no longer required
private static ImmutableMap<String,String> REQUIRED_STRICT_SHARD_FIELDS =
ImmutableMap.<String, String>builder()
.putAll(REQUIRED_FIELDS)
.put("alignment", ".*\\p{Punct}alignment\\p{Punct}.*")
.put("position", ".*\\p{Punct}position\\p{Punct}.*")
.build();
private final ShardBoundary shardBoundary;
private Predicate<Read> shardPredicate = null;

Expand Down Expand Up @@ -369,6 +381,14 @@ public boolean apply(Read read) {
.or(request));
}

@Override
public GenomicsRequestInitializer<GenomicsRequest<?>> setFieldsInitializer(final String fields) {
if(shardBoundary == ShardBoundary.STRICT) {
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_STRICT_SHARD_FIELDS);
}
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_FIELDS);
}

@Override Genomics.Reads getApi(Genomics genomics) {
return genomics.reads();
}
Expand Down Expand Up @@ -396,6 +416,13 @@ public static class Annotations extends Paginator<
SearchAnnotationsResponse,
Annotation> {

// TODO: When this is supported server-side, these additional fields are no longer required
private static ImmutableMap<String,String> REQUIRED_STRICT_SHARD_FIELDS =
ImmutableMap.<String, String>builder()
.putAll(REQUIRED_FIELDS)
.put("position", ".*\\p{Punct}position\\p{Punct}.*")
.put("start", ".*\\p{Punct}start\\p{Punct}.*")
.build();
private final ShardBoundary shardBoundary;
private Predicate<Annotation> shardPredicate = null;

Expand Down Expand Up @@ -445,6 +472,14 @@ public boolean apply(Annotation anno) {
.or(request));
}

@Override
public GenomicsRequestInitializer<GenomicsRequest<?>> setFieldsInitializer(final String fields) {
if(shardBoundary == ShardBoundary.STRICT) {
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_STRICT_SHARD_FIELDS);
}
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_FIELDS);
}

@Override Genomics.Annotations getApi(Genomics genomics) {
return genomics.annotations();
}
Expand Down Expand Up @@ -784,6 +819,12 @@ public static class Variants extends Paginator<
SearchVariantsResponse,
Variant> {

// TODO: When this is supported server-side, these additional fields are no longer required
private static ImmutableMap<String,String> REQUIRED_STRICT_SHARD_FIELDS =
ImmutableMap.<String, String>builder()
.putAll(REQUIRED_FIELDS)
.put("start", ".*\\p{Punct}start\\p{Punct}.*")
.build();
private final ShardBoundary shardBoundary;
private Predicate<Variant> shardPredicate = null;

Expand Down Expand Up @@ -834,6 +875,14 @@ public boolean apply(Variant variant) {
.or(request));
}

@Override
public GenomicsRequestInitializer<GenomicsRequest<?>> setFieldsInitializer(final String fields) {
if(shardBoundary == ShardBoundary.STRICT) {
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_STRICT_SHARD_FIELDS);
}
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_FIELDS);
}

@Override Genomics.Variants getApi(Genomics genomics) {
return genomics.variants();
}
Expand Down Expand Up @@ -905,17 +954,46 @@ private Variantsets(Genomics genomics) {
@Override public void initialize(GenomicsRequest<?> search) {}
};

public static GenomicsRequestInitializer<GenomicsRequest<?>> setFieldsInitializer(
final String fields) {
return new GenomicsRequestInitializer<GenomicsRequest<?>>() {
@Override public void initialize(GenomicsRequest<?> search) {
if (null != fields) {
search.setFields(fields);
}
}
};
protected static final ImmutableMap<String, String> REQUIRED_FIELDS = ImmutableMap
.<String, String>builder()
.put("nextPageToken", "(.+\\p{Punct})?nextPageToken(\\p{Punct}.+)?").
build();

public GenomicsRequestInitializer<GenomicsRequest<?>> setFieldsInitializer(final String fields) {
return new GenomicsSearchFieldRequestInitializer(fields, REQUIRED_FIELDS);
}

public static class GenomicsSearchFieldRequestInitializer implements
GenomicsRequestInitializer<GenomicsRequest<?>> {
private final String fields;
private final ImmutableMap<String, String> requiredFields;

public GenomicsSearchFieldRequestInitializer(String fields,
ImmutableMap<String, String> requiredFields) {
this.fields = fields;
this.requiredFields = requiredFields;
}

@Override
public void initialize(GenomicsRequest<?> search) {
if (null != fields) {
Map<String, String> missingFields =
Maps.filterValues(requiredFields, new Predicate<String>() {
@Override
public boolean apply(String input) {
return !fields.matches(input);
}
});
Preconditions
.checkArgument(
missingFields.isEmpty(),
"Required field(s) missing: '%s' Add this field to the list of fields to be returned in the partial response.",
Joiner.on(",").join(missingFields.keySet()));
search.setFields(fields);
}
}
}

private final Genomics genomics;

public Paginator(Genomics genomics) {
Expand Down
74 changes: 69 additions & 5 deletions src/test/java/com/google/cloud/genomics/utils/PaginatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@

import org.hamcrest.CoreMatchers;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mock;
Expand All @@ -55,6 +58,9 @@ public class PaginatorTest {
@Mock Genomics.Reads reads;
@Mock Genomics.Reads.Search readsSearch;

@Rule
public ExpectedException thrown = ExpectedException.none();

@Before
public void initMocks() {
MockitoAnnotations.initMocks(this);
Expand Down Expand Up @@ -120,16 +126,26 @@ public void testFields() throws Exception {
Paginator.ReadGroupSets paginator = Paginator.ReadGroupSets.create(genomics);
List<String> ids = Lists.newArrayList();
for (ReadGroupSet set : paginator.search(
new SearchReadGroupSetsRequest().setName("HG"), "readGroupSets(id,name)")) {
new SearchReadGroupSetsRequest().setName("HG"), "nextPageToken,readGroupSets(id,name)")) {
ids.add(set.getId());
}

assertEquals(Lists.newArrayList("r1"), ids);

// Make sure the fields parameter actually gets passed along
Mockito.verify(readGroupSetSearch, Mockito.atLeastOnce()).setFields("readGroupSets(id,name)");
Mockito.verify(readGroupSetSearch, Mockito.atLeastOnce()).setFields("nextPageToken,readGroupSets(id,name)");
}

@Test
public void testFieldsMissingNextPageToken() throws Exception {
Mockito.when(readGroupSets.search(new SearchReadGroupSetsRequest().setName("HG")))
.thenReturn(readGroupSetSearch);

Paginator.ReadGroupSets paginator = Paginator.ReadGroupSets.create(genomics);
thrown.expect(IllegalArgumentException.class);
paginator.search(new SearchReadGroupSetsRequest().setName("HG"), "readGroupSets(id,name)").iterator().next();
}

@Test
public void testVariantPagination() throws Exception {

Expand Down Expand Up @@ -157,15 +173,33 @@ public void testVariantPagination() throws Exception {
assertThat(filteredVariants, CoreMatchers.hasItems(atStartWithinExtent,
atStartOverlapExtent, beyondStartWithinExtent, beyondOverlapExtent));

// Ensure searches with fields verify the preconditions for strict shards.
final String nullFields = null;
assertNotNull(filteredPaginator.search(request, nullFields).iterator().next());
assertNotNull(filteredPaginator.search(request, "nextPageToken,variants(start,id,calls(genotype,callSetName))").iterator().next());
assertNotNull(filteredPaginator.search(request, "id,nextPageToken,variants(start,id,calls(genotype,callSetName))").iterator().next());
assertNotNull(filteredPaginator.search(request, "variants(start,id,calls(genotype,callSetName)),nextPageToken").iterator().next());
try {
filteredPaginator.search(request, "nextPageToken,variants(id,calls(genotype,callSetName))").iterator().next();
fail("should have thrown an IllegalArgumentxception");
} catch (IllegalArgumentException e) {}

Paginator.Variants overlappingPaginator = Paginator.Variants.create(genomics, ShardBoundary.OVERLAPS);
List<Variant> overlappingVariants = Lists.newArrayList();
for (Variant variant : overlappingPaginator.search(request)) {
overlappingVariants.add(variant);
}
assertEquals(6, overlappingVariants.size());
assertThat(overlappingVariants, CoreMatchers.hasItems(input));

// Ensure searches with fields verify the preconditions for overlapping shards.
assertNotNull(overlappingPaginator.search(request, nullFields).iterator().next());
assertNotNull(overlappingPaginator.search(request, "nextPageToken,variants(start,id,calls(genotype,callSetName))").iterator().next());
assertNotNull(overlappingPaginator.search(request, "nextPageToken,variants(id,calls(genotype,callSetName))").iterator().next());
assertNotNull(overlappingPaginator.search(request, "id,nextPageToken,variants(start,id,calls(genotype,callSetName))").iterator().next());
assertNotNull(overlappingPaginator.search(request, "variants(id,calls(genotype,callSetName)),nextPageToken").iterator().next());
}

@Test
public void testVariantPaginationEmptyShard() throws Exception {

Expand All @@ -178,8 +212,8 @@ public void testVariantPaginationEmptyShard() throws Exception {
Paginator.Variants filteredPaginator = Paginator.Variants.create(genomics, ShardBoundary.STRICT);
assertNotNull(filteredPaginator.search(request));
}

Read readHelper(int start, int end) {
static Read readHelper(int start, int end) {
Position position = new Position().setPosition((long) start);
LinearAlignment alignment = new LinearAlignment().setPosition(position);
return new Read().setAlignment(alignment).setFragmentLength(end-start);
Expand Down Expand Up @@ -221,4 +255,34 @@ public void testReadPagination() throws Exception {
assertThat(overlappingReads, CoreMatchers.hasItems(input));
}

@Test
public void testReadPaginationStrictShardPrecondition() throws Exception {
SearchReadsRequest request = new SearchReadsRequest().setStart(1000L).setEnd(2000L);
Mockito.when(reads.search(request)).thenReturn(readsSearch);

Paginator.Reads filteredPaginator = Paginator.Reads.create(genomics, ShardBoundary.STRICT);
thrown.expect(IllegalArgumentException.class);
filteredPaginator.search(request, "nextPageToken,reads(id,alignment(cigar))").iterator().next();
}

@Test
public void testStrictReadPaginationNextPageTokenPrecondition() throws Exception {
SearchReadsRequest request = new SearchReadsRequest().setStart(1000L).setEnd(2000L);
Mockito.when(reads.search(request)).thenReturn(readsSearch);

Paginator.Reads filteredPaginator = Paginator.Reads.create(genomics, ShardBoundary.STRICT);
thrown.expect(IllegalArgumentException.class);
filteredPaginator.search(request, "reads(id,alignment(cigar,position))").iterator().next();
}

@Test
public void testOverlappingReadPaginationNextPageTokenPrecondition() throws Exception {
SearchReadsRequest request = new SearchReadsRequest().setStart(1000L).setEnd(2000L);
Mockito.when(reads.search(request)).thenReturn(readsSearch);

Paginator.Reads overlappingPaginator = Paginator.Reads.create(genomics, ShardBoundary.OVERLAPS);
thrown.expect(IllegalArgumentException.class);
overlappingPaginator.search(request, "reads(id,alignment(cigar,position))").iterator().next();
}

}

0 comments on commit 517f581

Please sign in to comment.