Merge pull request #244 from aparton/release/92

Added format types to POST endpoints

lib/EnsEMBL/REST/Controller/VEP.pm

@@ -22,6 +22,7 @@ use Moose;
 use namespace::autoclean;
 use Data::Dumper;
 use Bio::EnsEMBL::VEP::Runner;
+use Bio::EnsEMBL::Utils::IO qw/slurp/;
 
 use Try::Tiny;
 
@@ -32,6 +33,52 @@ BEGIN {
   extends 'Catalyst::Controller::REST';
 }
 
+# This list describes user overridable variables for this endpoint. It protects other more fundamental variables
+has valid_user_params => ( 
+  is => 'ro', 
+  isa => 'HashRef', 
+  traits => ['Hash'], 
+  handles => { valid_user_param => 'exists' },
+  default => sub { return {
+    map {$_ => 1} (qw/
+    gencode_basic
+    refseq
+    merged
+    all_refseq
+
+    failed
+    variant_class
+    minimal
+
+    everything
+    appris
+    canonical
+    ccds
+    domains
+    hgvs
+    numbers
+    protein
+    tsl
+    uniprot
+    xref_refseq
+    phyloP
+    transcript_id
+    phastCons
+    distance
+    ambiguity
+
+    pick
+    pick_allele
+    per_gene
+    pick_allele_gene
+    flag_pick
+    flag_pick_allele
+    flag_pick_allele_gene
+    pick_order
+  /)}
+  }
+);
+
 with 'EnsEMBL::REST::Role::PostLimiter';
 
 # /vep/:species
@@ -83,7 +130,6 @@ sub get_region_GET {
 sub get_region_POST {
   my ( $self, $c ) = @_;
   my $post_data = $c->req->data;
-
   # $c->log->debug(Dumper $post_data);
   # $c->log->debug(Dumper $config->{'Controller::VEP'});
   # handle user config
@@ -256,6 +302,7 @@ sub get_id_POST {
   unless (exists $post_data->{ids}) {
     $c->go( 'ReturnError', 'custom', [ ' Cannot find "ids" key in your POST. Please check the format of your message against the documentation' ] );
   }
+$config->{format} = 'id';
   my @ids = @{$post_data->{ids}};
   $self->assert_post_size($c,\@ids);
   $self->_give_POST_to_VEP($c,\@ids,$config);
@@ -270,6 +317,7 @@ sub get_hgvs_POST {
   unless (exists $post_data->{hgvs_notations}) {
     $c->go( 'ReturnError', 'custom', [ ' Cannot find "hgvs_notations" key in your POST. Please check the format of your message against the documentation' ] );
   }
+  $config->{format} = 'hgvs';
   my @hgvs = @{$post_data->{hgvs_notations}};
   $self->assert_post_size($c,\@hgvs);
   $self->_give_POST_to_VEP($c,\@hgvs,$config);
@@ -278,44 +326,6 @@ sub get_hgvs_POST {
 sub _include_user_params {
   my ($self,$c,$user_config) = @_;
 
-  # This list stops users altering more crucial variables.
-  my %valid_keys = map {$_ => 1} (qw/
-    gencode_basic
-    refseq
-    merged
-    all_refseq
-
-    failed
-    variant_class
-    minimal
-
-    everything
-    appris
-    canonical
-    ccds
-    domains
-    hgvs
-    numbers
-    protein
-    tsl
-    uniprot
-    xref_refseq
-    phyloP
-    transcript_id
-    phastCons
-    distance
-    ambiguity
-
-    pick
-    pick_allele
-    per_gene
-    pick_allele_gene
-    flag_pick
-    flag_pick_allele
-    flag_pick_allele_gene
-    pick_order
-  /);
-  
   my %vep_params = %{ $c->config->{'Controller::VEP'} };
 
   # stash species
@@ -339,7 +349,7 @@ sub _include_user_params {
   map { $tmp_vep_params{$_} = $user_config->{$_}} keys %{$user_config};
 
   # only copy allowed keys to %vep_params as we don't want users to be able to meddle
-  $vep_params{$_} = $tmp_vep_params{$_} for grep { $valid_keys{$_} } keys %tmp_vep_params;
+  $vep_params{$_} = $tmp_vep_params{$_} for grep { $self->valid_user_param($_) } keys %tmp_vep_params;
   
   # we currently only have cache for human
   if ($c->stash->{species} ne 'homo_sapiens') {
@@ -381,11 +391,9 @@ sub _configure_plugins {
   my $plugin_config_file = $vep_config->{plugin_config};
   return [] unless $plugin_config_file && -e $plugin_config_file;
 
-  open IN, $plugin_config_file or return [];
-  my @content = <IN>;
-  close IN;
-  
-  my $VEP_PLUGIN_CONFIG = eval join('', @content);
+  my $content = [];
+  $content = slurp($plugin_config_file);
+  my $VEP_PLUGIN_CONFIG = eval $content;
   $c->log->warn("Could not eval VEP plugin config file: $@\n") if $@;
 
   # iterate over all defined plugins

t/vep.t

@@ -26,6 +26,8 @@ BEGIN {
 
 use Test::More;
 use Test::Differences;
+use Test::Deep;
+use Test::Exception;
 use Bio::EnsEMBL::Test::MultiTestDB;
 use Bio::EnsEMBL::Test::TestUtils;
 use Catalyst::Test ();
@@ -117,7 +119,7 @@ my $vep_output =
 
 # Test vep/region
 my $json = json_GET($vep_get,'GET a VEP region');
-eq_or_diff($json, $vep_output, 'Example vep region get message');
+cmp_bag($json, $vep_output, 'Example vep region get message');
 
 
 # test with non-toplevel sequence (should transform to toplevel)
@@ -149,6 +151,7 @@ my $vep_post = '/vep/homo_sapiens/region';
 my $vep_post_body = '{ "variants" : ["7 34381884 var1 C T . . .",
                                      "7 86442404 var2 T C . . ."]}';
 
+
 $vep_output =
 [{
      allele_string => 'C/T',
@@ -272,7 +275,17 @@ $vep_output =
 }];
 
 $json = json_POST($vep_post,$vep_post_body,'POST a selection of regions to the VEP');
-eq_or_diff($json,$vep_output, "VEP region POST");
+cmp_bag($json,$vep_output, "VEP region POST");
+
+#Ensure that providing an invalid ID is correctly handled by the ID endpoint
+$vep_post = '/vep/homo_sapiens/id';
+my $vep_post_body_invalid = '{ "ids" : ["invalid_id", "rs186950277", "rs17081232" ]}';
+lives_ok{$json = json_POST($vep_post,$vep_post_body_invalid,'POST a selection of IDs to VEP')} 'ensuring invalid ID is handled';
+
+#Ensure that providing invalid hgvs is correctly handled by the HGVS endpoint
+$vep_post = '/vep/homo_sapiens/hgvs';
+$vep_post_body_invalid = '{ "hgvs_notations" : ["invalid_hgvs", "ENST00000314040:c.311G>T"] }';
+lives_ok{$json = json_POST($vep_post,$vep_post_body_invalid,'POST a selection of HGVS variants to VEP')} 'ensuring invalid HGVS is handled';
 
 
 # test vep/id
@@ -321,7 +334,7 @@ $vep_output =
   }];
 
 $json = json_GET($vep_id_get,'GET consequences for Variation ID');
-eq_or_diff($json, $vep_output, 'VEP id GET');
+cmp_bag($json, $vep_output, 'VEP id GET');
 
 
 my $vep_id_post = '/vep/homo_sapiens/id';
@@ -406,7 +419,7 @@ $vep_output =
 
 
 $json = json_POST($vep_id_post,$vep_id_body,'VEP ID list POST');
-eq_or_diff($json, $vep_output, 'VEP id POST');
+cmp_bag($json, $vep_output, 'VEP id POST');
 
 
 # test vep/hgvs with a genomic coord
@@ -449,7 +462,7 @@ $vep_output = [{
 }];
 
 $json = json_GET($vep_hgvs_get,'GET consequences for genomic HGVS notation');
-eq_or_diff($json, $vep_output, 'VEP genomic HGVS GET');
+cmp_bag($json, $vep_output, 'VEP genomic HGVS GET');
 
 # test vep/hgvs with a transcript coord
 $vep_hgvs_get = '/vep/homo_sapiens/hgvs/ENST00000314040:c.311G>T?content-type=application/json';
@@ -509,7 +522,7 @@ $vep_output = [{
 }];
 
 $json = json_GET($vep_plugin_get,'GET consequences with test plugin');
-eq_or_diff($json, $vep_output, 'VEP plugin test');
+cmp_bag($json, $vep_output, 'VEP plugin test');
 
 # test using transcript_id
 my $vep_transcript_id_get = '/vep/homo_sapiens/region/7:86442404-86442404:1/C?content-type=application/json&transcript_id=ENST00000361669';
@@ -545,7 +558,7 @@ $vep_output = [
 ];                                               
 
 $json = json_GET($vep_transcript_id_get,'GET consequences with transcript_id');
-eq_or_diff($json, $vep_output, 'VEP transcript filter test');
+cmp_bag($json, $vep_output, 'VEP transcript filter test');
 
 # test using refseq cache
 my $vep_refseq_get = '/vep/homo_sapiens/region/7:34097707-34097707:1/C?content-type=application/json&refseq=1';